diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index f306c9b180..4824e6c1be 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -56,8 +56,6 @@ class TestProgramListing( def _create_course_and_enroll(self, student, org, course, run): """ Creates a course and associated enrollment. - - TODO: Use CourseEnrollmentFactory to avoid course creation. """ course_location = locator.CourseLocator(org, course, run) course = CourseFactory.create( diff --git a/lms/djangoapps/learner_dashboard/views.py b/lms/djangoapps/learner_dashboard/views.py index 5de432cf68..3a14cd5b24 100644 --- a/lms/djangoapps/learner_dashboard/views.py +++ b/lms/djangoapps/learner_dashboard/views.py @@ -11,7 +11,6 @@ from edxmako.shortcuts import render_to_response 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 student.views import get_course_enrollments @login_required @@ -22,8 +21,7 @@ def view_programs(request): if not show_program_listing: raise Http404 - enrollments = list(get_course_enrollments(request.user, None, [])) - meter = utils.ProgramProgressMeter(request.user, enrollments) + meter = utils.ProgramProgressMeter(request.user) programs = meter.engaged_programs # TODO: Pull 'xseries' string from configuration model. diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index 8e8227cfde..b80a8111b5 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 openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.utils import get_user_credentials from openedx.core.djangoapps.programs.models import ProgramsApiConfig -from openedx.core.djangoapps.programs.utils import get_completed_courses +from openedx.core.djangoapps.programs.utils import ProgramProgressMeter from openedx.core.lib.token_utils import get_id_token @@ -35,21 +35,20 @@ def get_api_client(api_config, student): return EdxRestApiClient(api_config.internal_api_url, jwt=id_token) -def get_completed_programs(client, course_certificates): +def get_completed_programs(student): """ Given a set of completed courses, determine which programs are completed. Args: - client: - programs API client (EdxRestApiClient) - course_certificates: - iterable of dicts with structure {'course_id': course_key, 'mode': cert_type} + student (User): Representing the student whose completed programs to check for. Returns: list of program ids """ - return client.programs.complete.post({'completed_courses': course_certificates})['program_ids'] + meter = ProgramProgressMeter(student) + + return meter.completed_programs def get_awarded_certificate_programs(student): @@ -147,29 +146,9 @@ def award_program_certificates(self, username): # Don't retry for this case - just conclude the task. return - # Fetch the set of all course runs for which the user has earned a - # certificate. - course_certs = get_completed_courses(student) - if not course_certs: - # Highly unlikely, since at present the only trigger for this task - # is the earning of a new course certificate. However, it could be - # that the transaction in which a course certificate was awarded - # was subsequently rolled back, which could lead to an empty result - # here, so we'll at least log that this happened before exiting. - # - # If this task is ever updated to support revocation of program - # certs, this branch should be removed, since it could make sense - # in that case to call this task for a user without any (valid) - # course certs. - LOGGER.warning('Task award_program_certificates was called for user %s with no completed courses', username) - return - - # Invoke the Programs API completion check endpoint to identify any - # programs that are satisfied by these course completions. - programs_client = get_api_client(config, student) - program_ids = get_completed_programs(programs_client, course_certs) + program_ids = get_completed_programs(student) if not program_ids: - # Again, no reason to continue beyond this point unless/until this + # No reason to continue beyond this point unless/until this # task gets updated to support revocation of program certs. LOGGER.info('Task award_program_certificates was called for user %s with no completed programs', username) return 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 e3bafd27e8..cbd75a13da 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -1,26 +1,31 @@ """ Tests for programs celery tasks. """ - -import ddt -import httpretty import json -import mock import unittest 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.credentials.tests.mixins import CredentialsApiConfigMixin +from openedx.core.djangoapps.programs.tests import factories from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.programs.tasks.v1 import tasks +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.tests.factories import UserFactory TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks' +UTILS_MODULE = 'openedx.core.djangoapps.programs.utils' @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -48,31 +53,65 @@ class GetApiClientTestCase(TestCase, ProgramsApiConfigMixin): self.assertEqual(api_client._store['session'].auth.token, 'test-token') # pylint: disable=protected-access +@httpretty.activate @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class GetCompletedProgramsTestCase(TestCase): +class GetCompletedProgramsTestCase(ProgramsApiConfigMixin, CacheIsolationTestCase): """ Test the get_completed_programs function """ + ENABLED_CACHES = ['default'] - @httpretty.activate - def test_get_completed_programs(self): + def setUp(self): + super(GetCompletedProgramsTestCase, self).setUp() + + self.user = UserFactory() + self.programs_config = self.create_programs_config(cache_ttl=5) + + ClientFactory(name=self.programs_config.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) + + cache.clear() + + 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 = self.programs_config.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): """ - Ensure the correct API call gets made + Verify that completed programs are found, using the cache when possible. """ - test_client = EdxRestApiClient('http://test-server', jwt='test-token') - httpretty.register_uri( - httpretty.POST, - 'http://test-server/programs/complete/', - body='{"program_ids": [1, 2, 3]}', - content_type='application/json', - ) - payload = [ - {'course_id': 'test-course-1', 'mode': 'verified'}, - {'course_id': 'test-course-2', 'mode': 'prof-ed'}, + course_id = 'org/course/run' + data = [ + factories.Program( + organizations=[factories.Organization()], + course_codes=[ + factories.CourseCode(run_modes=[ + factories.RunMode(course_key=course_id), + ]), + ] + ), ] - result = tasks.get_completed_programs(test_client, payload) - self.assertEqual(json.loads(httpretty.last_request().body), {'completed_courses': payload}) - self.assertEqual(result, [1, 2, 3]) + self._mock_programs_api(data) + + mock_get_completed_courses.return_value = [ + {'course_id': course_id, 'mode': MODES.verified} + ] + + for _ in range(2): + result = tasks.get_completed_programs(self.user) + self.assertEqual(result, [data[0]['id']]) + + # Verify that only one request to programs was made (i.e., the cache was hit). + self._assert_num_requests(1) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -150,7 +189,6 @@ class AwardProgramCertificateTestCase(TestCase): @mock.patch(TASKS_MODULE + '.award_program_certificate') @mock.patch(TASKS_MODULE + '.get_awarded_certificate_programs') @mock.patch(TASKS_MODULE + '.get_completed_programs') -@mock.patch(TASKS_MODULE + '.get_completed_courses') @override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username') class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, CredentialsApiConfigMixin): """ @@ -169,7 +207,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_completion_check( self, - mock_get_completed_courses, mock_get_completed_programs, mock_get_awarded_certificate_programs, # pylint: disable=unused-argument mock_award_program_certificate, # pylint: disable=unused-argument @@ -178,18 +215,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent Checks that the Programs API is used correctly to determine completed programs. """ - completed_courses = [ - {'course_id': 'course-1', 'type': 'verified'}, - {'course_id': 'course-2', 'type': 'prof-ed'}, - ] - mock_get_completed_courses.return_value = completed_courses - tasks.award_program_certificates.delay(self.student.username).get() - - self.assertEqual( - mock_get_completed_programs.call_args[0][1], - completed_courses - ) + mock_get_completed_programs.assert_called_once_with(self.student) @ddt.data( ([1], [2, 3]), @@ -201,7 +228,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent self, already_awarded_program_ids, expected_awarded_program_ids, - mock_get_completed_courses, # pylint: disable=unused-argument mock_get_completed_programs, mock_get_awarded_certificate_programs, mock_award_program_certificate, @@ -252,30 +278,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent for mock_helper in mock_helpers: self.assertFalse(mock_helper.called) - def test_abort_if_no_completed_courses( - self, - mock_get_completed_courses, - mock_get_completed_programs, - mock_get_awarded_certificate_programs, - mock_award_program_certificate, - ): - """ - Checks that the task will be aborted without further action if the - student does not have any completed courses, but that a warning is - logged. - """ - mock_get_completed_courses.return_value = [] - with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning: - tasks.award_program_certificates.delay(self.student.username).get() - self.assertTrue(mock_warning.called) - self.assertTrue(mock_get_completed_courses.called) - self.assertFalse(mock_get_completed_programs.called) - self.assertFalse(mock_get_awarded_certificate_programs.called) - self.assertFalse(mock_award_program_certificate.called) - def test_abort_if_no_completed_programs( self, - mock_get_completed_courses, mock_get_completed_programs, mock_get_awarded_certificate_programs, mock_award_program_certificate, @@ -286,7 +290,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent """ mock_get_completed_programs.return_value = [] tasks.award_program_certificates.delay(self.student.username).get() - self.assertTrue(mock_get_completed_courses.called) self.assertTrue(mock_get_completed_programs.called) self.assertFalse(mock_get_awarded_certificate_programs.called) self.assertFalse(mock_award_program_certificate.called) @@ -312,7 +315,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_continue_awarding_certs_if_error( self, - mock_get_completed_courses, # pylint: disable=unused-argument mock_get_completed_programs, mock_get_awarded_certificate_programs, mock_award_program_certificate, @@ -336,24 +338,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent mock_info.assert_any_call(mock.ANY, 1, self.student.username) mock_info.assert_any_call(mock.ANY, 2, self.student.username) - def test_retry_on_certificates_api_errors( - self, - mock_get_completed_courses, - *_mock_helpers # pylint: disable=unused-argument - ): - """ - Ensures that any otherwise-unhandled errors that arise while trying - to get existing course certificates (e.g. network issues or other - transient API errors) will cause the task to be failed and queued for - retry. - """ - mock_get_completed_courses.side_effect = self._make_side_effect([Exception('boom'), None]) - tasks.award_program_certificates.delay(self.student.username).get() - self.assertEqual(mock_get_completed_courses.call_count, 2) - def test_retry_on_programs_api_errors( self, - mock_get_completed_courses, # pylint: disable=unused-argument mock_get_completed_programs, *_mock_helpers # pylint: disable=unused-argument ): @@ -369,7 +355,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent def test_retry_on_credentials_api_errors( self, - mock_get_completed_courses, # pylint: disable=unused-argument mock_get_completed_programs, mock_get_awarded_certificate_programs, mock_award_program_certificate, diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index acaa5d5115..dee459c686 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -284,8 +284,9 @@ class GetCompletedCoursesTestCase(TestCase): ]) -@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @attr('shard_2') +@httpretty.activate +@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): """Tests of the program progress utility class.""" def setUp(self): @@ -307,7 +308,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): def _create_enrollments(self, *course_ids): """Variadic helper used to create course enrollments.""" - return [CourseEnrollmentFactory(user=self.user, course_id=c) for c in course_ids] + for course_id in course_ids: + CourseEnrollmentFactory(user=self.user, course_id=course_id) def _assert_progress(self, meter, *progresses): """Variadic helper used to verify progress calculations.""" @@ -317,7 +319,6 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): """Construct a list containing the display names of the indicated course codes.""" return [program['course_codes'][cc]['display_name'] for cc in course_codes] - @httpretty.activate def test_no_enrollments(self): """Verify behavior when programs exist, but no relevant enrollments do.""" data = [ @@ -330,23 +331,23 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ] self._mock_programs_api(data) - meter = utils.ProgramProgressMeter(self.user, []) + meter = utils.ProgramProgressMeter(self.user) self.assertEqual(meter.engaged_programs, []) self._assert_progress(meter) + self.assertEqual(meter.completed_programs, []) - @httpretty.activate def test_no_programs(self): """Verify behavior when enrollments exist, but no matching programs do.""" self._mock_programs_api([]) - enrollments = self._create_enrollments('org/course/run') - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments('org/course/run') + meter = utils.ProgramProgressMeter(self.user) self.assertEqual(meter.engaged_programs, []) self._assert_progress(meter) + self.assertEqual(meter.completed_programs, []) - @httpretty.activate def test_single_program_engagement(self): """ Verify that correct program is returned when the user has a single enrollment @@ -371,8 +372,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ] self._mock_programs_api(data) - enrollments = self._create_enrollments(course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments(course_id) + meter = utils.ProgramProgressMeter(self.user) program = data[0] self.assertEqual(meter.engaged_programs, [program]) @@ -383,8 +384,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): in_progress=self._extract_names(program, 0) ) ) + self.assertEqual(meter.completed_programs, []) - @httpretty.activate def test_mutiple_program_engagement(self): """ Verify that correct programs are returned in the correct order when the user @@ -417,8 +418,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ] self._mock_programs_api(data) - enrollments = self._create_enrollments(second_course_id, first_course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments(second_course_id, first_course_id) + meter = utils.ProgramProgressMeter(self.user) programs = data[:2] self.assertEqual(meter.engaged_programs, programs) @@ -427,8 +428,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): 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)) ) + self.assertEqual(meter.completed_programs, []) - @httpretty.activate def test_shared_enrollment_engagement(self): """ Verify that correct programs are returned when the user has a single enrollment @@ -470,8 +471,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): self._mock_programs_api(data) # Enrollment for the shared course ID created last (most recently). - enrollments = self._create_enrollments(solo_course_id, shared_course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments(solo_course_id, shared_course_id) + meter = utils.ProgramProgressMeter(self.user) programs = data[:3] self.assertEqual(meter.engaged_programs, programs) @@ -481,8 +482,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): 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)) ) + self.assertEqual(meter.completed_programs, []) - @httpretty.activate @mock.patch(UTILS_MODULE + '.get_completed_courses') def test_simulate_progress(self, mock_get_completed_courses): """Simulate the entirety of a user's progress through a program.""" @@ -499,16 +500,23 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ]), ] ), + factories.Program( + organizations=[factories.Organization()], + course_codes=[ + factories.CourseCode(run_modes=[factories.RunMode()]), + ] + ), ] self._mock_programs_api(data) # No enrollments, no program engaged. - meter = utils.ProgramProgressMeter(self.user, []) + meter = utils.ProgramProgressMeter(self.user) self._assert_progress(meter) + self.assertEqual(meter.completed_programs, []) # One enrollment, program engaged. - enrollments = self._create_enrollments(first_course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments(first_course_id) + meter = utils.ProgramProgressMeter(self.user) program, program_id = data[0], data[0]['id'] self._assert_progress( meter, @@ -518,10 +526,11 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): not_started=self._extract_names(program, 1) ) ) + self.assertEqual(meter.completed_programs, []) # Two enrollments, program in progress. - enrollments += self._create_enrollments(second_course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) + self._create_enrollments(second_course_id) + meter = utils.ProgramProgressMeter(self.user) self._assert_progress( meter, factories.Progress( @@ -529,11 +538,13 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): in_progress=self._extract_names(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}, ] + meter = utils.ProgramProgressMeter(self.user) self._assert_progress( meter, factories.Progress( @@ -542,12 +553,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): in_progress=self._extract_names(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}, ] + meter = utils.ProgramProgressMeter(self.user) self._assert_progress( meter, factories.Progress( @@ -556,12 +569,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): in_progress=self._extract_names(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}, ] + meter = utils.ProgramProgressMeter(self.user) self._assert_progress( meter, factories.Progress( @@ -569,12 +584,12 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): completed=self._extract_names(program, 0, 1) ) ) + self.assertEqual(meter.completed_programs, [program_id]) - @httpretty.activate @mock.patch(UTILS_MODULE + '.get_completed_courses') def test_nonstandard_run_mode_completion(self, mock_get_completed_courses): """ - A valid run mode isn't necessarily verified. Verify that the program can + A valid run mode isn't necessarily verified. Verify that a program can still be completed when this is the case. """ course_id = 'org/course/run' @@ -587,24 +602,70 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): course_key=course_id, mode_slug=MODES.honor ), + factories.RunMode(), ]), ] ), + factories.Program( + organizations=[factories.Organization()], + course_codes=[ + factories.CourseCode(run_modes=[factories.RunMode()]), + ] + ), ] self._mock_programs_api(data) - enrollments = self._create_enrollments(course_id) - meter = utils.ProgramProgressMeter(self.user, enrollments) - + self._create_enrollments(course_id) mock_get_completed_courses.return_value = [ {'course_id': course_id, 'mode': MODES.honor}, ] + meter = utils.ProgramProgressMeter(self.user) - program = data[0] + program, program_id = data[0], data[0]['id'] self._assert_progress( meter, - factories.Progress(id=program['id'], completed=self._extract_names(program, 0)) + factories.Progress(id=program_id, completed=self._extract_names(program, 0)) ) + self.assertEqual(meter.completed_programs, [program_id]) + + @mock.patch(UTILS_MODULE + '.get_completed_courses') + def test_completed_programs(self, mock_get_completed_courses): + """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) + + program_ids = [] + course_ids = [] + for program in data: + program_ids.append(program['id']) + + for course_code in program['course_codes']: + for run_mode in course_code['run_modes']: + course_ids.append(run_mode['course_key']) + + # Verify that no programs are complete. + meter = utils.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 + ] + + # Verify that all programs are complete. + meter = utils.ProgramProgressMeter(self.user) + self.assertEqual(meter.completed_programs, program_ids) @ddt.ddt diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index cf58feeecc..6ca9a842df 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -5,6 +5,7 @@ import logging from django.core.urlresolvers import reverse from django.utils import timezone +from django.utils.functional import cached_property from opaque_keys.edx.keys import CourseKey import pytz @@ -170,29 +171,27 @@ class ProgramProgressMeter(object): Arguments: user (User): The user for which to find programs. - enrollments (list): The user's active enrollments. """ - def __init__(self, user, enrollments): + def __init__(self, user): self.user = user + self.course_ids = None - enrollments = sorted(enrollments, key=lambda e: e.created, reverse=True) - # enrollment.course_id is really a course key ಠ_ಠ - self.course_ids = [unicode(e.course_id) for e in enrollments] + self.programs = get_programs(self.user) + self.course_certs = get_completed_courses(self.user) - self.engaged_programs = self._find_engaged_programs(self.user) - self.course_certs = None - - def _find_engaged_programs(self, user): + @cached_property + def engaged_programs(self): """Derive a list of programs in which the given user is engaged. - Arguments: - user (User): The user for which to find engaged programs. - Returns: list of program dicts, ordered by most recent enrollment. """ - programs = get_programs(user) - flattened = flatten_programs(programs, self.course_ids) + enrollments = CourseEnrollment.enrollments_for_user(self.user) + enrollments = sorted(enrollments, key=lambda e: e.created, reverse=True) + # enrollment.course_id is really a course key ಠ_ಠ + self.course_ids = [unicode(e.course_id) for e in enrollments] + + flattened = flatten_programs(self.programs, self.course_ids) engaged_programs = [] for course_id in self.course_ids: @@ -210,8 +209,6 @@ class ProgramProgressMeter(object): list of dict, each containing information about a user's progress towards completing a program. """ - self.course_certs = get_completed_courses(self.user) - progress = [] for program in self.engaged_programs: completed, in_progress, not_started = [], [], [] @@ -219,9 +216,9 @@ class ProgramProgressMeter(object): for course_code in program['course_codes']: name = course_code['display_name'] - if self._is_complete(course_code): + if self._is_course_code_complete(course_code): completed.append(name) - elif self._is_in_progress(course_code): + elif self._is_course_code_in_progress(course_code): in_progress.append(name) else: not_started.append(name) @@ -235,11 +232,33 @@ class ProgramProgressMeter(object): return progress - def _is_complete(self, course_code): + @property + def completed_programs(self): + """Identify programs completed by the student. + + Returns: + list of int, each the ID of a completed program. + """ + return [program['id'] 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. + + Arguments: + program (dict): Representing the program whose completion to assess. + + Returns: + bool, whether the program is complete. + """ + return all(self._is_course_code_complete(course_code) for course_code in program['course_codes']) + + def _is_course_code_complete(self, course_code): """Check if a user has completed a course code. - A course code qualifies as completed if the user has earned a - certificate in the right mode for any nested run. + A course code is completed if the user has earned a certificate + in the right mode for any nested run. Arguments: course_code (dict): Containing nested run modes. @@ -247,12 +266,9 @@ class ProgramProgressMeter(object): Returns: bool, whether the course code is complete. """ - return any([ - self._parse(run_mode) in self.course_certs - for run_mode in course_code['run_modes'] - ]) + return any(self._parse(run_mode) in self.course_certs for run_mode in course_code['run_modes']) - def _is_in_progress(self, course_code): + def _is_course_code_in_progress(self, course_code): """Check if a user is in the process of completing a course code. A user is in the process of completing a course code if they're @@ -264,10 +280,7 @@ class ProgramProgressMeter(object): Returns: bool, whether the course code is in progress. """ - return any([ - run_mode['course_key'] in self.course_ids - for run_mode in course_code['run_modes'] - ]) + 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.