PR feedback

Author: landonshumway-ia

backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py

@@ -1065,7 +1065,7 @@ def _get_privilege_update_records_directly(
 
         return [PrivilegeUpdateData.from_database_record(item) for item in response_items]
 
-    @logger_inject_kwargs(logger, 'compact', 'provider_id', 'detail', 'jurisdiction', 'license_type')
+    @logger_inject_kwargs(logger, 'compact', 'provider_id', 'detail', 'jurisdiction', 'license_type_abbr')
     def get_privilege_data(
         self,
         *,

backend/compact-connect/lambdas/python/disaster-recovery/handlers/rollback_license_upload.py

@@ -181,7 +181,7 @@ def from_dict(cls, data: dict) -> 'RollbackResults':
                         RevertedLicense(
                             jurisdiction=reverted_license['jurisdiction'],
                             license_type=reverted_license['licenseType'],
-                            revision_id=uuid4(),
+                            revision_id=reverted_license['revisionId'],
                             action=reverted_license['action'],
                         )
                         for reverted_license in summary.get('licensesReverted', [])
@@ -190,7 +190,7 @@ def from_dict(cls, data: dict) -> 'RollbackResults':
                         RevertedPrivilege(
                             jurisdiction=reverted_privilege['jurisdiction'],
                             license_type=reverted_privilege['licenseType'],
-                            revision_id=uuid4(),
+                            revision_id=reverted_privilege['revisionId'],
                             action=reverted_privilege['action'],
                         )
                         for reverted_privilege in summary.get('privilegesReverted', [])

backend/compact-connect/lambdas/python/disaster-recovery/tests/function/test_rollback_license_upload.py

@@ -1088,6 +1088,14 @@ def test_rollback_handles_pagination_when_provider_id_present_in_event_input(sel
         self.assertEqual(0, result_first['providersFailed'])
         self.assertEqual(mock_second_provider_id, result_first['continueFromProviderId'])
 
+        # Verify: S3 results contain first provider with revision id
+        s3_key = f'licenseUploadRollbacks/{MOCK_EXECUTION_NAME}/results.json'
+        s3_obj = self.config.s3_client.get_object(Bucket=self.config.disaster_recovery_results_bucket_name, Key=s3_key)
+        first_results_data = json.loads(s3_obj['Body'].read().decode('utf-8'))
+
+        # grab the revision id from the results which we will use when asserting on the final object
+        revision_id = first_results_data['revertedProviderSummaries'][0]['licensesReverted'][0]['revisionId']
+
         # Execute: Second invocation (continue from where we left off)
         # Reset mock time for second invocation
         mock_time.time.side_effect = [0, 1]  # Won't timeout this time
@@ -1117,7 +1125,7 @@ def test_rollback_handles_pagination_when_provider_id_present_in_event_input(sel
                                 'action': 'REVERT',
                                 'jurisdiction': 'oh',
                                 'licenseType': 'speech-language pathologist',
-                                'revisionId': ANY,
+                                'revisionId': revision_id,
                             }
                         ],
                         'privilegesReverted': [],
@@ -1132,6 +1140,7 @@ def test_rollback_handles_pagination_when_provider_id_present_in_event_input(sel
                                 'action': 'REVERT',
                                 'jurisdiction': 'oh',
                                 'licenseType': 'speech-language pathologist',
+                                # unknown random UUID, we won't check for it here
                                 'revisionId': ANY,
                             }
                         ],

backend/compact-connect/lambdas/python/migration/migrate_update_sort_keys/main.py

@@ -33,9 +33,10 @@ def on_delete(self, _properties: dict) -> CustomResourceResponse | None:
 def do_migration(_properties: dict) -> None:
     """
     This migration performs the following:
-    - Scans the provider table for all privilege update records
-    - For each update record, adds effectiveDate and createDate equal to that updates dateOfUpdate
-    - Handles batching for cases where there are more than 100 records to update
+    - Scans the provider table for all update records
+    - For each update record, load the records and serialize it again,
+        so the schema classes will generate the new sort key patterns
+    - Recreate the records by deleting the update records with the old sort key and storing the migrated records.
     """
     logger.info('Starting update record sort key migration')
 

backend/compact-connect/tests/smoke/rollback_license_upload_smoke_tests.py

@@ -176,11 +176,15 @@ def wait_for_all_providers_created(staff_headers: dict, expected_count: int, max
         },
     }
 
-    last_key = None
-    page_num = 1
-    all_provider_ids = []
+    # Use a set to track unique provider IDs across all retries
+    unique_provider_ids = set()
+
     while time.time() - start_time < max_wait_time:
-        # Collect all providers across all pages
+        # Reset pagination variables at the start of each outer loop iteration
+        last_key = None
+        page_num = 1
+
+        # Collect all providers across all pages for this iteration
         while True:
             query_body = base_query_body.copy()
             if last_key:
@@ -203,13 +207,13 @@ def wait_for_all_providers_created(staff_headers: dict, expected_count: int, max
             providers = response_data.get('providers', [])
             pagination = response_data.get('pagination', {})
 
-            # Collect provider IDs from this page
+            # Collect provider IDs from this page and add to set (automatically handles duplicates)
             page_provider_ids = [p['providerId'] for p in providers]
-            all_provider_ids.extend(page_provider_ids)
+            unique_provider_ids.update(page_provider_ids)
 
             logger.info(
                 f'Page {page_num}: Found {len(page_provider_ids)} providers '
-                f'(total: {len(all_provider_ids)}/{expected_count})'
+                f'(unique total: {len(unique_provider_ids)}/{expected_count})'
             )
 
             # Check if there are more pages
@@ -220,14 +224,15 @@ def wait_for_all_providers_created(staff_headers: dict, expected_count: int, max
 
             page_num += 1
 
-        num_found = len(all_provider_ids)
+        num_found = len(unique_provider_ids)
         logger.info(
             f'Found {num_found}/{expected_count} providers with family name "RollbackTest" (across {page_num} pages)'
         )
 
         if num_found >= expected_count:
             logger.info(f'All {expected_count} providers found!')
-            return all_provider_ids  # Return only the expected count
+            # Return a deterministic sorted list, sliced to expected_count
+            return sorted(list(unique_provider_ids))[:expected_count]
 
         elapsed = time.time() - start_time
         if elapsed < max_wait_time: