Merge pull request #949 from edx/mjfrey/basket-catalog-data

Move basket page to use catalog service for meta-data

ecommerce/coupons/tests/mixins.py

@@ -21,6 +21,33 @@ class CourseCatalogMockMixin(object):
         super(CourseCatalogMockMixin, self).setUp()
         cache.clear()
 
+    def mock_dynamic_catalog_single_course_runs_api(self, course_run, course_run_info=None):
+        """ Helper function to register a dynamic course catalog API endpoint for the course run information. """
+        if not course_run_info:
+            course_run_info = {
+                "course": "edX+DemoX",
+                "key": course_run.id,
+                "title": course_run.name,
+                "short_description": 'Foo',
+                "start": "2013-02-05T05:00:00Z",
+                "image": {
+                    "src": "/path/to/image.jpg",
+                },
+            }
+
+        course_run_info_json = json.dumps(course_run_info)
+        course_run_url = '{}course_runs/{}/?partner={}'.format(
+            settings.COURSE_CATALOG_API_URL,
+            course_run.id,
+            self.site.siteconfiguration.partner.short_code
+        )
+
+        httpretty.register_uri(
+            httpretty.GET, course_run_url,
+            body=course_run_info_json,
+            content_type='application/json'
+        )
+
     def mock_dynamic_catalog_course_runs_api(self, course_run=None, query=None, course_run_info=None):
         """ Helper function to register a dynamic course catalog API endpoint for the course run information. """
         if not course_run_info:

ecommerce/courses/tests/test_utils.py

@@ -7,11 +7,12 @@ from django.core.cache import cache
 
 from ecommerce.core.constants import ENROLLMENT_CODE_SWITCH
 from ecommerce.core.tests import toggle_switch
-from ecommerce.core.url_utils import get_lms_url
+from ecommerce.core.tests.decorators import mock_course_catalog_api_client
+from ecommerce.coupons.tests.mixins import CourseCatalogMockMixin
 from ecommerce.courses.models import Course
 from ecommerce.courses.tests.factories import CourseFactory
 from ecommerce.courses.utils import (
-    get_certificate_type_display_value, get_course_info_from_lms, mode_for_seat
+    get_certificate_type_display_value, get_course_info_from_catalog, mode_for_seat
 )
 from ecommerce.extensions.catalogue.tests.mixins import CourseCatalogTestMixin
 from ecommerce.tests.testcases import TestCase
@@ -19,7 +20,7 @@ from ecommerce.tests.testcases import TestCase
 
 @httpretty.activate
 @ddt.ddt
-class UtilsTests(CourseCatalogTestMixin, TestCase):
+class UtilsTests(CourseCatalogTestMixin, CourseCatalogMockMixin, TestCase):
     @ddt.unpack
     @ddt.data(
         ('', False, 'audit'),
@@ -41,19 +42,20 @@ class UtilsTests(CourseCatalogTestMixin, TestCase):
         if enrollment_code:  # We should only have enrollment codes for allowed types
             self.assertEqual(mode_for_seat(enrollment_code), mode)
 
-    def test_get_course_info_from_lms(self):
+    @mock_course_catalog_api_client
+    def test_get_course_info_from_catalog(self):
         """ Check to see if course info gets cached """
         course = CourseFactory()
-        course_url = get_lms_url('api/courses/v1/courses/{}/'.format(course.id))
-        httpretty.register_uri(httpretty.GET, course_url, body=course.name, status=200, content_type='application/json')
+        self.mock_dynamic_catalog_single_course_runs_api(course)
 
-        cache_key = 'courses_api_detail_{}'.format(course.id)
+        cache_key = 'courses_api_detail_{}{}'.format(course.id, self.site.siteconfiguration.partner.short_code)
         cache_hash = hashlib.md5(cache_key).hexdigest()
         cached_course = cache.get(cache_hash)
         self.assertIsNone(cached_course)
 
-        response = get_course_info_from_lms(course.id)
-        self.assertEqual(response, course.name)
+        response = get_course_info_from_catalog(self.request.site, course)
+
+        self.assertEqual(response['title'], course.name)
 
         cached_course = cache.get(cache_hash)
         self.assertEqual(cached_course, response)

ecommerce/courses/utils.py

@@ -3,9 +3,6 @@ import hashlib
 from django.conf import settings
 from django.core.cache import cache
 from django.utils.translation import ugettext_lazy as _
-from edx_rest_api_client.client import EdxRestApiClient
-
-from ecommerce.core.url_utils import get_lms_url
 
 
 def mode_for_seat(product):
@@ -23,16 +20,17 @@ def mode_for_seat(product):
     return mode
 
 
-def get_course_info_from_lms(course_key):
-    """ Get course information from LMS via the course api and cache """
-    api = EdxRestApiClient(get_lms_url('api/courses/v1/'))
-    cache_key = 'courses_api_detail_{}'.format(course_key)
+def get_course_info_from_catalog(site, course_key):
+    """ Get course information from catalog service and cache """
+    api = site.siteconfiguration.course_catalog_api_client
+    partner_short_code = site.siteconfiguration.partner.short_code
+    cache_key = 'courses_api_detail_{}{}'.format(course_key, partner_short_code)
     cache_hash = hashlib.md5(cache_key).hexdigest()
-    course = cache.get(cache_hash)
-    if not course:  # pragma: no cover
-        course = api.courses(course_key).get()
-        cache.set(cache_hash, course, settings.COURSES_API_CACHE_TIMEOUT)
-    return course
+    course_run = cache.get(cache_hash)
+    if not course_run:  # pragma: no cover
+        course_run = api.course_runs(course_key).get(partner=partner_short_code)
+        cache.set(cache_hash, course_run, settings.COURSES_API_CACHE_TIMEOUT)
+    return course_run
 
 
 def get_certificate_type_display_value(certificate_type):

ecommerce/extensions/api/v2/tests/views/test_vouchers.py

@@ -17,7 +17,6 @@ from rest_framework.test import APIRequestFactory
 from slumber.exceptions import SlumberBaseException
 
 from ecommerce.core.tests.decorators import mock_course_catalog_api_client
-from ecommerce.core.url_utils import get_lms_url
 from ecommerce.coupons.tests.mixins import CourseCatalogMockMixin, CouponMixin
 from ecommerce.coupons.views import get_voucher_and_products_from_code
 from ecommerce.courses.models import Course
@@ -147,10 +146,11 @@ class VoucherViewOffersEndpointTests(
 
         self.assertEqual(response.status_code, 400)
 
+    @mock_course_catalog_api_client
     def test_voucher_offers_listing_for_a_single_course_voucher(self):
         """ Verify the endpoint returns offers data when a single product is in voucher range. """
         course, seat = self.create_course_and_seat()
-        self.mock_course_api_response(course=course)
+        self.mock_dynamic_catalog_single_course_runs_api(course)
         new_range = RangeFactory(products=[seat, ])
         new_range.catalog = Catalog.objects.create(partner=self.partner)
         new_range.catalog.stock_records.add(StockRecord.objects.get(product=seat))
@@ -199,7 +199,7 @@ class VoucherViewOffersEndpointTests(
     def test_voucher_offers_listing_product_found(self):
         """ Verify the endpoint returns offers data for single product range. """
         course, seat = self.create_course_and_seat()
-        self.mock_course_api_response(course=course)
+        self.mock_dynamic_catalog_single_course_runs_api(course)
 
         new_range = RangeFactory(products=[seat, ])
         voucher, __ = prepare_voucher(_range=new_range, benefit_value=10)
@@ -260,7 +260,7 @@ class VoucherViewOffersEndpointTests(
         voucher, __ = prepare_voucher(_range=new_range, benefit_value=10)
         benefit = voucher.offers.first().benefit
         request = self.prepare_offers_listing_request(voucher.code)
-        self.mock_course_api_response(course=course)
+        self.mock_dynamic_catalog_single_course_runs_api(course)
         offers = VoucherViewSet().get_offers(request=request, voucher=voucher)['results']
         first_offer = offers[0]
 
@@ -273,7 +273,7 @@ class VoucherViewOffersEndpointTests(
             'contains_verified': True,
             'course_start_date': '2013-02-05T05:00:00Z',
             'id': course.id,
-            'image_url': get_lms_url('/asset-v1:test+test+test+type@asset+block@images_course_image.jpg'),
+            'image_url': '/path/to/image.jpg',
             'organization': CourseKey.from_string(course.id).org,
             'seat_type': course.type,
             'stockrecords': serializers.StockRecordSerializer(seat.stockrecords.first()).data,

ecommerce/extensions/api/v2/views/vouchers.py

@@ -13,9 +13,8 @@ from rest_framework_extensions.decorators import action
 from slumber.exceptions import SlumberBaseException
 
 from ecommerce.core.constants import DEFAULT_CATALOG_PAGE_SIZE
-from ecommerce.core.url_utils import get_lms_url
 from ecommerce.courses.models import Course
-from ecommerce.courses.utils import get_course_info_from_lms
+from ecommerce.courses.utils import get_course_info_from_catalog
 from ecommerce.coupons.utils import get_range_catalog_query_results
 from ecommerce.extensions.api import serializers
 from ecommerce.extensions.api.permissions import IsOffersOrIsAuthenticatedAndStaff
@@ -166,11 +165,9 @@ class VoucherViewSet(NonDestroyableModelViewSet):
             course_id = product.course_id
             course = get_object_or_404(Course, id=course_id)
             stock_record = get_object_or_404(StockRecord, product__id=product.id)
-            course_info = get_course_info_from_lms(course_id)
+            course_info = get_course_info_from_catalog(request.site, course_id)
 
             if course_info:
-                course_info['image'] = {'src': get_lms_url(course_info['media']['course_image']['uri'])}
-
                 offers.append(self.get_course_offer_data(
                     benefit=benefit,
                     course=course,

ecommerce/extensions/basket/tests/test_views.py

@@ -22,9 +22,10 @@ from testfixtures import LogCapture
 from ecommerce.core.constants import ENROLLMENT_CODE_PRODUCT_CLASS_NAME, ENROLLMENT_CODE_SWITCH
 from ecommerce.core.models import SiteConfiguration
 from ecommerce.core.tests import toggle_switch
+from ecommerce.core.tests.decorators import mock_course_catalog_api_client
 from ecommerce.core.url_utils import get_lms_enrollment_api_url
 from ecommerce.core.url_utils import get_lms_url
-from ecommerce.coupons.tests.mixins import CouponMixin
+from ecommerce.coupons.tests.mixins import CouponMixin, CourseCatalogMockMixin
 from ecommerce.courses.tests.factories import CourseFactory
 from ecommerce.extensions.basket.utils import get_basket_switch_data
 from ecommerce.extensions.basket.views import VoucherAddMessagesView
@@ -50,7 +51,7 @@ COUPON_CODE = 'COUPONTEST'
 
 
 @ddt.ddt
-class BasketSingleItemViewTests(CouponMixin, CourseCatalogTestMixin, LmsApiMockMixin, TestCase):
+class BasketSingleItemViewTests(CouponMixin, CourseCatalogTestMixin, CourseCatalogMockMixin, LmsApiMockMixin, TestCase):
     """ BasketSingleItemView view tests. """
     path = reverse('basket:single-item')
 
@@ -138,7 +139,7 @@ class BasketSingleItemViewTests(CouponMixin, CourseCatalogTestMixin, LmsApiMockM
         self.mock_enrollment_api_success_enrolled(self.course.id)
         self.create_coupon(catalog=self.catalog, code=COUPON_CODE, benefit_value=5)
 
-        self.mock_course_api_response(course=self.course)
+        self.mock_dynamic_catalog_course_runs_api(course_run=self.course)
         url = '{path}?sku={sku}&code={code}'.format(path=self.path, sku=self.stock_record.partner_sku,
                                                     code=COUPON_CODE)
         response = self.client.get(url)
@@ -211,7 +212,7 @@ class BasketSingleItemViewTests(CouponMixin, CourseCatalogTestMixin, LmsApiMockM
 @httpretty.activate
 @ddt.ddt
 @override_settings(PAYMENT_PROCESSORS=['ecommerce.extensions.payment.tests.processors.DummyProcessor'])
-class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMixin, TestCase):
+class BasketSummaryViewTests(CourseCatalogTestMixin, CourseCatalogMockMixin, LmsApiMockMixin, ApiMockMixin, TestCase):
     """ BasketSummaryView basket view tests. """
     path = reverse('basket:summary')
 
@@ -268,7 +269,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
             l.check(
                 (
                     logger_name, 'ERROR',
-                    u'Failed to retrieve data from Course API for course [{}].'.format(self.course.id)
+                    u'Failed to retrieve data from Catalog Service for course [{}].'.format(self.course.id)
                 )
             )
 
@@ -279,7 +280,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         course.create_or_update_seat('verified', False, 10, self.partner, create_enrollment_code=True)
         enrollment_code = Product.objects.get(product_class__name=ENROLLMENT_CODE_PRODUCT_CLASS_NAME)
         self.create_basket_and_add_product(enrollment_code)
-        self.mock_course_api_response(course)
+        self.mock_dynamic_catalog_course_runs_api(course_run=course)
 
         response = self.client.get(self.path)
         self.assertEqual(response.status_code, 200)
@@ -303,7 +304,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         seat_without_ec = no_ec_course.create_or_update_seat('verified', False, 10, self.partner)
         seat_with_ec = ec_course.create_or_update_seat('verified', False, 10, self.partner, create_enrollment_code=True)
         self.create_basket_and_add_product(seat_without_ec)
-        self.mock_course_api_response(no_ec_course)
+        self.mock_dynamic_catalog_course_runs_api(course_run=no_ec_course)
 
         response = self.client.get(self.path)
         self.assertFalse(response.context['switch_link_text'])
@@ -315,7 +316,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
 
         Basket.objects.all().delete()
         self.create_basket_and_add_product(seat_with_ec)
-        self.mock_course_api_response(ec_course)
+        self.mock_dynamic_catalog_course_runs_api(course_run=ec_course)
 
         response = self.client.get(self.path)
         enrollment_code = Product.objects.get(product_class__name=ENROLLMENT_CODE_PRODUCT_CLASS_NAME)
@@ -344,6 +345,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         (Benefit.FIXED, 50)
     )
     @ddt.unpack
+    @mock_course_catalog_api_client
     def test_response_success(self, benefit_type, benefit_value):
         """ Verify a successful response is returned. """
         seat = self.create_seat(self.course, 500)
@@ -351,7 +353,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         self.create_and_apply_benefit_to_basket(basket, seat, benefit_type, benefit_value)
 
         self.assertEqual(basket.lines.count(), 1)
-        self.mock_course_api_response(self.course)
+        self.mock_dynamic_catalog_single_course_runs_api(self.course)
 
         benefit, __ = Benefit.objects.get_or_create(type=benefit_type, value=benefit_value)
 
@@ -373,7 +375,7 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
 
     def test_line_item_discount_data(self):
         """ Verify that line item has correct discount data. """
-        self.mock_course_api_response(self.course)
+        self.mock_dynamic_catalog_course_runs_api(course_run=self.course)
         seat = self.create_seat(self.course)
         basket = self.create_basket_and_add_product(seat)
         self.create_and_apply_benefit_to_basket(basket, seat, Benefit.PERCENTAGE, 50)
@@ -387,14 +389,15 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         self.assertEqual(lines[0][1]['benefit_value'], '50%')
         self.assertEqual(lines[1][1]['benefit_value'], None)
 
+    @mock_course_catalog_api_client
     def test_cached_course(self):
         """ Verify that the course info is cached. """
         seat = self.create_seat(self.course, 50)
         basket = self.create_basket_and_add_product(seat)
         self.assertEqual(basket.lines.count(), 1)
-        self.mock_course_api_response(self.course)
+        self.mock_dynamic_catalog_single_course_runs_api(self.course)
 
-        cache_key = 'courses_api_detail_{}'.format(self.course.id)
+        cache_key = 'courses_api_detail_{}{}'.format(self.course.id, self.site.siteconfiguration.partner.short_code)
         cache_hash = hashlib.md5(cache_key).hexdigest()
         cached_course_before = cache.get(cache_hash)
         self.assertIsNone(cached_course_before)
@@ -402,24 +405,23 @@ class BasketSummaryViewTests(CourseCatalogTestMixin, LmsApiMockMixin, ApiMockMix
         response = self.client.get(self.path)
         self.assertEqual(response.status_code, 200)
         cached_course_after = cache.get(cache_hash)
-        self.assertEqual(cached_course_after['name'], self.course.name)
+        self.assertEqual(cached_course_after['title'], self.course.name)
 
     @ddt.data({
-        'name': 'Test',
+        'course': 'edX+DemoX',
         'short_description': None,
+        'title': 'Junk',
+        'start': '2013-02-05T05:00:00Z',
     }, {
-        'name': 'Test',
+        'course': 'edX+DemoX',
         'short_description': None,
-        'media': None
     })
-    def test_empty_course_api_response(self, course_info):
-        """ Check to see if we can handle empty response from the course api """
+    @mock_course_catalog_api_client
+    def test_empty_catalog_api_response(self, course_info):
+        """ Check to see if we can handle empty response from the catalog api """
         seat = self.create_seat(self.course)
         self.create_basket_and_add_product(seat)
-        course_url = get_lms_url('api/courses/v1/courses/{}/'.format(self.course.id))
-        course_info_json = json.dumps(course_info)
-        httpretty.register_uri(httpretty.GET, course_url, body=course_info_json, content_type='application/json')
-
+        self.mock_dynamic_catalog_single_course_runs_api(self.course, course_info)
         response = self.client.get(self.path)
         self.assertEqual(response.status_code, 200)
         line_data = response.context['formset_lines_data'][0][1]

ecommerce/extensions/basket/views.py

@@ -13,7 +13,7 @@ from slumber.exceptions import SlumberBaseException
 
 from ecommerce.core.constants import ENROLLMENT_CODE_PRODUCT_CLASS_NAME, SEAT_PRODUCT_CLASS_NAME
 from ecommerce.core.url_utils import get_lms_url
-from ecommerce.courses.utils import get_certificate_type_display_value, get_course_info_from_lms, mode_for_seat
+from ecommerce.courses.utils import get_certificate_type_display_value, get_course_info_from_catalog, mode_for_seat
 from ecommerce.extensions.analytics.utils import prepare_analytics_data
 from ecommerce.extensions.basket.utils import prepare_basket, get_basket_switch_data
 from ecommerce.extensions.offer.utils import format_benefit_value
@@ -104,15 +104,15 @@ class BasketSummaryView(BasketView):
             image_url = None
             short_description = None
             try:
-                course = get_course_info_from_lms(course_key)
+                course = get_course_info_from_catalog(self.request.site, course_key)
                 try:
-                    image_url = course['media']['image']['raw']
+                    image_url = course['image']['src']
                 except (KeyError, TypeError):
                     image_url = ''
                 short_description = course.get('short_description', '')
-                course_name = course['name']
+                course_name = course.get('title', '')
             except (ConnectionError, SlumberBaseException, Timeout):
-                logger.exception('Failed to retrieve data from Course API for course [%s].', course_key)
+                logger.exception('Failed to retrieve data from Catalog Service for course [%s].', course_key)
 
             if self.request.site.siteconfiguration.enable_enrollment_codes:
                 # Get variables for the switch link that toggles from enrollment codes and seat.