[PRMP-1596] Documents incorrectly sent to review queue when PDS response body fails to parse (#1176)

Co-authored-by: NogaNHS <127490765+NogaNHS@users.noreply.github.com>
Co-authored-by: Robert Gaskin <106234256+robg-test@users.noreply.github.com>

Author: 3 people

lambdas/services/bulk_upload_service.py

@@ -32,6 +32,8 @@
     PatientNotFoundException,
     PatientRecordAlreadyExistException,
     PdsErrorException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
     S3FileNotFoundException,
     VirusScanFailedException,
@@ -203,6 +205,21 @@ def handle_sqs_message(self, message: dict):
                         "PDS record is restricted",
                     )
 
+        except (PdsPatientValidationException, PdsHttpErrorException) as error:
+            logger.info(
+                f"Detected issue related to patient number: {staging_metadata.nhs_number}",
+            )
+            logger.error(error)
+            logger.info("Will stop processing Lloyd George record for this patient.")
+            logger.info("A PDS error occurred, not sending message to review queue.")
+            self.dynamo_repository.write_report_upload_to_dynamo(
+                staging_metadata,
+                UploadStatus.FAILED,
+                str(error),
+                patient_ods_code,
+                sent_to_review=False,
+            )
+            return
         except (
             InvalidNhsNumberException,
             LGInvalidFilesException,
@@ -214,13 +231,10 @@ def handle_sqs_message(self, message: dict):
             )
             logger.error(error)
             logger.info("Will stop processing Lloyd George record for this patient.")
-
-            reason = str(error)
-
             self.dynamo_repository.write_report_upload_to_dynamo(
                 staging_metadata,
                 UploadStatus.FAILED,
-                reason,
+                str(error),
                 patient_ods_code,
                 sent_to_review=self.send_to_review_enabled,
             )

lambdas/services/create_document_reference_service.py

@@ -25,6 +25,9 @@
     LGInvalidFilesException,
     OdsErrorException,
     PatientNotFoundException,
+    PdsErrorException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
 )
 from utils.lambda_exceptions import DocumentRefException
@@ -123,6 +126,9 @@ def create_document_reference_request(
             LGInvalidFilesException,
             PdsTooManyRequestsException,
             ConfigNotFoundException,
+            PdsErrorException,
+            PdsPatientValidationException,
+            PdsHttpErrorException,
         ) as e:
             logger.error(
                 f"{LambdaError.DocRefInvalidFiles.to_str()} :{str(e)}",

lambdas/services/document_review_processor_service.py

@@ -3,6 +3,7 @@
 from datetime import datetime, timezone
 
 from botocore.exceptions import ClientError
+
 from enums.document_review_status import DocumentReviewStatus
 from models.document_review import (
     DocumentReviewFileDetails,
@@ -35,9 +36,8 @@ def __init__(self):
         self.review_bucket_name = os.environ["PENDING_REVIEW_BUCKET_NAME"]
 
     def process_review_message(self, review_message: ReviewMessageBody) -> None:
-        logger.info("Processing review queue message")
-
         request_context.patient_nhs_no = review_message.nhs_number
+        logger.info("Processing review queue message")
 
         review_id = review_message.upload_id
         review_files = self._move_files_to_review_bucket(review_message, review_id)

lambdas/services/update_document_reference_service.py

@@ -19,6 +19,9 @@
     LGInvalidFilesException,
     OdsErrorException,
     PatientNotFoundException,
+    PdsErrorException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
 )
 from utils.lambda_exceptions import DocumentRefException
@@ -98,6 +101,9 @@ def update_document_reference_request(
             InvalidNhsNumberException,
             LGInvalidFilesException,
             PdsTooManyRequestsException,
+            PdsPatientValidationException,
+            PdsErrorException,
+            PdsHttpErrorException,
         ) as e:
             logger.error(
                 f"{LambdaError.DocRefInvalidFiles.to_str()} :{str(e)}",

lambdas/tests/unit/services/test_bulk_upload_service.py

@@ -52,6 +52,8 @@
     InvalidMessageException,
     PatientNotFoundException,
     PatientRecordAlreadyExistException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
     S3FileNotFoundException,
     TagNotFoundException,
@@ -1397,7 +1399,6 @@ def test_sends_to_review_queue_when_invalid_files(
     repo_with_review_enabled,
     mock_validate_files,
     mock_pds_service,
-    mocker,
 ):
     mocked_error = LGInvalidFilesException(
         "One or more of the files do not match naming convention",
@@ -1413,6 +1414,41 @@ def test_sends_to_review_queue_when_invalid_files(
     )
 
 
+def test_does_not_send_to_review_queue_pds_validation_error(
+    repo_with_review_enabled,
+    mocker,
+):
+    mocked_error = PdsPatientValidationException(
+        "Fail to parse the patient detail response from PDS API.",
+    )
+
+    mock_pds_service = mocker.patch(
+        "services.bulk_upload_service.getting_patient_info_from_pds",
+    )
+
+    mock_pds_service.side_effect = mocked_error
+
+    repo_with_review_enabled.handle_sqs_message(message=TEST_SQS_MESSAGE)
+
+    repo_with_review_enabled.sqs_repository.send_message_to_review_queue.assert_not_called()
+
+
+def test_does_not_send_to_review_queue_pds_http_error(mocker, repo_with_review_enabled):
+    mocked_error = PdsHttpErrorException(
+        "Failed to retrieve patient date from PDS",
+    )
+
+    mock_pds_service = mocker.patch(
+        "services.bulk_upload_service.getting_patient_info_from_pds",
+    )
+
+    mock_pds_service.side_effect = mocked_error
+
+    repo_with_review_enabled.handle_sqs_message(message=TEST_SQS_MESSAGE)
+
+    repo_with_review_enabled.sqs_repository.send_message_to_review_queue.assert_not_called()
+
+
 def test_does_not_send_to_review_queue_when_virus_scan_fails(
     repo_with_review_enabled,
     mock_validate_files,

lambdas/tests/unit/services/test_create_document_reference_service.py

@@ -33,7 +33,11 @@
 from utils.common_query_filters import NotDeleted
 from utils.constants.ssm import UPLOAD_PILOT_ODS_ALLOWED_LIST
 from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder
-from utils.exceptions import PatientNotFoundException
+from utils.exceptions import (
+    PatientNotFoundException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
+)
 from utils.lambda_exceptions import DocumentRefException
 from utils.lloyd_george_validator import LGInvalidFilesException
 from utils.request_context import request_context
@@ -305,7 +309,7 @@ def test_create_document_reference_failed_to_parse_pds_response(
     mock_getting_patient_info_from_pds,
     mock_fetch_available_document_references_by_type,
 ):
-    mock_getting_patient_info_from_pds.side_effect = LGInvalidFilesException
+    mock_getting_patient_info_from_pds.side_effect = PdsPatientValidationException
 
     with pytest.raises(Exception) as exc_info:
         mock_create_doc_ref_service.create_document_reference_request(
@@ -344,6 +348,29 @@ def test_cdr_nhs_number_not_found_raises_search_patient_exception(
     mock_create_document_reference.assert_not_called()
 
 
+def test_create_document_reference_pds_general_error_throws_exception(
+    mock_fhir_doc_ref_base_service,
+    mock_create_doc_ref_service,
+    mock_create_document_reference,
+    mock_getting_patient_info_from_pds,
+    mock_fetch_available_document_references_by_type,
+):
+    mock_getting_patient_info_from_pds.side_effect = PdsHttpErrorException
+
+    with pytest.raises(Exception) as exc_info:
+        mock_create_doc_ref_service.create_document_reference_request(
+            TEST_NHS_NUMBER,
+            LG_FILE_LIST,
+        )
+
+    exception = exc_info.value
+    assert isinstance(exception, DocumentRefException)
+    assert exception.status_code == 400
+    assert exception.message == "Invalid files or id"
+
+    mock_create_document_reference.assert_not_called()
+
+
 def test_cdr_non_pdf_file_raises_exception(
     mock_fhir_doc_ref_base_service,
     mock_create_doc_ref_service,

lambdas/tests/unit/services/test_update_document_reference_service.py

@@ -14,7 +14,11 @@
     create_test_doc_store_refs,
     create_test_lloyd_george_doc_store_refs,
 )
-from utils.exceptions import LGInvalidFilesException, PatientNotFoundException
+from utils.exceptions import (
+    LGInvalidFilesException,
+    PatientNotFoundException,
+    PdsHttpErrorException,
+)
 from utils.lambda_exceptions import DocumentRefException
 from utils.request_context import request_context
 
@@ -238,6 +242,32 @@ def test_nhs_number_not_found_raises_exception(
     mock_process_fhir_document_reference.assert_not_called()
 
 
+def test_general_pds_error_raises_exception(
+    mock_fhir_doc_ref_base_service,
+    mock_getting_patient_info_from_pds,
+    mock_update_doc_ref_service,
+    mock_check_if_ods_code_is_in_pilot,
+    mock_process_fhir_document_reference,
+    mock_fetch_documents_from_table,
+):
+    mock_getting_patient_info_from_pds.side_effect = PdsHttpErrorException
+    mock_fetch_documents_from_table.return_value = create_test_doc_store_refs()
+
+    with pytest.raises(DocumentRefException) as exc_info:
+        mock_update_doc_ref_service.update_document_reference_request(
+            TEST_NHS_NUMBER,
+            LG_FILE,
+            TEST_UUID,
+        )
+
+    exception = exc_info.value
+    assert isinstance(exception, DocumentRefException)
+    assert exception.status_code == 400
+
+    mock_check_if_ods_code_is_in_pilot.assert_not_called()
+    mock_process_fhir_document_reference.assert_not_called()
+
+
 # covers for number of files expected, non-pdf files, incorrect file name format, duplicate files
 def test_invalid_files_raises_exception(
     mock_fhir_doc_ref_base_service,

lambdas/tests/unit/utils/test_lloyd_george_validator.py

@@ -35,6 +35,9 @@
 from utils.exceptions import (
     PatientNotFoundException,
     PatientRecordAlreadyExistException,
+    PdsErrorException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
 )
 from utils.lloyd_george_validator import (
@@ -881,7 +884,7 @@ def test_bad_request_with_pds_service(mock_pds_call):
 
     mock_pds_call.return_value = response
 
-    with pytest.raises(LGInvalidFilesException) as e:
+    with pytest.raises(PdsHttpErrorException) as e:
         getting_patient_info_from_pds("9000000009")
     assert str(e.value) == "Failed to retrieve patient data from PDS"
 
@@ -916,19 +919,19 @@ def test_check_pds_response_404_status_raises_patient_not_found_exception():
         check_pds_response_status(response)
 
 
-def test_check_pds_response_4xx_or_5xx_status_raise_lg_invalid_files_exception():
+def test_check_pds_response_4xx_or_5xx_status_raise_pds_error_exception():
     response = Response()
     response.status_code = 500
 
-    with pytest.raises(LGInvalidFilesException):
+    with pytest.raises(PdsHttpErrorException):
         check_pds_response_status(response)
 
 
 def test_check_pds_response_200_status_not_raise_exception():
     response = Response()
     response.status_code = 200
 
-    with expect_not_to_raise(LGInvalidFilesException):
+    with expect_not_to_raise(PdsErrorException):
         check_pds_response_status(response)
 
 
@@ -949,7 +952,7 @@ def test_parse_pds_response_raise_error_when_model_validation_failed():
         b"""{"body": "some data that pydantic cannot parse"}"""
     )
 
-    with pytest.raises(LGInvalidFilesException) as error:
+    with pytest.raises(PdsPatientValidationException) as error:
         parse_pds_response(mock_invalid_pds_response)
 
     assert str(error.value) == "Fail to parse the patient detail response from PDS API."

lambdas/utils/exceptions.py

@@ -22,10 +22,18 @@ class PdsErrorException(Exception):
     pass
 
 
+class PdsPatientValidationException(Exception):
+    pass
+
+
 class PdsTooManyRequestsException(Exception):
     pass
 
 
+class PdsHttpErrorException(Exception):
+    pass
+
+
 class AuthorisationException(Exception):
     pass
 

lambdas/utils/lloyd_george_validator.py

@@ -19,6 +19,8 @@
     LGInvalidFilesException,
     PatientNotFoundException,
     PatientRecordAlreadyExistException,
+    PdsHttpErrorException,
+    PdsPatientValidationException,
     PdsTooManyRequestsException,
 )
 from utils.unicode_utils import (
@@ -383,7 +385,7 @@ def check_pds_response_status(pds_response: requests.Response):
         pds_response.raise_for_status()
     except HTTPError as e:
         logger.error(e)
-        raise LGInvalidFilesException("Failed to retrieve patient data from PDS")
+        raise PdsHttpErrorException("Failed to retrieve patient data from PDS")
 
 
 def parse_pds_response(pds_response: requests.Response) -> Patient:
@@ -393,7 +395,7 @@ def parse_pds_response(pds_response: requests.Response) -> Patient:
     except pydantic.ValidationError:
         error_msg = "Fail to parse the patient detail response from PDS API."
         logger.error(error_msg)
-        raise LGInvalidFilesException(error_msg)
+        raise PdsPatientValidationException(error_msg)
 
 
 def get_allowed_ods_codes() -> list[str]: