From 04115bfe43b2d425ed5b35be9360f8530cb39054 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 21 Dec 2016 13:21:47 -0500 Subject: [PATCH] Revert "Merge pull request #13235 from edx/jia/MA-2684" This reverts commit bde0f7b2a789744be547af558f8c477b79bcc281, reversing changes made to 71693c3a12e73430357011aebf090a7c381700a1. --- cms/djangoapps/contentstore/views/course.py | 4 +- common/djangoapps/util/course.py | 21 +- common/djangoapps/util/tests/test_course.py | 85 ++----- .../learner_dashboard/tests/test_programs.py | 56 ++--- .../mobile_api/users/serializers.py | 14 +- lms/djangoapps/mobile_api/users/tests.py | 53 +---- lms/djangoapps/mobile_api/users/views.py | 48 ++-- .../core/djangoapps/catalog/tests/mixins.py | 30 +-- .../djangoapps/catalog/tests/test_utils.py | 223 +++++------------- openedx/core/djangoapps/catalog/utils.py | 178 ++------------ .../djangoapps/programs/tests/test_utils.py | 44 +--- 11 files changed, 183 insertions(+), 573 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e83630113f..70686f0353 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -76,7 +76,7 @@ from student.auth import has_course_author_access, has_studio_write_access, has_ from student.roles import ( CourseInstructorRole, CourseStaffRole, CourseCreatorRole, GlobalStaff, UserBasedRole ) -from util.course import get_link_for_about_page +from util.course import get_lms_link_for_about_page from util.date_utils import get_default_time_display from util.json_request import JsonResponse, JsonResponseBadRequest, expect_json from util.milestones_helpers import ( @@ -1000,7 +1000,7 @@ def settings_handler(request, course_key_string): settings_context = { 'context_course': course_module, 'course_locator': course_key, - 'lms_link_for_about_page': get_link_for_about_page(course_key, request.user), + 'lms_link_for_about_page': get_lms_link_for_about_page(course_key), 'course_image_url': course_image_url(course_module, 'course_image'), 'banner_image_url': course_image_url(course_module, 'banner_image'), 'video_thumbnail_image_url': course_image_url(course_module, 'video_thumbnail_image'), diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 96a39cc762..46720b326d 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -2,30 +2,27 @@ Utility methods related to course """ import logging - from django.conf import settings -from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.catalog.utils import get_run_marketing_url +from opaque_keys.edx.keys import CourseKey log = logging.getLogger(__name__) -def get_link_for_about_page(course_key, user, catalog_course_run=None): +def get_lms_link_for_about_page(course_key): """ Returns the url to the course about page. """ assert isinstance(course_key, CourseKey) if settings.FEATURES.get('ENABLE_MKTG_SITE'): - if catalog_course_run: - marketing_url = catalog_course_run.get('marketing_url') - else: - marketing_url = get_run_marketing_url(course_key, user) - if marketing_url: - return marketing_url + # Root will be "https://www.edx.org". The complete URL will still not be exactly correct, + # but redirects exist from www.edx.org to get to the Drupal course about page URL. + about_base = settings.MKTG_URLS['ROOT'] + else: + about_base = settings.LMS_ROOT_URL return u"{about_base_url}/courses/{course_key}/about".format( - about_base_url=settings.LMS_ROOT_URL, - course_key=unicode(course_key) + about_base_url=about_base, + course_key=course_key.to_deprecated_string() ) diff --git a/common/djangoapps/util/tests/test_course.py b/common/djangoapps/util/tests/test_course.py index 77fe7aaeeb..491119546b 100644 --- a/common/djangoapps/util/tests/test_course.py +++ b/common/djangoapps/util/tests/test_course.py @@ -1,73 +1,36 @@ """ Tests for course utils. """ -from django.core.cache import cache -import httpretty + +from django.test import TestCase, override_settings import mock -from opaque_keys.edx.keys import CourseKey - -from openedx.core.djangoapps.catalog.tests import factories -from openedx.core.djangoapps.catalog.utils import CatalogCacheUtility -from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from student.tests.factories import UserFactory -from util.course import get_link_for_about_page +from opaque_keys.edx.locations import SlashSeparatedCourseKey +from util.course import get_lms_link_for_about_page -@httpretty.activate -class CourseAboutLinkTestCase(CatalogIntegrationMixin, CacheIsolationTestCase): - """ - Tests for Course About link. - """ +class LmsLinksTestCase(TestCase): + """ Tests for LMS links. """ - ENABLED_CACHES = ['default'] - - def setUp(self): - super(CourseAboutLinkTestCase, self).setUp() - self.user = UserFactory.create(password="password") - - self.course_key_string = "foo/bar/baz" - self.course_key = CourseKey.from_string("foo/bar/baz") - self.course_run = factories.CourseRun(key=self.course_key_string) - self.lms_course_about_url = "http://localhost:8000/courses/foo/bar/baz/about" - - self.catalog_integration = self.create_catalog_integration( - internal_api_url="http://catalog.example.com:443/api/v1", - cache_ttl=1 - ) - self.course_cache_key = "{}{}".format(CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_key_string) - - def test_about_page_lms(self): - """ - Get URL for about page, no marketing site. - """ + def test_about_page(self): + """ Get URL for about page, no marketing site """ with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - self.assertEquals( - get_link_for_about_page(self.course_key, self.user), self.lms_course_about_url - ) - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - self.register_catalog_course_run_response( - [self.course_key_string], [{"key": self.course_key_string, "marketing_url": None}] - ) - self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.lms_course_about_url) + self.assertEquals(self.get_about_page_link(), "http://localhost:8000/courses/mitX/101/test/about") - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) + @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root'}) def test_about_page_marketing_site(self): - """ - Get URL for about page, marketing site enabled. - """ - self.register_catalog_course_run_response([self.course_key_string], [self.course_run]) - self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.course_run["marketing_url"]) - cached_data = cache.get_many([self.course_cache_key]) - self.assertIn(self.course_cache_key, cached_data.keys()) + """ Get URL for about page, marketing root present. """ + with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), "https://dummy-root/courses/mitX/101/test/about") + with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): + self.assertEquals(self.get_about_page_link(), "http://localhost:8000/courses/mitX/101/test/about") - with mock.patch('openedx.core.djangoapps.catalog.utils.get_edx_api_data') as mock_method: - self.assertEquals(get_link_for_about_page(self.course_key, self.user), self.course_run["marketing_url"]) - self.assertEqual(0, mock_method.call_count) + @override_settings(MKTG_URLS={'ROOT': 'https://www.dummyhttps://x'}) + def test_about_page_marketing_site_https__edge(self): + """ Get URL for about page """ + with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): + self.assertEquals(self.get_about_page_link(), "https://www.dummyhttps://x/courses/mitX/101/test/about") - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) - def test_about_page_marketing_url_cached(self): - self.assertEquals( - get_link_for_about_page(self.course_key, self.user, self.course_run), - self.course_run["marketing_url"] - ) + def get_about_page_link(self): + """ create mock course and return the about page link.""" + course_key = SlashSeparatedCourseKey('mitX', '101', 'test') + return get_lms_link_for_about_page(course_key) diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index 0a2ec75101..cd4a46743f 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -17,8 +17,6 @@ import httpretty import mock from provider.constants import CONFIDENTIAL -from openedx.core.djangoapps.catalog.tests import factories as catalog_factories -from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.tests import factories as credentials_factories from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin @@ -267,10 +265,9 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar @httpretty.activate @override_settings(MKTG_URLS={'ROOT': 'https://www.example.com'}) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, CatalogIntegrationMixin): - """ - Unit tests for the program details page. - """ +@mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) +class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): + """Unit tests for the program details page.""" program_id = 123 password = 'test' url = reverse('program_details_view', args=[program_id]) @@ -282,7 +279,6 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) course = CourseFactory() - cls.course_id_string = unicode(course.id) # pylint: disable=no-member organization = programs_factories.Organization() run_mode = programs_factories.RunMode(course_key=unicode(course.id)) # pylint: disable=no-member course_code = programs_factories.CourseCode(run_modes=[run_mode]) @@ -291,7 +287,6 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata organizations=[organization], course_codes=[course_code] ) - cls.course_run = catalog_factories.CourseRun(key=cls.course_id_string) def setUp(self): super(TestProgramDetails, self).setUp() @@ -300,9 +295,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata self.client.login(username=self.user.username, password=self.password) def mock_programs_api(self, data, status=200): - """ - Helper for mocking out Programs API URLs. - """ + """Helper for mocking out Programs API URLs.""" self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') url = '{api_root}/programs/{id}/'.format( @@ -321,9 +314,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata ) def assert_program_data_present(self, response): - """ - Verify that program data is present. - """ + """Verify that program data is present.""" self.assertContains(response, 'programData') self.assertContains(response, 'urls') self.assertContains(response, 'program_listing_url') @@ -331,9 +322,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata self.assert_programs_tab_present(response) def assert_programs_tab_present(self, response): - """ - Verify that the programs tab is present in the nav. - """ + """Verify that the programs tab is present in the nav.""" soup = BeautifulSoup(response.content, 'html.parser') self.assertTrue( any(soup.find_all('a', class_='tab-nav-link', href=reverse('program_listing_view'))) @@ -343,24 +332,21 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata """ Verify that login is required to access the page. """ - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_id_string: self.course_run, - }): - self.create_programs_config() - self.mock_programs_api(self.data) + self.create_programs_config() + self.mock_programs_api(self.data) - self.client.logout() + self.client.logout() - response = self.client.get(self.url) - self.assertRedirects( - response, - '{}?next={}'.format(reverse('signin_user'), self.url) - ) + response = self.client.get(self.url) + self.assertRedirects( + response, + '{}?next={}'.format(reverse('signin_user'), self.url) + ) - self.client.login(username=self.user.username, password=self.password) + self.client.login(username=self.user.username, password=self.password) - response = self.client.get(self.url) - self.assert_program_data_present(response) + response = self.client.get(self.url) + self.assert_program_data_present(response) def test_404_if_disabled(self): """ @@ -372,9 +358,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata self.assertEqual(response.status_code, 404) def test_404_if_no_data(self): - """ - Verify that the page 404s if no program data is found. - """ + """Verify that the page 404s if no program data is found.""" self.create_programs_config() self.mock_programs_api(self.data, status=404) @@ -388,9 +372,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase, Cata self.assertEqual(response.status_code, 404) def test_page_routing(self): - """ - Verify that the page can be hit with or without a program name in the URL. - """ + """Verify that the page can be hit with or without a program name in the URL.""" self.create_programs_config() self.mock_programs_api(self.data) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 5f734f2bf1..e759f358e5 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -1,12 +1,14 @@ """ Serializer for user API """ +from opaque_keys.edx.keys import CourseKey from rest_framework import serializers from rest_framework.reverse import reverse + from courseware.access import has_access -from certificates.api import certificate_downloadable_status from student.models import CourseEnrollment, User -from util.course import get_link_for_about_page +from certificates.api import certificate_downloadable_status +from util.course import get_lms_link_for_about_page class CourseOverviewField(serializers.RelatedField): @@ -50,9 +52,7 @@ class CourseOverviewField(serializers.RelatedField): } }, 'course_image': course_overview.course_image_url, - 'course_about': get_link_for_about_page( - course_overview.id, request.user, self.context.get('catalog_course_run') - ), + 'course_about': get_lms_link_for_about_page(CourseKey.from_string(course_id)), 'course_updates': reverse( 'course-updates-list', kwargs={'course_id': course_id}, @@ -85,9 +85,7 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): certificate = serializers.SerializerMethodField() def get_certificate(self, model): - """ - Returns the information about the user's certificate in the course. - """ + """Returns the information about the user's certificate in the course.""" certificate_info = certificate_downloadable_status(model.user, model.course_id) if certificate_info['is_downloadable']: return { diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 091367411b..da40f21ca0 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -5,9 +5,7 @@ Tests for users API import datetime import ddt -import httpretty from mock import patch -import mock from nose.plugins.attrib import attr import pytz from django.conf import settings @@ -28,9 +26,6 @@ from courseware.access_response import ( ) from course_modes.models import CourseMode from lms.djangoapps.grades.tests.utils import mock_passing_grade -from openedx.core.djangoapps.catalog.tests import factories -from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin -from openedx.core.djangoapps.catalog.utils import get_course_runs from openedx.core.lib.courses import course_image_url from student.models import CourseEnrollment from util.milestones_helpers import set_prerequisite_courses @@ -468,8 +463,7 @@ class TestCourseStatusPATCH(CourseStatusAPITestCase, MobileAuthUserTestMixin, @attr(shard=2) @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) -@httpretty.activate -class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin, CatalogIntegrationMixin): +class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin): """ Test the course enrollment serializer """ @@ -478,13 +472,6 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin, self.login_and_enroll() self.request = RequestFactory().get('/') self.request.user = self.user - self.catalog_integration = self.create_catalog_integration( - internal_api_url="http://catalog.example.com:443/api/v1", - cache_ttl=1 - ) - self.course_id_string = unicode(self.course.id) - self.course_run = factories.CourseRun(key=self.course_id_string) - self.course_keys = [self.course_id_string] def test_success(self): serialized = CourseEnrollmentSerializer( @@ -506,41 +493,3 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin, ).data self.assertEqual(serialized['course']['number'], self.course.display_coursenumber) self.assertEqual(serialized['course']['org'], self.course.display_organization) - - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) - def test_course_about_marketing_url(self): - self.register_catalog_course_run_response(self.course_keys, [self.course_run]) - catalog_course_runs_against_course_keys = get_course_runs(self.course_keys, self.request.user) - enrollment = CourseEnrollment.enrollments_for_user(self.user)[0] - serialized = CourseEnrollmentSerializer( - enrollment, - context={ - 'request': self.request, - "catalog_course_run": ( - catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] - if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None - ) - }, - ).data - self.assertEqual( - serialized['course']['course_about'], self.course_run["marketing_url"] - ) - - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}) - def test_course_about_lms_url(self): - self.register_catalog_course_run_response(self.course_keys, [self.course_run]) - catalog_course_runs_against_course_keys = get_course_runs(self.course_keys, self.request.user) - enrollment = CourseEnrollment.enrollments_for_user(self.user)[0] - serialized = CourseEnrollmentSerializer( - enrollment, - context={ - 'request': self.request, - "catalog_course_run": ( - catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] - if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None - ) - }, - ).data - self.assertEqual( - serialized['course']['course_about'], "http://localhost:8000/courses/{}/about".format(self.course_id_string) - ) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index e63a737f48..3d8b4fba51 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -11,14 +11,12 @@ from rest_framework.response import Response from opaque_keys.edx.keys import UsageKey from opaque_keys import InvalidKeyError -from rest_framework.views import APIView from courseware.access import is_mobile_available_for_user from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from courseware.views.index import save_positions_recursively_up from courseware.views.views import get_current_child -from openedx.core.djangoapps.catalog.utils import get_course_runs from student.models import CourseEnrollment, User from xblock.fields import Scope @@ -206,7 +204,7 @@ class UserCourseStatus(views.APIView): @mobile_view(is_user=True) -class UserCourseEnrollmentsList(APIView): +class UserCourseEnrollmentsList(generics.ListAPIView): """ **Use Case** @@ -260,6 +258,17 @@ class UserCourseEnrollmentsList(APIView): certified). * url: URL to the downloadable version of the certificate, if exists. """ + queryset = CourseEnrollment.objects.all() + serializer_class = CourseEnrollmentSerializer + lookup_field = 'username' + + # In Django Rest Framework v3, there is a default pagination + # class that transmutes the response data into a dictionary + # with pagination information. The original response data (a list) + # is stored in a "results" value of the dictionary. + # For backwards compatibility with the existing API, we disable + # the default behavior by setting the pagination_class to None. + pagination_class = None def is_org(self, check_org, course_org): """ @@ -267,34 +276,17 @@ class UserCourseEnrollmentsList(APIView): """ return check_org is None or (check_org.lower() == course_org.lower()) - def get(self, request, username): - """ - Returns a list of courses enrolled by user. - """ - queryset = CourseEnrollment.objects.all() - course_ids = set(queryset.values_list('course_id', flat=True)) - catalog_course_runs_against_course_keys = get_course_runs(course_ids, request.user) - - enrollments = queryset.filter( - user__username=username, + def get_queryset(self): + enrollments = self.queryset.filter( + user__username=self.kwargs['username'], is_active=True ).order_by('created').reverse() - org = request.query_params.get('org', None) - - return Response([ - CourseEnrollmentSerializer( - enrollment, - context={ - "request": request, - "catalog_course_run": ( - catalog_course_runs_against_course_keys[unicode(enrollment.course_id)] - if unicode(enrollment.course_id) in catalog_course_runs_against_course_keys else None - ) - } - ).data for enrollment in enrollments + org = self.request.query_params.get('org', None) + return [ + enrollment for enrollment in enrollments if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and - is_mobile_available_for_user(request.user, enrollment.course_overview) - ]) + is_mobile_available_for_user(self.request.user, enrollment.course_overview) + ] @api_view(["GET"]) diff --git a/openedx/core/djangoapps/catalog/tests/mixins.py b/openedx/core/djangoapps/catalog/tests/mixins.py index 65886373c1..2372a0b33f 100644 --- a/openedx/core/djangoapps/catalog/tests/mixins.py +++ b/openedx/core/djangoapps/catalog/tests/mixins.py @@ -1,16 +1,9 @@ """Mixins to help test catalog integration.""" -import json -import urllib - -import httpretty - from openedx.core.djangoapps.catalog.models import CatalogIntegration class CatalogIntegrationMixin(object): - """ - Utility for working with the catalog service during testing. - """ + """Utility for working with the catalog service during testing.""" DEFAULTS = { 'enabled': True, @@ -19,27 +12,8 @@ class CatalogIntegrationMixin(object): } def create_catalog_integration(self, **kwargs): - """ - Creates a new CatalogIntegration with DEFAULTS, updated with any provided overrides. - """ + """Creates a new CatalogIntegration with DEFAULTS, updated with any provided overrides.""" fields = dict(self.DEFAULTS, **kwargs) CatalogIntegration(**fields).save() return CatalogIntegration.current() - - def register_catalog_course_run_response(self, course_keys, catalog_course_run_data): - """ - Register a mock response for GET on the catalog course run endpoint. - """ - httpretty.register_uri( - httpretty.GET, - "http://catalog.example.com:443/api/v1/course_runs/?keys={}&exclude_utm=1".format( - urllib.quote_plus(",".join(course_keys)) - ), - body=json.dumps({ - "results": catalog_course_run_data, - "next": "" - }), - content_type='application/json', - status=200 - ) diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 8638f8525b..0ae29a0da4 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -1,12 +1,9 @@ -""" -Tests covering utilities for integrating with the catalog service. -""" +"""Tests covering utilities for integrating with the catalog service.""" import uuid import copy -from django.core.cache import cache +import ddt from django.test import TestCase -import httpretty import mock from opaque_keys.edx.keys import CourseKey @@ -14,7 +11,6 @@ from openedx.core.djangoapps.catalog import utils from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.catalog.tests import factories, mixins from student.tests.factories import UserFactory, AnonymousUserFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils' @@ -23,9 +19,7 @@ UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils' # ConfigurationModels use the cache. Make every cache get a miss. @mock.patch('config_models.models.cache.get', return_value=None) class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): - """ - Tests covering retrieval of programs from the catalog service. - """ + """Tests covering retrieval of programs from the catalog service.""" def setUp(self): super(TestGetPrograms, self).setUp() @@ -35,9 +29,7 @@ class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): self.catalog_integration = self.create_catalog_integration(cache_ttl=1) def assert_contract(self, call_args, program_uuid=None, type=None): # pylint: disable=redefined-builtin - """ - Verify that API data retrieval utility is used correctly. - """ + """Verify that API data retrieval utility is used correctly.""" args, kwargs = call_args for arg in (self.catalog_integration, self.user, 'programs'): @@ -183,9 +175,7 @@ class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): class TestMungeCatalogProgram(TestCase): - """ - Tests covering querystring stripping. - """ + """Tests covering querystring stripping.""" catalog_program = factories.Program() def test_munge_catalog_program(self): @@ -230,173 +220,90 @@ class TestMungeCatalogProgram(TestCase): self.assertEqual(munged, expected) -@httpretty.activate -class TestGetCourseRun(mixins.CatalogIntegrationMixin, CacheIsolationTestCase): - """ - Tests covering retrieval of course runs from the catalog service. - """ - - ENABLED_CACHES = ['default'] - +@mock.patch(UTILS_MODULE + '.get_edx_api_data') +@mock.patch('config_models.models.cache.get', return_value=None) +class TestGetCourseRun(mixins.CatalogIntegrationMixin, TestCase): + """Tests covering retrieval of course runs from the catalog service.""" def setUp(self): super(TestGetCourseRun, self).setUp() self.user = UserFactory() - self.catalog_integration = self.create_catalog_integration( - internal_api_url="http://catalog.example.com:443/api/v1", - cache_ttl=1, - ) - self.course_runs = [factories.CourseRun() for __ in range(4)] - self.course_key_1 = CourseKey.from_string(self.course_runs[0]["key"]) - self.course_key_2 = CourseKey.from_string(self.course_runs[1]["key"]) - self.course_key_3 = CourseKey.from_string(self.course_runs[2]["key"]) - self.course_key_4 = CourseKey.from_string(self.course_runs[3]["key"]) + self.course_key = CourseKey.from_string('foo/bar/baz') + self.catalog_integration = self.create_catalog_integration() - def test_config_missing(self): - """ - Verify that no errors occur if this method is called when catalog config is missing. - """ - CatalogIntegration.objects.all().delete() + def assert_contract(self, call_args): + """Verify that API data retrieval utility is used correctly.""" + args, kwargs = call_args - data = utils.get_course_runs([], self.user) + for arg in (self.catalog_integration, self.user, 'course_runs'): + self.assertIn(arg, args) + + self.assertEqual(kwargs['resource_id'], unicode(self.course_key)) + self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url) # pylint: disable=protected-access + + return args, kwargs + + def test_get_course_run(self, _mock_cache, mock_get_catalog_data): + course_run = factories.CourseRun() + mock_get_catalog_data.return_value = course_run + + data = utils.get_course_run(self.course_key, self.user) + + self.assert_contract(mock_get_catalog_data.call_args) + self.assertEqual(data, course_run) + + def test_course_run_unavailable(self, _mock_cache, mock_get_catalog_data): + mock_get_catalog_data.return_value = [] + + data = utils.get_course_run(self.course_key, self.user) + + self.assert_contract(mock_get_catalog_data.call_args) self.assertEqual(data, {}) - def test_get_course_run(self): - course_keys = [self.course_key_1] - course_key_strings = [self.course_runs[0]["key"]] - self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0]]) + def test_cache_disabled(self, _mock_cache, mock_get_catalog_data): + utils.get_course_run(self.course_key, self.user) - course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) - expected_data = {self.course_runs[0]["key"]: self.course_runs[0]} - self.assertEqual(expected_data, course_catalog_data_dict) + _, kwargs = self.assert_contract(mock_get_catalog_data.call_args) - def test_get_multiple_course_run(self): - course_key_strings = [self.course_runs[0]["key"], self.course_runs[1]["key"], self.course_runs[2]["key"]] - course_keys = [self.course_key_1, self.course_key_2, self.course_key_3] - self.register_catalog_course_run_response( - course_key_strings, [self.course_runs[0], self.course_runs[1], self.course_runs[2]] - ) + self.assertIsNone(kwargs['cache_key']) - course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) - expected_data = { - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - self.course_runs[2]["key"]: self.course_runs[2], - } - self.assertEqual(expected_data, course_catalog_data_dict) + def test_cache_enabled(self, _mock_cache, mock_get_catalog_data): + catalog_integration = self.create_catalog_integration(cache_ttl=1) - def test_course_run_unavailable(self): - course_key_strings = [self.course_runs[0]["key"], self.course_runs[3]["key"]] - course_keys = [self.course_key_1, self.course_key_4] - self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0]]) + utils.get_course_run(self.course_key, self.user) - course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) - expected_data = {self.course_runs[0]["key"]: self.course_runs[0]} - self.assertEqual(expected_data, course_catalog_data_dict) + _, kwargs = mock_get_catalog_data.call_args - def test_cached_course_run_data(self): - course_key_strings = [self.course_runs[0]["key"], self.course_runs[1]["key"]] - course_keys = [self.course_key_1, self.course_key_2] - course_cached_keys = [ - "{}{}".format(utils.CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_runs[0]["key"]), - "{}{}".format(utils.CatalogCacheUtility.CACHE_KEY_PREFIX, self.course_runs[1]["key"]), - ] - self.register_catalog_course_run_response(course_key_strings, [self.course_runs[0], self.course_runs[1]]) - expected_data = { - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - } + self.assertEqual(kwargs['cache_key'], catalog_integration.CACHE_KEY) - course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) - self.assertEqual(expected_data, course_catalog_data_dict) - cached_data = cache.get_many(course_cached_keys) - self.assertEqual(set(course_cached_keys), set(cached_data.keys())) + def test_config_missing(self, _mock_cache, _mock_get_catalog_data): + """Verify that no errors occur if this method is called when catalog config is missing.""" + CatalogIntegration.objects.all().delete() - with mock.patch('openedx.core.djangoapps.catalog.utils.get_edx_api_data') as mock_method: - course_catalog_data_dict = utils.get_course_runs(course_keys, self.user) - self.assertEqual(0, mock_method.call_count) - self.assertEqual(expected_data, course_catalog_data_dict) + data = utils.get_course_run(self.course_key, self.user) + self.assertEqual(data, {}) -class TestGetRunMarketingUrl(TestCase, mixins.CatalogIntegrationMixin): - """ - Tests covering retrieval of course run marketing URLs. - """ +@mock.patch(UTILS_MODULE + '.get_course_run') +class TestGetRunMarketingUrl(TestCase): + """Tests covering retrieval of course run marketing URLs.""" def setUp(self): super(TestGetRunMarketingUrl, self).setUp() + + self.course_key = CourseKey.from_string('foo/bar/baz') self.user = UserFactory() - self.course_runs = [factories.CourseRun() for __ in range(2)] - self.course_key_1 = CourseKey.from_string(self.course_runs[0]["key"]) - def test_get_run_marketing_url(self): - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - }): - course_marketing_url = utils.get_run_marketing_url(self.course_key_1, self.user) - self.assertEqual(self.course_runs[0]["marketing_url"], course_marketing_url) + def test_get_run_marketing_url(self, mock_get_course_run): + course_run = factories.CourseRun() + mock_get_course_run.return_value = course_run - def test_marketing_url_catalog_course_run_not_found(self): - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - }): - course_marketing_url = utils.get_run_marketing_url(self.course_key_1, self.user) - self.assertEqual(self.course_runs[0]["marketing_url"], course_marketing_url) + url = utils.get_run_marketing_url(self.course_key, self.user) - def test_marketing_url_missing(self): - self.course_runs[1]["marketing_url"] = None - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - }): - course_marketing_url = utils.get_run_marketing_url(CourseKey.from_string("foo2/bar2/baz2"), self.user) - self.assertEqual(None, course_marketing_url) + self.assertEqual(url, course_run['marketing_url']) + def test_marketing_url_missing(self, mock_get_course_run): + mock_get_course_run.return_value = {} -class TestGetRunMarketingUrls(TestCase, mixins.CatalogIntegrationMixin): - """ - Tests covering retrieval of course run marketing URLs. - """ - def setUp(self): - super(TestGetRunMarketingUrls, self).setUp() - self.user = UserFactory() - self.course_runs = [factories.CourseRun() for __ in range(2)] - self.course_keys = [ - CourseKey.from_string(self.course_runs[0]["key"]), - CourseKey.from_string(self.course_runs[1]["key"]), - ] + url = utils.get_run_marketing_url(self.course_key, self.user) - def test_get_run_marketing_url(self): - expected_data = { - self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], - self.course_runs[1]["key"]: self.course_runs[1]["marketing_url"], - } - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - }): - course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) - self.assertEqual(expected_data, course_marketing_url_dict) - - def test_marketing_url_catalog_course_run_not_found(self): - expected_data = { - self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], - } - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - }): - course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) - self.assertEqual(expected_data, course_marketing_url_dict) - - def test_marketing_url_missing(self): - expected_data = { - self.course_runs[0]["key"]: self.course_runs[0]["marketing_url"], - self.course_runs[1]["key"]: None, - } - self.course_runs[1]["marketing_url"] = None - with mock.patch('openedx.core.djangoapps.catalog.utils.get_course_runs', return_value={ - self.course_runs[0]["key"]: self.course_runs[0], - self.course_runs[1]["key"]: self.course_runs[1], - }): - course_marketing_url_dict = utils.get_run_marketing_urls(self.course_keys, self.user) - self.assertEqual(expected_data, course_marketing_url_dict) + self.assertEqual(url, None) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index e902aa74e7..00b0aa99c4 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -1,9 +1,7 @@ """Helper functions for working with the catalog service.""" -import abc -import logging +from urlparse import urlparse from django.conf import settings -from django.core.cache import cache from django.contrib.auth.models import User from edx_rest_api_client.client import EdxRestApiClient from opaque_keys.edx.keys import CourseKey @@ -13,9 +11,6 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.token_utils import JwtBuilder -log = logging.getLogger(__name__) - - def create_catalog_api_client(user, catalog_integration): """Returns an API client which can be used to make catalog API requests.""" scopes = ['email', 'profile'] @@ -177,57 +172,38 @@ def munge_catalog_program(catalog_program): } -def get_and_cache_course_runs(course_keys, user): - """ - Get course run's data from the course catalog service and cache it. - """ - catalog_course_runs_against_course_keys = {} - catalog_integration = CatalogIntegration.current() - if catalog_integration.enabled: - api = create_catalog_api_client(user, catalog_integration) - - catalog_data = get_edx_api_data( - catalog_integration, - user, - 'course_runs', - api=api, - querystring={'keys': ",".join(course_keys), 'exclude_utm': 1}, - ) - if catalog_data: - for catalog_course_run in catalog_data: - CatalogCacheUtility.cache_course_run(catalog_course_run) - catalog_course_runs_against_course_keys[catalog_course_run["key"]] = catalog_course_run - return catalog_course_runs_against_course_keys - - -def get_course_runs(course_keys, user): - """ - Get course run data from the course catalog service if not available in cache. +def get_course_run(course_key, user): + """Get a course run's data from the course catalog service. Arguments: - course_keys ([CourseKey]): A list of Course key object identifying the run whose data we want. + course_key (CourseKey): Course key object identifying the run whose data we want. user (User): The user to authenticate as when making requests to the catalog service. Returns: - dict of catalog course runs against course keys, empty if no data could be retrieved. + dict, empty if no data could be retrieved. """ - catalog_course_runs_against_course_keys = CatalogCacheUtility.get_cached_catalog_course_runs( - course_keys - ) - if len(catalog_course_runs_against_course_keys.keys()) != len(course_keys): - missing_course_keys = CatalogCacheUtility.get_course_keys_not_found_in_cache( - course_keys, catalog_course_runs_against_course_keys.keys() - ) - catalog_course_runs_against_course_keys.update( - get_and_cache_course_runs(missing_course_keys, user) + catalog_integration = CatalogIntegration.current() + + if catalog_integration.enabled: + api = create_catalog_api_client(user, catalog_integration) + + data = get_edx_api_data( + catalog_integration, + user, + 'course_runs', + resource_id=unicode(course_key), + cache_key=catalog_integration.CACHE_KEY if catalog_integration.is_cache_enabled else None, + api=api, + querystring={'exclude_utm': 1}, ) - return catalog_course_runs_against_course_keys + return data if data else {} + else: + return {} def get_run_marketing_url(course_key, user): - """ - Get a course run's marketing URL from the course catalog service. + """Get a course run's marketing URL from the course catalog service. Arguments: course_key (CourseKey): Course key object identifying the run whose marketing URL we want. @@ -236,111 +212,5 @@ def get_run_marketing_url(course_key, user): Returns: string, the marketing URL, or None if no URL is available. """ - course_marketing_urls = get_run_marketing_urls([course_key], user) - return course_marketing_urls.get(unicode(course_key)) - - -def get_run_marketing_urls(course_keys, user): - """ - Get course run marketing URLs from the course catalog service against course keys. - - Arguments: - course_keys ([CourseKey]): A list of Course key object identifying the run whose data we want. - user (User): The user to authenticate as when making requests to the catalog service. - - Returns: - dict of run marketing URLs against course keys - """ - course_marketing_urls = {} - course_catalog_dict = get_course_runs(course_keys, user) - if not course_catalog_dict: - return course_marketing_urls - - for course_key in course_keys: - course_key_string = unicode(course_key) - if course_key_string in course_catalog_dict: - course_marketing_urls[course_key_string] = course_catalog_dict[course_key_string].get('marketing_url') - - return course_marketing_urls - - -class CatalogCacheUtility(object): - """ - Non-instantiatable class housing utility methods for caching catalog API data. - """ - __metaclass__ = abc.ABCMeta - CACHE_KEY_PREFIX = "catalog.course_runs." - - @classmethod - def get_course_keys_not_found_in_cache(cls, course_keys, cached_course_run_keys): - """ - Get course key strings for which course run data is not available in cache. - - Arguments: - course_key (CourseKey): CourseKey object to create corresponding catalog cache key. - - Returns: - list of string rep of course keys not found in catalog cache. - """ - missing_course_keys = [] - for course_key in course_keys: - course_key_string = unicode(course_key) - if course_key_string not in cached_course_run_keys: - missing_course_keys.append(course_key_string) - - log.info("Cached catalog course run data missing for: '{}'".format( - ", ".join(missing_course_keys) - )) - return missing_course_keys - - @classmethod - def get_cached_catalog_course_runs(cls, course_keys): - """ - Get course runs from cache against course keys. - - Arguments: - course_keys ([CourseKey]): List of CourseKey object identifying the run whose data we want. - - Returns: - dict of catalog course run against course key string - """ - course_catalog_run_cache_keys = [ - cls._get_cache_key_name(course_key) - for course_key in course_keys - ] - cached_catalog_course_runs = cache.get_many(course_catalog_run_cache_keys) - return { - cls._extract_course_key_from_cache_key_name(cached_key): cached_course_run - for cached_key, cached_course_run in cached_catalog_course_runs.iteritems() - } - - @classmethod - def cache_course_run(cls, catalog_course_run): - """ - Caches catalog course run for course key. - """ - cache.set( - cls._get_cache_key_name(catalog_course_run["key"]), - catalog_course_run, - CatalogIntegration.current().cache_ttl - ) - - @classmethod - def _get_cache_key_name(cls, course_key): - """ - Returns key name to use to cache catalog course run data for course key. - - Arguments: - course_key (CourseKey): CourseKey object to create corresponding catalog cache key. - - Returns: - string, catalog cache key against course key. - """ - return "{}{}".format(cls.CACHE_KEY_PREFIX, unicode(course_key)) - - @classmethod - def _extract_course_key_from_cache_key_name(cls, catalog_course_run_cache_key): - """ - Returns course_key extracted from cache key of catalog course run data. - """ - return catalog_course_run_cache_key.replace(cls.CACHE_KEY_PREFIX, '') + course_run = get_course_run(course_key, user) + return course_run.get('marketing_url') diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index b546b1d6ee..b482cbc1b3 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -13,16 +13,15 @@ from django.test import TestCase from django.test.utils import override_settings from django.utils import timezone from django.utils.text import slugify -from edx_oauth2_provider.tests.factories import ClientFactory import httpretty import mock from nose.plugins.attrib import attr from opaque_keys.edx.keys import CourseKey +from edx_oauth2_provider.tests.factories import ClientFactory from provider.constants import CONFIDENTIAL from lms.djangoapps.certificates.api import MODES from lms.djangoapps.commerce.tests.test_utils import update_commerce_config -from openedx.core.djangoapps.catalog.tests import factories as catalog_factories from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.credentials.tests import factories as credentials_factories from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin @@ -704,9 +703,7 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): - """ - Tests of the program data extender utility class. - """ + """Tests of the program data extender utility class.""" maxDiff = None sku = 'abc123' password = 'test' @@ -724,7 +721,6 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.course.start = timezone.now() - datetime.timedelta(days=1) self.course.end = timezone.now() + datetime.timedelta(days=1) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member - self.course_id_string = unicode(self.course.id) # pylint: disable=no-member self.organization = factories.Organization() self.run_mode = factories.RunMode(course_key=unicode(self.course.id)) # pylint: disable=no-member @@ -733,12 +729,9 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): organizations=[self.organization], course_codes=[self.course_code] ) - self.course_run = catalog_factories.CourseRun(key=self.course_id_string) def _assert_supplemented(self, actual, **kwargs): - """ - DRY helper used to verify that program data is extended correctly. - """ + """DRY helper used to verify that program data is extended correctly.""" course_overview = CourseOverview.get_from_id(self.course.id) # pylint: disable=no-member run_mode = dict( factories.RunMode( @@ -771,9 +764,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @ddt.unpack @mock.patch(UTILS_MODULE + '.CourseMode.mode_for_course') def test_student_enrollment_status(self, is_enrolled, enrolled_mode, is_upgrade_required, mock_get_mode): - """ - Verify that program data is supplemented with the student's enrollment status. - """ + """Verify that program data is supplemented with the student's enrollment status.""" expected_upgrade_url = '{root}/{path}?sku={sku}'.format( root=ECOMMERCE_URL_ROOT, path=self.checkout_path.strip('/'), @@ -799,9 +790,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @ddt.data(MODES.audit, MODES.verified) def test_inactive_enrollment_no_upgrade(self, enrolled_mode): - """ - Verify that a student with an inactive enrollment isn't encouraged to upgrade. - """ + """Verify that a student with an inactive enrollment isn't encouraged to upgrade.""" update_commerce_config(enabled=True, checkout_page=self.checkout_path) CourseEnrollmentFactory( @@ -817,9 +806,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.CourseMode.mode_for_course') def test_ecommerce_disabled(self, mock_get_mode): - """ - Verify that the utility can operate when the ecommerce service is disabled. - """ + """Verify that the utility can operate when the ecommerce service is disabled.""" update_commerce_config(enabled=False, checkout_page=self.checkout_path) mock_mode = mock.Mock() @@ -838,9 +825,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): ) @ddt.unpack def test_course_enrollment_status(self, start_offset, end_offset, is_enrollment_open): - """ - Verify that course enrollment status is reflected correctly. - """ + """Verify that course enrollment status is reflected correctly.""" self.course.enrollment_start = timezone.now() - datetime.timedelta(days=start_offset) self.course.enrollment_end = timezone.now() - datetime.timedelta(days=end_offset) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member @@ -854,8 +839,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): ) def test_no_enrollment_start_date(self): - """ - Verify that a closed course with no explicit enrollment start date doesn't cause an error. + """Verify that a closed course with no explicit enrollment start date doesn't cause an error. Regression test for ECOM-4973. """ @@ -873,9 +857,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.certificate_api.certificate_downloadable_status') @mock.patch(CERTIFICATES_API_MODULE + '.has_html_certificates_enabled') def test_certificate_url_retrieval(self, is_uuid_available, mock_html_certs_enabled, mock_get_cert_data): - """ - Verify that the student's run mode certificate is included, when available. - """ + """Verify that the student's run mode certificate is included, when available.""" test_uuid = uuid.uuid4().hex mock_get_cert_data.return_value = {'uuid': test_uuid} if is_uuid_available else {} mock_html_certs_enabled.return_value = True @@ -900,9 +882,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.get_organization_by_short_name') def test_organization_logo_exists(self, mock_get_organization_by_short_name): - """ - Verify the logo image is set from the organizations api. - """ + """ Verify the logo image is set from the organizations api """ mock_logo_url = 'edx/logo.png' mock_image = mock.Mock() mock_image.url = mock_logo_url @@ -915,9 +895,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): @mock.patch(UTILS_MODULE + '.get_organization_by_short_name') def test_organization_missing(self, mock_get_organization_by_short_name): - """ - Verify the logo image is not set if the organizations api returns None. - """ + """ Verify the logo image is not set if the organizations api returns None """ mock_get_organization_by_short_name.return_value = None data = utils.ProgramDataExtender(self.program, self.user).extend() self.assertEqual(data['organizations'][0].get('img'), None)