Allow adding/editing 'mail_template' for a release. #2494
Conversation
ghost
self-requested a review
as a 
|
jenkies test |
|
The CI failure is due to https://bugzilla.redhat.com/show_bug.cgi?id=1607916, and the fix for that might take a day or so to propagate to the mirrors. |
| @@ -229,6 +231,7 @@ def test_new_release_invalid_tags(self): | |||
| "candidate_tag": "epel42-candidate", | |||
| "override_tag": "epel42-override", | |||
| "csrf_token": self.get_csrf_token(), | |||
| "mail_template": "fedora_errata_template" | |||
There was a problem hiding this comment.
Why add mail_template to all these existing tests?
There was a problem hiding this comment.
Current logic requires mail_template while creating new release. Should we add default template fedora_errata_template when it is not provided?
There was a problem hiding this comment.
Yeah I think that's a sane default. Most new releases are going to be Fedora, and the Fedora template is probably fine for other releases anyway.
| @@ -291,7 +294,8 @@ def test_query_releases_html_two_releases_same_state(self): | |||
| "pending_signing_tag": "f42-updates-testing-signing", | |||
| "pending_testing_tag": "f42-updates-testing-pending", | |||
| "override_tag": "f42-override", | |||
| "csrf_token": self.get_csrf_token()} | |||
| "csrf_token": self.get_csrf_token(), | |||
| "mail_template": "fedora_errata_template"} | |||
There was a problem hiding this comment.
We need a test that shows that mail template can be changed via the API and that it alters the db.
bodhi/tests/client/test___init__.py
Outdated
| result = runner.invoke( | ||
| client.edit_release, | ||
| ['--name', 'F27', '--mail-template', 'edited_fedora_errata_template', '--url', | ||
| 'http://localhost:6543', '--username', 'bowlofeggs', '--password', 's3kr3t']) |
There was a problem hiding this comment.
We don't need the url, username, or password arguments here and the test will be cleaner without them.
bodhi/client/__init__.py
Outdated
| @@ -132,6 +132,7 @@ def _warn_if_url_and_staging_set(ctx, param, value): | |||
| click.option('--state', type=click.Choice(['disabled', 'pending', 'current', | |||
| 'archived']), | |||
| help='The state of the release'), | |||
| click.option('--mail-template', help='Name of the email template for this release'), | |||
| staging_option, | |||
| url_option] | |||
|
|
|||
There was a problem hiding this comment.
The CLI should also display the mail template to the user when a release is printed to the terminal.
bodhi/server/schemas.py
Outdated
| mail_template = colander.SchemaNode( | ||
| colander.String(), | ||
| missing="fedora_errata_template", | ||
| ) |
There was a problem hiding this comment.
We should validate that the referenced template exists, otherwise users could make a typo and cause the server to fail during mashing:
[vagrant@bodhi-dev bodhi]$ bodhi releases edit --name F28 --mail-template does_not_exist
Saved release:
Name: F28
Long Name: Fedora 28
Version: 28
Branch: f28
ID Prefix: FEDORA
Dist Tag: f28
Stable Tag: f28-updates
Testing Tag: f28-updates-testing
Candidate Tag: f28-updates-candidate
Pending Signing Tag: f28-signing-pending
Pending Testing Tag: f28-updates-testing-pending
Pending Stable Tag: f28-updates-pending
Override Tag: f28-override
State: current
Email Template: does_not_exist
fixes #2475 Signed-off-by: Vismay Golwala <[email protected]>
| location = config.get('mail.templates_basepath') | ||
| directory = get_absolute_path(location) | ||
| template_list = [os.path.splitext(file)[0] for file in os.listdir(directory)] | ||
| template_vals = ", ".join(template_list) |
There was a problem hiding this comment.
Does this test need the four lines above? It seems like they are not used.
There was a problem hiding this comment.
Yes, to keep it compatible with Python 2 and 3. Because both versions of python add elements to list in different order and hence we cannot directly compare string versions of those.
There was a problem hiding this comment.
I hadn't noticed it was in fact used on line 308 below, so nevermind!
fixes #2475
Signed-off-by: Vismay Golwala [email protected]