diff --git a/lambdas/enums/document_review_status.py b/lambdas/enums/document_review_status.py new file mode 100644 index 000000000..3fdc5c451 --- /dev/null +++ b/lambdas/enums/document_review_status.py @@ -0,0 +1,7 @@ +from enum import StrEnum + + +class DocumentReviewStatus(StrEnum): + PENDING_REVIEW = "PENDING_REVIEW" + APPROVED = "APPROVED" + REJECTED = "REJECTED" diff --git a/lambdas/enums/feature_flags.py b/lambdas/enums/feature_flags.py index 69d2f16e6..72a59e44b 100755 --- a/lambdas/enums/feature_flags.py +++ b/lambdas/enums/feature_flags.py @@ -10,3 +10,4 @@ class FeatureFlags(StrEnum): "lloydGeorgeValidationStrictModeEnabled" ) UPLOAD_DOCUMENT_ITERATION_2_ENABLED = "uploadDocumentIteration2Enabled" + UPLOAD_DOCUMENT_ITERATION_3_ENABLED = "uploadDocumentIteration3Enabled" diff --git a/lambdas/handlers/mns_notification_handler.py b/lambdas/handlers/mns_notification_handler.py index 8cf7cc5a9..ffc8df2a8 100644 --- a/lambdas/handlers/mns_notification_handler.py +++ b/lambdas/handlers/mns_notification_handler.py @@ -19,6 +19,7 @@ "APPCONFIG_CONFIGURATION", "APPCONFIG_ENVIRONMENT", "LLOYD_GEORGE_DYNAMODB_NAME", + "DOCUMENT_REVIEW_DYNAMODB_NAME", "MNS_NOTIFICATION_QUEUE_URL", ] ) diff --git a/lambdas/models/document_review.py b/lambdas/models/document_review.py new file mode 100644 index 000000000..0d2c698fb --- /dev/null +++ b/lambdas/models/document_review.py @@ -0,0 +1,48 @@ +import uuid +from typing import Optional + +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 + + +class DocumentReviewFileDetails(BaseModel): + model_config = ConfigDict( + validate_by_alias=True, + validate_by_name=True, + alias_generator=to_pascal, + ) + + file_name: str + file_location: str + + +class DocumentUploadReviewReference(BaseModel): + model_config = ConfigDict( + validate_by_alias=True, + validate_by_name=True, + alias_generator=to_pascal, + use_enum_values=True, + ) + id: str = Field( + default_factory=lambda: str(uuid.uuid4()), + alias=str(DocumentReferenceMetadataFields.ID.value), + ) + author: str + custodian: str + review_status: DocumentReviewStatus = Field( + default=DocumentReviewStatus.PENDING_REVIEW + ) + review_reason: str + review_date: int = Field(default=None) + reviewer: str = Field(default=None) + upload_date: int + files: list[DocumentReviewFileDetails] + nhs_number: str + ttl: Optional[int] = Field( + alias=str(DocumentReferenceMetadataFields.TTL.value), default=None + ) + document_reference_id: str = Field(default=None) + document_snomed_code_type: str = Field(default=SnomedCodes.LLOYD_GEORGE.value.code) diff --git a/lambdas/services/document_reference_service.py b/lambdas/services/document_reference_service.py new file mode 100644 index 000000000..72f4dca5e --- /dev/null +++ b/lambdas/services/document_reference_service.py @@ -0,0 +1,74 @@ +from datetime import datetime + +from enums.supported_document_types import SupportedDocumentTypes +from models.document_reference import DocumentReference +from services.document_service import DocumentService +from utils.audit_logging_setup import LoggingService +from utils.dynamo_utils import filter_uploaded_docs_and_recently_uploading_docs +from utils.exceptions import FileUploadInProgress, NoAvailableDocument + +logger = LoggingService(__name__) + + +class DocumentReferenceService(DocumentService): + """Service for handling DocumentReference operations.""" + + def __init__(self, doc_type: SupportedDocumentTypes = SupportedDocumentTypes.LG): + super().__init__() + self.doc_type = doc_type + + @property + def table_name(self) -> str: + return self.doc_type.get_dynamodb_table_name() + + @property + def model_class(self) -> type: + return DocumentReference + + @property + def s3_bucket(self) -> str: + return self.doc_type.get_s3_bucket_name() + + def get_available_lloyd_george_record_for_patient( + self, nhs_number: str + ) -> list[DocumentReference]: + """Get available Lloyd George records for a patient, checking for upload status.""" + filter_expression = filter_uploaded_docs_and_recently_uploading_docs() + available_docs = self.fetch_documents_from_table_with_nhs_number( + nhs_number, + query_filter=filter_expression, + ) + + file_in_progress_message = ( + "The patients Lloyd George record is in the process of being uploaded" + ) + if not available_docs: + raise NoAvailableDocument() + for document in available_docs: + if document.uploading and not document.uploaded: + raise FileUploadInProgress(file_in_progress_message) + return available_docs + + def update_patient_ods_code( + self, + patient_documents: list[DocumentReference], + updated_ods_code: str, + ) -> None: + update_field = {"current_gp_ods", "custodian", "last_updated"} + if not patient_documents: + return + + for reference in patient_documents: + logger.info("Updating patient document reference...") + + if ( + reference.current_gp_ods != updated_ods_code + or reference.custodian != updated_ods_code + ): + reference.current_gp_ods = updated_ods_code + reference.custodian = updated_ods_code + reference.last_updated = int(datetime.now().timestamp()) + + self.update_document( + document=reference, update_fields_name=update_field + ) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 715713bc5..3fd420704 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -5,25 +5,39 @@ from enums.metadata_field_names import DocumentReferenceMetadataFields from enums.supported_document_types import SupportedDocumentTypes from models.document_reference import DocumentReference -from pydantic import ValidationError +from pydantic import BaseModel, ValidationError from services.base.dynamo_service import DynamoDBService from services.base.s3_service import S3Service from utils.audit_logging_setup import LoggingService from utils.common_query_filters import NotDeleted -from utils.dynamo_utils import filter_uploaded_docs_and_recently_uploading_docs -from utils.exceptions import ( - DocumentServiceException, - FileUploadInProgress, - NoAvailableDocument, -) +from utils.exceptions import DocumentServiceException logger = LoggingService(__name__) class DocumentService: + """Service for document operations.""" + def __init__(self): self.s3_service = S3Service() self.dynamo_service = DynamoDBService() + self._lg_table_name = os.getenv("LLOYD_GEORGE_DYNAMODB_NAME") + self._lg_s3_bucket = os.getenv("LLOYD_GEORGE_BUCKET_NAME") + + @property + def table_name(self) -> str: + """DynamoDB table name. Can be overridden by child classes.""" + return self._lg_table_name + + @property + def s3_bucket(self) -> str: + """S3 bucket name. Can be overridden by child classes.""" + return self._lg_s3_bucket + + @property + def model_class(self) -> type[BaseModel]: + """Pydantic model class. Can be overridden by child classes.""" + return DocumentReference def fetch_available_document_references_by_type( self, @@ -34,34 +48,47 @@ def fetch_available_document_references_by_type( table_name = doc_type.get_dynamodb_table_name() return self.fetch_documents_from_table_with_nhs_number( - nhs_number, table_name, query_filter=query_filter + nhs_number, + table_name, + query_filter=query_filter, ) def fetch_documents_from_table_with_nhs_number( - self, nhs_number: str, table: str, query_filter: Attr | ConditionBase = None - ) -> list[DocumentReference]: + self, + nhs_number: str, + table: str | None = None, + query_filter: Attr | ConditionBase = None, + model_class: type[BaseModel] = None, + ) -> list: + """Fetch documents by NHS number from specified or configured table.""" + table_name = table or self.table_name + documents = self.fetch_documents_from_table( - table=table, index_name="NhsNumberIndex", search_key="NhsNumber", search_condition=nhs_number, query_filter=query_filter, + table_name=table_name, + model_class=model_class, ) - return documents def fetch_documents_from_table( self, - table: str, search_condition: str, search_key: str, index_name: str | None = None, query_filter: Attr | ConditionBase = None, - ) -> list[DocumentReference]: + table_name: str | None = None, + model_class: type[BaseModel] = None, + ) -> list: + """Fetch documents from specified or configured table using model_class.""" documents = [] + table_name = table_name or self.table_name + model_class = model_class or self.model_class response = self.dynamo_service.query_table( - table_name=table, + table_name=table_name, index_name=index_name, search_key=search_key, search_condition=search_condition, @@ -69,7 +96,7 @@ def fetch_documents_from_table( ) for item in response: try: - document = DocumentReference.model_validate(item) + document = model_class.model_validate(item) documents.append(document) except ValidationError as e: logger.error(f"Validation error on document: {item}") @@ -77,13 +104,18 @@ def fetch_documents_from_table( continue return documents - def get_nhs_numbers_based_on_ods_code(self, ods_code: str) -> list[str]: + def get_nhs_numbers_based_on_ods_code( + self, ods_code: str, table_name: str | None = None + ) -> 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( - table=os.environ["LLOYD_GEORGE_DYNAMODB_NAME"], index_name="OdsCodeIndex", search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, search_condition=ods_code, query_filter=NotDeleted, + table_name=table_name, ) nhs_numbers = list({document.nhs_number for document in documents}) return nhs_numbers @@ -139,21 +171,27 @@ def delete_document_object(self, bucket: str, key: str): def update_document( self, - table_name: str, - document_reference: DocumentReference, - update_fields_name: set[str] = None, + table_name: str | None = None, + document: BaseModel = None, + update_fields_name: set[str] | None = None, ): + """Update document in specified or configured table.""" + table_name = table_name or self.table_name + self.dynamo_service.update_item( table_name=table_name, - key_pair={DocumentReferenceMetadataFields.ID.value: document_reference.id}, - updated_fields=document_reference.model_dump( + key_pair={DocumentReferenceMetadataFields.ID.value: document.id}, + updated_fields=document.model_dump( exclude_none=True, by_alias=True, include=update_fields_name ), ) def hard_delete_metadata_records( - self, table_name: str, document_references: list[DocumentReference] + self, table_name: str, document_references: list[BaseModel] ): + """Permanently delete metadata from specified or configured table.""" + table_name = table_name or self.table_name + logger.info(f"Deleting items in table: {table_name} (HARD DELETE)") primary_key_name = DocumentReferenceMetadataFields.ID.value for reference in document_references: @@ -162,7 +200,8 @@ def hard_delete_metadata_records( self.dynamo_service.delete_item(table_name, deletion_key) @staticmethod - def is_upload_in_process(record: DocumentReference): + def is_upload_in_process(record: DocumentReference) -> bool: + """Check if a document upload is currently in progress.""" return ( not record.uploaded and record.uploading @@ -170,33 +209,17 @@ def is_upload_in_process(record: DocumentReference): and record.doc_status != "final" ) - def get_available_lloyd_george_record_for_patient( - self, nhs_number - ) -> list[DocumentReference]: - filter_expression = filter_uploaded_docs_and_recently_uploading_docs() - available_docs = self.fetch_available_document_references_by_type( - nhs_number, - SupportedDocumentTypes.LG, - query_filter=filter_expression, - ) - - file_in_progress_message = ( - "The patients Lloyd George record is in the process of being uploaded" - ) - if not available_docs: - raise NoAvailableDocument() - for document in available_docs: - if document.uploading and not document.uploaded: - raise FileUploadInProgress(file_in_progress_message) - return available_docs - def get_batch_document_references_by_id( self, document_ids: list[str], doc_type: SupportedDocumentTypes - ) -> list[DocumentReference]: + ) -> list: table_name = doc_type.get_dynamodb_table_name() + + table_name = table_name or self.table_name + model_class = self.model_class + response = self.dynamo_service.batch_get_items( table_name=table_name, key_list=document_ids ) - found_docs = [DocumentReference.model_validate(item) for item in response] + found_docs = [model_class.model_validate(item) for item in response] return found_docs diff --git a/lambdas/services/document_upload_review_service.py b/lambdas/services/document_upload_review_service.py new file mode 100644 index 000000000..c4c4ee1d2 --- /dev/null +++ b/lambdas/services/document_upload_review_service.py @@ -0,0 +1,47 @@ +import os + +from models.document_review import DocumentUploadReviewReference +from services.document_service import DocumentService +from utils.audit_logging_setup import LoggingService + +logger = LoggingService(__name__) + + +class DocumentUploadReviewService(DocumentService): + """Service for handling DocumentUploadReviewReference operations.""" + def __init__(self): + super().__init__() + self._table_name = os.environ.get("DOCUMENT_REVIEW_DYNAMODB_NAME") + self._s3_bucket = os.environ.get("DOCUMENT_REVIEW_S3_BUCKET_NAME") + + @property + def table_name(self) -> str: + return self._table_name + + @property + def model_class(self) -> type: + return DocumentUploadReviewReference + + @property + def s3_bucket(self) -> str: + return self._s3_bucket + + def update_document_review_custodian( + self, + patient_documents: list[DocumentUploadReviewReference], + updated_ods_code: str, + ): + review_update_field = {"custodian"} + if not patient_documents: + return + + for review in patient_documents: + logger.info("Updating document review custodian...") + + if review.custodian != updated_ods_code: + review.custodian = updated_ods_code + + self.update_document( + document=review, + update_fields_name=review_update_field, + ) diff --git a/lambdas/services/fhir_document_reference_service_base.py b/lambdas/services/fhir_document_reference_service_base.py index bb0c860c2..525614514 100644 --- a/lambdas/services/fhir_document_reference_service_base.py +++ b/lambdas/services/fhir_document_reference_service_base.py @@ -120,7 +120,7 @@ def _create_document_reference( def _get_document_reference(self, document_id: str, table) -> DocumentReference: documents = self.document_service.fetch_documents_from_table( - table=table, + table_name=table, search_condition=document_id, search_key="ID", query_filter=CurrentStatusFile, diff --git a/lambdas/services/get_fhir_document_reference_service.py b/lambdas/services/get_fhir_document_reference_service.py index cc38a4344..d4e3cd8a2 100644 --- a/lambdas/services/get_fhir_document_reference_service.py +++ b/lambdas/services/get_fhir_document_reference_service.py @@ -48,7 +48,7 @@ def _get_dynamo_table_for_doc_type(self, doc_type: SnomedCode) -> str: def get_document_references(self, document_id: str, table) -> DocumentReference: documents = self.document_service.fetch_documents_from_table( - table=table, + table_name=table, search_condition=document_id, search_key="ID", query_filter=CurrentStatusFile, diff --git a/lambdas/services/lloyd_george_generate_stitch_service.py b/lambdas/services/lloyd_george_generate_stitch_service.py index 7ed5062cd..c3538939f 100644 --- a/lambdas/services/lloyd_george_generate_stitch_service.py +++ b/lambdas/services/lloyd_george_generate_stitch_service.py @@ -12,7 +12,7 @@ from pikepdf import Pdf from pypdf.errors import PyPdfError from services.base.s3_service import S3Service -from services.document_service import DocumentService +from services.document_reference_service import DocumentReferenceService from utils.audit_logging_setup import LoggingService from utils.exceptions import NoAvailableDocument from utils.filename_utils import extract_page_number @@ -25,14 +25,13 @@ class LloydGeorgeStitchService: def __init__(self, stitch_trace: StitchTrace): - self.lloyd_george_table_name = os.environ.get("LLOYD_GEORGE_DYNAMODB_NAME") self.lloyd_george_bucket_name = os.environ.get("LLOYD_GEORGE_BUCKET_NAME") self.lifecycle_policy_tag = os.environ.get( "STITCHED_FILE_LIFECYCLE_POLICY_TAG", "autodelete" ) self.s3_service = S3Service() - self.document_service = DocumentService() + self.document_service = DocumentReferenceService() self.stitch_trace_object = stitch_trace self.stitch_trace_table = os.environ.get("STITCH_METADATA_DYNAMODB_NAME") self.stitch_file_name = f"patient-record-{str(uuid.uuid4())}" diff --git a/lambdas/services/lloyd_george_stitch_job_service.py b/lambdas/services/lloyd_george_stitch_job_service.py index ff8a7e059..e2a015ffc 100644 --- a/lambdas/services/lloyd_george_stitch_job_service.py +++ b/lambdas/services/lloyd_george_stitch_job_service.py @@ -10,7 +10,7 @@ from pydantic import ValidationError from services.base.dynamo_service import DynamoDBService from services.base.s3_service import S3Service -from services.document_service import DocumentService +from services.document_reference_service import DocumentReferenceService from utils.audit_logging_setup import LoggingService from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder from utils.exceptions import FileUploadInProgress, NoAvailableDocument @@ -28,7 +28,7 @@ def __init__(self): custom_aws_role=get_document_presign_url_aws_role_arn ) self.dynamo_service = DynamoDBService() - self.document_service = DocumentService() + self.document_service = DocumentReferenceService() self.stitch_trace_table = os.environ.get("STITCH_METADATA_DYNAMODB_NAME") self.lloyd_george_table_name = os.environ.get("LLOYD_GEORGE_DYNAMODB_NAME") self.cloudfront_table_name = os.environ.get("EDGE_REFERENCE_TABLE") diff --git a/lambdas/services/process_mns_message_service.py b/lambdas/services/process_mns_message_service.py index 5c63d2f8f..67941f6d5 100644 --- a/lambdas/services/process_mns_message_service.py +++ b/lambdas/services/process_mns_message_service.py @@ -1,17 +1,19 @@ import os -from datetime import datetime from botocore.exceptions import ClientError from enums.death_notification_status import DeathNotificationStatus +from enums.feature_flags import FeatureFlags from enums.mns_notification_types import MNSNotificationTypes from enums.patient_ods_inactive_status import PatientOdsInactiveStatus from models.document_reference import DocumentReference +from models.document_review import DocumentUploadReviewReference from models.sqs.mns_sqs_message import MNSSQSMessage from services.base.sqs_service import SQSService -from services.document_service import DocumentService +from services.document_reference_service import DocumentReferenceService +from services.document_upload_review_service import DocumentUploadReviewService +from services.feature_flags_service import FeatureFlagService from utils.audit_logging_setup import LoggingService from utils.exceptions import PdsErrorException -from utils.ods_utils import PCSE_ODS_CODE from utils.utilities import get_pds_service logger = LoggingService(__name__) @@ -19,13 +21,13 @@ class MNSNotificationService: def __init__(self): - self.document_service = DocumentService() - self.table = os.getenv("LLOYD_GEORGE_DYNAMODB_NAME") + self.document_review_service = DocumentUploadReviewService() + self.lg_document_service = DocumentReferenceService() self.pds_service = get_pds_service() self.sqs_service = SQSService() self.queue = os.getenv("MNS_NOTIFICATION_QUEUE_URL") - self.DOCUMENT_UPDATE_FIELDS = {"current_gp_ods", "custodian", "last_updated"} - self.PCSE_ODS = PCSE_ODS_CODE + self.feature_flag_service = FeatureFlagService() + self.is_review_feature_enabled = self.check_if_review_feature_enabled() def handle_mns_notification(self, message: MNSSQSMessage): try: @@ -53,15 +55,17 @@ def handle_mns_notification(self, message: MNSSQSMessage): raise e def handle_gp_change_notification(self, message: MNSSQSMessage) -> None: - patient_document_references = self.get_patient_documents( + lg_documents, review_documents = self.get_all_patient_documents( message.subject.nhs_number ) - if not patient_document_references: + if not lg_documents and not review_documents: return updated_ods_code = self.get_updated_gp_ods(message.subject.nhs_number) - self.update_patient_ods_code(patient_document_references, updated_ods_code) + self.update_all_patient_documents( + lg_documents, review_documents, updated_ods_code + ) logger.info("Update complete for change of GP") def handle_death_notification(self, message: MNSSQSMessage) -> None: @@ -75,58 +79,78 @@ def handle_death_notification(self, message: MNSSQSMessage) -> None: ) case DeathNotificationStatus.REMOVED: - patient_document_references = self.get_patient_documents(nhs_number) - if patient_document_references: + lg_documents, review_documents = self.get_all_patient_documents( + nhs_number + ) + + if lg_documents or review_documents: updated_ods_code = self.get_updated_gp_ods(nhs_number) - self.update_patient_ods_code( - patient_document_references, updated_ods_code + self.update_all_patient_documents( + lg_documents, review_documents, updated_ods_code ) logger.info("Update complete for death notification change.") case DeathNotificationStatus.FORMAL: - patient_document_references = self.get_patient_documents(nhs_number) - if patient_document_references: - self.update_patient_ods_code( - patient_document_references, PatientOdsInactiveStatus.DECEASED + lg_documents, review_documents = self.get_all_patient_documents( + nhs_number + ) + + if lg_documents or review_documents: + self.update_all_patient_documents( + lg_documents, + review_documents, + PatientOdsInactiveStatus.DECEASED, ) logger.info( f"Update complete, patient marked {PatientOdsInactiveStatus.DECEASED}." ) - def update_patient_ods_code( - self, patient_documents: list[DocumentReference], updated_ods_code: str - ) -> None: - if not patient_documents: - return - for reference in patient_documents: - logger.info("Updating patient document reference...") - - if ( - reference.current_gp_ods != updated_ods_code - or reference.custodian != updated_ods_code - ): - updated_custodian = updated_ods_code - if updated_ods_code in [ - PatientOdsInactiveStatus.DECEASED, - PatientOdsInactiveStatus.SUSPENDED, - ]: - updated_custodian = self.PCSE_ODS - reference.current_gp_ods = updated_ods_code - reference.custodian = updated_custodian - reference.last_updated = int(datetime.now().timestamp()) - - self.document_service.update_document( - self.table, - reference, - self.DOCUMENT_UPDATE_FIELDS, - ) - def get_updated_gp_ods(self, nhs_number: str) -> str: patient_details = self.pds_service.fetch_patient_details(nhs_number) return patient_details.general_practice_ods - def get_patient_documents(self, nhs_number: str) -> list[DocumentReference]: - """Fetch patient documents and return them if they exist.""" - return self.document_service.fetch_documents_from_table_with_nhs_number( - nhs_number, self.table + def get_all_patient_documents( + self, nhs_number: str + ) -> tuple[list[DocumentReference], list[DocumentUploadReviewReference]]: + """Fetch patient documents from both LG and document review tables.""" + lg_documents = ( + self.lg_document_service.fetch_documents_from_table_with_nhs_number( + nhs_number + ) ) + if self.is_review_feature_enabled: + review_documents = ( + self.document_review_service.fetch_documents_from_table_with_nhs_number( + nhs_number + ) + ) + else: + review_documents = [] + return lg_documents, review_documents + + def update_all_patient_documents( + self, + lg_documents: list[DocumentReference], + review_documents: list[DocumentUploadReviewReference], + updated_ods_code: str, + ) -> None: + """Update documents in both tables if they exist.""" + if lg_documents: + self.lg_document_service.update_patient_ods_code( + lg_documents, updated_ods_code + ) + if review_documents and self.is_review_feature_enabled: + self.document_review_service.update_document_review_custodian( + review_documents, updated_ods_code + ) + + def check_if_review_feature_enabled(self) -> bool: + upload_lambda_enabled_flag_object = ( + self.feature_flag_service.get_feature_flags_by_flag( + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED + ) + ) + + return upload_lambda_enabled_flag_object[ + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED + ] diff --git a/lambdas/services/upload_document_reference_service.py b/lambdas/services/upload_document_reference_service.py index 73a594884..40d057868 100644 --- a/lambdas/services/upload_document_reference_service.py +++ b/lambdas/services/upload_document_reference_service.py @@ -92,7 +92,7 @@ def _fetch_preliminary_document_reference( """Fetch document reference from the database""" try: documents = self.document_service.fetch_documents_from_table( - table=self.table_name, + table_name=self.table_name, search_condition=document_key, search_key="ID", query_filter=PreliminaryStatus, @@ -205,7 +205,7 @@ def _finalize_and_supersede_with_transaction(self, new_document: DocumentReferen existing_docs: list[DocumentReference] = ( self.document_service.fetch_documents_from_table( - table=self.table_name, + table_name=self.table_name, index_name="S3FileKeyIndex", search_condition=new_document.s3_file_key, search_key="S3FileKey", @@ -442,7 +442,7 @@ def _update_dynamo_table( self.document_service.update_document( table_name=self.table_name, - document_reference=document, + document=document, update_fields_name=update_fields, ) diff --git a/lambdas/tests/unit/conftest.py b/lambdas/tests/unit/conftest.py index 7eaf123bd..184a0a7d4 100644 --- a/lambdas/tests/unit/conftest.py +++ b/lambdas/tests/unit/conftest.py @@ -136,7 +136,8 @@ MOCK_TEAMS_WEBHOOK = "test_teams_webhook" MOCK_SLACK_BOT_TOKEN = f"xoxb-{TEST_UUID}" MOCK_ALERTING_SLACK_CHANNEL_ID = "slack_channel_id" - +MOCK_DOCUMENT_REVIEW_TABLE = "test_document_review" +MOCK_DOCUMENT_REVIEW_BUCKET = "test_document_review_bucket" @pytest.fixture def set_env(monkeypatch): @@ -225,6 +226,8 @@ def set_env(monkeypatch): monkeypatch.setenv("SLACK_BOT_TOKEN", MOCK_SLACK_BOT_TOKEN) monkeypatch.setenv("SLACK_CHANNEL_ID", MOCK_ALERTING_SLACK_CHANNEL_ID) monkeypatch.setenv("ITOC_TESTING_ODS_CODES", MOCK_ITOC_ODS_CODES) + monkeypatch.setenv("DOCUMENT_REVIEW_DYNAMODB_NAME", MOCK_DOCUMENT_REVIEW_TABLE) + monkeypatch.setenv("DOCUMENT_REVIEW_S3_BUCKET_NAME", MOCK_DOCUMENT_REVIEW_BUCKET) monkeypatch.setenv("STAGING_STORE_BUCKET_NAME", MOCK_STAGING_STORE_BUCKET) monkeypatch.setenv("METADATA_SQS_QUEUE_URL", MOCK_LG_METADATA_SQS_QUEUE) diff --git a/lambdas/tests/unit/services/test_document_reference_service.py b/lambdas/tests/unit/services/test_document_reference_service.py new file mode 100644 index 000000000..c59d7eee0 --- /dev/null +++ b/lambdas/tests/unit/services/test_document_reference_service.py @@ -0,0 +1,367 @@ +from datetime import datetime +from unittest.mock import MagicMock + +import pytest +from enums.supported_document_types import SupportedDocumentTypes +from freezegun import freeze_time +from models.document_reference import DocumentReference +from services.document_reference_service import DocumentReferenceService +from tests.unit.conftest import ( + MOCK_ARF_BUCKET, + MOCK_ARF_TABLE_NAME, + MOCK_LG_BUCKET, + MOCK_LG_TABLE_NAME, + TEST_CURRENT_GP_ODS, + TEST_NHS_NUMBER, +) +from tests.unit.helpers.data.test_documents import ( + create_test_lloyd_george_doc_store_refs, +) +from utils.exceptions import FileUploadInProgress, NoAvailableDocument + +MOCK_UPDATE_TIME = "2024-01-15 10:30:00" +NEW_ODS_CODE = "Z98765" + + +@pytest.fixture +def mock_lg_service(set_env, mocker): + """Fixture to create a DocumentReferenceService for Lloyd George documents.""" + mocker.patch("services.document_service.S3Service") + mocker.patch("services.document_service.DynamoDBService") + service = DocumentReferenceService(doc_type=SupportedDocumentTypes.LG) + yield service + + +@pytest.fixture +def mock_arf_service(set_env, mocker): + """Fixture to create a DocumentReferenceService for ARF documents.""" + mocker.patch("services.document_service.S3Service") + mocker.patch("services.document_service.DynamoDBService") + service = DocumentReferenceService(doc_type=SupportedDocumentTypes.ARF) + yield service + + +@pytest.fixture +def mock_document_references(): + """Create a list of mock document references.""" + docs = [] + for i in range(3): + doc = MagicMock(spec=DocumentReference) + doc.id = f"doc-id-{i}" + doc.nhs_number = TEST_NHS_NUMBER + doc.current_gp_ods = TEST_CURRENT_GP_ODS + doc.custodian = TEST_CURRENT_GP_ODS + doc.uploaded = True + doc.uploading = False + docs.append(doc) + return docs + + +def test_table_name_returns_lg_table(mock_lg_service): + """Test that table_name property returns correct table for LG documents.""" + assert mock_lg_service.table_name == MOCK_LG_TABLE_NAME + + +def test_table_name_returns_arf_table(mock_arf_service): + """Test that table_name property returns correct table for ARF documents.""" + assert mock_arf_service.table_name == MOCK_ARF_TABLE_NAME + + +def test_model_class_returns_document_reference(mock_lg_service): + """Test that the model_class property returns DocumentReference.""" + assert mock_lg_service.model_class == DocumentReference + + +def test_s3_bucket_returns_lg_bucket(mock_lg_service): + """Test that s3_bucket property returns the correct bucket for LG documents.""" + assert mock_lg_service.s3_bucket == MOCK_LG_BUCKET + + +def test_s3_bucket_returns_arf_bucket(mock_arf_service): + """Test that s3_bucket property returns the correct bucket for ARF documents.""" + assert mock_arf_service.s3_bucket == MOCK_ARF_BUCKET + + +def test_returns_available_documents_when_uploaded(mock_lg_service, mocker): + """Test that available documents are returned when they are uploaded.""" + mock_filter = mocker.patch( + "services.document_reference_service.filter_uploaded_docs_and_recently_uploading_docs" + ) + mock_filter.return_value = MagicMock() + + mock_docs = create_test_lloyd_george_doc_store_refs( + override={"uploaded": True, "uploading": False} + ) + + mocker.patch.object( + mock_lg_service, + "fetch_documents_from_table_with_nhs_number", + return_value=mock_docs, + ) + + result = mock_lg_service.get_available_lloyd_george_record_for_patient( + TEST_NHS_NUMBER + ) + + assert result == mock_docs + assert len(result) == 3 + mock_lg_service.fetch_documents_from_table_with_nhs_number.assert_called_once_with( + TEST_NHS_NUMBER, query_filter=mock_filter.return_value + ) + + +def test_raises_no_available_document_when_no_docs_found(mock_lg_service, mocker): + """Test that NoAvailableDocument is raised when no documents are found.""" + mock_filter = mocker.patch( + "services.document_reference_service.filter_uploaded_docs_and_recently_uploading_docs" + ) + mock_filter.return_value = MagicMock() + + mocker.patch.object( + mock_lg_service, + "fetch_documents_from_table_with_nhs_number", + return_value=[], + ) + + with pytest.raises(NoAvailableDocument): + mock_lg_service.get_available_lloyd_george_record_for_patient(TEST_NHS_NUMBER) + + +def test_raises_file_upload_in_progress_when_document_uploading( + mock_lg_service, mocker +): + """Test that FileUploadInProgress is raised when a document is being uploaded.""" + mock_filter = mocker.patch( + "services.document_reference_service.filter_uploaded_docs_and_recently_uploading_docs" + ) + mock_filter.return_value = MagicMock() + + mock_docs = create_test_lloyd_george_doc_store_refs( + override={"uploaded": False, "uploading": True} + ) + + mocker.patch.object( + mock_lg_service, + "fetch_documents_from_table_with_nhs_number", + return_value=mock_docs, + ) + + with pytest.raises(FileUploadInProgress) as exc_info: + mock_lg_service.get_available_lloyd_george_record_for_patient(TEST_NHS_NUMBER) + + assert "in the process of being uploaded" in str(exc_info.value).lower() + + +def test_returns_documents_when_uploaded_and_not_uploading(mock_lg_service, mocker): + """Test that documents are returned when uploaded is True and uploading is False.""" + mock_filter = mocker.patch( + "services.document_reference_service.filter_uploaded_docs_and_recently_uploading_docs" + ) + mock_filter.return_value = MagicMock() + + mock_docs = create_test_lloyd_george_doc_store_refs( + override={"uploaded": True, "uploading": False} + ) + + mocker.patch.object( + mock_lg_service, + "fetch_documents_from_table_with_nhs_number", + return_value=mock_docs, + ) + + result = mock_lg_service.get_available_lloyd_george_record_for_patient( + TEST_NHS_NUMBER + ) + + assert len(result) == 3 + for doc in result: + assert doc.uploaded is True + assert doc.uploading is False + + +@freeze_time(MOCK_UPDATE_TIME) +def test_updates_all_documents_with_different_ods_code( + mock_lg_service, mock_document_references, mocker +): + """Test that all documents are updated when ODS code differs.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify all documents were updated + assert mock_update.call_count == 3 + + # Verify each document's fields were changed + for doc in mock_document_references: + assert doc.current_gp_ods == NEW_ODS_CODE + assert doc.custodian == NEW_ODS_CODE + assert doc.last_updated == int( + datetime.fromisoformat(MOCK_UPDATE_TIME).timestamp() + ) + + # Verify update_document was called with the correct parameters + for doc in mock_document_references: + mock_update.assert_any_call( + document=doc, + update_fields_name={"current_gp_ods", "custodian", "last_updated"}, + ) + + +def test_returns_early_when_no_documents_provided(mock_lg_service, mocker): + """Test that method returns early when an empty list is provided.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + mock_lg_service.update_patient_ods_code([], NEW_ODS_CODE) + + mock_update.assert_not_called() + + +@freeze_time(MOCK_UPDATE_TIME) +def test_does_not_update_when_ods_codes_already_match( + mock_lg_service, mock_document_references, mocker +): + """Test that documents are not updated when ODS codes already match.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + # Set all documents to already have the new ODS code + for doc in mock_document_references: + doc.current_gp_ods = NEW_ODS_CODE + doc.custodian = NEW_ODS_CODE + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify update_document was not called + mock_update.assert_not_called() + + +@freeze_time(MOCK_UPDATE_TIME) +def test_updates_when_only_current_gp_ods_differs( + mock_lg_service, mock_document_references, mocker +): + """Test that documents are updated when only current_gp_ods differs.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + # Set custodian to match but current_gp_ods to differ + for doc in mock_document_references: + doc.current_gp_ods = TEST_CURRENT_GP_ODS + doc.custodian = NEW_ODS_CODE + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify all documents were updated + assert mock_update.call_count == 3 + + # Verify both fields are now updated + for doc in mock_document_references: + assert doc.current_gp_ods == NEW_ODS_CODE + assert doc.custodian == NEW_ODS_CODE + + +@freeze_time(MOCK_UPDATE_TIME) +def test_updates_when_only_custodian_differs( + mock_lg_service, mock_document_references, mocker +): + """Test that documents are updated when only custodian differs.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + # Set current_gp_ods to match but custodian to differ + for doc in mock_document_references: + doc.current_gp_ods = NEW_ODS_CODE + doc.custodian = TEST_CURRENT_GP_ODS + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify all documents were updated + assert mock_update.call_count == 3 + + # Verify both fields are now updated + for doc in mock_document_references: + assert doc.current_gp_ods == NEW_ODS_CODE + assert doc.custodian == NEW_ODS_CODE + + +@freeze_time(MOCK_UPDATE_TIME) +def test_updates_single_document(mock_lg_service, mocker): + """Test updating a single document.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + single_doc = MagicMock(spec=DocumentReference) + single_doc.id = "single-doc-id" + single_doc.current_gp_ods = TEST_CURRENT_GP_ODS + single_doc.custodian = TEST_CURRENT_GP_ODS + + mock_lg_service.update_patient_ods_code([single_doc], NEW_ODS_CODE) + + assert single_doc.current_gp_ods == NEW_ODS_CODE + assert single_doc.custodian == NEW_ODS_CODE + assert single_doc.last_updated == int( + datetime.fromisoformat(MOCK_UPDATE_TIME).timestamp() + ) + + mock_update.assert_called_once_with( + document=single_doc, + update_fields_name={"current_gp_ods", "custodian", "last_updated"}, + ) + + +@freeze_time(MOCK_UPDATE_TIME) +def test_updates_mixed_documents_some_matching( + mock_lg_service, mock_document_references, mocker +): + """Test updating documents where some already have correct ODS code.""" + mock_update = mocker.patch.object(mock_lg_service, "update_document") + + # Set the first document to already have new ODS code + mock_document_references[0].current_gp_ods = NEW_ODS_CODE + mock_document_references[0].custodian = NEW_ODS_CODE + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify only 2 documents were updated (not the first one) + assert mock_update.call_count == 2 + + # Verify the correct documents were updated + mock_update.assert_any_call( + document=mock_document_references[1], + update_fields_name={"current_gp_ods", "custodian", "last_updated"}, + ) + mock_update.assert_any_call( + document=mock_document_references[2], + update_fields_name={"current_gp_ods", "custodian", "last_updated"}, + ) + + +def test_logs_update_message(mock_lg_service, mock_document_references, mocker): + """Test that logging is performed during an update.""" + mocker.patch.object(mock_lg_service, "update_document") + mock_logger = mocker.patch("services.document_reference_service.logger") + + mock_lg_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + + # Verify logging was called for each document that needed updating + assert mock_logger.info.call_count == 3 + mock_logger.info.assert_any_call("Updating patient document reference...") + + +def test_initialisation_with_default_lg_type(set_env, mocker): + """Test that the service initialises with LG type by default.""" + mocker.patch("services.document_service.S3Service") + mocker.patch("services.document_service.DynamoDBService") + + service = DocumentReferenceService() + + assert service.doc_type == SupportedDocumentTypes.LG + assert service.table_name == MOCK_LG_TABLE_NAME + assert service.s3_bucket == MOCK_LG_BUCKET + + +def test_initialisation_with_arf_type(set_env, mocker): + """Test that the service initialises correctly with an ARF type.""" + mocker.patch("services.document_service.S3Service") + mocker.patch("services.document_service.DynamoDBService") + + service = DocumentReferenceService(doc_type=SupportedDocumentTypes.ARF) + + assert service.doc_type == SupportedDocumentTypes.ARF + assert service.table_name == MOCK_ARF_TABLE_NAME + assert service.s3_bucket == MOCK_ARF_BUCKET diff --git a/lambdas/tests/unit/services/test_document_service.py b/lambdas/tests/unit/services/test_document_service.py index e1fbf3e34..5d5d97ddf 100644 --- a/lambdas/tests/unit/services/test_document_service.py +++ b/lambdas/tests/unit/services/test_document_service.py @@ -340,12 +340,14 @@ def test_get_nhs_numbers_based_on_ods_code(mock_service, mocker): return_value=mock_documents, ) - result = mock_service.get_nhs_numbers_based_on_ods_code(ods_code) + result = mock_service.get_nhs_numbers_based_on_ods_code( + ods_code, MOCK_LG_TABLE_NAME + ) assert result == [expected_nhs_number] mock_fetch.assert_called_once_with( - table="test_lg_dynamoDB_table", + table_name=MOCK_LG_TABLE_NAME, index_name="OdsCodeIndex", search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value, search_condition=ods_code, diff --git a/lambdas/tests/unit/services/test_document_upload_review_service.py b/lambdas/tests/unit/services/test_document_upload_review_service.py new file mode 100644 index 000000000..e399a6ee3 --- /dev/null +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -0,0 +1,159 @@ +from unittest.mock import MagicMock + +import pytest +from models.document_review import DocumentUploadReviewReference +from services.document_upload_review_service import DocumentUploadReviewService +from tests.unit.conftest import TEST_NHS_NUMBER, MOCK_DOCUMENT_REVIEW_TABLE, MOCK_DOCUMENT_REVIEW_BUCKET + +TEST_ODS_CODE = "Y12345" +NEW_ODS_CODE = "Z98765" + + +@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, + ) + service = DocumentUploadReviewService() + service.s3_service = MagicMock() + service.dynamo_service = MagicMock() + yield service + + +@pytest.fixture +def mock_document_review_references(): + """Create a list of mock document review references.""" + reviews = [] + for i in range(3): + review = MagicMock(spec=DocumentUploadReviewReference) + review.id = f"review-id-{i}" + review.nhs_number = TEST_NHS_NUMBER + review.custodian = TEST_ODS_CODE + reviews.append(review) + return reviews + + +def test_table_name(mock_service): + """Test that table_name property returns correct environment variable.""" + + assert mock_service.table_name == MOCK_DOCUMENT_REVIEW_TABLE + + +def test_model_class(mock_service): + """Test that the model_class property returns DocumentUploadReviewReference.""" + assert mock_service.model_class == DocumentUploadReviewReference + + +def test_s3_bucket(mock_service, monkeypatch): + """Test that s3_bucket property returns the correct environment variable.""" + + assert mock_service.s3_bucket == MOCK_DOCUMENT_REVIEW_BUCKET + + +def test_update_document_review_custodian_updates_all_documents( + mock_service, mock_document_review_references, mocker +): + """Test that update_document_review_custodian updates all documents with different custodian.""" + mock_update_document = mocker.patch.object(mock_service, "update_document") + + mock_service.update_document_review_custodian( + mock_document_review_references, NEW_ODS_CODE + ) + + # Verify that all documents were updated + assert mock_update_document.call_count == 3 + + # Verify each document's custodian was changed + for review in mock_document_review_references: + assert review.custodian == NEW_ODS_CODE + + # 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"} + ) + + +def test_update_document_review_custodian_empty_list(mock_service, mocker): + """Test that update_document_review_custodian handles an empty list gracefully.""" + mock_update_document = mocker.patch.object(mock_service, "update_document") + + mock_service.update_document_review_custodian([], NEW_ODS_CODE) + + # Verify that update_document was not called + mock_update_document.assert_not_called() + + +def test_update_document_review_custodian_no_changes_needed( + mock_service, mock_document_review_references, mocker +): + """Test that update_document_review_custodian skips documents that already have the correct custodian.""" + mock_update_document = mocker.patch.object(mock_service, "update_document") + + # Set all reviews to already have the target custodian + for review in mock_document_review_references: + review.custodian = NEW_ODS_CODE + + mock_service.update_document_review_custodian( + mock_document_review_references, NEW_ODS_CODE + ) + + # Verify that update_document was not called since no changes needed + mock_update_document.assert_not_called() + + +def test_update_document_review_custodian_mixed_custodians( + mock_service, mock_document_review_references, mocker +): + """Test that update_document_review_custodian only updates documents that need updating.""" + mock_update_document = mocker.patch.object(mock_service, "update_document") + + # Set the first review to already have the new custodian + mock_document_review_references[0].custodian = NEW_ODS_CODE + # Keep the other two with the old custodian + + mock_service.update_document_review_custodian( + 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 + + +def test_update_document_review_custodian_logging( + mock_service, mock_document_review_references, mocker +): + """Test that update_document_review_custodian logs appropriately.""" + mocker.patch.object(mock_service, "update_document") + mock_logger = mocker.patch("services.document_upload_review_service.logger") + + mock_service.update_document_review_custodian( + 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.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"} + ) diff --git a/lambdas/tests/unit/services/test_lloyd_george_generate_stitch_service.py b/lambdas/tests/unit/services/test_lloyd_george_generate_stitch_service.py index 70dcd29fb..f47a90e31 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_generate_stitch_service.py +++ b/lambdas/tests/unit/services/test_lloyd_george_generate_stitch_service.py @@ -2,17 +2,13 @@ from io import BytesIO import pytest -from boto3.dynamodb.conditions import Attr, ConditionBase from botocore.exceptions import ClientError -from enums.supported_document_types import SupportedDocumentTypes from enums.trace_status import TraceStatus from models.document_reference import DocumentReference from models.stitch_trace import StitchTrace from pypdf import PdfWriter -from services.document_service import DocumentService from services.lloyd_george_generate_stitch_service import LloydGeorgeStitchService from tests.unit.conftest import MOCK_LG_BUCKET, TEST_NHS_NUMBER, TEST_UUID -from utils.dynamo_utils import filter_uploaded_docs_and_recently_uploading_docs from utils.lambda_exceptions import LGStitchServiceException @@ -81,7 +77,10 @@ def multiple_mock_docs(): @pytest.fixture -def stitch_service(set_env): +def stitch_service(set_env, mocker): + mocker.patch( + "services.lloyd_george_generate_stitch_service.DocumentReferenceService" + ) yield LloydGeorgeStitchService(MOCK_STITCH_TRACE_OBJECT) @@ -92,8 +91,10 @@ def patched_stitch_service(set_env, mocker): "get_lloyd_george_record_for_patient", return_value=MOCK_LLOYD_GEORGE_DOCUMENT_REFS, ) + mocker.patch( + "services.lloyd_george_generate_stitch_service.DocumentReferenceService" + ) mocker.patch("services.lloyd_george_generate_stitch_service.S3Service") - mocker.patch("services.lloyd_george_generate_stitch_service.DocumentService") mocker.patch.object( LloydGeorgeStitchService, "upload_stitched_lg_record", @@ -115,17 +116,17 @@ def patched_stitch_service(set_env, mocker): @pytest.fixture -def mock_fetch_doc_ref_by_type(mocker): +def mock_fetch_doc_ref_by_type(mocker, stitch_service): def mocked_method( - nhs_number: str, doc_type: str, query_filter: Attr | ConditionBase + nhs_number: str, ): - if nhs_number == TEST_NHS_NUMBER and doc_type == SupportedDocumentTypes.LG: + if nhs_number == TEST_NHS_NUMBER: return MOCK_LLOYD_GEORGE_DOCUMENT_REFS return [] yield mocker.patch.object( - DocumentService, - "fetch_available_document_references_by_type", + stitch_service.document_service, + "get_available_lloyd_george_record_for_patient", side_effect=mocked_method, ) @@ -203,15 +204,12 @@ def test_stitch_lloyd_george_record_raises_404_if_no_docs(patched_stitch_service def test_get_lloyd_george_record_for_patient( stitch_service, mock_fetch_doc_ref_by_type ): - mock_filters = filter_uploaded_docs_and_recently_uploading_docs() expected = MOCK_LLOYD_GEORGE_DOCUMENT_REFS actual = stitch_service.get_lloyd_george_record_for_patient() assert actual == expected - mock_fetch_doc_ref_by_type.assert_called_with( - TEST_NHS_NUMBER, SupportedDocumentTypes.LG, query_filter=mock_filters - ) + mock_fetch_doc_ref_by_type.assert_called_with(TEST_NHS_NUMBER) def test_sort_documents_by_filenames_base_case(stitch_service, multiple_mock_docs): diff --git a/lambdas/tests/unit/services/test_lloyd_george_stitch_job_service.py b/lambdas/tests/unit/services/test_lloyd_george_stitch_job_service.py index e8e0a9c15..43b4b957c 100644 --- a/lambdas/tests/unit/services/test_lloyd_george_stitch_job_service.py +++ b/lambdas/tests/unit/services/test_lloyd_george_stitch_job_service.py @@ -20,7 +20,7 @@ @pytest.fixture def stitch_service(set_env, mocker): mocker.patch("services.lloyd_george_stitch_job_service.S3Service") - mocker.patch("services.lloyd_george_stitch_job_service.DocumentService") + mocker.patch("services.lloyd_george_stitch_job_service.DocumentReferenceService") mocker.patch("services.lloyd_george_stitch_job_service.DynamoDBService") yield LloydGeorgeStitchJobService() diff --git a/lambdas/tests/unit/services/test_process_mns_message_service.py b/lambdas/tests/unit/services/test_process_mns_message_service.py index 2204cd8a3..cbbb9b115 100644 --- a/lambdas/tests/unit/services/test_process_mns_message_service.py +++ b/lambdas/tests/unit/services/test_process_mns_message_service.py @@ -1,12 +1,13 @@ -from datetime import datetime from unittest.mock import MagicMock import pytest from botocore.exceptions import ClientError +from enums.feature_flags import FeatureFlags from enums.patient_ods_inactive_status import PatientOdsInactiveStatus -from freezegun import freeze_time from models.document_reference import DocumentReference +from models.document_review import DocumentUploadReviewReference from models.sqs.mns_sqs_message import MNSSQSMessage +from services.feature_flags_service import FeatureFlagService from services.process_mns_message_service import MNSNotificationService from tests.unit.conftest import TEST_CURRENT_GP_ODS, TEST_NHS_NUMBER from tests.unit.handlers.test_mns_notification_handler import ( @@ -19,11 +20,25 @@ @pytest.fixture -def mns_service(mocker, set_env, monkeypatch): +def mns_service(mocker, set_env, monkeypatch, mock_upload_document_iteration_3_enabled): monkeypatch.setenv("PDS_FHIR_IS_STUBBED", "False") service = MNSNotificationService() mocker.patch.object(service, "pds_service") - mocker.patch.object(service, "document_service") + mocker.patch.object(service, "document_review_service") + mocker.patch.object(service, "lg_document_service") + mocker.patch.object(service, "sqs_service") + yield service + + +@pytest.fixture +def mns_service_feature_disabled( + mocker, set_env, monkeypatch, mock_upload_document_iteration_3_disabled +): + monkeypatch.setenv("PDS_FHIR_IS_STUBBED", "False") + service = MNSNotificationService() + mocker.patch.object(service, "pds_service") + mocker.patch.object(service, "document_review_service") + mocker.patch.object(service, "lg_document_service") mocker.patch.object(service, "sqs_service") yield service @@ -56,6 +71,37 @@ def mock_document_references(mocker): return docs +@pytest.fixture +def mock_document_review_references(mocker): + # Create a list of mock document review references + reviews = [] + for i in range(2): + review = MagicMock(spec=DocumentUploadReviewReference) + review.id = f"review-id-{i}" + review.nhs_number = TEST_NHS_NUMBER + review.custodian = TEST_CURRENT_GP_ODS + reviews.append(review) + return reviews + + +@pytest.fixture +def mock_upload_document_iteration_3_enabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_feature_flag = mock_function.return_value = { + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED: True + } + yield mock_feature_flag + + +@pytest.fixture +def mock_upload_document_iteration_3_disabled(mocker): + mock_function = mocker.patch.object(FeatureFlagService, "get_feature_flags_by_flag") + mock_feature_flag = mock_function.return_value = { + FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED: False + } + yield mock_feature_flag + + MOCK_UPDATE_TIME = "2024-01-01 12:00:00" NEW_ODS_CODE = "NEW123" @@ -107,246 +153,467 @@ def test_handle_mns_notification_error_handling_client_error(mns_service, mocker def test_handle_gp_change_notification_with_patient_documents( - mns_service, mock_document_references, mocker + mns_service, mock_document_references, mock_document_review_references, mocker ): - mocker.patch.object(mns_service, "get_patient_documents") - mns_service.get_patient_documents.return_value = mock_document_references + mocker.patch.object(mns_service, "get_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + mock_document_review_references, + ) mocker.patch.object(mns_service, "get_updated_gp_ods") mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE - mocker.patch.object(mns_service, "update_patient_ods_code") + mocker.patch.object(mns_service, "update_all_patient_documents") mns_service.handle_gp_change_notification(gp_change_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( gp_change_message.subject.nhs_number ) mns_service.get_updated_gp_ods.assert_called_once_with( gp_change_message.subject.nhs_number ) - mns_service.update_patient_ods_code.assert_called_once_with( - mock_document_references, NEW_ODS_CODE + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, mock_document_review_references, NEW_ODS_CODE ) def test_handle_gp_change_notification_no_patient_documents(mns_service, mocker): - mocker.patch.object(mns_service, "get_patient_documents") - mns_service.get_patient_documents.return_value = [] + mocker.patch.object(mns_service, "get_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ([], []) mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - - mns_service.get_patient_documents.return_value = [] + mocker.patch.object(mns_service, "update_all_patient_documents") mns_service.handle_gp_change_notification(gp_change_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( gp_change_message.subject.nhs_number ) mns_service.get_updated_gp_ods.assert_not_called() - mns_service.update_patient_ods_code.assert_not_called() + mns_service.update_all_patient_documents.assert_not_called() def test_handle_death_notification_informal(mns_service, mocker): - mocker.patch.object(mns_service, "get_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") + mocker.patch.object(mns_service, "update_all_patient_documents") mns_service.handle_death_notification(informal_death_notification_message) - mns_service.get_patient_documents.assert_not_called() + mns_service.get_all_patient_documents.assert_not_called() mns_service.get_updated_gp_ods.assert_not_called() - mns_service.update_patient_ods_code.assert_not_called() + mns_service.update_all_patient_documents.assert_not_called() def test_handle_death_notification_removed_with_documents( - mns_service, mock_document_references, mocker + mns_service, mock_document_references, mock_document_review_references, mocker ): - mocker.patch.object(mns_service, "get_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - mns_service.get_patient_documents.return_value = mock_document_references + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + mock_document_review_references, + ) mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE mns_service.handle_death_notification(removed_death_notification_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( removed_death_notification_message.subject.nhs_number ) mns_service.get_updated_gp_ods.assert_called_once_with( removed_death_notification_message.subject.nhs_number ) - mns_service.update_patient_ods_code.assert_called_once_with( - mock_document_references, NEW_ODS_CODE + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, mock_document_review_references, NEW_ODS_CODE ) def test_handle_death_notification_removed_no_documents(mns_service, mocker): - mocker.patch.object(mns_service, "get_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - mns_service.get_patient_documents.return_value = [] + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ([], []) mns_service.handle_death_notification(removed_death_notification_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( removed_death_notification_message.subject.nhs_number ) mns_service.get_updated_gp_ods.assert_not_called() - mns_service.update_patient_ods_code.assert_not_called() + mns_service.update_all_patient_documents.assert_not_called() def test_handle_death_notification_formal_with_documents( - mns_service, mock_document_references, mocker + mns_service, mock_document_references, mock_document_review_references, mocker ): - mocker.patch.object(mns_service, "get_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - mns_service.get_patient_documents.return_value = mock_document_references + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + mock_document_review_references, + ) mns_service.handle_death_notification(death_notification_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( death_notification_message.subject.nhs_number ) - mns_service.update_patient_ods_code.assert_called_once_with( - mock_document_references, PatientOdsInactiveStatus.DECEASED + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, + mock_document_review_references, + PatientOdsInactiveStatus.DECEASED, ) mns_service.get_updated_gp_ods.assert_not_called() def test_handle_death_notification_formal_no_documents(mns_service, mocker): - mocker.patch.object(mns_service, "get_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - mns_service.get_patient_documents.return_value = [] + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ([], []) mns_service.handle_death_notification(death_notification_message) - mns_service.get_patient_documents.assert_called_once_with( + mns_service.get_all_patient_documents.assert_called_once_with( death_notification_message.subject.nhs_number ) - mns_service.update_patient_ods_code.assert_not_called() + mns_service.update_all_patient_documents.assert_not_called() + + +def test_get_updated_gp_ods(mns_service): + expected_ods = NEW_ODS_CODE + patient_details_mock = MagicMock() + patient_details_mock.general_practice_ods = expected_ods + mns_service.pds_service.fetch_patient_details.return_value = patient_details_mock + result = mns_service.get_updated_gp_ods(TEST_NHS_NUMBER) -@freeze_time(MOCK_UPDATE_TIME) -def test_update_patient_ods_code_with_documents(mns_service, mock_document_references): - updated_ods_code = NEW_ODS_CODE + assert result == expected_ods + mns_service.pds_service.fetch_patient_details.assert_called_once_with( + TEST_NHS_NUMBER + ) - mns_service.update_patient_ods_code(mock_document_references, updated_ods_code) - for doc in mock_document_references: - assert doc.current_gp_ods == updated_ods_code - assert doc.custodian == updated_ods_code - assert doc.last_updated == int( - datetime.fromisoformat(MOCK_UPDATE_TIME).timestamp() - ) +def test_pds_is_called_death_notification_removed( + mns_service, mocker, mock_document_references, mock_document_review_references +): + mocker.patch.object(mns_service, "get_updated_gp_ods") + mocker.patch.object(mns_service, "update_all_patient_documents") + mocker.patch.object(mns_service, "get_all_patient_documents") + + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + mock_document_review_references, + ) + mns_service.handle_mns_notification(removed_death_notification_message) - mns_service.document_service.update_document.assert_any_call( - mns_service.table, - doc, - mns_service.DOCUMENT_UPDATE_FIELDS, - ) + mns_service.get_updated_gp_ods.assert_called() + mns_service.update_all_patient_documents.assert_called() + + +def test_get_all_patient_documents(mns_service, mocker): + expected_lg_docs = [MagicMock(spec=DocumentReference)] + expected_review_docs = [MagicMock(spec=DocumentUploadReviewReference)] + + mns_service.lg_document_service.fetch_documents_from_table_with_nhs_number.return_value = ( + expected_lg_docs + ) + mns_service.document_review_service.fetch_documents_from_table_with_nhs_number.return_value = ( + expected_review_docs + ) + + lg_docs, review_docs = mns_service.get_all_patient_documents(TEST_NHS_NUMBER) + + assert lg_docs == expected_lg_docs + assert review_docs == expected_review_docs + mns_service.lg_document_service.fetch_documents_from_table_with_nhs_number.assert_called_once_with( + TEST_NHS_NUMBER + ) + mns_service.document_review_service.fetch_documents_from_table_with_nhs_number.assert_called_once_with( + TEST_NHS_NUMBER + ) + + +def test_update_all_patient_documents_with_both_types( + mns_service, mock_document_references, mock_document_review_references, mocker +): + mns_service.update_all_patient_documents( + mock_document_references, mock_document_review_references, NEW_ODS_CODE + ) + + mns_service.lg_document_service.update_patient_ods_code.assert_called_once_with( + mock_document_references, NEW_ODS_CODE + ) + mns_service.document_review_service.update_document_review_custodian.assert_called_once_with( + mock_document_review_references, NEW_ODS_CODE + ) + + +def test_update_all_patient_documents_with_only_lg_documents( + mns_service, mock_document_references, mocker +): + mns_service.update_all_patient_documents(mock_document_references, [], NEW_ODS_CODE) + + mns_service.lg_document_service.update_patient_ods_code.assert_called_once_with( + mock_document_references, NEW_ODS_CODE + ) + mns_service.document_review_service.update_document_review_custodian.assert_not_called() + + +def test_update_all_patient_documents_with_only_review_documents( + mns_service, mock_document_review_references, mocker +): + mns_service.update_all_patient_documents( + [], mock_document_review_references, NEW_ODS_CODE + ) + + mns_service.lg_document_service.update_patient_ods_code.assert_not_called() + mns_service.document_review_service.update_document_review_custodian.assert_called_once_with( + mock_document_review_references, NEW_ODS_CODE + ) + + +def test_handle_gp_change_notification_with_only_lg_documents( + mns_service, mock_document_references, mocker +): + """Test GP change when only LG documents exist (no review documents)""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + [], + ) + mocker.patch.object(mns_service, "get_updated_gp_ods") + mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE + mocker.patch.object(mns_service, "update_all_patient_documents") + + mns_service.handle_gp_change_notification(gp_change_message) + + mns_service.get_all_patient_documents.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service.get_updated_gp_ods.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, [], NEW_ODS_CODE + ) -@freeze_time(MOCK_UPDATE_TIME) -def test_update_patient_ods_code_with_deceased_status( - mns_service, mock_document_references +def test_handle_gp_change_notification_with_only_review_documents( + mns_service, mock_document_review_references, mocker ): - mns_service.update_patient_ods_code( - mock_document_references, PatientOdsInactiveStatus.DECEASED + """Test GP change when only review documents exist (no LG documents)""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + [], + mock_document_review_references, ) + mocker.patch.object(mns_service, "get_updated_gp_ods") + mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE + mocker.patch.object(mns_service, "update_all_patient_documents") - for doc in mock_document_references: - assert doc.current_gp_ods == PatientOdsInactiveStatus.DECEASED - assert doc.custodian == mns_service.PCSE_ODS - assert doc.last_updated == int( - datetime.fromisoformat(MOCK_UPDATE_TIME).timestamp() - ) + mns_service.handle_gp_change_notification(gp_change_message) - mns_service.document_service.update_document.assert_any_call( - mns_service.table, - doc, - mns_service.DOCUMENT_UPDATE_FIELDS, - ) + mns_service.get_all_patient_documents.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service.get_updated_gp_ods.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + [], mock_document_review_references, NEW_ODS_CODE + ) -@freeze_time(MOCK_UPDATE_TIME) -def test_update_patient_ods_code_with_suspended_status( - mns_service, mock_document_references +def test_handle_death_notification_formal_with_only_lg_documents( + mns_service, mock_document_references, mocker ): - mns_service.update_patient_ods_code( - mock_document_references, PatientOdsInactiveStatus.SUSPENDED + """Test formal death notification when only LG documents exist""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mocker.patch.object(mns_service, "get_updated_gp_ods") + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + [], ) - for doc in mock_document_references: - assert doc.current_gp_ods == PatientOdsInactiveStatus.SUSPENDED - assert doc.custodian == mns_service.PCSE_ODS - assert doc.last_updated == int( - datetime.fromisoformat(MOCK_UPDATE_TIME).timestamp() - ) + mns_service.handle_death_notification(death_notification_message) - mns_service.document_service.update_document.assert_any_call( - mns_service.table, - doc, - mns_service.DOCUMENT_UPDATE_FIELDS, - ) + mns_service.get_all_patient_documents.assert_called_once_with( + death_notification_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, + [], + PatientOdsInactiveStatus.DECEASED, + ) + mns_service.get_updated_gp_ods.assert_not_called() -def test_update_patient_ods_code_no_documents(mns_service): - mns_service.update_patient_ods_code([], NEW_ODS_CODE) +def test_handle_death_notification_formal_with_only_review_documents( + mns_service, mock_document_review_references, mocker +): + """Test formal death notification when only review documents exist""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mocker.patch.object(mns_service, "get_updated_gp_ods") + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + [], + mock_document_review_references, + ) - mns_service.document_service.update_document.assert_not_called() + mns_service.handle_death_notification(death_notification_message) + mns_service.get_all_patient_documents.assert_called_once_with( + death_notification_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + [], + mock_document_review_references, + PatientOdsInactiveStatus.DECEASED, + ) + mns_service.get_updated_gp_ods.assert_not_called() -@freeze_time(MOCK_UPDATE_TIME) -def test_update_patient_ods_code_no_changes_needed( - mns_service, mock_document_references + +def test_handle_death_notification_removed_with_only_lg_documents( + mns_service, mock_document_references, mocker ): - for doc in mock_document_references: - doc.current_gp_ods = NEW_ODS_CODE - doc.custodian = NEW_ODS_CODE + """Test removed death notification when only LG documents exist""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mocker.patch.object(mns_service, "get_updated_gp_ods") + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + mock_document_references, + [], + ) + mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE - mns_service.update_patient_ods_code(mock_document_references, NEW_ODS_CODE) + mns_service.handle_death_notification(removed_death_notification_message) - mns_service.document_service.update_document.assert_not_called() + mns_service.get_all_patient_documents.assert_called_once_with( + removed_death_notification_message.subject.nhs_number + ) + mns_service.get_updated_gp_ods.assert_called_once_with( + removed_death_notification_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + mock_document_references, [], NEW_ODS_CODE + ) -def test_get_patient_documents(mns_service): - expected_documents = [MagicMock(spec=DocumentReference)] - mns_service.document_service.fetch_documents_from_table_with_nhs_number.return_value = ( - expected_documents +def test_handle_death_notification_removed_with_only_review_documents( + mns_service, mock_document_review_references, mocker +): + """Test removed death notification when only review documents exist""" + mocker.patch.object(mns_service, "get_all_patient_documents") + mocker.patch.object(mns_service, "get_updated_gp_ods") + mocker.patch.object(mns_service, "update_all_patient_documents") + mns_service.get_all_patient_documents.return_value = ( + [], + mock_document_review_references, ) + mns_service.get_updated_gp_ods.return_value = NEW_ODS_CODE - result = mns_service.get_patient_documents(TEST_NHS_NUMBER) + mns_service.handle_death_notification(removed_death_notification_message) - assert result == expected_documents - mns_service.document_service.fetch_documents_from_table_with_nhs_number.assert_called_once_with( - TEST_NHS_NUMBER, mns_service.table + mns_service.get_all_patient_documents.assert_called_once_with( + removed_death_notification_message.subject.nhs_number + ) + mns_service.get_updated_gp_ods.assert_called_once_with( + removed_death_notification_message.subject.nhs_number + ) + mns_service.update_all_patient_documents.assert_called_once_with( + [], mock_document_review_references, NEW_ODS_CODE ) -def test_get_updated_gp_ods(mns_service): - expected_ods = NEW_ODS_CODE - patient_details_mock = MagicMock() - patient_details_mock.general_practice_ods = expected_ods - mns_service.pds_service.fetch_patient_details.return_value = patient_details_mock +def test_get_all_patient_documents_when_feature_disabled( + mns_service_feature_disabled, mocker +): + """Test that review documents are not fetched when feature flag is disabled""" + expected_lg_docs = [MagicMock(spec=DocumentReference)] - result = mns_service.get_updated_gp_ods(TEST_NHS_NUMBER) + mns_service_feature_disabled.lg_document_service.fetch_documents_from_table_with_nhs_number.return_value = ( + expected_lg_docs + ) - assert result == expected_ods - mns_service.pds_service.fetch_patient_details.assert_called_once_with( + lg_docs, review_docs = mns_service_feature_disabled.get_all_patient_documents( TEST_NHS_NUMBER ) + assert lg_docs == expected_lg_docs + assert review_docs == [] + mns_service_feature_disabled.lg_document_service.fetch_documents_from_table_with_nhs_number.assert_called_once_with( + TEST_NHS_NUMBER + ) + mns_service_feature_disabled.document_review_service.fetch_documents_from_table_with_nhs_number.assert_not_called() -def test_pds_is_called_death_notification_removed( - mns_service, mocker, mock_document_references + +def test_update_all_patient_documents_when_feature_disabled( + mns_service_feature_disabled, + mock_document_references, + mock_document_review_references, + mocker, ): - mocker.patch.object(mns_service, "get_updated_gp_ods") - mocker.patch.object(mns_service, "update_patient_ods_code") - mocker.patch.object(mns_service, "get_patient_documents") + """Test that review documents are not updated when feature flag is disabled""" + mns_service_feature_disabled.update_all_patient_documents( + mock_document_references, mock_document_review_references, NEW_ODS_CODE + ) - mns_service.get_patient_documents.return_value = mock_document_references - mns_service.handle_mns_notification(removed_death_notification_message) + mns_service_feature_disabled.lg_document_service.update_patient_ods_code.assert_called_once_with( + mock_document_references, NEW_ODS_CODE + ) + mns_service_feature_disabled.document_review_service.update_document_review_custodian.assert_not_called() - mns_service.get_updated_gp_ods.assert_called() - mns_service.update_patient_ods_code.assert_called() + +def test_handle_gp_change_notification_when_feature_disabled( + mns_service_feature_disabled, mock_document_references, mocker +): + """Test GP change notification handling when feature flag is disabled""" + mocker.patch.object(mns_service_feature_disabled, "get_all_patient_documents") + mns_service_feature_disabled.get_all_patient_documents.return_value = ( + mock_document_references, + [], + ) + mocker.patch.object(mns_service_feature_disabled, "get_updated_gp_ods") + mns_service_feature_disabled.get_updated_gp_ods.return_value = NEW_ODS_CODE + mocker.patch.object(mns_service_feature_disabled, "update_all_patient_documents") + + mns_service_feature_disabled.handle_gp_change_notification(gp_change_message) + + mns_service_feature_disabled.get_all_patient_documents.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service_feature_disabled.get_updated_gp_ods.assert_called_once_with( + gp_change_message.subject.nhs_number + ) + mns_service_feature_disabled.update_all_patient_documents.assert_called_once_with( + mock_document_references, [], NEW_ODS_CODE + ) + + +def test_handle_death_notification_formal_when_feature_disabled( + mns_service_feature_disabled, mock_document_references, mocker +): + """Test formal death notification when feature flag is disabled""" + mocker.patch.object(mns_service_feature_disabled, "get_all_patient_documents") + mocker.patch.object(mns_service_feature_disabled, "get_updated_gp_ods") + mocker.patch.object(mns_service_feature_disabled, "update_all_patient_documents") + mns_service_feature_disabled.get_all_patient_documents.return_value = ( + mock_document_references, + [], + ) + + mns_service_feature_disabled.handle_death_notification(death_notification_message) + + mns_service_feature_disabled.get_all_patient_documents.assert_called_once_with( + death_notification_message.subject.nhs_number + ) + mns_service_feature_disabled.update_all_patient_documents.assert_called_once_with( + mock_document_references, + [], + PatientOdsInactiveStatus.DECEASED, + ) + mns_service_feature_disabled.get_updated_gp_ods.assert_not_called() diff --git a/lambdas/tests/unit/services/test_upload_document_reference_service.py b/lambdas/tests/unit/services/test_upload_document_reference_service.py index 0be754b9c..ffecdf6d9 100644 --- a/lambdas/tests/unit/services/test_upload_document_reference_service.py +++ b/lambdas/tests/unit/services/test_upload_document_reference_service.py @@ -170,7 +170,7 @@ def test_fetch_preliminary_document_reference_success(service, mock_document_ref assert result == mock_document_reference service.document_service.fetch_documents_from_table.assert_called_once_with( - table=MOCK_LG_TABLE_NAME, + table_name=MOCK_LG_TABLE_NAME, search_condition=document_key, search_key="ID", query_filter=PreliminaryStatus, @@ -399,7 +399,7 @@ def test_update_dynamo_table_clean_scan_result(service, mock_document_reference) service.document_service.update_document.assert_called_once_with( table_name=MOCK_LG_TABLE_NAME, - document_reference=mock_document_reference, + document=mock_document_reference, update_fields_name={ "virus_scanner_result", "doc_status", @@ -461,13 +461,13 @@ def test_document_key_extraction_from_object_key_for_lg( # Check first call (preliminary document) first_call = service.document_service.fetch_documents_from_table.call_args_list[0] - assert first_call[1]["table"] == MOCK_LG_TABLE_NAME + assert first_call[1]["table_name"] == MOCK_LG_TABLE_NAME assert first_call[1]["search_condition"] == expected_document_key assert first_call[1]["search_key"] == "ID" # Check second call (existing final documents) second_call = service.document_service.fetch_documents_from_table.call_args_list[1] - assert second_call[1]["table"] == MOCK_LG_TABLE_NAME + assert second_call[1]["table_name"] == MOCK_LG_TABLE_NAME assert second_call[1]["index_name"] == "S3FileKeyIndex" assert second_call[1]["search_condition"] == mock_document_reference.s3_file_key assert second_call[1]["search_key"] == "S3FileKey" @@ -476,7 +476,7 @@ def test_document_key_extraction_from_object_key_for_lg( def test_finalize_and_supersede_with_transaction_with_existing_finals( service, mock_document_reference, mocker ): - """Test transaction-based finalization with existing final documents to supersede""" + """Test transaction-based finalisation with existing final documents to supersede""" new_doc = mock_document_reference new_doc.id = "new-doc-id" new_doc.nhs_number = "9000000001" @@ -508,7 +508,7 @@ def test_finalize_and_supersede_with_transaction_with_existing_finals( service._finalize_and_supersede_with_transaction(new_doc) service.document_service.fetch_documents_from_table.assert_called_once_with( - table=MOCK_LG_TABLE_NAME, + table_name=MOCK_LG_TABLE_NAME, index_name="S3FileKeyIndex", search_condition=new_doc.s3_file_key, search_key="S3FileKey",