diff --git a/app/controllers/forms/check_your_answers_controller.rb b/app/controllers/forms/check_your_answers_controller.rb index e81cc2843..90945254b 100644 --- a/app/controllers/forms/check_your_answers_controller.rb +++ b/app/controllers/forms/check_your_answers_controller.rb @@ -1,12 +1,5 @@ module Forms class CheckYourAnswersController < BaseController - def set_request_logging_attributes - super - if params[:email_confirmation_input].present? && (email_confirmation_input_params[:send_confirmation] == "send_email") - CurrentRequestLoggingAttributes.confirmation_email_reference = email_confirmation_input_params[:confirmation_email_reference] - end - end - def show return redirect_to form_page_path(current_context.form.id, current_context.form.form_slug, current_context.next_page_slug, nil) unless current_context.can_visit?(CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG) @@ -58,7 +51,7 @@ def submit_answers private def email_confirmation_input_params - params.require(:email_confirmation_input).permit(:send_confirmation, :confirmation_email_address, :confirmation_email_reference) + params.require(:email_confirmation_input).permit(:send_confirmation, :confirmation_email_address) end def setup_check_your_answers diff --git a/app/input_objects/email_confirmation_input.rb b/app/input_objects/email_confirmation_input.rb index aceb8920f..178dab30f 100644 --- a/app/input_objects/email_confirmation_input.rb +++ b/app/input_objects/email_confirmation_input.rb @@ -3,7 +3,7 @@ class EmailConfirmationInput include ActiveModel::Validations include ActiveModel::Validations::Callbacks - attr_accessor :send_confirmation, :confirmation_email_address, :confirmation_email_reference + attr_accessor :send_confirmation, :confirmation_email_address before_validation :strip_email_whitespace @@ -12,11 +12,6 @@ class EmailConfirmationInput validates :confirmation_email_address, presence: true, if: :validate_email? validates :confirmation_email_address, email_address: { message: :invalid_email }, allow_blank: true, if: :validate_email? - def initialize(...) - super(...) - generate_notify_response_ids! unless @confirmation_email_reference - end - def validate_email? send_confirmation == "send_email" end @@ -26,13 +21,4 @@ def strip_email_whitespace self.confirmation_email_address = NotificationsUtils::Formatters.strip_and_remove_obscure_whitespace(confirmation_email_address) end end - -private - - def generate_notify_response_ids! - uuid = SecureRandom.uuid - self.attributes = { - confirmation_email_reference: "#{uuid}-confirmation-email", - } - end end diff --git a/app/jobs/send_confirmation_email_job.rb b/app/jobs/send_confirmation_email_job.rb index 684213e51..3cdf09c8b 100644 --- a/app/jobs/send_confirmation_email_job.rb +++ b/app/jobs/send_confirmation_email_job.rb @@ -2,15 +2,16 @@ class SendConfirmationEmailJob < ApplicationJob queue_as :confirmation_emails MailerOptions = Data.define(:title, :is_preview, :timestamp, :submission_reference, :payment_url) - def perform(submission:, notify_response_id:, confirmation_email_address:) + def perform(submission:, confirmation_email_address:) set_submission_logging_attributes(submission:) + confirmation_email_reference = "#{SecureRandom.uuid}-confirmation-email" I18n.with_locale(submission.submission_locale || I18n.default_locale) do form = submission.form mail = FormSubmissionConfirmationMailer.send_confirmation_email( what_happens_next_markdown: form.what_happens_next_markdown, support_contact_details: form.support_details, - notify_response_id:, + notify_response_id: confirmation_email_reference, confirmation_email_address:, mailer_options: mailer_options_for(submission:, form:), ) diff --git a/app/services/form_submission_service.rb b/app/services/form_submission_service.rb index 2e536effd..a7e1390e7 100644 --- a/app/services/form_submission_service.rb +++ b/app/services/form_submission_service.rb @@ -130,7 +130,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: email_confirmation_input.confirmation_email_address, ) do |job| next if job.successfully_enqueued? diff --git a/app/views/forms/check_your_answers/show.html.erb b/app/views/forms/check_your_answers/show.html.erb index a4466d320..c9fe6a482 100644 --- a/app/views/forms/check_your_answers/show.html.erb +++ b/app/views/forms/check_your_answers/show.html.erb @@ -28,8 +28,6 @@ <%= form.govuk_radio_button :send_confirmation, 'skip_confirmation' %> <% end %> - <%= form.hidden_field :confirmation_email_reference, id: 'confirmation-email-reference' %> - <%if @current_context.form.declaration_text.present? %>

<%= t('form.check_your_answers.declaration') %>

<%= HtmlMarkdownSanitizer.new.render_scrubbed_html(@current_context.form.declaration_text) %> diff --git a/spec/factories/input_objects/email_confirmation_input.rb b/spec/factories/input_objects/email_confirmation_input.rb index a420985a0..0e67586b9 100644 --- a/spec/factories/input_objects/email_confirmation_input.rb +++ b/spec/factories/input_objects/email_confirmation_input.rb @@ -2,7 +2,6 @@ factory :email_confirmation_input, class: "EmailConfirmationInput" do send_confirmation { nil } confirmation_email_address { nil } - confirmation_email_reference { "ffffffff-confirmation-email" } factory :email_confirmation_input_opted_in do send_confirmation { "send_email" } diff --git a/spec/input_objects/email_confirmation_input_spec.rb b/spec/input_objects/email_confirmation_input_spec.rb index fcf62517b..3a2fb71f4 100644 --- a/spec/input_objects/email_confirmation_input_spec.rb +++ b/spec/input_objects/email_confirmation_input_spec.rb @@ -83,31 +83,4 @@ expect(email_confirmation_input).to be_valid end end - - describe "submission references" do - uuid = /[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}/ - - let(:email_confirmation_input) do - described_class.new - end - - let(:confirmation_email_reference) { email_confirmation_input.confirmation_email_reference } - - it "generates a random email confirmation notification reference" do - expect(confirmation_email_reference) - .to match(uuid).and end_with("-confirmation-email") - end - - context "when intialised with references" do - let(:email_confirmation_input) do - described_class.new( - confirmation_email_reference: "foo", - ) - end - - it "does not generate new references" do - expect(confirmation_email_reference).to eq "foo" - end - end - end end diff --git a/spec/jobs/send_confirmation_email_job_spec.rb b/spec/jobs/send_confirmation_email_job_spec.rb index 7c3569bb0..7961d476f 100644 --- a/spec/jobs/send_confirmation_email_job_spec.rb +++ b/spec/jobs/send_confirmation_email_job_spec.rb @@ -23,19 +23,18 @@ submission_locale: "en", ) end - let(:notify_response_id) { "confirmation-ref" } let(:confirmation_email_address) { "testing@gov.uk" } before do Settings.govuk_notify.form_filler_confirmation_email_template_id = "123456" Settings.govuk_notify.form_filler_confirmation_email_welsh_template_id = "7891011" + allow(SecureRandom).to receive(:uuid).and_return("confirmation-ref") end it "sends the confirmation email" do expect { described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) }.to change(ActionMailer::Base.deliveries, :count).by(1) @@ -49,7 +48,6 @@ described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) @@ -62,7 +60,7 @@ url: "https://example.gov.uk/help", url_text: "Get help", ), - notify_response_id: "confirmation-ref", + notify_response_id: "confirmation-ref-confirmation-email", confirmation_email_address: "testing@gov.uk", mailer_options: an_instance_of(SendConfirmationEmailJob::MailerOptions), ) @@ -73,7 +71,6 @@ submission.update!(submission_locale: "cy") described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) @@ -92,7 +89,6 @@ expect { described_class.perform_now( submission:, - notify_response_id:, confirmation_email_address:, ) }.to raise_error(StandardError, "Test error") @@ -101,7 +97,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/requests/forms/check_your_answers_controller_spec.rb b/spec/requests/forms/check_your_answers_controller_spec.rb index 340b01c03..6bc91c66c 100644 --- a/spec/requests/forms/check_your_answers_controller_spec.rb +++ b/spec/requests/forms/check_your_answers_controller_spec.rb @@ -23,8 +23,7 @@ let(:email_confirmation_input) do { send_confirmation: "send_email", - confirmation_email_address: Faker::Internet.email, - confirmation_email_reference: } + confirmation_email_address: Faker::Internet.email } end let(:submission_email) { Faker::Internet.email(domain: "example.gov.uk") } @@ -87,7 +86,7 @@ let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase } let(:confirmation_email_id) { "2222" } - let(:confirmation_email_reference) { "confirmation-email-ref" } + let(:generated_confirmation_email_reference) { "generated-confirmation-uuid-confirmation-email" } before do ActiveResource::HttpMock.respond_to do |mock| @@ -101,23 +100,10 @@ end allow(ReferenceNumberService).to receive(:generate).and_return(reference) + allow(SecureRandom).to receive(:uuid).and_return("generated-confirmation-uuid") end describe "#show" do - shared_examples "for notification references" do - prepend_before do - allow(EmailConfirmationInput).to receive(:new).and_wrap_original do |original_method, *args| - double = original_method.call(*args) - allow(double).to receive_messages(confirmation_email_reference:) - double - end - end - - it "includes a notification reference for the confirmation email" do - expect(response.body).to include confirmation_email_reference - end - end - shared_examples "for redirecting if the form is incomplete" do context "without any questions answered" do let(:store) do @@ -172,8 +158,6 @@ it "does not log the form_check_answers event" do expect(EventLogger).not_to have_received(:log) end - - include_examples "for notification references" end end @@ -194,8 +178,6 @@ it "Logs the form_check_answers event" do expect(EventLogger).to have_received(:log_form_event).with("check_answers") end - - include_examples "for notification references" end end end @@ -209,12 +191,6 @@ ) end - shared_examples "for notification references" do - it "includes the confirmation_email_reference in the logging_context" do - expect(log_line["confirmation_email_reference"]).to eq(confirmation_email_reference) - end - end - context "with preview mode on" do let(:mode) { "preview-live" } @@ -243,8 +219,6 @@ it "includes the confirmation_email_id in the logging context" do expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id) end - - include_examples "for notification references" end context "with preview mode off" do @@ -275,8 +249,6 @@ it "includes the confirmation_email_id in the logging context" do expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id) end - - include_examples "for notification references" end context "when the submission type is s3" do @@ -359,7 +331,6 @@ let(:email_confirmation_input) do { send_confirmation: nil, - confirmation_email_reference:, } end @@ -374,10 +345,6 @@ it "renders the check your answers page" do expect(response).to render_template("forms/check_your_answers/show") end - - it "does not generate a new submission reference" do - expect(response.body).to include confirmation_email_reference - end end context "when user has not specified the confirmation email address" do @@ -385,7 +352,6 @@ { send_confirmation: "send_email", confirmation_email_address: nil, - confirmation_email_reference:, } end @@ -400,12 +366,6 @@ it "renders the check your answers page" do expect(response).to render_template("forms/check_your_answers/show") end - - it "does not generate a new submission reference" do - expect(response.body).to include confirmation_email_reference - end - - include_examples "for notification references" end context "when user has not requested a confirmation email" do @@ -413,7 +373,6 @@ { send_confirmation: "skip_confirmation", confirmation_email_address: nil, - confirmation_email_reference:, } end @@ -424,21 +383,12 @@ it "redirects to confirmation page" do expect(response).to redirect_to(form_submitted_path) end - - it "does not include the confirmation_email_id in the logging context" do - expect(log_line.keys).not_to include("confirmation_email_id") - end - - it "does not include confirmation_email_reference in logging context" do - expect(log_line.keys).not_to include("confirmation_email_reference") - end end context "when user has requested a confirmation email" do let(:email_confirmation_input) do { send_confirmation: "send_email", - confirmation_email_address: Faker::Internet.email, - confirmation_email_reference: } + confirmation_email_address: Faker::Internet.email } end before do @@ -474,21 +424,18 @@ expect(mail.body.raw_source).to include(expected_personalisation.to_s) - expect(mail.govuk_notify_reference).to eq confirmation_email_reference + expect(mail.govuk_notify_reference).to eq generated_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) end - - include_examples "for notification references" end context "when there is a submission error" do let(:email_confirmation_input) do { send_confirmation: "send_email", - confirmation_email_address: Faker::Internet.email, - confirmation_email_reference: } + confirmation_email_address: Faker::Internet.email } end before do @@ -511,8 +458,6 @@ it "returns 500" do expect(response).to have_http_status(:internal_server_error) end - - include_examples "for notification references" end context "when there is an ActionMailer error with the confirmation email address" do diff --git a/spec/views/forms/check_your_answers/show.html.erb_spec.rb b/spec/views/forms/check_your_answers/show.html.erb_spec.rb index 8cd94a04c..06024eb61 100644 --- a/spec/views/forms/check_your_answers/show.html.erb_spec.rb +++ b/spec/views/forms/check_your_answers/show.html.erb_spec.rb @@ -56,10 +56,6 @@ expect(rendered).to have_css(".govuk-grid-column-two-thirds-from-desktop input[type='radio']") end - it "contains a hidden notify reference for the confirmation email" do - expect(rendered).to have_field("confirmation-email-reference", type: "hidden", with: email_confirmation_input.confirmation_email_reference) - end - it "displays the help link" do expect(rendered).to have_text(I18n.t("support_details.get_help_with_this_form")) end