-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prmdr 180 #83
Prmdr 180 #83
Changes from 18 commits
8360dfe
bbcd5c0
b88e24e
5e86907
b424aa2
969c008
e61405f
6ed5c43
b62546a
ce71612
36d5b1b
892eb09
76eab37
7b93ad7
cbed6c6
bda7459
67bedf0
a0d0197
0f4882d
e16ba88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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,20 @@ or... | |||||
} | ||||||
``` | ||||||
|
||||||
### document_manifest_by_nhs_number_handler | ||||||
|
||||||
The manifest lambda expects two query string parameters called patientId and docType. | ||||||
|
||||||
PatientID is an NHS Number | ||||||
|
||||||
This is an array and can be set to the following values: | ||||||
|
||||||
For just Lloyd George docs ["LG"] | ||||||
|
||||||
For just ARF docs ["ARF"] | ||||||
|
||||||
For all docs ["LG", "ARF"] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from the implementation and tests, seems to me that options of docType that we support are
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation has been updated :) |
||||||
|
||||||
If the value is not one of the above speified then a 204 will be returned | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
|
||||||
this applies to branch PRMDR-180 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"] | ||
NogaNHS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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"] | ||
AlexHerbertNHS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want this doc_type to be a string or an array? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's going to be a string from the UI side; I'll update the readme. Apologies for the confusion |
||
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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[ | ||
AlexHerbertNHS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DocumentReferenceMetadataFields.VIRUS_SCAN_RESULT.field_name | ||
], | ||
file_location=item[ | ||
DocumentReferenceMetadataFields.FILE_LOCATION.field_name | ||
], | ||
) | ||
documents.append(document) | ||
return documents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be
docType is an array / a string and can be set to the following values:
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done