fix: enrollment intention saves should not be blocked on catalog incl…

…usion

Before this commit, normal replication delays from discovery->catalog
can easily result in enrollment intentions being un-saveable when
simultaneously modifying catalog definitions to include the content.
Conversely, normal replication delays can also allow saving an
enrollment intention that would eventually become invalid due to content
being removed from the catalog, creating a false sense of correctness.

After this commit, we no longer assert during intention save() that the
customer's catalogs actually contain the content. Instead, we assert
that the content exists at all in the enterprise-catalog database by
querying the customer- and catalog-agnostic
`/api/v1/content-metadata/?content_identifiers=<content_key>` endpoint.

Subsequent commits/PRs may introduce dynamic warning messaging
(non-blocking) on django admin pages for the customer and intention when
the intention's content key is not contained in the customer's catalogs.

ENT-9840

docs/decisions/0015-default-enrollments.rst

@@ -65,7 +65,7 @@ will always primarily be associated with the course run associated with the ``De
 * If the content_key is a specific course run, we'll always try to enroll in the explicitly
   configured course run key (not the advertised course run).
 
-This content_key will be passed to the ``EnterpriseCatalogApiClient().get_content_metadata_content_identifier``
+This content_key will be passed to the ``EnterpriseCatalogApiClient().get_customer_content_metadata_content_identifier``
 method. When passing a course run key to this endpoint, it'll actually return the top-level *course* metadata
 associated with the course run, not just the specified course run's metadata
 (this is primarily due to catalogs containing courses, not course runs, and we generally say that

enterprise/api_client/discovery.py

@@ -283,7 +283,7 @@ def get_course_run(self, course_run_id):
             course_run_id(string): Course run ID (aka Course Key) in string format.
 
         Returns:
-            dict: Course run data provided by Course Catalog API.
+            dict: Course run data provided by Course Catalog API, or empty dict if not found.
 
         """
         return self._load_data(

enterprise/api_client/enterprise_catalog.py

@@ -29,8 +29,9 @@ class EnterpriseCatalogApiClient(UserAPIClient):
     REFRESH_CATALOG_ENDPOINT = ENTERPRISE_CATALOG_ENDPOINT + '/{}/refresh_metadata'
     CATALOG_DIFF_ENDPOINT = ENTERPRISE_CATALOG_ENDPOINT + '/{}/generate_diff'
     ENTERPRISE_CUSTOMER_ENDPOINT = 'enterprise-customer'
-    CONTENT_METADATA_IDENTIFIER_ENDPOINT = ENTERPRISE_CUSTOMER_ENDPOINT + \
+    CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT = ENTERPRISE_CUSTOMER_ENDPOINT + \
         "/{}/content-metadata/" + "{}"
+    CONTENT_METADATA_IDENTIFIER_ENDPOINT = "content-metadata/"
     CATALOG_QUERIES_ENDPOINT = 'catalog-queries'
     GET_CONTENT_FILTER_HASH_ENDPOINT = CATALOG_QUERIES_ENDPOINT + '/get_content_filter_hash'
     GET_QUERY_BY_HASH_ENDPOINT = CATALOG_QUERIES_ENDPOINT + '/get_query_by_hash?hash={}'
@@ -386,14 +387,14 @@ def enterprise_contains_content_items(self, enterprise_uuid, content_ids):
         return response.json()
 
     @UserAPIClient.refresh_token
-    def get_content_metadata_content_identifier(self, enterprise_uuid, content_id):
+    def get_customer_content_metadata_content_identifier(self, enterprise_uuid, content_id):
         """
         Return all content metadata contained in the catalogs associated with the
         given EnterpriseCustomer and content_id.
         """
         try:
             api_url = self.get_api_url(
-                f"{self.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(enterprise_uuid, content_id)}"
+                f"{self.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(enterprise_uuid, content_id)}"
             )
             response = self.client.get(api_url)
             response.raise_for_status()
@@ -406,6 +407,37 @@ def get_content_metadata_content_identifier(self, enterprise_uuid, content_id):
                 str(exc),
             )
             return {}
+        except (RequestException, ConnectionError, Timeout) as exc:
+            LOGGER.exception(
+                (
+                    "Exception raised in "
+                    "EnterpriseCatalogApiClient::get_customer_content_metadata_content_identifier: [%s]"
+                ),
+                str(exc),
+            )
+            return {}
+
+    @UserAPIClient.refresh_token
+    def get_content_metadata_content_identifier(self, content_id):
+        """
+        Return the content metadata for the content_id.
+        """
+        try:
+            api_url = self.get_api_url(self.CONTENT_METADATA_IDENTIFIER_ENDPOINT)
+            query_params = {"content_identifiers": content_id}
+            response = self.client.get(
+                api_url,
+                params=query_params,
+            )
+            response.raise_for_status()
+            return response.json()
+        except NotFound as exc:
+            LOGGER.exception(
+                "No matching content found in catalog for content_id: [%s], Error: %s",
+                content_id,
+                str(exc),
+            )
+            return {}
         except (RequestException, ConnectionError, Timeout) as exc:
             LOGGER.exception(
                 "Exception raised in EnterpriseCatalogApiClient::get_content_metadata_content_identifier: [%s]",

enterprise/content_metadata/api.py

@@ -16,6 +16,44 @@
 DEFAULT_CACHE_TIMEOUT = getattr(settings, 'CONTENT_METADATA_CACHE_TIMEOUT', 60 * 5)
 
 
+def get_and_cache_content_metadata(content_key, timeout=None):
+    """
+    Returns the metadata corresponding to the requested ``content_key``
+    REGARDLESS of catalog/customer associations.
+
+    The response is cached in a ``TieredCache`` (meaning in both the RequestCache,
+    _and_ the django cache for the configured expiration period).
+
+    Returns: A dict with content metadata for the given key.
+    Raises: An HTTPError if there's a problem getting the content metadata
+      via the enterprise-catalog service.
+    """
+    cache_key = versioned_cache_key('get_content_metadata_content_identifier', content_key)
+    cached_response = TieredCache.get_cached_response(cache_key)
+    if cached_response.is_found:
+        logger.info(f'cache hit for content_key {content_key}')
+        return cached_response.value
+
+    try:
+        result = EnterpriseCatalogApiClient().get_content_metadata_content_identifier(
+            content_id=content_key,
+        )
+    except HTTPError as exc:
+        raise exc
+
+    if not result:
+        logger.warning('No content found for content_key %s', content_key)
+        return {}
+
+    logger.info(
+        'Fetched catalog for content_key %s. Result = %s',
+        content_key,
+        result,
+    )
+    TieredCache.set_all_tiers(cache_key, result, timeout or DEFAULT_CACHE_TIMEOUT)
+    return result
+
+
 def get_and_cache_customer_content_metadata(enterprise_customer_uuid, content_key, timeout=None):
     """
     Returns the metadata corresponding to the requested
@@ -28,14 +66,18 @@ def get_and_cache_customer_content_metadata(enterprise_customer_uuid, content_ke
     Raises: An HTTPError if there's a problem getting the content metadata
       via the enterprise-catalog service.
     """
-    cache_key = versioned_cache_key('get_content_metadata_content_identifier', enterprise_customer_uuid, content_key)
+    cache_key = versioned_cache_key(
+        'get_customer_content_metadata_content_identifier',
+        enterprise_customer_uuid,
+        content_key,
+    )
     cached_response = TieredCache.get_cached_response(cache_key)
     if cached_response.is_found:
         logger.info(f'cache hit for enterprise customer {enterprise_customer_uuid} and content {content_key}')
         return cached_response.value
 
     try:
-        result = EnterpriseCatalogApiClient().get_content_metadata_content_identifier(
+        result = EnterpriseCatalogApiClient().get_customer_content_metadata_content_identifier(
             enterprise_uuid=enterprise_customer_uuid,
             content_id=content_key,
         )

enterprise/models.py

@@ -62,7 +62,7 @@
     json_serialized_course_modes,
 )
 from enterprise.content_metadata.api import (
-    get_and_cache_customer_content_metadata,
+    get_and_cache_content_metadata,
     get_and_cache_enterprise_contains_content_items,
 )
 from enterprise.errors import LinkUserToEnterpriseError
@@ -2541,11 +2541,16 @@ class Meta:
     @property
     def content_metadata_for_content_key(self):
         """
-        Retrieves the content metadata for the instance's enterprise customer and content key.
+        Retrieves the content metadata for the instance's content key.
+
+        NOTE (ENT-9840): Prior versions of this method used `get_and_cache_customer_content_metadata()` instead of
+        `get_and_cache_content_metadata()`. The goal was to ensure that model saves only succeed when the requested
+        content is actually contained in the customer's catalogs.  However, as part of ENT-9840 we are relaxing this
+        requirement because delays in discovery->catalog replication can easily result in default intentions being
+        un-saveable when simultaneously modifying catalog definitions to include the content.
         """
         try:
-            return get_and_cache_customer_content_metadata(
-                enterprise_customer_uuid=self.enterprise_customer.uuid,
+            return get_and_cache_content_metadata(
                 content_key=self.content_key,
             )
         except HTTPError as e:
@@ -2638,6 +2643,8 @@ def applicable_enterprise_catalog_uuids(self):
             enterprise_customer_uuid=self.enterprise_customer.uuid,
             content_keys=[self.course_run_key],
         )
+        if not contains_content_items_response.get('contains_content_items'):
+            return []
         return contains_content_items_response.get('catalog_list', [])
 
     def determine_content_type(self):
@@ -2693,6 +2700,11 @@ def clean(self):
         if not self.course_run:
             # NOTE: This validation check also acts as an inferred check on the derived content_type
             # from the content metadata.
+            # NOTE 2: This check does NOT assert that the content is actually contained in the
+            # customer's catalogs. ADR 0015, as written, would _like_ for that check to exist in
+            # this clean() method, but that has proven infeasible due to the nature of how data
+            # replication delays from discovery->catalog cause contains_content_items calls to be
+            # temporarily unreliable. Instead this only checks that the content key exists at all.
             raise ValidationError({
                 'content_key': _('The content key did not resolve to a valid course run.')
             })

integrated_channels/degreed2/client.py

@@ -349,7 +349,7 @@ def _fetch_and_assign_skills_to_course(self, external_id):
         # We need to do 2 steps here:
         # 1. Fetch skills from enterprise-catalog
 
-        metadata = self.enterprise_catalog_api_client.get_content_metadata_content_identifier(
+        metadata = self.enterprise_catalog_api_client.get_customer_content_metadata_content_identifier(
             enterprise_uuid=self.enterprise_configuration.enterprise_customer.uuid,
             content_id=external_id)
         LOGGER.info(

tests/test_enterprise/api/test_default_enrollment_views.py

@@ -200,7 +200,6 @@ def test_default_enterprise_enrollment_intentions_not_in_catalog(self, mock_cata
         enrollment_intention = create_mock_default_enterprise_enrollment_intention(
             enterprise_customer=self.enterprise_customer,
             mock_catalog_api_client=mock_catalog_api_client,
-            contains_content_items=False,
             catalog_list=[],
         )
         query_params = f'enterprise_customer_uuid={str(self.enterprise_customer.uuid)}'
@@ -370,7 +369,6 @@ def test_default_enrollment_intentions_learner_status_content_not_in_catalog(
         enrollment_intention = create_mock_default_enterprise_enrollment_intention(
             enterprise_customer=self.enterprise_customer,
             mock_catalog_api_client=mock_catalog_api_client,
-            contains_content_items=False,
             catalog_list=[],
         )
         mock_get_best_mode_from_course_key.return_value = VERIFIED_COURSE_MODE

tests/test_enterprise/api/test_views.py

@@ -16,6 +16,7 @@
 import ddt
 import pytz
 import responses
+from edx_django_utils.cache import TieredCache
 from edx_toggles.toggles.testutils import override_waffle_flag
 from faker import Faker
 from oauth2_provider.models import get_application_model
@@ -177,19 +178,18 @@ def create_mock_default_enterprise_enrollment_intention(
     enterprise_customer,
     mock_catalog_api_client,
     content_metadata=None,
-    contains_content_items=False,
     catalog_list=None,
 ):
     """
     Create a mock default enterprise enrollment intention.
     """
     mock_content_metadata = content_metadata or fake_catalog_api.FAKE_COURSE
-    mock_contains_content_items = contains_content_items
     mock_catalog_list = (
         catalog_list
         if catalog_list is not None
         else [fake_catalog_api.FAKE_CATALOG_RESULT.get('uuid')]
     )
+    mock_contains_content_items = bool(mock_catalog_list)
 
     mock_catalog_api_client.return_value = mock.Mock(
         get_content_metadata_content_identifier=mock.Mock(
@@ -239,6 +239,13 @@ def setUp(self):
         super().setUp()
         self.set_jwt_cookie(ENTERPRISE_OPERATOR_ROLE, ALL_ACCESS_CONTEXT)
 
+    def tearDown(self):
+        """
+        Clears TieredCache so that mock catalog API calls do not bleed across tests.
+        """
+        super().tearDown()
+        TieredCache.dangerous_clear_all_tiers()
+
     @staticmethod
     def create_user(username=TEST_USERNAME, password=TEST_PASSWORD, **kwargs):
         # AED 2024-06-12: For simplicity, I've tried to refactor the test setup in the base APITest

tests/test_integrated_channels/test_degreed2/test_client.py

@@ -256,7 +256,7 @@ def test_create_content_metadata_success(self):
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},
@@ -316,7 +316,7 @@ def test_create_content_metadata_retry_success(self):
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},
@@ -422,7 +422,7 @@ def test_create_content_metadata_course_exists(self):
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},
@@ -479,7 +479,7 @@ def test_update_content_metadata_success(self, mock_fetch_degreed_course_id):
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},
@@ -651,7 +651,7 @@ def test_update_content_metadata_retry_success(self, mock_fetch_degreed_course_i
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},
@@ -730,7 +730,7 @@ def test_update_content_metadata_retry_exhaust(self, mock_fetch_degreed_course_i
         responses.add(
             responses.GET,
             EnterpriseCatalogApiClient.API_BASE_URL
-            + EnterpriseCatalogApiClient.CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
+            + EnterpriseCatalogApiClient.CUSTOMER_CONTENT_METADATA_IDENTIFIER_ENDPOINT.format(
                 enterprise_config.enterprise_customer.uuid, "key/"
             ),
             json={"skill_names": ["Supply Chain", "Supply Chain Management"]},

tests/test_models.py

@@ -247,11 +247,11 @@ def tearDown(self):
         DefaultEnterpriseEnrollmentIntention.objects.all().delete()
 
     @mock.patch(
-        'enterprise.models.get_and_cache_customer_content_metadata',
+        'enterprise.models.get_and_cache_content_metadata',
         return_value=mock.MagicMock(),
     )
-    def test_retrieve_course_run_advertised_course_run(self, mock_get_and_cache_customer_content_metadata):
-        mock_get_and_cache_customer_content_metadata.return_value = self.mock_course
+    def test_retrieve_course_run_advertised_course_run(self, mock_get_and_cache_content_metadata):
+        mock_get_and_cache_content_metadata.return_value = self.mock_course
         default_enterprise_enrollment_intention = DefaultEnterpriseEnrollmentIntention.objects.create(
             enterprise_customer=self.test_enterprise_customer_1,
             content_key='edX+DemoX',
@@ -262,11 +262,11 @@ def test_retrieve_course_run_advertised_course_run(self, mock_get_and_cache_cust
         assert default_enterprise_enrollment_intention.content_type == DefaultEnterpriseEnrollmentIntention.COURSE
 
     @mock.patch(
-        'enterprise.models.get_and_cache_customer_content_metadata',
+        'enterprise.models.get_and_cache_content_metadata',
         return_value=mock.MagicMock(),
     )
-    def test_retrieve_course_run_with_explicit_course_run(self, mock_get_and_cache_customer_content_metadata):
-        mock_get_and_cache_customer_content_metadata.return_value = self.mock_course
+    def test_retrieve_course_run_with_explicit_course_run(self, mock_get_and_cache_content_metadata):
+        mock_get_and_cache_content_metadata.return_value = self.mock_course
         default_enterprise_enrollment_intention = DefaultEnterpriseEnrollmentIntention.objects.create(
             enterprise_customer=self.test_enterprise_customer_1,
             content_key='course-v1:edX+DemoX+2T2023',
@@ -277,12 +277,12 @@ def test_retrieve_course_run_with_explicit_course_run(self, mock_get_and_cache_c
         assert default_enterprise_enrollment_intention.content_type == DefaultEnterpriseEnrollmentIntention.COURSE_RUN
 
     @mock.patch(
-        'enterprise.models.get_and_cache_customer_content_metadata',
+        'enterprise.models.get_and_cache_content_metadata',
         return_value=mock.MagicMock(),
     )
-    def test_validate_missing_course_run(self, mock_get_and_cache_customer_content_metadata):
+    def test_validate_missing_course_run(self, mock_get_and_cache_content_metadata):
         # Simulate a 404 Not Found by raising an HTTPError exception
-        mock_get_and_cache_customer_content_metadata.side_effect = HTTPError
+        mock_get_and_cache_content_metadata.side_effect = HTTPError
         with self.assertRaises(ValidationError) as exc_info:
             DefaultEnterpriseEnrollmentIntention.objects.create(
                 enterprise_customer=self.test_enterprise_customer_1,
@@ -291,12 +291,12 @@ def test_validate_missing_course_run(self, mock_get_and_cache_customer_content_m
         self.assertIn('The content key did not resolve to a valid course run.', str(exc_info.exception))
 
     @mock.patch(
-        'enterprise.models.get_and_cache_customer_content_metadata',
+        'enterprise.models.get_and_cache_content_metadata',
         return_value=mock.MagicMock(),
     )
     @override_settings(ROOT_URLCONF="test_utils.admin_urls")
-    def test_validate_existing_soft_deleted_record(self, mock_get_and_cache_customer_content_metadata):
-        mock_get_and_cache_customer_content_metadata.return_value = self.mock_course
+    def test_validate_existing_soft_deleted_record(self, mock_get_and_cache_content_metadata):
+        mock_get_and_cache_content_metadata.return_value = self.mock_course
         existing_record = DefaultEnterpriseEnrollmentIntention.objects.create(
             enterprise_customer=self.test_enterprise_customer_1,
             content_key='edX+DemoX',