Skip to content

Commit 009c374

Browse files
committed
Move confirmation email reference generation to job runtime
Move confirmation email reference creation from EmailConfirmationInput request-time handling into SendConfirmationEmailJob, so the Notify reference is generated when the job executes only when we need it.
1 parent eb8ae4e commit 009c374

File tree

10 files changed

+13
-128
lines changed

10 files changed

+13
-128
lines changed

app/controllers/forms/check_your_answers_controller.rb

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
module Forms
22
class CheckYourAnswersController < BaseController
3-
def set_request_logging_attributes
4-
super
5-
if params[:email_confirmation_input].present? && (email_confirmation_input_params[:send_confirmation] == "send_email")
6-
CurrentRequestLoggingAttributes.confirmation_email_reference = email_confirmation_input_params[:confirmation_email_reference]
7-
end
8-
end
9-
103
def show
114
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)
125

@@ -58,7 +51,7 @@ def submit_answers
5851
private
5952

6053
def email_confirmation_input_params
61-
params.require(:email_confirmation_input).permit(:send_confirmation, :confirmation_email_address, :confirmation_email_reference)
54+
params.require(:email_confirmation_input).permit(:send_confirmation, :confirmation_email_address)
6255
end
6356

6457
def setup_check_your_answers

app/input_objects/email_confirmation_input.rb

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ class EmailConfirmationInput
33
include ActiveModel::Validations
44
include ActiveModel::Validations::Callbacks
55

6-
attr_accessor :send_confirmation, :confirmation_email_address, :confirmation_email_reference
6+
attr_accessor :send_confirmation, :confirmation_email_address
77

88
before_validation :strip_email_whitespace
99

@@ -12,11 +12,6 @@ class EmailConfirmationInput
1212
validates :confirmation_email_address, presence: true, if: :validate_email?
1313
validates :confirmation_email_address, email_address: { message: :invalid_email }, allow_blank: true, if: :validate_email?
1414

15-
def initialize(...)
16-
super(...)
17-
generate_notify_response_ids! unless @confirmation_email_reference
18-
end
19-
2015
def validate_email?
2116
send_confirmation == "send_email"
2217
end
@@ -26,13 +21,4 @@ def strip_email_whitespace
2621
self.confirmation_email_address = NotificationsUtils::Formatters.strip_and_remove_obscure_whitespace(confirmation_email_address)
2722
end
2823
end
29-
30-
private
31-
32-
def generate_notify_response_ids!
33-
uuid = SecureRandom.uuid
34-
self.attributes = {
35-
confirmation_email_reference: "#{uuid}-confirmation-email",
36-
}
37-
end
3824
end

app/jobs/send_confirmation_email_job.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ class SendConfirmationEmailJob < ApplicationJob
22
queue_as :confirmation_emails
33
MailerOptions = Data.define(:title, :is_preview, :timestamp, :submission_reference, :payment_url)
44

5-
def perform(submission:, notify_response_id:, confirmation_email_address:)
5+
def perform(submission:, confirmation_email_address:)
66
set_submission_logging_attributes(submission:)
7+
confirmation_email_reference = "#{SecureRandom.uuid}-confirmation-email"
78

89
I18n.with_locale(submission.submission_locale || I18n.default_locale) do
910
form = submission.form
1011
mail = FormSubmissionConfirmationMailer.send_confirmation_email(
1112
what_happens_next_markdown: form.what_happens_next_markdown,
1213
support_contact_details: form.support_details,
13-
notify_response_id:,
14+
notify_response_id: confirmation_email_reference,
1415
confirmation_email_address:,
1516
mailer_options: mailer_options_for(submission:, form:),
1617
)

app/services/form_submission_service.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ def validate_confirmation_email_address
130130
def enqueue_send_confirmation_email_job(submission:)
131131
SendConfirmationEmailJob.perform_later(
132132
submission:,
133-
notify_response_id: email_confirmation_input.confirmation_email_reference,
134133
confirmation_email_address: email_confirmation_input.confirmation_email_address,
135134
) do |job|
136135
next if job.successfully_enqueued?

app/views/forms/check_your_answers/show.html.erb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
<%= form.govuk_radio_button :send_confirmation, 'skip_confirmation' %>
2929
<% end %>
3030

31-
<%= form.hidden_field :confirmation_email_reference, id: 'confirmation-email-reference' %>
32-
3331
<%if @current_context.form.declaration_text.present? %>
3432
<h2 class="govuk-heading-m govuk-!-margin-top-7"><%= t('form.check_your_answers.declaration') %></h2>
3533
<%= HtmlMarkdownSanitizer.new.render_scrubbed_html(@current_context.form.declaration_text) %>

spec/factories/input_objects/email_confirmation_input.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
factory :email_confirmation_input, class: "EmailConfirmationInput" do
33
send_confirmation { nil }
44
confirmation_email_address { nil }
5-
confirmation_email_reference { "ffffffff-confirmation-email" }
65

76
factory :email_confirmation_input_opted_in do
87
send_confirmation { "send_email" }

spec/input_objects/email_confirmation_input_spec.rb

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -83,31 +83,4 @@
8383
expect(email_confirmation_input).to be_valid
8484
end
8585
end
86-
87-
describe "submission references" do
88-
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}/
89-
90-
let(:email_confirmation_input) do
91-
described_class.new
92-
end
93-
94-
let(:confirmation_email_reference) { email_confirmation_input.confirmation_email_reference }
95-
96-
it "generates a random email confirmation notification reference" do
97-
expect(confirmation_email_reference)
98-
.to match(uuid).and end_with("-confirmation-email")
99-
end
100-
101-
context "when intialised with references" do
102-
let(:email_confirmation_input) do
103-
described_class.new(
104-
confirmation_email_reference: "foo",
105-
)
106-
end
107-
108-
it "does not generate new references" do
109-
expect(confirmation_email_reference).to eq "foo"
110-
end
111-
end
112-
end
11386
end

spec/jobs/send_confirmation_email_job_spec.rb

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,18 @@
2323
submission_locale: "en",
2424
)
2525
end
26-
let(:notify_response_id) { "confirmation-ref" }
2726
let(:confirmation_email_address) { "testing@gov.uk" }
2827

2928
before do
3029
Settings.govuk_notify.form_filler_confirmation_email_template_id = "123456"
3130
Settings.govuk_notify.form_filler_confirmation_email_welsh_template_id = "7891011"
31+
allow(SecureRandom).to receive(:uuid).and_return("confirmation-ref")
3232
end
3333

3434
it "sends the confirmation email" do
3535
expect {
3636
described_class.perform_now(
3737
submission:,
38-
notify_response_id:,
3938
confirmation_email_address:,
4039
)
4140
}.to change(ActionMailer::Base.deliveries, :count).by(1)
@@ -49,7 +48,6 @@
4948

5049
described_class.perform_now(
5150
submission:,
52-
notify_response_id:,
5351
confirmation_email_address:,
5452
)
5553

@@ -62,7 +60,7 @@
6260
url: "https://example.gov.uk/help",
6361
url_text: "Get help",
6462
),
65-
notify_response_id: "confirmation-ref",
63+
notify_response_id: "confirmation-ref-confirmation-email",
6664
confirmation_email_address: "testing@gov.uk",
6765
mailer_options: an_instance_of(SendConfirmationEmailJob::MailerOptions),
6866
)
@@ -73,7 +71,6 @@
7371
submission.update!(submission_locale: "cy")
7472
described_class.perform_now(
7573
submission:,
76-
notify_response_id:,
7774
confirmation_email_address:,
7875
)
7976

@@ -92,7 +89,6 @@
9289
expect {
9390
described_class.perform_now(
9491
submission:,
95-
notify_response_id:,
9692
confirmation_email_address:,
9793
)
9894
}.to raise_error(StandardError, "Test error")
@@ -101,7 +97,6 @@
10197
it "sends cloudwatch metric for failure" do
10298
described_class.perform_now(
10399
submission:,
104-
notify_response_id:,
105100
confirmation_email_address:,
106101
)
107102
expect(CloudWatchService).to have_received(:record_job_failure_metric).with("SendConfirmationEmailJob")

spec/requests/forms/check_your_answers_controller_spec.rb

Lines changed: 6 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323

2424
let(:email_confirmation_input) do
2525
{ send_confirmation: "send_email",
26-
confirmation_email_address: Faker::Internet.email,
27-
confirmation_email_reference: }
26+
confirmation_email_address: Faker::Internet.email }
2827
end
2928

3029
let(:submission_email) { Faker::Internet.email(domain: "example.gov.uk") }
@@ -87,7 +86,7 @@
8786

8887
let(:reference) { Faker::Alphanumeric.alphanumeric(number: 8).upcase }
8988
let(:confirmation_email_id) { "2222" }
90-
let(:confirmation_email_reference) { "confirmation-email-ref" }
89+
let(:generated_confirmation_email_reference) { "generated-confirmation-uuid-confirmation-email" }
9190

9291
before do
9392
ActiveResource::HttpMock.respond_to do |mock|
@@ -101,23 +100,10 @@
101100
end
102101

103102
allow(ReferenceNumberService).to receive(:generate).and_return(reference)
103+
allow(SecureRandom).to receive(:uuid).and_return("generated-confirmation-uuid")
104104
end
105105

106106
describe "#show" do
107-
shared_examples "for notification references" do
108-
prepend_before do
109-
allow(EmailConfirmationInput).to receive(:new).and_wrap_original do |original_method, *args|
110-
double = original_method.call(*args)
111-
allow(double).to receive_messages(confirmation_email_reference:)
112-
double
113-
end
114-
end
115-
116-
it "includes a notification reference for the confirmation email" do
117-
expect(response.body).to include confirmation_email_reference
118-
end
119-
end
120-
121107
shared_examples "for redirecting if the form is incomplete" do
122108
context "without any questions answered" do
123109
let(:store) do
@@ -172,8 +158,6 @@
172158
it "does not log the form_check_answers event" do
173159
expect(EventLogger).not_to have_received(:log)
174160
end
175-
176-
include_examples "for notification references"
177161
end
178162
end
179163

@@ -194,8 +178,6 @@
194178
it "Logs the form_check_answers event" do
195179
expect(EventLogger).to have_received(:log_form_event).with("check_answers")
196180
end
197-
198-
include_examples "for notification references"
199181
end
200182
end
201183
end
@@ -209,12 +191,6 @@
209191
)
210192
end
211193

212-
shared_examples "for notification references" do
213-
it "includes the confirmation_email_reference in the logging_context" do
214-
expect(log_line["confirmation_email_reference"]).to eq(confirmation_email_reference)
215-
end
216-
end
217-
218194
context "with preview mode on" do
219195
let(:mode) { "preview-live" }
220196

@@ -243,8 +219,6 @@
243219
it "includes the confirmation_email_id in the logging context" do
244220
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
245221
end
246-
247-
include_examples "for notification references"
248222
end
249223

250224
context "with preview mode off" do
@@ -275,8 +249,6 @@
275249
it "includes the confirmation_email_id in the logging context" do
276250
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
277251
end
278-
279-
include_examples "for notification references"
280252
end
281253

282254
context "when the submission type is s3" do
@@ -359,7 +331,6 @@
359331
let(:email_confirmation_input) do
360332
{
361333
send_confirmation: nil,
362-
confirmation_email_reference:,
363334
}
364335
end
365336

@@ -374,18 +345,13 @@
374345
it "renders the check your answers page" do
375346
expect(response).to render_template("forms/check_your_answers/show")
376347
end
377-
378-
it "does not generate a new submission reference" do
379-
expect(response.body).to include confirmation_email_reference
380-
end
381348
end
382349

383350
context "when user has not specified the confirmation email address" do
384351
let(:email_confirmation_input) do
385352
{
386353
send_confirmation: "send_email",
387354
confirmation_email_address: nil,
388-
confirmation_email_reference:,
389355
}
390356
end
391357

@@ -400,20 +366,13 @@
400366
it "renders the check your answers page" do
401367
expect(response).to render_template("forms/check_your_answers/show")
402368
end
403-
404-
it "does not generate a new submission reference" do
405-
expect(response.body).to include confirmation_email_reference
406-
end
407-
408-
include_examples "for notification references"
409369
end
410370

411371
context "when user has not requested a confirmation email" do
412372
let(:email_confirmation_input) do
413373
{
414374
send_confirmation: "skip_confirmation",
415375
confirmation_email_address: nil,
416-
confirmation_email_reference:,
417376
}
418377
end
419378

@@ -424,21 +383,12 @@
424383
it "redirects to confirmation page" do
425384
expect(response).to redirect_to(form_submitted_path)
426385
end
427-
428-
it "does not include the confirmation_email_id in the logging context" do
429-
expect(log_line.keys).not_to include("confirmation_email_id")
430-
end
431-
432-
it "does not include confirmation_email_reference in logging context" do
433-
expect(log_line.keys).not_to include("confirmation_email_reference")
434-
end
435386
end
436387

437388
context "when user has requested a confirmation email" do
438389
let(:email_confirmation_input) do
439390
{ send_confirmation: "send_email",
440-
confirmation_email_address: Faker::Internet.email,
441-
confirmation_email_reference: }
391+
confirmation_email_address: Faker::Internet.email }
442392
end
443393

444394
before do
@@ -474,21 +424,18 @@
474424

475425
expect(mail.body.raw_source).to include(expected_personalisation.to_s)
476426

477-
expect(mail.govuk_notify_reference).to eq confirmation_email_reference
427+
expect(mail.govuk_notify_reference).to eq generated_confirmation_email_reference
478428
end
479429

480430
it "includes the confirmation_email_id in the logging context" do
481431
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
482432
end
483-
484-
include_examples "for notification references"
485433
end
486434

487435
context "when there is a submission error" do
488436
let(:email_confirmation_input) do
489437
{ send_confirmation: "send_email",
490-
confirmation_email_address: Faker::Internet.email,
491-
confirmation_email_reference: }
438+
confirmation_email_address: Faker::Internet.email }
492439
end
493440

494441
before do
@@ -511,8 +458,6 @@
511458
it "returns 500" do
512459
expect(response).to have_http_status(:internal_server_error)
513460
end
514-
515-
include_examples "for notification references"
516461
end
517462

518463
context "when there is an ActionMailer error with the confirmation email address" do

0 commit comments

Comments
 (0)