Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions app/controllers/forms/check_your_answers_controller.rb
Original file line number Diff line number Diff line change
@@ -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)

Expand Down Expand Up @@ -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
Expand Down
16 changes: 1 addition & 15 deletions app/input_objects/email_confirmation_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
5 changes: 3 additions & 2 deletions app/jobs/send_confirmation_email_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:),
)
Expand Down
1 change: 0 additions & 1 deletion app/services/form_submission_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 0 additions & 2 deletions app/views/forms/check_your_answers/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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? %>
<h2 class="govuk-heading-m govuk-!-margin-top-7"><%= t('form.check_your_answers.declaration') %></h2>
<%= HtmlMarkdownSanitizer.new.render_scrubbed_html(@current_context.form.declaration_text) %>
Expand Down
1 change: 0 additions & 1 deletion spec/factories/input_objects/email_confirmation_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
27 changes: 0 additions & 27 deletions spec/input_objects/email_confirmation_input_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 2 additions & 7 deletions spec/jobs/send_confirmation_email_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -49,7 +48,6 @@

described_class.perform_now(
submission:,
notify_response_id:,
confirmation_email_address:,
)

Expand All @@ -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),
)
Expand All @@ -73,7 +71,6 @@
submission.update!(submission_locale: "cy")
described_class.perform_now(
submission:,
notify_response_id:,
confirmation_email_address:,
)

Expand All @@ -92,7 +89,6 @@
expect {
described_class.perform_now(
submission:,
notify_response_id:,
confirmation_email_address:,
)
}.to raise_error(StandardError, "Test error")
Expand All @@ -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")
Expand Down
67 changes: 6 additions & 61 deletions spec/requests/forms/check_your_answers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
Expand Down Expand Up @@ -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|
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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" }

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -359,7 +331,6 @@
let(:email_confirmation_input) do
{
send_confirmation: nil,
confirmation_email_reference:,
}
end

Expand All @@ -374,18 +345,13 @@
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
let(:email_confirmation_input) do
{
send_confirmation: "send_email",
confirmation_email_address: nil,
confirmation_email_reference:,
}
end

Expand All @@ -400,20 +366,13 @@
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
let(:email_confirmation_input) do
{
send_confirmation: "skip_confirmation",
confirmation_email_address: nil,
confirmation_email_reference:,
}
end

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading