Merge pull request #564 from NHSDigital/feature/thju1-NRL-641-fixSonarCloudWarnings

NRL-641 Fix SonarCloud warnings

Author: eesa456

api/consumer/readDocumentReference/tests/test_read_document_reference_consumer.py

@@ -3,6 +3,7 @@
 from moto import mock_aws
 
 from api.consumer.readDocumentReference.read_document_reference import handler
+from nrlf.core.constants import PointerTypes
 from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository
 from nrlf.tests.data import load_document_reference
 from nrlf.tests.dynamodb import mock_repository
@@ -109,9 +110,7 @@ def test_read_document_reference_unauthorised_for_type(
     repository.create(doc_pointer)
 
     event = create_test_api_gateway_event(
-        headers=create_headers(
-            pointer_types=["http://snomed.info/sct|887701000000100"]
-        ),
+        headers=create_headers(pointer_types=[PointerTypes.EMERGENCY_HEALTHCARE_PLAN]),
         path_parameters={"id": doc_pointer.id},
     )
 
@@ -156,7 +155,7 @@ def test_document_reference_invalid_json(repository: DocumentPointerRepository):
     repository.create(doc_pointer)
 
     event = create_test_api_gateway_event(
-        headers=create_headers(pointer_types=["http://snomed.info/sct|736253002"]),
+        headers=create_headers(pointer_types=[PointerTypes.MENTAL_HEALTH_PLAN]),
         path_parameters={"id": doc_pointer.id},
     )
 

api/producer/createDocumentReference/create_document_reference.py

@@ -1,8 +1,13 @@
 from uuid import uuid4
 
-from nrlf.core.constants import PERMISSION_AUDIT_DATES_FROM_PAYLOAD
+from nrlf.core.codes import SpineErrorConcept
+from nrlf.core.constants import (
+    PERMISSION_AUDIT_DATES_FROM_PAYLOAD,
+    PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL,
+)
 from nrlf.core.decorators import request_handler
 from nrlf.core.dynamodb.repository import DocumentPointer, DocumentPointerRepository
+from nrlf.core.errors import OperationOutcomeError
 from nrlf.core.logger import LogReference, logger
 from nrlf.core.model import ConnectionMetadata
 from nrlf.core.response import NRLResponse, Response, SpineErrorResponse
@@ -37,48 +42,28 @@ def _set_create_time_fields(
     return document_reference
 
 
-@request_handler(body=DocumentReference)
-def handler(
-    metadata: ConnectionMetadata,
-    repository: DocumentPointerRepository,
-    body: DocumentReference,
-) -> Response:
+def _create_core_model(resource: DocumentReference, metadata: ConnectionMetadata):
     """
-    Creates a document reference.
-
-    Args:
-        metadata (ConnectionMetadata): The connection metadata.
-        repository (DocumentPointerRepository): The document pointer repository.
-        body (DocumentReference): The document reference to create.
-
-    Returns:
-        Response: The response indicating the result of the operation.
+    Create the DocumentPointer model from the provided DocumentReference
     """
-    logger.log(LogReference.PROCREATE000)
-
-    logger.log(LogReference.PROCREATE001, resource=body)
-
-    id_prefix = "|".join(metadata.ods_code_parts)
-    body.id = f"{id_prefix}-{uuid4()}"
-
-    validator = DocumentReferenceValidator()
-    result = validator.validate(body)
-
-    if not result.is_valid:
-        logger.log(LogReference.PROCREATE002)
-        return Response.from_issues(issues=result.issues, statusCode="400")
-
     creation_time = create_fhir_instant()
     document_reference = _set_create_time_fields(
         creation_time,
-        document_reference=result.resource,
+        document_reference=resource,
         nrl_permissions=metadata.nrl_permissions,
     )
 
-    core_model = DocumentPointer.from_document_reference(
+    return DocumentPointer.from_document_reference(
         document_reference, created_on=creation_time
     )
 
+
+def _check_permissions(
+    core_model: DocumentPointer, metadata: ConnectionMetadata
+) -> Response | None:
+    """
+    Check the requester has permissions to create the DocumentReference
+    """
     custodian_parts = tuple(
         filter(None, (core_model.custodian, core_model.custodian_suffix))
     )
@@ -105,71 +90,175 @@ def handler(
             expression="type.coding[0].code",
         )
 
+
+def _get_document_ids_to_supersede(
+    resource: DocumentReference,
+    core_model: DocumentPointer,
+    metadata: ConnectionMetadata,
+    repository: DocumentPointerRepository,
+    can_ignore_delete_fail: bool,
+):
+    """
+    Get the list of document IDs to supersede based on the relatesTo field
+    """
+    if not resource.relatesTo:
+        return []
+
+    logger.log(LogReference.PROCREATE006, relatesTo=resource.relatesTo)
     ids_to_delete = []
 
-    if result.resource.relatesTo:
-        logger.log(LogReference.PROCREATE006, relatesTo=result.resource.relatesTo)
-
-        for idx, relates_to in enumerate(result.resource.relatesTo):
-            if not (identifier := getattr(relates_to.target.identifier, "value", None)):
-                logger.log(LogReference.PROCREATE007a)
-                return SpineErrorResponse.BAD_REQUEST(
-                    diagnostics="No identifier value provided for relatesTo target",
-                    expression=f"relatesTo[{idx}].target.identifier.value",
-                )
-
-            producer_id = identifier.split("-", 1)[0]
-            if metadata.ods_code_parts != tuple(producer_id.split("|")):
-                logger.log(
-                    LogReference.PROCREATE007b,
-                    related_identifier=identifier,
-                    ods_code_parts=metadata.ods_code_parts,
-                )
-                return SpineErrorResponse.BAD_REQUEST(
-                    diagnostics="The relatesTo target identifier value does not include the expected ODS code for this organisation",
-                    expression=f"relatesTo[{idx}].target.identifier.value",
-                )
-
-            if not (existing_pointer := repository.get_by_id(identifier)):
-                logger.log(LogReference.PROCREATE007c, related_identifier=identifier)
-                return SpineErrorResponse.BAD_REQUEST(
-                    diagnostics="The relatesTo target document does not exist",
-                    expression=f"relatesTo[{idx}].target.identifier.value",
-                )
-
-            if existing_pointer.nhs_number != core_model.nhs_number:
-                logger.log(LogReference.PROCREATE007d, related_identifier=identifier)
-                return SpineErrorResponse.BAD_REQUEST(
-                    diagnostics="The relatesTo target document NHS number does not match the NHS number in the request",
-                    expression=f"relatesTo[{idx}].target.identifier.value",
-                )
-
-            if existing_pointer.type != core_model.type:
-                logger.log(LogReference.PROCREATE007e, related_identifier=identifier)
-                return SpineErrorResponse.BAD_REQUEST(
-                    diagnostics="The relatesTo target document type does not match the type in the request",
-                    expression=f"relatesTo[{idx}].target.identifier.value",
-                )
-
-            if relates_to.code == "replaces":
-                logger.log(
-                    LogReference.PROCREATE008,
-                    relates_to_code=relates_to.code,
-                    identifier=identifier,
-                )
-                ids_to_delete.append(identifier)
-
-    if ids_to_delete:
+    for idx, relates_to in enumerate(resource.relatesTo):
+        identifier = _validate_identifier(relates_to, idx)
+        _validate_producer_id(identifier, metadata, idx)
+
+        if not can_ignore_delete_fail:
+            existing_pointer = _check_existing_pointer(identifier, repository, idx)
+            _validate_pointer_details(existing_pointer, core_model, identifier, idx)
+
+        _append_id_if_replaces(relates_to, ids_to_delete, identifier)
+
+    return ids_to_delete
+
+
+def _validate_identifier(relates_to, idx):
+    """
+    Validate that there is a identifier in relatesTo target
+    """
+    identifier = getattr(relates_to.target.identifier, "value", None)
+    if not identifier:
+        logger.log(LogReference.PROCREATE007a)
+        _raise_operation_outcome_error(
+            "No identifier value provided for relatesTo target", idx
+        )
+    return identifier
+
+
+def _validate_producer_id(identifier, metadata, idx):
+    """
+    Validate that there is an ODS code in the relatesTo target identifier
+    """
+    producer_id = identifier.split("-", 1)[0]
+    if metadata.ods_code_parts != tuple(producer_id.split("|")):
+        logger.log(
+            LogReference.PROCREATE007b,
+            related_identifier=identifier,
+            ods_code_parts=metadata.ods_code_parts,
+        )
+        _raise_operation_outcome_error(
+            "The relatesTo target identifier value does not include the expected ODS code for this organisation",
+            idx,
+        )
+
+
+def _check_existing_pointer(identifier, repository, idx):
+    """
+    Check that there is an existing pointer that will be deleted when superseding
+    """
+    existing_pointer = repository.get_by_id(identifier)
+    if not existing_pointer:
+        logger.log(LogReference.PROCREATE007c, related_identifier=identifier)
+        _raise_operation_outcome_error(
+            "The relatesTo target document does not exist", idx
+        )
+    return existing_pointer
+
+
+def _validate_pointer_details(existing_pointer, core_model, identifier, idx):
+    """
+    Validate that the nhs numbers and type matches between the existing pointer and the requested one.
+    """
+    if existing_pointer.nhs_number != core_model.nhs_number:
+        logger.log(LogReference.PROCREATE007d, related_identifier=identifier)
+        _raise_operation_outcome_error(
+            "The relatesTo target document NHS number does not match the NHS number in the request",
+            idx,
+        )
+
+    if existing_pointer.type != core_model.type:
+        logger.log(LogReference.PROCREATE007e, related_identifier=identifier)
+        _raise_operation_outcome_error(
+            "The relatesTo target document type does not match the type in the request",
+            idx,
+        )
+
+
+def _append_id_if_replaces(relates_to, ids_to_delete, identifier):
+    """
+    Append pointer ID if the if the relatesTo code is 'replaces'
+    """
+    if relates_to.code == "replaces":
+        logger.log(
+            LogReference.PROCREATE008,
+            relates_to_code=relates_to.code,
+            identifier=identifier,
+        )
+        ids_to_delete.append(identifier)
+
+
+def _raise_operation_outcome_error(diagnostics, idx):
+    """
+    General function to raise an operation outcome error
+    """
+    raise OperationOutcomeError(
+        severity="error",
+        code="invalid",
+        details=SpineErrorConcept.from_code("BAD_REQUEST"),
+        diagnostics=diagnostics,
+        expression=[f"relatesTo[{idx}].target.identifier.value"],
+    )
+
+
+@request_handler(body=DocumentReference)
+def handler(
+    metadata: ConnectionMetadata,
+    repository: DocumentPointerRepository,
+    body: DocumentReference,
+) -> Response:
+    """
+    Creates a document reference.
+
+    Args:
+        metadata (ConnectionMetadata): The connection metadata.
+        repository (DocumentPointerRepository): The document pointer repository.
+        body (DocumentReference): The document reference to create.
+
+    Returns:
+        Response: The response indicating the result of the operation.
+    """
+    logger.log(LogReference.PROCREATE000)
+    logger.log(LogReference.PROCREATE001, resource=body)
+
+    id_prefix = "|".join(metadata.ods_code_parts)
+    body.id = f"{id_prefix}-{uuid4()}"
+
+    validator = DocumentReferenceValidator()
+    result = validator.validate(body)
+
+    if not result.is_valid:
+        logger.log(LogReference.PROCREATE002)
+        return Response.from_issues(issues=result.issues, statusCode="400")
+
+    core_model = _create_core_model(result.resource, metadata)
+    if error_response := _check_permissions(core_model, metadata):
+        return error_response
+
+    can_ignore_delete_fail = (
+        PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL in metadata.nrl_permissions
+    )
+
+    if ids_to_delete := _get_document_ids_to_supersede(
+        result.resource, core_model, metadata, repository, can_ignore_delete_fail
+    ):
         logger.log(
             LogReference.PROCREATE010,
             pointer_id=result.resource.id,
             ids_to_delete=ids_to_delete,
         )
-        saved_model = repository.supersede(core_model, ids_to_delete)
+        repository.supersede(core_model, ids_to_delete, can_ignore_delete_fail)
         logger.log(LogReference.PROCREATE999)
         return NRLResponse.RESOURCE_SUPERSEDED(resource_id=result.resource.id)
 
     logger.log(LogReference.PROCREATE009, pointer_id=result.resource.id)
-    saved_model = repository.create(core_model)
+    repository.create(core_model)
     logger.log(LogReference.PROCREATE999)
     return NRLResponse.RESOURCE_CREATED(resource_id=result.resource.id)