fix(review): observation dept_manager button gate + appreciation status guard

Two issues from the whole-branch review:

1. (Important) Observation detail's can_respond_to_department context var
   did not include is_department_manager, so even though Task 2 let dept
   managers pass the view's permission check, the 'Submit Response'
   button stayed hidden from them. Now the context var matches the view
   predicate. Regression test added.

2. (Minor) appreciation_list had no guard against now-invalid status
   query params (?status=acknowledged from an old bookmark would show
   an unexplained empty list). Unknown values are now silently reset
   to 'all statuses'.
This commit is contained in:
ismail 2026-07-20 22:38:12 +03:00
parent c88c5cf973
commit 615b8467ef
3 changed files with 46 additions and 1 deletions

View File

@ -63,6 +63,11 @@ def appreciation_list(request):
queryset = queryset.none() queryset = queryset.none()
status_filter_val = request.GET.get("status", "") status_filter_val = request.GET.get("status", "")
# Guard: silently reset unknown/legacy status values (e.g. a bookmarked
# ?status=acknowledged from before the 3-state collapse) to "all" so the
# user doesn't see an unexplained empty list.
if status_filter_val and status_filter_val not in dict(AppreciationStatus.choices):
status_filter_val = ""
if status_filter_val: if status_filter_val:
queryset = queryset.filter(status=status_filter_val) queryset = queryset.filter(status=status_filter_val)

View File

@ -67,3 +67,43 @@ class TestObservationDepartmentResponsePermission(TestCase):
) )
assert response.status_code == 302, \ assert response.status_code == 302, \
f"dept_manager of other dept should be redirected (denied), got {response.status_code}" f"dept_manager of other dept should be redirected (denied), got {response.status_code}"
@pytest.mark.django_db
class TestObservationDetailRespondButtonGate(TestCase):
"""The 'Submit Response' button must be visible to dept_manager of the
assigned department not just pass the view's permission check.
Locks in the fix for the Important issue found in whole-branch review:
Task 2 added dept_manager to the view predicate but the template context
var `can_respond_to_department` was missed, so the button stayed hidden.
"""
def setUp(self):
self.hospital = Hospital.objects.create(name="Test Hospital Gate", code="TH04")
self.dept = Department.objects.create(name="Dept Gate", hospital=self.hospital, code="GT")
_ensure_group("Department Manager")
self.dept_manager = User.objects.create_user(
username="gdmgr", email="gdmgr@test", password="x", department=self.dept
)
self.dept_manager.groups.add(Group.objects.get(name="Department Manager"))
self.observation = Observation.objects.create(
description="Gate test observation",
hospital=self.hospital,
assigned_department=self.dept,
sent_to_department=True,
department_responded_at=None,
status="in_progress",
)
def test_dept_manager_sees_respond_button(self):
"""The department detail page renders the respond control for dept managers."""
self.client.force_login(self.dept_manager)
response = self.client.get(
reverse("observations:observation_detail", kwargs={"pk": self.observation.pk})
)
assert response.status_code == 200
assert response.context["can_respond_to_department"] is True, \
"dept_manager of assigned dept must see can_respond_to_department=True"

View File

@ -653,7 +653,7 @@ def observation_detail(request, pk):
"can_triage": user.has_perm("observations.triage_observation") or user.is_px_admin() or user.is_px_employee(), "can_triage": user.has_perm("observations.triage_observation") or user.is_px_admin() or user.is_px_employee(),
"can_convert": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(), "can_convert": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(),
"can_send_to_department": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee() or user.is_department_manager() or user.is_px_management(), "can_send_to_department": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee() or user.is_department_manager() or user.is_px_management(),
"can_respond_to_department": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee() or (user.is_champion() and observation.assigned_department == user.department), "can_respond_to_department": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee() or (user.is_champion() and observation.assigned_department == user.department) or (user.is_department_manager() and observation.assigned_department == user.department),
"can_send_reminder": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(), "can_send_reminder": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(),
"can_delete": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(), "can_delete": user.is_px_admin() or user.is_hospital_admin() or user.is_px_management() or user.is_px_employee(),
"can_admin": user.is_px_admin() or user.is_hospital_admin() or user.is_px_employee(), "can_admin": user.is_px_admin() or user.is_hospital_admin() or user.is_px_employee(),