diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index a3f28b56cd..228104665b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -123,7 +123,7 @@ import newrelic_custom_metrics # Note that this lives in LMS, so this dependency should be refactored. from notification_prefs.views import enable_notifications -from openedx.core.djangoapps.catalog.utils import get_programs_with_type_logo +from openedx.core.djangoapps.catalog.utils import get_programs_with_type from openedx.core.djangoapps.credit.email_utils import get_credit_provider_display_names, make_providers_strings from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.programs.models import ProgramsApiConfig @@ -211,13 +211,15 @@ def index(request, extra_context=None, user=AnonymousUser()): # Insert additional context for use in the template context.update(extra_context) - # Getting all the programs from course-catalog service. The programs_list is being added to the context but it's - # not being used currently in lms/templates/index.html. To use this list, you need to create a custom theme that - # overrides index.html. The modifications to index.html to display the programs will be done after the support - # for edx-pattern-library is added. - if configuration_helpers.get_value("DISPLAY_PROGRAMS_ON_MARKETING_PAGES", - settings.FEATURES.get("DISPLAY_PROGRAMS_ON_MARKETING_PAGES")): - programs_list = get_programs_with_type_logo() + # Get the active programs of the type configured for the current site from the catalog service. The programs_list + # is being added to the context but it's not being used currently in courseware/courses.html. To use this list, + # you need to create a custom theme that overrides courses.html. The modifications to courses.html to display the + # programs will be done after the support for edx-pattern-library is added. + program_types = configuration_helpers.get_value('ENABLED_PROGRAM_TYPES') + + # Do not add programs to the context if there are no program types enabled for the site. + if program_types: + programs_list = get_programs_with_type(program_types) context["programs_list"] = programs_list diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index 6a3f3f34aa..89b109c50a 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -18,6 +18,7 @@ from edxmako.shortcuts import render_to_response from branding.views import index from courseware.tests.helpers import LoginEnrollmentTestCase from milestones.tests.utils import MilestonesTestCaseMixin +from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from util.milestones_helpers import set_prerequisite_courses from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -289,29 +290,27 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase): @ddt.ddt @attr(shard=1) -class IndexPageProgramsTests(ModuleStoreTestCase): +class IndexPageProgramsTests(SiteMixin, ModuleStoreTestCase): """ Tests for Programs List in Marketing Pages. """ + @ddt.data([], ['fake_program_type']) + def test_get_programs_with_type_called(self, program_types): + self.site_configuration.values.update({ + 'ENABLED_PROGRAM_TYPES': program_types + }) + self.site_configuration.save() - def setUp(self): - super(IndexPageProgramsTests, self).setUp() - self.client.login(username=self.user.username, password=self.user_password) + views = [ + (reverse('root'), 'student.views.get_programs_with_type'), + (reverse('branding.views.courses'), 'courseware.views.views.get_programs_with_type'), + ] + for url, dotted_path in views: + with patch(dotted_path) as mock_get_programs_with_type: + response = self.client.get(url) + self.assertEqual(response.status_code, 200) - @ddt.data(True, False) - def test_programs_with_type_logo_called(self, display_programs): - with patch.dict('django.conf.settings.FEATURES', {'DISPLAY_PROGRAMS_ON_MARKETING_PAGES': display_programs}): - views = [ - (reverse('dashboard'), 'student.views.get_programs_with_type_logo'), - (reverse('branding.views.courses'), 'courseware.views.views.get_programs_with_type_logo'), - ] - - for url, dotted_path in views: - with patch(dotted_path) as mock_get_programs_with_type_logo: - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - if display_programs: - mock_get_programs_with_type_logo.assert_called_once() - else: - mock_get_programs_with_type_logo.assert_not_called_() + if program_types: + mock_get_programs_with_type.assert_called_once() + else: + mock_get_programs_with_type.assert_not_called() diff --git a/lms/djangoapps/branding/tests/test_views.py b/lms/djangoapps/branding/tests/test_views.py index 5364a0f82b..2098f654d9 100644 --- a/lms/djangoapps/branding/tests/test_views.py +++ b/lms/djangoapps/branding/tests/test_views.py @@ -268,3 +268,12 @@ class TestIndex(SiteMixin, TestCase): self.client.login(username=self.user.username, password="password") response = self.client.get(reverse("dashboard")) self.assertIn(self.site_configuration_other.values["MKTG_URLS"]["ROOT"], response.content) + + def test_index_with_enabled_program_types(self): + """ Test index view with Enabled Program Types.""" + self.site_configuration.values.update({'ENABLED_PROGRAM_TYPES': ['TestProgramType']}) + self.site_configuration.save() + with mock.patch('student.views.get_programs_with_type') as patched_get_programs_with_type: + patched_get_programs_with_type.return_value = [] + response = self.client.get(reverse("root")) + self.assertEqual(response.status_code, 200) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index e2d448f462..45782714e1 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -40,7 +40,6 @@ from lms.djangoapps.instructor.enrollment import uses_shib from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException -from openedx.core.djangoapps.catalog.utils import get_programs_with_type_logo import shoppingcart import survey.utils import survey.views @@ -72,6 +71,7 @@ from courseware.models import StudentModule, BaseStudentModuleHistory from courseware.url_helpers import get_redirect_url, get_redirect_url_for_global_staff from courseware.user_state_client import DjangoXBlockUserStateClient from edxmako.shortcuts import render_to_response, render_to_string, marketing_link +from openedx.core.djangoapps.catalog.utils import get_programs_with_type from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.coursetalk.helpers import inject_coursetalk_keys_into_context from openedx.core.djangoapps.credit.api import ( @@ -149,13 +149,15 @@ def courses(request): else: courses_list = sort_by_announcement(courses_list) - # Getting all the programs from course-catalog service. The programs_list is being added to the context but it's - # not being used currently in courseware/courses.html. To use this list, you need to create a custom theme that - # overrides courses.html. The modifications to courses.html to display the programs will be done after the support - # for edx-pattern-library is added. - if configuration_helpers.get_value("DISPLAY_PROGRAMS_ON_MARKETING_PAGES", - settings.FEATURES.get("DISPLAY_PROGRAMS_ON_MARKETING_PAGES")): - programs_list = get_programs_with_type_logo() + # Get the active programs of the type configured for the current site from the catalog service. The programs_list + # is being added to the context but it's not being used currently in courseware/courses.html. To use this list, + # you need to create a custom theme that overrides courses.html. The modifications to courses.html to display the + # programs will be done after the support for edx-pattern-library is added. + program_types = configuration_helpers.get_value('ENABLED_PROGRAM_TYPES') + + # Do not add programs to the context if there are no program types enabled for the site. + if program_types: + programs_list = get_programs_with_type(program_types) return render_to_response( "courseware/courses.html", diff --git a/lms/envs/common.py b/lms/envs/common.py index 4b224e106d..54857fbc33 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -254,10 +254,6 @@ FEATURES = { # Set to True to change the course sorting behavior by their start dates, latest first. 'ENABLE_COURSE_SORTING_BY_START_DATE': True, - # When set to True, a list of programs is displayed along with the list of courses - # when the user visits the homepage or the find courses page. - 'DISPLAY_PROGRAMS_ON_MARKETING_PAGES': False, - # Expose Mobile REST API. Note that if you use this, you must also set # ENABLE_OAUTH2_PROVIDER to True 'ENABLE_MOBILE_REST_API': False, diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 447aee68d2..6b6ae68305 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -6,7 +6,6 @@ import copy from django.contrib.auth import get_user_model from django.test import TestCase import mock -from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, ProgramTypeFactory @@ -14,7 +13,7 @@ from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.catalog.utils import ( get_programs, get_program_types, - get_programs_with_type_logo, + get_programs_with_type, ) from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory @@ -32,12 +31,12 @@ class TestGetPrograms(CatalogIntegrationMixin, TestCase): super(TestGetPrograms, self).setUp() self.uuid = str(uuid.uuid4()) - self.type = 'FooBar' + self.types = ['Foo', 'Bar', 'FooBar'] self.catalog_integration = self.create_catalog_integration(cache_ttl=1) UserFactory(username=self.catalog_integration.service_username) - def assert_contract(self, call_args, program_uuid=None, type=None): # pylint: disable=redefined-builtin + def assert_contract(self, call_args, program_uuid=None, types=None): # pylint: disable=redefined-builtin """Verify that API data retrieval utility is used correctly.""" args, kwargs = call_args @@ -46,9 +45,10 @@ class TestGetPrograms(CatalogIntegrationMixin, TestCase): self.assertEqual(kwargs['resource_id'], program_uuid) - cache_key = '{base}.programs{type}'.format( + types_param = ','.join(types) if types and isinstance(types, list) else None + cache_key = '{base}.programs{types}'.format( base=self.catalog_integration.CACHE_KEY, - type='.' + type if type else '' + types='.' + types_param if types_param else '' ) self.assertEqual( kwargs['cache_key'], @@ -61,8 +61,10 @@ class TestGetPrograms(CatalogIntegrationMixin, TestCase): 'marketable': 1, 'exclude_utm': 1, } - if type: - querystring['type'] = type + if program_uuid: + querystring['use_full_course_serializer'] = 1 + if types: + querystring['types'] = types_param self.assertEqual(kwargs['querystring'], querystring) return args, kwargs @@ -85,13 +87,13 @@ class TestGetPrograms(CatalogIntegrationMixin, TestCase): self.assert_contract(mock_get_edx_api_data.call_args, program_uuid=self.uuid) self.assertEqual(data, program) - def test_get_programs_by_type(self, mock_get_edx_api_data): + def test_get_programs_by_types(self, mock_get_edx_api_data): programs = ProgramFactory.create_batch(2) mock_get_edx_api_data.return_value = programs - data = get_programs(type=self.type) + data = get_programs(types=self.types) - self.assert_contract(mock_get_edx_api_data.call_args, type=self.type) + self.assert_contract(mock_get_edx_api_data.call_args, types=self.types) self.assertEqual(data, programs) def test_programs_unavailable(self, mock_get_edx_api_data): @@ -129,13 +131,37 @@ class TestGetPrograms(CatalogIntegrationMixin, TestCase): data = get_programs() self.assertEqual(data, []) + @mock.patch(UTILS_MODULE + '.get_programs') + @mock.patch(UTILS_MODULE + '.get_program_types') + def test_get_programs_with_type(self, mock_get_program_types, mock_get_programs, _mock_get_edx_api_data): + """Verify get_programs_with_type returns the expected list of programs.""" + programs_with_program_type = [] + programs = ProgramFactory.create_batch(2) + program_types = [] + + for program in programs: + program_type = ProgramTypeFactory(name=program['type']) + program_types.append(program_type) + + program_with_type = copy.deepcopy(program) + program_with_type['type'] = program_type + programs_with_program_type.append(program_with_type) + + mock_get_programs.return_value = programs + mock_get_program_types.return_value = program_types + + actual = get_programs_with_type() + + self.assertEqual(actual, programs_with_program_type) + @skip_unless_lms @mock.patch(UTILS_MODULE + '.get_edx_api_data') class TestGetProgramTypes(CatalogIntegrationMixin, TestCase): """Tests covering retrieval of program types from the catalog service.""" def test_get_program_types(self, mock_get_edx_api_data): - program_types = [ProgramTypeFactory() for __ in range(3)] + """Verify get_program_types returns the expected list of program types.""" + program_types = ProgramTypeFactory.create_batch(3) mock_get_edx_api_data.return_value = program_types # Catalog integration is disabled. @@ -147,28 +173,6 @@ class TestGetProgramTypes(CatalogIntegrationMixin, TestCase): data = get_program_types() self.assertEqual(data, program_types) - def test_get_programs_with_type_logo(self, _mock_get_edx_api_data): - programs = [] - program_types = [] - programs_with_type_logo = [] - - for index in range(3): - # Creating the Programs and their corresponding program types. - type_name = 'type_name_{postfix}'.format(postfix=index) - program = ProgramFactory(type=type_name) - program_type = ProgramTypeFactory(name=type_name) - - programs.append(program) - program_types.append(program_type) - - program_with_type_logo = copy.deepcopy(program) - program_with_type_logo['logo_image'] = program_type['logo_image'] - programs_with_type_logo.append(program_with_type_logo) - - with mock.patch('openedx.core.djangoapps.catalog.utils.get_programs') as patched_get_programs: - with mock.patch('openedx.core.djangoapps.catalog.utils.get_program_types') as patched_get_program_types: - patched_get_programs.return_value = programs - patched_get_program_types.return_value = program_types - - actual = get_programs_with_type_logo() - self.assertEqual(actual, programs_with_type_logo) + program = program_types[0] + data = get_program_types(name=program['name']) + self.assertEqual(data, program) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 463cdb53c0..62572fd9f6 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -1,8 +1,9 @@ """Helper functions for working with the catalog service.""" +import copy + from django.conf import settings from django.contrib.auth import get_user_model from edx_rest_api_client.client import EdxRestApiClient -from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.lib.edx_api_utils import get_edx_api_data @@ -21,12 +22,14 @@ def create_catalog_api_client(user, catalog_integration): return EdxRestApiClient(catalog_integration.internal_api_url, jwt=jwt) -def get_programs(uuid=None, type=None): # pylint: disable=redefined-builtin +def get_programs(uuid=None, types=None): # pylint: disable=redefined-builtin """Retrieve marketable programs from the catalog service. Keyword Arguments: uuid (string): UUID identifying a specific program. - type (string): Filter programs by type (e.g., "MicroMasters" will only return MicroMasters programs). + types (list of string): List of program type names used to filter programs by type + (e.g., ["MicroMasters"] will only return MicroMasters programs, + ["MicroMasters", "XSeries"] will return MicroMasters and XSeries programs). Returns: list of dict, representing programs. @@ -40,18 +43,21 @@ def get_programs(uuid=None, type=None): # pylint: disable=redefined-builtin return [] api = create_catalog_api_client(user, catalog_integration) + types_param = ','.join(types) if types else None - cache_key = '{base}.programs{type}'.format( + cache_key = '{base}.programs{types}'.format( base=catalog_integration.CACHE_KEY, - type='.' + type if type else '' + types='.' + types_param if types_param else '' ) querystring = { 'marketable': 1, 'exclude_utm': 1, } - if type: - querystring['type'] = type + if uuid: + querystring['use_full_course_serializer'] = 1 + if types_param: + querystring['types'] = types_param return get_edx_api_data( catalog_integration, @@ -66,11 +72,15 @@ def get_programs(uuid=None, type=None): # pylint: disable=redefined-builtin return [] -def get_program_types(): - """Retrieve all program types from the catalog service. +def get_program_types(name=None): + """Retrieve program types from the catalog service. + + Keyword Arguments: + name (string): Name identifying a specific program. Returns: list of dict, representing program types. + dict, if a specific program type is requested. """ catalog_integration = CatalogIntegration.current() if catalog_integration.enabled: @@ -82,27 +92,46 @@ def get_program_types(): api = create_catalog_api_client(user, catalog_integration) cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY) - return get_edx_api_data( + data = get_edx_api_data( catalog_integration, user, 'program_types', cache_key=cache_key if catalog_integration.is_cache_enabled else None, api=api ) + + # Filter by name if a name was provided + if name: + data = next(program_type for program_type in data if program_type['name'] == name) + + return data else: return [] -def get_programs_with_type_logo(): +def get_programs_with_type(types=None): """ - Join program type logos with programs of corresponding type. + Return the list of programs. You can filter the types of programs returned using the optional + types parameter. If no filter is provided, all programs of all types will be returned. + + The program dict is updated with the fully serialized program type. + + Keyword Arguments: + types (list): List of program type slugs to filter by. + + Return: + list of dict, representing the active programs. """ - programs_list = get_programs() - program_types = get_program_types() + programs_with_type = [] + programs = get_programs(types=types) - type_logo_map = {program_type['name']: program_type['logo_image'] for program_type in program_types} + if programs: + program_types = {program_type['name']: program_type for program_type in get_program_types()} + for program in programs: + # deepcopy the program dict here so we are not adding + # the type to the cached object + program_with_type = copy.deepcopy(program) + program_with_type['type'] = program_types[program['type']] + programs_with_type.append(program_with_type) - for program in programs_list: - program['logo_image'] = type_logo_map[program['type']] - - return programs_list + return programs_with_type diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index f3f7be342a..72c29ac363 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -1,17 +1,14 @@ """Tests covering Programs utilities.""" # pylint: disable=no-member import datetime -import json import uuid import ddt -from django.core.cache import cache from django.core.urlresolvers import reverse from django.test import TestCase from django.test.utils import override_settings import mock from nose.plugins.attrib import attr -from opaque_keys.edx.keys import CourseKey from pytz import utc from lms.djangoapps.certificates.api import MODES @@ -21,15 +18,12 @@ from openedx.core.djangoapps.catalog.tests.factories import ( ProgramFactory, CourseFactory, CourseRunFactory, - OrganizationFactory, ) -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin from openedx.core.djangoapps.programs.tests.factories import ProgressFactory from openedx.core.djangoapps.programs.utils import ( DEFAULT_ENROLLMENT_START_DATE, ProgramProgressMeter, ProgramDataExtender ) -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory, CourseEnrollmentFactory from util.date_utils import strftime_localized from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -388,6 +382,18 @@ class TestProgramDataExtender(ModuleStoreTestCase): maxDiff = None sku = 'abc123' checkout_path = '/basket' + instructors = { + 'instructors': [ + { + 'name': 'test-instructor1', + 'organization': 'TextX', + }, + { + 'name': 'test-instructor2', + 'organization': 'TextX', + } + ] + } def setUp(self): super(TestProgramDataExtender, self).setUp() @@ -395,6 +401,7 @@ class TestProgramDataExtender(ModuleStoreTestCase): self.course = ModuleStoreCourseFactory() self.course.start = datetime.datetime.now(utc) - datetime.timedelta(days=1) self.course.end = datetime.datetime.now(utc) + datetime.timedelta(days=1) + self.course.instructor_info = self.instructors self.course = self.update_course(self.course, self.user.id) self.course_run = CourseRunFactory(key=unicode(self.course.id)) @@ -561,3 +568,9 @@ class TestProgramDataExtender(ModuleStoreTestCase): ) if is_uuid_available else None self._assert_supplemented(data, certificate_url=expected_url) + + def test_instructors_retrieval(self): + data = ProgramDataExtender(self.program, self.user).extend(include_instructors=True) + + self.program.update(self.instructors['instructors']) + self.assertEqual(data, self.program) diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 874df4e7ad..7aa6f1fd5d 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -5,6 +5,7 @@ import datetime from urlparse import urljoin from django.conf import settings +from django.core.cache import cache from django.core.urlresolvers import reverse from django.utils.functional import cached_property from opaque_keys.edx.keys import CourseKey @@ -15,9 +16,9 @@ from lms.djangoapps.certificates import api as certificate_api from lms.djangoapps.commerce.utils import EcommerceService from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.lib.edx_api_utils import get_edx_api_data from student.models import CourseEnrollment from util.date_utils import strftime_localized +from xmodule.modulestore.django import modulestore # The datetime module's strftime() methods require a year >= 1900. @@ -247,9 +248,12 @@ class ProgramDataExtender(object): self.course_overview = None self.enrollment_start = None - def extend(self): + def extend(self, include_instructors=False): """Execute extension handlers, returning the extended data.""" - self._execute('_extend') + if include_instructors: + self._execute('_extend') + else: + self._execute('_extend_course_runs') return self.data def _execute(self, prefix, *args): @@ -261,6 +265,9 @@ class ProgramDataExtender(object): """Returns a generator yielding method names beginning with the given prefix.""" return (name for name in cls.__dict__ if name.startswith(prefix)) + def _extend_with_instructors(self): + self._execute('_attach_instructors') + def _extend_course_runs(self): """Execute course run data handlers.""" for course in self.data['courses']: @@ -326,3 +333,32 @@ class ProgramDataExtender(object): run_mode['upgrade_url'] = None else: run_mode['upgrade_url'] = None + + def _attach_instructors(self): + """ + Extend the program data with instructor data. The instructor data added here is persisted + on each course in modulestore and can be edited in Studio. Once the course metadata publisher tool + supports the authoring of course instructor data, we will be able to migrate course + instructor data into the catalog, retrieve it via the catalog API, and remove this code. + """ + cache_key = 'program.instructors.{uuid}'.format( + uuid=self.data['uuid'] + ) + program_instructors = cache.get(cache_key) + if not program_instructors: + instructors_by_name = {} + module_store = modulestore() + for course in self.data['courses']: + for course_run in course['course_runs']: + course_run_key = CourseKey.from_string(course_run['key']) + course_descriptor = module_store.get_course(course_run_key) + if course_descriptor: + course_instructors = getattr(course_descriptor, 'instructor_info', {}) + # Deduplicate program instructors using instructor name + instructors_by_name.update({instructor.get('name'): instructor for instructor + in course_instructors.get('instructors', [])}) + + program_instructors = instructors_by_name.values() + cache.set(cache_key, program_instructors, 3600) + + self.data['instructors'] = program_instructors