resolve wider issues

Author: Akol125

lambdas/filenameprocessor/src/file_validation.py

@@ -37,7 +37,7 @@ def is_valid_datetime(timestamp: str) -> bool:
     return True
 
 
-def validate_extended_attributes_file_key(file_key: str) -> str:
+def validate_extended_attributes_file_key(file_key: str) -> tuple[str, str]:
     """
     Checks that all elements of the file key are valid, raises an exception otherwise.
     Returns a string containing the organization code.

lambdas/filenameprocessor/src/supplier_permissions.py

@@ -22,7 +22,7 @@ def validate_vaccine_type_permissions(vaccine_type: str, supplier: str) -> list:
     return supplier_permissions
 
 
-def validate_permissions_for_extended_attributes_files(vaccine_type: str, supplier: str) -> tuple[str]:
+def validate_permissions_for_extended_attributes_files(vaccine_type: str, supplier: str) -> str:
     """
     Checks that the supplier has COVID vaccine type and its CUD permissions.
     Raises an exception if the supplier does not have at least one permission for the vaccine type.

lambdas/filenameprocessor/tests/test_file_key_validation.py

@@ -135,7 +135,7 @@ def test_validate_extended_attributes_file_key(self, mock_get_redis_client):
                 mock_redis.hget.side_effect = create_mock_hget(MOCK_ODS_CODE_TO_SUPPLIER, {})
                 mock_redis.hkeys.return_value = ["COVID"]
                 mock_get_redis_client.return_value = mock_redis
-                vaccine_type, supplier = validate_extended_attributes_file_key(file_key)
+                _, supplier = validate_extended_attributes_file_key(file_key)
                 self.assertEqual(
                     supplier,
                     expected_result,

lambdas/filenameprocessor/tests/test_lambda_handler.py

@@ -427,37 +427,40 @@ def raise_connection_error(*args, **kwargs):
     @patch("elasticache.get_redis_client")
     def test_lambda_handler_extended_attributes_invalid_timestamp(self, mock_get_redis_client):
         """
-        Invalid timestamps (too short or non-parseable) should fail validation and move to archive.
+        Invalid timestamps (too short or non-parseable) should fail validation
+        and move to extended-attributes-archive/.
         """
-        # Valid Redis
+        # Set up valid Redis responses
         mock_redis = fakeredis.FakeStrictRedis()
         mock_redis.hget = Mock(side_effect=create_mock_hget({"X8E5B": "RAVS"}, {}))
         mock_redis.hkeys = Mock(return_value=["COVID", *all_vaccine_types_in_this_test_file])
         mock_get_redis_client.return_value = mock_redis
 
-        # Case 1: too short timestamp
-        invalid_timestamp_key = "Vaccination_Extended_Attributes_v1_5_X8E5B_20000101T0000.csv"
-        s3_client.put_object(
-            Bucket=BucketNames.SOURCE, Key=invalid_timestamp_key, Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT
-        )
-        with patch("file_name_processor.uuid4", return_value="invalid_timestamp_id"):
-            lambda_handler(self.make_event([self.make_record(invalid_timestamp_key)]), None)
-        # Failed audit and archive
-        item1 = self.get_audit_table_items()[0]
-        self.assertEqual(item1[AuditTableKeys.STATUS]["S"], "Failed")
-        s3_client.get_object(Bucket=BucketNames.SOURCE, Key=f"extended-attributes-archive/{invalid_timestamp_key}")
-
-        # Case 2: non-parseable timestamp
-        invalid_timestamp_key2 = "Vaccination_Extended_Attributes_v1_5_X8E5B_20XX0101T00000001.csv"
-        s3_client.put_object(
-            Bucket=BucketNames.SOURCE, Key=invalid_timestamp_key2, Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT
-        )
-        with patch("file_name_processor.uuid4", return_value="invalid_timestamp_id2"):
-            lambda_handler(self.make_event([self.make_record(invalid_timestamp_key2)]), None)
-        # Failed audit and archive
-        item2 = self.get_audit_table_items()[-1]
-        self.assertEqual(item2[AuditTableKeys.STATUS]["S"], "Failed")
-        s3_client.get_object(Bucket=BucketNames.SOURCE, Key=f"extended-attributes-archive/{invalid_timestamp_key2}")
+        invalid_cases = [
+            ("Vaccination_Extended_Attributes_v1_5_X8E5B_20000101T0000.csv", "invalid_timestamp_id"),
+            ("Vaccination_Extended_Attributes_v1_5_X8E5B_20XX0101T00000001.csv", "invalid_timestamp_id2"),
+        ]
+
+        for file_key, fake_msg_id in invalid_cases:
+            with self.subTest(f"Invalid timestamp test for: {file_key}"):
+                # Upload the invalid file
+                s3_client.put_object(
+                    Bucket=BucketNames.SOURCE,
+                    Key=file_key,
+                    Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT,
+                )
+
+                with patch("file_name_processor.uuid4", return_value=fake_msg_id):
+                    lambda_handler(self.make_event([self.make_record(file_key)]), None)
+
+                # Validate audit entry
+                audit_items = self.get_audit_table_items()
+                last_item = audit_items[-1]  # always get the most recent write
+                self.assertEqual(last_item[AuditTableKeys.STATUS]["S"], "Failed")
+
+                archived_key = f"extended-attributes-archive/{file_key}"
+                retrieved = s3_client.get_object(Bucket=BucketNames.SOURCE, Key=archived_key)
+                self.assertIsNotNone(retrieved)
 
     @patch("elasticache.get_redis_client")
     def test_lambda_handler_extended_attributes_extension_checks(self, mock_get_redis_client):
@@ -569,16 +572,11 @@ def test_lambda_handler_extended_attributes_extension_checks(self, mock_get_redi
             patch("file_name_processor.uuid4", return_value=test_cases[0].message_id),
             patch(
                 "file_name_processor.copy_file_to_external_bucket",
-                side_effect=lambda src_bucket, key, dst_bucket, dst_key, exp_owner, exp_src_owner: (
-                    # effectively do nothing
-                    None,
-                ),
+                side_effect=lambda src_bucket, key, dst_bucket, dst_key, exp_owner, exp_src_owner: (None,),
             ),
         ):
             lambda_handler(self.make_event([self.make_record(invalid_file_key)]), None)
 
-        # Assert audit table entry captured with Failed and queue_name set to the identifier.
-        # Assert that the ClientError message is an InvalidFileKeyError.
         table_items = self.get_audit_table_items()
         # Removed brittle assertion on total audit count; subsequent checks below verify the expected audit content
         item = table_items[-1]
@@ -594,7 +592,6 @@ def test_lambda_handler_extended_attributes_extension_checks(self, mock_get_redi
         self.assertEqual(item[AuditTableKeys.EXPIRES_AT]["N"], str(test_cases[0].expires_at))
         # File should be moved to source under archive/
         dest_key = f"extended-attributes-archive/{invalid_file_key}"
-        print(f" destination file is at {s3_client.list_objects(Bucket=BucketNames.SOURCE)}")
         retrieved = s3_client.get_object(Bucket=BucketNames.SOURCE, Key=dest_key)
         self.assertIsNotNone(retrieved)
 

lambdas/filenameprocessor/tests/test_supplier_permissions.py

@@ -91,17 +91,14 @@ def test_validate_permissions_for_extended_attributes_files_fail_no_covid(self):
 
     def test_validate_permissions_for_extended_attributes_files_fail_partial_ops(self):
         """Supplier with only partial COVID permissions (e.g., C only) should raise error as CUD required."""
-        # Note: Implementation checks only the first matching COVID entry's operation string.
-        # Therefore, entries like COVID.CRUD (which includes C, U, D letters) will pass.
-        # The following cases should fail because the first COVID entry lacks at least one of C/U/D.
         partial_permission_cases = [
             ["COVID.C"],
             ["COVID.U"],
             ["COVID.D"],
             ["COVID.CU"],
             ["COVID.UD"],
             ["COVID.CD"],
-            ["COVID.S"],  # status only
+            ["COVID.S"],
         ]
 
         for permissions in partial_permission_cases:
@@ -119,15 +116,13 @@ def test_validate_permissions_for_extended_attributes_files_fail_partial_ops(sel
 
     def test_validate_permissions_for_extended_attributes_files_multiple_entries(self):
         """Multiple COVID permission entries should pass only if the first matching COVID entry contains CUD."""
-        # Case: First entry has CUDS -> success
         with patch(
             "supplier_permissions.get_supplier_permissions_from_cache",
             return_value=["COVID.CUDS", "COVID.C"],
         ):
             result = validate_permissions_for_extended_attributes_files("COVID", "RAVS")
             self.assertEqual(result, "RAVS_COVID")
 
-        # Case: First entry lacks CUD (even if later one has CUDS) -> fail
         with patch(
             "supplier_permissions.get_supplier_permissions_from_cache",
             return_value=["COVID.C", "COVID.CUDS"],