Skip to content

Commit eb90806

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 573afac commit eb90806

File tree

10 files changed

+17
-119
lines changed

10 files changed

+17
-119
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 & 14 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
@@ -27,12 +22,4 @@ def strip_email_whitespace
2722
end
2823
end
2924

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
3825
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 & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -84,30 +84,4 @@
8484
end
8585
end
8686

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
11387
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("generated-uuid")
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: "generated-uuid-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: 10 additions & 54 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
@@ -173,7 +159,6 @@
173159
expect(EventLogger).not_to have_received(:log)
174160
end
175161

176-
include_examples "for notification references"
177162
end
178163
end
179164

@@ -195,7 +180,6 @@
195180
expect(EventLogger).to have_received(:log_form_event).with("check_answers")
196181
end
197182

198-
include_examples "for notification references"
199183
end
200184
end
201185
end
@@ -209,12 +193,6 @@
209193
)
210194
end
211195

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-
218196
context "with preview mode on" do
219197
let(:mode) { "preview-live" }
220198

@@ -243,8 +221,6 @@
243221
it "includes the confirmation_email_id in the logging context" do
244222
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
245223
end
246-
247-
include_examples "for notification references"
248224
end
249225

250226
context "with preview mode off" do
@@ -275,8 +251,6 @@
275251
it "includes the confirmation_email_id in the logging context" do
276252
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
277253
end
278-
279-
include_examples "for notification references"
280254
end
281255

282256
context "when the submission type is s3" do
@@ -359,7 +333,6 @@
359333
let(:email_confirmation_input) do
360334
{
361335
send_confirmation: nil,
362-
confirmation_email_reference:,
363336
}
364337
end
365338

@@ -375,8 +348,8 @@
375348
expect(response).to render_template("forms/check_your_answers/show")
376349
end
377350

378-
it "does not generate a new submission reference" do
379-
expect(response.body).to include confirmation_email_reference
351+
it "does not render a hidden confirmation reference field" do
352+
expect(response.body).not_to include("confirmation-email-reference")
380353
end
381354
end
382355

@@ -385,7 +358,6 @@
385358
{
386359
send_confirmation: "send_email",
387360
confirmation_email_address: nil,
388-
confirmation_email_reference:,
389361
}
390362
end
391363

@@ -401,19 +373,17 @@
401373
expect(response).to render_template("forms/check_your_answers/show")
402374
end
403375

404-
it "does not generate a new submission reference" do
405-
expect(response.body).to include confirmation_email_reference
376+
it "does not render a hidden confirmation reference field" do
377+
expect(response.body).not_to include("confirmation-email-reference")
406378
end
407379

408-
include_examples "for notification references"
409380
end
410381

411382
context "when user has not requested a confirmation email" do
412383
let(:email_confirmation_input) do
413384
{
414385
send_confirmation: "skip_confirmation",
415386
confirmation_email_address: nil,
416-
confirmation_email_reference:,
417387
}
418388
end
419389

@@ -424,21 +394,12 @@
424394
it "redirects to confirmation page" do
425395
expect(response).to redirect_to(form_submitted_path)
426396
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
435397
end
436398

437399
context "when user has requested a confirmation email" do
438400
let(:email_confirmation_input) do
439401
{ send_confirmation: "send_email",
440-
confirmation_email_address: Faker::Internet.email,
441-
confirmation_email_reference: }
402+
confirmation_email_address: Faker::Internet.email }
442403
end
443404

444405
before do
@@ -474,21 +435,18 @@
474435

475436
expect(mail.body.raw_source).to include(expected_personalisation.to_s)
476437

477-
expect(mail.govuk_notify_reference).to eq confirmation_email_reference
438+
expect(mail.govuk_notify_reference).to eq generated_confirmation_email_reference
478439
end
479440

480441
it "includes the confirmation_email_id in the logging context" do
481442
expect(log_lines.last["confirmation_email_id"]).to eq(confirmation_email_id)
482443
end
483-
484-
include_examples "for notification references"
485444
end
486445

487446
context "when there is a submission error" do
488447
let(:email_confirmation_input) do
489448
{ send_confirmation: "send_email",
490-
confirmation_email_address: Faker::Internet.email,
491-
confirmation_email_reference: }
449+
confirmation_email_address: Faker::Internet.email }
492450
end
493451

494452
before do
@@ -511,8 +469,6 @@
511469
it "returns 500" do
512470
expect(response).to have_http_status(:internal_server_error)
513471
end
514-
515-
include_examples "for notification references"
516472
end
517473

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

spec/views/forms/check_your_answers/show.html.erb_spec.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@
5656
expect(rendered).to have_css(".govuk-grid-column-two-thirds-from-desktop input[type='radio']")
5757
end
5858

59-
it "contains a hidden notify reference for the confirmation email" do
60-
expect(rendered).to have_field("confirmation-email-reference", type: "hidden", with: email_confirmation_input.confirmation_email_reference)
61-
end
62-
6359
it "displays the help link" do
6460
expect(rendered).to have_text(I18n.t("support_details.get_help_with_this_form"))
6561
end

0 commit comments

Comments
 (0)