From c9ba2b08f6d5e47e9f6164daf9e3f04284e202bd Mon Sep 17 00:00:00 2001 From: Roman Karpovich Date: Mon, 1 Jul 2024 12:02:46 +0800 Subject: [PATCH] don't allow visit lead to approve report for own activity --- .../field_monitoring/planning/models.py | 6 ++--- .../planning/tests/test_views.py | 25 +++++++++++++++++++ .../planning/transitions/permissions.py | 6 ++++- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/etools/applications/field_monitoring/planning/models.py b/src/etools/applications/field_monitoring/planning/models.py index 055f1c360..64cbb5fec 100644 --- a/src/etools/applications/field_monitoring/planning/models.py +++ b/src/etools/applications/field_monitoring/planning/models.py @@ -25,8 +25,8 @@ from etools.applications.field_monitoring.fm_settings.models import LocationSite, Method, Option, Question from etools.applications.field_monitoring.planning.mixins import ProtectUnknownTransitionsMeta from etools.applications.field_monitoring.planning.transitions.permissions import ( + approve_final_report_permission, user_is_field_monitor_permission, - user_is_pme_or_approver_permission, user_is_visit_lead_permission, ) from etools.applications.locations.models import Location @@ -578,12 +578,12 @@ def submit_report(self): pass @transition(field=status, source=STATUSES.submitted, target=STATUSES.completed, - permission=user_is_pme_or_approver_permission) + permission=approve_final_report_permission) def complete(self): pass @transition(field=status, source=STATUSES.submitted, target=STATUSES.report_finalization, - permission=user_is_pme_or_approver_permission) + permission=approve_final_report_permission) def reject_report(self): pass diff --git a/src/etools/applications/field_monitoring/planning/tests/test_views.py b/src/etools/applications/field_monitoring/planning/tests/test_views.py index 299f25256..17348da81 100644 --- a/src/etools/applications/field_monitoring/planning/tests/test_views.py +++ b/src/etools/applications/field_monitoring/planning/tests/test_views.py @@ -516,11 +516,36 @@ def test_complete_reviewed_by_set(self): activity = MonitoringActivityFactory(monitor_type='staff', status='submitted') approver = UserFactory(approver=True) + details_response = self._test_retrieve(approver, activity) + self.assertEqual( + ['complete', 'reject_report'], + [t['transition'] for t in details_response.data['transitions']], + ) + self.assertIsNone(activity.reviewed_by) self._test_update(approver, activity, {'status': 'completed'}) activity.refresh_from_db() self.assertEqual(activity.reviewed_by, approver) + @override_settings(UNICEF_USER_EMAIL="@example.com") + def test_complete_by_visit_lead(self): + approver = UserFactory(approver=True) + activity = MonitoringActivityFactory(monitor_type='staff', status='submitted', visit_lead=approver) + + details_response = self._test_retrieve(approver, activity) + self.assertEqual( + [], + [t['transition'] for t in details_response.data['transitions']], + ) + + self._test_update( + approver, + activity, + {'status': 'completed'}, + expected_status=status.HTTP_400_BAD_REQUEST, + basic_errors=['generic_transition_fail'] + ) + @override_settings(UNICEF_USER_EMAIL="@example.com") def test_report_reject_reviewed_by_set(self): activity = MonitoringActivityFactory(monitor_type='staff', status='submitted') diff --git a/src/etools/applications/field_monitoring/planning/transitions/permissions.py b/src/etools/applications/field_monitoring/planning/transitions/permissions.py index 27cf83d8f..6b7148f55 100644 --- a/src/etools/applications/field_monitoring/planning/transitions/permissions.py +++ b/src/etools/applications/field_monitoring/planning/transitions/permissions.py @@ -10,5 +10,9 @@ def user_is_visit_lead_permission(activity, user): return user == activity.visit_lead -def user_is_pme_or_approver_permission(activity, user): +def user_is_pme_or_approver_permission(_activity, user): return user.groups.filter(name__in=[PME.name, MonitoringVisitApprover.name]).exists() + + +def approve_final_report_permission(activity, user): + return user_is_pme_or_approver_permission(activity, user) and not user_is_visit_lead_permission(activity, user)