diff --git a/backend/compact-connect/lambdas/python/common/cc_common/data_model/transaction_client.py b/backend/compact-connect/lambdas/python/common/cc_common/data_model/transaction_client.py index b73b0a6de..763f9dff7 100644 --- a/backend/compact-connect/lambdas/python/common/cc_common/data_model/transaction_client.py +++ b/backend/compact-connect/lambdas/python/common/cc_common/data_model/transaction_client.py @@ -125,17 +125,16 @@ def _set_privilege_id_in_line_item(self, line_items: list[dict], item_id_prefix: if line_item.get('itemId').lower().startswith(item_id_prefix.lower()): line_item['privilegeId'] = privilege_id - def add_privilege_ids_to_transactions(self, compact: str, transactions: list[dict]) -> list[dict]: + def add_privilege_information_to_transactions(self, compact: str, transactions: list[dict]) -> list[dict]: """ - Add privilege IDs to transaction line items based on the jurisdiction they were purchased for. + Add privilege and licensee IDs to transaction line items based on the jurisdiction they were purchased for. :param compact: The compact name :param transactions: List of transaction records to process - :return: Modified list of transactions with privilege IDs added to line items + :return: Modified list of transactions with privilege and licensee IDs added to line items """ for transaction in transactions: line_items = transaction['lineItems'] - licensee_id = transaction['licenseeId'] # Extract jurisdictions from line items with format priv:{compact}-{jurisdiction}-{license type abbr} jurisdictions_to_process = set() for line_item in line_items: @@ -152,6 +151,48 @@ def add_privilege_ids_to_transactions(self, compact: str, transactions: list[dic KeyConditionExpression=Key('compactTransactionIdGSIPK').eq(gsi_pk), ) + # Verify that the query returned at least one record + records_for_transaction_id = response.get('Items', []) + if not records_for_transaction_id: + logger.error( + 'No privilege records found for this transaction id.', + compact=compact, + transaction_id=transaction['transactionId'], + # attempt to grab the licensee id from the authorize.net data, which may be invalid if it was masked + licensee_id=transaction['licenseeId'], + ) + # We mark the data as UNKNOWN so it still shows up in the history, + # and move onto the next transaction + for jurisdiction in jurisdictions_to_process: + item_id_prefix = f'priv:{compact}-{jurisdiction}' + # we set the privilege id to UNKNOWN, so that it will be visible in the report + self._set_privilege_id_in_line_item( + line_items=line_items, item_id_prefix=item_id_prefix, privilege_id='UNKNOWN' + ) + continue + + # ensure we only map to one provider for this transaction id + provider_ids = { + item['providerId'] + for item in records_for_transaction_id + if item['type'] == 'privilege' or item['type'] == 'privilegeUpdate' + } + # there should only be one provider id in the set + if len(provider_ids) > 1: + logger.error( + 'More than one matching provider id found for a transaction id.', + compact=compact, + transaction_id=transaction['transactionId'], + # attempt to grab the licensee id from the authorize.net data, which may be invalid if it was masked + provider_ids=transaction['licenseeId'], + ) + + # The licensee id recorded in Authorize.net cannot be trusted, as Authorize.net masks any values that look + # like a credit card number (consecutive digits separated by dashes). We need to grab the provider id from + # the privileges associated with this transaction and set the licensee id on the transaction to that value + # to ensure it is valid. + transaction['licenseeId'] = provider_ids.pop() + # Process each privilege record for jurisdiction in jurisdictions_to_process: # Currently, we only support one license type per transaction when purchasing privileges @@ -159,11 +200,7 @@ def add_privilege_ids_to_transactions(self, compact: str, transactions: list[dic item_id_prefix = f'priv:{compact}-{jurisdiction}' # find the first privilege record for the jurisdiction that matches the provider ID matching_privilege = next( - ( - item - for item in response.get('Items', []) - if item['jurisdiction'].lower() == jurisdiction and item['providerId'] == licensee_id - ), + (item for item in records_for_transaction_id if item['jurisdiction'].lower() == jurisdiction), None, ) if matching_privilege: @@ -185,7 +222,7 @@ def add_privilege_ids_to_transactions(self, compact: str, transactions: list[dic compact=compact, transactionId=transaction['transactionId'], jurisdiction=jurisdiction, - provider_id=licensee_id, + provider_id=transaction['licenseeId'], matching_privilege_records=response.get('Items', []), ) # we set the privilege id to UNKNOWN, so that it will be visible in the report diff --git a/backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_transaction_client.py b/backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_transaction_client.py index c0763526d..9dd7284d0 100644 --- a/backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_transaction_client.py +++ b/backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_transaction_client.py @@ -88,7 +88,7 @@ def test_store_multiple_transactions(self): # Verify the batch writer was called twice self.assertEqual(self.mock_batch_writer.put_item.call_count, 2) - def test_add_privilege_ids_to_transactions(self): + def test_add_privilege_information_to_transactions(self): # Mock the provider table query response self.mock_config.provider_table = MagicMock() self.mock_config.compact_transaction_id_gsi_name = 'compactTransactionIdGSI' @@ -123,7 +123,7 @@ def test_add_privilege_ids_to_transactions(self): ] # Call the method - result = self.client.add_privilege_ids_to_transactions('aslp', test_transactions) + result = self.client.add_privilege_information_to_transactions('aslp', test_transactions) # Verify the GSI query was called with correct parameters self.mock_config.provider_table.query.assert_called_once_with( @@ -136,7 +136,9 @@ def test_add_privilege_ids_to_transactions(self): self.assertEqual(result[0]['lineItems'][1]['privilegeId'], 'priv-456') # NY line item self.assertNotIn('privilegeId', result[0]['lineItems'][2]) # other item - def test_add_privilege_ids_to_transactions_performs_check_on_provider_id_for_match(self): + def test_add_privilege_information_to_transactions_maps_provider_id_to_transaction(self): + expected_provider_id = 'abcd1234-5678-9012-3456-7890a0d12345' + expected_privilege_id = 'priv-123' # Mock the provider table query response self.mock_config.provider_table = MagicMock() self.mock_config.compact_transaction_id_gsi_name = 'compactTransactionIdGSI' @@ -145,16 +147,8 @@ def test_add_privilege_ids_to_transactions_performs_check_on_provider_id_for_mat { 'type': 'privilege', 'jurisdiction': 'CA', - 'privilegeId': 'priv-123', - 'providerId': 'prov-123', - }, - { - 'type': 'privilegeUpdate', - 'jurisdiction': 'NY', - 'previous': {'privilegeId': 'priv-456'}, - # this should never happen in practice, but we're testing for it here - # as a sanity check - 'providerId': 'prov-456', + 'privilegeId': expected_privilege_id, + 'providerId': expected_provider_id, }, ] } @@ -163,17 +157,17 @@ def test_add_privilege_ids_to_transactions_performs_check_on_provider_id_for_mat test_transactions = [ { 'transactionId': 'tx123', - 'licenseeId': 'prov-123', + # reproducing real case where licensee id was masked in authorize.net + 'licenseeId': 'abcdXXXXXXXXXXXXXXXXXXX-4927a0d12345', 'lineItems': [ {'itemId': 'priv:aslp-CA', 'unitPrice': 100}, - {'itemId': 'priv:aslp-NY', 'unitPrice': 200}, {'itemId': 'credit-card-transaction-fee', 'unitPrice': 50}, ], } ] # Call the method - result = self.client.add_privilege_ids_to_transactions('aslp', test_transactions) + result = self.client.add_privilege_information_to_transactions('aslp', test_transactions) # Verify the GSI query was called with correct parameters self.mock_config.provider_table.query.assert_called_once_with( @@ -181,9 +175,8 @@ def test_add_privilege_ids_to_transactions_performs_check_on_provider_id_for_mat KeyConditionExpression=Key('compactTransactionIdGSIPK').eq('COMPACT#aslp#TX#tx123#'), ) - # Verify privilege IDs were added to correct line items - self.assertEqual(result[0]['lineItems'][0]['privilegeId'], 'priv-123') # CA line item - # In this case, the privilege ID is unknown because the provider ID does not match - # again, this should never happen in practice - self.assertEqual(result[0]['lineItems'][1]['privilegeId'], 'UNKNOWN') - self.assertNotIn('privilegeId', result[0]['lineItems'][2]) # other item + # Verify the correct provider ID was added to the transaction + self.assertEqual(expected_provider_id, result[0]['licenseeId']) + # Verify the privilege id is mapped as expected + self.assertEqual(expected_privilege_id, result[0]['lineItems'][0]['privilegeId']) + self.assertNotIn('privilegeId', result[0]['lineItems'][1]) # credit card fee line item diff --git a/backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py b/backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py index dd13b30b0..9c290f788 100644 --- a/backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py +++ b/backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py @@ -97,7 +97,7 @@ def process_settled_transactions(event: dict, context: LambdaContext) -> dict: logger.info('Fetching privilege ids for transactions', compact=compact) # first we must add the associated privilege ids to each transaction so we can show the association in our # reports - transactions_with_privilege_ids = config.transaction_client.add_privilege_ids_to_transactions( + transactions_with_privilege_ids = config.transaction_client.add_privilege_information_to_transactions( compact=compact, transactions=transaction_response['transactions'] ) logger.info('Storing transactions in DynamoDB', compact=compact)