-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds a certificate template modifier #34042
Merged
deborahgu
merged 15 commits into
master
from
dkaplan1/APER-2851_replicate-share-certificate-in-facebook-improvements-for-edx.org
Jan 22, 2024
Merged
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
9657645
feat: adds a certificate template modifier
deborahgu 7bdb8ee
Merge branch 'master' into dkaplan1/APER-2851_replicate-share-certifi…
deborahgu 18027ad
feat: reordering includes
deborahgu ce48516
feat: fixing a long line problem
deborahgu 3ac66de
feat: adding migration
deborahgu 392110b
feat: pep8 fixes
deborahgu 817c8f2
Merge branch 'master' into dkaplan1/APER-2851_replicate-share-certifi…
deborahgu cccb960
feat:tests for certificate template modifier
deborahgu 7c78473
Merge branch 'master' into dkaplan1/APER-2851_replicate-share-certifi…
deborahgu f89c6ff
Merge branch 'master' into dkaplan1/APER-2851_replicate-share-certifi…
deborahgu 994b8a3
feat: adding tests to modify cert template
deborahgu 35d041e
Merge branch 'master' into dkaplan1/APER-2851_replicate-share-certifi…
deborahgu e0ef013
feat: fixing some codestyle/lint errors
deborahgu a35a6f9
feat: linter
deborahgu b4bf076
feat: fixes from code review
deborahgu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
91 changes: 91 additions & 0 deletions
91
lms/djangoapps/certificates/management/commands/modify_cert_template.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
""" | ||
|
||
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. | ||
Comment on lines
+23
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kudos 👍. Thanks for pointing out that this is a potentially dangerous and destructive action. |
||
""" | ||
|
||
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) |
44 changes: 44 additions & 0 deletions
44
lms/djangoapps/certificates/management/commands/tests/test_modify_certs_template.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
""" | ||
Tests for the modify_cert_template command | ||
""" | ||
|
||
import pytest | ||
from django.core.management import CommandError, call_command | ||
from django.test import ( | ||
TestCase, | ||
override_settings, | ||
) # lint-amnesty, pylint: disable=unused-import | ||
|
||
|
||
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_old_text(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" | ||
) |
29 changes: 29 additions & 0 deletions
29
...jangoapps/certificates/migrations/0036_modifiedcertificatetemplatecommandconfiguration.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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', | ||
}, | ||
), | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example seems incomplete at a first glance. Should this also have an example of
--old-text
and how to use the other options (like--new_text {blah}
)? I'd love to see an example of how to specify the templates too.