From 2574b479235eab2d7ba199cdf6a731830132b95d Mon Sep 17 00:00:00 2001 From: lizerk Date: Tue, 14 Apr 2026 01:18:32 +0300 Subject: [PATCH] fix(api): enforce single active department membership --- backend/api/v1/employees/views/departments.py | 31 ++++++- backend/employees/utils.py | 32 ++++++++ .../api/v1/employees/test_departments.py | 81 +++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/backend/api/v1/employees/views/departments.py b/backend/api/v1/employees/views/departments.py index 951f4c2b..691eccf5 100644 --- a/backend/api/v1/employees/views/departments.py +++ b/backend/api/v1/employees/views/departments.py @@ -27,7 +27,11 @@ from employees.models import ( EmployeeDepartment, RoleAssignment, ) -from employees.utils import _build_links_for_dept, _validate_head_active +from employees.utils import ( + _build_links_for_dept, + _other_active_department_link, + _validate_head_active, +) from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated @@ -473,7 +477,30 @@ class DepartmentViewSet(viewsets.ModelViewSet): emp_id = payload.validated_data["employee_id"] employee_model = Department._meta.get_field("head").remote_field.model - get_object_or_404(employee_model, id=emp_id) + employee = get_object_or_404(employee_model, id=emp_id) + + if getattr(employee, "is_active", True) is False: + return Response( + {"employee_id": ["Employee is inactive."]}, + status=status.HTTP_400_BAD_REQUEST, + ) + + other_active_link = _other_active_department_link( + emp_id, + exclude_department_id=dept.id, + ) + if other_active_link is not None: + return Response( + { + "employee_id": [ + ( + "Employee already belongs to another active " + f"department: {other_active_link.department.name}." + ) + ] + }, + status=status.HTTP_400_BAD_REQUEST, + ) link, created = EmployeeDepartment.objects.get_or_create( employee_id=emp_id, diff --git a/backend/employees/utils.py b/backend/employees/utils.py index 4ebb640c..5933f3b6 100644 --- a/backend/employees/utils.py +++ b/backend/employees/utils.py @@ -17,6 +17,24 @@ from phonenumbers import PhoneNumberFormat Employee = get_user_model() +def _other_active_department_link( + employee_id: int, + *, + exclude_department_id: int | None = None, +) -> EmployeeDepartment | None: + qs = ( + EmployeeDepartment.objects.filter( + employee_id=employee_id, + is_active=True, + ) + .select_related("department") + .order_by("id") + ) + if exclude_department_id is not None: + qs = qs.exclude(department_id=exclude_department_id) + return qs.first() + + def _to_bool(val: str | None) -> bool | None: if val is None: return None @@ -87,6 +105,20 @@ def _validate_head_active( if require_email_verified and getattr(emp, "email_verified", True) is False: return False, {"head_id": ["Employee is inactive."]} + other_active_link = _other_active_department_link( + employee_id, + exclude_department_id=dept.id, + ) + if other_active_link is not None: + return False, { + "head_id": [ + ( + "Employee already belongs to another active department: " + f"{other_active_link.department.name}." + ) + ] + } + return True, None diff --git a/backend/tests/api/v1/employees/test_departments.py b/backend/tests/api/v1/employees/test_departments.py index 2df38085..ed4c05d8 100644 --- a/backend/tests/api/v1/employees/test_departments.py +++ b/backend/tests/api/v1/employees/test_departments.py @@ -514,6 +514,58 @@ def test_set_head_requires_perm(api_client: APIClient): == status.HTTP_403_FORBIDDEN ) + +def test_set_head_rejects_employee_from_another_active_department( + api_client: APIClient, +): + d1 = Department.objects.create(name="Dept 1") + d2 = Department.objects.create(name="Dept 2") + + manager = make_user("head-manager@example.com") + api_client.force_authenticate(user=manager) + r_change = make_role(d1, "head-assigner", ["change_department_head"]) + EmployeeDepartment.objects.create( + employee=manager, + department=d1, + is_active=True, + role=r_change, + ) + + candidate = make_user("foreign-head@example.com") + EmployeeDepartment.objects.create( + employee=candidate, + department=d2, + is_active=True, + ) + + url = reverse("api:v1:departments-set-head", args=[d1.pk]) + resp = api_client.post(url, {"head_id": candidate.id}, format="json") + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert "another active department" in resp.json()["head_id"][0] + + +def test_set_head_makes_head_active_department_member(api_client: APIClient): + d = Department.objects.create(name="Dept Head Link") + + manager = make_user("head-manager-2@example.com") + api_client.force_authenticate(user=manager) + r_change = make_role(d, "head-assigner", ["change_department_head"]) + EmployeeDepartment.objects.create( + employee=manager, + department=d, + is_active=True, + role=r_change, + ) + + candidate = make_user("new-head@example.com") + + url = reverse("api:v1:departments-set-head", args=[d.pk]) + resp = api_client.post(url, {"head_id": candidate.id}, format="json") + assert resp.status_code == status.HTTP_200_OK + + link = EmployeeDepartment.objects.get(employee=candidate, department=d) + assert link.is_active is True + # ---------- tests: action set_member_role ---------- def test_set_member_role_happy_path(api_client: APIClient): @@ -592,3 +644,32 @@ def test_set_member_role_missing_employee_id(api_client: APIClient): url = reverse("api:v1:departments-set-member-role", args=[d.pk]) resp = api_client.post(url, {"role_id": None}, format="json") assert resp.status_code == status.HTTP_400_BAD_REQUEST + + +def test_add_member_rejects_employee_from_another_active_department( + api_client: APIClient, +): + d1 = Department.objects.create(name="Target Dept") + d2 = Department.objects.create(name="Other Dept") + + manager = make_user("dept-manager@example.com") + api_client.force_authenticate(user=manager) + r_manage = make_role(d1, "manager", ["manage_department"]) + EmployeeDepartment.objects.create( + employee=manager, + department=d1, + is_active=True, + role=r_manage, + ) + + employee = make_user("busy-member@example.com") + EmployeeDepartment.objects.create( + employee=employee, + department=d2, + is_active=True, + ) + + url = reverse("api:v1:departments-add-member", args=[d1.pk]) + resp = api_client.post(url, {"employee_id": employee.id}, format="json") + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert "another active department" in resp.json()["employee_id"][0]