diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 16fc94f5c5..61979f204f 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -1103,13 +1103,17 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= ] def setUp(self): - """ Initialize course used to test enrollment fields. """ + """ + Initialize course used to test enrollment fields. + """ super(CourseEnrollmentEndFieldTest, self).setUp() self.course = CourseFactory.create(org='edX', number='dummy', display_name='Marketing Site Course') self.course_details_url = reverse_course_url('settings_handler', unicode(self.course.id)) def _get_course_details_response(self, global_staff): - """ Return the course details page as either global or non-global staff""" + """ + Return the course details page as either global or non-global staff + """ user = UserFactory(is_staff=global_staff) CourseInstructorRole(self.course.id).add_users(user) @@ -1118,7 +1122,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= return self.client.get_html(self.course_details_url) def _verify_editable(self, response): - """ Verify that the response has expected editable fields. + """ + Verify that the response has expected editable fields. Assert that all editable field content exists and no uneditable field content exists for enrollment end fields. @@ -1131,7 +1136,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= self.assertContains(response, element) def _verify_not_editable(self, response): - """ Verify that the response has expected non-editable fields. + """ + Verify that the response has expected non-editable fields. Assert that all uneditable field content exists and no editable field content exists for enrollment end fields. @@ -1145,7 +1151,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_MKTG_SITE': False}) def test_course_details_with_disabled_setting_global_staff(self): - """ Test that user enrollment end date is editable in response. + """ + Test that user enrollment end date is editable in response. Feature flag 'ENABLE_MKTG_SITE' is not enabled. User is global staff. @@ -1154,7 +1161,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_MKTG_SITE': False}) def test_course_details_with_disabled_setting_non_global_staff(self): - """ Test that user enrollment end date is editable in response. + """ + Test that user enrollment end date is editable in response. Feature flag 'ENABLE_MKTG_SITE' is not enabled. User is non-global staff. @@ -1164,7 +1172,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) def test_course_details_with_enabled_setting_global_staff(self): - """ Test that user enrollment end date is editable in response. + """ + Test that user enrollment end date is editable in response. Feature flag 'ENABLE_MKTG_SITE' is enabled. User is global staff. @@ -1174,7 +1183,8 @@ id=\"course-enrollment-end-time\" value=\"\" placeholder=\"HH:MM\" autocomplete= @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) def test_course_details_with_enabled_setting_non_global_staff(self): - """ Test that user enrollment end date is not editable in response. + """ + Test that user enrollment end date is not editable in response. Feature flag 'ENABLE_MKTG_SITE' is enabled. User is non-global staff. diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e1bf0c2d79..6b40256408 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -74,7 +74,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_lms_link_for_about_page +from util.course import get_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 ( @@ -987,7 +987,7 @@ def settings_handler(request, course_key_string): settings_context = { 'context_course': course_module, 'course_locator': course_key, - 'lms_link_for_about_page': get_lms_link_for_about_page(course_key), + 'lms_link_for_about_page': get_link_for_about_page(course_module), '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/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py b/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py index a93e7d8641..f90fde577a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py +++ b/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py @@ -14,7 +14,8 @@ from openedx.core.djangoapps.credit.signals import on_course_publish class CreditEligibilityTest(CourseTestCase): - """Base class to test the course settings details view in Studio for credit + """ + Base class to test the course settings details view in Studio for credit eligibility requirements. """ def setUp(self): @@ -24,7 +25,8 @@ class CreditEligibilityTest(CourseTestCase): @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_CREDIT_ELIGIBILITY': False}) def test_course_details_with_disabled_setting(self): - """Test that user don't see credit eligibility requirements in response + """ + Test that user don't see credit eligibility requirements in response if the feature flag 'ENABLE_CREDIT_ELIGIBILITY' is not enabled. """ response = self.client.get_html(self.course_details_url) @@ -34,7 +36,8 @@ class CreditEligibilityTest(CourseTestCase): @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_CREDIT_ELIGIBILITY': True}) def test_course_details_with_enabled_setting(self): - """Test that credit eligibility requirements are present in + """ + Test that credit eligibility requirements are present in response if the feature flag 'ENABLE_CREDIT_ELIGIBILITY' is enabled. """ # verify that credit eligibility requirements block don't show if the diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 46720b326d..ce85903607 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -4,25 +4,26 @@ Utility methods related to course import logging from django.conf import settings -from opaque_keys.edx.keys import CourseKey - log = logging.getLogger(__name__) -def get_lms_link_for_about_page(course_key): +def get_link_for_about_page(course): """ - Returns the url to the course about page. - """ - assert isinstance(course_key, CourseKey) + Arguments: + course: This can be either a course overview object or a course descriptor. - if settings.FEATURES.get('ENABLE_MKTG_SITE'): - # 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'] + Returns the course sharing url, this can be one of course's social sharing url, marketing url, or + lms course about url. + """ + is_social_sharing_enabled = getattr(settings, 'SOCIAL_SHARING_SETTINGS', {}).get('CUSTOM_COURSE_URLS') + if is_social_sharing_enabled and course.social_sharing_url: + course_about_url = course.social_sharing_url + elif settings.FEATURES.get('ENABLE_MKTG_SITE') and getattr(course, 'marketing_url', None): + course_about_url = course.marketing_url else: - about_base = settings.LMS_ROOT_URL + course_about_url = u'{about_base_url}/courses/{course_key}/about'.format( + about_base_url=settings.LMS_ROOT_URL, + course_key=unicode(course.id), + ) - return u"{about_base_url}/courses/{course_key}/about".format( - about_base_url=about_base, - course_key=course_key.to_deprecated_string() - ) + return course_about_url diff --git a/common/djangoapps/util/tests/test_course.py b/common/djangoapps/util/tests/test_course.py index 491119546b..dacdd15b02 100644 --- a/common/djangoapps/util/tests/test_course.py +++ b/common/djangoapps/util/tests/test_course.py @@ -1,36 +1,129 @@ """ Tests for course utils. """ - -from django.test import TestCase, override_settings +import ddt import mock -from opaque_keys.edx.locations import SlashSeparatedCourseKey -from util.course import get_lms_link_for_about_page + +from django.conf import settings + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from util.course import get_link_for_about_page -class LmsLinksTestCase(TestCase): - """ Tests for LMS links. """ +@ddt.ddt +class TestCourseSharingLinks(ModuleStoreTestCase): + """ + Tests for course sharing links. + """ + def setUp(self): + super(TestCourseSharingLinks, self).setUp() - 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(self.get_about_page_link(), "http://localhost:8000/courses/mitX/101/test/about") + # create test mongo course + self.course = CourseFactory.create( + org='test_org', + number='test_number', + run='test_run', + default_store=ModuleStoreEnum.Type.split, + social_sharing_url='test_social_sharing_url', + ) - @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root'}) - def test_about_page_marketing_site(self): - """ 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") + # load this course into course overview and set it's marketing url + self.course_overview = CourseOverview.get_from_id(self.course.id) + self.course_overview.marketing_url = 'test_marketing_url' + self.course_overview.save() - @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") + def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_overview=True): + """ + Get course sharing link. - 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) + Arguments: + enable_social_sharing(Boolean): To indicate whether social sharing is enabled. + enable_mktg_site(Boolean): A feature flag to decide activation of marketing site. + + Keyword Arguments: + use_overview: indicates whether course overview or course descriptor should get + past to get_link_for_about_page. + + Returns course sharing url. + """ + mock_settings = { + 'FEATURES': { + 'ENABLE_MKTG_SITE': enable_mktg_site + }, + 'SOCIAL_SHARING_SETTINGS': { + 'CUSTOM_COURSE_URLS': enable_social_sharing + }, + } + + with mock.patch.multiple('django.conf.settings', **mock_settings): + course_sharing_link = get_link_for_about_page( + self.course_overview if use_overview else self.course + ) + + return course_sharing_link + + @ddt.data( + (True, True, 'test_social_sharing_url'), + (False, True, 'test_marketing_url'), + (True, False, 'test_social_sharing_url'), + (False, False, '{}/courses/course-v1:test_org+test_number+test_run/about'.format(settings.LMS_ROOT_URL)), + ) + @ddt.unpack + def test_sharing_link_with_settings(self, enable_social_sharing, enable_mktg_site, expected_course_sharing_link): + """ + Verify the method gives correct course sharing url on settings manipulations. + """ + actual_course_sharing_link = self.get_course_sharing_link( + enable_social_sharing=enable_social_sharing, + enable_mktg_site=enable_mktg_site, + ) + self.assertEqual(actual_course_sharing_link, expected_course_sharing_link) + + @ddt.data( + (['social_sharing_url'], 'test_marketing_url'), + (['marketing_url'], 'test_social_sharing_url'), + ( + ['social_sharing_url', 'marketing_url'], + '{}/courses/course-v1:test_org+test_number+test_run/about'.format(settings.LMS_ROOT_URL) + ), + ) + @ddt.unpack + def test_sharing_link_with_course_overview_attrs(self, overview_attrs, expected_course_sharing_link): + """ + Verify the method gives correct course sharing url when: + 1. Neither marketing url nor social sharing url is set. + 2. Either marketing url or social sharing url is set. + """ + for overview_attr in overview_attrs: + setattr(self.course_overview, overview_attr, None) + self.course_overview.save() + + actual_course_sharing_link = self.get_course_sharing_link( + enable_social_sharing=True, + enable_mktg_site=True, + ) + self.assertEqual(actual_course_sharing_link, expected_course_sharing_link) + + @ddt.data( + (True, 'test_social_sharing_url'), + ( + False, + '{}/courses/course-v1:test_org+test_number+test_run/about'.format(settings.LMS_ROOT_URL) + ), + ) + @ddt.unpack + def test_sharing_link_with_course_descriptor(self, enable_social_sharing, expected_course_sharing_link): + """ + Verify the method gives correct course sharing url on passing + course descriptor as a parameter. + """ + actual_course_sharing_link = self.get_course_sharing_link( + enable_social_sharing=enable_social_sharing, + enable_mktg_site=True, + use_overview=False, + ) + self.assertEqual(actual_course_sharing_link, expected_course_sharing_link) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 729637699e..e22612bc5b 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -8,7 +8,7 @@ from rest_framework.reverse import reverse from certificates.api import certificate_downloadable_status from courseware.access import has_access from student.models import CourseEnrollment, User -from util.course import get_lms_link_for_about_page +from util.course import get_link_for_about_page class CourseOverviewField(serializers.RelatedField): @@ -52,7 +52,7 @@ class CourseOverviewField(serializers.RelatedField): } }, 'course_image': course_overview.course_image_url, - 'course_about': get_lms_link_for_about_page(CourseKey.from_string(course_id)), + 'course_about': get_link_for_about_page(course_overview), 'course_updates': reverse( 'course-updates-list', kwargs={'course_id': course_id}, diff --git a/openedx/core/djangoapps/catalog/management/__init__.py b/openedx/core/djangoapps/catalog/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/catalog/management/commands/__init__.py b/openedx/core/djangoapps/catalog/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/catalog/management/commands/sync_course_runs.py b/openedx/core/djangoapps/catalog/management/commands/sync_course_runs.py new file mode 100644 index 0000000000..1d588fb3ba --- /dev/null +++ b/openedx/core/djangoapps/catalog/management/commands/sync_course_runs.py @@ -0,0 +1,70 @@ +""" +Sync course runs from catalog service. +""" +import logging + +from django.core.management.base import BaseCommand +from opaque_keys.edx.keys import CourseKey + +from openedx.core.djangoapps.catalog.utils import get_course_runs +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Purpose is to sync course runs data from catalog service to make it accessible in edx-platform. + It just happens to only be syncing marketing URLs from catalog course runs for now. + """ + help = 'Refresh marketing urls from catalog service.' + + def update_course_overviews(self, course_runs): + """ + Refresh marketing urls for the given catalog course runs. + + Arguments: + course_runs: A list containing catalog course runs. + """ + # metrics for observability + # number of catalog course runs retrieved. + catalog_course_runs_retrieved = len(course_runs) + # number of catalog course runs found in course overview. + course_runs_found_in_cache = 0 + # number of course overview records actually get updated. + course_metadata_updated = 0 + + for course_run in course_runs: + marketing_url = course_run['marketing_url'] + course_key = CourseKey.from_string(course_run['key']) + try: + course_overview = CourseOverview.objects.get(id=course_key) + course_runs_found_in_cache += 1 + except CourseOverview.DoesNotExist: + log.info( + '[sync_course_runs] course overview record not found for course run: %s', + unicode(course_key), + ) + continue + + # Check whether course overview's marketing url is outdated - this saves a db hit. + if course_overview.marketing_url != marketing_url: + course_overview.marketing_url = marketing_url + course_overview.save() + course_metadata_updated += 1 + + return catalog_course_runs_retrieved, course_runs_found_in_cache, course_metadata_updated + + def handle(self, *args, **options): + log.info('[sync_course_runs] Fetching course runs from catalog service.') + course_runs = get_course_runs() + course_runs_retrieved, course_runs_found, course_metadata_updated = self.update_course_overviews(course_runs) + + log.info( + ('[sync_course_runs] course runs retrieved: %d, course runs found in course overview: %d,' + ' course runs not found in course overview: %d, course overviews metadata updated: %d,'), + course_runs_retrieved, + course_runs_found, + course_runs_retrieved - course_runs_found, + course_metadata_updated, + ) diff --git a/openedx/core/djangoapps/catalog/management/commands/tests/__init__.py b/openedx/core/djangoapps/catalog/management/commands/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/catalog/management/commands/tests/test_sync_course_runs.py b/openedx/core/djangoapps/catalog/management/commands/tests/test_sync_course_runs.py new file mode 100644 index 0000000000..44af166dc2 --- /dev/null +++ b/openedx/core/djangoapps/catalog/management/commands/tests/test_sync_course_runs.py @@ -0,0 +1,89 @@ +""" +Tests for the sync course runs management command. +""" +import ddt +import mock + +from django.core.management import call_command + +from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +COMMAND_MODULE = 'openedx.core.djangoapps.catalog.management.commands.sync_course_runs' + + +@ddt.ddt +@mock.patch(COMMAND_MODULE + '.get_course_runs') +class TestSyncCourseRunsCommand(ModuleStoreTestCase): + """ + Test for the sync course runs management command. + """ + def setUp(self): + super(TestSyncCourseRunsCommand, self).setUp() + # create mongo course + self.course = CourseFactory.create() + # load this course into course overview + CourseOverview.get_from_id(self.course.id) + # create a catalog course run with the same course id. + self.catalog_course_run = CourseRunFactory( + key=unicode(self.course.id), + marketing_url='test_marketing_url' + ) + + def get_course_overview_marketing_url(self, course_id): + """ + Get course overview marketing url. + """ + return CourseOverview.objects.get(id=course_id).marketing_url + + def test_marketing_url_on_sync(self, mock_catalog_course_runs): + """ + Verify the updated marketing url on execution of the management command. + """ + mock_catalog_course_runs.return_value = [self.catalog_course_run] + earlier_marketing_url = self.get_course_overview_marketing_url(self.course.id) + + call_command('sync_course_runs') + updated_marketing_url = self.get_course_overview_marketing_url(self.course.id) + # Assert that the Marketing URL has changed. + self.assertNotEqual(earlier_marketing_url, updated_marketing_url) + self.assertEqual(updated_marketing_url, 'test_marketing_url') + + @mock.patch(COMMAND_MODULE + '.log.info') + def test_course_overview_does_not_exist(self, mock_log_info, mock_catalog_course_runs): + """ + Verify no error in case if a course run is not found in course overview. + """ + nonexistent_course_run = CourseRunFactory() + mock_catalog_course_runs.return_value = [self.catalog_course_run, nonexistent_course_run] + + call_command('sync_course_runs') + + mock_log_info.assert_any_call( + '[sync_course_runs] course overview record not found for course run: %s', + nonexistent_course_run['key'], + ) + updated_marketing_url = self.get_course_overview_marketing_url(self.course.id) + self.assertEqual(updated_marketing_url, 'test_marketing_url') + + @mock.patch(COMMAND_MODULE + '.log.info') + def test_starting_and_ending_logs(self, mock_log_info, mock_catalog_course_runs): + """ + Verify logging at start and end of the command. + """ + mock_catalog_course_runs.return_value = [self.catalog_course_run, CourseRunFactory(), CourseRunFactory()] + + call_command('sync_course_runs') + # Assert the logs at the start of the command. + mock_log_info.assert_any_call('[sync_course_runs] Fetching course runs from catalog service.') + # Assert the log metrics at it's completion. + mock_log_info.assert_any_call( + ('[sync_course_runs] course runs retrieved: %d, course runs found in course overview: %d,' + ' course runs not found in course overview: %d, course overviews metadata updated: %d,'), + 3, + 1, + 2, + 1, + ) diff --git a/openedx/core/djangoapps/catalog/migrations/0003_catalogintegration_page_size.py b/openedx/core/djangoapps/catalog/migrations/0003_catalogintegration_page_size.py new file mode 100644 index 0000000000..567c60fcf9 --- /dev/null +++ b/openedx/core/djangoapps/catalog/migrations/0003_catalogintegration_page_size.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('catalog', '0002_catalogintegration_username'), + ] + + operations = [ + migrations.AddField( + model_name='catalogintegration', + name='page_size', + field=models.PositiveIntegerField(default=100, help_text='Maximum number of records in paginated response of a single request to catalog service.', verbose_name='Page Size'), + ), + ] diff --git a/openedx/core/djangoapps/catalog/models.py b/openedx/core/djangoapps/catalog/models.py index bcb2f30169..735f1cdb0f 100644 --- a/openedx/core/djangoapps/catalog/models.py +++ b/openedx/core/djangoapps/catalog/models.py @@ -35,6 +35,14 @@ class CatalogIntegration(ConfigurationModel): ) ) + page_size = models.PositiveIntegerField( + verbose_name=_('Page Size'), + default=100, + help_text=_( + 'Maximum number of records in paginated response of a single request to catalog service.' + ) + ) + @property def is_cache_enabled(self): """Whether responses from the catalog API will be cached.""" diff --git a/openedx/core/djangoapps/catalog/tests/mixins.py b/openedx/core/djangoapps/catalog/tests/mixins.py index 5228187c55..4ea51bf9f1 100644 --- a/openedx/core/djangoapps/catalog/tests/mixins.py +++ b/openedx/core/djangoapps/catalog/tests/mixins.py @@ -10,6 +10,7 @@ class CatalogIntegrationMixin(object): 'internal_api_url': 'https://catalog-internal.example.com/api/v1/', 'cache_ttl': 0, 'service_username': 'lms_catalog_service_user', + 'page_size': 20, } def create_catalog_integration(self, **kwargs): diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 6b6ae68305..eb1c9790ef 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -8,12 +8,13 @@ from django.test import TestCase import mock from openedx.core.djangoapps.catalog.models import CatalogIntegration -from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, ProgramTypeFactory +from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory, ProgramFactory, ProgramTypeFactory 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, + get_course_runs, ) from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory @@ -176,3 +177,73 @@ class TestGetProgramTypes(CatalogIntegrationMixin, TestCase): program = program_types[0] data = get_program_types(name=program['name']) self.assertEqual(data, program) + + +@skip_unless_lms +@mock.patch(UTILS_MODULE + '.get_edx_api_data') +class TestGetCourseRuns(CatalogIntegrationMixin, TestCase): + """ + Tests covering retrieval of course runs from the catalog service. + """ + def setUp(self): + super(TestGetCourseRuns, self).setUp() + + self.catalog_integration = self.create_catalog_integration(cache_ttl=1) + self.user = UserFactory(username=self.catalog_integration.service_username) + + def assert_contract(self, call_args): # pylint: disable=redefined-builtin + """ + Verify that API data retrieval utility is used correctly. + """ + args, kwargs = call_args + + for arg in (self.catalog_integration, self.user, 'course_runs'): + self.assertIn(arg, args) + + self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url) # pylint: disable=protected-access + + querystring = { + 'page_size': 20, + 'exclude_utm': 1, + } + + self.assertEqual(kwargs['querystring'], querystring) + + return args, kwargs + + def test_config_missing(self, mock_get_edx_api_data): + """ + Verify that no errors occur when catalog config is missing. + """ + CatalogIntegration.objects.all().delete() + + data = get_course_runs() + self.assertFalse(mock_get_edx_api_data.called) + self.assertEqual(data, []) + + @mock.patch(UTILS_MODULE + '.log.error') + def test_service_user_missing(self, mock_log_error, mock_get_edx_api_data): + """ + Verify that no errors occur when the catalog service user is missing. + """ + catalog_integration = self.create_catalog_integration(service_username='nonexistent-user') + + data = get_course_runs() + mock_log_error.any_call( + 'Catalog service user with username [%s] does not exist. Course runs will not be retrieved.', + catalog_integration.service_username, + ) + self.assertFalse(mock_get_edx_api_data.called) + self.assertEqual(data, []) + + def test_get_course_runs(self, mock_get_edx_api_data): + """ + Test retrieval of course runs. + """ + catalog_course_runs = [CourseRunFactory() for __ in xrange(10)] + mock_get_edx_api_data.return_value = catalog_course_runs + + data = get_course_runs() + self.assertTrue(mock_get_edx_api_data.called) + self.assert_contract(mock_get_edx_api_data.call_args) + self.assertEqual(data, catalog_course_runs) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 62572fd9f6..e134f8b401 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -1,5 +1,6 @@ """Helper functions for working with the catalog service.""" import copy +import logging from django.conf import settings from django.contrib.auth import get_user_model @@ -10,6 +11,8 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.token_utils import JwtBuilder +log = logging.getLogger(__name__) + User = get_user_model() # pylint: disable=invalid-name @@ -135,3 +138,40 @@ def get_programs_with_type(types=None): programs_with_type.append(program_with_type) return programs_with_type + + +def get_course_runs(): + """ + Retrieve all the course runs from the catalog service. + + Returns: + list of dict with each record representing a course run. + """ + catalog_integration = CatalogIntegration.current() + course_runs = [] + if catalog_integration.enabled: + try: + user = User.objects.get(username=catalog_integration.service_username) + except User.DoesNotExist: + log.error( + 'Catalog service user with username [%s] does not exist. Course runs will not be retrieved.', + catalog_integration.service_username, + ) + return course_runs + + api = create_catalog_api_client(user, catalog_integration) + + querystring = { + 'page_size': catalog_integration.page_size, + 'exclude_utm': 1, + } + + course_runs = get_edx_api_data( + catalog_integration, + user, + 'course_runs', + api=api, + querystring=querystring, + ) + + return course_runs diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0011_courseoverview_marketing_url.py b/openedx/core/djangoapps/content/course_overviews/migrations/0011_courseoverview_marketing_url.py new file mode 100644 index 0000000000..4a3713deaa --- /dev/null +++ b/openedx/core/djangoapps/content/course_overviews/migrations/0011_courseoverview_marketing_url.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_overviews', '0010_auto_20160329_2317'), + ] + + operations = [ + migrations.AddField( + model_name='courseoverview', + name='marketing_url', + field=models.TextField(null=True), + ), + ] diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 346afd26ec..b3e2c2d634 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -97,6 +97,7 @@ class CourseOverview(TimeStampedModel): course_video_url = TextField(null=True) effort = TextField(null=True) self_paced = BooleanField(default=False) + marketing_url = TextField(null=True) @classmethod def _create_from_course(cls, course):