Adding decorator to verify submission status (#4502)

* Adding decorator to verify submission status

* Param name tweak

* Adding unit tests

* Handling missing sac

backend/audit/test_verify_status.py

@@ -0,0 +1,83 @@
+from .models import (
+    Access,
+    SingleAuditChecklist,
+    User,
+)
+from audit.models.models import STATUS
+from audit.views.views import verify_status
+
+from django.core.exceptions import PermissionDenied
+from django.test import TestCase
+from django.urls import reverse
+
+from model_bakery import baker
+from unittest.mock import Mock
+
+
+class TestVerifyStatus(TestCase):
+    def _test_verify_status(self, sac_status, required_status, sac_exists):
+        """
+        Helper to test the behavior of a method decorated with verify_status
+        sac_status - Current status of the sac
+        required_status - The required status to be provided to the decorator
+        """
+        user = baker.make(User)
+        self.client.force_login(user)
+
+        if sac_exists:
+            sac = baker.make(SingleAuditChecklist, submission_status=sac_status)
+            baker.make(Access, user=user, sac=sac, role="editor")
+
+        # Set up a fake method to decorate
+        decorator = verify_status(required_status)
+        func = Mock()
+        mock_view = Mock()
+        mock_request = Mock()
+        decorated_func = decorator(func)
+
+        if sac_exists:
+            result = decorated_func(mock_view, mock_request, report_id=sac.report_id)
+
+            if sac_status == required_status:
+                # The decorated method should be called as normal if the statuses match
+                func.assert_called()
+            else:
+                # The decorated method should redirect if the statuses don't match
+                result.client = self.client
+                self.assertRedirects(
+                    result,
+                    reverse("audit:SubmissionProgress", args=[sac.report_id]),
+                )
+        else:
+            with self.assertRaises(PermissionDenied):
+                decorated_func(mock_view, mock_request, report_id="bad_report_id")
+
+    def test_valid(self):
+        """
+        Current and required statuses matching should pass
+        """
+        self._test_verify_status(
+            sac_status=STATUS.IN_PROGRESS,
+            required_status=STATUS.IN_PROGRESS,
+            sac_exists=True,
+        )
+
+    def test_invalid(self):
+        """
+        Current and required statuses differing should pass
+        """
+        self._test_verify_status(
+            sac_status=STATUS.IN_PROGRESS,
+            required_status=STATUS.AUDITEE_CERTIFIED,
+            sac_exists=True,
+        )
+
+    def test_no_sac(self):
+        """
+        Current and required statuses differing should throw
+        """
+        self._test_verify_status(
+            sac_status=STATUS.IN_PROGRESS,
+            required_status=STATUS.AUDITEE_CERTIFIED,
+            sac_exists=False,
+        )

backend/audit/views/views.py

@@ -61,6 +61,38 @@ def _friendly_status(status):
     return dict(SingleAuditChecklist.STATUS_CHOICES)[status]
 
 
+def verify_status(status):
+    """
+    Decorator to be applied to view request methods (i.e. get, post) to verify
+    that the submission is in the correct state before allowing the user to
+    proceed. An incorrect status usually happens via direct URL access. If the
+    given status does not match the submission's, it will redirect them back to
+    the submission progress page.
+    """
+
+    def decorator_verify_status(request_method):
+        def verify(view, request, *args, **kwargs):
+            report_id = kwargs["report_id"]
+
+            try:
+                sac = SingleAuditChecklist.objects.get(report_id=report_id)
+            except SingleAuditChecklist.DoesNotExist:
+                raise PermissionDenied("You do not have access to this audit.")
+
+            # Return to checklist, the Audit is not in the correct state.
+            if sac.submission_status != status:
+                logger.warning(
+                    f"Expected submission status {status} but it's currently {sac.submission_status}"
+                )
+                return redirect(f"/audit/submission-progress/{sac.report_id}")
+            else:
+                return request_method(view, request, *args, **kwargs)
+
+        return verify
+
+    return decorator_verify_status
+
+
 class MySubmissions(LoginRequiredMixin, generic.View):
     redirect_field_name = "Home"
 
@@ -345,6 +377,7 @@ def post(self, request, *args, **kwargs):
 
 
 class AuditorCertificationStep1View(CertifyingAuditorRequiredMixin, generic.View):
+    @verify_status(STATUS.READY_FOR_CERTIFICATION)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -373,6 +406,7 @@ def get(self, request, *args, **kwargs):
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.READY_FOR_CERTIFICATION)
     def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -410,6 +444,7 @@ def post(self, request, *args, **kwargs):
 
 
 class AuditorCertificationStep2View(CertifyingAuditorRequiredMixin, generic.View):
+    @verify_status(STATUS.READY_FOR_CERTIFICATION)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -447,6 +482,7 @@ def get(self, request, *args, **kwargs):
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.READY_FOR_CERTIFICATION)
     def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -496,6 +532,7 @@ def post(self, request, *args, **kwargs):
 
 
 class AuditeeCertificationStep1View(CertifyingAuditeeRequiredMixin, generic.View):
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -515,15 +552,12 @@ def get(self, request, *args, **kwargs):
                 "form": form,
             }
 
-            # Return to checklist, the Audit is not in the correct state.
-            if sac.submission_status != STATUS.AUDITOR_CERTIFIED:
-                return redirect(f"/audit/submission-progress/{sac.report_id}")
-
             return render(request, "audit/auditee-certification-step-1.html", context)
 
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -542,10 +576,6 @@ def post(self, request, *args, **kwargs):
                 "submission_status": sac.submission_status,
             }
 
-            # Return to checklist, the Audit is not in the correct state.
-            if sac.submission_status != STATUS.AUDITOR_CERTIFIED:
-                return redirect(f"/audit/submission-progress/{sac.report_id}")
-
             if form.is_valid():
                 # Save to session. Retrieved and saved after step 2.
                 request.session["AuditeeCertificationStep1Session"] = form.cleaned_data
@@ -561,6 +591,7 @@ def post(self, request, *args, **kwargs):
 
 
 class AuditeeCertificationStep2View(CertifyingAuditeeRequiredMixin, generic.View):
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -589,15 +620,12 @@ def get(self, request, *args, **kwargs):
                 "form": form,
             }
 
-            # Return to checklist, the Audit is not in the correct state.
-            if sac.submission_status != STATUS.AUDITOR_CERTIFIED:
-                return redirect(f"/audit/submission-progress/{sac.report_id}")
-
             return render(request, "audit/auditee-certification-step-2.html", context)
 
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -614,10 +642,6 @@ def post(self, request, *args, **kwargs):
                 "submission_status": sac.submission_status,
             }
 
-            # Return to checklist, the Audit is not in the correct state.
-            if sac.submission_status != STATUS.AUDITOR_CERTIFIED:
-                return redirect(f"/audit/submission-progress/{sac.report_id}")
-
             if form2.is_valid():
                 form_cleaned = {
                     "auditee_certification": form1_cleaned,
@@ -646,6 +670,7 @@ def post(self, request, *args, **kwargs):
 
 
 class CertificationView(CertifyingAuditeeRequiredMixin, generic.View):
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -661,6 +686,7 @@ def get(self, request, *args, **kwargs):
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.AUDITOR_CERTIFIED)
     def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
 
@@ -676,6 +702,7 @@ def post(self, request, *args, **kwargs):
 
 
 class SubmissionView(CertifyingAuditeeRequiredMixin, generic.View):
+    @verify_status(STATUS.AUDITEE_CERTIFIED)
     def get(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
         try:
@@ -690,6 +717,7 @@ def get(self, request, *args, **kwargs):
         except SingleAuditChecklist.DoesNotExist:
             raise PermissionDenied("You do not have access to this audit.")
 
+    @verify_status(STATUS.AUDITEE_CERTIFIED)
     def post(self, request, *args, **kwargs):
         # RACE HAZARD WARNING
         # It is possible for a user to enter the submission multiple times,