diff --git a/app/src/types/blocks/documentReview.ts b/app/src/types/blocks/documentReview.ts index 8e808f295c..7dd64cbf6c 100644 --- a/app/src/types/blocks/documentReview.ts +++ b/app/src/types/blocks/documentReview.ts @@ -21,15 +21,15 @@ export type DocumentReviewStatusDto = { }; export enum DocumentReviewStatus { - PENDING_REVIEW = 'PENDING_REVIEW', - APPROVED = 'APPROVED', - APPROVED_PENDING_DOCUMENTS = 'APPROVED_PENDING_DOCUMENTS', - REJECTED = 'REJECTED', - AWAITING_DOCUMENTS = 'AWAITING_DOCUMENTS', - REJECTED_DUPLICATE = 'REJECTED_DUPLICATE', - REASSIGNED = 'REASSIGNED', - REASSIGNED_PATIENT_UNKNOWN = 'REASSIGNED_PATIENT_UNKNOWN', - NEVER_REVIEWED = 'NEVER_REVIEWED', - REVIEW_PENDING_UPLOAD = 'REVIEW_PENDING_UPLOAD', - VIRUS_SCAN_FAILED = 'VIRUS_SCAN_FAILED', + PENDING_REVIEW = "PENDING_REVIEW", + APPROVED = "APPROVED", + REVIEW_IN_PROGRESS = "REVIEW_IN_PROGRESS", + REJECTED = "REJECTED", + AWAITING_DOCUMENTS = "AWAITING_DOCUMENTS", + REJECTED_DUPLICATE = "REJECTED_DUPLICATE", + REASSIGNED = "REASSIGNED", + REASSIGNED_PATIENT_UNKNOWN = "REASSIGNED_PATIENT_UNKNOWN", + NEVER_REVIEWED = "NEVER_REVIEWED", + REVIEW_PENDING_UPLOAD = "REVIEW_PENDING_UPLOAD", + VIRUS_SCAN_FAILED = "VIRUS_SCAN_FAILED", } diff --git a/lambdas/enums/document_review_status.py b/lambdas/enums/document_review_status.py index 462df49d0d..266e182719 100644 --- a/lambdas/enums/document_review_status.py +++ b/lambdas/enums/document_review_status.py @@ -4,7 +4,7 @@ class DocumentReviewStatus(StrEnum): PENDING_REVIEW = "PENDING_REVIEW" APPROVED = "APPROVED" - APPROVED_PENDING_DOCUMENTS = "APPROVED_PENDING_DOCUMENTS" + REVIEW_IN_PROGRESS = "REVIEW_IN_PROGRESS" REJECTED = "REJECTED" REJECTED_DUPLICATE = "REJECTED_DUPLICATE" REASSIGNED = "REASSIGNED" diff --git a/lambdas/services/document_upload_review_service.py b/lambdas/services/document_upload_review_service.py index 1ec5b04992..deca152d8a 100644 --- a/lambdas/services/document_upload_review_service.py +++ b/lambdas/services/document_upload_review_service.py @@ -154,15 +154,6 @@ def update_pending_review_status( 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, diff --git a/lambdas/services/update_document_review_service.py b/lambdas/services/update_document_review_service.py index a4a7f09c95..90dc2a29d0 100644 --- a/lambdas/services/update_document_review_service.py +++ b/lambdas/services/update_document_review_service.py @@ -35,8 +35,6 @@ class UpdateDocumentReviewService: DocumentReviewStatus.APPROVED, ] - APPROVED_PENDING_REVIEW_STATUSES = [DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS] - UNKNOWN_NHS_NUMBER = "0000000000" FAILED_LOG_MESSAGE = "Failed to update document review" @@ -112,7 +110,6 @@ def _validate_patient_id_match(self, document, patient_id: str): 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}. " @@ -133,17 +130,6 @@ def _validate_user_match_custodian(self, document, reviewer_ods_code: str): 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, @@ -151,7 +137,6 @@ def _process_review_status_update( 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) @@ -169,11 +154,9 @@ def _execute_status_action(self, 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) + self._handle_rejection_or_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) + self._handle_rejection_or_approval(document, update_fields) else: logger.error( f"Invalid status update attempted: {document.review_status}", @@ -183,23 +166,12 @@ def _execute_status_action(self, document, update_data, document_id): 400, LambdaError.DocumentReviewGeneralError ) - def _handle_rejection(self, document, update_fields): + def _handle_rejection_or_approval(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}" 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 02ce6bb17c..47aa704a7d 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -251,7 +251,7 @@ def test_update_document_review_for_patient_conditional_check_failed( field_names = {"review_status"} with pytest.raises(DocumentReviewException): - mock_service.update_approved_pending_review_status( + mock_service.update_pending_review_status( review_update=mock_review_update, field_names=field_names, ) @@ -262,7 +262,7 @@ def test_update_document_review_for_patient_conditional_check_failed( expected_condition = ( Attr(DocumentReferenceMetadataFields.ID.value).exists() & Attr("NhsNumber").eq(mock_review_update.nhs_number) - & Attr("ReviewStatus").eq(DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS) + & Attr("ReviewStatus").eq(DocumentReviewStatus.PENDING_REVIEW) ) assert condition_expr == expected_condition @@ -287,7 +287,7 @@ def test_update_document_review_for_patient_other_client_error( field_names = {"review_status"} with pytest.raises(DocumentReviewException): - mock_service.update_approved_pending_review_status( + mock_service.update_pending_review_status( review_update=mock_review_update, field_names=field_names, ) diff --git a/lambdas/tests/unit/services/test_update_document_review_service.py b/lambdas/tests/unit/services/test_update_document_review_service.py index ecc35962aa..3181cce04c 100644 --- a/lambdas/tests/unit/services/test_update_document_review_service.py +++ b/lambdas/tests/unit/services/test_update_document_review_service.py @@ -88,7 +88,7 @@ def mock_pds_service(mocker): def test_update_document_review_successfully_approves_document( mock_service, mock_document_review ): - mock_document_review.review_status = DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS + mock_document_review.review_status = DocumentReviewStatus.PENDING_REVIEW update_data = PatchDocumentReviewRequest( review_status=DocumentReviewStatus.APPROVED, document_reference_id=TEST_DOCUMENT_REFERENCE_ID, @@ -108,10 +108,10 @@ def test_update_document_review_successfully_approves_document( 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() + mock_service.document_review_service.update_pending_review_status.assert_called_once() call_args = ( - mock_service.document_review_service.update_approved_pending_review_status.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 @@ -251,38 +251,6 @@ def test_update_document_review_successfully_reassigns_document_when_patient_unk 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, @@ -307,31 +275,6 @@ def test_update_document_review_raises_exception_when_document_not_found(mock_se 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 ): @@ -382,7 +325,6 @@ def test_validate_patient_id_match_raises_exception_when_nhs_numbers_dont_match( "valid_status", [ DocumentReviewStatus.PENDING_REVIEW, - DocumentReviewStatus.APPROVED_PENDING_DOCUMENTS, ], ) def test_validate_review_status_passes_when_status_is_valid( @@ -524,31 +466,11 @@ def test_create_reassigned_document_preserves_document_id( 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 + mock_document_review.review_status = DocumentReviewStatus.PENDING_REVIEW update_data = PatchDocumentReviewRequest( review_status=DocumentReviewStatus.APPROVED, document_reference_id=TEST_DOCUMENT_REFERENCE_ID, @@ -558,9 +480,9 @@ def test_process_review_status_update_calls_approved_service_for_approval( 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() + mock_service.document_review_service.update_pending_review_status.assert_called_once() call_args = ( - mock_service.document_review_service.update_approved_pending_review_status.call_args + mock_service.document_review_service.update_pending_review_status.call_args ) assert ( call_args.kwargs["review_update"].document_reference_id @@ -623,30 +545,12 @@ def test_process_review_status_update_calls_handle_reassignment_for_reassignment ) -@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 + mock_document_review.review_status = DocumentReviewStatus.PENDING_REVIEW update_data = PatchDocumentReviewRequest( review_status=DocumentReviewStatus.APPROVED, document_reference_id=TEST_DOCUMENT_REFERENCE_ID, @@ -725,11 +629,10 @@ def test_handle_reassignment_status_creates_new_document_and_updates_with_transa DocumentReviewStatus.REASSIGNED_PATIENT_UNKNOWN, ], ) -def test_update_document_review_raises_exception_when_updating_approved_pending_to_invalid_status( +def test_update_document_review_raises_exception_when_updating_approved_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 + mock_document_review.review_status = DocumentReviewStatus.APPROVED update_data = PatchDocumentReviewRequest( review_status=invalid_target_status, nhs_number=TEST_REASSIGNED_NHS_NUMBER if "REASSIGNED" in invalid_target_status.value else None, @@ -764,7 +667,6 @@ def test_update_document_review_raises_exception_when_updating_approved_pending_ 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,