Skip to content
Merged
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
15 changes: 15 additions & 0 deletions app/controllers/idv/by_mail/enter_code_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ def create
result = @gpo_verify_form.submit(resolved_authn_context_result.enhanced_ipp?)
analytics.idv_verify_by_mail_enter_code_submitted(**result)

send_please_call_email_if_necessary(result:)

if !result.success?
if rate_limiter.limited?
redirect_to idv_enter_code_rate_limited_url
Expand Down Expand Up @@ -120,6 +122,19 @@ def rate_limiter
)
end

# @param [FormResponse] result GpoVerifyForm result
def send_please_call_email_if_necessary(result:)
return if !result.success?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


return if result.extra[:pending_in_person_enrollment]

return if !result.extra[:fraud_check_failed]

return if !FeatureManagement.proofing_device_profiling_decisioning_enabled?
Comment on lines +129 to +133
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer what you have, but I feel like this would more typically be written like:

if result.extra[:pending_in_person_enrollment] ||
  !result.extra[:fraud_check_failed] ||
  !FeatureManagement.proofing_device_profiling_decisioning_enabled?
  return

But I like the idea of normalizing simpler lines. 👏


current_user.send_email_to_all_addresses(:idv_please_call)
end

def build_gpo_verify_form
GpoVerifyForm.new(
user: current_user,
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/idv/enter_password_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ def init_profile
log_letter_enqueued_analytics(resend: false)
end

if profile.fraud_review_pending? && !profile.in_person_verification_pending?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

current_user.send_email_to_all_addresses(:idv_please_call)
end

if profile.active?
create_user_event(:account_verified)
UserAlerts::AlertUserAboutAccountVerified.call(
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/get_usps_proofing_results_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ def send_failed_fraud_email(enrollment:, visited_location_name:)
def send_please_call_email(enrollment:, visited_location_name:)
enrollment.user.confirmed_email_addresses.each do |email_address|
# rubocop:disable IdentityIdp/MailLaterLinter
UserMailer.with(user: enrollment.user, email_address: email_address).in_person_please_call(
UserMailer.with(user: enrollment.user, email_address: email_address).idv_please_call(
enrollment: enrollment,
visited_location_name: visited_location_name,
).deliver_later(**notification_delivery_params(enrollment))
Expand Down
3 changes: 3 additions & 0 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ def account_verified(profile:)
end

def idv_please_call(**)
attachments.inline['phone_icon.png'] =
Rails.root.join('app/assets/images/email/phone_icon.png').read

with_user_locale(user) do
@hide_title = true

Expand Down
2 changes: 1 addition & 1 deletion app/views/user_mailer/idv_please_call.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= image_tag(
asset_url('email/phone_icon.png'),
attachments['phone_icon.png'].url,
Comment thread
matthinz marked this conversation as resolved.
Outdated
alt: t('image_description.phone_icon'),
width: 88,
height: 88,
Expand Down
16 changes: 16 additions & 0 deletions spec/controllers/idv/by_mail/enter_code_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,14 @@
expect(event_count).to eq 1
expect(response).to redirect_to(idv_personal_key_url)
end

it 'does not send the "Please Call" email' do
action
expect_email_not_delivered(
to: user.confirmed_email_addresses.first.email,
subject: t('user_mailer.idv_please_call.subject', app_name: APP_NAME),
)
end
end
end

Expand Down Expand Up @@ -323,6 +331,14 @@

expect(UserAlerts::AlertUserAboutAccountVerified).not_to have_received(:call)
end

it 'sends the "Please Call" email' do
action
expect_delivered_email(
to: user.confirmed_email_addresses.first.email,
subject: t('user_mailer.idv_please_call.subject', app_name: APP_NAME),
)
end
end

context 'with threatmetrix status of "review"' do
Expand Down
73 changes: 73 additions & 0 deletions spec/controllers/idv/enter_password_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
end
let(:applicant) { Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE }
let(:use_gpo) { false }
let(:threatmetrix_enabled) { true }
let(:threatmetrix_result) { 'pass' }
let(:idv_session) do
subject.idv_session
end
Expand All @@ -28,6 +30,7 @@
subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT)
subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE[:ssn]
subject.idv_session.threatmetrix_session_id = 'random-session-id'
subject.idv_session.threatmetrix_review_status = threatmetrix_result
subject.idv_session.resolution_successful = true
subject.idv_session.applicant = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE
subject.idv_session.resolution_successful = true
Expand All @@ -43,6 +46,9 @@
end

subject.idv_session.applicant = applicant.with_indifferent_access

allow(IdentityConfig.store).to receive(:proofing_device_profiling)
.and_return(threatmetrix_enabled ? :enabled : :disabled)
end

describe '#step_info' do
Expand Down Expand Up @@ -404,6 +410,41 @@ def show
expect(events_count).to eq 1
end

context 'user was flagged by ThreatMetrix' do
let(:threatmetrix_result) { 'reject' }

it 'sends the idv_please_call email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_delivered_email(
to: user.confirmed_email_addresses.first.email,
subject: t('user_mailer.idv_please_call.subject', app_name: APP_NAME),
)
end

it 'does not send the account_verified email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_email_not_delivered(
subject: t('user_mailer.account_verified.subject', app_name: APP_NAME),
)
end

context 'but ThreatMetrix disabled' do
let(:threatmetrix_enabled) { false }
it 'does not send the idv_please_call email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_email_not_delivered(
subject: t('user_mailer.idv_please_call.subject'),
)
end
it 'sends the account_verified email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_delivered_email(
subject: t('user_mailer.account_verified.subject', app_name: APP_NAME),
)
end
end
end

context 'with in person profile' do
let!(:enrollment) do
create(:in_person_enrollment, :establishing, user: user, profile: nil)
Expand Down Expand Up @@ -472,6 +513,17 @@ def show
)
end

context 'user was flagged by ThreatMetrix' do
let(:threatmetrix_result) { 'reject' }

it 'does not send the idv_please_call email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_email_not_delivered(
subject: t('user_mailer.idv_please_call.subject'),
)
end
end

context 'when there is a 4xx error' do
before do
stub_request_enroll_bad_request_response
Expand Down Expand Up @@ -965,6 +1017,27 @@ def show
expect(user.reload.gpo_verification_pending_profile).to be_nil
end
end

context 'user was flagged by ThreatMetrix' do
let(:threatmetrix_review_status) { 'reject' }

it 'does not send the idv_please_call email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_email_not_delivered(
subject: t('user_mailer.idv_please_call.subject'),
)
end

context 'but ThreatMetrix disabled' do
let(:threatmetrix_enabled) { false }
it 'does not send the idv_please_call email' do
put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }
expect_email_not_delivered(
subject: t('user_mailer.idv_please_call.subject'),
)
end
end
end
end

context 'user is going through enhanced ipp' do
Expand Down
4 changes: 2 additions & 2 deletions spec/jobs/get_usps_proofing_results_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,7 @@
allow(analytics).to receive(
:idv_in_person_usps_proofing_results_job_please_call_email_initiated,
)
allow(user_mailer).to receive(:in_person_please_call).and_return(mail_deliverer)
allow(user_mailer).to receive(:idv_please_call).and_return(mail_deliverer)
subject.perform(current_time)
end

Expand Down Expand Up @@ -1545,7 +1545,7 @@
end

it 'sends the please call email' do
expect(user_mailer).to have_received(:in_person_please_call).with(
expect(user_mailer).to have_received(:idv_please_call).with(
enrollment: enrollment,
visited_location_name: visited_location_name,
)
Expand Down
7 changes: 7 additions & 0 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,13 @@ def expect_email_body_to_have_help_and_contact_links

mail.deliver_later
end

it 'attaches the icon inline' do
icon_part = mail.attachments['phone_icon.png']
expect(icon_part).not_to be(nil)
expect(icon_part.inline?).to eql(true)
expect(icon_part.url).to start_with('cid:')
end
end

context 'in person emails' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
expect_delivered_email(
to: [user.confirmed_email_addresses.first.email],
subject: t('user_mailer.account_verified.subject', app_name: APP_NAME),
body: ['<table class="button expanded large radius">', 'localhost:3000'],
body: [
'http://www.example.com/redirect/return_to_sp/account_verified_cta',
],
)
end
end
Expand All @@ -50,7 +52,9 @@
described_class.call(profile: profile)

email_body = last_email.text_part.decoded.squish
expect(email_body).to_not include('<table class="button expanded large radius">')
expect(email_body).to_not include(
'http://www.example.com/redirect/return_to_sp/account_verified_cta',
)
end
end

Expand All @@ -69,7 +73,7 @@
expect_delivered_email(
to: [user.confirmed_email_addresses.first.email],
subject: t('user_mailer.account_verified.subject', app_name: APP_NAME),
body: ['<table class="button expanded large radius">', 'http://example.com'],
body: ['http://example.com'],
)
end
end
Expand Down
104 changes: 92 additions & 12 deletions spec/support/mailer_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,106 @@ def strip_tags(str)
ActionController::Base.helpers.strip_tags(str)
end

# @param [String,String[],nil] to The email address(es) the message must've been sent to.
# @param [String,nil] subject The subject the email must've had
# @param [String[],nil] Array of substrings that must appear in body.
def expect_delivered_email(to: nil, subject: nil, body: nil)
email = ActionMailer::Base.deliveries.find do |sent_mail|
next unless to.present? && sent_mail.to == to
next unless subject.present? && sent_mail.subject == subject
if body.present?
delivered_body = sent_mail.text_part.decoded.squish
body.to_a.each do |expected_body|
next unless delivered_body.include?(expected_body)
end
end
true
end
email = find_sent_email(to:, subject:, body:)

error_message = <<~ERROR
Unable to find email matching args:
to: #{to}
subject: #{subject}
body: #{body}
Sent mails: #{ActionMailer::Base.deliveries}
Sent mails:
#{summarize_all_deliveries(to:, subject:, body:).indent(2)}
ERROR

expect(email).to_not be(nil), error_message
end

# @param [String,nil] to If provided, the email address the message must've been sent to
# @param [String,nil] subject If provided, the subject the email must've had
# @param [String[],nil] Array of substrings that must appear in body.
def expect_email_not_delivered(to: nil, subject: nil, body: nil)
email = find_sent_email(to:, subject:, body:)

error_message = <<~ERROR
Found an email matching the below (but shouldn't have):
to: #{to}
subject: #{subject}
body: #{body}
Sent mails:
#{summarize_all_deliveries(to:, subject:, body:).indent(2)}
ERROR

expect(email).to be(nil), error_message
end

private

def body_matches(email:, body:)
return true if body.nil?

delivered_body = email.text_part.decoded.squish

Array.wrap(body).all? do |expected_substring|
delivered_body.include?(expected_substring)
end
end

def to_matches(email:, to:)
return true if to.nil?

to = Array.wrap(to).to_set

(email.to.to_set - to).empty?
end

def find_sent_email(
to:,
subject:,
body:
)
ActionMailer::Base.deliveries.find do |email|
to_ok = to_matches(email:, to:)
subject_ok = subject.nil? || email.subject == subject
body_ok = body_matches(email:, body:)

to_ok && subject_ok && body_ok
end
end

def summarize_delivery(
email:,
to:,
subject:,
body:
)
body_text = email.text_part.decoded.squish

body_summary = body.presence && Array.wrap(body).map do |substring|
found = body_text.include?(substring)
"- #{substring.inspect} (#{found ? 'found' : 'not found'})"
end.join("\n")

to_ok = to_matches(email:, to:)
subject_ok = subject.nil? || subject == email.subject

[
"To: #{email.to}#{to_ok ? '' : ' (did not match)'}",
"Subject: #{email.subject}#{subject_ok ? '' : ' (did not match)'}",
body.presence && "Body:\n#{body_summary.indent(2)}",
].compact.join("\n")
end

def summarize_all_deliveries(query)
ActionMailer::Base.deliveries.map do |email|
summary = summarize_delivery(email:, **query)
[
"- #{summary.lines.first.chomp}",
*summary.lines.drop(1).map { |l| l.chomp.indent(2) },
].join("\n")
end.join("\n")
end
end
Loading