Skip to content

Commit

Permalink
fix: use coerce_to_parent_course when fetching content for default en…
Browse files Browse the repository at this point in the history
…rollment intentions

ENT-9840
  • Loading branch information
pwnage101 committed Jan 24, 2025
1 parent 215e57a commit 1d696b3
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 12 deletions.
22 changes: 14 additions & 8 deletions enterprise/api_client/enterprise_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
from logging import getLogger
from urllib.parse import urljoin

from requests.exceptions import ConnectionError, RequestException, Timeout # pylint: disable=redefined-builtin
from rest_framework.exceptions import NotFound
from requests.exceptions import ( # pylint: disable=redefined-builtin
ConnectionError,
HTTPError,
RequestException,
Timeout,
)

from django.conf import settings

Expand Down Expand Up @@ -399,7 +403,7 @@ def get_customer_content_metadata_content_identifier(self, enterprise_uuid, cont
response = self.client.get(api_url)
response.raise_for_status()
return response.json()
except NotFound as exc:
except HTTPError as exc:
LOGGER.exception(
"No matching content found in catalog for customer: [%s] or content_id: [%s], Error: %s",
enterprise_uuid,
Expand All @@ -418,22 +422,24 @@ def get_customer_content_metadata_content_identifier(self, enterprise_uuid, cont
return {}

@UserAPIClient.refresh_token
def get_content_metadata_content_identifier(self, content_id):
def get_content_metadata_content_identifier(self, content_id, coerce_to_parent_course=False):
"""
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}
query_params = {}
if coerce_to_parent_course:
query_params["coerce_to_parent_course"] = True
response = self.client.get(
api_url,
api_url + content_id,
params=query_params,
)
response.raise_for_status()
return response.json()
except NotFound as exc:
except HTTPError as exc:
LOGGER.exception(
"No matching content found in catalog for content_id: [%s], Error: %s",
"No matching content found for content_id: [%s], Error: %s",
content_id,
str(exc),
)
Expand Down
10 changes: 6 additions & 4 deletions enterprise/content_metadata/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
DEFAULT_CACHE_TIMEOUT = getattr(settings, 'CONTENT_METADATA_CACHE_TIMEOUT', 60 * 5)


def get_and_cache_content_metadata(content_key, timeout=None):
def get_and_cache_content_metadata(content_key, timeout=None, coerce_to_parent_course=False):
"""
Returns the metadata corresponding to the requested ``content_key``
REGARDLESS of catalog/customer associations.
Expand All @@ -28,15 +28,16 @@ def get_and_cache_content_metadata(content_key, timeout=None):
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)
cache_key = versioned_cache_key('get_content_metadata_content_identifier', content_key, coerce_to_parent_course)
cached_response = TieredCache.get_cached_response(cache_key)
if cached_response.is_found:
logger.info(f'cache hit for content_key {content_key}')
logger.info(f'cache hit for content_key {content_key} and coerce_to_parent_course={coerce_to_parent_course}')
return cached_response.value

try:
result = EnterpriseCatalogApiClient().get_content_metadata_content_identifier(
content_id=content_key,
coerce_to_parent_course=coerce_to_parent_course,
)
except HTTPError as exc:
raise exc

Check warning on line 43 in enterprise/content_metadata/api.py

View check run for this annotation

Codecov / codecov/patch

enterprise/content_metadata/api.py#L42-L43

Added lines #L42 - L43 were not covered by tests
Expand All @@ -46,8 +47,9 @@ def get_and_cache_content_metadata(content_key, timeout=None):
return {}

Check warning on line 47 in enterprise/content_metadata/api.py

View check run for this annotation

Codecov / codecov/patch

enterprise/content_metadata/api.py#L46-L47

Added lines #L46 - L47 were not covered by tests

logger.info(
'Fetched catalog for content_key %s. Result = %s',
'Fetched catalog for content_key %s and coerce_to_parent_course=%s. Result = %s',
content_key,
coerce_to_parent_course,
result,
)
TieredCache.set_all_tiers(cache_key, result, timeout or DEFAULT_CACHE_TIMEOUT)
Expand Down
1 change: 1 addition & 0 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2554,6 +2554,7 @@ def content_metadata_for_content_key(self):
try:
return get_and_cache_content_metadata(
content_key=self.content_key,
coerce_to_parent_course=True,
)
except HTTPError as e:
LOGGER.error(
Expand Down
117 changes: 117 additions & 0 deletions tests/test_enterprise/api_client/test_enterprise_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import responses
from pytest import mark, raises
from requests.exceptions import ConnectionError, RequestException, Timeout # pylint: disable=redefined-builtin
from testfixtures import LogCapture

from enterprise.api_client import enterprise_catalog
from enterprise.models import EnterpriseCustomerCatalog
Expand Down Expand Up @@ -652,3 +653,119 @@ def test_get_content_metadata_without_enterprise_catalogs():

request_url = responses.calls[0][0].url
assert url == request_url


@responses.activate
@mark.django_db
@mock.patch('enterprise.api_client.client.JwtBuilder', mock.Mock())
def test_get_customer_content_metadata_content_identifier():
responses.reset()
client = enterprise_catalog.EnterpriseCatalogApiClient('staff-user-goes-here')

# Set up a mock response for requesting a course (via run key).
url = _url(f"enterprise-customer/{TEST_ENTERPRISE_ID}/content-metadata/course-v1:org+course+run/")
expected_response = {
'content_type': 'course',
'key': 'org+course',
'data': 'foo',
}
responses.add(responses.GET, url, json=expected_response)

# Set up a mock response for requesting content that does not exist.
url = _url(f"enterprise-customer/{TEST_ENTERPRISE_ID}/content-metadata/course-v1:does+not+exist/")
responses.add(responses.GET, url, status=404)

# Test requesting the course (via run key) directly.
results = client.get_customer_content_metadata_content_identifier(TEST_ENTERPRISE_ID, 'course-v1:org+course+run')
expected_results = {
'content_type': 'course',
'key': 'org+course',
'data': 'foo',
}
assert results == expected_results

# Test requesting non-existing content.
with LogCapture(level=logging.INFO) as log_capture:
results = client.get_customer_content_metadata_content_identifier(
TEST_ENTERPRISE_ID,
'course-v1:does+not+exist',
)
expected_results = {}
assert results == expected_results
logging_messages = [log_msg.getMessage() for log_msg in log_capture.records]
assert any('No matching content found' in message for message in logging_messages)

# Test making a request, but the connection fails.
with LogCapture(level=logging.INFO) as log_capture:
results = client.get_customer_content_metadata_content_identifier(
TEST_ENTERPRISE_ID,
'course-v1:does+not+match',
)
expected_results = {}
assert results == expected_results
logging_messages = [log_msg.getMessage() for log_msg in log_capture.records]
assert any('Exception raised in' in message for message in logging_messages)


@responses.activate
@mark.django_db
@mock.patch('enterprise.api_client.client.JwtBuilder', mock.Mock())
def test_get_content_metadata_content_identifier():
responses.reset()
client = enterprise_catalog.EnterpriseCatalogApiClient('staff-user-goes-here')

# Set up a mock response for requesting a run directly.
url = _url('content-metadata/course-v1:org+course+run')
expected_response = {
'content_type': 'courserun',
'key': 'course-v1:org+course+run',
'data': 'foo',
}
responses.add(responses.GET, url, json=expected_response)

# Set up a mock response for requesting a run, but asking it to be coerced to a parent course.
url = _url('content-metadata/course-v1:org+course+run/?coerce_to_parent_course=true')
expected_response = {
'content_type': 'course',
'key': 'org+course',
'data': 'foo',
}
responses.add(responses.GET, url, json=expected_response)

# Set up a mock response for requesting content that does not exist.
url = _url('content-metadata/course-v1:does+not+exist')
responses.add(responses.GET, url, status=404)

# Test requesting the run directly.
results = client.get_content_metadata_content_identifier('course-v1:org+course+run')
expected_results = {
'content_type': 'courserun',
'key': 'course-v1:org+course+run',
'data': 'foo',
}
assert results == expected_results

# Test requesting the run, asking it to be coerced to a parent course.
results = client.get_content_metadata_content_identifier('course-v1:org+course+run', coerce_to_parent_course=True)
expected_results = {
'content_type': 'courserun',
'key': 'course-v1:org+course+run',
'data': 'foo',
}
assert results == expected_results

# Test requesting non-existing content.
with LogCapture(level=logging.INFO) as log_capture:
results = client.get_content_metadata_content_identifier('course-v1:does+not+exist')
expected_results = {}
assert results == expected_results
logging_messages = [log_msg.getMessage() for log_msg in log_capture.records]
assert any('No matching content found' in message for message in logging_messages)

# Test making a request, but the connection fails.
with LogCapture(level=logging.INFO) as log_capture:
results = client.get_content_metadata_content_identifier('course-v1:does+not+match')
expected_results = {}
assert results == expected_results
logging_messages = [log_msg.getMessage() for log_msg in log_capture.records]
assert any('Exception raised in' in message for message in logging_messages)

0 comments on commit 1d696b3

Please sign in to comment.