Merge pull request #2270 from uktrade/dev

UAT Release

Pipfile

@@ -79,6 +79,7 @@ dj-database-url = "~=2.2.0"
 certifi = "~=2024.7.4"
 pytz = "~=2024.1"
 drf-spectacular = "~=0.27.2"
+djangorestframework-csv = "~=3.0.2"
 
 [requires]
 python_version = "3.9"

README.md

@@ -169,21 +169,9 @@ Gegenerate diagrams
 
 `pipenv run bandit -r .`
 
-## Adding new control list entries
+## Control list entries cache
 
-The control list entries are maintained in an Excel sheet in an internal repo. They are arranged in a tree structure with each category in a different sheet. As with the changes in policy we need to update the list. This involves adding new entries, decontrolling existing entries or updating the description of entries.
-
-To add a new entry simply add the description, rating at the correct level. Mark whether it is decontrolled or not by entering 'x' in the decontrolled column. Usually everything is controlled unless otherwise marked in this column.
-
-If only description is to be updated then just edit the description text.
-
-Once the changes are done run the seed command to populate the database with the updated entries and ensure no errors are reported.
-
-`pipenv run ./manage.py seedcontrollistentries`
-
-Deploy the API so that it picks up the updated entries in the corresponding environment. Because of the way the seeding commands are executed during deployment we have to deploy twice to see these changes. During deployment it first runs the seed commands and then deploys new changes so during the first deployment we are still seeding existing entries. If we deploy twice then the updated entries get seeded.
-
-Once the API is deployed then restart the frontends because the control list entries are cached in the frontend and to discard them and pull the updated list we need to restart the app.
+We have a 24 hour cache for CLEs on the frontend.
 
 ## Makefile commands
 

api/applications/models.py

@@ -492,6 +492,7 @@ class AbstractGoodOnApplication(TimestampableModel):
     is_good_incorporated = models.BooleanField(null=True, blank=True, default=None)
     is_good_controlled = models.BooleanField(default=None, blank=True, null=True)
     comment = models.TextField(help_text="control review comment", default=None, blank=True, null=True)
+    # DEPRECATED: We should remove this after checking that report_summaries supersedes it (including data)
     report_summary = models.TextField(default=None, blank=True, null=True)
     application = models.ForeignKey(BaseApplication, on_delete=models.CASCADE, null=False)
     control_list_entries = models.ManyToManyField(ControlListEntry)
@@ -550,17 +551,19 @@ class GoodOnApplication(AbstractGoodOnApplication, Clonable):
     unit = models.CharField(choices=Units.choices, max_length=50, null=True, blank=True, default=None)
     value = models.DecimalField(max_digits=15, decimal_places=2, null=True, blank=True, default=None)
 
-    # Report Summary prefix and subject
+    # DEPRECATED: we should remove this after checking report_summaries fully supersedes this (including legacy data)
     report_summary_prefix = models.ForeignKey(
         ReportSummaryPrefix, on_delete=models.PROTECT, blank=True, null=True, related_name="prefix_good_on_application"
     )
+    # DEPRECATED: we should remove this after checking report_summaries fully supersedes this (including legacy data)
     report_summary_subject = models.ForeignKey(
         ReportSummarySubject,
         on_delete=models.PROTECT,
         blank=True,
         null=True,
         related_name="subject_good_on_application",
     )
+    # report_summaries supersedes report_summary_subject and report_summary_prefix
     report_summaries = models.ManyToManyField(ReportSummary, related_name="goods_on_application")
 
     # Exhibition applications are the only applications that contain the following as such may be null

api/assessments/serializers.py

@@ -51,26 +51,33 @@ def validate(self, data):
         return data
 
 
+# It would be great to make this a nested serializer instead, but to do so we must
+# override AssessmentSerializer.update() completely (i.e. it should not call super())
+# which makes this a larger change to achieve
 class ReportSummaryField(PrimaryKeyRelatedField):
     queryset = ReportSummary.objects.all()
 
     def to_internal_value(self, data):
-        prefix = data.get("prefix", None)
+        # prefix could be empty string so ensure it is None in that case
+        prefix = data.get("prefix", None) or None
         subject = data.get("subject", None)
 
         if not subject:
             raise serializers.ValidationError("You must include a report summary if this item is controlled.")
 
+        if prefix:
+            try:
+                prefix = ReportSummaryPrefix.objects.get(id=prefix)
+            except ReportSummaryPrefix.DoesNotExist:
+                raise serializers.ValidationError("Report summary prefix does not exist")
+
         try:
-            if prefix:
-                return ReportSummary.objects.get(prefix=prefix, subject=subject)
-            else:
-                # prefix can either be missing or an empty string which is not a valid UUID
-                # so use None in the query in this case
-                return ReportSummary.objects.get(prefix=None, subject=subject)
+            subject = ReportSummarySubject.objects.get(id=subject)
+        except ReportSummarySubject.DoesNotExist:
+            raise serializers.ValidationError("Report summary subject does not exist")
 
-        except ReportSummary.DoesNotExist:
-            raise serializers.ValidationError("Report summary with given prefix and subject does not exist")
+        report_summary, _ = ReportSummary.objects.get_or_create(prefix=prefix, subject=subject)
+        return report_summary
 
 
 class AssessmentSerializer(GoodControlReviewSerializer):

api/assessments/tests/test_views.py

@@ -562,7 +562,7 @@ def test_data_with_multiple_report_summary_updates(self):
         assert good_on_application3.assessed_by == self.gov_user
         assert good_on_application3.assessment_date.isoformat() == "2023-11-03T12:00:00+00:00"
 
-    def test_invalid_report_subject_raises_error(self):
+    def test_missing_report_summary_subject(self):
         regime_entry = RegimeEntry.objects.first()
         report_summary = ReportSummary.objects.last()
         data = [
@@ -572,9 +572,6 @@ def test_invalid_report_subject_raises_error(self):
                 "regime_entries": [regime_entry.id],
                 "is_good_controlled": True,
                 "report_summaries": [
-                    {
-                        "subject": str(report_summary.subject_id),
-                    },
                     {
                         # invalid subject id
                         "subject": str(report_summary.prefix_id),
@@ -586,7 +583,62 @@ def test_invalid_report_subject_raises_error(self):
         response = self.client.put(self.assessment_url, data, **self.gov_headers)
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
-        expected_response_data = {
-            "errors": [{"report_summaries": ["Report summary with given prefix and subject does not exist"]}]
-        }
+        expected_response_data = {"errors": [{"report_summaries": ["Report summary subject does not exist"]}]}
+        self.assertDictEqual(response.json(), expected_response_data)
+
+    def test_missing_report_summary_prefix(self):
+        regime_entry = RegimeEntry.objects.first()
+        report_summary = ReportSummary.objects.last()
+        data = [
+            {
+                "id": self.good_on_application.id,
+                "control_list_entries": [],
+                "regime_entries": [regime_entry.id],
+                "is_good_controlled": True,
+                "report_summaries": [
+                    {
+                        # invalid prefix id
+                        "prefix": str(report_summary.subject_id),
+                        "subject": str(report_summary.subject_id),
+                    },
+                ],
+                "comment": "some comment",
+            }
+        ]
+        response = self.client.put(self.assessment_url, data, **self.gov_headers)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        expected_response_data = {"errors": [{"report_summaries": ["Report summary prefix does not exist"]}]}
         self.assertDictEqual(response.json(), expected_response_data)
+
+    def test_missing_report_summary_lazy_create(self):
+        regime_entry = RegimeEntry.objects.first()
+        starting_number_of_report_summary_records = ReportSummary.objects.count()
+        # Create new subject/prefix records but avoid creating a new ReportSummary record linking the two
+        report_summary_subject = ReportSummarySubject.objects.create(name="some new subject", code_level=1)
+        report_summary_prefix = ReportSummaryPrefix.objects.create(name="some new prefix")
+        data = [
+            {
+                "id": self.good_on_application.id,
+                "control_list_entries": [],
+                "regime_entries": [regime_entry.id],
+                "is_good_controlled": True,
+                "report_summaries": [
+                    {
+                        "prefix": str(report_summary_prefix.id),
+                        "subject": str(report_summary_subject.id),
+                    },
+                ],
+                "comment": "some comment",
+            }
+        ]
+        response = self.client.put(self.assessment_url, data, **self.gov_headers)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        self.good_on_application.refresh_from_db()
+        assert self.good_on_application.report_summary == f"{report_summary_prefix.name} {report_summary_subject.name}"
+        assert self.good_on_application.report_summaries.count() == 1
+        report_summary = self.good_on_application.report_summaries.first()
+        assert report_summary.prefix == report_summary_prefix
+        assert report_summary.subject == report_summary_subject
+        assert ReportSummary.objects.count() == starting_number_of_report_summary_records + 1

api/staticdata/management/commands/seedall.py

@@ -10,7 +10,6 @@
         "seedsystemuser",
         "seedadminteam",
         "seedinternalusers",
-        "seedcontrollistentries",
         "seeddenialreasons",
         "seedcountries",
         "seedlayouts",
@@ -24,7 +23,6 @@
         "seedsystemuser",
         "seedadminteam",
         "seedinternalusers",
-        "seedcontrollistentries",
         "seeddenialreasons",
         "seedcountries",
         "seedlayouts",

api/staticdata/report_summaries/migrations/0009_add_ars_subject_prefix_oct_2024.py

@@ -0,0 +1,26 @@
+import json
+
+from django.db import migrations
+
+DATA_PATH = "api/staticdata/report_summaries/migrations/data/0009_add_report_summaries_oct_2024/"
+
+
+def populate_report_summaries(apps, schema_editor):
+
+    ReportSummaryPrefix = apps.get_model("report_summaries", "ReportSummaryPrefix")
+    with open(f"{DATA_PATH}/report_summary_prefix.json") as json_file:
+        records = json.load(json_file)
+        for attributes in records:
+            ReportSummaryPrefix.objects.create(**attributes)
+
+    ReportSummarySubject = apps.get_model("report_summaries", "ReportSummarySubject")
+    with open(f"{DATA_PATH}/report_summary_subject.json") as json_file:
+        records = json.load(json_file)
+        for attributes in records:
+            ReportSummarySubject.objects.create(**attributes)
+
+
+class Migration(migrations.Migration):
+    dependencies = [("report_summaries", "0008_back_populate_multiple_ars_data")]
+
+    operations = [migrations.RunPython(populate_report_summaries, migrations.RunPython.noop)]

api/staticdata/report_summaries/migrations/data/0009_add_report_summaries_oct_2024/report_summary_prefix.json

@@ -0,0 +1,6 @@
+[
+    {
+        "id": "d9d1b5fe-71ad-4124-a4a0-2e1bb43fdfaa",
+        "name": "devices containing"
+    }
+]

api/staticdata/report_summaries/migrations/data/0009_add_report_summaries_oct_2024/report_summary_subject.json

@@ -0,0 +1,27 @@
+[
+    {
+        "id": "9ab0a713-2218-461c-a33f-b60d20a9bf82",
+        "name": "cryogenic integrated circuits",
+        "code_level": 1
+    },
+    {
+        "id": "f8125cad-471b-42a1-a28a-b229bdc904de",
+        "name": "quantum computers",
+        "code_level": 1
+    },
+    {
+        "id": "41841900-bc0a-438c-b1a7-0728cc73e0cb",
+        "name": "high performance integrated circuits",
+        "code_level": 1
+    },
+    {
+        "id": "8d392a69-ec45-4631-bbf2-534bf3a881fe",
+        "name": "additive manufacturing equipment",
+        "code_level": 1
+    },
+    {
+        "id": "8438cb42-c791-4463-9bb2-aca0f301e207",
+        "name": "biological systems",
+        "code_level": 1
+    }
+]

api/staticdata/report_summaries/migrations/tests/test_0009_add_ars_subject_prefix_oct_2024.py

@@ -0,0 +1,39 @@
+import json
+import pytest
+
+FIXTURE_BASE = "api/staticdata/report_summaries/migrations/data/0009_add_report_summaries_oct_2024/"
+INITIAL_MIGRATION = "0008_back_populate_multiple_ars_data"
+MIGRATION_UNDER_TEST = "0009_add_ars_subject_prefix_oct_2024"
+
+
+@pytest.mark.django_db()
+def test_add_cles(migrator):
+    with open(FIXTURE_BASE + "report_summary_prefix.json") as prefix_json_file:
+        report_summary_prefix_data = json.load(prefix_json_file)
+
+    with open(FIXTURE_BASE + "report_summary_subject.json") as subject_json_file:
+        report_summary_subject_data = json.load(subject_json_file)
+
+    old_state = migrator.apply_initial_migration(("report_summaries", INITIAL_MIGRATION))
+    ReportSummaryPrefix = old_state.apps.get_model("report_summaries", "ReportSummaryPrefix")
+    ReportSummarySubject = old_state.apps.get_model("report_summaries", "ReportSummarySubject")
+
+    for prefix_to_add in report_summary_prefix_data:
+        assert not ReportSummaryPrefix.objects.filter(name=prefix_to_add["name"]).exists()
+
+    for subject_to_add in report_summary_subject_data:
+        assert not ReportSummarySubject.objects.filter(name=subject_to_add["name"]).exists()
+
+    new_state = migrator.apply_tested_migration(("report_summaries", MIGRATION_UNDER_TEST))
+
+    ReportSummaryPrefix = new_state.apps.get_model("report_summaries", "ReportSummaryPrefix")
+    ReportSummarySubject = new_state.apps.get_model("report_summaries", "ReportSummarySubject")
+
+    for expected_prefix in report_summary_prefix_data:
+        prefix = ReportSummaryPrefix.objects.get(name=expected_prefix["name"])
+        assert str(prefix.id) == expected_prefix["id"]
+
+    for expected_subject in report_summary_subject_data:
+        subject = ReportSummarySubject.objects.get(name=expected_subject["name"])
+        assert str(subject.id) == expected_subject["id"]
+        assert subject.code_level == expected_subject["code_level"]

api/workflow/routing_rules/models.py

@@ -16,7 +16,6 @@
 from api.teams.models import Team
 from api.users.models import GovUser
 from api.workflow.routing_rules.enum import RoutingRulesAdditionalFields
-from lite_routing.routing_rules_internal.routing_rules_criteria import run_criteria_function
 
 
 class RoutingRuleManager(models.Manager):
@@ -114,11 +113,6 @@ def natural_key(self):
             self.country_id,  # country code
         )
 
-    def is_python_criteria_satisfied(self, case):
-        if not self.is_python_criteria:
-            raise NotImplementedError(f"is_python_criteria_satisfied was run for non-python rule {self.id}")
-        return run_criteria_function(self.id, case)
-
     natural_key.dependencies = ["teams.Team", "queues.Queue", "users.GovUser", "countries.Country"]
 
 

api/workflow/routing_rules/tests/test_models.py

@@ -1,13 +1,10 @@
-import unittest
-
 from django.conf import settings
 from parameterized import parameterized
 
 from api.cases.models import CaseType, CaseStatus
 from api.cases.tests.factories import CaseFactory
 from api.flags.models import Flag, FlaggingRule
 from api.workflow.routing_rules.models import RoutingRule, RoutingHistory
-from api.teams.models import Team
 from api.queues.models import Queue
 from api.staticdata.statuses.models import CaseStatus
 from api.staticdata.statuses.enums import CaseStatusEnum
@@ -16,30 +13,6 @@
 from test_helpers.clients import DataTestClient
 
 
-class RoutingRuleCreationTests(DataTestClient):
-    def test_is_python_criteria_satisfied_non_python_rule(self):
-        rule = RoutingRule.objects.create(
-            team=Team.objects.first(),
-            queue=Queue.objects.first(),
-            status=CaseStatus.objects.first(),
-            tier=2,
-        )
-        self.assertRaises(NotImplementedError, rule.is_python_criteria_satisfied, unittest.mock.Mock())
-
-    @unittest.mock.patch("api.workflow.routing_rules.models.run_criteria_function")
-    def test_is_python_criteria_satisfied_calls_criteria_function(self, mocked_run_criteria_function):
-        mocked_run_criteria_function.return_value = True
-        rule = RoutingRule.objects.create(
-            team=Team.objects.first(),
-            queue=Queue.objects.first(),
-            status=CaseStatus.objects.first(),
-            tier=2,
-            is_python_criteria=True,
-        )
-        assert rule.is_python_criteria_satisfied(unittest.mock.Mock()) == True
-        assert mocked_run_criteria_function.called
-
-
 class RoutingHistoryCreationTests(DataTestClient):
     @parameterized.expand(
         [