From 17064fc33c7b8045f0154132acd796890caa78a5 Mon Sep 17 00:00:00 2001 From: ismail Date: Mon, 20 Jul 2026 21:40:43 +0300 Subject: [PATCH] fix(inquiry): dept_manager of handling dept can now record department response The inquiry_department_response view excluded dept managers even though they can resolve the inquiry via inquiry_change_status. Now both actions use the same handling_department_id predicate so forwarding locks the old department out consistently. --- apps/complaints/test_workflow_realignment.py | 72 ++++++++++++++++++++ apps/complaints/ui_views.py | 6 +- conftest.py | 30 ++++++++ 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 apps/complaints/test_workflow_realignment.py create mode 100644 conftest.py diff --git a/apps/complaints/test_workflow_realignment.py b/apps/complaints/test_workflow_realignment.py new file mode 100644 index 0000000..6bfd5d4 --- /dev/null +++ b/apps/complaints/test_workflow_realignment.py @@ -0,0 +1,72 @@ +"""Tests for the feedback workflow realignment (Inquiry department-ownership).""" +from django.contrib.auth.models import Group + +import pytest +from django.test import TestCase +from django.urls import reverse + +from apps.accounts.models import User +from apps.organizations.models import Department, Hospital +from apps.complaints.models import Inquiry + + +def _ensure_group(name): + """Get or create a role group (pytest runs with --nomigrations, so seeded + groups may not exist in the test DB — mirrors the pattern in + tests_routing_rejection.py and tests_workflow_fixes.py).""" + grp, _ = Group.objects.get_or_create(name=name) + return grp + + +@pytest.mark.django_db +class TestInquiryDepartmentResponsePermission(TestCase): + """A department manager of the handling department can record a response. + + Uses self.client (full middleware incl. django.contrib.messages) rather + than calling the view directly with RequestFactory, because the view + calls messages.error() on the permission-denied path. + """ + + def setUp(self): + self.hospital = Hospital.objects.create(name="Test Hospital", code="TH01") + self.dept = Department.objects.create(name="Dept A", hospital=self.hospital, code="DA") + self.other_dept = Department.objects.create(name="Dept B", hospital=self.hospital, code="DB") + + _ensure_group("Department Manager") + self.dept_manager = User.objects.create_user( + username="dmgr", email="dmgr@test", password="x", department=self.dept + ) + self.dept_manager.groups.add(Group.objects.get(name="Department Manager")) + + self.other_manager = User.objects.create_user( + username="other_dmgr", email="o@test", password="x", department=self.other_dept + ) + self.other_manager.groups.add(Group.objects.get(name="Department Manager")) + + self.inquiry = Inquiry.objects.create( + hospital=self.hospital, + department=self.dept, + subject="Test", + message="msg", + status="in_progress", + sent_to_department=True, + ) + + def test_dept_manager_of_handling_dept_can_access_response_view(self): + """GET to inquiry_department_response succeeds (200) for dept manager of handling dept.""" + self.client.force_login(self.dept_manager) + response = self.client.get( + reverse("inquiries:inquiry_department_response", kwargs={"pk": self.inquiry.pk}) + ) + # 200 = permission granted, 302 = permission denied redirect. + assert response.status_code == 200, \ + f"dept_manager of handling dept should be allowed, got {response.status_code}" + + def test_dept_manager_of_other_dept_cannot_access(self): + """Dept manager of a non-handling department is denied (302 redirect).""" + self.client.force_login(self.other_manager) + response = self.client.get( + reverse("inquiries:inquiry_department_response", kwargs={"pk": self.inquiry.pk}) + ) + assert response.status_code == 302, \ + f"dept_manager of other dept should be redirected (denied), got {response.status_code}" diff --git a/apps/complaints/ui_views.py b/apps/complaints/ui_views.py index d741dd4..c051faf 100644 --- a/apps/complaints/ui_views.py +++ b/apps/complaints/ui_views.py @@ -4475,12 +4475,10 @@ def inquiry_department_response(request, pk): if not ( user.is_px_admin() or user.is_hospital_admin() - or (user.is_champion() and ( - inquiry.department == user.department - or inquiry.outgoing_department == user.department - )) or user.is_px_management() or user.is_px_employee() + or (user.is_champion() and user.department_id == inquiry.handling_department_id) + or (user.is_department_manager() and user.department_id == inquiry.handling_department_id) ): messages.error(request, "You don't have permission to respond to this inquiry.") return redirect("inquiries:inquiry_detail", pk=pk) diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..4487a17 --- /dev/null +++ b/conftest.py @@ -0,0 +1,30 @@ +"""Project-wide pytest fixtures. + +The default test environment runs without a collected staticfiles manifest +(``addopts = "--reuse-db --nomigrations"`` in pyproject.toml), so any view +that renders a template containing ``{% static %}`` tags against +``CompressedManifestStaticFilesStorage`` raises ``ValueError: Missing +staticfiles manifest entry for ...``. + +This autouse fixture swaps in plain ``StaticFilesStorage`` for the duration +of every test, so tests can assert ``status_code == 200`` on template-rendering +views without first running ``collectstatic``. +""" +import pytest +from django.test import override_settings + +from config.settings.base import STORAGES as BASE_STORAGES + + +@pytest.fixture(autouse=True) +def _disable_static_manifest(): + """Use plain StaticFilesStorage (no manifest hashing) during tests.""" + staticfiles_storage = { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage", + } + storages_override = {**BASE_STORAGES, "staticfiles": staticfiles_storage} + with override_settings( + STATICFILES_STORAGE="django.contrib.staticfiles.storage.StaticFilesStorage", + STORAGES=storages_override, + ): + yield