From d4e529d782331c9aee797afc0148243a856c0a34 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Mon, 16 Sep 2024 12:43:26 +0100 Subject: [PATCH 01/10] create management command and added tests --- .../management/commands/update_party_name.py | 42 +++++++++++++ .../tests/test_update_party_name.py | 61 +++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 api/applications/management/commands/update_party_name.py create mode 100644 api/applications/tests/test_update_party_name.py diff --git a/api/applications/management/commands/update_party_name.py b/api/applications/management/commands/update_party_name.py new file mode 100644 index 000000000..99b178dfc --- /dev/null +++ b/api/applications/management/commands/update_party_name.py @@ -0,0 +1,42 @@ +import csv +from django.core.management.base import BaseCommand +from api.parties.models import Party +from api.users.models import BaseUser +from api.audit_trail import service as audit_trail_service +from api.audit_trail.enums import AuditType +from django.db import transaction + + +class Command(BaseCommand): + help = "Update Name for multiple parties from a CSV file" + + def add_arguments(self, parser): + parser.add_argument("csv_file", type=open, help="The path to the CSV file containing updates") + + def handle(self, *args, **kwargs): + csv_file = kwargs["csv_file"] + + reader = csv.DictReader(csv_file) + with transaction.atomic(): + for row in reader: + party_id = row["party_id"] + name = row["name"].replace("\\r\\n", "\r\n") + new_name = row["new_name"] + additional_text = row["additional_text"] + + self.update_field_on_party(party_id, name, new_name, additional_text) + + def update_field_on_party(self, party_id, name, new_name, additional_text): + party = Party.objects.get(id=party_id, name=name) + system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") + + audit_trail_service.create( + actor=system_user, + verb=AuditType.DEVELOPER_INTERVENTION, + target=party, + payload={"name": {"new": new_name, "old": name}, "additional_text": additional_text}, + ) + + party.name = new_name + party.save() + self.stdout.write(f"Updated name for Party {party_id} from {name} to {new_name}.") diff --git a/api/applications/tests/test_update_party_name.py b/api/applications/tests/test_update_party_name.py new file mode 100644 index 000000000..4e050a300 --- /dev/null +++ b/api/applications/tests/test_update_party_name.py @@ -0,0 +1,61 @@ +from api.audit_trail.models import Audit +from api.audit_trail.enums import AuditType +from django.core.management import call_command +from tempfile import NamedTemporaryFile +import pytest +from api.parties.models import Party +from test_helpers.clients import DataTestClient + + +class UpdatePartyFromCSVTests(DataTestClient): + def setUp(self): + super().setUp() + self.standard_application = self.create_draft_standard_application(self.organisation) + + def test_update_field_on_party_from_csv(self): + + new_name = "Bangarang 3000" + old_name = self.standard_application.end_user.party.name + party_id = self.standard_application.end_user.party.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "party_id,name,new_name,additional_text", + f"""{party_id},"{old_name}",{new_name},added by John Smith as per LTD-XXX""", + ] + tmp_file.write("\n".join(rows).encode("utf-8")) + tmp_file.flush() + + call_command("update_party_name", tmp_file.name) + self.standard_application.refresh_from_db() + self.assertEqual(self.standard_application.end_user.party.name, new_name) + + audit = Audit.objects.get() + + self.assertEqual(audit.actor, self.system_user) + self.assertEqual(audit.target.id, party_id) + self.assertEqual(audit.verb, AuditType.DEVELOPER_INTERVENTION) + self.assertEqual( + audit.payload, + { + "name": {"new": new_name, "old": old_name}, + "additional_text": "added by John Smith as per LTD-XXX", + }, + ) + + def test_update_field_on_party_from_csv_invalid(self): + + new_name = "Bangarang 3000" + old_name = "This is not an name" + party_id = self.standard_application.end_user.party.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "party_id,name,new_name,additional_text", + f"""{party_id},"{old_name}",{new_name},added by John Smith as per LTD-XXX""", + ] + tmp_file.write("\n".join(rows).encode("utf-8")) + tmp_file.flush() + + with pytest.raises(Party.DoesNotExist): + call_command("update_party_name", tmp_file.name) From 54bacf9e8361e59dc53552f588d5159521050214 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 11 Sep 2024 13:10:58 +0100 Subject: [PATCH 02/10] Remove unused payload variable from emails --- api/cases/notify.py | 2 -- api/cases/tests/test_notify.py | 2 -- gov_notify/payloads.py | 2 -- 3 files changed, 6 deletions(-) diff --git a/api/cases/notify.py b/api/cases/notify.py index 95ee74080..c554d0953 100644 --- a/api/cases/notify.py +++ b/api/cases/notify.py @@ -78,7 +78,6 @@ def notify_exporter_licence_revoked(licence): { "user_first_name": exporter.first_name, "application_reference": case.reference_code, - "exporter_frontend_url": get_exporter_frontend_url("/"), }, ) @@ -188,6 +187,5 @@ def notify_exporter_appeal_acknowledgement(case): payload = ExporterAppealAcknowledgement( user_first_name=exporter.first_name, application_reference=case.reference_code, - exporter_frontend_url=get_exporter_frontend_url("/"), ) send_email(exporter.email, TemplateType.EXPORTER_APPEAL_ACKNOWLEDGEMENT, payload) diff --git a/api/cases/tests/test_notify.py b/api/cases/tests/test_notify.py index e359fce74..24ae750a5 100644 --- a/api/cases/tests/test_notify.py +++ b/api/cases/tests/test_notify.py @@ -69,7 +69,6 @@ def test_notify_licence_revoked(self, mock_send_email): expected_payload = ExporterLicenceRevoked( user_first_name=self.exporter_user.first_name, application_reference=self.licence.case.reference_code, - exporter_frontend_url="https://exporter.lite.service.localhost.uktrade.digital/", ) notify_exporter_licence_revoked(self.licence) @@ -154,7 +153,6 @@ def test_notify_exporter_appeal_acknowledgement(self, mock_send_email): expected_payload = ExporterAppealAcknowledgement( user_first_name=self.case.submitted_by.first_name, application_reference=self.case.reference_code, - exporter_frontend_url="https://exporter.lite.service.localhost.uktrade.digital/", ) mock_send_email.assert_called_with( self.case.submitted_by.email, diff --git a/gov_notify/payloads.py b/gov_notify/payloads.py index 0ed611277..c90a6b666 100644 --- a/gov_notify/payloads.py +++ b/gov_notify/payloads.py @@ -54,7 +54,6 @@ class ExporterLicenceRefused(EmailData): class ExporterLicenceRevoked(EmailData): user_first_name: str application_reference: str - exporter_frontend_url: str @dataclass(frozen=True) @@ -122,7 +121,6 @@ class ExporterInformLetter(EmailData): class ExporterAppealAcknowledgement(EmailData): user_first_name: str application_reference: str - exporter_frontend_url: str @dataclass(frozen=True) From dd63d166b427e996f1e0132147b1cac406766915 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 11 Sep 2024 13:22:49 +0100 Subject: [PATCH 03/10] Remove unused gov notify payload classes --- gov_notify/payloads.py | 14 -------------- gov_notify/tests/test_payloads.py | 20 +------------------- 2 files changed, 1 insertion(+), 33 deletions(-) diff --git a/gov_notify/payloads.py b/gov_notify/payloads.py index c90a6b666..bbb97f68b 100644 --- a/gov_notify/payloads.py +++ b/gov_notify/payloads.py @@ -11,20 +11,6 @@ def as_dict(self): return {field.name: getattr(self, field.name) for field in fields(self)} -@dataclass(frozen=True) -class EcjuCreatedEmailData(EmailData): - case_reference: str - application_reference: str - link: str - - -@dataclass(frozen=True) -class ApplicationStatusEmailData(EmailData): - case_reference: str - application_reference: str - link: str - - @dataclass(frozen=True) class ExporterRegistration(EmailData): organisation_name: str diff --git a/gov_notify/tests/test_payloads.py b/gov_notify/tests/test_payloads.py index 6b2dcc0b1..fe3896bc2 100644 --- a/gov_notify/tests/test_payloads.py +++ b/gov_notify/tests/test_payloads.py @@ -1,5 +1,3 @@ -from unittest import mock - from parameterized import parameterized from rest_framework.test import APITestCase @@ -9,22 +7,6 @@ class DataclassTests(APITestCase): @parameterized.expand( [ - ( - payloads.EcjuCreatedEmailData, - { - "case_reference": "testref", - "application_reference": "testref2", - "link": "testlink", - }, - ), - ( - payloads.ApplicationStatusEmailData, - { - "case_reference": "testref", - "application_reference": "testref2", - "link": "testlink", - }, - ), ( payloads.ExporterRegistration, { @@ -97,7 +79,7 @@ class DataclassTests(APITestCase): payloads.CaseWorkerNewRegistration, { "organisation_name": "testref", - "applicant_email": "test@user.com", + "applicant_email": "test@user.com", # /PS-IGNORE }, ), ] From 04c88ff3220a42c3a31947cdcd1c982c6c902eb6 Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Tue, 10 Sep 2024 09:22:46 +0100 Subject: [PATCH 04/10] add new caseworker add user --- api/audit_trail/enums.py | 1 + .../migrations/0027_alter_audit_verb.py | 284 ++++++++++++++++++ api/audit_trail/payload.py | 1 + api/conf/caseworker_urls.py | 1 + api/core/celery_tasks.py | 10 - api/core/permissions.py | 6 + api/core/tests/test_celery_tasks.py | 6 - api/organisations/caseworker/__init__.py | 0 .../caseworker/serializers/__init__.py | 0 .../caseworker/serializers/serializers.py | 62 ++++ api/organisations/caseworker/urls.py | 9 + .../caseworker/views/__init__.py | 0 .../caseworker/views/tests/test_users.py | 100 ++++++ api/organisations/caseworker/views/users.py | 35 +++ api/organisations/models.py | 23 ++ 15 files changed, 522 insertions(+), 16 deletions(-) create mode 100644 api/audit_trail/migrations/0027_alter_audit_verb.py create mode 100644 api/organisations/caseworker/__init__.py create mode 100644 api/organisations/caseworker/serializers/__init__.py create mode 100644 api/organisations/caseworker/serializers/serializers.py create mode 100644 api/organisations/caseworker/urls.py create mode 100644 api/organisations/caseworker/views/__init__.py create mode 100644 api/organisations/caseworker/views/tests/test_users.py create mode 100644 api/organisations/caseworker/views/users.py diff --git a/api/audit_trail/enums.py b/api/audit_trail/enums.py index af58aaf20..77dff0a48 100644 --- a/api/audit_trail/enums.py +++ b/api/audit_trail/enums.py @@ -145,6 +145,7 @@ class AuditType(LiteEnum): EXPORTER_SUBMITTED_AMENDMENT = autostr() AMENDMENT_CREATED = autostr() DEVELOPER_INTERVENTION = autostr() + ADD_EXPORTER_USER_TO_ORGANISATION = autostr() def human_readable(self): """ diff --git a/api/audit_trail/migrations/0027_alter_audit_verb.py b/api/audit_trail/migrations/0027_alter_audit_verb.py new file mode 100644 index 000000000..9959553bf --- /dev/null +++ b/api/audit_trail/migrations/0027_alter_audit_verb.py @@ -0,0 +1,284 @@ +# Generated by Django 4.2.15 on 2024-09-13 13:38 + +import api.audit_trail.enums +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("audit_trail", "0026_alter_audit_verb"), + ] + + operations = [ + migrations.AlterField( + model_name="audit", + name="verb", + field=models.CharField( + choices=[ + (api.audit_trail.enums.AuditType["CREATED"], "created"), + (api.audit_trail.enums.AuditType["OGL_CREATED"], "ogl_created"), + (api.audit_trail.enums.AuditType["OGL_FIELD_EDITED"], "ogl_field_edited"), + (api.audit_trail.enums.AuditType["OGL_MULTI_FIELD_EDITED"], "ogl_multi_field_edited"), + (api.audit_trail.enums.AuditType["ADD_FLAGS"], "add_flags"), + (api.audit_trail.enums.AuditType["REMOVE_FLAGS"], "remove_flags"), + (api.audit_trail.enums.AuditType["GOOD_REVIEWED"], "good_reviewed"), + (api.audit_trail.enums.AuditType["GOOD_ADD_FLAGS"], "good_add_flags"), + (api.audit_trail.enums.AuditType["GOOD_REMOVE_FLAGS"], "good_remove_flags"), + (api.audit_trail.enums.AuditType["GOOD_ADD_REMOVE_FLAGS"], "good_add_remove_flags"), + (api.audit_trail.enums.AuditType["DESTINATION_ADD_FLAGS"], "destination_add_flags"), + (api.audit_trail.enums.AuditType["DESTINATION_REMOVE_FLAGS"], "destination_remove_flags"), + (api.audit_trail.enums.AuditType["ADD_GOOD_TO_APPLICATION"], "add_good_to_application"), + (api.audit_trail.enums.AuditType["REMOVE_GOOD_FROM_APPLICATION"], "remove_good_from_application"), + (api.audit_trail.enums.AuditType["ADD_GOOD_TYPE_TO_APPLICATION"], "add_good_type_to_application"), + ( + api.audit_trail.enums.AuditType["REMOVE_GOOD_TYPE_FROM_APPLICATION"], + "remove_good_type_from_application", + ), + ( + api.audit_trail.enums.AuditType["UPDATE_APPLICATION_END_USE_DETAIL"], + "update_application_end_use_detail", + ), + ( + api.audit_trail.enums.AuditType["UPDATE_APPLICATION_TEMPORARY_EXPORT"], + "update_application_temporary_export", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_SITES_FROM_APPLICATION"], + "removed_sites_from_application", + ), + (api.audit_trail.enums.AuditType["ADD_SITES_TO_APPLICATION"], "add_sites_to_application"), + ( + api.audit_trail.enums.AuditType["REMOVED_EXTERNAL_LOCATIONS_FROM_APPLICATION"], + "removed_external_locations_from_application", + ), + ( + api.audit_trail.enums.AuditType["ADD_EXTERNAL_LOCATIONS_TO_APPLICATION"], + "add_external_locations_to_application", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_COUNTRIES_FROM_APPLICATION"], + "removed_countries_from_application", + ), + (api.audit_trail.enums.AuditType["ADD_COUNTRIES_TO_APPLICATION"], "add_countries_to_application"), + ( + api.audit_trail.enums.AuditType["ADD_ADDITIONAL_CONTACT_TO_CASE"], + "add_additional_contact_to_case", + ), + (api.audit_trail.enums.AuditType["MOVE_CASE"], "move_case"), + (api.audit_trail.enums.AuditType["ASSIGN_CASE"], "assign_case"), + (api.audit_trail.enums.AuditType["ASSIGN_USER_TO_CASE"], "assign_user_to_case"), + (api.audit_trail.enums.AuditType["REMOVE_USER_FROM_CASE"], "remove_user_from_case"), + (api.audit_trail.enums.AuditType["REMOVE_CASE"], "remove_case"), + (api.audit_trail.enums.AuditType["REMOVE_CASE_FROM_ALL_QUEUES"], "remove_case_from_all_queues"), + ( + api.audit_trail.enums.AuditType["REMOVE_CASE_FROM_ALL_USER_ASSIGNMENTS"], + "remove_case_from_all_user_assignments", + ), + (api.audit_trail.enums.AuditType["CLC_RESPONSE"], "clc_response"), + (api.audit_trail.enums.AuditType["PV_GRADING_RESPONSE"], "pv_grading_response"), + (api.audit_trail.enums.AuditType["CREATED_CASE_NOTE"], "created_case_note"), + ( + api.audit_trail.enums.AuditType["CREATED_CASE_NOTE_WITH_MENTIONS"], + "created_case_note_with_mentions", + ), + (api.audit_trail.enums.AuditType["ECJU_QUERY"], "ecju_query"), + (api.audit_trail.enums.AuditType["ECJU_QUERY_RESPONSE"], "ecju_query_response"), + (api.audit_trail.enums.AuditType["ECJU_QUERY_MANUALLY_CLOSED"], "ecju_query_manually_closed"), + (api.audit_trail.enums.AuditType["UPDATED_STATUS"], "updated_status"), + (api.audit_trail.enums.AuditType["UPDATED_SUB_STATUS"], "updated_sub_status"), + (api.audit_trail.enums.AuditType["UPDATED_APPLICATION_NAME"], "updated_application_name"), + ( + api.audit_trail.enums.AuditType["UPDATE_APPLICATION_LETTER_REFERENCE"], + "update_application_letter_reference", + ), + ( + api.audit_trail.enums.AuditType["UPDATE_APPLICATION_F680_CLEARANCE_TYPES"], + "update_application_f680_clearance_types", + ), + ( + api.audit_trail.enums.AuditType["ADDED_APPLICATION_LETTER_REFERENCE"], + "added_application_letter_reference", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_APPLICATION_LETTER_REFERENCE"], + "removed_application_letter_reference", + ), + (api.audit_trail.enums.AuditType["ASSIGNED_COUNTRIES_TO_GOOD"], "assigned_countries_to_good"), + (api.audit_trail.enums.AuditType["REMOVED_COUNTRIES_FROM_GOOD"], "removed_countries_from_good"), + (api.audit_trail.enums.AuditType["CREATED_FINAL_ADVICE"], "created_final_advice"), + (api.audit_trail.enums.AuditType["CLEARED_FINAL_ADVICE"], "cleared_final_advice"), + (api.audit_trail.enums.AuditType["CREATED_TEAM_ADVICE"], "created_team_advice"), + (api.audit_trail.enums.AuditType["CLEARED_TEAM_ADVICE"], "cleared_team_advice"), + (api.audit_trail.enums.AuditType["REVIEW_COMBINE_ADVICE"], "review_combine_advice"), + (api.audit_trail.enums.AuditType["CREATED_USER_ADVICE"], "created_user_advice"), + (api.audit_trail.enums.AuditType["CLEARED_USER_ADVICE"], "cleared_user_advice"), + (api.audit_trail.enums.AuditType["ADD_PARTY"], "add_party"), + (api.audit_trail.enums.AuditType["REMOVE_PARTY"], "remove_party"), + (api.audit_trail.enums.AuditType["UPLOAD_PARTY_DOCUMENT"], "upload_party_document"), + (api.audit_trail.enums.AuditType["DELETE_PARTY_DOCUMENT"], "delete_party_document"), + (api.audit_trail.enums.AuditType["UPLOAD_APPLICATION_DOCUMENT"], "upload_application_document"), + (api.audit_trail.enums.AuditType["DELETE_APPLICATION_DOCUMENT"], "delete_application_document"), + (api.audit_trail.enums.AuditType["UPLOAD_CASE_DOCUMENT"], "upload_case_document"), + (api.audit_trail.enums.AuditType["GENERATE_CASE_DOCUMENT"], "generate_case_document"), + (api.audit_trail.enums.AuditType["ADD_CASE_OFFICER_TO_CASE"], "add_case_officer_to_case"), + (api.audit_trail.enums.AuditType["REMOVE_CASE_OFFICER_FROM_CASE"], "remove_case_officer_from_case"), + (api.audit_trail.enums.AuditType["GRANTED_APPLICATION"], "granted_application"), + (api.audit_trail.enums.AuditType["REINSTATED_APPLICATION"], "reinstated_application"), + (api.audit_trail.enums.AuditType["FINALISED_APPLICATION"], "finalised_application"), + (api.audit_trail.enums.AuditType["UNASSIGNED_QUEUES"], "unassigned_queues"), + (api.audit_trail.enums.AuditType["UNASSIGNED"], "unassigned"), + (api.audit_trail.enums.AuditType["CREATED_DOCUMENT_TEMPLATE"], "created_document_template"), + (api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_NAME"], "updated_letter_template_name"), + ( + api.audit_trail.enums.AuditType["ADDED_LETTER_TEMPLATE_CASE_TYPES"], + "added_letter_template_case_types", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_CASE_TYPES"], + "updated_letter_template_case_types", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_LETTER_TEMPLATE_CASE_TYPES"], + "removed_letter_template_case_types", + ), + ( + api.audit_trail.enums.AuditType["ADDED_LETTER_TEMPLATE_DECISIONS"], + "added_letter_template_decisions", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_DECISIONS"], + "updated_letter_template_decisions", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_LETTER_TEMPLATE_DECISIONS"], + "removed_letter_template_decisions", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_PARAGRAPHS"], + "updated_letter_template_paragraphs", + ), + ( + api.audit_trail.enums.AuditType["REMOVED_LETTER_TEMPLATE_PARAGRAPHS"], + "removed_letter_template_paragraphs", + ), + ( + api.audit_trail.enums.AuditType["ADDED_LETTER_TEMPLATE_PARAGRAPHS"], + "added_letter_template_paragraphs", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_LAYOUT"], + "updated_letter_template_layout", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_PARAGRAPHS_ORDERING"], + "updated_letter_template_paragraphs_ordering", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_LETTER_TEMPLATE_INCLUDE_DIGITAL_SIGNATURE"], + "updated_letter_template_include_digital_signature", + ), + (api.audit_trail.enums.AuditType["CREATED_PICKLIST"], "created_picklist"), + (api.audit_trail.enums.AuditType["UPDATED_PICKLIST_TEXT"], "updated_picklist_text"), + (api.audit_trail.enums.AuditType["UPDATED_PICKLIST_NAME"], "updated_picklist_name"), + (api.audit_trail.enums.AuditType["DEACTIVATE_PICKLIST"], "deactivate_picklist"), + (api.audit_trail.enums.AuditType["REACTIVATE_PICKLIST"], "reactivate_picklist"), + ( + api.audit_trail.enums.AuditType["UPDATED_EXHIBITION_DETAILS_TITLE"], + "updated_exhibition_details_title", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_EXHIBITION_DETAILS_START_DATE"], + "updated_exhibition_details_start_date", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_EXHIBITION_DETAILS_REQUIRED_BY_DATE"], + "updated_exhibition_details_required_by_date", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_EXHIBITION_DETAILS_REASON_FOR_CLEARANCE"], + "updated_exhibition_details_reason_for_clearance", + ), + (api.audit_trail.enums.AuditType["UPDATED_ROUTE_OF_GOODS"], "updated_route_of_goods"), + (api.audit_trail.enums.AuditType["UPDATED_ORGANISATION"], "updated_organisation"), + (api.audit_trail.enums.AuditType["CREATED_ORGANISATION"], "created_organisation"), + (api.audit_trail.enums.AuditType["REGISTER_ORGANISATION"], "register_organisation"), + (api.audit_trail.enums.AuditType["REJECTED_ORGANISATION"], "rejected_organisation"), + (api.audit_trail.enums.AuditType["APPROVED_ORGANISATION"], "approved_organisation"), + (api.audit_trail.enums.AuditType["REMOVED_FLAG_ON_ORGANISATION"], "removed_flag_on_organisation"), + (api.audit_trail.enums.AuditType["ADDED_FLAG_ON_ORGANISATION"], "added_flag_on_organisation"), + (api.audit_trail.enums.AuditType["RERUN_ROUTING_RULES"], "rerun_routing_rules"), + (api.audit_trail.enums.AuditType["ENFORCEMENT_CHECK"], "enforcement_check"), + (api.audit_trail.enums.AuditType["UPDATED_SITE"], "updated_site"), + (api.audit_trail.enums.AuditType["CREATED_SITE"], "created_site"), + (api.audit_trail.enums.AuditType["UPDATED_SITE_NAME"], "updated_site_name"), + (api.audit_trail.enums.AuditType["COMPLIANCE_SITE_CASE_CREATE"], "compliance_site_case_create"), + ( + api.audit_trail.enums.AuditType["COMPLIANCE_SITE_CASE_NEW_LICENCE"], + "compliance_site_case_new_licence", + ), + (api.audit_trail.enums.AuditType["ADDED_NEXT_REVIEW_DATE"], "added_next_review_date"), + (api.audit_trail.enums.AuditType["EDITED_NEXT_REVIEW_DATE"], "edited_next_review_date"), + (api.audit_trail.enums.AuditType["REMOVED_NEXT_REVIEW_DATE"], "removed_next_review_date"), + (api.audit_trail.enums.AuditType["COMPLIANCE_VISIT_CASE_CREATED"], "compliance_visit_case_created"), + (api.audit_trail.enums.AuditType["COMPLIANCE_VISIT_CASE_UPDATED"], "compliance_visit_case_updated"), + ( + api.audit_trail.enums.AuditType["COMPLIANCE_PEOPLE_PRESENT_CREATED"], + "compliance_people_present_created", + ), + ( + api.audit_trail.enums.AuditType["COMPLIANCE_PEOPLE_PRESENT_UPDATED"], + "compliance_people_present_updated", + ), + ( + api.audit_trail.enums.AuditType["COMPLIANCE_PEOPLE_PRESENT_DELETED"], + "compliance_people_present_deleted", + ), + ( + api.audit_trail.enums.AuditType["UPDATED_GOOD_ON_DESTINATION_MATRIX"], + "updated_good_on_destination_matrix", + ), + (api.audit_trail.enums.AuditType["LICENCE_UPDATED_GOOD_USAGE"], "licence_updated_good_usage"), + (api.audit_trail.enums.AuditType["OGEL_REISSUED"], "ogel_reissued"), + (api.audit_trail.enums.AuditType["LICENCE_UPDATED_STATUS"], "licence_updated_status"), + ( + api.audit_trail.enums.AuditType["DOCUMENT_ON_ORGANISATION_CREATE"], + "document_on_organisation_create", + ), + ( + api.audit_trail.enums.AuditType["DOCUMENT_ON_ORGANISATION_DELETE"], + "document_on_organisation_delete", + ), + ( + api.audit_trail.enums.AuditType["DOCUMENT_ON_ORGANISATION_UPDATE"], + "document_on_organisation_update", + ), + (api.audit_trail.enums.AuditType["REPORT_SUMMARY_UPDATED"], "report_summary_updated"), + (api.audit_trail.enums.AuditType["COUNTERSIGN_ADVICE"], "countersign_advice"), + (api.audit_trail.enums.AuditType["UPDATED_SERIAL_NUMBERS"], "updated_serial_numbers"), + (api.audit_trail.enums.AuditType["PRODUCT_REVIEWED"], "product_reviewed"), + (api.audit_trail.enums.AuditType["LICENCE_UPDATED_PRODUCT_USAGE"], "licence_updated_product_usage"), + (api.audit_trail.enums.AuditType["CREATED_FINAL_RECOMMENDATION"], "created_final_recommendation"), + (api.audit_trail.enums.AuditType["GENERATE_DECISION_LETTER"], "generate_decision_letter"), + (api.audit_trail.enums.AuditType["DECISION_LETTER_SENT"], "decision_letter_sent"), + (api.audit_trail.enums.AuditType["LU_ADVICE"], "lu_advice"), + (api.audit_trail.enums.AuditType["LU_EDIT_ADVICE"], "lu_edit_advice"), + (api.audit_trail.enums.AuditType["LU_COUNTERSIGN"], "lu_countersign"), + (api.audit_trail.enums.AuditType["LU_EDIT_MEETING_NOTE"], "lu_edit_meeting_note"), + (api.audit_trail.enums.AuditType["LU_CREATE_MEETING_NOTE"], "lu_create_meeting_note"), + (api.audit_trail.enums.AuditType["CREATE_REFUSAL_CRITERIA"], "create_refusal_criteria"), + (api.audit_trail.enums.AuditType["EXPORTER_APPEALED_REFUSAL"], "exporter_appealed_refusal"), + (api.audit_trail.enums.AuditType["EXPORTER_CREATED_AMENDMENT"], "exporter_created_amendment"), + (api.audit_trail.enums.AuditType["EXPORTER_SUBMITTED_AMENDMENT"], "exporter_submitted_amendment"), + (api.audit_trail.enums.AuditType["AMENDMENT_CREATED"], "amendment_created"), + (api.audit_trail.enums.AuditType["DEVELOPER_INTERVENTION"], "developer_intervention"), + ( + api.audit_trail.enums.AuditType["ADD_EXPORTER_USER_TO_ORGANISATION"], + "add_exporter_user_to_organisation", + ), + ], + db_index=True, + max_length=255, + ), + ), + ] diff --git a/api/audit_trail/payload.py b/api/audit_trail/payload.py index 928905b73..a7a422797 100644 --- a/api/audit_trail/payload.py +++ b/api/audit_trail/payload.py @@ -163,4 +163,5 @@ def format_payload(audit_type, payload): AuditType.EXPORTER_SUBMITTED_AMENDMENT: formatters.exporter_submitted_amendment, AuditType.AMENDMENT_CREATED: formatters.amendment_created, AuditType.DEVELOPER_INTERVENTION: "updated application information", + AuditType.ADD_EXPORTER_USER_TO_ORGANISATION: " added exporter {exporter_email} to sites {site_names}", } diff --git a/api/conf/caseworker_urls.py b/api/conf/caseworker_urls.py index 0c9944998..18f43f1bb 100644 --- a/api/conf/caseworker_urls.py +++ b/api/conf/caseworker_urls.py @@ -2,4 +2,5 @@ urlpatterns = [ path("applications/", include("api.applications.caseworker.urls")), + path("organisations/", include("api.organisations.caseworker.urls")), ] diff --git a/api/core/celery_tasks.py b/api/core/celery_tasks.py index c30127447..f16f380f2 100644 --- a/api/core/celery_tasks.py +++ b/api/core/celery_tasks.py @@ -1,6 +1,4 @@ from celery import shared_task -from api.cases.models import Case - from django.conf import settings from notifications_python_client import NotificationsAPIClient @@ -13,14 +11,6 @@ def debug_add(x, y): return x + y -@shared_task -def debug_count_cases(): - """ - Simple debug celery task to count the number of cases in the app. - """ - return Case.objects.count() - - @shared_task def debug_exception(): """ diff --git a/api/core/permissions.py b/api/core/permissions.py index b8fd764d7..ad900e7e7 100644 --- a/api/core/permissions.py +++ b/api/core/permissions.py @@ -1,5 +1,6 @@ from rest_framework import permissions +from api.core.constants import GovPermissions from api.core.exceptions import PermissionDeniedError from api.organisations.libraries.get_organisation import get_request_user_organisation from api.organisations.models import Organisation @@ -34,3 +35,8 @@ def has_permission(self, request, view): class CaseInCaseworkerOperableStatus(permissions.BasePermission): def has_permission(self, request, view): return view.get_case().status.is_caseworker_operable + + +class CanCaseworkersManageOrgainsation(permissions.BasePermission): + def has_permission(self, request, view): + return check_user_has_permission(request.user.govuser, GovPermissions.MANAGE_ORGANISATIONS) diff --git a/api/core/tests/test_celery_tasks.py b/api/core/tests/test_celery_tasks.py index ac8709ba3..8674c583e 100644 --- a/api/core/tests/test_celery_tasks.py +++ b/api/core/tests/test_celery_tasks.py @@ -1,6 +1,5 @@ from test_helpers.clients import DataTestClient -from api.cases.tests.factories import CaseFactory from api.core import celery_tasks @@ -9,10 +8,5 @@ def test_debug_add(self): res = celery_tasks.debug_add(1, 2) assert res == 3 - def test_debug_count_cases(self): - CaseFactory() - res = celery_tasks.debug_count_cases() - assert res == 1 - def test_debug_exception(self): self.assertRaises(Exception, celery_tasks.debug_exception) diff --git a/api/organisations/caseworker/__init__.py b/api/organisations/caseworker/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/organisations/caseworker/serializers/__init__.py b/api/organisations/caseworker/serializers/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/organisations/caseworker/serializers/serializers.py b/api/organisations/caseworker/serializers/serializers.py new file mode 100644 index 000000000..d8066294e --- /dev/null +++ b/api/organisations/caseworker/serializers/serializers.py @@ -0,0 +1,62 @@ +from typing import Dict + +from phonenumber_field.serializerfields import PhoneNumberField +from rest_framework import serializers + +from api.organisations.models import Organisation, Site +from api.users.enums import UserType +from api.users.models import ( + ExporterUser, + BaseUser, + UserOrganisationRelationship, + Role, +) + + +class ExporterUserCreateSerializer(serializers.ModelSerializer): + id = serializers.ReadOnlyField(source="baseuser_ptr_id") + email = serializers.EmailField(required=True) + organisation = serializers.PrimaryKeyRelatedField( + queryset=Organisation.objects.all(), required=True, write_only=True + ) + role = serializers.PrimaryKeyRelatedField(queryset=Role.objects.all(), write_only=True, required=True) + sites = serializers.PrimaryKeyRelatedField(queryset=Site.objects.all(), write_only=True, many=True) + phone_number = PhoneNumberField(required=False, allow_blank=True) + + class Meta: + model = ExporterUser + fields = ( + "id", + "email", + "phone_number", + "role", + "organisation", + "sites", + ) + + def validate_email(self, email): + return email.lower() + + def create(self, validated_data: Dict): + phone_number = validated_data.pop("phone_number", "") + email = validated_data.pop("email") + organisation = validated_data.pop("organisation") + sites = validated_data.pop("sites") + role = validated_data.pop("role") + base_user_defaults = { + "email": email, + "phone_number": phone_number, + } + + if phone_number: + phone_number = phone_number.as_e164 + + base_user, _ = BaseUser.objects.get_or_create( + email__iexact=email, type=UserType.EXPORTER, defaults=base_user_defaults + ) + exporter, _ = ExporterUser.objects.get_or_create(baseuser_ptr=base_user, defaults=validated_data) + relationship = UserOrganisationRelationship(user=exporter, organisation=organisation, role=role) + relationship.save() + relationship.sites.set(sites) + + return exporter diff --git a/api/organisations/caseworker/urls.py b/api/organisations/caseworker/urls.py new file mode 100644 index 000000000..41e759663 --- /dev/null +++ b/api/organisations/caseworker/urls.py @@ -0,0 +1,9 @@ +from django.urls import path + +from api.organisations.caseworker.views import users + +app_name = "caseworker_organisations" + +urlpatterns = [ + path("/exporter-users/", users.CreateExporterUser.as_view(), name="exporter_user"), +] diff --git a/api/organisations/caseworker/views/__init__.py b/api/organisations/caseworker/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/organisations/caseworker/views/tests/test_users.py b/api/organisations/caseworker/views/tests/test_users.py new file mode 100644 index 000000000..e00bffb95 --- /dev/null +++ b/api/organisations/caseworker/views/tests/test_users.py @@ -0,0 +1,100 @@ +from api.users.models import ExporterUser, UserOrganisationRelationship +from parameterized import parameterized + +from django.urls import reverse +from rest_framework import status +from unittest import mock + +from api.audit_trail.models import Audit +from api.audit_trail.enums import AuditType +from api.applications.tests.factories import OrganisationFactory + + +from test_helpers.clients import DataTestClient +from api.core.constants import GovPermissions, Roles +from uuid import uuid4 +from faker import Faker + + +class TestAddExporterUserToOrganisation(DataTestClient): + + def setUp(self): + super().setUp() + self.faker = Faker() + self.organisation = OrganisationFactory() + self.url = reverse( + "caseworker_organisations:exporter_user", + kwargs={ + "org_pk": self.organisation.id, + }, + ) + self.data = { + "role": Roles.EXPORTER_ADMINISTRATOR_ROLE_ID, + "email": self.faker.email(), + "sites": [self.organisation.primary_site.id], + "phone_number": "+441234567895", + } + self.gov_user.role.permissions.set([GovPermissions.MANAGE_ORGANISATIONS.name]) + + @mock.patch("api.organisations.models.notify_exporter_user_added") + def test_create_exporter_user_success(self, mocked_notify): + + previous_count = ExporterUser.objects.count() + + response = self.client.post(self.url, **self.gov_headers, data=self.data) + response_data = response.json() + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response_data["email"], self.data["email"]) + + self.assertEqual(ExporterUser.objects.count(), previous_count + 1) + mocked_notify.assert_called_with( + self.data["email"], + { + "organisation_name": self.organisation.name, + "exporter_frontend_url": "https://exporter.lite.service.localhost.uktrade.digital/", + }, + ) + + user_org_sites = UserOrganisationRelationship.objects.get( + organisation_id=self.organisation.id, sites__in=self.data["sites"] + ) + site_names_list = list(user_org_sites.sites.values_list("name", flat=True)) + site_names = ",".join(site_names_list) + + audit_entry = Audit.objects.get(verb=AuditType.ADD_EXPORTER_USER_TO_ORGANISATION) + self.assertEqual( + audit_entry.payload, + {"exporter_email": self.data["email"], "site_names": site_names}, + ) + + @parameterized.expand( + [ + ("sites", ["12345"], {"sites": ["“12345” is not a valid UUID."]}), + ("email", "", {"email": ["This field may not be blank."]}), + ] + ) + def test_create_exporter_user_bad_data(self, key, data_value, expected_error): + self.data[key] = data_value + response = self.client.post(self.url, **self.gov_headers, data=self.data) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json()["errors"], expected_error) + + def test_create_exporter_user_organisation_not_found(self): + self.url = reverse( + "caseworker_organisations:exporter_user", + kwargs={ + "org_pk": uuid4(), + }, + ) + response = self.client.post(self.url, **self.gov_headers, data=self.data) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_create_exporter_user_no_permision(self): + self.gov_user.role.permissions.set([GovPermissions.MANAGE_FLAGGING_RULES.name]) + response = self.client.post(self.url, **self.gov_headers, data=self.data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_create_exporter_user_exporter_user_not_allowed(self): + self.gov_user.role.permissions.set([GovPermissions.MANAGE_FLAGGING_RULES.name]) + response = self.client.post(self.url, **self.exporter_headers, data=self.data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/api/organisations/caseworker/views/users.py b/api/organisations/caseworker/views/users.py new file mode 100644 index 000000000..993bc8423 --- /dev/null +++ b/api/organisations/caseworker/views/users.py @@ -0,0 +1,35 @@ +from api.core.authentication import GovAuthentication +from api.core.permissions import CanCaseworkersManageOrgainsation +from api.organisations.models import Organisation +from django.http import Http404 + +from api.organisations.caseworker.serializers.serializers import ExporterUserCreateSerializer + +from rest_framework.generics import CreateAPIView + + +class CreateExporterUser(CreateAPIView): + authentication_classes = (GovAuthentication,) + + serializer_class = ExporterUserCreateSerializer + permission_classes = [CanCaseworkersManageOrgainsation] + + def setup(self, request, *args, **kwargs): + super().setup(request, *args, **kwargs) + + try: + self.organisation = Organisation.objects.get(pk=self.kwargs["org_pk"]) + except Organisation.DoesNotExist: + raise Http404() + + def perform_create(self, serializer): + super().perform_create(serializer) + sites = [site.id for site in serializer.validated_data["sites"]] + email = serializer.validated_data["email"] + + self.organisation.notify_exporter_user_added(email) + self.organisation.add_case_note_add_export_user(self.request.user, sites, email) + + def get_serializer(self, data): + data["organisation"] = self.organisation.id + return self.serializer_class(data=data) diff --git a/api/organisations/models.py b/api/organisations/models.py index ed64af01c..eac48fdfe 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -6,15 +6,21 @@ from rest_framework.exceptions import ValidationError from api.addresses.models import Address + +from api.audit_trail.enums import AuditType +from api.audit_trail import service as audit_trail_service from api.common.models import TimestampableModel from api.core.constants import ExporterPermissions from api.core.exceptions import NotFoundError +from api.core.helpers import get_exporter_frontend_url from api.flags.models import Flag from api.open_general_licences.enums import OpenGeneralLicenceStatus from api.organisations.enums import LocationType, OrganisationDocumentType, OrganisationStatus, OrganisationType from api.staticdata.countries.models import Country from api.staticdata.statuses.enums import CaseStatusEnum from api.staticdata.statuses.libraries.get_case_status import get_case_status_by_status + +from api.users.notify import notify_exporter_user_added from api.users.libraries.get_user import get_user_organisation_relationship from api.users.models import UserOrganisationRelationship @@ -107,6 +113,23 @@ def save(self, **kwargs): self.primary_site.organisation = self self.primary_site.save() + def notify_exporter_user_added(self, email): + notify_exporter_user_added( + email, + {"organisation_name": self.name, "exporter_frontend_url": get_exporter_frontend_url("/")}, + ) + + def add_case_note_add_export_user(self, actor, sites, exporter_email): + + user_org_sites = Site.objects.filter(id__in=sites).values_list("name", flat=True) + site_names = ",".join(list(user_org_sites)) + audit_trail_service.create( + actor=actor, + verb=AuditType.ADD_EXPORTER_USER_TO_ORGANISATION, + payload={"exporter_email": exporter_email, "site_names": site_names}, + target=self, + ) + class Meta: db_table = "organisation" ordering = ["name"] From 56836008530102d1a5fc4390aac7d4573d0c3090 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Fri, 13 Sep 2024 12:02:57 +0100 Subject: [PATCH 05/10] Add helper for creating developer intervention audit entries --- .../management/commands/update_good_name.py | 25 ++-- .../commands/update_party_address.py | 25 ++-- api/conf/settings_test.py | 1 + api/support/helpers.py | 34 +++++ .../commands/remove_case_final_advice.py | 43 +++---- api/support/tests/__init__.py | 0 api/support/tests/apps.py | 6 + api/support/tests/migrations/0001_initial.py | 20 +++ api/support/tests/migrations/__init__.py | 0 api/support/tests/models.py | 5 + api/support/tests/test_helpers.py | 120 ++++++++++++++++++ 11 files changed, 229 insertions(+), 50 deletions(-) create mode 100644 api/support/helpers.py create mode 100644 api/support/tests/__init__.py create mode 100644 api/support/tests/apps.py create mode 100644 api/support/tests/migrations/0001_initial.py create mode 100644 api/support/tests/migrations/__init__.py create mode 100644 api/support/tests/models.py create mode 100644 api/support/tests/test_helpers.py diff --git a/api/applications/management/commands/update_good_name.py b/api/applications/management/commands/update_good_name.py index eafad3ae0..bc9584749 100644 --- a/api/applications/management/commands/update_good_name.py +++ b/api/applications/management/commands/update_good_name.py @@ -1,10 +1,9 @@ import csv + from django.core.management.base import BaseCommand + from api.goods.models import Good -from api.users.models import BaseUser -from api.audit_trail import service as audit_trail_service -from api.audit_trail.enums import AuditType -from django.db import transaction +from api.support.helpers import developer_intervention class Command(BaseCommand): @@ -17,24 +16,24 @@ def handle(self, *args, **kwargs): csv_file = kwargs["csv_file"] reader = csv.DictReader(csv_file) - with transaction.atomic(): + with developer_intervention(dry_run=False) as audit_log: for row in reader: good_id = row["good_id"] name = row["name"] new_name = row["new_name"] additional_text = row["additional_text"] - self.update_good_name(good_id, name, new_name, additional_text) + self.update_good_name(good_id, name, new_name, additional_text, audit_log) - def update_good_name(self, good_id, name, new_name, additional_text): + def update_good_name(self, good_id, name, new_name, additional_text, audit_log): good = Good.objects.get(id=good_id, name=name) - system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") - audit_trail_service.create( - actor=system_user, - verb=AuditType.DEVELOPER_INTERVENTION, - target=good, - payload={"name": {"new": new_name, "old": name}, "additional_text": additional_text}, + audit_log( + good, + additional_text, + { + "name": {"new": new_name, "old": name}, + }, ) good.name = new_name diff --git a/api/applications/management/commands/update_party_address.py b/api/applications/management/commands/update_party_address.py index 65a2f06f3..0f76a978a 100644 --- a/api/applications/management/commands/update_party_address.py +++ b/api/applications/management/commands/update_party_address.py @@ -1,10 +1,9 @@ import csv + from django.core.management.base import BaseCommand + from api.parties.models import Party -from api.users.models import BaseUser -from api.audit_trail import service as audit_trail_service -from api.audit_trail.enums import AuditType -from django.db import transaction +from api.support.helpers import developer_intervention class Command(BaseCommand): @@ -17,24 +16,24 @@ def handle(self, *args, **kwargs): csv_file = kwargs["csv_file"] reader = csv.DictReader(csv_file) - with transaction.atomic(): + with developer_intervention(dry_run=False) as audit_log: for row in reader: party_id = row["party_id"] address = row["address"].replace("\\r\\n", "\r\n") new_address = row["new_address"] additional_text = row["additional_text"] - self.update_field_on_party(party_id, address, new_address, additional_text) + self.update_field_on_party(party_id, address, new_address, additional_text, audit_log) - def update_field_on_party(self, party_id, address, new_address, additional_text): + def update_field_on_party(self, party_id, address, new_address, additional_text, audit_log): party = Party.objects.get(id=party_id, address=address) - system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") - audit_trail_service.create( - actor=system_user, - verb=AuditType.DEVELOPER_INTERVENTION, - target=party, - payload={"address": {"new": new_address, "old": address}, "additional_text": additional_text}, + audit_log( + party, + additional_text, + { + "address": {"new": new_address, "old": address}, + }, ) party.address = new_address diff --git a/api/conf/settings_test.py b/api/conf/settings_test.py index 502121952..204b5d34b 100644 --- a/api/conf/settings_test.py +++ b/api/conf/settings_test.py @@ -11,6 +11,7 @@ INSTALLED_APPS += [ "api.core.tests.apps.CoreTestsConfig", + "api.support.tests.apps.SupportTestsConfig", ] diff --git a/api/support/helpers.py b/api/support/helpers.py new file mode 100644 index 000000000..8369a4138 --- /dev/null +++ b/api/support/helpers.py @@ -0,0 +1,34 @@ +from contextlib import contextmanager + +from django.db import transaction + +from api.audit_trail import service as audit_trail_service +from api.audit_trail.enums import AuditType + + +@contextmanager +def developer_intervention(*, dry_run=True): + logs = [] + + def audit_log(target, message, additional_payload=None): + if not additional_payload: + additional_payload = {} + + audit_trail_service.create_system_user_audit( + verb=AuditType.DEVELOPER_INTERVENTION, + target=target, + payload={ + "additional_text": message, + **additional_payload, + }, + ) + logs.append((target, message)) + + with transaction.atomic(): + yield audit_log + + if not logs: + raise ValueError("Expected at least one audit event to be logged") + + if dry_run: + transaction.set_rollback(True) diff --git a/api/support/management/commands/remove_case_final_advice.py b/api/support/management/commands/remove_case_final_advice.py index d0404b909..205325394 100644 --- a/api/support/management/commands/remove_case_final_advice.py +++ b/api/support/management/commands/remove_case_final_advice.py @@ -1,7 +1,6 @@ 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 @@ -13,6 +12,8 @@ from api.staticdata.statuses.libraries.get_case_status import get_case_status_by_status from lite_routing.routing_rules_internal.enums import QueuesEnum +from api.support.helpers import developer_intervention + logger = logging.getLogger(__name__) @@ -38,7 +39,7 @@ def handle(self, *args, **options): case_reference = options.pop("case_reference") dry_run = options.pop("dry_run") - with transaction.atomic(): + with developer_intervention(dry_run=dry_run) as audit_log: try: case = Case.objects.get(reference_code=case_reference) except Case.DoesNotExist as e: @@ -52,34 +53,28 @@ def handle(self, *args, **options): 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)) + # 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() - for item in final_advice: - item.delete() + audit_log(case, "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.DEVELOPER_INTERVENTION, + verb=AuditType.UPDATED_STATUS, target=case, payload={ - "additional_text": "Removed final advice.", + "status": {"new": case.status.status, "old": prev_case_status}, + "additional_text": "", }, ) - # 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/tests/__init__.py b/api/support/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/support/tests/apps.py b/api/support/tests/apps.py new file mode 100644 index 000000000..f843a4455 --- /dev/null +++ b/api/support/tests/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class SupportTestsConfig(AppConfig): + name = "api.support.tests" + label = "api_support_tests" diff --git a/api/support/tests/migrations/0001_initial.py b/api/support/tests/migrations/0001_initial.py new file mode 100644 index 000000000..421f4ddaf --- /dev/null +++ b/api/support/tests/migrations/0001_initial.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.16 on 2024-09-13 10:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="FakeModel", + fields=[ + ("id", models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("thing", models.CharField(max_length=100)), + ], + ), + ] diff --git a/api/support/tests/migrations/__init__.py b/api/support/tests/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/support/tests/models.py b/api/support/tests/models.py new file mode 100644 index 000000000..5b86a4c31 --- /dev/null +++ b/api/support/tests/models.py @@ -0,0 +1,5 @@ +from django.db import models + + +class FakeModel(models.Model): + thing = models.CharField(max_length=100) diff --git a/api/support/tests/test_helpers.py b/api/support/tests/test_helpers.py new file mode 100644 index 000000000..477fd49dd --- /dev/null +++ b/api/support/tests/test_helpers.py @@ -0,0 +1,120 @@ +from django.test import TestCase + +from api.audit_trail.enums import AuditType +from api.audit_trail.models import Audit +from api.support.helpers import developer_intervention +from api.support.tests.models import FakeModel +from api.users.enums import SystemUser +from api.users.models import BaseUser + + +class DeveloperInterventionTests(TestCase): + def setUp(self, *args, **kwargs): + super().setUp(*args, **kwargs) + + self.system_user = BaseUser.objects.get(id=SystemUser.id) + + def test_dry_run_makes_no_changes(self): + self.assertEqual(Audit.objects.count(), 0) + + fake_model = FakeModel.objects.create(thing="testing") + with developer_intervention() as audit_log: + fake_model.thing = "something else" + fake_model.save() + audit_log(fake_model, "Changed the model") + + fake_model.refresh_from_db() + self.assertEqual(fake_model.thing, "testing") + self.assertEqual(Audit.objects.count(), 0) + + def test_calling_without_logging_errors(self): + self.assertEqual(Audit.objects.count(), 0) + + fake_model = FakeModel.objects.create(thing="testing") + with self.assertRaises(ValueError): + with developer_intervention(dry_run=False): + fake_model.thing = "something else" + fake_model.save() + + fake_model.refresh_from_db() + self.assertEqual(fake_model.thing, "testing") + self.assertEqual(Audit.objects.count(), 0) + + def test_changes_saved(self): + self.assertEqual(Audit.objects.count(), 0) + + fake_model = FakeModel.objects.create(thing="testing") + with developer_intervention(dry_run=False) as audit_log: + fake_model.thing = "something else" + fake_model.save() + audit_log(fake_model, "Changed the model") + + fake_model.refresh_from_db() + self.assertEqual(fake_model.thing, "something else") + self.assertEqual(Audit.objects.count(), 1) + audit = Audit.objects.get() + self.assertEqual(audit.target, fake_model) + self.assertEqual(audit.payload, {"additional_text": "Changed the model"}) + self.assertEqual(audit.verb, AuditType.DEVELOPER_INTERVENTION) + self.assertEqual(audit.actor, self.system_user) + + def test_multiple_log_entries(self): + self.assertEqual(Audit.objects.count(), 0) + + fake_model = FakeModel.objects.create(thing="testing") + another_fake_model = FakeModel.objects.create(thing="another") + with developer_intervention(dry_run=False) as audit_log: + fake_model.thing = "something else" + fake_model.save() + audit_log(fake_model, "Changed the first model") + + another_fake_model.thing = "another something else" + another_fake_model.save() + audit_log(another_fake_model, "Changed the second model") + + fake_model.refresh_from_db() + self.assertEqual(fake_model.thing, "something else") + + another_fake_model.refresh_from_db() + self.assertEqual(another_fake_model.thing, "another something else") + + self.assertEqual(Audit.objects.count(), 2) + audit = Audit.objects.order_by("created_at")[0] + self.assertEqual(audit.target, fake_model) + self.assertEqual(audit.payload, {"additional_text": "Changed the first model"}) + self.assertEqual(audit.verb, AuditType.DEVELOPER_INTERVENTION) + self.assertEqual(audit.actor, self.system_user) + + audit = Audit.objects.order_by("created_at")[1] + self.assertEqual(audit.target, another_fake_model) + self.assertEqual(audit.payload, {"additional_text": "Changed the second model"}) + self.assertEqual(audit.verb, AuditType.DEVELOPER_INTERVENTION) + self.assertEqual(audit.actor, self.system_user) + + def test_additional_kwargs(self): + self.assertEqual(Audit.objects.count(), 0) + + fake_model = FakeModel.objects.create(thing="testing") + with developer_intervention(dry_run=False) as audit_log: + fake_model.thing = "something else" + fake_model.save() + audit_log( + fake_model, + "Changed the model", + additional_payload={"something_else": "this is something else"}, + ) + + fake_model.refresh_from_db() + self.assertEqual(fake_model.thing, "something else") + self.assertEqual(Audit.objects.count(), 1) + audit = Audit.objects.get() + self.assertEqual(audit.target, fake_model) + self.assertEqual( + audit.payload, + { + "additional_text": "Changed the model", + "something_else": "this is something else", + }, + ) + self.assertEqual(audit.verb, AuditType.DEVELOPER_INTERVENTION) + self.assertEqual(audit.actor, self.system_user) From 6a10c2fd4add17803a5f014f87df831c14ecd309 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 19 Sep 2024 10:34:06 +0100 Subject: [PATCH 06/10] Create a separate serializer for licences in Data Workspace views --- api/data_workspace/v1/licence_views.py | 4 +-- api/data_workspace/v1/serializers.py | 25 +++++++++++++++---- .../v1/tests/test_licence_views.py | 2 +- .../v1/tests/test_serializers.py | 8 +++--- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/api/data_workspace/v1/licence_views.py b/api/data_workspace/v1/licence_views.py index 8d2c5d235..d6b7c7ba3 100644 --- a/api/data_workspace/v1/licence_views.py +++ b/api/data_workspace/v1/licence_views.py @@ -2,7 +2,7 @@ from rest_framework.pagination import LimitOffsetPagination from api.core.authentication import DataWorkspaceOnlyAuthentication -from api.data_workspace.v1.serializers import LicenceWithoutGoodsSerializer +from api.data_workspace.v1.serializers import LicenceSerializer from api.licences import models from api.licences.serializers import view_licence as serializers @@ -16,6 +16,6 @@ class GoodOnLicenceList(viewsets.ReadOnlyModelViewSet): class LicencesList(viewsets.ReadOnlyModelViewSet): authentication_classes = (DataWorkspaceOnlyAuthentication,) - serializer_class = LicenceWithoutGoodsSerializer + serializer_class = LicenceSerializer pagination_class = LimitOffsetPagination queryset = models.Licence.objects.all() diff --git a/api/data_workspace/v1/serializers.py b/api/data_workspace/v1/serializers.py index 9a71ab58b..98142a996 100644 --- a/api/data_workspace/v1/serializers.py +++ b/api/data_workspace/v1/serializers.py @@ -1,8 +1,10 @@ from api.audit_trail.models import Audit +from api.core.serializers import KeyValueChoiceField from api.survey.models import SurveyResponse from api.teams.models import Department from api.cases.models import CaseAssignment, EcjuQuery, DepartmentSLA -from api.licences.serializers.view_licence import LicenceListSerializer +from api.licences.enums import LicenceStatus +from api.licences.models import Licence from api.queues.models import Queue from api.organisations.models import Site from rest_framework import serializers @@ -113,10 +115,23 @@ class AdviceDenialReasonSerializer(serializers.Serializer): denialreason_id = serializers.CharField() -class LicenceWithoutGoodsSerializer(LicenceListSerializer): - def get_goods(self, instance): - # Good are not required by reporting - return [] +class LicenceSerializer(serializers.ModelSerializer): + application = serializers.SerializerMethodField() + status = KeyValueChoiceField(choices=LicenceStatus.choices) + + class Meta: + model = Licence + fields = ( + "id", + "application", + "reference_code", + "status", + ) + read_only_fields = fields + ordering = ["created_at"] + + def get_application(self, instance): + return {"application_id": instance.case.pk} class SurveyResponseSerializer(serializers.ModelSerializer): diff --git a/api/data_workspace/v1/tests/test_licence_views.py b/api/data_workspace/v1/tests/test_licence_views.py index facc22025..0d842cc35 100644 --- a/api/data_workspace/v1/tests/test_licence_views.py +++ b/api/data_workspace/v1/tests/test_licence_views.py @@ -61,7 +61,7 @@ def test_good_on_licenses(self): def test_licenses(self): url = reverse("data_workspace:v1:dw-licences-list") - expected_fields = ("id", "reference_code", "status", "application", "goods") + expected_fields = ("id", "application", "reference_code", "status") response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) results = response.json()["results"] diff --git a/api/data_workspace/v1/tests/test_serializers.py b/api/data_workspace/v1/tests/test_serializers.py index b43fa763f..0cd093787 100644 --- a/api/data_workspace/v1/tests/test_serializers.py +++ b/api/data_workspace/v1/tests/test_serializers.py @@ -6,7 +6,7 @@ EcjuQuerySerializer, AuditUpdatedCaseStatusSerializer, AuditUpdatedLicenceStatusSerializer, - LicenceWithoutGoodsSerializer, + LicenceSerializer, SiteSerializer, ) from api.cases.tests.factories import EcjuQueryFactory, CaseAssignmentFactory @@ -55,18 +55,16 @@ def test_AuditUpdatedLicenceStatusSerializer(db): assert set(serialized.data.keys()) == expected_fields -def test_LicenceWithoutGoodsSerializer(db): +def test_LicenceSerializer(db): licence = StandardLicenceFactory() - serialized = LicenceWithoutGoodsSerializer(licence) + serialized = LicenceSerializer(licence) expected_fields = { "id", "reference_code", "status", "application", - "goods", } assert set(serialized.data) == expected_fields - assert serialized.data["goods"] == [] def test_SiteSerializer(db): From b272549a55a1ce0779b84d053251d2c11b1847f3 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 19 Sep 2024 10:55:55 +0100 Subject: [PATCH 07/10] Use specific goods serializer for licence list --- api/licences/serializers/view_licence.py | 32 +++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/api/licences/serializers/view_licence.py b/api/licences/serializers/view_licence.py index d4964b561..fb42f80f0 100644 --- a/api/licences/serializers/view_licence.py +++ b/api/licences/serializers/view_licence.py @@ -15,7 +15,10 @@ from api.goodstype.models import GoodsType from api.licences.enums import LicenceStatus from api.licences.helpers import serialize_goods_on_licence, get_approved_countries -from api.licences.models import Licence +from api.licences.models import ( + GoodOnLicence, + Licence, +) from api.parties.enums import PartyRole from api.parties.models import Party, PartyDocument from api.staticdata.control_list_entries.serializers import ControlListEntrySerializer @@ -392,22 +395,39 @@ def get_destinations(self, instance): ] +class GoodOnLicenceLicenceListSerializer(serializers.ModelSerializer): + good_on_application_id = serializers.UUIDField(source="good.id") + control_list_entries = ControlListEntrySerializer(source="good.good.control_list_entries", many=True) + assessed_control_list_entries = ControlListEntrySerializer(source="good.control_list_entries", many=True) + description = serializers.CharField(source="good.good.description") + name = serializers.CharField(source="good.good.name") + + class Meta: + model = GoodOnLicence + fields = ( + "id", + "good_on_application_id", + "control_list_entries", + "assessed_control_list_entries", + "description", + "name", + ) + read_only_fields = fields + + class LicenceListSerializer(serializers.ModelSerializer): application = ApplicationLicenceListSerializer(source="case.baseapplication") - goods = serializers.SerializerMethodField() status = KeyValueChoiceField(choices=LicenceStatus.choices) + goods = GoodOnLicenceLicenceListSerializer(many=True) class Meta: model = Licence fields = ( "id", + "application", "reference_code", "status", - "application", "goods", ) read_only_fields = fields ordering = ["created_at"] - - def get_goods(self, instance): - return serialize_goods_on_licence(instance) From 91e837f56a46edbad43c4e09ce5456654a142e70 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 19 Sep 2024 10:56:18 +0100 Subject: [PATCH 08/10] Add optimisations for licence list endpoint --- api/licences/views/main.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/licences/views/main.py b/api/licences/views/main.py index a66376f57..e0e164930 100644 --- a/api/licences/views/main.py +++ b/api/licences/views/main.py @@ -76,6 +76,11 @@ def get_queryset(self): if active_only: licences = licences.exclude(case__status__in=self.non_active_states) + licences = licences.prefetch_related( + "goods__good__good", + "goods__good__control_list_entries", + ) + return licences.order_by("created_at").reverse() From 453a18e12e6c4b8a806b538d2bd3ce5b12deca87 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 19 Sep 2024 11:41:51 +0100 Subject: [PATCH 09/10] Improve tests for data workspace licence endpoints --- api/data_workspace/v1/serializers.py | 2 +- .../v1/tests/test_licence_views.py | 17 +++++++++++++++-- api/data_workspace/v1/tests/test_serializers.py | 15 +++++++++------ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/api/data_workspace/v1/serializers.py b/api/data_workspace/v1/serializers.py index 98142a996..b58228a9d 100644 --- a/api/data_workspace/v1/serializers.py +++ b/api/data_workspace/v1/serializers.py @@ -131,7 +131,7 @@ class Meta: ordering = ["created_at"] def get_application(self, instance): - return {"application_id": instance.case.pk} + return {"application_id": str(instance.case.pk)} class SurveyResponseSerializer(serializers.ModelSerializer): diff --git a/api/data_workspace/v1/tests/test_licence_views.py b/api/data_workspace/v1/tests/test_licence_views.py index 0d842cc35..696c4dfd4 100644 --- a/api/data_workspace/v1/tests/test_licence_views.py +++ b/api/data_workspace/v1/tests/test_licence_views.py @@ -5,6 +5,7 @@ from api.cases.tests.factories import FinalAdviceFactory from api.cases.enums import AdviceType from api.goods.tests.factories import GoodFactory +from api.licences.enums import LicenceStatus from api.licences.tests.factories import StandardLicenceFactory, GoodOnLicenceFactory from test_helpers.clients import DataTestClient @@ -19,10 +20,11 @@ def setUp(self): is_good_controlled=True, control_list_entries=["ML21"], ) + self.licence = StandardLicenceFactory(case=case) FinalAdviceFactory(user=self.gov_user, team=self.team, case=case, good=good, type=AdviceType.APPROVE) GoodOnLicenceFactory( good=GoodOnApplicationFactory(application=case, good=good), - licence=StandardLicenceFactory(case=case), + licence=self.licence, quantity=100, value=1, ) @@ -66,7 +68,18 @@ def test_licenses(self): self.assertEqual(response.status_code, status.HTTP_200_OK) results = response.json()["results"] self.assertGreater(len(results), 0) - self.assertEqual(tuple(results[0].keys()), expected_fields) + self.assertEqual( + results[0], + { + "id": str(self.licence.pk), + "application": {"application_id": str(self.licence.case.pk)}, + "reference_code": self.licence.reference_code, + "status": { + "key": self.licence.status, + "value": LicenceStatus.to_str(self.licence.status), + }, + }, + ) response = self.client.options(url) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/api/data_workspace/v1/tests/test_serializers.py b/api/data_workspace/v1/tests/test_serializers.py index 0cd093787..327b5934f 100644 --- a/api/data_workspace/v1/tests/test_serializers.py +++ b/api/data_workspace/v1/tests/test_serializers.py @@ -10,6 +10,7 @@ SiteSerializer, ) from api.cases.tests.factories import EcjuQueryFactory, CaseAssignmentFactory +from api.licences.enums import LicenceStatus from api.licences.tests.factories import StandardLicenceFactory from api.organisations.tests.factories import SiteFactory @@ -58,13 +59,15 @@ def test_AuditUpdatedLicenceStatusSerializer(db): def test_LicenceSerializer(db): licence = StandardLicenceFactory() serialized = LicenceSerializer(licence) - expected_fields = { - "id", - "reference_code", - "status", - "application", + assert serialized.data == { + "id": str(licence.pk), + "application": {"application_id": str(licence.case.pk)}, + "reference_code": licence.reference_code, + "status": { + "key": licence.status, + "value": LicenceStatus.to_str(licence.status), + }, } - assert set(serialized.data) == expected_fields def test_SiteSerializer(db): From 2324b5b748e08df00c62a49c4d2f1938bb932a7f Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 19 Sep 2024 12:22:59 +0100 Subject: [PATCH 10/10] Improve tests for licences endpoint --- api/licences/tests/test_get_licences.py | 40 ++++++++++++++++++------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/api/licences/tests/test_get_licences.py b/api/licences/tests/test_get_licences.py index 27d6fb4b1..46a2b6931 100644 --- a/api/licences/tests/test_get_licences.py +++ b/api/licences/tests/test_get_licences.py @@ -22,6 +22,7 @@ from api.licences.views.main import LicenceType from api.licences.tests.factories import StandardLicenceFactory from api.open_general_licences.tests.factories import OpenGeneralLicenceCaseFactory, OpenGeneralLicenceFactory +from api.staticdata.control_list_entries.models import ControlListEntry from api.staticdata.countries.models import Country from api.staticdata.statuses.enums import CaseStatusEnum from api.staticdata.statuses.models import CaseStatus @@ -47,6 +48,7 @@ def setUp(self): for application, licence in self.licences.items(): for good in application.goods.all(): + good.control_list_entries.add(ControlListEntry.objects.get(rating="ML2b")) FinalAdviceFactory(user=self.gov_user, good=good.good, case=application) GoodOnLicenceFactory(licence=licence, good=good, quantity=good.quantity, value=good.value) @@ -69,16 +71,34 @@ def test_get_all_licences(self): for licence in self.licences.values(): licence_data = node_by_id(response_data, licence.id) - good_on_application = licence.case.goods.first() - - self.assertEqual( - licence_data["goods"][0]["good_on_application_id"], - str(good_on_application.id), - ) - self.assertEqual( - licence_data["goods"][0]["control_list_entries"][0]["rating"], - good_on_application.good.control_list_entries.all()[0].rating, - ) + for goods_data, good_on_licence in zip(licence_data["goods"], licence.goods.all()): + good_on_application = good_on_licence.good + good = good_on_application.good + self.assertEqual( + goods_data, + { + "id": str(good_on_licence.pk), + "assessed_control_list_entries": [ + { + "id": str(cle.pk), + "rating": cle.rating, + "text": cle.text, + } + for cle in good_on_application.control_list_entries.all() + ], + "control_list_entries": [ + { + "id": str(cle.pk), + "rating": cle.rating, + "text": cle.text, + } + for cle in good.control_list_entries.all() + ], + "description": good.description, + "good_on_application_id": str(good_on_application.pk), + "name": good.name, + }, + ) def test_get_standard_licences_only(self): response = self.client.get(self.url + "?licence_type=" + LicenceType.LICENCE, **self.exporter_headers)