From 9c756f58b98577eec176b9c26461ad77928f93c8 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 10:45:37 +0100 Subject: [PATCH 01/58] boilerplate method added --- .../0022_denialentity_entity_type.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/api/external_data/migrations/0022_denialentity_entity_type.py b/api/external_data/migrations/0022_denialentity_entity_type.py index 488f631852..daea0bf0e4 100644 --- a/api/external_data/migrations/0022_denialentity_entity_type.py +++ b/api/external_data/migrations/0022_denialentity_entity_type.py @@ -1,6 +1,36 @@ # Generated by Django 4.2.11 on 2024-04-18 08:45 from django.db import migrations, models +from django.db import IntegrityError +from django.db import transaction +import logging + +log = logging.getLogger(__name__) + + +def set_denial_entity_type(apps, schema_editor): + + DenialEntity = apps.get_model("external_data", "DenialEntity") + duplicate_denial_errors = [] + + with transaction.atomic(): + sid = transaction.savepoint() + for denial_entity in DenialEntity.objects.all(): + try: + # code to set entity type in each record goes here + # denial_entity.entity_type = + denial_entity.save() + except IntegrityError as e: + duplicate_denial_errors.append(denial_entity.entity_type) + + if duplicate_denial_errors: + log.info( + "There are the following duplicate denials in the database rolling back this migration: -> %s", + duplicate_denial_errors, + ) + transaction.savepoint_rollback(sid) + else: + transaction.savepoint_commit(sid) class Migration(migrations.Migration): From 0795df98323282c53141df047f921e2edf24a05d Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 11:10:16 +0100 Subject: [PATCH 02/58] refined method to bare bones --- .../0022_denialentity_entity_type.py | 33 ++++--------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/api/external_data/migrations/0022_denialentity_entity_type.py b/api/external_data/migrations/0022_denialentity_entity_type.py index daea0bf0e4..b2e3f4948a 100644 --- a/api/external_data/migrations/0022_denialentity_entity_type.py +++ b/api/external_data/migrations/0022_denialentity_entity_type.py @@ -1,36 +1,16 @@ # Generated by Django 4.2.11 on 2024-04-18 08:45 from django.db import migrations, models -from django.db import IntegrityError -from django.db import transaction -import logging +from external_data.helpers import get_denial_entity_type -log = logging.getLogger(__name__) - -def set_denial_entity_type(apps, schema_editor): +def set_denial_entity_type(apps): DenialEntity = apps.get_model("external_data", "DenialEntity") - duplicate_denial_errors = [] - - with transaction.atomic(): - sid = transaction.savepoint() - for denial_entity in DenialEntity.objects.all(): - try: - # code to set entity type in each record goes here - # denial_entity.entity_type = - denial_entity.save() - except IntegrityError as e: - duplicate_denial_errors.append(denial_entity.entity_type) - - if duplicate_denial_errors: - log.info( - "There are the following duplicate denials in the database rolling back this migration: -> %s", - duplicate_denial_errors, - ) - transaction.savepoint_rollback(sid) - else: - transaction.savepoint_commit(sid) + + for denial_entity in DenialEntity.objects.all(): + denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) + denial_entity.save() class Migration(migrations.Migration): @@ -51,4 +31,5 @@ class Migration(migrations.Migration): null=True, ), ), + migrations.RunPython(set_denial_entity_type, migrations.RunPython.noop), ] From 446317995c0b13e347f60543a3a347c0066b61fe Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 12:24:46 +0100 Subject: [PATCH 03/58] unresolved dependency repaired --- api/external_data/migrations/0022_denialentity_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/migrations/0022_denialentity_entity_type.py b/api/external_data/migrations/0022_denialentity_entity_type.py index b2e3f4948a..cec64e4c50 100644 --- a/api/external_data/migrations/0022_denialentity_entity_type.py +++ b/api/external_data/migrations/0022_denialentity_entity_type.py @@ -1,7 +1,7 @@ # Generated by Django 4.2.11 on 2024-04-18 08:45 from django.db import migrations, models -from external_data.helpers import get_denial_entity_type +from api.external_data.helpers import get_denial_entity_type def set_denial_entity_type(apps): From 4dde02f8ba95f93bd7683b5fee05e1fe94348ba9 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 13:03:01 +0100 Subject: [PATCH 04/58] new migration added for population of entity_type --- .../0022_denialentity_entity_type.py | 11 ---------- .../migrations/0023_set_denial_entity_type.py | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 11 deletions(-) create mode 100644 api/external_data/migrations/0023_set_denial_entity_type.py diff --git a/api/external_data/migrations/0022_denialentity_entity_type.py b/api/external_data/migrations/0022_denialentity_entity_type.py index cec64e4c50..488f631852 100644 --- a/api/external_data/migrations/0022_denialentity_entity_type.py +++ b/api/external_data/migrations/0022_denialentity_entity_type.py @@ -1,16 +1,6 @@ # Generated by Django 4.2.11 on 2024-04-18 08:45 from django.db import migrations, models -from api.external_data.helpers import get_denial_entity_type - - -def set_denial_entity_type(apps): - - DenialEntity = apps.get_model("external_data", "DenialEntity") - - for denial_entity in DenialEntity.objects.all(): - denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) - denial_entity.save() class Migration(migrations.Migration): @@ -31,5 +21,4 @@ class Migration(migrations.Migration): null=True, ), ), - migrations.RunPython(set_denial_entity_type, migrations.RunPython.noop), ] diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py new file mode 100644 index 0000000000..a2538c421c --- /dev/null +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.11 on 2024-04-18 12:00 + +from django.db import migrations +from api.external_data.helpers import get_denial_entity_type + + +def set_denial_entity_type(apps): + + DenialEntity = apps.get_model("external_data", "DenialEntity") + + for denial_entity in DenialEntity.objects.all(): + denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) + denial_entity.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("external_data", "0022_denialentity_entity_type"), + ] + + operations = [migrations.RunPython(set_denial_entity_type, migrations.RunPython.noop)] From 2ea06c2e503c81802122b7bfd3f1918ab08f76fa Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 13:58:35 +0100 Subject: [PATCH 05/58] localise method in migration --- .../migrations/0023_set_denial_entity_type.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index a2538c421c..33031c62af 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -1,7 +1,24 @@ # Generated by Django 4.2.11 on 2024-04-18 12:00 from django.db import migrations -from api.external_data.helpers import get_denial_entity_type + + +def get_denial_entity_type(data): + entity_type = "" + for key in ["end_user_flag", "consignee_flag", "other_role"]: + if data.get(key) and isinstance(data[key], str): + data[key] = data[key].lower() == "true" + if data.get("end_user_flag", False) is True: + entity_type = "End-user" + elif data.get("end_user_flag", False) is False and data.get("consignee_flag", False) is True: + entity_type = "Consignee" + elif ( + data.get("end_user_flag", False) is False + and data.get("consignee_flag", False) is False + and data.get("other_role", False) is True + ): + entity_type = "Third-party" + return entity_type def set_denial_entity_type(apps): From 12b27c4bd5c3d4a796d6adf4e6526c52de43b26d Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 14:04:28 +0100 Subject: [PATCH 06/58] correct indentation error --- api/external_data/migrations/0023_set_denial_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 33031c62af..5eeb637933 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -18,7 +18,7 @@ def get_denial_entity_type(data): and data.get("other_role", False) is True ): entity_type = "Third-party" - return entity_type + return entity_type def set_denial_entity_type(apps): From d526bf157e66a12a8a429283b8d4ef0200c3a5fe Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Mon, 15 Apr 2024 16:42:17 +0100 Subject: [PATCH 07/58] Update required_headers --- api/external_data/serializers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 9d87629d5b..4286635ded 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -55,8 +55,10 @@ def get_entity_type(self, obj): class DenialFromCSVFileSerializer(serializers.Serializer): csv_file = serializers.CharField() + required_headers = [ "reference", + "regime_reg_ref", "name", "address", "notifying_government", @@ -65,6 +67,8 @@ class DenialFromCSVFileSerializer(serializers.Serializer): "item_description", "consignee_name", "end_use", + "reason_for_refusal", + "spire_entity_id", ] @transaction.atomic From 8e2682da4e93d96e940a7b4bc501d39989924efa Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Mon, 15 Apr 2024 17:41:05 +0100 Subject: [PATCH 08/58] Refactor function validate_csv_file --- api/external_data/serializers.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 4286635ded..72b9d705a8 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -74,30 +74,29 @@ class DenialFromCSVFileSerializer(serializers.Serializer): @transaction.atomic def validate_csv_file(self, value): csv_file = io.StringIO(value) - dialect = csv.Sniffer().sniff(csv_file.read(1024)) - csv_file.seek(0) - reader = csv.reader(csv_file, dialect=dialect) - headers = next(reader, None) + reader = csv.DictReader(csv_file) + + if reader.fieldnames is None: + raise serializers.ValidationError("CSV file is empty or headers are missing") + + # Check if required headers are present + if not (set(self.required_headers)).issubset(set(reader.fieldnames)): + raise serializers.ValidationError("Missing required headers in CSV file") + errors = [] valid_serializers = [] for i, row in enumerate(reader, start=1): - data = dict(zip(headers, row)) - serializer = DenialEntitySerializer( - data={ - "data": data, - "created_by": self.context["request"].user, - **{field: data.pop(field, None) for field in self.required_headers}, - } - ) - + data = {field: row.get(field, None) for field in self.required_headers} + serializer = DenialEntitySerializer(data={"data": data, "created_by": self.context["request"].user, **data}) if serializer.is_valid(): valid_serializers.append(serializer) else: - self.add_bulk_errors(errors=errors, row_number=i + 1, line_errors=serializer.errors) + self.add_bulk_errors(errors, i, serializer.errors) + if errors: raise serializers.ValidationError(errors) else: - # only save if no errors + # Only save if no errors for serializer in valid_serializers: serializer.save() return csv_file From c39d558946113f8af5cb05f9f2600fc6444daf69 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 16 Apr 2024 09:06:09 +0100 Subject: [PATCH 09/58] Use update_or_create method --- api/external_data/serializers.py | 33 ++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 72b9d705a8..e683ced30e 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -1,5 +1,6 @@ import csv import io +import logging from django.db import transaction @@ -76,6 +77,7 @@ def validate_csv_file(self, value): csv_file = io.StringIO(value) reader = csv.DictReader(csv_file) + # Check headers exist if reader.fieldnames is None: raise serializers.ValidationError("CSV file is empty or headers are missing") @@ -84,21 +86,36 @@ def validate_csv_file(self, value): raise serializers.ValidationError("Missing required headers in CSV file") errors = [] - valid_serializers = [] for i, row in enumerate(reader, start=1): - data = {field: row.get(field, None) for field in self.required_headers} - serializer = DenialEntitySerializer(data={"data": data, "created_by": self.context["request"].user, **data}) + data = { + **{field: row.get(field, None) for field in self.required_headers}, + "created_by": self.context["request"].user, + } + # Create a serializer instance to validate data + serializer = DenialEntitySerializer(data=data) if serializer.is_valid(): - valid_serializers.append(serializer) + lookup_fields = { + "reference": row.get("reference"), + "regime_reg_ref": row.get("regime_reg_ref"), + "name": row.get("name"), + "address": row.get("address"), + } + # Try to update an existing record or create a new one + obj, created = models.DenialEntity.objects.update_or_create(defaults=serializer.validated_data, **lookup_fields) # type: ignore + + if created: + logging.info(f"Created new record at row {i}") + + if not created: + logging.info( + f"Updated existing record at row {i} based on reference, regime_reg_ref, name, address" + ) else: self.add_bulk_errors(errors, i, serializer.errors) if errors: raise serializers.ValidationError(errors) - else: - # Only save if no errors - for serializer in valid_serializers: - serializer.save() + return csv_file @staticmethod From 2884a709eb42c1b5e0fa08c6621e58f959e67dc6 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 16 Apr 2024 14:59:09 +0100 Subject: [PATCH 10/58] Use update_or_create to link Denial to DenialEntity --- api/external_data/serializers.py | 59 ++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index e683ced30e..9bdba05ac4 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -1,9 +1,8 @@ import csv import io -import logging +import logging from django.db import transaction - from django_elasticsearch_dsl_drf.serializers import DocumentSerializer from rest_framework import serializers @@ -87,28 +86,50 @@ def validate_csv_file(self, value): errors = [] for i, row in enumerate(reader, start=1): - data = { - **{field: row.get(field, None) for field in self.required_headers}, + denial_entity_data = { + **{field: row[field] for field in self.required_headers}, "created_by": self.context["request"].user, } # Create a serializer instance to validate data - serializer = DenialEntitySerializer(data=data) - if serializer.is_valid(): - lookup_fields = { - "reference": row.get("reference"), - "regime_reg_ref": row.get("regime_reg_ref"), - "name": row.get("name"), - "address": row.get("address"), + serializer = DenialEntitySerializer(data=denial_entity_data) + if serializer.is_valid() and isinstance(serializer.validated_data, dict): + # Try to update an existing Denial record or create a new one + regime_reg_ref = serializer.validated_data["regime_reg_ref"] + denial_data = { + "reference": serializer.validated_data["reference"], + "notifying_government": serializer.validated_data["notifying_government"], + "item_list_codes": serializer.validated_data["item_list_codes"], + "item_description": serializer.validated_data["item_description"], + "end_use": serializer.validated_data["end_use"], + "reason_for_refusal": serializer.validated_data["reason_for_refusal"], } - # Try to update an existing record or create a new one - obj, created = models.DenialEntity.objects.update_or_create(defaults=serializer.validated_data, **lookup_fields) # type: ignore - - if created: - logging.info(f"Created new record at row {i}") - - if not created: + denial, is_denial_created = models.Denial.objects.update_or_create( + regime_reg_ref=regime_reg_ref, defaults=denial_data + ) + + if is_denial_created: + logging.info(f"Created new Denial record at row {i}") + else: + logging.info(f"Updated existing Denial record at row {i} based on regime_reg_ref") + + # We assume that a DenialEntity object already exists if we can + # match on all of the following fields + denial_entity_lookup_fields = { + "reference": serializer.validated_data["reference"], + "regime_reg_ref": regime_reg_ref, + "name": serializer.validated_data["name"], + "address": serializer.validated_data["address"], + } + # Link the validated DenialEntity data with the Denial + denial_entity, is_denial_entity_created = models.DenialEntity.objects.update_or_create( + defaults=serializer.validated_data, denial=denial, **denial_entity_lookup_fields + ) + + if is_denial_entity_created: + logging.info(f"Created new DenialEntity record at row {i}") + else: logging.info( - f"Updated existing record at row {i} based on reference, regime_reg_ref, name, address" + f"Updated existing DenialEntity record at row {i} based on reference, regime_reg_ref, name, address" ) else: self.add_bulk_errors(errors, i, serializer.errors) From 4f561aa75379c94a97b151eecc64963f3daf644f Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 16 Apr 2024 15:57:38 +0100 Subject: [PATCH 11/58] Update test test_populate_denial_entity_objects --- api/external_data/tests/denial_valid.csv | 9 +++---- api/external_data/tests/test_views.py | 34 ++++++++++++------------ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index 4fab6ec7bc..f2229dfae3 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,5 +1,4 @@ -reference,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,end_user_flag,consignee_flag,other_role -FOO123,Jim Example,123 fake street,France,Germany,ABC123,Foo,Fred Food,used in car,true,true,false -BAR123,Jak Example,123 fake street,France,Germany,ABC123,Foo,Fred Food,used in car,false,true,false -BAG124,Bob Example,123 fake street,France,Germany,ABC123,Foo,Fred Food,used in car,false,false,true -BAT123,James Jones,Bob Avenue,France,Germany,ABC123,Foo,Fred Food,used in car,false,false,true +reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id +DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 +DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Example Name 3,Used in other industry,Risk of outcome 3,125 +DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Example Name 2,Used in other industry,Risk of outcome 2,124 diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 661e9c563b..d8bb308bc6 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -125,7 +125,7 @@ def test_create_validation_error_duplicate(self): ) -class DenialSearchView(DataTestClient): +class DenialEntitySearchView(DataTestClient): @pytest.mark.elasticsearch @parameterized.expand( [ @@ -133,7 +133,7 @@ class DenialSearchView(DataTestClient): ({"page": 1},), ] ) - def test_populate_denials(self, page_query): + def test_populate_denial_entity_objects(self, page_query): call_command("search_index", models=["external_data.denialentity"], action="rebuild", force=True) url = reverse("external_data:denial-list") file_path = os.path.join(settings.BASE_DIR, "external_data/tests/denial_valid.csv") @@ -141,28 +141,28 @@ def test_populate_denials(self, page_query): content = f.read() response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 201) - self.assertEqual(models.DenialEntity.objects.count(), 4) + self.assertEqual(models.DenialEntity.objects.count(), 3) - # and one of them is revoked - denial = models.DenialEntity.objects.get(name="Jak Example") - denial.is_revoked = True - denial.save() + # Set one of them as revoked + denial_entity = models.DenialEntity.objects.get(name="Organisation Name") + denial_entity.is_revoked = True + denial_entity.save() - # then only 2 denials will be returned when searching + # Then only 2 denial entity objects will be returned when searching url = reverse("external_data:denial_search-list") - response = self.client.get(url, {**page_query, "search": "name:Example"}, **self.gov_headers) + response = self.client.get(url, {**page_query, "search": "name:Organisation Name XYZ"}, **self.gov_headers) self.assertEqual(response.status_code, 200) response_json = response.json() expected_result = { - "address": "123 fake street", - "country": "Germany", - "item_description": "Foo", - "item_list_codes": "ABC123", - "name": "Jim Example", - "notifying_government": "France", - "end_use": "used in car", - "reference": "FOO123", + "address": "2000 Street Name, City Name 2", + "country": "Country Name 2", + "item_description": "Large Size Widget", + "item_list_codes": "0A00200", + "name": "Organisation Name XYZ", + "notifying_government": "Country Name 2", + "end_use": "Used in other industry", + "reference": "DN3000/0000", } for key, value in expected_result.items(): From e8724727d536a435c130526ee597c3e64118b90a Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 16 Apr 2024 16:05:27 +0100 Subject: [PATCH 12/58] Update test test_denial_entity_search --- api/external_data/tests/denial_valid.csv | 1 + api/external_data/tests/test_views.py | 10 ++++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index f2229dfae3..2856dd541b 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,4 +1,5 @@ reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Example Name 3,Used in other industry,Risk of outcome 3,125 +DN2010/0001,AB-XY-EF-900,The Widget Company,"2 Example Road, Example City",Example Country,Country Name X,"catch all",Extra Large Size Widget,Example Name 4,Used in unknown industry,Risk of outcome 4,126 DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Example Name 2,Used in other industry,Risk of outcome 2,124 diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index d8bb308bc6..e818fd8bbf 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -174,14 +174,12 @@ def test_populate_denial_entity_objects(self, page_query): @pytest.mark.elasticsearch @parameterized.expand( [ - ({"search": "name:Bob"}, 1), - ({"search": "name:Example"}, 3), - ({"search": "name:Jones"}, 1), - ({"search": "address:123 fake street"}, 3), - ({"search": "address:Bob Avenue"}, 1), + ({"search": "name:Organisation Name"}, 3), + ({"search": "name:The Widget Company"}, 1), + ({"search": "name:XYZ"}, 1), ] ) - def test_denial_search(self, query, quantity): + def test_denial_entity_search(self, query, quantity): call_command("search_index", models=["external_data.denialentity"], action="rebuild", force=True) url = reverse("external_data:denial-list") file_path = os.path.join(settings.BASE_DIR, "external_data/tests/denial_valid.csv") From 46f3174e6092379fcf9201fb513b0bdda26f3f58 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 16 Apr 2024 16:07:14 +0100 Subject: [PATCH 13/58] Refactor test file --- api/external_data/tests/test_views.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index e818fd8bbf..ae3b55840a 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -125,7 +125,7 @@ def test_create_validation_error_duplicate(self): ) -class DenialEntitySearchView(DataTestClient): +class DenialSearchViewTests(DataTestClient): @pytest.mark.elasticsearch @parameterized.expand( [ @@ -141,7 +141,7 @@ def test_populate_denial_entity_objects(self, page_query): content = f.read() response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 201) - self.assertEqual(models.DenialEntity.objects.count(), 3) + self.assertEqual(models.DenialEntity.objects.count(), 4) # Set one of them as revoked denial_entity = models.DenialEntity.objects.get(name="Organisation Name") @@ -196,8 +196,6 @@ def test_denial_entity_search(self, query, quantity): response_json = response.json() self.assertEqual(len(response_json["results"]), quantity) - -class DenialSearchViewTests(DataTestClient): @pytest.mark.elasticsearch def test_search(self): Index("sanctions-alias-test").create(ignore=[400]) From 969af8fe2d201fd78419eb9aebc29a78b080cbe9 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 17 Apr 2024 12:18:36 +0100 Subject: [PATCH 14/58] Make changes backwards compatible so tests pass on lite-routing --- api/external_data/serializers.py | 54 ++++++++++++++++++++++++++++++++ api/external_data/views.py | 7 ++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 9bdba05ac4..62cf214703 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -52,6 +52,60 @@ def get_entity_type(self, obj): return get_denial_entity_type(obj.data) +# TODO: this is for backwards compatibility and should be removed +# once the lite-routing test data has been updated +class DenialFromCSVFileOldSerializer(serializers.Serializer): + + csv_file = serializers.CharField() + required_headers = [ + "reference", + "name", + "address", + "notifying_government", + "country", + "item_list_codes", + "item_description", + "consignee_name", + "end_use", + ] + + @transaction.atomic + def validate_csv_file(self, value): + csv_file = io.StringIO(value) + dialect = csv.Sniffer().sniff(csv_file.read(1024)) + csv_file.seek(0) + reader = csv.reader(csv_file, dialect=dialect) + headers = next(reader, None) + errors = [] + valid_serializers = [] + for i, row in enumerate(reader, start=1): + data = dict(zip(headers, row)) + serializer = DenialEntitySerializer( + data={ + "data": data, + "created_by": self.context["request"].user, + **{field: data.pop(field, None) for field in self.required_headers}, + } + ) + + if serializer.is_valid(): + valid_serializers.append(serializer) + else: + self.add_bulk_errors(errors=errors, row_number=i + 1, line_errors=serializer.errors) + if errors: + raise serializers.ValidationError(errors) + else: + # only save if no errors + for serializer in valid_serializers: + serializer.save() + return csv_file + + @staticmethod + def add_bulk_errors(errors, row_number, line_errors): + for key, values in line_errors.items(): + errors.append(f"[Row {row_number}] {key}: {','.join(values)}") + + class DenialFromCSVFileSerializer(serializers.Serializer): csv_file = serializers.CharField() diff --git a/api/external_data/views.py b/api/external_data/views.py index 59eb136ba6..2fd5bbc23e 100644 --- a/api/external_data/views.py +++ b/api/external_data/views.py @@ -19,7 +19,12 @@ class DenialViewSet(viewsets.ModelViewSet): def get_serializer_class(self): if self.action == "create": - return serializers.DenialFromCSVFileSerializer + # TODO: this is for backwards compatibility and should be removed + # once the lite-routing test data has been updated + if bool("regime_reg_ref" not in str(self.request.data["csv_file"])): # type: ignore + return serializers.DenialFromCSVFileOldSerializer + else: + return serializers.DenialFromCSVFileSerializer return serializers.DenialEntitySerializer def perform_create(self, serializer): From a3addc326fd6e5fd9f1fdecea7d52fd1ee668326 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 17 Apr 2024 13:24:59 +0100 Subject: [PATCH 15/58] Update DenialViewSetTests --- api/external_data/tests/test_views.py | 92 +++++++++++++++------------ 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index ae3b55840a..19e477c5be 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -26,52 +26,64 @@ def test_create_success(self): list(models.DenialEntity.objects.values(*serializers.DenialFromCSVFileSerializer.required_headers, "data")), [ { - "address": "123 fake street", - "consignee_name": "Fred Food", - "data": {"end_user_flag": "true", "consignee_flag": "true", "other_role": "false"}, - "country": "Germany", - "item_description": "Foo", - "item_list_codes": "ABC123", - "name": "Jim Example", - "notifying_government": "France", - "end_use": "used in car", - "reference": "FOO123", + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "name": "Organisation Name", + "address": "1000 Street Name, City Name", + "notifying_government": "Country Name", + "country": "Country Name", + "item_list_codes": "0A00100", + "item_description": "Medium Size Widget", + "consignee_name": "Example Name", + "end_use": "Used in industry", + "reason_for_refusal": "Risk of outcome", + "spire_entity_id": 123, + "data": {}, }, { - "address": "123 fake street", - "consignee_name": "Fred Food", - "data": {"end_user_flag": "false", "consignee_flag": "true", "other_role": "false"}, - "country": "Germany", - "item_description": "Foo", - "item_list_codes": "ABC123", - "name": "Jak Example", - "notifying_government": "France", - "end_use": "used in car", - "reference": "BAR123", + "reference": "DN2000/0010", + "regime_reg_ref": "AB-CD-EF-300", + "name": "Organisation Name 3", + "address": "2001 Street Name, City Name 3", + "notifying_government": "Country Name 3", + "country": "Country Name 3", + "item_list_codes": "0A00201", + "item_description": "Unspecified Size Widget", + "consignee_name": "Example Name 3", + "end_use": "Used in other industry", + "reason_for_refusal": "Risk of outcome 3", + "spire_entity_id": 125, + "data": {}, }, { - "address": "123 fake street", - "consignee_name": "Fred Food", - "data": {"end_user_flag": "false", "consignee_flag": "false", "other_role": "true"}, - "country": "Germany", - "item_description": "Foo", - "item_list_codes": "ABC123", - "name": "Bob Example", - "notifying_government": "France", - "end_use": "used in car", - "reference": "BAG124", + "reference": "DN2010/0001", + "regime_reg_ref": "AB-XY-EF-900", + "name": "The Widget Company", + "address": "2 Example Road, Example City", + "notifying_government": "Example Country", + "country": "Country Name X", + "item_list_codes": "catch all", + "item_description": "Extra Large Size Widget", + "consignee_name": "Example Name 4", + "end_use": "Used in unknown industry", + "reason_for_refusal": "Risk of outcome 4", + "spire_entity_id": 126, + "data": {}, }, { - "address": "Bob Avenue", - "consignee_name": "Fred Food", - "data": {"end_user_flag": "false", "consignee_flag": "false", "other_role": "true"}, - "country": "Germany", - "item_description": "Foo", - "item_list_codes": "ABC123", - "name": "James Jones", - "notifying_government": "France", - "end_use": "used in car", - "reference": "BAT123", + "reference": "DN3000/0000", + "regime_reg_ref": "AB-CD-EF-100", + "name": "Organisation Name XYZ", + "address": "2000 Street Name, City Name 2", + "notifying_government": "Country Name 2", + "country": "Country Name 2", + "item_list_codes": "0A00200", + "item_description": "Large Size Widget", + "consignee_name": "Example Name 2", + "end_use": "Used in other industry", + "reason_for_refusal": "Risk of outcome 2", + "spire_entity_id": 124, + "data": {}, }, ], ) From c6d39ca1de9d30759d29847914b066dec88a6142 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 17 Apr 2024 13:59:08 +0100 Subject: [PATCH 16/58] Update ApplicationDenialMatchesOnApplicationTests --- .../tests/test_matching_denials.py | 35 ++++++------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/api/applications/tests/test_matching_denials.py b/api/applications/tests/test_matching_denials.py index f292ff6825..d5a3c6323b 100644 --- a/api/applications/tests/test_matching_denials.py +++ b/api/applications/tests/test_matching_denials.py @@ -1,40 +1,25 @@ -import csv -import io import pytest +import os +from django.conf import settings from django.urls import reverse -from faker import Faker from rest_framework import status from api.applications.tests.factories import DenialMatchFactory -from api.external_data import models, serializers +from api.external_data import models from test_helpers.clients import DataTestClient class ApplicationDenialMatchesOnApplicationTests(DataTestClient): def setUp(self): super().setUp() - self.faker = Faker() self.application = self.create_standard_application_case(self.organisation) - denials = [ - {name: self.faker.word() for name in serializers.DenialFromCSVFileSerializer.required_headers} - for _ in range(5) - ] - - content = io.StringIO() - writer = csv.DictWriter( - content, - fieldnames=[*serializers.DenialFromCSVFileSerializer.required_headers, "field_n"], - delimiter=",", - quoting=csv.QUOTE_MINIMAL, - ) - writer.writeheader() - writer.writerows(denials) - response = self.client.post( - reverse("external_data:denial-list"), {"csv_file": content.getvalue()}, **self.gov_headers - ) + file_path = os.path.join(settings.BASE_DIR, "external_data/tests/denial_valid.csv") + with open(file_path, "rb") as f: + content = f.read() + response = self.client.post(reverse("external_data:denial-list"), {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(models.DenialEntity.objects.count(), 5) + self.assertEqual(models.DenialEntity.objects.count(), 4) @pytest.mark.xfail(reason="This test is flaky and should be rewritten") # Occasionally causes this error: @@ -60,7 +45,7 @@ def test_adding_denials_to_application(self): def test_revoke_denial_without_comment_failure(self): response = self.client.get(reverse("external_data:denial-list"), **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json()["count"], 5) + self.assertEqual(response.json()["count"], 4) denials = response.json()["results"] @@ -77,7 +62,7 @@ def test_revoke_denial_without_comment_failure(self): def test_revoke_denial_success(self): response = self.client.get(reverse("external_data:denial-list"), **self.gov_headers) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.json()["count"], 5) + self.assertEqual(response.json()["count"], 4) denials = response.json()["results"] From 89ee2e6680af8de36c69c786680981dcc33b2fc8 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 17 Apr 2024 14:20:53 +0100 Subject: [PATCH 17/58] Re-add address test cases to test_denial_entity_search --- api/external_data/tests/test_views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 19e477c5be..6933ce3c9f 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -189,6 +189,8 @@ def test_populate_denial_entity_objects(self, page_query): ({"search": "name:Organisation Name"}, 3), ({"search": "name:The Widget Company"}, 1), ({"search": "name:XYZ"}, 1), + ({"search": "address:Street Name"}, 3), + ({"search": "address:Example"}, 1), ] ) def test_denial_entity_search(self, query, quantity): From f8043bdabb0b5d5f43538b226b5639056cc57681 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 17 Apr 2024 17:08:18 +0100 Subject: [PATCH 18/58] Add test for missing headers error --- api/external_data/serializers.py | 6 +----- api/external_data/tests/test_views.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 62cf214703..30db5be6a8 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -130,12 +130,8 @@ def validate_csv_file(self, value): csv_file = io.StringIO(value) reader = csv.DictReader(csv_file) - # Check headers exist - if reader.fieldnames is None: - raise serializers.ValidationError("CSV file is empty or headers are missing") - # Check if required headers are present - if not (set(self.required_headers)).issubset(set(reader.fieldnames)): + if not (set(self.required_headers)).issubset(set(reader.fieldnames)): # type: ignore raise serializers.ValidationError("Missing required headers in CSV file") errors = [] diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 6933ce3c9f..89bd806a22 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -109,6 +109,17 @@ def test_create_validation_error(self): }, ) + def test_create_error_missing_required_headers(self): + url = reverse("external_data:denial-list") + # missing the 'reference' header + content = """ + regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id + AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + """ + response = self.client.post(url, {"csv_file": content}, **self.gov_headers) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), {"errors": {"csv_file": ["Missing required headers in CSV file"]}}) + @pytest.mark.skip( reason="Unique constraint on reference is removed temporarily, enable this test once we reinstate that constraint" ) From 5c7ff8f7d4f67d80b698c9ae5c5c6c7f8ca17436 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 18 Apr 2024 10:59:54 +0100 Subject: [PATCH 19/58] Add test_update_success --- api/external_data/tests/test_views.py | 97 +++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 89bd806a22..8806564f38 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -11,6 +11,16 @@ from api.external_data import documents, models, serializers from test_helpers.clients import DataTestClient +denial_data_fields = [ + "reference", + "regime_reg_ref", + "notifying_government", + "item_list_codes", + "item_description", + "end_use", + "reason_for_refusal", +] + class DenialViewSetTests(DataTestClient): def test_create_success(self): @@ -120,6 +130,93 @@ def test_create_error_missing_required_headers(self): self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {"errors": {"csv_file": ["Missing required headers in CSV file"]}}) + def test_update_success(self): + url = reverse("external_data:denial-list") + content = """ + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + """ + response = self.client.post(url, {"csv_file": content}, **self.gov_headers) + self.assertEqual(response.status_code, 201) + self.assertEqual(models.Denial.objects.count(), 1) + self.assertEqual(models.DenialEntity.objects.count(), 1) + self.assertEqual( + list(models.Denial.objects.values(*denial_data_fields)), + [ + { + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "notifying_government": "Country Name", + "item_list_codes": "0A00100", + "item_description": "Medium Size Widget", + "end_use": "Used in industry", + "reason_for_refusal": "Risk of outcome", + }, + ], + ) + self.assertEqual( + list(models.DenialEntity.objects.values(*serializers.DenialFromCSVFileSerializer.required_headers, "data")), + [ + { + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "name": "Organisation Name", + "address": "1000 Street Name, City Name", + "notifying_government": "Country Name", + "country": "Country Name", + "item_list_codes": "0A00100", + "item_description": "Medium Size Widget", + "consignee_name": "Example Name", + "end_use": "Used in industry", + "reason_for_refusal": "Risk of outcome", + "spire_entity_id": 123, + "data": {}, + }, + ], + ) + updated_content = """ + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name 2,Country Name 2,0A00200,Medium Size Widget 2,Example Name 2,Used in industry 2,Risk of outcome 2,124 + """ + response = self.client.post(url, {"csv_file": updated_content}, **self.gov_headers) + self.assertEqual(response.status_code, 201) + self.assertEqual(models.Denial.objects.count(), 1) + self.assertEqual(models.DenialEntity.objects.count(), 1) + self.assertEqual( + list(models.Denial.objects.values(*denial_data_fields)), + [ + { + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "notifying_government": "Country Name 2", + "item_list_codes": "0A00200", + "item_description": "Medium Size Widget 2", + "end_use": "Used in industry 2", + "reason_for_refusal": "Risk of outcome 2", + }, + ], + ) + self.assertEqual( + list(models.DenialEntity.objects.values(*serializers.DenialFromCSVFileSerializer.required_headers, "data")), + [ + { + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "name": "Organisation Name", + "address": "1000 Street Name, City Name", + "notifying_government": "Country Name 2", + "country": "Country Name 2", + "item_list_codes": "0A00200", + "item_description": "Medium Size Widget 2", + "consignee_name": "Example Name 2", + "end_use": "Used in industry 2", + "reason_for_refusal": "Risk of outcome 2", + "spire_entity_id": 124, + "data": {}, + }, + ], + ) + @pytest.mark.skip( reason="Unique constraint on reference is removed temporarily, enable this test once we reinstate that constraint" ) From 861383f9f1222afe3d7e7cb86431bdff912281da Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 18 Apr 2024 11:16:31 +0100 Subject: [PATCH 20/58] Update DenialEntitySerializer validation --- api/external_data/serializers.py | 5 +++++ api/external_data/tests/test_views.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 30db5be6a8..624df1a9a5 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -46,6 +46,11 @@ def validate(self, data): validated_data = super().validate(data) if validated_data.get("is_revoked") and not validated_data.get("is_revoked_comment"): raise serializers.ValidationError({"is_revoked_comment": "This field is required"}) + # TODO: this uses DenialFromCSVFileOldSerializer instead of + # DenialFromCSVFileSerializer which is for backwards compatibility, + # this should be updated once lite-routing test data has been updated + if not (set(DenialFromCSVFileOldSerializer.required_headers)).issubset(set(validated_data.keys())): + raise serializers.ValidationError("Missing required fields in data") return validated_data def get_entity_type(self, obj): diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 8806564f38..b3bd9c491d 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -217,6 +217,16 @@ def test_update_success(self): ], ) + def test_create_error_serializer_errors(self): + url = reverse("external_data:denial-list") + content = """ + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id + ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + """ + response = self.client.post(url, {"csv_file": content}, **self.gov_headers) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.json(), {"errors": {"csv_file": ["[Row 1] reference: This field may not be blank."]}}) + @pytest.mark.skip( reason="Unique constraint on reference is removed temporarily, enable this test once we reinstate that constraint" ) From adae234a8fea10463672728106d656794ef3b78c Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 18 Apr 2024 11:43:57 +0100 Subject: [PATCH 21/58] Fix test_revoke_denial_success --- api/external_data/serializers.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 624df1a9a5..a52fd02f36 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -44,13 +44,15 @@ class Meta: def validate(self, data): validated_data = super().validate(data) - if validated_data.get("is_revoked") and not validated_data.get("is_revoked_comment"): - raise serializers.ValidationError({"is_revoked_comment": "This field is required"}) - # TODO: this uses DenialFromCSVFileOldSerializer instead of - # DenialFromCSVFileSerializer which is for backwards compatibility, - # this should be updated once lite-routing test data has been updated - if not (set(DenialFromCSVFileOldSerializer.required_headers)).issubset(set(validated_data.keys())): - raise serializers.ValidationError("Missing required fields in data") + if validated_data.get("is_revoked"): + if not validated_data.get("is_revoked_comment"): + raise serializers.ValidationError({"is_revoked_comment": "This field is required"}) + else: + # TODO: this uses DenialFromCSVFileOldSerializer instead of + # DenialFromCSVFileSerializer which is for backwards compatibility, + # this should be updated once lite-routing test data has been updated + if not (set(DenialFromCSVFileOldSerializer.required_headers)).issubset(set(validated_data.keys())): + raise serializers.ValidationError("Missing required fields in data") return validated_data def get_entity_type(self, obj): From dcb34b88b86c349efd518895896e44292e24a5d9 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 18 Apr 2024 12:18:13 +0100 Subject: [PATCH 22/58] Update serializer tests --- api/external_data/serializers.py | 11 ++--------- api/external_data/tests/test_views.py | 3 +-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index a52fd02f36..30db5be6a8 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -44,15 +44,8 @@ class Meta: def validate(self, data): validated_data = super().validate(data) - if validated_data.get("is_revoked"): - if not validated_data.get("is_revoked_comment"): - raise serializers.ValidationError({"is_revoked_comment": "This field is required"}) - else: - # TODO: this uses DenialFromCSVFileOldSerializer instead of - # DenialFromCSVFileSerializer which is for backwards compatibility, - # this should be updated once lite-routing test data has been updated - if not (set(DenialFromCSVFileOldSerializer.required_headers)).issubset(set(validated_data.keys())): - raise serializers.ValidationError("Missing required fields in data") + if validated_data.get("is_revoked") and not validated_data.get("is_revoked_comment"): + raise serializers.ValidationError({"is_revoked_comment": "This field is required"}) return validated_data def get_entity_type(self, obj): diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index b3bd9c491d..7d1c8708e1 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -219,8 +219,7 @@ def test_update_success(self): def test_create_error_serializer_errors(self): url = reverse("external_data:denial-list") - content = """ - reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id + content = """reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 """ response = self.client.post(url, {"csv_file": content}, **self.gov_headers) From 907b97fe6b8604da2ef8d8009984ea48e62684cb Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Thu, 18 Apr 2024 15:35:46 +0100 Subject: [PATCH 23/58] Update to log regime_reg_ref values --- api/external_data/serializers.py | 45 +++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 30db5be6a8..219c3506e7 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -134,6 +134,11 @@ def validate_csv_file(self, value): if not (set(self.required_headers)).issubset(set(reader.fieldnames)): # type: ignore raise serializers.ValidationError("Missing required headers in CSV file") + logging_counts = {"denial": {"created": 0, "updated": 0}, "denial_entity": {"created": 0, "updated": 0}} + logging_regime_reg_ref_values = { + "denial": {"created": [], "updated": []}, + "denial_entity": {"created": [], "updated": []}, + } errors = [] for i, row in enumerate(reader, start=1): denial_entity_data = { @@ -158,9 +163,11 @@ def validate_csv_file(self, value): ) if is_denial_created: - logging.info(f"Created new Denial record at row {i}") + logging_counts["denial"]["created"] += 1 + logging_regime_reg_ref_values["denial"]["created"].append(denial.regime_reg_ref) else: - logging.info(f"Updated existing Denial record at row {i} based on regime_reg_ref") + logging_counts["denial"]["updated"] += 1 + logging_regime_reg_ref_values["denial"]["updated"].append(denial.regime_reg_ref) # We assume that a DenialEntity object already exists if we can # match on all of the following fields @@ -176,17 +183,43 @@ def validate_csv_file(self, value): ) if is_denial_entity_created: - logging.info(f"Created new DenialEntity record at row {i}") + logging_counts["denial_entity"]["created"] += 1 + logging_regime_reg_ref_values["denial_entity"]["created"].append(denial_entity.regime_reg_ref) else: - logging.info( - f"Updated existing DenialEntity record at row {i} based on reference, regime_reg_ref, name, address" - ) + logging_counts["denial_entity"]["updated"] += 1 + logging_regime_reg_ref_values["denial_entity"]["updated"].append(denial_entity.regime_reg_ref) else: self.add_bulk_errors(errors, i, serializer.errors) if errors: raise serializers.ValidationError(errors) + if logging_counts["denial"]["created"]: + logging.info( + "Created %s Denial records with regime_reg_ref values:\n%s", + logging_counts["denial"]["created"], + "\n".join(logging_regime_reg_ref_values["denial"]["created"]), + ) + if logging_counts["denial"]["updated"]: + logging.info( + "Updated %s Denial records with regime_reg_ref values:\n%s", + logging_counts["denial"]["updated"], + "\n".join(logging_regime_reg_ref_values["denial"]["updated"]), + ) + + if logging_counts["denial_entity"]["created"]: + logging.info( + "Created %s DenialEntity records with regime_reg_ref values:\n%s", + logging_counts["denial_entity"]["created"], + "\n".join(logging_regime_reg_ref_values["denial_entity"]["created"]), + ) + if logging_counts["denial_entity"]["updated"]: + logging.info( + "Updated %s DenialEntity records with regime_reg_ref values:\n%s", + logging_counts["denial_entity"]["updated"], + "\n".join(logging_regime_reg_ref_values["denial_entity"]["updated"]), + ) + return csv_file @staticmethod From 0ebd46696aef9173edb12f26bbc508d098406675 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 15:41:47 +0100 Subject: [PATCH 24/58] testing first pass and add schema _editor back in to see if that helps with tests failing --- .../migrations/0023_set_denial_entity_type.py | 2 +- .../tests/test_0023_set_denial_entity_type.py | 95 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 api/external_data/tests/test_0023_set_denial_entity_type.py diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 5eeb637933..91d3761532 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -21,7 +21,7 @@ def get_denial_entity_type(data): return entity_type -def set_denial_entity_type(apps): +def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py new file mode 100644 index 0000000000..5c148a8fef --- /dev/null +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -0,0 +1,95 @@ +import pytest + +from django_test_migrations.contrib.unittest_case import MigratorTestCase + + +test_data = [ + { + "reference": "DN2010\/0057", + "regime_reg_ref": "reg.123.123", + "name": "name 1", + "address": "address 1", + "notifying_government": "UK", + "country": "UK", + "item_list_codes": "all", + "item_description": "desc a", + "end_use": "use 1", + "reason_for_refusal": "a", + "entity_type": {"end_user_flag": True, "consignee_flag": False}, + "other_role": False, + }, + { + "reference": "DN2010\/0057", + "regime_reg_ref": "reg.123.1234", + "name": "name 2", + "address": "address 2", + "notifying_government": "UK", + "country": "UK", + "item_list_codes": "all", + "item_description": "desc a", + "end_use": "use 1", + "reason_for_refusal": "a", + "entity_type": {"end_user_flag": False, "consignee_flag": True}, + "other_role": False, + }, + { + "reference": "DN2010\/0057", + "regime_reg_ref": "reg.123.1234", + "name": "name 3", + "address": "address 3", + "notifying_government": "UK", + "country": "UK", + "item_list_codes": "all", + "item_description": "desc a", + "end_use": "use 1", + "reason_for_refusal": "a", + "entity_type": {"end_user_flag": False, "consignee_flag": False}, + "other_role": True, + }, + { + "reference": "DN2010\/0057", + "regime_reg_ref": "reg.123.1234", + "name": "name 4", + "address": "address 4", + "notifying_government": "UK", + "country": "UK", + "item_list_codes": "all", + "item_description": "desc a", + "end_use": "use 1", + "reason_for_refusal": "a", + "entity_type": {"end_user_flag": False, "consignee_flag": False}, + "other_role": False, + }, + { + "reference": "DN2010\/0057", + "name": "bad record", + "address": "bad record", + "notifying_government": "UK", + "country": "bad", + "item_list_codes": "all", + "item_description": "bad", + "end_use": "bad", + "reason_for_refusal": "bad ", + }, +] + + +@pytest.mark.django_db() +class TestDenialEntityTypeSet(MigratorTestCase): + migrate_from = ("external_data", "0022_denialentity_entity_type") + migrate_to = ("external_data", "0023_set_denial_entity_type") + + def prepare(self): + DenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity") + for row in test_data: + DenialEntity.objects.create(**row) + + def test_0023_set_denial_entity_type(self): + OldDenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity") + NewDenialEntity = self.new_state.apps.get_model("external_data", "NewDenialEntity") + + assert OldDenialEntity.objects.all().count() == 4 + assert NewDenialEntity.objects.all().count() == 3 + assert NewDenialEntity.objects.get(entity_type="End-user").denial_entity.count() == 1 + assert NewDenialEntity.objects.get(entity_type="Consignee").denial_entity.count() == 1 + assert NewDenialEntity.objects.get(entity_type="Third-party").denial_entity.count() == 1 From e298bf8f0b047bd334662161e47b6e6a2ee994fc Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 18 Apr 2024 17:03:55 +0100 Subject: [PATCH 25/58] test data update --- .../tests/test_0023_set_denial_entity_type.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 5c148a8fef..0a4d6df26d 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -15,8 +15,7 @@ "item_description": "desc a", "end_use": "use 1", "reason_for_refusal": "a", - "entity_type": {"end_user_flag": True, "consignee_flag": False}, - "other_role": False, + "entity_type": {"end_user_flag": True, "consignee_flag": False, "other_role": False}, }, { "reference": "DN2010\/0057", @@ -29,8 +28,7 @@ "item_description": "desc a", "end_use": "use 1", "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": True}, - "other_role": False, + "entity_type": {"end_user_flag": False, "consignee_flag": True, "other_role": False}, }, { "reference": "DN2010\/0057", @@ -43,8 +41,7 @@ "item_description": "desc a", "end_use": "use 1", "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": False}, - "other_role": True, + "entity_type": {"end_user_flag": False, "consignee_flag": False, "other_role": True}, }, { "reference": "DN2010\/0057", @@ -57,8 +54,7 @@ "item_description": "desc a", "end_use": "use 1", "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": False}, - "other_role": False, + "entity_type": {"end_user_flag": False, "consignee_flag": False, "other_role": False}, }, { "reference": "DN2010\/0057", From fb6e3544faf4dc6bda783ef1234ffc1b9bed338e Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Fri, 19 Apr 2024 13:51:24 +0100 Subject: [PATCH 26/58] methods amended to reflect json data coming from spire --- .../migrations/0023_set_denial_entity_type.py | 47 +++++++++--- .../tests/test_0023_set_denial_entity_type.py | 71 +++---------------- 2 files changed, 46 insertions(+), 72 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 91d3761532..3038c23109 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -2,32 +2,59 @@ from django.db import migrations +# from django.db import transaction +# import logging + + +# log = logging.getLogger(__name__) + def get_denial_entity_type(data): entity_type = "" - for key in ["end_user_flag", "consignee_flag", "other_role"]: - if data.get(key) and isinstance(data[key], str): - data[key] = data[key].lower() == "true" - if data.get("end_user_flag", False) is True: + + normalised_entity_type_dict = {keys.lower() and str(values.lower()): values for keys, values in dict(data).items()} + + if normalised_entity_type_dict.get("end_user_flag", "false") == "true": entity_type = "End-user" - elif data.get("end_user_flag", False) is False and data.get("consignee_flag", False) is True: + elif ( + normalised_entity_type_dict.get("end_user_flag", "false") == "false" + and normalised_entity_type_dict.get("consignee_flag", "false") == "true" + ): entity_type = "Consignee" elif ( - data.get("end_user_flag", False) is False - and data.get("consignee_flag", False) is False - and data.get("other_role", False) is True + normalised_entity_type_dict.get("end_user_flag", "false") == "false" + and normalised_entity_type_dict.get("consignee_flag", "false") == "false" + and isinstance(normalised_entity_type_dict["other_role"], str) ): entity_type = "Third-party" + return entity_type def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") - for denial_entity in DenialEntity.objects.all(): - denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) + denial_entity.entity_type = get_denial_entity_type(denial_entity.data) denial_entity.save() + # entity_type_attribute_errors = [] + + # with transaction.atomic(): + # sid = transaction.savepoint() + # for denial_entity in DenialEntity.objects.all(): + # try: + # denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) + # denial_entity.save() + # except AttributeError as e: + # entity_type_attribute_errors.append(denial_entity.entity_type) + # if entity_type_attribute_errors: + # log.info( + # "There are the following denials entity_type_errors in the database rolling back this migration: -> %s", + # entity_type_attribute_errors, + # ) + # transaction.savepoint_rollback(sid) + # else: + # transaction.savepoint_commit(sid) class Migration(migrations.Migration): diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 0a4d6df26d..8b166b2713 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -5,67 +5,16 @@ test_data = [ { - "reference": "DN2010\/0057", - "regime_reg_ref": "reg.123.123", - "name": "name 1", - "address": "address 1", - "notifying_government": "UK", - "country": "UK", - "item_list_codes": "all", - "item_description": "desc a", - "end_use": "use 1", - "reason_for_refusal": "a", - "entity_type": {"end_user_flag": True, "consignee_flag": False, "other_role": False}, + "entity_type": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, }, { - "reference": "DN2010\/0057", - "regime_reg_ref": "reg.123.1234", - "name": "name 2", - "address": "address 2", - "notifying_government": "UK", - "country": "UK", - "item_list_codes": "all", - "item_description": "desc a", - "end_use": "use 1", - "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": True, "other_role": False}, + "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, }, { - "reference": "DN2010\/0057", - "regime_reg_ref": "reg.123.1234", - "name": "name 3", - "address": "address 3", - "notifying_government": "UK", - "country": "UK", - "item_list_codes": "all", - "item_description": "desc a", - "end_use": "use 1", - "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": False, "other_role": True}, + "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, }, { - "reference": "DN2010\/0057", - "regime_reg_ref": "reg.123.1234", - "name": "name 4", - "address": "address 4", - "notifying_government": "UK", - "country": "UK", - "item_list_codes": "all", - "item_description": "desc a", - "end_use": "use 1", - "reason_for_refusal": "a", - "entity_type": {"end_user_flag": False, "consignee_flag": False, "other_role": False}, - }, - { - "reference": "DN2010\/0057", - "name": "bad record", - "address": "bad record", - "notifying_government": "UK", - "country": "bad", - "item_list_codes": "all", - "item_description": "bad", - "end_use": "bad", - "reason_for_refusal": "bad ", + "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": "other"}, }, ] @@ -81,11 +30,9 @@ def prepare(self): DenialEntity.objects.create(**row) def test_0023_set_denial_entity_type(self): - OldDenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity") - NewDenialEntity = self.new_state.apps.get_model("external_data", "NewDenialEntity") + DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") - assert OldDenialEntity.objects.all().count() == 4 - assert NewDenialEntity.objects.all().count() == 3 - assert NewDenialEntity.objects.get(entity_type="End-user").denial_entity.count() == 1 - assert NewDenialEntity.objects.get(entity_type="Consignee").denial_entity.count() == 1 - assert NewDenialEntity.objects.get(entity_type="Third-party").denial_entity.count() == 1 + assert DenialEntity.objects.all().count() == 4 + assert DenialEntity.objects.get(entity_type="End-user").denial_entity.count() == 1 + assert DenialEntity.objects.get(entity_type="Consignee").denial_entity.count() == 1 + assert DenialEntity.objects.get(entity_type="Third-party").denial_entity.count() == 2 From 2bbf0e793b7a83eba769d0304b74780f97c260ba Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Fri, 19 Apr 2024 15:03:05 +0100 Subject: [PATCH 27/58] normalise keys and values in data --- api/external_data/migrations/0023_set_denial_entity_type.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 3038c23109..137c225bbb 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -12,9 +12,9 @@ def get_denial_entity_type(data): entity_type = "" - normalised_entity_type_dict = {keys.lower() and str(values.lower()): values for keys, values in dict(data).items()} + normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} - if normalised_entity_type_dict.get("end_user_flag", "false") == "true": + if normalised_entity_type_dict.get("end_user_flag") == "true": entity_type = "End-user" elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "false" From b87940e8e54ed8e043f5e6ed7559af5dc1423135 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Fri, 19 Apr 2024 18:18:49 +0100 Subject: [PATCH 28/58] tests passing --- .../tests/test_0023_set_denial_entity_type.py | 67 ++++++++++++++++--- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 8b166b2713..e7e63a1eda 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -2,19 +2,70 @@ from django_test_migrations.contrib.unittest_case import MigratorTestCase - test_data = [ { - "entity_type": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "name": "Organisation Name", + "address": "1000 Street Name, City Name", + "notifying_government": "Country Name", + "country": "Country Name", + "item_list_codes": "0A00100", + "item_description": "Medium Size Widget", + "consignee_name": "Example Name", + "end_use": "Used in industry", + "reason_for_refusal": "Risk of outcome", + "spire_entity_id": 123, + "data": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, + "entity_type": "", }, { - "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, + "reference": "DN2000/0010", + "regime_reg_ref": "AB-CD-EF-300", + "name": "Organisation Name 3", + "address": "2001 Street Name, City Name 3", + "notifying_government": "Country Name 3", + "country": "Country Name 3", + "item_list_codes": "0A00201", + "item_description": "Unspecified Size Widget", + "consignee_name": "Example Name 3", + "end_use": "Used in other industry", + "reason_for_refusal": "Risk of outcome 3", + "spire_entity_id": 125, + "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, + "entity_type": "", }, { - "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, + "reference": "DN2010/0001", + "regime_reg_ref": "AB-XY-EF-900", + "name": "The Widget Company", + "address": "2 Example Road, Example City", + "notifying_government": "Example Country", + "country": "Country Name X", + "item_list_codes": "catch all", + "item_description": "Extra Large Size Widget", + "consignee_name": "Example Name 4", + "end_use": "Used in unknown industry", + "reason_for_refusal": "Risk of outcome 4", + "spire_entity_id": 126, + "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, + "entity_type": "", }, { - "entity_type": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": "other"}, + "reference": "DN3000/0000", + "regime_reg_ref": "AB-CD-EF-100", + "name": "Organisation Name XYZ", + "address": "2000 Street Name, City Name 2", + "notifying_government": "Country Name 2", + "country": "Country Name 2", + "item_list_codes": "0A00200", + "item_description": "Large Size Widget", + "consignee_name": "Example Name 2", + "end_use": "Used in other industry", + "reason_for_refusal": "Risk of outcome 2", + "spire_entity_id": 124, + "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": "other"}, + "entity_type": "", }, ] @@ -33,6 +84,6 @@ def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.get(entity_type="End-user").denial_entity.count() == 1 - assert DenialEntity.objects.get(entity_type="Consignee").denial_entity.count() == 1 - assert DenialEntity.objects.get(entity_type="Third-party").denial_entity.count() == 2 + assert DenialEntity.objects.filter(entity_type="End-user").count() == 1 + assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 + assert DenialEntity.objects.filter(entity_type="Third-party").count() == 2 From cfadba426152c08078528230c4c021d387cefd3c Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Mon, 22 Apr 2024 09:00:33 +0100 Subject: [PATCH 29/58] tidy --- .../migrations/0023_set_denial_entity_type.py | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 137c225bbb..59c2c59a8b 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -10,8 +10,8 @@ def get_denial_entity_type(data): - entity_type = "" + entity_type = "" normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} if normalised_entity_type_dict.get("end_user_flag") == "true": @@ -37,24 +37,6 @@ def set_denial_entity_type(apps, schema_editor): for denial_entity in DenialEntity.objects.all(): denial_entity.entity_type = get_denial_entity_type(denial_entity.data) denial_entity.save() - # entity_type_attribute_errors = [] - - # with transaction.atomic(): - # sid = transaction.savepoint() - # for denial_entity in DenialEntity.objects.all(): - # try: - # denial_entity.entity_type = get_denial_entity_type(denial_entity.entity_type) - # denial_entity.save() - # except AttributeError as e: - # entity_type_attribute_errors.append(denial_entity.entity_type) - # if entity_type_attribute_errors: - # log.info( - # "There are the following denials entity_type_errors in the database rolling back this migration: -> %s", - # entity_type_attribute_errors, - # ) - # transaction.savepoint_rollback(sid) - # else: - # transaction.savepoint_commit(sid) class Migration(migrations.Migration): From 6c06cf9e34d4d729d817b04d0deff62ae396d90f Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Mon, 22 Apr 2024 13:18:03 +0100 Subject: [PATCH 30/58] getting entity type only runs if there is no entity type --- .../migrations/0023_set_denial_entity_type.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 59c2c59a8b..21426b07a2 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -2,12 +2,6 @@ from django.db import migrations -# from django.db import transaction -# import logging - - -# log = logging.getLogger(__name__) - def get_denial_entity_type(data): @@ -35,8 +29,9 @@ def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") for denial_entity in DenialEntity.objects.all(): - denial_entity.entity_type = get_denial_entity_type(denial_entity.data) - denial_entity.save() + if not denial_entity.entity_type: + denial_entity.entity_type = get_denial_entity_type(denial_entity.data) + denial_entity.save() class Migration(migrations.Migration): From d333030515962afd858b09cb0c37834457199913 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Mon, 22 Apr 2024 16:13:32 +0100 Subject: [PATCH 31/58] improved db call --- .../migrations/0023_set_denial_entity_type.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 21426b07a2..81000c141d 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -28,10 +28,9 @@ def get_denial_entity_type(data): def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") - for denial_entity in DenialEntity.objects.all(): - if not denial_entity.entity_type: - denial_entity.entity_type = get_denial_entity_type(denial_entity.data) - denial_entity.save() + for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): + denial_entity.entity_type = get_denial_entity_type(denial_entity.data) + denial_entity.save() class Migration(migrations.Migration): From 69d3da946db74d58e13c4324720c71f1713d96c3 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 09:17:52 +0100 Subject: [PATCH 32/58] changed filtering and updating on entity_type method --- api/external_data/migrations/0023_set_denial_entity_type.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 81000c141d..b3621baba2 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -29,8 +29,9 @@ def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): - denial_entity.entity_type = get_denial_entity_type(denial_entity.data) - denial_entity.save() + denial_entity_type = get_denial_entity_type(denial_entity.data) + if denial_entity_type in ["End-user", "Consignee", "Third-party"]: + denial_entity.update(entity_type=denial_entity_type) class Migration(migrations.Migration): From 43708bba8810de8b6e0d134439ac502853069b79 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 09:18:50 +0100 Subject: [PATCH 33/58] amended tests to reflect method --- .../tests/test_0023_set_denial_entity_type.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index e7e63a1eda..1c4d4d4855 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -35,22 +35,6 @@ "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, "entity_type": "", }, - { - "reference": "DN2010/0001", - "regime_reg_ref": "AB-XY-EF-900", - "name": "The Widget Company", - "address": "2 Example Road, Example City", - "notifying_government": "Example Country", - "country": "Country Name X", - "item_list_codes": "catch all", - "item_description": "Extra Large Size Widget", - "consignee_name": "Example Name 4", - "end_use": "Used in unknown industry", - "reason_for_refusal": "Risk of outcome 4", - "spire_entity_id": 126, - "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, - "entity_type": "", - }, { "reference": "DN3000/0000", "regime_reg_ref": "AB-CD-EF-100", @@ -86,4 +70,4 @@ def test_0023_set_denial_entity_type(self): assert DenialEntity.objects.all().count() == 4 assert DenialEntity.objects.filter(entity_type="End-user").count() == 1 assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 - assert DenialEntity.objects.filter(entity_type="Third-party").count() == 2 + assert DenialEntity.objects.filter(entity_type="Third-party").count() == 1 From d4f57c7ca18d4b2c4ca96a9db8e800a67a93bb84 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 09:25:27 +0100 Subject: [PATCH 34/58] handled condition where end_user and consignee flags are true --- .../migrations/0023_set_denial_entity_type.py | 5 +++++ .../tests/test_0023_set_denial_entity_type.py | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index b3621baba2..0e330d6ab3 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -15,6 +15,11 @@ def get_denial_entity_type(data): and normalised_entity_type_dict.get("consignee_flag", "false") == "true" ): entity_type = "Consignee" + elif ( + normalised_entity_type_dict.get("end_user_flag", "false") == "true" + and normalised_entity_type_dict.get("consignee_flag", "false") == "true" + ): + entity_type = "End-user" elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "false" and normalised_entity_type_dict.get("consignee_flag", "false") == "false" diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 1c4d4d4855..7a87674c8e 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -35,6 +35,22 @@ "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, "entity_type": "", }, + { + "reference": "DN2000/0000", + "regime_reg_ref": "AB-CD-EF-000", + "name": "Organisation Name", + "address": "1000 Street Name, City Name", + "notifying_government": "Country Name", + "country": "Country Name", + "item_list_codes": "0A00100", + "item_description": "Medium Size Widget", + "consignee_name": "Example Name", + "end_use": "Used in industry", + "reason_for_refusal": "Risk of outcome", + "spire_entity_id": 123, + "data": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, + "entity_type": "", + }, { "reference": "DN3000/0000", "regime_reg_ref": "AB-CD-EF-100", @@ -68,6 +84,6 @@ def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.filter(entity_type="End-user").count() == 1 + assert DenialEntity.objects.filter(entity_type="End-user").count() == 2 assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 assert DenialEntity.objects.filter(entity_type="Third-party").count() == 1 From 73dad0116733521eac9f06e5ac966383b53ead99 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 09:44:50 +0100 Subject: [PATCH 35/58] failing test updated --- api/external_data/tests/test_0023_set_denial_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 7a87674c8e..3be131d0f7 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -83,7 +83,7 @@ def prepare(self): def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") - assert DenialEntity.objects.all().count() == 4 + assert DenialEntity.objects.all().count() == 3 assert DenialEntity.objects.filter(entity_type="End-user").count() == 2 assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 assert DenialEntity.objects.filter(entity_type="Third-party").count() == 1 From d4fa65dc8205bda98f0178faa967e96cdeabaf13 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 09:58:47 +0100 Subject: [PATCH 36/58] Update test_0023_set_denial_entity_type.py --- api/external_data/tests/test_0023_set_denial_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 3be131d0f7..7a87674c8e 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -83,7 +83,7 @@ def prepare(self): def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") - assert DenialEntity.objects.all().count() == 3 + assert DenialEntity.objects.all().count() == 4 assert DenialEntity.objects.filter(entity_type="End-user").count() == 2 assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 assert DenialEntity.objects.filter(entity_type="Third-party").count() == 1 From 8e82a40797b548b8d8b9bb84f5e5db72e2434274 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 12:12:58 +0100 Subject: [PATCH 37/58] still using save but filtering is better any test data should be better --- .../migrations/0023_set_denial_entity_type.py | 11 ++++++++--- .../tests/test_0023_set_denial_entity_type.py | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 0e330d6ab3..a4684f243e 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -8,7 +8,10 @@ def get_denial_entity_type(data): entity_type = "" normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} - if normalised_entity_type_dict.get("end_user_flag") == "true": + if ( + normalised_entity_type_dict.get("end_user_flag") == "true" + and normalised_entity_type_dict.get("consignee_flag", "false") == "false" + ): entity_type = "End-user" elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "false" @@ -33,10 +36,12 @@ def get_denial_entity_type(data): def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") - for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): + filtered_denial_entities_queryset = DenialEntity.objects.filter(entity_type__isnull=True) + + for denial_entity in filtered_denial_entities_queryset: denial_entity_type = get_denial_entity_type(denial_entity.data) if denial_entity_type in ["End-user", "Consignee", "Third-party"]: - denial_entity.update(entity_type=denial_entity_type) + denial_entity.save(denial_entity_type) class Migration(migrations.Migration): diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 7a87674c8e..3d8d41b9f1 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -17,7 +17,7 @@ "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, "data": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": ""}, - "entity_type": "", + "entity_type": None, }, { "reference": "DN2000/0010", @@ -33,7 +33,7 @@ "reason_for_refusal": "Risk of outcome 3", "spire_entity_id": 125, "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, - "entity_type": "", + "entity_type": None, }, { "reference": "DN2000/0000", @@ -49,7 +49,7 @@ "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, "data": {"END_USER_FLAG": "true", "CONSIGNEE_FLAG": "true", "OTHER_ROLE": ""}, - "entity_type": "", + "entity_type": None, }, { "reference": "DN3000/0000", @@ -65,7 +65,7 @@ "reason_for_refusal": "Risk of outcome 2", "spire_entity_id": 124, "data": {"END_USER_FLAG": "false", "CONSIGNEE_FLAG": "false", "OTHER_ROLE": "other"}, - "entity_type": "", + "entity_type": None, }, ] From e25dbf89a9908600a22a25d10cc41cb86edda19d Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 12:34:55 +0100 Subject: [PATCH 38/58] Update 0023_set_denial_entity_type.py --- .../migrations/0023_set_denial_entity_type.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index a4684f243e..e91939438b 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -1,6 +1,7 @@ # Generated by Django 4.2.11 on 2024-04-18 12:00 from django.db import migrations +from api.external_data.enums import DenialEntityType def get_denial_entity_type(data): @@ -12,23 +13,23 @@ def get_denial_entity_type(data): normalised_entity_type_dict.get("end_user_flag") == "true" and normalised_entity_type_dict.get("consignee_flag", "false") == "false" ): - entity_type = "End-user" + entity_type = DenialEntityType.END_USER elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "false" and normalised_entity_type_dict.get("consignee_flag", "false") == "true" ): - entity_type = "Consignee" + entity_type = DenialEntityType.CONSIGNEE elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "true" and normalised_entity_type_dict.get("consignee_flag", "false") == "true" ): - entity_type = "End-user" + entity_type = DenialEntityType.END_USER elif ( normalised_entity_type_dict.get("end_user_flag", "false") == "false" and normalised_entity_type_dict.get("consignee_flag", "false") == "false" and isinstance(normalised_entity_type_dict["other_role"], str) ): - entity_type = "Third-party" + entity_type = DenialEntityType.THIRD_PARTY return entity_type @@ -37,7 +38,6 @@ def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") filtered_denial_entities_queryset = DenialEntity.objects.filter(entity_type__isnull=True) - for denial_entity in filtered_denial_entities_queryset: denial_entity_type = get_denial_entity_type(denial_entity.data) if denial_entity_type in ["End-user", "Consignee", "Third-party"]: From 73fff84d9e19e5c238c1152930f988d6c9add475 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 12:36:41 +0100 Subject: [PATCH 39/58] Update 0023_set_denial_entity_type.py --- api/external_data/migrations/0023_set_denial_entity_type.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index e91939438b..82cb49501e 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -37,8 +37,9 @@ def get_denial_entity_type(data): def set_denial_entity_type(apps, schema_editor): DenialEntity = apps.get_model("external_data", "DenialEntity") - filtered_denial_entities_queryset = DenialEntity.objects.filter(entity_type__isnull=True) - for denial_entity in filtered_denial_entities_queryset: + + for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): + denial_entity_type = get_denial_entity_type(denial_entity.data) if denial_entity_type in ["End-user", "Consignee", "Third-party"]: denial_entity.save(denial_entity_type) From 0b27c01e660d2a9ec8d663d3af4df4ce362e98de Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 23 Apr 2024 13:53:27 +0100 Subject: [PATCH 40/58] Remove old denial csv file serializer --- api/external_data/serializers.py | 54 -------------------------------- api/external_data/views.py | 7 +---- 2 files changed, 1 insertion(+), 60 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index 219c3506e7..b8ab96f9cd 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -52,60 +52,6 @@ def get_entity_type(self, obj): return get_denial_entity_type(obj.data) -# TODO: this is for backwards compatibility and should be removed -# once the lite-routing test data has been updated -class DenialFromCSVFileOldSerializer(serializers.Serializer): - - csv_file = serializers.CharField() - required_headers = [ - "reference", - "name", - "address", - "notifying_government", - "country", - "item_list_codes", - "item_description", - "consignee_name", - "end_use", - ] - - @transaction.atomic - def validate_csv_file(self, value): - csv_file = io.StringIO(value) - dialect = csv.Sniffer().sniff(csv_file.read(1024)) - csv_file.seek(0) - reader = csv.reader(csv_file, dialect=dialect) - headers = next(reader, None) - errors = [] - valid_serializers = [] - for i, row in enumerate(reader, start=1): - data = dict(zip(headers, row)) - serializer = DenialEntitySerializer( - data={ - "data": data, - "created_by": self.context["request"].user, - **{field: data.pop(field, None) for field in self.required_headers}, - } - ) - - if serializer.is_valid(): - valid_serializers.append(serializer) - else: - self.add_bulk_errors(errors=errors, row_number=i + 1, line_errors=serializer.errors) - if errors: - raise serializers.ValidationError(errors) - else: - # only save if no errors - for serializer in valid_serializers: - serializer.save() - return csv_file - - @staticmethod - def add_bulk_errors(errors, row_number, line_errors): - for key, values in line_errors.items(): - errors.append(f"[Row {row_number}] {key}: {','.join(values)}") - - class DenialFromCSVFileSerializer(serializers.Serializer): csv_file = serializers.CharField() diff --git a/api/external_data/views.py b/api/external_data/views.py index 2fd5bbc23e..59eb136ba6 100644 --- a/api/external_data/views.py +++ b/api/external_data/views.py @@ -19,12 +19,7 @@ class DenialViewSet(viewsets.ModelViewSet): def get_serializer_class(self): if self.action == "create": - # TODO: this is for backwards compatibility and should be removed - # once the lite-routing test data has been updated - if bool("regime_reg_ref" not in str(self.request.data["csv_file"])): # type: ignore - return serializers.DenialFromCSVFileOldSerializer - else: - return serializers.DenialFromCSVFileSerializer + return serializers.DenialFromCSVFileSerializer return serializers.DenialEntitySerializer def perform_create(self, serializer): From e5f940327338f86c55a4b1e0f42f78d991b2eda3 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 23 Apr 2024 14:05:49 +0100 Subject: [PATCH 41/58] Update lite-routing sha --- lite_routing | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lite_routing b/lite_routing index 1ccd775cff..92bfcb2c8f 160000 --- a/lite_routing +++ b/lite_routing @@ -1 +1 @@ -Subproject commit 1ccd775cff757c1e2587576a78d529ab3f3d40d0 +Subproject commit 92bfcb2c8f3acbfd72ed9f668d2daad374f9bbef From f2af7160408e0b7e940f9deeba49fca026d2f19d Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 14:14:14 +0100 Subject: [PATCH 42/58] changes to enums and knock on effects --- api/external_data/enums.py | 2 +- .../migrations/0023_set_denial_entity_type.py | 27 +++++++------------ .../tests/test_0023_set_denial_entity_type.py | 6 ++--- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/api/external_data/enums.py b/api/external_data/enums.py index 13033c0c7c..48eaa6f765 100644 --- a/api/external_data/enums.py +++ b/api/external_data/enums.py @@ -13,4 +13,4 @@ class DenialEntityType: END_USER = "end_user" THIRD_PARTY = "third_party" - choices = ((CONSIGNEE, "Consignee"), (END_USER, "End-user"), (THIRD_PARTY, "Third party")) + choices = ((CONSIGNEE, "consignee"), (END_USER, "end_user"), (THIRD_PARTY, "third_party")) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 82cb49501e..6c4772c089 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -9,26 +9,17 @@ def get_denial_entity_type(data): entity_type = "" normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} - if ( - normalised_entity_type_dict.get("end_user_flag") == "true" - and normalised_entity_type_dict.get("consignee_flag", "false") == "false" - ): + is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true" + is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true" + third_party_string_has_value = isinstance(normalised_entity_type_dict["other_role"], str) + + if is_end_user_flag and is_consignee_flag: entity_type = DenialEntityType.END_USER - elif ( - normalised_entity_type_dict.get("end_user_flag", "false") == "false" - and normalised_entity_type_dict.get("consignee_flag", "false") == "true" - ): + elif not is_end_user_flag and is_consignee_flag: entity_type = DenialEntityType.CONSIGNEE - elif ( - normalised_entity_type_dict.get("end_user_flag", "false") == "true" - and normalised_entity_type_dict.get("consignee_flag", "false") == "true" - ): + elif is_end_user_flag and not is_consignee_flag: entity_type = DenialEntityType.END_USER - elif ( - normalised_entity_type_dict.get("end_user_flag", "false") == "false" - and normalised_entity_type_dict.get("consignee_flag", "false") == "false" - and isinstance(normalised_entity_type_dict["other_role"], str) - ): + elif is_end_user_flag == False and is_consignee_flag == False and third_party_string_has_value == True: entity_type = DenialEntityType.THIRD_PARTY return entity_type @@ -41,7 +32,7 @@ def set_denial_entity_type(apps, schema_editor): for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): denial_entity_type = get_denial_entity_type(denial_entity.data) - if denial_entity_type in ["End-user", "Consignee", "Third-party"]: + if denial_entity_type in ["end_user", "consignee", "third_party"]: denial_entity.save(denial_entity_type) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 3d8d41b9f1..8e842f4b31 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -84,6 +84,6 @@ def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.filter(entity_type="End-user").count() == 2 - assert DenialEntity.objects.filter(entity_type="Consignee").count() == 1 - assert DenialEntity.objects.filter(entity_type="Third-party").count() == 1 + assert DenialEntity.objects.filter(entity_type="end-user").count() == 2 + assert DenialEntity.objects.filter(entity_type="consignee").count() == 1 + assert DenialEntity.objects.filter(entity_type="third_party").count() == 1 From 27ff93302217ae753c9b18b5f6c50a431274d0bb Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Tue, 23 Apr 2024 14:32:54 +0100 Subject: [PATCH 43/58] Update test_create_error_missing_required_headers --- api/external_data/tests/test_views.py | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index 7d1c8708e1..e477efa684 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -98,35 +98,13 @@ def test_create_success(self): ], ) - def test_create_validation_error(self): + def test_create_error_missing_required_headers(self): url = reverse("external_data:denial-list") file_path = os.path.join(settings.BASE_DIR, "external_data/tests/denial_invalid.csv") with open(file_path, "rb") as f: content = f.read() response = self.client.post(url, {"csv_file": content}, **self.gov_headers) - self.assertEqual(response.status_code, 400) - self.assertEqual( - response.json(), - { - "errors": { - "csv_file": [ - "[Row 2] reference: This field may not be null.", - "[Row 3] reference: This field may not be null.", - "[Row 4] reference: This field may not be null.", - ] - } - }, - ) - - def test_create_error_missing_required_headers(self): - url = reverse("external_data:denial-list") - # missing the 'reference' header - content = """ - regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 - """ - response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {"errors": {"csv_file": ["Missing required headers in CSV file"]}}) From 010cb402ea298089cf12c69b69c51f2446977fd5 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 14:39:41 +0100 Subject: [PATCH 44/58] Update enums.py --- api/external_data/enums.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/enums.py b/api/external_data/enums.py index 48eaa6f765..13033c0c7c 100644 --- a/api/external_data/enums.py +++ b/api/external_data/enums.py @@ -13,4 +13,4 @@ class DenialEntityType: END_USER = "end_user" THIRD_PARTY = "third_party" - choices = ((CONSIGNEE, "consignee"), (END_USER, "end_user"), (THIRD_PARTY, "third_party")) + choices = ((CONSIGNEE, "Consignee"), (END_USER, "End-user"), (THIRD_PARTY, "Third party")) From 6c4836a1deb54f8f9a53e3920fc223b5a23ecb95 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 14:55:49 +0100 Subject: [PATCH 45/58] changes to saving of entity_type --- api/external_data/migrations/0023_set_denial_entity_type.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 6c4772c089..297f49b8bd 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -32,8 +32,10 @@ def set_denial_entity_type(apps, schema_editor): for denial_entity in DenialEntity.objects.filter(entity_type__isnull=True): denial_entity_type = get_denial_entity_type(denial_entity.data) + if denial_entity_type in ["end_user", "consignee", "third_party"]: - denial_entity.save(denial_entity_type) + denial_entity.entity_type = denial_entity_type + denial_entity.save() class Migration(migrations.Migration): From 13326f11d8ddef225e5402f0ddbe98b0be8f0702 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 15:19:12 +0100 Subject: [PATCH 46/58] Update test_0023_set_denial_entity_type.py --- api/external_data/tests/test_0023_set_denial_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 8e842f4b31..17488850e6 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -84,6 +84,6 @@ def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.filter(entity_type="end-user").count() == 2 + assert DenialEntity.objects.filter(entity_type="end_user").count() == 2 assert DenialEntity.objects.filter(entity_type="consignee").count() == 1 assert DenialEntity.objects.filter(entity_type="third_party").count() == 1 From be2afc8b2fdfb4c315b44a0879103654fb04ea61 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 16:00:22 +0100 Subject: [PATCH 47/58] amendments made to other role flag --- .../migrations/0023_set_denial_entity_type.py | 4 ++-- .../tests/test_0023_set_denial_entity_type.py | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 297f49b8bd..343c3b1edc 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -11,7 +11,7 @@ def get_denial_entity_type(data): is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true" is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true" - third_party_string_has_value = isinstance(normalised_entity_type_dict["other_role"], str) + is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0 if is_end_user_flag and is_consignee_flag: entity_type = DenialEntityType.END_USER @@ -19,7 +19,7 @@ def get_denial_entity_type(data): entity_type = DenialEntityType.CONSIGNEE elif is_end_user_flag and not is_consignee_flag: entity_type = DenialEntityType.END_USER - elif is_end_user_flag == False and is_consignee_flag == False and third_party_string_has_value == True: + elif is_end_user_flag == False and is_consignee_flag == False and is_other_role == True: entity_type = DenialEntityType.THIRD_PARTY return entity_type diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/tests/test_0023_set_denial_entity_type.py index 17488850e6..b38e112d2d 100644 --- a/api/external_data/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/tests/test_0023_set_denial_entity_type.py @@ -1,6 +1,7 @@ import pytest from django_test_migrations.contrib.unittest_case import MigratorTestCase +from api.external_data.enums import DenialEntityType test_data = [ { @@ -84,6 +85,6 @@ def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.filter(entity_type="end_user").count() == 2 - assert DenialEntity.objects.filter(entity_type="consignee").count() == 1 - assert DenialEntity.objects.filter(entity_type="third_party").count() == 1 + assert DenialEntity.objects.filter(entity_type=DenialEntityType.END_USER).count() == 2 + assert DenialEntity.objects.filter(entity_type=DenialEntityType.CONSIGNEE).count() == 1 + assert DenialEntity.objects.filter(entity_type=DenialEntityType.THIRD_PARTY).count() == 1 From a1dd6ddd194928a458debe589cdc168087cf8f3e Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Tue, 23 Apr 2024 16:53:28 +0100 Subject: [PATCH 48/58] Update 0023_set_denial_entity_type.py --- api/external_data/migrations/0023_set_denial_entity_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index 343c3b1edc..c1a21a0ab2 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -19,7 +19,7 @@ def get_denial_entity_type(data): entity_type = DenialEntityType.CONSIGNEE elif is_end_user_flag and not is_consignee_flag: entity_type = DenialEntityType.END_USER - elif is_end_user_flag == False and is_consignee_flag == False and is_other_role == True: + elif not is_end_user_flag and not is_consignee_flag and is_other_role: entity_type = DenialEntityType.THIRD_PARTY return entity_type From 42ea7ec79bcfed193f702e46e6da5f5bd2b3945e Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Wed, 24 Apr 2024 09:41:52 +0100 Subject: [PATCH 49/58] check if data is a dict --- .../migrations/0023_set_denial_entity_type.py | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/api/external_data/migrations/0023_set_denial_entity_type.py b/api/external_data/migrations/0023_set_denial_entity_type.py index c1a21a0ab2..253ad64833 100644 --- a/api/external_data/migrations/0023_set_denial_entity_type.py +++ b/api/external_data/migrations/0023_set_denial_entity_type.py @@ -6,23 +6,24 @@ def get_denial_entity_type(data): - entity_type = "" - normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} - - is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true" - is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true" - is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0 - - if is_end_user_flag and is_consignee_flag: - entity_type = DenialEntityType.END_USER - elif not is_end_user_flag and is_consignee_flag: - entity_type = DenialEntityType.CONSIGNEE - elif is_end_user_flag and not is_consignee_flag: - entity_type = DenialEntityType.END_USER - elif not is_end_user_flag and not is_consignee_flag and is_other_role: - entity_type = DenialEntityType.THIRD_PARTY - - return entity_type + if isinstance(data, dict): + entity_type = "" + normalised_entity_type_dict = {keys.lower(): values.lower() for keys, values in data.items()} + + is_end_user_flag = normalised_entity_type_dict.get("end_user_flag", "false") == "true" + is_consignee_flag = normalised_entity_type_dict.get("consignee_flag", "false") == "true" + is_other_role = len(normalised_entity_type_dict.get("other_role", "")) > 0 + + if is_end_user_flag and is_consignee_flag: + entity_type = DenialEntityType.END_USER + elif not is_end_user_flag and is_consignee_flag: + entity_type = DenialEntityType.CONSIGNEE + elif is_end_user_flag and not is_consignee_flag: + entity_type = DenialEntityType.END_USER + elif not is_end_user_flag and not is_consignee_flag and is_other_role: + entity_type = DenialEntityType.THIRD_PARTY + + return entity_type def set_denial_entity_type(apps, schema_editor): From 47f351c373efb06826b3c6d856034b729e2170e8 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 10:05:30 +0100 Subject: [PATCH 50/58] Update lite-api test csv files to match example csv --- api/external_data/tests/denial_valid.csv | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index 2856dd541b..c212188f62 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,5 +1,5 @@ -reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id -DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 -DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Example Name 3,Used in other industry,Risk of outcome 3,125 -DN2010/0001,AB-XY-EF-900,The Widget Company,"2 Example Road, Example City",Example Country,Country Name X,"catch all",Extra Large Size Widget,Example Name 4,Used in unknown industry,Risk of outcome 4,126 -DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Example Name 2,Used in other industry,Risk of outcome 2,124 +reference,regime_reg_ref,notifying_government,item_list_codes,item_description,end_use,reason_for_refusal,name,address,country,consignee_name,spire_entity_id,end_user_flag,consignee_flag,other_role +DN2000/0000,AB-CD-EF-000,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,Organisation Name,"1000 Street Name, City Name",Country Name,Example Name,123,TRUE,FALSE, +DN2000/0010,AB-CD-EF-300,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Example Name 3,125,TRUE,FALSE, +DN2010/0001,AB-XY-EF-900,Example Country,catch all,Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,The Widget Company,"2 Example Road, Example City",Country Name X,Example Name 4,126,TRUE,FALSE, +DN3000/0000,AB-CD-EF-100,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Example Name 2,124,TRUE,FALSE, From cca9f17bf220d8b766de97d355bb76b5bc402463 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 12:23:07 +0100 Subject: [PATCH 51/58] Remove consignee_name column as this is no longer needed --- api/external_data/tests/denial_valid.csv | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index c212188f62..c021b828f3 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,5 +1,5 @@ -reference,regime_reg_ref,notifying_government,item_list_codes,item_description,end_use,reason_for_refusal,name,address,country,consignee_name,spire_entity_id,end_user_flag,consignee_flag,other_role -DN2000/0000,AB-CD-EF-000,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,Organisation Name,"1000 Street Name, City Name",Country Name,Example Name,123,TRUE,FALSE, -DN2000/0010,AB-CD-EF-300,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Example Name 3,125,TRUE,FALSE, -DN2010/0001,AB-XY-EF-900,Example Country,catch all,Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,The Widget Company,"2 Example Road, Example City",Country Name X,Example Name 4,126,TRUE,FALSE, -DN3000/0000,AB-CD-EF-100,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Example Name 2,124,TRUE,FALSE, +reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id +DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 +DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,125 +DN2010/0001,AB-XY-EF-900,The Widget Company,"2 Example Road, Example City",Example Country,Country Name X,"catch all",Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,126 +DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,124 From 887f318eda0aa10498503eb6a4a802f36a24c785 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 12:27:19 +0100 Subject: [PATCH 52/58] Remove consignee_name from serializer and tests --- api/external_data/serializers.py | 1 - api/external_data/tests/test_views.py | 18 ++++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index b8ab96f9cd..5b1ef5ac09 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -65,7 +65,6 @@ class DenialFromCSVFileSerializer(serializers.Serializer): "country", "item_list_codes", "item_description", - "consignee_name", "end_use", "reason_for_refusal", "spire_entity_id", diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index e477efa684..23f80f4517 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -44,7 +44,6 @@ def test_create_success(self): "country": "Country Name", "item_list_codes": "0A00100", "item_description": "Medium Size Widget", - "consignee_name": "Example Name", "end_use": "Used in industry", "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, @@ -59,7 +58,6 @@ def test_create_success(self): "country": "Country Name 3", "item_list_codes": "0A00201", "item_description": "Unspecified Size Widget", - "consignee_name": "Example Name 3", "end_use": "Used in other industry", "reason_for_refusal": "Risk of outcome 3", "spire_entity_id": 125, @@ -74,7 +72,6 @@ def test_create_success(self): "country": "Country Name X", "item_list_codes": "catch all", "item_description": "Extra Large Size Widget", - "consignee_name": "Example Name 4", "end_use": "Used in unknown industry", "reason_for_refusal": "Risk of outcome 4", "spire_entity_id": 126, @@ -89,7 +86,6 @@ def test_create_success(self): "country": "Country Name 2", "item_list_codes": "0A00200", "item_description": "Large Size Widget", - "consignee_name": "Example Name 2", "end_use": "Used in other industry", "reason_for_refusal": "Risk of outcome 2", "spire_entity_id": 124, @@ -111,8 +107,8 @@ def test_create_error_missing_required_headers(self): def test_update_success(self): url = reverse("external_data:denial-list") content = """ - reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 """ response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 201) @@ -144,7 +140,6 @@ def test_update_success(self): "country": "Country Name", "item_list_codes": "0A00100", "item_description": "Medium Size Widget", - "consignee_name": "Example Name", "end_use": "Used in industry", "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, @@ -153,8 +148,8 @@ def test_update_success(self): ], ) updated_content = """ - reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name 2,Country Name 2,0A00200,Medium Size Widget 2,Example Name 2,Used in industry 2,Risk of outcome 2,124 + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name 2,Country Name 2,0A00200,Medium Size Widget 2,Used in industry 2,Risk of outcome 2,124 """ response = self.client.post(url, {"csv_file": updated_content}, **self.gov_headers) self.assertEqual(response.status_code, 201) @@ -186,7 +181,6 @@ def test_update_success(self): "country": "Country Name 2", "item_list_codes": "0A00200", "item_description": "Medium Size Widget 2", - "consignee_name": "Example Name 2", "end_use": "Used in industry 2", "reason_for_refusal": "Risk of outcome 2", "spire_entity_id": 124, @@ -197,8 +191,8 @@ def test_update_success(self): def test_create_error_serializer_errors(self): url = reverse("external_data:denial-list") - content = """reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + content = """reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 """ response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 400) From 7d537eb9af14a25b8cc63f86c01476f72933c36f Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Fri, 12 Apr 2024 17:44:44 +0100 Subject: [PATCH 53/58] data migration --- .../migrations/0024_denials_data_migration.py | 64 +++++++++++++++++++ .../tests/test_0024_denials_data_migration.py | 59 +++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 api/external_data/migrations/0024_denials_data_migration.py create mode 100644 api/external_data/migrations/tests/test_0024_denials_data_migration.py diff --git a/api/external_data/migrations/0024_denials_data_migration.py b/api/external_data/migrations/0024_denials_data_migration.py new file mode 100644 index 0000000000..4a7bfafec6 --- /dev/null +++ b/api/external_data/migrations/0024_denials_data_migration.py @@ -0,0 +1,64 @@ +# Generated by Django 4.2.10 on 2024-04-02 14:59 + +import logging +from django.db import migrations +from django.forms.models import model_to_dict +from django.db.models import Q +from django.db import IntegrityError +from django.db import transaction + +log = logging.getLogger(__name__) + +required_fields = [ + "reference", + "regime_reg_ref", + "notifying_government", + "item_list_codes", + "item_description", + "end_use", + "is_revoked", + "is_revoked_comment", + "reason_for_refusal", +] + +def denials_data_migration(apps, schema_editor): + DenialEntity = apps.get_model("external_data", "DenialEntity") + Denial = apps.get_model("external_data", "Denial") + + # There are a handfull (10) of regime_reg_refs that are null which was in the initial load + # Assumption here is that they can be deleted since it's erroneous data as we now know + # regime_reg_ref is considered a unique DN record. + + total_null_regime_reg_ref = DenialEntity.objects.filter(Q(regime_reg_ref__isnull=True) | Q(regime_reg_ref='')) + log.info( + "Delete null regime_reg_ref total -> %s", + total_null_regime_reg_ref.count(), + ) + total_null_regime_reg_ref.delete() + duplicate_denial_errors = [] + with transaction.atomic(): + sid = transaction.savepoint() + for denial_entity in DenialEntity.objects.all(): + try: + denial_entity_dict = {key:value for (key,value) in model_to_dict(denial_entity).items() if key in required_fields} + denial , _ = Denial.objects.get_or_create(**denial_entity_dict) + denial_entity.denial = denial + denial_entity.save() + except IntegrityError as e: + duplicate_denial_errors.append(denial_entity.regime_reg_ref) + + if duplicate_denial_errors: + log.info( + "There are the following duplicate denials in the database rolling back this migration: -> %s", + duplicate_denial_errors, + ) + transaction.savepoint_rollback(sid) + else: + transaction.savepoint_commit(sid) + +class Migration(migrations.Migration): + dependencies = [ + ("external_data", "0023_set_denial_entity_type"), + ] + + operations = [migrations.RunPython(denials_data_migration, migrations.RunPython.noop, atomic=False)] diff --git a/api/external_data/migrations/tests/test_0024_denials_data_migration.py b/api/external_data/migrations/tests/test_0024_denials_data_migration.py new file mode 100644 index 0000000000..dc23ccd62b --- /dev/null +++ b/api/external_data/migrations/tests/test_0024_denials_data_migration.py @@ -0,0 +1,59 @@ +import pytest + +from django_test_migrations.contrib.unittest_case import MigratorTestCase + + +test_data = [ +{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.123","name":"name 1","address":"address 1","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"}, +{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 2","address":"address 2","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"}, +{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 3","address":"address 3","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"}, +{"reference":"DN2010\/0057","regime_reg_ref":"reg.123.1234","name":"name 4","address":"address 4","notifying_government":"UK","country":"UK","item_list_codes":"all","item_description":"desc a","end_use":"use 1","reason_for_refusal":"a"}, +{"reference":"DN2010\/0057","name":"bad record","address":"bad record","notifying_government":"UK","country":"bad","item_list_codes":"all","item_description":"bad","end_use":"bad","reason_for_refusal":"bad "} +] + + +@pytest.mark.django_db() +class TestDenialDataMigration(MigratorTestCase): + + migrate_from = ("external_data", "0021_denialentity_denial_id") + migrate_to = ("external_data", "0023_denials_data_migration") + + + def prepare(self): + DenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity") + for row in test_data: + DenialEntity.objects.create(**row) + + + + + def test_0023_denials_data_migration(self): + DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") + Denial = self.new_state.apps.get_model("external_data", "Denial") + + assert DenialEntity.objects.all().count() == 4 + assert Denial.objects.all().count() == 2 + assert Denial.objects.get(regime_reg_ref='reg.123.1234').denial_entity.count() == 3 + + +@pytest.mark.django_db() +class TestDenialDataDuplicatesMigration(MigratorTestCase): + + migrate_from = ("external_data", "0023_set_denial_entity_type") + migrate_to = ("external_data", "0024_denials_data_migration") + + + def prepare(self): + DenialEntity = self.old_state.apps.get_model("external_data", "DenialEntity") + for row in test_data: + DenialEntity.objects.create(**row) + test_data[0]["end_use"] = "end_use b" + DenialEntity.objects.create(**test_data[0]) + + + + def test_0024_denials_data_migration_duplicates(self): + DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") + Denial = self.new_state.apps.get_model("external_data", "Denial") + assert DenialEntity.objects.all().count() == 5 + assert Denial.objects.all().count() == 0 From 00812f1f351a29499f87a7fcb4a5919f8c814a47 Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 10:05:30 +0100 Subject: [PATCH 54/58] Update lite-api test csv files to match example csv --- api/external_data/tests/denial_valid.csv | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index 2856dd541b..c212188f62 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,5 +1,5 @@ -reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id -DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 -DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Example Name 3,Used in other industry,Risk of outcome 3,125 -DN2010/0001,AB-XY-EF-900,The Widget Company,"2 Example Road, Example City",Example Country,Country Name X,"catch all",Extra Large Size Widget,Example Name 4,Used in unknown industry,Risk of outcome 4,126 -DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Example Name 2,Used in other industry,Risk of outcome 2,124 +reference,regime_reg_ref,notifying_government,item_list_codes,item_description,end_use,reason_for_refusal,name,address,country,consignee_name,spire_entity_id,end_user_flag,consignee_flag,other_role +DN2000/0000,AB-CD-EF-000,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,Organisation Name,"1000 Street Name, City Name",Country Name,Example Name,123,TRUE,FALSE, +DN2000/0010,AB-CD-EF-300,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Example Name 3,125,TRUE,FALSE, +DN2010/0001,AB-XY-EF-900,Example Country,catch all,Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,The Widget Company,"2 Example Road, Example City",Country Name X,Example Name 4,126,TRUE,FALSE, +DN3000/0000,AB-CD-EF-100,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Example Name 2,124,TRUE,FALSE, From 8150d18055bb51bddfe1bc749b6cdb3cc66708af Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 12:23:07 +0100 Subject: [PATCH 55/58] Remove consignee_name column as this is no longer needed --- api/external_data/tests/denial_valid.csv | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/external_data/tests/denial_valid.csv b/api/external_data/tests/denial_valid.csv index c212188f62..c021b828f3 100644 --- a/api/external_data/tests/denial_valid.csv +++ b/api/external_data/tests/denial_valid.csv @@ -1,5 +1,5 @@ -reference,regime_reg_ref,notifying_government,item_list_codes,item_description,end_use,reason_for_refusal,name,address,country,consignee_name,spire_entity_id,end_user_flag,consignee_flag,other_role -DN2000/0000,AB-CD-EF-000,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,Organisation Name,"1000 Street Name, City Name",Country Name,Example Name,123,TRUE,FALSE, -DN2000/0010,AB-CD-EF-300,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Example Name 3,125,TRUE,FALSE, -DN2010/0001,AB-XY-EF-900,Example Country,catch all,Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,The Widget Company,"2 Example Road, Example City",Country Name X,Example Name 4,126,TRUE,FALSE, -DN3000/0000,AB-CD-EF-100,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Example Name 2,124,TRUE,FALSE, +reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id +DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 +DN2000/0010,AB-CD-EF-300,Organisation Name 3,"2001 Street Name, City Name 3",Country Name 3,Country Name 3,0A00201,Unspecified Size Widget,Used in other industry,Risk of outcome 3,125 +DN2010/0001,AB-XY-EF-900,The Widget Company,"2 Example Road, Example City",Example Country,Country Name X,"catch all",Extra Large Size Widget,Used in unknown industry,Risk of outcome 4,126 +DN3000/0000,AB-CD-EF-100,Organisation Name XYZ,"2000 Street Name, City Name 2",Country Name 2,Country Name 2,0A00200,Large Size Widget,Used in other industry,Risk of outcome 2,124 From 716b230f28c4845a7983384f5b34bb5365b19bcf Mon Sep 17 00:00:00 2001 From: Henry Cooksley Date: Wed, 24 Apr 2024 12:27:19 +0100 Subject: [PATCH 56/58] Remove consignee_name from serializer and tests --- .../tests/test_0023_set_denial_entity_type.py | 0 .../tests/test_0024_denials_data_migration.py | 4 ++-- api/external_data/serializers.py | 1 - api/external_data/tests/test_views.py | 18 ++++++------------ 4 files changed, 8 insertions(+), 15 deletions(-) rename api/external_data/{ => migrations}/tests/test_0023_set_denial_entity_type.py (100%) diff --git a/api/external_data/tests/test_0023_set_denial_entity_type.py b/api/external_data/migrations/tests/test_0023_set_denial_entity_type.py similarity index 100% rename from api/external_data/tests/test_0023_set_denial_entity_type.py rename to api/external_data/migrations/tests/test_0023_set_denial_entity_type.py diff --git a/api/external_data/migrations/tests/test_0024_denials_data_migration.py b/api/external_data/migrations/tests/test_0024_denials_data_migration.py index dc23ccd62b..0069ab9ea8 100644 --- a/api/external_data/migrations/tests/test_0024_denials_data_migration.py +++ b/api/external_data/migrations/tests/test_0024_denials_data_migration.py @@ -15,8 +15,8 @@ @pytest.mark.django_db() class TestDenialDataMigration(MigratorTestCase): - migrate_from = ("external_data", "0021_denialentity_denial_id") - migrate_to = ("external_data", "0023_denials_data_migration") + migrate_from = ("external_data", "0023_set_denial_entity_type") + migrate_to = ("external_data", "0024_denials_data_migration") def prepare(self): diff --git a/api/external_data/serializers.py b/api/external_data/serializers.py index b8ab96f9cd..5b1ef5ac09 100644 --- a/api/external_data/serializers.py +++ b/api/external_data/serializers.py @@ -65,7 +65,6 @@ class DenialFromCSVFileSerializer(serializers.Serializer): "country", "item_list_codes", "item_description", - "consignee_name", "end_use", "reason_for_refusal", "spire_entity_id", diff --git a/api/external_data/tests/test_views.py b/api/external_data/tests/test_views.py index e477efa684..23f80f4517 100644 --- a/api/external_data/tests/test_views.py +++ b/api/external_data/tests/test_views.py @@ -44,7 +44,6 @@ def test_create_success(self): "country": "Country Name", "item_list_codes": "0A00100", "item_description": "Medium Size Widget", - "consignee_name": "Example Name", "end_use": "Used in industry", "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, @@ -59,7 +58,6 @@ def test_create_success(self): "country": "Country Name 3", "item_list_codes": "0A00201", "item_description": "Unspecified Size Widget", - "consignee_name": "Example Name 3", "end_use": "Used in other industry", "reason_for_refusal": "Risk of outcome 3", "spire_entity_id": 125, @@ -74,7 +72,6 @@ def test_create_success(self): "country": "Country Name X", "item_list_codes": "catch all", "item_description": "Extra Large Size Widget", - "consignee_name": "Example Name 4", "end_use": "Used in unknown industry", "reason_for_refusal": "Risk of outcome 4", "spire_entity_id": 126, @@ -89,7 +86,6 @@ def test_create_success(self): "country": "Country Name 2", "item_list_codes": "0A00200", "item_description": "Large Size Widget", - "consignee_name": "Example Name 2", "end_use": "Used in other industry", "reason_for_refusal": "Risk of outcome 2", "spire_entity_id": 124, @@ -111,8 +107,8 @@ def test_create_error_missing_required_headers(self): def test_update_success(self): url = reverse("external_data:denial-list") content = """ - reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 """ response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 201) @@ -144,7 +140,6 @@ def test_update_success(self): "country": "Country Name", "item_list_codes": "0A00100", "item_description": "Medium Size Widget", - "consignee_name": "Example Name", "end_use": "Used in industry", "reason_for_refusal": "Risk of outcome", "spire_entity_id": 123, @@ -153,8 +148,8 @@ def test_update_success(self): ], ) updated_content = """ - reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name 2,Country Name 2,0A00200,Medium Size Widget 2,Example Name 2,Used in industry 2,Risk of outcome 2,124 + reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + DN2000/0000,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name 2,Country Name 2,0A00200,Medium Size Widget 2,Used in industry 2,Risk of outcome 2,124 """ response = self.client.post(url, {"csv_file": updated_content}, **self.gov_headers) self.assertEqual(response.status_code, 201) @@ -186,7 +181,6 @@ def test_update_success(self): "country": "Country Name 2", "item_list_codes": "0A00200", "item_description": "Medium Size Widget 2", - "consignee_name": "Example Name 2", "end_use": "Used in industry 2", "reason_for_refusal": "Risk of outcome 2", "spire_entity_id": 124, @@ -197,8 +191,8 @@ def test_update_success(self): def test_create_error_serializer_errors(self): url = reverse("external_data:denial-list") - content = """reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,consignee_name,end_use,reason_for_refusal,spire_entity_id - ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Example Name,Used in industry,Risk of outcome,123 + content = """reference,regime_reg_ref,name,address,notifying_government,country,item_list_codes,item_description,end_use,reason_for_refusal,spire_entity_id + ,AB-CD-EF-000,Organisation Name,"1000 Street Name, City Name",Country Name,Country Name,0A00100,Medium Size Widget,Used in industry,Risk of outcome,123 """ response = self.client.post(url, {"csv_file": content}, **self.gov_headers) self.assertEqual(response.status_code, 400) From 782af0d5166e640b3a319a7923f8ff4266c9c3ed Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Wed, 24 Apr 2024 16:33:54 +0100 Subject: [PATCH 57/58] f --- .../tests/test_0023_set_denial_entity_type.py | 8 ++++---- .../tests/test_0024_denials_data_migration.py | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/external_data/migrations/tests/test_0023_set_denial_entity_type.py b/api/external_data/migrations/tests/test_0023_set_denial_entity_type.py index b38e112d2d..8510c63e80 100644 --- a/api/external_data/migrations/tests/test_0023_set_denial_entity_type.py +++ b/api/external_data/migrations/tests/test_0023_set_denial_entity_type.py @@ -84,7 +84,7 @@ def prepare(self): def test_0023_set_denial_entity_type(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") - assert DenialEntity.objects.all().count() == 4 - assert DenialEntity.objects.filter(entity_type=DenialEntityType.END_USER).count() == 2 - assert DenialEntity.objects.filter(entity_type=DenialEntityType.CONSIGNEE).count() == 1 - assert DenialEntity.objects.filter(entity_type=DenialEntityType.THIRD_PARTY).count() == 1 + self.assertEqual(DenialEntity.objects.all().count(),4) + self.assertEqual(DenialEntity.objects.filter(entity_type=DenialEntityType.END_USER).count(),2) + self.assertEqual(DenialEntity.objects.filter(entity_type=DenialEntityType.CONSIGNEE).count(),1) + self.assertEqual(DenialEntity.objects.filter(entity_type=DenialEntityType.THIRD_PARTY).count(),1) diff --git a/api/external_data/migrations/tests/test_0024_denials_data_migration.py b/api/external_data/migrations/tests/test_0024_denials_data_migration.py index 0069ab9ea8..471b58f61f 100644 --- a/api/external_data/migrations/tests/test_0024_denials_data_migration.py +++ b/api/external_data/migrations/tests/test_0024_denials_data_migration.py @@ -31,9 +31,9 @@ def test_0023_denials_data_migration(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") Denial = self.new_state.apps.get_model("external_data", "Denial") - assert DenialEntity.objects.all().count() == 4 - assert Denial.objects.all().count() == 2 - assert Denial.objects.get(regime_reg_ref='reg.123.1234').denial_entity.count() == 3 + self.assertEqual(DenialEntity.objects.all().count(), 4) + self.assertEqual(Denial.objects.all().count(), 2) + self.assertEqual(Denial.objects.get(regime_reg_ref='reg.123.1234').denial_entity.count(), 3) @pytest.mark.django_db() @@ -55,5 +55,5 @@ def prepare(self): def test_0024_denials_data_migration_duplicates(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") Denial = self.new_state.apps.get_model("external_data", "Denial") - assert DenialEntity.objects.all().count() == 5 - assert Denial.objects.all().count() == 0 + self.assertEqual(DenialEntity.objects.all().count(),5) + self.assertEqual(Denial.objects.all().count() == 0) From 76b5dec637c39d0cafabad51e597797609cfeccb Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal <44236490+depsiatwal@users.noreply.github.com> Date: Wed, 24 Apr 2024 17:55:06 +0100 Subject: [PATCH 58/58] Update test_0024_denials_data_migration.py fix test --- .../migrations/tests/test_0024_denials_data_migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/external_data/migrations/tests/test_0024_denials_data_migration.py b/api/external_data/migrations/tests/test_0024_denials_data_migration.py index 471b58f61f..a17be6567c 100644 --- a/api/external_data/migrations/tests/test_0024_denials_data_migration.py +++ b/api/external_data/migrations/tests/test_0024_denials_data_migration.py @@ -55,5 +55,5 @@ def prepare(self): def test_0024_denials_data_migration_duplicates(self): DenialEntity = self.new_state.apps.get_model("external_data", "DenialEntity") Denial = self.new_state.apps.get_model("external_data", "Denial") - self.assertEqual(DenialEntity.objects.all().count(),5) - self.assertEqual(Denial.objects.all().count() == 0) + self.assertEqual(DenialEntity.objects.all().count(), 5) + self.assertEqual(Denial.objects.all().count(), 0)