feat: add lms_user_email to redemptions

enterprise_subsidy/apps/api/v1/serializers.py

@@ -128,7 +128,9 @@ class Meta:
             "state",
             "idempotency_key",
             "lms_user_id",
+            "lms_user_email",
             "content_key",
+            "content_title",
             "quantity",
             "unit",  # Manually fetch from parent ledger via get_unit().
             "fulfillment_identifier",

enterprise_subsidy/apps/api/v1/views/content_metadata.py

@@ -106,7 +106,8 @@ def get(self, request, content_identifier, enterprise_customer_uuid):
         try:
             content_summary = self.content_metadata_api().get_content_summary(
                 enterprise_customer_uuid[0],
-                content_identifier
+                content_identifier,
+                skip_customer_fetch=True,
             )
             if not content_summary.get('content_price'):
                 logger.warning(f"Could not find course price in metadata for {content_identifier}")

enterprise_subsidy/apps/api/v1/views/transaction.py

@@ -217,6 +217,10 @@ def base_queryset(self):
         for learner_only_context in learner_only_contexts:
             # For each context (enterprise_customer_uuid) that the requester only has learner access to, filter
             # transactions related to that context to only include their own transactions.
+            # AED 2023-10-31: locally, pylint complains of the binary operation.
+            # In github actions, pylint complains of a useless-suppression.
+            # Suppressing both and letting the code gods sort it out.
+            # pylint: disable=unsupported-binary-operation,useless-suppression
             if request_jwt.get('user_id'):
                 queryset = queryset.filter(
                     (

enterprise_subsidy/apps/api/v2/tests/test_transaction_views.py

@@ -33,6 +33,7 @@ class APITestBase(APITestMixin):
     Contains boilerplate to create a couple of subsidies with related ledgers and starting transactions.
     """
 
+    lms_user_email = '[email protected]'
     enterprise_1_uuid = STATIC_ENTERPRISE_UUID
     enterprise_2_uuid = str(uuid.uuid4())
 
@@ -56,6 +57,8 @@ class APITestBase(APITestMixin):
 
     content_key_1 = "course-v1:edX+test+course.1"
     content_key_2 = "course-v1:edX+test+course.2"
+    content_title_1 = "edX: Test Course 1"
+    content_title_2 = "edx: Test Course 2"
 
     @classmethod
     def setUpClass(cls):
@@ -108,8 +111,10 @@ def _setup_transactions(cls):
             quantity=-1000,
             ledger=cls.subsidy_1.ledger,
             lms_user_id=STATIC_LMS_USER_ID+1000,
+            lms_user_email=cls.lms_user_email,
             subsidy_access_policy_uuid=cls.subsidy_access_policy_2_uuid,
             content_key=cls.content_key_2,
+            content_title=cls.content_title_2,
         )
         # Also create a reversed transaction, and also include metadata on both the transaction and reversal.
         cls.subsidy_1_transaction_3 = TransactionFactory(
@@ -118,8 +123,10 @@ def _setup_transactions(cls):
             quantity=-1000,
             ledger=cls.subsidy_1.ledger,
             lms_user_id=STATIC_LMS_USER_ID,
+            lms_user_email=cls.lms_user_email,
             subsidy_access_policy_uuid=cls.subsidy_access_policy_2_uuid,
             content_key=cls.content_key_2,
+            content_title=cls.content_title_2,
             metadata={"bin": "baz"},
         )
         cls.subsidy_1_transaction_3_reversal = ReversalFactory(
@@ -256,8 +263,10 @@ def setUpClass(cls):
             quantity=0,
             ledger=cls.subsidy_1.ledger,
             lms_user_id=STATIC_LMS_USER_ID,  # This is the only transaction belonging to the requester.
+            lms_user_email=cls.lms_user_email,
             subsidy_access_policy_uuid=cls.subsidy_access_policy_1_uuid,
             content_key=cls.content_key_1,
+            content_title=cls.content_title_1,
         )
 
     def test_list_transactions_metadata_format(self):
@@ -705,8 +714,10 @@ def test_operator_creation_happy_path_transaction_exists(self, mock_price_for_co
     @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client")
     @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content")
     @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary")
+    @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client")
     def test_operator_creation_happy_path_201(
         self,
+        mock_lms_user_client,
         mock_get_content_summary,
         mock_price_for_content,
         mock_enterprise_client
@@ -717,11 +728,15 @@ def test_operator_creation_happy_path_201(
         """
         self.set_up_operator()
 
+        mock_lms_user_client.return_value.best_effort_user_data.return_value = {
+            'email': self.lms_user_email,
+        }
         mock_enterprise_client.enroll.return_value = 'my-fulfillment-id'
         mock_price_for_content.return_value = 1000
         mock_get_content_summary.return_value = {
             'content_uuid': self.content_key_1,
             'content_key': self.content_key_1,
+            'content_title': self.content_title_1,
             'source': 'edX',
             'mode': 'verified',
             'content_price': 10000,

enterprise_subsidy/apps/api_client/enterprise_catalog.py

@@ -31,7 +31,9 @@ def content_metadata_url(self, enterprise_customer_uuid, content_identifier):
             f'content-metadata/{content_identifier}/'
         )
 
-    def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_identifier):
+    def get_content_metadata_for_customer(
+        self, enterprise_customer_uuid, content_identifier, skip_customer_fetch=False, **kwargs
+    ):
         """
         Returns Enterprise Customer related data for a specified piece on content.
 
@@ -40,6 +42,12 @@ def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_id
             content_identifier (str): **Either** the content UUID or content key identifier for a content record.
                 Note: the content needs to be owned by a catalog associated with the provided customer else this
                 method will throw an HTTPError.
+            skip_customer_fetch (bool): Forces enterprise-catalog to skip a sub-call to an edx-enterprise
+                API endpoint running in the edx-platform runtime. This sub-call helps the catalog service
+                understand the last time a catalog's customer record was modified, and also helps
+                to construct course and course run enrollment URLs that are usually not needed
+                in the context of enterprise-subsidy or callers of the EnterpriseCustomerViewSet.
+                Defaults to False.
         Returns:
             response (dict): JSON response object associated with a content metadata record
         Raises:
@@ -48,8 +56,11 @@ def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_id
             does not exist, or is not present in a catalog associated with the customer.
         """
         content_metadata_url = self.content_metadata_url(enterprise_customer_uuid, content_identifier)
+        query_params = {}
+        if skip_customer_fetch:
+            query_params['skip_customer_fetch'] = skip_customer_fetch
         try:
-            response = self.client.get(content_metadata_url)
+            response = self.client.get(content_metadata_url, params=query_params)
             response.raise_for_status()
             return response.json()
         except requests.exceptions.HTTPError as exc:

enterprise_subsidy/apps/content_metadata/api.py

@@ -136,13 +136,14 @@ def get_course_run(self, content_identifier, content_data):
                 return course_run
         return {}
 
-    def get_content_summary(self, enterprise_customer_uuid, content_identifier):
+    def get_content_summary(self, enterprise_customer_uuid, content_identifier, **kwargs):
         """
         Returns a summary dict some content metadata, makes the client call
         """
         course_details = self.get_content_metadata(
             enterprise_customer_uuid,
-            content_identifier
+            content_identifier,
+            **kwargs,
         )
         return self.summary_data_for_content(content_identifier, course_details)
 
@@ -201,7 +202,7 @@ def get_geag_variant_id(self, enterprise_customer_uuid, content_identifier):
         return self.get_content_summary(enterprise_customer_uuid, content_identifier).get('geag_variant_id')
 
     @staticmethod
-    def get_content_metadata(enterprise_customer_uuid, content_identifier):
+    def get_content_metadata(enterprise_customer_uuid, content_identifier, **kwargs):
         """
         Fetches details about the given content from a tiered (request + django) cache;
         or it fetches from the enterprise-catalog API if not present in the cache,
@@ -214,7 +215,8 @@ def get_content_metadata(enterprise_customer_uuid, content_identifier):
 
         course_details = EnterpriseCatalogApiClient().get_content_metadata_for_customer(
             enterprise_customer_uuid,
-            content_identifier
+            content_identifier,
+            **kwargs,
         )
         if course_details:
             TieredCache.set_all_tiers(

enterprise_subsidy/apps/subsidy/migrations/0019_alter_historicalsubsidy_options.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.2.19 on 2023-10-31 13:02
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('subsidy', '0018_alter_historicalsubsidy_options'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='historicalsubsidy',
+            options={'get_latest_by': 'history_date', 'ordering': ('-history_date', '-history_id'), 'verbose_name': 'historical subsidy'},
+        ),
+    ]

enterprise_subsidy/apps/subsidy/migrations/0020_remove_subsidy_unique_reference_id_non_internal.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.2.19 on 2023-10-31 13:04
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('subsidy', '0019_alter_historicalsubsidy_options'),
+    ]
+
+    operations = [
+        migrations.RemoveConstraint(
+            model_name='subsidy',
+            name='unique_reference_id_non_internal',
+        ),
+    ]

enterprise_subsidy/apps/subsidy/models.py

@@ -13,8 +13,8 @@
 from unittest import mock
 from uuid import uuid4
 
+from django.core.exceptions import ValidationError
 from django.db import models
-from django.db.models import Q
 from django.utils.functional import cached_property
 from edx_rbac.models import UserRole, UserRoleAssignment
 from edx_rbac.utils import ALL_ACCESS_CONTEXT
@@ -105,13 +105,6 @@ class Meta:
         """
         Metaclass for Subsidy.
         """
-        constraints = [
-            models.UniqueConstraint(
-                condition=Q(internal_only=False),  # Allow more flexibility for internal/test subsidies.
-                fields=["reference_id", "reference_type"],
-                name="unique_reference_id_non_internal",
-            )
-        ]
         ordering = ['-created']
 
     # Please reserve the "subsidy_type" field name for the future when we use it to distinguish between
@@ -219,6 +212,30 @@ class Meta:
     objects = ActiveSubsidyManager()
     all_objects = models.Manager()
 
+    def clean(self):
+        """
+        Ensures that non-internal-only subsidies are unique
+        on (reference_id, reference_type).  This is necessary
+        because MySQL does not support conditional unique constraints.
+        """
+        if not self.internal_only:
+            other_record = Subsidy.objects.filter(
+                reference_id=self.reference_id,
+                reference_type=self.reference_type,
+            ).exclude(uuid=self.uuid).first()
+            if other_record:
+                raise ValidationError(
+                    f'Subsidy {other_record.uuid} already exists with the same '
+                    f'reference_id {self.reference_id} and reference_type {self.reference_type}'
+                )
+
+    def save(self, *args, **kwargs):
+        """
+        Overrides default save() method to run full_clean.
+        """
+        self.full_clean()
+        super().save(*args, **kwargs)
+
     @property
     def is_active(self):
         """
@@ -268,7 +285,7 @@ def email_for_learner(self, lms_user_id):
         if isinstance(user_data, dict):
             return user_data.get('email')
         return None
-    
+
     def title_for_content(self, content_key):
         """
         Best effort return the title of the given content.
@@ -278,15 +295,18 @@ def title_for_content(self, content_key):
         """
         content_title = None
         try:
-            content_summary = self.content_metadata_api().get_content_summary(self.enterprise_customer_uuid, content_key)
+            content_summary = self.content_metadata_api().get_content_summary(
+                self.enterprise_customer_uuid,
+                content_key
+            )
             if content_summary:
-                    content_title = content_summary.get('title')
+                content_title = content_summary.get('content_title')
         except HTTPError as exc:
             if exc.response.status_code == status.HTTP_404_NOT_FOUND:
                 raise ContentNotFoundForCustomerException(
                     'The given content_key is not in any catalog for this customer.'
                 ) from exc
-        return content_title    
+        return content_title
 
     def price_for_content(self, content_key):
         """

enterprise_subsidy/apps/subsidy/tests/test_api.py

@@ -1,11 +1,13 @@
 """
 Tests for functions defined in the ``api.py`` module.
 """
+from datetime import timedelta
 from unittest import mock
 from uuid import uuid4
 
 import pytest
 from django.test import TestCase
+from django.utils import timezone
 from openedx_ledger.models import Reversal, TransactionStateChoices, UnitChoices
 from openedx_ledger.test_utils.factories import TransactionFactory
 
@@ -25,8 +27,8 @@ def learner_credit_fixture():
         default_enterprise_customer_uuid=uuid4(),
         default_unit=UnitChoices.USD_CENTS,
         default_starting_balance=1000000,
-        default_active_datetime=None,
-        default_expiration_datetime=None,
+        default_active_datetime=timezone.now() - timedelta(days=365),
+        default_expiration_datetime=timezone.now() + timedelta(days=365),
     )
     return subsidy
 
@@ -72,8 +74,8 @@ def test_create_internal_only_subsidy_record(learner_credit_fixture):  # pylint:
         default_enterprise_customer_uuid=other_customer_uuid,
         default_unit=UnitChoices.USD_CENTS,
         default_starting_balance=42,
-        default_active_datetime=None,
-        default_expiration_datetime=None,
+        default_active_datetime=timezone.now() - timedelta(days=365),
+        default_expiration_datetime=timezone.now() + timedelta(days=365),
         default_internal_only=True
     )
     assert created