Prmdr 180 (#83)

* lloyd george document manifest lambda and test

* lloyd george manifest code tidy up

* Separate dynamoDB repo code into service

* Filter documents by type in the manifest handler

* [PRMDR-270]
Adding explicit import for proper compiling
Fixing test so it can be run by Windows users

* [PRMDR-180]

finished on the code; need to rework a few tests due to refactoring

* PRMDR-180

* PRMDR-180

Adding more tests, some renaming, some refactoring, some import cleanup

* PRMDR-180

Finished testing the Manifest Dynamo Service

* PRMDR-180

Small cleanup, removing unnecessary comments and imports

* Tests

* PRMDR-180

Fix for existing tests

* PRMDR-180

More test fixes

* PRMDR-180

Adding final bits for comprehensive unit tests

* PRMDR-180

Making changes and refactors as per code review

* PRMDR-180

fixes for tests based on code review changes

* PRMDR-180

Changing doctype validation logic and adding test as per code review

* PRMDR-180

Changes as per code review

---------

Co-authored-by: Richard Smith <richard.smith33@nhs.net>

lambdas/README.md

@@ -96,9 +96,7 @@ If successful, the lambda will return status code 200 with patient details as th
 
 ```json
 {
-  "givenName": [
-    "Jane"
-  ],
+  "givenName": ["Jane"],
   "familyName": "Smith",
   "birthDate": "2010-10-22",
   "postalCode": "LS1 6AE",
@@ -122,7 +120,7 @@ Testing in AWS on the lambda directly:
 ```json
 {
   "queryStringParameters": {
-      "patientId":"9449305552"
+    "patientId": "9449305552"
   }
 }
 ```
@@ -139,7 +137,6 @@ Hitting URL directly:
 https://{url}/SearchDocumentReferences?patientId=9449305552
 ```
 
-
 #### Possible outputs
 
 Success:
@@ -232,3 +229,19 @@ or...
 }
 ```
 
+### document_manifest_by_nhs_number_handler
+
+The manifest lambda expects two query string parameters called patientId and docType.
+
+**patientId** is to be supplied as a String, and should conform to standard NHS Number format
+
+**docType** is a String and expects a single or comma-seperated list of types of document you're searching for.
+It can be set to the following values:
+
+For just Lloyd George docs "LG"
+
+For just ARF docs "ARF"
+
+For all docs "LG,ARF"
+
+If the parameter is not supplied, the values contain something unspecified, or it is an empty String, a 400 error will be returned

lambdas/handlers/document_manifest_by_nhs_number_handler.py

@@ -2,49 +2,46 @@
 import os
 
 from botocore.exceptions import ClientError
-from enums.metadata_field_names import DocumentReferenceMetadataFields
-from models.document import Document
+from enums.supported_document_types import SupportedDocumentTypes
+from lambdas.utils.decorators.validate_patient_id import validate_patient_id
+from lambdas.utils.decorators.ensure_env_var import ensure_environment_variables
+from services.manifest_dynamo_service import ManifestDynamoService
 from services.document_manifest_service import DocumentManifestService
-from services.dynamo_service import DynamoDBService
-from utils.exceptions import (DynamoDbException, InvalidResourceIdException,
+from utils.decorators.validate_document_type import validate_document_type
+from utils.exceptions import (DynamoDbException,
                               ManifestDownloadException)
 from utils.lambda_response import ApiGatewayResponse
-from utils.utilities import validate_id
 
 logger = logging.getLogger()
 logger.setLevel(logging.INFO)
 
 
+@validate_patient_id
+@validate_document_type
+@ensure_environment_variables(
+    names=["DOCUMENT_STORE_DYNAMODB_NAME",
+           "LLOYD_GEORGE_DYNAMODB_NAME",
+           "ZIPPED_STORE_BUCKET_NAME",
+           "ZIPPED_STORE_DYNAMODB_NAME"
+           ]
+)
 def lambda_handler(event, context):
     logger.info("Starting document manifest process")
 
     try:
         nhs_number = event["queryStringParameters"]["patientId"]
-        validate_id(nhs_number)
+        doc_type = event["queryStringParameters"]["docType"]
 
-        document_store_table_name = os.environ["DOCUMENT_STORE_DYNAMODB_NAME"]
-        lloyd_george_table_name = os.environ["LLOYD_GEORGE_DYNAMODB_NAME"]
         zip_output_bucket = os.environ["ZIPPED_STORE_BUCKET_NAME"]
         zip_trace_table_name = os.environ["ZIPPED_STORE_DYNAMODB_NAME"]
         # zip_trace_ttl = os.environ["DOCUMENT_ZIP_TRACE_TTL_IN_DAYS"]
 
-        dynamo_service = DynamoDBService()
-
-        logger.info("Retrieving doc store documents")
-        ds_documents = query_documents(
-            dynamo_service, document_store_table_name, nhs_number
-        )
-
-        logger.info("Retrieving lloyd george documents")
-        lg_documents = query_documents(
-            dynamo_service, lloyd_george_table_name, nhs_number
-        )
-
-        documents = lg_documents + ds_documents
+        dynamo_service = ManifestDynamoService()
+        documents = dynamo_service.discover_uploaded_documents(nhs_number, doc_type)
 
         if not documents:
             return ApiGatewayResponse(
-                204, "No documents found for given NHS number", "GET"
+                204, "No documents found for given NHS number and document type", "GET"
             ).create_api_gateway_response()
 
         logger.info("Starting document manifest process")
@@ -59,14 +56,6 @@ def lambda_handler(event, context):
 
         return ApiGatewayResponse(200, response, "GET").create_api_gateway_response()
 
-    except InvalidResourceIdException:
-        return ApiGatewayResponse(
-            400, "Invalid NHS number", "GET"
-        ).create_api_gateway_response()
-    except KeyError as e:
-        return ApiGatewayResponse(
-            400, f"An error occurred due to missing key: {str(e)}", "GET"
-        ).create_api_gateway_response()
     except ManifestDownloadException as e:
         return ApiGatewayResponse(
             500,
@@ -85,38 +74,3 @@ def lambda_handler(event, context):
             500, "An error occurred when creating document manifest", "POST"
         ).create_api_gateway_response()
         return response
-
-
-def query_documents(
-    dynamo_service: DynamoDBService, document_table: str, nhs_number: str
-) -> list[Document]:
-    documents = []
-
-    response = dynamo_service.query_service(
-        document_table,
-        "NhsNumberIndex",
-        "NhsNumber",
-        nhs_number,
-        [
-            DocumentReferenceMetadataFields.FILE_NAME,
-            DocumentReferenceMetadataFields.FILE_LOCATION,
-            DocumentReferenceMetadataFields.VIRUS_SCAN_RESULT,
-        ],
-    )
-    if response is None or ("Items" not in response):
-        logger.error(f"Unrecognised response from DynamoDB: {response}")
-        raise DynamoDbException("Unrecognised response from DynamoDB")
-
-    for item in response["Items"]:
-        document = Document(
-            nhs_number=nhs_number,
-            file_name=item[DocumentReferenceMetadataFields.FILE_NAME.field_name],
-            virus_scanner_result=item[
-                DocumentReferenceMetadataFields.VIRUS_SCAN_RESULT.field_name
-            ],
-            file_location=item[
-                DocumentReferenceMetadataFields.FILE_LOCATION.field_name
-            ],
-        )
-        documents.append(document)
-    return documents

lambdas/services/manifest_dynamo_service.py

@@ -0,0 +1,55 @@
+import logging
+import os
+
+from enums.supported_document_types import SupportedDocumentTypes
+from services.dynamo_service import DynamoDBService
+from enums.metadata_field_names import DocumentReferenceMetadataFields
+from models.document import Document
+from utils.exceptions import DynamoDbException
+
+logger = logging.getLogger()
+logger.setLevel(logging.INFO)
+
+
+class ManifestDynamoService(DynamoDBService):
+
+    def discover_uploaded_documents(
+            self, nhs_number: str, doc_types: str
+    ) -> list[Document]:
+        arf_documents = []
+        lg_documents = []
+
+        if SupportedDocumentTypes.ARF.name in doc_types:
+            arf_documents = self.fetch_documents_from_table(nhs_number, os.environ["DOCUMENT_STORE_DYNAMODB_NAME"])
+        if SupportedDocumentTypes.LG.name in doc_types:
+            lg_documents = self.fetch_documents_from_table(nhs_number, os.environ["LLOYD_GEORGE_DYNAMODB_NAME"])
+
+        return arf_documents + lg_documents
+
+    def fetch_documents_from_table(self, nhs_number: str, table: str) -> list[Document]:
+        documents = []
+        response = self.query_service(
+            table,
+            "NhsNumberIndex",
+            "NhsNumber",
+            nhs_number,
+            [
+                DocumentReferenceMetadataFields.FILE_NAME,
+                DocumentReferenceMetadataFields.FILE_LOCATION,
+                DocumentReferenceMetadataFields.VIRUS_SCAN_RESULT,
+            ],
+        )
+
+        for item in response["Items"]:
+            document = Document(
+                nhs_number=nhs_number,
+                file_name=item[DocumentReferenceMetadataFields.FILE_NAME.field_name],
+                virus_scanner_result=item[
+                    DocumentReferenceMetadataFields.VIRUS_SCAN_RESULT.field_name
+                ],
+                file_location=item[
+                    DocumentReferenceMetadataFields.FILE_LOCATION.field_name
+                ],
+            )
+            documents.append(document)
+        return documents

lambdas/tests/unit/handlers/conftest.py

@@ -2,6 +2,8 @@
 
 import pytest
 
+from lambdas.enums.supported_document_types import SupportedDocumentTypes
+
 
 @pytest.fixture
 def valid_id_event():
@@ -11,6 +13,38 @@ def valid_id_event():
     return api_gateway_proxy_event
 
 
+@pytest.fixture
+def valid_id_and_both_doctype_event():
+    api_gateway_proxy_event = {
+        "queryStringParameters": {"patientId": "9000000009", "docType": "LG,ARF"},
+    }
+    return api_gateway_proxy_event
+
+
+@pytest.fixture
+def valid_id_and_arf_doctype_event():
+    api_gateway_proxy_event = {
+        "queryStringParameters": {"patientId": "9000000009", "docType": "ARF"},
+    }
+    return api_gateway_proxy_event
+
+
+@pytest.fixture
+def valid_id_and_lg_doctype_event():
+    api_gateway_proxy_event = {
+        "queryStringParameters": {"patientId": "9000000009", "docType": "LG"},
+    }
+    return api_gateway_proxy_event
+
+
+@pytest.fixture
+def valid_id_and_invalid_doctype_event():
+    api_gateway_proxy_event = {
+        "queryStringParameters": {"patientId": "9000000009", "docType": "MANGO"},
+    }
+    return api_gateway_proxy_event
+
+
 @pytest.fixture
 def invalid_id_event():
     api_gateway_proxy_event = {