feat: rollback_transaction() also cancels external fulfillments

This is needed for cases where an external fulfillment has been created,
but the subsequent platform fulfillment fails (e.g. due to transient
networking issues).  Before this PR, the external fulfillment would not
get rolled back, resulting in an "orphaned" fulfillment that allows
learners to effectively redeem content for free.  After this PR, that
loophole is fixed.

ENT-8866

enterprise_subsidy/apps/fulfillment/api.py

@@ -185,6 +185,9 @@ def _fulfill_in_geag(self, allocation_payload):
     def fulfill(self, transaction):
         """
         Attempt to fulfill a ledger transaction with the GEAG fulfillment system
+
+        Returns:
+            ExternalTransactionReference: A new ExternalTransactionReference representing the new external fulfillment.
         """
         self._validate(transaction)
         allocation_payload = self._create_allocation_payload(transaction)
@@ -209,6 +212,12 @@ def cancel_fulfillment(self, external_transaction_reference):
         """
         Cancels the provided external reference's (related to some ``Transaction`` record)
         related enterprise allocation.
+
+        Raises:
+            HTTPError:
+                Calling the external platform API to cancel an external fulfillment failed. Currently, this could happen
+                if the external reference does not refer to a GEAG fulfillment because we currently only support GEAG
+                external fulfillments.
         """
         self.get_smarter_client().cancel_enterprise_allocation(
             external_transaction_reference.external_reference_id,

enterprise_subsidy/apps/subsidy/models.py

@@ -417,13 +417,45 @@ def commit_transaction(self, ledger_transaction, fulfillment_identifier=None, ex
         ledger_transaction.state = TransactionStateChoices.COMMITTED
         ledger_transaction.save()
 
-    def rollback_transaction(self, ledger_transaction):
+    def rollback_transaction(self, ledger_transaction, external_transaction_reference=None):
         """
-        Progress the transaction to a failed state.
+        Progress the transaction to a failed state. Also attempt to cancel any external fulfillments given.
+
+        Args:
+            ledger_transaction (openedx_ledger.models.Transaction):
+                The transaction to rollback.
+            external_transaction_reference (openedx_ledger.models.ExternalTransactionReference):
+                The external fulfillment to cancel (optional). Must link back to the given transaction.
+
+        Raises:
+            I made a best effort to avoid raising an exception unless there's local database issues like locking or
+            other read errors on transaction.save().
         """
-        logger.info(f'Setting transaction {ledger_transaction.uuid} state to failed.')
-        ledger_transaction.state = TransactionStateChoices.FAILED
-        ledger_transaction.save()
+        try:
+            if external_transaction_reference and external_transaction_reference.transaction == ledger_transaction:
+                logger.info(
+                    '[rollback_transaction] Attempting to cancel external fulfillment for transaction %s.',
+                    ledger_transaction.uuid,
+                )
+                self.geag_fulfillment_handler().cancel_fulfillment(external_transaction_reference)
+        except HTTPError as exc:
+            logger.error(
+                "[rollback_transaction] Error canceling external fulfillment %s: %s",
+                external_transaction_reference.external_reference_id,
+                exc,
+            )
+        except Exception as exc:  # pylint: disable=broad-except
+            # We are extra sensitive to raising an exception from this method because it's already running inside a
+            # rollback context, so the caller already knows something went wrong and is trying to recover.
+            logger.error(
+                "[rollback_transaction] Swallowing uncaught exception trying to cancel external fulfillment: %s",
+                exc,
+            )
+        finally:
+            # No matter what, we absolutely need to progress the transaction to a failed state.
+            logger.info('[rollback_transaction] Setting transaction %s state to failed.', ledger_transaction.uuid)
+            ledger_transaction.state = TransactionStateChoices.FAILED
+            ledger_transaction.save()
 
     def redeem(
         self,
@@ -576,9 +608,10 @@ def _create_redemption(
         ledger_transaction.state = TransactionStateChoices.PENDING
         ledger_transaction.save()
 
+        external_transaction_reference = None
         try:
             if self.geag_fulfillment_handler().can_fulfill(ledger_transaction):
-                self.geag_fulfillment_handler().fulfill(ledger_transaction)
+                external_transaction_reference = self.geag_fulfillment_handler().fulfill(ledger_transaction)
         except Exception as exc:
             logger.exception(
                 f'Failed to fulfill transaction {ledger_transaction.uuid} with the GEAG handler.'
@@ -596,7 +629,7 @@ def _create_redemption(
             logger.exception(
                 f'Failed to enroll for transaction {ledger_transaction.uuid} via the enterprise client.'
             )
-            self.rollback_transaction(ledger_transaction)
+            self.rollback_transaction(ledger_transaction, external_transaction_reference)
             raise exc
 
         return ledger_transaction

enterprise_subsidy/apps/subsidy/tests/test_models.py

@@ -475,13 +475,20 @@ def test_redeem_with_geag_exception(self, mock_get_content_summary, mock_enterpr
             'source': 'edX',
             'mode': 'verified',
             'content_price': 10000,
+            # When this key value is non-None, it triggers an attempt to create an external fulfillment. This attempt
+            # will fail because the metadata below is missing a bunch of required keys, e.g. 'geag_date_of_birth'.
             'geag_variant_id': str(uuid4()),
         }
         mock_price_for_content.return_value = mock_content_price
         mock_enterprise_client.enroll.return_value = mock_enterprise_fulfillment_uuid
         tx_metadata = {
             'geag_first_name': 'Donny',
             'geag_last_name': 'Kerabatsos',
+            # The following required keys are missing and will cause external fulfillment to fail.
+            # 'geag_email': ,
+            # 'geag_date_of_birth': ,
+            # 'geag_terms_accepted_at': ,
+            # 'geag_data_share_consent': ,
         }
         with pytest.raises(InvalidFulfillmentMetadataException):
             self.subsidy.redeem(
@@ -493,6 +500,67 @@ def test_redeem_with_geag_exception(self, mock_get_content_summary, mock_enterpr
         created_transaction = Transaction.objects.latest('created')
         assert created_transaction.state == TransactionStateChoices.FAILED
 
+    @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content')
+    @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client')
+    @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary")
+    @mock.patch("enterprise_subsidy.apps.api_client.enterprise.EnterpriseApiClient.get_enterprise_customer_data")
+    @mock.patch("enterprise_subsidy.apps.fulfillment.api.GEAGFulfillmentHandler._fulfill_in_geag")
+    @mock.patch("enterprise_subsidy.apps.fulfillment.api.GEAGFulfillmentHandler.cancel_fulfillment")
+    def test_redeem_with_platform_exception_rolls_back_geag(
+        self,
+        mock_cancel_fulfillment,
+        mock_fulfill_in_geag,
+        mock_get_enterprise_customer_data,
+        mock_get_content_summary,
+        mock_enterprise_client,
+        mock_price_for_content,
+    ):
+        """
+        Test Subsidy.redeem() rollback upon platform networking exception handles geag cancellation.
+        """
+        lms_user_id = 1
+        content_key = "course-v1:edX+test+course"
+        subsidy_access_policy_uuid = str(uuid4())
+        mock_content_price = 1000
+        mock_fulfillment_order_uuid = str(uuid4())
+        mock_get_content_summary.return_value = {
+            'content_uuid': 'course-v1:edX+test+course',
+            'content_key': 'course-v1:edX+test+course',
+            'source': 'edX',
+            'mode': 'verified',
+            'content_price': 10000,
+            'geag_variant_id': str(uuid4()),
+        }
+        mock_price_for_content.return_value = mock_content_price
+        # Create the conditions for a failed platform fulfillment after a successful external fulfillment.
+        mock_enterprise_client.enroll.side_effect = HTTPError(
+            response=MockResponse(None, status.HTTP_500_INTERNAL_SERVER_ERROR),
+        )
+        mock_get_enterprise_customer_data.return_value = {}
+        mock_fulfill_in_geag.return_value.json.return_value = {
+            'orderUuid': mock_fulfillment_order_uuid,
+        }
+        tx_metadata = {
+            'geag_first_name': 'Donny',
+            'geag_last_name': 'Kerabatsos',
+            'geag_email': '[email protected]',
+            'geag_date_of_birth': '1990-01-01',
+            'geag_terms_accepted_at': '2024-01-01T00:00:00Z',
+            'geag_data_share_consent': True,
+        }
+        with pytest.raises(HTTPError):
+            self.subsidy.redeem(
+                lms_user_id,
+                content_key,
+                subsidy_access_policy_uuid,
+                metadata=tx_metadata
+            )
+        created_transaction = Transaction.objects.latest('created')
+        assert created_transaction.state == TransactionStateChoices.FAILED
+        # The meat of what's being tested: Did we attempt to cancel the external fulfillment?
+        assert mock_cancel_fulfillment.called
+        assert mock_cancel_fulfillment.call_args.args[0].external_reference_id == mock_fulfillment_order_uuid
+
 
 class SubsidyManagerTestCase(TestCase):
     """

enterprise_subsidy/apps/transaction/api.py

@@ -52,11 +52,16 @@ def cancel_transaction_fulfillment(transaction):
 
 def cancel_transaction_external_fulfillment(transaction):
     """
-    Cancels all related external GEAG allocations for the given transaction.
+    Cancels all related external fulfillments for the given transaction.
 
-    raises:
-      FulfillmentException if the related external references for the transaction
-        are not for a GEAG fulfillment provider.
+    Note: All related external fulfillments that do NOT refer to a GEAG allocation _are skipped_, as only GEAG external
+    fulfillments are currently supported. A warning will be logged.
+
+    Raises:
+        TransactionFulfillmentCancelationException: The transaction is not committed, and no actions were taken.
+        requests.exceptions.HTTPError:
+            Calling the external platform API to cancel an external fulfillment failed for at least one external
+            reference related to the given transaction.
     """
     if transaction.state != TransactionStateChoices.COMMITTED:
         logger.info(