diff --git a/.github/workflows/base-lambdas-reusable-deploy-all.yml b/.github/workflows/base-lambdas-reusable-deploy-all.yml index 7f8fad3ed..2a43e45d3 100644 --- a/.github/workflows/base-lambdas-reusable-deploy-all.yml +++ b/.github/workflows/base-lambdas-reusable-deploy-all.yml @@ -488,6 +488,20 @@ jobs: secrets: AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }} + deploy_patch_document_review_lambda: + name: Deploy patch document review lambda + uses: ./.github/workflows/base-lambdas-reusable-deploy.yml + with: + environment: ${{ inputs.environment}} + python_version: ${{ inputs.python_version }} + build_branch: ${{ inputs.build_branch}} + sandbox: ${{ inputs.sandbox }} + lambda_handler_name: patch_document_review_handler + lambda_aws_name: PatchDocumentReview + lambda_layer_names: "core_lambda_layer" + secrets: + AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }} + deploy_get_document_review_lambda: name: Deploy get document review lambda uses: ./.github/workflows/base-lambdas-reusable-deploy.yml diff --git a/lambdas/enums/document_review_accepted_querystring_parameters.py b/lambdas/enums/document_review_accepted_querystring_parameters.py index 77e72145b..95c59e8f2 100644 --- a/lambdas/enums/document_review_accepted_querystring_parameters.py +++ b/lambdas/enums/document_review_accepted_querystring_parameters.py @@ -6,4 +6,3 @@ class DocumentReviewQuerystringParameters(StrEnum): NEXT_PAGE_TOKEN = "nextPageToken" UPLOADER = "uploader" NHS_NUMBER = "nhsNumber" - diff --git a/lambdas/enums/document_review_status.py b/lambdas/enums/document_review_status.py index 3fdc5c451..bf12b97bb 100644 --- a/lambdas/enums/document_review_status.py +++ b/lambdas/enums/document_review_status.py @@ -4,4 +4,9 @@ class DocumentReviewStatus(StrEnum): PENDING_REVIEW = "PENDING_REVIEW" APPROVED = "APPROVED" + APPROVED_PENDING_DOCUMENTS = "APPROVED_PENDING_DOCUMENTS" REJECTED = "REJECTED" + REJECTED_DUPLICATE = "REJECTED_DUPLICATE" + REASSIGNED = "REASSIGNED" + REASSIGNED_PATIENT_UNKNOWN = "REASSIGNED_PATIENT_UNKNOWN" + NEVER_REVIEWED = "NEVER_REVIEWED" diff --git a/lambdas/enums/lambda_error.py b/lambdas/enums/lambda_error.py index 9f3a21b50..3322e279d 100644 --- a/lambdas/enums/lambda_error.py +++ b/lambdas/enums/lambda_error.py @@ -486,6 +486,33 @@ def create_error_body(self, params: Optional[dict] = None, **kwargs) -> str: "message": "User is unauthorised to view record", "fhir_coding": UKCoreSpineError.ACCESS_DENIED, } + """ + Errors for document review lambda + """ + DocumentReviewNotFound = { + "err_code": "DRV_4041", + "message": "Document review not found", + "fhir_coding": UKCoreSpineError.RESOURCE_NOT_FOUND, + } + DocumentReviewGeneralError = { + "err_code": "DRV_4002", + "message": "An error occurred while fetching the document review", + "fhir_coding": FhirIssueCoding.EXCEPTION, + } + UpdateDocStatusUnavailable = { + "err_code": "DRV_4003", + "message": "This Document is not available for review update", + "fhir_coding": FhirIssueCoding.FORBIDDEN, + } + DocumentReviewInvalidBody = { + "err_code": "DRV_4004", + "message": "Invalid request body", + } + DocumentReviewInvalidNhsNumber = { + "err_code": "DRV_4005", + "message": "The NHS number provided is invalid", + } + """ Errors for get ods report lambda """ diff --git a/lambdas/enums/logging_app_interaction.py b/lambdas/enums/logging_app_interaction.py index c1d7fc73f..744fc6580 100644 --- a/lambdas/enums/logging_app_interaction.py +++ b/lambdas/enums/logging_app_interaction.py @@ -20,3 +20,4 @@ class LoggingAppInteraction(Enum): UPLOAD_CONFIRMATION = "Upload confirmation" UPDATE_UPLOAD_STATE = "Update upload state" GET_REVIEW_DOCUMENTS = "Get review documents" + UPDATE_REVIEW = "Update review" diff --git a/lambdas/handlers/get_document_review_handler.py b/lambdas/handlers/get_document_review_handler.py index 13ed5621c..b90f174de 100644 --- a/lambdas/handlers/get_document_review_handler.py +++ b/lambdas/handlers/get_document_review_handler.py @@ -12,6 +12,7 @@ from utils.decorators.set_audit_arg import set_request_context_for_logging from utils.decorators.validate_patient_id import validate_patient_id from utils.lambda_exceptions import GetDocumentReviewException +from utils.lambda_handler_utils import validate_review_path_parameters from utils.lambda_response import ApiGatewayResponse from utils.request_context import request_context @@ -35,15 +36,10 @@ def lambda_handler(event, context): logger.info("Get Document Review handler has been triggered") feature_flag_service = FeatureFlagService() - upload_lambda_enabled_flag_object = feature_flag_service.get_feature_flags_by_flag( + feature_flag_service.validate_feature_flag( FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED ) - if not upload_lambda_enabled_flag_object[FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED]: - logger.info("Feature flag not enabled, event will not be processed") - raise GetDocumentReviewException(404, LambdaError.FeatureFlagDisabled) - - # Extract patient_id from query string parameters query_params = event.get("queryStringParameters", {}) patient_id = query_params.get("patientId", "") @@ -53,15 +49,7 @@ def lambda_handler(event, context): 400, LambdaError.DocumentReferenceMissingParameters ) - # Extract id from path parameters - path_params = event.get("pathParameters", {}) - document_id = path_params.get("id") - - if not document_id: - logger.error("Missing id in path parameters") - raise GetDocumentReviewException( - 400, LambdaError.DocumentReferenceMissingParameters - ) + document_id, document_version = validate_review_path_parameters(event) request_context.patient_nhs_no = patient_id @@ -69,10 +57,11 @@ def lambda_handler(event, context): f"Retrieving document review for patient_id: {patient_id}, document_id: {document_id}" ) - # Get document review service document_review_service = GetDocumentReviewService() document_review = document_review_service.get_document_review( - patient_id=patient_id, document_id=document_id + patient_id=patient_id, + document_id=document_id, + document_version=document_version, ) if document_review: diff --git a/lambdas/handlers/patch_document_review_handler.py b/lambdas/handlers/patch_document_review_handler.py new file mode 100644 index 000000000..2d0ca35bc --- /dev/null +++ b/lambdas/handlers/patch_document_review_handler.py @@ -0,0 +1,79 @@ +from enums.feature_flags import FeatureFlags +from enums.lambda_error import LambdaError +from enums.logging_app_interaction import LoggingAppInteraction +from models.document_review import PatchDocumentReviewRequest +from pydantic import ValidationError +from services.feature_flags_service import FeatureFlagService +from services.update_document_review_service import UpdateDocumentReviewService +from utils.audit_logging_setup import LoggingService +from utils.decorators.ensure_env_var import ensure_environment_variables +from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions +from utils.decorators.override_error_check import override_error_check +from utils.decorators.set_audit_arg import set_request_context_for_logging +from utils.decorators.validate_patient_id import validate_patient_id +from utils.lambda_exceptions import UpdateDocumentReviewException +from utils.lambda_handler_utils import validate_review_path_parameters +from utils.lambda_response import ApiGatewayResponse +from utils.request_context import request_context + +logger = LoggingService(__name__) + + +@set_request_context_for_logging +@validate_patient_id +@ensure_environment_variables( + names=[ + "DOCUMENT_REVIEW_DYNAMODB_NAME", + ] +) +@override_error_check +@handle_lambda_exceptions +def lambda_handler(event, context): + request_context.app_interaction = LoggingAppInteraction.UPDATE_REVIEW.value + + logger.info("Patch Document Review handler has been triggered") + feature_flag_service = FeatureFlagService() + feature_flag_service.validate_feature_flag( + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED + ) + + query_params = event.get("queryStringParameters", {}) + patient_id = query_params.get("patientId") + + if not patient_id: + logger.error("Missing patient_id in query string parameters") + raise UpdateDocumentReviewException(400, LambdaError.PatientIdNoKey) + + document_id, document_version = validate_review_path_parameters(event) + + reviewer_ods_code = request_context.authorization.get( + "selected_organisation", {} + ).get("org_ods_code") + + if not reviewer_ods_code: + logger.error("Missing ODS code in authorization token") + raise UpdateDocumentReviewException( + 401, LambdaError.DocumentReferenceUnauthorised + ) + + body = event.get("body") + try: + review_request = PatchDocumentReviewRequest.model_validate_json(body) + except ValidationError as e: + logger.error(f"Invalid request body: {str(e)}") + raise UpdateDocumentReviewException(400, LambdaError.DocumentReviewInvalidBody) + + document_review_service = UpdateDocumentReviewService() + document_review_service.update_document_review( + patient_id=patient_id, + document_id=document_id, + document_version=document_version, + update_data=review_request, + reviewer_ods_code=reviewer_ods_code, + ) + + logger.info( + "Document review updated successfully", + {"Result": "Successful document review update"}, + ) + return ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response() diff --git a/lambdas/models/document_review.py b/lambdas/models/document_review.py index 902a57900..749224b60 100644 --- a/lambdas/models/document_review.py +++ b/lambdas/models/document_review.py @@ -3,8 +3,10 @@ from enums.document_review_status import DocumentReviewStatus from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.snomed_codes import SnomedCodes -from pydantic import BaseModel, ConfigDict, Field -from pydantic.alias_generators import to_pascal, to_camel +from pydantic import BaseModel, ConfigDict, Field, model_validator +from pydantic.alias_generators import to_camel, to_pascal +from utils.exceptions import InvalidNhsNumberException +from utils.utilities import validate_nhs_number class DocumentReviewFileDetails(BaseModel): @@ -41,10 +43,8 @@ class DocumentUploadReviewReference(BaseModel): upload_date: int files: list[DocumentReviewFileDetails] = Field(min_length=1) nhs_number: str - ttl: int | None = Field( - alias=str(DocumentReferenceMetadataFields.TTL.value), default=None - ) - document_reference_id: str | None = Field(default=None) + version: int = Field(default=1) + document_reference_id: str = Field(default=None) document_snomed_code_type: str = Field(default=SnomedCodes.LLOYD_GEORGE.value.code) def model_dump_camel_case(self, *args, **kwargs): @@ -66,4 +66,63 @@ def camelize(self, model: dict) -> dict: value = result camel_case_dict[to_camel(key)] = value - return camel_case_dict \ No newline at end of file + return camel_case_dict + +class PatchDocumentReviewRequest(BaseModel): + model_config = ConfigDict( + validate_by_alias=True, + populate_by_name=True, + alias_generator=to_camel, + use_enum_values=True, + ) + + review_status: DocumentReviewStatus = Field(..., description="Review outcome") + document_reference_id: str | None = Field( + default=None, + description="Document reference ID (required when status is APPROVED)", + ) + nhs_number: str | None = Field( + default=None, + description="New NHS number (required when status is REASSIGNED)", + ) + + @model_validator(mode="after") + def validate_document_reference_id(self): + """Ensure document_reference_id is provided when review_status is APPROVED.""" + if ( + self.review_status == DocumentReviewStatus.APPROVED + and not self.document_reference_id + ): + raise ValueError( + "document_reference_id is required when review_status is APPROVED" + ) + elif ( + self.review_status != DocumentReviewStatus.APPROVED + and self.document_reference_id + ): + raise ValueError( + "document_reference_id is not required when review_status is not APPROVED" + ) + return self + + @model_validator(mode="after") + def validate_reassign_nhs_number(self): + """ + Validate the reassignment of the NHS number after the input data model has been populated. + + Checks whether the `reassigned_nhs_number` field has been provided and is valid when the + `review_status` reflects a reassigned state. Raises an error if validation fails. + """ + if ( + self.review_status == DocumentReviewStatus.REASSIGNED + and not self.nhs_number + ): + raise ValueError( + "reassigned_nhs_number is required when review_status is REASSIGNED or REASSIGNED_PATIENT_UNKNOWN" + ) + elif self.review_status == DocumentReviewStatus.REASSIGNED and self.nhs_number: + try: + validate_nhs_number(self.nhs_number) + except InvalidNhsNumberException: + raise ValueError("Invalid NHS number") + return self diff --git a/lambdas/services/authoriser_service.py b/lambdas/services/authoriser_service.py index 43345c836..2cfe7b3bb 100644 --- a/lambdas/services/authoriser_service.py +++ b/lambdas/services/authoriser_service.py @@ -97,7 +97,9 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None): case doc_ref if re.match(doc_ref_pattern, doc_ref): deny_resource = True - if (is_user_gp_admin or is_user_gp_clinical) and patient_access_is_allowed: + if ( + is_user_gp_admin or is_user_gp_clinical + ) and patient_access_is_allowed: deny_resource = False if patient_access_is_allowed and access_to_deceased_patient: deny_resource = True @@ -125,9 +127,7 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None): not patient_access_is_allowed or is_user_gp_clinical or is_user_pcse ) case path if path.startswith("/DocumentReview/"): - deny_resource = ( - not patient_access_is_allowed - ) + deny_resource = not patient_access_is_allowed case "/UploadState": deny_resource = ( diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 07dbe3ef8..5e10588f3 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -106,15 +106,17 @@ def fetch_documents_from_table( return documents def get_item( - self, - document_id: str, - table_name: str = None, - model_class: type[BaseModel] = None, + self, + document_id: str, + sort_key: dict = None, + table_name: str = None, + model_class: type[BaseModel] = None, ) -> Optional[BaseModel]: - """Fetch a single document by ID from specified or configured table. + """Fetch a single document by ID from a specified or configured table. Args: document_id: The document ID to retrieve. + sort_key: Optional sort key, defaults to None. table_name: Optional table name, defaults to self.table_name. model_class: Optional model class, defaults to self.model_class. @@ -123,10 +125,12 @@ def get_item( """ table_to_use = table_name or self.table_name model_to_use = model_class or self.model_class - + document_key = {"ID": document_id} + if sort_key: + document_key.update(sort_key) try: response = self.dynamo_service.get_item( - table_name=table_to_use, key={"ID": document_id} + table_name=table_to_use, key=document_key ) if "Item" not in response: @@ -137,7 +141,6 @@ def get_item( return document except ValidationError as e: - logger.error(f"Validation error on document: {response.get('Item')}") logger.error(f"{e}") return None @@ -146,6 +149,7 @@ def get_nhs_numbers_based_on_ods_code( ) -> list[str]: """Get unique NHS numbers for patients with given ODS code.""" table_name = table_name or self.table_name + documents = self.fetch_documents_from_table( index_name="OdsCodeIndex", search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, @@ -211,19 +215,32 @@ def update_document( update_key: dict[str, str] | None = None, document: BaseModel = None, update_fields_name: set[str] | None = None, + condition_expression: str | Attr | ConditionBase = None, + expression_attribute_values: dict = None, ): """Update document in specified or configured table.""" table_name = table_name or self.table_name - if not update_key: - update_key = {DocumentReferenceMetadataFields.ID.value: document.id} - self.dynamo_service.update_item( - table_name=table_name, - key_pair=update_key, - updated_fields=document.model_dump( + update_kwargs = { + "table_name": table_name, + "updated_fields": document.model_dump( exclude_none=True, by_alias=True, include=update_fields_name ), - ) + } + if update_key: + update_kwargs["key_pair"] = update_key + else: + update_kwargs["key_pair"] = { + DocumentReferenceMetadataFields.ID.value: document.id + } + + if condition_expression: + update_kwargs["condition_expression"] = condition_expression + + if expression_attribute_values: + update_kwargs["expression_attribute_values"] = expression_attribute_values + + return self.dynamo_service.update_item(**update_kwargs) def hard_delete_metadata_records( self, table_name: str, document_references: list[BaseModel] diff --git a/lambdas/services/document_upload_review_service.py b/lambdas/services/document_upload_review_service.py index 719adf9b0..25bd8e6ce 100644 --- a/lambdas/services/document_upload_review_service.py +++ b/lambdas/services/document_upload_review_service.py @@ -4,13 +4,15 @@ from botocore.exceptions import ClientError from enums.document_review_status import DocumentReviewStatus from enums.dynamo_filter import AttributeOperator -from enums.lambda_error import LambdaError +from enums.metadata_field_names import DocumentReferenceMetadataFields +from models.document_reference import S3_PREFIX from models.document_review import DocumentUploadReviewReference from pydantic import ValidationError from services.document_service import DocumentService from utils.audit_logging_setup import LoggingService from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder -from utils.lambda_exceptions import DocumentReviewException +from utils.dynamo_utils import build_transaction_item +from utils.exceptions import DocumentReviewException logger = LoggingService(__name__) @@ -70,7 +72,7 @@ def query_docs_pending_review_by_custodian_with_limit( except ClientError as e: logger.error(e) - raise DocumentReviewException(500, LambdaError.DocumentReviewDB) + raise DocumentReviewException("Failed to query document reviews") def _validate_review_references( self, items: list[dict] @@ -83,7 +85,9 @@ def _validate_review_references( return review_references except ValidationError as e: logger.error(e) - raise DocumentReviewException(500, LambdaError.DocumentReviewValidation) + raise DocumentReviewException( + "Failed to validate document review references" + ) def update_document_review_custodian( self, @@ -102,9 +106,159 @@ def update_document_review_custodian( self.update_document( document=review, + update_key={"ID": review.id, "Version": review.version}, update_fields_name=review_update_field, ) + def get_document_review_by_id(self, document_id: str, document_version: int): + return self.get_item(document_id, {"Version": document_version}) + + def update_pending_review_status( + self, review_update: DocumentUploadReviewReference, field_names: set[str] + ) -> None: + self.update_review_document_with_status_filter( + review_update, + field_names, + DocumentReviewStatus.PENDING_REVIEW, + ) + + def update_approved_pending_review_status( + self, review_update: DocumentUploadReviewReference, field_names: set[str] + ) -> None: + self.update_review_document_with_status_filter( + review_update, + field_names, + DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, + ) + + def update_review_document_with_status_filter( + self, + review_update: DocumentUploadReviewReference, + field_names: set[str], + status: DocumentReviewStatus, + ): + condition_expression = ( + Attr(DocumentReferenceMetadataFields.ID.value).exists() + & Attr("NhsNumber").eq(review_update.nhs_number) + & Attr("ReviewStatus").eq(status) + ) + self.update_document_review_for_patient( + review_update, field_names, condition_expression + ) + + def update_document_review_for_patient( + self, + review_update: DocumentUploadReviewReference, + field_names: set[str], + condition_expression, + ): + try: + return self.update_document( + document=review_update, + update_key={"ID": review_update.id, "Version": review_update.version}, + update_fields_name=field_names, + condition_expression=condition_expression, + ) + except ClientError as e: + error_code = e.response.get("Error", {}).get("Code", "") + + if error_code == "ConditionalCheckFailedException": + logger.error( + f"Condition check failed: Document ID {review_update.id}", + {"Result": "Failed to update document review"}, + ) + raise DocumentReviewException( + f"Document ID {review_update.id} does not meet the required conditions for update" + ) + + logger.error( + f"DynamoDB error updating document review: {str(e)}", + {"Result": "Failed to update document review"}, + ) + raise DocumentReviewException(f"Failed to update document review: {str(e)}") + + def update_document_review_with_transaction( + self, new_review_item, existing_review_item + ): + transact_items = [] + try: + new_doc_transaction = build_transaction_item( + table_name=self.table_name, + action="Update", + key={"ID": new_review_item.id, "Version": new_review_item.version}, + update_fields=new_review_item.model_dump( + exclude_none=True, by_alias=True, exclude={"version", "id"} + ), + conditions=[{"field": "ID", "operator": "attribute_not_exists"}], + ) + transact_items.append(new_doc_transaction) + + existing_update_fields = { + "review_status", + "review_date", + "reviewer", + } + existing_doc_transaction = build_transaction_item( + table_name=self.table_name, + action="Update", + key={ + "ID": existing_review_item.id, + "Version": existing_review_item.version, + }, + update_fields=existing_review_item.model_dump( + exclude_none=True, by_alias=True, include=existing_update_fields + ), + conditions=[ + { + "field": "ReviewStatus", + "operator": "=", + "value": DocumentReviewStatus.PENDING_REVIEW, + }, + { + "field": "NhsNumber", + "operator": "=", + "value": existing_review_item.nhs_number, + }, + { + "field": "Custodian", + "operator": "=", + "value": existing_review_item.custodian, + }, + ], + ) + except ValueError as e: + logger.error(f"Failed to build transaction item: {str(e)}") + raise DocumentReviewException(f"Failed to build transaction item: {str(e)}") + transact_items.append(existing_doc_transaction) + + try: + response = self.dynamo_service.transact_write_items(transact_items) + logger.info("Transaction completed successfully") + except ClientError as e: + error_code = e.response.get("Error", {}).get("Code", "") + if error_code == "TransactionCanceledException": + logger.error( + f"Condition check failed: Document ID {existing_review_item.id} ", + {"Result": "Failed to update document review"}, + ) + raise DocumentReviewException(f"Failed to update document review: {str(e)}") + return response + + def delete_document_review_files( + self, document_review: DocumentUploadReviewReference + ): + for file in document_review.files: + location_without_prefix = file.file_location.replace(S3_PREFIX, "") + bucket, file_key = location_without_prefix.split("/", 1) + try: + self.s3_service.delete_object(bucket, file_key) + except ClientError as e: + logger.warning( + f"Unable to delete file {file.file_name} from S3 due to error: {e}" + ) + logger.warning(f"Skipping file deletion for {file.file_name}") + continue + def build_review_query_filter( self, nhs_number: str | None = None, uploader: str | None = None ) -> Attr | ConditionBase: diff --git a/lambdas/services/get_document_review_service.py b/lambdas/services/get_document_review_service.py index 8f38df9b0..204b6765d 100644 --- a/lambdas/services/get_document_review_service.py +++ b/lambdas/services/get_document_review_service.py @@ -1,7 +1,6 @@ import os import uuid from datetime import datetime, timezone -from typing import Optional from enums.lambda_error import LambdaError from services.base.s3_service import S3Service @@ -26,12 +25,15 @@ def __init__(self): self.cloudfront_table_name = os.environ.get("EDGE_REFERENCE_TABLE") self.cloudfront_url = os.environ.get("CLOUDFRONT_URL") - def get_document_review(self, patient_id: str, document_id: str) -> Optional[dict]: + def get_document_review( + self, patient_id: str, document_id: str, document_version: int + ) -> dict | None: """Retrieve a document review for a given patient and document. Args: patient_id: The patient ID (NHS number). document_id: The document ID to retrieve. + document_version: The document version to retrieve. Returns: Dictionary containing the document review details, or None if not found. @@ -41,7 +43,11 @@ def get_document_review(self, patient_id: str, document_id: str) -> Optional[dic f"Fetching document review for patient_id: {patient_id}, document_id: {document_id}" ) - document_review_item = self.document_review_service.get_item(document_id) + document_review_item = ( + self.document_review_service.get_document_review_by_id( + document_id=document_id, document_version=document_version + ) + ) if not document_review_item: logger.info(f"No document review found for document_id: {document_id}") @@ -63,6 +69,7 @@ def get_document_review(self, patient_id: str, document_id: str) -> Optional[dic document_review = document_review_item.model_dump_camel_case( include={ "id": True, + "version": True, "upload_date": True, "files": {"__all__": {"file_name": True, "presigned_url": True}}, "document_snomed_code_type": True, diff --git a/lambdas/services/search_document_review_service.py b/lambdas/services/search_document_review_service.py index 9d4251de2..860faa551 100644 --- a/lambdas/services/search_document_review_service.py +++ b/lambdas/services/search_document_review_service.py @@ -38,6 +38,7 @@ def process_request( exclude_none=True, include={ "id", + "version", "nhs_number", "review_reason", "document_snomed_code_type", diff --git a/lambdas/services/update_document_review_service.py b/lambdas/services/update_document_review_service.py new file mode 100644 index 000000000..a4a7f09c9 --- /dev/null +++ b/lambdas/services/update_document_review_service.py @@ -0,0 +1,254 @@ +from datetime import datetime, timezone + +from enums.document_review_status import DocumentReviewStatus +from enums.lambda_error import LambdaError +from models.document_review import ( + DocumentUploadReviewReference, + PatchDocumentReviewRequest, +) +from services.base.s3_service import S3Service +from services.document_upload_review_service import DocumentUploadReviewService +from utils.audit_logging_setup import LoggingService +from utils.exceptions import PdsErrorException, DocumentReviewException, InvalidResourceIdException, \ + PatientNotFoundException +from utils.lambda_exceptions import UpdateDocumentReviewException +from utils.ods_utils import PCSE_ODS_CODE +from utils.utilities import get_pds_service + +logger = LoggingService(__name__) + + +class UpdateDocumentReviewService: + """Service for updating document review status in DynamoDB.""" + + REJECTED_REVIEW_STATUSES = [ + DocumentReviewStatus.REJECTED, + DocumentReviewStatus.REJECTED_DUPLICATE, + ] + + REASSIGNMENT_STATUSES = [ + DocumentReviewStatus.REASSIGNED, + DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + ] + + APPROVED_REVIEW_STATUSES = [ + DocumentReviewStatus.APPROVED, + ] + + APPROVED_PENDING_REVIEW_STATUSES = [DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS] + + UNKNOWN_NHS_NUMBER = "0000000000" + + FAILED_LOG_MESSAGE = "Failed to update document review" + + def __init__(self): + self.document_review_service = DocumentUploadReviewService() + self.s3_service = S3Service() + + def update_document_review( + self, + patient_id: str, + document_id: str, + document_version: int, + update_data: PatchDocumentReviewRequest, + reviewer_ods_code: str, + ): + logger.info( + f"Updating document review for patient_id: {patient_id}, document_id: {document_id}" + ) + try: + review_document = self._fetch_document(document_id, document_version) + self._validate_document_for_update( + review_document, patient_id, reviewer_ods_code + ) + self._process_review_status_update( + review_document, update_data, document_id, reviewer_ods_code + ) + + logger.info( + f"Successfully updated document review for document_id: {document_id}" + ) + except DocumentReviewException: + raise UpdateDocumentReviewException(400, LambdaError.DocumentReviewGeneralError) + + def _fetch_document(self, document_id: str, document_version: int): + review_document = self.document_review_service.get_document_review_by_id( + document_id=document_id, document_version=document_version + ) + + if not review_document: + logger.error( + f"Document review not found for document_id: {document_id}", + {"Result": self.FAILED_LOG_MESSAGE}, + ) + raise UpdateDocumentReviewException( + 404, LambdaError.DocumentReviewNotFound + ) + + return review_document + + def _validate_document_for_update( + self, + document, + patient_id: str, + reviewer_ods_code: str, + ): + logger.info(f"Validating document for update: {document.id}") + self._validate_patient_id_match(document, patient_id) + self._validate_review_status(document) + self._validate_user_match_custodian(document, reviewer_ods_code) + + def _validate_patient_id_match(self, document, patient_id: str): + if document.nhs_number != patient_id: + logger.error( + f"NHS number mismatch for document_id: {document.id}. " + f"Expected: {patient_id}, Got: {document.nhs_number}", + {"Result": self.FAILED_LOG_MESSAGE}, + ) + raise UpdateDocumentReviewException( + 400, LambdaError.UpdateDocNHSNumberMismatch + ) + + def _validate_review_status(self, document): + if document.review_status not in [ + DocumentReviewStatus.PENDING_REVIEW, + DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, + ]: + logger.error( + f"Invalid review status for document_id: {document.id}. " + f"Expected: PENDING_REVIEW, Got: {document.review_status}", + {"Result": self.FAILED_LOG_MESSAGE}, + ) + raise UpdateDocumentReviewException( + 400, LambdaError.UpdateDocStatusUnavailable + ) + + def _validate_user_match_custodian(self, document, reviewer_ods_code: str): + if document.custodian != reviewer_ods_code: + logger.error( + f"Reviewer ODS code mismatch for document_id: {document.id}", + {"Result": self.FAILED_LOG_MESSAGE}, + ) + raise UpdateDocumentReviewException( + 403, LambdaError.DocumentReferenceUnauthorised + ) + + def _ensure_approved_status_follows_pending_review(self, document, update_data): + is_approved_pending_review = document.review_status == DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + is_update_status_approved = update_data.review_status == DocumentReviewStatus.APPROVED + + is_invalid_approved_pending_transition = is_approved_pending_review and not is_update_status_approved + is_approved_status_not_follows_pending_review = is_update_status_approved and not is_approved_pending_review + if is_approved_status_not_follows_pending_review or is_invalid_approved_pending_transition: + raise UpdateDocumentReviewException( + 400, LambdaError.UpdateDocStatusUnavailable + ) + + def _process_review_status_update( + self, + document, + update_data: PatchDocumentReviewRequest, + document_id: str, + reviewer_ods_code: str, + ): + self._ensure_approved_status_follows_pending_review(document, update_data) + self._set_review_metadata(document, update_data, reviewer_ods_code) + self._execute_status_action(document, update_data, document_id) + + def _set_review_metadata(self, document, update_data, reviewer_ods_code): + review_date = int(datetime.now(timezone.utc).timestamp()) + document.review_status = update_data.review_status + document.review_date = review_date + document.reviewer = reviewer_ods_code + + def _execute_status_action(self, document, update_data, document_id): + update_fields = {"review_status", "review_date", "reviewer"} + + if document.review_status in self.REASSIGNMENT_STATUSES: + self._handle_reassignment_status(document, update_data, document_id) + elif document.review_status in self.APPROVED_REVIEW_STATUSES: + document.document_reference_id = update_data.document_reference_id + update_fields.add("document_reference_id") + self._handle_approval(document, update_fields) + elif document.review_status in self.REJECTED_REVIEW_STATUSES: + self._handle_rejection(document, update_fields) + elif document.review_status in self.APPROVED_PENDING_REVIEW_STATUSES: + self._handle_approved_pending(document, update_fields) + else: + logger.error( + f"Invalid status update attempted: {document.review_status}", + {"Result": self.FAILED_LOG_MESSAGE}, + ) + raise UpdateDocumentReviewException( + 400, LambdaError.DocumentReviewGeneralError + ) + + def _handle_rejection(self, document, update_fields): + self.document_review_service.update_pending_review_status( + review_update=document, field_names=update_fields + ) + self._handle_soft_delete(document) + + def _handle_approval(self, document, update_fields): + self.document_review_service.update_approved_pending_review_status( + review_update=document, field_names=update_fields + ) + self._handle_soft_delete(document) + + def _handle_approved_pending(self, document, update_fields): + self.document_review_service.update_pending_review_status( + review_update=document, field_names=update_fields + ) + + def _handle_soft_delete(self, review_document: DocumentUploadReviewReference): + logger.info( + f"Deleting document review files for document_id: {review_document.id}" + ) + self.document_review_service.delete_document_review_files(review_document) + + def _handle_reassignment_status( + self, document, update_data: PatchDocumentReviewRequest, document_id: str + ): + new_document_review = self._create_reassigned_document(document, update_data) + + self.document_review_service.update_document_review_with_transaction( + new_review_item=new_document_review, existing_review_item=document + ) + + logger.info( + f"Document {document_id} reassigned to patient {update_data.nhs_number}" + ) + + def _create_reassigned_document( + self, document, update_data: PatchDocumentReviewRequest + ) -> DocumentUploadReviewReference: + new_document_review = document.model_copy(deep=True) + new_document_review.review_date = None + new_document_review.reviewer = None + if update_data.review_status == DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN: + new_document_review.nhs_number = self.UNKNOWN_NHS_NUMBER + new_document_review.custodian = PCSE_ODS_CODE + + else: + new_document_review.nhs_number = update_data.nhs_number + new_document_review.custodian = self._get_patient_custodian( + update_data.nhs_number + ) + + new_document_review.review_status = DocumentReviewStatus.PENDING_REVIEW + new_document_review.version = new_document_review.version + 1 + + return new_document_review + + def _get_patient_custodian(self, patient_nhs_number: str) -> str: + try: + pds_service = get_pds_service() + patient_details = pds_service.fetch_patient_details(patient_nhs_number) + return patient_details.general_practice_ods + except (PdsErrorException, PatientNotFoundException, InvalidResourceIdException): + logger.error( + f"Failed to fetch patient details for NHS number: {patient_nhs_number}" + ) + raise UpdateDocumentReviewException( + 400, LambdaError.DocumentReviewInvalidNhsNumber + ) diff --git a/lambdas/tests/unit/handlers/conftest.py b/lambdas/tests/unit/handlers/conftest.py index 32f659f10..c8c8e1f45 100755 --- a/lambdas/tests/unit/handlers/conftest.py +++ b/lambdas/tests/unit/handlers/conftest.py @@ -1,4 +1,5 @@ import pytest + from enums.feature_flags import FeatureFlags from services.feature_flags_service import FeatureFlagService @@ -166,4 +167,3 @@ def mock_upload_document_iteration_3_enabled(mocker): FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED: True } yield mock_feature_flag - diff --git a/lambdas/tests/unit/handlers/test_get_document_review_handler.py b/lambdas/tests/unit/handlers/test_get_document_review_handler.py index 145568817..5516bcea3 100644 --- a/lambdas/tests/unit/handlers/test_get_document_review_handler.py +++ b/lambdas/tests/unit/handlers/test_get_document_review_handler.py @@ -1,7 +1,6 @@ import json import pytest -from enums.feature_flags import FeatureFlags from enums.lambda_error import LambdaError from handlers.get_document_review_handler import lambda_handler from utils.lambda_exceptions import GetDocumentReviewException @@ -29,7 +28,7 @@ def valid_get_document_review_event(): api_gateway_proxy_event = { "httpMethod": "GET", "queryStringParameters": {"patientId": "9000000009"}, - "pathParameters": {"id": "test-document-id"}, + "pathParameters": {"id": "test-document-id", "version": "1"}, } return api_gateway_proxy_event @@ -49,7 +48,7 @@ def missing_document_id_event(): api_gateway_proxy_event = { "httpMethod": "GET", "queryStringParameters": {"patientId": "9000000009"}, - "pathParameters": {}, + "pathParameters": {"version": "1"}, } return api_gateway_proxy_event @@ -59,11 +58,20 @@ def invalid_patient_id_event(): api_gateway_proxy_event = { "httpMethod": "GET", "queryStringParameters": {"patientId": "900000000900"}, - "pathParameters": {"id": "test-document-id"}, + "pathParameters": {"id": "test-document-id", "version": "1"}, } return api_gateway_proxy_event +@pytest.fixture +def missing_document_version_event(): + api_gateway_proxy_event = { + "httpMethod": "GET", + "queryStringParameters": {"patientId": "9000000009"}, + "pathParameters": {"id": "test-document-id"}, + } + return api_gateway_proxy_event + @pytest.fixture def mocked_service(set_env, mocker): @@ -75,7 +83,11 @@ def mocked_service(set_env, mocker): def test_lambda_handler_returns_200_with_document_review( - mocked_service, valid_get_document_review_event, context, set_env, mock_upload_document_iteration_3_enabled + mocked_service, + valid_get_document_review_event, + context, + set_env, + mock_upload_document_iteration_3_enabled, ): mocked_service.get_document_review.return_value = MOCK_DOCUMENT_REVIEW_RESPONSE @@ -87,12 +99,15 @@ def test_lambda_handler_returns_200_with_document_review( assert expected == actual mocked_service.get_document_review.assert_called_once_with( - patient_id="9000000009", document_id="test-document-id" + patient_id="9000000009", document_id="test-document-id", document_version=1 ) def test_lambda_handler_returns_404_when_document_review_not_found( - mocked_service, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled + mocked_service, + valid_get_document_review_event, + context, + mock_upload_document_iteration_3_enabled, ): mocked_service.get_document_review.return_value = None @@ -113,7 +128,10 @@ def test_lambda_handler_returns_404_when_document_review_not_found( def test_lambda_handler_raises_exception_returns_500( - mocked_service, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled + mocked_service, + valid_get_document_review_event, + context, + mock_upload_document_iteration_3_enabled, ): mocked_service.get_document_review.side_effect = GetDocumentReviewException( 500, LambdaError.MockError @@ -146,7 +164,10 @@ def test_lambda_handler_when_patient_id_not_valid_returns_400( def test_lambda_handler_when_document_id_not_supplied_returns_400( - set_env, missing_document_id_event, context, mock_upload_document_iteration_3_enabled + set_env, + missing_document_id_event, + context, + mock_upload_document_iteration_3_enabled, ): actual = lambda_handler(missing_document_id_event, context) @@ -156,8 +177,26 @@ def test_lambda_handler_when_document_id_not_supplied_returns_400( assert expected == actual +def test_lambda_handler_when_document_version_not_supplied_returns_400( + set_env, + missing_document_version_event, + context, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(missing_document_version_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "GET" + ).create_api_gateway_response() + assert expected == actual + + def test_lambda_handler_missing_environment_variables_returns_500( - set_env, monkeypatch, valid_get_document_review_event, context, mock_upload_document_iteration_3_enabled + set_env, + monkeypatch, + valid_get_document_review_event, + context, + mock_upload_document_iteration_3_enabled, ): monkeypatch.delenv("DOCUMENT_REVIEW_DYNAMODB_NAME") diff --git a/lambdas/tests/unit/handlers/test_patch_document_review_handler.py b/lambdas/tests/unit/handlers/test_patch_document_review_handler.py new file mode 100644 index 000000000..721b65088 --- /dev/null +++ b/lambdas/tests/unit/handlers/test_patch_document_review_handler.py @@ -0,0 +1,373 @@ +import json + +import pytest +from enums.document_review_status import DocumentReviewStatus +from enums.lambda_error import LambdaError +from handlers.patch_document_review_handler import lambda_handler +from utils.lambda_exceptions import UpdateDocumentReviewException +from utils.lambda_response import ApiGatewayResponse + +TEST_PATIENT_ID = "9000000009" +TEST_DOCUMENT_ID = "test-document-id-123" +TEST_VERSION = 1 +TEST_DOCUMENT_REFERENCE_ID = "doc-ref-12345" +TEST_REVIEWER_ODS_CODE = "Y12345" + + +@pytest.fixture +def valid_put_document_review_event_approved(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": str(TEST_VERSION)}, + "body": json.dumps( + { + "reviewStatus": "APPROVED", + "documentReferenceId": TEST_DOCUMENT_REFERENCE_ID, + } + ), + } + + +@pytest.fixture +def valid_put_document_review_event_rejected(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": str(TEST_VERSION)}, + "body": json.dumps({"reviewStatus": "REJECTED"}), + } + + +@pytest.fixture +def missing_patient_id_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": str(TEST_VERSION)}, + "body": json.dumps( + { + "reviewStatus": "APPROVED", + "documentReferenceId": TEST_DOCUMENT_REFERENCE_ID, + } + ), + } + + +@pytest.fixture +def missing_document_id_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {}, + "body": json.dumps( + { + "reviewStatus": "APPROVED", + "documentReferenceId": TEST_DOCUMENT_REFERENCE_ID, + } + ), + } + + +@pytest.fixture +def invalid_body_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": str(TEST_VERSION)}, + "body": "invalid-json", + } + + +@pytest.fixture +def approved_without_document_reference_id_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": str(TEST_VERSION)}, + "body": json.dumps({"reviewStatus": "APPROVED"}), + } + + +@pytest.fixture +def invalid_version_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID, "version": "not-a-number"}, + "body": json.dumps( + { + "reviewStatus": "APPROVED", + "documentReferenceId": TEST_DOCUMENT_REFERENCE_ID, + } + ), + } + + +@pytest.fixture +def missing_version_event(): + return { + "httpMethod": "PATCH", + "queryStringParameters": {"patientId": TEST_PATIENT_ID}, + "pathParameters": {"id": TEST_DOCUMENT_ID}, + "body": json.dumps( + { + "reviewStatus": "APPROVED", + "documentReferenceId": TEST_DOCUMENT_REFERENCE_ID, + } + ), + } + + +@pytest.fixture +def mocked_service(set_env, mocker): + mocked_class = mocker.patch( + "handlers.patch_document_review_handler.UpdateDocumentReviewService" + ) + mocked_service = mocked_class.return_value + yield mocked_service + + +@pytest.fixture +def mock_authorization(mocker): + mocked_context = mocker.MagicMock() + mocked_context.authorization = { + "selected_organisation": {"org_ods_code": TEST_REVIEWER_ODS_CODE}, + } + yield mocker.patch( + "handlers.patch_document_review_handler.request_context", mocked_context + ) + + +@pytest.fixture +def mock_missing_authorization(mocker): + mocked_context = mocker.MagicMock() + mocked_context.authorization = { + "selected_organisation": {"org_ods_code": None}, + } + yield mocker.patch( + "handlers.patch_document_review_handler.request_context", mocked_context + ) + + +def test_lambda_handler_returns_200_when_document_review_approved( + mocked_service, + valid_put_document_review_event_approved, + context, + set_env, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + mocked_service.update_document_review.return_value = None + + expected = ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response() + + actual = lambda_handler(valid_put_document_review_event_approved, context) + + assert expected == actual + mocked_service.update_document_review.assert_called_once() + call_args = mocked_service.update_document_review.call_args + assert call_args[1]["patient_id"] == TEST_PATIENT_ID + assert call_args[1]["document_id"] == TEST_DOCUMENT_ID + assert call_args[1]["document_version"] == TEST_VERSION + assert call_args[1]["reviewer_ods_code"] == TEST_REVIEWER_ODS_CODE + assert call_args[1]["update_data"].review_status == DocumentReviewStatus.APPROVED + assert ( + call_args[1]["update_data"].document_reference_id == TEST_DOCUMENT_REFERENCE_ID + ) + + +def test_lambda_handler_returns_200_when_document_review_rejected( + mocked_service, + valid_put_document_review_event_rejected, + context, + set_env, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + mocked_service.update_document_review.return_value = None + + expected = ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response() + + actual = lambda_handler(valid_put_document_review_event_rejected, context) + + assert expected == actual + mocked_service.update_document_review.assert_called_once() + call_args = mocked_service.update_document_review.call_args + assert call_args[1]["patient_id"] == TEST_PATIENT_ID + assert call_args[1]["document_id"] == TEST_DOCUMENT_ID + assert call_args[1]["document_version"] == TEST_VERSION + assert call_args[1]["reviewer_ods_code"] == TEST_REVIEWER_ODS_CODE + assert call_args[1]["update_data"].review_status == DocumentReviewStatus.REJECTED + assert call_args[1]["update_data"].document_reference_id is None + + +def test_lambda_handler_returns_400_when_patient_id_missing( + set_env, + missing_patient_id_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(missing_patient_id_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.PatientIdNoKey.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_400_when_document_id_missing( + set_env, + missing_document_id_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(missing_document_id_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_400_when_body_invalid_json( + set_env, + invalid_body_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(invalid_body_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReviewInvalidBody.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + assert actual["statusCode"] == 400 + + +def test_lambda_handler_returns_400_when_approved_without_document_reference_id( + set_env, + approved_without_document_reference_id_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(approved_without_document_reference_id_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReviewInvalidBody.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert actual["statusCode"] == 400 + assert expected == actual + + +def test_lambda_handler_returns_401_when_ods_code_missing( + set_env, + valid_put_document_review_event_approved, + context, + mock_missing_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(valid_put_document_review_event_approved, context) + + expected = ApiGatewayResponse( + 401, LambdaError.DocumentReferenceUnauthorised.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_500_when_service_raises_exception( + mocked_service, + valid_put_document_review_event_approved, + context, + set_env, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + mocked_service.update_document_review.side_effect = UpdateDocumentReviewException( + 500, LambdaError.DocumentReferenceGeneralError + ) + + actual = lambda_handler(valid_put_document_review_event_approved, context) + + expected = ApiGatewayResponse( + 500, LambdaError.DocumentReferenceGeneralError.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_404_when_document_not_found( + mocked_service, + valid_put_document_review_event_approved, + context, + set_env, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + mocked_service.update_document_review.side_effect = UpdateDocumentReviewException( + 404, LambdaError.DocumentReferenceMissingParameters + ) + + actual = lambda_handler(valid_put_document_review_event_approved, context) + + expected = ApiGatewayResponse( + 404, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_500_when_environment_variable_missing( + set_env, + monkeypatch, + valid_put_document_review_event_approved, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + monkeypatch.delenv("DOCUMENT_REVIEW_DYNAMODB_NAME") + + expected_body = json.dumps( + { + "message": "An error occurred due to missing environment variable: 'DOCUMENT_REVIEW_DYNAMODB_NAME'", + "err_code": "ENV_5001", + "interaction_id": "88888888-4444-4444-4444-121212121212", + } + ) + expected = ApiGatewayResponse( + 500, expected_body, "PATCH" + ).create_api_gateway_response() + actual = lambda_handler(valid_put_document_review_event_approved, context) + assert expected == actual + + +def test_lambda_handler_returns_400_when_version_invalid( + set_env, + invalid_version_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(invalid_version_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual + + +def test_lambda_handler_returns_400_when_version_missing( + set_env, + missing_version_event, + context, + mock_authorization, + mock_upload_document_iteration_3_enabled, +): + actual = lambda_handler(missing_version_event, context) + + expected = ApiGatewayResponse( + 400, LambdaError.DocumentReferenceMissingParameters.create_error_body(), "PATCH" + ).create_api_gateway_response() + assert expected == actual diff --git a/lambdas/tests/unit/helpers/data/search_document_review/dynamo_response.py b/lambdas/tests/unit/helpers/data/search_document_review/dynamo_response.py index 43080e823..6e1df3c8d 100644 --- a/lambdas/tests/unit/helpers/data/search_document_review/dynamo_response.py +++ b/lambdas/tests/unit/helpers/data/search_document_review/dynamo_response.py @@ -13,6 +13,7 @@ "Items": [ { "ID": "3d8683b9-1665-40d2-8499-6e8302d507ff", + "Version": 1, "Files": [ { "FileLocation": f"s3://{MOCK_STAGING_STORE_BUCKET}/{TEST_NHS_NUMBER}/test-key-123", @@ -34,6 +35,7 @@ }, { "ID": "4d8683b9-1665-40d2-8499-6e8302d507ff", + "Version": 2, "Files": [ { "FileLocation": f"s3://{MOCK_STAGING_STORE_BUCKET}/{TEST_NHS_NUMBER}/test-key-123", @@ -55,6 +57,7 @@ }, { "ID": "5d8683b9-1665-40d2-8499-6e8302d507ff", + "Version": 3, "Files": [ { "FileLocation": f"s3://{MOCK_STAGING_STORE_BUCKET}/{TEST_NHS_NUMBER}/test-key-123", diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index a68e35d7f..4bb36ea95 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -2,6 +2,7 @@ from unittest.mock import call import pytest +from boto3.dynamodb.conditions import Attr from botocore.exceptions import ClientError from enums.document_retention import DocumentRetentionDays from enums.dynamo_filter import AttributeOperator @@ -246,6 +247,51 @@ def test_update_document(mock_service, mock_dynamo_service): mock_dynamo_service.update_item.assert_has_calls([update_item_call]) +def test_update_document_with_custom_key_pair(mock_service, mock_dynamo_service): + test_doc_ref = DocumentReference.model_validate(MOCK_DOCUMENT) + + test_update_fields = {"doc_status"} + custom_key_pair = {"ID": test_doc_ref.id, "NhsNumber": test_doc_ref.nhs_number} + + update_item_call = call( + table_name=MOCK_TABLE_NAME, + key_pair=custom_key_pair, + updated_fields={"DocStatus": "final"}, + ) + + mock_service.update_document( + MOCK_TABLE_NAME, custom_key_pair, test_doc_ref, test_update_fields, + ) + + mock_dynamo_service.update_item.assert_has_calls([update_item_call]) + + +def test_update_document_with_condition_expression(mock_service, mock_dynamo_service): + test_doc_ref = DocumentReference.model_validate(MOCK_DOCUMENT) + + test_update_fields = {"doc_status"} + condition_expr = Attr("Uploaded").eq(False) + expression_attr_values = {":uploaded": False} + + update_item_call = call( + table_name=MOCK_TABLE_NAME, + key_pair={"ID": test_doc_ref.id}, + updated_fields={"DocStatus": "final"}, + condition_expression=condition_expr, + expression_attribute_values=expression_attr_values, + ) + + mock_service.update_document( + table_name=MOCK_TABLE_NAME, + document=test_doc_ref, + update_fields_name=test_update_fields, + condition_expression=condition_expr, + expression_attribute_values=expression_attr_values, + ) + + mock_dynamo_service.update_item.assert_has_calls([update_item_call]) + + def test_hard_delete_metadata_records(mock_service, mock_dynamo_service): test_doc_refs = [ DocumentReference.model_validate(mock_document) @@ -430,7 +476,7 @@ def test_get_batch_document_references_by_id_client_error( "document_id,table_name,file_name,expected_table", [ ("test-doc-id-123", None, "test-file.pdf", MOCK_LG_TABLE_NAME), - ("test-doc-id-456", MOCK_ARF_TABLE_NAME, "arf-file.pdf", MOCK_ARF_TABLE_NAME), + ("test-doc-id-456", MOCK_TABLE_NAME, "lg-file.pdf", MOCK_TABLE_NAME), ], ) def test_get_item_success( @@ -470,18 +516,17 @@ def test_get_item_success( @pytest.mark.parametrize( - "document_id,mock_response,expected_log", + "document_id,mock_response", [ - ("non-existent-doc", {}, "No document found for document_id: non-existent-doc"), + ("non-existent-doc", {}, ), ( "invalid-doc", {"Item": {"ID": "invalid-doc", "InvalidField": "invalid-value"}}, - "Validation error on document:", ), ], ) def test_get_item_returns_none( - mock_service, mock_dynamo_service, caplog, document_id, mock_response, expected_log + mock_service, mock_dynamo_service, document_id, mock_response, ): """Test get_item returns None for not found or invalid documents.""" mock_dynamo_service.get_item.return_value = mock_response @@ -492,7 +537,6 @@ def test_get_item_returns_none( table_name=MOCK_LG_TABLE_NAME, key={"ID": document_id} ) assert result is None - assert expected_log in caplog.text @pytest.mark.parametrize( @@ -515,11 +559,11 @@ def test_get_item_with_custom_model_class( file_name, expected_table, ): - """Test retrieval using a custom model class with default or custom table.""" mock_dynamo_response = { "Item": { "ID": document_id, + "Author": "Y12345", "Custodian": "Y12345", "ReviewStatus": "PENDING_REVIEW", @@ -556,3 +600,14 @@ def test_get_item_with_custom_model_class( assert isinstance(result, DocumentUploadReviewReference) assert result.id == document_id assert result.nhs_number == TEST_NHS_NUMBER + + +def test_get_item_document_id_with_sort_key(mock_service, mock_dynamo_service): + document_id = "test-doc-id-123" + sort_key = {"sort-key-name": "test-sort-value"} + + mock_service.get_item(document_id, sort_key) + + mock_dynamo_service.get_item.assert_called_once_with( + table_name=MOCK_LG_TABLE_NAME, key={"ID": document_id, **sort_key} + ) \ No newline at end of file diff --git a/lambdas/tests/unit/services/test_document_upload_review_service.py b/lambdas/tests/unit/services/test_document_upload_review_service.py index 96b1cc1f2..9f82d8c9b 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -2,6 +2,9 @@ import pytest from boto3.dynamodb.conditions import Attr +from botocore.exceptions import ClientError +from enums.document_review_status import DocumentReviewStatus +from enums.metadata_field_names import DocumentReferenceMetadataFields from models.document_review import DocumentUploadReviewReference from services.document_upload_review_service import DocumentUploadReviewService from tests.unit.conftest import ( @@ -9,6 +12,7 @@ MOCK_DOCUMENT_REVIEW_TABLE, TEST_NHS_NUMBER, ) +from utils.exceptions import DocumentReviewException TEST_ODS_CODE = "Y12345" NEW_ODS_CODE = "Z98765" @@ -18,14 +22,9 @@ @pytest.fixture def mock_service(set_env, mocker): """Fixture to create a DocumentUploadReviewService with mocked dependencies.""" - - mocker.patch( - "services.document_upload_review_service.DocumentService.__init__", - return_value=None, - ) + mocker.patch("services.document_service.S3Service") + mocker.patch("services.document_service.DynamoDBService") service = DocumentUploadReviewService() - service.s3_service = MagicMock() - service.dynamo_service = MagicMock() yield service @@ -36,12 +35,25 @@ def mock_document_review_references(): for i in range(3): review = MagicMock(spec=DocumentUploadReviewReference) review.id = f"review-id-{i}" + review.version = i review.nhs_number = TEST_NHS_NUMBER review.custodian = TEST_ODS_CODE reviews.append(review) return reviews +@pytest.fixture +def mock_review_update(): + """Fixture to create a mock review update object.""" + review_update = MagicMock(spec=DocumentUploadReviewReference) + review_update.id = "test-review-id" + review_update.version = 1 + review_update.nhs_number = TEST_NHS_NUMBER + review_update.review_status = DocumentReviewStatus.APPROVED + review_update.document_reference_id = "test-doc-ref-id" + return review_update + + def test_table_name(mock_service): """Test that table_name property returns correct environment variable.""" @@ -79,7 +91,9 @@ def test_update_document_review_custodian_updates_all_documents( # Verify update_document was called with the correct parameters for review in mock_document_review_references: mock_update_document.assert_any_call( - document=review, update_fields_name={"custodian"} + document=review, + update_fields_name={"custodian"}, + update_key={"ID": review.id, "Version": review.version}, ) @@ -125,10 +139,8 @@ def test_update_document_review_custodian_mixed_custodians( mock_document_review_references, NEW_ODS_CODE ) - # Verify that update_document was only called twice (for the 2 documents that needed updating) assert mock_update_document.call_count == 2 - # Verify all documents now have the new custodian for review in mock_document_review_references: assert review.custodian == NEW_ODS_CODE @@ -144,25 +156,273 @@ def test_update_document_review_custodian_logging( mock_document_review_references, NEW_ODS_CODE ) - # Verify logging was called for each document assert mock_logger.info.call_count == 3 mock_logger.info.assert_any_call("Updating document review custodian...") def test_update_document_review_custodian_single_document(mock_service, mocker): - """Test update_document_review_custodian with a single document.""" mock_update_document = mocker.patch.object(mock_service, "update_document") single_review = MagicMock(spec=DocumentUploadReviewReference) single_review.id = "single-review-id" + single_review.version = 1 single_review.custodian = TEST_ODS_CODE mock_service.update_document_review_custodian([single_review], NEW_ODS_CODE) assert single_review.custodian == NEW_ODS_CODE mock_update_document.assert_called_once_with( - document=single_review, update_fields_name={"custodian"} + document=single_review, + update_fields_name={"custodian"}, + update_key={"ID": single_review.id, "Version": single_review.version}, + ) + + +def test_get_document_review_by_id( + mock_service, mock_document_review_references, mocker +): + mock_get_item = mocker.patch.object(mock_service, "get_item") + mock_get_item.side_effect = mock_document_review_references + + mock_review = mock_service.get_document_review_by_id("review-id-0", 34) + + assert mock_review == mock_document_review_references[0] + mock_get_item.assert_called_once_with("review-id-0", {"Version": 34}) + + +def test_update_document_review_for_patient_success( + mock_service, mock_review_update, mocker +): + mock_update_document = mocker.patch.object(mock_service, "update_document") + mock_update_document.return_value = {"Attributes": {"ID": "test-review-id"}} + + field_names = {"review_status", "document_reference_id"} + + result = mock_service.update_document_review_for_patient( + review_update=mock_review_update, + field_names=field_names, + condition_expression="test condition expression", + ) + + mock_update_document.assert_called_once() + call_args = mock_update_document.call_args + + assert call_args.kwargs["document"] == mock_review_update + assert call_args.kwargs["update_fields_name"] == field_names + + condition_expr = call_args.kwargs["condition_expression"] + assert condition_expr == "test condition expression" + + assert result == {"Attributes": {"ID": "test-review-id"}} + + +def test_update_document_review_for_patient_builds_correct_condition_expression( + mock_service, mock_review_update, mocker +): + mock_update_document = mocker.patch.object(mock_service, "update_document") + + field_names = {"review_status"} + + mock_service.update_pending_review_status( + review_update=mock_review_update, + field_names=field_names, + ) + + call_args = mock_update_document.call_args + condition_expr = call_args.kwargs["condition_expression"] + + expected_condition = ( + Attr(DocumentReferenceMetadataFields.ID.value).exists() + & Attr("NhsNumber").eq(mock_review_update.nhs_number) + & Attr("ReviewStatus").eq(DocumentReviewStatus.PENDING_REVIEW) + ) + + assert condition_expr == expected_condition + assert call_args.kwargs["update_fields_name"] == field_names + assert call_args.kwargs["document"] == mock_review_update + assert call_args.kwargs["update_key"] == { + "ID": mock_review_update.id, + "Version": mock_review_update.version, + } + + +def test_update_document_review_for_patient_conditional_check_failed( + mock_service, mock_review_update, mocker +): + client_error = ClientError( + {"Error": {"Code": "ConditionalCheckFailedException"}}, "UpdateItem" + ) + mock_update_document = mocker.patch.object(mock_service, "update_document") + mock_update_document.side_effect = client_error + + field_names = {"review_status"} + + with pytest.raises(DocumentReviewException): + mock_service.update_approved_pending_review_status( + review_update=mock_review_update, + field_names=field_names, + ) + + call_args = mock_update_document.call_args + condition_expr = call_args.kwargs["condition_expression"] + + expected_condition = ( + Attr(DocumentReferenceMetadataFields.ID.value).exists() + & Attr("NhsNumber").eq(mock_review_update.nhs_number) + & Attr("ReviewStatus").eq(DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS) + ) + + assert condition_expr == expected_condition + assert call_args.kwargs["update_fields_name"] == field_names + assert call_args.kwargs["document"] == mock_review_update + assert call_args.kwargs["update_key"] == { + "ID": mock_review_update.id, + "Version": mock_review_update.version, + } + + +def test_update_document_review_for_patient_other_client_error( + mock_service, mock_review_update, mocker +): + client_error = ClientError( + {"Error": {"Code": "ResourceNotFoundException", "Message": "Table not found"}}, + "UpdateItem", + ) + mock_update_document = mocker.patch.object(mock_service, "update_document") + mock_update_document.side_effect = client_error + + field_names = {"review_status"} + + with pytest.raises(DocumentReviewException): + mock_service.update_approved_pending_review_status( + review_update=mock_review_update, + field_names=field_names, + ) + + +def test_update_document_review_with_transaction_builds_correct_items( + mock_service, mock_review_update, mocker +): + """Test that transaction items are built correctly for new and existing reviews.""" + mock_transact_write = mocker.patch.object( + mock_service.dynamo_service, "transact_write_items" + ) + mock_transact_item_builder = mocker.patch( + "services.document_upload_review_service.build_transaction_item" + ) + new_review = MagicMock(spec=DocumentUploadReviewReference) + new_review.id = "new-review-id" + new_review.version = 2 + + existing_review = mock_review_update + existing_review.custodian = TEST_ODS_CODE + + mock_service.update_document_review_with_transaction(new_review, existing_review) + + new_review.model_dump.assert_called_with( + exclude_none=True, by_alias=True, exclude={"version", "id"} + ) + + existing_review.model_dump.assert_called_with( + exclude_none=True, + by_alias=True, + include={"review_status", "review_date", "reviewer"}, + ) + + mock_transact_write.assert_called_once() + assert mock_transact_item_builder.call_count == 2 + call_args = mock_transact_write.call_args[0][0] + assert len(call_args) == 2 + + first_call = mock_transact_item_builder.call_args_list[0] + assert first_call.kwargs["conditions"] == [ + {"field": "ID", "operator": "attribute_not_exists"} + ] + # Verify second call (existing review) has proper conditions + second_call = mock_transact_item_builder.call_args_list[1] + expected_conditions = [ + { + "field": "ReviewStatus", + "operator": "=", + "value": DocumentReviewStatus.PENDING_REVIEW, + }, + {"field": "NhsNumber", "operator": "=", "value": TEST_NHS_NUMBER}, + {"field": "Custodian", "operator": "=", "value": TEST_ODS_CODE}, + ] + assert second_call.kwargs["conditions"] == expected_conditions + + +def test_delete_document_review_files_success(mock_service, mocker): + mock_delete_object = mocker.patch.object(mock_service.s3_service, "delete_object") + + document_review = MagicMock(spec=DocumentUploadReviewReference) + mock_file_1 = MagicMock() + mock_file_1.file_name = "file1.pdf" + mock_file_1.file_location = "s3://test-bucket/folder/file1.pdf" + + mock_file_2 = MagicMock() + mock_file_2.file_name = "file2.pdf" + mock_file_2.file_location = "s3://test-bucket/folder/file2.pdf" + + document_review.files = [mock_file_1, mock_file_2] + + mock_service.delete_document_review_files(document_review) + + assert mock_delete_object.call_count == 2 + mock_delete_object.assert_any_call("test-bucket", "folder/file1.pdf") + mock_delete_object.assert_any_call("test-bucket", "folder/file2.pdf") + + +def test_delete_document_review_files_handles_s3_error(mock_service, mocker): + mock_delete_object = mocker.patch.object(mock_service.s3_service, "delete_object") + + client_error = ClientError( + {"Error": {"Code": "NoSuchKey", "Message": "Object not found"}}, + "DeleteObject", ) + mock_delete_object.side_effect = [client_error, None] + + document_review = MagicMock(spec=DocumentUploadReviewReference) + mock_file_1 = MagicMock() + mock_file_1.file_name = "file1.pdf" + mock_file_1.file_location = "s3://test-bucket/folder/file1.pdf" + + mock_file_2 = MagicMock() + mock_file_2.file_name = "file2.pdf" + mock_file_2.file_location = "s3://test-bucket/folder/file2.pdf" + + document_review.files = [mock_file_1, mock_file_2] + + mock_service.delete_document_review_files(document_review) + + assert mock_delete_object.call_count == 2 + + +def test_update_document_review_with_transaction_transaction_cancelled( + mock_service, mock_review_update, mocker +): + """Test handling of TransactionCanceledException.""" + client_error = ClientError( + {"Error": {"Code": "TransactionCanceledException"}}, "TransactWriteItems" + ) + mock_transact_write = mocker.patch.object( + mock_service.dynamo_service, "transact_write_items" + ) + mock_transact_write.side_effect = client_error + + new_review = MagicMock(spec=DocumentUploadReviewReference) + new_review.id = "new-review-id" + new_review.version = 2 + + existing_review = mock_review_update + existing_review.custodian = TEST_ODS_CODE + + with pytest.raises(DocumentReviewException) as exc_info: + mock_service.update_document_review_with_transaction( + new_review, existing_review + ) + + assert "Failed to update document review" in str(exc_info.value) def test_build_review_query_filter_creates_filter_from_nhs_number(mock_service): diff --git a/lambdas/tests/unit/services/test_get_document_review_service.py b/lambdas/tests/unit/services/test_get_document_review_service.py index bb4fbc1ee..82b2696a2 100644 --- a/lambdas/tests/unit/services/test_get_document_review_service.py +++ b/lambdas/tests/unit/services/test_get_document_review_service.py @@ -19,6 +19,7 @@ from utils.lambda_exceptions import GetDocumentReviewException TEST_DOCUMENT_ID = "test-document-id-123" +TEST_DOCUMENT_VERSION = 1 TEST_DIFFERENT_NHS_NUMBER = "9000000010" TEST_ODS_CODE = "Y12345" TEST_FILE_LOCATION_1 = f"s3://{MOCK_DOCUMENT_REVIEW_BUCKET}/file1.pdf" @@ -39,6 +40,7 @@ def mock_service(set_env, mocker): service.s3_service.presigned_url_expiry = 1800 # 30 minutes service.document_review_service = MagicMock() service.document_review_service.dynamo_service = MagicMock() + service.cloudfront_table_name = MOCK_EDGE_REFERENCE_TABLE service.cloudfront_url = TEST_CLOUDFRONT_URL yield service @@ -75,7 +77,9 @@ def mock_document_review(): def test_get_document_review_success(mock_service, mock_document_review, mocker): """Test successful retrieval of a document review with pre-signed URLs.""" - mock_service.document_review_service.get_item.return_value = mock_document_review + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) mock_service.s3_service.create_download_presigned_url.side_effect = [ TEST_PRESIGNED_URL_1, @@ -88,10 +92,13 @@ def test_get_document_review_success(mock_service, mock_document_review, mocker) ) result = mock_service.get_document_review( - patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_DOCUMENT_VERSION, ) assert result is not None assert result["id"] == TEST_DOCUMENT_ID + assert result["version"] == TEST_DOCUMENT_VERSION assert result["uploadDate"] == 1699000000 assert result["documentSnomedCodeType"] == SnomedCodes.LLOYD_GEORGE.value.code assert len(result["files"]) == 2 @@ -106,8 +113,8 @@ def test_get_document_review_success(mock_service, mock_document_review, mocker) assert result["files"][1]["fileName"] == "file2.pdf" assert result["files"][1]["presignedUrl"].startswith(TEST_CLOUDFRONT_URL) - mock_service.document_review_service.get_item.assert_called_once_with( - TEST_DOCUMENT_ID + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_DOCUMENT_VERSION ) assert mock_service.s3_service.create_download_presigned_url.call_count == 2 @@ -127,29 +134,35 @@ def test_get_document_review_success(mock_service, mock_document_review, mocker) def test_get_document_review_not_found(mock_service): """Test when a document review is not found in DynamoDB.""" - mock_service.document_review_service.get_item.return_value = None + mock_service.document_review_service.get_document_review_by_id.return_value = None result = mock_service.get_document_review( - patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_DOCUMENT_VERSION, ) assert result is None - mock_service.document_review_service.get_item.assert_called_once_with( - TEST_DOCUMENT_ID + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_DOCUMENT_VERSION ) def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_review): """Test when document review exists but the NHS number doesn't match.""" - mock_service.document_review_service.get_item.return_value = mock_document_review + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) result = mock_service.get_document_review( - patient_id=TEST_DIFFERENT_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + patient_id=TEST_DIFFERENT_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_DOCUMENT_VERSION, ) assert result is None - mock_service.document_review_service.get_item.assert_called_once_with( - TEST_DOCUMENT_ID + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_DOCUMENT_VERSION ) mock_service.s3_service.create_download_presigned_url.assert_not_called() @@ -157,13 +170,15 @@ def test_get_document_review_nhs_number_mismatch(mock_service, mock_document_rev def test_get_document_review_dynamo_service_exception(mock_service): """Test handling of DynamoServiceException.""" - mock_service.document_review_service.get_item.side_effect = DynamoServiceException( - "DynamoDB error" + mock_service.document_review_service.get_document_review_by_id.side_effect = ( + DynamoServiceException("DynamoDB error") ) with pytest.raises(GetDocumentReviewException) as exc_info: mock_service.get_document_review( - patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_DOCUMENT_VERSION, ) assert exc_info.value.status_code == 500 @@ -172,13 +187,15 @@ def test_get_document_review_dynamo_service_exception(mock_service): def test_get_document_review_unexpected_exception(mock_service): """Test handling of unexpected exceptions.""" - mock_service.document_review_service.get_item.side_effect = Exception( - "Unexpected error" + mock_service.document_review_service.get_document_review_by_id.side_effect = ( + Exception("Unexpected error") ) with pytest.raises(GetDocumentReviewException) as exc_info: mock_service.get_document_review( - patient_id=TEST_NHS_NUMBER, document_id=TEST_DOCUMENT_ID + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_DOCUMENT_VERSION, ) assert exc_info.value.status_code == 500 diff --git a/lambdas/tests/unit/services/test_search_document_review_service.py b/lambdas/tests/unit/services/test_search_document_review_service.py index fb31fe02d..de6fbb80c 100644 --- a/lambdas/tests/unit/services/test_search_document_review_service.py +++ b/lambdas/tests/unit/services/test_search_document_review_service.py @@ -14,7 +14,6 @@ TEST_QUERY_LIMIT = "20" TEST_LAST_EVALUATED_KEY = {"id": TEST_UUID, "UploadDate": 0} -TEST_ENCODED_START_KEY = "" TEST_ENCODED_START_KEY = base64.b64encode( json.dumps(TEST_LAST_EVALUATED_KEY).encode("ascii") ).decode("utf-8") @@ -60,6 +59,7 @@ def test_handle_gateway_api_request_happy_path(search_document_review_service, m [ { "id": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0]["ID"], + "version": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0]["Version"], "reviewReason": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0][ "ReviewReason" ], @@ -76,6 +76,7 @@ def test_handle_gateway_api_request_happy_path(search_document_review_service, m }, { "id": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][1]["ID"], + "version": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][1]["Version"], "reviewReason": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][1][ "ReviewReason" ], @@ -92,6 +93,7 @@ def test_handle_gateway_api_request_happy_path(search_document_review_service, m }, { "id": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][2]["ID"], + "version": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][2]["Version"], "reviewReason": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][2][ "ReviewReason" ], diff --git a/lambdas/tests/unit/services/test_update_document_review_service.py b/lambdas/tests/unit/services/test_update_document_review_service.py new file mode 100644 index 000000000..ecc35962a --- /dev/null +++ b/lambdas/tests/unit/services/test_update_document_review_service.py @@ -0,0 +1,787 @@ +from unittest.mock import MagicMock + +import pytest +from enums.document_review_status import DocumentReviewStatus +from enums.lambda_error import LambdaError +from enums.snomed_codes import SnomedCodes +from freezegun import freeze_time +from models.document_review import ( + DocumentReviewFileDetails, + DocumentUploadReviewReference, + PatchDocumentReviewRequest, +) +from models.pds_models import PatientDetails +from services.update_document_review_service import UpdateDocumentReviewService +from tests.unit.conftest import MOCK_DOCUMENT_REVIEW_BUCKET, TEST_NHS_NUMBER +from utils.lambda_exceptions import UpdateDocumentReviewException +from utils.ods_utils import PCSE_ODS_CODE + +TEST_DOCUMENT_ID = "test-document-id-123" +TEST_DIFFERENT_NHS_NUMBER = "9000000017" +TEST_REASSIGNED_NHS_NUMBER = "9000000025" +TEST_ODS_CODE = "Y12345" +TEST_REVIEWER_ODS_CODE = "Y12345" +TEST_NEW_ODS_CODE = "Z99999" +TEST_FILE_LOCATION = f"s3://{MOCK_DOCUMENT_REVIEW_BUCKET}/file1.pdf" +TEST_DOCUMENT_REFERENCE_ID = "doc-ref-12345" +TEST_UPLOAD_DATE = 1699000000 +TEST_REVIEW_DATE = 1699100000 +TEST_FROZEN_TIME = "2024-01-15 12:00:00" +TEST_FROZEN_TIME_TIMESTAMP = 1705320000 +TEST_VERSION = 1 + + +@pytest.fixture +def mock_service(set_env, mocker): + mocker.patch("services.update_document_review_service.DocumentUploadReviewService") + mocker.patch("services.update_document_review_service.S3Service") + + service = UpdateDocumentReviewService() + service.document_review_service = MagicMock() + + yield service + + +@pytest.fixture +def mock_document_review(): + files = [ + DocumentReviewFileDetails( + file_name="file1.pdf", + file_location=TEST_FILE_LOCATION, + ), + ] + + review = DocumentUploadReviewReference( + id=TEST_DOCUMENT_ID, + author=TEST_ODS_CODE, + custodian=TEST_ODS_CODE, + review_status=DocumentReviewStatus.PENDING_REVIEW, + review_reason="Uploaded for review", + upload_date=TEST_UPLOAD_DATE, + files=files, + nhs_number=TEST_NHS_NUMBER, + document_snomed_code_type=SnomedCodes.LLOYD_GEORGE.value.code, + ) + + return review + + +@pytest.fixture +def mock_pds_service(mocker): + mock_pds = MagicMock() + + mock_patient_details = PatientDetails( + nhsNumber=TEST_REASSIGNED_NHS_NUMBER, + generalPracticeOds=TEST_NEW_ODS_CODE, + superseded=False, + restricted=False, + ) + mock_pds.fetch_patient_details.return_value = mock_patient_details + + mocker.patch( + "services.update_document_review_service.get_pds_service", return_value=mock_pds + ) + return mock_pds + + +@freeze_time(TEST_FROZEN_TIME) +def test_update_document_review_successfully_approves_document( + mock_service, mock_document_review +): + mock_document_review.review_status = DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_VERSION + ) + mock_service.document_review_service.update_approved_pending_review_status.assert_called_once() + + call_args = ( + mock_service.document_review_service.update_approved_pending_review_status.call_args + ) + updated_doc = call_args.kwargs["review_update"] + assert updated_doc.review_status == DocumentReviewStatus.APPROVED + assert updated_doc.document_reference_id == TEST_DOCUMENT_REFERENCE_ID + assert updated_doc.reviewer == TEST_REVIEWER_ODS_CODE + assert updated_doc.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert "review_status" in call_args.kwargs["field_names"] + assert "document_reference_id" in call_args.kwargs["field_names"] + assert "reviewer" in call_args.kwargs["field_names"] + assert "review_date" in call_args.kwargs["field_names"] + + +@pytest.mark.parametrize( + "rejection_status", + [ + DocumentReviewStatus.REJECTED, + DocumentReviewStatus.REJECTED_DUPLICATE, + ], +) +@freeze_time(TEST_FROZEN_TIME) +def test_update_document_review_successfully_rejects_document( + mock_service, mock_document_review, rejection_status +): + update_data = PatchDocumentReviewRequest( + review_status=rejection_status, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_VERSION + ) + mock_service.document_review_service.update_pending_review_status.assert_called_once() + + call_args = ( + mock_service.document_review_service.update_pending_review_status.call_args + ) + updated_doc = call_args.kwargs["review_update"] + assert updated_doc.review_status == rejection_status + assert updated_doc.reviewer == TEST_REVIEWER_ODS_CODE + assert updated_doc.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert "document_reference_id" not in call_args.kwargs["field_names"] + assert "reviewer" in call_args.kwargs["field_names"] + assert "review_date" in call_args.kwargs["field_names"] + + +@freeze_time(TEST_FROZEN_TIME) +def test_update_document_review_successfully_reassigns_document_to_new_patient( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + mock_service.document_review_service.update_document_review_with_transaction.assert_called_once() + call_args = ( + mock_service.document_review_service.update_document_review_with_transaction.call_args + ) + + new_review = call_args.kwargs["new_review_item"] + existing_review = call_args.kwargs["existing_review_item"] + + assert new_review.nhs_number == TEST_REASSIGNED_NHS_NUMBER + assert new_review.review_status == DocumentReviewStatus.PENDING_REVIEW + assert new_review.version == 2 + assert new_review.custodian == TEST_NEW_ODS_CODE + + assert existing_review.review_status == DocumentReviewStatus.REASSIGNED + assert existing_review.reviewer == TEST_REVIEWER_ODS_CODE + assert existing_review.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert existing_review.version == 1 + assert existing_review.custodian == TEST_ODS_CODE + assert existing_review.nhs_number == TEST_NHS_NUMBER + + +@freeze_time(TEST_FROZEN_TIME) +def test_update_document_review_successfully_reassigns_document_when_patient_unknown( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + mock_service.document_review_service.update_document_review_with_transaction.assert_called_once() + call_args = ( + mock_service.document_review_service.update_document_review_with_transaction.call_args + ) + + new_review = call_args.kwargs["new_review_item"] + existing_review = call_args.kwargs["existing_review_item"] + + assert new_review.nhs_number == "0000000000" + assert new_review.review_status == DocumentReviewStatus.PENDING_REVIEW + assert new_review.version == 2 + assert new_review.custodian == PCSE_ODS_CODE + + assert ( + existing_review.review_status == DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN + ) + assert existing_review.reviewer == TEST_REVIEWER_ODS_CODE + assert existing_review.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert existing_review.version == 1 + assert existing_review.custodian == TEST_ODS_CODE + assert existing_review.nhs_number == TEST_NHS_NUMBER + + +@freeze_time(TEST_FROZEN_TIME) +def test_update_document_review_successfully_sets_approved_pending_documents_status( + mock_service, mock_document_review +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + mock_service.document_review_service.update_pending_review_status.assert_called_once() + call_args = ( + mock_service.document_review_service.update_pending_review_status.call_args + ) + updated_doc = call_args.kwargs["review_update"] + assert updated_doc.review_status == DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + assert updated_doc.reviewer == TEST_REVIEWER_ODS_CODE + assert updated_doc.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert "document_reference_id" not in call_args.kwargs["field_names"] + assert "reviewer" in call_args.kwargs["field_names"] + assert "review_date" in call_args.kwargs["field_names"] + + +def test_update_document_review_raises_exception_when_document_not_found(mock_service): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = None + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + assert exc_info.value.status_code == 404 + assert exc_info.value.error == LambdaError.DocumentReviewNotFound + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_VERSION + ) + mock_service.document_review_service.update_pending_review_status.assert_not_called() + + +def test_update_document_review_raises_exception_when_approving_from_pending_review( + mock_service, mock_document_review +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocStatusUnavailable + mock_service.document_review_service.update_pending_review_status.assert_not_called() + + +def test_fetch_document_returns_document_when_exists( + mock_service, mock_document_review +): + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + result = mock_service._fetch_document(TEST_DOCUMENT_ID, TEST_VERSION) + + assert result == mock_document_review + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_VERSION + ) + + +def test_fetch_document_raises_404_when_document_not_found(mock_service): + mock_service.document_review_service.get_document_review_by_id.return_value = None + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service._fetch_document(TEST_DOCUMENT_ID, TEST_VERSION) + + assert exc_info.value.status_code == 404 + assert exc_info.value.error == LambdaError.DocumentReviewNotFound + mock_service.document_review_service.get_document_review_by_id.assert_called_once_with( + document_id=TEST_DOCUMENT_ID, document_version=TEST_VERSION + ) + + +def test_validate_patient_id_match_succeeds_when_nhs_numbers_match( + mock_service, mock_document_review +): + mock_service._validate_patient_id_match(mock_document_review, TEST_NHS_NUMBER) + + +def test_validate_patient_id_match_raises_exception_when_nhs_numbers_dont_match( + mock_service, mock_document_review +): + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service._validate_patient_id_match( + mock_document_review, TEST_DIFFERENT_NHS_NUMBER + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocNHSNumberMismatch + + +@pytest.mark.parametrize( + "valid_status", + [ + DocumentReviewStatus.PENDING_REVIEW, + DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, + ], +) +def test_validate_review_status_passes_when_status_is_valid( + mock_service, mock_document_review, valid_status +): + mock_document_review.review_status = valid_status + + mock_service._validate_review_status(mock_document_review) + + +@pytest.mark.parametrize( + "invalid_status", + [ + DocumentReviewStatus.APPROVED, + DocumentReviewStatus.REJECTED, + DocumentReviewStatus.REJECTED_DUPLICATE, + DocumentReviewStatus.REASSIGNED, + DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + ], +) +def test_validate_review_status_raises_exception_when_status_is_not_valid_for_update( + mock_service, mock_document_review, invalid_status +): + mock_document_review.review_status = invalid_status + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service._validate_review_status(mock_document_review) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocStatusUnavailable + + +def test_validate_user_match_custodian_succeeds_when_ods_codes_match( + mock_service, mock_document_review +): + mock_service._validate_user_match_custodian( + mock_document_review, TEST_REVIEWER_ODS_CODE + ) + + +def test_validate_user_match_custodian_raises_exception_when_ods_codes_dont_match( + mock_service, mock_document_review +): + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service._validate_user_match_custodian( + mock_document_review, "DIFFERENT_ODS" + ) + + assert exc_info.value.status_code == 403 + assert exc_info.value.error == LambdaError.DocumentReferenceUnauthorised + + +def test_validate_document_for_update_calls_all_validation_methods( + mock_service, mock_document_review, mocker +): + mock_validate_patient = mocker.patch.object( + mock_service, "_validate_patient_id_match" + ) + mock_validate_status = mocker.patch.object(mock_service, "_validate_review_status") + mock_validate_custodian = mocker.patch.object( + mock_service, "_validate_user_match_custodian" + ) + + mock_service._validate_document_for_update( + mock_document_review, TEST_NHS_NUMBER, TEST_REVIEWER_ODS_CODE + ) + + mock_validate_patient.assert_called_once_with(mock_document_review, TEST_NHS_NUMBER) + mock_validate_status.assert_called_once_with(mock_document_review) + mock_validate_custodian.assert_called_once_with( + mock_document_review, TEST_REVIEWER_ODS_CODE + ) + + +def test_create_reassigned_document_sets_new_nhs_number_for_normal_reassignment( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + + result = mock_service._create_reassigned_document(mock_document_review, update_data) + + assert result.nhs_number == TEST_REASSIGNED_NHS_NUMBER + assert result.review_status == DocumentReviewStatus.PENDING_REVIEW + assert result.version == 2 + assert result.custodian == TEST_NEW_ODS_CODE + assert result.review_date is None + assert result.reviewer is None + mock_pds_service.fetch_patient_details.assert_called_once_with( + TEST_REASSIGNED_NHS_NUMBER + ) + + +def test_create_reassigned_document_sets_unknown_nhs_number_for_unknown_patient( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + + result = mock_service._create_reassigned_document(mock_document_review, update_data) + + assert result.nhs_number == "0000000000" + assert result.review_status == DocumentReviewStatus.PENDING_REVIEW + assert result.version == 2 + assert result.custodian == PCSE_ODS_CODE + assert result.review_date is None + assert result.reviewer is None + mock_pds_service.fetch_patient_details.assert_not_called() + + +def test_create_reassigned_document_increments_version( + mock_service, mock_document_review, mock_pds_service +): + mock_document_review.version = 3 + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + + result = mock_service._create_reassigned_document(mock_document_review, update_data) + + assert result.version == 4 + + +def test_create_reassigned_document_preserves_document_id( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + + result = mock_service._create_reassigned_document(mock_document_review, update_data) + + assert result.id == TEST_DOCUMENT_ID + + +@freeze_time(TEST_FROZEN_TIME) +def test_process_review_status_update_sets_review_date_and_reviewer( + mock_service, mock_document_review +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, + ) + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + assert mock_document_review.review_date == TEST_FROZEN_TIME_TIMESTAMP + assert mock_document_review.reviewer == TEST_REVIEWER_ODS_CODE + assert ( + mock_document_review.review_status + == DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + ) + + +@freeze_time(TEST_FROZEN_TIME) +def test_process_review_status_update_calls_approved_service_for_approval( + mock_service, mock_document_review +): + mock_document_review.review_status = DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + mock_service.document_review_service.update_approved_pending_review_status.assert_called_once() + call_args = ( + mock_service.document_review_service.update_approved_pending_review_status.call_args + ) + assert ( + call_args.kwargs["review_update"].document_reference_id + == TEST_DOCUMENT_REFERENCE_ID + ) + assert call_args.kwargs["field_names"] == { + "review_status", + "document_reference_id", + "reviewer", + "review_date", + } + + +@freeze_time(TEST_FROZEN_TIME) +@pytest.mark.parametrize( + "rejection_status", + [DocumentReviewStatus.REJECTED, DocumentReviewStatus.REJECTED_DUPLICATE], +) +def test_process_review_status_update_calls_pending_service_for_rejection( + mock_service, mock_document_review, rejection_status +): + update_data = PatchDocumentReviewRequest(review_status=rejection_status) + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + mock_service.document_review_service.update_pending_review_status.assert_called_once() + call_args = ( + mock_service.document_review_service.update_pending_review_status.call_args + ) + assert call_args.kwargs["review_update"].review_status == rejection_status + assert call_args.kwargs["field_names"] == { + "review_status", + "review_date", + "reviewer", + } + + +@freeze_time(TEST_FROZEN_TIME) +@pytest.mark.parametrize( + "reassign_status", + [DocumentReviewStatus.REASSIGNED, DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN], +) +def test_process_review_status_update_calls_handle_reassignment_for_reassignment( + mock_service, mock_document_review, reassign_status, mocker +): + update_data = PatchDocumentReviewRequest( + review_status=reassign_status, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + mock_handle = mocker.patch.object(mock_service, "_handle_reassignment_status") + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + mock_handle.assert_called_once_with( + mock_document_review, update_data, TEST_DOCUMENT_ID + ) + + +@freeze_time(TEST_FROZEN_TIME) +def test_process_review_status_update_raises_exception_when_approving_from_wrong_status( + mock_service, mock_document_review +): + mock_document_review.review_status = DocumentReviewStatus.PENDING_REVIEW + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocStatusUnavailable + + +@freeze_time(TEST_FROZEN_TIME) +def test_process_review_status_update_calls_soft_delete_for_approval( + mock_service, mock_document_review, mocker +): + mock_document_review.review_status = DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + mock_soft_delete = mocker.patch.object(mock_service, "_handle_soft_delete") + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + mock_soft_delete.assert_called_once_with(mock_document_review) + + +@freeze_time(TEST_FROZEN_TIME) +@pytest.mark.parametrize( + "rejection_status", + [DocumentReviewStatus.REJECTED, DocumentReviewStatus.REJECTED_DUPLICATE], +) +def test_process_review_status_update_calls_soft_delete_for_rejection( + mock_service, mock_document_review, rejection_status, mocker +): + update_data = PatchDocumentReviewRequest(review_status=rejection_status) + mock_soft_delete = mocker.patch.object(mock_service, "_handle_soft_delete") + + mock_service._process_review_status_update( + mock_document_review, update_data, TEST_DOCUMENT_ID, TEST_REVIEWER_ODS_CODE + ) + + mock_soft_delete.assert_called_once_with(mock_document_review) + + +def test_handle_soft_delete_calls_delete_document_review_files( + mock_service, mock_document_review +): + mock_service._handle_soft_delete(mock_document_review) + + mock_service.document_review_service.delete_document_review_files.assert_called_once_with( + mock_document_review + ) + + +@freeze_time(TEST_FROZEN_TIME) +def test_handle_reassignment_status_creates_new_document_and_updates_with_transaction( + mock_service, mock_document_review, mock_pds_service +): + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.REASSIGNED, + nhs_number=TEST_REASSIGNED_NHS_NUMBER, + ) + + mock_service._handle_reassignment_status( + mock_document_review, update_data, TEST_DOCUMENT_ID + ) + + mock_service.document_review_service.update_document_review_with_transaction.assert_called_once() + call_args = ( + mock_service.document_review_service.update_document_review_with_transaction.call_args + ) + + new_review = call_args.kwargs["new_review_item"] + existing_review = call_args.kwargs["existing_review_item"] + + assert new_review.nhs_number == TEST_REASSIGNED_NHS_NUMBER + assert new_review.version == 2 + assert existing_review == mock_document_review + assert existing_review.version == 1 + + +@pytest.mark.parametrize( + "invalid_target_status", + [ + DocumentReviewStatus.PENDING_REVIEW, + DocumentReviewStatus.REJECTED, + DocumentReviewStatus.REJECTED_DUPLICATE, + DocumentReviewStatus.REASSIGNED, + DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + ], +) +def test_update_document_review_raises_exception_when_updating_approved_pending_to_invalid_status( + mock_service, mock_document_review, invalid_target_status +): + """Test that APPROVED_PENDING_DOCUMENTS can only transition to APPROVED""" + mock_document_review.review_status = DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + update_data = PatchDocumentReviewRequest( + review_status=invalid_target_status, + nhs_number=TEST_REASSIGNED_NHS_NUMBER if "REASSIGNED" in invalid_target_status.value else None, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocStatusUnavailable + + +@pytest.mark.parametrize( + "from_status", + [ + DocumentReviewStatus.APPROVED, + DocumentReviewStatus.REJECTED, + DocumentReviewStatus.REJECTED_DUPLICATE, + DocumentReviewStatus.REASSIGNED, + DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, + ], +) +def test_update_document_review_raises_exception_when_approving_from_non_approved_pending_status( + mock_service, mock_document_review, from_status +): + """Test that only APPROVED_PENDING_DOCUMENTS can transition to APPROVED""" + mock_document_review.review_status = from_status + update_data = PatchDocumentReviewRequest( + review_status=DocumentReviewStatus.APPROVED, + document_reference_id=TEST_DOCUMENT_REFERENCE_ID, + ) + mock_service.document_review_service.get_document_review_by_id.return_value = ( + mock_document_review + ) + + with pytest.raises(UpdateDocumentReviewException) as exc_info: + mock_service.update_document_review( + patient_id=TEST_NHS_NUMBER, + document_id=TEST_DOCUMENT_ID, + document_version=TEST_VERSION, + update_data=update_data, + reviewer_ods_code=TEST_REVIEWER_ODS_CODE, + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.error == LambdaError.UpdateDocStatusUnavailable \ No newline at end of file diff --git a/lambdas/tests/unit/utils/test_dynamo_utils.py b/lambdas/tests/unit/utils/test_dynamo_utils.py index 7013bd4ea..1195f0634 100644 --- a/lambdas/tests/unit/utils/test_dynamo_utils.py +++ b/lambdas/tests/unit/utils/test_dynamo_utils.py @@ -1,11 +1,11 @@ import json -from _pytest import monkeypatch import pytest from enums.infrastructure import DynamoTables from enums.lambda_error import LambdaError from enums.metadata_field_names import DocumentReferenceMetadataFields from tests.unit.conftest import ( + MOCK_TABLE_NAME, TEST_CURRENT_GP_ODS, TEST_DOCUMENT_LOCATION, TEST_FILE_KEY, @@ -19,6 +19,9 @@ ) from utils.dynamo_utils import ( DocTypeTableRouter, + build_general_transaction_item, + build_mixed_condition_expression, + build_transaction_item, create_expression_attribute_placeholder, create_expression_attribute_values, create_expression_value_placeholder, @@ -195,3 +198,334 @@ def test_dynamo_table_mapping_fails(set_env, doc_type): assert excinfo.value.status_code == 400 assert excinfo.value.error == LambdaError.DocTypeDB + + +def test_create_expression_value_placeholder_with_suffix(): + test_value = "VirusScannerResult" + expected = ":VirusScannerResult_condition_val" + + actual = create_expression_value_placeholder(test_value, "_condition") + + assert actual == expected + + +def test_create_expression_value_placeholder_with_empty_suffix(): + test_value = "VirusScannerResult" + expected = ":VirusScannerResult_val" + + actual = create_expression_value_placeholder(test_value, "") + + assert actual == expected + + +def test_build_mixed_condition_expression_single_condition(): + conditions = [{"field": "DocStatus", "operator": "=", "value": "final"}] + expected_expr = "#DocStatus_attr = :DocStatus_condition_val" + expected_names = {"#DocStatus_attr": "DocStatus"} + expected_values = {":DocStatus_condition_val": "final"} + + actual_expr, actual_names, actual_values = build_mixed_condition_expression( + conditions + ) + + assert actual_expr == expected_expr + assert actual_names == expected_names + assert actual_values == expected_values + + +def test_build_mixed_condition_expression_mixed_operators(): + conditions = [ + {"field": "DocStatus", "operator": "=", "value": "final"}, + {"field": "Deleted", "operator": "attribute_not_exists"}, + {"field": "Version", "operator": ">", "value": 0}, + ] + expected_names = { + "#DocStatus_attr": "DocStatus", + "#Deleted_attr": "Deleted", + "#Version_attr": "Version", + } + expected_values = {":DocStatus_condition_val": "final", ":Version_condition_val": 0} + + actual_expr, actual_names, actual_values = build_mixed_condition_expression( + conditions + ) + + assert "#DocStatus_attr = :DocStatus_condition_val" in actual_expr + assert "attribute_not_exists(#Deleted_attr)" in actual_expr + assert "#Version_attr > :Version_condition_val" in actual_expr + assert " AND " in actual_expr + assert actual_names == expected_names + assert actual_values == expected_values + + +def test_build_mixed_condition_expression_with_custom_suffix(): + conditions = [{"field": "Status", "operator": "=", "value": "active"}] + expected_expr = "#Status_attr = :Status_update_val" + expected_names = {"#Status_attr": "Status"} + expected_values = {":Status_update_val": "active"} + + actual_expr, actual_names, actual_values = build_mixed_condition_expression( + conditions, suffix="_update" + ) + + assert actual_expr == expected_expr + assert actual_names == expected_names + assert actual_values == expected_values + + +def test_build_general_transaction_item_put_action(): + table_name = MOCK_TABLE_NAME + item = {"ID": "test_id", "FileName": "test.pdf", "Status": "active"} + + result = build_general_transaction_item( + table_name=table_name, action="Put", item=item + ) + + assert "Put" in result + put_item = result["Put"] + assert put_item["TableName"] == table_name + assert put_item["Item"] == item + assert "ConditionExpression" not in put_item + + +def test_build_general_transaction_item_put_with_condition(): + table_name = MOCK_TABLE_NAME + item = {"ID": "test_id", "FileName": "test.pdf"} + conditions = [ + {"field": "Version", "operator": "=", "value": 5}, + {"field": "Deleted", "operator": "attribute_not_exists"}, + {"field": "Status", "operator": "<>", "value": "archived"}, + ] + + condition_expr, attr_names, attr_values = build_mixed_condition_expression( + conditions + ) + + result = build_general_transaction_item( + table_name=table_name, + action="Put", + item=item, + condition_expression=condition_expr, + expression_attribute_names=attr_names, + expression_attribute_values=attr_values, + ) + + assert "Put" in result + put_item = result["Put"] + assert put_item["TableName"] == table_name + assert put_item["Item"] == item + + condition_expr = put_item["ConditionExpression"] + assert "#Version_attr = :Version_condition_val" in condition_expr + assert "attribute_not_exists(#Deleted_attr)" in condition_expr + assert "#Status_attr <> :Status_condition_val" in condition_expr + assert " AND " in condition_expr + + assert put_item["ExpressionAttributeNames"]["#Version_attr"] == "Version" + assert put_item["ExpressionAttributeNames"]["#Deleted_attr"] == "Deleted" + assert put_item["ExpressionAttributeNames"]["#Status_attr"] == "Status" + + assert put_item["ExpressionAttributeValues"][":Version_condition_val"] == 5 + assert put_item["ExpressionAttributeValues"][":Status_condition_val"] == "archived" + + +def test_build_general_transaction_item_update_action(): + table_name = MOCK_TABLE_NAME + key = {"ID": "test_id"} + update_fields = {"FileName": "updated.pdf", "Status": "completed"} + + field_names = list(update_fields.keys()) + update_expr = create_update_expression(field_names) + _, attr_names = create_expressions(field_names) + attr_values = create_expression_attribute_values(update_fields) + + result = build_general_transaction_item( + table_name=table_name, + action="Update", + key=key, + update_expression=update_expr, + expression_attribute_names=attr_names, + expression_attribute_values=attr_values, + ) + + assert "Update" in result + update_item = result["Update"] + assert update_item["TableName"] == table_name + assert update_item["Key"] == key + assert "SET" in update_item["UpdateExpression"] + assert "#FileName_attr" in update_item["UpdateExpression"] + assert "#Status_attr" in update_item["UpdateExpression"] + assert update_item["ExpressionAttributeNames"]["#FileName_attr"] == "FileName" + assert update_item["ExpressionAttributeNames"]["#Status_attr"] == "Status" + assert update_item["ExpressionAttributeValues"][":FileName_val"] == "updated.pdf" + assert update_item["ExpressionAttributeValues"][":Status_val"] == "completed" + + +def test_build_general_transaction_item_update_with_condition(): + """Test Update action with both update expression and condition""" + table_name = MOCK_TABLE_NAME + key = {"ID": "test_id"} + update_fields = {"Status": "completed"} + conditions = [{"field": "Status", "operator": "=", "value": "pending"}] + + field_names = list(update_fields.keys()) + update_expr = create_update_expression(field_names) + _, update_attr_names = create_expressions(field_names) + update_attr_values = create_expression_attribute_values(update_fields) + + condition_expr, condition_attr_names, condition_attr_values = ( + build_mixed_condition_expression(conditions) + ) + + merged_names = {**update_attr_names, **condition_attr_names} + merged_values = {**update_attr_values, **condition_attr_values} + + result = build_general_transaction_item( + table_name=table_name, + action="Update", + key=key, + update_expression=update_expr, + condition_expression=condition_expr, + expression_attribute_names=merged_names, + expression_attribute_values=merged_values, + ) + + assert "Update" in result + update_item = result["Update"] + assert update_item["ConditionExpression"] == condition_expr + assert ":Status_val" in str(update_item["ExpressionAttributeValues"]) + assert ":Status_condition_val" in str(update_item["ExpressionAttributeValues"]) + + +def test_build_general_transaction_item_delete_with_condition(): + """Test Delete action with condition expression""" + table_name = MOCK_TABLE_NAME + key = {"ID": "test_id"} + conditions = [{"field": "Status", "operator": "=", "value": "archived"}] + + condition_expr, attr_names, attr_values = build_mixed_condition_expression( + conditions + ) + + result = build_general_transaction_item( + table_name=table_name, + action="Delete", + key=key, + condition_expression=condition_expr, + expression_attribute_names=attr_names, + expression_attribute_values=attr_values, + ) + + assert "Delete" in result + delete_item = result["Delete"] + assert delete_item["ConditionExpression"] == condition_expr + assert delete_item["ExpressionAttributeNames"] == attr_names + assert delete_item["ExpressionAttributeValues"] == attr_values + + +def test_build_general_transaction_item_put_without_item(): + table_name = MOCK_TABLE_NAME + + with pytest.raises(ValueError): + build_general_transaction_item(table_name=table_name, action="Put") + + +def test_build_general_transaction_item_update_without_key(): + table_name = MOCK_TABLE_NAME + update_fields = {"Status": "completed"} + + field_names = list(update_fields.keys()) + update_expr = create_update_expression(field_names) + _, attr_names = create_expressions(field_names) + attr_values = create_expression_attribute_values(update_fields) + + with pytest.raises(ValueError): + build_general_transaction_item( + table_name=table_name, + action="Update", + update_expression=update_expr, + expression_attribute_names=attr_names, + expression_attribute_values=attr_values, + ) + + +def test_build_transaction_item_put_action(): + table_name = MOCK_TABLE_NAME + item = {"ID": "test_id", "Name": "Test Name", "Status": "active"} + + result = build_transaction_item(table_name=table_name, action="Put", item=item) + + assert "Put" in result + put_item = result["Put"] + assert put_item["TableName"] == table_name + assert put_item["Item"] == item + assert "ConditionExpression" not in put_item + + +def test_build_transaction_item_update_with_fields_and_conditions(): + """Test Update action with both update fields and conditions""" + table_name = MOCK_TABLE_NAME + key = {"ID": "test_id"} + update_fields = {"Status": "completed", "LastUpdated": 1234567890} + conditions = [ + {"field": "Status", "operator": "=", "value": "pending"}, + {"field": "Deleted", "operator": "attribute_not_exists"}, + ] + + result = build_transaction_item( + table_name=table_name, + action="Update", + key=key, + update_fields=update_fields, + conditions=conditions, + condition_join_operator="AND", + ) + + assert "Update" in result + update_item = result["Update"] + assert update_item["TableName"] == table_name + assert update_item["Key"] == key + assert "UpdateExpression" in update_item + assert "SET" in update_item["UpdateExpression"] + assert "#Status_attr" in update_item["UpdateExpression"] + assert "#LastUpdated_attr" in update_item["UpdateExpression"] + assert "ConditionExpression" in update_item + assert "attribute_not_exists" in update_item["ConditionExpression"] + assert update_item["ExpressionAttributeNames"]["#Status_attr"] == "Status" + assert update_item["ExpressionAttributeNames"]["#Deleted_attr"] == "Deleted" + assert update_item["ExpressionAttributeValues"][":Status_val"] == "completed" + assert ( + update_item["ExpressionAttributeValues"][":Status_condition_val"] == "pending" + ) + + +def test_build_transaction_item_delete_with_conditions(): + table_name = MOCK_TABLE_NAME + key = {"ID": "test_id"} + conditions = [ + {"field": "Status", "operator": "=", "value": "archived"}, + {"field": "Expired", "operator": "=", "value": True}, + ] + + result = build_transaction_item( + table_name=table_name, + action="Delete", + key=key, + conditions=conditions, + condition_join_operator="OR", + ) + + assert "Delete" in result + delete_item = result["Delete"] + assert delete_item["TableName"] == table_name + assert delete_item["Key"] == key + assert "ConditionExpression" in delete_item + assert " OR " in delete_item["ConditionExpression"] + assert "#Status_attr" in delete_item["ConditionExpression"] + assert "#Expired_attr" in delete_item["ConditionExpression"] + assert delete_item["ExpressionAttributeNames"]["#Status_attr"] == "Status" + assert delete_item["ExpressionAttributeNames"]["#Expired_attr"] == "Expired" + assert ( + delete_item["ExpressionAttributeValues"][":Status_condition_val"] == "archived" + ) + assert delete_item["ExpressionAttributeValues"][":Expired_condition_val"] is True diff --git a/lambdas/utils/dynamo_utils.py b/lambdas/utils/dynamo_utils.py index f35b998d7..bfa9acb1b 100644 --- a/lambdas/utils/dynamo_utils.py +++ b/lambdas/utils/dynamo_utils.py @@ -95,18 +95,20 @@ def create_expression_attribute_values(attribute_field_values: dict) -> dict: return expression_attribute_values -def create_expression_value_placeholder(value: str) -> str: +def create_expression_value_placeholder(value: str, suffix: str = "") -> str: """ Creates a placeholder value for an expression attribute name :param value: Value to change into a placeholder + :param suffix: Optional suffix to add before "_val" (e.g. "_condition") - example usage: + Example usage: placeholder = create_expression_value_placeholder("VirusScanResult") - - result: - ":VirusScanResult_val" + # Result: ":VirusScanResult_val" + placeholder = create_expression_value_placeholder("VirusScanResult", "_condition") + # Result: ":VirusScanResult_condition_val" """ - return f":{inflection.camelize(value, uppercase_first_letter=True)}_val" + camelized = inflection.camelize(value, uppercase_first_letter=True) + return f":{camelized}{suffix}_val" def create_expression_attribute_placeholder(value: str) -> str: @@ -157,6 +159,172 @@ def parse_dynamo_record(dynamodb_record: Dict[str, Any]) -> Dict[str, Any]: return result +def build_mixed_condition_expression( + conditions: list[dict[str, Any]], + join_operator: str = "AND", + suffix: str = "_condition", +) -> tuple[str, dict[str, str], dict[str, Any]]: + """ + Build a condition expression with mixed operators and conditions. + + Args: + conditions: List of condition dictionaries, each with: + - "field": field name + - "operator": comparison operator or "attribute_exists"/"attribute_not_exists" + - "value": value to compare (not needed for existence checks) + Example: [ + {"field": "DocStatus", "operator": "=", "value": "final"}, + {"field": "Deleted", "operator": "attribute_not_exists"} + ] + join_operator: Logical operator to join conditions (default: "AND") + suffix: Suffix to add to value placeholders (default: "_condition") + + Returns: + Tuple of (condition_expression, expression_attribute_names, expression_attribute_values) + """ + condition_expressions = [] + condition_attribute_names = {} + condition_attribute_values = {} + + for condition in conditions: + field_name = condition["field"] + operator = condition["operator"] + field_value = condition.get("value") + + condition_placeholder = create_expression_attribute_placeholder(field_name) + condition_attribute_names[condition_placeholder] = field_name + + if operator in ["attribute_exists", "attribute_not_exists"]: + condition_expressions.append(f"{operator}({condition_placeholder})") + else: + condition_value_placeholder = create_expression_value_placeholder( + field_name, suffix + ) + condition_expressions.append( + f"{condition_placeholder} {operator} {condition_value_placeholder}" + ) + condition_attribute_values[condition_value_placeholder] = field_value + + condition_expression = f" {join_operator} ".join(condition_expressions) + + return condition_expression, condition_attribute_names, condition_attribute_values + + +def build_general_transaction_item( + table_name: str, + action: str, + key: dict | None = None, + item: dict | None = None, + update_expression: str | None = None, + condition_expression: str | None = None, + expression_attribute_names: dict | None = None, + expression_attribute_values: dict | None = None, +) -> dict[str, dict[str, Any]]: + """ + Build a general DynamoDB transaction item for any action type. + All expressions and attributes must be pre-formatted. + + Args: + table_name: The name of the DynamoDB table + action: Transaction action type ('Put', 'Update', 'Delete', 'ConditionCheck') + key: The primary key of the item (required for Update, Delete, ConditionCheck) + item: The complete item to put (required for Put) + update_expression: Pre-formatted update expression (optional for Update) + condition_expression: Pre-formatted condition expression (optional) + expression_attribute_names: Pre-formatted expression attribute names (optional) + expression_attribute_values: Pre-formatted expression attribute values (optional) + + Returns: + A transaction item dict ready for transact_write_items + + Raises: + ValueError: If required parameters are missing for the specified action + """ + action = action.capitalize() + + if action not in ["Put", "Update", "Delete", "Conditioncheck"]: + raise ValueError( + f"Invalid action: {action}. Must be one of: Put, Update, Delete, ConditionCheck" + ) + + transaction_item: dict[str, dict[str, Any]] = {action: {"TableName": table_name}} + + if action == "Put": + if item is None: + raise ValueError("'item' is required for Put action") + transaction_item[action]["Item"] = item + + elif action == "Update": + if key is None: + raise ValueError("'key' is required for Update action") + transaction_item[action]["Key"] = key + if update_expression: + transaction_item[action]["UpdateExpression"] = update_expression + + elif action in ["Delete", "Conditioncheck"]: + if key is None: + raise ValueError(f"'key' is required for {action} action") + transaction_item[action]["Key"] = key + + if condition_expression: + transaction_item[action]["ConditionExpression"] = condition_expression + + if expression_attribute_names: + transaction_item[action][ + "ExpressionAttributeNames" + ] = expression_attribute_names + + if expression_attribute_values: + transaction_item[action][ + "ExpressionAttributeValues" + ] = expression_attribute_values + + return transaction_item + + +def build_transaction_item( + table_name: str, + action: str, + key: dict | None = None, + item: dict | None = None, + update_fields: dict | None = None, + conditions: list[dict] | None = None, + condition_join_operator: str = "AND", +) -> dict: + update_expression = None + condition_expression = None + expression_attribute_names = {} + expression_attribute_values = {} + + if action.lower() == "update" and update_fields: + field_names = list(update_fields.keys()) + update_expression = create_update_expression(field_names) + _, update_attr_names = create_expressions(field_names) + update_attr_values = create_expression_attribute_values(update_fields) + + expression_attribute_names.update(update_attr_names) + expression_attribute_values.update(update_attr_values) + + if conditions: + condition_expr, condition_attr_names, condition_attr_values = ( + build_mixed_condition_expression(conditions, condition_join_operator) + ) + condition_expression = condition_expr + expression_attribute_names.update(condition_attr_names) + expression_attribute_values.update(condition_attr_values) + + return build_general_transaction_item( + table_name=table_name, + action=action, + key=key, + item=item, + update_expression=update_expression, + condition_expression=condition_expression, + expression_attribute_names=expression_attribute_names or None, + expression_attribute_values=expression_attribute_values or None, + ) + + class DocTypeTableRouter: def __init__(self): self._define_tables() diff --git a/lambdas/utils/exceptions.py b/lambdas/utils/exceptions.py index a67ae9b2b..0cf3cbf02 100644 --- a/lambdas/utils/exceptions.py +++ b/lambdas/utils/exceptions.py @@ -50,6 +50,10 @@ class DocumentServiceException(Exception): pass +class DocumentReviewException(Exception): + pass + + class MissingEnvVarException(Exception): pass @@ -64,7 +68,6 @@ class FileProcessingException(Exception): class LGFileTypeException(ValueError): """One or more of the files do not match the required file type.""" - pass diff --git a/lambdas/utils/lambda_exceptions.py b/lambdas/utils/lambda_exceptions.py index b53ee4c7f..4a3c51c8a 100644 --- a/lambdas/utils/lambda_exceptions.py +++ b/lambdas/utils/lambda_exceptions.py @@ -115,3 +115,8 @@ class DocumentReviewException(LambdaException): class GetDocumentReviewException(LambdaException): pass + + +class UpdateDocumentReviewException(LambdaException): + pass + diff --git a/lambdas/utils/lambda_handler_utils.py b/lambdas/utils/lambda_handler_utils.py index 3654e3e91..1ed3a0b8b 100644 --- a/lambdas/utils/lambda_handler_utils.py +++ b/lambdas/utils/lambda_handler_utils.py @@ -6,7 +6,7 @@ from utils.lambda_exceptions import ( GetFhirDocumentReferenceException, DocumentRefSearchException, - DocumentRefException, + DocumentRefException, DocumentReviewException, ) logger = LoggingService(__name__) @@ -36,3 +36,21 @@ def extract_bearer_token(event, context): return bearer_token return None + + +def validate_review_path_parameters(event): + path_parameters = event.get("pathParameters", {}) + document_id = path_parameters.get("id") + document_version = path_parameters.get("version") + if not document_id or not document_version: + raise DocumentReviewException( + 400, LambdaError.DocumentReferenceMissingParameters + ) + try: + document_version = int(document_version) + except ValueError: + logger.error(f"Invalid document version: {document_version}") + raise DocumentReviewException( + 400, LambdaError.DocumentReferenceMissingParameters + ) + return document_id, document_version \ No newline at end of file