Skip to content

Commit c4108fc

Browse files
committed
[PRMP-1621] Enhance invalid filename handling to check file existence before sending to review
1 parent 0c26273 commit c4108fc

File tree

2 files changed

+58
-3
lines changed

2 files changed

+58
-3
lines changed

lambdas/services/bulk_upload_metadata_processor_service.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,10 @@ def handle_invalid_filename(
366366
ods_code: str,
367367
failed_files: dict[tuple[str, str], list[BulkUploadQueueMetadata]],
368368
) -> None:
369-
"""Handle invalid filenames by logging, storing failure in Dynamo, and tracking for review."""
369+
"""Handle invalid filenames by logging, storing failure in Dynamo, and tracking for review.
370+
Files that do not exist on the staging bucket are marked as failed only —
371+
they are never added to the review queue since they cannot be reviewed.
372+
"""
370373
logger.error(
371374
f"Failed to process {file_metadata.file_path} due to error: {error}",
372375
)
@@ -375,14 +378,21 @@ def handle_invalid_filename(
375378
file_metadata.file_path,
376379
)
377380
failed_file.file_path = self.add_directory_path_to_file_path(file_metadata)
378-
failed_files[(nhs_number, ods_code)].append(failed_file)
381+
382+
file_exists = self.s3_repo.file_exists_on_staging_bucket(failed_file.file_path)
383+
if not file_exists:
384+
logger.info(
385+
f"File {failed_file.file_path} not found on staging bucket. Will not send to review.",
386+
)
387+
else:
388+
failed_files[(nhs_number, ods_code)].append(failed_file)
379389

380390
failed_entry = StagingSqsMetadata(nhs_number=nhs_number, files=[failed_file])
381391
self.dynamo_repository.write_report_upload_to_dynamo(
382392
failed_entry,
383393
UploadStatus.FAILED,
384394
str(error),
385-
sent_to_review=self.send_to_review_enabled,
395+
sent_to_review=self.send_to_review_enabled and file_exists,
386396
)
387397

388398
def send_failed_files_to_review_queue(

lambdas/tests/unit/services/test_bulk_upload_metadata_processor_service.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,8 @@ def test_handle_invalid_filename_writes_failed_entry_to_dynamo(
585585
failed_files = defaultdict(list)
586586
error = InvalidFileNameException("Invalid filename format")
587587

588+
test_service.s3_repo.file_exists_on_staging_bucket.return_value = True
589+
588590
mock_staging_metadata = mocker.patch(
589591
"services.bulk_upload_metadata_processor_service.StagingSqsMetadata",
590592
)
@@ -636,6 +638,9 @@ def test_handle_invalid_filename_sets_sent_to_review_true_when_review_enabled(
636638
failed_files = defaultdict(list)
637639
error = InvalidFileNameException("Invalid filename format")
638640

641+
test_service_with_review_enabled.s3_repo.file_exists_on_staging_bucket.return_value = (
642+
True
643+
)
639644
mock_write = mocker.patch.object(
640645
test_service_with_review_enabled.dynamo_repository,
641646
"write_report_upload_to_dynamo",
@@ -656,6 +661,43 @@ def test_handle_invalid_filename_sets_sent_to_review_true_when_review_enabled(
656661
str(error),
657662
sent_to_review=True,
658663
)
664+
assert (nhs_number, ods_code) in failed_files
665+
666+
667+
def test_handle_invalid_filename_does_not_send_to_review_when_file_missing(
668+
mocker,
669+
test_service_with_review_enabled,
670+
base_metadata_file,
671+
):
672+
nhs_number = "1234567890"
673+
ods_code = "Y12345"
674+
failed_files = defaultdict(list)
675+
error = InvalidFileNameException("Invalid filename format")
676+
677+
test_service_with_review_enabled.s3_repo.file_exists_on_staging_bucket.return_value = (
678+
False
679+
)
680+
mock_write = mocker.patch.object(
681+
test_service_with_review_enabled.dynamo_repository,
682+
"write_report_upload_to_dynamo",
683+
)
684+
mocker.patch("services.bulk_upload_metadata_processor_service.StagingSqsMetadata")
685+
686+
test_service_with_review_enabled.handle_invalid_filename(
687+
base_metadata_file,
688+
error,
689+
nhs_number,
690+
ods_code,
691+
failed_files,
692+
)
693+
694+
mock_write.assert_called_once_with(
695+
mocker.ANY,
696+
UploadStatus.FAILED,
697+
str(error),
698+
sent_to_review=False,
699+
)
700+
assert (nhs_number, ods_code) not in failed_files
659701

660702

661703
def test_csv_to_sqs_metadata_sends_failed_files_to_review_queue_when_enabled(
@@ -676,6 +718,9 @@ def test_csv_to_sqs_metadata_sends_failed_files_to_review_queue_when_enabled(
676718
"validate_and_correct_filename",
677719
side_effect=InvalidFileNameException("invalid"),
678720
)
721+
test_service_with_review_enabled.s3_repo.file_exists_on_staging_bucket.return_value = (
722+
True
723+
)
679724

680725
result = test_service_with_review_enabled.csv_to_sqs_metadata(MOCK_METADATA_CSV)
681726

0 commit comments

Comments
 (0)