Skip to content

Commit d0645d4

Browse files
whabanksCopilot
andauthored
Fix multi-line conditional parsing in HTML email template previews (#393)
* Fix multi-line conditional parsing bug - Multi-line conditionals ending with a trailing line break will no longer insert a `</mark>` tag into the rendered template preview * Micro-optimization * Fix typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c87186d commit d0645d4

3 files changed

Lines changed: 60 additions & 6 deletions

File tree

notifications_utils/field.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class Field:
7070

7171
placeholder_tag = "<mark class='placeholder'>(({}))</mark>"
7272
conditional_placeholder_tag = "<mark class='placeholder-conditional'><span class='condition'>(({}??</span>{}))</mark>"
73+
conditional_placeholder_tag_block = "<div class='placeholder-conditional'><span class='condition'>(({}??</span>{}))</div>"
7374
placeholder_tag_translated = "<span class='placeholder-no-brackets'>[{}]</span>"
7475
placeholder_tag_redacted = "<mark class='placeholder-redacted'>[hidden]</mark>"
7576

@@ -81,10 +82,12 @@ def __init__(
8182
markdown_lists: bool = False,
8283
redact_missing_personalisation: bool = False,
8384
translated: bool = False,
85+
markdown_renderer: Optional[Callable] = None,
8486
):
8587
self.content = content
8688
self.values = values
8789
self.markdown_lists = markdown_lists
90+
self.markdown_renderer = markdown_renderer
8891
if translated:
8992
self.placeholder_tag = self.placeholder_tag_translated
9093

@@ -128,9 +131,21 @@ def format_match(self, match):
128131
return self.placeholder_tag_redacted
129132

130133
if placeholder.is_conditional():
131-
return self.conditional_placeholder_tag.format(
132-
self.sanitizer(placeholder.name), self.sanitizer(placeholder.conditional_text)
133-
)
134+
conditional_text = self.sanitizer(placeholder.conditional_text)
135+
sanitized_name = self.sanitizer(placeholder.name)
136+
137+
if "\n" in conditional_text and self.markdown_renderer:
138+
# Multi-line conditional: pre-render markdown so lists, links, etc.
139+
# display correctly. Use a block-level <div> wrapper so the outer
140+
# mistune pass treats it as an HTML block and won't break it apart.
141+
stripped = conditional_text.strip()
142+
rendered = self.markdown_renderer(stripped).strip() if stripped else ""
143+
return self.conditional_placeholder_tag_block.format(sanitized_name, rendered) + "\n"
144+
elif "\n" in conditional_text:
145+
# Multi-line but no renderer available: convert newlines to <br>
146+
conditional_text = conditional_text.strip("\n").replace("\n", "<br>")
147+
148+
return self.conditional_placeholder_tag.format(sanitized_name, conditional_text)
134149

135150
return self.placeholder_tag.format(self.sanitizer(placeholder.name))
136151

notifications_utils/template.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,7 @@ def get_html_email_body(template_content, template_values, redact_missing_person
819819
html=html,
820820
markdown_lists=True,
821821
redact_missing_personalisation=redact_missing_personalisation,
822+
markdown_renderer=_render_conditional_email_markdown,
822823
)
823824
)
824825
.then(unlink_govuk_escaped)
@@ -834,6 +835,22 @@ def get_html_email_body(template_content, template_values, redact_missing_person
834835
)
835836

836837

838+
def _render_conditional_email_markdown(content):
839+
"""Render markdown for multiline conditional preview content.
840+
841+
Conditionals are pre-rendered, so we need to mirror the parsing pipeline in get_html_email_body
842+
to ensure that language tags, RTL, lists, etc. that are inside conditionals are also pre-rendered
843+
else the main markdown rendering pass in get_html_email_body breaks them apart and mangles formatting.
844+
"""
845+
result = escape_lang_tags(content)
846+
result = escape_rtl_tags(result)
847+
result = notify_email_markdown(result)
848+
result = remove_nested_list_padding(result)
849+
result = add_language_divs(result)
850+
result = add_rtl_divs(result)
851+
return result
852+
853+
837854
def do_nice_typography(value):
838855
return (
839856
Take(value)

tests/test_template_types.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
SMSPreviewTemplate,
2121
Template,
2222
WithSubjectTemplate,
23+
_render_conditional_email_markdown,
2324
)
2425

2526

@@ -943,15 +944,29 @@ def test_subject_line_gets_replaced():
943944
{},
944945
[
945946
mock.call("subject", {}, html="escape", redact_missing_personalisation=False),
946-
mock.call("content", {}, html="escape", markdown_lists=True, redact_missing_personalisation=False),
947+
mock.call(
948+
"content",
949+
{},
950+
html="escape",
951+
markdown_lists=True,
952+
redact_missing_personalisation=False,
953+
markdown_renderer=_render_conditional_email_markdown,
954+
),
947955
mock.call("content", {}, html="escape", markdown_lists=True),
948956
],
949957
),
950958
(
951959
EmailPreviewTemplate,
952960
{},
953961
[
954-
mock.call("content", {}, html="escape", markdown_lists=True, redact_missing_personalisation=False),
962+
mock.call(
963+
"content",
964+
{},
965+
html="escape",
966+
markdown_lists=True,
967+
redact_missing_personalisation=False,
968+
markdown_renderer=_render_conditional_email_markdown,
969+
),
955970
mock.call("subject", {}, html="escape", redact_missing_personalisation=False),
956971
mock.call("((email address))", {}),
957972
],
@@ -1035,7 +1050,14 @@ def test_subject_line_gets_replaced():
10351050
EmailPreviewTemplate,
10361051
{"redact_missing_personalisation": True},
10371052
[
1038-
mock.call("content", {}, html="escape", markdown_lists=True, redact_missing_personalisation=True),
1053+
mock.call(
1054+
"content",
1055+
{},
1056+
html="escape",
1057+
markdown_lists=True,
1058+
redact_missing_personalisation=True,
1059+
markdown_renderer=_render_conditional_email_markdown,
1060+
),
10391061
mock.call("subject", {}, html="escape", redact_missing_personalisation=True),
10401062
mock.call("((email address))", {}),
10411063
],

0 commit comments

Comments
 (0)