Skip to content

Commit

Permalink
[PRMP-844] renamed bulk upload FailureReason to Reason (#448)
Browse files Browse the repository at this point in the history
  • Loading branch information
AndyFlintNHS authored Oct 21, 2024
1 parent 4fcc50b commit 22d9b68
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 67 deletions.
2 changes: 1 addition & 1 deletion lambdas/enums/metadata_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
class MetadataReport(StrEnum):
NhsNumber = "NhsNumber"
UploadStatus = "UploadStatus"
FailureReason = "FailureReason"
Reason = "Reason"
PdsOdsCode = "PdsOdsCode"
UploaderOdsCode = "UploaderOdsCode"
FilePath = "FilePath"
Expand Down
4 changes: 1 addition & 3 deletions lambdas/models/bulk_upload_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ class BulkUploadReport(BaseModel):
file_path: str = Field(alias=MetadataReport.FilePath)
pds_ods_code: str = Field(alias=MetadataReport.PdsOdsCode)
uploader_ods_code: str = Field(alias=MetadataReport.UploaderOdsCode)
failure_reason: Optional[str] = Field(
default="", alias=MetadataReport.FailureReason
)
reason: Optional[str] = Field(default="", alias=MetadataReport.Reason)


def date_string_yyyymmdd(time_now: datetime) -> str:
Expand Down
14 changes: 7 additions & 7 deletions lambdas/models/bulk_upload_report_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def process_successful_report_item(self, item: BulkUploadReport):
if item.pds_ods_code == PatientOdsInactiveStatus.SUSPENDED:
self.total_suspended.add((item.nhs_number, item.date))
elif item.pds_ods_code == PatientOdsInactiveStatus.DECEASED:
self.total_deceased.add((item.nhs_number, item.date, item.failure_reason))
self.total_deceased.add((item.nhs_number, item.date, item.reason))
elif item.pds_ods_code == PatientOdsInactiveStatus.RESTRICTED:
self.total_restricted.add((item.nhs_number, item.date))
elif (
Expand All @@ -95,15 +95,15 @@ def process_failed_report_item(self, item: BulkUploadReport):
< item.timestamp
)

if (item.failure_reason and is_new_failure) or is_timestamp_newer:
if (item.reason and is_new_failure) or is_timestamp_newer:
self.failures_per_patient.update(
{
item.nhs_number: item.model_dump(
include={
underscore(str(MetadataReport.Date)),
underscore(str(MetadataReport.Timestamp)),
underscore(str(MetadataReport.UploaderOdsCode)),
underscore(str(MetadataReport.FailureReason)),
underscore(str(MetadataReport.Reason)),
},
by_alias=True,
)
Expand All @@ -120,13 +120,13 @@ def set_unique_failures(self):
self.failures_per_patient.pop(patient)

for patient_data in self.failures_per_patient.values():
reason = patient_data.get(MetadataReport.FailureReason)
reason = patient_data.get(MetadataReport.Reason)
self.unique_failures[reason] = self.unique_failures.get(reason, 0) + 1

def get_unsuccessful_reasons_data_rows(self):
return [
[MetadataReport.FailureReason, failure_reason, count]
for failure_reason, count in self.unique_failures.items()
[MetadataReport.Reason, reason, count]
for reason, count in self.unique_failures.items()
]


Expand All @@ -153,7 +153,7 @@ def populate_report(self):
for reason, count in report.unique_failures.items():
self.reason_summary.append(
[
f"{MetadataReport.FailureReason} for {report.uploader_ods_code}",
f"{MetadataReport.Reason} for {report.uploader_ods_code}",
reason,
count,
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def write_report_upload_to_dynamo(
self,
staging_metadata: StagingMetadata,
upload_status: UploadStatus,
failure_reason: str = None,
reason: str = None,
pds_ods_code: str = "",
):
nhs_number = staging_metadata.nhs_number
Expand All @@ -41,7 +41,7 @@ def write_report_upload_to_dynamo(
dynamo_record = BulkUploadReport(
upload_status=upload_status,
nhs_number=nhs_number,
failure_reason=failure_reason,
reason=reason,
file_path=file.file_path,
pds_ods_code=pds_ods_code,
uploader_ods_code=file.gp_practice_code,
Expand Down
6 changes: 3 additions & 3 deletions lambdas/services/bulk_upload_report_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def generate_deceased_report(self, ods_reports: list[OdsReport]):
MetadataReport.NhsNumber,
MetadataReport.UploaderOdsCode,
MetadataReport.Date,
MetadataReport.FailureReason,
MetadataReport.Reason,
]
data_rows = []
for report in ods_reports:
Expand Down Expand Up @@ -268,7 +268,7 @@ def generate_rejected_report(self, ods_reports: list[OdsReport]):
MetadataReport.NhsNumber,
MetadataReport.UploaderOdsCode,
MetadataReport.Date,
MetadataReport.FailureReason,
MetadataReport.Reason,
]

data_rows = []
Expand All @@ -279,7 +279,7 @@ def generate_rejected_report(self, ods_reports: list[OdsReport]):
nhs_number,
report_item[MetadataReport.UploaderOdsCode],
report_item[MetadataReport.Date],
report_item[MetadataReport.FailureReason],
report_item[MetadataReport.Reason],
]
)

Expand Down
4 changes: 2 additions & 2 deletions lambdas/services/bulk_upload_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ def handle_sqs_message(self, message: dict):
logger.error(error)
logger.info("Will stop processing Lloyd George record for this patient.")

failure_reason = str(error)
reason = str(error)
self.dynamo_repository.write_report_upload_to_dynamo(
staging_metadata, UploadStatus.FAILED, failure_reason, patient_ods_code
staging_metadata, UploadStatus.FAILED, reason, patient_ods_code
)
return

Expand Down
22 changes: 11 additions & 11 deletions lambdas/tests/unit/helpers/data/bulk_upload/dynamo_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"NhsNumber": "9000000001",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Patient is deceased - INFORMAL",
"Reason": "Patient is deceased - INFORMAL",
"UploadStatus": "complete",
"ID": TEST_UUID,
"PdsOdsCode": "DECE",
Expand Down Expand Up @@ -54,7 +54,7 @@
"NhsNumber": "9000000003",
"FilePath": "/0000000000/1of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Could not find the given patient on PDS",
"Reason": "Could not find the given patient on PDS",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_1,
Expand Down Expand Up @@ -85,7 +85,7 @@
"NhsNumber": "9000000005",
"FilePath": "/0000000000/1of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Could not find the given patient on PDS",
"Reason": "Could not find the given patient on PDS",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_1,
Expand All @@ -96,7 +96,7 @@
"NhsNumber": "9000000006",
"FilePath": "/0000000000/1of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Could not find the given patient on PDS",
"Reason": "Could not find the given patient on PDS",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_1,
Expand All @@ -107,7 +107,7 @@
"NhsNumber": "9000000006",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Lloyd George file already exists",
"Reason": "Lloyd George file already exists",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_1,
Expand All @@ -118,7 +118,7 @@
"NhsNumber": "9000000007",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_1,
"FailureReason": "Lloyd George file already exists",
"Reason": "Lloyd George file already exists",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_1,
Expand All @@ -142,7 +142,7 @@
"NhsNumber": "9000000010",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_2,
"FailureReason": "Patient is deceased - FORMAL",
"Reason": "Patient is deceased - FORMAL",
"UploadStatus": "complete",
"ID": TEST_UUID,
"PdsOdsCode": "DECE",
Expand Down Expand Up @@ -193,7 +193,7 @@
"NhsNumber": "9000000014",
"FilePath": "/0000000000/1of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_2,
"FailureReason": "Could not find the given patient on PDS",
"Reason": "Could not find the given patient on PDS",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_2,
Expand All @@ -204,7 +204,7 @@
"NhsNumber": "9000000015",
"FilePath": "/0000000000/1of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_2,
"FailureReason": "Could not find the given patient on PDS",
"Reason": "Could not find the given patient on PDS",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_2,
Expand All @@ -215,7 +215,7 @@
"NhsNumber": "9000000015",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_2,
"FailureReason": "Lloyd George file already exists",
"Reason": "Lloyd George file already exists",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_2,
Expand All @@ -226,7 +226,7 @@
"NhsNumber": "9000000016",
"FilePath": "/0000000000/2of2_Lloyd_George_Record_[NAME_2]_[0000000000]_[DOB].pdf",
"UploaderOdsCode": TEST_UPLOADER_ODS_2,
"FailureReason": "Lloyd George file already exists",
"Reason": "Lloyd George file already exists",
"UploadStatus": "failed",
"ID": TEST_UUID,
"PdsOdsCode": TEST_UPLOADER_ODS_2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ Type,Description,Count
Total,Total Successful,1
Total,Successful - Registered Elsewhere,0
Total,Suspended,0
FailureReason,File name not matching Lloyd George naming convention,1
Reason,File name not matching Lloyd George naming convention,1
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Total,Successful - Deceased,2
Total,Successful - Restricted,2
Success by ODS,Y12345,5
Success by ODS,Z12345,5
FailureReason for Y12345,Could not find the given patient on PDS,2
FailureReason for Y12345,Lloyd George file already exists,1
FailureReason for Z12345,Could not find the given patient on PDS,2
FailureReason for Z12345,Lloyd George file already exists,1
Reason for Y12345,Could not find the given patient on PDS,2
Reason for Y12345,Lloyd George file already exists,1
Reason for Z12345,Could not find the given patient on PDS,2
Reason for Z12345,Lloyd George file already exists,1
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
NhsNumber,UploaderOdsCode,Date,FailureReason
NhsNumber,UploaderOdsCode,Date,Reason
9000000001,Y12345,2012-01-13,Patient is deceased - INFORMAL
9000000010,Z12345,2012-01-13,Patient is deceased - FORMAL
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ Type,Description,Count
Total,Total Successful,5
Total,Successful - Registered Elsewhere,1
Total,Successful - Suspended,1
FailureReason,Could not find the given patient on PDS,2
FailureReason,Lloyd George file already exists,1
Reason,Could not find the given patient on PDS,2
Reason,Lloyd George file already exists,1
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ Type,Description,Count
Total,Total Successful,5
Total,Successful - Registered Elsewhere,1
Total,Successful - Suspended,1
FailureReason,Could not find the given patient on PDS,2
FailureReason,Lloyd George file already exists,1
Reason,Could not find the given patient on PDS,2
Reason,Lloyd George file already exists,1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
NhsNumber,UploaderOdsCode,Date,FailureReason
NhsNumber,UploaderOdsCode,Date,Reason
9000000005,Y12345,2012-01-13,Could not find the given patient on PDS
9000000006,Y12345,2012-01-13,Could not find the given patient on PDS
9000000007,Y12345,2012-01-13,Lloyd George file already exists
Expand Down
12 changes: 6 additions & 6 deletions lambdas/tests/unit/models/test_bulk_upload_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
"UploaderOdsCode": "Y12345",
}

MOCK_FAILURE_REASON = "File name not matching Lloyd George naming convention"
MOCK_REASON = "File name not matching Lloyd George naming convention"
MOCK_DATA_FAILED_UPLOAD = {
"ID": TEST_UUID,
"NhsNumber": "9000000025",
"Timestamp": 1698661500,
"Date": "2023-10-30",
"UploadStatus": "failed",
"FailureReason": MOCK_FAILURE_REASON,
"Reason": MOCK_REASON,
"FilePath": "/9000000025/invalid_filename.pdf",
"PdsOdsCode": "",
"UploaderOdsCode": "Y12345",
Expand All @@ -30,7 +30,7 @@

def test_create_successful_upload():
expected = MOCK_DATA_COMPLETE_UPLOAD
expected.update({"FailureReason": ""})
expected.update({"Reason": ""})

actual = BulkUploadReport(
ID=TEST_UUID,
Expand All @@ -54,7 +54,7 @@ def test_create_failed_upload():
timestamp=1698661500,
date="2023-10-30",
upload_status=UploadStatus.FAILED,
failure_reason=MOCK_FAILURE_REASON,
reason=MOCK_REASON,
file_path="/9000000025/invalid_filename.pdf",
pds_ods_code="",
uploader_ods_code="Y12345",
Expand All @@ -66,7 +66,7 @@ def test_create_failed_upload():
@freeze_time("2023-10-30 10:25:00")
def test_successful_upload_ids_and_timestamp_are_auto_populated_if_not_given(mock_uuid):
expected = MOCK_DATA_COMPLETE_UPLOAD
expected.update({"FailureReason": ""})
expected.update({"Reason": ""})

actual = BulkUploadReport(
nhs_number="9000000009",
Expand All @@ -85,7 +85,7 @@ def test_failed_upload_ids_and_timestamp_are_auto_populated_if_not_given(mock_uu
actual = BulkUploadReport(
nhs_number="9000000025",
file_path="/9000000025/invalid_filename.pdf",
failure_reason=MOCK_FAILURE_REASON,
reason=MOCK_REASON,
pds_ods_code="",
uploader_ods_code="Y12345",
upload_status=UploadStatus.FAILED,
Expand Down
Loading

0 comments on commit 22d9b68

Please sign in to comment.