Skip to content

Commit

Permalink
Merge pull request #316 from openedx/pwnage101/ENT-9410
Browse files Browse the repository at this point in the history
feat: update get_content_metadata_for_customer to use v2 endpoint
  • Loading branch information
pwnage101 authored Nov 18, 2024
2 parents 8b6bb2a + 736851b commit 3cd53a6
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 21 deletions.
6 changes: 3 additions & 3 deletions enterprise_subsidy/apps/api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from rest_framework.reverse import reverse

from enterprise_subsidy.apps.api.v1.tests.mixins import STATIC_ENTERPRISE_UUID, STATIC_LMS_USER_ID, APITestMixin
from enterprise_subsidy.apps.api_client.enterprise_catalog import EnterpriseCatalogApiClient
from enterprise_subsidy.apps.api_client.enterprise_catalog import EnterpriseCatalogApiClientV2
from enterprise_subsidy.apps.subsidy.constants import SYSTEM_ENTERPRISE_ADMIN_ROLE, SYSTEM_ENTERPRISE_LEARNER_ROLE
from enterprise_subsidy.apps.subsidy.models import RevenueCategoryChoices, Subsidy
from enterprise_subsidy.apps.subsidy.tests.factories import SubsidyFactory
Expand Down Expand Up @@ -1849,7 +1849,7 @@ def test_successful_get(
# Validate that, in the first, non-cached request, we call
# the enterprise catalog endpoint via the client, and that
# a `skip_customer_fetch` parameter is included in the request.
catalog_customer_base = EnterpriseCatalogApiClient().enterprise_customer_endpoint
catalog_customer_base = EnterpriseCatalogApiClientV2().enterprise_customer_endpoint
expected_request_url = f"{catalog_customer_base}{customer_uuid}/content-metadata/{expected_content_key}/"
mock_oauth_client.return_value.get.assert_called_once_with(
expected_request_url,
Expand Down Expand Up @@ -1908,7 +1908,7 @@ def test_failure_exception_while_gather_metadata(self, catalog_status_code, expe

assert response.status_code == catalog_status_code
assert response.json() == expected_response
catalog_customer_endpoint = EnterpriseCatalogApiClient().enterprise_customer_endpoint
catalog_customer_endpoint = EnterpriseCatalogApiClientV2().enterprise_customer_endpoint
expected_request_url = f"{catalog_customer_endpoint}{customer_uuid}/content-metadata/content_key/"
mock_oauth_client.return_value.get.assert_called_once_with(
expected_request_url,
Expand Down
25 changes: 22 additions & 3 deletions enterprise_subsidy/apps/api_client/enterprise_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@ class EnterpriseCatalogApiClient(BaseOAuthClient):
"""
API client for calls to the enterprise service.
"""
api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, 'api/v1/')
metadata_endpoint = urljoin(api_base_url, 'content-metadata/')
enterprise_customer_endpoint = urljoin(api_base_url, 'enterprise-customer/')
api_version = 'v1'

def __init__(self):
self.api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, f'api/{self.api_version}/')
self.metadata_endpoint = urljoin(self.api_base_url, 'content-metadata/')
self.enterprise_customer_endpoint = urljoin(self.api_base_url, 'enterprise-customer/')
super().__init__()

def enterprise_customer_url(self, enterprise_customer_uuid):
return urljoin(
Expand Down Expand Up @@ -92,3 +96,18 @@ def get_content_metadata_for_customer(
f'Failed to fetch enterprise customer data for {enterprise_customer_uuid} because {response.text}',
)
raise exc


class EnterpriseCatalogApiClientV2(EnterpriseCatalogApiClient):
"""
V2 API client for calls to the enterprise service.
Right now this just extends the V1 class to avoid duplicate logic.
"""
api_version = 'v2'

def get_content_metadata(self, content_identifier, **kwargs):
"""
Non-customer-based endpoint does not currently exist with a v2 version.
"""
raise NotImplementedError
15 changes: 5 additions & 10 deletions enterprise_subsidy/apps/content_metadata/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
from django.conf import settings
from edx_django_utils.cache import TieredCache

from enterprise_subsidy.apps.api_client.enterprise_catalog import EnterpriseCatalogApiClient
from enterprise_subsidy.apps.api_client.enterprise_catalog import (
EnterpriseCatalogApiClient,
EnterpriseCatalogApiClientV2
)
from enterprise_subsidy.apps.core.utils import versioned_cache_key
from enterprise_subsidy.apps.subsidy.constants import CENTS_PER_DOLLAR

Expand Down Expand Up @@ -46,14 +49,6 @@ class ContentMetadataApi:
An API for interacting with enterprise catalog content metadata.
"""

def catalog_client(self):
"""
Get a client for access the Enterprise Catalog service API (enterprise-catalog endpoints). This contains
functions used for fetching full content metadata and pricing data on courses. Cached to reduce the chance of
repeated calls to auth.
"""
return EnterpriseCatalogApiClient()

def price_for_content_fallback(self, content_data, course_run_data):
"""
Fallback logic for `price_for_content` logic if the `normalized_metadata_by_run` field is None.
Expand Down Expand Up @@ -311,7 +306,7 @@ def get_content_metadata_for_customer(enterprise_customer_uuid, content_identifi
if cached_response.is_found:
return cached_response.value

course_details = EnterpriseCatalogApiClient().get_content_metadata_for_customer(
course_details = EnterpriseCatalogApiClientV2().get_content_metadata_for_customer(
enterprise_customer_uuid,
content_identifier,
**kwargs,
Expand Down
12 changes: 7 additions & 5 deletions enterprise_subsidy/apps/content_metadata/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,16 +420,18 @@ def test_price_for_content(self, content_data, course_run_data, expected_price):
actual_price = self.content_metadata_api.price_for_content(content_data, course_run_data)
self.assertEqual(expected_price, actual_price)

@mock.patch('enterprise_subsidy.apps.content_metadata.api.EnterpriseCatalogApiClientV2')
@mock.patch('enterprise_subsidy.apps.content_metadata.api.EnterpriseCatalogApiClient')
def test_tiered_caching_works(self, mock_catalog_client):
def test_tiered_caching_works(self, mock_catalog_client_v1, mock_catalog_client_v2):
"""
Tests that consecutive calls for the same content metadata
within the same request utilize the cache.
"""
cache_key = content_metadata_for_customer_cache_key(self.enterprise_customer_uuid, self.course_key)
self.assertFalse(TieredCache.get_cached_response(cache_key).is_found)
client_instance = mock_catalog_client.return_value
client_instance.get_content_metadata_for_customer.return_value = {'the': 'metadata'}
client_instance_v1 = mock_catalog_client_v1.return_value
client_instance_v2 = mock_catalog_client_v2.return_value
client_instance_v2.get_content_metadata_for_customer.return_value = {'the': 'metadata'}

_ = ContentMetadataApi.get_content_metadata_for_customer(self.enterprise_customer_uuid, self.course_key)

Expand All @@ -446,7 +448,7 @@ def test_tiered_caching_works(self, mock_catalog_client):

cache_key = content_metadata_cache_key(self.course_key)
self.assertFalse(TieredCache.get_cached_response(cache_key).is_found)
client_instance.get_content_metadata.return_value = {'the': 'metadata'}
client_instance_v1.get_content_metadata.return_value = {'the': 'metadata'}

_ = ContentMetadataApi.get_content_metadata(self.course_key)

Expand All @@ -459,7 +461,7 @@ def test_tiered_caching_works(self, mock_catalog_client):
ContentMetadataApi.get_content_metadata(self.course_key),
{'the': 'metadata'},
)
assert client_instance.get_content_metadata.call_count == 1
assert client_instance_v1.get_content_metadata.call_count == 1
TieredCache.delete_all_tiers(cache_key)

@ddt.data(True, False)
Expand Down

0 comments on commit 3cd53a6

Please sign in to comment.