Merge pull request #2102 from uktrade/dev

UAT release

api/applications/managers.py

@@ -1,3 +1,5 @@
+from django.db.models import Q
+
 from model_utils.managers import InheritanceManager
 
 from api.staticdata.statuses.enums import CaseStatusEnum
@@ -10,8 +12,14 @@ def drafts(self, organisation, sort_by):
         return self.get_queryset().filter(status=draft, organisation=organisation).order_by(sort_by)
 
     def submitted(self, organisation, sort_by):
+        finalised = get_case_status_by_status(CaseStatusEnum.FINALISED)
         draft = get_case_status_by_status(CaseStatusEnum.DRAFT)
-        return self.get_queryset().filter(organisation=organisation).exclude(status=draft).order_by(sort_by)
+        return (
+            self.get_queryset()
+            .filter(organisation=organisation)
+            .exclude(Q(status=draft) | Q(status=finalised))
+            .order_by(sort_by)
+        )
 
     def finalised(self, organisation, sort_by):
         finalised = get_case_status_by_status(CaseStatusEnum.FINALISED)

api/applications/models.py

@@ -354,10 +354,9 @@ class StandardApplication(BaseApplication, Clonable):
     def clone(self, exclusions=None, **overrides):
         cloned_application = super().clone(exclusions=exclusions, **overrides)
 
-        # TODO: Figure out whether it is desirable to clone ApplicationDocument records
-        # application_documents = ApplicationDocument.objects.filter(application=self)
-        # for application_document in application_documents:
-        #    application_document.clone(application=cloned_application)
+        application_documents = ApplicationDocument.objects.filter(application=self, safe=True)
+        for application_document in application_documents:
+            application_document.clone(application=cloned_application)
 
         site_on_applications = SiteOnApplication.objects.filter(application=self)
         for site_on_application in site_on_applications:
@@ -390,7 +389,7 @@ def create_amendment(self, user):
             ignore_case_status=True,
         )
         system_user = BaseUser.objects.get(id=SystemUser.id)
-        self.case_ptr.change_status(system_user, get_case_status_by_status(CaseStatusEnum.SUPERSEDED_BY_AMENDMENT))
+        self.case_ptr.change_status(system_user, get_case_status_by_status(CaseStatusEnum.SUPERSEDED_BY_EXPORTER_EDIT))
         return amendment_application
 
 
@@ -596,14 +595,15 @@ def clone(self, exclusions=None, **overrides):
             cloned_good_on_application.firearm_details = self.firearm_details.clone()
             cloned_good_on_application.save()
 
-        good_on_application_documents = GoodOnApplicationDocument.objects.filter(good_on_application=self)
+        good_on_application_documents = GoodOnApplicationDocument.objects.filter(good_on_application=self, safe=True)
         for good_on_application_document in good_on_application_documents:
             good_on_application_document.clone(
                 good_on_application=cloned_good_on_application, application=overrides["application"]
             )
 
         good_on_application_internal_documents = GoodOnApplicationInternalDocument.objects.filter(
-            good_on_application=self
+            good_on_application=self,
+            safe=True,
         )
         for good_on_application_internal_document in good_on_application_internal_documents:
             good_on_application_internal_document.clone(good_on_application=cloned_good_on_application)
@@ -717,10 +717,14 @@ def delete(self, *args, **kwargs):
         "id",
         "application",
         "flags",
+        "party",
     ]
-    clone_mappings = {
-        "party": "party_id",
-    }
+
+    def clone(self, exclusions=None, **overrides):
+        if not overrides.get("party"):
+            cloned_party = self.party.clone()
+            overrides["party"] = cloned_party
+        return super().clone(exclusions=exclusions, **overrides)
 
 
 class DenialMatchOnApplication(TimestampableModel):

api/applications/tests/test_models.py

@@ -8,6 +8,7 @@
 from api.cases.models import CaseType, Queue
 from api.flags.models import Flag
 from api.applications.models import (
+    ApplicationDocument,
     GoodOnApplication,
     GoodOnApplicationDocument,
     GoodOnApplicationInternalDocument,
@@ -31,6 +32,7 @@
 from api.staticdata.control_list_entries.models import ControlListEntry
 from api.staticdata.report_summaries.models import ReportSummary, ReportSummaryPrefix, ReportSummarySubject
 from api.staticdata.statuses.models import CaseStatus, CaseSubStatus
+from api.staticdata.statuses.enums import CaseStatusEnum
 from api.users.models import ExporterUser
 
 
@@ -92,7 +94,7 @@ def test_create_amendment(self):
         assert amendment_application.status.status == "draft"
         assert amendment_application.amendment_of == original_application.case_ptr
         original_application.refresh_from_db()
-        assert original_application.status.status == "superseded_by_amendment"
+        assert original_application.status.status == CaseStatusEnum.SUPERSEDED_BY_EXPORTER_EDIT
         assert original_application.queues.all().count() == 0
         audit_entries = Audit.objects.all()
         supersede_audit_entry = audit_entries[1]
@@ -106,7 +108,9 @@ def test_create_amendment(self):
         assert amendment_audit_entry.verb == "exporter_created_amendment"
         assert amendment_audit_entry.actor == exporter_user
         status_change_audit_entry = audit_entries[0]
-        assert status_change_audit_entry.payload == {"status": {"new": "Superseded by amendment", "old": "ogd_advice"}}
+        assert status_change_audit_entry.payload == {
+            "status": {"new": "Superseded by exporter edit", "old": "ogd_advice"}
+        }
         assert status_change_audit_entry.verb == "updated_status"
 
     def test_clone(self):
@@ -170,6 +174,12 @@ def test_clone(self):
         original_site_on_application = SiteOnApplicationFactory(application=original_application)
         original_good_on_application = GoodOnApplicationFactory(application=original_application)
         original_party_on_application = PartyOnApplicationFactory(application=original_application)
+        original_application_safe_document = ApplicationDocumentFactory(
+            application=original_application, s3_key="some safe key", safe=True
+        )
+        original_application_unsafe_document = ApplicationDocumentFactory(
+            application=original_application, s3_key="some unsafe key", safe=False
+        )
         cloned_application = original_application.clone()
 
         assert cloned_application.id != original_application.id
@@ -240,6 +250,9 @@ def test_clone(self):
         assert SiteOnApplication.objects.filter(application=cloned_application).count() == 1
         assert GoodOnApplication.objects.filter(application=cloned_application).count() == 1
         assert PartyOnApplication.objects.filter(application=cloned_application).count() == 1
+        assert list(
+            ApplicationDocument.objects.filter(application=cloned_application).values_list("s3_key", flat=True)
+        ) == ["some safe key"]
 
 
 class TestApplicationDocument(DataTestClient):
@@ -351,21 +364,39 @@ def test_clone(self):
         original_good_on_application_internal_document = GoodOnApplicationInternalDocumentFactory(
             document_title="some title",
             name="some name",
-            s3_key="doc.xlsx",
+            s3_key="safe.xlsx",
             safe=True,
             size=100,
             good_on_application=original_good_on_application,
         )
+        original_good_on_application_internal_document_unsafe = GoodOnApplicationInternalDocumentFactory(
+            document_title="some title",
+            name="some name",
+            s3_key="unsafe.xlsx",
+            safe=False,
+            size=100,
+            good_on_application=original_good_on_application,
+        )
         original_good_on_application_document = GoodOnApplicationDocumentFactory(
             document_type="some type",
             name="some name",
-            s3_key="doc.xlsx",
+            s3_key="safe.xlsx",
             safe=True,
             size=100,
             good_on_application=original_good_on_application,
             good=original_good_on_application.good,
             application=original_good_on_application.application,
         )
+        original_good_on_application_document_unsafe = GoodOnApplicationDocumentFactory(
+            document_type="some type",
+            name="some name",
+            s3_key="unsafe.xlsx",
+            safe=False,
+            size=100,
+            good_on_application=original_good_on_application,
+            good=original_good_on_application.good,
+            application=original_good_on_application.application,
+        )
 
         new_application = StandardApplicationFactory()
         cloned_good_on_application = original_good_on_application.clone(application=new_application)
@@ -409,11 +440,16 @@ def test_clone(self):
         cloned by default or not and adjust GoodOnApplication.clone_* attributes accordingly.
         """
         # Defer checking of related models' clone() methods to specific unit tests
-        assert (
-            GoodOnApplicationInternalDocument.objects.filter(good_on_application=cloned_good_on_application).count()
-            == 1
-        )
-        assert GoodOnApplicationDocument.objects.filter(good_on_application=cloned_good_on_application).count() == 1
+        assert list(
+            GoodOnApplicationInternalDocument.objects.filter(
+                good_on_application=cloned_good_on_application
+            ).values_list("s3_key", flat=True)
+        ) == ["safe.xlsx"]
+        assert list(
+            GoodOnApplicationDocument.objects.filter(good_on_application=cloned_good_on_application).values_list(
+                "s3_key", flat=True
+            )
+        ) == ["safe.xlsx"]
 
 
 class TestGoodOnApplicationDocument(DataTestClient):
@@ -498,6 +534,32 @@ def test_clone(self):
         cloned_party_on_application = original_party_on_application.clone(application=new_application)
         assert cloned_party_on_application.id != original_party_on_application.id
         assert cloned_party_on_application.application_id == new_application.id
+        assert cloned_party_on_application.party_id != original_party_on_application.party_id
+        assert model_to_dict(cloned_party_on_application) == {
+            "id": cloned_party_on_application.id,
+            "application": new_application.id,
+            "deleted_at": original_party_on_application.deleted_at,
+            "flags": [],
+            "party": cloned_party_on_application.party_id,
+        }, """
+        The attributes on the cloned record were not as expected. If this is the result
+        of a schema migration, think carefully about whether the new fields should be
+        cloned by default or not and adjust PartyOnApplication.clone_*
+        attributes accordingly.
+        """
+
+    def test_clone_with_party_override(self):
+        original_party_on_application = PartyOnApplicationFactory(
+            deleted_at=timezone.now(),
+        )
+        original_party_on_application.flags.add(Flag.objects.first())
+        original_party_on_application.save()
+        new_application = StandardApplicationFactory()
+        cloned_party_on_application = original_party_on_application.clone(
+            application=new_application, party=original_party_on_application.party
+        )
+        assert cloned_party_on_application.id != original_party_on_application.id
+        assert cloned_party_on_application.application_id == new_application.id
         assert model_to_dict(cloned_party_on_application) == {
             "id": cloned_party_on_application.id,
             "application": new_application.id,

api/applications/tests/test_view_application.py

@@ -185,7 +185,7 @@ def test_view_applications_invalid_submitted_value(self):
         self.assertEqual(response.json()["count"], 0)
 
     def test_view_submitted_applications(self):
-        url = reverse("applications:applications") + "?submitted=true"
+        url = reverse("applications:applications") + "?sort_by=submitted_at&selected_filter=submitted_applications"
         response = self.client.get(url, **self.exporter_headers)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.json()["count"], 0)
@@ -194,7 +194,28 @@ def test_view_submitted_applications(self):
         application = self.create_draft_standard_application(self.organisation)
 
         self.submit_application(application)
-        url = reverse("applications:applications") + "?submitted=true"
+
+        application_finalised = self.create_standard_application_case(self.organisation)
+        FinalAdviceFactory(user=self.gov_user, case=application_finalised, type=AdviceType.APPROVE)
+        template = self.create_letter_template(
+            name="Template",
+            case_types=[CaseTypeEnum.SIEL.id],
+            decisions=[Decision.objects.get(name=AdviceType.APPROVE)],
+        )
+
+        self.gov_user.role.permissions.set([GovPermissions.MANAGE_LICENCE_FINAL_ADVICE.name])
+        licence = StandardLicenceFactory(case=application_finalised, status=LicenceStatus.DRAFT)
+        self.create_generated_case_document(
+            application_finalised, template, advice_type=AdviceType.APPROVE, licence=licence
+        )
+
+        finalised_url = reverse("cases:finalise", kwargs={"pk": application_finalised.id})
+        response = self.client.put(finalised_url, data={}, **self.gov_headers)
+
+        application_finalised.refresh_from_db()
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(application_finalised.status, CaseStatus.objects.get(status=CaseStatusEnum.FINALISED))
+
         response = self.client.get(url, **self.exporter_headers)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.json()["count"], 1)
@@ -254,7 +275,7 @@ def test_view_finalised_applications(self):
 
         response = self.client.get(url, **self.exporter_headers)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.json()["count"], 2)
+        self.assertEqual(response.json()["count"], 1)
 
         data = response.json()
         submitted_dates = [

api/applications/views/amendments.py

@@ -4,10 +4,12 @@
 from rest_framework import serializers
 from rest_framework import status
 
+from api.applications.libraries.get_applications import get_application
 from api.core.authentication import ExporterAuthentication
-from api.core.exceptions import NotFoundError
+from api.core.decorators import application_can_invoke_major_edit
+from api.core.exceptions import NotFoundError, BadRequestError
 from api.core.permissions import IsExporterInOrganisation
-from api.applications.libraries.get_applications import get_application
+from api.licences.models import Licence
 
 
 class AmendmentSerializer(serializers.Serializer):
@@ -29,15 +31,26 @@ def setup(self, request, *args, **kwargs):
             self.application = get_application(pk=self.kwargs["pk"])
         except (ObjectDoesNotExist, NotFoundError):
             raise Http404()
-        # TODO: More validation to prevent creating amendment of something we should not
 
     def get_organisation(self):
         return self.application.organisation
 
     def perform_create(self, serializer):
+        # Create a clone of the application in question and set the original application
+        # to a superseded status.  Amendment applications are new copies of the original
+        # which can be edited by exporters as if they were a completely new draft.
+        # Caseworker commentary is not copied to the amendment application but persists
+        # on the old superseded application
         self.amendment_application = self.application.create_amendment(self.request.user)
 
+    @application_can_invoke_major_edit
     def create(self, request, *args, **kwargs):
+        # At this stage we aren't totally sure how we should deal with applications
+        # that have licences being amended.  So raise a meaningful error when this is attempted.
+        application_has_licence = Licence.objects.filter_non_draft_licences(application=self.application).count() > 0
+        if application_has_licence:
+            raise BadRequestError({"non_field_errors": "Application has at least one licence so cannot be amended."})
+
         super().create(request, *args, **kwargs)
         return JsonResponse(
             {

api/applications/views/tests/test_amendments.py

@@ -1,11 +1,17 @@
+from parameterized import parameterized
 from unittest import mock
+
 from django.urls import reverse
 from rest_framework import status
 
 from api.applications import models as application_models
 from api.applications.models import StandardApplication
 from api.applications.tests.factories import StandardApplicationFactory, GoodOnApplicationFactory
+from api.licences.tests.factories import StandardLicenceFactory
+from api.licences.enums import LicenceStatus
 from api.organisations.tests.factories import OrganisationFactory
+from api.staticdata.statuses.enums import CaseStatusEnum
+from api.staticdata.statuses.models import CaseStatus
 
 from test_helpers.clients import DataTestClient
 
@@ -23,15 +29,18 @@ def setUp(self):
             },
         )
 
-    def test_create_amendment(self):
+    @parameterized.expand(CaseStatusEnum.can_invoke_major_edit_statuses)
+    def test_create_amendment(self, case_status):
+        self.application.status = CaseStatus.objects.get(status=case_status)
+        self.application.save()
         response = self.client.post(self.url, **self.exporter_headers)
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         amendment_id = response.json().get("id")
         amendment_application = StandardApplication.objects.get(id=amendment_id)
         self.assertEqual(amendment_application.name, self.application.name)
         self.assertEqual(amendment_application.amendment_of_id, self.application.id)
         self.application.refresh_from_db()
-        self.assertEqual(self.application.status.status, "superseded_by_amendment")
+        self.assertEqual(self.application.status.status, CaseStatusEnum.SUPERSEDED_BY_EXPORTER_EDIT)
 
     @mock.patch.object(application_models.GoodOnApplication, "clone")
     def test_create_amendment_partial_failure(self, mocked_good_on_application_clone):
@@ -51,3 +60,18 @@ def test_create_amendment_application_does_not_exist(self):
         self.application.delete()
         response = self.client.post(self.url, **self.exporter_headers)
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    @parameterized.expand(CaseStatusEnum.can_not_invoke_major_edit_statuses)
+    def test_create_amendment_application_wrong_status(self, case_status):
+        self.application.status = CaseStatus.objects.get(status=case_status)
+        self.application.save()
+        response = self.client.post(self.url, **self.exporter_headers)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+    def test_create_amendment_licence_exists_on_original(self):
+        licence = StandardLicenceFactory(case=self.application.case_ptr, status=LicenceStatus.ISSUED)
+        response = self.client.post(self.url, **self.exporter_headers)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.json()["errors"]["non_field_errors"], "Application has at least one licence so cannot be amended."
+        )