Merge pull request #34042 from openedx/dkaplan1/APER-2851_replicate-share-certificate-in-facebook-improvements-for-edx.org

feat: adds a certificate template modifier
This commit is contained in:
Deborah Kaplan
2024-01-22 09:23:19 -05:00
committed by GitHub
8 changed files with 445 additions and 46 deletions

View File

@@ -20,7 +20,8 @@ from lms.djangoapps.certificates.models import (
CertificateHtmlViewConfiguration,
CertificateTemplate,
CertificateTemplateAsset,
GeneratedCertificate
GeneratedCertificate,
ModifiedCertificateTemplateCommandConfiguration,
)
@@ -92,6 +93,11 @@ class CertificateGenerationCourseSettingAdmin(admin.ModelAdmin):
show_full_result_count = False
@admin.register(ModifiedCertificateTemplateCommandConfiguration)
class ModifiedCertificateTemplateCommandConfigurationAdmin(ConfigurationModelAdmin):
pass
@admin.register(CertificateGenerationCommandConfiguration)
class CertificateGenerationCommandConfigurationAdmin(ConfigurationModelAdmin):
pass

View File

@@ -0,0 +1,91 @@
"""Management command to modify certificate templates."""
import logging
import shlex
from argparse import RawDescriptionHelpFormatter
from django.core.management.base import BaseCommand, CommandError
from lms.djangoapps.certificates.models import (
ModifiedCertificateTemplateCommandConfiguration,
)
from lms.djangoapps.certificates.tasks import handle_modify_cert_template
log = logging.getLogger(__name__)
class Command(BaseCommand):
"""Management command to modify certificate templates.
Example usage:
./manage.py lms modify_cert_template --old-text "</head>" --new text "<p>boo!</p></head>" --templates 867 3509
"""
help = """Modify one or more certificate templates.
This is DANGEROUS.
* This uses string replacement to modify HTML-like templates, because the presence of
Django Templating makes it impossible to do true parsing.
* This isn't parameterizing the replacement text, for the same reason. It has
no way of knowing what is template language and what is HTML.
Do not trust that this will get the conversion right without verification,
and absolutely do not accepted untrusted user input for the replacement text. This is
to be run by trusted users only.
Always run this with dry-run or in a reliable test environment.
"""
def add_arguments(self, parser):
parser.formatter_class = RawDescriptionHelpFormatter
parser.add_argument(
"--dry-run",
action="store_true",
help="Just show a preview of what would happen.",
)
parser.add_argument(
"--old-text",
help="Text to replace in the template.",
)
parser.add_argument(
"--new-text",
help="Replacement text for the template.",
)
parser.add_argument(
"--templates",
nargs="+",
help="Certificate templates to modify.",
)
parser.add_argument(
"--args-from-database",
action="store_true",
help="Use arguments from the ModifyCertificateTemplateConfiguration model instead of the command line.",
)
def get_args_from_database(self):
"""
Returns an options dictionary from the current ModifiedCertificateTemplateCommandConfiguration instance.
"""
config = ModifiedCertificateTemplateCommandConfiguration.current()
if not config.enabled:
raise CommandError(
"ModifyCertificateTemplateCommandConfiguration is disabled, but --args-from-database was requested"
)
args = shlex.split(config.arguments)
parser = self.create_parser("manage.py", "modify_cert_template")
return vars(parser.parse_args(args))
def handle(self, *args, **options):
# database args will override cmd line args
if options["args_from_database"]:
options = self.get_args_from_database()
# Check required arguments here. We can't rely on marking args "required" because they might come from django
if not (options["old_text"] and options["new_text"] and options["templates"]):
raise CommandError(
"The following arguments are required: --old-text, --new-text, --templates"
)
log.info(
"modify_cert_template starting, dry-run={dry_run}, templates={templates}, "
"old-text={old}, new-text={new}".format(
dry_run=options["dry_run"],
templates=options["templates"],
old=options["old_text"],
new=options["new_text"],
)
)
handle_modify_cert_template.delay(options)

View File

@@ -0,0 +1,41 @@
"""
Tests for the modify_cert_template command
"""
import pytest
from django.core.management import CommandError, call_command
from django.test import TestCase
class ModifyCertTemplateTests(TestCase):
"""Tests for the modify_cert_template management command"""
def test_command_with_missing_param_old_text(self):
"""Verify command with a missing param --old-text."""
with pytest.raises(
CommandError,
match="The following arguments are required: --old-text, --new-text, --templates",
):
call_command(
"modify_cert_template", "--new-text", "blah", "--templates", "1 2 3"
)
def test_command_with_missing_param_new_text(self):
"""Verify command with a missing param --new-text."""
with pytest.raises(
CommandError,
match="The following arguments are required: --old-text, --new-text, --templates",
):
call_command(
"modify_cert_template", "--old-text", "blah", "--templates", "1 2 3"
)
def test_command_with_missing_param_templates(self):
"""Verify command with a missing param --templates."""
with pytest.raises(
CommandError,
match="The following arguments are required: --old-text, --new-text, --templates",
):
call_command(
"modify_cert_template", "--new-text", "blah", "--old-text", "xyzzy"
)

View File

@@ -0,0 +1,29 @@
# Generated by Django 3.2.23 on 2024-01-16 18:57
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),
('certificates', '0035_auto_20230808_0944'),
]
operations = [
migrations.CreateModel(
name='ModifiedCertificateTemplateCommandConfiguration',
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, default='', help_text='Arguments for the \'modify_cert_template\' management command. Specify like \'--old-text "foo" --new-text "bar" --template_ids <id1> <id2>\'')),
('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': 'modify_cert_template argument',
},
),
]

View File

@@ -2,11 +2,11 @@
Course certificates are created for a student and an offering of a course (a course run).
"""
from datetime import timezone
import json
import logging
import os
import uuid
from datetime import timezone
from config_models.models import ConfigurationModel
from django.apps import apps
@@ -16,7 +16,6 @@ from django.core.exceptions import ValidationError
from django.db import models, transaction
from django.db.models import Count
from django.dispatch import receiver
from django.utils.translation import gettext_lazy as _
from model_utils import Choices
from model_utils.models import TimeStampedModel
@@ -1243,6 +1242,30 @@ class CertificateTemplateAsset(TimeStampedModel):
app_label = "certificates"
class ModifiedCertificateTemplateCommandConfiguration(ConfigurationModel):
"""
Manages configuration for a run of the modify_cert_template management command.
.. no_pii:
"""
class Meta:
app_label = "certificates"
verbose_name = "modify_cert_template argument"
arguments = models.TextField(
blank=True,
help_text=(
"Arguments for the 'modify_cert_template' management command. Specify like '--old-text \"foo\" "
"--new-text \"bar\" --template_ids <id1> <id2>'"
),
default="",
)
def __str__(self):
return str(self.arguments)
class CertificateGenerationCommandConfiguration(ConfigurationModel):
"""
Manages configuration for a run of the cert_generation management command.

View File

@@ -1,17 +1,20 @@
"""
Tasks that generate a course certificate for a user
Tasks that operate on course certificates for a user
"""
from difflib import unified_diff
from logging import getLogger
from typing import Any, Dict, List
from celery import shared_task
from celery_utils.persist_on_failure import LoggedPersistOnFailureTask
from celery_utils.persist_on_failure import LoggedPersistOnFailureTask, LoggedTask
from django.contrib.auth import get_user_model
from edx_django_utils.monitoring import set_code_owner_attribute
from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.generation import generate_course_certificate
from lms.djangoapps.certificates.models import CertificateTemplate
log = getLogger(__name__)
User = get_user_model()
@@ -21,7 +24,9 @@ User = get_user_model()
CERTIFICATE_DELAY_SECONDS = 2
@shared_task(base=LoggedPersistOnFailureTask, bind=True, default_retry_delay=30, max_retries=2)
@shared_task(
base=LoggedPersistOnFailureTask, bind=True, default_retry_delay=30, max_retries=2
)
@set_code_owner_attribute
def generate_certificate(self, **kwargs): # pylint: disable=unused-argument
"""
@@ -37,12 +42,125 @@ def generate_certificate(self, **kwargs): # pylint: disable=unused-argument
- generation_mode: Used when emitting an event. Options are "self" (implying the user generated the cert
themself) and "batch" for everything else. Defaults to 'batch'.
"""
student = User.objects.get(id=kwargs.pop('student'))
course_key = CourseKey.from_string(kwargs.pop('course_key'))
status = kwargs.pop('status', CertificateStatuses.downloadable)
enrollment_mode = kwargs.pop('enrollment_mode')
course_grade = kwargs.pop('course_grade', '')
generation_mode = kwargs.pop('generation_mode', 'batch')
student = User.objects.get(id=kwargs.pop("student"))
course_key = CourseKey.from_string(kwargs.pop("course_key"))
status = kwargs.pop("status", CertificateStatuses.downloadable)
enrollment_mode = kwargs.pop("enrollment_mode")
course_grade = kwargs.pop("course_grade", "")
generation_mode = kwargs.pop("generation_mode", "batch")
generate_course_certificate(user=student, course_key=course_key, status=status, enrollment_mode=enrollment_mode,
course_grade=course_grade, generation_mode=generation_mode)
generate_course_certificate(
user=student,
course_key=course_key,
status=status,
enrollment_mode=enrollment_mode,
course_grade=course_grade,
generation_mode=generation_mode,
)
@shared_task(base=LoggedTask, ignore_result=True)
@set_code_owner_attribute
def handle_modify_cert_template(options: Dict[str, Any]) -> None:
"""
Celery task to handle the modify_cert_template management command.
Args:
old_text (string): Text in the template of which the first instance should be changed
new_text (string): Replacement text for old_text
template_ids (list[string]): List of template IDs for this run.
dry_run (boolean): Don't do the work, just report the changes that would happen
"""
template_ids = options["templates"]
if not template_ids:
template_ids = []
log.info(
"[modify_cert_template] Attempting to modify {num} templates".format(
num=len(template_ids)
)
)
templates_changed = get_changed_cert_templates(options)
for template in templates_changed:
template.save()
def get_changed_cert_templates(options: Dict[str, Any]) -> List[CertificateTemplate]:
"""
Loop through the templates and return instances with changed template text.
Args:
old_text (string): Text in the template of which the first instance should be changed
new_text (string): Replacement text for old_text
template_ids (list[string]): List of template IDs for this run.
dry_run (boolean): Don't do the work, just report the changes that would happen
"""
template_ids = options["templates"]
if not template_ids:
template_ids = []
log.info(
"[modify_cert_template] Attempting to modify {num} templates".format(
num=len(template_ids)
)
)
dry_run = options.get("dry_run", None)
templates_changed = []
for template_id in template_ids:
template = None
try:
template = CertificateTemplate.objects.get(id=template_id)
except CertificateTemplate.DoesNotExist:
log.warning(f"Template {template_id} could not be found")
if template is not None:
log.info(
"[modify_cert_template] Calling for template {template_id} : {name}".format(
template_id=template_id, name=template.description
)
)
new_template = template.template.replace(
options["old_text"], options["new_text"], 1
)
if template.template == new_template:
log.info(
"[modify_cert_template] No changes to {template_id}".format(
template_id=template_id
)
)
else:
if not dry_run:
log.info(
"[modify_cert_template] Modifying template {template} ({description})".format(
template=template_id,
description=template.description,
)
)
template.template = new_template
templates_changed.append(template)
else:
log.info(
"DRY-RUN: Not making the following template change to {id}.".format(
id=template_id
)
)
log.info(
"\n".join(
unified_diff(
template.template.splitlines(),
new_template.splitlines(),
lineterm="",
fromfile="old_template",
tofile="new_template",
)
),
)
log.info(
"[modify_cert_template] Modified {num} templates".format(
num=len(templates_changed)
)
)
return templates_changed

View File

@@ -6,6 +6,7 @@ Certificates factories
import datetime
from uuid import uuid4
from factory import Sequence
from factory.django import DjangoModelFactory
from common.djangoapps.student.models import LinkedInAddToProfileConfiguration
@@ -15,7 +16,8 @@ from lms.djangoapps.certificates.models import (
CertificateHtmlViewConfiguration,
CertificateInvalidation,
CertificateStatuses,
GeneratedCertificate
CertificateTemplate,
GeneratedCertificate,
)
@@ -23,15 +25,16 @@ class GeneratedCertificateFactory(DjangoModelFactory):
"""
GeneratedCertificate factory
"""
class Meta:
model = GeneratedCertificate
course_id = None
status = CertificateStatuses.unavailable
mode = GeneratedCertificate.MODES.honor
name = ''
name = ""
verify_uuid = uuid4().hex
grade = ''
grade = ""
class CertificateAllowlistFactory(DjangoModelFactory):
@@ -44,7 +47,7 @@ class CertificateAllowlistFactory(DjangoModelFactory):
course_id = None
allowlist = True
notes = 'Test Notes'
notes = "Test Notes"
class CertificateInvalidationFactory(DjangoModelFactory):
@@ -55,7 +58,7 @@ class CertificateInvalidationFactory(DjangoModelFactory):
class Meta:
model = CertificateInvalidation
notes = 'Test Notes'
notes = "Test Notes"
active = True
@@ -112,8 +115,21 @@ class CertificateDateOverrideFactory(DjangoModelFactory):
"""
CertificateDateOverride factory
"""
class Meta:
model = CertificateDateOverride
date = datetime.datetime(2021, 5, 11, 0, 0, tzinfo=datetime.timezone.utc)
reason = "Learner really wanted this on their birthday"
class CertificateTemplateFactory(DjangoModelFactory):
"""CertificateTemplate factory"""
class Meta:
model = CertificateTemplate
name = Sequence("template{}".format)
description = Sequence("description for template{}".format)
template = ""
is_active = True

View File

@@ -3,6 +3,7 @@ Tests for course certificate tasks.
"""
from textwrap import dedent
from unittest import mock
from unittest.mock import patch
@@ -13,7 +14,11 @@ from opaque_keys.edx.keys import CourseKey
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tasks import generate_certificate
from lms.djangoapps.certificates.tasks import (
generate_certificate,
get_changed_cert_templates,
)
from lms.djangoapps.certificates.tests.factories import CertificateTemplateFactory
@ddt.ddt
@@ -21,23 +26,24 @@ class GenerateUserCertificateTest(TestCase):
"""
Tests for course certificate tasks
"""
def setUp(self):
super().setUp()
self.user = UserFactory()
self.course_key = 'course-v1:edX+DemoX+Demo_Course'
self.course_key = "course-v1:edX+DemoX+Demo_Course"
@ddt.data('student', 'course_key', 'enrollment_mode')
@ddt.data("student", "course_key", "enrollment_mode")
def test_missing_args(self, missing_arg):
kwargs = {
'student': self.user.id,
'course_key': self.course_key,
'other_arg': 'shiny',
'enrollment_mode': CourseMode.MASTERS
"student": self.user.id,
"course_key": self.course_key,
"other_arg": "shiny",
"enrollment_mode": CourseMode.MASTERS,
}
del kwargs[missing_arg]
with patch('lms.djangoapps.certificates.tasks.User.objects.get'):
with patch("lms.djangoapps.certificates.tasks.User.objects.get"):
with self.assertRaisesRegex(KeyError, missing_arg):
generate_certificate.apply_async(kwargs=kwargs).get()
@@ -48,13 +54,13 @@ class GenerateUserCertificateTest(TestCase):
enrollment_mode = CourseMode.VERIFIED
with mock.patch(
'lms.djangoapps.certificates.tasks.generate_course_certificate',
return_value=None
"lms.djangoapps.certificates.tasks.generate_course_certificate",
return_value=None,
) as mock_generate_cert:
kwargs = {
'student': self.user.id,
'course_key': self.course_key,
'enrollment_mode': enrollment_mode
"student": self.user.id,
"course_key": self.course_key,
"enrollment_mode": enrollment_mode,
}
generate_certificate.apply_async(kwargs=kwargs)
@@ -63,31 +69,31 @@ class GenerateUserCertificateTest(TestCase):
course_key=CourseKey.from_string(self.course_key),
status=CertificateStatuses.downloadable,
enrollment_mode=enrollment_mode,
course_grade='',
generation_mode='batch'
course_grade="",
generation_mode="batch",
)
def test_generation_custom(self):
"""
Verify the task handles certificate generation custom params
"""
gen_mode = 'self'
gen_mode = "self"
status = CertificateStatuses.notpassing
enrollment_mode = CourseMode.AUDIT
course_grade = '0.89'
course_grade = "0.89"
with mock.patch(
'lms.djangoapps.certificates.tasks.generate_course_certificate',
return_value=None
"lms.djangoapps.certificates.tasks.generate_course_certificate",
return_value=None,
) as mock_generate_cert:
kwargs = {
'status': status,
'student': self.user.id,
'course_key': self.course_key,
'course_grade': course_grade,
'enrollment_mode': enrollment_mode,
'generation_mode': gen_mode,
'what_about': 'dinosaurs'
"status": status,
"student": self.user.id,
"course_key": self.course_key,
"course_grade": course_grade,
"enrollment_mode": enrollment_mode,
"generation_mode": gen_mode,
"what_about": "dinosaurs",
}
generate_certificate.apply_async(kwargs=kwargs)
@@ -97,5 +103,74 @@ class GenerateUserCertificateTest(TestCase):
status=status,
enrollment_mode=enrollment_mode,
course_grade=course_grade,
generation_mode=gen_mode
generation_mode=gen_mode,
)
class ModifyCertTemplateTests(TestCase):
"""Tests for get_changed_cert_templates"""
def test_command_changes_called_templates(self):
"""Verify command changes for all and only those templates for which it is called."""
template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
template2 = CertificateTemplateFactory.create(
template="violadee-doo violadee-dah"
)
template3 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
template1.save()
template2.save()
template3.save()
expected1 = "fiddleeep-doo fiddledee-dah"
expected2 = "violaeep-doo violadee-dah"
options = {
"old_text": "dee",
"new_text": "eep",
"templates": [1, 2],
}
new_templates = get_changed_cert_templates(options)
assert len(new_templates) == 2
assert new_templates[0].template == expected1
assert new_templates[1].template == expected2
def test_dry_run(self):
"""Verify command doesn't change anything on dry-run."""
template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
template2 = CertificateTemplateFactory.create(
template="violadee-doo violadee-dah"
)
template1.save()
template2.save()
options = {
"old_text": "dee",
"new_text": "eep",
"templates": [1, 2],
"dry_run": True,
}
new_templates = get_changed_cert_templates(options)
assert not new_templates
def test_multiline_change(self):
"""Verify template change works with a multiline change string."""
template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
template1.save()
new_text = """
there's something happening here
what it is ain't exactly clear
"""
expected = f"fiddle{dedent(new_text)}-doo fiddledee-dah"
options = {
"old_text": "dee",
"new_text": dedent(new_text),
"templates": [1],
}
new_templates = get_changed_cert_templates(options)
assert len(new_templates) == 1
assert new_templates[0].template == expected