From dbe6dfc2ae5523ea0db35bdfa3543512b33308dc Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Wed, 11 May 2016 17:43:12 -0400 Subject: [PATCH] Tune program credential backpopulation command Filter certificates by passing status when looking for users who may be eligible for a program certificate. Doing so dramatically accelerates the query and further reduces the set of targeted users. Part of ECOM-3924. --- .../backpopulate_program_credentials.py | 8 ++- .../test_backpopulate_program_credentials.py | 59 ++++++++++++++++++- 2 files changed, 63 insertions(+), 4 deletions(-) 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 101133d0ee..2843433dba 100644 --- a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py @@ -7,7 +7,7 @@ from django.db.models import Q from opaque_keys.edx.keys import CourseKey from provider.oauth2.models import Client -from certificates.models import GeneratedCertificate # pylint: disable=import-error +from certificates.models import GeneratedCertificate, CertificateStatuses # pylint: disable=import-error from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.tasks.v1.tasks import award_program_certificates from openedx.core.djangoapps.programs.utils import get_programs @@ -109,11 +109,13 @@ class Command(BaseCommand): This is done by finding users who have earned a certificate in at least one program course code's run mode. """ - query = reduce( + status_query = Q(status__in=CertificateStatuses.PASSED_STATUSES) + run_mode_query = reduce( lambda x, y: x | y, [Q(course_id=r.course_key, mode=r.mode_slug) for r in self.run_modes] ) - # TODO: Filter further, by passing status? + query = status_query & run_mode_query + username_dicts = GeneratedCertificate.eligible_certificates.filter(query).values('user__username').distinct() self.usernames = [d['user__username'] for d in username_dicts] 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 5143cf1cdc..e28f9204a7 100644 --- a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py @@ -11,6 +11,7 @@ import httpretty import mock from provider.constants import CONFIDENTIAL +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.programs.models import ProgramsApiConfig @@ -38,7 +39,10 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): self.oauth2_user = UserFactory() self.oauth2_client = ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL) - self.create_programs_config() + # Disable certification to prevent the task from being triggered when + # setting up test data (i.e., certificates with a passing status), thereby + # skewing mock call counts. + self.create_programs_config(enable_certification=False) def _link_oauth2_user(self): """Helper to link user and OAuth2 client.""" @@ -74,12 +78,14 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): user=self.alice, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, course_id=self.alternate_course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) call_command('backpopulate_program_credentials', commit=commit) @@ -142,12 +148,14 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): user=self.alice, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, course_id=self.alternate_course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) call_command('backpopulate_program_credentials', commit=True) @@ -178,12 +186,14 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): user=self.alice, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.alice, course_id=self.alternate_course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) call_command('backpopulate_program_credentials', commit=True) @@ -211,12 +221,56 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): GeneratedCertificateFactory( user=self.alice, course_id=self.course_id, + status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, + ) + + call_command('backpopulate_program_credentials', commit=True) + + mock_task.assert_called_once_with(self.alice.username) + + def test_handle_passing_status(self, mock_task): + """Verify that only certificates with a passing status are selected.""" + data = [ + factories.Program( + organizations=[factories.Organization()], + course_codes=[ + factories.CourseCode(run_modes=[ + factories.RunMode(course_key=self.course_id), + factories.RunMode(course_key=self.alternate_course_id), + ]), + ] + ), + ] + self._mock_programs_api(data) + self._link_oauth2_user() + + passing_status = CertificateStatuses.downloadable + failing_status = CertificateStatuses.notpassing + + self.assertIn(passing_status, CertificateStatuses.PASSED_STATUSES) + self.assertNotIn(failing_status, CertificateStatuses.PASSED_STATUSES) + + GeneratedCertificateFactory( + user=self.alice, + course_id=self.course_id, + mode=MODES.verified, + status=passing_status, + ) + + # The alternate course is used here to verify that the status and run_mode + # queries are being ANDed together correctly. + GeneratedCertificateFactory( + user=self.bob, + course_id=self.alternate_course_id, + mode=MODES.verified, + status=failing_status, ) call_command('backpopulate_program_credentials', commit=True) @@ -241,6 +295,7 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): user=self.alice, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) with self.assertRaises(CommandError): @@ -275,12 +330,14 @@ class BackpopulateProgramCredentialsTests(ProgramsApiConfigMixin, TestCase): user=self.alice, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, course_id=self.course_id, mode=MODES.verified, + status=CertificateStatuses.downloadable, ) call_command('backpopulate_program_credentials', commit=True)