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

feat: add management command to clean stale certificate availability dates

Browse Source 
[MICROBA-1806]

We are aware of product issue where it is possible for a self-paced course-run to get have a `certificate_availability_date` created in the course settings. This can have an adverse effect on the Credentials IDA where a learner's Program Record does not correctly display the course certificates they have earned because of this data. This not only causes confusion for our learners, as it appears that a course certificate a learner can access and share in the LMS is displayed as unearned in the Credential's program record, but this can also cause issues when a learner attempts to share their program record through a credit pathway and the program record would not accurately reflect their program completion.

Unfortunately, the settings that manage the certificate availability date are hidden for self-paced courses in Studio (as they should only be used in instructor-paced courses).

For this reason, we are introducing a management command that will remove a certificate available date for a specified (self-paced) course-run. This will allow us to fix issues for individual learners while we work on a longer-term fix for the larger issue.

* Add new `clean_stale_certificate_available_dates` management command
* Add new `CleanStaleCertificateAvailabilityDates` Configuration Model
* Add tests for the new management command
* (Unrelated cleanup) Fix potential issue with private.py settings in the CMS being overwritten in devstack.py for developers using devstack.
This commit is contained in:
Justin Hynes
2022-06-30 08:43:30 -04:00
parent 915ddb98d7
commit 26ae8b4bcd
6 changed files with 329 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
cms/djangoapps/contentstore/admin.py
View File

@@ -10,7 +10,11 @@ from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
from django.utils.translation import gettext as _
from edx_django_utils.admin.mixins import ReadOnlyAdminMixin
from cms.djangoapps.contentstore.models import BackfillCourseTabsConfig, VideoUploadConfig
from cms.djangoapps.contentstore.models import (
BackfillCourseTabsConfig,
CleanStaleCertificateAvailabilityDatesConfig,
VideoUploadConfig
)
from cms.djangoapps.contentstore.outlines_regenerate import CourseOutlineRegenerate
from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines
@@ -78,6 +82,11 @@ class CourseOutlineRegenerateAdmin(ReadOnlyAdminMixin, admin.ModelAdmin):
return super().changelist_view(request, extra_context)
class CleanStaleCertificateAvailabilityDatesConfigAdmin(ConfigurationModelAdmin):
pass
admin.site.register(BackfillCourseTabsConfig, ConfigurationModelAdmin)
admin.site.register(VideoUploadConfig, ConfigurationModelAdmin)
admin.site.register(CourseOutlineRegenerate, CourseOutlineRegenerateAdmin)
admin.site.register(CleanStaleCertificateAvailabilityDatesConfig, ConfigurationModelAdmin)

124
cms/djangoapps/contentstore/management/commands/clean_stale_certificate_available_dates.py Normal file
View File

@@ -0,0 +1,124 @@
"""
A management command that can be used to remove a stale `certificate_available_date` from a course-run.
"""
import logging
import shlex
from django.core.management.base import BaseCommand, CommandError
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from cms.djangoapps.contentstore.models import CleanStaleCertificateAvailabilityDatesConfig
from openedx.core.lib.courses import get_course_by_id
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
log = logging.getLogger(__name__)
class Command(BaseCommand):
"""
Example usage:
To see info regarding the course-run's certificate display behavior and availability date data:
$ ./manage.py cms clean_stale_certificate_available_dates --course-runs <course_run_key>
To remove a stale certificate availability date:
$ ./manage.py cms clean_stale_certificate_available_dates --course-runs <course_run_key> --delete
To run this command using arguments stored in the database (as part of the
`CleanStaleCertificateAvailabilityDatesConfig` configuration model):
$ ./manage.py cms clean_stale_certificate_available_dates --args-from-database
"""
help = (
"Removes the certificate_available_date data from the specified self-paced course-runs."
)
def add_arguments(self, parser):
parser.add_argument(
"--course-runs",
nargs="+",
help=(
"A space seperated list of course-runs where we want to remove the `certificate_available_date` from"
)
)
parser.add_argument(
"--delete",
action='store_true',
help=(
"Optional. If included, this flag tells the management command to remove the "
"`certificiate_available_date` data for the given courses"
)
)
parser.add_argument(
"--args-from-database",
action='store_true',
help=(
"Use arguments from the CleanStaleCertificateAvailabilityDatesConfig configuration model instead of "
"the command line",
)
)
def get_args_from_database(self):
"""
Returns an options dictionary from the current CleanStaleCertificateAvailabilityDatesConfig model.
"""
config = CleanStaleCertificateAvailabilityDatesConfig.current()
if not config.enabled:
raise CommandError(
"CleanStaleCertificateAvailabilityDatesConfig is disabled, but --args-from-database was requested."
)
args = shlex.split(config.arguments)
parser = self.create_parser("manage.py", "clean_stale_certificate_available_dates")
return vars(parser.parse_args(args))
def log_certificate_info_for_course(self, course, course_run):
"""
A utility function that prints information regarding the current course's pacing, certificate display behavior,
and certificate availability date.
"""
logging.info(f"Is course '{course_run}' self-paced? {course.self_paced}")
logging.info(
f"The certificate display behavior for course '{course_run}' is {course.certificates_display_behavior}"
)
logging.info(
f"The certificate_available_date for course '{course_run}' is '{course.certificate_available_date}'"
)
def update_course_settings(self, course):
"""
A utility function that updates-and-commits the certificate display behavior and certificate_available_date
for the course to fix the stale certificate availability date.
"""
del course.certificate_available_date
course.certificates_display_behavior = CertificatesDisplayBehaviors.EARLY_NO_INFO
# commit the changes
modulestore().update_item(course, ModuleStoreEnum.UserID.mgmt_command)
def handle(self, *args, **options):
if options["args_from_database"]:
options = self.get_args_from_database()
log.info("The `clean_stale_certificate_available_dates` command is starting.")
course_runs = options.get("course_runs")
for run in course_runs:
# verify that the course-key specified is valid
logging.info(f"Checking that '{run}' is a parsable CourseKey")
try:
course_key = CourseKey.from_string(run)
except InvalidKeyError as key_error:
raise CommandError(f"{run} is not a parsable CourseKey") from key_error
course = get_course_by_id(course_key)
self.log_certificate_info_for_course(course, run)
if options['delete']:
if not course.self_paced:
raise CommandError(
f"The course '{run}' is instructor-paced and the certificate availability date can be adjusted "
"via Studio in the UI. Aborting operation."
)
logging.info(f"Attempting to remove the certificate available date in course '{run}'")
self.update_course_settings(course)

139
cms/djangoapps/contentstore/management/commands/tests/test_clean_stale_certificate_availability_dates.py Normal file
View File

@@ -0,0 +1,139 @@
"""
Tests for the `clean_stale_certificate_available_dates` management command.
"""
from datetime import datetime, timedelta
from django.core.management import CommandError, call_command
import pytz
from cms.djangoapps.contentstore.models import CleanStaleCertificateAvailabilityDatesConfig
from openedx.core.lib.courses import get_course_by_id
from xmodule.data import CertificatesDisplayBehaviors
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
class CleanStaleCertAvailableDateTests(ModuleStoreTestCase):
"""
Tests for the `clean_stale_certificate_available_dates` management command.
"""
def setUp(self):
super().setUp()
self.self_paced_course1 = CourseFactory()
self.self_paced_course2 = CourseFactory()
self.instructor_paced_course1 = CourseFactory()
# set the self-paced courses to self-paced, create and insert an invalid certificate available date for them
self.certificate_available_date = datetime.now(pytz.UTC) + timedelta(days=30)
self.self_paced_course1.self_paced = True
self.self_paced_course1.certificate_available_date = self.certificate_available_date
self.self_paced_course2.self_paced = True
self.self_paced_course2.certificate_available_date = self.certificate_available_date
self.instructor_paced_course1.self_paced = False
self.instructor_paced_course1.certificate_available_date = self.certificate_available_date
self.update_course(self.self_paced_course1, ModuleStoreEnum.UserID.test)
self.update_course(self.self_paced_course2, ModuleStoreEnum.UserID.test)
self.update_course(self.instructor_paced_course1, ModuleStoreEnum.UserID.test)
def test_remove_certificate_available_date_from_self_paced_courses(self):
"""
Happy path test that attempts to remove a certificate available date from a course that shouldn't have one.
"""
call_command(
"clean_stale_certificate_available_dates",
"--course-runs",
self.self_paced_course1.id,
self.self_paced_course2.id,
"--delete"
)
course = get_course_by_id(self.self_paced_course1.id)
assert course.certificate_available_date is None
assert course.certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
course = get_course_by_id(self.self_paced_course2.id)
assert course.certificate_available_date is None
assert course.certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
def test_remove_certificate_available_date_no_delete_flag(self):
"""
Verifies the behavior of the management command when the delete flag is _not_ passed in. When the management
command is run without this flag, we should just log information about the course and no action should be taken
to remove the data from the course.
"""
call_command(
"clean_stale_certificate_available_dates",
"--course-runs",
self.self_paced_course1.id
)
course = get_course_by_id(self.self_paced_course1.id)
assert course.certificate_available_date is not None
# I can't compare the two datetime's directly as the datetime coming from the course/modulestore is slightly
# different (the TZ info is saved as a FixedOffset instead of the specific time zone, so I compare the two
# datetimes as an ISO8601 string)
assert (
course.certificate_available_date.isoformat(timespec='minutes') ==
self.certificate_available_date.isoformat(timespec='minutes')
)
assert course.certificates_display_behavior == CertificatesDisplayBehaviors.END
def test_remove_certificate_available_date_from_instructor_paced_course_expect_error(self):
"""
Verifies behavior of the management command when it is run with an instructor-paced course.
"""
expected_exception_message = (
f"The course '{self.instructor_paced_course1.id}' is instructor-paced and the certificate availability "
"date can be adjusted via Studio in the UI. Aborting operation."
)
with self.assertRaises(CommandError) as error:
call_command(
"clean_stale_certificate_available_dates",
"--course-runs",
self.instructor_paced_course1.id,
"--delete"
)
assert str(error.exception) == expected_exception_message
def test_remove_certificate_available_date_with_args_from_database(self):
"""
Verifies the behavior of the management command when it is run via using arguments stored in our database as
part of the `CleanStaleCertificateAvailabilityDatesConfig` configuration model.
"""
# try running the command but expect it to fail since there are no options in the database and the configuration
# model is disabled
expected_error_message = (
"CleanStaleCertificateAvailabilityDatesConfig is disabled, but --args-from-database was requested."
)
with self.assertRaises(CommandError) as error:
call_command("clean_stale_certificate_available_dates", "--args-from-database")
assert str(error.exception) == expected_error_message
# add a configuration and enable it
config = CleanStaleCertificateAvailabilityDatesConfig.current()
config.arguments = f"--course-runs {self.self_paced_course1.id} {self.self_paced_course2.id} --delete"
config.enabled = True
config.save()
call_command("clean_stale_certificate_available_dates", "--args-from-database")
course = get_course_by_id(self.self_paced_course1.id)
assert course.certificate_available_date is None
assert course.certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
course = get_course_by_id(self.self_paced_course2.id)
assert course.certificate_available_date is None
assert course.certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO
# explicitly disable the configuration and try running one more time
config.enabled = False
config.save()
with self.assertRaises(CommandError) as disabled_error:
call_command("clean_stale_certificate_available_dates", "--args-from-database")
assert str(error.exception) == expected_error_message

30
cms/djangoapps/contentstore/migrations/0008_cleanstalecertificateavailabilitydatesconfig.py Normal file
View File

@@ -0,0 +1,30 @@
# Generated by Django 3.2.13 on 2022-07-11 17:08
from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('contentstore', '0007_backfillcoursetabsconfig'),
]
operations = [
migrations.CreateModel(
name='CleanStaleCertificateAvailabilityDatesConfig',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
('arguments', models.TextField(blank=True, help_text="A space seperated collection of arguments to be used when running the `clean_stale_certificate_available_dates` management command.' See the management command for options.")),
('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')),
],
options={
'verbose_name': "Arguments for 'clean_stale_certificate_availability_dates'",
'verbose_name_plural': "Arguments for 'clean_stale_certificate_availability_dates'",
},
),
]

20
cms/djangoapps/contentstore/models.py
View File

@@ -43,3 +43,23 @@ class BackfillCourseTabsConfig(ConfigurationModel):
help_text='How many courses to backfill in this run (or zero for all courses)',
default=0,
)
class CleanStaleCertificateAvailabilityDatesConfig(ConfigurationModel):
"""
Manages configuration for a run of the `clean_stale_certificate_availability_dates` management command.
.. no_pii:
"""
class Meta:
app_label = "contentstore"
verbose_name = "Arguments for 'clean_stale_certificate_availability_dates'"
verbose_name_plural = "Arguments for 'clean_stale_certificate_availability_dates'"
arguments = TextField(
blank=True,
help_text=(
"A space seperated collection of arguments to be used when running the "
"`clean_stale_certificate_available_dates` management command.' See the management command for options."
)
)

11
cms/envs/devstack.py
View File

@@ -248,11 +248,6 @@ MODULESTORE = convert_module_store_setting_if_needed(MODULESTORE)
# Dummy secret key for dev
SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
###############################################################################
# See if the developer has any local overrides.
if os.path.isfile(join(dirname(abspath(__file__)), 'private.py')):
from .private import * # pylint: disable=import-error,wildcard-import
############# CORS headers for cross-domain requests #################
FEATURES['ENABLE_CORS_HEADERS'] = True
CORS_ALLOW_CREDENTIALS = True
@@ -295,3 +290,9 @@ SOCIAL_AUTH_REDIRECT_IS_HTTPS = False
#################### Network configuration ####################
# Devstack is directly exposed to the caller
CLOSEST_CLIENT_IP_FROM_HEADERS = []
################# New settings must go ABOVE this line #################
########################################################################
# See if the developer has any local overrides.
if os.path.isfile(join(dirname(abspath(__file__)), 'private.py')):
from .private import * # pylint: disable=import-error,wildcard-import