From aa7483395f98d022477dbf5ac2cda33053f748b9 Mon Sep 17 00:00:00 2001 From: alice-carr Date: Tue, 16 Jun 2026 14:43:34 +0100 Subject: [PATCH] wip: Use SES to send all confirmation emails --- app/jobs/send_confirmation_email_job.rb | 16 +- .../form_submission_confirmation_mailer.rb | 73 ---- app/services/form_submission_service.rb | 1 - spec/jobs/send_confirmation_email_job_spec.rb | 41 +-- ...orm_submission_confirmation_mailer_spec.rb | 318 ------------------ .../check_your_answers_controller_spec.rb | 16 +- spec/services/form_submission_service_spec.rb | 16 +- 7 files changed, 26 insertions(+), 455 deletions(-) delete mode 100644 app/mailers/form_submission_confirmation_mailer.rb delete mode 100644 spec/mailers/form_submission_confirmation_mailer_spec.rb diff --git a/app/jobs/send_confirmation_email_job.rb b/app/jobs/send_confirmation_email_job.rb index 9e27963ba..0ff0bd4f1 100644 --- a/app/jobs/send_confirmation_email_job.rb +++ b/app/jobs/send_confirmation_email_job.rb @@ -1,24 +1,18 @@ class SendConfirmationEmailJob < ApplicationJob queue_as :confirmation_emails - def perform(submission:, notify_response_id:, confirmation_email_address:, include_copy_of_answers: false) + def perform(submission:, confirmation_email_address:, include_copy_of_answers: false) set_submission_logging_attributes(submission:) # The job will use the locale at the time it was created. Force it to be "en" as we send multilingual emails for # forms submitted in Welsh. I18n.with_locale("en") do - mail = if include_copy_of_answers - AwsSesSubmissionConfirmationMailer.submission_confirmation_email( - submission:, confirmation_email_address:, include_copy_of_answers:, - ) - else - FormSubmissionConfirmationMailer.send_confirmation_email( - submission:, notify_response_id:, confirmation_email_address:, - ) - end + mail = AwsSesSubmissionConfirmationMailer.submission_confirmation_email( + submission:, confirmation_email_address:, include_copy_of_answers:, + ) mail.deliver_now - CurrentJobLoggingAttributes.confirmation_email_id = mail.govuk_notify_response&.id.presence || mail.message_id + CurrentJobLoggingAttributes.confirmation_email_id = mail.message_id end rescue StandardError CloudWatchService.record_job_failure_metric(self.class.name) diff --git a/app/mailers/form_submission_confirmation_mailer.rb b/app/mailers/form_submission_confirmation_mailer.rb deleted file mode 100644 index d88685199..000000000 --- a/app/mailers/form_submission_confirmation_mailer.rb +++ /dev/null @@ -1,73 +0,0 @@ -class FormSubmissionConfirmationMailer < GovukNotifyRails::Mailer - include NotifyUtils - include EmailFormatHelper - - def send_confirmation_email(submission:, notify_response_id:, confirmation_email_address:) - @submission_locale = submission.submission_locale.to_sym - set_template(template_id) - - form = submission.form - welsh_form = submission.welsh_form - what_happens_next_text = form.what_happens_next_markdown.presence || default_what_happens_next_text - set_personalisation( - title: form.name, - title_cy: welsh_form&.name || form.name, - what_happens_next_text:, - what_happens_next_text_cy: welsh_form&.what_happens_next_markdown.presence || what_happens_next_text, - support_contact_details: format_support_details(form.support_details).presence || default_support_contact_details_text, - support_contact_details_cy: welsh_support_details(form, welsh_form), - submission_time: submission.submission_time.strftime("%l:%M%P").strip, - submission_date: I18n.l(submission.submission_time, format: "%-d %B %Y", locale: :en), - submission_date_cy: I18n.l(submission.submission_time, format: "%-d %B %Y", locale: :cy), - test: make_notify_boolean(submission.preview?), - submission_reference: submission.reference, - include_payment_link: make_notify_boolean(submission.payment_url.present?), - payment_link: form.payment_url_with_reference(submission.reference) || "", - payment_link_cy: welsh_form&.payment_url_with_reference(submission.reference) || "", - ) - - set_reference(notify_response_id) - - set_email_reply_to(Settings.govuk_notify.form_submission_email_reply_to_id) - - mail(to: confirmation_email_address) - end - - def format_support_details(support_details, locale: :en) - phone = support_details&.phone - call_charges_url = support_details&.call_charges_url - email = support_details&.email - url = support_details&.url - url_text = support_details&.url_text - - support_details = [] - support_details << normalize_whitespace(phone) if phone.present? - support_details << "[#{I18n.t('support_details.call_charges', locale: locale)}](#{call_charges_url})" if phone.present? - support_details << "[#{email}](mailto:#{email})" if email.present? - support_details << "[#{url_text}](#{url})" if url.present? && url_text.present? - - support_details.compact_blank.join("\n\n") - end - -private - - def welsh_support_details(form, welsh_form) - format_support_details(welsh_form&.support_details, locale: :cy).presence || - format_support_details(form.support_details, locale: :cy).presence || - default_support_contact_details_text - end - - def default_what_happens_next_text - I18n.t("mailer.submission_confirmation.default_what_happens_next") - end - - def default_support_contact_details_text - I18n.t("mailer.submission_confirmation.default_support_contact_details") - end - - def template_id - return Settings.govuk_notify.form_filler_confirmation_email_welsh_template_id if @submission_locale == :cy - - Settings.govuk_notify.form_filler_confirmation_email_template_id - end -end diff --git a/app/services/form_submission_service.rb b/app/services/form_submission_service.rb index c2b15b6b1..3ddd7666d 100644 --- a/app/services/form_submission_service.rb +++ b/app/services/form_submission_service.rb @@ -141,7 +141,6 @@ def validate_confirmation_email_address def enqueue_send_confirmation_email_job(submission:) SendConfirmationEmailJob.perform_later( submission:, - notify_response_id: email_confirmation_input.confirmation_email_reference, confirmation_email_address: confirmation_email_address, include_copy_of_answers: send_copy_of_answers?, ) do |job| diff --git a/spec/jobs/send_confirmation_email_job_spec.rb b/spec/jobs/send_confirmation_email_job_spec.rb index febe24310..b476aa6cc 100644 --- a/spec/jobs/send_confirmation_email_job_spec.rb +++ b/spec/jobs/send_confirmation_email_job_spec.rb @@ -30,7 +30,6 @@ submission_locale: "en", ) end - let(:notify_response_id) { "confirmation-ref" } let(:confirmation_email_address) { "testing@gov.uk" } context "when include_copy_of_answers is false" do @@ -43,7 +42,6 @@ expect { described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) }.to change(ActionMailer::Base.deliveries, :count).by(1) @@ -53,45 +51,19 @@ end it "builds mailer arguments from the submission" do - allow(FormSubmissionConfirmationMailer).to receive(:send_confirmation_email).and_call_original + allow(AwsSesSubmissionConfirmationMailer).to receive(:submission_confirmation_email).and_call_original described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) - expect(FormSubmissionConfirmationMailer).to have_received(:send_confirmation_email).with( + expect(AwsSesSubmissionConfirmationMailer).to have_received(:submission_confirmation_email).with( submission:, - notify_response_id: "confirmation-ref", confirmation_email_address: "testing@gov.uk", + include_copy_of_answers: false, ) end - - context "when submission locale is Welsh" do - let(:welsh_form_document) { build(:v2_form_document, name: "Welsh Form") } - - before do - submission.update!(submission_locale: "cy") - allow(Api::V2::FormDocumentRepository).to receive(:find_with_mode).and_call_original - allow(Api::V2::FormDocumentRepository).to receive(:find_with_mode).with( - form_id: anything, - mode: anything, - language: :cy, - ).and_return(welsh_form_document) - end - - it "uses the bilingual template" do - described_class.perform_now( - submission:, - notify_response_id:, - confirmation_email_address:, - ) - - mail = ActionMailer::Base.deliveries.last - expect(mail.govuk_notify_template).to eq("7891011") - end - end end context "when include_copy_of_answers is true" do @@ -99,7 +71,6 @@ expect { described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, include_copy_of_answers: true, ) @@ -115,7 +86,6 @@ I18n.with_locale(:cy) do described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, include_copy_of_answers: true, ) @@ -148,7 +118,6 @@ it "passes the confirmation email configuration set name to SES" do described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, include_copy_of_answers: true, ) @@ -162,7 +131,7 @@ context "when there is an error during processing" do before do - allow(FormSubmissionConfirmationMailer).to receive(:send_confirmation_email).and_raise(StandardError, "Test error") + allow(AwsSesSubmissionConfirmationMailer).to receive(:submission_confirmation_email).and_raise(StandardError, "Test error") allow(CloudWatchService).to receive(:record_job_failure_metric) end @@ -170,7 +139,6 @@ expect { described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) }.to raise_error(StandardError, "Test error") @@ -179,7 +147,6 @@ it "sends cloudwatch metric for failure" do described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) expect(CloudWatchService).to have_received(:record_job_failure_metric).with("SendConfirmationEmailJob") diff --git a/spec/mailers/form_submission_confirmation_mailer_spec.rb b/spec/mailers/form_submission_confirmation_mailer_spec.rb deleted file mode 100644 index ce55b06a7..000000000 --- a/spec/mailers/form_submission_confirmation_mailer_spec.rb +++ /dev/null @@ -1,318 +0,0 @@ -require "rails_helper" - -describe FormSubmissionConfirmationMailer, type: :mailer do - let(:submission_locale) { :en } - let(:mail) do - described_class.send_confirmation_email(submission:, - notify_response_id: "for-my-ref", - confirmation_email_address:) - end - let(:what_happens_next_markdown) { "Please wait for a response" } - let(:support_email) { nil } - let(:support_phone) { "0203 222 2222" } - let(:support_url) { nil } - let(:support_url_text) { nil } - let(:is_preview) { false } - let(:confirmation_email_address) { "testing@gov.uk" } - let(:submission_timestamp) { Time.zone.now } - let(:submission_reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } - let(:payment_url) { nil } - let(:submission) do - build :submission, - form_document:, - welsh_form_document:, - created_at: submission_timestamp, - reference: submission_reference, - submission_locale:, - is_preview: - end - let(:form_document) do - build :v2_form_document, - what_happens_next_markdown:, - support_email:, - support_phone:, - support_url:, - support_url_text:, - payment_url: - end - let(:form) { Form.new(form_document) } - let(:welsh_form_document) { nil } - - context "when form filler wants an form submission confirmation email" do - before do - Settings.govuk_notify.form_filler_confirmation_email_template_id = "123456" - Settings.govuk_notify.form_filler_confirmation_email_welsh_template_id = "7891011" - end - - it "sends an email to the form filler's email address" do - expect(mail.to).to eq(["testing@gov.uk"]) - end - - it "includes the form title" do - expect(mail.govuk_notify_personalisation[:title]).to eq(form.name) - end - - context "when no Welsh form is provided" do - it "falls back to the English title for title_cy" do - expect(mail.govuk_notify_personalisation[:title_cy]).to eq(form.name) - end - end - - context "when a Welsh form is provided" do - let(:welsh_form_document) { build(:v2_form_document, name: "Ffurflen 1") } - - it "uses the Welsh form name as title_cy" do - expect(mail.govuk_notify_personalisation[:title_cy]).to eq("Ffurflen 1") - end - end - - it "includes the forms what happens next" do - expect(mail.govuk_notify_personalisation[:what_happens_next_text]).to eq("Please wait for a response") - end - - it "includes the forms support contact details" do - expect(mail.govuk_notify_personalisation[:support_contact_details]).to eq("0203 222 2222\n\n[Find out about call charges](https://www.gov.uk/call-charges)") - end - - context "when what happens next is missing" do - let(:what_happens_next_markdown) { nil } - - it "uses placeholder text" do - expect(mail.govuk_notify_personalisation[:what_happens_next_text]).to eq(I18n.t("mailer.submission_confirmation.default_what_happens_next")) - end - end - - context "when what happens next is blank" do - let(:what_happens_next_markdown) { "" } - - it "uses placeholder text" do - expect(mail.govuk_notify_personalisation[:what_happens_next_text]).to eq(I18n.t("mailer.submission_confirmation.default_what_happens_next")) - end - end - - context "when support contact details are missing" do - let(:support_phone) { nil } - - it "uses placeholder text" do - expect(mail.govuk_notify_personalisation[:support_contact_details]).to eq(I18n.t("mailer.submission_confirmation.default_support_contact_details")) - end - end - - it "includes an email reference (mostly used to retrieve specific email in notify for e2e tests)" do - expect(mail.govuk_notify_reference).to eq("for-my-ref") - end - - it "does include an email-reply-to" do - Settings.govuk_notify.form_submission_email_reply_to_id = "send-this-to-me@gov.uk" - expect(mail.govuk_notify_email_reply_to).to eq("send-this-to-me@gov.uk") - end - - context "when a payment url is set" do - let(:payment_url) { "https://www.gov.uk/payments/test-service/pay-for-licence" } - - it "sets the boolean for the payment content to 'yes'" do - expect(mail.govuk_notify_personalisation[:include_payment_link]).to eq("yes") - end - - it "sets the payment_link" do - expect(mail.govuk_notify_personalisation[:payment_link]).to eq("#{payment_url}?reference=#{submission_reference}") - end - - it "sets payment_link_cy to an empty string when no Welsh form is provided" do - expect(mail.govuk_notify_personalisation[:payment_link_cy]).to eq("") - end - - context "when the Welsh form has a payment url" do - let(:welsh_payment_url) { "https://www.gov.uk/payments/welsh-service/pay-for-licence" } - let(:welsh_form_document) { build(:v2_form_document, payment_url: welsh_payment_url) } - - it "sets payment_link_cy to the Welsh payment url with reference" do - expect(mail.govuk_notify_personalisation[:payment_link_cy]).to eq("#{welsh_payment_url}?reference=#{submission_reference}") - end - end - end - - context "when a payment url is not set" do - let(:payment_url) { nil } - - it "sets the boolean for the payment content to 'no'" do - expect(mail.govuk_notify_personalisation[:include_payment_link]).to eq("no") - end - - it "sets the payment link personalisation to an empty string" do - expect(mail.govuk_notify_personalisation[:payment_link]).to eq("") - end - - it "sets the Welsh payment link personalisation to an empty string" do - expect(mail.govuk_notify_personalisation[:payment_link_cy]).to eq("") - end - end - - context "when submission_locale is :en" do - let(:submission_locale) { :en } - - it "uses the English language template" do - expect(mail.govuk_notify_template).to eq("123456") - end - end - - context "when submission_locale is :cy" do - let(:submission_locale) { :cy } - - it "uses the bilingual template" do - expect(mail.govuk_notify_template).to eq("7891011") - end - - context "when a Welsh version of the form is present" do - let(:welsh_what_happens_next_markdown) { "Arhoswch am ymateb" } - let(:welsh_support_phone) { "0291 111 1111" } - let(:welsh_form_document) do - build :v2_form_document, - what_happens_next_markdown: welsh_what_happens_next_markdown, - support_phone: welsh_support_phone - end - - it "includes the Welsh what happens next" do - expect(mail.govuk_notify_personalisation[:what_happens_next_text_cy]).to eq(welsh_what_happens_next_markdown) - end - - it "uses the Welsh support details formatted with Welsh locale" do - expect(mail.govuk_notify_personalisation[:support_contact_details_cy]).to eq("0291 111 1111\n\n[Darganfyddwch am gostau galwadau](https://www.gov.uk/call-charges)") - end - - context "when the what happens next is not set on the Welsh form" do - let(:welsh_what_happens_next_markdown) { nil } - - it "falls back to the English what happens next" do - expect(mail.govuk_notify_personalisation[:what_happens_next_text_cy]).to eq(what_happens_next_markdown) - end - end - - context "when the contact details are not set on the Welsh form" do - let(:welsh_support_phone) { nil } - - it "falls back to support_contact_details formatted with Welsh locale" do - expect(mail.govuk_notify_personalisation[:support_contact_details_cy]).to eq("0203 222 2222\n\n[Darganfyddwch am gostau galwadau](https://www.gov.uk/call-charges)") - end - end - end - end - - describe "submission date/time" do - context "with a time in BST" do - let(:timestamp) { Time.utc(2022, 9, 14, 8, 0o0, 0o0) } - - it "includes the time user submitted the form" do - travel_to timestamp do - expect(mail.govuk_notify_personalisation[:submission_time]).to eq("9:00am") - end - end - - it "includes the date user submitted the form in English" do - travel_to timestamp do - expect(mail.govuk_notify_personalisation[:submission_date]).to eq("14 September 2022") - end - end - - it "includes the date user submitted the form in Welsh" do - travel_to timestamp do - expect(mail.govuk_notify_personalisation[:submission_date_cy]).to eq("14 Medi 2022") - end - end - end - - context "with a time in GMT" do - let(:timestamp) { Time.utc(2022, 12, 14, 13, 0o0, 0o0) } - - it "includes the time user submitted the form" do - travel_to timestamp do - expect(mail.govuk_notify_personalisation[:submission_time]).to eq("1:00pm") - end - end - - it "includes the date user submitted the form" do - travel_to timestamp do - expect(mail.govuk_notify_personalisation[:submission_date]).to eq("14 December 2022") - end - end - end - end - - context "when the submission is from preview mode" do - let(:is_preview) { true } - - it "uses the preview personalisation" do - expect(mail.govuk_notify_personalisation[:test]).to eq("yes") - end - end - end - - describe "#format_support_details" do - let(:mailer) { described_class.new } - - context "with phone number only" do - let(:support_contact_details) { OpenStruct.new(phone: "0203 222 2222", email: nil, url: nil, url_text: nil, call_charges_url: "https://www.gov.uk/call-charges") } - - it "formats phone number with call charges link" do - result = mailer.format_support_details(support_contact_details) - expect(result).to eq("0203 222 2222\n\n[Find out about call charges](https://www.gov.uk/call-charges)") - end - end - - context "with email only" do - let(:support_contact_details) { OpenStruct.new(phone: nil, email: "help@example.gov.uk", url: nil, url_text: nil, call_charges_url: "https://www.gov.uk/call-charges") } - - it "formats email as a mailto link" do - result = mailer.format_support_details(support_contact_details) - expect(result).to eq("[help@example.gov.uk](mailto:help@example.gov.uk)") - end - end - - context "with support URL only" do - let(:support_contact_details) { OpenStruct.new(phone: nil, email: nil, url: "https://example.gov.uk/help", url_text: "Get help", call_charges_url: "https://www.gov.uk/call-charges") } - - it "formats support URL as a link" do - result = mailer.format_support_details(support_contact_details) - expect(result).to eq("[Get help](https://example.gov.uk/help)") - end - end - - context "with all support details" do - let(:support_contact_details) { OpenStruct.new(phone: "0203 222 2222", email: "help@example.gov.uk", url: "https://example.gov.uk/help", url_text: "Get help", call_charges_url: "https://www.gov.uk/call-charges") } - - it "formats all details with proper separation" do - result = mailer.format_support_details(support_contact_details) - expected = "0203 222 2222\n\n[Find out about call charges](https://www.gov.uk/call-charges)\n\n[help@example.gov.uk](mailto:help@example.gov.uk)\n\n[Get help](https://example.gov.uk/help)" - expect(result).to eq(expected) - end - end - - context "with no support details" do - let(:support_contact_details) { OpenStruct.new(phone: nil, email: nil, url: nil, url_text: nil, call_charges_url: "https://www.gov.uk/call-charges") } - - it "returns empty string" do - result = mailer.format_support_details(support_contact_details) - expect(result).to eq("") - end - end - - context "with phone number that has extra whitespace" do - let(:support_contact_details) { OpenStruct.new(phone: " 0203 222 2222\n\n ", email: nil, url: nil, url_text: nil, call_charges_url: "https://www.gov.uk/call-charges") } - - it "normalizes whitespace" do - result = mailer.format_support_details(support_contact_details) - expect(result).to eq("0203 222 2222\n\n[Find out about call charges](https://www.gov.uk/call-charges)") - end - end - end - -private - - def submission_timezone - Rails.configuration.x.submission.time_zone || "UTC" - end - - def submission_timestamp - Time.use_zone(submission_timezone) { Time.zone.now } - end -end diff --git a/spec/requests/forms/check_your_answers_controller_spec.rb b/spec/requests/forms/check_your_answers_controller_spec.rb index 1d47e1439..8add74140 100644 --- a/spec/requests/forms/check_your_answers_controller_spec.rb +++ b/spec/requests/forms/check_your_answers_controller_spec.rb @@ -281,8 +281,8 @@ describe "#submit_answers" do before do allow_mailer_to_return_mail_with_govuk_notify_response_with( - FormSubmissionConfirmationMailer, - :send_confirmation_email, + AwsSesSubmissionConfirmationMailer, + :submission_confirmation_email, id: confirmation_email_id, ) end @@ -319,7 +319,8 @@ end it "includes the confirmation_email_id in the logging context" do - expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id) + deliveries = ActionMailer::Base.deliveries + expect(log_lines.last["confirmation_email_id"]).to eq(deliveries[1].message_id) end include_examples "for notification references" @@ -351,7 +352,8 @@ end it "includes the confirmation_email_id in the logging context" do - expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id) + deliveries = ActionMailer::Base.deliveries + expect(log_lines.last["confirmation_email_id"]).to eq(deliveries[1].message_id) end include_examples "for notification references" @@ -618,13 +620,15 @@ payment_link_cy: "", } + # TODO: mail.body is coming back as nil. expect(mail.body.raw_source).to include(expected_personalisation.to_s) - + # TODO: also failing expect(mail.govuk_notify_reference).to eq confirmation_email_reference end it "includes the confirmation_email_id in the logging context" do - expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id) + deliveries = ActionMailer::Base.deliveries + expect(log_lines.last["confirmation_email_id"]).to eq(deliveries[1].message_id) end include_examples "for notification references" diff --git a/spec/services/form_submission_service_spec.rb b/spec/services/form_submission_service_spec.rb index 158bdb406..97ba3830e 100644 --- a/spec/services/form_submission_service_spec.rb +++ b/spec/services/form_submission_service_spec.rb @@ -372,7 +372,6 @@ expect(args).to include( "submission" => hash_including("_aj_globalid"), - "notify_response_id" => email_confirmation_input.confirmation_email_reference, "confirmation_email_address" => confirmation_email_address, "include_copy_of_answers" => false, ) @@ -428,15 +427,15 @@ end end end + end - context "when user does not want a confirmation email" do - let(:email_confirmation_input) { build :email_confirmation_input } + context "when user does not want a confirmation email" do + let(:email_confirmation_input) { build :email_confirmation_input } - it "does not call FormSubmissionConfirmationMailer" do - allow(FormSubmissionConfirmationMailer).to receive(:send_confirmation_email) - service.submit - expect(FormSubmissionConfirmationMailer).not_to have_received(:send_confirmation_email) - end + it "does not call AwsSesSubmissionConfirmationMailer" do + allow(AwsSesSubmissionConfirmationMailer).to receive(:submission_confirmation_email) + service.submit + expect(AwsSesSubmissionConfirmationMailer).not_to have_received(:submission_confirmation_email) end end @@ -454,7 +453,6 @@ expect(args).to include( "submission" => hash_including("_aj_globalid"), - "notify_response_id" => email_confirmation_input.confirmation_email_reference, "confirmation_email_address" => copy_of_answers_email_address, "include_copy_of_answers" => true, )