Merge pull request #301 from openedx/pwnage101/ENT-7807

feat: redeem() support for assignment-based policies

enterprise_access/apps/api/v1/views/subsidy_access_policy.py

@@ -47,7 +47,11 @@
     TransactionStateChoices
 )
 from enterprise_access.apps.subsidy_access_policy.content_metadata_api import get_and_cache_content_metadata
-from enterprise_access.apps.subsidy_access_policy.exceptions import ContentPriceNullException, SubsidyAPIHTTPError
+from enterprise_access.apps.subsidy_access_policy.exceptions import (
+    ContentPriceNullException,
+    MissingAssignment,
+    SubsidyAPIHTTPError
+)
 from enterprise_access.apps.subsidy_access_policy.models import (
     SubsidyAccessPolicy,
     SubsidyAccessPolicyLockAttemptFailed
@@ -537,6 +541,11 @@ def redeem(self, request, *args, **kwargs):
             raise RedemptionRequestException(
                 detail=error_payload,
             ) from exc
+        except MissingAssignment as exc:
+            logger.exception(f'{exc} when redeeming assigned learner credit.')
+            raise RedemptionRequestException(
+                detail=f'Assignments race-condition: {exc}',
+            ) from exc
 
     def get_existing_redemptions(self, policies, lms_user_id):
         """

enterprise_access/apps/subsidy_access_policy/exceptions.py

@@ -50,3 +50,10 @@ def error_payload(self):
         return {
             'detail': str(self),
         }
+
+
+class MissingAssignment(SubsidyAccessPolicyException):
+    """
+    Raised in rare/impossible cases where attempts to redeem assigned content resulted in a race condition where an
+    assignment couldn't be found.
+    """

enterprise_access/apps/subsidy_access_policy/models.py

@@ -37,7 +37,12 @@
     TransactionStateChoices
 )
 from .content_metadata_api import get_and_cache_catalog_contains_content, get_and_cache_content_metadata
-from .exceptions import ContentPriceNullException, SubsidyAccessPolicyLockAttemptFailed, SubsidyAPIHTTPError
+from .exceptions import (
+    ContentPriceNullException,
+    MissingAssignment,
+    SubsidyAccessPolicyLockAttemptFailed,
+    SubsidyAPIHTTPError
+)
 from .subsidy_api import get_and_cache_transactions_for_learner
 from .utils import (
     ProxyAwareHistoricalRecords,
@@ -646,10 +651,15 @@ def _redemptions_for_idempotency_key(self, all_transactions):
     def redeem(self, lms_user_id, content_key, all_transactions, metadata=None):
         """
         Redeem a subsidy for the given learner and content.
+
         Returns:
-            A ledger transaction, or None if the subsidy was not redeemed.
+            A ledger transaction.
+
+        Raises:
+            SubsidyAPIHTTPError if the Subsidy API request failed.
+            ValueError if the access method of this policy is invalid.
         """
-        if self.access_method == AccessMethods.DIRECT:
+        if self.access_method in (AccessMethods.DIRECT, AccessMethods.ASSIGNED):
             idempotency_key = create_idempotency_key_for_transaction(
                 subsidy_uuid=str(self.subsidy_uuid),
                 lms_user_id=lms_user_id,
@@ -1048,7 +1058,45 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False):
         return (True, None, existing_redemptions)
 
     def redeem(self, lms_user_id, content_key, all_transactions, metadata=None):
-        raise NotImplementedError
+        """
+        Redeem content, but only if there's a matching assignment.  On successful redemption, the assignment state will
+        be set to 'accepted', otherwise 'errored'.
+
+        Returns:
+            A ledger transaction.
+
+        Raises:
+            SubsidyAPIHTTPError if the Subsidy API request failed.
+            ValueError if the access method of this policy is invalid.
+        """
+        found_assignment = assignments_api.get_assignment_for_learner(
+            self.assignment_configuration,
+            lms_user_id,
+            content_key,
+        )
+        # The following checks for non-allocated assignments only exist to be defensive against race-conditions, but
+        # in practice should never happen if the caller locks the policy and runs can_redeem() before redeem().
+        if not found_assignment:
+            raise MissingAssignment(
+                f'No assignment was found for lms_user_id={lms_user_id} and content_key=<{content_key}>.'
+            )
+        if found_assignment.state != LearnerContentAssignmentStateChoices.ALLOCATED:
+            raise MissingAssignment(
+                f"Only an assignment with state='{found_assignment.state}' was found for lms_user_id={lms_user_id} "
+                f"and content_key=<{content_key}>."
+            )
+        try:
+            ledger_transaction = super().redeem(lms_user_id, content_key, all_transactions, metadata)
+        except SubsidyAPIHTTPError:
+            # Migrate assignment to errored if the subsidy API call errored.
+            found_assignment.state = LearnerContentAssignmentStateChoices.ERRORED
+            found_assignment.save()
+            raise
+        # Migrate assignment to accepted.
+        found_assignment.state = LearnerContentAssignmentStateChoices.ACCEPTED
+        found_assignment.transaction_uuid = ledger_transaction.get('uuid')  # uuid should always be in the API response.
+        found_assignment.save()
+        return ledger_transaction
 
     def can_allocate(self, number_of_learners, content_key, content_price_cents):
         """

enterprise_access/apps/subsidy_access_policy/tests/test_models.py

@@ -28,6 +28,7 @@
     REASON_POLICY_SPEND_LIMIT_REACHED,
     REASON_SUBSIDY_EXPIRED
 )
+from enterprise_access.apps.subsidy_access_policy.exceptions import MissingAssignment, SubsidyAPIHTTPError
 from enterprise_access.apps.subsidy_access_policy.models import (
     AssignedLearnerCreditAccessPolicy,
     PerLearnerEnrollmentCreditAccessPolicy,
@@ -858,12 +859,109 @@ def test_can_redeem(
 
         assert can_redeem_result == expected_policy_can_redeem
 
-    def test_redeem(self):
+    @ddt.data(
+        # Happy path, assignment exists and state='allocated', and afterwards gets updated to 'accepted'.
+        {
+            'assignment_starting_state': LearnerContentAssignmentStateChoices.ALLOCATED,
+            'assignment_ending_state': LearnerContentAssignmentStateChoices.ACCEPTED,
+            'fail_subsidy_create_transaction': False,
+            'redeem_raises': None,
+        },
+        # Sad path, no assignment exists.
+        {
+            'assignment_starting_state': None,
+            'assignment_ending_state': None,
+            'fail_subsidy_create_transaction': False,
+            'redeem_raises': MissingAssignment,
+        },
+        # Sad path, assignment has state='accepted'.
+        {
+            'assignment_starting_state': LearnerContentAssignmentStateChoices.ACCEPTED,
+            'assignment_ending_state': LearnerContentAssignmentStateChoices.ACCEPTED,
+            'fail_subsidy_create_transaction': False,
+            'redeem_raises': MissingAssignment,
+        },
+        # Sad path, assignment has state='cancelled'.
+        {
+            'assignment_starting_state': LearnerContentAssignmentStateChoices.CANCELLED,
+            'assignment_ending_state': LearnerContentAssignmentStateChoices.CANCELLED,
+            'fail_subsidy_create_transaction': False,
+            'redeem_raises': MissingAssignment,
+        },
+        # Sad path, assignment has state='errored'.
+        {
+            'assignment_starting_state': LearnerContentAssignmentStateChoices.ERRORED,
+            'assignment_ending_state': LearnerContentAssignmentStateChoices.ERRORED,
+            'fail_subsidy_create_transaction': False,
+            'redeem_raises': MissingAssignment,
+        },
+        # Sad path, request to subsidy API failed.
+        {
+            'assignment_starting_state': LearnerContentAssignmentStateChoices.ALLOCATED,
+            'assignment_ending_state': LearnerContentAssignmentStateChoices.ERRORED,
+            'fail_subsidy_create_transaction': True,
+            'redeem_raises': SubsidyAPIHTTPError,
+        },
+    )
+    @ddt.unpack
+    def test_redeem(
+        self,
+        assignment_starting_state,
+        assignment_ending_state,
+        fail_subsidy_create_transaction,
+        redeem_raises,
+    ):
         """
-        Test stub for non-implemented method.
+        Test redeem() for assigned learner credit policies.
         """
-        with self.assertRaises(NotImplementedError):
-            self.active_policy.redeem(123, 'abc', [])
+        self.assignments_api_patcher.stop()
+
+        # Set up the entire environment to make the policy happy about all non-assignment stuff.
+        self.mock_lms_api_client.enterprise_contains_learner.return_value = True
+        self.mock_catalog_contains_content_key.return_value = True
+        self.mock_get_content_metadata.return_value = {
+            'content_price': 200,
+        }
+        self.mock_subsidy_client.can_redeem.return_value = {'can_redeem': True, 'active': True}
+        self.mock_transactions_cache_for_learner.return_value = {
+            'transactions': [],
+            'aggregates': {'total_quantity': -100},
+        }
+        self.mock_subsidy_client.list_subsidy_transactions.return_value = {
+            'results': [],
+            'aggregates': {'total_quantity': -200},
+        }
+
+        # Optionally simulate a failed subsidy API request to create a transaction:
+        test_transaction_uuid = uuid4()
+        if fail_subsidy_create_transaction:
+            self.mock_subsidy_client.create_subsidy_transaction.side_effect = requests.exceptions.HTTPError
+        else:
+            self.mock_subsidy_client.create_subsidy_transaction.return_value = {'uuid': str(test_transaction_uuid)}
+
+        # Create a single assignment (or not) per the test case.
+        assignment = None
+        if assignment_starting_state:
+            assignment = LearnerContentAssignmentFactory.create(
+                assignment_configuration=self.assignment_configuration,
+                content_key=self.course_key,
+                lms_user_id=self.lms_user_id,
+                state=assignment_starting_state,
+            )
+
+        if redeem_raises:
+            with self.assertRaises(redeem_raises):
+                self.active_policy.redeem(self.lms_user_id, self.course_run_key, [])
+        else:
+            self.active_policy.redeem(self.lms_user_id, self.course_run_key, [])
+
+        # Finally, assert that the assignment object was correctly updated to reflect the success/failure.
+        if assignment:
+            assignment.refresh_from_db()
+            assert assignment.state == assignment_ending_state
+            # happy path should result in an updated transaction_uuid.
+            if not redeem_raises:
+                assert assignment.transaction_uuid == test_transaction_uuid
 
     def test_can_allocate_inactive_policy(self):
         """