Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FC-0001: Replace usages enterprise_catalogs Enterprise API V1 #3742

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions ecommerce/coupons/tests/mixins.py
@@ -1,9 +1,11 @@


import datetime
from urllib.parse import urljoin

import mock
import responses
from django.conf import settings
from django.test import RequestFactory
from edx_django_utils.cache import TieredCache
from oscar.core.utils import slugify
Expand All @@ -20,6 +22,8 @@

class DiscoveryMockMixin:
""" Mocks for the Discovery service response. """
ENTERPRISE_CATALOG_URL = urljoin(settings.ENTERPRISE_CATALOG_API_URL, 'enterprise-catalogs/')

def setUp(self):
super(DiscoveryMockMixin, self).setUp()
TieredCache.dangerous_clear_all_tiers()
Expand Down Expand Up @@ -210,7 +214,7 @@ def mock_course_runs_endpoint(
)

def mock_enterprise_catalog_course_endpoint(
self, enterprise_api_url, enterprise_catalog_id, course_run=None, course_info=None
self, enterprise_catalog_id, course_run=None, course_info=None
):
"""
Helper function to register a enterprise API endpoint for getting course information.
Expand Down Expand Up @@ -240,13 +244,9 @@ def mock_enterprise_catalog_course_endpoint(
'course_runs': [],
}],
}
enterprise_catalog_url = '{}enterprise_catalogs/{}/'.format(
enterprise_api_url,
enterprise_catalog_id
)
responses.add(
responses.GET,
enterprise_catalog_url,
url=urljoin(f'{self.ENTERPRISE_CATALOG_URL}{enterprise_catalog_id}/', 'get_content_metadata/'),
json=course_info,
content_type='application/json'
)
Expand Down
46 changes: 28 additions & 18 deletions ecommerce/enterprise/tests/mixins.py
Expand Up @@ -2,7 +2,7 @@

import json
import re
from urllib.parse import urlencode
from urllib.parse import urlencode, urljoin
from uuid import uuid4

import requests
Expand Down Expand Up @@ -46,9 +46,6 @@ class EnterpriseServiceMockMixin:
ENTERPRISE_CATALOG_URL_CUSTOMER_RESOURCE = '{}enterprise-customer/'.format(
settings.ENTERPRISE_CATALOG_API_URL
)
LEGACY_ENTERPRISE_CATALOG_URL = '{}enterprise_catalogs/'.format(
settings.ENTERPRISE_API_URL
)

def setUp(self):
super(EnterpriseServiceMockMixin, self).setUp()
Expand Down Expand Up @@ -79,11 +76,13 @@ def mock_enterprise_customer_list_api_get(self):

def mock_enterprise_catalog_api_get(self, enterprise_catalog_uuid, custom_response=None):
"""
Helper function to register the legacy enterprise catalog API endpoint using responses.
Helper function to register the enterprise catalog API endpoint using responses.
"""
enterprise_catalog_api_response = {
"count": 60,
"next": "{}{}/?page=2".format(self.LEGACY_ENTERPRISE_CATALOG_URL, enterprise_catalog_uuid),
"next": urljoin(
f'{self.ENTERPRISE_CATALOG_URL}{enterprise_catalog_uuid}/', 'get_content_metadata/?page=2'
),
"previous": None,
"results": [
{
Expand Down Expand Up @@ -146,7 +145,7 @@ def mock_enterprise_catalog_api_get(self, enterprise_catalog_uuid, custom_respon
self.mock_access_token_response()
responses.add(
method=responses.GET,
url='{}{}/?'.format(self.LEGACY_ENTERPRISE_CATALOG_URL, enterprise_catalog_uuid),
url=urljoin(f'{self.ENTERPRISE_CATALOG_URL}{enterprise_catalog_uuid}/', 'get_content_metadata/'),
json=enterprise_catalog_api_response,
content_type='application/json'
)
Expand Down Expand Up @@ -565,23 +564,34 @@ def mock_enterprise_catalog_api(self, enterprise_customer_uuid, raise_exception=
"""
enterprise_catalog_api_response = {
'count': 10,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the total number of results which should not change because we're mocking a single page in the response I think.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does not make sense, becouse we're mocking, returned as it was

'num_pages': 3,
'current_page': 2,
'next': urljoin(
self.ENTERPRISE_CATALOG_URL,
f'?enterprise_customer={enterprise_customer_uuid}/page=3&page_size=2'
),
'previous': urljoin(
self.ENTERPRISE_CATALOG_URL,
f'?enterprise_customer={enterprise_customer_uuid}/page=1&page_size=2'
),
'results': [
{
'enterprise_customer': '6ae013d4-c5c4-474d-8da9-0e559b2448e2',
'uuid': '869d26dd-2c44-487b-9b6a-24eee973f9a4',
'title': 'batman_catalog'
'title': 'batman_catalog',
'enterprise_customer': '6ae013d4-c5c4-474d-8da9-0e559b2448e2',
'catalog_query_uuid': None,
'content_last_modified': None,
'catalog_modified': '2022-05-26T12:56:20.346837Z',
'query_title': None
},
{
'uuid': '3fa85f64-5717-4562-b3fc-2c963f66afa7',
'title': 'updated_catalog',
'enterprise_customer': '6ae013d4-c5c4-474d-8da9-0e559b2448e2',
'uuid': '1a61de70-f8e8-4e8c-a76e-01783a930ae6',
'title': 'new catalog'
}
'catalog_query_uuid': None,
'content_last_modified': None,
'catalog_modified': '2022-05-26T12:56:20.346837Z',
'query_title': None
},
],
'next': "{}?enterprise_customer={}&page=3".format(self.ENTERPRISE_CATALOG_URL, enterprise_customer_uuid),
'previous': "{}?enterprise_customer={}".format(self.ENTERPRISE_CATALOG_URL, enterprise_customer_uuid),
'start': 0,
}

self.mock_access_token_response()
Expand All @@ -591,7 +601,7 @@ def mock_enterprise_catalog_api(self, enterprise_customer_uuid, raise_exception=
)
responses.add(
method=responses.GET,
url='{}'.format(self.LEGACY_ENTERPRISE_CATALOG_URL),
url=self.ENTERPRISE_CATALOG_URL,
body=body,
content_type='application/json'
)
Expand Down
4 changes: 2 additions & 2 deletions ecommerce/enterprise/tests/test_utils.py
Expand Up @@ -272,7 +272,7 @@ def test_get_enterprise_customer_catalogs(self):
Verify that "get_enterprise_customer_catalogs" works as expected with and without caching.
"""
enterprise_customer_uuid = str(uuid.uuid4())
base_url = self.LEGACY_ENTERPRISE_CATALOG_URL
base_url = self.ENTERPRISE_CATALOG_URL

self.mock_access_token_response()
self.mock_enterprise_catalog_api(enterprise_customer_uuid)
Expand All @@ -294,7 +294,7 @@ def test_get_enterprise_customer_catalogs_with_exception(self):
Verify that "get_enterprise_customer_catalogs" return default response on exception.
"""
enterprise_customer_uuid = str(uuid.uuid4())
base_url = self.LEGACY_ENTERPRISE_CATALOG_URL
base_url = self.ENTERPRISE_CATALOG_URL

self.mock_access_token_response()
self.mock_enterprise_catalog_api(enterprise_customer_uuid, raise_exception=True)
Expand Down
139 changes: 97 additions & 42 deletions ecommerce/enterprise/utils.py
Expand Up @@ -32,13 +32,10 @@
log = logging.getLogger(__name__)

CUSTOMER_CATALOGS_DEFAULT_RESPONSE = {
'count': 0,
'num_pages': 0,
'current_page': 0,
'start': 0,
'next': None,
'previous': None,
'results': [],
"count": 0,
"next": None,
"previous": None,
"results": []
}


Expand Down Expand Up @@ -150,6 +147,7 @@ def get_enterprise_customer_catalogs(site, endpoint_request_url, enterprise_cust

Args:
site (Site): The site which is handling the current request
endpoint_request_url (str): endpoint request url
enterprise_customer_uuid (str): The uuid of the Enterprise Customer

Returns:
Expand All @@ -158,35 +156,34 @@ def get_enterprise_customer_catalogs(site, endpoint_request_url, enterprise_cust
Response will look like

{
'count': 2,
'num_pages': 1,
'current_page': 1,
'results': [
"count": 2,
"next": None,
"previous": None,
"results": [
{
'enterprise_customer': '6ae013d4-c5c4-474d-8da9-0e559b2448e2',
'uuid': '869d26dd-2c44-487b-9b6a-24eee973f9a4',
'title': 'batman_catalog'
"uuid": "869d26dd-2c44-487b-9b6a-24eee973f9a4",
"title": "batman_catalog",
"enterprise_customer": "6ae013d4-c5c4-474d-8da9-0e559b2448e2",
"catalog_query_uuid": None,
"content_last_modified": None,
"catalog_modified": "2022-05-26T12:56:20.346837Z",
"query_title": None
},
{
'enterprise_customer': '6ae013d4-c5c4-474d-8da9-0e559b2448e2',
'uuid': '1a61de70-f8e8-4e8c-a76e-01783a930ae6',
'title': 'new catalog'
"uuid": "1a61de70-f8e8-4e8c-a76e-01783a930ae6",
"title": "new_catalog",
"enterprise_customer": "6ae013d4-c5c4-474d-8da9-0e559b2448e2",
"catalog_query_uuid": None,
"content_last_modified": None,
"catalog_modified": "2022-05-26T12:56:20.346837Z",
"query_title": None
}
],
'next': None,
'start': 0,
'previous': None
}
"""
resource = 'enterprise_catalogs'
partner_code = site.siteconfiguration.partner.short_code
cache_key = u'{site_domain}_{partner_code}_{resource}_{uuid}_{page}'.format(
site_domain=site.domain,
partner_code=partner_code,
resource=resource,
uuid=enterprise_customer_uuid,
page=page,
)
cache_key = f'{site.domain}_{partner_code}_{resource}_{enterprise_customer_uuid}_{page}'
cache_key = hashlib.md5(cache_key.encode('utf-8')).hexdigest()

cached_response = TieredCache.get_cached_response(cache_key)
Expand All @@ -195,7 +192,7 @@ def get_enterprise_customer_catalogs(site, endpoint_request_url, enterprise_cust

api_client = site.siteconfiguration.oauth_api_client
enterprise_api_url = urljoin(
f"{site.siteconfiguration.enterprise_api_url}/", f"{resource}/"
f"{site.siteconfiguration.enterprise_catalog_api_url}/", "enterprise-catalogs/"
)

try:
Expand Down Expand Up @@ -527,31 +524,89 @@ def has_enterprise_offer(basket):
return False


def get_enterprise_catalog(site, enterprise_catalog, limit, page, endpoint_request_url=None):
# pylint: disable=line-too-long
def get_enterprise_catalog(site, enterprise_catalog, page_size, page, endpoint_request_url=None):
"""
Get the EnterpriseCustomerCatalog for a given catalog uuid.
Get the EnterpriseCatalog for a given catalog uuid.

Args:
site (Site): The site which is handling the current request
enterprise_catalog (str): The uuid of the Enterprise Catalog
limit (int): The number of results to return per page.
page_size (int): The number of results to return per page.
page (int): The page number to fetch.
endpoint_request_url (str): This is used to replace the lms url with ecommerce url

Returns:
dict: The result set containing the content objects associated with the Enterprise Catalog.
NoneType: Return None if no catalog with that uuid is found.

Response will look like

{
"count": 1,
"next": null,
"previous": null,
"uuid": "5589518b-f858-4459-a8c3-e44df4fc7af6",
"title": "All Course Runs",
"enterprise_customer": "caf06525-701b-4cf4-a05f-2c78659c4cb7",
"results": [
{
"aggregation_key": "courserun:edX+DemoX",
"content_type": "courserun",
"authoring_organization_uuids": [
"7aeefb23-e236-41e3-92a2-bed343ec8bfe"
],
"availability": "Current",
"end": null,
"enrollment_end": null,
"enrollment_start": null,
"first_enrollable_paid_seat_sku": "8CF08E5",
"first_enrollable_paid_seat_price": 149,
"full_description": null,
"go_live_date": null,
"has_enrollable_seats": true,
"image_url": null,
"is_enrollable": true,
"key": "course-v1:edX+DemoX+Demo_Course",
"language": null,
"level_type": null,
"logo_image_urls": [],
"marketing_url": "course/demonstration-course-course-v1edxdemoxdemo_course?utm_medium=enterprise&utm_source=test-enterprise",
"max_effort": null,
"min_effort": null,
"mobile_available": false,
"number": "DemoX",
"org": "edX",
"pacing_type": "instructor_paced",
"partner": "edx",
"program_types": [
"MicroMasters"
],
"published": false,
"seat_types": [
"verified",
"audit"
],
"skill_names": [],
"skills": [],
"short_description": null,
"staff_uuids": [],
"start": "2013-02-05T05:00:00Z",
"subject_uuids": [],
"title": "Demonstration Course",
"transcript_languages": [],
"type": "verified",
"weeks_to_complete": null,
"content_last_modified": "2022-06-08T15:24:08.020654Z",
"enrollment_url": "http://localhost:8734/test-enterprise/course/edX+DemoX?course_run_key=course-v1%3AedX%2BDemoX%2BDemo_Course&utm_medium=enterprise&utm_source=test-enterprise",
"xapi_activity_id": "http://edx.devstack.lms:18000/xapi/activities/courserun/course-v1:edX+DemoX+Demo_Course"
}
]
}
"""
resource = 'enterprise_catalogs'
partner_code = site.siteconfiguration.partner.short_code
cache_key = u'{site_domain}_{partner_code}_{resource}_{catalog}_{limit}_{page}'.format(
site_domain=site.domain,
partner_code=partner_code,
resource=resource,
catalog=enterprise_catalog,
limit=limit,
page=page
)
cache_key = f'{site.domain}_{partner_code}_{resource}_{enterprise_catalog}_{page_size}_{page}'
cache_key = hashlib.md5(cache_key.encode('utf-8')).hexdigest()

cached_response = TieredCache.get_cached_response(cache_key)
Expand All @@ -560,14 +615,14 @@ def get_enterprise_catalog(site, enterprise_catalog, limit, page, endpoint_reque

api_client = site.siteconfiguration.oauth_api_client
enterprise_api_url = urljoin(
f"{site.siteconfiguration.enterprise_api_url}/",
f"{resource}/{str(enterprise_catalog)}/"
f"{site.siteconfiguration.enterprise_catalog_api_url}/",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change probably deserves some sort of change log entry, at the very least we should update the olive page to tell people the old setting will not work and they'll need to add this new setting to properly connect ecommerce to the enterprise catalog.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be done on the olive page and linked back to here before this PR is ready to merge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dianakhuang can you help confirm that this setting is set in stage and prod for edx.org? I think if it is, then this is safe to merge.

Copy link
Contributor

@feanil feanil Jul 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular I just want to confirm that settings.ENTERPRISE_CATALOG_API_URL is set correctly since this is just a passthrough for that.

f"enterprise-catalogs/{str(enterprise_catalog)}/get_content_metadata/"
)

response = api_client.get(
enterprise_api_url,
params={
"limit": limit,
"page_size": page_size,
"page": page,
}
)
Expand Down
4 changes: 2 additions & 2 deletions ecommerce/extensions/api/v2/tests/views/test_vouchers.py
Expand Up @@ -485,7 +485,7 @@ def test_get_offers_for_enterprise_catalog_voucher(self):
course, seat = self.create_course_and_seat()
enterprise_catalog_id = str(uuid4())
self.mock_enterprise_catalog_course_endpoint(
self.site_configuration.enterprise_api_url, enterprise_catalog_id, course_run=course
enterprise_catalog_id, course_run=course
)
new_range, __ = Range.objects.get_or_create(
catalog_query='*:*',
Expand Down Expand Up @@ -526,7 +526,7 @@ def test_get_offers_for_enterprise_offer(self):
enterprise_customer_id = str(uuid4())
enterprise_catalog_id = str(uuid4())
self.mock_enterprise_catalog_course_endpoint(
self.site_configuration.enterprise_api_url, enterprise_catalog_id, course_run=course
enterprise_catalog_id, course_run=course
)
voucher = prepare_enterprise_voucher(
benefit_value=10,
Expand Down
2 changes: 1 addition & 1 deletion ecommerce/extensions/api/v2/views/enterprise.py
Expand Up @@ -128,7 +128,7 @@ def retrieve(self, request, **kwargs):
catalog = get_enterprise_catalog(
site=request.site,
enterprise_catalog=kwargs.get('enterprise_catalog_uuid'),
limit=request.GET.get('limit', DEFAULT_CATALOG_PAGE_SIZE),
page_size=request.GET.get('limit', DEFAULT_CATALOG_PAGE_SIZE),
page=request.GET.get('page', '1'),
endpoint_request_url=endpoint_request_url
)
Expand Down