From fc6d9519d26af0471e7ffc86372ce118b7bff518 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Sat, 4 Feb 2017 23:34:53 -0500 Subject: [PATCH 1/2] Make service URLs optional on programs config model We're moving away from this config model and no longer need to specify these URLs. ECOM-4422 --- .../migrations/0010_auto_20170204_2332.py | 24 +++++++++++++++++++ openedx/core/djangoapps/programs/models.py | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 openedx/core/djangoapps/programs/migrations/0010_auto_20170204_2332.py diff --git a/openedx/core/djangoapps/programs/migrations/0010_auto_20170204_2332.py b/openedx/core/djangoapps/programs/migrations/0010_auto_20170204_2332.py new file mode 100644 index 0000000000..02e583777d --- /dev/null +++ b/openedx/core/djangoapps/programs/migrations/0010_auto_20170204_2332.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('programs', '0009_programsapiconfig_marketing_path'), + ] + + operations = [ + migrations.AlterField( + model_name='programsapiconfig', + name='internal_service_url', + field=models.URLField(verbose_name='Internal Service URL', blank=True), + ), + migrations.AlterField( + model_name='programsapiconfig', + name='public_service_url', + field=models.URLField(verbose_name='Public Service URL', blank=True), + ), + ] diff --git a/openedx/core/djangoapps/programs/models.py b/openedx/core/djangoapps/programs/models.py index 52eb0421cd..b46b314615 100644 --- a/openedx/core/djangoapps/programs/models.py +++ b/openedx/core/djangoapps/programs/models.py @@ -19,8 +19,8 @@ class ProgramsApiConfig(ConfigurationModel): api_version_number = models.IntegerField(verbose_name=_("API Version")) - internal_service_url = models.URLField(verbose_name=_("Internal Service URL")) - public_service_url = models.URLField(verbose_name=_("Public Service URL")) + internal_service_url = models.URLField(verbose_name=_("Internal Service URL"), blank=True) + public_service_url = models.URLField(verbose_name=_("Public Service URL"), blank=True) marketing_path = models.CharField( max_length=255, From e7771148b7697180f2118f1e9b795a717d34cba4 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Tue, 24 Jan 2017 11:31:29 -0500 Subject: [PATCH 2/2] Load all programs from the catalog This commit contains back end changes necessary to load programs from the catalog in all contexts. The existing program munging utility is applied as late as possible to avoid conflating this work with changes to the front end; those will be made separately. ECOM-4422 --- common/djangoapps/student/tests/tests.py | 75 +- common/djangoapps/student/views.py | 20 +- common/djangoapps/terrain/stubs/catalog.py | 34 +- common/djangoapps/terrain/stubs/programs.py | 51 -- common/djangoapps/terrain/stubs/start.py | 2 - common/test/acceptance/fixtures/__init__.py | 3 - common/test/acceptance/fixtures/catalog.py | 34 +- common/test/acceptance/fixtures/programs.py | 31 +- common/test/acceptance/pages/lms/programs.py | 6 +- .../acceptance/tests/lms/test_programs.py | 97 +-- docs/en_us/internal/testing.rst | 2 +- lms/djangoapps/branding/tests/test_page.py | 59 +- lms/djangoapps/courseware/views/views.py | 4 +- .../learner_dashboard/tests/test_programs.py | 224 ++---- lms/djangoapps/learner_dashboard/urls.py | 4 +- lms/djangoapps/learner_dashboard/views.py | 39 +- .../djangoapps/catalog/tests/factories.py | 42 +- .../core/djangoapps/catalog/tests/mixins.py | 11 +- .../djangoapps/catalog/tests/test_utils.py | 319 +++----- openedx/core/djangoapps/catalog/utils.py | 145 ++-- .../credentials/tests/test_utils.py | 17 +- openedx/core/djangoapps/credentials/utils.py | 7 +- .../backpopulate_program_credentials.py | 15 +- .../djangoapps/programs/tasks/v1/tasks.py | 2 +- .../programs/tasks/v1/tests/test_tasks.py | 63 +- .../djangoapps/programs/tests/factories.py | 64 +- .../core/djangoapps/programs/tests/mixins.py | 89 -- .../test_backpopulate_program_credentials.py | 75 +- .../djangoapps/programs/tests/test_utils.py | 758 +++++++----------- openedx/core/djangoapps/programs/utils.py | 291 +++---- openedx/core/lib/tests/test_edx_api_utils.py | 104 +-- pavelib/utils/envs.py | 5 - 32 files changed, 952 insertions(+), 1740 deletions(-) delete mode 100644 common/djangoapps/terrain/stubs/programs.py diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 73ff1bd788..90da0735b9 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -14,7 +14,6 @@ from django.contrib.auth.models import User, AnonymousUser from django.core.urlresolvers import reverse from django.test import TestCase, override_settings from django.test.client import Client -from edx_oauth2_provider.tests.factories import ClientFactory import httpretty from markupsafe import escape from mock import Mock, patch @@ -30,11 +29,15 @@ from certificates.tests.factories import GeneratedCertificateFactory # pylint: from config_models.models import cache from course_modes.models import CourseMode from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.tests import factories as programs_factories +from openedx.core.djangoapps.catalog.tests.factories import ( + generate_course_run_key, + ProgramFactory, + CourseFactory as CatalogCourseFactory, + CourseRunFactory, +) from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms import shoppingcart # pylint: disable=import-error from student.models import ( anonymous_id_for_user, user_by_anonymous_id, CourseEnrollment, @@ -988,11 +991,10 @@ class AnonymousLookupTable(ModuleStoreTestCase): @attr(shard=3) -@httpretty.activate -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +@skip_unless_lms +@patch('openedx.core.djangoapps.programs.utils.get_programs') class RelatedProgramsTests(ProgramsApiConfigMixin, SharedModuleStoreTestCase): """Tests verifying that related programs appear on the course dashboard.""" - url = None maxDiff = None password = 'test' related_programs_preface = 'Related Programs' @@ -1004,18 +1006,6 @@ class RelatedProgramsTests(ProgramsApiConfigMixin, SharedModuleStoreTestCase): cls.user = UserFactory() cls.course = CourseFactory() cls.enrollment = CourseEnrollmentFactory(user=cls.user, course_id=cls.course.id) # pylint: disable=no-member - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - - cls.organization = programs_factories.Organization() - run_mode = programs_factories.RunMode(course_key=unicode(cls.course.id)) # pylint: disable=no-member - course_code = programs_factories.CourseCode(run_modes=[run_mode]) - - cls.programs = [ - programs_factories.Program( - organizations=[cls.organization], - course_codes=[course_code] - ) for __ in range(2) - ] def setUp(self): super(RelatedProgramsTests, self).setUp() @@ -1025,14 +1015,9 @@ class RelatedProgramsTests(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.create_programs_config() self.client.login(username=self.user.username, password=self.password) - def mock_programs_api(self, data): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') - - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' - body = json.dumps({'results': data}) - - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') + course_run = CourseRunFactory(key=unicode(self.course.id)) # pylint: disable=no-member + course = CatalogCourseFactory(course_runs=[course_run]) + self.programs = [ProgramFactory(courses=[course]) for __ in range(2)] def assert_related_programs(self, response, are_programs_present=True): """Assertion for verifying response contents.""" @@ -1045,42 +1030,40 @@ class RelatedProgramsTests(ProgramsApiConfigMixin, SharedModuleStoreTestCase): def expected_link_text(self, program): """Construct expected dashboard link text.""" - return u'{name} {category}'.format(name=program['name'], category=program['category']) + return u'{title} {type}'.format(title=program['title'], type=program['type']) - def test_related_programs_listed(self): - """Verify that related programs are listed when the programs API returns data.""" - self.mock_programs_api(self.programs) + def test_related_programs_listed(self, mock_get_programs): + """Verify that related programs are listed when available.""" + mock_get_programs.return_value = self.programs response = self.client.get(self.url) self.assert_related_programs(response) - def test_no_data_no_programs(self): - """Verify that related programs aren't listed if the programs API returns no data.""" - self.mock_programs_api([]) + def test_no_data_no_programs(self, mock_get_programs): + """Verify that related programs aren't listed when none are available.""" + mock_get_programs.return_value = [] response = self.client.get(self.url) self.assert_related_programs(response, are_programs_present=False) - def test_unrelated_program_not_listed(self): + def test_unrelated_program_not_listed(self, mock_get_programs): """Verify that unrelated programs don't appear in the listing.""" - run_mode = programs_factories.RunMode(course_key='some/nonexistent/run') - course_code = programs_factories.CourseCode(run_modes=[run_mode]) + nonexistent_course_run_id = generate_course_run_key() - unrelated_program = programs_factories.Program( - organizations=[self.organization], - course_codes=[course_code] - ) + course_run = CourseRunFactory(key=nonexistent_course_run_id) + course = CatalogCourseFactory(course_runs=[course_run]) + unrelated_program = ProgramFactory(courses=[course]) - self.mock_programs_api(self.programs + [unrelated_program]) + mock_get_programs.return_value = self.programs + [unrelated_program] response = self.client.get(self.url) self.assert_related_programs(response) - self.assertNotContains(response, unrelated_program['name']) + self.assertNotContains(response, unrelated_program['title']) - def test_program_title_unicode(self): + def test_program_title_unicode(self, mock_get_programs): """Verify that the dashboard can deal with programs whose titles contain Unicode.""" - self.programs[0]['name'] = u'Bases matemáticas para estudiar ingeniería' - self.mock_programs_api(self.programs) + self.programs[0]['title'] = u'Bases matemáticas para estudiar ingeniería' + mock_get_programs.return_value = self.programs response = self.client.get(self.url) self.assert_related_programs(response) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 2f05e68d3e..009b39b710 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -13,7 +13,6 @@ from django.views.generic import TemplateView from pytz import UTC from requests import HTTPError from ipware.ip import get_ip -import waffle import edx_oauth2_provider from django.conf import settings @@ -123,12 +122,13 @@ from notification_prefs.views import enable_notifications 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 import utils as programs_utils +from openedx.core.djangoapps.catalog.utils import munge_catalog_program from openedx.core.djangoapps.programs.models import ProgramsApiConfig +from openedx.core.djangoapps.programs.utils import ProgramProgressMeter from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers from openedx.core.djangoapps.user_api.preferences import api as preferences_api -from openedx.core.djangoapps.catalog.utils import get_programs_data +from openedx.core.djangoapps.catalog.utils import get_programs_with_type_logo log = logging.getLogger("edx.student") @@ -215,7 +215,7 @@ def index(request, extra_context=None, user=AnonymousUser()): # 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_data(user) + programs_list = get_programs_with_type_logo() context["programs_list"] = programs_list @@ -658,12 +658,14 @@ def dashboard(request): and has_access(request.user, 'view_courseware_with_prerequisites', enrollment.course_overview) ) - # Find programs associated with courses being displayed. This information + # Find programs associated with course runs being displayed. This information # is passed in the template context to allow rendering of program-related # information on the dashboard. - use_catalog = waffle.switch_is_active('get_programs_from_catalog') - meter = programs_utils.ProgramProgressMeter(user, enrollments=course_enrollments, use_catalog=use_catalog) - programs_by_run = meter.engaged_programs(by_run=True) + meter = ProgramProgressMeter(user, enrollments=course_enrollments) + inverted_programs = meter.invert_programs() + + for program_list in inverted_programs.itervalues(): + program_list[:] = [munge_catalog_program(program) for program in program_list] # Construct a dictionary of course mode information # used to render the course list. We re-use the course modes dict @@ -787,7 +789,7 @@ def dashboard(request): 'order_history_list': order_history_list, 'courses_requirements_not_met': courses_requirements_not_met, 'nav_hidden': True, - 'programs_by_run': programs_by_run, + 'programs_by_run': inverted_programs, 'show_program_listing': ProgramsApiConfig.current().show_program_listing, 'disable_courseware_js': True, 'display_course_modes_on_dashboard': enable_verified_certificates and display_course_modes_on_dashboard, diff --git a/common/djangoapps/terrain/stubs/catalog.py b/common/djangoapps/terrain/stubs/catalog.py index 1ff580dbdf..bf98ee92d0 100644 --- a/common/djangoapps/terrain/stubs/catalog.py +++ b/common/djangoapps/terrain/stubs/catalog.py @@ -1,51 +1,47 @@ """ Stub implementation of catalog service for acceptance tests """ +# pylint: disable=invalid-name, missing-docstring import re import urlparse from .http import StubHttpRequestHandler, StubHttpService -class StubCatalogServiceHandler(StubHttpRequestHandler): # pylint: disable=missing-docstring +class StubCatalogServiceHandler(StubHttpRequestHandler): - def do_GET(self): # pylint: disable=invalid-name, missing-docstring + def do_GET(self): pattern_handlers = { - r'/api/v1/programs/$': self.get_programs, - r'/api/v1/course_runs/(?P[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)/$': self.get_course_run, + r'/api/v1/programs/$': self.program_list, + r'/api/v1/programs/([0-9a-f-]+)/$': self.program_detail, } if self.match_pattern(pattern_handlers): return - self.send_response(404, content="404 Not Found") + self.send_response(404, content='404 Not Found') def match_pattern(self, pattern_handlers): """ Find the correct handler method given the path info from the HTTP request. """ path = urlparse.urlparse(self.path).path - for pattern in pattern_handlers: + for pattern, handler in pattern_handlers.items(): match = re.match(pattern, path) if match: - pattern_handlers[pattern](*match.groups()) + handler(*match.groups()) return True - return None - def get_programs(self): - """ - Stubs the catalog's programs endpoint. - """ + def program_list(self): + """Stub the catalog's program list endpoint.""" programs = self.server.config.get('catalog.programs', []) self.send_json_response(programs) - def get_course_run(self, course_id): - """ - Stubs the catalog's course run endpoint. - """ - course_run = self.server.config.get('course_run.{}'.format(course_id), []) - self.send_json_response(course_run) + def program_detail(self, program_uuid): + """Stub the catalog's program detail endpoint.""" + program = self.server.config.get('catalog.programs.' + program_uuid) + self.send_json_response(program) -class StubCatalogService(StubHttpService): # pylint: disable=missing-docstring +class StubCatalogService(StubHttpService): HANDLER_CLASS = StubCatalogServiceHandler diff --git a/common/djangoapps/terrain/stubs/programs.py b/common/djangoapps/terrain/stubs/programs.py deleted file mode 100644 index 5631b8de03..0000000000 --- a/common/djangoapps/terrain/stubs/programs.py +++ /dev/null @@ -1,51 +0,0 @@ -""" -Stub implementation of programs service for acceptance tests -""" -import re -import urlparse - -from .http import StubHttpRequestHandler, StubHttpService - - -class StubProgramsServiceHandler(StubHttpRequestHandler): # pylint: disable=missing-docstring - - def do_GET(self): # pylint: disable=invalid-name, missing-docstring - pattern_handlers = { - r'/api/v1/programs/$': self.get_programs_list, - r'/api/v1/programs/(\d+)/$': self.get_program_details, - } - - if self.match_pattern(pattern_handlers): - return - - self.send_response(404, content="404 Not Found") - - def match_pattern(self, pattern_handlers): - """ - Find the correct handler method given the path info from the HTTP request. - """ - path = urlparse.urlparse(self.path).path - for pattern in pattern_handlers: - match = re.match(pattern, path) - if match: - pattern_handlers[pattern](*match.groups()) - return True - return None - - def get_programs_list(self): - """ - Stubs the programs list endpoint. - """ - programs = self.server.config.get('programs', []) - self.send_json_response(programs) - - def get_program_details(self, program_id): - """ - Stubs a program details endpoint. - """ - program = self.server.config.get('programs.{}'.format(program_id), []) - self.send_json_response(program) - - -class StubProgramsService(StubHttpService): # pylint: disable=missing-docstring - HANDLER_CLASS = StubProgramsServiceHandler diff --git a/common/djangoapps/terrain/stubs/start.py b/common/djangoapps/terrain/stubs/start.py index f0920ac8b3..1e02f63a96 100644 --- a/common/djangoapps/terrain/stubs/start.py +++ b/common/djangoapps/terrain/stubs/start.py @@ -12,7 +12,6 @@ from .youtube import StubYouTubeService from .lti import StubLtiService from .video_source import VideoSourceHttpService from .edxnotes import StubEdxNotesService -from .programs import StubProgramsService from .catalog import StubCatalogService @@ -25,7 +24,6 @@ SERVICES = { 'lti': StubLtiService, 'video': VideoSourceHttpService, 'edxnotes': StubEdxNotesService, - 'programs': StubProgramsService, 'ecommerce': StubEcommerceService, 'catalog': StubCatalogService, } diff --git a/common/test/acceptance/fixtures/__init__.py b/common/test/acceptance/fixtures/__init__.py index e059751856..ae494659e3 100644 --- a/common/test/acceptance/fixtures/__init__.py +++ b/common/test/acceptance/fixtures/__init__.py @@ -19,8 +19,5 @@ COMMENTS_STUB_URL = os.environ.get('comments_url', 'http://localhost:4567') # Get the URL of the EdxNotes service stub used in the test EDXNOTES_STUB_URL = os.environ.get('edxnotes_url', 'http://localhost:8042') -# Get the URL of the Programs service stub used in the test -PROGRAMS_STUB_URL = os.environ.get('programs_url', 'http://localhost:8090') - # Get the URL of the Catalog service stub used in the test CATALOG_STUB_URL = os.environ.get('catalog_url', 'http://localhost:8091') diff --git a/common/test/acceptance/fixtures/catalog.py b/common/test/acceptance/fixtures/catalog.py index 002e7671e4..32a6b8e05d 100644 --- a/common/test/acceptance/fixtures/catalog.py +++ b/common/test/acceptance/fixtures/catalog.py @@ -13,31 +13,31 @@ class CatalogFixture(object): """ Interface to set up mock responses from the Catalog stub server. """ - def install_programs(self, programs): + def install_programs(self, data): """Set response data for the catalog's course run API.""" key = 'catalog.programs' - requests.put( - '{}/set_config'.format(CATALOG_STUB_URL), - data={key: json.dumps(programs)}, - ) + if isinstance(data, dict): + key += '.' + data['uuid'] - def install_course_run(self, course_run): - """Set response data for the catalog's course run API.""" - key = 'catalog.{}'.format(course_run['key']) - - requests.put( - '{}/set_config'.format(CATALOG_STUB_URL), - data={key: json.dumps(course_run)}, - ) + requests.put( + '{}/set_config'.format(CATALOG_STUB_URL), + data={key: json.dumps(data)}, + ) + else: + requests.put( + '{}/set_config'.format(CATALOG_STUB_URL), + data={key: json.dumps({'results': data})}, + ) -class CatalogConfigMixin(object): +class CatalogIntegrationMixin(object): """Mixin providing a method used to configure the catalog integration.""" - def set_catalog_configuration(self, is_enabled=False, service_url=CATALOG_STUB_URL): - """Dynamically adjusts the catalog config model during tests.""" + def set_catalog_integration(self, is_enabled=False, service_username=None): + """Use this to change the catalog integration config model during tests.""" ConfigModelFixture('/config/catalog', { 'enabled': is_enabled, - 'internal_api_url': '{}/api/v1/'.format(service_url), + 'internal_api_url': '{}/api/v1/'.format(CATALOG_STUB_URL), 'cache_ttl': 0, + 'service_username': service_username, }).install() diff --git a/common/test/acceptance/fixtures/programs.py b/common/test/acceptance/fixtures/programs.py index c33d4c2459..7c16344774 100644 --- a/common/test/acceptance/fixtures/programs.py +++ b/common/test/acceptance/fixtures/programs.py @@ -1,46 +1,19 @@ """ Tools to create programs-related data for use in bok choy tests. """ -import json - -import requests - -from common.test.acceptance.fixtures import PROGRAMS_STUB_URL from common.test.acceptance.fixtures.config import ConfigModelFixture -class ProgramsFixture(object): - """ - Interface to set up mock responses from the Programs stub server. - """ - def install_programs(self, programs, is_list=True): - """Sets the response data for Programs API endpoints.""" - if is_list: - key = 'programs' - api_result = {'results': programs} - else: - program = programs[0] - key = 'programs.{}'.format(program['id']) - api_result = program - - requests.put( - '{}/set_config'.format(PROGRAMS_STUB_URL), - data={key: json.dumps(api_result)}, - ) - - class ProgramsConfigMixin(object): """Mixin providing a method used to configure the programs feature.""" - def set_programs_api_configuration(self, is_enabled=False, api_version=1, api_url=PROGRAMS_STUB_URL): + def set_programs_api_configuration(self, is_enabled=False, api_version=1): """Dynamically adjusts the Programs config model during tests.""" ConfigModelFixture('/config/programs', { 'enabled': is_enabled, 'api_version_number': api_version, - 'internal_service_url': api_url, - 'public_service_url': api_url, 'cache_ttl': 0, + 'marketing_path': '/foo', 'enable_student_dashboard': is_enabled, - 'enable_studio_tab': is_enabled, 'enable_certification': is_enabled, 'program_listing_enabled': is_enabled, 'program_details_enabled': is_enabled, diff --git a/common/test/acceptance/pages/lms/programs.py b/common/test/acceptance/pages/lms/programs.py index f3290f989e..26695e485c 100644 --- a/common/test/acceptance/pages/lms/programs.py +++ b/common/test/acceptance/pages/lms/programs.py @@ -1,4 +1,6 @@ """LMS-hosted Programs pages""" +from uuid import uuid4 + from bok_choy.page_object import PageObject from common.test.acceptance.pages.lms import BASE_URL @@ -24,8 +26,8 @@ class ProgramListingPage(PageObject): class ProgramDetailsPage(PageObject): """Program details page.""" - program_id = 123 - url = BASE_URL + '/dashboard/programs/{}/program-name/'.format(program_id) + program_uuid = str(uuid4()) + url = '{base}/dashboard/programs/{program_uuid}/'.format(base=BASE_URL, program_uuid=program_uuid) def is_browser_on_page(self): return self.q(css='.js-program-details-wrapper').present diff --git a/common/test/acceptance/tests/lms/test_programs.py b/common/test/acceptance/tests/lms/test_programs.py index b5e7d140fe..1780d715b3 100644 --- a/common/test/acceptance/tests/lms/test_programs.py +++ b/common/test/acceptance/tests/lms/test_programs.py @@ -1,67 +1,48 @@ """Acceptance tests for LMS-hosted Programs pages""" from nose.plugins.attrib import attr -from common.test.acceptance.fixtures.catalog import CatalogFixture, CatalogConfigMixin -from common.test.acceptance.fixtures.programs import ProgramsFixture, ProgramsConfigMixin +from common.test.acceptance.fixtures.catalog import CatalogFixture, CatalogIntegrationMixin +from common.test.acceptance.fixtures.programs import ProgramsConfigMixin from common.test.acceptance.fixtures.course import CourseFixture from common.test.acceptance.tests.helpers import UniqueCourseTest from common.test.acceptance.pages.lms.auto_auth import AutoAuthPage from common.test.acceptance.pages.lms.programs import ProgramListingPage, ProgramDetailsPage -from openedx.core.djangoapps.catalog.tests import factories as catalog_factories -from openedx.core.djangoapps.programs.tests import factories as program_factories +from openedx.core.djangoapps.catalog.tests.factories import ( + ProgramFactory, CourseFactory, CourseRunFactory +) -class ProgramPageBase(ProgramsConfigMixin, CatalogConfigMixin, UniqueCourseTest): +class ProgramPageBase(ProgramsConfigMixin, CatalogIntegrationMixin, UniqueCourseTest): """Base class used for program listing page tests.""" def setUp(self): super(ProgramPageBase, self).setUp() self.set_programs_api_configuration(is_enabled=True) - self.programs = [catalog_factories.Program() for __ in range(3)] - self.course_run = catalog_factories.CourseRun(key=self.course_id) - self.stub_catalog_api() - - def create_program(self, program_id=None, course_id=None): - """DRY helper for creating test program data.""" - course_id = course_id if course_id else self.course_id - - run_mode = program_factories.RunMode(course_key=course_id) - course_code = program_factories.CourseCode(run_modes=[run_mode]) - org = program_factories.Organization(key=self.course_info['org']) - - if program_id: - program = program_factories.Program( - id=program_id, - status='active', - organizations=[org], - course_codes=[course_code] - ) - else: - program = program_factories.Program( - status='active', - organizations=[org], - course_codes=[course_code] - ) - - return program - - def stub_programs_api(self, programs, is_list=True): - """Stub out the programs API with fake data.""" - ProgramsFixture().install_programs(programs, is_list=is_list) - - def stub_catalog_api(self): - """Stub out the catalog API's program and course run endpoints.""" - self.set_catalog_configuration(is_enabled=True) - CatalogFixture().install_programs(self.programs) - CatalogFixture().install_course_run(self.course_run) + self.programs = ProgramFactory.create_batch(3) + self.username = None def auth(self, enroll=True): """Authenticate, enrolling the user in the configured course if requested.""" CourseFixture(**self.course_info).install() course_id = self.course_id if enroll else None - AutoAuthPage(self.browser, course_id=course_id).visit() + auth_page = AutoAuthPage(self.browser, course_id=course_id) + auth_page.visit() + + self.username = auth_page.user_info['username'] + + def create_program(self): + """DRY helper for creating test program data.""" + course_run = CourseRunFactory(key=self.course_id) + course = CourseFactory(course_runs=[course_run]) + + return ProgramFactory(courses=[course]) + + def stub_catalog_api(self, data=None): + """Stub out the catalog API's program and course run endpoints.""" + self.set_catalog_integration(is_enabled=True, service_username=self.username) + CatalogFixture().install_programs(data or self.programs) class ProgramListingPageTest(ProgramPageBase): @@ -73,9 +54,8 @@ class ProgramListingPageTest(ProgramPageBase): def test_no_enrollments(self): """Verify that no cards appear when the user has no enrollments.""" - program = self.create_program() - self.stub_programs_api([program]) self.auth(enroll=False) + self.stub_catalog_api() self.listing_page.visit() @@ -87,14 +67,8 @@ class ProgramListingPageTest(ProgramPageBase): Verify that no cards appear when the user has enrollments but none are included in an active program. """ - course_id = self.course_id.replace( - self.course_info['run'], - 'other_run' - ) - - program = self.create_program(course_id=course_id) - self.stub_programs_api([program]) self.auth() + self.stub_catalog_api() self.listing_page.visit() @@ -106,10 +80,11 @@ class ProgramListingPageTest(ProgramPageBase): Verify that cards appear when the user has enrollments which are included in at least one active program. """ - program = self.create_program() - self.stub_programs_api([program]) self.auth() + program = self.create_program() + self.stub_catalog_api(data=[program]) + self.listing_page.visit() self.assertTrue(self.listing_page.is_sidebar_present) @@ -124,12 +99,13 @@ class ProgramListingPageA11yTest(ProgramPageBase): self.listing_page = ProgramListingPage(self.browser) - program = self.create_program() - self.stub_programs_api([program]) + self.program = self.create_program() def test_empty_a11y(self): """Test a11y of the page's empty state.""" self.auth(enroll=False) + self.stub_catalog_api(data=[self.program]) + self.listing_page.visit() self.assertTrue(self.listing_page.is_sidebar_present) @@ -139,6 +115,8 @@ class ProgramListingPageA11yTest(ProgramPageBase): def test_cards_a11y(self): """Test a11y when program cards are present.""" self.auth() + self.stub_catalog_api(data=[self.program]) + self.listing_page.visit() self.assertTrue(self.listing_page.is_sidebar_present) @@ -154,11 +132,14 @@ class ProgramDetailsPageA11yTest(ProgramPageBase): self.details_page = ProgramDetailsPage(self.browser) - program = self.create_program(program_id=self.details_page.program_id) - self.stub_programs_api([program], is_list=False) + self.program = self.create_program() + self.program['uuid'] = self.details_page.program_uuid def test_a11y(self): """Test the page's a11y compliance.""" self.auth() + self.stub_catalog_api(data=self.program) + self.details_page.visit() + self.details_page.a11y_audit.check_for_accessibility_errors() diff --git a/docs/en_us/internal/testing.rst b/docs/en_us/internal/testing.rst index 8a691acac2..b96830ecab 100644 --- a/docs/en_us/internal/testing.rst +++ b/docs/en_us/internal/testing.rst @@ -486,7 +486,7 @@ and the Automated Accessibility Tests `openedx Confluence page **Prerequisites**: These prerequisites are all automatically installed and available in `Devstack -`__ (since the Cypress release), the supported development enviornment for the edX Platform. +`__ (since the Cypress release), the supported development environment for the edX Platform. * Mongo diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index ef2b4cd658..6a3f3f34aa 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -1,30 +1,27 @@ """ Tests for branding page """ - -import mock import datetime +import ddt from django.conf import settings from django.contrib.auth.models import AnonymousUser +from django.core.urlresolvers import reverse from django.http import HttpResponseRedirect from django.test.utils import override_settings from django.test.client import RequestFactory - -from pytz import UTC from mock import patch, Mock from nose.plugins.attrib import attr +from pytz import UTC 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 util.milestones_helpers import set_prerequisite_courses from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from django.core.urlresolvers import reverse -from courseware.tests.helpers import LoginEnrollmentTestCase - -from util.milestones_helpers import set_prerequisite_courses -from milestones.tests.utils import MilestonesTestCaseMixin FEATURES_WITH_STARTDATE = settings.FEATURES.copy() FEATURES_WITH_STARTDATE['DISABLE_START_DATES'] = False @@ -290,35 +287,31 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase): self.assertEqual(context['courses'][2].id, self.course_with_default_start_date.id) +@ddt.ddt @attr(shard=1) class IndexPageProgramsTests(ModuleStoreTestCase): """ Tests for Programs List in Marketing Pages. """ - @patch.dict('django.conf.settings.FEATURES', {'DISPLAY_PROGRAMS_ON_MARKETING_PAGES': False}) - def test_get_programs_not_called(self): - with mock.patch("student.views.get_programs_data") as patched_get_programs_data: - # check the /dashboard - response = self.client.get('/') - self.assertEqual(response.status_code, 200) - self.assertEqual(patched_get_programs_data.call_count, 0) - with mock.patch("courseware.views.views.get_programs_data") as patched_get_programs_data: - # check the /courses view - response = self.client.get(reverse('branding.views.courses')) - self.assertEqual(response.status_code, 200) - self.assertEqual(patched_get_programs_data.call_count, 0) + def setUp(self): + super(IndexPageProgramsTests, self).setUp() + self.client.login(username=self.user.username, password=self.user_password) - @patch.dict('django.conf.settings.FEATURES', {'DISPLAY_PROGRAMS_ON_MARKETING_PAGES': True}) - def test_get_programs_called(self): - with mock.patch("student.views.get_programs_data") as patched_get_programs_data: - # check the /dashboard - response = self.client.get('/') - self.assertEqual(response.status_code, 200) - self.assertEqual(patched_get_programs_data.call_count, 1) + @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'), + ] - with mock.patch("courseware.views.views.get_programs_data") as patched_get_programs_data: - # check the /courses view - response = self.client.get(reverse('branding.views.courses')) - self.assertEqual(response.status_code, 200) - self.assertEqual(patched_get_programs_data.call_count, 1) + 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_() diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 1823345d8a..7eef57c1b9 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -40,7 +40,7 @@ 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_data +from openedx.core.djangoapps.catalog.utils import get_programs_with_type_logo import shoppingcart import survey.utils import survey.views @@ -153,7 +153,7 @@ def courses(request): # 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_data(request.user) + programs_list = get_programs_with_type_logo() return render_to_response( "courseware/courses.html", diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index f703a0fd94..da4547fc48 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -6,36 +6,34 @@ import json import re import unittest from urlparse import urljoin +from uuid import uuid4 from bs4 import BeautifulSoup from django.conf import settings from django.core.urlresolvers import reverse from django.test import override_settings -from django.utils.text import slugify -from edx_oauth2_provider.tests.factories import ClientFactory -import httpretty import mock -from provider.constants import CONFIDENTIAL -from openedx.core.djangoapps.credentials.models import CredentialsApiConfig -from openedx.core.djangoapps.credentials.tests import factories as credentials_factories +from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, CourseFactory, CourseRunFactory +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangoapps.catalog.utils import munge_catalog_program +from openedx.core.djangoapps.credentials.tests.factories import UserCredential, ProgramCredential from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin -from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.tests import factories as programs_factories from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin -from openedx.core.djangolib.testing.utils import skip_unless_lms, toggle_switch +from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory, CourseEnrollmentFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory as ModuleStoreCourseFactory -UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' -MARKETING_URL = 'https://www.example.com/marketing/path' +CATALOG_UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils' +CREDENTIALS_UTILS_MODULE = 'openedx.core.djangoapps.credentials.utils' +PROGRAMS_UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' @skip_unless_lms -@httpretty.activate @override_settings(MKTG_URLS={'ROOT': 'https://www.example.com'}) +@mock.patch(PROGRAMS_UTILS_MODULE + '.get_programs') class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, SharedModuleStoreTestCase): """Unit tests for the program listing page.""" maxDiff = None @@ -46,22 +44,12 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar def setUpClass(cls): super(TestProgramListing, cls).setUpClass() - for name in [ProgramsApiConfig.OAUTH2_CLIENT_NAME, CredentialsApiConfig.OAUTH2_CLIENT_NAME]: - ClientFactory(name=name, client_type=CONFIDENTIAL) + cls.course = ModuleStoreCourseFactory() + course_run = CourseRunFactory(key=unicode(cls.course.id)) # pylint: disable=no-member + course = CourseFactory(course_runs=[course_run]) - cls.course = CourseFactory() - organization = programs_factories.Organization() - run_mode = programs_factories.RunMode(course_key=unicode(cls.course.id)) # pylint: disable=no-member - course_code = programs_factories.CourseCode(run_modes=[run_mode]) - - cls.first_program = programs_factories.Program( - organizations=[organization], - course_codes=[course_code] - ) - cls.second_program = programs_factories.Program( - organizations=[organization], - course_codes=[course_code] - ) + cls.first_program = ProgramFactory(courses=[course]) + cls.second_program = ProgramFactory(courses=[course]) cls.data = sorted([cls.first_program, cls.second_program], key=cls.program_sort_key) @@ -76,7 +64,13 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar """ Helper function used to sort dictionaries representing programs. """ - return program['id'] + try: + return program['title'] + except: # pylint: disable=bare-except + # This is here temporarily because programs are still being munged + # to look like they came from the programs service before going out + # to the front end. + return program['name'] def credential_sort_key(self, credential): """ @@ -87,27 +81,6 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar except KeyError: return credential['credential_url'] - def mock_programs_api(self, data): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') - - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' - body = json.dumps({'results': data}) - - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') - - def mock_credentials_api(self, data): - """Helper for mocking out Credentials API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Credentials API calls.') - - url = '{base}/credentials/?username={username}'.format( - base=CredentialsApiConfig.current().internal_api_url.strip('/'), - username=self.user.username - ) - body = json.dumps({'results': data}) - - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') - def load_serialized_data(self, response, key): """ Extract and deserialize serialized data from the response. @@ -131,12 +104,12 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar self.assertEqual(subset, intersection) - def test_login_required(self): + def test_login_required(self, mock_get_programs): """ Verify that login is required to access the page. """ self.create_programs_config() - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data self.client.logout() @@ -151,7 +124,7 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar response = self.client.get(self.url) self.assertEqual(response.status_code, 200) - def test_404_if_disabled(self): + def test_404_if_disabled(self, _mock_get_programs): """ Verify that the page 404s if disabled. """ @@ -160,22 +133,22 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar response = self.client.get(self.url) self.assertEqual(response.status_code, 404) - def test_empty_state(self): + def test_empty_state(self, mock_get_programs): """ Verify that the response contains no programs data when no programs are engaged. """ self.create_programs_config() - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data response = self.client.get(self.url) self.assertContains(response, 'programsData: []') - def test_programs_listed(self): + def test_programs_listed(self, mock_get_programs): """ Verify that the response contains accurate programs data when programs are engaged. """ self.create_programs_config() - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data CourseEnrollmentFactory(user=self.user, course_id=self.course.id) # pylint: disable=no-member @@ -184,27 +157,27 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar actual = sorted(actual, key=self.program_sort_key) for index, actual_program in enumerate(actual): - expected_program = self.data[index] + expected_program = munge_catalog_program(self.data[index]) self.assert_dict_contains_subset(actual_program, expected_program) - def test_program_discovery(self): + def test_program_discovery(self, mock_get_programs): """ Verify that a link to a programs marketing page appears in the response. """ self.create_programs_config(marketing_path='bar') - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data marketing_root = urljoin(settings.MKTG_URLS.get('ROOT'), 'bar').rstrip('/') response = self.client.get(self.url) self.assertContains(response, marketing_root) - def test_links_to_detail_pages(self): + def test_links_to_detail_pages(self, mock_get_programs): """ Verify that links to detail pages are present. """ self.create_programs_config() - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data CourseEnrollmentFactory(user=self.user, course_id=self.course.id) # pylint: disable=no-member @@ -215,43 +188,41 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar for index, actual_program in enumerate(actual): expected_program = self.data[index] - base = reverse('program_details_view', args=[expected_program['id']]).rstrip('/') - slug = slugify(expected_program['name']) - self.assertEqual( - actual_program['detail_url'], - '{}/{}'.format(base, slug) - ) + expected_url = reverse('program_details_view', kwargs={'program_uuid': expected_program['uuid']}) + self.assertEqual(actual_program['detail_url'], expected_url) - def test_certificates_listed(self): + @mock.patch(CREDENTIALS_UTILS_MODULE + '.get_user_credentials') + @mock.patch(CREDENTIALS_UTILS_MODULE + '.get_programs') + def test_certificates_listed(self, mock_get_programs, mock_get_user_credentials, __): """ Verify that the response contains accurate certificate data when certificates are available. """ self.create_programs_config() self.create_credentials_config(is_learner_issuance_enabled=True) - self.mock_programs_api(self.data) + mock_get_programs.return_value = self.data - first_credential = credentials_factories.UserCredential( + first_credential = UserCredential( username=self.user.username, - credential=credentials_factories.ProgramCredential( - program_id=self.first_program['id'] + credential=ProgramCredential( + program_uuid=self.first_program['uuid'] ) ) - second_credential = credentials_factories.UserCredential( + second_credential = UserCredential( username=self.user.username, - credential=credentials_factories.ProgramCredential( - program_id=self.second_program['id'] + credential=ProgramCredential( + program_uuid=self.second_program['uuid'] ) ) credentials_data = sorted([first_credential, second_credential], key=self.credential_sort_key) - - self.mock_credentials_api(credentials_data) + mock_get_user_credentials.return_value = credentials_data response = self.client.get(self.url) actual = self.load_serialized_data(response, 'certificatesData') actual = sorted(actual, key=self.credential_sort_key) + self.assertEqual(len(actual), len(credentials_data)) for index, actual_credential in enumerate(actual): expected_credential = credentials_data[index] @@ -262,51 +233,24 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar expected_credential['certificate_url'] ) - def test_switch_to_catalog(self): - """ - Verify that the 'get_programs_from_catalog' switch can be used to route - traffic between the programs and catalog services. - """ - self.create_programs_config() - switch_name = 'get_programs_from_catalog' - - with mock.patch('openedx.core.djangoapps.programs.utils.get_programs') as mock_get_programs: - mock_get_programs.return_value = self.data - - toggle_switch(switch_name) - self.client.get(self.url) - mock_get_programs.assert_called_with(self.user, use_catalog=True) - - toggle_switch(switch_name) - self.client.get(self.url) - mock_get_programs.assert_called_with(self.user, use_catalog=False) - @skip_unless_lms -@httpretty.activate -@override_settings(MKTG_URLS={'ROOT': 'https://www.example.com'}) -@mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) -class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): +@mock.patch(CATALOG_UTILS_MODULE + '.get_edx_api_data') +class TestProgramDetails(ProgramsApiConfigMixin, CatalogIntegrationMixin, SharedModuleStoreTestCase): """Unit tests for the program details page.""" - program_id = 123 + program_uuid = str(uuid4()) password = 'test' - url = reverse('program_details_view', args=[program_id]) + url = reverse('program_details_view', kwargs={'program_uuid': program_uuid}) @classmethod def setUpClass(cls): super(TestProgramDetails, cls).setUpClass() - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) + modulestore_course = ModuleStoreCourseFactory() + course_run = CourseRunFactory(key=unicode(modulestore_course.id)) # pylint: disable=no-member + course = CourseFactory(course_runs=[course_run]) - course = CourseFactory() - 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]) - - cls.data = programs_factories.Program( - organizations=[organization], - course_codes=[course_code] - ) + cls.data = ProgramFactory(uuid=cls.program_uuid, courses=[course]) def setUp(self): super(TestProgramDetails, self).setUp() @@ -314,31 +258,12 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): self.user = UserFactory() 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.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') - - url = '{api_root}/programs/{id}/'.format( - api_root=ProgramsApiConfig.current().internal_api_url.strip('/'), - id=self.program_id - ) - - body = json.dumps(data) - - httpretty.register_uri( - httpretty.GET, - url, - body=body, - status=status, - content_type='application/json', - ) - def assert_program_data_present(self, response): """Verify that program data is present.""" self.assertContains(response, 'programData') self.assertContains(response, 'urls') self.assertContains(response, 'program_listing_url') - self.assertContains(response, self.data['name']) + self.assertContains(response, self.data['title']) self.assert_programs_tab_present(response) def assert_programs_tab_present(self, response): @@ -348,12 +273,16 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): any(soup.find_all('a', class_='tab-nav-link', href=reverse('program_listing_view'))) ) - def test_login_required(self): + def test_login_required(self, mock_get_edx_api_data): """ Verify that login is required to access the page. """ self.create_programs_config() - self.mock_programs_api(self.data) + + catalog_integration = self.create_catalog_integration() + UserFactory(username=catalog_integration.service_username) + + mock_get_edx_api_data.return_value = self.data self.client.logout() @@ -368,7 +297,7 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): response = self.client.get(self.url) self.assert_program_data_present(response) - def test_404_if_disabled(self): + def test_404_if_disabled(self, _mock_get_edx_api_data): """ Verify that the page 404s if disabled. """ @@ -377,30 +306,9 @@ class TestProgramDetails(ProgramsApiConfigMixin, SharedModuleStoreTestCase): response = self.client.get(self.url) self.assertEqual(response.status_code, 404) - def test_404_if_no_data(self): + def test_404_if_no_data(self, _mock_get_edx_api_data): """Verify that the page 404s if no program data is found.""" self.create_programs_config() - self.mock_programs_api(self.data, status=404) response = self.client.get(self.url) self.assertEqual(response.status_code, 404) - - httpretty.reset() - - self.mock_programs_api({}) - response = self.client.get(self.url) - 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.""" - self.create_programs_config() - self.mock_programs_api(self.data) - - response = self.client.get(self.url) - self.assert_program_data_present(response) - - response = self.client.get(self.url + 'program_name/') - self.assert_program_data_present(response) - - response = self.client.get(self.url + 'program_name/invalid/') - self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/learner_dashboard/urls.py b/lms/djangoapps/learner_dashboard/urls.py index 5fb34ad628..27e5e83ecd 100644 --- a/lms/djangoapps/learner_dashboard/urls.py +++ b/lms/djangoapps/learner_dashboard/urls.py @@ -6,7 +6,5 @@ from . import views urlpatterns = [ url(r'^programs/$', views.program_listing, name='program_listing_view'), - # Matches paths like 'programs/123/' and 'programs/123/foo/', but not 'programs/123/foo/bar/'. - # Also accepts strings that look like UUIDs, to support retrieval of catalog-based MicroMasters. - url(r'^programs/(?P[0-9a-f-]+)/[\w\-]*/?$', views.program_details, name='program_details_view'), + url(r'^programs/(?P[0-9a-f-]+)/$', views.program_details, name='program_details_view'), ] diff --git a/lms/djangoapps/learner_dashboard/views.py b/lms/djangoapps/learner_dashboard/views.py index 0da18e5c38..89bcf60827 100644 --- a/lms/djangoapps/learner_dashboard/views.py +++ b/lms/djangoapps/learner_dashboard/views.py @@ -1,6 +1,4 @@ """Learner dashboard views""" -import uuid - from django.contrib.auth.decorators import login_required from django.core.urlresolvers import reverse from django.http import Http404 @@ -8,12 +6,16 @@ from django.views.decorators.http import require_GET from edxmako.shortcuts import render_to_response from lms.djangoapps.learner_dashboard.utils import strip_course_id, FAKE_COURSE_KEY -from openedx.core.djangoapps.catalog.utils import get_programs as get_catalog_programs, munge_catalog_program +from openedx.core.djangoapps.catalog.utils import get_programs, munge_catalog_program from openedx.core.djangoapps.credentials.utils import get_programs_credentials from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs import utils +from openedx.core.djangoapps.programs.utils import ( + get_program_marketing_url, + munge_progress_map, + ProgramProgressMeter, + ProgramDataExtender, +) from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences -import waffle @login_required @@ -24,16 +26,17 @@ def program_listing(request): if not programs_config.show_program_listing: raise Http404 - use_catalog = waffle.switch_is_active('get_programs_from_catalog') - meter = utils.ProgramProgressMeter(request.user, use_catalog=use_catalog) + meter = ProgramProgressMeter(request.user) + engaged_programs = [munge_catalog_program(program) for program in meter.engaged_programs] + progress = [munge_progress_map(progress_map) for progress_map in meter.progress] context = { 'credentials': get_programs_credentials(request.user), 'disable_courseware_js': True, - 'marketing_url': utils.get_program_marketing_url(programs_config), + 'marketing_url': get_program_marketing_url(programs_config), 'nav_hidden': True, - 'programs': meter.engaged_programs(), - 'progress': meter.progress, + 'programs': engaged_programs, + 'progress': progress, 'show_program_listing': programs_config.show_program_listing, 'uses_pattern_library': True, } @@ -43,26 +46,18 @@ def program_listing(request): @login_required @require_GET -def program_details(request, program_id): +def program_details(request, program_uuid): """View details about a specific program.""" programs_config = ProgramsApiConfig.current() if not programs_config.show_program_details: raise Http404 - try: - # If the ID is a UUID, the requested program resides in the catalog. - uuid.UUID(program_id) - - program_data = get_catalog_programs(request.user, uuid=program_id) - if program_data: - program_data = munge_catalog_program(program_data) - except ValueError: - program_data = utils.get_programs(request.user, program_id=program_id) - + program_data = get_programs(uuid=program_uuid) if not program_data: raise Http404 - program_data = utils.ProgramDataExtender(program_data, request.user).extend() + program_data = munge_catalog_program(program_data) + program_data = ProgramDataExtender(program_data, request.user).extend() urls = { 'program_listing_url': reverse('program_listing_view'), diff --git a/openedx/core/djangoapps/catalog/tests/factories.py b/openedx/core/djangoapps/catalog/tests/factories.py index ebd00d2bb7..e212facf7c 100644 --- a/openedx/core/djangoapps/catalog/tests/factories.py +++ b/openedx/core/djangoapps/catalog/tests/factories.py @@ -1,6 +1,6 @@ """Factories for generating fake catalog data.""" # pylint: disable=missing-docstring, invalid-name -from random import randint +from functools import partial import factory from faker import Faker @@ -9,6 +9,14 @@ from faker import Faker fake = Faker() +def generate_instances(factory_class, count=3): + """ + Use this to populate fields with values derived from other factories. If + the array is used directly, the same value will be used repeatedly. + """ + return factory_class.create_batch(count) + + def generate_course_key(): return '+'.join(fake.words(2)) @@ -26,17 +34,17 @@ def generate_zulu_datetime(): return fake.date_time().isoformat() + 'Z' -class DictFactory(factory.Factory): +class DictFactoryBase(factory.Factory): class Meta(object): model = dict -class ImageFactory(DictFactory): +class ImageFactoryBase(DictFactoryBase): height = factory.Faker('random_int') width = factory.Faker('random_int') -class Image(ImageFactory): +class ImageFactory(ImageFactoryBase): """ For constructing dicts mirroring the catalog's serialized representation of ImageFields. @@ -46,7 +54,7 @@ class Image(ImageFactory): src = factory.Faker('image_url') -class StdImage(ImageFactory): +class StdImageFactory(ImageFactoryBase): """ For constructing dicts mirroring the catalog's serialized representation of StdImageFields. @@ -57,21 +65,21 @@ class StdImage(ImageFactory): def generate_sized_stdimage(): return { - size: StdImage() for size in ['large', 'medium', 'small', 'x-small'] + size: StdImageFactory() for size in ['large', 'medium', 'small', 'x-small'] } -class Organization(DictFactory): +class OrganizationFactory(DictFactoryBase): key = factory.Faker('word') name = factory.Faker('company') uuid = factory.Faker('uuid4') -class CourseRun(DictFactory): +class CourseRunFactory(DictFactoryBase): end = factory.LazyFunction(generate_zulu_datetime) enrollment_end = factory.LazyFunction(generate_zulu_datetime) enrollment_start = factory.LazyFunction(generate_zulu_datetime) - image = Image() + image = ImageFactory() key = factory.LazyFunction(generate_course_run_key) marketing_url = factory.Faker('url') pacing_type = 'self_paced' @@ -82,20 +90,20 @@ class CourseRun(DictFactory): uuid = factory.Faker('uuid4') -class Course(DictFactory): - course_runs = [CourseRun() for __ in range(randint(3, 5))] - image = Image() +class CourseFactory(DictFactoryBase): + course_runs = factory.LazyFunction(partial(generate_instances, CourseRunFactory)) + image = ImageFactory() key = factory.LazyFunction(generate_course_key) - owners = [Organization()] + owners = factory.LazyFunction(partial(generate_instances, OrganizationFactory, count=1)) title = factory.Faker('catch_phrase') uuid = factory.Faker('uuid4') -class Program(DictFactory): - authoring_organizations = [Organization()] +class ProgramFactory(DictFactoryBase): + authoring_organizations = factory.LazyFunction(partial(generate_instances, OrganizationFactory, count=1)) banner_image = factory.LazyFunction(generate_sized_stdimage) card_image_url = factory.Faker('image_url') - courses = [Course() for __ in range(randint(3, 5))] + courses = factory.LazyFunction(partial(generate_instances, CourseFactory)) marketing_slug = factory.Faker('slug') marketing_url = factory.Faker('url') status = 'active' @@ -105,6 +113,6 @@ class Program(DictFactory): uuid = factory.Faker('uuid4') -class ProgramType(DictFactory): +class ProgramTypeFactory(DictFactoryBase): name = factory.Faker('word') logo_image = factory.LazyFunction(generate_sized_stdimage) diff --git a/openedx/core/djangoapps/catalog/tests/mixins.py b/openedx/core/djangoapps/catalog/tests/mixins.py index db6142aadb..5228187c55 100644 --- a/openedx/core/djangoapps/catalog/tests/mixins.py +++ b/openedx/core/djangoapps/catalog/tests/mixins.py @@ -5,16 +5,19 @@ from openedx.core.djangoapps.catalog.models import CatalogIntegration class CatalogIntegrationMixin(object): """Utility for working with the catalog service during testing.""" - DEFAULTS = { + catalog_integration_defaults = { 'enabled': True, 'internal_api_url': 'https://catalog-internal.example.com/api/v1/', 'cache_ttl': 0, - 'service_username': 'lms_catalog_service_user' + 'service_username': 'lms_catalog_service_user', } def create_catalog_integration(self, **kwargs): - """Creates a new CatalogIntegration with DEFAULTS, updated with any provided overrides.""" - fields = dict(self.DEFAULTS, **kwargs) + """ + Creates a new CatalogIntegration with catalog_integration_defaults, + updated with any provided overrides. + """ + fields = dict(self.catalog_integration_defaults, **kwargs) CatalogIntegration(**fields).save() return CatalogIntegration.current() diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index b74aca9847..18d09abcb0 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -1,37 +1,48 @@ """Tests covering utilities for integrating with the catalog service.""" +# pylint: disable=missing-docstring import uuid 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 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.djangoapps.catalog.tests.factories import ProgramFactory, ProgramTypeFactory +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangoapps.catalog.utils import ( + get_programs, + munge_catalog_program, + get_program_types, + get_programs_with_type_logo, +) +from openedx.core.djangolib.testing.utils import skip_unless_lms +from student.tests.factories import UserFactory + UTILS_MODULE = 'openedx.core.djangoapps.catalog.utils' +User = get_user_model() # pylint: disable=invalid-name +@skip_unless_lms @mock.patch(UTILS_MODULE + '.get_edx_api_data') -# 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): +class TestGetPrograms(CatalogIntegrationMixin, TestCase): """Tests covering retrieval of programs from the catalog service.""" def setUp(self): super(TestGetPrograms, self).setUp() - self.user = UserFactory() self.uuid = str(uuid.uuid4()) self.type = '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 """Verify that API data retrieval utility is used correctly.""" args, kwargs = call_args - for arg in (self.catalog_integration, self.user, 'programs'): + for arg in (self.catalog_integration, 'programs'): self.assertIn(arg, args) self.assertEqual(kwargs['resource_id'], program_uuid) @@ -57,138 +68,88 @@ class TestGetPrograms(mixins.CatalogIntegrationMixin, TestCase): return args, kwargs - def test_get_programs(self, _mock_cache, mock_get_catalog_data): - programs = [factories.Program() for __ in range(3)] - mock_get_catalog_data.return_value = programs + def test_get_programs(self, mock_get_edx_api_data): + programs = [ProgramFactory() for __ in range(3)] + mock_get_edx_api_data.return_value = programs - data = utils.get_programs(self.user) + data = get_programs() - self.assert_contract(mock_get_catalog_data.call_args) + self.assert_contract(mock_get_edx_api_data.call_args) self.assertEqual(data, programs) - def test_get_programs_anonymous_user(self, _mock_cache, mock_get_catalog_data): - programs = [factories.Program() for __ in range(3)] - mock_get_catalog_data.return_value = programs + def test_get_one_program(self, mock_get_edx_api_data): + program = ProgramFactory() + mock_get_edx_api_data.return_value = program - anonymous_user = AnonymousUserFactory() + data = get_programs(uuid=self.uuid) - # The user is an Anonymous user but the Catalog Service User has not been created yet. - data = utils.get_programs(anonymous_user) - # This should not return programs. - self.assertEqual(data, []) - - UserFactory(username='lms_catalog_service_user') - # After creating the service user above, - data = utils.get_programs(anonymous_user) - # the programs should be returned successfully. - self.assertEqual(data, programs) - - def test_get_program_types(self, _mock_cache, mock_get_catalog_data): - program_types = [factories.ProgramType() for __ in range(3)] - mock_get_catalog_data.return_value = program_types - - # Creating Anonymous user but the Catalog Service User has not been created yet. - anonymous_user = AnonymousUserFactory() - data = utils.get_program_types(anonymous_user) - # This should not return programs. - self.assertEqual(data, []) - - # Creating Catalog Service User user - UserFactory(username='lms_catalog_service_user') - data = utils.get_program_types(anonymous_user) - # the programs should be returned successfully. - self.assertEqual(data, program_types) - - # Catalog integration is disabled now. - self.catalog_integration = self.create_catalog_integration(enabled=False) - data = utils.get_program_types(anonymous_user) - # This should not return programs. - self.assertEqual(data, []) - - def test_get_programs_data(self, _mock_cache, mock_get_catalog_data): # pylint: disable=unused-argument - programs = [] - program_types = [] - programs_data = [] - - for index in range(3): - # Creating the Programs and their corresponding program types. - type_name = "type_name_{postfix}".format(postfix=index) - program = factories.Program(type=type_name) - program_type = factories.ProgramType(name=type_name) - - # Maintaining the programs, program types and program data(program+logo_image) lists. - programs.append(program) - program_types.append(program_type) - programs_data.append(copy.deepcopy(program)) - - # Adding the logo image in program data. - programs_data[-1]['logo_image'] = program_type["logo_image"] - - 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: - # Mocked the "get_programs" and "get_program_types" - patched_get_programs.return_value = programs - patched_get_program_types.return_value = program_types - - programs_data = utils.get_programs_data() - self.assertEqual(programs_data, programs) - - def test_get_one_program(self, _mock_cache, mock_get_catalog_data): - program = factories.Program() - mock_get_catalog_data.return_value = program - - data = utils.get_programs(self.user, uuid=self.uuid) - - self.assert_contract(mock_get_catalog_data.call_args, program_uuid=self.uuid) + 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_cache, mock_get_catalog_data): - programs = [factories.Program() for __ in range(2)] - mock_get_catalog_data.return_value = programs + def test_get_programs_by_type(self, mock_get_edx_api_data): + programs = ProgramFactory.create_batch(2) + mock_get_edx_api_data.return_value = programs - data = utils.get_programs(self.user, type=self.type) + data = get_programs(type=self.type) - self.assert_contract(mock_get_catalog_data.call_args, type=self.type) + self.assert_contract(mock_get_edx_api_data.call_args, type=self.type) self.assertEqual(data, programs) - def test_programs_unavailable(self, _mock_cache, mock_get_catalog_data): - mock_get_catalog_data.return_value = [] + def test_programs_unavailable(self, mock_get_edx_api_data): + mock_get_edx_api_data.return_value = [] - data = utils.get_programs(self.user) + data = get_programs() - self.assert_contract(mock_get_catalog_data.call_args) + self.assert_contract(mock_get_edx_api_data.call_args) self.assertEqual(data, []) - def test_cache_disabled(self, _mock_cache, mock_get_catalog_data): + def test_cache_disabled(self, mock_get_edx_api_data): self.catalog_integration = self.create_catalog_integration(cache_ttl=0) - utils.get_programs(self.user) - self.assert_contract(mock_get_catalog_data.call_args) + get_programs() + self.assert_contract(mock_get_edx_api_data.call_args) - 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.""" + def test_config_missing(self, _mock_get_edx_api_data): + """ + Verify that no errors occur if this method is called when catalog config + is missing. + """ CatalogIntegration.objects.all().delete() - data = utils.get_programs(self.user) + data = get_programs() + self.assertEqual(data, []) + + def test_service_user_missing(self, _mock_get_edx_api_data): + """ + Verify that no errors occur if this method is called when the catalog + service user is missing. + """ + # Note: Deleting the service user would be ideal, but causes mysterious + # errors on Jenkins. + self.create_catalog_integration(service_username='nonexistent-user') + + data = get_programs() self.assertEqual(data, []) class TestMungeCatalogProgram(TestCase): - """Tests covering querystring stripping.""" - catalog_program = factories.Program() + def setUp(self): + super(TestMungeCatalogProgram, self).setUp() - def test_munge_catalog_program(self): - munged = utils.munge_catalog_program(self.catalog_program) + self.catalog_program = ProgramFactory() + + def assert_munged(self, program): + munged = munge_catalog_program(program) expected = { - 'id': self.catalog_program['uuid'], - 'name': self.catalog_program['title'], - 'subtitle': self.catalog_program['subtitle'], - 'category': self.catalog_program['type'], - 'marketing_slug': self.catalog_program['marketing_slug'], + 'id': program['uuid'], + 'name': program['title'], + 'subtitle': program['subtitle'], + 'category': program['type'], + 'marketing_slug': program['marketing_slug'], 'organizations': [ { 'display_name': organization['name'], 'key': organization['key'] - } for organization in self.catalog_program['authoring_organizations'] + } for organization in program['authoring_organizations'] ], 'course_codes': [ { @@ -200,108 +161,72 @@ class TestMungeCatalogProgram(TestCase): }, 'run_modes': [ { - 'course_key': run['key'], - 'run_key': CourseKey.from_string(run['key']).run, - 'mode_slug': 'verified' - } for run in course['course_runs'] + 'course_key': course_run['key'], + 'run_key': CourseKey.from_string(course_run['key']).run, + 'mode_slug': course_run['type'], + 'marketing_url': course_run['marketing_url'], + } for course_run in course['course_runs'] ], - } for course in self.catalog_program['courses'] + } for course in program['courses'] ], 'banner_image_urls': { - 'w1440h480': self.catalog_program['banner_image']['large']['url'], - 'w726h242': self.catalog_program['banner_image']['medium']['url'], - 'w435h145': self.catalog_program['banner_image']['small']['url'], - 'w348h116': self.catalog_program['banner_image']['x-small']['url'], + 'w1440h480': program['banner_image']['large']['url'], + 'w726h242': program['banner_image']['medium']['url'], + 'w435h145': program['banner_image']['small']['url'], + 'w348h116': program['banner_image']['x-small']['url'], }, + 'detail_url': program.get('detail_url'), } self.assertEqual(munged, expected) + def test_munge_catalog_program(self): + self.assert_munged(self.catalog_program) + def test_munge_with_detail_url(self): + self.catalog_program['detail_url'] = 'foo' + self.assert_munged(self.catalog_program) + + +@skip_unless_lms @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() +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)] + mock_get_edx_api_data.return_value = program_types - self.user = UserFactory() - self.course_key = CourseKey.from_string('foo/bar/baz') - self.catalog_integration = self.create_catalog_integration() + # Catalog integration is disabled. + data = get_program_types() + self.assertEqual(data, []) - def assert_contract(self, call_args): - """Verify that API data retrieval utility is used correctly.""" - args, kwargs = call_args + catalog_integration = self.create_catalog_integration() + UserFactory(username=catalog_integration.service_username) + data = get_program_types() + self.assertEqual(data, program_types) - for arg in (self.catalog_integration, self.user, 'course_runs'): - self.assertIn(arg, args) + def test_get_programs_with_type_logo(self, _mock_get_edx_api_data): + programs = [] + program_types = [] + programs_with_type_logo = [] - 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 + 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) - return args, kwargs + programs.append(program) + program_types.append(program_type) - def test_get_course_run(self, _mock_cache, mock_get_catalog_data): - course_run = factories.CourseRun() - mock_get_catalog_data.return_value = course_run + 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) - data = utils.get_course_run(self.course_key, self.user) + 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 - 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_cache_disabled(self, _mock_cache, mock_get_catalog_data): - utils.get_course_run(self.course_key, self.user) - - _, kwargs = self.assert_contract(mock_get_catalog_data.call_args) - - self.assertIsNone(kwargs['cache_key']) - - def test_cache_enabled(self, _mock_cache, mock_get_catalog_data): - catalog_integration = self.create_catalog_integration(cache_ttl=1) - - utils.get_course_run(self.course_key, self.user) - - _, kwargs = mock_get_catalog_data.call_args - - self.assertEqual(kwargs['cache_key'], catalog_integration.CACHE_KEY) - - 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() - - data = utils.get_course_run(self.course_key, self.user) - self.assertEqual(data, {}) - - -@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() - - def test_get_run_marketing_url(self, mock_get_course_run): - course_run = factories.CourseRun() - mock_get_course_run.return_value = course_run - - url = utils.get_run_marketing_url(self.course_key, self.user) - - self.assertEqual(url, course_run['marketing_url']) - - def test_marketing_url_missing(self, mock_get_course_run): - mock_get_course_run.return_value = {} - - url = utils.get_run_marketing_url(self.course_key, self.user) - - self.assertEqual(url, None) + actual = get_programs_with_type_logo() + self.assertEqual(actual, programs_with_type_logo) diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index 528f1ed268..229749a26a 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -1,6 +1,6 @@ """Helper functions for working with the catalog service.""" from django.conf import settings -from django.contrib.auth.models import User +from django.contrib.auth import get_user_model from edx_rest_api_client.client import EdxRestApiClient from opaque_keys.edx.keys import CourseKey @@ -9,6 +9,9 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.token_utils import JwtBuilder +User = get_user_model() # pylint: disable=invalid-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'] @@ -18,20 +21,7 @@ def create_catalog_api_client(user, catalog_integration): return EdxRestApiClient(catalog_integration.internal_api_url, jwt=jwt) -def _get_service_user(user, service_username): - """ - Retrieve and return the Catalog Integration Service User Object - if the passed user is None or anonymous - """ - if not user or user.is_anonymous(): - try: - user = User.objects.get(username=service_username) - except User.DoesNotExist: - user = None - return user - - -def get_programs(user=None, uuid=None, type=None): # pylint: disable=redefined-builtin +def get_programs(uuid=None, type=None): # pylint: disable=redefined-builtin """Retrieve marketable programs from the catalog service. Keyword Arguments: @@ -44,8 +34,9 @@ def get_programs(user=None, uuid=None, type=None): # pylint: disable=redefined- """ catalog_integration = CatalogIntegration.current() if catalog_integration.enabled: - user = _get_service_user(user, catalog_integration.service_username) - if not user: + try: + user = User.objects.get(username=catalog_integration.service_username) + except User.DoesNotExist: return [] api = create_catalog_api_client(user, catalog_integration) @@ -75,54 +66,15 @@ def get_programs(user=None, uuid=None, type=None): # pylint: disable=redefined- return [] -def get_program_types(user=None): # pylint: disable=redefined-builtin - """Retrieve all program types from the catalog service. - - Returns: - list of dict, representing program types. - """ - catalog_integration = CatalogIntegration.current() - if catalog_integration.enabled: - user = _get_service_user(user, catalog_integration.service_username) - if not user: - return [] - - api = create_catalog_api_client(user, catalog_integration) - cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY) - - return get_edx_api_data( - catalog_integration, - user, - 'program_types', - cache_key=cache_key if catalog_integration.is_cache_enabled else None, - api=api - ) - else: - return [] - - -def get_programs_data(user=None): - """Return the list of Programs after adding the ProgramType Logo Image""" - - programs_list = get_programs(user) - program_types = get_program_types(user) - - program_types_lookup_dict = {program_type["name"]: program_type for program_type in program_types} - - for program in programs_list: - program["logo_image"] = program_types_lookup_dict[program["type"]]["logo_image"] - - return programs_list - - def munge_catalog_program(catalog_program): - """Make a program from the catalog service look like it came from the programs service. + """ + Make a program from the catalog service look like it came from the programs service. - Catalog-based MicroMasters need to be displayed in the LMS. However, the LMS - currently retrieves all program data from the soon-to-be-retired programs service. - Consuming program data exclusively from the catalog service would have taken more time - than we had prior to the MicroMasters launch. This is a functional middle ground - introduced by ECOM-5460. Cleaning up this debt is tracked by ECOM-4418. + We want to display programs from the catalog service on the LMS. The LMS + originally retrieved all program data from the deprecated programs service. + This temporary utility is here to help incrementally swap out the backend. + + Clean up of this debt is tracked by ECOM-4418. Arguments: catalog_program (dict): The catalog service's representation of a program. @@ -153,10 +105,11 @@ def munge_catalog_program(catalog_program): } if course['owners'] else {}, 'run_modes': [ { - 'course_key': run['key'], - 'run_key': CourseKey.from_string(run['key']).run, - 'mode_slug': 'verified' - } for run in course['course_runs'] + 'course_key': course_run['key'], + 'run_key': CourseKey.from_string(course_run['key']).run, + 'mode_slug': course_run['type'], + 'marketing_url': course_run['marketing_url'], + } for course_run in course['course_runs'] ], } for course in catalog_program['courses'] ], @@ -166,48 +119,48 @@ def munge_catalog_program(catalog_program): 'w435h145': catalog_program['banner_image']['small']['url'], 'w348h116': catalog_program['banner_image']['x-small']['url'], }, + # If a detail URL has been added, we don't want to lose it. + 'detail_url': catalog_program.get('detail_url'), } -def get_course_run(course_key, user): - """Get a course run's data from the course catalog service. - - Arguments: - 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. +def get_program_types(): + """Retrieve all program types from the catalog service. Returns: - dict, empty if no data could be retrieved. + list of dict, representing program types. """ catalog_integration = CatalogIntegration.current() - if catalog_integration.enabled: - api = create_catalog_api_client(user, catalog_integration) + try: + user = User.objects.get(username=catalog_integration.service_username) + except User.DoesNotExist: + return [] - data = get_edx_api_data( + api = create_catalog_api_client(user, catalog_integration) + cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY) + + return 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}, + 'program_types', + cache_key=cache_key if catalog_integration.is_cache_enabled else None, + api=api ) - - return data if data else {} else: - return {} + return [] -def get_run_marketing_url(course_key, user): - """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. - user (User): The user to authenticate as when making requests to the catalog service. - - Returns: - string, the marketing URL, or None if no URL is available. +def get_programs_with_type_logo(): """ - course_run = get_course_run(course_key, user) - return course_run.get('marketing_url') + Join program type logos with programs of corresponding type. + """ + programs_list = get_programs() + program_types = get_program_types() + + type_logo_map = {program_type['name']: program_type['logo_image'] for program_type in program_types} + + for program in programs_list: + program['logo_image'] = type_logo_map[program['type']] + + return programs_list diff --git a/openedx/core/djangoapps/credentials/tests/test_utils.py b/openedx/core/djangoapps/credentials/tests/test_utils.py index d51a85f25a..096cd6d763 100644 --- a/openedx/core/djangoapps/credentials/tests/test_utils.py +++ b/openedx/core/djangoapps/credentials/tests/test_utils.py @@ -8,7 +8,7 @@ import mock from nose.plugins.attrib import attr from provider.constants import CONFIDENTIAL -from openedx.core.djangoapps.catalog.tests import factories as catalog_factories +from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin from openedx.core.djangoapps.credentials.utils import ( @@ -18,8 +18,6 @@ from openedx.core.djangoapps.credentials.utils import ( get_programs_for_credentials ) from openedx.core.djangoapps.credentials.tests import factories -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin, ProgramsDataMixin -from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from student.tests.factories import UserFactory @@ -37,7 +35,6 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, super(TestCredentialsRetrieval, self).setUp() ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) self.user = UserFactory() self.primary_uuid = str(uuid.uuid4()) self.alternate_uuid = str(uuid.uuid4()) @@ -129,7 +126,7 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, } self.mock_credentials_api(self.user, data=credentials_api_response, reset_url=False) programs = [ - catalog_factories.Program(uuid=primary_uuid), catalog_factories.Program(uuid=alternate_uuid) + ProgramFactory(uuid=primary_uuid), ProgramFactory(uuid=alternate_uuid) ] with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs_for_credentials") as mock_get_programs: @@ -165,7 +162,7 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, } self.mock_credentials_api(self.user, data=credentials_api_response, reset_url=False) programs = [ - catalog_factories.Program(uuid=primary_uuid), catalog_factories.Program(uuid=alternate_uuid) + ProgramFactory(uuid=primary_uuid), ProgramFactory(uuid=alternate_uuid) ] with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as mock_get_programs: @@ -199,14 +196,14 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, def test_get_program_for_certificates(self): """Verify programs data can be retrieved and parsed correctly for certificates.""" programs = [ - catalog_factories.Program(uuid=self.primary_uuid), - catalog_factories.Program(uuid=self.alternate_uuid) + ProgramFactory(uuid=self.primary_uuid), + ProgramFactory(uuid=self.alternate_uuid) ] program_credentials_data = self._expected_program_credentials_data() with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as patched_get_programs: patched_get_programs.return_value = programs - actual = get_programs_for_credentials(self.user, program_credentials_data) + actual = get_programs_for_credentials(program_credentials_data) self.assertEqual(len(actual), 2) self.assertEqual(actual, programs) @@ -216,6 +213,6 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, program_credentials_data = self._expected_program_credentials_data() with mock.patch("openedx.core.djangoapps.credentials.utils.get_programs") as patched_get_programs: patched_get_programs.return_value = [] - actual = get_programs_for_credentials(self.user, program_credentials_data) + actual = get_programs_for_credentials(program_credentials_data) self.assertEqual(actual, []) diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index e9aacd3df1..e22aadb6d8 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -31,12 +31,11 @@ def get_user_credentials(user): return credentials -def get_programs_for_credentials(user, programs_credentials): +def get_programs_for_credentials(programs_credentials): """ Given a user and an iterable of credentials, get corresponding programs data and return it as a list of dictionaries. Arguments: - user (User): The user to authenticate as for requesting programs. programs_credentials (list): List of credentials awarded to the user for completion of a program. @@ -44,7 +43,7 @@ def get_programs_for_credentials(user, programs_credentials): list, containing programs dictionaries. """ certified_programs = [] - programs = get_programs(user) + programs = get_programs() for program in programs: for credential in programs_credentials: if program['uuid'] == credential['credential']['program_uuid']: @@ -84,7 +83,7 @@ def get_user_program_credentials(user): log.exception('Invalid credential structure: %r', credential) if programs_credentials: - programs_credentials_data = get_programs_for_credentials(user, programs_credentials) + programs_credentials_data = get_programs_for_credentials(programs_credentials) return programs_credentials_data diff --git a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py index 8bcd6dd869..c670b646c4 100644 --- a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py @@ -80,10 +80,10 @@ class Command(BaseCommand): course_runs = set() for program in programs: for course in program['courses']: - for run in course['course_runs']: - key = CourseKey.from_string(run['key']) + for course_run in course['course_runs']: + key = CourseKey.from_string(course_run['key']) course_runs.add( - CourseRun(key, run['type']) + CourseRun(key, course_run['type']) ) return course_runs @@ -97,14 +97,7 @@ class Command(BaseCommand): status_query = Q(status__in=CertificateStatuses.PASSED_STATUSES) course_run_query = reduce( lambda x, y: x | y, - # A course run's type is assumed to indicate which mode must be - # completed in order for the run to count towards program completion. - # This supports the same flexible program construction allowed by the - # old programs service (e.g., completion of an old honor-only run may - # count towards completion of a course in a program). This may change - # in the future to make use of the more rigid set of "applicable seat - # types" associated with each program type in the catalog. - [Q(course_id=run.key, mode=run.type) for run in self.course_runs] + [Q(course_id=course_run.key, mode=course_run.type) for course_run in self.course_runs] ) query = status_query & course_run_query diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index cce1cd7e88..7aec4e7bf0 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tasks.py @@ -65,7 +65,7 @@ def get_completed_programs(student): list of program UUIDs """ - meter = ProgramProgressMeter(student, use_catalog=True) + meter = ProgramProgressMeter(student) return meter.completed_programs diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 310a8e7289..7903a715aa 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -6,24 +6,20 @@ import json from celery.exceptions import MaxRetriesExceededError import ddt from django.conf import settings -from django.core.cache import cache from django.test import override_settings, TestCase from edx_rest_api_client.client import EdxRestApiClient from edx_oauth2_provider.tests.factories import ClientFactory import httpretty import mock -from provider.constants import CONFIDENTIAL -from lms.djangoapps.certificates.api import MODES -from openedx.core.djangoapps.catalog.tests import factories, mixins -from openedx.core.djangoapps.catalog.utils import munge_catalog_program +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin from openedx.core.djangoapps.programs.tasks.v1 import tasks -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 + TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks' -UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' @skip_unless_lms @@ -49,57 +45,6 @@ class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase): self.assertEqual(api_client._store['session'].auth.token, 'test-token') # pylint: disable=protected-access -@httpretty.activate -@skip_unless_lms -class GetCompletedProgramsTestCase(mixins.CatalogIntegrationMixin, CacheIsolationTestCase): - """ - Test the get_completed_programs function - """ - ENABLED_CACHES = ['default'] - - def setUp(self): - super(GetCompletedProgramsTestCase, self).setUp() - - self.user = UserFactory() - self.catalog_integration = self.create_catalog_integration(cache_ttl=1) - - def _mock_programs_api(self, data): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock API calls.') - - url = self.catalog_integration.internal_api_url.strip('/') + '/programs/' - body = json.dumps({'results': data}) - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') - - def _assert_num_requests(self, count): - """DRY helper for verifying request counts.""" - self.assertEqual(len(httpretty.httpretty.latest_requests), count) - - @mock.patch(UTILS_MODULE + '.get_completed_courses') - def test_get_completed_programs(self, mock_get_completed_courses): - """ - Verify that completed programs are found, using the cache when possible. - """ - data = [ - factories.Program(), - ] - self._mock_programs_api(data) - - munged_program = munge_catalog_program(data[0]) - course_codes = munged_program['course_codes'] - - mock_get_completed_courses.return_value = [ - {'course_id': run_mode['course_key'], 'mode': run_mode['mode_slug']} - for run_mode in course_codes[0]['run_modes'] - ] - for _ in range(2): - result = tasks.get_completed_programs(self.user) - self.assertEqual(result[0], munged_program['id']) - - # Verify that only one request to the catalog was made (i.e., the cache was hit). - self._assert_num_requests(1) - - @skip_unless_lms class GetAwardedCertificateProgramsTestCase(TestCase): """ @@ -175,7 +120,7 @@ class AwardProgramCertificateTestCase(TestCase): @mock.patch(TASKS_MODULE + '.get_certified_programs') @mock.patch(TASKS_MODULE + '.get_completed_programs') @override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username') -class AwardProgramCertificatesTestCase(mixins.CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): +class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """ Tests for the 'award_program_certificates' celery task. """ diff --git a/openedx/core/djangoapps/programs/tests/factories.py b/openedx/core/djangoapps/programs/tests/factories.py index 23484c0dd7..19efc5d8b4 100644 --- a/openedx/core/djangoapps/programs/tests/factories.py +++ b/openedx/core/djangoapps/programs/tests/factories.py @@ -1,67 +1,17 @@ """Factories for generating fake program-related data.""" +# pylint: disable=missing-docstring, invalid-name import factory -from factory.fuzzy import FuzzyText +from faker import Faker -class Program(factory.Factory): - """ - Factory for stubbing program resources from the Programs API (v1). - """ +fake = Faker() + + +class ProgressFactory(factory.Factory): class Meta(object): model = dict - id = factory.Sequence(lambda n: n) # pylint: disable=invalid-name - name = FuzzyText(prefix='Program ') - subtitle = FuzzyText(prefix='Subtitle ') - category = 'FooBar' - status = 'active' - marketing_slug = FuzzyText(prefix='slug_') - organizations = [] - course_codes = [] - banner_image_urls = {} - - -class Organization(factory.Factory): - """ - Factory for stubbing nested organization resources from the Programs API (v1). - """ - class Meta(object): - model = dict - - key = FuzzyText(prefix='org_') - display_name = FuzzyText(prefix='Display Name ') - - -class CourseCode(factory.Factory): - """ - Factory for stubbing nested course code resources from the Programs API (v1). - """ - class Meta(object): - model = dict - - display_name = FuzzyText(prefix='Display Name ') - run_modes = [] - - -class RunMode(factory.Factory): - """ - Factory for stubbing nested run mode resources from the Programs API (v1). - """ - class Meta(object): - model = dict - - course_key = FuzzyText(prefix='org/', suffix='/run') - mode_slug = 'verified' - - -class Progress(factory.Factory): - """ - Factory for stubbing program progress dicts. - """ - class Meta(object): - model = dict - - id = factory.Sequence(lambda n: n) # pylint: disable=invalid-name + uuid = factory.Faker('uuid4') completed = [] in_progress = [] not_started = [] diff --git a/openedx/core/djangoapps/programs/tests/mixins.py b/openedx/core/djangoapps/programs/tests/mixins.py index e393167b8f..42d73293b7 100644 --- a/openedx/core/djangoapps/programs/tests/mixins.py +++ b/openedx/core/djangoapps/programs/tests/mixins.py @@ -1,10 +1,5 @@ """Mixins for use during testing.""" -import json - -import httpretty - from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.tests import factories class ProgramsApiConfigMixin(object): @@ -29,87 +24,3 @@ class ProgramsApiConfigMixin(object): ProgramsApiConfig(**fields).save() return ProgramsApiConfig.current() - - -class ProgramsDataMixin(object): - """Mixin mocking Programs API URLs and providing fake data for testing. - - NOTE: This mixin is DEPRECATED. Tests should create and manage their own data. - """ - PROGRAM_NAMES = [ - 'Test Program A', - 'Test Program B', - 'Test Program C', - ] - - COURSE_KEYS = [ - 'organization-a/course-a/fall', - 'organization-a/course-a/winter', - 'organization-a/course-b/fall', - 'organization-a/course-b/winter', - 'organization-b/course-c/fall', - 'organization-b/course-c/winter', - 'organization-b/course-d/fall', - 'organization-b/course-d/winter', - ] - - PROGRAMS_API_RESPONSE = { - 'results': [ - factories.Program( - id=1, - name=PROGRAM_NAMES[0], - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=COURSE_KEYS[0]), - factories.RunMode(course_key=COURSE_KEYS[1]), - ]), - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=COURSE_KEYS[2]), - factories.RunMode(course_key=COURSE_KEYS[3]), - ]), - ] - ), - factories.Program( - id=2, - name=PROGRAM_NAMES[1], - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=COURSE_KEYS[4]), - factories.RunMode(course_key=COURSE_KEYS[5]), - ]), - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=COURSE_KEYS[6]), - factories.RunMode(course_key=COURSE_KEYS[7]), - ]), - ] - ), - factories.Program( - id=3, - name=PROGRAM_NAMES[2], - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=COURSE_KEYS[7]), - ]), - ] - ), - ] - } - - def mock_programs_api(self, data=None, program_id='', status_code=200): - """Utility for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') - - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' - if program_id: - url += '{}/'.format(str(program_id)) - - if data is None: - data = self.PROGRAMS_API_RESPONSE - - body = json.dumps(data) - - httpretty.reset() - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json', status=status_code) diff --git a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py index 5573161318..40ff3e3983 100644 --- a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py @@ -7,7 +7,12 @@ import mock from certificates.models import CertificateStatuses # pylint: disable=import-error from lms.djangoapps.certificates.api import MODES from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory -from openedx.core.djangoapps.catalog.tests import factories +from openedx.core.djangoapps.catalog.tests.factories import ( + generate_course_run_key, + ProgramFactory, + CourseFactory, + CourseRunFactory, +) from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -23,7 +28,7 @@ COMMAND_MODULE = 'openedx.core.djangoapps.programs.management.commands.backpopul @skip_unless_lms class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """Tests for the backpopulate_program_credentials management command.""" - course_run_key, alternate_course_run_key = (factories.generate_course_run_key() for __ in range(2)) + course_run_key, alternate_course_run_key = (generate_course_run_key() for __ in range(2)) def setUp(self): super(BackpopulateProgramCredentialsTests, self).setUp() @@ -36,8 +41,8 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp # skewing mock call counts. self.create_credentials_config(enable_learner_issuance=False) - self.catalog_integration = self.create_catalog_integration() - self.service_user = UserFactory(username=self.catalog_integration.service_username) + catalog_integration = self.create_catalog_integration() + UserFactory(username=catalog_integration.service_username) @ddt.data(True, False) def test_handle(self, commit, mock_task, mock_get_programs): @@ -45,10 +50,10 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp Verify that relevant tasks are only enqueued when the commit option is passed. """ data = [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=self.course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=self.course_run_key), ]), ] ), @@ -78,39 +83,39 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp @ddt.data( [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=course_run_key), ]), ] ), - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=alternate_course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=alternate_course_run_key), ]), ] ), ], [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=course_run_key), ]), - factories.Course(course_runs=[ - factories.CourseRun(key=alternate_course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=alternate_course_run_key), ]), ] ), ], [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=course_run_key), - factories.CourseRun(key=alternate_course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=course_run_key), + CourseRunFactory(key=alternate_course_run_key), ]), ] ), @@ -148,11 +153,11 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp course run certificates. """ data = [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=self.course_run_key), - factories.CourseRun(key=self.alternate_course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=self.course_run_key), + CourseRunFactory(key=self.alternate_course_run_key), ]), ] ), @@ -183,10 +188,10 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp qualifying course run certificates. """ data = [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=self.course_run_key, type='honor'), + CourseFactory(course_runs=[ + CourseRunFactory(key=self.course_run_key, type='honor'), ]), ] ), @@ -216,10 +221,10 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp Verify that only course run certificates with a passing status are selected. """ data = [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=self.course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=self.course_run_key), ]), ] ), @@ -261,10 +266,10 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp mock_task.side_effect = side_effect data = [ - factories.Program( + ProgramFactory( courses=[ - factories.Course(course_runs=[ - factories.CourseRun(key=self.course_run_key), + CourseFactory(course_runs=[ + CourseRunFactory(key=self.course_run_key), ]), ] ), diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index 91bf6aeb17..6cc346431c 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -1,5 +1,4 @@ """Tests covering Programs utilities.""" -import copy import datetime import json import uuid @@ -9,630 +8,413 @@ from django.core.cache import cache from django.core.urlresolvers import reverse from django.test import TestCase from django.test.utils import override_settings -from django.utils.text import slugify -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 pytz import utc from lms.djangoapps.certificates.api import MODES from lms.djangoapps.commerce.tests.test_utils import update_commerce_config +from openedx.core.djangoapps.catalog.utils import munge_catalog_program +from openedx.core.djangoapps.catalog.tests.factories import ( + generate_course_run_key, + 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 import utils -from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.tests import factories -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin, ProgramsDataMixin +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 student.tests.factories import UserFactory, CourseEnrollmentFactory from util.date_utils import strftime_localized from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory as ModuleStoreCourseFactory UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' CERTIFICATES_API_MODULE = 'lms.djangoapps.certificates.api' ECOMMERCE_URL_ROOT = 'https://example-ecommerce.com' -MARKETING_URL = 'https://www.example.com/marketing/path' - - -@ddt.ddt -@attr(shard=2) -@httpretty.activate -@skip_unless_lms -class TestProgramRetrieval(ProgramsApiConfigMixin, ProgramsDataMixin, CredentialsDataMixin, - CredentialsApiConfigMixin, CacheIsolationTestCase): - """Tests covering the retrieval of programs from the Programs service.""" - - ENABLED_CACHES = ['default'] - - def setUp(self): - super(TestProgramRetrieval, self).setUp() - - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - self.user = UserFactory() - - cache.clear() - - def test_get_programs(self): - """Verify programs data can be retrieved.""" - self.create_programs_config() - self.mock_programs_api() - - actual = utils.get_programs(self.user) - self.assertEqual( - actual, - self.PROGRAMS_API_RESPONSE['results'] - ) - - # Verify the API was actually hit (not the cache). - self.assertEqual(len(httpretty.httpretty.latest_requests), 1) - - def test_get_programs_caching(self): - """Verify that when enabled, the cache is used for non-staff users.""" - self.create_programs_config(cache_ttl=1) - self.mock_programs_api() - - # Warm up the cache. - utils.get_programs(self.user) - - # Hit the cache. - utils.get_programs(self.user) - - # Verify only one request was made. - self.assertEqual(len(httpretty.httpretty.latest_requests), 1) - - staff_user = UserFactory(is_staff=True) - - # Hit the Programs API twice. - for _ in range(2): - utils.get_programs(staff_user) - - # Verify that three requests have been made (one for student, two for staff). - self.assertEqual(len(httpretty.httpretty.latest_requests), 3) - - def test_get_programs_programs_disabled(self): - """Verify behavior when programs is disabled.""" - self.create_programs_config(enabled=False) - - actual = utils.get_programs(self.user) - self.assertEqual(actual, []) - - @mock.patch('edx_rest_api_client.client.EdxRestApiClient.__init__') - def test_get_programs_client_initialization_failure(self, mock_init): - """Verify behavior when API client fails to initialize.""" - self.create_programs_config() - mock_init.side_effect = Exception - - actual = utils.get_programs(self.user) - self.assertEqual(actual, []) - self.assertTrue(mock_init.called) - - def test_get_programs_data_retrieval_failure(self): - """Verify behavior when data can't be retrieved from Programs.""" - self.create_programs_config() - self.mock_programs_api(status_code=500) - - actual = utils.get_programs(self.user) - self.assertEqual(actual, []) - - -@skip_unless_lms -class GetProgramsByRunTests(TestCase): - """Tests verifying that programs are inverted correctly.""" - maxDiff = None - - @classmethod - def setUpClass(cls): - super(GetProgramsByRunTests, cls).setUpClass() - - cls.user = UserFactory() - - course_keys = [ - CourseKey.from_string('some/course/run'), - CourseKey.from_string('some/other/run'), - ] - - cls.enrollments = [CourseEnrollmentFactory(user=cls.user, course_id=c) for c in course_keys] - cls.course_ids = [unicode(c) for c in course_keys] - - organization = factories.Organization() - joint_programs = sorted([ - factories.Program( - organizations=[organization], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=cls.course_ids[0]), - ]), - ] - ) for __ in range(2) - ], key=lambda p: p['name']) - - cls.programs = joint_programs + [ - factories.Program( - organizations=[organization], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=cls.course_ids[1]), - ]), - ] - ), - factories.Program( - organizations=[organization], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key='yet/another/run'), - ]), - ] - ), - ] - - def test_get_programs_by_run(self): - """Verify that programs are organized by run ID.""" - programs_by_run, course_ids = utils.get_programs_by_run(self.programs, self.enrollments) - - self.assertEqual(programs_by_run[self.course_ids[0]], self.programs[:2]) - self.assertEqual(programs_by_run[self.course_ids[1]], self.programs[2:3]) - - self.assertEqual(course_ids, self.course_ids) - - def test_no_programs(self): - """Verify that the utility can cope with missing programs data.""" - programs_by_run, course_ids = utils.get_programs_by_run([], self.enrollments) - self.assertEqual(programs_by_run, {}) - self.assertEqual(course_ids, self.course_ids) - - def test_no_enrollments(self): - """Verify that the utility can cope with missing enrollment data.""" - programs_by_run, course_ids = utils.get_programs_by_run(self.programs, []) - self.assertEqual(programs_by_run, {}) - self.assertEqual(course_ids, []) - - -@skip_unless_lms -class GetCompletedCoursesTestCase(TestCase): - """ - Test the get_completed_courses function - """ - - def make_cert_result(self, **kwargs): - """ - Helper to create dummy results from the certificates API - """ - result = { - 'username': 'dummy-username', - 'course_key': 'dummy-course', - 'type': 'dummy-type', - 'status': 'dummy-status', - 'download_url': 'http://www.example.com/cert.pdf', - 'grade': '0.98', - 'created': '2015-07-31T00:00:00Z', - 'modified': '2015-07-31T00:00:00Z', - } - result.update(**kwargs) - return result - - @mock.patch(UTILS_MODULE + '.certificate_api.get_certificates_for_user') - def test_get_completed_courses(self, mock_get_certs_for_user): - """ - Ensure the function correctly calls to and handles results from the - certificates API - """ - student = UserFactory(username='test-username') - mock_get_certs_for_user.return_value = [ - self.make_cert_result(status='downloadable', type='verified', course_key='downloadable-course'), - self.make_cert_result(status='generating', type='professional', course_key='generating-course'), - self.make_cert_result(status='unknown', type='honor', course_key='unknown-course'), - ] - - result = utils.get_completed_courses(student) - self.assertEqual(mock_get_certs_for_user.call_args[0], (student.username, )) - self.assertEqual(result, [ - {'course_id': 'downloadable-course', 'mode': 'verified'}, - {'course_id': 'generating-course', 'mode': 'professional'}, - ]) @attr(shard=2) -@httpretty.activate @skip_unless_lms -class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): +@mock.patch(UTILS_MODULE + '.get_programs') +class TestProgramProgressMeter(TestCase): """Tests of the program progress utility class.""" def setUp(self): super(TestProgramProgressMeter, self).setUp() self.user = UserFactory() - self.create_programs_config() - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - - def _mock_programs_api(self, data): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') - - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' - body = json.dumps({'results': data}) - - httpretty.register_uri(httpretty.GET, url, body=body, content_type='application/json') - - def _create_enrollments(self, *course_ids): - """Variadic helper used to create course enrollments.""" - for course_id in course_ids: - CourseEnrollmentFactory(user=self.user, course_id=course_id) + def _create_enrollments(self, *course_run_ids): + """Variadic helper used to create course run enrollments.""" + for course_run_id in course_run_ids: + CourseEnrollmentFactory(user=self.user, course_id=course_run_id) def _assert_progress(self, meter, *progresses): """Variadic helper used to verify progress calculations.""" self.assertEqual(meter.progress, list(progresses)) - def _extract_names(self, program, *course_codes): - """Construct a list containing the display names of the indicated course codes.""" - return [program['course_codes'][cc]['display_name'] for cc in course_codes] + def _extract_titles(self, program, *indices): + """Construct a list containing the titles of the indicated courses.""" + return [program['courses'][index]['title'] for index in indices] def _attach_detail_url(self, programs): """Add expected detail URLs to a list of program dicts.""" for program in programs: - base = reverse('program_details_view', kwargs={'program_id': program['id']}).rstrip('/') - slug = slugify(program['name']) + program['detail_url'] = reverse('program_details_view', kwargs={'program_uuid': program['uuid']}) - program['detail_url'] = '{base}/{slug}'.format(base=base, slug=slug) - - def test_no_enrollments(self): + def test_no_enrollments(self, mock_get_programs): """Verify behavior when programs exist, but no relevant enrollments do.""" - data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), - ] - self._mock_programs_api(data) + data = [ProgramFactory()] + mock_get_programs.return_value = data - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) - self.assertEqual(meter.engaged_programs(), []) + self.assertEqual(meter.engaged_programs, []) self._assert_progress(meter) self.assertEqual(meter.completed_programs, []) - def test_no_programs(self): + def test_no_programs(self, mock_get_programs): """Verify behavior when enrollments exist, but no matching programs do.""" - self._mock_programs_api([]) + mock_get_programs.return_value = [] - self._create_enrollments('org/course/run') - meter = utils.ProgramProgressMeter(self.user) + course_run_id = generate_course_run_key() + self._create_enrollments(course_run_id) + meter = ProgramProgressMeter(self.user) - self.assertEqual(meter.engaged_programs(), []) + self.assertEqual(meter.engaged_programs, []) self._assert_progress(meter) self.assertEqual(meter.completed_programs, []) - def test_single_program_engagement(self): + def test_single_program_engagement(self, mock_get_programs): """ - Verify that correct program is returned when the user has a single enrollment - appearing in one program. + Verify that correct program is returned when the user is enrolled in a + course run appearing in one program. """ - course_id = 'org/course/run' + course_run_key = generate_course_run_key() data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=course_id), + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=course_run_key), ]), ] ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), + ProgramFactory(), ] - self._mock_programs_api(data) + mock_get_programs.return_value = data - self._create_enrollments(course_id) - meter = utils.ProgramProgressMeter(self.user) + self._create_enrollments(course_run_key) + meter = ProgramProgressMeter(self.user) self._attach_detail_url(data) program = data[0] - self.assertEqual(meter.engaged_programs(), [program]) + self.assertEqual(meter.engaged_programs, [program]) self._assert_progress( meter, - factories.Progress( - id=program['id'], - in_progress=self._extract_names(program, 0) + ProgressFactory( + uuid=program['uuid'], + in_progress=self._extract_titles(program, 0) ) ) self.assertEqual(meter.completed_programs, []) - def test_mutiple_program_engagement(self): + def test_mutiple_program_engagement(self, mock_get_programs): """ - Verify that correct programs are returned in the correct order when the user - has multiple enrollments. + Verify that correct programs are returned in the correct order when the + user is enrolled in course runs appearing in programs. """ - first_course_id, second_course_id = 'org/first-course/run', 'org/second-course/run' + newer_course_run_key, older_course_run_key = (generate_course_run_key() for __ in range(2)) data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=first_course_id), + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=newer_course_run_key), ]), ] ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=second_course_id), + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=older_course_run_key), ]), ] ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), + ProgramFactory(), ] - self._mock_programs_api(data) + mock_get_programs.return_value = data - self._create_enrollments(second_course_id, first_course_id) - meter = utils.ProgramProgressMeter(self.user) + # The creation time of the enrollments matters to the test. We want + # the first_course_run_key to represent the newest enrollment. + self._create_enrollments(older_course_run_key, newer_course_run_key) + meter = ProgramProgressMeter(self.user) self._attach_detail_url(data) programs = data[:2] - self.assertEqual(meter.engaged_programs(), programs) + self.assertEqual(meter.engaged_programs, programs) self._assert_progress( meter, - factories.Progress(id=programs[0]['id'], in_progress=self._extract_names(programs[0], 0)), - factories.Progress(id=programs[1]['id'], in_progress=self._extract_names(programs[1], 0)) + *( + ProgressFactory(uuid=program['uuid'], in_progress=self._extract_titles(program, 0)) + for program in programs + ) ) self.assertEqual(meter.completed_programs, []) - def test_shared_enrollment_engagement(self): + def test_shared_enrollment_engagement(self, mock_get_programs): """ - Verify that correct programs are returned when the user has a single enrollment - appearing in multiple programs. + Verify that correct programs are returned when the user is enrolled in a + single course run appearing in multiple programs. """ - shared_course_id, solo_course_id = 'org/shared-course/run', 'org/solo-course/run' + shared_course_run_key, solo_course_run_key = (generate_course_run_key() for __ in range(2)) - joint_programs = sorted([ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=shared_course_id), + batch = [ + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=shared_course_run_key), ]), ] - ) for __ in range(2) - ], key=lambda p: p['name']) - - data = joint_programs + [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=solo_course_id), - ]), - ] - ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), + ) + for __ in range(2) ] - self._mock_programs_api(data) + joint_programs = sorted(batch, key=lambda program: program['title']) + data = joint_programs + [ + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=solo_course_run_key), + ]), + ] + ), + ProgramFactory(), + ] - # Enrollment for the shared course ID created last (most recently). - self._create_enrollments(solo_course_id, shared_course_id) - meter = utils.ProgramProgressMeter(self.user) + mock_get_programs.return_value = data + + # Enrollment for the shared course run created last (most recently). + self._create_enrollments(solo_course_run_key, shared_course_run_key) + meter = ProgramProgressMeter(self.user) self._attach_detail_url(data) programs = data[:3] - self.assertEqual(meter.engaged_programs(), programs) + self.assertEqual(meter.engaged_programs, programs) self._assert_progress( meter, - factories.Progress(id=programs[0]['id'], in_progress=self._extract_names(programs[0], 0)), - factories.Progress(id=programs[1]['id'], in_progress=self._extract_names(programs[1], 0)), - factories.Progress(id=programs[2]['id'], in_progress=self._extract_names(programs[2], 0)) + *( + ProgressFactory(uuid=program['uuid'], in_progress=self._extract_titles(program, 0)) + for program in programs + ) ) self.assertEqual(meter.completed_programs, []) - @mock.patch(UTILS_MODULE + '.get_completed_courses') - def test_simulate_progress(self, mock_get_completed_courses): + @mock.patch(UTILS_MODULE + '.ProgramProgressMeter.completed_course_runs', new_callable=mock.PropertyMock) + def test_simulate_progress(self, mock_completed_course_runs, mock_get_programs): """Simulate the entirety of a user's progress through a program.""" - first_course_id, second_course_id = 'org/first-course/run', 'org/second-course/run' + first_course_run_key, second_course_run_key = (generate_course_run_key() for __ in range(2)) data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=first_course_id), + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=first_course_run_key), ]), - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=second_course_id), + CourseFactory(course_runs=[ + CourseRunFactory(key=second_course_run_key), ]), ] ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), + ProgramFactory(), ] - self._mock_programs_api(data) + mock_get_programs.return_value = data - # No enrollments, no program engaged. - meter = utils.ProgramProgressMeter(self.user) + # No enrollments, no programs in progress. + meter = ProgramProgressMeter(self.user) self._assert_progress(meter) self.assertEqual(meter.completed_programs, []) - # One enrollment, program engaged. - self._create_enrollments(first_course_id) - meter = utils.ProgramProgressMeter(self.user) - program, program_id = data[0], data[0]['id'] + # One enrollment, one program in progress. + self._create_enrollments(first_course_run_key) + meter = ProgramProgressMeter(self.user) + program, program_uuid = data[0], data[0]['uuid'] self._assert_progress( meter, - factories.Progress( - id=program_id, - in_progress=self._extract_names(program, 0), - not_started=self._extract_names(program, 1) + ProgressFactory( + uuid=program_uuid, + in_progress=self._extract_titles(program, 0), + not_started=self._extract_titles(program, 1) ) ) self.assertEqual(meter.completed_programs, []) - # Two enrollments, program in progress. - self._create_enrollments(second_course_id) - meter = utils.ProgramProgressMeter(self.user) + # Two enrollments, all courses in progress. + self._create_enrollments(second_course_run_key) + meter = ProgramProgressMeter(self.user) self._assert_progress( meter, - factories.Progress( - id=program_id, - in_progress=self._extract_names(program, 0, 1) + ProgressFactory( + uuid=program_uuid, + in_progress=self._extract_titles(program, 0, 1) ) ) self.assertEqual(meter.completed_programs, []) - # One valid certificate earned, one course code complete. - mock_get_completed_courses.return_value = [ - {'course_id': first_course_id, 'mode': MODES.verified}, + # One valid certificate earned, one course complete. + mock_completed_course_runs.return_value = [ + {'course_run_id': first_course_run_key, 'type': MODES.verified}, ] - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) self._assert_progress( meter, - factories.Progress( - id=program_id, - completed=self._extract_names(program, 0), - in_progress=self._extract_names(program, 1) + ProgressFactory( + uuid=program_uuid, + completed=self._extract_titles(program, 0), + in_progress=self._extract_titles(program, 1) ) ) self.assertEqual(meter.completed_programs, []) - # Invalid certificate earned, still one course code to complete. - mock_get_completed_courses.return_value = [ - {'course_id': first_course_id, 'mode': MODES.verified}, - {'course_id': second_course_id, 'mode': MODES.honor}, + # Invalid certificate earned, still one course to complete. + mock_completed_course_runs.return_value = [ + {'course_run_id': first_course_run_key, 'type': MODES.verified}, + {'course_run_id': second_course_run_key, 'type': MODES.honor}, ] - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) self._assert_progress( meter, - factories.Progress( - id=program_id, - completed=self._extract_names(program, 0), - in_progress=self._extract_names(program, 1) + ProgressFactory( + uuid=program_uuid, + completed=self._extract_titles(program, 0), + in_progress=self._extract_titles(program, 1) ) ) self.assertEqual(meter.completed_programs, []) - # Second valid certificate obtained, all course codes complete. - mock_get_completed_courses.return_value = [ - {'course_id': first_course_id, 'mode': MODES.verified}, - {'course_id': second_course_id, 'mode': MODES.verified}, + # Second valid certificate obtained, all courses complete. + mock_completed_course_runs.return_value = [ + {'course_run_id': first_course_run_key, 'type': MODES.verified}, + {'course_run_id': second_course_run_key, 'type': MODES.verified}, ] - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) self._assert_progress( meter, - factories.Progress( - id=program_id, - completed=self._extract_names(program, 0, 1) + ProgressFactory( + uuid=program_uuid, + completed=self._extract_titles(program, 0, 1) ) ) - self.assertEqual(meter.completed_programs, [program_id]) + self.assertEqual(meter.completed_programs, [program_uuid]) - @mock.patch(UTILS_MODULE + '.get_completed_courses') - def test_nonstandard_run_mode_completion(self, mock_get_completed_courses): + @mock.patch(UTILS_MODULE + '.ProgramProgressMeter.completed_course_runs', new_callable=mock.PropertyMock) + def test_nonverified_course_run_completion(self, mock_completed_course_runs, mock_get_programs): """ - A valid run mode isn't necessarily verified. Verify that a program can + Course runs aren't necessarily of type verified. Verify that a program can still be completed when this is the case. """ - course_id = 'org/course/run' + course_run_key = generate_course_run_key() data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode( - course_key=course_id, - mode_slug=MODES.honor - ), - factories.RunMode(), + ProgramFactory( + courses=[ + CourseFactory(course_runs=[ + CourseRunFactory(key=course_run_key, type='honor'), + CourseRunFactory(), ]), ] ), - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode()]), - ] - ), + ProgramFactory(), ] - self._mock_programs_api(data) + mock_get_programs.return_value = data - self._create_enrollments(course_id) - mock_get_completed_courses.return_value = [ - {'course_id': course_id, 'mode': MODES.honor}, + self._create_enrollments(course_run_key) + mock_completed_course_runs.return_value = [ + {'course_run_id': course_run_key, 'type': MODES.honor}, ] - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) - program, program_id = data[0], data[0]['id'] + program, program_uuid = data[0], data[0]['uuid'] self._assert_progress( meter, - factories.Progress(id=program_id, completed=self._extract_names(program, 0)) + ProgressFactory(uuid=program_uuid, completed=self._extract_titles(program, 0)) ) - self.assertEqual(meter.completed_programs, [program_id]) + self.assertEqual(meter.completed_programs, [program_uuid]) - @mock.patch(UTILS_MODULE + '.get_completed_courses') - def test_completed_programs(self, mock_get_completed_courses): + @mock.patch(UTILS_MODULE + '.ProgramProgressMeter.completed_course_runs', new_callable=mock.PropertyMock) + def test_completed_programs(self, mock_completed_course_runs, mock_get_programs): """Verify that completed programs are correctly identified.""" - program_count, course_code_count, run_mode_count = 3, 2, 2 - data = [ - factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[factories.RunMode() for _ in range(run_mode_count)]) - for _ in range(course_code_count) - ] - ) - for _ in range(program_count) - ] - self._mock_programs_api(data) + data = ProgramFactory.create_batch(3) + mock_get_programs.return_value = data - program_ids = [] - course_ids = [] + program_uuids = [] + course_run_keys = [] for program in data: - program_ids.append(program['id']) + program_uuids.append(program['uuid']) - for course_code in program['course_codes']: - for run_mode in course_code['run_modes']: - course_ids.append(run_mode['course_key']) + for course in program['courses']: + for course_run in course['course_runs']: + course_run_keys.append(course_run['key']) # Verify that no programs are complete. - meter = utils.ProgramProgressMeter(self.user) + meter = ProgramProgressMeter(self.user) self.assertEqual(meter.completed_programs, []) - # "Complete" all programs. - self._create_enrollments(*course_ids) - mock_get_completed_courses.return_value = [ - {'course_id': course_id, 'mode': MODES.verified} for course_id in course_ids + # Complete all programs. + self._create_enrollments(*course_run_keys) + mock_completed_course_runs.return_value = [ + {'course_run_id': course_run_key, 'type': MODES.verified} + for course_run_key in course_run_keys ] # Verify that all programs are complete. - meter = utils.ProgramProgressMeter(self.user) - self.assertEqual(meter.completed_programs, program_ids) + meter = ProgramProgressMeter(self.user) + self.assertEqual(meter.completed_programs, program_uuids) + + @mock.patch(UTILS_MODULE + '.certificate_api.get_certificates_for_user') + def test_completed_course_runs(self, mock_get_certificates_for_user, _mock_get_programs): + """ + Verify that the method can find course run certificates when not mocked out. + """ + def make_certificate_result(**kwargs): + """Helper to create dummy results from the certificates API.""" + result = { + 'username': 'dummy-username', + 'course_key': 'dummy-course', + 'type': 'dummy-type', + 'status': 'dummy-status', + 'download_url': 'http://www.example.com/cert.pdf', + 'grade': '0.98', + 'created': '2015-07-31T00:00:00Z', + 'modified': '2015-07-31T00:00:00Z', + } + result.update(**kwargs) + return result + + mock_get_certificates_for_user.return_value = [ + make_certificate_result(status='downloadable', type='verified', course_key='downloadable-course'), + make_certificate_result(status='generating', type='honor', course_key='generating-course'), + make_certificate_result(status='unknown', course_key='unknown-course'), + ] + + meter = ProgramProgressMeter(self.user) + self.assertEqual( + meter.completed_course_runs, + [ + {'course_run_id': 'downloadable-course', 'type': 'verified'}, + {'course_run_id': 'generating-course', 'type': 'honor'}, + ] + ) + mock_get_certificates_for_user.assert_called_with(self.user.username) @ddt.ddt @override_settings(ECOMMERCE_PUBLIC_URL_ROOT=ECOMMERCE_URL_ROOT) @skip_unless_lms -@mock.patch(UTILS_MODULE + '.get_run_marketing_url', mock.Mock(return_value=MARKETING_URL)) -class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): +class TestProgramDataExtender(ModuleStoreTestCase): """Tests of the program data extender utility class.""" maxDiff = None sku = 'abc123' @@ -645,47 +427,47 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.user = UserFactory() self.client.login(username=self.user.username, password=self.password) - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - - self.course = CourseFactory() + 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 = self.update_course(self.course, self.user.id) # pylint: disable=no-member - self.organization = factories.Organization() - self.run_mode = factories.RunMode(course_key=unicode(self.course.id)) # pylint: disable=no-member - self.course_code = factories.CourseCode(run_modes=[self.run_mode]) - self.program = factories.Program( - organizations=[self.organization], - course_codes=[self.course_code] - ) + organization = OrganizationFactory() + course_run = CourseRunFactory(key=unicode(self.course.id)) # pylint: disable=no-member + course = CourseFactory(course_runs=[course_run]) + program = ProgramFactory(authoring_organizations=[organization], courses=[course]) + + self.program = munge_catalog_program(program) + self.course_code = self.program['course_codes'][0] + self.run_mode = self.course_code['run_modes'][0] def _assert_supplemented(self, actual, **kwargs): """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( - certificate_url=None, - course_image_url=course_overview.course_image_url, - course_key=unicode(self.course.id), # pylint: disable=no-member - course_url=reverse('course_root', args=[self.course.id]), # pylint: disable=no-member - end_date=self.course.end.replace(tzinfo=utc), - enrollment_open_date=strftime_localized(utils.DEFAULT_ENROLLMENT_START_DATE, 'SHORT_DATE'), - is_course_ended=self.course.end < datetime.datetime.now(utc), - is_enrolled=False, - is_enrollment_open=True, - marketing_url=MARKETING_URL, - start_date=self.course.start.replace(tzinfo=utc), - upgrade_url=None, - advertised_start=None - ), + { + 'certificate_url': None, + 'course_image_url': course_overview.course_image_url, + 'course_key': unicode(self.course.id), # pylint: disable=no-member + 'course_url': reverse('course_root', args=[self.course.id]), # pylint: disable=no-member + 'end_date': self.course.end.replace(tzinfo=utc), + 'enrollment_open_date': strftime_localized(DEFAULT_ENROLLMENT_START_DATE, 'SHORT_DATE'), + 'is_course_ended': self.course.end < datetime.datetime.now(utc), + 'is_enrolled': False, + 'is_enrollment_open': True, + 'marketing_url': self.run_mode['marketing_url'], + 'mode_slug': 'verified', + 'start_date': self.course.start.replace(tzinfo=utc), + 'upgrade_url': None, + 'advertised_start': None, + }, **kwargs ) - course_code = factories.CourseCode(display_name=self.course_code['display_name'], run_modes=[run_mode]) - expected = copy.deepcopy(self.program) - expected['course_codes'] = [course_code] - self.assertEqual(actual, expected) + self.course_code['run_modes'] = [run_mode] + self.program['course_codes'] = [self.course_code] + + self.assertEqual(actual, self.program) @ddt.data( (False, None, False), @@ -711,7 +493,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): if is_enrolled: CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=enrolled_mode) # pylint: disable=no-member - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented( data, @@ -731,7 +513,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): is_active=False, ) - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented(data) @@ -746,7 +528,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=MODES.audit) # pylint: disable=no-member - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented(data, is_enrolled=True, upgrade_url=None) @@ -764,7 +546,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented( data, @@ -780,7 +562,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.course.enrollment_end = datetime.datetime.now(utc) - datetime.timedelta(days=1) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented( data, @@ -796,7 +578,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): mock_get_cert_data.return_value = {'uuid': test_uuid} if is_uuid_available else {} mock_html_certs_enabled.return_value = True - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() expected_url = reverse( 'certificates:render_cert_by_uuid', @@ -810,7 +592,7 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): self.course.end = datetime.datetime.now(utc) + datetime.timedelta(days=days_offset) self.course = self.update_course(self.course, self.user.id) # pylint: disable=no-member - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self._assert_supplemented(data) @@ -824,14 +606,14 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): 'logo': mock_image } - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self.assertEqual(data['organizations'][0].get('img'), mock_logo_url) @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 """ mock_get_organization_by_short_name.return_value = None - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self.assertEqual(data['organizations'][0].get('img'), None) @mock.patch(UTILS_MODULE + '.get_organization_by_short_name') @@ -841,5 +623,5 @@ class TestProgramDataExtender(ProgramsApiConfigMixin, ModuleStoreTestCase): but the logo is not available """ mock_get_organization_by_short_name.return_value = {'logo': None} - data = utils.ProgramDataExtender(self.program, self.user).extend() + data = ProgramDataExtender(self.program, self.user).extend() self.assertEqual(data['organizations'][0].get('img'), None) diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 0a43e139d5..ce404c5c03 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -1,24 +1,20 @@ # -*- coding: utf-8 -*- """Helper functions for working with Programs.""" +from collections import defaultdict import datetime from urlparse import urljoin from django.conf import settings from django.core.urlresolvers import reverse -from django.utils.text import slugify +from django.utils.functional import cached_property from opaque_keys.edx.keys import CourseKey from pytz import utc from course_modes.models import CourseMode 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 as get_catalog_programs, - munge_catalog_program, - get_run_marketing_url, -) +from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.lib.edx_api_utils import get_edx_api_data from student.models import CourseEnrollment from util.date_utils import strftime_localized @@ -29,78 +25,6 @@ from util.organizations_helpers import get_organization_by_short_name DEFAULT_ENROLLMENT_START_DATE = datetime.datetime(1900, 1, 1, tzinfo=utc) -def get_programs(user, program_id=None, use_catalog=False): - """Given a user, get programs from the Programs service. - - Returned value is cached depending on user permissions. Staff users making requests - against Programs will receive unpublished programs, while regular users will only receive - published programs. - - Arguments: - user (User): The user to authenticate as when requesting programs. - - Keyword Arguments: - program_id (int): Identifies a specific program for which to retrieve data. - - Returns: - list of dict, representing programs returned by the Programs service. - dict, if a specific program is requested. - """ - - if use_catalog: - programs = [munge_catalog_program(program) for program in get_catalog_programs(user)] - else: - programs_config = ProgramsApiConfig.current() - - # Bypass caching for staff users, who may be creating Programs and want - # to see them displayed immediately. - cache_key = programs_config.CACHE_KEY if programs_config.is_cache_enabled and not user.is_staff else None - - programs = get_edx_api_data(programs_config, user, 'programs', resource_id=program_id, cache_key=cache_key) - - # Mix in munged MicroMasters data from the catalog. - if not program_id: - programs += [ - munge_catalog_program(micromaster) for micromaster in get_catalog_programs(user, type='MicroMasters') - ] - - return programs - - -def get_programs_by_run(programs, enrollments): - """Intersect programs and enrollments. - - Builds a dictionary of program dict lists keyed by course ID. The resulting dictionary - is suitable for use in applications where programs must be filtered by the course - runs they contain (e.g., student dashboard). - - Arguments: - programs (list): Containing dictionaries representing programs. - enrollments (list): Enrollments from which course IDs to key on can be extracted. - - Returns: - tuple, dict of programs keyed by course ID and list of course IDs themselves - """ - programs_by_run = {} - # enrollment.course_id is really a course key (╯ಠ_ಠ）╯︵ ┻━┻ - course_ids = [unicode(e.course_id) for e in enrollments] - - for program in programs: - for course_code in program['course_codes']: - for run in course_code['run_modes']: - run_id = run['course_key'] - if run_id in course_ids: - program_list = programs_by_run.setdefault(run_id, list()) - if program not in program_list: - program_list.append(program) - - # Sort programs by name for consistent presentation. - for program_list in programs_by_run.itervalues(): - program_list.sort(key=lambda p: p['name']) - - return programs_by_run, course_ids - - def get_program_marketing_url(programs_config): """Build a URL to be used when linking to program details on a marketing site.""" return urljoin(settings.MKTG_URLS.get('ROOT'), programs_config.marketing_path).rstrip('/') @@ -118,31 +42,21 @@ def attach_program_detail_url(programs): list, containing extended program dicts """ for program in programs: - base = reverse('program_details_view', kwargs={'program_id': program['id']}).rstrip('/') - slug = slugify(program['name']) - program['detail_url'] = '{base}/{slug}'.format(base=base, slug=slug) + program['detail_url'] = reverse('program_details_view', kwargs={'program_uuid': program['uuid']}) return programs -def get_completed_courses(student): +def munge_progress_map(progress_map): """ - Determine which courses have been completed by the user. - - Args: - student: - User object representing the student - - Returns: - iterable of dicts with structure {'course_id': course_key, 'mode': cert_type} + Temporary utility for making progress maps look like they were built using + data from the deprecated programs service. + Clean up of this debt is tracked by ECOM-4418. """ - all_certs = certificate_api.get_certificates_for_user(student.username) - return [ - {'course_id': unicode(cert['course_key']), 'mode': cert['type']} - for cert in all_certs - if certificate_api.is_passing_status(cert['status']) - ] + progress_map['id'] = progress_map.pop('uuid') + + return progress_map class ProgramProgressMeter(object): @@ -154,33 +68,62 @@ class ProgramProgressMeter(object): Keyword Arguments: enrollments (list): List of the user's enrollments. """ - def __init__(self, user, enrollments=None, use_catalog=False): + def __init__(self, user, enrollments=None): self.user = user - self.enrollments = enrollments - self.course_ids = None - self.course_certs = None - self.use_catalog = use_catalog - self.programs = attach_program_detail_url(get_programs(self.user, use_catalog=use_catalog)) + self.enrollments = enrollments or list(CourseEnrollment.enrollments_for_user(self.user)) + self.enrollments.sort(key=lambda e: e.created, reverse=True) - def engaged_programs(self, by_run=False): + # enrollment.course_id is really a CourseKey (╯ಠ_ಠ）╯︵ ┻━┻ + self.course_run_ids = [unicode(e.course_id) for e in self.enrollments] + + self.programs = attach_program_detail_url(get_programs()) + + def invert_programs(self): + """Intersect programs and enrollments. + + Builds a dictionary of program dict lists keyed by course run ID. The + resulting dictionary is suitable in applications where programs must be + filtered by the course runs they contain (e.g., the student dashboard). + + Returns: + defaultdict, programs keyed by course run ID + """ + inverted_programs = defaultdict(list) + + for program in self.programs: + for course in program['courses']: + for course_run in course['course_runs']: + course_run_id = course_run['key'] + if course_run_id in self.course_run_ids: + program_list = inverted_programs[course_run_id] + if program not in program_list: + program_list.append(program) + + # Sort programs by title for consistent presentation. + for program_list in inverted_programs.itervalues(): + program_list.sort(key=lambda p: p['title']) + + return inverted_programs + + @cached_property + def engaged_programs(self): """Derive a list of programs in which the given user is engaged. Returns: - list of program dicts, ordered by most recent enrollment, - or dict of programs, keyed by course ID. + list of program dicts, ordered by most recent enrollment """ - self.enrollments = self.enrollments or list(CourseEnrollment.enrollments_for_user(self.user)) - self.enrollments.sort(key=lambda e: e.created, reverse=True) - - programs_by_run, self.course_ids = get_programs_by_run(self.programs, self.enrollments) - - if by_run: - return programs_by_run + inverted_programs = self.invert_programs() programs = [] - for course_id in self.course_ids: - for program in programs_by_run.get(course_id, []): + # Remember that these course run ids are derived from a list of + # enrollments sorted from most recent to least recent. Iterating + # over the values in inverted_programs alone won't yield a program + # ordering consistent with the user's enrollments. + for course_run_id in self.course_run_ids: + for program in inverted_programs[course_run_id]: + # Dicts aren't a hashable type, so we can't use a set. Sets also + # aren't ordered, which is important here. if program not in programs: programs.append(program) @@ -195,21 +138,23 @@ class ProgramProgressMeter(object): towards completing a program. """ progress = [] - for program in self.engaged_programs(): + for program in self.engaged_programs: completed, in_progress, not_started = [], [], [] - for course_code in program['course_codes']: - name = course_code['display_name'] + for course in program['courses']: + # TODO: What are these titles used for? If they're not used by + # the front-end, pass integer counts instead. + title = course['title'] - if self._is_course_code_complete(course_code): - completed.append(name) - elif self._is_course_code_in_progress(course_code): - in_progress.append(name) + if self._is_course_complete(course): + completed.append(title) + elif self._is_course_in_progress(course): + in_progress.append(title) else: - not_started.append(name) + not_started.append(title) progress.append({ - 'id': program['id'], + 'uuid': program['uuid'], 'completed': completed, 'in_progress': in_progress, 'not_started': not_started, @@ -222,75 +167,94 @@ class ProgramProgressMeter(object): """Identify programs completed by the student. Returns: - list of int, each the ID of a completed program. + list of UUIDs, each identifying a completed program. """ - return [program['id'] for program in self.programs if self._is_program_complete(program)] + return [program['uuid'] for program in self.programs if self._is_program_complete(program)] def _is_program_complete(self, program): """Check if a user has completed a program. - A program is completed if the user has completed all nested course codes. + A program is completed if the user has completed all nested courses. Arguments: program (dict): Representing the program whose completion to assess. Returns: - bool, whether the program is complete. + bool, indicating whether the program is complete. """ - return all(self._is_course_code_complete(course_code) for course_code in program['course_codes']) + return all(self._is_course_complete(course) for course in program['courses']) - def _is_course_code_complete(self, course_code): - """Check if a user has completed a course code. + def _is_course_complete(self, course): + """Check if a user has completed a course. - A course code is completed if the user has earned a certificate - in the right mode for any nested run. + A course is completed if the user has earned a certificate for any of + the nested course runs. Arguments: - course_code (dict): Containing nested run modes. + course (dict): Containing nested course runs. Returns: - bool, whether the course code is complete. + bool, indicating whether the course is complete. """ - self.course_certs = self.course_certs or get_completed_courses(self.user) - return any(self._parse(run_mode) in self.course_certs for run_mode in course_code['run_modes']) - def _is_course_code_in_progress(self, course_code): - """Check if a user is in the process of completing a course code. + def reshape(course_run): + """ + Modify the structure of a course run dict to facilitate comparison + with course run certificates. + """ + return { + 'course_run_id': course_run['key'], + # A course run's type is assumed to indicate which mode must be + # completed in order for the run to count towards program completion. + # This supports the same flexible program construction allowed by the + # old programs service (e.g., completion of an old honor-only run may + # count towards completion of a course in a program). This may change + # in the future to make use of the more rigid set of "applicable seat + # types" associated with each program type in the catalog. + 'type': course_run['type'], + } - A user is in the process of completing a course code if they're - enrolled in the course. + return any(reshape(course_run) in self.completed_course_runs for course_run in course['course_runs']) + + @property + def completed_course_runs(self): + """ + Determine which course runs have been completed by the user. + + Returns: + list of dicts, each representing a course run certificate + """ + course_run_certificates = certificate_api.get_certificates_for_user(self.user.username) + return [ + {'course_run_id': unicode(certificate['course_key']), 'type': certificate['type']} + for certificate in course_run_certificates + if certificate_api.is_passing_status(certificate['status']) + ] + + def _is_course_in_progress(self, course): + """Check if a user is in the process of completing a course. + + A user is considered to be in the process of completing a course if + they're enrolled in any of the nested course runs. Arguments: - course_code (dict): Containing nested run modes. + course (dict): Containing nested course runs. Returns: - bool, whether the course code is in progress. + bool, indicating whether the course is in progress. """ - return any(run_mode['course_key'] in self.course_ids for run_mode in course_code['run_modes']) - - def _parse(self, run_mode): - """Modify the structure of a run mode dict. - - Arguments: - run_mode (dict): With `course_key` and `mode_slug` keys. - - Returns: - dict, with `course_id` and `mode` keys. - """ - parsed = { - 'course_id': run_mode['course_key'], - 'mode': run_mode['mode_slug'], - } - - return parsed + return any(course_run['key'] in self.course_run_ids for course_run in course['course_runs']) # pylint: disable=missing-docstring class ProgramDataExtender(object): - """Utility for extending program course codes with CourseOverview and CourseEnrollment data. + """ + Utility for extending program course codes with CourseOverview and + CourseEnrollment data. Arguments: - program_data (dict): Representation of a program. + program_data (dict): Representation of a program. Note that this dict must + be formatted as if it was returned by the deprecated program service. user (User): The user whose enrollments to inspect. """ def __init__(self, program_data, user): @@ -370,9 +334,6 @@ class ProgramDataExtender(object): enrollment_end = self.course_overview.enrollment_end or datetime.datetime.max.replace(tzinfo=utc) run_mode['is_enrollment_open'] = self.enrollment_start <= datetime.datetime.now(utc) < enrollment_end - def _attach_run_mode_marketing_url(self, run_mode): - run_mode['marketing_url'] = get_run_marketing_url(self.course_key, self.user) - def _attach_run_mode_start_date(self, run_mode): run_mode['start_date'] = self.course_overview.start diff --git a/openedx/core/lib/tests/test_edx_api_utils.py b/openedx/core/lib/tests/test_edx_api_utils.py index 6b0e29c4f0..27db7d151e 100644 --- a/openedx/core/lib/tests/test_edx_api_utils.py +++ b/openedx/core/lib/tests/test_edx_api_utils.py @@ -1,4 +1,5 @@ """Tests covering edX API utilities.""" +# pylint: disable=missing-docstring import json from django.core.cache import cache @@ -9,9 +10,11 @@ from nose.plugins.attrib import attr from edx_oauth2_provider.tests.factories import ClientFactory from provider.constants import CONFIDENTIAL -from openedx.core.djangoapps.commerce.utils import ecommerce_api_client -from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin +from openedx.core.djangoapps.catalog.models import CatalogIntegration +from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin +from openedx.core.djangoapps.catalog.utils import create_catalog_api_client +from openedx.core.djangoapps.credentials.models import CredentialsApiConfig +from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.lib.edx_api_utils import get_edx_api_data from student.tests.factories import UserFactory @@ -24,24 +27,21 @@ TEST_API_URL = 'http://www-internal.example.com/api' @skip_unless_lms @attr(shard=2) @httpretty.activate -class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): +class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, CacheIsolationTestCase): """Tests for edX API data retrieval utility.""" - ENABLED_CACHES = ['default'] def setUp(self): super(TestGetEdxApiData, self).setUp() self.user = UserFactory() - ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) cache.clear() - def _mock_programs_api(self, responses, url=None): - """Helper for mocking out Programs API URLs.""" - self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.') + def _mock_catalog_api(self, responses, url=None): + self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Catalog API calls.') - url = url if url else ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/' + url = url if url else CatalogIntegration.current().internal_api_url.strip('/') + '/programs/' httpretty.register_uri(httpretty.GET, url, responses=responses) @@ -51,7 +51,8 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): def test_get_unpaginated_data(self): """Verify that unpaginated data can be retrieved.""" - program_config = self.create_programs_config() + catalog_integration = self.create_catalog_integration() + api = create_catalog_api_client(self.user, catalog_integration) expected_collection = ['some', 'test', 'data'] data = { @@ -59,22 +60,27 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): 'results': expected_collection, } - self._mock_programs_api( + self._mock_catalog_api( [httpretty.Response(body=json.dumps(data), content_type='application/json')] ) - actual_collection = get_edx_api_data(program_config, self.user, 'programs') - self.assertEqual(actual_collection, expected_collection) + with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init: + actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) + + # Verify that the helper function didn't initialize its own client. + self.assertFalse(mock_init.called) + self.assertEqual(actual_collection, expected_collection) # Verify the API was actually hit (not the cache) self._assert_num_requests(1) def test_get_paginated_data(self): """Verify that paginated data can be retrieved.""" - program_config = self.create_programs_config() + catalog_integration = self.create_catalog_integration() + api = create_catalog_api_client(self.user, catalog_integration) expected_collection = ['some', 'test', 'data'] - url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/?page={}' + url = CatalogIntegration.current().internal_api_url.strip('/') + '/programs/?page={}' responses = [] for page, record in enumerate(expected_collection, start=1): @@ -88,38 +94,40 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): httpretty.Response(body=body, content_type='application/json') ) - self._mock_programs_api(responses) + self._mock_catalog_api(responses) - actual_collection = get_edx_api_data(program_config, self.user, 'programs') + actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) self.assertEqual(actual_collection, expected_collection) self._assert_num_requests(len(expected_collection)) def test_get_specific_resource(self): """Verify that a specific resource can be retrieved.""" - program_config = self.create_programs_config() + catalog_integration = self.create_catalog_integration() + api = create_catalog_api_client(self.user, catalog_integration) resource_id = 1 url = '{api_root}/programs/{resource_id}/'.format( - api_root=ProgramsApiConfig.current().internal_api_url.strip('/'), + api_root=CatalogIntegration.current().internal_api_url.strip('/'), resource_id=resource_id, ) expected_resource = {'key': 'value'} - self._mock_programs_api( + self._mock_catalog_api( [httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')], url=url ) - actual_resource = get_edx_api_data(program_config, self.user, 'programs', resource_id=resource_id) + actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id) self.assertEqual(actual_resource, expected_resource) self._assert_num_requests(1) def test_cache_utilization(self): """Verify that when enabled, the cache is used.""" - program_config = self.create_programs_config(cache_ttl=5) + catalog_integration = self.create_catalog_integration(cache_ttl=5) + api = create_catalog_api_client(self.user, catalog_integration) expected_collection = ['some', 'test', 'data'] data = { @@ -127,35 +135,37 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): 'results': expected_collection, } - self._mock_programs_api( + self._mock_catalog_api( [httpretty.Response(body=json.dumps(data), content_type='application/json')], ) resource_id = 1 url = '{api_root}/programs/{resource_id}/'.format( - api_root=ProgramsApiConfig.current().internal_api_url.strip('/'), + api_root=CatalogIntegration.current().internal_api_url.strip('/'), resource_id=resource_id, ) expected_resource = {'key': 'value'} - self._mock_programs_api( + self._mock_catalog_api( [httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')], url=url ) - cache_key = ProgramsApiConfig.current().CACHE_KEY + cache_key = CatalogIntegration.current().CACHE_KEY # Warm up the cache. - get_edx_api_data(program_config, self.user, 'programs', cache_key=cache_key) - get_edx_api_data(program_config, self.user, 'programs', resource_id=resource_id, cache_key=cache_key) + get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key) + get_edx_api_data( + catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key + ) # Hit the cache. - actual_collection = get_edx_api_data(program_config, self.user, 'programs', cache_key=cache_key) + actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key) self.assertEqual(actual_collection, expected_collection) actual_resource = get_edx_api_data( - program_config, self.user, 'programs', resource_id=resource_id, cache_key=cache_key + catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key ) self.assertEqual(actual_resource, expected_resource) @@ -165,9 +175,9 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): @mock.patch(UTILITY_MODULE + '.log.warning') def test_api_config_disabled(self, mock_warning): """Verify that no data is retrieved if the provided config model is disabled.""" - program_config = self.create_programs_config(enabled=False) + catalog_integration = self.create_catalog_integration(enabled=False) - actual = get_edx_api_data(program_config, self.user, 'programs') + actual = get_edx_api_data(catalog_integration, self.user, 'programs') self.assertTrue(mock_warning.called) self.assertEqual(actual, []) @@ -178,9 +188,9 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): """Verify that an exception is logged when the API client fails to initialize.""" mock_init.side_effect = Exception - program_config = self.create_programs_config() + catalog_integration = self.create_catalog_integration() - actual = get_edx_api_data(program_config, self.user, 'programs') + actual = get_edx_api_data(catalog_integration, self.user, 'programs') self.assertTrue(mock_exception.called) self.assertEqual(actual, []) @@ -188,24 +198,24 @@ class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase): @mock.patch(UTILITY_MODULE + '.log.exception') def test_data_retrieval_failure(self, mock_exception): """Verify that an exception is logged when data can't be retrieved.""" - program_config = self.create_programs_config() + catalog_integration = self.create_catalog_integration() + api = create_catalog_api_client(self.user, catalog_integration) - self._mock_programs_api( + self._mock_catalog_api( [httpretty.Response(body='clunk', content_type='application/json', status_code=500)] ) - actual = get_edx_api_data(program_config, self.user, 'programs') + actual = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) self.assertTrue(mock_exception.called) self.assertEqual(actual, []) - @override_settings(ECOMMERCE_API_URL=TEST_API_URL) - def test_client_passed(self): - """ Verify that when API client is passed edx_rest_api_client is not - used. - """ - program_config = self.create_programs_config() - api = ecommerce_api_client(self.user) + def test_api_client_not_provided(self): + """Verify that an API client doesn't need to be provided.""" + ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) + + credentials_api_config = self.create_credentials_config() + with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init: - get_edx_api_data(program_config, self.user, 'orders', api=api) - self.assertFalse(mock_init.called) + get_edx_api_data(credentials_api_config, self.user, 'credentials') + self.assertTrue(mock_init.called) diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index 26ef3de820..15e5e997a2 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -111,11 +111,6 @@ class Env(object): 'log': BOK_CHOY_LOG_DIR / "bok_choy_ecommerce.log", }, - 'programs': { - 'port': 8090, - 'log': BOK_CHOY_LOG_DIR / "bok_choy_programs.log", - }, - 'catalog': { 'port': 8091, 'log': BOK_CHOY_LOG_DIR / "bok_choy_catalog.log",