From ddb97b5e832aaba8007490849dfbcbd98e2d4778 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 2 Jan 2025 13:05:34 +0000 Subject: [PATCH 1/8] remove check against rejected organisations and add test to check this --- api/organisations/serializers.py | 6 +++++- api/organisations/tests/test_organisations.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index 6d5469c423..8f8f2ca457 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -502,7 +502,11 @@ class OrganisationRegistrationNumberSerializer(serializers.Serializer): def validate_registration_number(self, value): # Check for uniqueness only when creating a new Organisation if not self.instance: - if Organisation.objects.filter(registration_number=value).exists(): + if ( + Organisation.objects.filter(registration_number=value) + .exclude(status=OrganisationStatus.REJECTED) + .exists() + ): raise serializers.ValidationError("This registration number is already in use.") return value diff --git a/api/organisations/tests/test_organisations.py b/api/organisations/tests/test_organisations.py index 17ef022ce6..50498dfddc 100644 --- a/api/organisations/tests/test_organisations.py +++ b/api/organisations/tests/test_organisations.py @@ -1,7 +1,6 @@ import pytest from unittest import mock -from django.conf import settings from faker import Faker from parameterized import parameterized @@ -14,7 +13,6 @@ from api.core.authentication import EXPORTER_USER_TOKEN_HEADER from api.core.constants import Roles, GovPermissions from lite_content.lite_api.strings import Organisations -from api.organisations.constants import UK_VAT_VALIDATION_REGEX, UK_EORI_VALIDATION_REGEX from api.organisations.enums import OrganisationType, OrganisationStatus from api.organisations.tests.factories import OrganisationFactory from api.organisations.models import Organisation @@ -974,3 +972,12 @@ def test_validate_registration_number_fail(self): self.assertEqual( response.json(), {"errors": {"registration_number": ["This registration number is already in use."]}} ) + + def test_validate_registration_number_success_if_rejected_crn_used(self): + self.organisation.refresh_from_db() + self.organisation.status = OrganisationStatus.REJECTED + self.organisation.save() + data = {"registration_number": self.organisation.registration_number} + response = self.client.post(self.url, data, **self.exporter_headers) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), data) From 5132ebd7b4ccae7fc4517a9adfd9800e46fe0c14 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 2 Jan 2025 14:35:55 +0000 Subject: [PATCH 2/8] remove check against draft apps as per ticket and add testing for this --- api/organisations/serializers.py | 1 + api/organisations/tests/test_organisations.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index 8f8f2ca457..426e3ddeb4 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -505,6 +505,7 @@ def validate_registration_number(self, value): if ( Organisation.objects.filter(registration_number=value) .exclude(status=OrganisationStatus.REJECTED) + .exclude(status=OrganisationStatus.DRAFT) .exists() ): raise serializers.ValidationError("This registration number is already in use.") diff --git a/api/organisations/tests/test_organisations.py b/api/organisations/tests/test_organisations.py index 50498dfddc..ee53cf5d72 100644 --- a/api/organisations/tests/test_organisations.py +++ b/api/organisations/tests/test_organisations.py @@ -973,9 +973,10 @@ def test_validate_registration_number_fail(self): response.json(), {"errors": {"registration_number": ["This registration number is already in use."]}} ) - def test_validate_registration_number_success_if_rejected_crn_used(self): + @parameterized.expand([OrganisationStatus.REJECTED, OrganisationStatus.DRAFT]) + def test_validate_registration_number_success_for_rejected_or_draft_org(self, org_status): self.organisation.refresh_from_db() - self.organisation.status = OrganisationStatus.REJECTED + self.organisation.status = org_status self.organisation.save() data = {"registration_number": self.organisation.registration_number} response = self.client.post(self.url, data, **self.exporter_headers) From ccb1311489919d1f6dc5f145c3344a4df2c7f026 Mon Sep 17 00:00:00 2001 From: "mark.j0hnst0n" Date: Thu, 2 Jan 2025 16:52:54 +0000 Subject: [PATCH 3/8] simplified --- api/organisations/serializers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/organisations/serializers.py b/api/organisations/serializers.py index 426e3ddeb4..23c5efc638 100644 --- a/api/organisations/serializers.py +++ b/api/organisations/serializers.py @@ -504,8 +504,7 @@ def validate_registration_number(self, value): if not self.instance: if ( Organisation.objects.filter(registration_number=value) - .exclude(status=OrganisationStatus.REJECTED) - .exclude(status=OrganisationStatus.DRAFT) + .exclude(status__in=[OrganisationStatus.REJECTED, OrganisationStatus.DRAFT]) .exists() ): raise serializers.ValidationError("This registration number is already in use.") From e6650ed76d4f9ba3fbd2760bb43bdba90d1c8d34 Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Fri, 3 Jan 2025 15:23:47 +0000 Subject: [PATCH 4/8] update list --- api/conf/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/conf/settings.py b/api/conf/settings.py index 4222882301..68eaf8be5f 100644 --- a/api/conf/settings.py +++ b/api/conf/settings.py @@ -404,7 +404,7 @@ { "un_sanctions_file": "https://scsanctions.un.org/resources/xml/en/consolidated.xml", "office_financial_sanctions_file": "https://ofsistorage.blob.core.windows.net/publishlive/2022format/ConList.xml", - "uk_sanctions_file": "https://assets.publishing.service.gov.uk/media/65ca02639c5b7f0012951caf/UK_Sanctions_List.xml", # /PS-IGNORE + "uk_sanctions_file": "https://assets.publishing.service.gov.uk/media/6776a7d49d03f12136308d1c/UK_Sanctions_List.xml", # /PS-IGNORE }, ) LITE_INTERNAL_NOTIFICATION_EMAILS = env.json("LITE_INTERNAL_NOTIFICATION_EMAILS", {}) From deb6b79f20ae809fe8d3adb6f37b77875fed0079 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 Jan 2025 20:48:23 +0000 Subject: [PATCH 5/8] Bump jinja2 from 3.1.4 to 3.1.5 Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.4 to 3.1.5. - [Release notes](https://github.com/pallets/jinja/releases) - [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst) - [Commits](https://github.com/pallets/jinja/compare/3.1.4...3.1.5) --- updated-dependencies: - dependency-name: jinja2 dependency-type: indirect ... Signed-off-by: dependabot[bot] --- Pipfile.lock | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 61dbfc301f..514691ed58 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -2543,11 +2543,12 @@ }, "jinja2": { "hashes": [ - "sha256:4a3aee7acbbe7303aede8e9648d13b8bf88a429282aa6122a993f0ac800cb369", - "sha256:bc5dd2abb727a5319567b7a813e6a2e7318c39f4f487cfe6c89c6f9c7d25197d" + "sha256:8fefff8dc3034e27bb80d67c671eb8a9bc424c0ef4c0826edbff304cceff43bb", + "sha256:aba0f4dc9ed8013c424088f68a5c226f7d6097ed89b246d7749c2ec4175c6adb" ], + "index": "pypi", "markers": "python_version >= '3.7'", - "version": "==3.1.4" + "version": "==3.1.5" }, "jmespath": { "hashes": [ From 883bee7ffa4f7c8177e3ffb2c24539a675150810 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Tue, 7 Jan 2025 17:16:35 +0000 Subject: [PATCH 6/8] Ensure that case flags are fully present on all cases returned by case search endpoint --- api/cases/managers.py | 1 + api/cases/serializers.py | 3 +++ api/cases/views/search/service.py | 21 +-------------------- api/cases/views/search/views.py | 1 - 4 files changed, 5 insertions(+), 21 deletions(-) diff --git a/api/cases/managers.py b/api/cases/managers.py index e777fdfc52..fb87617898 100644 --- a/api/cases/managers.py +++ b/api/cases/managers.py @@ -323,6 +323,7 @@ def search( # noqa "queues", "queues__team", "baseapplication__licences", + "flags", Prefetch( "baseapplication__parties", to_attr="end_user_parties", diff --git a/api/cases/serializers.py b/api/cases/serializers.py index 2262a2a728..3146bc8744 100644 --- a/api/cases/serializers.py +++ b/api/cases/serializers.py @@ -33,6 +33,8 @@ from api.compliance.serializers.ComplianceVisitCaseSerializers import ComplianceVisitSerializer from api.core.serializers import KeyValueChoiceField, PrimaryKeyRelatedSerializerField from api.documents.libraries.process_document import process_document +from api.flags.serializers import CaseListFlagSerializer +from api.flags.models import Flag from api.gov_users.serializers import GovUserSimpleSerializer from api.organisations.models import Organisation from api.organisations.serializers import TinyOrganisationViewSerializer @@ -122,6 +124,7 @@ class CaseListSerializer(serializers.Serializer): intended_end_use = serializers.SerializerMethodField() end_users = serializers.SerializerMethodField() sub_status = CaseSubStatusSerializer() + flags = PrimaryKeyRelatedSerializerField(many=True, queryset=Flag.objects.all(), serializer=CaseListFlagSerializer) def __init__(self, *args, **kwargs): self.team = kwargs.pop("team", None) diff --git a/api/cases/views/search/service.py b/api/cases/views/search/service.py index aa85f8c990..5f718fcbbc 100644 --- a/api/cases/views/search/service.py +++ b/api/cases/views/search/service.py @@ -16,6 +16,7 @@ from api.applications.serializers.advice import AdviceSearchViewSerializer from api.cases.models import Case, EcjuQuery, Advice from api.common.dates import working_days_in_range, number_of_days_since +from api.flags.models import Flag from api.flags.serializers import CaseListFlagSerializer from api.organisations.models import Organisation from api.staticdata.statuses.enums import CaseStatusEnum @@ -50,27 +51,7 @@ def get_advice_types_list(): return AdviceType.to_representation() -def populate_other_flags(cases: List[Dict]): - from api.flags.models import Flag - - case_ids = [case["id"] for case in cases] - - union_flags = set( - [ - *Flag.objects.filter(cases__id__in=case_ids).annotate(case_id=F("cases__id")), - *Flag.objects.filter(organisations__cases__id__in=case_ids).annotate(case_id=F("organisations__cases__id")), - ] - ) - - for case in cases: - case_id = str(case["id"]) - flags = [flag for flag in union_flags if str(flag.case_id) == case_id] - case["flags"] = CaseListFlagSerializer(flags, many=True).data - - def populate_goods_flags(cases: List[Dict]): - from api.flags.models import Flag - case_ids = [case["id"] for case in cases] qs1 = Flag.objects.filter(goods__goods_on_application__application_id__in=case_ids).annotate( case_id=F("goods__goods_on_application__application_id"), diff --git a/api/cases/views/search/views.py b/api/cases/views/search/views.py index c57c984d0f..4c838ec840 100644 --- a/api/cases/views/search/views.py +++ b/api/cases/views/search/views.py @@ -59,7 +59,6 @@ def get(self, request, *args, **kwargs): # Populate certain fields outside of the serializer for performance improvements service.populate_goods_flags(cases) service.populate_destinations_flags(cases) - service.populate_other_flags(cases) service.populate_organisation(cases) service.populate_is_recently_updated(cases) service.populate_activity_updates(case_map) From e4dddb9e2d7f7f5eb1ba0d833212482b14c463e4 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Wed, 8 Jan 2025 10:59:46 +0000 Subject: [PATCH 7/8] Relocate case search tests next to file in question --- api/cases/views/search/tests/__init__.py | 0 .../views/search => views/search/tests}/test_service.py | 0 .../search/tests/test_views.py} | 6 +++--- 3 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 api/cases/views/search/tests/__init__.py rename api/cases/{tests/views/search => views/search/tests}/test_service.py (100%) rename api/cases/{tests/test_case_search.py => views/search/tests/test_views.py} (99%) diff --git a/api/cases/views/search/tests/__init__.py b/api/cases/views/search/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/api/cases/tests/views/search/test_service.py b/api/cases/views/search/tests/test_service.py similarity index 100% rename from api/cases/tests/views/search/test_service.py rename to api/cases/views/search/tests/test_service.py diff --git a/api/cases/tests/test_case_search.py b/api/cases/views/search/tests/test_views.py similarity index 99% rename from api/cases/tests/test_case_search.py rename to api/cases/views/search/tests/test_views.py index bf4bc5fbca..172b0d37bb 100644 --- a/api/cases/tests/test_case_search.py +++ b/api/cases/views/search/tests/test_views.py @@ -910,7 +910,7 @@ def setUp(self): super().setUp() self.other_team = self.create_team("other team") - self.other_team_gov_user = self.create_gov_user("new_user@digital.trade.gov.uk", self.other_team) + self.other_team_gov_user = self.create_gov_user("new_user@digital.trade.gov.uk", self.other_team) # /PS-IGNORE self.queue = self.create_queue("my new queue", self.team) self.case = self.create_standard_application_case(self.organisation) self.case.queues.set([self.queue]) @@ -1011,7 +1011,7 @@ def test_api_success(self): self._create_data() self.advice = FinalAdviceFactory(user=self.gov_user, case=self.case, type=AdviceType.APPROVE, good=self.good) self.gov_user2 = self.create_gov_user( - team=self.create_team(name="other_team"), email="new_user2@digital.trade.gov.uk" + team=self.create_team(name="other_team"), email="new_user2@digital.trade.gov.uk" # /PS-IGNORE ) self.group_advice = FinalAdviceFactory( user=self.gov_user2, case=self.case, type=AdviceType.REFUSE, good=self.good @@ -1148,7 +1148,7 @@ def test_api_success(self): ], ) - # Reflect rest framework's way of rendering datetime objects... https://github.com/encode/django-rest-framework/blob/c9e7b68a4c1db1ac60e962053380acda549609f3/rest_framework/utils/encoders.py#L29 + # Reflect rest framework's way of rendering datetime objects... https://github.com/encode/django-rest-framework/blob/c9e7b68a4c1db1ac60e962053380acda549609f3/rest_framework/utils/encoders.py#L29 /PS-IGNORE expected_submitted_at = self.case.submitted_at.isoformat() if expected_submitted_at.endswith("+00:00"): expected_submitted_at = expected_submitted_at[:-6] + "Z" From bc7e1597c567770db05f03973d69a8849939e3f8 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Wed, 8 Jan 2025 12:01:09 +0000 Subject: [PATCH 8/8] Add test case to prove case search flags are correct across multiple cases --- api/cases/views/search/tests/test_views.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/api/cases/views/search/tests/test_views.py b/api/cases/views/search/tests/test_views.py index 172b0d37bb..8eda5cd6a9 100644 --- a/api/cases/views/search/tests/test_views.py +++ b/api/cases/views/search/tests/test_views.py @@ -1154,6 +1154,27 @@ def test_api_success(self): expected_submitted_at = expected_submitted_at[:-6] + "Z" self.assertEqual(case_api_result["submitted_at"], expected_submitted_at) + def test_api_multiple_cases_flags_correct(self): + # Create two cases.. + self._create_data() + self._create_data() + # Add a case flag to each case.. + flag_alias = "REFER_TO_FCDO_MEUC_CONCERNS" + flag = Flag.objects.get(alias=flag_alias) + all_cases = Case.objects.all() + for case in all_cases: + case.flags.add(flag) + + # Perform the search + response = self.client.get(self.url, **self.gov_headers) + response_data = response.json()["results"] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response_data["cases"]), 2) + for search_result in response_data["cases"]: + self.assertEqual(len(search_result["flags"]), 1) + self.assertEqual(search_result["flags"][0]["alias"], flag_alias) + def test_api_no_advice(self): self._create_data() response = self.client.get(self.url, **self.gov_headers)