feat: list_price is always populated in can_redeem

enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py

@@ -1784,6 +1784,13 @@ def setup_mocks(self):
         lms_client_instance = lms_client.return_value
         lms_client_instance.get_enterprise_user.return_value = TEST_USER_RECORD
 
+        catalog_get_and_cache_content_metadata_patcher = mock.patch(
+            'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
+        )
+        self.mock_catalog_get_and_cache_content_metadata = catalog_get_and_cache_content_metadata_patcher.start()
+        self.mock_catalog_get_and_cache_content_metadata.return_value = {}
+
+        self.addCleanup(catalog_get_and_cache_content_metadata_patcher.stop)
         self.addCleanup(lms_client_patcher.stop)
         self.addCleanup(subsidy_client_patcher.stop)
         self.addCleanup(contains_key_patcher.stop)
@@ -1956,6 +1963,12 @@ def test_can_redeem_policy_none_redeemable(
             'unit': 'usd_cents',
             'all_transactions': [],
         }
+        self.mock_catalog_get_and_cache_content_metadata.return_value = {
+            'normalized_metadata': {
+                'content_price': 50,  # normalized_metadata always serializes USD.
+            },
+            'normalized_metadata_by_run': {},
+        }
         test_content_key_1 = "course-v1:edX+Privacy101+3T2020"
         test_content_key_2 = "course-v1:edX+Privacy101+3T2020_2"
         test_content_key_1_metadata_price = 29900
@@ -1996,9 +2009,9 @@ def mock_get_subsidy_content_data(*args, **kwargs):
 
         # Check the response for the first content_key given.
         assert response_list[0]["content_key"] == test_content_key_1
-        # We should not assume that a list price is fetchable if the
-        # content cant' be redeemed - the content may not be in any catalog for any policy.
-        assert response_list[0]["list_price"] is None
+        # When there's no redeemable policy, the price returned is the current
+        # fixed price fetched directly from catalog.
+        assert response_list[0]["list_price"] == {'usd': 50.0, 'usd_cents': 5000}
         assert len(response_list[0]["redemptions"]) == 0
         assert response_list[0]["has_successful_redemption"] is False
         assert response_list[0]["redeemable_subsidy_access_policy"] is None
@@ -2031,7 +2044,7 @@ def mock_get_subsidy_content_data(*args, **kwargs):
 
         # Check the response for the second content_key given.
         assert response_list[1]["content_key"] == test_content_key_2
-        assert response_list[1]["list_price"] is None
+        assert response_list[1]["list_price"] == {'usd': 50.0, 'usd_cents': 5000}
 
         assert len(response_list[1]["redemptions"]) == 0
         assert response_list[1]["has_successful_redemption"] is False
@@ -2263,6 +2276,12 @@ def test_can_redeem_policy_beyond_enrollment_deadline(self, mock_lms_client, moc
             "content_price": 1234,
             "enroll_by_date": '2001-01-01T00:00:00Z',
         }
+        self.mock_catalog_get_and_cache_content_metadata.return_value = {
+            'normalized_metadata': {
+                'content_price': 12.34,  # normalized_metadata always serializes USD.
+            },
+            'normalized_metadata_by_run': {},
+        }
 
         with mock.patch(
             'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
@@ -2368,6 +2387,13 @@ def test_can_redeem_lms_user_id_override_for_staff(
             'content_price': 19900,
         }
         self.mock_get_content_metadata.return_value = mock_subsidy_content_data
+        self.mock_catalog_get_and_cache_content_metadata.return_value = {
+            'normalized_metadata': {
+                'content_price': 199.00,  # normalized_metadata always serializes USD.
+            },
+            'normalized_metadata_by_run': {},
+        }
+
         query_params = {'content_key': test_content_key}
         if lms_user_id_override:
             query_params['lms_user_id'] = lms_user_id_override
@@ -2793,6 +2819,12 @@ def test_can_redeem_no_assignment_for_content(
             "content_price": content_key_for_redemption_metadata_price,
         }
         self.mock_get_content_metadata.return_value = mock_get_subsidy_content_data
+        self.mock_catalog_get_and_cache_content_metadata.return_value = {
+            'normalized_metadata': {
+                'content_price': content_key_for_redemption_metadata_price / 100,
+            },
+            'normalized_metadata_by_run': {},
+        }
 
         # It's an unredeemable response, so mock out some admin users to return
         mock_lms_client.return_value.get_enterprise_customer_data.return_value = {
@@ -2816,7 +2848,10 @@ def test_can_redeem_no_assignment_for_content(
 
         # Check the response for the first content_key given.
         assert response_list[0]["content_key"] == content_key_for_redemption
-        assert response_list[0]["list_price"] is None
+        assert response_list[0]["list_price"] == {
+            'usd': content_key_for_redemption_metadata_price / 100.0,
+            'usd_cents': content_key_for_redemption_metadata_price,
+        }
         assert response_list[0]["redemptions"] == []
         assert response_list[0]["has_successful_redemption"] is False
         assert response_list[0]["redeemable_subsidy_access_policy"] is None

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

@@ -27,6 +27,7 @@
 from enterprise_access.apps.api.mixins import UserDetailsFromJwtMixin
 from enterprise_access.apps.api_client.lms_client import LmsApiClient
 from enterprise_access.apps.content_assignments.api import AllocationException
+from enterprise_access.apps.content_metadata.api import get_and_cache_content_metadata
 from enterprise_access.apps.core.constants import (
     SUBSIDY_ACCESS_POLICY_ALLOCATION_PERMISSION,
     SUBSIDY_ACCESS_POLICY_READ_PERMISSION,
@@ -53,6 +54,7 @@
     MissingSubsidyAccessReasonUserMessages,
     TransactionStateChoices
 )
+from enterprise_access.apps.subsidy_access_policy.content_metadata_api import make_list_price_dict
 from enterprise_access.apps.subsidy_access_policy.exceptions import (
     ContentPriceNullException,
     MissingAssignment,
@@ -744,13 +746,18 @@ def can_redeem(self, request, enterprise_customer_uuid):
             redeemable_policies = []
             non_redeemable_policies = []
             resolved_policy = None
-            list_price = None
+            list_price_dict = None
 
-            redemptions_by_policy_uuid = redemptions_by_content_and_policy[content_key]
+            redemptions_by_policy = redemptions_by_content_and_policy[content_key]
+            policy_by_redemption_uuid = {
+                redemption['uuid']: policy
+                for policy, redemptions in redemptions_by_policy.items()
+                for redemption in redemptions
+            }
             # Flatten dict of lists because the response doesn't need to be bucketed by policy_uuid.
             redemptions = [
                 redemption
-                for redemptions in redemptions_by_policy_uuid.values()
+                for redemptions in redemptions_by_policy.values()
                 for redemption in redemptions
             ]
 
@@ -787,27 +794,40 @@ def can_redeem(self, request, enterprise_customer_uuid):
 
             try:
                 if resolved_policy:
-                    list_price = resolved_policy.get_list_price(lms_user_id, content_key)
+                    list_price_dict = resolved_policy.get_list_price(lms_user_id, content_key)
                 elif successful_redemptions:
-                    # Get the policy record used at time of successful redemption.
-                    # [2023-12-05] TODO: consider cleaning this up.
-                    # This is kind of silly, b/c we only need this policy to compute the
-                    # list price, and it's really only *necessary* to fetch that price
-                    # from within the context of a *policy record* for cases where that successful
-                    # policy was assignment-based (because the list price for assignments might
-                    # slightly different from the current list price in the canonical content metadata).
-                    successfully_redeemed_policy = self.get_queryset().filter(
-                        uuid=successful_redemptions[0]['subsidy_access_policy_uuid'],
-                    ).first()
-                    list_price = successfully_redeemed_policy.get_list_price(lms_user_id, content_key)
+                    # Get the policy used for redemption and use that to compute the price. If the redemption was the
+                    # result of assignment, the historical assignment price might differ from the canonical price. We
+                    # prefer to display the redeemed price to avoid confusion.
+                    successfully_redeemed_policy = policy_by_redemption_uuid[successful_redemptions[0]['uuid']]
+                    list_price_dict = successfully_redeemed_policy.get_list_price(lms_user_id, content_key)
+                else:
+                    # In the case where the learner cannot redeem and has never redeemed this content, bypass the
+                    # subsidy metadata endpoint and go straight to the source (enterprise-catalog) to find normalized
+                    # price data. In this case, the list price returned rarely actually drives the display price in the
+                    # learner portal frontend, but we still need to maintain a non-null list price for a few reasons:
+                    # * Enterprise customers that leverage the API directly always expect a non-null price.
+                    # * On rare occasion, this price actually does drive the price display in learner-portal.  We think
+                    #   this can happen when courses are searchable and there is an assignment-based policy, but nothing
+                    #   has been assigned.
+                    # * Long-term, we will use can_redeem for all subsidy types, at which point we will also rely on
+                    #   this list_price for price display 100% of the time.
+                    course_metadata = get_and_cache_content_metadata(content_key, coerce_to_parent_course=True)
+                    decimal_dollars = (
+                        course_metadata['normalized_metadata_by_run'].get(content_key, {}).get('content_price') or
+                        course_metadata['normalized_metadata'].get('content_price')
+                    )
+                    if decimal_dollars is None:
+                        raise ContentPriceNullException('Failed to obtain content price from enterprise-catalog.')
+                    list_price_dict = make_list_price_dict(decimal_dollars=decimal_dollars)
             except ContentPriceNullException as exc:
                 raise RedemptionRequestException(
                     detail=f'Could not determine list price for content_key: {content_key}',
                 ) from exc
 
             element_response = {
                 "content_key": content_key,
-                "list_price": list_price,
+                "list_price": list_price_dict,
                 "redemptions": redemptions,
                 "has_successful_redemption": bool(successful_redemptions),
                 "redeemable_subsidy_access_policy": resolved_policy,

enterprise_access/apps/api_client/enterprise_catalog_client.py

@@ -1,6 +1,8 @@
 """
 API client for enterprise-catalog service.
 """
+from urllib.parse import urljoin
+
 import backoff
 from django.conf import settings
 
@@ -10,10 +12,14 @@
 
 class EnterpriseCatalogApiClient(BaseOAuthClient):
     """
-    API client for calls to the enterprise catalog service.
+    V2 API client for calls to the enterprise catalog service.
     """
-    api_base_url = settings.ENTERPRISE_CATALOG_URL + '/api/v2/'
-    enterprise_catalog_endpoint = api_base_url + 'enterprise-catalogs/'
+    api_version = 'v2'
+
+    def __init__(self):
+        self.api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, f'api/{self.api_version}/')
+        self.enterprise_catalog_endpoint = urljoin(self.api_base_url, 'enterprise-catalogs/')
+        super().__init__()
 
     @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions)
     def contains_content_items(self, catalog_uuid, content_ids):
@@ -84,3 +90,37 @@ def get_content_metadata_count(self, catalog_uuid):
         response = self.client.get(endpoint)
         response.raise_for_status()
         return response.json()['count']
+
+    def content_metadata(self, content_id):
+        raise NotImplementedError('There is currently no v2 API implementation for this endpoint.')
+
+
+class EnterpriseCatalogApiV1Client(EnterpriseCatalogApiClient):
+    """
+    V1 API client for calls to the enterprise catalog service.
+    """
+    api_version = 'v1'
+
+    def __init__(self):
+        super().__init__()
+        self.content_metadata_endpoint = urljoin(self.api_base_url, 'content-metadata/')
+
+    @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions)
+    def content_metadata(self, content_id, coerce_to_parent_course=False):
+        """
+        Fetch catalog-/customer-agnostic content metadata.
+
+        Arguments:
+            content_id (str): ID of content to fetch.
+
+        Returns:
+            dict: serialized content metadata, or None if not found.
+        """
+        query_params = {}
+        if coerce_to_parent_course:
+            query_params |= {'coerce_to_parent_course': True}
+        kwargs = {'params': query_params} if query_params else {}
+        endpoint = self.content_metadata_endpoint + content_id
+        response = self.client.get(endpoint, **kwargs)
+        response.raise_for_status()
+        return response.json()

enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py

@@ -5,11 +5,15 @@
 from unittest import mock
 from uuid import uuid4
 
+import ddt
 from django.test import TestCase
 from requests import Response
 from requests.exceptions import HTTPError
 
-from enterprise_access.apps.api_client.enterprise_catalog_client import EnterpriseCatalogApiClient
+from enterprise_access.apps.api_client.enterprise_catalog_client import (
+    EnterpriseCatalogApiClient,
+    EnterpriseCatalogApiV1Client
+)
 
 
 class TestEnterpriseCatalogApiClient(TestCase):
@@ -111,3 +115,56 @@ def test_get_content_metadata_count(self, mock_oauth_client):
         mock_oauth_client.return_value.get.assert_called_with(
             f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{catalog_uuid}/get_content_metadata/',
         )
+
+
+@ddt.ddt
+class TestEnterpriseCatalogApiV1Client(TestCase):
+    """
+    Test EnterpriseCatalogApiV1Client.
+    """
+
+    @ddt.data(
+        {'coerce_to_parent_course': False},
+        {'coerce_to_parent_course': True},
+    )
+    @ddt.unpack
+    @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient')
+    def test_content_metadata(self, mock_oauth_client, coerce_to_parent_course):
+        content_key = 'course+A'
+        mock_response_json = {
+            'key': content_key,
+            'other_metadata': 'foo',
+        }
+
+        request_response = Response()
+        request_response.status_code = 200
+        mock_oauth_client.return_value.get.return_value.json.return_value = mock_response_json
+
+        client = EnterpriseCatalogApiV1Client()
+        fetched_metadata = client.content_metadata(content_key, coerce_to_parent_course=coerce_to_parent_course)
+
+        self.assertEqual(fetched_metadata, mock_response_json)
+        expected_query_params_kwarg = {}
+        if coerce_to_parent_course:
+            expected_query_params_kwarg |= {'params': {'coerce_to_parent_course': True}}
+        mock_oauth_client.return_value.get.assert_called_with(
+            f'http://enterprise-catalog.example.com/api/v1/content-metadata/{content_key}',
+            **expected_query_params_kwarg,
+        )
+
+    @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient')
+    def test_content_metadata_raises_http_error(self, mock_oauth_client):
+        content_key = 'course+A'
+        request_response = Response()
+        request_response.status_code = 400
+
+        mock_oauth_client.return_value.get.return_value = request_response
+
+        client = EnterpriseCatalogApiV1Client()
+
+        with self.assertRaises(HTTPError):
+            client.content_metadata(content_key)
+
+        mock_oauth_client.return_value.get.assert_called_with(
+            f'http://enterprise-catalog.example.com/api/v1/content-metadata/{content_key}',
+        )