Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor!: Remove backpopulate_program_credentials in favor of using

Browse Source 
notify_credentials command

In this PR we are removing a management command that has performance issues and duplicates logic not available in the notify_credentials management command.
This commit is contained in:
Albert (AJ) St. Aubin
2021-05-04 19:25:27 -04:00
parent 5bfc790014
commit ee71594714
5 changed files with 17 additions and 573 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
openedx/core/djangoapps/programs/admin.py
View File

@@ -6,7 +6,7 @@ django admin pages for program support models
from config_models.admin import ConfigurationModelAdmin
from django.contrib import admin
from openedx.core.djangoapps.programs.models import ProgramsApiConfig, CustomProgramsConfig
from openedx.core.djangoapps.programs.models import ProgramsApiConfig
class ProgramsApiConfigAdmin(ConfigurationModelAdmin):
@@ -14,4 +14,3 @@ class ProgramsApiConfigAdmin(ConfigurationModelAdmin):
admin.site.register(ProgramsApiConfig, ProgramsApiConfigAdmin)
admin.site.register(CustomProgramsConfig, ConfigurationModelAdmin)

163
openedx/core/djangoapps/programs/management/commands/backpopulate_program_credentials.py
View File

@@ -1,163 +0,0 @@
"""Management command for backpopulating missing program credentials."""
import logging
from collections import namedtuple
from functools import reduce # pylint: disable=redefined-builtin, useless-suppression
from django.contrib.sites.models import Site
from django.core.management.base import BaseCommand, CommandError
from django.db.models import Q
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.course_modes.models import CourseMode
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
from openedx.core.djangoapps.catalog.utils import get_programs
from openedx.core.djangoapps.programs.tasks import award_program_certificates
from openedx.core.djangoapps.programs.models import CustomProgramsConfig
# TODO: Log to console, even with debug mode disabled?
logger = logging.getLogger(__name__) # pylint: disable=invalid-name
CourseRun = namedtuple('CourseRun', ['key', 'type'])
class Command(BaseCommand):
"""Management command for backpopulating missing program credentials.
The command's goal is to pass a narrow subset of usernames to an idempotent
Celery task for further (parallelized) processing.
"""
help = 'Backpopulate missing program credentials.'
course_runs = None
usernames = None
def add_arguments(self, parser):
parser.add_argument(
'-c', '--commit',
action='store_true',
dest='commit',
default=False,
help='Submit tasks for processing.'
)
parser.add_argument(
'--args-from-database',
action='store_true',
default=False,
help='Use arguments from the Config model instead of the command line.',
)
parser.add_argument(
'--program-uuids',
nargs='+',
help='Award certificates only for specific programs.',
)
parser.add_argument(
'--usernames',
nargs='+',
help='Award certificates only to specific users.',
)
def get_args_from_database(self):
""" Returns an options dictionary from the current NotifyCredentialsConfig model. """
config = CustomProgramsConfig.current()
if not config.enabled:
raise CommandError('CustomProgramsConfig is disabled, but --args-from-database was requested.')
# We don't need fancy shell-style whitespace/quote handling - none of our arguments are complicated
argv = config.arguments.split()
parser = self.create_parser('manage.py', 'backpopulate_program_credentials')
return parser.parse_args(argv).__dict__ # we want a dictionary, not a non-iterable Namespace object
def handle(self, *args, **options):
program_uuids, usernames = None, None
if options['args_from_database']:
logger.info('Loading arguments from the database for custom programs or learners.')
arguments = self.get_args_from_database()
program_uuids = arguments.get('program-uuids', None)
usernames = arguments.get('usernames', None)
logger.info('Loading programs from the catalog.')
self._load_course_runs(program_uuids=program_uuids)
logger.info('Looking for users who may be eligible for a program certificate.')
self._load_usernames(users=usernames)
if options.get('commit'):
logger.info('Enqueuing program certification tasks for %d candidates.', len(self.usernames))
else:
logger.info(
'Found %d candidates. To enqueue program certification tasks, pass the -c or --commit flags.',
len(self.usernames)
)
return
succeeded, failed = 0, 0
for username in self.usernames:
try:
award_program_certificates.delay(username)
except: # pylint: disable=bare-except
failed += 1
logger.exception('Failed to enqueue task for user [%s]', username)
else:
succeeded += 1
logger.debug('Successfully enqueued task for user [%s]', username)
logger.info(
'Done. Successfully enqueued tasks for %d candidates. '
'Failed to enqueue tasks for %d candidates.',
succeeded,
failed
)
def _load_course_runs(self, program_uuids=None):
"""Find all course runs which are part of a program."""
programs = []
if program_uuids:
programs.extend(get_programs(uuids=program_uuids))
else:
for site in Site.objects.all():
logger.info('Loading programs from the catalog for site %s.', site.domain)
programs.extend(get_programs(site))
self.course_runs = self._flatten(programs)
def _flatten(self, programs):
"""Flatten programs into a set of course runs."""
course_runs = set()
for program in programs:
for course in program['courses']:
for course_run in course['course_runs']:
key = CourseKey.from_string(course_run['key'])
course_runs.add(
CourseRun(key, course_run['type'])
)
return course_runs
def _load_usernames(self, users=None):
"""Identify a subset of users who may be eligible for a program certificate.
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)
course_run_query = reduce(
lambda x, y: x | y,
[Q(course_id=course_run.key, mode=course_run.type) for course_run in self.course_runs]
)
# Account for the fact that no-id-professional and professional are equivalent
for course_run in self.course_runs:
if course_run.type == CourseMode.PROFESSIONAL:
course_run_query |= Q(course_id=course_run.key, mode=CourseMode.NO_ID_PROFESSIONAL_MODE)
query = status_query & course_run_query
username_dicts = GeneratedCertificate.eligible_available_certificates.filter(
query
).values('user__username').distinct()
self.usernames = [d['user__username'] for d in username_dicts]
if users:
# keeping only those learners who are in the arguments
self.usernames = list(set(self.usernames) & set(users))

16
openedx/core/djangoapps/programs/migrations/0014_delete_customprogramsconfig.py Normal file
View File

@@ -0,0 +1,16 @@
# Generated by Django 2.2.20 on 2021-05-04 19:36
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('programs', '0013_customprogramsconfig'),
]
operations = [
migrations.DeleteModel(
name='CustomProgramsConfig',
),
]

18
openedx/core/djangoapps/programs/models.py
View File

@@ -22,21 +22,3 @@ class ProgramsApiConfig(ConfigurationModel):
'Path used to construct URLs to programs marketing pages (e.g., "/foo").'
)
)
class CustomProgramsConfig(ConfigurationModel): # pylint: disable=model-missing-unicode, useless-suppression
"""
Manages configuration for a run of the backpopulate_program_credentials management command.
"""
class Meta:
app_label = 'programs'
verbose_name = 'backpopulate_program_credentials argument'
arguments = models.TextField(
blank=True,
help_text='Useful for manually running a Jenkins job. Specify like "--usernames A B --program-uuids X Y".',
default='',
)
def __str__(self):
return str(self.arguments)

390
openedx/core/djangoapps/programs/tests/test_backpopulate_program_credentials.py
View File

@@ -1,390 +0,0 @@
"""Tests for the backpopulate_program_credentials management command."""
from unittest import mock
import ddt
from django.core.management import call_command
from django.test import TestCase
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.course_modes.models import CourseMode
from lms.djangoapps.certificates.api import MODES
from lms.djangoapps.certificates.models import CertificateStatuses
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from openedx.core.djangoapps.catalog.tests.factories import (
CourseFactory,
CourseRunFactory,
ProgramFactory,
generate_course_run_key
)
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangolib.testing.utils import skip_unless_lms
from common.djangoapps.student.tests.factories import UserFactory
COMMAND_MODULE = 'openedx.core.djangoapps.programs.management.commands.backpopulate_program_credentials'
@ddt.ddt
@mock.patch(COMMAND_MODULE + '.get_programs')
@mock.patch(COMMAND_MODULE + '.award_program_certificates.delay')
@skip_unless_lms
class BackpopulateProgramCredentialsTests(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase):
"""Tests for the backpopulate_program_credentials management command."""
course_run_key, alternate_course_run_key, course_run_key_no_course_overview = (
generate_course_run_key() for __ in range(3)
)
# Constants for the _get_programs_data hierarchy types used in test_flatten()
SEPARATE_PROGRAMS = 'separate_programs'
SEPARATE_COURSES = 'separate_courses'
SAME_COURSE = 'same_course'
def setUp(self):
super().setUp()
self.alice = UserFactory()
self.bob = UserFactory()
# We need CourseOverview instances to exist for the test courses
CourseOverviewFactory(id=CourseKey.from_string(self.course_run_key))
CourseOverviewFactory(id=CourseKey.from_string(self.alternate_course_run_key))
# 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_credentials_config(enable_learner_issuance=False)
catalog_integration = self.create_catalog_integration()
UserFactory(username=catalog_integration.service_username)
def _get_programs_data(self, hierarchy_type):
"""
Generate a mock response for get_programs() with the given type of
course hierarchy. Dramatically simplifies (and makes consistent
between test runs) the ddt-generated test_flatten methods.
"""
if hierarchy_type == self.SEPARATE_PROGRAMS:
return [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
]),
]
),
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.alternate_course_run_key),
]),
]
),
]
elif hierarchy_type == self.SEPARATE_COURSES:
return [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
]),
CourseFactory(course_runs=[
CourseRunFactory(key=self.alternate_course_run_key),
]),
]
),
]
else: # SAME_COURSE
return [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
CourseRunFactory(key=self.alternate_course_run_key),
]),
]
),
]
@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.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.alternate_course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=commit)
if commit:
mock_task.assert_called_once_with(self.alice.username)
else:
mock_task.assert_not_called()
def test_handle_professional(self, mock_task, mock_get_programs):
""" Verify the task can handle both professional and no-id-professional modes. """
mock_get_programs.return_value = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key, type='professional'),
]),
]
),
]
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=CourseMode.PROFESSIONAL,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key,
mode=CourseMode.NO_ID_PROFESSIONAL_MODE,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=True)
# The task should be called for both users since professional and no-id-professional are equivalent.
mock_task.assert_has_calls([mock.call(self.alice.username), mock.call(self.bob.username)], any_order=True)
@ddt.data(SEPARATE_PROGRAMS, SEPARATE_COURSES, SAME_COURSE)
def test_handle_flatten(self, hierarchy_type, mock_task, mock_get_programs):
"""Verify that program structures are flattened correctly."""
mock_get_programs.return_value = self._get_programs_data(hierarchy_type)
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.alternate_course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=True)
calls = [
mock.call(self.alice.username),
mock.call(self.bob.username)
]
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
course run certificates.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
CourseRunFactory(key=self.alternate_course_run_key),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.alice,
course_id=self.alternate_course_run_key,
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_mode_slugs(self, mock_task, mock_get_programs):
"""
Verify that course run types are taken into account when identifying
qualifying course run certificates.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key, type='honor'),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.honor,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key,
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, mock_get_programs):
"""
Verify that only course run certificates with a passing status are selected.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
]),
]
),
]
mock_get_programs.return_value = data
passing_status = CertificateStatuses.downloadable
failing_status = CertificateStatuses.notpassing
assert passing_status in CertificateStatuses.PASSED_STATUSES
assert failing_status not in CertificateStatuses.PASSED_STATUSES
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=passing_status,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key,
mode=MODES.verified,
status=failing_status,
)
call_command('backpopulate_program_credentials', commit=True)
mock_task.assert_called_once_with(self.alice.username)
def test_handle_no_course_overview(self, mock_task, mock_get_programs):
"""
Verify that the task is not enqueued for a user whose only certificate
is for a course with no CourseOverview.
"""
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
CourseRunFactory(key=self.course_run_key_no_course_overview),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key_no_course_overview,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=True)
mock_task.assert_called_once_with(self.alice.username)
@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."""
def side_effect(username):
"""Simulate failure to enqueue a task."""
if username == self.alice.username:
raise Exception
mock_task.side_effect = side_effect
data = [
ProgramFactory(
courses=[
CourseFactory(course_runs=[
CourseRunFactory(key=self.course_run_key),
]),
]
),
]
mock_get_programs.return_value = data
GeneratedCertificateFactory(
user=self.alice,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
GeneratedCertificateFactory(
user=self.bob,
course_id=self.course_run_key,
mode=MODES.verified,
status=CertificateStatuses.downloadable,
)
call_command('backpopulate_program_credentials', commit=True)
assert mock_log.called
calls = [
mock.call(self.alice.username),
mock.call(self.bob.username)
]
mock_task.assert_has_calls(calls, any_order=True)