diff --git a/openedx/core/djangoapps/catalog/models.py b/openedx/core/djangoapps/catalog/models.py index 80e95a60d2..bcb2f30169 100644 --- a/openedx/core/djangoapps/catalog/models.py +++ b/openedx/core/djangoapps/catalog/models.py @@ -27,7 +27,7 @@ class CatalogIntegration(ConfigurationModel): service_username = models.CharField( max_length=100, - default="lms_catalog_service_user", + default='lms_catalog_service_user', null=False, blank=False, help_text=_( 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 5d000656db..8bcd6dd869 100644 --- a/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py @@ -3,21 +3,18 @@ from collections import namedtuple import logging from django.contrib.auth.models import User -from django.core.management import BaseCommand, CommandError +from django.core.management import BaseCommand from django.db.models import Q from opaque_keys.edx.keys import CourseKey -from provider.oauth2.models import Client -import waffle from certificates.models import GeneratedCertificate, CertificateStatuses # pylint: disable=import-error -from openedx.core.djangoapps.catalog.models import CatalogIntegration +from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.programs.tasks.v1.tasks import award_program_certificates -from openedx.core.djangoapps.programs.utils import get_programs # TODO: Log to console, even with debug mode disabled? logger = logging.getLogger(__name__) # pylint: disable=invalid-name -RunMode = namedtuple('RunMode', ['course_key', 'mode_slug']) +CourseRun = namedtuple('CourseRun', ['key', 'type']) class Command(BaseCommand): @@ -27,8 +24,7 @@ class Command(BaseCommand): Celery task for further (parallelized) processing. """ help = 'Backpopulate missing program credentials.' - client = None - run_modes = None + course_runs = None usernames = None def add_arguments(self, parser): @@ -41,20 +37,10 @@ class Command(BaseCommand): ) def handle(self, *args, **options): - catalog_config = CatalogIntegration.current() - - try: - user = User.objects.get(username=catalog_config.service_username) - except: - raise CommandError( - 'User with username [{}] not found. ' - 'A service user is required to run this command.'.format(catalog_config.service_username) - ) - - self._load_run_modes(user) + logger.info('Loading programs from the catalog.') + self._load_course_runs() logger.info('Looking for users who may be eligible for a program certificate.') - self._load_usernames() if options.get('commit'): @@ -84,38 +70,44 @@ class Command(BaseCommand): failed ) - def _load_run_modes(self, user): - """Find all run modes which are part of a program.""" - use_catalog = waffle.switch_is_active('get_programs_from_catalog') - programs = get_programs(user, use_catalog=use_catalog) - self.run_modes = self._flatten(programs) + def _load_course_runs(self): + """Find all course runs which are part of a program.""" + programs = get_programs() + self.course_runs = self._flatten(programs) def _flatten(self, programs): - """Flatten program dicts into a set of run modes.""" - run_modes = set() + """Flatten programs into a set of course runs.""" + course_runs = set() for program in programs: - for course_code in program['course_codes']: - for run in course_code['run_modes']: - course_key = CourseKey.from_string(run['course_key']) - run_modes.add( - RunMode(course_key, run['mode_slug']) + for course in program['courses']: + for run in course['course_runs']: + key = CourseKey.from_string(run['key']) + course_runs.add( + CourseRun(key, run['type']) ) - return run_modes + return course_runs def _load_usernames(self): """Identify a subset of users who may be eligible for a program certificate. - This is done by finding users who have earned a certificate in at least one - program course code's run mode. + This is done by finding users who have earned a qualifying certificate in + at least one program course's course run. """ status_query = Q(status__in=CertificateStatuses.PASSED_STATUSES) - run_mode_query = reduce( + course_run_query = reduce( lambda x, y: x | y, - [Q(course_id=r.course_key, mode=r.mode_slug) for r in self.run_modes] + # A course run's type is assumed to indicate which mode must be + # completed in order for the run to count towards program completion. + # This supports the same flexible program construction allowed by the + # old programs service (e.g., completion of an old honor-only run may + # count towards completion of a course in a program). This may change + # in the future to make use of the more rigid set of "applicable seat + # types" associated with each program type in the catalog. + [Q(course_id=run.key, mode=run.type) for run in self.course_runs] ) - query = status_query & run_mode_query + query = status_query & course_run_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 393b771e61..5573161318 100644 --- a/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py +++ b/openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py @@ -1,16 +1,13 @@ """Tests for the backpopulate_program_credentials management command.""" -import json - import ddt -from django.core.management import call_command, CommandError +from django.core.management import call_command from django.test import TestCase -import httpretty import mock from certificates.models import CertificateStatuses # pylint: disable=import-error from lms.djangoapps.certificates.api import MODES from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory -from openedx.core.djangoapps.programs.tests import factories +from openedx.core.djangoapps.catalog.tests import factories from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -26,7 +23,7 @@ COMMAND_MODULE = 'openedx.core.djangoapps.programs.management.commands.backpopul @skip_unless_lms class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """Tests for the backpopulate_program_credentials management command.""" - course_id, alternate_course_id = 'org/course/run', 'org/alternate/run' + course_run_key, alternate_course_run_key = (factories.generate_course_run_key() for __ in range(2)) def setUp(self): super(BackpopulateProgramCredentialsTests, self).setUp() @@ -44,13 +41,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp @ddt.data(True, False) def test_handle(self, commit, mock_task, mock_get_programs): - """Verify that relevant tasks are only enqueued when the commit option is passed.""" + """ + Verify that relevant tasks are only enqueued when the commit option is passed. + """ data = [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=self.course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=self.course_run_key), ]), ] ), @@ -59,14 +57,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, - course_id=self.alternate_course_id, + course_id=self.alternate_course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) @@ -81,42 +79,38 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp @ddt.data( [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=course_run_key), ]), ] ), factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=alternate_course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=alternate_course_run_key), ]), ] ), ], [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=course_run_key), ]), - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=alternate_course_id), + factories.Course(course_runs=[ + factories.CourseRun(key=alternate_course_run_key), ]), ] ), ], [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=course_id), - factories.RunMode(course_key=alternate_course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=course_run_key), + factories.CourseRun(key=alternate_course_run_key), ]), ] ), @@ -128,14 +122,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, - course_id=self.alternate_course_id, + course_id=self.alternate_course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) @@ -149,14 +143,16 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp mock_task.assert_has_calls(calls, any_order=True) def test_handle_username_dedup(self, mock_task, mock_get_programs): - """Verify that only one task is enqueued for a user with multiple eligible certs.""" + """ + Verify that only one task is enqueued for a user with multiple eligible + course run certificates. + """ 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), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=self.course_run_key), + factories.CourseRun(key=self.alternate_course_run_key), ]), ] ), @@ -165,14 +161,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.alice, - course_id=self.alternate_course_id, + course_id=self.alternate_course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) @@ -182,16 +178,15 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp mock_task.assert_called_once_with(self.alice.username) def test_handle_mode_slugs(self, mock_task, mock_get_programs): - """Verify that mode slugs are taken into account.""" + """ + Verify that course run types are taken into account when identifying + qualifying course run certificates. + """ data = [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode( - course_key=self.course_id, - mode_slug=MODES.honor - ), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=self.course_run_key, type='honor'), ]), ] ), @@ -200,13 +195,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, + mode=MODES.honor, status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) @@ -216,14 +212,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp mock_task.assert_called_once_with(self.alice.username) def test_handle_passing_status(self, mock_task, mock_get_programs): - """Verify that only certificates with a passing status are selected.""" + """ + Verify that only course run 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), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=self.course_run_key), ]), ] ), @@ -238,16 +234,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, 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, + course_id=self.course_run_key, mode=MODES.verified, status=failing_status, ) @@ -256,14 +250,6 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp mock_task.assert_called_once_with(self.alice.username) - def test_handle_missing_service_user(self, mock_task, __): - """Verify that the command fails when no service user exists.""" - self.catalog_integration = self.create_catalog_integration(service_username='test') - with self.assertRaises(CommandError): - call_command('backpopulate_program_credentials') - - mock_task.assert_not_called() - @mock.patch(COMMAND_MODULE + '.logger.exception') def test_handle_enqueue_failure(self, mock_log, mock_task, mock_get_programs): """Verify that failure to enqueue a task doesn't halt execution.""" @@ -276,10 +262,9 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp data = [ factories.Program( - organizations=[factories.Organization()], - course_codes=[ - factories.CourseCode(run_modes=[ - factories.RunMode(course_key=self.course_id), + courses=[ + factories.Course(course_runs=[ + factories.CourseRun(key=self.course_run_key), ]), ] ), @@ -288,14 +273,14 @@ class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsAp GeneratedCertificateFactory( user=self.alice, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, ) GeneratedCertificateFactory( user=self.bob, - course_id=self.course_id, + course_id=self.course_run_key, mode=MODES.verified, status=CertificateStatuses.downloadable, )