From f00059f28c45b55a264fbec232e5d31a89b7ac40 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Wed, 29 Mar 2017 14:10:09 -0400 Subject: [PATCH] Collect data needed for program progress sidebar This includes a representation of the user's progress towards completing each course in the program and a list of any relevant course and/or program certificates the user has earned. ECOM-7386 --- .../learner_dashboard/tests/test_programs.py | 6 +- lms/djangoapps/learner_dashboard/views.py | 8 +- .../program_details_2017.html | 2 + .../credentials/tests/test_utils.py | 14 +-- openedx/core/djangoapps/credentials/utils.py | 24 ++++-- .../djangoapps/programs/tasks/v1/tasks.py | 4 +- .../programs/tasks/v1/tests/test_tasks.py | 8 +- .../djangoapps/programs/tests/test_utils.py | 85 ++++++++++++++++++- openedx/core/djangoapps/programs/utils.py | 48 ++++++++++- 9 files changed, 172 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index d9bd9652c0..da54c65e18 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -184,9 +184,9 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar expected_url = reverse('program_details_view', kwargs={'program_uuid': expected_program['uuid']}) self.assertEqual(actual_program['detail_url'], expected_url) - @mock.patch(CREDENTIALS_UTILS_MODULE + '.get_user_credentials') + @mock.patch(CREDENTIALS_UTILS_MODULE + '.get_credentials') @mock.patch(CREDENTIALS_UTILS_MODULE + '.get_programs') - def test_certificates_listed(self, mock_get_programs, mock_get_user_credentials, __): + def test_certificates_listed(self, mock_get_programs, mock_get_credentials, __): """ Verify that the response contains accurate certificate data when certificates are available. """ @@ -209,7 +209,7 @@ class TestProgramListing(ProgramsApiConfigMixin, CredentialsApiConfigMixin, Shar ) credentials_data = sorted([first_credential, second_credential], key=self.credential_sort_key) - mock_get_user_credentials.return_value = credentials_data + mock_get_credentials.return_value = credentials_data response = self.client.get(self.url) actual = self.load_serialized_data(response, 'certificatesData') diff --git a/lms/djangoapps/learner_dashboard/views.py b/lms/djangoapps/learner_dashboard/views.py index 7f4e43b74c..22b9e1f91f 100644 --- a/lms/djangoapps/learner_dashboard/views.py +++ b/lms/djangoapps/learner_dashboard/views.py @@ -14,6 +14,7 @@ from openedx.core.djangoapps.programs.utils import ( get_program_marketing_url, ProgramProgressMeter, ProgramDataExtender, + get_certificates, ) from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences @@ -76,12 +77,15 @@ def program_details(request, program_uuid): } if waffle.switch_is_active('new_program_progress'): - course_progress = meter.progress(programs=[program_data], count_only=False)[0] + course_data = meter.progress(programs=[program_data], count_only=False)[0] + certificate_data = get_certificates(request.user, program_data) + program_data.pop('courses') context.update({ 'program_data': program_data, - 'course_progress': course_progress, + 'course_data': course_data, + 'certificate_data': certificate_data, }) return render_to_response('learner_dashboard/program_details_2017.html', context) diff --git a/lms/templates/learner_dashboard/program_details_2017.html b/lms/templates/learner_dashboard/program_details_2017.html index 0a2dafe1f5..03f5dc6da8 100644 --- a/lms/templates/learner_dashboard/program_details_2017.html +++ b/lms/templates/learner_dashboard/program_details_2017.html @@ -15,6 +15,8 @@ from openedx.core.djangolib.js_utils import ( <%static:require_module module_name="js/learner_dashboard/program_details_factory_2017" class_name="ProgramDetailsFactory2017"> ProgramDetailsFactory2017({ programData: ${program_data | n, dump_js_escaped_json}, + courseData: ${course_data | n, dump_js_escaped_json}, + certificateData: ${certificate_data | n, dump_js_escaped_json}, urls: ${urls | n, dump_js_escaped_json}, userPreferences: ${user_preferences | n, dump_js_escaped_json}, }); diff --git a/openedx/core/djangoapps/credentials/tests/test_utils.py b/openedx/core/djangoapps/credentials/tests/test_utils.py index 096cd6d763..17c1812091 100644 --- a/openedx/core/djangoapps/credentials/tests/test_utils.py +++ b/openedx/core/djangoapps/credentials/tests/test_utils.py @@ -12,7 +12,7 @@ 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 ( - get_user_credentials, + get_credentials, get_user_program_credentials, get_programs_credentials, get_programs_for_credentials @@ -57,25 +57,25 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, ] @httpretty.activate - def test_get_user_credentials(self): + def test_get_credentials(self): """Verify user credentials data can be retrieve.""" self.create_credentials_config() self.mock_credentials_api(self.user) - actual = get_user_credentials(self.user) + actual = get_credentials(self.user) self.assertEqual(actual, self.CREDENTIALS_API_RESPONSE['results']) @httpretty.activate - def test_get_user_credentials_caching(self): + def test_get_credentials_caching(self): """Verify that when enabled, the cache is used for non-staff users.""" self.create_credentials_config(cache_ttl=1) self.mock_credentials_api(self.user) # Warm up the cache. - get_user_credentials(self.user) + get_credentials(self.user) # Hit the cache. - get_user_credentials(self.user) + get_credentials(self.user) # Verify only one request was made. self.assertEqual(len(httpretty.httpretty.latest_requests), 1) @@ -84,7 +84,7 @@ class TestCredentialsRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, # Hit the Credentials API twice. for _ in range(2): - get_user_credentials(staff_user) + get_credentials(staff_user) # Verify that three requests have been made (one for student, two for staff). self.assertEqual(len(httpretty.httpretty.latest_requests), 3) diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index e22aadb6d8..9e381492d6 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -10,25 +10,35 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data log = logging.getLogger(__name__) -def get_user_credentials(user): - """Given a user, get credentials earned from the Credentials service. +def get_credentials(user, program_uuid=None): + """ + Given a user, get credentials earned from the credentials service. + Arguments: user (User): The user to authenticate as when requesting credentials. + + Keyword Arguments: + program_uuid (str): UUID of the program whose credential to retrieve. + Returns: list of dict, representing credentials returned by the Credentials service. """ credential_configuration = CredentialsApiConfig.current() - user_query = {'status': 'awarded', 'username': user.username} + + querystring = {'username': user.username, 'status': 'awarded'} + + if program_uuid: + querystring['program_uuid'] = program_uuid + # Bypass caching for staff users, who may be generating credentials and # want to see them displayed immediately. use_cache = credential_configuration.is_cache_enabled and not user.is_staff cache_key = credential_configuration.CACHE_KEY + '.' + user.username if use_cache else None - credentials = get_edx_api_data( - credential_configuration, user, 'credentials', querystring=user_query, cache_key=cache_key + return get_edx_api_data( + credential_configuration, user, 'credentials', querystring=querystring, cache_key=cache_key ) - return credentials def get_programs_for_credentials(programs_credentials): @@ -69,7 +79,7 @@ def get_user_program_credentials(user): log.debug('Display of certificates for programs is disabled.') return programs_credentials_data - credentials = get_user_credentials(user) + credentials = get_credentials(user) if not credentials: log.info('No credential earned by the given user.') return programs_credentials_data diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index 7aec4e7bf0..4fd3a5a66e 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tasks.py @@ -10,7 +10,7 @@ from edx_rest_api_client.client import EdxRestApiClient from provider.oauth2.models import Client from openedx.core.djangoapps.credentials.models import CredentialsApiConfig -from openedx.core.djangoapps.credentials.utils import get_user_credentials +from openedx.core.djangoapps.credentials.utils import get_credentials from openedx.core.djangoapps.programs.utils import ProgramProgressMeter from openedx.core.lib.token_utils import JwtBuilder @@ -83,7 +83,7 @@ def get_certified_programs(student): """ certified_programs = [] - for credential in get_user_credentials(student): + for credential in get_credentials(student): if 'program_uuid' in credential['credential']: certified_programs.append(credential['credential']['program_uuid']) return certified_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 7903a715aa..ba3e82cf9e 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -69,19 +69,19 @@ class GetAwardedCertificateProgramsTestCase(TestCase): result.update(**kwargs) return result - @mock.patch(TASKS_MODULE + '.get_user_credentials') - def test_get_certified_programs(self, mock_get_user_credentials): + @mock.patch(TASKS_MODULE + '.get_credentials') + def test_get_certified_programs(self, mock_get_credentials): """ Ensure the API is called and results handled correctly. """ student = UserFactory(username='test-username') - mock_get_user_credentials.return_value = [ + mock_get_credentials.return_value = [ self.make_credential_result(status='awarded', credential={'program_uuid': 1}), self.make_credential_result(status='awarded', credential={'course_id': 2}), ] result = tasks.get_certified_programs(student) - self.assertEqual(mock_get_user_credentials.call_args[0], (student, )) + self.assertEqual(mock_get_credentials.call_args[0], (student, )) self.assertEqual(result, [1]) diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index 8bcddb05c5..d7087f35a5 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -22,7 +22,11 @@ from openedx.core.djangoapps.catalog.tests.factories import ( ) from openedx.core.djangoapps.programs.tests.factories import ProgressFactory from openedx.core.djangoapps.programs.utils import ( - DEFAULT_ENROLLMENT_START_DATE, ProgramProgressMeter, ProgramDataExtender, ProgramMarketingDataExtender + DEFAULT_ENROLLMENT_START_DATE, + ProgramProgressMeter, + ProgramDataExtender, + ProgramMarketingDataExtender, + get_certificates, ) from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory, CourseEnrollmentFactory @@ -591,6 +595,85 @@ class TestProgramDataExtender(ModuleStoreTestCase): self._assert_supplemented(data, certificate_url=expected_url) +@skip_unless_lms +@mock.patch(UTILS_MODULE + '.get_credentials') +class TestGetCertificates(TestCase): + """ + Tests of the function used to get certificates associated with a program. + """ + def setUp(self): + super(TestGetCertificates, self).setUp() + + self.user = UserFactory() + self.program = ProgramFactory() + self.course_certificate_url = 'fake-course-certificate-url' + self.program_certificate_url = 'fake-program-certificate-url' + + def test_get_certificates(self, mock_get_credentials): + """ + Verify course and program certificates are found when present. Only one + course run certificate should be returned for each course when the user + has earned certificates in multiple runs of the same course. + """ + expected = [] + for course in self.program['courses']: + # Give all course runs a certificate URL, but only expect one to come + # back. This verifies the break in the function under test that ensures + # only one certificate per course comes back. + for index, course_run in enumerate(course['course_runs']): + course_run['certificate_url'] = self.course_certificate_url + + if index == 0: + expected.append({ + 'type': 'course', + 'title': course_run['title'], + 'url': self.course_certificate_url, + }) + + expected.append({ + 'type': 'program', + 'title': self.program['title'], + 'url': self.program_certificate_url, + }) + + mock_get_credentials.return_value = [{ + 'certificate_url': self.program_certificate_url + }] + + certificates = get_certificates(self.user, self.program) + self.assertEqual(certificates, expected) + + def test_course_run_certificates_missing(self, mock_get_credentials): + """ + Verify an empty list is returned when course run certificates are missing, + and that no attempt is made to retrieve program certificates. + """ + certificates = get_certificates(self.user, self.program) + self.assertEqual(certificates, []) + self.assertFalse(mock_get_credentials.called) + + def test_program_certificate_missing(self, mock_get_credentials): + """ + Verify that the function can handle a missing program certificate. + """ + expected = [] + for course in self.program['courses']: + for index, course_run in enumerate(course['course_runs']): + course_run['certificate_url'] = self.course_certificate_url + + if index == 0: + expected.append({ + 'type': 'course', + 'title': course_run['title'], + 'url': self.course_certificate_url, + }) + + mock_get_credentials.return_value = [] + + certificates = get_certificates(self.user, self.program) + self.assertEqual(certificates, expected) + + @ddt.ddt @override_settings(ECOMMERCE_PUBLIC_URL_ROOT=ECOMMERCE_URL_ROOT) @skip_unless_lms diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 2fe993a1d1..5935d421ee 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -18,6 +18,7 @@ from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.courseware.access import has_access from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.credentials.utils import get_credentials from student.models import CourseEnrollment from util.date_utils import strftime_localized from xmodule.modulestore.django import modulestore @@ -216,7 +217,7 @@ class ProgramProgressMeter(object): return any(reshape(course_run) in self.completed_course_runs for course_run in course['course_runs']) - @property + @cached_property def completed_course_runs(self): """ Determine which course runs have been completed by the user. @@ -345,6 +346,51 @@ class ProgramDataExtender(object): run_mode['upgrade_url'] = None +def get_certificates(user, extended_program): + """ + Find certificates a user has earned related to a given program. + + Arguments: + user (User): The user whose enrollments to inspect. + extended_program (dict): The program for which to locate certificates. + This is expected to be an "extended" program whose course runs already + have certificate URLs attached. + + Returns: + list: Contains dicts representing course run and program certificates the + given user has earned which are associated with the given program. + """ + certificates = [] + + for course in extended_program['courses']: + for course_run in course['course_runs']: + url = course_run.get('certificate_url') + if url: + certificates.append({ + 'type': 'course', + 'title': course_run['title'], + 'url': url, + }) + + # We only want one certificate per course to be returned. + break + + # A user can only have earned a program certificate if they've earned certificates + # in associated course runs. If they haven't earned any course run certificates, + # they can't have earned a program certificate, and we can save a network call + # to the credentials service. + if certificates: + program_credentials = get_credentials(user, program_uuid=extended_program['uuid']) + if program_credentials: + certificates.append({ + 'type': 'program', + 'title': extended_program['title'], + 'url': program_credentials[0]['certificate_url'], + }) + + return certificates + + # pylint: disable=missing-docstring class ProgramMarketingDataExtender(ProgramDataExtender): """