Date: Mon, 13 Jan 2025 02:46:24 +0100
Subject: [PATCH 015/208] admin changes - role assignments on user and partner
---
src/hct_mis_api/apps/account/admin/forms.py | 3 -
src/hct_mis_api/apps/account/admin/partner.py | 160 +++++++++---------
.../apps/account/admin/user_role.py | 27 ++-
src/hct_mis_api/apps/account/celery_tasks.py | 3 +-
.../admin/account/parent/permissions.html | 28 ++-
5 files changed, 126 insertions(+), 95 deletions(-)
diff --git a/src/hct_mis_api/apps/account/admin/forms.py b/src/hct_mis_api/apps/account/admin/forms.py
index bbdbbff6e2..d0bc943f96 100644
--- a/src/hct_mis_api/apps/account/admin/forms.py
+++ b/src/hct_mis_api/apps/account/admin/forms.py
@@ -56,9 +56,6 @@ class RoleAssignmentInlineFormSet(forms.BaseInlineFormSet):
def add_fields(self, form: "forms.Form", index: Optional[int]) -> None:
super().add_fields(form, index)
- form.fields["business_area"].choices = [
- (str(x.id), str(x)) for x in BusinessArea.objects.filter(is_split=False)
- ]
form.fields["role"].required = True
def clean(self) -> None:
diff --git a/src/hct_mis_api/apps/account/admin/partner.py b/src/hct_mis_api/apps/account/admin/partner.py
index bd15f5c2d7..da543c4d4f 100644
--- a/src/hct_mis_api/apps/account/admin/partner.py
+++ b/src/hct_mis_api/apps/account/admin/partner.py
@@ -12,7 +12,8 @@
from admin_extra_buttons.decorators import button
from hct_mis_api.apps.account import models as account_models
-from hct_mis_api.apps.account.models import IncompatibleRoles, Role
+from hct_mis_api.apps.account.admin.user_role import RoleAssignmentInline
+from hct_mis_api.apps.account.models import IncompatibleRoles, Role, RoleAssignment
from hct_mis_api.apps.core.models import BusinessArea, BusinessAreaPartnerThrough
from hct_mis_api.apps.geo.models import Area
from hct_mis_api.apps.program.models import Program
@@ -27,16 +28,19 @@ def can_add_business_area_to_partner(request: Any, *args: Any, **kwargs: Any) ->
return request.user.can_add_business_area_to_partner()
-def business_area_role_form_custom_query(queryset: "QuerySet") -> Any:
- class BusinessAreaRoleForm(forms.Form):
- business_area = forms.ModelChoiceField(queryset=BusinessArea.objects.all(), required=True)
- roles = forms.ModelMultipleChoiceField(queryset=Role.objects.all(), required=True)
-
- def __init__(self, *args: Any, **kwargs: Any) -> None:
- super().__init__(*args, **kwargs)
- self.fields["business_area"].queryset = queryset
-
- return BusinessAreaRoleForm
+# TODO: perm - not needed but replace with LimitAreasForPartner
+# def business_area_role_form_custom_query(queryset: "QuerySet") -> Any:
+# class BusinessAreaRoleForm(forms.Form):
+# business_area = forms.ModelChoiceField(queryset=BusinessArea.objects.all(), required=True)
+# program = forms.ModelChoiceField(queryset=Program.objects.all(), required=False)
+# role = forms.ModelChoiceField(queryset=Role.objects.filter(is_available_for_partner=True).all(), required=True)
+# expiry_date = forms.DateField(required=False)
+#
+# def __init__(self, *args: Any, **kwargs: Any) -> None:
+# super().__init__(*args, **kwargs)
+# self.fields["business_area"].queryset = queryset
+#
+# return BusinessAreaRoleForm
class ProgramAreaForm(forms.Form):
@@ -56,6 +60,7 @@ class PartnerAdmin(HopeModelAdminMixin, admin.ModelAdmin):
"is_un",
)
filter_horizontal = ("allowed_business_areas",)
+ inlines = (RoleAssignmentInline,)
def sub_partners(self, obj: Any) -> Optional[str]:
return self.links_to_objects(obj.get_children()) if obj else None
@@ -94,75 +99,64 @@ def get_form(
form.base_fields["parent"].queryset = queryset
return form
- @button(enabled=lambda obj: obj.original.is_editable)
- def permissions(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
- context = self.get_common_context(request, pk, title="Partner permissions")
- partner: account_models.Partner = context["original"]
- user_can_add_ba_to_partner = request.user.can_add_business_area_to_partner()
- permissions_list = partner.business_area_partner_through.all()
- context["can_add_business_area_to_partner"] = user_can_add_ba_to_partner
-
- BusinessAreaRoleFormSet = formset_factory(
- business_area_role_form_custom_query(partner.allowed_business_areas.all()),
- extra=0,
- can_delete=True,
- )
- if request.method == "GET":
- business_area_role_data = []
- for permission in permissions_list:
- if permission.roles:
- business_area_role_data.append(
- {"business_area": permission.business_area_id, "roles": permission.roles.all()}
- )
- business_area_role_form_set = BusinessAreaRoleFormSet(
- initial=business_area_role_data, prefix="business_area_role"
- )
- else:
- business_area_role_form_set = BusinessAreaRoleFormSet(request.POST or None, prefix="business_area_role")
- incompatible_roles = defaultdict(list)
- ba_partner_through_data = {}
- ba_partner_through_to_be_deleted = []
-
- business_area_role_form_set_is_valid = business_area_role_form_set.is_valid()
- if user_can_add_ba_to_partner and business_area_role_form_set_is_valid:
- for form in business_area_role_form_set.cleaned_data:
- if form and not form["DELETE"]:
- business_area_id = str(form["business_area"].id)
- role_ids = list(map(lambda role: str(role.id), form["roles"]))
-
- if incompatible_role := IncompatibleRoles.objects.filter(
- role_one__in=role_ids, role_two__in=role_ids
- ).first():
- incompatible_roles[form["business_area"]].append(str(incompatible_role))
- else:
- ba_partner, _ = BusinessAreaPartnerThrough.objects.get_or_create(
- partner=partner,
- business_area_id=business_area_id,
- )
- ba_partner_through_data[ba_partner] = form["roles"]
- elif form["DELETE"]:
- ba_partner_through_to_be_deleted.append(
- BusinessAreaPartnerThrough.objects.filter(
- partner=partner, business_area=form["business_area"]
- )
- .first()
- .id
- )
-
- if incompatible_roles:
- for business_area, roles in incompatible_roles.items():
- self.message_user(
- request, f"Roles in {business_area} are incompatible: {', '.join(roles)}", messages.ERROR
- )
-
- if business_area_role_form_set_is_valid and not incompatible_roles:
- if ba_partner_through_to_be_deleted:
- BusinessAreaPartnerThrough.objects.filter(pk__in=ba_partner_through_to_be_deleted).delete()
- for ba_partner_through, areas in ba_partner_through_data.items():
- ba_partner_through.roles.add(*areas)
-
- return HttpResponseRedirect(reverse("admin:account_partner_change", args=[pk]))
-
- context["business_area_role_formset"] = business_area_role_form_set
-
- return TemplateResponse(request, "admin/account/parent/permissions.html", context)
+ # TODO: perm - not needed but replace with LimitAreasForPartner
+ # @button(enabled=lambda obj: obj.original.is_editable)
+ # def permissions(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
+ # context = self.get_common_context(request, pk, title="Partner permissions")
+ # partner: account_models.Partner = context["original"]
+ # user_can_add_ba_to_partner = request.user.can_add_business_area_to_partner()
+ # role_assignment_list = partner.role_assignments.all()
+ # context["can_add_business_area_to_partner"] = user_can_add_ba_to_partner
+ #
+ # BusinessAreaRoleFormSet = formset_factory(
+ # business_area_role_form_custom_query(partner.allowed_business_areas.all()),
+ # extra=0,
+ # can_delete=True,
+ # )
+ # if request.method == "GET":
+ # business_area_role_data = []
+ # for role_assignment in role_assignment_list:
+ # print("dassad")
+ # print(role_assignment.__dict__)
+ # business_area_role_data.append(
+ # {"business_area": role_assignment.business_area_id, "program": role_assignment.program.id if role_assignment.program else None, "role": role_assignment.role, "expiry_date": str(role_assignment.expiry_date)}
+ # )
+ # business_area_role_form_set = BusinessAreaRoleFormSet(
+ # initial=business_area_role_data, prefix="business_area_role"
+ # )
+ # else:
+ # business_area_role_form_set = BusinessAreaRoleFormSet(request.POST or None, prefix="business_area_role")
+ # role_assignment_data_be_deleted = []
+ #
+ # business_area_role_form_set_is_valid = business_area_role_form_set.is_valid()
+ # if user_can_add_ba_to_partner and business_area_role_form_set_is_valid:
+ # for form in business_area_role_form_set.cleaned_data:
+ # if form and not form["DELETE"]:
+ # business_area_id = str(form["business_area"].id)
+ # program_id = str(form["program"].id) if form["program"] else None
+ # role_id = str(form["role"].id)
+ #
+ # RoleAssignment.objects.get_or_create(
+ # partner=partner,
+ # business_area_id=business_area_id,
+ # program_id=program_id,
+ # role_id=role_id,
+ # )
+ # elif form["DELETE"]:
+ # role_assignment_data_be_deleted.append(
+ # RoleAssignment.objects.filter(
+ # partner=partner, business_area=form["business_area"], program=form["program"], role=form["role"]
+ # )
+ # .first()
+ # .id
+ # )
+ #
+ # if business_area_role_form_set_is_valid:
+ # if role_assignment_data_be_deleted:
+ # RoleAssignment.objects.filter(pk__in=role_assignment_data_be_deleted).delete()
+ #
+ # return HttpResponseRedirect(reverse("admin:account_partner_change", args=[pk]))
+ #
+ # context["business_area_role_formset"] = business_area_role_form_set
+ #
+ # return TemplateResponse(request, "admin/account/parent/permissions.html", context)
diff --git a/src/hct_mis_api/apps/account/admin/user_role.py b/src/hct_mis_api/apps/account/admin/user_role.py
index d4f409c6cf..fe95a03c40 100644
--- a/src/hct_mis_api/apps/account/admin/user_role.py
+++ b/src/hct_mis_api/apps/account/admin/user_role.py
@@ -14,7 +14,8 @@
RoleAssignmentAdminForm,
RoleAssignmentInlineFormSet,
)
-from hct_mis_api.apps.account.models import Role
+from hct_mis_api.apps.account.models import Partner, Role
+from hct_mis_api.apps.core.models import BusinessArea
from hct_mis_api.apps.utils.admin import HOPEModelAdminBase
logger = logging.getLogger(__name__)
@@ -22,14 +23,31 @@
class RoleAssignmentInline(admin.TabularInline):
model = account_models.RoleAssignment
- fields = ["business_area", "role", "expiry_date"]
+ fields = ["business_area", "program", "role", "expiry_date"]
extra = 0
formset = RoleAssignmentInlineFormSet
+ def formfield_for_foreignkey(self, db_field: Any, request=None, **kwargs: Any) -> Any:
+ if db_field.name == "business_area":
+ partner_id = request.resolver_match.kwargs.get("object_id")
+
+ if partner_id and partner_id.isdigit():
+ partner = Partner.objects.get(id=partner_id)
+ kwargs["queryset"] = BusinessArea.objects.filter(
+ id__in=partner.allowed_business_areas.all().values("id"),
+ is_split=False,
+ )
+ else:
+ kwargs["queryset"] = BusinessArea.objects.filter(is_split=False)
+ return super().formfield_for_foreignkey(db_field, request, **kwargs)
+
+ def has_change_permission(self, request: HttpRequest, obj: Optional[Any] = None) -> bool:
+ return request.user.can_add_business_area_to_partner()
+
@admin.register(account_models.RoleAssignment)
class RoleAssignmentAdmin(HOPEModelAdminBase):
- list_display = ("user", "role", "business_area")
+ list_display = ("user", "partner", "role", "business_area", "program")
form = RoleAssignmentAdminForm
autocomplete_fields = ("role",)
raw_id_fields = ("user", "business_area", "role")
@@ -40,6 +58,7 @@ class RoleAssignmentAdmin(HOPEModelAdminBase):
)
list_filter = (
("business_area", AutoCompleteFilter),
+ ("program", AutoCompleteFilter),
("role", AutoCompleteFilter),
("role__subsystem", AllValuesComboFilter),
)
@@ -50,7 +69,9 @@ def get_queryset(self, request: HttpRequest) -> QuerySet:
.get_queryset(request)
.select_related(
"business_area",
+ "program",
"user",
+ "partner",
"role",
)
)
diff --git a/src/hct_mis_api/apps/account/celery_tasks.py b/src/hct_mis_api/apps/account/celery_tasks.py
index dbf8c7935b..916af9b3ec 100644
--- a/src/hct_mis_api/apps/account/celery_tasks.py
+++ b/src/hct_mis_api/apps/account/celery_tasks.py
@@ -1,5 +1,6 @@
import datetime
import logging
+from typing import Any
from django.db.models import Q
from django.utils import timezone
@@ -16,7 +17,7 @@
@app.task(bind=True, default_retry_delay=60, max_retries=3)
@log_start_and_end
@sentry_tags
-def invalidate_permissions_cache_for_user_if_expired_role() -> bool:
+def invalidate_permissions_cache_for_user_if_expired_role(self: Any) -> bool:
# Invalidate permissions cache for users with roles that expired a day before
day_ago = timezone.now() - datetime.timedelta(days=1)
users = User.objects.filter(
diff --git a/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html b/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
index 4bdf639c61..5568faa848 100644
--- a/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
+++ b/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
@@ -21,7 +21,9 @@ Business Area Roles
Business area |
- Roles |
+ Program |
+ Role |
+ Expiry date |
Delete? |
@@ -39,9 +41,17 @@ Business Area Roles
{{ form.business_area.errors.as_ul }}
{{ form.business_area }}
+
+ {{ form.program.errors.as_ul }}
+ {{ form.program }}
+ |
- {{ form.roles.errors.as_ul }}
- {{ form.roles }}
+ {{ form.role.errors.as_ul }}
+ {{ form.role }}
+ |
+
+ {{ form.expiry_date.errors.as_ul }}
+ {{ form.expiry_date }}
|
{{ form.DELETE }}
@@ -53,9 +63,17 @@ Business Area Roles
{{ business_area_role_formset.empty_form.business_area.errors.as_ul }}
{{ business_area_role_formset.empty_form.business_area }}
|
+
+ {{ business_area_role_formset.empty_form.program.errors.as_ul }}
+ {{ business_area_role_formset.empty_form.program }}
+ |
- {{ business_area_role_formset.empty_form.roles.errors.as_ul }}
- {{ business_area_role_formset.empty_form.roles }}
+ {{ business_area_role_formset.empty_form.role.errors.as_ul }}
+ {{ business_area_role_formset.empty_form.role }}
+ |
+
+ {{ business_area_role_formset.empty_form.expiry_date.errors.as_ul }}
+ {{ business_area_role_formset.empty_form.expiry_date }}
|
{{ business_area_role_formset.empty_form.DELETE }}
From 7c8d0370c9c2a436a4f33b03ab1773b41e7a237a Mon Sep 17 00:00:00 2001
From: Paulina Kujawa
Date: Mon, 13 Jan 2025 19:12:16 +0100
Subject: [PATCH 016/208] additional validations for models, admin page for
partner's role assignments with validation against allowed BAs, tests
---
src/hct_mis_api/apps/account/admin/partner.py | 35 +++++-
src/hct_mis_api/apps/account/models.py | 31 +++++-
tests/unit/apps/account/test_models.py | 105 +++++++++++++++++-
3 files changed, 166 insertions(+), 5 deletions(-)
diff --git a/src/hct_mis_api/apps/account/admin/partner.py b/src/hct_mis_api/apps/account/admin/partner.py
index da543c4d4f..c4e0fe03a1 100644
--- a/src/hct_mis_api/apps/account/admin/partner.py
+++ b/src/hct_mis_api/apps/account/admin/partner.py
@@ -1,8 +1,9 @@
from collections import defaultdict
from typing import TYPE_CHECKING, Any, Optional, Sequence, Type, Union
-
+from django.utils.translation import gettext_lazy as _
from django import forms
from django.contrib import admin, messages
+from django.core.exceptions import ValidationError
from django.forms import CheckboxSelectMultiple, ModelForm, formset_factory
from django.http import HttpRequest, HttpResponseRedirect
from django.template.response import TemplateResponse
@@ -10,10 +11,9 @@
from django.utils.html import format_html
from admin_extra_buttons.decorators import button
-
from hct_mis_api.apps.account import models as account_models
from hct_mis_api.apps.account.admin.user_role import RoleAssignmentInline
-from hct_mis_api.apps.account.models import IncompatibleRoles, Role, RoleAssignment
+from hct_mis_api.apps.account.models import IncompatibleRoles, Role, RoleAssignment, Partner
from hct_mis_api.apps.core.models import BusinessArea, BusinessAreaPartnerThrough
from hct_mis_api.apps.geo.models import Area
from hct_mis_api.apps.program.models import Program
@@ -49,8 +49,37 @@ class ProgramAreaForm(forms.Form):
areas = TreeNodeMultipleChoiceField(queryset=Area.objects.all(), widget=CheckboxSelectMultiple(), required=False)
+class PartnerAdminForm(forms.ModelForm):
+ class Meta:
+ model = Partner
+ fields = ['name', 'allowed_business_areas', 'is_un', 'parent']
+
+ def clean_allowed_business_areas(self):
+ # Get the original allowed business areas for the partner
+ partner = self.instance
+ previous_allowed_ba = set(partner.allowed_business_areas.all())
+
+ # Get the allowed business from form submission
+ current_allowed_ba = set(self.cleaned_data.get('allowed_business_areas'))
+
+ # Identify which business areas were removed
+ removed_ba = previous_allowed_ba - current_allowed_ba
+
+ # Check if there are any removed business areas with existing role assignments
+ for ba in removed_ba:
+ if RoleAssignment.objects.filter(partner=partner, business_area=ba).exists():
+ # Add a form error instead of raising a ValidationError
+ self.add_error(
+ 'allowed_business_areas',
+ f"You cannot remove {ba} because there are existing role assignments for this business area."
+ )
+
+ return self.cleaned_data.get('allowed_business_areas', [])
+
+
@admin.register(account_models.Partner)
class PartnerAdmin(HopeModelAdminMixin, admin.ModelAdmin):
+ form = PartnerAdminForm
list_filter = ("is_un", "parent")
search_fields = ("name",)
readonly_fields = ("sub_partners",)
diff --git a/src/hct_mis_api/apps/account/models.py b/src/hct_mis_api/apps/account/models.py
index e3f267b0dc..1a0922c114 100644
--- a/src/hct_mis_api/apps/account/models.py
+++ b/src/hct_mis_api/apps/account/models.py
@@ -354,11 +354,35 @@ class RoleAssignment(NaturalKeyModel, TimeStampedUUIDModel):
class Meta:
constraints = [
- # either user or partner should be assigned; not both
+ # Either user or partner should be assigned; not both
models.CheckConstraint(
check=Q(user__isnull=False, partner__isnull=True) | Q(user__isnull=True, partner__isnull=False),
name="user_or_partner_not_both",
),
+ # Unique constraint for user + role + business_area + program when program is NOT NULL
+ models.UniqueConstraint(
+ fields=["user", "role", "business_area", "program"],
+ name="unique_user_role_business_area_program",
+ condition=Q(user__isnull=False),
+ ),
+ # Unique constraint for user + role + business_area when program is NULL
+ models.UniqueConstraint(
+ fields=["user", "role", "business_area"],
+ name="unique_user_role_business_area_no_program",
+ condition=Q(user__isnull=False, program__isnull=True),
+ ),
+ # Unique constraint for partner + role + business_area + program when program is NOT NULL
+ models.UniqueConstraint(
+ fields=["partner", "role", "business_area", "program"],
+ name="unique_partner_role_business_area_program",
+ condition=Q(partner__isnull=False),
+ ),
+ # Unique constraint for partner + role + business_area when program is NULL
+ models.UniqueConstraint(
+ fields=["partner", "role", "business_area"],
+ name="unique_partner_role_business_area_no_program",
+ condition=Q(partner__isnull=False, program__isnull=True),
+ ),
]
def clean(self) -> None:
@@ -370,6 +394,11 @@ def clean(self) -> None:
# Ensure partner can only be assigned roles that have flag is_available_for_partner as True
if self.partner and self.role and not self.role.is_available_for_partner:
errors.append("Partner can only be assigned roles that are available for partners.")
+ # Validate that business_area is within the partner's allowed_business_areas
+ if self.partner:
+ if self.business_area not in self.partner.allowed_business_areas.all():
+ errors.append(f"{self.business_area} is not within the allowed business areas for {self.partner}.")
+
if errors:
raise ValidationError(errors)
diff --git a/tests/unit/apps/account/test_models.py b/tests/unit/apps/account/test_models.py
index 241a265a9f..42d0f36737 100644
--- a/tests/unit/apps/account/test_models.py
+++ b/tests/unit/apps/account/test_models.py
@@ -1,4 +1,5 @@
from django.core.exceptions import ValidationError
+from django.db import IntegrityError
from django.test import TransactionTestCase
from hct_mis_api.apps.account.fixtures import (
@@ -8,7 +9,7 @@
UserFactory,
)
from hct_mis_api.apps.account.models import RoleAssignment
-from hct_mis_api.apps.core.fixtures import create_afghanistan
+from hct_mis_api.apps.core.fixtures import create_afghanistan, create_ukraine
from hct_mis_api.apps.program.fixtures import ProgramFactory
from hct_mis_api.apps.program.models import Program
@@ -26,6 +27,7 @@ def setUp(self) -> None:
)
self.user = UserFactory(first_name="Test", last_name="User")
self.partner = PartnerFactory(name="Partner")
+ self.partner.allowed_business_areas.add(self.business_area)
self.program1 = ProgramFactory(
business_area=self.business_area,
name="Program 1",
@@ -104,3 +106,104 @@ def test_is_available_for_partner_flag(self) -> None:
business_area=self.business_area,
)
self.assertIsNotNone(role_assignment_user.id)
+
+ def test_partner_role_in_business_area_vs_allowed_business_areas(self) -> None:
+ # Possible to create RoleAssignment for a business area that is allowed for the partner
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=self.role,
+ business_area=self.business_area,
+ )
+
+ # Partner with a different business area should raise a validation error
+ not_allowed_ba = create_ukraine()
+ with self.assertRaises(ValidationError) as ve_context:
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=self.role,
+ business_area=not_allowed_ba,
+ )
+ self.assertIn(
+ f"{not_allowed_ba} is not within the allowed business areas for {self.partner}.",
+ str(ve_context.exception),
+ )
+
+ # Validation not relevant for user
+ RoleAssignment.objects.create(
+ user=self.user,
+ partner=None,
+ role=self.role,
+ business_area=not_allowed_ba,
+ )
+
+ def test_unique_user_role_business_area_program_constraint(self) -> None:
+ # Creating a second role assignment with the same user, role, business area, and program should raise an error
+ role_new = RoleFactory(name="Test Role Duplicate")
+ RoleAssignment.objects.create(
+ user=self.user,
+ partner=None,
+ role=role_new,
+ business_area=self.business_area,
+ program=self.program1,
+ )
+ with self.assertRaises(IntegrityError):
+ RoleAssignment.objects.create(
+ user=self.user,
+ partner=None,
+ role=role_new,
+ business_area=self.business_area,
+ program=self.program1,
+ )
+
+ RoleAssignment.objects.create(
+ user=self.user,
+ partner=None,
+ role=role_new,
+ business_area=self.business_area,
+ program=None,
+ )
+ with self.assertRaises(IntegrityError):
+ RoleAssignment.objects.create(
+ user=self.user,
+ partner=None,
+ role=role_new,
+ business_area=self.business_area,
+ program=None,
+ )
+
+ def test_unique_partner_role_business_area_program_constraint(self) -> None:
+ # Creating a second role assignment with the same partner, role, business area, and program should raise an error
+ role_new = RoleFactory(name="Test Role Duplicate")
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=role_new,
+ business_area=self.business_area,
+ program=self.program1,
+ )
+ with self.assertRaises(IntegrityError):
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=role_new,
+ business_area=self.business_area,
+ program=self.program1,
+ )
+
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=role_new,
+ business_area=self.business_area,
+ program=None,
+ )
+ with self.assertRaises(IntegrityError):
+ RoleAssignment.objects.create(
+ user=None,
+ partner=self.partner,
+ role=role_new,
+ business_area=self.business_area,
+ program=None,
+ )
From c523cdd84a8f9624084e2888910efb3e6de2f134 Mon Sep 17 00:00:00 2001
From: Paulina Kujawa
Date: Wed, 15 Jan 2025 11:53:28 +0100
Subject: [PATCH 017/208] allowed_partners in ba admin, additional validations
for area limits, role_assignments and partners, tests
---
src/hct_mis_api/apps/account/admin/partner.py | 129 +-----------------
.../apps/account/migrations/0005_migration.py | 21 +++
.../apps/account/migrations/0006_migration.py | 21 +++
src/hct_mis_api/apps/account/models.py | 32 +++--
src/hct_mis_api/apps/account/signals.py | 6 +-
.../admin/account/parent/permissions.html | 127 -----------------
src/hct_mis_api/apps/core/admin.py | 61 ++++++++-
.../core/admin/allowed_partners.html | 19 +++
tests/unit/apps/account/test_models.py | 94 ++++++++++++-
9 files changed, 242 insertions(+), 268 deletions(-)
create mode 100644 src/hct_mis_api/apps/account/migrations/0005_migration.py
create mode 100644 src/hct_mis_api/apps/account/migrations/0006_migration.py
delete mode 100644 src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
create mode 100644 src/hct_mis_api/apps/core/templates/core/admin/allowed_partners.html
diff --git a/src/hct_mis_api/apps/account/admin/partner.py b/src/hct_mis_api/apps/account/admin/partner.py
index c4e0fe03a1..2caaed9395 100644
--- a/src/hct_mis_api/apps/account/admin/partner.py
+++ b/src/hct_mis_api/apps/account/admin/partner.py
@@ -1,85 +1,32 @@
-from collections import defaultdict
-from typing import TYPE_CHECKING, Any, Optional, Sequence, Type, Union
-from django.utils.translation import gettext_lazy as _
+from typing import Any, Optional, Sequence, Type
from django import forms
-from django.contrib import admin, messages
-from django.core.exceptions import ValidationError
-from django.forms import CheckboxSelectMultiple, ModelForm, formset_factory
-from django.http import HttpRequest, HttpResponseRedirect
-from django.template.response import TemplateResponse
+from django.contrib import admin
+from django.forms import CheckboxSelectMultiple, ModelForm
+from django.http import HttpRequest
from django.urls import reverse
from django.utils.html import format_html
-from admin_extra_buttons.decorators import button
from hct_mis_api.apps.account import models as account_models
from hct_mis_api.apps.account.admin.user_role import RoleAssignmentInline
-from hct_mis_api.apps.account.models import IncompatibleRoles, Role, RoleAssignment, Partner
-from hct_mis_api.apps.core.models import BusinessArea, BusinessAreaPartnerThrough
+from hct_mis_api.apps.core.models import BusinessArea
from hct_mis_api.apps.geo.models import Area
from hct_mis_api.apps.program.models import Program
from hct_mis_api.apps.utils.admin import HopeModelAdminMixin
from mptt.forms import TreeNodeMultipleChoiceField
-if TYPE_CHECKING:
- from django.db.models.query import QuerySet
-
def can_add_business_area_to_partner(request: Any, *args: Any, **kwargs: Any) -> bool:
return request.user.can_add_business_area_to_partner()
-# TODO: perm - not needed but replace with LimitAreasForPartner
-# def business_area_role_form_custom_query(queryset: "QuerySet") -> Any:
-# class BusinessAreaRoleForm(forms.Form):
-# business_area = forms.ModelChoiceField(queryset=BusinessArea.objects.all(), required=True)
-# program = forms.ModelChoiceField(queryset=Program.objects.all(), required=False)
-# role = forms.ModelChoiceField(queryset=Role.objects.filter(is_available_for_partner=True).all(), required=True)
-# expiry_date = forms.DateField(required=False)
-#
-# def __init__(self, *args: Any, **kwargs: Any) -> None:
-# super().__init__(*args, **kwargs)
-# self.fields["business_area"].queryset = queryset
-#
-# return BusinessAreaRoleForm
-
-
class ProgramAreaForm(forms.Form):
business_area = forms.ModelChoiceField(queryset=BusinessArea.objects.all(), required=True)
program = forms.ModelChoiceField(queryset=Program.objects.all(), required=True)
areas = TreeNodeMultipleChoiceField(queryset=Area.objects.all(), widget=CheckboxSelectMultiple(), required=False)
-class PartnerAdminForm(forms.ModelForm):
- class Meta:
- model = Partner
- fields = ['name', 'allowed_business_areas', 'is_un', 'parent']
-
- def clean_allowed_business_areas(self):
- # Get the original allowed business areas for the partner
- partner = self.instance
- previous_allowed_ba = set(partner.allowed_business_areas.all())
-
- # Get the allowed business from form submission
- current_allowed_ba = set(self.cleaned_data.get('allowed_business_areas'))
-
- # Identify which business areas were removed
- removed_ba = previous_allowed_ba - current_allowed_ba
-
- # Check if there are any removed business areas with existing role assignments
- for ba in removed_ba:
- if RoleAssignment.objects.filter(partner=partner, business_area=ba).exists():
- # Add a form error instead of raising a ValidationError
- self.add_error(
- 'allowed_business_areas',
- f"You cannot remove {ba} because there are existing role assignments for this business area."
- )
-
- return self.cleaned_data.get('allowed_business_areas', [])
-
-
@admin.register(account_models.Partner)
class PartnerAdmin(HopeModelAdminMixin, admin.ModelAdmin):
- form = PartnerAdminForm
list_filter = ("is_un", "parent")
search_fields = ("name",)
readonly_fields = ("sub_partners",)
@@ -88,7 +35,7 @@ class PartnerAdmin(HopeModelAdminMixin, admin.ModelAdmin):
"sub_partners",
"is_un",
)
- filter_horizontal = ("allowed_business_areas",)
+ exclude = ("allowed_business_areas",)
inlines = (RoleAssignmentInline,)
def sub_partners(self, obj: Any) -> Optional[str]:
@@ -109,8 +56,6 @@ def get_readonly_fields(self, request: HttpRequest, obj: Optional[account_models
additional_fields = []
if obj and obj.is_unicef:
additional_fields.append("name")
- if not request.user.has_perm("account.can_change_allowed_business_areas"):
- additional_fields.append("allowed_business_areas")
return list(super().get_readonly_fields(request, obj)) + additional_fields
def get_form(
@@ -127,65 +72,3 @@ def get_form(
form.base_fields["parent"].queryset = queryset
return form
-
- # TODO: perm - not needed but replace with LimitAreasForPartner
- # @button(enabled=lambda obj: obj.original.is_editable)
- # def permissions(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
- # context = self.get_common_context(request, pk, title="Partner permissions")
- # partner: account_models.Partner = context["original"]
- # user_can_add_ba_to_partner = request.user.can_add_business_area_to_partner()
- # role_assignment_list = partner.role_assignments.all()
- # context["can_add_business_area_to_partner"] = user_can_add_ba_to_partner
- #
- # BusinessAreaRoleFormSet = formset_factory(
- # business_area_role_form_custom_query(partner.allowed_business_areas.all()),
- # extra=0,
- # can_delete=True,
- # )
- # if request.method == "GET":
- # business_area_role_data = []
- # for role_assignment in role_assignment_list:
- # print("dassad")
- # print(role_assignment.__dict__)
- # business_area_role_data.append(
- # {"business_area": role_assignment.business_area_id, "program": role_assignment.program.id if role_assignment.program else None, "role": role_assignment.role, "expiry_date": str(role_assignment.expiry_date)}
- # )
- # business_area_role_form_set = BusinessAreaRoleFormSet(
- # initial=business_area_role_data, prefix="business_area_role"
- # )
- # else:
- # business_area_role_form_set = BusinessAreaRoleFormSet(request.POST or None, prefix="business_area_role")
- # role_assignment_data_be_deleted = []
- #
- # business_area_role_form_set_is_valid = business_area_role_form_set.is_valid()
- # if user_can_add_ba_to_partner and business_area_role_form_set_is_valid:
- # for form in business_area_role_form_set.cleaned_data:
- # if form and not form["DELETE"]:
- # business_area_id = str(form["business_area"].id)
- # program_id = str(form["program"].id) if form["program"] else None
- # role_id = str(form["role"].id)
- #
- # RoleAssignment.objects.get_or_create(
- # partner=partner,
- # business_area_id=business_area_id,
- # program_id=program_id,
- # role_id=role_id,
- # )
- # elif form["DELETE"]:
- # role_assignment_data_be_deleted.append(
- # RoleAssignment.objects.filter(
- # partner=partner, business_area=form["business_area"], program=form["program"], role=form["role"]
- # )
- # .first()
- # .id
- # )
- #
- # if business_area_role_form_set_is_valid:
- # if role_assignment_data_be_deleted:
- # RoleAssignment.objects.filter(pk__in=role_assignment_data_be_deleted).delete()
- #
- # return HttpResponseRedirect(reverse("admin:account_partner_change", args=[pk]))
- #
- # context["business_area_role_formset"] = business_area_role_form_set
- #
- # return TemplateResponse(request, "admin/account/parent/permissions.html", context)
diff --git a/src/hct_mis_api/apps/account/migrations/0005_migration.py b/src/hct_mis_api/apps/account/migrations/0005_migration.py
new file mode 100644
index 0000000000..e0fba356a6
--- /dev/null
+++ b/src/hct_mis_api/apps/account/migrations/0005_migration.py
@@ -0,0 +1,21 @@
+# Generated by Django 3.2.25 on 2025-01-13 13:31
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('account', '0004_migration'),
+ ]
+
+ operations = [
+ migrations.AddConstraint(
+ model_name='roleassignment',
+ constraint=models.UniqueConstraint(condition=models.Q(('user__isnull', False)), fields=('user', 'role', 'business_area', 'program'), name='unique_user_role_business_area_program'),
+ ),
+ migrations.AddConstraint(
+ model_name='roleassignment',
+ constraint=models.UniqueConstraint(condition=models.Q(('partner__isnull', False)), fields=('partner', 'role', 'business_area', 'program'), name='unique_partner_role_business_area_program'),
+ ),
+ ]
diff --git a/src/hct_mis_api/apps/account/migrations/0006_migration.py b/src/hct_mis_api/apps/account/migrations/0006_migration.py
new file mode 100644
index 0000000000..fc60274b56
--- /dev/null
+++ b/src/hct_mis_api/apps/account/migrations/0006_migration.py
@@ -0,0 +1,21 @@
+# Generated by Django 3.2.25 on 2025-01-13 13:44
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('account', '0005_migration'),
+ ]
+
+ operations = [
+ migrations.AddConstraint(
+ model_name='roleassignment',
+ constraint=models.UniqueConstraint(condition=models.Q(('program__isnull', True), ('user__isnull', False)), fields=('user', 'role', 'business_area'), name='unique_user_role_business_area_no_program'),
+ ),
+ migrations.AddConstraint(
+ model_name='roleassignment',
+ constraint=models.UniqueConstraint(condition=models.Q(('partner__isnull', False), ('program__isnull', True)), fields=('partner', 'role', 'business_area'), name='unique_partner_role_business_area_no_program'),
+ ),
+ ]
diff --git a/src/hct_mis_api/apps/account/models.py b/src/hct_mis_api/apps/account/models.py
index 1a0922c114..fddaf8966b 100644
--- a/src/hct_mis_api/apps/account/models.py
+++ b/src/hct_mis_api/apps/account/models.py
@@ -4,6 +4,7 @@
from uuid import UUID
from django import forms
+from django.apps import apps
from django.conf import settings
from django.contrib.admin.widgets import FilteredSelectMultiple
from django.contrib.auth.models import AbstractUser, Group, Permission
@@ -59,19 +60,17 @@ class Partner(LimitBusinessAreaModelMixin, MPTTModel):
verbose_name=_("Parent"),
)
is_un = models.BooleanField(verbose_name="U.N.", default=False)
- """
- permissions structure
- {
- "business_area_id": {
- "roles": ["role_id_1", "role_id_2"],
- "programs": {"program_id":["admin_id"]}
- }
- }
- """
def __str__(self) -> str:
return f"{self.name} [Sub-Partner of {self.parent.name}]" if self.parent else self.name
+ def save(self, *args, **kwargs):
+ # Partner cannot be a parent if it has RoleAssignments
+ if self.parent:
+ if RoleAssignment.objects.filter(partner=self.parent).exists():
+ raise ValidationError(f"{self.parent} cannot become a parent as it has RoleAssignments.")
+ super().save(*args, **kwargs)
+
@property
def is_child(self) -> bool:
return self.parent is None
@@ -318,6 +317,7 @@ class Meta:
("restrict_help_desk", "Limit fields to be editable for help desk"),
("can_reindex_programs", "Can reindex programs"),
("can_add_business_area_to_partner", "Can add business area to partner"),
+ ("can_change_allowed_partners", "Can change allowed partners"),
)
@@ -394,10 +394,13 @@ def clean(self) -> None:
# Ensure partner can only be assigned roles that have flag is_available_for_partner as True
if self.partner and self.role and not self.role.is_available_for_partner:
errors.append("Partner can only be assigned roles that are available for partners.")
- # Validate that business_area is within the partner's allowed_business_areas
if self.partner:
+ # Validate that business_area is within the partner's allowed_business_areas
if self.business_area not in self.partner.allowed_business_areas.all():
errors.append(f"{self.business_area} is not within the allowed business areas for {self.partner}.")
+ # Only partners that are not parents can have role assignments
+ if self.partner.is_parent:
+ errors.append(f"{self.partner} is a parent partner and cannot have role assignments.")
if errors:
raise ValidationError(errors)
@@ -421,6 +424,15 @@ class AdminAreaLimitedTo(TimeStampedUUIDModel):
program = models.ForeignKey("program.Program", related_name="admin_area_limits", on_delete=models.CASCADE)
areas = models.ManyToManyField("geo.Area", related_name="admin_area_limits", blank=True)
+ def clean(self) -> None:
+ if self.program.partner_access != self.program.SELECTED_PARTNERS_ACCESS:
+ raise ValidationError(
+ f"Area limits cannot be set for programs with {self.program.partner_access} access."
+ )
+
+ def save(self, *args: Any, **kwargs: Any) -> None:
+ self.clean()
+ super().save(*args, **kwargs)
class UserGroup(NaturalKeyModel, models.Model):
business_area = models.ForeignKey("core.BusinessArea", related_name="user_groups", on_delete=models.CASCADE)
diff --git a/src/hct_mis_api/apps/account/signals.py b/src/hct_mis_api/apps/account/signals.py
index 06d88e5e97..8858762b26 100644
--- a/src/hct_mis_api/apps/account/signals.py
+++ b/src/hct_mis_api/apps/account/signals.py
@@ -11,7 +11,7 @@
from hct_mis_api.api.caches import get_or_create_cache_key
from hct_mis_api.apps.account.caches import get_user_permissions_version_key
from hct_mis_api.apps.account.models import Partner, Role, RoleAssignment, User
-from hct_mis_api.apps.core.models import BusinessArea, BusinessAreaPartnerThrough
+from hct_mis_api.apps.core.models import BusinessArea
@receiver(post_save, sender=RoleAssignment)
@@ -43,7 +43,7 @@ def post_save_user(sender: Any, instance: User, created: bool, *args: Any, **kwa
def allowed_business_areas_changed(sender: Any, instance: Partner, action: str, pk_set: set, **kwargs: Any) -> None:
if action == "post_remove":
removed_business_areas_ids = pk_set
- BusinessAreaPartnerThrough.objects.filter(
+ RoleAssignment.objects.filter(
partner=instance, business_area_id__in=removed_business_areas_ids
).delete()
@@ -52,7 +52,7 @@ def allowed_business_areas_changed(sender: Any, instance: Partner, action: str,
elif action == "post_clear":
removed_business_areas = getattr(instance, "_removed_business_areas", [])
- BusinessAreaPartnerThrough.objects.filter(partner=instance, business_area__in=removed_business_areas).delete()
+ RoleAssignment.objects.filter(partner=instance, business_area__in=removed_business_areas).delete()
# Signals for permissions caches invalidation
diff --git a/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html b/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
deleted file mode 100644
index 5568faa848..0000000000
--- a/src/hct_mis_api/apps/account/templates/admin/account/parent/permissions.html
+++ /dev/null
@@ -1,127 +0,0 @@
-{% extends "admin_extra_buttons/action_page.html" %}
-{% load i18n admin_urls static admin_modify mptt_tags engine %}
-
-{% block extrahead %}{{ block.super }}
-
-
-
- {{ media }}
- {{ business_area_role_formset.media }}
-{% endblock %}
-{% block action-content %}
-
-{% endblock %}
-{% block admin_change_form_document_ready %}{{ block.super }}
-
-
-{% endblock %}
diff --git a/src/hct_mis_api/apps/core/admin.py b/src/hct_mis_api/apps/core/admin.py
index 91fd4c590e..d477b8678e 100644
--- a/src/hct_mis_api/apps/core/admin.py
+++ b/src/hct_mis_api/apps/core/admin.py
@@ -1,7 +1,11 @@
import csv
import logging
from io import StringIO
-from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple, Union
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple, Union, Sequence
+
+from django.contrib.admin.widgets import FilteredSelectMultiple
+
+from hct_mis_api.apps.account import models as account_models
from django import forms
from django.contrib import admin, messages
@@ -22,7 +26,7 @@
HttpResponsePermanentRedirect,
HttpResponseRedirect,
)
-from django.shortcuts import get_object_or_404, redirect
+from django.shortcuts import get_object_or_404, redirect, render
from django.template.defaultfilters import slugify
from django.template.response import TemplateResponse
from django.urls import reverse
@@ -42,7 +46,7 @@
from jsoneditor.forms import JSONEditor
from xlrd import XLRDError
-from hct_mis_api.apps.account.models import Role, User
+from hct_mis_api.apps.account.models import Role, User, Partner, RoleAssignment
from hct_mis_api.apps.administration.widgets import JsonWidget
from hct_mis_api.apps.core.celery_tasks import (
upload_new_kobo_template_and_update_flex_fields_task,
@@ -265,6 +269,57 @@ def split_business_area(self, request: HttpRequest, pk: "UUID") -> Union[HttpRes
return TemplateResponse(request, "core/admin/split_ba.html", context)
+ @button(label="Partners", permission="account.can_change_allowed_partners")
+ def allowed_partners(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
+ business_area = get_object_or_404(BusinessArea, pk=pk)
+
+ class AllowedPartnersForm(forms.Form):
+ partners = forms.ModelMultipleChoiceField(
+ queryset=Partner.objects.all(),
+ required=False,
+ widget=FilteredSelectMultiple("Partners", is_stacked=False)
+ )
+
+ if request.method == "POST":
+ form = AllowedPartnersForm(request.POST)
+ if form.is_valid():
+ selected_partners = form.cleaned_data["partners"]
+ # Get the current allowed partners for the business area
+ previous_allowed_partners = set(Partner.objects.filter(allowed_business_areas=business_area))
+
+ # Identify which partners were removed
+ removed_partners = previous_allowed_partners - set(selected_partners)
+ # Check if there are any removed partners with existing role assignments in this business area
+ for partner in removed_partners:
+ if RoleAssignment.objects.filter(partner=partner, business_area=business_area).exists():
+ self.message_user(
+ request,
+ f"You cannot remove {partner.name} because it has existing role assignments in this business area.",
+ messages.ERROR,
+ )
+ return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+
+ for partner in Partner.objects.all():
+ if partner in selected_partners:
+ partner.allowed_business_areas.add(business_area)
+ else:
+ partner.allowed_business_areas.remove(business_area)
+ messages.success(request, "Allowed partners successfully updated.")
+ return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+
+ else:
+ form = AllowedPartnersForm(initial={
+ "partners": Partner.objects.filter(allowed_business_areas=business_area)
+ })
+
+ context = self.get_common_context(request, pk)
+ context.update({
+ "business_area": business_area,
+ "form": form,
+ })
+
+ return TemplateResponse(request, "core/admin/allowed_partners.html", context)
+
def _get_doap_matrix(self, obj: Any) -> List[Any]:
matrix = []
ca_roles = Role.objects.filter(subsystem=Role.CA).order_by("name").values_list("name", flat=True)
diff --git a/src/hct_mis_api/apps/core/templates/core/admin/allowed_partners.html b/src/hct_mis_api/apps/core/templates/core/admin/allowed_partners.html
new file mode 100644
index 0000000000..d769f3ff2d
--- /dev/null
+++ b/src/hct_mis_api/apps/core/templates/core/admin/allowed_partners.html
@@ -0,0 +1,19 @@
+{% extends "admin_extra_buttons/action_page.html" %}
+{% load i18n admin_urls static admin_modify mptt_tags engine %}
+
+{% block action-content %}
+
+Allowed Partners for {{ business_area.name }}
+
+
+
+{% endblock %}
\ No newline at end of file
diff --git a/tests/unit/apps/account/test_models.py b/tests/unit/apps/account/test_models.py
index 42d0f36737..40725745ee 100644
--- a/tests/unit/apps/account/test_models.py
+++ b/tests/unit/apps/account/test_models.py
@@ -6,10 +6,11 @@
PartnerFactory,
RoleAssignmentFactory,
RoleFactory,
- UserFactory,
+ UserFactory, AdminAreaLimitedToFactory,
)
-from hct_mis_api.apps.account.models import RoleAssignment
+from hct_mis_api.apps.account.models import RoleAssignment, AdminAreaLimitedTo
from hct_mis_api.apps.core.fixtures import create_afghanistan, create_ukraine
+from hct_mis_api.apps.geo.fixtures import AreaFactory
from hct_mis_api.apps.program.fixtures import ProgramFactory
from hct_mis_api.apps.program.models import Program
@@ -44,6 +45,8 @@ def setUp(self) -> None:
business_area=self.business_area,
)
+ self.area_1 = AreaFactory(name="Area 1", p_code="AREA1")
+
def test_user_or_partner(self) -> None:
# Either user or partner must be set
with self.assertRaises(ValidationError) as ve_context:
@@ -207,3 +210,90 @@ def test_unique_partner_role_business_area_program_constraint(self) -> None:
business_area=self.business_area,
program=None,
)
+
+ def test_role_assignment_for_parent_partner(self) -> None:
+ parent_partner = PartnerFactory(name="Parent Partner")
+ child_partner = PartnerFactory(name="Child Partner", parent=parent_partner)
+ parent_partner.allowed_business_areas.add(self.business_area)
+ child_partner.allowed_business_areas.add(self.business_area)
+
+ # Can create RoleAssignment for child partner
+ RoleAssignment.objects.create(
+ user=None,
+ partner=child_partner,
+ role=self.role,
+ business_area=self.business_area,
+ )
+
+ with self.assertRaises(ValidationError) as ve_context:
+ RoleAssignment.objects.create(
+ user=None,
+ partner=parent_partner,
+ role=self.role,
+ business_area=self.business_area,
+ )
+ self.assertIn(
+ f"{parent_partner} is a parent partner and cannot have role assignments.",
+ str(ve_context.exception),
+ )
+
+ def test_parent_partner_with_role_assignment(self) -> None:
+ parent_partner = PartnerFactory(name="Parent Partner")
+ parent_partner.allowed_business_areas.add(self.business_area)
+
+ # Role for the Partner
+ RoleAssignment.objects.create(
+ user=None,
+ partner=parent_partner,
+ role=self.role,
+ business_area=self.business_area,
+ )
+
+ with self.assertRaises(ValidationError) as ve_context:
+ PartnerFactory(name="Child Partner", parent=parent_partner)
+
+ self.assertIn(
+ f"{parent_partner} cannot become a parent as it has RoleAssignments.",
+ str(ve_context.exception),
+ )
+
+ def test_area_limits_for_program_with_selected_partner_access(self) -> None:
+ # Possible to have area limits for a program with selected partner access
+ self.program1.partner_access = Program.SELECTED_PARTNERS_ACCESS
+ self.program1.save()
+
+ AdminAreaLimitedToFactory(
+ partner=self.partner,
+ program=self.program1,
+ areas=[self.area_1]
+ )
+
+ def test_area_limits_for_program_with_all_partner_access(self) -> None:
+ # Not possible to have area limits for a program with ALL_PARTNERS_ACCESS
+ self.program1.partner_access = Program.ALL_PARTNERS_ACCESS
+ self.program1.save()
+ with self.assertRaises(ValidationError) as ve_context:
+ AdminAreaLimitedToFactory(
+ partner=self.partner,
+ program=self.program1,
+ areas=[self.area_1]
+ )
+ self.assertIn(
+ f"Area limits cannot be set for programs with {self.program1.partner_access} access.",
+ str(ve_context.exception),
+ )
+
+ def test_area_limits_for_program_with_none_partner_access(self) -> None:
+ # Not possible to have area limits for a program with NONE_PARTNERS_ACCESS
+ self.program1.partner_access = Program.NONE_PARTNERS_ACCESS
+ self.program1.save()
+ with self.assertRaises(ValidationError) as ve_context:
+ AdminAreaLimitedToFactory(
+ partner=self.partner,
+ program=self.program1,
+ areas=[self.area_1]
+ )
+ self.assertIn(
+ f"Area limits cannot be set for programs with {self.program1.partner_access} access.",
+ str(ve_context.exception),
+ )
From 97f08c7b5be01757aa97e246a578af5a7e72fd52 Mon Sep 17 00:00:00 2001
From: Paulina Kujawa
Date: Fri, 17 Jan 2025 00:24:04 +0100
Subject: [PATCH 018/208] admin changes - area limits on Program, Allowed BAs,
new validations
---
src/hct_mis_api/apps/account/admin/partner.py | 1 +
.../apps/account/admin/user_role.py | 18 ++++++-
.../apps/account/migrations/0007_migration.py | 17 ++++++
src/hct_mis_api/apps/account/models.py | 9 ++--
src/hct_mis_api/apps/account/signals.py | 4 +-
src/hct_mis_api/apps/core/admin.py | 37 ++++++-------
src/hct_mis_api/apps/program/admin.py | 54 +++++++++----------
...r_access.html => program_area_limits.html} | 4 +-
...html => program_area_limits_readonly.html} | 2 +-
tests/unit/apps/account/test_models.py | 23 +++-----
10 files changed, 92 insertions(+), 77 deletions(-)
create mode 100644 src/hct_mis_api/apps/account/migrations/0007_migration.py
rename src/hct_mis_api/apps/program/templates/admin/program/program/{program_partner_access.html => program_area_limits.html} (98%)
rename src/hct_mis_api/apps/program/templates/admin/program/program/{program_partner_access_readonly.html => program_area_limits_readonly.html} (99%)
diff --git a/src/hct_mis_api/apps/account/admin/partner.py b/src/hct_mis_api/apps/account/admin/partner.py
index 2caaed9395..4acd6c9cfa 100644
--- a/src/hct_mis_api/apps/account/admin/partner.py
+++ b/src/hct_mis_api/apps/account/admin/partner.py
@@ -1,4 +1,5 @@
from typing import Any, Optional, Sequence, Type
+
from django import forms
from django.contrib import admin
from django.forms import CheckboxSelectMultiple, ModelForm
diff --git a/src/hct_mis_api/apps/account/admin/user_role.py b/src/hct_mis_api/apps/account/admin/user_role.py
index fe95a03c40..608df8f6fb 100644
--- a/src/hct_mis_api/apps/account/admin/user_role.py
+++ b/src/hct_mis_api/apps/account/admin/user_role.py
@@ -27,7 +27,7 @@ class RoleAssignmentInline(admin.TabularInline):
extra = 0
formset = RoleAssignmentInlineFormSet
- def formfield_for_foreignkey(self, db_field: Any, request=None, **kwargs: Any) -> Any:
+ def formfield_for_foreignkey(self, db_field: Any, request: Any = None, **kwargs: Any) -> Any:
if db_field.name == "business_area":
partner_id = request.resolver_match.kwargs.get("object_id")
@@ -41,8 +41,22 @@ def formfield_for_foreignkey(self, db_field: Any, request=None, **kwargs: Any) -
kwargs["queryset"] = BusinessArea.objects.filter(is_split=False)
return super().formfield_for_foreignkey(db_field, request, **kwargs)
+ def has_add_permission(self, request: HttpRequest, obj: Optional[Any] = None) -> bool:
+ if isinstance(obj, Partner):
+ if obj.is_parent:
+ return False # Disable adding if Partner is a parent
+ return request.user.can_add_business_area_to_partner()
+ return True
+
def has_change_permission(self, request: HttpRequest, obj: Optional[Any] = None) -> bool:
- return request.user.can_add_business_area_to_partner()
+ if isinstance(obj, Partner):
+ return request.user.can_add_business_area_to_partner()
+ return True
+
+ def has_delete_permission(self, request: HttpRequest, obj: Optional[Any] = None) -> bool:
+ if isinstance(obj, Partner):
+ return request.user.can_add_business_area_to_partner()
+ return True
@admin.register(account_models.RoleAssignment)
diff --git a/src/hct_mis_api/apps/account/migrations/0007_migration.py b/src/hct_mis_api/apps/account/migrations/0007_migration.py
new file mode 100644
index 0000000000..70af0e25f8
--- /dev/null
+++ b/src/hct_mis_api/apps/account/migrations/0007_migration.py
@@ -0,0 +1,17 @@
+# Generated by Django 3.2.25 on 2025-01-16 23:01
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('account', '0006_migration'),
+ ]
+
+ operations = [
+ migrations.AlterModelOptions(
+ name='user',
+ options={'permissions': (('can_load_from_ad', 'Can load users from ActiveDirectory'), ('can_sync_with_ad', 'Can synchronise user with ActiveDirectory'), ('can_create_kobo_user', 'Can create users in Kobo'), ('can_import_from_kobo', 'Can import and sync users from Kobo'), ('can_upload_to_kobo', 'Can upload CSV file to Kobo'), ('can_debug', 'Can access debug informations'), ('can_inspect', 'Can inspect objects'), ('quick_links', 'Can see quick links in admin'), ('restrict_help_desk', 'Limit fields to be editable for help desk'), ('can_reindex_programs', 'Can reindex programs'), ('can_add_business_area_to_partner', 'Can add business area to partner'), ('can_change_allowed_partners', 'Can change allowed partners'), ('can_change_area_limits', 'Can change area limits'))},
+ ),
+ ]
diff --git a/src/hct_mis_api/apps/account/models.py b/src/hct_mis_api/apps/account/models.py
index fddaf8966b..8d159f4206 100644
--- a/src/hct_mis_api/apps/account/models.py
+++ b/src/hct_mis_api/apps/account/models.py
@@ -4,7 +4,6 @@
from uuid import UUID
from django import forms
-from django.apps import apps
from django.conf import settings
from django.contrib.admin.widgets import FilteredSelectMultiple
from django.contrib.auth.models import AbstractUser, Group, Permission
@@ -64,7 +63,7 @@ class Partner(LimitBusinessAreaModelMixin, MPTTModel):
def __str__(self) -> str:
return f"{self.name} [Sub-Partner of {self.parent.name}]" if self.parent else self.name
- def save(self, *args, **kwargs):
+ def save(self, *args: Any, **kwargs: Any) -> None:
# Partner cannot be a parent if it has RoleAssignments
if self.parent:
if RoleAssignment.objects.filter(partner=self.parent).exists():
@@ -318,6 +317,7 @@ class Meta:
("can_reindex_programs", "Can reindex programs"),
("can_add_business_area_to_partner", "Can add business area to partner"),
("can_change_allowed_partners", "Can change allowed partners"),
+ ("can_change_area_limits", "Can change area limits"),
)
@@ -426,14 +426,13 @@ class AdminAreaLimitedTo(TimeStampedUUIDModel):
def clean(self) -> None:
if self.program.partner_access != self.program.SELECTED_PARTNERS_ACCESS:
- raise ValidationError(
- f"Area limits cannot be set for programs with {self.program.partner_access} access."
- )
+ raise ValidationError(f"Area limits cannot be set for programs with {self.program.partner_access} access.")
def save(self, *args: Any, **kwargs: Any) -> None:
self.clean()
super().save(*args, **kwargs)
+
class UserGroup(NaturalKeyModel, models.Model):
business_area = models.ForeignKey("core.BusinessArea", related_name="user_groups", on_delete=models.CASCADE)
user = models.ForeignKey("account.User", related_name="user_groups", on_delete=models.CASCADE)
diff --git a/src/hct_mis_api/apps/account/signals.py b/src/hct_mis_api/apps/account/signals.py
index 8858762b26..63ef981546 100644
--- a/src/hct_mis_api/apps/account/signals.py
+++ b/src/hct_mis_api/apps/account/signals.py
@@ -43,9 +43,7 @@ def post_save_user(sender: Any, instance: User, created: bool, *args: Any, **kwa
def allowed_business_areas_changed(sender: Any, instance: Partner, action: str, pk_set: set, **kwargs: Any) -> None:
if action == "post_remove":
removed_business_areas_ids = pk_set
- RoleAssignment.objects.filter(
- partner=instance, business_area_id__in=removed_business_areas_ids
- ).delete()
+ RoleAssignment.objects.filter(partner=instance, business_area_id__in=removed_business_areas_ids).delete()
elif action == "pre_clear":
instance._removed_business_areas = list(instance.allowed_business_areas.all())
diff --git a/src/hct_mis_api/apps/core/admin.py b/src/hct_mis_api/apps/core/admin.py
index d477b8678e..30c6dd2e19 100644
--- a/src/hct_mis_api/apps/core/admin.py
+++ b/src/hct_mis_api/apps/core/admin.py
@@ -1,16 +1,13 @@
import csv
import logging
from io import StringIO
-from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple, Union, Sequence
-
-from django.contrib.admin.widgets import FilteredSelectMultiple
-
-from hct_mis_api.apps.account import models as account_models
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple, Union
from django import forms
from django.contrib import admin, messages
from django.contrib.admin import SimpleListFilter, TabularInline
from django.contrib.admin.templatetags.admin_urls import add_preserved_filters
+from django.contrib.admin.widgets import FilteredSelectMultiple
from django.contrib.messages import ERROR
from django.contrib.postgres.aggregates import ArrayAgg
from django.contrib.postgres.fields import JSONField
@@ -26,7 +23,7 @@
HttpResponsePermanentRedirect,
HttpResponseRedirect,
)
-from django.shortcuts import get_object_or_404, redirect, render
+from django.shortcuts import get_object_or_404, redirect
from django.template.defaultfilters import slugify
from django.template.response import TemplateResponse
from django.urls import reverse
@@ -46,7 +43,7 @@
from jsoneditor.forms import JSONEditor
from xlrd import XLRDError
-from hct_mis_api.apps.account.models import Role, User, Partner, RoleAssignment
+from hct_mis_api.apps.account.models import Partner, Role, RoleAssignment, User
from hct_mis_api.apps.administration.widgets import JsonWidget
from hct_mis_api.apps.core.celery_tasks import (
upload_new_kobo_template_and_update_flex_fields_task,
@@ -275,9 +272,11 @@ def allowed_partners(self, request: HttpRequest, pk: int) -> Union[TemplateRespo
class AllowedPartnersForm(forms.Form):
partners = forms.ModelMultipleChoiceField(
- queryset=Partner.objects.all(),
+ queryset=Partner.objects.exclude(
+ id__in=Partner.objects.filter(parent__isnull=False).values_list("parent_id", flat=True)
+ ),
required=False,
- widget=FilteredSelectMultiple("Partners", is_stacked=False)
+ widget=FilteredSelectMultiple("Partners", is_stacked=False),
)
if request.method == "POST":
@@ -297,7 +296,7 @@ class AllowedPartnersForm(forms.Form):
f"You cannot remove {partner.name} because it has existing role assignments in this business area.",
messages.ERROR,
)
- return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+ return HttpResponseRedirect(request.get_full_path())
for partner in Partner.objects.all():
if partner in selected_partners:
@@ -305,18 +304,20 @@ class AllowedPartnersForm(forms.Form):
else:
partner.allowed_business_areas.remove(business_area)
messages.success(request, "Allowed partners successfully updated.")
- return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+ return HttpResponseRedirect(request.get_full_path())
else:
- form = AllowedPartnersForm(initial={
- "partners": Partner.objects.filter(allowed_business_areas=business_area)
- })
+ form = AllowedPartnersForm(
+ initial={"partners": Partner.objects.filter(allowed_business_areas=business_area)}
+ )
context = self.get_common_context(request, pk)
- context.update({
- "business_area": business_area,
- "form": form,
- })
+ context.update(
+ {
+ "business_area": business_area,
+ "form": form,
+ }
+ )
return TemplateResponse(request, "core/admin/allowed_partners.html", context)
diff --git a/src/hct_mis_api/apps/program/admin.py b/src/hct_mis_api/apps/program/admin.py
index 98a4596296..d21c3db506 100644
--- a/src/hct_mis_api/apps/program/admin.py
+++ b/src/hct_mis_api/apps/program/admin.py
@@ -14,12 +14,12 @@
from adminfilters.filters import ChoicesFieldComboFilter
from adminfilters.mixin import AdminAutoCompleteSearchMixin
-from hct_mis_api.apps.account.models import Partner
+from hct_mis_api.apps.account.models import AdminAreaLimitedTo, Partner
from hct_mis_api.apps.geo.models import Area
from hct_mis_api.apps.household.documents import HouseholdDocument, get_individual_doc
from hct_mis_api.apps.household.forms import CreateTargetPopulationTextForm
from hct_mis_api.apps.household.models import Household, Individual
-from hct_mis_api.apps.program.models import Program, ProgramCycle, ProgramPartnerThrough
+from hct_mis_api.apps.program.models import Program, ProgramCycle
from hct_mis_api.apps.registration_datahub.services.biometric_deduplication import (
BiometricDeduplicationService,
)
@@ -57,10 +57,10 @@ class ProgramCycleAdminInline(admin.TabularInline):
ordering = ["-start_date"]
-class PartnerAreaForm(forms.Form):
+class PartnerAreaLimitForm(forms.Form):
partner = forms.ModelChoiceField(queryset=Partner.objects.all(), required=True)
areas = TreeNodeMultipleChoiceField(
- queryset=Area.objects.filter(area_type__area_level__lte=3), widget=CheckboxSelectMultiple(), required=False
+ queryset=Area.objects.filter(area_type__area_level__lte=3), widget=CheckboxSelectMultiple(), required=True
)
@@ -127,63 +127,59 @@ def create_target_population_from_list(self, request: HttpRequest, pk: str) -> O
context["form"] = form
return TemplateResponse(request, "admin/program/program/create_target_population_from_text.html", context)
- @button()
- def partners(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
- context = self.get_common_context(request, pk, title="Partner access")
+ @button(permission="account.can_change_area_limits")
+ def area_limits(self, request: HttpRequest, pk: int) -> Union[TemplateResponse, HttpResponseRedirect]:
+ context = self.get_common_context(request, pk, title="Admin Area Limits")
program: Program = context["original"]
- PartnerAreaFormSet = formset_factory(PartnerAreaForm, extra=0, can_delete=True)
+ PartnerAreaLimitFormSet = formset_factory(PartnerAreaLimitForm, extra=0, can_delete=True)
is_editable = program.partner_access == Program.SELECTED_PARTNERS_ACCESS
if request.method == "GET" or not is_editable:
partner_area_data = []
- for partner_program_through in program.program_partner_through.all():
+ for area_limits in program.admin_area_limits.all():
partner_area_data.append(
{
- "partner": partner_program_through.partner,
- "areas": [
- str(area_id) for area_id in partner_program_through.areas.values_list("id", flat=True)
- ],
+ "partner": area_limits.partner,
+ "areas": [str(area_id) for area_id in area_limits.areas.values_list("id", flat=True)],
}
)
- partner_area_form_set = PartnerAreaFormSet(initial=partner_area_data, prefix="program_areas")
+ partner_area_form_set = PartnerAreaLimitFormSet(initial=partner_area_data, prefix="program_areas")
elif request.method == "POST":
- partner_area_form_set = PartnerAreaFormSet(request.POST or None, prefix="program_areas")
+ partner_area_form_set = PartnerAreaLimitFormSet(request.POST or None, prefix="program_areas")
if partner_area_form_set.is_valid():
for partner_area_form in partner_area_form_set:
form = partner_area_form.cleaned_data
if form and not form["DELETE"]:
areas_ids = list(map(lambda area: str(area.id), form["areas"]))
- program_partner, _ = ProgramPartnerThrough.objects.update_or_create(
+ program_partner, _ = AdminAreaLimitedTo.objects.update_or_create(
partner=form["partner"],
program=program,
)
- if not areas_ids:
- program_partner.full_area_access = True
- program_partner.save(update_fields=["full_area_access"])
- else:
- program_partner.full_area_access = False
- program_partner.save(update_fields=["full_area_access"])
- program_partner.areas.set(areas_ids)
+ program_partner.areas.set(areas_ids)
elif form and form["DELETE"]:
- ProgramPartnerThrough.objects.filter(partner=form["partner"], program=program).delete()
- return HttpResponseRedirect(reverse("admin:program_program_partners", args=[pk]))
+ AdminAreaLimitedTo.objects.filter(partner=form["partner"], program=program).delete()
+ return HttpResponseRedirect(reverse("admin:program_program_area_limits", args=[pk]))
context["program_area_formset"] = partner_area_form_set
context["business_area"] = program.business_area
context["areas"] = Area.objects.filter(area_type__country__business_areas__id=program.business_area.id)
+ # it's only possible to create area limits for partners that have a role in this program
context["partners"] = (
- Partner.objects.filter(Q(allowed_business_areas=program.business_area))
- .exclude(name="UNICEF")
+ Partner.objects.filter(
+ Q(role_assignments__program=program)
+ | (Q(role_assignments__business_area=program.business_area) & Q(role_assignments__program__isnull=True))
+ )
+ .exclude(parent__name="UNICEF")
.order_by("name")
)
context["program"] = program
context["unicef_partner_id"] = Partner.objects.get(name="UNICEF").id
if is_editable:
- return TemplateResponse(request, "admin/program/program/program_partner_access.html", context)
+ return TemplateResponse(request, "admin/program/program/program_area_limits.html", context)
else:
- return TemplateResponse(request, "admin/program/program/program_partner_access_readonly.html", context)
+ return TemplateResponse(request, "admin/program/program/program_area_limits_readonly.html", context)
@button(permission="account.can_reindex_programs")
def reindex_program(self, request: HttpRequest, pk: int) -> HttpResponseRedirect:
diff --git a/src/hct_mis_api/apps/program/templates/admin/program/program/program_partner_access.html b/src/hct_mis_api/apps/program/templates/admin/program/program/program_area_limits.html
similarity index 98%
rename from src/hct_mis_api/apps/program/templates/admin/program/program/program_partner_access.html
rename to src/hct_mis_api/apps/program/templates/admin/program/program/program_area_limits.html
index 00051da3c6..e878f9c858 100644
--- a/src/hct_mis_api/apps/program/templates/admin/program/program/program_partner_access.html
+++ b/src/hct_mis_api/apps/program/templates/admin/program/program/program_area_limits.html
@@ -27,7 +27,7 @@
|