From 2207e1194d013422d9e509ba4594f8714b884461 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Tue, 16 Jun 2026 15:09:30 +0100 Subject: [PATCH 1/9] Use newest version of markdown gem --- Gemfile | 2 +- Gemfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index fda0d60af..37d135131 100644 --- a/Gemfile +++ b/Gemfile @@ -49,7 +49,7 @@ gem "govuk-components", "~> 6" gem "govuk_design_system_formbuilder", "~> 6" # Our own custom markdown renderer -gem "govuk-forms-markdown", github: "govuk-forms/govuk-forms-markdown", tag: "0.9.0" +gem "govuk-forms-markdown", github: "govuk-forms/govuk-forms-markdown", tag: "0.10.0" # For compiling our frontend assets gem "vite_rails" diff --git a/Gemfile.lock b/Gemfile.lock index dce276f53..12262911b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -22,10 +22,10 @@ GIT GIT remote: https://github.com/govuk-forms/govuk-forms-markdown.git - revision: 84c6b2296accca578e7b77e7d1730472d9c9e7af - tag: 0.9.0 + revision: 3ca93546da82c3d39fb087e0e94891bafb2f9142 + tag: 0.10.0 specs: - govuk-forms-markdown (0.9.0) + govuk-forms-markdown (0.10.0) redcarpet (~> 3.6) GEM @@ -898,7 +898,7 @@ CHECKSUMS google-protobuf (4.35.0-x86_64-linux-musl) sha256=be0218520d77b2aee898b363514b03819f6f63f9c041ae0d0d79b4ce5247bffd googleapis-common-protos-types (1.23.0) sha256=992e740a523794d9fc5f29a504465d8fc737aaa16c930fe7228e3346860faf0a govuk-components (6.3.0) sha256=9e10aba5f88f9bead1ac792814e3551aadf17813080a74a17d69c6e0446bd88d - govuk-forms-markdown (0.9.0) + govuk-forms-markdown (0.10.0) govuk_design_system_formbuilder (6.2.0) sha256=d8cd1543776b7b15ba3d1774f16e3b8458c59446b0cad56ccbbc1e517a575e93 govuk_notify_rails (3.0.0) sha256=e0e1b8b0a7c47f90963034f290fb2982652aa8051a733c25c178680d44f2d7aa hana (1.3.7) sha256=5425db42d651fea08859811c29d20446f16af196308162894db208cac5ce9b0d From f234bc39bd35a11ee0b4d28a002a2827c1c77805 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Thu, 18 Jun 2026 12:46:22 +0100 Subject: [PATCH 2/9] Add markdown handler to allow ERB markdown partials --- app/lib/markdown_handler.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 app/lib/markdown_handler.rb diff --git a/app/lib/markdown_handler.rb b/app/lib/markdown_handler.rb new file mode 100644 index 000000000..6a9fc1662 --- /dev/null +++ b/app/lib/markdown_handler.rb @@ -0,0 +1,11 @@ +module MarkdownHandler + def self.erb + @erb ||= ActionView::Template.registered_template_handler(:erb) + end + + def self.call(template) + erb.call(template) + end +end + +ActionView::Template.register_template_handler :md, MarkdownHandler From 44da54f73590ad28bfb63217b0d8d0446455a672 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Thu, 18 Jun 2026 17:17:12 +0100 Subject: [PATCH 3/9] Add formatting for answer markdown --- app/lib/ses_email_formatter.rb | 36 ++++++++ app/models/submission.rb | 4 + spec/lib/ses_email_formatter_spec.rb | 122 +++++++++++++++++++++++++++ spec/models/submission_spec.rb | 14 +++ 4 files changed, 176 insertions(+) diff --git a/app/lib/ses_email_formatter.rb b/app/lib/ses_email_formatter.rb index 53217bb25..af4207eb1 100644 --- a/app/lib/ses_email_formatter.rb +++ b/app/lib/ses_email_formatter.rb @@ -26,6 +26,13 @@ def build_question_answers_section_plain_text }.join(H_RULE_PLAIN_TEXT) end + def build_question_answers_section_markdown(heading_level: 3) + @steps.map { |step| + [prep_question_title_markdown(step, heading_level), + prep_answer_text_markdown(step)].join("\n\n") + }.join(H_RULE_PLAIN_TEXT) + end + private def prep_question_title_html(step, heading_level) @@ -39,6 +46,17 @@ def prep_question_title_html(step, heading_level) end end + def prep_question_title_markdown(step, heading_level) + case heading_level + when 3 + "### #{prep_question_title_plain_text(step)}" + when 4 + "#### #{prep_question_title_plain_text(step)}" + else + raise FormattingError, "unsupported heading level: #{heading_level}" + end + end + def prep_answer_text_html(step) if step.is_selection_with_none_of_the_above_answer? prep_html_none_of_the_above_answer_text(step) @@ -49,6 +67,16 @@ def prep_answer_text_html(step) raise FormattingError, "could not format answer for question step #{step.id}" end + def prep_answer_text_markdown(step) + if step.is_selection_with_none_of_the_above_answer? + prep_markdown_none_of_the_above_answer_text(step) + else + prep_answer_text(step) + end + rescue StandardError + raise FormattingError, "could not format answer for question step #{step.id}" + end + def prep_html_none_of_the_above_answer_text(step) [ "

#{convert_newlines_to_html(prep_answer_text(step))}

", @@ -57,6 +85,14 @@ def prep_html_none_of_the_above_answer_text(step) ] end + def prep_markdown_none_of_the_above_answer_text(step) + [ + prep_answer_text(step), + "#### #{prep_none_of_the_above_question_text(step)}", + prep_none_of_the_above_answer_text(step), + ].join("\n\n") + end + def prep_question_title_plain_text(step) step.question.question_text end diff --git a/app/models/submission.rb b/app/models/submission.rb index b83e0ffdf..d0ef21187 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -67,6 +67,10 @@ def answer_content_for_email_plain_text(locale: :en, confirmation_email: false) ses_email_formatter(locale, confirmation_email).build_question_answers_section_plain_text end + def answer_content_for_email_markdown(heading_level:, locale: :en, confirmation_email: false) + ses_email_formatter(locale, confirmation_email).build_question_answers_section_markdown(heading_level:) + end + private def answer_store diff --git a/spec/lib/ses_email_formatter_spec.rb b/spec/lib/ses_email_formatter_spec.rb index b507c96c6..724de4505 100644 --- a/spec/lib/ses_email_formatter_spec.rb +++ b/spec/lib/ses_email_formatter_spec.rb @@ -307,4 +307,126 @@ end end end + + describe "#build_question_answers_section_markdown" do + context "when there is one step" do + it "returns question and and answer markdown" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What is the meaning of life?\n\n42") + end + + it "formats with the specified heading level" do + expect(ses_email_formatter.build_question_answers_section_markdown(heading_level: 4)).to eq("#### What is the meaning of life?\n\n42") + end + + it "raises an error if the heading level is unsupported" do + expect { + ses_email_formatter.build_question_answers_section_markdown(heading_level: 2) + }.to raise_error(SesEmailFormatter::FormattingError, "unsupported heading level: 2") + end + end + + context "when the answer has multiple attributes" do + let(:steps) { [name_step] } + + it "inserts line breaks between answer attributes" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What is your name?\n\nFirst name: #{name_question.first_name}\n\nLast name: #{name_question.last_name}") + end + end + + context "when the answer is blank i.e. skipped" do + let(:text_question) { build :text, question_text: "What is the meaning of life?", text: nil } + let(:steps) { [text_step] } + + it "returns the blank answer text" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What is the meaning of life?\n\n[This question was skipped]") + end + + context "when formatting for a confirmation email" do + let(:confirmation_email) { true } + + it "returns the blank answer text" do + expect(ses_email_formatter.build_question_answers_section_markdown).to include("Not completed") + end + end + end + + context "when there is more than one step" do + let(:steps) { [text_step, name_step] } + + it "returns all question an answers separated by a horizontal rule" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What is the meaning of life?\n\n42\n\n---\n\n### What is your name?\n\nFirst name: #{name_question.first_name}\n\nLast name: #{name_question.last_name}") + end + end + + context "when there are special characters in the answer" do + let(:steps) { [text_step] } + + it "returns the sanitized answer" do + [ + { input: "\n\nTest\n\nTest 2", output: "Test\n\nTest 2" }, + { input: " paragraph 1\n\n\n\n\n\n\n\n\n\n\n\n\n Another Paragraph with trailing space \n\n\n\n\n", output: "paragraph 1\n\nAnother Paragraph with trailing space" }, + + ].each do |test_case| + text_question.text = test_case[:input] + + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What is the meaning of life?\n\n#{test_case[:output]}") + end + end + end + + context "when none of the above is selected in a none of the above question" do + let(:steps) { [none_of_the_above_step] } + + it "returns the sanitized answer including the none of the above answer" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What sandwich do you want?\n\nNone of the above\n\n#### Specify your desired sandwich\n\nCheese and pickle") + end + + context "when the none of the above question is optional and no answer is provided" do + let(:none_of_the_above_answer) { nil } + let(:none_of_the_above_question_is_optional) { "true" } + + it "returns the skipped none of the above answer text" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### What sandwich do you want?\n\nNone of the above\n\n#### Specify your desired sandwich (optional)\n\n[This question was skipped]") + end + + context "when formatting for a confirmation email" do + let(:confirmation_email) { true } + + it "returns the skipped none of the above answer text" do + expect(ses_email_formatter.build_question_answers_section_markdown).to include("Not completed") + end + end + end + end + + context "when there is a file question" do + let(:steps) { [file_step] } + + context "when formatting for a submission email" do + it "returns the content for a submission email" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### Upload a file\n\na-file_SUB-12345.txt (attached to this email)") + end + end + + context "when formatting for a confirmation email" do + let(:confirmation_email) { true } + + it "returns the content for a confirmation email" do + expect(ses_email_formatter.build_question_answers_section_markdown).to eq("### Upload a file\n\nYou uploaded a file called a-file.txt") + end + end + end + + context "when there is an error formatting an answer" do + before do + allow(text_step).to receive(:show_answer_in_email).and_raise(NoMethodError, "undefined method 'strip' for an instance of Array") + end + + it "raises an error with the page id" do + expect { + ses_email_formatter.build_question_answers_section_markdown + }.to raise_error(SesEmailFormatter::FormattingError, "could not format answer for question step #{text_step.id}") + end + end + end end diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index b5dc23652..84abf2bfa 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -210,6 +210,20 @@ expect(result).to start_with("Beth yw eich hoff liw?") end end + + describe "#answer_content_for_email_markdown" do + it "uses the English form document to construct the markdown by default" do + result = submission.answer_content_for_email_markdown(heading_level: 4) + + expect(result).to include("What is your favourite colour?") + end + + it "uses the Welsh form document to construct the markdown when the locale is :cy" do + result = submission.answer_content_for_email_markdown(heading_level: 4, locale: :cy) + + expect(result).to include("Beth yw eich hoff liw?") + end + end end describe "#sent?" do From ffda493c519bd45eab80ac80db9ae73f04852257 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Mon, 22 Jun 2026 16:33:27 +0100 Subject: [PATCH 4/9] Use markdown template for submission email --- .../_content.md.erb | 45 +++++++++++++++ .../submission_email.html.erb | 55 +------------------ .../submission_email.text.erb | 40 +------------- config/locales/cy.yml | 4 +- config/locales/en.yml | 4 +- .../aws_ses_form_submission_mailer_spec.rb | 4 +- 6 files changed, 53 insertions(+), 99 deletions(-) create mode 100644 app/views/aws_ses_form_submission_mailer/_content.md.erb diff --git a/app/views/aws_ses_form_submission_mailer/_content.md.erb b/app/views/aws_ses_form_submission_mailer/_content.md.erb new file mode 100644 index 000000000..bc764184b --- /dev/null +++ b/app/views/aws_ses_form_submission_mailer/_content.md.erb @@ -0,0 +1,45 @@ +<% if @submission.preview? %> +<%= I18n.t("mailer.submission.preview") %> + +<% end %> +<%= I18n.t("mailer.form_name", form_name: @submission.form.name) %> + +<%= I18n.t("mailer.submission.time", time: format_time(@submission.submission_time), date: format_date(@submission.submission_time) ) %> + +<% if @welsh_submission %> +<%= I18n.t("mailer.submission.welsh_submission") %> + +<% end %> +<%= I18n.t("mailer.submission.reference", submission_reference: @submission.reference) %> + +<% if @submission.payment_url.present? %> +<%= I18n.t("mailer.submission.payment") %> + +<% end %> +^<%= I18n.t("mailer.check_before_using") %>^ + +--- + +## <%= I18n.t("mailer.submission.answers_submitted") %> + +<% if @csv_filename.present? %> +<%= I18n.t("mailer.submission.csv_file", filename: @csv_filename) %> + +<% end %> +<% if @json_filename.present? %> +<%= I18n.t("mailer.submission.json_file", filename: @json_filename) %> + +<% end %> +<%= @submission.answer_content_for_email_markdown(heading_level: 3) %> + +--- + +^^^ + +## <%= I18n.t("mailer.cannot_reply.heading") %> + +<%= I18n.t("mailer.cannot_reply.contact_form_filler_markdown") %> + +<%= I18n.t("mailer.cannot_reply.contact_forms_team_markdown") %> + +^^^ diff --git a/app/views/aws_ses_form_submission_mailer/submission_email.html.erb b/app/views/aws_ses_form_submission_mailer/submission_email.html.erb index 1de8f537f..9793cf368 100644 --- a/app/views/aws_ses_form_submission_mailer/submission_email.html.erb +++ b/app/views/aws_ses_form_submission_mailer/submission_email.html.erb @@ -1,54 +1 @@ -<% if @submission.preview? %> -

- <%= I18n.t("mailer.submission.preview") %> -

-<% end %> - -

<%= I18n.t("mailer.form_name", form_name: @submission.form.name) %>

- -

- <%= I18n.t("mailer.submission.time", time: format_time(@submission.submission_time), date: format_date(@submission.submission_time) ) %> -

-

- <% if @welsh_submission %> - <%= I18n.t("mailer.submission.welsh_submission") %> - <% end %> -

-

- <%= I18n.t("mailer.submission.reference", submission_reference: @submission.reference) %> -

- -<% if @submission.payment_url.present? %> -

- <%= I18n.t("mailer.submission.payment") %> -

-<% end %> - -

- <%= I18n.t("mailer.check_before_using") %> -

- -
- -

<%= I18n.t("mailer.submission.answers_submitted") %>

- -<% if @csv_filename.present? %> -

<%= I18n.t("mailer.submission.csv_file", filename: @csv_filename) %>

-<% end %> - -<% if @json_filename.present? %> -

<%= I18n.t("mailer.submission.json_file", filename: @json_filename) %>

-<% end %> - -<%= @submission.answer_content_for_email_html(heading_level: 3).html_safe %> - -
- -
-

<%= I18n.t("mailer.cannot_reply.heading") %>

- <%= I18n.t("mailer.cannot_reply.contact_form_filler_html").html_safe %> - <%= I18n.t("mailer.cannot_reply.contact_forms_team_html").html_safe %> -
+<%= GovukFormsMarkdown.render_for_email(render(partial: "content", formats: [:md])).html_safe %> diff --git a/app/views/aws_ses_form_submission_mailer/submission_email.text.erb b/app/views/aws_ses_form_submission_mailer/submission_email.text.erb index c844ccbc7..81312f4ae 100644 --- a/app/views/aws_ses_form_submission_mailer/submission_email.text.erb +++ b/app/views/aws_ses_form_submission_mailer/submission_email.text.erb @@ -1,39 +1 @@ -<% if @submission.preview? %> - <%= I18n.t("mailer.submission.preview") %> - -<% end %> -<%= I18n.t("mailer.form_name", form_name: @submission.form.name) %> - -<%= I18n.t("mailer.submission.time", time: format_time(@submission.submission_time), date: format_date(@submission.submission_time) ) %> - -<% if @welsh_submission %> -<%= I18n.t("mailer.submission.welsh_submission") %> -<% end %> - -<%= I18n.t("mailer.submission.reference", submission_reference: @submission.reference) %> - -<% if @submission.payment_url.present? %> - <%= I18n.t("mailer.submission.payment") %> - -<% end %> -<%= I18n.t("mailer.check_before_using") %> - ---- -<%= I18n.t("mailer.submission.answers_submitted") %> - -<% if @csv_filename.present? %> - <%= I18n.t("mailer.submission.csv_file", filename: @csv_filename) %> -<% end %> - -<% if @json_filename.present? %> - <%= I18n.t("mailer.submission.json_file", filename: @json_filename) %> -<% end %> - -<%= @submission.answer_content_for_email_plain_text %> - ---- - -<%= I18n.t("mailer.cannot_reply.heading") %> - -<%= I18n.t("mailer.cannot_reply.contact_form_filler_plain") %> -<%= I18n.t("mailer.cannot_reply.contact_forms_team_plain") %> +<%= GovukFormsMarkdown.render_plain_text(render(partial: "content", formats: [:md])).html_safe %> diff --git a/config/locales/cy.yml b/config/locales/cy.yml index 3644f80b2..0d5cb52c0 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -480,9 +480,9 @@ cy: bounced_weekly_batch: Weekly batch sent at %{sent_at_time} on %{sent_at_date} contacted_group_admins_paragraph: The form is in the group “%{group_name}”, which has no group admins or whose group admins have not responded. cannot_reply: - contact_form_filler_html: "

If you need to contact the person who completed this form, you’ll need to contact them directly.

" - contact_form_filler_plain: If you need to contact the person who completed this form, you’ll need to contact them directly. + contact_form_filler_markdown: If you need to contact the person who completed this form, you’ll need to contact them directly. contact_forms_team_html:

If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team with details of the issue and the form it relates to.

+ contact_forms_team_markdown: If you’re experiencing a technical issue with this form, [contact the GOV.​UK Forms team](https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. contact_forms_team_plain: If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team (https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. heading: You cannot reply to this email check_before_using: Check that these answers look safe before you use them diff --git a/config/locales/en.yml b/config/locales/en.yml index 942603a06..3a57db9a8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -480,9 +480,9 @@ en: bounced_weekly_batch: Weekly batch sent at %{sent_at_time} on %{sent_at_date} contacted_group_admins_paragraph: The form is in the group “%{group_name}”, which has no group admins or whose group admins have not responded. cannot_reply: - contact_form_filler_html: "

If you need to contact the person who completed this form, you’ll need to contact them directly.

" - contact_form_filler_plain: If you need to contact the person who completed this form, you’ll need to contact them directly. + contact_form_filler_markdown: If you need to contact the person who completed this form, you’ll need to contact them directly. contact_forms_team_html:

If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team with details of the issue and the form it relates to.

+ contact_forms_team_markdown: If you’re experiencing a technical issue with this form, [contact the GOV.​UK Forms team](https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. contact_forms_team_plain: If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team (https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. heading: You cannot reply to this email check_before_using: Check that these answers look safe before you use them diff --git a/spec/mailers/aws_ses_form_submission_mailer_spec.rb b/spec/mailers/aws_ses_form_submission_mailer_spec.rb index e0abaa607..d86e2a47d 100644 --- a/spec/mailers/aws_ses_form_submission_mailer_spec.rb +++ b/spec/mailers/aws_ses_form_submission_mailer_spec.rb @@ -79,7 +79,7 @@ it "includes the warning about not replying" do expect(part.body).to have_css("h2", text: I18n.t("mailer.cannot_reply.heading")) - expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_form_filler_html")) + expect(part.body).to include(GovukFormsMarkdown.render_for_email(I18n.t("mailer.cannot_reply.contact_form_filler_markdown"))) expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_forms_team_html")) end @@ -156,7 +156,7 @@ it "includes the warning about not replying" do expect(part.body).to have_text(I18n.t("mailer.cannot_reply.heading")) - expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_form_filler_plain")) + expect(part.body).to include(GovukFormsMarkdown.render_plain_text(I18n.t("mailer.cannot_reply.contact_form_filler_markdown"))) expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_forms_team_plain")) end From 3de36fdca8e73c7722d7040cce303b3c924d2c2c Mon Sep 17 00:00:00 2001 From: David Biddle Date: Mon, 22 Jun 2026 16:53:53 +0100 Subject: [PATCH 5/9] Use markdown template for daily batch email --- .../_daily_submission_batch_content.md.erb | 36 ++++++++++++++++++ .../daily_submission_batch_email.html.erb | 38 +------------------ .../daily_submission_batch_email.text.erb | 27 +------------ config/locales/en.yml | 2 +- 4 files changed, 39 insertions(+), 64 deletions(-) create mode 100644 app/views/aws_ses_submission_batch_mailer/_daily_submission_batch_content.md.erb diff --git a/app/views/aws_ses_submission_batch_mailer/_daily_submission_batch_content.md.erb b/app/views/aws_ses_submission_batch_mailer/_daily_submission_batch_content.md.erb new file mode 100644 index 000000000..217509df7 --- /dev/null +++ b/app/views/aws_ses_submission_batch_mailer/_daily_submission_batch_content.md.erb @@ -0,0 +1,36 @@ +<% if @mode.preview? %> +<%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> + +<% end %> + +<%= I18n.t("mailer.form_name", form_name: @form_name) %> + +<% if @filenames.size > 1 %> +<%= I18n.t("mailer.submission_batch.daily.multiple_files_explainer", date: @date) %> + +<% else %> +<%= I18n.t("mailer.submission_batch.daily.single_file_explainer", date: @date) %> + +<% end %> + +<% @filenames.each do |filename| %> + +- <%= filename %> + +<% end %> + +^<%= I18n.t("mailer.check_before_using") %>^ + +<%= I18n.t("mailer.submission_batch.submissions_are_split").html_safe %> + +<%= I18n.t("mailer.submission_batch.file_upload_questions") %> + +--- + +^^^ + +## <%= I18n.t("mailer.cannot_reply.heading") %> + +<%= I18n.t("mailer.cannot_reply.contact_forms_team_markdown").html_safe %> + +^^^ diff --git a/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.html.erb b/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.html.erb index 1aaf87339..75d65a56f 100644 --- a/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.html.erb +++ b/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.html.erb @@ -1,37 +1 @@ -<% if @mode.preview? %> -

- <%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> -

-<% end %> - -

<%= I18n.t("mailer.form_name", form_name: @form_name) %>

- -<% if @filenames.size > 1 %> -

<%= I18n.t("mailer.submission_batch.daily.multiple_files_explainer", date: @date) %>

-<% else %> -

<%= I18n.t("mailer.submission_batch.daily.single_file_explainer", date: @date) %>

-<% end %> - -
    - <% @filenames.each do |filename| %> -
  • <%= filename %>
    • - <% end %> -
- -

- <%= I18n.t("mailer.check_before_using") %> -

- -

<%= I18n.t("mailer.submission_batch.submissions_are_split") %>

- -

<%= I18n.t("mailer.submission_batch.file_upload_questions") %>

- -
- -
-

<%= I18n.t("mailer.cannot_reply.heading") %>

- <%= I18n.t("mailer.cannot_reply.contact_forms_team_html").html_safe %> -
+<%= GovukFormsMarkdown.render_for_email(render(partial: "daily_submission_batch_content", formats: [:md])).html_safe %> diff --git a/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.text.erb b/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.text.erb index bdc6e613d..8d156701b 100644 --- a/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.text.erb +++ b/app/views/aws_ses_submission_batch_mailer/daily_submission_batch_email.text.erb @@ -1,26 +1 @@ -<% if @mode.preview? %> -<%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> -<% end %> - -<%= I18n.t("mailer.form_name", form_name: @form_name) %> - -<% if @filenames.size > 1 %> -<%= I18n.t("mailer.submission_batch.daily.multiple_files_explainer", date: @date) %> -<% else %> -<%= I18n.t("mailer.submission_batch.daily.single_file_explainer", date: @date) %> -<% end %> - -<% @filenames.each do |filename| %> - • <%= filename %> -<% end %> - -<%= I18n.t("mailer.check_before_using") %> - -<%= I18n.t("mailer.submission_batch.submissions_are_split") %> - -<%= I18n.t("mailer.submission_batch.file_upload_questions") %> - ---- - -<%= I18n.t("mailer.cannot_reply.heading") %> -<%= I18n.t("mailer.cannot_reply.contact_forms_team_plain") %> +<%= GovukFormsMarkdown.render_plain_text(render(partial: "daily_submission_batch_content", formats: [:md])) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 3a57db9a8..8fcd83dfa 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -512,7 +512,7 @@ en: archived: These are test submissions to a preview of an archived form. draft: These are test submissions to a preview of a draft form. live: These are test submissions to a preview of a live form. - submissions_are_split: Submissions are split into separate CSV files if the form's been changed in a way that affects the structure of the CSV. + submissions_are_split: Submissions are split into separate CSV files if the form’s been changed in a way that affects the structure of the CSV. weekly: multiple_files_explainer: 'All submissions to this form from %{begin_date} to %{end_date} are attached to this email in CSV files named:' single_file_explainer: 'All submissions to this form from %{begin_date} to %{end_date} are attached to this email in a CSV file named:' From 3e5834b624e9c787bd9cc1bc6608181cee2b1494 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Mon, 22 Jun 2026 16:59:14 +0100 Subject: [PATCH 6/9] Use markdown template for weekly batch email --- .../_weekly_submission_batch_content.md.erb | 36 ++++++++++++++++++ .../weekly_submission_batch_email.html.erb | 38 +------------------ .../weekly_submission_batch_email.text.erb | 27 +------------ config/locales/cy.yml | 2 - config/locales/en.yml | 2 - .../aws_ses_form_submission_mailer_spec.rb | 4 +- .../aws_ses_submission_batch_mailer_spec.rb | 6 +-- 7 files changed, 43 insertions(+), 72 deletions(-) create mode 100644 app/views/aws_ses_submission_batch_mailer/_weekly_submission_batch_content.md.erb diff --git a/app/views/aws_ses_submission_batch_mailer/_weekly_submission_batch_content.md.erb b/app/views/aws_ses_submission_batch_mailer/_weekly_submission_batch_content.md.erb new file mode 100644 index 000000000..e4313e7dc --- /dev/null +++ b/app/views/aws_ses_submission_batch_mailer/_weekly_submission_batch_content.md.erb @@ -0,0 +1,36 @@ +<% if @mode.preview? %> +<%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> +<% end %> + +<%= I18n.t("mailer.form_name", form_name: @form_name) %> + +<% if @filenames.size > 1 %> + +<%= I18n.t("mailer.submission_batch.weekly.multiple_files_explainer", begin_date: @begin_date, end_date: @end_date) %> + +<% else %> +<%= I18n.t("mailer.submission_batch.weekly.single_file_explainer", begin_date: @begin_date, end_date: @end_date) %> + +<% end %> + +<% @filenames.each do |filename| %> + +- <%= filename %> + +<% end %> + +^<%= I18n.t("mailer.check_before_using") %>^ + +<%= I18n.t("mailer.submission_batch.submissions_are_split") %> + +<%= I18n.t("mailer.submission_batch.file_upload_questions") %> + +--- + +^^^ + +## <%= I18n.t("mailer.cannot_reply.heading") %> + +<%= I18n.t("mailer.cannot_reply.contact_forms_team_markdown") %> + +^^^ diff --git a/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.html.erb b/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.html.erb index 72809c395..44a271a78 100644 --- a/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.html.erb +++ b/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.html.erb @@ -1,37 +1 @@ -<% if @mode.preview? %> -

- <%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> -

-<% end %> - -

<%= I18n.t("mailer.form_name", form_name: @form_name) %>

- -<% if @filenames.size > 1 %> -

<%= I18n.t("mailer.submission_batch.weekly.multiple_files_explainer", begin_date: @begin_date, end_date: @end_date) %>

-<% else %> -

<%= I18n.t("mailer.submission_batch.weekly.single_file_explainer", begin_date: @begin_date, end_date: @end_date) %>

-<% end %> - -
    - <% @filenames.each do |filename| %> -
  • <%= filename %>
    • - <% end %> -
- -

- <%= I18n.t("mailer.check_before_using") %> -

- -

<%= I18n.t("mailer.submission_batch.submissions_are_split") %>

- -

<%= I18n.t("mailer.submission_batch.file_upload_questions") %>

- -
- -
-

<%= I18n.t("mailer.cannot_reply.heading") %>

- <%= I18n.t("mailer.cannot_reply.contact_forms_team_html").html_safe %> -
+<%= GovukFormsMarkdown.render_for_email(render(partial: "weekly_submission_batch_content", formats: [:md])).html_safe %> diff --git a/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.text.erb b/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.text.erb index b826b23ad..ffd6dce6c 100644 --- a/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.text.erb +++ b/app/views/aws_ses_submission_batch_mailer/weekly_submission_batch_email.text.erb @@ -1,26 +1 @@ -<% if @mode.preview? %> -<%= I18n.t("mailer.submission_batch.preview.#{@mode.tag}") %> -<% end %> - -<%= I18n.t("mailer.form_name", form_name: @form_name) %> - -<% if @filenames.size > 1 %> -<%= I18n.t("mailer.submission_batch.weekly.multiple_files_explainer", begin_date: @begin_date, end_date: @end_date) %> -<% else %> -<%= I18n.t("mailer.submission_batch.weekly.single_file_explainer", begin_date: @begin_date, end_date: @end_date) %> -<% end %> - -<% @filenames.each do |filename| %> - • <%= filename %> -<% end %> - -<%= I18n.t("mailer.check_before_using") %> - -<%= I18n.t("mailer.submission_batch.submissions_are_split") %> - -<%= I18n.t("mailer.submission_batch.file_upload_questions") %> - ---- - -<%= I18n.t("mailer.cannot_reply.heading") %> -<%= I18n.t("mailer.cannot_reply.contact_forms_team_plain") %> +<%= GovukFormsMarkdown.render_plain_text(render(partial: "weekly_submission_batch_content", formats: [:md])) %> diff --git a/config/locales/cy.yml b/config/locales/cy.yml index 0d5cb52c0..62ebaed37 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -481,9 +481,7 @@ cy: contacted_group_admins_paragraph: The form is in the group “%{group_name}”, which has no group admins or whose group admins have not responded. cannot_reply: contact_form_filler_markdown: If you need to contact the person who completed this form, you’ll need to contact them directly. - contact_forms_team_html:

If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team with details of the issue and the form it relates to.

contact_forms_team_markdown: If you’re experiencing a technical issue with this form, [contact the GOV.​UK Forms team](https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. - contact_forms_team_plain: If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team (https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. heading: You cannot reply to this email check_before_using: Check that these answers look safe before you use them form_name: 'Form name: “%{form_name}”' diff --git a/config/locales/en.yml b/config/locales/en.yml index 8fcd83dfa..a37e7bdb9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -481,9 +481,7 @@ en: contacted_group_admins_paragraph: The form is in the group “%{group_name}”, which has no group admins or whose group admins have not responded. cannot_reply: contact_form_filler_markdown: If you need to contact the person who completed this form, you’ll need to contact them directly. - contact_forms_team_html:

If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team with details of the issue and the form it relates to.

contact_forms_team_markdown: If you’re experiencing a technical issue with this form, [contact the GOV.​UK Forms team](https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. - contact_forms_team_plain: If you’re experiencing a technical issue with this form, contact the GOV.​UK Forms team (https://www.forms.service.gov.uk/support) with details of the issue and the form it relates to. heading: You cannot reply to this email check_before_using: Check that these answers look safe before you use them form_name: 'Form name: “%{form_name}”' diff --git a/spec/mailers/aws_ses_form_submission_mailer_spec.rb b/spec/mailers/aws_ses_form_submission_mailer_spec.rb index d86e2a47d..7dcfc3e62 100644 --- a/spec/mailers/aws_ses_form_submission_mailer_spec.rb +++ b/spec/mailers/aws_ses_form_submission_mailer_spec.rb @@ -80,7 +80,7 @@ it "includes the warning about not replying" do expect(part.body).to have_css("h2", text: I18n.t("mailer.cannot_reply.heading")) expect(part.body).to include(GovukFormsMarkdown.render_for_email(I18n.t("mailer.cannot_reply.contact_form_filler_markdown"))) - expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_forms_team_html")) + expect(part.body).to include(GovukFormsMarkdown.render_for_email(I18n.t("mailer.cannot_reply.contact_forms_team_markdown"))) end describe "submission date/time" do @@ -157,7 +157,7 @@ it "includes the warning about not replying" do expect(part.body).to have_text(I18n.t("mailer.cannot_reply.heading")) expect(part.body).to include(GovukFormsMarkdown.render_plain_text(I18n.t("mailer.cannot_reply.contact_form_filler_markdown"))) - expect(part.body).to include(I18n.t("mailer.cannot_reply.contact_forms_team_plain")) + expect(part.body).to include(GovukFormsMarkdown.render_plain_text(I18n.t("mailer.cannot_reply.contact_forms_team_markdown"))) end describe "submission date/time" do diff --git a/spec/mailers/aws_ses_submission_batch_mailer_spec.rb b/spec/mailers/aws_ses_submission_batch_mailer_spec.rb index 0e4ac02ae..ffa0456dd 100644 --- a/spec/mailers/aws_ses_submission_batch_mailer_spec.rb +++ b/spec/mailers/aws_ses_submission_batch_mailer_spec.rb @@ -110,7 +110,7 @@ end it "lists the filename of the attachment" do - expect(part.body).to have_text(" • filename.csv") + expect(part.body).to have_text("• filename.csv") end end @@ -122,8 +122,8 @@ end it "lists the filenames of the attachments" do - expect(part.body).to have_text(" • filename.csv") - expect(part.body).to have_text(" • filename-2.csv") + expect(part.body).to have_text("• filename.csv") + expect(part.body).to have_text("• filename-2.csv") end end end From 4e4cf8dabea0f6bd8053d14d5429df5e28e3eb3f Mon Sep 17 00:00:00 2001 From: David Biddle Date: Mon, 22 Jun 2026 17:20:47 +0100 Subject: [PATCH 7/9] Use markdown template for SES confirmation email --- app/helpers/email_format_helper.rb | 9 +++ .../_content.html.erb | 57 --------------- .../_content.md.erb | 70 +++++++++++++++++++ .../_content.text.erb | 55 --------------- .../submission_confirmation_email.html.erb | 7 +- .../submission_confirmation_email.text.erb | 5 +- config/locales/cy.yml | 2 +- config/locales/en.yml | 2 +- 8 files changed, 88 insertions(+), 119 deletions(-) delete mode 100644 app/views/aws_ses_submission_confirmation_mailer/_content.html.erb create mode 100644 app/views/aws_ses_submission_confirmation_mailer/_content.md.erb delete mode 100644 app/views/aws_ses_submission_confirmation_mailer/_content.text.erb diff --git a/app/helpers/email_format_helper.rb b/app/helpers/email_format_helper.rb index ae60ce4f5..ef910ac93 100644 --- a/app/helpers/email_format_helper.rb +++ b/app/helpers/email_format_helper.rb @@ -9,11 +9,20 @@ def convert_newlines_to_html(text) text.gsub("\n", "
") end + def convert_newlines_to_markdown(text) + text.gsub("\n", " \n") + end + def normalize_whitespace_and_convert_to_html(text) output = normalize_whitespace(text) convert_newlines_to_html(output) end + def normalize_whitespace_and_convert_to_markdown(text) + output = normalize_whitespace(text) + convert_newlines_to_markdown(output) + end + def markdown_to_html(markdown) HtmlMarkdownSanitizer.new.render_scrubbed_markdown(markdown, allow_headings: false, for_email: true) end diff --git a/app/views/aws_ses_submission_confirmation_mailer/_content.html.erb b/app/views/aws_ses_submission_confirmation_mailer/_content.html.erb deleted file mode 100644 index 3d2b5d046..000000000 --- a/app/views/aws_ses_submission_confirmation_mailer/_content.html.erb +++ /dev/null @@ -1,57 +0,0 @@ -<% if @submission.preview? %> -

<%= I18n.t("mailer.submission_confirmation.title_preview") %>

-

<%= I18n.t("mailer.submission_confirmation.this_is_a_test") %>

-<% else %> -

<%= I18n.t("mailer.submission_confirmation.title") %>

-<% end %> - -

<%= I18n.t("mailer.submission_confirmation.submitted_at", form_name: form.name, - time: format_time(@submission.submission_time), - date: format_date(@submission.submission_time)) %>

- -

<%= I18n.t("mailer.submission_confirmation.reference_number", reference: @submission.reference) %>

- -
-

<%= I18n.t("mailer.submission_confirmation.you_cannot_reply") %>

-
- - -<% if form.payment_url.present? %> -

<%= I18n.t("mailer.submission_confirmation.payment_link_heading") %>

-

<%= I18n.t("mailer.submission_confirmation.payment_link_html", payment_link: form.payment_url_with_reference(@submission.reference)).html_safe %>

-<% end %> - -

<%= I18n.t("mailer.submission_confirmation.what_happens_next_heading") %>

-<% if form.what_happens_next_markdown.present? %> - <%= markdown_to_html(form.what_happens_next_markdown) %> -<% else %> -

<%= I18n.t("mailer.submission_confirmation.default_what_happens_next") %>

-<% end %> - -<% if @include_copy_of_answers %> -

<%= I18n.t("mailer.submission_confirmation.answers_submitted_heading") %>

- <%= @submission.answer_content_for_email_html(heading_level: 4, locale: locale, confirmation_email: true).html_safe %> -
-<% end %> - -

<%= I18n.t("mailer.submission_confirmation.contact_details_heading") %>

- -<% if form.support_details.phone || form.support_details.email || form.support_details.url %> - <% if form.support_details.phone.present? %> -

<%= normalize_whitespace_and_convert_to_html(form.support_details.phone).html_safe %>

-

<%= I18n.t('support_details.call_charges') %>

- <% end %> - - <% if form.support_details.email.present? %> -

<%= form.support_details.email %>

- <% end %> - - <% if form.support_details.url.present? && form.support_details.url_text.present? %> -

<%= form.support_details.url_text %>

- <% end %> -<% else %> -

<%= I18n.t("mailer.submission_confirmation.default_support_contact_details") %>

-<% end %> diff --git a/app/views/aws_ses_submission_confirmation_mailer/_content.md.erb b/app/views/aws_ses_submission_confirmation_mailer/_content.md.erb new file mode 100644 index 000000000..7bc8fcec4 --- /dev/null +++ b/app/views/aws_ses_submission_confirmation_mailer/_content.md.erb @@ -0,0 +1,70 @@ +<% if @submission.preview? %> + +## <%= I18n.t("mailer.submission_confirmation.title_preview") %> + +<%= I18n.t("mailer.submission_confirmation.this_is_a_test") %> + +<% else %> + +## <%= I18n.t("mailer.submission_confirmation.title") %> + +<% end %> + +<%= I18n.t("mailer.submission_confirmation.submitted_at", form_name: form.name, +time: format_time(@submission.submission_time), +date: format_date(@submission.submission_time)) %> + +<%= I18n.t("mailer.submission_confirmation.reference_number", reference: @submission.reference) %> + +^<%= I18n.t("mailer.submission_confirmation.you_cannot_reply") %>^ + +<% if form.payment_url.present? %> + +### <%= I18n.t("mailer.submission_confirmation.payment_link_heading") %> + +<%= I18n.t("mailer.submission_confirmation.payment_link_markdown", payment_link: form.payment_url_with_reference(@submission.reference)) %> + +<% end %> + +### <%= I18n.t("mailer.submission_confirmation.what_happens_next_heading") %> + +<% if form.what_happens_next_markdown.present? %> +<%= form.what_happens_next_markdown %> +<% else %> + +<%= I18n.t("mailer.submission_confirmation.default_what_happens_next") %> +<% end %> + +<% if @include_copy_of_answers %> + +### <%= I18n.t("mailer.submission_confirmation.answers_submitted_heading") %> + +<%= @submission.answer_content_for_email_markdown(heading_level: 4, locale: locale, confirmation_email: true) %> + +--- + +<% end %> + +### <%= I18n.t("mailer.submission_confirmation.contact_details_heading") %> + +<% if form.support_details.phone || form.support_details.email || form.support_details.url %> +<% if form.support_details.phone.present? %> + +<%= normalize_whitespace_and_convert_to_markdown(form.support_details.phone) %> + +[<%= I18n.t('support_details.call_charges') %>](<%= form.support_details.call_charges_url %>) +<% end %> + +<% if form.support_details.email.present? %> + +[<%= form.support_details.email %>](mailto:<%= form.support_details.email %>) +<% end %> + +<% if form.support_details.url.present? && form.support_details.url_text.present? %> + +[<%= form.support_details.url_text %>](<%= form.support_details.url %>) +<% end %> +<% else %> + +<%= I18n.t("mailer.submission_confirmation.default_support_contact_details") %> +<% end %> diff --git a/app/views/aws_ses_submission_confirmation_mailer/_content.text.erb b/app/views/aws_ses_submission_confirmation_mailer/_content.text.erb deleted file mode 100644 index e8f831b76..000000000 --- a/app/views/aws_ses_submission_confirmation_mailer/_content.text.erb +++ /dev/null @@ -1,55 +0,0 @@ -<% if @submission.preview? %> -<%= I18n.t("mailer.submission_confirmation.title_preview") %> - -<%= I18n.t("mailer.submission_confirmation.this_is_a_test") %> -<% else %> -<%= I18n.t("mailer.submission_confirmation.title") %> -<% end %> - -<%= I18n.t("mailer.submission_confirmation.submitted_at", form_name: form.name, - time: format_time(@submission.submission_time), - date: format_date(@submission.submission_time)) %> - -<%= I18n.t("mailer.submission_confirmation.reference_number", reference: @submission.reference) %> - -<%= I18n.t("mailer.submission_confirmation.you_cannot_reply") %> - -<% if form.payment_url.present? %> -<%= I18n.t("mailer.submission_confirmation.payment_link_heading") %> - -<%= strip_tags(I18n.t("mailer.submission_confirmation.payment_link_html", payment_link: form.payment_url_with_reference(@submission.reference))) %> - -<% end %> -<%= I18n.t("mailer.submission_confirmation.what_happens_next_heading") %> - -<% if form.what_happens_next_markdown.present? %> -<%= markdown_to_plain_text(form.what_happens_next_markdown) %> -<% else %> -<%= I18n.t("mailer.submission_confirmation.default_what_happens_next") %> -<% end %> - -<% if @include_copy_of_answers %> -<%= I18n.t("mailer.submission_confirmation.answers_submitted_heading") %> - -<%= @submission.answer_content_for_email_plain_text(locale: locale, confirmation_email: true) %> - -<% end %> -<%= I18n.t("mailer.submission_confirmation.contact_details_heading") %> - -<% if form.support_details.phone || form.support_details.email || form.support_details.url %> - <% if form.support_details.phone.present? %> -<%= normalize_whitespace(form.support_details.phone).html_safe %> - -<%= I18n.t('support_details.call_charges') %>: <%= form.support_details.call_charges_url %> - <% end %> - - <% if form.support_details.email.present? %> -<%= form.support_details.email %> - <% end %> - - <% if form.support_details.url.present? && form.support_details.url_text.present? %> -<%= form.support_details.url_text %>: <%= form.support_details.url %> - <% end %> -<% else %> - <%= I18n.t("mailer.submission_confirmation.default_support_contact_details") %> -<% end %> diff --git a/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.html.erb b/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.html.erb index 708df345e..bab3a619f 100644 --- a/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.html.erb +++ b/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.html.erb @@ -1,8 +1,9 @@ -<%= render partial: "content", locals: { form: @form, locale: :en } %> +<%= GovukFormsMarkdown.render_for_email(render(partial: "content", formats: [:md], locals: { form: @form, locale: :en })).html_safe %> <% if @submission_locale == :cy %> <% I18n.with_locale(:cy) do %> -
- <%= render partial: "content", locals: { form: @welsh_form, locale: :cy } %> +
+ +<%= GovukFormsMarkdown.render_for_email(render(partial: "content", formats: [:md], locals: { form: @welsh_form, locale: :cy })).html_safe %> <% end %> <% end %> diff --git a/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.text.erb b/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.text.erb index 841c72464..8138cbb9d 100644 --- a/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.text.erb +++ b/app/views/aws_ses_submission_confirmation_mailer/submission_confirmation_email.text.erb @@ -1,8 +1,9 @@ -<%= render partial: "content", locals: { form: @form, locale: :en } %> +<%= GovukFormsMarkdown.render_plain_text(render(partial: "content", formats: [:md], locals: { form: @form, locale: :en })) %> + <% if @submission_locale == :cy %> <% I18n.with_locale(:cy) do %> --- -<%= render partial: "content", locals: { form: @welsh_form, locale: :cy } %> +<%= GovukFormsMarkdown.render_plain_text(render(partial: "content", formats: [:md], locals: { form: @welsh_form, locale: :cy })) %> <% end %> <% end %> diff --git a/config/locales/cy.yml b/config/locales/cy.yml index 62ebaed37..d8a51bd92 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -524,7 +524,7 @@ cy: file_answer: Rydych wedi llwytho ffeil o’r enw %{filename} not_completed: Heb ei gwblhau payment_link_heading: Os ydych dal angen talu - payment_link_html: 'Os ydych angen gwneud taliad ac nad ydych wedi gwneud hynny eisoes, defnyddiwch y ddolen dalu hon i dalu nawr: %{payment_link}' + payment_link_markdown: 'Os ydych angen gwneud taliad ac nad ydych wedi gwneud hynny eisoes, defnyddiwch y ddolen dalu hon i dalu nawr: [%{payment_link}](%{payment_link})' reference_number: Rhif cyfeirnod eich ffurflen yw %{reference}. subject: 'Mae eich ffurflen wedi’i chyflwyno’n llwyddiannus – cyfeirnod: %{reference}' subject_preview: 'TEST EMAIL CONFIRMING SUBMISSION: Mae eich ffurflen wedi’i chyflwyno’n llwyddiannus – cyfeirnod: %{reference}' diff --git a/config/locales/en.yml b/config/locales/en.yml index a37e7bdb9..caeb5b742 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -524,7 +524,7 @@ en: file_answer: You uploaded a file called %{filename} not_completed: Not completed payment_link_heading: If you still need to pay - payment_link_html: 'If you need to make a payment and have not done so already, use this payment link to pay now: %{payment_link}' + payment_link_markdown: 'If you need to make a payment and have not done so already, use this payment link to pay now: [%{payment_link}](%{payment_link})' reference_number: Your form reference number is %{reference}. subject: 'Your form has been successfully submitted – reference: %{reference}' subject_preview: 'TEST EMAIL CONFIRMING SUBMISSION: Your form has been successfully submitted – reference: %{reference}' From 192161f08e9760ddb27556878b366b66378d2491 Mon Sep 17 00:00:00 2001 From: David Biddle Date: Wed, 24 Jun 2026 11:44:20 +0100 Subject: [PATCH 8/9] Use curly apostrophe in translation The Welsh translation already has a curly apostrophe, so this makes it consistent --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index caeb5b742..8cf83b572 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -529,7 +529,7 @@ en: subject: 'Your form has been successfully submitted – reference: %{reference}' subject_preview: 'TEST EMAIL CONFIRMING SUBMISSION: Your form has been successfully submitted – reference: %{reference}' submitted_at: "“%{form_name}” was successfully submitted at %{time} on %{date}." - this_is_a_test: This is a test email confirming that a form's been submitted. + this_is_a_test: This is a test email confirming that a form’s been submitted. title: Your form has been successfully submitted title_preview: 'TEST EMAIL CONFIRMING SUBMISSION: Your form has been successfully submitted' what_happens_next_heading: What happens next From d4504bbd6016e82b722b071b2694173a0d3b3fbd Mon Sep 17 00:00:00 2001 From: David Biddle Date: Thu, 25 Jun 2026 14:42:51 +0100 Subject: [PATCH 9/9] Delete unused HTML and plaintext methods --- app/lib/ses_email_formatter.rb | 81 +------- app/models/submission.rb | 8 - spec/lib/ses_email_formatter_spec.rb | 281 --------------------------- spec/models/submission_spec.rb | 28 --- 4 files changed, 7 insertions(+), 391 deletions(-) diff --git a/app/lib/ses_email_formatter.rb b/app/lib/ses_email_formatter.rb index af4207eb1..95a42ee92 100644 --- a/app/lib/ses_email_formatter.rb +++ b/app/lib/ses_email_formatter.rb @@ -12,20 +12,6 @@ def initialize(submission_reference:, steps:, confirmation_email:) @confirmation_email = confirmation_email end - def build_question_answers_section_html(heading_level: 3) - @steps.map { |step| - [prep_question_title_html(step, heading_level), - prep_answer_text_html(step)].join - }.join(H_RULE) - end - - def build_question_answers_section_plain_text - @steps.map { |step| - [prep_question_title_plain_text(step), - prep_answer_text_plain_text(step)].join("\n\n") - }.join(H_RULE_PLAIN_TEXT) - end - def build_question_answers_section_markdown(heading_level: 3) @steps.map { |step| [prep_question_title_markdown(step, heading_level), @@ -35,41 +21,20 @@ def build_question_answers_section_markdown(heading_level: 3) private - def prep_question_title_html(step, heading_level) - case heading_level - when 3 - "

#{prep_question_title_plain_text(step)}

" - when 4 - "

#{prep_question_title_plain_text(step)}

" - else - raise FormattingError, "unsupported heading level: #{heading_level}" - end - end - def prep_question_title_markdown(step, heading_level) case heading_level when 3 - "### #{prep_question_title_plain_text(step)}" + "### #{step.question.question_text}" when 4 - "#### #{prep_question_title_plain_text(step)}" + "#### #{step.question.question_text}" else raise FormattingError, "unsupported heading level: #{heading_level}" end end - def prep_answer_text_html(step) - if step.is_selection_with_none_of_the_above_answer? - prep_html_none_of_the_above_answer_text(step) - else - "

#{convert_newlines_to_html(prep_answer_text(step))}

" - end - rescue StandardError - raise FormattingError, "could not format answer for question step #{step.id}" - end - def prep_answer_text_markdown(step) if step.is_selection_with_none_of_the_above_answer? - prep_markdown_none_of_the_above_answer_text(step) + prep_none_of_the_above_answer_text_markdown(step) else prep_answer_text(step) end @@ -77,30 +42,14 @@ def prep_answer_text_markdown(step) raise FormattingError, "could not format answer for question step #{step.id}" end - def prep_html_none_of_the_above_answer_text(step) - [ - "

#{convert_newlines_to_html(prep_answer_text(step))}

", - "

#{convert_newlines_to_html(prep_none_of_the_above_question_text(step))}

", - "

#{convert_newlines_to_html(prep_none_of_the_above_answer_text(step))}

", - ] - end - - def prep_markdown_none_of_the_above_answer_text(step) + def prep_none_of_the_above_answer_text_markdown(step) [ prep_answer_text(step), - "#### #{prep_none_of_the_above_question_text(step)}", - prep_none_of_the_above_answer_text(step), + "#### #{step.question.none_of_the_above_question_text}", + prep_none_of_the_above_answer_markdown(step), ].join("\n\n") end - def prep_question_title_plain_text(step) - step.question.question_text - end - - def prep_none_of_the_above_question_text(step) - step.question.none_of_the_above_question_text - end - def prep_answer_text(step) answer = step.show_answer_in_email(submission_reference: @submission_reference, confirmation_email: @confirmation_email) @@ -111,7 +60,7 @@ def prep_answer_text(step) raise FormattingError, "could not format answer for question step #{step.id}" end - def prep_none_of_the_above_answer_text(step) + def prep_none_of_the_above_answer_markdown(step) answer = step.question.none_of_the_above_answer return skipped_question_text if answer.blank? @@ -121,22 +70,6 @@ def prep_none_of_the_above_answer_text(step) raise FormattingError, "could not format none of the above answer for question step #{step.id}" end - def prep_answer_text_plain_text(step) - if step.is_selection_with_none_of_the_above_answer? - prep_plain_text_none_of_the_above_answer_text(step) - else - prep_answer_text(step) - end - end - - def prep_plain_text_none_of_the_above_answer_text(step) - [ - prep_answer_text(step), - prep_none_of_the_above_question_text(step), - prep_none_of_the_above_answer_text(step), - ].join("\n\n") - end - def sanitize(text) text .then { normalize_whitespace _1 } diff --git a/app/models/submission.rb b/app/models/submission.rb index d0ef21187..b343e2e2f 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -59,14 +59,6 @@ def mode_object Mode.new(mode) end - def answer_content_for_email_html(heading_level:, locale: :en, confirmation_email: false) - ses_email_formatter(locale, confirmation_email).build_question_answers_section_html(heading_level:) - end - - def answer_content_for_email_plain_text(locale: :en, confirmation_email: false) - ses_email_formatter(locale, confirmation_email).build_question_answers_section_plain_text - end - def answer_content_for_email_markdown(heading_level:, locale: :en, confirmation_email: false) ses_email_formatter(locale, confirmation_email).build_question_answers_section_markdown(heading_level:) end diff --git a/spec/lib/ses_email_formatter_spec.rb b/spec/lib/ses_email_formatter_spec.rb index 724de4505..2361b9063 100644 --- a/spec/lib/ses_email_formatter_spec.rb +++ b/spec/lib/ses_email_formatter_spec.rb @@ -27,287 +27,6 @@ let(:none_of_the_above_step) { build :step, question: none_of_the_above_question } let(:steps) { [text_step] } - describe "#build_question_answers_section_html" do - context "when there is one step" do - it "returns question and and answer HTML" do - expected = <<~HTML.strip.gsub("\n", "") -

What is the meaning of life?

-

42

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - - it "formats with heading level 4" do - expected = <<~HTML.strip.gsub("\n", "") -

What is the meaning of life?

-

42

- HTML - expect(ses_email_formatter.build_question_answers_section_html(heading_level: 4)).to eq(expected) - end - - it "raises an error if the heading level is unsupported" do - expect { - ses_email_formatter.build_question_answers_section_html(heading_level: 2) - }.to raise_error(SesEmailFormatter::FormattingError, "unsupported heading level: 2") - end - end - - context "when the answer has multiple attributes" do - let(:steps) { [name_step] } - - it "inserts line breaks between answer attributes" do - expected = <<~HTML.strip.gsub("\n", "") -

What is your name?

-

First name: #{name_question.first_name}

Last name: #{name_question.last_name}

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - end - - context "when the answer is blank i.e. skipped" do - let(:text_question) { build :text, question_text: "What is the meaning of life?", text: nil } - let(:steps) { [text_step] } - - it "returns the blank answer text" do - expected = <<~HTML.strip.gsub("\n", "") -

What is the meaning of life?

-

[This question was skipped]

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the blank answer text" do - expect(ses_email_formatter.build_question_answers_section_html).to include("

Not completed

") - end - end - end - - context "when there is more than one step" do - let(:steps) { [text_step, name_step] } - - it "returns all question an answers separated by a horizontal rule" do - expected = <<~HTML.strip.gsub("\n", "") -

What is the meaning of life?

-

42

-
-

What is your name?

-

First name: #{name_question.first_name}

Last name: #{name_question.last_name}

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - end - - context "when there are special characters in the answer" do - let(:steps) { [text_step] } - - it "returns the sanitized answer" do - [ - { input: "\n\nTest\n\nTest 2", output: "Test

Test 2" }, - { input: " paragraph 1\n\n\n\n\n\n\n\n\n\n\n\n\n Another Paragraph with trailing space \n\n\n\n\n", output: "paragraph 1

Another Paragraph with trailing space" }, - - ].each do |test_case| - text_question.text = test_case[:input] - - expected = <<~HTML.strip.gsub("\n", "") -

What is the meaning of life?

-

#{test_case[:output]}

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - end - end - - context "when none of the above is selected in a none of the above question" do - let(:steps) { [none_of_the_above_step] } - - it "returns the sanitized answer including the none of the above answer" do - expected = <<~HTML.strip.gsub("\n", "") -

What sandwich do you want?

-

None of the above

-

Specify your desired sandwich

-

Cheese and pickle

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - - context "when the none of the above question has no answer is provided" do - let(:none_of_the_above_answer) { nil } - let(:none_of_the_above_question_is_optional) { "true" } - - it "returns the skipped none of the above answer text" do - expected = <<~HTML.strip.gsub("\n", "") -

What sandwich do you want?

-

None of the above

-

Specify your desired sandwich (optional)

-

[This question was skipped]

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the skipped none of the above answer text for a confirmation email" do - expect(ses_email_formatter.build_question_answers_section_html).to include("

Not completed

") - end - end - end - end - - context "when there is a file question" do - let(:steps) { [file_step] } - - context "when formatting for a submission email" do - it "returns the content for a submission email" do - expected = <<~HTML.strip.gsub("\n", "") -

Upload a file

-

a-file_SUB-12345.txt (attached to this email)

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the content for a confirmation email" do - expected = <<~HTML.strip.gsub("\n", "") -

Upload a file

-

You uploaded a file called a-file.txt

- HTML - expect(ses_email_formatter.build_question_answers_section_html).to eq(expected) - end - end - end - - context "when there is an error formatting an answer" do - before do - allow(text_step).to receive(:show_answer_in_email).and_raise(NoMethodError, "undefined method 'strip' for an instance of Array") - end - - it "raises an error with the page id" do - expect { - ses_email_formatter.build_question_answers_section_html - }.to raise_error(SesEmailFormatter::FormattingError, "could not format answer for question step #{text_step.id}") - end - end - end - - describe "#build_question_answers_section_plain_text" do - context "when there is one step" do - it "returns question and and answer HTML" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What is the meaning of life?\n\n42") - end - end - - context "when the answer has multiple attributes" do - let(:steps) { [name_step] } - - it "inserts line breaks between answer attributes" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What is your name?\n\nFirst name: #{name_question.first_name}\n\nLast name: #{name_question.last_name}") - end - end - - context "when the answer is blank i.e. skipped" do - let(:text_question) { build :text, question_text: "What is the meaning of life?", text: nil } - let(:steps) { [text_step] } - - it "returns the blank answer text" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What is the meaning of life?\n\n[This question was skipped]") - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the blank answer text" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to include("Not completed") - end - end - end - - context "when there is more than one step" do - let(:steps) { [text_step, name_step] } - - it "returns all question an answers separated by a horizontal rule" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What is the meaning of life?\n\n42\n\n---\n\nWhat is your name?\n\nFirst name: #{name_question.first_name}\n\nLast name: #{name_question.last_name}") - end - end - - context "when there are special characters in the answer" do - let(:steps) { [text_step] } - - it "returns the sanitized answer" do - [ - { input: "\n\nTest\n\nTest 2", output: "Test\n\nTest 2" }, - { input: " paragraph 1\n\n\n\n\n\n\n\n\n\n\n\n\n Another Paragraph with trailing space \n\n\n\n\n", output: "paragraph 1\n\nAnother Paragraph with trailing space" }, - - ].each do |test_case| - text_question.text = test_case[:input] - - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What is the meaning of life?\n\n#{test_case[:output]}") - end - end - end - - context "when none of the above is selected in a none of the above question" do - let(:steps) { [none_of_the_above_step] } - - it "returns the sanitized answer including the none of the above answer" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What sandwich do you want?\n\nNone of the above\n\nSpecify your desired sandwich\n\nCheese and pickle") - end - - context "when the none of the above question is optional and no answer is provided" do - let(:none_of_the_above_answer) { nil } - let(:none_of_the_above_question_is_optional) { "true" } - - it "returns the skipped none of the above answer text" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("What sandwich do you want?\n\nNone of the above\n\nSpecify your desired sandwich (optional)\n\n[This question was skipped]") - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the skipped none of the above answer text" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to include("Not completed") - end - end - end - end - - context "when there is a file question" do - let(:steps) { [file_step] } - - context "when formatting for a submission email" do - it "returns the content for a submission email" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("Upload a file\n\na-file_SUB-12345.txt (attached to this email)") - end - end - - context "when formatting for a confirmation email" do - let(:confirmation_email) { true } - - it "returns the content for a confirmation email" do - expect(ses_email_formatter.build_question_answers_section_plain_text).to eq("Upload a file\n\nYou uploaded a file called a-file.txt") - end - end - end - - context "when there is an error formatting an answer" do - before do - allow(text_step).to receive(:show_answer_in_email).and_raise(NoMethodError, "undefined method 'strip' for an instance of Array") - end - - it "raises an error with the page id" do - expect { - ses_email_formatter.build_question_answers_section_plain_text - }.to raise_error(SesEmailFormatter::FormattingError, "could not format answer for question step #{text_step.id}") - end - end - end - describe "#build_question_answers_section_markdown" do context "when there is one step" do it "returns question and and answer markdown" do diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 84abf2bfa..a9f12d887 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -183,34 +183,6 @@ end let(:answers) { { "q1" => { text: "blue" }, "q2" => { first_name: "Jane", last_name: "Doe" } } } - describe "#answer_content_for_email_html" do - it "uses the English form document to construct the HTML by default" do - result = submission.answer_content_for_email_html(heading_level: 4) - - expect(result).to include("What is your favourite colour?") - end - - it "uses the Welsh form document to construct the HTML when the locale is :cy" do - result = submission.answer_content_for_email_html(heading_level: 4, locale: :cy) - - expect(result).to include("Beth yw eich hoff liw?") - end - end - - describe "#answer_content_for_email_plain_text" do - it "uses the English form document to construct the answer content by default" do - result = submission.answer_content_for_email_plain_text - - expect(result).to start_with("What is your favourite colour?") - end - - it "uses the Welsh form document to construct the answer content when the locale is :cy" do - result = submission.answer_content_for_email_plain_text(locale: :cy) - - expect(result).to start_with("Beth yw eich hoff liw?") - end - end - describe "#answer_content_for_email_markdown" do it "uses the English form document to construct the markdown by default" do result = submission.answer_content_for_email_markdown(heading_level: 4)