From e3067ed205b075c4e05efac7ab1bbb85312a37e0 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 21 Aug 2024 13:46:56 +0100 Subject: [PATCH 1/8] committing just to share with the team --- .../commands/update_parties_on_application.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 api/applications/management/commands/update_parties_on_application.py diff --git a/api/applications/management/commands/update_parties_on_application.py b/api/applications/management/commands/update_parties_on_application.py new file mode 100644 index 0000000000..267d803cdf --- /dev/null +++ b/api/applications/management/commands/update_parties_on_application.py @@ -0,0 +1,36 @@ +from django.core.management.base import BaseCommand +from api.parties.models import Party +from api.audit_trail import service as audit_trail_service +from api.audit_trail.enums import AuditType +from api.users.models import BaseUser + + +class Command(BaseCommand): + help = "Rename a field within party" + + def add_argument(self, parser): + parser.add_argument("party_id", type=str, help="Party ID to be updated") + + parser.add_argument("field", type=str, help="The field you would like to update i.e address") + + parser.add_argument("value", type=str, help="the value to set the field to") + + def handle(self, *args, **kwargs): + party_id = kwargs["party_id"] + field = kwargs["field"] + value = kwargs["value"] + + self.update_field_on_party(self, party_id, field, value) + + def update_field_on_party(self, party_id, field, value): + party = Party.objects.get(party_id) + system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") + + audit_trail_service.create( + actor=system_user, + verb=AuditType.DELETE_APPLICATION_DOCUMENT, + target=party, + payload={f"{field}": {"new": value, "old": party.__dict__.get(field)}}, + ) + + party.update(**{self.field: self.value}) From eecfd260c6b98be856cf715e8eca9bf0fa31523d Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 21 Aug 2024 16:22:09 +0100 Subject: [PATCH 2/8] tests and new enums --- .../commands/update_parties_on_application.py | 36 ---------------- .../commands/update_party_address.py | 42 +++++++++++++++++++ .../tests/test_update_parties_address.py | 37 ++++++++++++++++ api/audit_trail/enums.py | 1 + api/audit_trail/payload.py | 1 + 5 files changed, 81 insertions(+), 36 deletions(-) delete mode 100644 api/applications/management/commands/update_parties_on_application.py create mode 100644 api/applications/management/commands/update_party_address.py create mode 100644 api/applications/tests/test_update_parties_address.py diff --git a/api/applications/management/commands/update_parties_on_application.py b/api/applications/management/commands/update_parties_on_application.py deleted file mode 100644 index 267d803cdf..0000000000 --- a/api/applications/management/commands/update_parties_on_application.py +++ /dev/null @@ -1,36 +0,0 @@ -from django.core.management.base import BaseCommand -from api.parties.models import Party -from api.audit_trail import service as audit_trail_service -from api.audit_trail.enums import AuditType -from api.users.models import BaseUser - - -class Command(BaseCommand): - help = "Rename a field within party" - - def add_argument(self, parser): - parser.add_argument("party_id", type=str, help="Party ID to be updated") - - parser.add_argument("field", type=str, help="The field you would like to update i.e address") - - parser.add_argument("value", type=str, help="the value to set the field to") - - def handle(self, *args, **kwargs): - party_id = kwargs["party_id"] - field = kwargs["field"] - value = kwargs["value"] - - self.update_field_on_party(self, party_id, field, value) - - def update_field_on_party(self, party_id, field, value): - party = Party.objects.get(party_id) - system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") - - audit_trail_service.create( - actor=system_user, - verb=AuditType.DELETE_APPLICATION_DOCUMENT, - target=party, - payload={f"{field}": {"new": value, "old": party.__dict__.get(field)}}, - ) - - party.update(**{self.field: self.value}) diff --git a/api/applications/management/commands/update_party_address.py b/api/applications/management/commands/update_party_address.py new file mode 100644 index 0000000000..32a16f3a1d --- /dev/null +++ b/api/applications/management/commands/update_party_address.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 + + +class Command(BaseCommand): + help = "Update address 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) + for row in reader: + party_id = row["party_id"] + address = row["address"] + new_address = row["new_address"] + + self.update_field_on_party(party_id, address, new_address) + + def update_field_on_party(self, party_id, address, new_address): + party = Party.objects.get(id=party_id) + 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}, + }, + ) + + assert party.address == address + party.address = new_address + party.save() + self.stdout.write(f"Updated address for Party {party_id} from {address} to {new_address}.") diff --git a/api/applications/tests/test_update_parties_address.py b/api/applications/tests/test_update_parties_address.py new file mode 100644 index 0000000000..f5381c7092 --- /dev/null +++ b/api/applications/tests/test_update_parties_address.py @@ -0,0 +1,37 @@ +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 + +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_address = "56 Heathwood Road Broadstairs Kent" + old_address = self.standard_application.end_user.party.address + party_id = self.standard_application.end_user.party.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "party_id,address,new_address", + f"""{party_id},"{old_address}",{new_address}""", + ] + tmp_file.write("\n".join(rows).encode("utf-8")) + tmp_file.flush() + + call_command("update_party_address", tmp_file.name) + self.standard_application.refresh_from_db() + self.assertEqual(self.standard_application.end_user.party.address, new_address) + + 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, {"address": {"new": new_address, "old": old_address}}) diff --git a/api/audit_trail/enums.py b/api/audit_trail/enums.py index e77d2af671..af58aaf202 100644 --- a/api/audit_trail/enums.py +++ b/api/audit_trail/enums.py @@ -144,6 +144,7 @@ class AuditType(LiteEnum): EXPORTER_CREATED_AMENDMENT = autostr() EXPORTER_SUBMITTED_AMENDMENT = autostr() AMENDMENT_CREATED = autostr() + DEVELOPER_INTERVENTION = autostr() def human_readable(self): """ diff --git a/api/audit_trail/payload.py b/api/audit_trail/payload.py index 8cffd01050..928905b739 100644 --- a/api/audit_trail/payload.py +++ b/api/audit_trail/payload.py @@ -162,4 +162,5 @@ def format_payload(audit_type, payload): AuditType.EXPORTER_CREATED_AMENDMENT: " opened the application for a 'major edit'", AuditType.EXPORTER_SUBMITTED_AMENDMENT: formatters.exporter_submitted_amendment, AuditType.AMENDMENT_CREATED: formatters.amendment_created, + AuditType.DEVELOPER_INTERVENTION: "updated application information", } From 99bc6946fa62d3fed25e30e8eef16ff10f787a63 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 22 Aug 2024 13:24:20 +0100 Subject: [PATCH 3/8] updated based on comments, made transational and added additional_text --- .../commands/update_party_address.py | 35 ++++++++++--------- .../tests/test_update_parties_address.py | 32 ++++++++++++++--- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/api/applications/management/commands/update_party_address.py b/api/applications/management/commands/update_party_address.py index 32a16f3a1d..f0838d84ea 100644 --- a/api/applications/management/commands/update_party_address.py +++ b/api/applications/management/commands/update_party_address.py @@ -1,9 +1,10 @@ import csv -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError 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): @@ -20,23 +21,25 @@ def handle(self, *args, **kwargs): party_id = row["party_id"] address = row["address"] new_address = row["new_address"] + additional_text = row["additional_text"] - self.update_field_on_party(party_id, address, new_address) + self.update_field_on_party(party_id, address, new_address, additional_text) - def update_field_on_party(self, party_id, address, new_address): + def update_field_on_party(self, party_id, address, new_address, additional_text): party = Party.objects.get(id=party_id) 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}, - }, - ) - - assert party.address == address - party.address = new_address - party.save() - self.stdout.write(f"Updated address for Party {party_id} from {address} to {new_address}.") + with transaction.atomic(): + audit_trail_service.create( + actor=system_user, + verb=AuditType.DEVELOPER_INTERVENTION, + target=party, + payload={"address": {"new": new_address, "old": address}, "additional_text": additional_text}, + ) + + if party.address == address: + party.address = new_address + party.save() + self.stdout.write(f"Updated address for Party {party_id} from {address} to {new_address}.") + else: + raise CommandError("Current address does not match csv address, please check csv values") diff --git a/api/applications/tests/test_update_parties_address.py b/api/applications/tests/test_update_parties_address.py index f5381c7092..a6e223fa1a 100644 --- a/api/applications/tests/test_update_parties_address.py +++ b/api/applications/tests/test_update_parties_address.py @@ -2,7 +2,8 @@ from api.audit_trail.enums import AuditType from django.core.management import call_command from tempfile import NamedTemporaryFile - +import pytest +from django.core.management.base import CommandError from test_helpers.clients import DataTestClient @@ -19,8 +20,8 @@ def test_update_field_on_party_from_csv(self): with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: rows = [ - "party_id,address,new_address", - f"""{party_id},"{old_address}",{new_address}""", + "party_id,address,new_address,additional_text", + f"""{party_id},"{old_address}",{new_address},added by John Smith as per LTD-XXX""", ] tmp_file.write("\n".join(rows).encode("utf-8")) tmp_file.flush() @@ -34,4 +35,27 @@ def test_update_field_on_party_from_csv(self): 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, {"address": {"new": new_address, "old": old_address}}) + self.assertEqual( + audit.payload, + { + "address": {"new": new_address, "old": old_address}, + "additional_text": "added by John Smith as per LTD-XXX", + }, + ) + + def test_update_field_on_party_from_csv_invalid(self): + + new_address = "56 Heathwood Road Broadstairs Kent" + old_address = "This is not an address" + party_id = self.standard_application.end_user.party.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "party_id,address,new_address,additional_text", + f"""{party_id},"{old_address}",{new_address},added by John Smith as per LTD-XXX""", + ] + tmp_file.write("\n".join(rows).encode("utf-8")) + tmp_file.flush() + + with pytest.raises(CommandError): + call_command("update_party_address", tmp_file.name) From 1127703340f737148a31463048c7e343c21f923e Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 22 Aug 2024 13:57:23 +0100 Subject: [PATCH 4/8] add migration --- .../migrations/0026_alter_audit_verb.py | 280 ++++++++++++++++++ 1 file changed, 280 insertions(+) create mode 100644 api/audit_trail/migrations/0026_alter_audit_verb.py diff --git a/api/audit_trail/migrations/0026_alter_audit_verb.py b/api/audit_trail/migrations/0026_alter_audit_verb.py new file mode 100644 index 0000000000..1906ff736e --- /dev/null +++ b/api/audit_trail/migrations/0026_alter_audit_verb.py @@ -0,0 +1,280 @@ +# Generated by Django 4.2.14 on 2024-08-22 12:25 + +import api.audit_trail.enums +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("audit_trail", "0025_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"), + ], + db_index=True, + max_length=255, + ), + ), + ] From 23eb698a6f1f128feee585245aa5633f4b542521 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 22 Aug 2024 16:06:27 +0100 Subject: [PATCH 5/8] same as previous commit but good name --- .../management/commands/update_good_name.py | 45 +++++++++++++ .../tests/test_update_good_name.py | 65 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 api/applications/management/commands/update_good_name.py create mode 100644 api/applications/tests/test_update_good_name.py diff --git a/api/applications/management/commands/update_good_name.py b/api/applications/management/commands/update_good_name.py new file mode 100644 index 0000000000..8134239ae7 --- /dev/null +++ b/api/applications/management/commands/update_good_name.py @@ -0,0 +1,45 @@ +import csv +from django.core.management.base import BaseCommand, CommandError +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 + + +class Command(BaseCommand): + help = "Update name for multiple goods 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) + 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) + + def update_good_name(self, good_id, name, new_name, additional_text): + good = Good.objects.get(id=good_id) + system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") + + with transaction.atomic(): + audit_trail_service.create( + actor=system_user, + verb=AuditType.DEVELOPER_INTERVENTION, + target=good, + payload={"name": {"new": new_name, "old": name}, "additional_text": additional_text}, + ) + + if good.name == name: + good.name = new_name + good.save() + self.stdout.write(f"Updated name for Good {good_id} from {name} to {new_name}.") + else: + raise CommandError("Current name does not match csv name, please check csv values") diff --git a/api/applications/tests/test_update_good_name.py b/api/applications/tests/test_update_good_name.py new file mode 100644 index 0000000000..fb21b957e5 --- /dev/null +++ b/api/applications/tests/test_update_good_name.py @@ -0,0 +1,65 @@ +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 django.core.management.base import CommandError +from test_helpers.clients import DataTestClient + + +class UpdateGoodFromCSVTests(DataTestClient): + def setUp(self): + super().setUp() + self.standard_application = self.create_draft_standard_application(self.organisation) + + def test_update_good_name_from_csv(self): + + new_name = "Bangarang 3000" + goodonapplication = self.standard_application.goods.get() + good = goodonapplication.good + old_name = good.name + good_id = good.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "good_id,name,new_name,additional_text", + f"""{good_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_good_name", tmp_file.name) + good.refresh_from_db() + self.assertEqual(good.name, new_name) + + audit = Audit.objects.get() + + self.assertEqual(audit.actor, self.system_user) + self.assertEqual(audit.target.id, good_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_good_name_from_csv_invalid(self): + + new_name = "Bangarang 3000" + goodonapplication = self.standard_application.goods.get() + good = goodonapplication.good + old_name = "Definitely not this" + good_id = good.id + + with NamedTemporaryFile(suffix=".csv", delete=True) as tmp_file: + rows = [ + "good_id,name,new_name,additional_text", + f"""{good_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(CommandError): + call_command("update_good_name", tmp_file.name) From 63c9f1a718a2e90926289bcc68b966ddc8ad899c Mon Sep 17 00:00:00 2001 From: Arun Siluvery Date: Mon, 19 Aug 2024 14:21:31 +0100 Subject: [PATCH 6/8] Display only the CLEs assessed for this application on the Licence Currently we are using CLEs from Good when displaying them on the licence document. Usually this will be the same as recorded in GoodOnApplication but if the underlying good is re-used in other applications where it is assessed differently then they both will differ. In such cases we will be displaying these values from both the assessments. Fix this by using the entries from GoodOnApplication. Add test to ensure correct values are displayed. --- api/applications/tests/factories.py | 2 + .../tests/test_generate_document.py | 120 +++++++++++++++++- api/cases/tests/test_good_precedents_view.py | 4 +- api/letter_templates/context_generator.py | 5 + .../templates/letter_templates/siel.html | 2 +- 5 files changed, 129 insertions(+), 4 deletions(-) diff --git a/api/applications/tests/factories.py b/api/applications/tests/factories.py index f0e0e7fc0a..90463f10eb 100644 --- a/api/applications/tests/factories.py +++ b/api/applications/tests/factories.py @@ -22,6 +22,7 @@ from api.organisations.tests.factories import OrganisationFactory, SiteFactory, ExternalLocationFactory from api.parties.tests.factories import ConsigneeFactory, EndUserFactory, PartyFactory, ThirdPartyFactory from api.users.tests.factories import ExporterUserFactory, GovUserFactory +from api.staticdata.units.enums import Units from api.staticdata.control_list_entries.helpers import get_control_list_entry from api.staticdata.regimes.helpers import get_regime_entry from api.staticdata.statuses.enums import CaseStatusEnum @@ -89,6 +90,7 @@ class GoodOnApplicationFactory(factory.django.DjangoModelFactory): application = factory.SubFactory(StandardApplicationFactory) good = factory.SubFactory(GoodFactory) is_good_controlled = None + unit = Units.NAR @factory.post_generation def control_list_entries(self, create, extracted, **kwargs): diff --git a/api/cases/generated_documents/tests/test_generate_document.py b/api/cases/generated_documents/tests/test_generate_document.py index f30a6e79c1..5a6040a1a1 100644 --- a/api/cases/generated_documents/tests/test_generate_document.py +++ b/api/cases/generated_documents/tests/test_generate_document.py @@ -1,4 +1,5 @@ import os, io +import re from datetime import datetime from unittest import mock @@ -14,21 +15,45 @@ from api.audit_trail.serializers import AuditSerializer from api.audit_trail.models import Audit +from api.applications.tests.factories import GoodOnApplicationFactory, StandardApplicationFactory from api.cases.enums import CaseTypeEnum, AdviceType from api.cases.generated_documents.helpers import html_to_pdf +from api.cases.tests.factories import FinalAdviceFactory from api.letter_templates.helpers import generate_preview, DocumentPreviewError from api.cases.generated_documents.models import GeneratedCaseDocument +from api.goods.tests.factories import GoodFactory from api.cases.generated_documents.signing import sign_pdf from api.licences.enums import LicenceStatus -from api.licences.tests.factories import StandardLicenceFactory +from api.licences.tests.factories import GoodOnLicenceFactory, StandardLicenceFactory from api.staticdata.decisions.models import Decision from api.staticdata.statuses.enums import CaseStatusEnum from api.staticdata.statuses.models import CaseStatus +from api.staticdata.regimes.models import RegimeEntry +from api.staticdata.report_summaries.models import ReportSummarySubject, ReportSummaryPrefix from lite_content.lite_api import strings from test_helpers.clients import DataTestClient from api.users.models import ExporterNotification +def get_tag(html, tag, id): + lines = [] + all_lines = html.split("\n") + for index, line in enumerate(all_lines): + match = re.search(f'<{tag} id="{id}"', line) + if match: + # element spans multiple lines + if f"" not in line: + index = index + 1 + while f"" not in all_lines[index]: + lines.append(all_lines[index]) + index = index + 1 + else: + lines.append(line) + break + + return lines + + class GenerateDocumentTests(DataTestClient): def setUp(self): super().setUp() @@ -452,6 +477,99 @@ def test_generate_document_post_raises_attributeerror( self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(e.exception, "Failed to get preview") + def test_licence_document_shows_correct_cles(self): + """ + Test to ensure licence document shows CLEs for the products assessed as part of this application. + This is usually the case but can be different if the underlying Good is re-used in multiple applications. + + In this test we create two applications that reuse the same underlying Good but in each application + this is assessed with different set of CLEs. Because of the way we record CLEs on the Good model + it will retain previous assessments as well (unlike GoodOnApplication which contains assessments + for that application). + + Test generates licence document and ensures it contains CLEs assessed for this application. + """ + application1 = StandardApplicationFactory(organisation=self.organisation) + good1 = GoodFactory(organisation=self.organisation) + good_on_application1 = GoodOnApplicationFactory(good=good1, application=application1, quantity=10, value=500) + regime_entry = RegimeEntry.objects.first() + report_summary_prefix = ReportSummaryPrefix.objects.first() + report_summary_subject = ReportSummarySubject.objects.first() + data = [ + { + "id": good_on_application1.id, + "control_list_entries": ["ML3a", "ML15d", "ML9a"], + "regime_entries": [regime_entry.id], + "report_summary_prefix": report_summary_prefix.id, + "report_summary_subject": report_summary_subject.id, + "is_good_controlled": True, + "comment": "some comment", + "report_summary": "some string we expect to be overwritten", + "is_ncsc_military_information_security": True, + } + ] + assessment_url = reverse("assessments:make_assessments", kwargs={"case_pk": application1.id}) + response = self.client.put(assessment_url, data, **self.gov_headers) + self.assertEqual(response.status_code, status.HTTP_200_OK) + good_on_application1.refresh_from_db() + + all_cles = [cle.rating for cle in good_on_application1.control_list_entries.all()] + assert sorted(all_cles) == sorted(["ML3a", "ML9a", "ML15d"]) + + FinalAdviceFactory(user=self.gov_user, case=application1, good=good1, type=AdviceType.APPROVE) + + licence = StandardLicenceFactory(case=application1, status=LicenceStatus.DRAFT) + GoodOnLicenceFactory( + good=good_on_application1, + quantity=good_on_application1.quantity, + value=good_on_application1.value, + licence=licence, + ) + + # Create another application and reuse the same good + application2 = StandardApplicationFactory(organisation=self.organisation) + good_on_application2 = GoodOnApplicationFactory(good=good1, application=application2, quantity=20, value=1000) + + data[0]["id"] = good_on_application2.id + data[0]["control_list_entries"] = ["ML5d", "ML2a", "ML18a"] + assessment_url = reverse("assessments:make_assessments", kwargs={"case_pk": application2.id}) + response = self.client.put(assessment_url, data, **self.gov_headers) + self.assertEqual(response.status_code, status.HTTP_200_OK) + good_on_application2.refresh_from_db() + + all_cles = [cle.rating for cle in good_on_application2.control_list_entries.all()] + assert sorted(all_cles) == sorted(["ML5d", "ML2a", "ML18a"]) + + # As this is reused this will have CLEs from both assessments + good1.refresh_from_db() + all_cles = [cle.rating for cle in good1.control_list_entries.all()] + assert sorted(all_cles) == sorted(["ML3a", "ML9a", "ML15d", "ML5d", "ML2a", "ML18a"]) + + # Generate licence document (only preview is enough for this test) + url = ( + reverse("cases:generated_documents:preview", kwargs={"pk": str(application1.id)}) + + "?template=" + + str(self.letter_template.id) + ) + response = self.client.get(url, **self.gov_headers) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + preview = response.json()["preview"] + + cle_lines = get_tag(preview, "td", "row-1-control-list-entries") + actual_cles = "".join(item.strip() for item in cle_lines) + + all_cles = [cle.rating for cle in good_on_application1.control_list_entries.all()] + assert sorted(all_cles) == sorted(actual_cles.split(",")) + + value_element = get_tag(preview, "td", "row-1-value") + value_element = "".join(value_element) + self.assertIn(f"£{str(good_on_application1.value)}", value_element) + + quantity_element = get_tag(preview, "td", "row-1-quantity") + quantity_element = "".join(quantity_element) + self.assertIn(f"{good_on_application1.quantity} Items", quantity_element) + class GetGeneratedDocumentsTests(DataTestClient): def setUp(self): diff --git a/api/cases/tests/test_good_precedents_view.py b/api/cases/tests/test_good_precedents_view.py index 794af5ffde..45c5773faa 100644 --- a/api/cases/tests/test_good_precedents_view.py +++ b/api/cases/tests/test_good_precedents_view.py @@ -130,7 +130,7 @@ def test_get_with_matching_precedents(self): "good": str(self.good.id), "report_summary": "test2", "quantity": 10.0, - "unit": None, + "unit": "NAR", "value": None, "control_list_entries": ["ML1a"], "destinations": expected_destinations, @@ -260,7 +260,7 @@ def test_get_expected_previous_assessments(self, status): "%Y-%m-%dT%H:%M:%S.%f" )[:-3] + "Z", - "unit": None, + "unit": "NAR", "value": None, "wassenaar": True, } diff --git a/api/letter_templates/context_generator.py b/api/letter_templates/context_generator.py index f6579a8206..7db1fdc574 100644 --- a/api/letter_templates/context_generator.py +++ b/api/letter_templates/context_generator.py @@ -491,6 +491,7 @@ class Meta: "applied_for_quantity", "applied_for_value", "created_at", + "control_list_entries", ] good = GoodSerializer() @@ -498,6 +499,7 @@ class Meta: is_incorporated = FriendlyBooleanField(source="is_good_incorporated") applied_for_quantity = serializers.SerializerMethodField() applied_for_value = serializers.SerializerMethodField() + control_list_entries = serializers.SerializerMethodField() def get_applied_for_quantity(self, obj): if hasattr(obj, "quantity"): @@ -509,6 +511,9 @@ def get_applied_for_value(self, obj): return f"£{obj.value}" return "" + def get_control_list_entries(self, obj): + return [clc.rating for clc in obj.control_list_entries.all()] + class GoodsQuerySerializer(serializers.ModelSerializer): class Meta: diff --git a/api/letter_templates/templates/letter_templates/siel.html b/api/letter_templates/templates/letter_templates/siel.html index ba97a383c4..a81a60f5d0 100644 --- a/api/letter_templates/templates/letter_templates/siel.html +++ b/api/letter_templates/templates/letter_templates/siel.html @@ -309,7 +309,7 @@

Standard Individual Export Licence

- {% for clc in good.control_list_entries %} + {% for clc in good_item.control_list_entries %} {{ clc }}{% if not forloop.last %},{% endif %} {% endfor %} From 1f7c3b5f5c979b8580d11a6fcf1ba887f8ab655c Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 29 Aug 2024 09:19:20 +0100 Subject: [PATCH 7/8] updated query, error and transactional --- .../management/commands/update_good_name.py | 41 +++++++++---------- .../tests/test_update_good_name.py | 4 +- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/api/applications/management/commands/update_good_name.py b/api/applications/management/commands/update_good_name.py index 8134239ae7..eafad3ae08 100644 --- a/api/applications/management/commands/update_good_name.py +++ b/api/applications/management/commands/update_good_name.py @@ -1,5 +1,5 @@ import csv -from django.core.management.base import BaseCommand, CommandError +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 @@ -17,29 +17,26 @@ def handle(self, *args, **kwargs): csv_file = kwargs["csv_file"] reader = csv.DictReader(csv_file) - for row in reader: - good_id = row["good_id"] - name = row["name"] - new_name = row["new_name"] - additional_text = row["additional_text"] + with transaction.atomic(): + 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) def update_good_name(self, good_id, name, new_name, additional_text): - good = Good.objects.get(id=good_id) + good = Good.objects.get(id=good_id, name=name) system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") - with transaction.atomic(): - audit_trail_service.create( - actor=system_user, - verb=AuditType.DEVELOPER_INTERVENTION, - target=good, - payload={"name": {"new": new_name, "old": name}, "additional_text": additional_text}, - ) - - if good.name == name: - good.name = new_name - good.save() - self.stdout.write(f"Updated name for Good {good_id} from {name} to {new_name}.") - else: - raise CommandError("Current name does not match csv name, please check csv values") + audit_trail_service.create( + actor=system_user, + verb=AuditType.DEVELOPER_INTERVENTION, + target=good, + payload={"name": {"new": new_name, "old": name}, "additional_text": additional_text}, + ) + + good.name = new_name + good.save() + self.stdout.write(f"Updated name for Good {good_id} from {name} to {new_name}.") diff --git a/api/applications/tests/test_update_good_name.py b/api/applications/tests/test_update_good_name.py index fb21b957e5..430df9b523 100644 --- a/api/applications/tests/test_update_good_name.py +++ b/api/applications/tests/test_update_good_name.py @@ -3,7 +3,7 @@ from django.core.management import call_command from tempfile import NamedTemporaryFile import pytest -from django.core.management.base import CommandError +from api.goods.models import Good from test_helpers.clients import DataTestClient @@ -61,5 +61,5 @@ def test_update_good_name_from_csv_invalid(self): tmp_file.write("\n".join(rows).encode("utf-8")) tmp_file.flush() - with pytest.raises(CommandError): + with pytest.raises(Good.DoesNotExist): call_command("update_good_name", tmp_file.name) From f2f0bbe7b5499aee7cf171ae2e5fc396f19513c1 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 29 Aug 2024 14:00:24 +0100 Subject: [PATCH 8/8] moved transactional and updated tests --- .../commands/update_party_address.py | 41 +++++++++---------- .../tests/test_update_parties_address.py | 4 +- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/api/applications/management/commands/update_party_address.py b/api/applications/management/commands/update_party_address.py index f0838d84ea..2147914c3f 100644 --- a/api/applications/management/commands/update_party_address.py +++ b/api/applications/management/commands/update_party_address.py @@ -1,5 +1,5 @@ import csv -from django.core.management.base import BaseCommand, CommandError +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 @@ -17,29 +17,26 @@ def handle(self, *args, **kwargs): csv_file = kwargs["csv_file"] reader = csv.DictReader(csv_file) - for row in reader: - party_id = row["party_id"] - address = row["address"] - new_address = row["new_address"] - additional_text = row["additional_text"] + with transaction.atomic(): + for row in reader: + party_id = row["party_id"] + address = row["address"] + 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) def update_field_on_party(self, party_id, address, new_address, additional_text): - party = Party.objects.get(id=party_id) + party = Party.objects.get(id=party_id, address=address) system_user = BaseUser.objects.get(id="00000000-0000-0000-0000-000000000001") - with transaction.atomic(): - audit_trail_service.create( - actor=system_user, - verb=AuditType.DEVELOPER_INTERVENTION, - target=party, - payload={"address": {"new": new_address, "old": address}, "additional_text": additional_text}, - ) - - if party.address == address: - party.address = new_address - party.save() - self.stdout.write(f"Updated address for Party {party_id} from {address} to {new_address}.") - else: - raise CommandError("Current address does not match csv address, please check csv values") + audit_trail_service.create( + actor=system_user, + verb=AuditType.DEVELOPER_INTERVENTION, + target=party, + payload={"address": {"new": new_address, "old": address}, "additional_text": additional_text}, + ) + + party.address = new_address + party.save() + self.stdout.write(f"Updated address for Party {party_id} from {address} to {new_address}.") diff --git a/api/applications/tests/test_update_parties_address.py b/api/applications/tests/test_update_parties_address.py index a6e223fa1a..e29a5e60ab 100644 --- a/api/applications/tests/test_update_parties_address.py +++ b/api/applications/tests/test_update_parties_address.py @@ -3,7 +3,7 @@ from django.core.management import call_command from tempfile import NamedTemporaryFile import pytest -from django.core.management.base import CommandError +from api.parties.models import Party from test_helpers.clients import DataTestClient @@ -57,5 +57,5 @@ def test_update_field_on_party_from_csv_invalid(self): tmp_file.write("\n".join(rows).encode("utf-8")) tmp_file.flush() - with pytest.raises(CommandError): + with pytest.raises(Party.DoesNotExist): call_command("update_party_address", tmp_file.name)