Skip to content

Commit

Permalink
Notifications: use Template's Django engine to render them
Browse files Browse the repository at this point in the history 
We dealt a few times when rendering notifications using simple Python's
`.format()` and we changed our mind to use Django's engine instead.

Closes #11022
  • Loading branch information
@humitos
humitos committed Jan 10, 2024
1 parent 4a864c3 commit de42c5e
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 94 deletions.
54 changes: 27 additions & 27 deletions readthedocs/config/notifications.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
body=_(
textwrap.dedent(
"""
Configuration file not found in <code>{directory}</code>.
Configuration file not found in <code>{{directory}}</code>.
"""
).strip(),
),
Expand All @@ -53,7 +53,7 @@
body=_(
textwrap.dedent(
"""
The <code>{key}</code> configuration option is not supported in this version.
The <code>{{key}}</code> configuration option is not supported in this version.
"""
).strip(),
),
Expand Down Expand Up @@ -105,11 +105,11 @@
body=_(
textwrap.dedent(
"""
Invalid configuration option: <code>{key}</code>.
Invalid configuration option: <code>{{key}}</code>.
Read the Docs configuration file: <code>{source_file}</code>.
Read the Docs configuration file: <code>{{source_file}}</code>.
<code>{error_message}</code>
<code>{{error_message}}</code>
"""
).strip(),
),
Expand Down Expand Up @@ -145,7 +145,7 @@
body=_(
textwrap.dedent(
"""
The name of the packages (e.g. <code>{package}</code>) can't start with <code>{prefix}</code>
The name of the packages (e.g. <code>{{package}}</code>) can't start with <code>{{prefix}}</code>
"""
).strip(),
),
Expand All @@ -157,7 +157,7 @@
body=_(
textwrap.dedent(
"""
The name of the package <code>{pacakge}</name> is invalid.
The name of the package <code>{{pacakge}}</name> is invalid.
"""
).strip(),
),
Expand Down Expand Up @@ -213,11 +213,11 @@
),
Message(
id=ConfigError.INVALID_KEY_NAME,
header=_("Invalid configuration key: {key}"),
header=_("Invalid configuration key: {{key}}"),
body=_(
textwrap.dedent(
"""
Make sure the key name <code>{key}</code> is correct.
Make sure the key name <code>{{key}}</code> is correct.
"""
).strip(),
),
Expand All @@ -229,9 +229,9 @@
body=_(
textwrap.dedent(
"""
Error while parsing <code>{filename}</code>.
Error while parsing <code>{{filename}}</code>.
{error_message}
{{error_message}}
"""
).strip(),
),
Expand All @@ -248,8 +248,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Expected one of (0, 1, true, false), got <code>{value}</code>.
Config validation error in <code>{{key}}</code>.
Expected one of (0, 1, true, false), got <code>{{value}}</code>.
"""
).strip(),
),
Expand All @@ -261,8 +261,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Expected one of ({choices}), got <code>{value}</code>.
Config validation error in <code>{{key}}</code>.
Expected one of ({{choices}}), got <code>{{value}}</code>.
"""
).strip(),
),
Expand All @@ -274,8 +274,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Expected a dictionary, got <code>{value}</code>.
Config validation error in <code>{{key}}</code>.
Expected a dictionary, got <code>{{value}}</code>.
"""
).strip(),
),
Expand All @@ -287,8 +287,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
The path <code>{value}</code> does not exist.
Config validation error in <code>{{key}}</code>.
The path <code>{{value}}</code> does not exist.
"""
).strip(),
),
Expand All @@ -300,8 +300,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
The path <code>{value}</code> is not a valid path pattern.
Config validation error in <code>{{key}}</code>.
The path <code>{{value}}</code> is not a valid path pattern.
"""
).strip(),
),
Expand All @@ -313,8 +313,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Expected a string, got <code>{value}</code>.
Config validation error in <code>{{key}}</code>.
Expected a string, got <code>{{value}}</code>.
"""
).strip(),
),
Expand All @@ -326,8 +326,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Expected a list, got <code>{value}</code>.
Config validation error in <code>{{key}}</code>.
Expected a list, got <code>{{value}}</code>.
"""
).strip(),
),
Expand All @@ -339,8 +339,8 @@
body=_(
textwrap.dedent(
"""
Config validation error in <code>{key}</code>.
Value <code>{value}</code> not found.
Config validation error in <code>{{key}}</code>.
Value <code>{{value}}</code> not found.
"""
).strip(),
),
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/notifications.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
textwrap.dedent(
"""
Your primary email address is not verified.
Please <a href="{account_email_url}">verify it here</a>.
Please <a href="{{account_email_url}}">verify it here</a>.
"""
).strip(),
),
Expand Down
12 changes: 6 additions & 6 deletions readthedocs/domains/notifications.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class PendingCustomDomainValidation(EmailNotification):
messages = [
Message(
id=MESSAGE_DOMAIN_VALIDATION_PENDING,
header=_("Pending configuration of custom domain: {domain}"),
header=_("Pending configuration of custom domain: {{domain}}"),
body=_(
textwrap.dedent(
"""
The configuration of your custom domain <code>{domain}</code> is pending.
Go to the <a href="{domain_url}">domain page</a> and follow the instructions to complete it.
The configuration of your custom domain <code>{{domain}}</code> is pending.
Go to the <a href="{{domain_url}}">domain page</a> and follow the instructions to complete it.
"""
).strip(),
),
Expand All @@ -36,12 +36,12 @@ class PendingCustomDomainValidation(EmailNotification):
# ``message_id``
Message(
id=MESSAGE_DOMAIN_VALIDATION_EXPIRED,
header=_("Validation of custom domain expired: {domain}"),
header=_("Validation of custom domain expired: {{domain}}"),
body=_(
textwrap.dedent(
"""
The validation period of your custom domain <code>{domain}</code> has ended.
Go to the <a href="{domain_url}">domain page</a> and click on "Save" to restart the process.
The validation period of your custom domain <code>{{domain}}</code> has ended.
Go to the <a href="{{domain_url}}">domain page</a> and click on "Save" to restart the process.
"""
).strip(),
),
Expand Down
61 changes: 17 additions & 44 deletions readthedocs/notifications/messages.py
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import textwrap
from collections import defaultdict

import structlog
from django.utils.html import escape
from django.template import Context, Template
from django.utils.translation import gettext_noop as _

from readthedocs.doc_builder.exceptions import (
Expand Down Expand Up @@ -34,26 +33,8 @@ def __repr__(self):
def __str__(self):
return f"Message: {self.id} | {self.header}"

def _escape_format_values(self, format_values):
"""
Escape all potential HTML tags included in format values.
This is a protection against rendering potential values defined by the user.
It uses the Django's util function ``escape`` (similar to ``|escape`` template tag filter)
to convert HTML characters into regular characters.
NOTE: currently, we don't support values that are not ``str`` or ``int``.
If we want to support other types or nested dictionaries,
we will need to iterate recursively to apply the ``escape`` function.
"""
return {
key: escape(value)
for key, value in format_values.items()
if isinstance(value, (str, int))
}

def set_format_values(self, format_values):
self.format_values = self._escape_format_values(format_values)
self.format_values = format_values

def get_display_icon_classes(self):
if self.icon_classes:
Expand All @@ -78,20 +59,12 @@ def get_display_icon_classes(self):
return " ".join(classes)

def get_rendered_header(self):
try:
return self.header.format(**self.format_values)
except KeyError:
# There was a key missing
log.exception("There was a missing key when formating a header's Message.")
return self.header.format_map(defaultdict(str, **self.format_values))
template = Template(self.header)
return template.render(context=Context(self.format_values))

def get_rendered_body(self):
try:
return self.body.format(**self.format_values)
except KeyError:
# There was a key missing
log.exception("There was a missing key when formating a body's Message.")
return self.body.format_map(defaultdict(str, **self.format_values))
template = Template(self.body)
return template.render(context=Context(self.format_values))


# TODO: review the copy of these notifications/messages on PR review and adapt them.
Expand All @@ -109,7 +82,7 @@ def get_rendered_body(self):
There was a problem with Read the Docs while building your documentation.
Please try again later.
If this problem persists,
report this error to us with your build id ({instance.pk}).
report this error to us with your build id ({{instance.pk}}).
"""
).strip(),
),
Expand All @@ -123,7 +96,7 @@ def get_rendered_body(self):
"""
This build was terminated due to inactivity.
If you continue to encounter this error,
file a support request and reference this build id ({instance.pk}).
file a support request and reference this build id ({{instance.pk}}).
"""
).strip(),
),
Expand All @@ -149,7 +122,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Concurrency limit reached ({limit}), retrying in 5 minutes.
Concurrency limit reached ({{limit}}), retrying in 5 minutes.
"""
).strip(),
),
Expand Down Expand Up @@ -210,7 +183,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Build exited due to unknown error: {message}
Build exited due to unknown error: {{message}}
"""
).strip(),
),
Expand All @@ -236,7 +209,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Your project, organization, or user is currently building the maximum concurrency builds allowed ({limit}).
Your project, organization, or user is currently building the maximum concurrency builds allowed ({{limit}}).
It will automatically retry in 5 minutes.
"""
).strip(),
Expand All @@ -261,7 +234,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Build output directory for format "{artifact_type}" is not a directory.
Build output directory for format "{{artifact_type}}" is not a directory.
"""
).strip(),
),
Expand All @@ -273,7 +246,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Build output directory for format "{artifact_type}" does not contain any files.
Build output directory for format "{{artifact_type}}" does not contain any files.
It seems the build process created the directory but did not save any file to it.
"""
).strip(),
Expand All @@ -286,9 +259,9 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Build output directory for format "{artifact_type}" contains multiple files
Build output directory for format "{{artifact_type}}" contains multiple files
and it is not currently supported.
Please, remove all the files but the "{artifact_type}" you want to upload.
Please, remove all the files but the "{{artifact_type}}" you want to upload.
"""
).strip(),
),
Expand Down Expand Up @@ -421,7 +394,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
Problem parsing MkDocs YAML configuration. {exception}
Problem parsing MkDocs YAML configuration. {{exception}}
"""
).strip(),
),
Expand Down Expand Up @@ -457,7 +430,7 @@ def get_rendered_body(self):
body=_(
textwrap.dedent(
"""
The "{config}" config from your MkDocs YAML config file has to be a
The "{{config}}" config from your MkDocs YAML config file has to be a
list of relative paths.
"""
).strip(),
Expand Down
Loading

0 comments on commit de42c5e

Please sign in to comment.