Revert "Refactor to not update the resource"

Change in requirements - we now want to update both identifier_pk and the identifier within the resource

This reverts commit a782fa2.

Author: edhall-nhs

lambdas/recordforwarder/src/repository/fhir_batch_repository.py

@@ -17,11 +17,6 @@
 )
 from common.models.utils.generic_utils import get_nhs_number
 
-TPP_V2_SUPPLIER_IDENTIFIER_SYSTEM = "YGA"
-TPP_V5_SUPPLIER_IDENTIFIER_SYSTEM = "https://tpp-uk.com/Id/ve/vacc"
-EMIS_V2_SUPPLIER_IDENTIFIER_SYSTEM = "YGJ"
-EMIS_V5_SUPPLIER_IDENTIFIER_SYSTEM = "https://emishealth.com/identifiers/vacc"
-
 
 def create_table(region_name="eu-west-2"):
     table_name = os.environ["DYNAMODB_TABLE_NAME"]
@@ -104,8 +99,7 @@ def create_immunization(
         immunization["id"] = new_id
         attr = RecordAttributes(immunization, vax_type, supplier_system, 0)
 
-        identifier_pk = self._get_identifier_pk_from_immunization(immunization)
-        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", identifier_pk, is_present)
+        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", attr.identifier, is_present)
 
         if query_response is not None:
             raise IdentifierDuplicationError(identifier=attr.identifier)
@@ -117,7 +111,7 @@ def create_immunization(
                     "PatientPK": attr.patient_pk,
                     "PatientSK": attr.patient_sk,
                     "Resource": json.dumps(attr.resource, use_decimal=True),
-                    "IdentifierPK": identifier_pk,
+                    "IdentifierPK": attr.identifier,
                     "Operation": "CREATE",
                     "Version": attr.version,
                     "SupplierSystem": attr.supplier,
@@ -146,10 +140,10 @@ def update_immunization(
         table: any,
         is_present: bool,
     ) -> dict:
-        identifier_pk = self._get_identifier_pk_from_immunization(immunization)
-        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", identifier_pk, is_present)
+        identifier = self._identifier_response(immunization)
+        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", identifier, is_present)
         if query_response is None:
-            raise ResourceNotFoundError(resource_type="Immunization", resource_id=identifier_pk)
+            raise ResourceNotFoundError(resource_type="Immunization", resource_id=identifier)
         old_id, version = self._get_id_version(query_response)
         deleted_at_required, update_reinstated, is_reinstate = self._get_record_status(query_response)
 
@@ -174,10 +168,10 @@ def delete_immunization(
         table: any,
         is_present: bool,
     ) -> dict:
-        identifier_pk = self._get_identifier_pk_from_immunization(immunization)
-        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", identifier_pk, is_present)
+        identifier = self._identifier_response(immunization)
+        query_response = _query_identifier(table, "IdentifierGSI", "IdentifierPK", identifier, is_present)
         if query_response is None:
-            raise ResourceNotFoundError(resource_type="Immunization", resource_id=identifier_pk)
+            raise ResourceNotFoundError(resource_type="Immunization", resource_id=identifier)
         try:
             now_timestamp = int(time.time())
             imms_id = self._get_pk(query_response)
@@ -213,19 +207,9 @@ def _handle_dynamo_response(response, imms_id):
             raise UnhandledResponseError(message="Non-200 response from dynamodb", response=response)
 
     @staticmethod
-    def _get_identifier_pk_from_immunization(immunization: any):
+    def _identifier_response(immunization: any):
         system_id = immunization["identifier"][0]["system"]
         system_value = immunization["identifier"][0]["value"]
-
-        # The below checks can be safely removed once DPS carries out it's data migration to update legacy
-        # identifiers as it should become redundant. However, it may be worth keeping in case legacy format identifiers
-        # are received for some reason. Please see issue VED-904 for more information.
-        if system_id == TPP_V2_SUPPLIER_IDENTIFIER_SYSTEM:
-            system_id = TPP_V5_SUPPLIER_IDENTIFIER_SYSTEM
-
-        if system_id == EMIS_V2_SUPPLIER_IDENTIFIER_SYSTEM:
-            system_id = EMIS_V5_SUPPLIER_IDENTIFIER_SYSTEM
-
         return f"{system_id}#{system_value}"
 
     @staticmethod

lambdas/recordforwarder/src/service/fhir_batch_service.py

@@ -4,6 +4,30 @@
 
 IMMUNIZATION_VALIDATOR = ImmunizationValidator()
 
+TPP_V2_SUPPLIER_IDENTIFIER_SYSTEM = "YGA"
+TPP_V5_SUPPLIER_IDENTIFIER_SYSTEM = "https://tpp-uk.com/Id/ve/vacc"
+EMIS_V2_SUPPLIER_IDENTIFIER_SYSTEM = "YGJ"
+EMIS_V5_SUPPLIER_IDENTIFIER_SYSTEM = "https://emishealth.com/identifiers/vacc"
+
+
+def uplift_legacy_identifier(immunization: dict):
+    # This code the above constants can be safely removed once DPS carries out it's data migration to update legacy
+    # identifiers as it should become redundant. However, it may be worth keeping in case legacy format identifiers are
+    # received for some reason. Please see issue VED-904 for more information.
+    identifier = immunization.get("identifier")
+
+    if identifier is None or len(identifier) == 0:
+        # Return here to allow validation to raise appropriate error
+        return
+
+    identifier_system = immunization.get("identifier")[0].get("system")
+
+    if identifier_system == TPP_V2_SUPPLIER_IDENTIFIER_SYSTEM:
+        immunization["identifier"][0]["system"] = TPP_V5_SUPPLIER_IDENTIFIER_SYSTEM
+
+    if identifier_system == EMIS_V2_SUPPLIER_IDENTIFIER_SYSTEM:
+        immunization["identifier"][0]["system"] = EMIS_V5_SUPPLIER_IDENTIFIER_SYSTEM
+
 
 class ImmunizationBatchService:
     def __init__(
@@ -28,6 +52,9 @@ def create_immunization(
         the record in the database.
         """
 
+        # TODO: Remove after DPS data migration to new identifiers
+        uplift_legacy_identifier(immunization)
+
         try:
             self.validator.validate(immunization)
         except (ValueError, MandatoryError) as error:
@@ -49,6 +76,9 @@ def update_immunization(
         the record in the database.
         """
 
+        # TODO: Remove after DPS data migration to new identifiers
+        uplift_legacy_identifier(immunization)
+
         try:
             self.validator.validate(immunization)
         except (ValueError, MandatoryError) as error:
@@ -70,4 +100,7 @@ def delete_immunization(
         the record in the database.
         """
 
+        # TODO: Remove after DPS data migration to new identifiers
+        uplift_legacy_identifier(immunization)
+
         return self.immunization_repo.delete_immunization(immunization, supplier_system, vax_type, table, is_present)

lambdas/recordforwarder/tests/service/test_fhir_batch_service.py

@@ -97,6 +97,38 @@ def test_create_immunization_post_validation_error(self):
         self.assertTrue(expected_msg in error.exception.message)
         self.mock_repo.create_immunization.assert_not_called()
 
+    def test_create_immunization_uplifts_legacy_identifier_system_tpp(self):
+        """it should recognise a legacy TPP identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGA"
+
+        _ = self.service.create_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://tpp-uk.com/Id/ve/vacc")
+
+    def test_create_immunization_uplifts_legacy_identifier_system_emis(self):
+        """it should recognise a legacy EMIS identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGJ"
+
+        _ = self.service.create_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://emishealth.com/identifiers/vacc")
+
 
 class TestUpdateImmunizationBatchService(TestFhirBatchServiceBase):
     def setUp(self):
@@ -169,6 +201,38 @@ def test_update_immunization_post_validation_error(self):
         self.assertTrue(expected_msg in error.exception.message)
         self.mock_repo.update_immunization.assert_not_called()
 
+    def test_update_immunization_uplifts_legacy_identifier_system_tpp(self):
+        """it should recognise a legacy TPP identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGA"
+
+        _ = self.service.update_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://tpp-uk.com/Id/ve/vacc")
+
+    def test_update_immunization_uplifts_legacy_identifier_system_emis(self):
+        """it should recognise a legacy EMIS identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGJ"
+
+        _ = self.service.update_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://emishealth.com/identifiers/vacc")
+
 
 class TestDeleteImmunizationBatchService(unittest.TestCase):
     def setUp(self):
@@ -194,6 +258,38 @@ def test_delete_immunization_valid(self):
         )
         self.assertEqual(result, imms_id)
 
+    def test_delete_immunization_uplifts_legacy_identifier_system_tpp(self):
+        """it should recognise a legacy TPP identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGA"
+
+        _ = self.service.delete_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://tpp-uk.com/Id/ve/vacc")
+
+    def test_delete_immunization_uplifts_legacy_identifier_system_emis(self):
+        """it should recognise a legacy EMIS identifier system and use the new style identifier"""
+
+        immunisation = create_covid_immunization_dict_no_id()
+        immunisation["identifier"][0]["system"] = "YGJ"
+
+        _ = self.service.delete_immunization(
+            immunization=immunisation,
+            supplier_system="test_supplier",
+            vax_type="test_vax",
+            table=self.mock_table,
+            is_present=True,
+        )
+
+        self.assertEqual(immunisation["identifier"][0]["system"], "https://emishealth.com/identifiers/vacc")
+
 
 if __name__ == "__main__":
     unittest.main()