feat: support run-based assignments in can-redeem; ensure assigned …

…policies have highest priority (#562)

enterprise_access/apps/content_assignments/api.py

@@ -142,7 +142,7 @@ def get_assignments_for_admin(
         assignment_configuration (AssignmentConfiguration):
             The assignment configuration within which to search for assignments.
         learner_emails (list of str): A list of emails for which the admin intends to find existing assignments.
-        content_key (str): A content key representing a course which the assignments are for.
+        content_key (str): A content key representing a course or course run which the assignments are for.
 
     Returns:
         queryset of ``LearnerContentAssignment``: Existing records relevant to an admin's allocation request.
@@ -176,20 +176,21 @@ def get_assignment_for_learner(
     both course run keys, so a simple string comparison may not always suffice.  Method of content_key comparison for
     assignment lookup:
 
-    +---+------------------------+-----------------------+----------------------------------------------+
-    | # | assignment content_key | requested content_key |              How to compare?                 |
-    +---+------------------------+-----------------------+----------------------------------------------+
-    | 1 | course                 | course                | Simple comparison.                           |
-    | 2 | course                 | course run            | Convert everything to courses, then compare. | (most common)
-    | 3 | course run             | course                | Not supported.                               |
-    | 4 | course run             | course run            | Not supported.                               |
-    +---+------------------------+-----------------------+----------------------------------------------+
+    +---+------------------------+-----------------------+-------------------------------------------------+
+    | # | assignment content_key | requested content_key |                     How to compare?             |
+    +---+------------------------+-----------------------+-------------------------------------------------+
+    | 1 | course                 | course                | Simple comparison.                              |
+    | 2 | course                 | course run            | Convert course run to course key, then compare. |
+    | 3 | course run             | course                | Simple comparison via the parent_content_key,   |
+    |   |                        |                       | returning first run-based assignment match.     |
+    | 4 | course run             | course run            | Simple comparison.                              |
+    +---+------------------------+-----------------------+-------------------------------------------------+
 
     Args:
         assignment_configuration (AssignmentConfiguration):
             The assignment configuration within which to search for assignments.
         lms_user_id (int): One lms_user_id which the assignments are for.
-        content_key (str): A content key representing a course which the assignments are for.
+        content_key (str): A content key representing a course or course run which the assignments are for.
 
     Returns:
         ``LearnerContentAssignment``: Existing assignment relevant to a learner's redemption request, or None if not
@@ -200,19 +201,47 @@ def get_assignment_for_learner(
         constraint across [assignment_configuration,lms_user_id,content_key].  BUT still technically possible if
         internal staff managed to create a duplicate assignment configuration for a single enterprise.
     """
+    queryset = LearnerContentAssignment.objects.select_related('assignment_configuration')
+
+    try:
+        # First, try to find a corresponding assignment based on the specific content key,
+        # considering both assignments' content key and parent content key.
+        return queryset.get(
+            Q(content_key=content_key) | Q(parent_content_key=content_key),
+            assignment_configuration=assignment_configuration,
+            lms_user_id=lms_user_id,
+        )
+    except LearnerContentAssignment.DoesNotExist:
+        logger.info(
+            f'No assignment found with content_key or parent_content_key {content_key} '
+            f'for {assignment_configuration} and lms_user_id {lms_user_id}',
+        )
+    except LearnerContentAssignment.MultipleObjectsReturned as exc:
+        logger.error(
+            f'Multiple assignments found with content_key or parent_content_key {content_key} '
+            f'for {assignment_configuration} and lms_user_id {lms_user_id}',
+        )
+        raise exc
+
+    # If no exact match was found, try to normalize the content key and find a match. This happens when
+    # the content_key is a course run key and the assignment's content_key is a course key, as depicted
+    # by row 2 in the above docstring matrix.
     content_key_to_match = _normalize_course_key_from_metadata(assignment_configuration, content_key)
     if not content_key_to_match:
         logger.error(f'Unable to normalize content_key {content_key} for {assignment_configuration} and {lms_user_id}')
         return None
-    queryset = LearnerContentAssignment.objects.select_related('assignment_configuration')
+
     try:
         return queryset.get(
+            content_key=content_key_to_match,
             assignment_configuration=assignment_configuration,
             lms_user_id=lms_user_id,
-            # assignment content_key is assumed to always be a course with no namespace prefix.
-            content_key=content_key_to_match,
         )
     except LearnerContentAssignment.DoesNotExist:
+        logger.info(
+            f'No assignment found with normalized content_key {content_key_to_match} '
+            f'for {assignment_configuration} and lms_user_id {lms_user_id}',
+        )
         return None
 
 
@@ -245,7 +274,7 @@ def allocate_assignments(assignment_configuration, learner_emails, content_key,
     Params:
       - ``assignment_configuration``: The AssignmentConfiguration record under which assignments should be allocated.
       - ``learner_emails``: A list of learner email addresses to whom assignments should be allocated.
-      - ``content_key``: Typically a *course* key to which the learner is assigned.
+      - ``content_key``: Either a course or course run key, representing the content to be allocated.
       - ``content_price_cents``: The cost of redeeming the content, in USD cents, at the time of allocation. Should
         always be an integer >= 0.
 
@@ -446,7 +475,7 @@ def _get_lms_user_ids_by_email(emails):
 
 
 def _get_existing_assignments_for_allocation(
-        assignment_configuration, learner_emails_to_allocate, content_key, lms_user_ids_by_email,
+    assignment_configuration, learner_emails_to_allocate, content_key, lms_user_ids_by_email,
 ):
     """
     Finds any existing assignments records related to the provided ``assignment_cofiguration``,

enterprise_access/apps/content_assignments/tests/test_api.py

@@ -116,57 +116,146 @@ def test_get_assignments_for_configuration_different_states(self):
         return_value=mock.MagicMock(),
     )
     @ddt.data(
-        # Standard happy path.
+        # [course run] Standard happy path.
         {
-            'assignment_content_key': 'test+course',
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
             'assignment_lms_user_id': 1,
             'request_default_assignment_configuration': True,
             'request_lms_user_id': 1,
             'request_content_key': 'course-v1:test+course+run',
             'expect_assignment_found': True,
         },
-        # Happy path, requested content is a course (with prefix).
+        # [course] Standard happy path.
         {
             'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': True,
+            'request_lms_user_id': 1,
+            'request_content_key': 'course-v1:test+course+run',
+            'expect_assignment_found': True,
+        },
+        # [course run] Happy path, requested content is a course
+        {
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
             'assignment_lms_user_id': 1,
             'request_default_assignment_configuration': True,
             'request_lms_user_id': 1,
-            'request_content_key': 'course-v1:test+course',  # This is a course! With a prefix!
+            'request_content_key': 'test+course',
             'expect_assignment_found': True,
         },
-        # Happy path, requested content is a course (without prefix).
+        # [course] Happy path, requested content is a course
         {
             'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
             'assignment_lms_user_id': 1,
             'request_default_assignment_configuration': True,
             'request_lms_user_id': 1,
-            'request_content_key': 'test+course',  # This is a course! Without a prefix!
+            'request_content_key': 'test+course',
             'expect_assignment_found': True,
         },
-        # Different lms_user_id.
+        # [course run] Different lms_user_id, requested content is a course
+        {
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': True,
+            'request_lms_user_id': 2,  # Different lms_user_id!
+            'request_content_key': 'test+course',
+            'expect_assignment_found': False,
+        },
+        # [course] Different lms_user_id, requested content is a course
+        {
+            'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': True,
+            'request_lms_user_id': 2,  # Different lms_user_id!
+            'request_content_key': 'test+course',
+            'expect_assignment_found': False,
+        },
+        # [course run] Different lms_user_id
+        {
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': True,
+            'request_lms_user_id': 2,  # Different lms_user_id!
+            'request_content_key': 'course-v1:test+course+run',
+            'expect_assignment_found': False,
+        },
+        # [course] Different lms_user_id
         {
             'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
             'assignment_lms_user_id': 1,
             'request_default_assignment_configuration': True,
             'request_lms_user_id': 2,  # Different lms_user_id!
+            'request_content_key': 'course-v1:test+course+run',
+            'expect_assignment_found': False,
+        },
+        # [course run] Different customer, requested content is a course
+        {
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': False,  # Different customer!
+            'request_lms_user_id': 1,
             'request_content_key': 'test+course',
             'expect_assignment_found': False,
         },
-        # Different customer.
+        # [course] Different customer, requested content is a course
         {
             'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
             'assignment_lms_user_id': 1,
             'request_default_assignment_configuration': False,  # Different customer!
             'request_lms_user_id': 1,
             'request_content_key': 'test+course',
             'expect_assignment_found': False,
         },
+        # [course run] Different customer
+        {
+            'assignment_content_key': 'course-v1:test+course+run',
+            'assignment_parent_content_key': 'test+course',
+            'assignment_is_course_run': True,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': False,  # Different customer!
+            'request_lms_user_id': 1,
+            'request_content_key': 'course-v1:test+course+run',
+            'expect_assignment_found': False,
+        },
+        # [course] Different customer
+        {
+            'assignment_content_key': 'test+course',
+            'assignment_parent_content_key': None,
+            'assignment_is_course_run': False,
+            'assignment_lms_user_id': 1,
+            'request_default_assignment_configuration': False,  # Different customer!
+            'request_lms_user_id': 1,
+            'request_content_key': 'course-v1:test+course+run',
+            'expect_assignment_found': False,
+        },
     )
     @ddt.unpack
     def test_get_assignment_for_learner(
         self,
         mock_get_and_cache_content_metadata,
         assignment_content_key,
+        assignment_parent_content_key,
+        assignment_is_course_run,
         assignment_lms_user_id,
         request_default_assignment_configuration,
         request_lms_user_id,
@@ -182,6 +271,8 @@ def test_get_assignment_for_learner(
         LearnerContentAssignmentFactory.create(
             assignment_configuration=self.assignment_configuration,
             content_key=assignment_content_key,
+            parent_content_key=assignment_parent_content_key,
+            is_assigned_course_run=assignment_is_course_run,
             lms_user_id=assignment_lms_user_id,
             state=LearnerContentAssignmentStateChoices.ALLOCATED,
         )

enterprise_access/apps/subsidy_access_policy/constants.py

@@ -30,6 +30,7 @@ class SegmentEvents:
 
 # Configure the priority of each policy type here.  When given multiple redeemable policies to select for redemption,
 # the policy resolution engine will select policies with the lowest priority number.
+ASSIGNED_CREDIT_POLICY_TYPE_PRIORITY = 0
 CREDIT_POLICY_TYPE_PRIORITY = 1
 SUBSCRIPTION_POLICY_TYPE_PRIORITY = 2
 

enterprise_access/apps/subsidy_access_policy/models.py

@@ -25,6 +25,7 @@
 
 from ..content_assignments.models import AssignmentConfiguration
 from .constants import (
+    ASSIGNED_CREDIT_POLICY_TYPE_PRIORITY,
     CREDIT_POLICY_TYPE_PRIORITY,
     FORCE_ENROLLMENT_KEYWORD,
     REASON_BEYOND_ENROLLMENT_DEADLINE,
@@ -1107,6 +1108,16 @@ def priority(self):
         return CREDIT_POLICY_TYPE_PRIORITY
 
 
+class AssignedCreditPolicyMixin:
+    """
+    Mixin class for assigned credit type policies.
+    """
+
+    @property
+    def priority(self):
+        return ASSIGNED_CREDIT_POLICY_TYPE_PRIORITY
+
+
 class PerLearnerEnrollmentCreditAccessPolicy(CreditPolicyMixin, SubsidyAccessPolicy):
     """
     Policy that limits the number of enrollments transactions for a learner in a subsidy.
@@ -1273,7 +1284,7 @@ def remaining_balance_per_user(self, lms_user_id=None):
         return self.per_learner_spend_limit - positive_spent_amount
 
 
-class AssignedLearnerCreditAccessPolicy(CreditPolicyMixin, SubsidyAccessPolicy):
+class AssignedLearnerCreditAccessPolicy(AssignedCreditPolicyMixin, SubsidyAccessPolicy):
     """
     Policy based on LearnerContentAssignments, backed by a learner credit type of subsidy.
 
@@ -1737,15 +1748,15 @@ def create_assignment(self):
             assignment_configuration.enterprise_customer_uuid,
             self.course_run_key,
         )
-        course_key = content_metadata.get('content_key')
+        content_key = content_metadata.get('content_key')
         user_record = User.objects.filter(lms_user_id=self.lms_user_id).first()
         if not user_record:
             raise Exception(f'No email could be found for lms_user_id {self.lms_user_id}')
 
         return assignments_api.allocate_assignments(
             assignment_configuration,
             [user_record.email],
-            course_key,
+            content_key,
             self.content_price_cents,
         )
 

enterprise_access/apps/subsidy_access_policy/tests/test_models.py

@@ -3,7 +3,7 @@
 """
 import contextlib
 from datetime import datetime, timedelta
-from unittest.mock import ANY, PropertyMock, patch
+from unittest.mock import ANY, patch
 from uuid import uuid4
 
 import ddt
@@ -900,6 +900,7 @@ def setUp(self):
         self.policy_two = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory.create()
         self.policy_three = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory.create()
         self.policy_four = PerLearnerSpendCapLearnerCreditAccessPolicyFactory.create()
+        self.policy_five = AssignedLearnerCreditAccessPolicyFactory.create()
 
         policy_one_subsidy_patcher = patch.object(
             self.policy_one, 'subsidy_record'
@@ -925,10 +926,17 @@ def setUp(self):
         self.mock_policy_four_subsidy_record = policy_four_subsidy_patcher.start()
         self.mock_policy_four_subsidy_record.return_value = self.mock_subsidy_four
 
+        policy_five_subsidy_patcher = patch.object(
+            self.policy_five, 'subsidy_record'
+        )
+        self.mock_policy_five_subsidy_record = policy_five_subsidy_patcher.start()
+        self.mock_policy_five_subsidy_record.return_value = self.mock_subsidy_one
+
         self.addCleanup(policy_one_subsidy_patcher.stop)
         self.addCleanup(policy_two_subsidy_patcher.stop)
         self.addCleanup(policy_three_subsidy_patcher.stop)
         self.addCleanup(policy_four_subsidy_patcher.stop)
+        self.addCleanup(policy_five_subsidy_patcher.stop)
 
     def test_setup(self):
         """
@@ -937,6 +945,8 @@ def test_setup(self):
         assert self.policy_one.subsidy_record() == self.mock_subsidy_one
         assert self.policy_two.subsidy_record() == self.mock_subsidy_two
         assert self.policy_three.subsidy_record() == self.mock_subsidy_three
+        assert self.policy_four.subsidy_record() == self.mock_subsidy_four
+        assert self.policy_five.subsidy_record() == self.mock_subsidy_one
 
     @override_settings(MULTI_POLICY_RESOLUTION_ENABLED=True)
     def test_resolve_one_policy(self):
@@ -965,16 +975,16 @@ def test_resolve_two_policies_by_expiration(self):
         assert SubsidyAccessPolicy.resolve_policy(policies) == self.policy_one
 
     @override_settings(MULTI_POLICY_RESOLUTION_ENABLED=True)
-    def test_resolve_two_policies_by_type_priority(self):
+    def test_resolve_policies_by_type_priority(self):
         """
         Test resolve given a two policies with same balances, same expiration
         but different type-priority.
         """
-        policies = [self.policy_four, self.policy_one]
-        # artificially set the priority attribute higher on one of the policies (lower priority takes precedent)
-        with patch.object(PerLearnerSpendCreditAccessPolicy, 'priority', new_callable=PropertyMock) as mock:
-            mock.return_value = 100
-            assert SubsidyAccessPolicy.resolve_policy(policies) == self.policy_one
+        policies = [self.policy_one, self.policy_four, self.policy_five]
+        # Assert the AssignedLearnerCreditAccessPolicy is resolved with highest
+        # priority, regardless of the ordering of the input policies.
+        assert SubsidyAccessPolicy.resolve_policy(policies) == self.policy_five
+        assert SubsidyAccessPolicy.resolve_policy(list(reversed(policies))) == self.policy_five
 
 
 @ddt.ddt