Skip to content

E
edx-platform
Commit 6d7938fd by Renzo Lucioni
Options

Use cached data to find completed programs on LMS 

These changes improve the performance of the LMS' program certification task. They also reduce strain on the programs service, especially when attempting to award program certificates to large numbers of students. ECOM-4490.
parent 6bd35fd6
Showing with 203 additions and 169 deletions
lms/djangoapps/learner_dashboard/tests/test_programs.py
...@@ -56,8 +56,6 @@ class TestProgramListing( ...@@ -56,8 +56,6 @@ class TestProgramListing(
def _create_course_and_enroll(self, student, org, course, run): def _create_course_and_enroll(self, student, org, course, run):
""" """
Creates a course and associated enrollment. Creates a course and associated enrollment.
TODO: Use CourseEnrollmentFactory to avoid course creation.
""" """
course_location = locator.CourseLocator(org, course, run) course_location = locator.CourseLocator(org, course, run)
course = CourseFactory.create( course = CourseFactory.create(
......
lms/djangoapps/learner_dashboard/views.py
...@@ -11,7 +11,6 @@ from edxmako.shortcuts import render_to_response ...@@ -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.credentials.utils import get_programs_credentials
from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.models import ProgramsApiConfig
from openedx.core.djangoapps.programs import utils from openedx.core.djangoapps.programs import utils
from student.views import get_course_enrollments
@login_required @login_required
...@@ -22,8 +21,7 @@ def view_programs(request): ...@@ -22,8 +21,7 @@ def view_programs(request):
if not show_program_listing: if not show_program_listing:
raise Http404 raise Http404
enrollments = list(get_course_enrollments(request.user, None, [])) meter = utils.ProgramProgressMeter(request.user)
meter = utils.ProgramProgressMeter(request.user, enrollments)
programs = meter.engaged_programs programs = meter.engaged_programs
# TODO: Pull 'xseries' string from configuration model. # TODO: Pull 'xseries' string from configuration model.
......
openedx/core/djangoapps/programs/tasks/v1/tasks.py
...@@ -10,7 +10,7 @@ from edx_rest_api_client.client import EdxRestApiClient ...@@ -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.models import CredentialsApiConfig
from openedx.core.djangoapps.credentials.utils import get_user_credentials from openedx.core.djangoapps.credentials.utils import get_user_credentials
from openedx.core.djangoapps.programs.models import ProgramsApiConfig 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 from openedx.core.lib.token_utils import get_id_token
...@@ -35,21 +35,20 @@ def get_api_client(api_config, student): ...@@ -35,21 +35,20 @@ def get_api_client(api_config, student):
return EdxRestApiClient(api_config.internal_api_url, jwt=id_token) 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. Given a set of completed courses, determine which programs are completed.
Args: Args:
client: student (User): Representing the student whose completed programs to check for.
programs API client (EdxRestApiClient)
course_certificates:
iterable of dicts with structure {'course_id': course_key, 'mode': cert_type}
Returns: Returns:
list of program ids 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): def get_awarded_certificate_programs(student):
...@@ -147,29 +146,9 @@ def award_program_certificates(self, username): ...@@ -147,29 +146,9 @@ def award_program_certificates(self, username):
# Don't retry for this case - just conclude the task. # Don't retry for this case - just conclude the task.
return return
# Fetch the set of all course runs for which the user has earned a program_ids = get_completed_programs(student)
# 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)
if not program_ids: 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. # 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) LOGGER.info('Task award_program_certificates was called for user %s with no completed programs', username)
return return
......
openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py
""" """
Tests for programs celery tasks. Tests for programs celery tasks.
""" """
import ddt
import httpretty
import json import json
import mock
import unittest import unittest
from celery.exceptions import MaxRetriesExceededError from celery.exceptions import MaxRetriesExceededError
import ddt
from django.conf import settings from django.conf import settings
from django.core.cache import cache
from django.test import override_settings, TestCase from django.test import override_settings, TestCase
from edx_rest_api_client.client import EdxRestApiClient from edx_rest_api_client.client import EdxRestApiClient
from edx_oauth2_provider.tests.factories import ClientFactory 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.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.tests.mixins import ProgramsApiConfigMixin
from openedx.core.djangoapps.programs.tasks.v1 import tasks from openedx.core.djangoapps.programs.tasks.v1 import tasks
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks' 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') @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
...@@ -48,31 +53,65 @@ class GetApiClientTestCase(TestCase, ProgramsApiConfigMixin): ...@@ -48,31 +53,65 @@ class GetApiClientTestCase(TestCase, ProgramsApiConfigMixin):
self.assertEqual(api_client._store['session'].auth.token, 'test-token') # pylint: disable=protected-access 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') @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 Test the get_completed_programs function
""" """
ENABLED_CACHES = ['default']
@httpretty.activate def setUp(self):
def test_get_completed_programs(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') course_id = 'org/course/run'
httpretty.register_uri( data = [
httpretty.POST, factories.Program(
'http://test-server/programs/complete/', organizations=[factories.Organization()],
body='{"program_ids": [1, 2, 3]}', course_codes=[
content_type='application/json', factories.CourseCode(run_modes=[
) factories.RunMode(course_key=course_id),
payload = [ ]),
{'course_id': 'test-course-1', 'mode': 'verified'}, ]
{'course_id': 'test-course-2', 'mode': 'prof-ed'}, ),
]
self._mock_programs_api(data)
mock_get_completed_courses.return_value = [
{'course_id': course_id, 'mode': MODES.verified}
] ]
result = tasks.get_completed_programs(test_client, payload)
self.assertEqual(json.loads(httpretty.last_request().body), {'completed_courses': payload}) for _ in range(2):
self.assertEqual(result, [1, 2, 3]) 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') @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
...@@ -150,7 +189,6 @@ class AwardProgramCertificateTestCase(TestCase): ...@@ -150,7 +189,6 @@ class AwardProgramCertificateTestCase(TestCase):
@mock.patch(TASKS_MODULE + '.award_program_certificate') @mock.patch(TASKS_MODULE + '.award_program_certificate')
@mock.patch(TASKS_MODULE + '.get_awarded_certificate_programs') @mock.patch(TASKS_MODULE + '.get_awarded_certificate_programs')
@mock.patch(TASKS_MODULE + '.get_completed_programs') @mock.patch(TASKS_MODULE + '.get_completed_programs')
@mock.patch(TASKS_MODULE + '.get_completed_courses')
@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username') @override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, CredentialsApiConfigMixin): class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, CredentialsApiConfigMixin):
""" """
...@@ -169,7 +207,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -169,7 +207,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
def test_completion_check( def test_completion_check(
self, self,
mock_get_completed_courses,
mock_get_completed_programs, mock_get_completed_programs,
mock_get_awarded_certificate_programs, # pylint: disable=unused-argument mock_get_awarded_certificate_programs, # pylint: disable=unused-argument
mock_award_program_certificate, # pylint: disable=unused-argument mock_award_program_certificate, # pylint: disable=unused-argument
...@@ -178,18 +215,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -178,18 +215,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
Checks that the Programs API is used correctly to determine completed Checks that the Programs API is used correctly to determine completed
programs. 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() tasks.award_program_certificates.delay(self.student.username).get()
mock_get_completed_programs.assert_called_once_with(self.student)
self.assertEqual(
mock_get_completed_programs.call_args[0][1],
completed_courses
)
@ddt.data( @ddt.data(
([1], [2, 3]), ([1], [2, 3]),
...@@ -201,7 +228,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -201,7 +228,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
self, self,
already_awarded_program_ids, already_awarded_program_ids,
expected_awarded_program_ids, expected_awarded_program_ids,
mock_get_completed_courses, # pylint: disable=unused-argument
mock_get_completed_programs, mock_get_completed_programs,
mock_get_awarded_certificate_programs, mock_get_awarded_certificate_programs,
mock_award_program_certificate, mock_award_program_certificate,
...@@ -252,30 +278,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -252,30 +278,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
for mock_helper in mock_helpers: for mock_helper in mock_helpers:
self.assertFalse(mock_helper.called) 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( def test_abort_if_no_completed_programs(
self, self,
mock_get_completed_courses,
mock_get_completed_programs, mock_get_completed_programs,
mock_get_awarded_certificate_programs, mock_get_awarded_certificate_programs,
mock_award_program_certificate, mock_award_program_certificate,
...@@ -286,7 +290,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -286,7 +290,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
""" """
mock_get_completed_programs.return_value = [] mock_get_completed_programs.return_value = []
tasks.award_program_certificates.delay(self.student.username).get() tasks.award_program_certificates.delay(self.student.username).get()
self.assertTrue(mock_get_completed_courses.called)
self.assertTrue(mock_get_completed_programs.called) self.assertTrue(mock_get_completed_programs.called)
self.assertFalse(mock_get_awarded_certificate_programs.called) self.assertFalse(mock_get_awarded_certificate_programs.called)
self.assertFalse(mock_award_program_certificate.called) self.assertFalse(mock_award_program_certificate.called)
...@@ -312,7 +315,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -312,7 +315,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
def test_continue_awarding_certs_if_error( def test_continue_awarding_certs_if_error(
self, self,
mock_get_completed_courses, # pylint: disable=unused-argument
mock_get_completed_programs, mock_get_completed_programs,
mock_get_awarded_certificate_programs, mock_get_awarded_certificate_programs,
mock_award_program_certificate, mock_award_program_certificate,
...@@ -336,24 +338,8 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -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, 1, self.student.username)
mock_info.assert_any_call(mock.ANY, 2, 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( def test_retry_on_programs_api_errors(
self, self,
mock_get_completed_courses, # pylint: disable=unused-argument
mock_get_completed_programs, mock_get_completed_programs,
*_mock_helpers # pylint: disable=unused-argument *_mock_helpers # pylint: disable=unused-argument
): ):
...@@ -369,7 +355,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent ...@@ -369,7 +355,6 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
def test_retry_on_credentials_api_errors( def test_retry_on_credentials_api_errors(
self, self,
mock_get_completed_courses, # pylint: disable=unused-argument
mock_get_completed_programs, mock_get_completed_programs,
mock_get_awarded_certificate_programs, mock_get_awarded_certificate_programs,
mock_award_program_certificate, mock_award_program_certificate,
......
openedx/core/djangoapps/programs/tests/test_utils.py
...@@ -284,8 +284,9 @@ class GetCompletedCoursesTestCase(TestCase): ...@@ -284,8 +284,9 @@ class GetCompletedCoursesTestCase(TestCase):
]) ])
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@attr('shard_2') @attr('shard_2')
@httpretty.activate
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
"""Tests of the program progress utility class.""" """Tests of the program progress utility class."""
def setUp(self): def setUp(self):
...@@ -307,7 +308,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -307,7 +308,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
def _create_enrollments(self, *course_ids): def _create_enrollments(self, *course_ids):
"""Variadic helper used to create course enrollments.""" """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): def _assert_progress(self, meter, *progresses):
"""Variadic helper used to verify progress calculations.""" """Variadic helper used to verify progress calculations."""
...@@ -317,7 +319,6 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -317,7 +319,6 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
"""Construct a list containing the display names of the indicated 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] return [program['course_codes'][cc]['display_name'] for cc in course_codes]
@httpretty.activate
def test_no_enrollments(self): def test_no_enrollments(self):
"""Verify behavior when programs exist, but no relevant enrollments do.""" """Verify behavior when programs exist, but no relevant enrollments do."""
data = [ data = [
...@@ -330,23 +331,23 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -330,23 +331,23 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
] ]
self._mock_programs_api(data) self._mock_programs_api(data)
meter = utils.ProgramProgressMeter(self.user, []) meter = utils.ProgramProgressMeter(self.user)
self.assertEqual(meter.engaged_programs, []) self.assertEqual(meter.engaged_programs, [])
self._assert_progress(meter) self._assert_progress(meter)
self.assertEqual(meter.completed_programs, [])
@httpretty.activate
def test_no_programs(self): def test_no_programs(self):
"""Verify behavior when enrollments exist, but no matching programs do.""" """Verify behavior when enrollments exist, but no matching programs do."""
self._mock_programs_api([]) self._mock_programs_api([])
enrollments = self._create_enrollments('org/course/run') self._create_enrollments('org/course/run')
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
self.assertEqual(meter.engaged_programs, []) self.assertEqual(meter.engaged_programs, [])
self._assert_progress(meter) self._assert_progress(meter)
self.assertEqual(meter.completed_programs, [])
@httpretty.activate
def test_single_program_engagement(self): def test_single_program_engagement(self):
""" """
Verify that correct program is returned when the user has a single enrollment Verify that correct program is returned when the user has a single enrollment
...@@ -371,8 +372,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -371,8 +372,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
] ]
self._mock_programs_api(data) self._mock_programs_api(data)
enrollments = self._create_enrollments(course_id) self._create_enrollments(course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
program = data[0] program = data[0]
self.assertEqual(meter.engaged_programs, [program]) self.assertEqual(meter.engaged_programs, [program])
...@@ -383,8 +384,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -383,8 +384,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
in_progress=self._extract_names(program, 0) in_progress=self._extract_names(program, 0)
) )
) )
self.assertEqual(meter.completed_programs, [])
@httpretty.activate
def test_mutiple_program_engagement(self): def test_mutiple_program_engagement(self):
""" """
Verify that correct programs are returned in the correct order when the user Verify that correct programs are returned in the correct order when the user
...@@ -417,8 +418,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -417,8 +418,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
] ]
self._mock_programs_api(data) self._mock_programs_api(data)
enrollments = self._create_enrollments(second_course_id, first_course_id) self._create_enrollments(second_course_id, first_course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
programs = data[:2] programs = data[:2]
self.assertEqual(meter.engaged_programs, programs) self.assertEqual(meter.engaged_programs, programs)
...@@ -427,8 +428,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -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[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[1]['id'], in_progress=self._extract_names(programs[1], 0))
) )
self.assertEqual(meter.completed_programs, [])
@httpretty.activate
def test_shared_enrollment_engagement(self): def test_shared_enrollment_engagement(self):
""" """
Verify that correct programs are returned when the user has a single enrollment Verify that correct programs are returned when the user has a single enrollment
...@@ -470,8 +471,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -470,8 +471,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
self._mock_programs_api(data) self._mock_programs_api(data)
# Enrollment for the shared course ID created last (most recently). # Enrollment for the shared course ID created last (most recently).
enrollments = self._create_enrollments(solo_course_id, shared_course_id) self._create_enrollments(solo_course_id, shared_course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
programs = data[:3] programs = data[:3]
self.assertEqual(meter.engaged_programs, programs) self.assertEqual(meter.engaged_programs, programs)
...@@ -481,8 +482,8 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -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[1]['id'], in_progress=self._extract_names(programs[1], 0)),
factories.Progress(id=programs[2]['id'], in_progress=self._extract_names(programs[2], 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') @mock.patch(UTILS_MODULE + '.get_completed_courses')
def test_simulate_progress(self, mock_get_completed_courses): def test_simulate_progress(self, mock_get_completed_courses):
"""Simulate the entirety of a user's progress through a program.""" """Simulate the entirety of a user's progress through a program."""
...@@ -499,16 +500,23 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -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) self._mock_programs_api(data)
# No enrollments, no program engaged. # No enrollments, no program engaged.
meter = utils.ProgramProgressMeter(self.user, []) meter = utils.ProgramProgressMeter(self.user)
self._assert_progress(meter) self._assert_progress(meter)
self.assertEqual(meter.completed_programs, [])
# One enrollment, program engaged. # One enrollment, program engaged.
enrollments = self._create_enrollments(first_course_id) self._create_enrollments(first_course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
program, program_id = data[0], data[0]['id'] program, program_id = data[0], data[0]['id']
self._assert_progress( self._assert_progress(
meter, meter,
...@@ -518,10 +526,11 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -518,10 +526,11 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
not_started=self._extract_names(program, 1) not_started=self._extract_names(program, 1)
) )
) )
self.assertEqual(meter.completed_programs, [])
# Two enrollments, program in progress. # Two enrollments, program in progress.
enrollments += self._create_enrollments(second_course_id) self._create_enrollments(second_course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments) meter = utils.ProgramProgressMeter(self.user)
self._assert_progress( self._assert_progress(
meter, meter,
factories.Progress( factories.Progress(
...@@ -529,11 +538,13 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -529,11 +538,13 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
in_progress=self._extract_names(program, 0, 1) in_progress=self._extract_names(program, 0, 1)
) )
) )
self.assertEqual(meter.completed_programs, [])
# One valid certificate earned, one course code complete. # One valid certificate earned, one course code complete.
mock_get_completed_courses.return_value = [ mock_get_completed_courses.return_value = [
{'course_id': first_course_id, 'mode': MODES.verified}, {'course_id': first_course_id, 'mode': MODES.verified},
] ]
meter = utils.ProgramProgressMeter(self.user)
self._assert_progress( self._assert_progress(
meter, meter,
factories.Progress( factories.Progress(
...@@ -542,12 +553,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -542,12 +553,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
in_progress=self._extract_names(program, 1) in_progress=self._extract_names(program, 1)
) )
) )
self.assertEqual(meter.completed_programs, [])
# Invalid certificate earned, still one course code to complete. # Invalid certificate earned, still one course code to complete.
mock_get_completed_courses.return_value = [ mock_get_completed_courses.return_value = [
{'course_id': first_course_id, 'mode': MODES.verified}, {'course_id': first_course_id, 'mode': MODES.verified},
{'course_id': second_course_id, 'mode': MODES.honor}, {'course_id': second_course_id, 'mode': MODES.honor},
] ]
meter = utils.ProgramProgressMeter(self.user)
self._assert_progress( self._assert_progress(
meter, meter,
factories.Progress( factories.Progress(
...@@ -556,12 +569,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -556,12 +569,14 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
in_progress=self._extract_names(program, 1) in_progress=self._extract_names(program, 1)
) )
) )
self.assertEqual(meter.completed_programs, [])
# Second valid certificate obtained, all course codes complete. # Second valid certificate obtained, all course codes complete.
mock_get_completed_courses.return_value = [ mock_get_completed_courses.return_value = [
{'course_id': first_course_id, 'mode': MODES.verified}, {'course_id': first_course_id, 'mode': MODES.verified},
{'course_id': second_course_id, 'mode': MODES.verified}, {'course_id': second_course_id, 'mode': MODES.verified},
] ]
meter = utils.ProgramProgressMeter(self.user)
self._assert_progress( self._assert_progress(
meter, meter,
factories.Progress( factories.Progress(
...@@ -569,12 +584,12 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -569,12 +584,12 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
completed=self._extract_names(program, 0, 1) completed=self._extract_names(program, 0, 1)
) )
) )
self.assertEqual(meter.completed_programs, [program_id])
@httpretty.activate
@mock.patch(UTILS_MODULE + '.get_completed_courses') @mock.patch(UTILS_MODULE + '.get_completed_courses')
def test_nonstandard_run_mode_completion(self, mock_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. still be completed when this is the case.
""" """
course_id = 'org/course/run' course_id = 'org/course/run'
...@@ -587,24 +602,70 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase): ...@@ -587,24 +602,70 @@ class TestProgramProgressMeter(ProgramsApiConfigMixin, TestCase):
course_key=course_id, course_key=course_id,
mode_slug=MODES.honor mode_slug=MODES.honor
), ),
factories.RunMode(),
]), ]),
] ]
), ),
factories.Program(
organizations=[factories.Organization()],
course_codes=[
factories.CourseCode(run_modes=[factories.RunMode()]),
]
),
] ]
self._mock_programs_api(data) self._mock_programs_api(data)
enrollments = self._create_enrollments(course_id) self._create_enrollments(course_id)
meter = utils.ProgramProgressMeter(self.user, enrollments)
mock_get_completed_courses.return_value = [ mock_get_completed_courses.return_value = [
{'course_id': course_id, 'mode': MODES.honor}, {'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( self._assert_progress(
meter, 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 @ddt.ddt
......
openedx/core/djangoapps/programs/utils.py
...@@ -5,6 +5,7 @@ import logging ...@@ -5,6 +5,7 @@ import logging
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.utils import timezone from django.utils import timezone
from django.utils.functional import cached_property
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
import pytz import pytz
...@@ -170,29 +171,27 @@ class ProgramProgressMeter(object): ...@@ -170,29 +171,27 @@ class ProgramProgressMeter(object):
Arguments: Arguments:
user (User): The user for which to find programs. 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.user = user
self.course_ids = None
enrollments = sorted(enrollments, key=lambda e: e.created, reverse=True) self.programs = get_programs(self.user)
# enrollment.course_id is really a course key ಠ_ಠ self.course_certs = get_completed_courses(self.user)
self.course_ids = [unicode(e.course_id) for e in enrollments]
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. """Derive a list of programs in which the given user is engaged.
Arguments:
user (User): The user for which to find engaged programs.
Returns: Returns:
list of program dicts, ordered by most recent enrollment. list of program dicts, ordered by most recent enrollment.
""" """
programs = get_programs(user) enrollments = CourseEnrollment.enrollments_for_user(self.user)
flattened = flatten_programs(programs, self.course_ids) 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 = [] engaged_programs = []
for course_id in self.course_ids: for course_id in self.course_ids:
...@@ -210,8 +209,6 @@ class ProgramProgressMeter(object): ...@@ -210,8 +209,6 @@ class ProgramProgressMeter(object):
list of dict, each containing information about a user's progress list of dict, each containing information about a user's progress
towards completing a program. towards completing a program.
""" """
self.course_certs = get_completed_courses(self.user)
progress = [] progress = []
for program in self.engaged_programs: for program in self.engaged_programs:
completed, in_progress, not_started = [], [], [] completed, in_progress, not_started = [], [], []
...@@ -219,9 +216,9 @@ class ProgramProgressMeter(object): ...@@ -219,9 +216,9 @@ class ProgramProgressMeter(object):
for course_code in program['course_codes']: for course_code in program['course_codes']:
name = course_code['display_name'] name = course_code['display_name']
if self._is_complete(course_code): if self._is_course_code_complete(course_code):
completed.append(name) completed.append(name)
elif self._is_in_progress(course_code): elif self._is_course_code_in_progress(course_code):
in_progress.append(name) in_progress.append(name)
else: else:
not_started.append(name) not_started.append(name)
...@@ -235,11 +232,33 @@ class ProgramProgressMeter(object): ...@@ -235,11 +232,33 @@ class ProgramProgressMeter(object):
return progress 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. """Check if a user has completed a course code.
A course code qualifies as completed if the user has earned a A course code is completed if the user has earned a certificate
certificate in the right mode for any nested run. in the right mode for any nested run.
Arguments: Arguments:
course_code (dict): Containing nested run modes. course_code (dict): Containing nested run modes.
...@@ -247,12 +266,9 @@ class ProgramProgressMeter(object): ...@@ -247,12 +266,9 @@ class ProgramProgressMeter(object):
Returns: Returns:
bool, whether the course code is complete. bool, whether the course code is complete.
""" """
return any([ return any(self._parse(run_mode) in self.course_certs for run_mode in course_code['run_modes'])
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. """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 A user is in the process of completing a course code if they're
...@@ -264,10 +280,7 @@ class ProgramProgressMeter(object): ...@@ -264,10 +280,7 @@ class ProgramProgressMeter(object):
Returns: Returns:
bool, whether the course code is in progress. bool, whether the course code is in progress.
""" """
return any([ return any(run_mode['course_key'] in self.course_ids for run_mode in course_code['run_modes'])
run_mode['course_key'] in self.course_ids
for run_mode in course_code['run_modes']
])
def _parse(self, run_mode): def _parse(self, run_mode):
"""Modify the structure of a run mode dict. """Modify the structure of a run mode dict.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment