From 791400866338c74259c37b1852369d779734f98a Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 29 Aug 2024 19:00:13 +0100 Subject: [PATCH 01/11] Create separate CLEs list view with ExporterAuthentication only --- .../control_list_entries/tests/test_views.py | 25 +++++++++++++++++++ api/staticdata/control_list_entries/urls.py | 1 + api/staticdata/control_list_entries/views.py | 18 ++++++++++++- 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 api/staticdata/control_list_entries/tests/test_views.py diff --git a/api/staticdata/control_list_entries/tests/test_views.py b/api/staticdata/control_list_entries/tests/test_views.py new file mode 100644 index 0000000000..fe74c82b0f --- /dev/null +++ b/api/staticdata/control_list_entries/tests/test_views.py @@ -0,0 +1,25 @@ +from rest_framework import status +from rest_framework.reverse import reverse + +from test_helpers.clients import DataTestClient + + +class ExporterControlListEntriesListTests(DataTestClient): + def setUp(self): + self.url = reverse("staticdata:control_list_entries:exporter_list") + super().setUp() + + def test_exporter_list_view_success(self): + response = self.client.get(self.url, **self.exporter_headers) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.assertEqual(list(response.json().keys()), ["control_list_entries"]) + + for cle in response.json()["control_list_entries"]: + self.assertEqual(list(cle.keys()), ["rating", "text"]) + + def test_exporter_list_view_failure_bad_headers(self): + response = self.client.get(self.url, **self.gov_headers) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/api/staticdata/control_list_entries/urls.py b/api/staticdata/control_list_entries/urls.py index c8be414141..f24655102b 100755 --- a/api/staticdata/control_list_entries/urls.py +++ b/api/staticdata/control_list_entries/urls.py @@ -6,6 +6,7 @@ urlpatterns = [ path("", views.ControlListEntriesList.as_view(), name="control_list_entries"), + path("exporter-list/", views.ExporterControlListEntriesList.as_view(), name="exporter_list"), path( "/", views.ControlListEntryDetail.as_view(), diff --git a/api/staticdata/control_list_entries/views.py b/api/staticdata/control_list_entries/views.py index b19d86171d..6de02881ce 100755 --- a/api/staticdata/control_list_entries/views.py +++ b/api/staticdata/control_list_entries/views.py @@ -3,7 +3,7 @@ from rest_framework.decorators import permission_classes from rest_framework.views import APIView -from api.core.authentication import SharedAuthentication +from api.core.authentication import SharedAuthentication, ExporterAuthentication from api.staticdata.control_list_entries.helpers import get_control_list_entry, convert_control_list_entries_to_tree from api.staticdata.control_list_entries.models import ControlListEntry from api.staticdata.control_list_entries.serializers import ControlListEntrySerializerWithLinks @@ -42,3 +42,19 @@ def get(self, request, rating): control_list_entry = get_control_list_entry(rating) serializer = ControlListEntrySerializerWithLinks(control_list_entry) return JsonResponse(data={"control_list_entry": serializer.data}) + + +class ExporterControlListEntriesList(APIView): + authentication_classes = (ExporterAuthentication,) + + def get_queryset(self): + return ControlListEntry.objects.filter(controlled=True) + + def get(self, request): + """ + Returns list of all Control List Entries + """ + + queryset = self.get_queryset() + + return JsonResponse(data={"control_list_entries": list(queryset.values("rating", "text"))}) From ad0dde73df39507d7060120e524246581d7467e4 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Fri, 30 Aug 2024 15:00:25 +0100 Subject: [PATCH 02/11] Refactor to use generic ListAPIView --- .../control_list_entries/serializers.py | 6 +++++ .../control_list_entries/tests/test_views.py | 4 +-- api/staticdata/control_list_entries/views.py | 25 ++++++++----------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/api/staticdata/control_list_entries/serializers.py b/api/staticdata/control_list_entries/serializers.py index b79d4f5ed1..51ad0dbd75 100755 --- a/api/staticdata/control_list_entries/serializers.py +++ b/api/staticdata/control_list_entries/serializers.py @@ -23,3 +23,9 @@ class ControlListEntriesListSerializer(serializers.ModelSerializer): class Meta: model = ControlListEntry fields = "__all__" + + +class ExporterControlListEntriesListSerializer(serializers.ModelSerializer): + class Meta: + model = ControlListEntry + fields = ("rating", "text") diff --git a/api/staticdata/control_list_entries/tests/test_views.py b/api/staticdata/control_list_entries/tests/test_views.py index fe74c82b0f..d09fe0d34e 100644 --- a/api/staticdata/control_list_entries/tests/test_views.py +++ b/api/staticdata/control_list_entries/tests/test_views.py @@ -14,9 +14,7 @@ def test_exporter_list_view_success(self): self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(list(response.json().keys()), ["control_list_entries"]) - - for cle in response.json()["control_list_entries"]: + for cle in response.json(): self.assertEqual(list(cle.keys()), ["rating", "text"]) def test_exporter_list_view_failure_bad_headers(self): diff --git a/api/staticdata/control_list_entries/views.py b/api/staticdata/control_list_entries/views.py index 6de02881ce..cbf20c12ae 100755 --- a/api/staticdata/control_list_entries/views.py +++ b/api/staticdata/control_list_entries/views.py @@ -1,12 +1,15 @@ from django.http import JsonResponse -from rest_framework import permissions +from rest_framework import generics, permissions from rest_framework.decorators import permission_classes from rest_framework.views import APIView from api.core.authentication import SharedAuthentication, ExporterAuthentication from api.staticdata.control_list_entries.helpers import get_control_list_entry, convert_control_list_entries_to_tree from api.staticdata.control_list_entries.models import ControlListEntry -from api.staticdata.control_list_entries.serializers import ControlListEntrySerializerWithLinks +from api.staticdata.control_list_entries.serializers import ( + ControlListEntrySerializerWithLinks, + ExporterControlListEntriesListSerializer, +) @permission_classes((permissions.AllowAny,)) @@ -44,17 +47,9 @@ def get(self, request, rating): return JsonResponse(data={"control_list_entry": serializer.data}) -class ExporterControlListEntriesList(APIView): +class ExporterControlListEntriesList(generics.ListAPIView): authentication_classes = (ExporterAuthentication,) - - def get_queryset(self): - return ControlListEntry.objects.filter(controlled=True) - - def get(self, request): - """ - Returns list of all Control List Entries - """ - - queryset = self.get_queryset() - - return JsonResponse(data={"control_list_entries": list(queryset.values("rating", "text"))}) + model = ControlListEntry + pagination_class = None + serializer_class = ExporterControlListEntriesListSerializer + queryset = ControlListEntry.objects.all() From 7bfb5f04d015b7b52c6d7fe3b8b144f2d5d1a778 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Fri, 30 Aug 2024 15:03:47 +0100 Subject: [PATCH 03/11] Fix queryset --- api/staticdata/control_list_entries/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/staticdata/control_list_entries/views.py b/api/staticdata/control_list_entries/views.py index cbf20c12ae..9458a4800b 100755 --- a/api/staticdata/control_list_entries/views.py +++ b/api/staticdata/control_list_entries/views.py @@ -52,4 +52,4 @@ class ExporterControlListEntriesList(generics.ListAPIView): model = ControlListEntry pagination_class = None serializer_class = ExporterControlListEntriesListSerializer - queryset = ControlListEntry.objects.all() + queryset = ControlListEntry.objects.filter(controlled=True) From 93b2fbcc69f2d489c1d0fcb72070aade6f1a1b76 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 3 Sep 2024 12:22:35 +0100 Subject: [PATCH 04/11] Refactor directory structure --- api/conf/exporter_urls.py | 1 + .../control_list_entries/serializers.py | 6 ------ api/staticdata/control_list_entries/urls.py | 1 - api/staticdata/control_list_entries/views.py | 17 +++-------------- .../control_list_entries/serializers.py | 9 +++++++++ .../control_list_entries/tests/test_views.py | 8 ++++---- .../exporter/control_list_entries/urls.py | 7 +++++++ .../exporter/control_list_entries/views.py | 13 +++++++++++++ api/staticdata/exporter/urls.py | 7 +++++++ 9 files changed, 44 insertions(+), 25 deletions(-) create mode 100644 api/staticdata/exporter/control_list_entries/serializers.py rename api/staticdata/{ => exporter}/control_list_entries/tests/test_views.py (69%) create mode 100644 api/staticdata/exporter/control_list_entries/urls.py create mode 100644 api/staticdata/exporter/control_list_entries/views.py create mode 100644 api/staticdata/exporter/urls.py diff --git a/api/conf/exporter_urls.py b/api/conf/exporter_urls.py index d14a5d0b5e..c95eee8a7b 100644 --- a/api/conf/exporter_urls.py +++ b/api/conf/exporter_urls.py @@ -2,4 +2,5 @@ urlpatterns = [ path("applications/", include("api.applications.exporter.urls")), + path("static/", include("api.staticdata.exporter.urls")), ] diff --git a/api/staticdata/control_list_entries/serializers.py b/api/staticdata/control_list_entries/serializers.py index 51ad0dbd75..b79d4f5ed1 100755 --- a/api/staticdata/control_list_entries/serializers.py +++ b/api/staticdata/control_list_entries/serializers.py @@ -23,9 +23,3 @@ class ControlListEntriesListSerializer(serializers.ModelSerializer): class Meta: model = ControlListEntry fields = "__all__" - - -class ExporterControlListEntriesListSerializer(serializers.ModelSerializer): - class Meta: - model = ControlListEntry - fields = ("rating", "text") diff --git a/api/staticdata/control_list_entries/urls.py b/api/staticdata/control_list_entries/urls.py index f24655102b..c8be414141 100755 --- a/api/staticdata/control_list_entries/urls.py +++ b/api/staticdata/control_list_entries/urls.py @@ -6,7 +6,6 @@ urlpatterns = [ path("", views.ControlListEntriesList.as_view(), name="control_list_entries"), - path("exporter-list/", views.ExporterControlListEntriesList.as_view(), name="exporter_list"), path( "/", views.ControlListEntryDetail.as_view(), diff --git a/api/staticdata/control_list_entries/views.py b/api/staticdata/control_list_entries/views.py index 9458a4800b..b19d86171d 100755 --- a/api/staticdata/control_list_entries/views.py +++ b/api/staticdata/control_list_entries/views.py @@ -1,15 +1,12 @@ from django.http import JsonResponse -from rest_framework import generics, permissions +from rest_framework import permissions from rest_framework.decorators import permission_classes from rest_framework.views import APIView -from api.core.authentication import SharedAuthentication, ExporterAuthentication +from api.core.authentication import SharedAuthentication from api.staticdata.control_list_entries.helpers import get_control_list_entry, convert_control_list_entries_to_tree from api.staticdata.control_list_entries.models import ControlListEntry -from api.staticdata.control_list_entries.serializers import ( - ControlListEntrySerializerWithLinks, - ExporterControlListEntriesListSerializer, -) +from api.staticdata.control_list_entries.serializers import ControlListEntrySerializerWithLinks @permission_classes((permissions.AllowAny,)) @@ -45,11 +42,3 @@ def get(self, request, rating): control_list_entry = get_control_list_entry(rating) serializer = ControlListEntrySerializerWithLinks(control_list_entry) return JsonResponse(data={"control_list_entry": serializer.data}) - - -class ExporterControlListEntriesList(generics.ListAPIView): - authentication_classes = (ExporterAuthentication,) - model = ControlListEntry - pagination_class = None - serializer_class = ExporterControlListEntriesListSerializer - queryset = ControlListEntry.objects.filter(controlled=True) diff --git a/api/staticdata/exporter/control_list_entries/serializers.py b/api/staticdata/exporter/control_list_entries/serializers.py new file mode 100644 index 0000000000..a16605bcd6 --- /dev/null +++ b/api/staticdata/exporter/control_list_entries/serializers.py @@ -0,0 +1,9 @@ +from rest_framework import serializers + +from api.staticdata.control_list_entries.models import ControlListEntry + + +class ControlListEntriesListSerializer(serializers.ModelSerializer): + class Meta: + model = ControlListEntry + fields = ("rating", "text") diff --git a/api/staticdata/control_list_entries/tests/test_views.py b/api/staticdata/exporter/control_list_entries/tests/test_views.py similarity index 69% rename from api/staticdata/control_list_entries/tests/test_views.py rename to api/staticdata/exporter/control_list_entries/tests/test_views.py index d09fe0d34e..5805e0663f 100644 --- a/api/staticdata/control_list_entries/tests/test_views.py +++ b/api/staticdata/exporter/control_list_entries/tests/test_views.py @@ -4,12 +4,12 @@ from test_helpers.clients import DataTestClient -class ExporterControlListEntriesListTests(DataTestClient): +class ControlListEntriesListTests(DataTestClient): def setUp(self): - self.url = reverse("staticdata:control_list_entries:exporter_list") + self.url = reverse("exporter_staticdata:control_list_entries:control_list_entries") super().setUp() - def test_exporter_list_view_success(self): + def test_list_view_success(self): response = self.client.get(self.url, **self.exporter_headers) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -17,7 +17,7 @@ def test_exporter_list_view_success(self): for cle in response.json(): self.assertEqual(list(cle.keys()), ["rating", "text"]) - def test_exporter_list_view_failure_bad_headers(self): + def test_list_view_failure_bad_headers(self): response = self.client.get(self.url, **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/api/staticdata/exporter/control_list_entries/urls.py b/api/staticdata/exporter/control_list_entries/urls.py new file mode 100644 index 0000000000..79b02cf0cc --- /dev/null +++ b/api/staticdata/exporter/control_list_entries/urls.py @@ -0,0 +1,7 @@ +from django.urls import path + +from api.staticdata.exporter.control_list_entries import views + +app_name = "control_list_entries" + +urlpatterns = [path("", views.ControlListEntriesList.as_view(), name="control_list_entries")] diff --git a/api/staticdata/exporter/control_list_entries/views.py b/api/staticdata/exporter/control_list_entries/views.py new file mode 100644 index 0000000000..5643f48855 --- /dev/null +++ b/api/staticdata/exporter/control_list_entries/views.py @@ -0,0 +1,13 @@ +from rest_framework import generics + +from api.core.authentication import ExporterAuthentication +from api.staticdata.control_list_entries.models import ControlListEntry +from api.staticdata.exporter.control_list_entries.serializers import ControlListEntriesListSerializer + + +class ControlListEntriesList(generics.ListAPIView): + authentication_classes = (ExporterAuthentication,) + model = ControlListEntry + pagination_class = None + serializer_class = ControlListEntriesListSerializer + queryset = ControlListEntry.objects.filter(controlled=True) diff --git a/api/staticdata/exporter/urls.py b/api/staticdata/exporter/urls.py new file mode 100644 index 0000000000..0cddab9611 --- /dev/null +++ b/api/staticdata/exporter/urls.py @@ -0,0 +1,7 @@ +from django.urls import path, include + +app_name = "exporter_staticdata" + +urlpatterns = [ + path("control-list-entries/", include("api.staticdata.exporter.control_list_entries.urls")), +] From dde03e8b46dead5aa65ee383fd836a70c19f5d11 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 3 Sep 2024 16:25:28 +0100 Subject: [PATCH 05/11] sort returned documents by data created instead of expiry date --- api/organisations/serializers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index a88d21c999..6d5469c423 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -388,9 +388,7 @@ def get_flags(self, instance): return list(instance.flags.values("id", "name", "colour", "label", "priority")) def get_documents(self, instance): - queryset = instance.document_on_organisations.order_by("document_type", "-expiry_date").distinct( - "document_type" - ) + queryset = instance.document_on_organisations.order_by("document_type", "-created_at").distinct("document_type") serializer = DocumentOnOrganisationSerializer(queryset, many=True) return serializer.data From f4eafa126c6c3a38de85d6674bc4d3f810f131a3 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 3 Sep 2024 16:25:38 +0100 Subject: [PATCH 06/11] Add test for exact response --- .../control_list_entries/tests/test_views.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/api/staticdata/exporter/control_list_entries/tests/test_views.py b/api/staticdata/exporter/control_list_entries/tests/test_views.py index 5805e0663f..ef7770e185 100644 --- a/api/staticdata/exporter/control_list_entries/tests/test_views.py +++ b/api/staticdata/exporter/control_list_entries/tests/test_views.py @@ -1,6 +1,8 @@ from rest_framework import status from rest_framework.reverse import reverse +from api.staticdata.control_list_entries.models import ControlListEntry +from api.staticdata.control_list_entries.factories import ControlListEntriesFactory from test_helpers.clients import DataTestClient @@ -14,6 +16,8 @@ def test_list_view_success(self): self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertTrue(len(response.json()) > 0) + for cle in response.json(): self.assertEqual(list(cle.keys()), ["rating", "text"]) @@ -21,3 +25,24 @@ def test_list_view_failure_bad_headers(self): response = self.client.get(self.url, **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_list_view_success_exact_response(self): + # Set up empty CLE db table for this test only + ControlListEntry.objects.all().delete() + + cle_1 = ControlListEntriesFactory(rating="ABC123", controlled=True) + cle_2 = ControlListEntriesFactory(rating="1Z101", controlled=True) + cle_3 = ControlListEntriesFactory(rating="ZXYW", controlled=True) + + response = self.client.get(self.url, **self.exporter_headers) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.assertEqual( + response.json(), + [ + {"rating": cle_1.rating, "text": cle_1.text}, + {"rating": cle_2.rating, "text": cle_2.text}, + {"rating": cle_3.rating, "text": cle_3.text}, + ], + ) From 849c9a414c09707a1704edda5e82d94ccf89b3ef Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 3 Sep 2024 16:49:07 +0100 Subject: [PATCH 07/11] Reformat list --- api/staticdata/exporter/control_list_entries/urls.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/staticdata/exporter/control_list_entries/urls.py b/api/staticdata/exporter/control_list_entries/urls.py index 79b02cf0cc..1e39f4ae63 100644 --- a/api/staticdata/exporter/control_list_entries/urls.py +++ b/api/staticdata/exporter/control_list_entries/urls.py @@ -4,4 +4,6 @@ app_name = "control_list_entries" -urlpatterns = [path("", views.ControlListEntriesList.as_view(), name="control_list_entries")] +urlpatterns = [ + path("", views.ControlListEntriesList.as_view(), name="control_list_entries"), +] From 70402816e9c78a814448e67c671498a5d90d5ba2 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 3 Sep 2024 16:50:16 +0100 Subject: [PATCH 08/11] Remove model assignment as not needed --- api/staticdata/exporter/control_list_entries/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/staticdata/exporter/control_list_entries/views.py b/api/staticdata/exporter/control_list_entries/views.py index 5643f48855..d3097850a7 100644 --- a/api/staticdata/exporter/control_list_entries/views.py +++ b/api/staticdata/exporter/control_list_entries/views.py @@ -7,7 +7,6 @@ class ControlListEntriesList(generics.ListAPIView): authentication_classes = (ExporterAuthentication,) - model = ControlListEntry pagination_class = None serializer_class = ControlListEntriesListSerializer queryset = ControlListEntry.objects.filter(controlled=True) From 5a9ddb702aa1511b46809419377f585eb82b2a48 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Wed, 4 Sep 2024 10:26:35 +0100 Subject: [PATCH 09/11] changed tests to use dynamic future dates --- api/organisations/tests/test_documents.py | 38 +++++++++++++++-------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/api/organisations/tests/test_documents.py b/api/organisations/tests/test_documents.py index 0908215396..7bd00ff2f0 100644 --- a/api/organisations/tests/test_documents.py +++ b/api/organisations/tests/test_documents.py @@ -1,4 +1,4 @@ -import datetime +from datetime import datetime, timedelta from unittest import mock @@ -29,9 +29,12 @@ def test_create_organisation_document(self, mock_virus_scan, mock_s3_operations_ mock_virus_scan.return_value = False url = reverse("organisations:documents", kwargs={"pk": self.organisation.pk}) + + future_date = datetime.now() + timedelta(days=365) + future_date_formatted = future_date.strftime("%Y-%m-%d") data = { "document": {"name": "some-document", "s3_key": "some-document", "size": 476}, - "expiry_date": "2026-01-01", + "expiry_date": future_date_formatted, "reference_code": "123", "document_type": OrganisationDocumentType.FIREARM_SECTION_FIVE, } @@ -46,7 +49,8 @@ def test_create_organisation_document(self, mock_virus_scan, mock_s3_operations_ self.assertEqual(instance.document.s3_key, "some-document") self.assertEqual(instance.reference_code, "123") self.assertEqual(instance.document.size, 476) - self.assertEqual(instance.expiry_date, datetime.date(2026, 1, 1)) + + self.assertEqual(instance.expiry_date.strftime("%Y-%m-%d"), future_date_formatted) self.assertEqual(instance.document_type, OrganisationDocumentType.FIREARM_SECTION_FIVE) self.assertEqual(instance.organisation, self.organisation) @@ -58,9 +62,10 @@ def test_create_organisation_document_other_organisation(self, mock_virus_scan, other_organisation, _ = self.create_organisation_with_exporter_user() url = reverse("organisations:documents", kwargs={"pk": other_organisation.pk}) + future_date = datetime.now() + timedelta(days=365) data = { "document": {"name": "some-document", "s3_key": "some-document", "size": 476}, - "expiry_date": "2026-01-01", + "expiry_date": future_date, "reference_code": "123", "document_type": OrganisationDocumentType.FIREARM_SECTION_FIVE, } @@ -74,18 +79,21 @@ def test_list_organisation_documents(self): document__s3_key="thisisakey", document__safe=True, organisation=self.organisation, + expiry_date=datetime.now() + timedelta(days=4 * 365), ) DocumentOnOrganisationFactory.create( document__name="some-document-two", document__s3_key="thisisakey", document__safe=True, organisation=self.organisation, + expiry_date=datetime.now() + timedelta(days=3 * 365), ) DocumentOnOrganisationFactory.create( document__name="some-document-three", document__s3_key="thisisakey", document__safe=True, organisation=self.organisation, + expiry_date=datetime.now() + timedelta(days=2 * 365), ) other_organisation, _ = self.create_organisation_with_exporter_user() DocumentOnOrganisationFactory.create( @@ -93,6 +101,7 @@ def test_list_organisation_documents(self): document__s3_key="thisisakey", document__safe=True, organisation=other_organisation, + expiry_date=datetime.now() + timedelta(days=365), ) url = reverse("organisations:documents", kwargs={"pk": self.organisation.pk}) @@ -111,9 +120,11 @@ def test_list_organisation_documents_other_organisation(self): self.assertEqual(response.status_code, 403) def test_retrieve_organisation_documents(self): + future_date = datetime.now() + timedelta(days=365) + future_date_formatted = future_date.strftime("%d %B %Y") document_on_application = DocumentOnOrganisationFactory.create( organisation=self.organisation, - expiry_date=datetime.date(2026, 1, 1), + expiry_date=future_date, document_type=OrganisationDocumentType.FIREARM_SECTION_FIVE, reference_code="123", document__name="some-document-one", @@ -128,14 +139,13 @@ def test_retrieve_organisation_documents(self): ) response = self.client.get(url, **self.exporter_headers) - self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), { "id": str(document_on_application.pk), - "expiry_date": "01 January 2026", + "expiry_date": future_date_formatted, "document_type": "section-five-certificate", "organisation": str(self.organisation.id), "is_expired": False, @@ -155,7 +165,7 @@ def test_retrieve_organisation_documents_invalid_organisation(self): other_organisation, _ = self.create_organisation_with_exporter_user() document_on_application = DocumentOnOrganisationFactory.create( organisation=other_organisation, - expiry_date=datetime.date(2026, 1, 1), + expiry_date=datetime.now() + timedelta(days=365), document_type=OrganisationDocumentType.FIREARM_SECTION_FIVE, reference_code="123", document__name="some-document-one", @@ -215,11 +225,12 @@ def test_update_organisation_documents(self): "document_on_application_pk": document_on_application.pk, }, ) - + future_date = datetime.now() + timedelta(days=365) + future_date_formatted = future_date.strftime("%Y-%m-%d") response = self.client.put( url, data={ - "expiry_date": "2030-12-12", + "expiry_date": future_date_formatted, "reference_code": "567", }, **self.exporter_headers, @@ -228,8 +239,8 @@ def test_update_organisation_documents(self): document_on_application.refresh_from_db() self.assertEqual( - document_on_application.expiry_date, - datetime.date(2030, 12, 12), + document_on_application.expiry_date.strftime("%Y-%m-%d"), + future_date_formatted, ) self.assertEqual( document_on_application.reference_code, @@ -240,6 +251,7 @@ def test_update_organisation_documents_other_organisation(self): other_organisation, _ = self.create_organisation_with_exporter_user() document_on_application = DocumentOnOrganisationFactory.create(organisation=other_organisation) + future_date = datetime.now() + timedelta(days=365) url = reverse( "organisations:documents", kwargs={ @@ -251,7 +263,7 @@ def test_update_organisation_documents_other_organisation(self): response = self.client.put( url, data={ - "expiry_date": "2030-12-12", + "expiry_date": future_date, "reference_code": "567", }, **self.exporter_headers, From c2a8b1fcc11a795fe9f16beb6451159170877677 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Wed, 4 Sep 2024 11:02:13 +0100 Subject: [PATCH 10/11] add test to make sure last created doc is presented to organisation details --- api/organisations/tests/test_documents.py | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/api/organisations/tests/test_documents.py b/api/organisations/tests/test_documents.py index 7bd00ff2f0..dd231f39b7 100644 --- a/api/organisations/tests/test_documents.py +++ b/api/organisations/tests/test_documents.py @@ -161,6 +161,41 @@ def test_retrieve_organisation_documents(self): }, ) + def test_retrieve_documents_on_organisation_details(self): + # We create 2 documents on the organisation + + DocumentOnOrganisationFactory.create( + organisation=self.organisation, + expiry_date=datetime.now() + timedelta(days=365), + document_type=OrganisationDocumentType.REGISTERED_FIREARM_DEALER_CERTIFICATE, + reference_code="123", + document__name="some-document-one", + document__s3_key="some-document-one", + document__size=476, + document__safe=True, + created_at=datetime.now() - timedelta(hours=1), + ) + DocumentOnOrganisationFactory.create( + organisation=self.organisation, + expiry_date=datetime.now() + timedelta(days=365), + document_type=OrganisationDocumentType.REGISTERED_FIREARM_DEALER_CERTIFICATE, + reference_code="321", + document__name="some-document-two", + document__s3_key="some-document-two", + document__size=476, + document__safe=True, + created_at=datetime.now(), + ) + + url = reverse("organisations:organisation", kwargs={"pk": self.organisation.pk}) + + response = self.client.get(url, **self.exporter_headers) + + # We check that the first document delivered to organisation details is the most recently created + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["documents"][0]["document"]["name"], "some-document-two") + def test_retrieve_organisation_documents_invalid_organisation(self): other_organisation, _ = self.create_organisation_with_exporter_user() document_on_application = DocumentOnOrganisationFactory.create( From 5ffef4da8a26e8bcdced7bd0c3d4e63c98f5be0c Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Tue, 3 Sep 2024 15:24:08 +0100 Subject: [PATCH 11/11] add new cmd to delete all final advice --- api/applications/tests/factories.py | 28 +++++- .../commands/remove_case_final_advice.py | 85 +++++++++++++++++ .../tests/test_remove_case_final_advice.py | 91 +++++++++++++++++++ 3 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 api/support/management/commands/remove_case_final_advice.py create mode 100644 api/support/management/commands/tests/test_remove_case_final_advice.py diff --git a/api/applications/tests/factories.py b/api/applications/tests/factories.py index 90463f10eb..43423bafb3 100644 --- a/api/applications/tests/factories.py +++ b/api/applications/tests/factories.py @@ -1,5 +1,5 @@ import factory - +import factory.fuzzy from faker import Faker from api.applications.enums import ApplicationExportType, ApplicationExportLicenceOfficialType @@ -14,7 +14,8 @@ GoodOnApplicationInternalDocument, StandardApplication, ) -from api.cases.enums import CaseTypeEnum +from api.cases.enums import AdviceLevel, AdviceType, CaseTypeEnum +from api.cases.models import Advice from api.external_data.models import Denial, DenialEntity, SanctionMatch from api.documents.tests.factories import DocumentFactory from api.staticdata.statuses.models import CaseStatus @@ -240,3 +241,26 @@ def _create(cls, model_class, *args, **kwargs): PartyOnApplicationFactory(application=obj, party=ThirdPartyFactory(organisation=obj.organisation)) return obj + + +class AdviceFactory(factory.django.DjangoModelFactory): + user = factory.SubFactory(GovUserFactory) + case = factory.SubFactory(StandardApplicationFactory) + type = factory.fuzzy.FuzzyChoice(AdviceType.choices, getter=lambda t: t[0]) + level = factory.fuzzy.FuzzyChoice(AdviceLevel.choices, getter=lambda t: t[0]) + + class Meta: + model = Advice + + +class FinalAdviceOnApplicationFactory(StandardApplicationFactory): + + @classmethod + def _create(cls, model_class, *args, **kwargs): + obj = model_class(*args, **kwargs) + obj.status = get_case_status_by_status(CaseStatusEnum.UNDER_FINAL_REVIEW) + obj.save() + + AdviceFactory(case=obj, level=AdviceLevel.FINAL) + + return obj diff --git a/api/support/management/commands/remove_case_final_advice.py b/api/support/management/commands/remove_case_final_advice.py new file mode 100644 index 0000000000..d0404b909c --- /dev/null +++ b/api/support/management/commands/remove_case_final_advice.py @@ -0,0 +1,85 @@ +import logging + +from django.core.management.base import BaseCommand, CommandError +from django.db import transaction + +from api.audit_trail.enums import AuditType +from api.cases.enums import AdviceLevel +from api.cases.models import Case +from api.queues.models import Queue +from api.audit_trail import service as audit_trail_service + +from api.staticdata.statuses.enums import CaseStatusEnum +from api.staticdata.statuses.libraries.get_case_status import get_case_status_by_status +from lite_routing.routing_rules_internal.enums import QueuesEnum + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = """ + Command to delete Final advice when a case is ready to finalise + + When an application goes back to TAU and they change their mind we need to + invalidate all the final advice. This is unsed where cases are ready to proceed + with finial review. i.e a refusal. + """ + + def add_arguments(self, parser): + parser.add_argument( + "--case_reference", + type=str, + nargs="?", + help="Reference code of the Case (eg GBSIEL/2023/0000001/P)", + ) + parser.add_argument("--dry_run", help="Is it a test run?", action="store_true") + + def handle(self, *args, **options): + case_reference = options.pop("case_reference") + dry_run = options.pop("dry_run") + + with transaction.atomic(): + try: + case = Case.objects.get(reference_code=case_reference) + except Case.DoesNotExist as e: + logger.error("Invalid Case reference %s, does not exist", case_reference) + raise CommandError(e) + + final_advice = case.advice.filter(level=AdviceLevel.FINAL) + + # Ensure final advice given by LU + if not final_advice.exists(): + logger.error("Invalid Advice data, no final advice on this case") + raise CommandError(Exception("Invalid Advice data, no final advice on this case")) + + if not dry_run: + # Move the Case to 'LU Post-Circulation Cases to Finalise' queue + # as it needs to be in this queue to finalise and issue + # also need to ensure the status is under final review. + case.queues.set(Queue.objects.filter(id=QueuesEnum.LU_POST_CIRC)) + + for item in final_advice: + item.delete() + + audit_trail_service.create_system_user_audit( + verb=AuditType.DEVELOPER_INTERVENTION, + target=case, + payload={ + "additional_text": "Removed final advice.", + }, + ) + # If the case isn't under final review update the status + if case.status.status != CaseStatusEnum.UNDER_FINAL_REVIEW: + prev_case_status = case.status.status + case.status = get_case_status_by_status(CaseStatusEnum.UNDER_FINAL_REVIEW) + case.save() + audit_trail_service.create_system_user_audit( + verb=AuditType.UPDATED_STATUS, + target=case, + payload={ + "status": {"new": case.status.status, "old": prev_case_status}, + "additional_text": "", + }, + ) + + logging.info("[%s] can now be finalised by LU to issue a licence", case_reference) diff --git a/api/support/management/commands/tests/test_remove_case_final_advice.py b/api/support/management/commands/tests/test_remove_case_final_advice.py new file mode 100644 index 0000000000..7f811936c4 --- /dev/null +++ b/api/support/management/commands/tests/test_remove_case_final_advice.py @@ -0,0 +1,91 @@ +from api.staticdata.statuses.libraries.get_case_status import get_case_status_by_status +import pytest + +from django.core.management import call_command, CommandError + +from api.applications.tests.factories import AdviceFactory, FinalAdviceOnApplicationFactory +from api.audit_trail.enums import AuditType +from api.audit_trail.models import Audit +from api.cases.enums import AdviceLevel +from api.staticdata.statuses.enums import CaseStatusEnum +from test_helpers.clients import DataTestClient + + +class RemoveCaseFinalAdviceMgmtCommandTests(DataTestClient): + + def setUp(self): + super().setUp() + self.application = FinalAdviceOnApplicationFactory() + self.application.advice.set([AdviceFactory(level=AdviceLevel.USER)]) + + def test_remove_case_final_advice_command(self): + + self.application.status = get_case_status_by_status(CaseStatusEnum.SUBMITTED) + self.application.save() + + self.assertEqual(self.application.advice.filter(level=AdviceLevel.FINAL).exists(), True) + self.assertEqual(self.application.advice.all().count(), 2) + + call_command("remove_case_final_advice", case_reference=self.application.reference_code) + + self.application.refresh_from_db() + self.assertEqual(self.application.status.status, CaseStatusEnum.UNDER_FINAL_REVIEW) + self.assertEqual(self.application.advice.filter(level=AdviceLevel.FINAL).exists(), False) + self.assertEqual(self.application.advice.all().count(), 1) + + audit_dev = Audit.objects.get(verb=AuditType.DEVELOPER_INTERVENTION) + self.assertEqual(audit_dev.actor, self.system_user) + self.assertEqual(audit_dev.target.id, self.application.id) + + self.assertEqual( + audit_dev.payload, + { + "additional_text": "Removed final advice.", + }, + ) + + audit = Audit.objects.get(verb=AuditType.UPDATED_STATUS) + self.assertEqual(audit.actor, self.system_user) + self.assertEqual(audit.target.id, self.application.id) + + self.assertEqual( + audit.payload, + { + "status": {"new": self.application.status.status, "old": CaseStatusEnum.SUBMITTED}, + "additional_text": "", + }, + ) + + def test_remove_case_final_advice_command_status_not_updated(self): + + call_command("remove_case_final_advice", case_reference=self.application.reference_code) + + self.assertEqual(Audit.objects.filter(verb=AuditType.UPDATED_STATUS).exists(), False) + + def test_remove_case_status_change_command_dry_run(self): + + self.assertEqual(self.application.advice.filter(level=AdviceLevel.FINAL).exists(), True) + self.assertEqual(self.application.advice.all().count(), 2) + + call_command("remove_case_final_advice", "--dry_run", case_reference=self.application.reference_code) + + self.application.refresh_from_db() + self.assertEqual(self.application.advice.filter(level=AdviceLevel.FINAL).exists(), True) + self.assertEqual(self.application.advice.all().count(), 2) + self.assertEqual(Audit.objects.filter(verb=AuditType.DEVELOPER_INTERVENTION).exists(), False) + + def test_remove_case_final_advice_command_no_final_advise(self): + + self.application.advice.filter(level=AdviceLevel.FINAL).delete() + with pytest.raises(CommandError): + call_command("remove_case_final_advice", case_reference=self.application.reference_code) + + def test_remove_case_final_advice_command_missing_case_ref(self): + + self.application.advice.filter(level=AdviceLevel.FINAL).delete() + self.assertEqual(self.application.advice.all().count(), 1) + + with pytest.raises(CommandError): + call_command("remove_case_final_advice", case_reference="bad-ref") + + self.assertEqual(self.application.advice.all().count(), 1)