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

feat: adding tests to modify cert template

Browse Source 
Re-factored to make the code more testable, and added some more tests,
also improved dry run logging

FIXES: APER-2851
This commit is contained in:
Deborah Kaplan
2024-01-17 21:41:21 +00:00
parent f89c6ff677
commit 994b8a3d49
3 changed files with 96 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lms/djangoapps/certificates/management/commands/tests/test_modify_certs_template.py
View File

@@ -13,9 +13,6 @@ from django.test import (
class ModifyCertTemplateTests(TestCase):
"""Tests for the modify_cert_template management command"""
def setUp(self):
super().setUp()
def test_command_with_missing_param_old_text(self):
"""Verify command with a missing param --old-text."""
with pytest.raises(

107
lms/djangoapps/certificates/tasks.py
View File

@@ -4,7 +4,7 @@ Tasks that operate on course certificates for a user
from difflib import unified_diff
from logging import getLogger
from typing import Any, Dict
from typing import Any, Dict, List
from celery import shared_task
from celery_utils.persist_on_failure import LoggedPersistOnFailureTask, LoggedTask
@@ -61,7 +61,7 @@ def generate_certificate(self, **kwargs): # pylint: disable=unused-argument
@shared_task(base=LoggedTask, ignore_result=True)
@set_code_owner_attribute
def handle_modify_cert_template(options: Dict[str, Any]):
def handle_modify_cert_template(options: Dict[str, Any]) -> None:
"""
Celery task to handle the modify_cert_template management command.
@@ -81,7 +81,34 @@ def handle_modify_cert_template(options: Dict[str, Any]):
num=len(template_ids)
)
)
templates_changed = 0
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:
@@ -94,8 +121,8 @@ def handle_modify_cert_template(options: Dict[str, Any]):
template_id=template_id, name=template.description
)
)
new_template = get_modified_template_text(
template.template, options["old_text"], options["new_text"]
new_template = template.template.replace(
options["old_text"], options["new_text"], 1
)
if template.template == new_template:
log.info(
@@ -104,48 +131,36 @@ def handle_modify_cert_template(options: Dict[str, Any]):
)
)
else:
log.info(
"[modify_cert_template] Modifying template {template} ({description})".format(
template=template_id,
description=template.description,
)
)
templates_changed += 1
if not options["dry_run"]:
template.template = new_template
template.save()
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",
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=templates_changed)
"[modify_cert_template] Modified {num} templates".format(
num=len(templates_changed)
)
)
def get_modified_template_text(
template_text: str,
old: str,
new: str,
):
"""
Returns the original template text with the first instance of `old` replaced with `new`.
Case-sensitive.
Although this is a trivial method, it's factored into its own method to allow us to
write unit tests that can be easily modified if the testing algorithm is made more complex.
"""
return template_text.replace(old, new, 1)
return templates_changed

62
lms/djangoapps/certificates/tests/test_tasks.py
View File

@@ -16,7 +16,7 @@ from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tasks import (
generate_certificate,
handle_modify_cert_template,
get_changed_cert_templates,
)
from lms.djangoapps.certificates.tests.factories import CertificateTemplateFactory
@@ -108,52 +108,59 @@ class GenerateUserCertificateTest(TestCase):
class ModifyCertTemplateTests(TestCase):
"""Tests for handle_modify_cert_template"""
# FIXME: put in mocks for .get and .save
def setUp(self):
super().setUp()
self.cert1 = CertificateTemplateFactory
self.cert2 = CertificateTemplateFactory
self.cert3 = CertificateTemplateFactory
"""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."""
self.cert1.template = "fiddledee-doo fiddledee-dah"
self.cert2.template = "violadee-doo violadee-dah"
self.cert3.template = "fiddledee-doo fiddledee-dah"
self.template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
self.template2 = CertificateTemplateFactory.create(
template="violadee-doo violadee-dah"
)
self.template3 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
self.template1.save()
self.template2.save()
self.template3.save()
expected1 = "fiddleeep-doo fiddledee-dah"
expected2 = "violaeep-doo violadee-dah"
expected3 = "fiddledee-doo fiddledee-dah"
options = {
"old_text": "dee",
"new_text": "eep",
"templates": [1, 2],
}
handle_modify_cert_template(options)
assert self.cert1.template == expected1
assert self.cert2.template == expected2
assert self.cert3.template == expected3
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."""
self.cert1.template = "fiddledee-doo fiddledee-dah"
self.cert2.template = "violadee-doo violadee-dah"
expected1 = "fiddledee-doo fiddledee-dah"
expected2 = "violadee-doo violadee-dah"
self.template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
self.template2 = CertificateTemplateFactory.create(
template="violadee-doo violadee-dah"
)
self.template1.save()
self.template2.save()
options = {
"old_text": "dee",
"new_text": "eep",
"templates": [1, 2],
"dry_run": True,
}
handle_modify_cert_template(options)
assert self.cert1.template == expected1
assert self.cert2.template == expected2
new_templates = get_changed_cert_templates(options)
assert new_templates == []
def test_multiline_change(self):
"""Verify template change works with a multiline change string."""
self.cert1.template = "fiddledee-doo fiddledee-dah"
self.template1 = CertificateTemplateFactory.create(
template="fiddledee-doo fiddledee-dah"
)
self.template1.save()
new_text = """
there's something happening here
what it is ain't exactly clear
@@ -164,5 +171,6 @@ class ModifyCertTemplateTests(TestCase):
"new_text": dedent(new_text),
"templates": [1],
}
handle_modify_cert_template(options)
assert self.cert1.template == expected
new_templates = get_changed_cert_templates(options)
assert len(new_templates) == 1
assert new_templates[0].template == expected