emails: Inline CSS in emails in build_email.

Previously, we had an architecture where CSS inlining for emails was
done at provision time in inline_email_css.py. This was necessary
because the library we were using for this, Premailer, was extremely
slow, and doing the inlining for every outgoing email would have been
prohibitively expensive.

Now that we've migrated to a more modern library that inlines the
small amount of CSS we have into emails nearly instantly, we are able
to remove the complex architecture built to work around Premailer
being slow and just do the CSS inlining as the final step in sending
each individual email.

This has several significant benefits:

* Removes a fiddly provisioning step that made the edit/refresh cycle
  for modifying email templates confusing; there's no longer a CSS
  inlining step that, if you forget to do it, results in your testing a
  stale variant of the email templates.
* Fixes internationalization problems related to translators working
  with pre-CSS-inlined emails, and then Django trying to apply the
  translators to the post-CSS-inlined version.
* Makes the send_custom_email pipeline simpler and easier to improve.

Signed-off-by: Daniil Fadeev <fadeevd@zulip.com>
This commit is contained in:
Daniil Fadeev 2023-04-05 13:19:58 +04:00 committed by Tim Abbott
parent 7202a98438
commit 2f203f4de1
33 changed files with 91 additions and 214 deletions

View File

@ -13,12 +13,12 @@ with only a few things you need to know to get started.
- All email templates are in `templates/zerver/emails/`. Each email has three - All email templates are in `templates/zerver/emails/`. Each email has three
template files: `<template_prefix>.subject.txt`, `<template_prefix>.txt`, and template files: `<template_prefix>.subject.txt`, `<template_prefix>.txt`, and
`<template_prefix>.source.html`. Email templates, along with all other templates `<template_prefix>.html`. Email templates, along with all other templates
in the `templates/` directory, are Jinja2 templates. in the `templates/` directory, are Jinja2 templates.
- Most of the CSS and HTML layout for emails is in `email_base.html`. Note - Most of the CSS and HTML layout for emails is in `email_base_default.html`. Note
that email has to ship with all of its CSS and HTML, so nothing in that email has to ship with all of its CSS and HTML, so nothing in
`static/` is useful for an email. If you're adding new CSS or HTML for an `static/` is useful for an email. If you're adding new CSS or HTML for an
email, there's a decent chance it should go in `email_base.html`. email, there's a decent chance it should go in `email_base_default.html`.
- All email is eventually sent by `zerver.lib.send_email.send_email`. There - All email is eventually sent by `zerver.lib.send_email.send_email`. There
are several other functions in `zerver.lib.send_email`, but all of them are several other functions in `zerver.lib.send_email`, but all of them
eventually call the `send_email` function. The most interesting one is eventually call the `send_email` function. The most interesting one is
@ -104,11 +104,6 @@ email_password = gmail_password
### Notes ### Notes
- After changing any HTML email or `email_base.html`, you need to run
`scripts/setup/inline_email_css.py` for the changes to be reflected
in the development environment. The script generates files like
`templates/zerver/emails/compiled/<template_prefix>.html`.
- Images won't be displayed in a real email client unless you change - Images won't be displayed in a real email client unless you change
the `base_image_uri` used for emails to a public URL such as the `base_image_uri` used for emails to a public URL such as
`https://chat.zulip.org/static/images/emails` (image links to `https://chat.zulip.org/static/images/emails` (image links to
@ -134,29 +129,18 @@ using a combination of the
[css-inline](https://github.com/Stranger6667/css-inline) library and having [css-inline](https://github.com/Stranger6667/css-inline) library and having
two copies of each email (plain-text and HTML). two copies of each email (plain-text and HTML).
So for each email, there are two source templates: the `.txt` version So, for each email, there are two source templates: the `.txt` version
(for plain-text format) as well as a `.source.html` template. The (for plain-text format) as well as a `.html` template. The `.txt` version
`.txt` version is used directly; while the `.source.html` template is is used directly, while `.html` is processed by `css-inline`, which injects
processed by `scripts/setup/inline_email_css.py` (generating a `.html` template the CSS we use for styling our emails (`templates/zerver/emails/email.css`)
under `templates/zerver/emails/compiled`); that tool (powered by into the templates just before sending an email.
`css-inline`) injects the CSS we use for styling our emails
(`templates/zerver/emails/email.css`) into the templates inline.
What this means is that when you're editing emails, **you need to run
`scripts/setup/inline_email_css.py`** after making changes to see the changes
take effect. Our tooling automatically runs this as part of
`tools/provision` and production deployments; but you should bump
`PROVISION_VERSION` when making changes to emails that change test
behavior, or other developers will get test failures until they
provision.
While this model is great for the markup side, it isn't ideal for While this model is great for the markup side, it isn't ideal for
[translations](../translating/translating.md). The Django [translations](../translating/translating.md). The Django
translation system works with exact strings, and having different new translation system works with exact strings, and having different new
markup can require translators to re-translate strings, which can markup can require translators to re-translate strings, which can
result in problems like needing 2 copies of each string (one for result in problems like needing 2 copies of each string (one for
plain-text, one for HTML) and/or needing to re-translate a bunch of plain-text, one for HTML). Re-translating these strings is
strings after making a CSS tweak. Re-translating these strings is
relatively easy in Transifex, but annoying. relatively easy in Transifex, but annoying.
So when writing email templates, we try to translate individual So when writing email templates, we try to translate individual
@ -166,7 +150,7 @@ translators to not have to deal with multiple versions of each string
in our emails. in our emails.
One can test whether you did the translating part right by running One can test whether you did the translating part right by running
`scripts/setup/inline_email_css.py && manage.py makemessages` and then searching `manage.py makemessages` and then searching
for the strings in `locale/en/LC_MESSAGES/django.po`; if there for the strings in `locale/en/LC_MESSAGES/django.po`; if there
are multiple copies or they contain CSS colors, you did it wrong. are multiple copies or they contain CSS colors, you did it wrong.
@ -179,5 +163,5 @@ code path for the "you don't have an account email" might not be,
since we might not know what language to use in the second case. since we might not know what language to use in the second case.
Future work in this space could be to actually generate the plain-text Future work in this space could be to actually generate the plain-text
versions of emails from the `.source.html` markup, so that we don't versions of emails from the `.html` markup, so that we don't
need to maintain two copies of each email's text. need to maintain two copies of each email's text.

View File

@ -1,91 +0,0 @@
#!/usr/bin/env python3
import os
from typing import Set
import css_inline
ZULIP_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "../../")
EMAIL_TEMPLATES_PATH = os.path.join(ZULIP_PATH, "templates", "zerver", "emails")
CSS_SOURCE_PATH = os.path.join(EMAIL_TEMPLATES_PATH, "email.css")
def get_inliner_instance() -> css_inline.CSSInliner:
with open(CSS_SOURCE_PATH) as file:
content = file.read()
return css_inline.CSSInliner(extra_css=content)
inliner = get_inliner_instance()
def inline_template(template_source_name: str) -> None:
template_name = template_source_name.split(".source.html")[0]
template_path = os.path.join(EMAIL_TEMPLATES_PATH, template_source_name)
compiled_template_path = os.path.join(
os.path.dirname(template_path), "compiled", os.path.basename(template_name) + ".html"
)
os.makedirs(os.path.dirname(compiled_template_path), exist_ok=True)
with open(template_path) as template_source_file:
template_str = template_source_file.read()
output = inliner.inline(template_str)
output = escape_jinja2_characters(output)
# Inline method of css-inline will try to complete the DOM tree,
# adding html, head, and body tags if they aren't there.
# While this is correct for the email_base_default template,
# it is wrong for the other templates that extend this
# template, since we'll end up with 2 copies of those tags.
# Thus, we strip this stuff out if the template extends
# another template.
if template_name not in ["email_base_default", "email_base_marketing", "macros"]:
output = strip_unnecessary_tags(output)
if (
"zerver/emails/compiled/email_base_default.html" in output
or "zerver/emails/compiled/email_base_marketing.html" in output
or "zerver/emails/email_base_messages.html" in output
):
assert output.count("<html>") == 0
assert output.count("<body>") == 0
assert output.count("</html>") == 0
assert output.count("</body>") == 0
with open(compiled_template_path, "w") as final_template_file:
final_template_file.write(output)
def escape_jinja2_characters(text: str) -> str:
escaped_jinja2_characters = [("%7B%7B%20", "{{ "), ("%20%7D%7D", " }}"), ("&gt;", ">")]
for escaped, original in escaped_jinja2_characters:
text = text.replace(escaped, original)
return text
def strip_unnecessary_tags(text: str) -> str:
end_block = "</body></html>"
start_block = "{% extends"
start = text.find(start_block)
end = text.rfind(end_block)
if start != -1 and end != -1:
text = text[start:end]
return text
else:
raise ValueError(f"Template does not have {start_block} or {end_block}")
def get_all_templates_from_directory(directory: str) -> Set[str]:
result = set()
for f in os.listdir(directory):
if f.endswith(".source.html"):
result.add(f)
return result
if __name__ == "__main__":
templates_to_inline = get_all_templates_from_directory(EMAIL_TEMPLATES_PATH)
for template_source_name in templates_to_inline:
inline_template(template_source_name)

View File

@ -45,9 +45,6 @@
</tr> </tr>
<tr> <tr>
<td><a href="/emails">/emails</a></td> <td><a href="/emails">/emails</a></td>
<td><code>./scripts/setup/inline_email_css.py</code><br />
Run the command if you made changes to source.html email templates.
</td>
<td>View outgoing and example emails.</td> <td>View outgoing and example emails.</td>
</tr> </tr>
<tr> <tr>

View File

@ -16,7 +16,7 @@
<h1> Zulip digest </h1> <h1> Zulip digest </h1>
</div> </div>
<div class="digest-email-html"> <div class="digest-email-html">
{% include 'zerver/emails/compiled/digest.html' %} {% include 'zerver/emails/digest.html' %}
<img id="digest-footer" src="{{ static('images/emails/footer.png') }}"/> <img id="digest-footer" src="{{ static('images/emails/footer.png') }}"/>
</div> </div>
<br /> <br />

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/> <img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_marketing.html" %} {% extends "zerver/emails/email_base_marketing.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% import 'zerver/emails/compiled/macros.html' as macros %} {% import 'zerver/emails/macros.html' as macros %}
<!DOCTYPE html> <!DOCTYPE html>
<html lang="en"> <html lang="en">
<head> <head>

View File

@ -1,4 +1,4 @@
{% import 'zerver/emails/compiled/macros.html' as macros %} {% import 'zerver/emails/macros.html' as macros %}
<!DOCTYPE html> <!DOCTYPE html>
<html lang="en"> <html lang="en">
<head> <head>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/day2_1.png" alt=""/> <img src="{{ email_images_base_uri }}/day2_1.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/> <img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/invitation_reminder.png" alt=""/> <img src="{{ email_images_base_uri }}/invitation_reminder.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block preheader %} {% block preheader %}
{% trans organization_url=realm_uri %}Organization: {{ organization_url }} Time: {{ login_time }} Email: {{ user_email }}{% endtrans %} {% trans organization_url=realm_uri %}Organization: {{ organization_url }} Time: {{ login_time }} Email: {{ user_email }}{% endtrans %}

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/email_logo.png" alt=""/> <img src="{{ email_images_base_uri }}/email_logo.png" alt=""/>

View File

@ -1,4 +1,4 @@
{% extends "zerver/emails/compiled/email_base_default.html" %} {% extends "zerver/emails/email_base_default.html" %}
{% block illustration %} {% block illustration %}
<img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/> <img src="{{ email_images_base_uri }}/registration_confirmation.png" alt=""/>

View File

@ -109,7 +109,7 @@ echo "$version" >version
cd "$OUTPUT_DIR" cd "$OUTPUT_DIR"
tar --append -f "$TARBALL" "$prefix/prod-static" "$prefix/build_id" "$prefix/version" "$prefix/zulip-git-version" "$prefix/locale" "$prefix/staticfiles.json" "$prefix/templates/zerver/emails/compiled" "$prefix/webpack-stats-production.json" tar --append -f "$TARBALL" "$prefix/prod-static" "$prefix/build_id" "$prefix/version" "$prefix/zulip-git-version" "$prefix/locale" "$prefix/staticfiles.json" "$prefix/webpack-stats-production.json"
rm -rf "$prefix" rm -rf "$prefix"

View File

@ -23,7 +23,7 @@ EXCLUDED_FILES = [
## Test data Files for testing modules in tests ## Test data Files for testing modules in tests
"tools/tests/test_template_data", "tools/tests/test_template_data",
# Our parser doesn't handle the way its conditionals are layered # Our parser doesn't handle the way its conditionals are layered
"templates/zerver/emails/missed_message.source.html", "templates/zerver/emails/missed_message.html",
# Previously unchecked and our parser doesn't like its indentation # Previously unchecked and our parser doesn't like its indentation
"web/images/icons/template.hbs", "web/images/icons/template.hbs",
# Template checker recommends very hard to read indentation. # Template checker recommends very hard to read indentation.

View File

@ -72,15 +72,6 @@ def compilemessages_paths() -> List[str]:
return paths return paths
def inline_email_css_paths() -> List[str]:
paths = [
"scripts/setup/inline_email_css.py",
"templates/zerver/emails/email.css",
]
paths += glob.glob("templates/zerver/emails/*.source.html")
return paths
def configure_rabbitmq_paths() -> List[str]: def configure_rabbitmq_paths() -> List[str]:
paths = [ paths = [
"scripts/setup/configure-rabbitmq", "scripts/setup/configure-rabbitmq",
@ -180,16 +171,6 @@ def need_to_run_compilemessages() -> bool:
) )
def need_to_run_inline_email_css() -> bool:
if not os.path.exists("templates/zerver/emails/compiled/"):
return True
return is_digest_obsolete(
"last_email_source_files_hash",
inline_email_css_paths(),
)
def need_to_run_configure_rabbitmq(settings_list: List[str]) -> bool: def need_to_run_configure_rabbitmq(settings_list: List[str]) -> bool:
obsolete = is_digest_obsolete( obsolete = is_digest_obsolete(
"last_configure_rabbitmq_hash", "last_configure_rabbitmq_hash",
@ -246,15 +227,6 @@ def main(options: argparse.Namespace) -> int:
else: else:
print("No need to run `tools/setup/build_timezone_values`.") print("No need to run `tools/setup/build_timezone_values`.")
if options.is_force or need_to_run_inline_email_css():
run(["scripts/setup/inline_email_css.py"])
write_new_digest(
"last_email_source_files_hash",
inline_email_css_paths(),
)
else:
print("No need to run `scripts/setup/inline_email_css.py`.")
if not options.is_build_release_tarball_only: if not options.is_build_release_tarball_only:
# The following block is skipped when we just need the development # The following block is skipped when we just need the development
# environment to build a release tarball. # environment to build a release tarball.

View File

@ -33,9 +33,6 @@ setup_node_modules(production=True)
# Build emoji # Build emoji
run(["./tools/setup/emoji/build_emoji"]) run(["./tools/setup/emoji/build_emoji"])
# Inline CSS in emails
run(["./scripts/setup/inline_email_css.py"])
# Copy over static files from the zulip_bots package # Copy over static files from the zulip_bots package
run(["./tools/setup/generate_zulip_bots_static_files.py"]) run(["./tools/setup/generate_zulip_bots_static_files.py"])

View File

@ -11,6 +11,7 @@ from email.utils import formataddr, parseaddr
from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple, Union from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple, Union
import backoff import backoff
import css_inline
import orjson import orjson
from django.conf import settings from django.conf import settings
from django.core.mail import EmailMultiAlternatives, get_connection from django.core.mail import EmailMultiAlternatives, get_connection
@ -21,18 +22,19 @@ from django.core.management import CommandError
from django.db import transaction from django.db import transaction
from django.http import HttpRequest from django.http import HttpRequest
from django.template import loader from django.template import loader
from django.template.exceptions import TemplateDoesNotExist
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
from django.utils.translation import gettext as _ from django.utils.translation import gettext as _
from django.utils.translation import override as override_language from django.utils.translation import override as override_language
from confirmation.models import generate_key, one_click_unsubscribe_link from confirmation.models import generate_key, one_click_unsubscribe_link
from scripts.setup.inline_email_css import inline_template
from zerver.lib.logging_util import log_to_file from zerver.lib.logging_util import log_to_file
from zerver.models import EMAIL_TYPES, Realm, ScheduledEmail, UserProfile, get_user_profile_by_id from zerver.models import EMAIL_TYPES, Realm, ScheduledEmail, UserProfile, get_user_profile_by_id
from zproject.email_backends import EmailLogBackEnd, get_forward_address from zproject.email_backends import EmailLogBackEnd, get_forward_address
MAX_CONNECTION_TRIES = 3 MAX_CONNECTION_TRIES = 3
ZULIP_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "../../")
EMAIL_TEMPLATES_PATH = os.path.join(ZULIP_PATH, "templates", "zerver", "emails")
CSS_SOURCE_PATH = os.path.join(EMAIL_TEMPLATES_PATH, "email.css")
## Logging setup ## ## Logging setup ##
@ -40,6 +42,12 @@ logger = logging.getLogger("zulip.send_email")
log_to_file(logger, settings.EMAIL_LOG_PATH) log_to_file(logger, settings.EMAIL_LOG_PATH)
def get_inliner_instance() -> css_inline.CSSInliner:
with open(CSS_SOURCE_PATH) as file:
content = file.read()
return css_inline.CSSInliner(extra_css=content)
class FromAddress: class FromAddress:
SUPPORT = parseaddr(settings.ZULIP_ADMINISTRATOR)[1] SUPPORT = parseaddr(settings.ZULIP_ADMINISTRATOR)[1]
NOREPLY = parseaddr(settings.NOREPLY_EMAIL_ADDRESS)[1] NOREPLY = parseaddr(settings.NOREPLY_EMAIL_ADDRESS)[1]
@ -124,6 +132,10 @@ def build_email(
"physical_address": settings.PHYSICAL_ADDRESS, "physical_address": settings.PHYSICAL_ADDRESS,
} }
def get_inlined_template(template: str) -> str:
inliner = get_inliner_instance()
return inliner.inline(template)
def render_templates() -> Tuple[str, str, str]: def render_templates() -> Tuple[str, str, str]:
email_subject = ( email_subject = (
loader.render_to_string( loader.render_to_string(
@ -136,14 +148,8 @@ def build_email(
template_prefix + ".txt", context=context, using="Jinja2_plaintext" template_prefix + ".txt", context=context, using="Jinja2_plaintext"
) )
try: html_message = loader.render_to_string(template_prefix + ".html", context)
html_message = loader.render_to_string(template_prefix + ".html", context) return (get_inlined_template(html_message), message, email_subject)
except TemplateDoesNotExist:
emails_dir = os.path.dirname(template_prefix)
template = os.path.basename(template_prefix)
compiled_template_prefix = os.path.join(emails_dir, "compiled", template)
html_message = loader.render_to_string(compiled_template_prefix + ".html", context)
return (html_message, message, email_subject)
# The i18n story for emails is a bit complicated. For emails # The i18n story for emails is a bit complicated. For emails
# going to a single user, we want to use the language that user # going to a single user, we want to use the language that user
@ -518,13 +524,12 @@ def send_custom_email(
parsed_email_template = Parser(policy=default).parsestr(text) parsed_email_template = Parser(policy=default).parsestr(text)
email_template_hash = hashlib.sha256(text.encode()).hexdigest()[0:32] email_template_hash = hashlib.sha256(text.encode()).hexdigest()[0:32]
email_filename = f"custom/custom_email_{email_template_hash}.source.html"
email_id = f"zerver/emails/custom/custom_email_{email_template_hash}" email_id = f"zerver/emails/custom/custom_email_{email_template_hash}"
markdown_email_base_template_path = "templates/zerver/emails/custom_email_base.pre.html" markdown_email_base_template_path = "templates/zerver/emails/custom_email_base.pre.html"
html_source_template_path = f"templates/{email_id}.source.html" html_template_path = f"templates/{email_id}.html"
plain_text_template_path = f"templates/{email_id}.txt" plain_text_template_path = f"templates/{email_id}.txt"
subject_path = f"templates/{email_id}.subject.txt" subject_path = f"templates/{email_id}.subject.txt"
os.makedirs(os.path.dirname(html_source_template_path), exist_ok=True) os.makedirs(os.path.dirname(html_template_path), exist_ok=True)
# First, we render the Markdown input file just like our # First, we render the Markdown input file just like our
# user-facing docs with render_markdown_path. # user-facing docs with render_markdown_path.
@ -536,18 +541,13 @@ def send_custom_email(
rendered_input = render_markdown_path(plain_text_template_path.replace("templates/", "")) rendered_input = render_markdown_path(plain_text_template_path.replace("templates/", ""))
# And then extend it with our standard email headers. # And then extend it with our standard email headers.
with open(html_source_template_path, "w") as f: with open(html_template_path, "w") as f:
with open(markdown_email_base_template_path) as base_template: with open(markdown_email_base_template_path) as base_template:
# Note that we're doing a hacky non-Jinja2 substitution here; f.write(base_template.read())
# we do this because the normal render_markdown_path ordering
# doesn't commute properly with inline_email_css.
f.write(base_template.read().replace("{{ rendered_input }}", rendered_input))
with open(subject_path, "w") as f: with open(subject_path, "w") as f:
f.write(get_header(options.get("subject"), parsed_email_template.get("subject"), "subject")) f.write(get_header(options.get("subject"), parsed_email_template.get("subject"), "subject"))
inline_template(email_filename)
# Finally, we send the actual emails. # Finally, we send the actual emails.
for user_profile in users: for user_profile in users:
if options.get("admins_only") and not user_profile.is_realm_admin: if options.get("admins_only") and not user_profile.is_realm_admin:
@ -556,6 +556,7 @@ def send_custom_email(
"realm_uri": user_profile.realm.uri, "realm_uri": user_profile.realm.uri,
"realm_name": user_profile.realm.name, "realm_name": user_profile.realm.name,
"unsubscribe_link": one_click_unsubscribe_link(user_profile, "marketing"), "unsubscribe_link": one_click_unsubscribe_link(user_profile, "marketing"),
"rendered_input": rendered_input,
} }
with suppress(EmailNotDeliveredError): with suppress(EmailNotDeliveredError):
send_email( send_email(

View File

@ -150,7 +150,6 @@ class Command(makemessages.Command):
try: try:
ignore_patterns = options.get("ignore_patterns", []) ignore_patterns = options.get("ignore_patterns", [])
ignore_patterns.append("docs/*") ignore_patterns.append("docs/*")
ignore_patterns.append("templates/zerver/emails/compiled/*")
ignore_patterns.append("templates/zerver/emails/custom/*") ignore_patterns.append("templates/zerver/emails/custom/*")
ignore_patterns.append("var/*") ignore_patterns.append("var/*")
options["ignore_patterns"] = ignore_patterns options["ignore_patterns"] = ignore_patterns

View File

@ -101,7 +101,7 @@ class TestCustomEmails(ZulipTestCase):
def test_send_custom_email_headers(self) -> None: def test_send_custom_email_headers(self) -> None:
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
markdown_template_path = ( markdown_template_path = (
"zerver/tests/fixtures/email/custom_emails/email_base_headers_test.source.html" "zerver/tests/fixtures/email/custom_emails/email_base_headers_test.html"
) )
send_custom_email( send_custom_email(
[hamlet], [hamlet],
@ -120,7 +120,9 @@ class TestCustomEmails(ZulipTestCase):
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
from_name = "from_name_test" from_name = "from_name_test"
email_subject = "subject_test" email_subject = "subject_test"
markdown_template_path = "zerver/tests/fixtures/email/custom_emails/email_base_headers_no_headers_test.source.html" markdown_template_path = (
"zerver/tests/fixtures/email/custom_emails/email_base_headers_no_headers_test.html"
)
from zerver.lib.send_email import NoEmailArgumentError from zerver.lib.send_email import NoEmailArgumentError
@ -151,7 +153,7 @@ class TestCustomEmails(ZulipTestCase):
from_name = "from_name_test" from_name = "from_name_test"
email_subject = "subject_test" email_subject = "subject_test"
markdown_template_path = ( markdown_template_path = (
"zerver/tests/fixtures/email/custom_emails/email_base_headers_test.source.html" "zerver/tests/fixtures/email/custom_emails/email_base_headers_test.html"
) )
from zerver.lib.send_email import DoubledEmailArgumentError from zerver.lib.send_email import DoubledEmailArgumentError
@ -185,7 +187,7 @@ class TestCustomEmails(ZulipTestCase):
non_admin_user = self.example_user("cordelia") non_admin_user = self.example_user("cordelia")
markdown_template_path = ( markdown_template_path = (
"zerver/tests/fixtures/email/custom_emails/email_base_headers_test.source.html" "zerver/tests/fixtures/email/custom_emails/email_base_headers_test.html"
) )
send_custom_email( send_custom_email(
[admin_user, non_admin_user], [admin_user, non_admin_user],
@ -449,6 +451,10 @@ class TestMissedMessages(ZulipTestCase):
s = s.strip() s = s.strip()
return re.sub(r"\s+", " ", s) return re.sub(r"\s+", " ", s)
def remove_style_attribute(self, s: str) -> str:
pattern = r'\sstyle="[^"]+"'
return re.sub(pattern, "", s)
def _get_tokens(self) -> List[str]: def _get_tokens(self) -> List[str]:
return ["mm" + str(random.getrandbits(32)) for _ in range(30)] return ["mm" + str(random.getrandbits(32)) for _ in range(30)]
@ -463,6 +469,7 @@ class TestMissedMessages(ZulipTestCase):
verify_body_does_not_include: Sequence[str] = [], verify_body_does_not_include: Sequence[str] = [],
trigger: str = "", trigger: str = "",
mentioned_user_group_id: Optional[int] = None, mentioned_user_group_id: Optional[int] = None,
remove_style: bool = False,
) -> None: ) -> None:
othello = self.example_user("othello") othello = self.example_user("othello")
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
@ -500,7 +507,10 @@ class TestMissedMessages(ZulipTestCase):
if verify_html_body: if verify_html_body:
for text in verify_body_include: for text in verify_body_include:
assert isinstance(msg.alternatives[0][0], str) assert isinstance(msg.alternatives[0][0], str)
self.assertIn(text, self.normalize_string(msg.alternatives[0][0])) html = self.normalize_string(msg.alternatives[0][0])
if remove_style:
html = self.remove_style_attribute(html)
self.assertIn(text, html)
else: else:
for text in verify_body_include: for text in verify_body_include:
self.assertIn(text, self.normalize_string(msg.body)) self.assertIn(text, self.normalize_string(msg.body))
@ -1261,11 +1271,16 @@ class TestMissedMessages(ZulipTestCase):
f"http://zulip.testserver/user_avatars/{realm.id}/emoji/images/{realm_emoji_id}.png" f"http://zulip.testserver/user_avatars/{realm.id}/emoji/images/{realm_emoji_id}.png"
) )
verify_body_include = [ verify_body_include = [
f'<img alt=":green_tick:" src="{realm_emoji_url}" title="green tick" style="height: 20px;">' f'<img alt=":green_tick:" src="{realm_emoji_url}" title="green tick">'
] ]
email_subject = "DMs with Othello, the Moor of Venice" email_subject = "DMs with Othello, the Moor of Venice"
self._test_cases( self._test_cases(
msg_id, verify_body_include, email_subject, send_as_user=False, verify_html_body=True msg_id,
verify_body_include,
email_subject,
send_as_user=False,
verify_html_body=True,
remove_style=True,
) )
def test_emojiset_in_missed_message(self) -> None: def test_emojiset_in_missed_message(self) -> None:
@ -1278,11 +1293,16 @@ class TestMissedMessages(ZulipTestCase):
"Extremely personal message with a hamburger :hamburger:!", "Extremely personal message with a hamburger :hamburger:!",
) )
verify_body_include = [ verify_body_include = [
'<img alt=":hamburger:" src="http://testserver/static/generated/emoji/images-twitter-64/1f354.png" title="hamburger" style="height: 20px;">' '<img alt=":hamburger:" src="http://testserver/static/generated/emoji/images-twitter-64/1f354.png" title="hamburger">'
] ]
email_subject = "DMs with Othello, the Moor of Venice" email_subject = "DMs with Othello, the Moor of Venice"
self._test_cases( self._test_cases(
msg_id, verify_body_include, email_subject, send_as_user=False, verify_html_body=True msg_id,
verify_body_include,
email_subject,
send_as_user=False,
verify_html_body=True,
remove_style=True,
) )
def test_stream_link_in_missed_message(self) -> None: def test_stream_link_in_missed_message(self) -> None:
@ -1298,7 +1318,12 @@ class TestMissedMessages(ZulipTestCase):
] ]
email_subject = "DMs with Othello, the Moor of Venice" email_subject = "DMs with Othello, the Moor of Venice"
self._test_cases( self._test_cases(
msg_id, verify_body_include, email_subject, send_as_user=False, verify_html_body=True msg_id,
verify_body_include,
email_subject,
send_as_user=False,
verify_html_body=True,
remove_style=True,
) )
def test_pm_link_in_missed_message_header(self) -> None: def test_pm_link_in_missed_message_header(self) -> None:
@ -1338,7 +1363,8 @@ class TestMissedMessages(ZulipTestCase):
self.assertIn("Iago:\n> @**King Hamlet**\n\n--\nYou are", mail.outbox[0].body) self.assertIn("Iago:\n> @**King Hamlet**\n\n--\nYou are", mail.outbox[0].body)
# If message content starts with <p> tag the sender name is appended inside the <p> tag. # If message content starts with <p> tag the sender name is appended inside the <p> tag.
self.assertIn( self.assertIn(
'<p><b>Iago</b>: <span class="user-mention"', mail.outbox[0].alternatives[0][0] '<p><b>Iago</b>: <span class="user-mention"',
self.remove_style_attribute(mail.outbox[0].alternatives[0][0]),
) )
assert isinstance(mail.outbox[1], EmailMultiAlternatives) assert isinstance(mail.outbox[1], EmailMultiAlternatives)
@ -1346,8 +1372,8 @@ class TestMissedMessages(ZulipTestCase):
self.assertIn("Iago:\n> * 1\n> *2\n\n--\nYou are receiving", mail.outbox[1].body) self.assertIn("Iago:\n> * 1\n> *2\n\n--\nYou are receiving", mail.outbox[1].body)
# If message content does not starts with <p> tag sender name is appended before the <p> tag # If message content does not starts with <p> tag sender name is appended before the <p> tag
self.assertIn( self.assertIn(
" <b>Iago</b>: <div><ul>\n<li>1<br/>\n *2</li>\n</ul></div>\n", " <b>Iago</b>: <div><ul>\n<li>1<br>\n *2</li>\n</ul></div>\n",
mail.outbox[1].alternatives[0][0], self.remove_style_attribute(mail.outbox[1].alternatives[0][0]),
) )
assert isinstance(mail.outbox[2], EmailMultiAlternatives) assert isinstance(mail.outbox[2], EmailMultiAlternatives)
@ -1356,7 +1382,7 @@ class TestMissedMessages(ZulipTestCase):
# Sender name is not appended to message for PM missed messages # Sender name is not appended to message for PM missed messages
self.assertIn( self.assertIn(
">\n \n <div><p>Hello</p></div>\n", ">\n \n <div><p>Hello</p></div>\n",
mail.outbox[2].alternatives[0][0], self.remove_style_attribute(mail.outbox[2].alternatives[0][0]),
) )
def test_multiple_missed_personal_messages(self) -> None: def test_multiple_missed_personal_messages(self) -> None:

View File

@ -1,5 +1,4 @@
import os import os
import subprocess
import urllib import urllib
from contextlib import suppress from contextlib import suppress
from typing import Optional from typing import Optional
@ -52,13 +51,6 @@ def clear_emails(request: HttpRequest) -> HttpResponse:
@require_safe @require_safe
def generate_all_emails(request: HttpRequest) -> HttpResponse: def generate_all_emails(request: HttpRequest) -> HttpResponse:
if not settings.TEST_SUITE: # nocoverage
# It's really convenient to automatically inline the email CSS
# here, since that saves a step when testing out changes to
# the email CSS. But we don't run this inside the test suite,
# because by role, the tests shouldn't be doing a provision-like thing.
subprocess.check_call(["./scripts/setup/inline_email_css.py"])
# We import the Django test client inside the view function, # We import the Django test client inside the view function,
# because it isn't needed in production elsewhere, and not # because it isn't needed in production elsewhere, and not
# importing it saves ~50ms of unnecessary manage.py startup time. # importing it saves ~50ms of unnecessary manage.py startup time.