From e53e6877ed353020a1d6586270c01c314ad8274e Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Wed, 18 Mar 2026 15:23:51 +0000 Subject: [PATCH 01/10] attempt 1 to sort --- .../DocumentSearchResultsPage.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index b98092188..4620773a1 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -62,12 +62,18 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { baseHeaders, }); + const sortedResults = results.sort((a, b) => { + const dateA = new Date(a.created).getTime(); + const dateB = new Date(b.created).getTime(); + return dateB - dateA; // descending order + }); + if (config.featureFlags.uploadDocumentIteration3Enabled) { const { count } = await getReviews(baseUrl, baseHeaders, nhsNumber, '', 1); hasReviews.current = count > 0; } - setSearchResults(results ?? []); + setSearchResults(sortedResults ?? []); setSubmissionState(SUBMISSION_STATE.SUCCEEDED); } catch (e) { const error = e as AxiosError; From 50a507f57e6eeca55827cf6f55f9f7aef6686e78 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Wed, 18 Mar 2026 15:24:53 +0000 Subject: [PATCH 02/10] place test logs --- .../documentSearchResultsPage/DocumentSearchResultsPage.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index 4620773a1..8f0ae1006 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -62,12 +62,16 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { baseHeaders, }); + console.log(results); + const sortedResults = results.sort((a, b) => { const dateA = new Date(a.created).getTime(); const dateB = new Date(b.created).getTime(); return dateB - dateA; // descending order }); + console.log(sortedResults); + if (config.featureFlags.uploadDocumentIteration3Enabled) { const { count } = await getReviews(baseUrl, baseHeaders, nhsNumber, '', 1); hasReviews.current = count > 0; From 548eeb96e850a970282275d88c356696aebebb80 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Wed, 18 Mar 2026 16:25:44 +0000 Subject: [PATCH 03/10] attempt 2 --- .../DocumentSearchResultsPage.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index 8f0ae1006..c30a140c6 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -64,11 +64,9 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { console.log(results); - const sortedResults = results.sort((a, b) => { - const dateA = new Date(a.created).getTime(); - const dateB = new Date(b.created).getTime(); - return dateB - dateA; // descending order - }); + const sortedResults = [...results].sort( + (a, b) => Date.parse(b.created) - Date.parse(a.created), + ); console.log(sortedResults); From e9cfab58c40cb7a32280f24106330fdce69ea385 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 13:11:10 +0000 Subject: [PATCH 04/10] scan nhsNumberIndex forward for search --- lambdas/services/base/dynamo_service.py | 4 ++++ lambdas/services/document_reference_search_service.py | 1 + lambdas/services/document_service.py | 2 ++ .../unit/services/test_document_reference_search_service.py | 2 ++ .../unit/services/test_document_upload_review_service.py | 1 + 5 files changed, 10 insertions(+) diff --git a/lambdas/services/base/dynamo_service.py b/lambdas/services/base/dynamo_service.py index 8186b0867..b8ec4e8d8 100644 --- a/lambdas/services/base/dynamo_service.py +++ b/lambdas/services/base/dynamo_service.py @@ -509,6 +509,7 @@ def query_table_with_paginator( limit: int = 20, page_size: int = 1, start_key: str | None = None, + scan_index_forward: bool | None = False, ) -> dict: try: @@ -528,6 +529,9 @@ def query_table_with_paginator( "PaginationConfig": pagination_config, } + if scan_index_forward: + query_params["ScanIndexForward"] = scan_index_forward + if expression_attribute_values is None: expression_attribute_values = {} diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 2fe77fdbc..1e9ba2c68 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -281,6 +281,7 @@ def get_paginated_references_by_nhs_number( filter_expression=filter_expression, expression_attribute_names=condition_attribute_names, expression_attribute_values=condition_attribute_values, + scan_index_forward=True, ) logger.info("Validating upload status") diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 32a2927d1..aae421929 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -368,6 +368,7 @@ def query_table_with_paginator( index_name: str, search_key: str, search_condition: str, + scan_index_forward: bool | None = False, table_name: str | None = None, filter_expression: str | None = None, expression_attribute_names: dict | None = None, @@ -393,6 +394,7 @@ def query_table_with_paginator( limit=limit, page_size=page_size, start_key=start_key, + scan_index_forward=scan_index_forward, ) references = [ diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index b9d751d89..efc860c4f 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -526,6 +526,7 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( filter_expression="#Deleted_attr = :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)", expression_attribute_names={"#Deleted_attr": "Deleted"}, expression_attribute_values={":Deleted_condition_val": ""}, + scan_index_forward=True, ) assert actual == expected @@ -561,6 +562,7 @@ def test_get_paginated_references_by_nhs_number_handles_filters(mock_document_se ":Deleted_condition_val": "", ":DocStatus_condition_val": "final", }, + scan_index_forward=True, ) 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 549d8a919..2f9c5712a 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -663,6 +663,7 @@ def test_query_docs_pending_review_with_paginator(mock_service): limit=50, start_key=None, page_size=1, + scan_index_forward=False, ) assert actual == expected From d82593a364225f45e2b5ec09fb4b944def8ef100 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 13:12:42 +0000 Subject: [PATCH 05/10] comment out ui sort --- .../DocumentSearchResultsPage.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index c30a140c6..985b2653d 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -62,20 +62,20 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { baseHeaders, }); - console.log(results); + // console.log(results); - const sortedResults = [...results].sort( - (a, b) => Date.parse(b.created) - Date.parse(a.created), - ); + // const sortedResults = [...results].sort( + // (a, b) => Date.parse(b.created) - Date.parse(a.created), + // ); - console.log(sortedResults); + // console.log(sortedResults); if (config.featureFlags.uploadDocumentIteration3Enabled) { const { count } = await getReviews(baseUrl, baseHeaders, nhsNumber, '', 1); hasReviews.current = count > 0; } - setSearchResults(sortedResults ?? []); + setSearchResults(results ?? []); setSubmissionState(SUBMISSION_STATE.SUCCEEDED); } catch (e) { const error = e as AxiosError; From ebe5fd6ac8c757ca6ebfc524576aa8315bb5b32c Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 13:54:11 +0000 Subject: [PATCH 06/10] remove ui sort and fix scanIndexForward --- .../DocumentSearchResultsPage.tsx | 8 -------- lambdas/services/base/dynamo_service.py | 4 ++-- lambdas/services/document_reference_search_service.py | 2 +- lambdas/services/document_service.py | 4 ++-- .../services/test_document_reference_search_service.py | 4 ++-- .../unit/services/test_document_upload_review_service.py | 2 +- 6 files changed, 8 insertions(+), 16 deletions(-) diff --git a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx index 985b2653d..b98092188 100644 --- a/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx +++ b/app/src/pages/documentSearchResultsPage/DocumentSearchResultsPage.tsx @@ -62,14 +62,6 @@ const DocumentSearchResultsPage = (): React.JSX.Element => { baseHeaders, }); - // console.log(results); - - // const sortedResults = [...results].sort( - // (a, b) => Date.parse(b.created) - Date.parse(a.created), - // ); - - // console.log(sortedResults); - if (config.featureFlags.uploadDocumentIteration3Enabled) { const { count } = await getReviews(baseUrl, baseHeaders, nhsNumber, '', 1); hasReviews.current = count > 0; diff --git a/lambdas/services/base/dynamo_service.py b/lambdas/services/base/dynamo_service.py index b8ec4e8d8..30ab49953 100644 --- a/lambdas/services/base/dynamo_service.py +++ b/lambdas/services/base/dynamo_service.py @@ -509,7 +509,7 @@ def query_table_with_paginator( limit: int = 20, page_size: int = 1, start_key: str | None = None, - scan_index_forward: bool | None = False, + scan_index_forward: bool | None = None, ) -> dict: try: @@ -529,7 +529,7 @@ def query_table_with_paginator( "PaginationConfig": pagination_config, } - if scan_index_forward: + if scan_index_forward is not None: query_params["ScanIndexForward"] = scan_index_forward if expression_attribute_values is None: diff --git a/lambdas/services/document_reference_search_service.py b/lambdas/services/document_reference_search_service.py index 1e9ba2c68..6a01f28c5 100644 --- a/lambdas/services/document_reference_search_service.py +++ b/lambdas/services/document_reference_search_service.py @@ -281,7 +281,7 @@ def get_paginated_references_by_nhs_number( filter_expression=filter_expression, expression_attribute_names=condition_attribute_names, expression_attribute_values=condition_attribute_values, - scan_index_forward=True, + scan_index_forward=False, ) logger.info("Validating upload status") diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index aae421929..2e39fcac5 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -368,7 +368,7 @@ def query_table_with_paginator( index_name: str, search_key: str, search_condition: str, - scan_index_forward: bool | None = False, + scan_index_forward: bool | None = None, table_name: str | None = None, filter_expression: str | None = None, expression_attribute_names: dict | None = None, @@ -394,7 +394,7 @@ def query_table_with_paginator( limit=limit, page_size=page_size, start_key=start_key, - scan_index_forward=scan_index_forward, + scan_index_forward=scan_index_forward if scan_index_forward is not None else None, ) references = [ diff --git a/lambdas/tests/unit/services/test_document_reference_search_service.py b/lambdas/tests/unit/services/test_document_reference_search_service.py index efc860c4f..ca5254312 100644 --- a/lambdas/tests/unit/services/test_document_reference_search_service.py +++ b/lambdas/tests/unit/services/test_document_reference_search_service.py @@ -526,7 +526,7 @@ def test_get_paginated_references_by_nhs_number_returns_references_and_token( filter_expression="#Deleted_attr = :Deleted_condition_val OR attribute_not_exists(#Deleted_attr)", expression_attribute_names={"#Deleted_attr": "Deleted"}, expression_attribute_values={":Deleted_condition_val": ""}, - scan_index_forward=True, + scan_index_forward=False, ) assert actual == expected @@ -562,7 +562,7 @@ def test_get_paginated_references_by_nhs_number_handles_filters(mock_document_se ":Deleted_condition_val": "", ":DocStatus_condition_val": "final", }, - scan_index_forward=True, + scan_index_forward=False, ) 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 2f9c5712a..28d11b5bd 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -663,7 +663,7 @@ def test_query_docs_pending_review_with_paginator(mock_service): limit=50, start_key=None, page_size=1, - scan_index_forward=False, + scan_index_forward=None, ) assert actual == expected From 1b9553dd4f584cd2c81f1787e52b1a918b0619b1 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 14:03:00 +0000 Subject: [PATCH 07/10] format --- lambdas/services/document_service.py | 4 +- .../test_document_upload_review_service.py | 150 ++++++++++++------ 2 files changed, 106 insertions(+), 48 deletions(-) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 2e39fcac5..593282649 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -394,7 +394,9 @@ def query_table_with_paginator( limit=limit, page_size=page_size, start_key=start_key, - scan_index_forward=scan_index_forward if scan_index_forward is not None else None, + scan_index_forward=( + scan_index_forward if scan_index_forward is not None else None + ), ) references = [ 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 28d11b5bd..1353db453 100644 --- a/lambdas/tests/unit/services/test_document_upload_review_service.py +++ b/lambdas/tests/unit/services/test_document_upload_review_service.py @@ -3,9 +3,10 @@ import pytest from boto3.dynamodb.conditions import Attr from botocore.exceptions import ClientError +from freezegun import freeze_time + from enums.document_review_status import DocumentReviewStatus from enums.metadata_field_names import DocumentReferenceMetadataFields -from freezegun import freeze_time from models.document_review import DocumentUploadReviewReference from services.document_upload_review_service import DocumentUploadReviewService from tests.unit.conftest import ( @@ -74,14 +75,18 @@ def test_s3_bucket(mock_service, monkeypatch): def test_update_document_review_custodian_updates_all_documents( - mock_service, mock_document_review_references, mocker + mock_service, + mock_document_review_references, + mocker, ): mock_handle_standard = mocker.patch.object( - mock_service, "_handle_standard_custodian_update" + mock_service, + "_handle_standard_custodian_update", ) mock_service.update_document_review_custodian( - mock_document_review_references, NEW_ODS_CODE + mock_document_review_references, + NEW_ODS_CODE, ) assert mock_handle_standard.call_count == 3 @@ -92,10 +97,12 @@ def test_update_document_review_custodian_updates_all_documents( def test_update_document_review_custodian_empty_list(mock_service, mocker): mock_handle_standard = mocker.patch.object( - mock_service, "_handle_standard_custodian_update" + mock_service, + "_handle_standard_custodian_update", ) mock_handle_pending = mocker.patch.object( - mock_service, "_handle_pending_review_custodian_update" + mock_service, + "_handle_pending_review_custodian_update", ) mock_service.update_document_review_custodian([], NEW_ODS_CODE) @@ -105,20 +112,25 @@ def test_update_document_review_custodian_empty_list(mock_service, mocker): def test_update_document_review_custodian_no_changes_needed( - mock_service, mock_document_review_references, mocker + mock_service, + mock_document_review_references, + mocker, ): mock_handle_standard = mocker.patch.object( - mock_service, "_handle_standard_custodian_update" + mock_service, + "_handle_standard_custodian_update", ) mock_handle_pending = mocker.patch.object( - mock_service, "_handle_pending_review_custodian_update" + mock_service, + "_handle_pending_review_custodian_update", ) 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 + mock_document_review_references, + NEW_ODS_CODE, ) mock_handle_standard.assert_not_called() @@ -126,36 +138,45 @@ def test_update_document_review_custodian_no_changes_needed( def test_update_document_review_custodian_mixed_custodians( - mock_service, mock_document_review_references, mocker + mock_service, + mock_document_review_references, + mocker, ): mock_handle_standard = mocker.patch.object( - mock_service, "_handle_standard_custodian_update" + mock_service, + "_handle_standard_custodian_update", ) mock_document_review_references[0].custodian = NEW_ODS_CODE mock_service.update_document_review_custodian( - mock_document_review_references, NEW_ODS_CODE + mock_document_review_references, + NEW_ODS_CODE, ) assert mock_handle_standard.call_count == 2 def test_update_document_review_custodian_continues_on_error( - mock_service, mock_document_review_references, mocker + mock_service, + mock_document_review_references, + mocker, ): mock_handle_standard = mocker.patch.object( - mock_service, "_handle_standard_custodian_update" + mock_service, + "_handle_standard_custodian_update", ) mock_handle_standard.side_effect = [ DocumentReviewException("Test error"), ClientError( - {"Error": {"Code": "ConditionalCheckFailedException"}}, "UpdateItem" + {"Error": {"Code": "ConditionalCheckFailedException"}}, + "UpdateItem", ), None, ] mock_service.update_document_review_custodian( - mock_document_review_references, NEW_ODS_CODE + mock_document_review_references, + NEW_ODS_CODE, ) assert mock_handle_standard.call_count == 3 @@ -182,7 +203,9 @@ def test_handle_standard_custodian_update_updates_document(mock_service, mocker) def test_get_document_review_by_id( - mock_service, mock_document_review_references, mocker + mock_service, + mock_document_review_references, + mocker, ): mock_get_item = mocker.patch.object(mock_service, "get_item") mock_get_item.side_effect = mock_document_review_references @@ -194,7 +217,9 @@ def test_get_document_review_by_id( def test_update_document_review_for_patient_success( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): mock_update_document = mocker.patch.object(mock_service, "update_document") mock_update_document.return_value = {"Attributes": {"ID": "test-review-id"}} @@ -220,7 +245,9 @@ def test_update_document_review_for_patient_success( def test_update_document_review_for_patient_builds_correct_condition_expression( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): mock_update_document = mocker.patch.object(mock_service, "update_document") @@ -250,10 +277,13 @@ def test_update_document_review_for_patient_builds_correct_condition_expression( def test_update_document_review_for_patient_conditional_check_failed( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): client_error = ClientError( - {"Error": {"Code": "ConditionalCheckFailedException"}}, "UpdateItem" + {"Error": {"Code": "ConditionalCheckFailedException"}}, + "UpdateItem", ) mock_update_document = mocker.patch.object(mock_service, "update_document") mock_update_document.side_effect = client_error @@ -285,7 +315,9 @@ def test_update_document_review_for_patient_conditional_check_failed( def test_update_document_review_for_patient_other_client_error( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): client_error = ClientError( {"Error": {"Code": "ResourceNotFoundException", "Message": "Table not found"}}, @@ -304,14 +336,17 @@ def test_update_document_review_for_patient_other_client_error( def test_update_document_review_with_transaction_builds_correct_items( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): """Test that transaction items are built correctly for new and existing reviews.""" mock_transact_write = mocker.patch.object( - mock_service.dynamo_service, "transact_write_items" + mock_service.dynamo_service, + "transact_write_items", ) mock_transact_item_builder = mocker.patch( - "services.document_upload_review_service.build_transaction_item" + "services.document_upload_review_service.build_transaction_item", ) new_review = MagicMock(spec=DocumentUploadReviewReference) new_review.id = "new-review-id" @@ -323,7 +358,9 @@ def test_update_document_review_with_transaction_builds_correct_items( mock_service.update_document_review_with_transaction(new_review, existing_review) new_review.model_dump.assert_called_with( - exclude_none=True, by_alias=True, exclude={"version", "id"} + exclude_none=True, + by_alias=True, + exclude={"version", "id"}, ) existing_review.model_dump.assert_called_with( @@ -339,7 +376,7 @@ def test_update_document_review_with_transaction_builds_correct_items( first_call = mock_transact_item_builder.call_args_list[0] assert first_call.kwargs["conditions"] == [ - {"field": "ID", "operator": "attribute_not_exists"} + {"field": "ID", "operator": "attribute_not_exists"}, ] # Verify second call (existing review) has proper conditions second_call = mock_transact_item_builder.call_args_list[1] @@ -404,13 +441,17 @@ def test_delete_document_review_files_handles_s3_error(mock_service, mocker): def test_update_document_review_with_transaction_transaction_cancelled( - mock_service, mock_review_update, mocker + mock_service, + mock_review_update, + mocker, ): client_error = ClientError( - {"Error": {"Code": "TransactionCanceledException"}}, "TransactWriteItems" + {"Error": {"Code": "TransactionCanceledException"}}, + "TransactWriteItems", ) mock_transact_write = mocker.patch.object( - mock_service.dynamo_service, "transact_write_items" + mock_service.dynamo_service, + "transact_write_items", ) mock_transact_write.side_effect = client_error @@ -423,14 +464,16 @@ def test_update_document_review_with_transaction_transaction_cancelled( with pytest.raises(DocumentReviewException): mock_service.update_document_review_with_transaction( - new_review, existing_review + new_review, + existing_review, ) def test_handle_standard_custodian_update_with_client_error(mock_service, mocker): mock_update_document = mocker.patch.object(mock_service, "update_document") mock_update_document.side_effect = ClientError( - {"Error": {"Code": "ConditionalCheckFailedException"}}, "UpdateItem" + {"Error": {"Code": "ConditionalCheckFailedException"}}, + "UpdateItem", ) review = DocumentUploadReviewReference.model_construct() @@ -440,16 +483,20 @@ def test_handle_standard_custodian_update_with_client_error(mock_service, mocker with pytest.raises(ClientError): mock_service._handle_standard_custodian_update( - review, NEW_ODS_CODE, {"custodian"} + review, + NEW_ODS_CODE, + {"custodian"}, ) @freeze_time("2024-01-15 10:30:00") def test_handle_pending_review_custodian_update_creates_new_version( - mock_service, mocker + mock_service, + mocker, ): mock_transaction_update = mocker.patch.object( - mock_service, "update_document_review_with_transaction" + mock_service, + "update_document_review_with_transaction", ) expected_timestamp = 1705314600 @@ -465,7 +512,9 @@ def test_handle_pending_review_custodian_update_creates_new_version( new_review_copy.custodian = NEW_ODS_CODE mock_service._handle_pending_review_custodian_update( - review, NEW_ODS_CODE, {"custodian"} + review, + NEW_ODS_CODE, + {"custodian"}, ) assert review.review_status == DocumentReviewStatus.NEVER_REVIEWED @@ -481,10 +530,12 @@ def test_handle_pending_review_custodian_update_creates_new_version( def test_handle_pending_review_custodian_update_with_transaction_failure( - mock_service, mocker + mock_service, + mocker, ): mock_transaction_update = mocker.patch.object( - mock_service, "update_document_review_with_transaction" + mock_service, + "update_document_review_with_transaction", ) mock_transaction_update.side_effect = DocumentReviewException("Transaction failed") @@ -496,13 +547,15 @@ def test_handle_pending_review_custodian_update_with_transaction_failure( with pytest.raises(DocumentReviewException): mock_service._handle_pending_review_custodian_update( - review, NEW_ODS_CODE, {"custodian"} + review, + NEW_ODS_CODE, + {"custodian"}, ) def test_build_review_dynamo_filter_creates_filter_from_nhs_number(mock_service): expected = Attr("ReviewStatus").eq("PENDING_REVIEW") & Attr("NhsNumber").eq( - TEST_NHS_NUMBER + TEST_NHS_NUMBER, ) actual = mock_service.build_review_dynamo_filter(nhs_number=TEST_NHS_NUMBER) @@ -511,7 +564,7 @@ def test_build_review_dynamo_filter_creates_filter_from_nhs_number(mock_service) def test_build_review_dynamo_filter_creates_filter_from_uploader_ods_code(mock_service): expected = Attr("ReviewStatus").eq("PENDING_REVIEW") & Attr("Author").eq( - TEST_ODS_CODE + TEST_ODS_CODE, ) actual = mock_service.build_review_dynamo_filter(uploader=TEST_ODS_CODE) @@ -525,7 +578,8 @@ def test_build_filter_handles_both_nhs_number_and_uploader(mock_service): & Attr("Author").eq(TEST_ODS_CODE) ) actual = mock_service.build_review_dynamo_filter( - nhs_number=TEST_NHS_NUMBER, uploader=TEST_ODS_CODE + nhs_number=TEST_NHS_NUMBER, + uploader=TEST_ODS_CODE, ) assert actual == expected @@ -533,10 +587,10 @@ def test_build_filter_handles_both_nhs_number_and_uploader(mock_service): def test_get_document_returns_review_document(mock_service): mock_service.dynamo_service.get_item.return_value = { - "Item": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0] + "Item": MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0], } expected = DocumentUploadReviewReference.model_validate( - MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0] + MOCK_DOCUMENT_REVIEW_SEARCH_RESPONSE["Items"][0], ) actual = mock_service.get_document(TEST_UUID, 1) @@ -559,7 +613,8 @@ def test_get_document_returns_none_no_documents_found(mock_service): def test_get_document_by_id_raises_exception_client_error(mock_service): mock_service.dynamo_service.get_item.side_effect = ClientError( - {"Error": {"Code": "500", "Message": "mocked error"}}, "get_item" + {"Error": {"Code": "500", "Message": "mocked error"}}, + "get_item", ) with pytest.raises(DocumentReviewException): @@ -626,7 +681,8 @@ def test_get_document_by_id_raises_exception_client_error(mock_service): ) def test_build_paginator_query_filter(mock_service, nhs_number, uploader, expected): actual = mock_service.build_paginator_query_filter( - nhs_number=nhs_number, uploader=uploader + nhs_number=nhs_number, + uploader=uploader, ) assert actual == expected From 7bf9c0adcd301534de9c6c10a2cca27342fb7c85 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 14:19:34 +0000 Subject: [PATCH 08/10] increase coverage --- lambdas/tests/unit/services/base/test_dynamo_service.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lambdas/tests/unit/services/base/test_dynamo_service.py b/lambdas/tests/unit/services/base/test_dynamo_service.py index e10adaae6..d4779fb27 100755 --- a/lambdas/tests/unit/services/base/test_dynamo_service.py +++ b/lambdas/tests/unit/services/base/test_dynamo_service.py @@ -1521,6 +1521,7 @@ def test_query_table_using_paginator(mock_service): index_name="NhsNumberIndex", key="NhsNumber", condition=TEST_NHS_NUMBER, + scan_index_forward=False, ) mock_paginator.paginate.assert_called_with( @@ -1529,6 +1530,7 @@ def test_query_table_using_paginator(mock_service): KeyConditionExpression="NhsNumber=:i", ExpressionAttributeValues={":i": {"S": TEST_NHS_NUMBER}}, PaginationConfig={"MaxItems": 20, "PageSize": 1}, + ScanIndexForward=False, ) assert actual == expected From 42899a152e4b2827303cd2b5835d91d000803ff6 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 14:25:33 +0000 Subject: [PATCH 09/10] format --- lambdas/tests/unit/services/base/test_dynamo_service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lambdas/tests/unit/services/base/test_dynamo_service.py b/lambdas/tests/unit/services/base/test_dynamo_service.py index d4779fb27..e66723578 100755 --- a/lambdas/tests/unit/services/base/test_dynamo_service.py +++ b/lambdas/tests/unit/services/base/test_dynamo_service.py @@ -4,6 +4,7 @@ import pytest from boto3.dynamodb.conditions import And, Attr, Equals, Key from botocore.exceptions import ClientError + from enums.dynamo_filter import AttributeOperator from enums.metadata_field_names import DocumentReferenceMetadataFields from services.base.dynamo_service import DynamoDBService From ddde106c45ef5452e83e2082f2ba3e926f9d3083 Mon Sep 17 00:00:00 2001 From: Kamen Bachvarov Date: Thu, 19 Mar 2026 14:55:17 +0000 Subject: [PATCH 10/10] minor change of scan_index_forward param passing --- lambdas/services/document_service.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lambdas/services/document_service.py b/lambdas/services/document_service.py index 593282649..f7ed68af6 100644 --- a/lambdas/services/document_service.py +++ b/lambdas/services/document_service.py @@ -394,9 +394,7 @@ def query_table_with_paginator( limit=limit, page_size=page_size, start_key=start_key, - scan_index_forward=( - scan_index_forward if scan_index_forward is not None else None - ), + scan_index_forward=scan_index_forward, ) references = [