Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Document Upload Failure] Email veteran on Form 0781/Form 0781a upload retry exhaustion #18206

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# frozen_string_literal: true

require 'va_notify/service'

module EVSS
module DisabilityCompensationForm
class Form0781DocumentUploadFailureEmail < Job

Choose a reason for hiding this comment

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

Assumes too much for instance variable '@notify_client' - InstanceVariableAssumption

STATSD_METRIC_PREFIX = 'api.form_526.veteran_notifications.form0781_upload_failure_email'

# retry for one day
sidekiq_options retry: 14

sidekiq_retries_exhausted do |msg, _ex|
job_id = msg['jid']
error_class = msg['error_class']
error_message = msg['error_message']
timestamp = Time.now.utc
form526_submission_id = msg['args'].first

# Job status records are upserted in the JobTracker module
# when the retryable_error_handler is called
form_job_status = Form526JobStatus.find_by(job_id:)
bgjob_errors = form_job_status.bgjob_errors || {}
new_error = {
"#{timestamp.to_i}": {
caller_method: __method__.to_s,
timestamp:,
form526_submission_id:
}
}
form_job_status.update(
status: Form526JobStatus::STATUS[:exhausted],
bgjob_errors: bgjob_errors.merge(new_error)
)

Rails.logger.warn(
'Form0781DocumentUploadFailureEmail retries exhausted',
{
job_id:,
timestamp:,
form526_submission_id:,
error_class:,
error_message:
}
)

StatsD.increment("#{STATSD_METRIC_PREFIX}.exhausted")
rescue => e
Rails.logger.error(
'Failure in Form0781DocumentUploadFailureEmail#sidekiq_retries_exhausted',
{
job_id:,
messaged_content: e.message,
submission_id: form526_submission_id,
pre_exhaustion_failure: {
error_class:,
error_message:
Copy link
Contributor

Choose a reason for hiding this comment

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

are error_class and error_message defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, on lines 15 and 16: the rescue is within the block so should have access to those local variables, unless did you catch me missing a Ruby quirk here you're hinting at?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah i see.

}
}
)
raise e
end

def perform(form526_submission_id)

Choose a reason for hiding this comment

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

Has approx 10 statements - TooManyStatements

form526_submission = Form526Submission.find(form526_submission_id)

with_tracking('Form0781DocumentUploadFailureEmail', form526_submission.saved_claim_id, form526_submission_id) do
Copy link

Choose a reason for hiding this comment

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

Refers to 'form526_submission' more than self (maybe move it to another class?) - FeatureEnvy

notify_client = VaNotify::Service.new(Settings.vanotify.services.benefits_disability.api_key)

email_address = form526_submission.veteran_email_address
first_name = form526_submission.get_first_name
date_submitted = form526_submission.format_creation_time_for_mailers

notify_client.send_email(
email_address:,
template_id: mailer_template_id,
personalisation: {
first_name:,
date_submitted:
}
)

StatsD.increment("#{STATSD_METRIC_PREFIX}.success")
end
rescue => e
retryable_error_handler(e)
end

private

def retryable_error_handler(error)
# Needed to log the error properly in the Sidekiq::Form526JobStatusTracker::JobTracker,
# which is included near the top of this job's inheritance tree in EVSS::DisabilityCompensationForm::JobStatus
super(error)
raise error
end

def mailer_template_id
Settings.vanotify.services
.benefits_disability.template_id.form0781_upload_failure_notification_template_id
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class SubmitForm0781 < Job

StatsD.increment("#{STATSD_KEY_PREFIX}.exhausted")

if Flipper.enabled?(:form526_send_0781_failure_notification)
EVSS::DisabilityCompensationForm::Form0781DocumentUploadFailureEmail.perform_async(form526_submission_id)
end

::Rails.logger.warn(
'Submit Form 0781 Retries exhausted',
{ job_id:, error_class:, error_message:, timestamp:, form526_submission_id: }
Expand Down
4 changes: 4 additions & 0 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,10 @@ features:
actor_type: user
description: Enables enqueuing a Form4142DocumentUploadFailureEmail if a SubmitForm4142Job job exhausts its retries
enable_in_development: true
form526_send_0781_failure_notification:
actor_type: user
description: Enables enqueuing a Form0781DocumentUploadFailureEmail if a SubmitForm0781Job job exhausts its retries
enable_in_development: true
form0994_confirmation_email:
actor_type: user
description: Enables form 0994 email submission confirmation (VaNotify)
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1345,6 +1345,7 @@ vanotify:
template_id:
form526_document_upload_failure_notification_template_id: form526_document_upload_failure_notification_template_id
form4142_upload_failure_notification_template_id: form4142_upload_failure_notification_template_id
form0781_upload_failure_notification_template_id: form0781_upload_failure_notification_template_id
ivc_champva:
api_key: fake_secret
template_id:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe EVSS::DisabilityCompensationForm::Form0781DocumentUploadFailureEmail, type: :job do
subject { described_class }

freeheeling marked this conversation as resolved.
Show resolved Hide resolved
let!(:form526_submission) { create(:form526_submission) }
let(:notification_client) { double('Notifications::Client') }
let(:formatted_submit_date) do
# We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT"
form526_submission.created_at.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.')
end

before do
Sidekiq::Job.clear_all
allow(Notifications::Client).to receive(:new).and_return(notification_client)
end

describe '#perform' do
it 'dispatches a failure notification email' do
expect(notification_client).to receive(:send_email).with(
# Email address and first_name are from our User fixtures
# form0781_upload_failure_notification_template_id is a placeholder in settings.yml
{
email_address: '[email protected]',
template_id: 'form0781_upload_failure_notification_template_id',
personalisation: {
first_name: 'BEYONCE',
date_submitted: formatted_submit_date
}
}
)

subject.perform_async(form526_submission.id)
subject.drain
end
end

describe 'logging' do
it 'increments a Statsd metric' do
allow(notification_client).to receive(:send_email)

expect do
subject.perform_async(form526_submission.id)
subject.drain
end.to trigger_statsd_increment(
'api.form_526.veteran_notifications.form0781_upload_failure_email.success'
)
end

it 'creates a Form526JobStatus' do
allow(notification_client).to receive(:send_email)

expect do
subject.perform_async(form526_submission.id)
subject.drain
end.to change(Form526JobStatus, :count).by(1)
end

context 'when an error throws when sending an email' do
before do
allow_any_instance_of(VaNotify::Service).to receive(:send_email).and_raise(Common::Client::Errors::ClientError)
end

it 'passes the error to the included JobTracker retryable_error_handler and re-raises the error' do
# Sidekiq::Form526JobStatusTracker::JobTracker is included in this job's inheritance hierarchy
expect_any_instance_of(
Sidekiq::Form526JobStatusTracker::JobTracker
).to receive(:retryable_error_handler).with(an_instance_of(Common::Client::Errors::ClientError))

expect do
subject.perform_async(form526_submission.id)
subject.drain
end.to raise_error(Common::Client::Errors::ClientError)
end
end
end

context 'when retries are exhausted' do
let!(:form526_job_status) { create(:form526_job_status, :retryable_error, form526_submission:, job_id: 123) }
let(:retry_params) do
{
'jid' => 123,
'error_class' => 'JennyNotFound',
'error_message' => 'I tried to call you before but I lost my nerve',
'args' => [form526_submission.id]
}
end

let(:exhaustion_time) { DateTime.new(1985, 10, 26).utc }

before do
allow(notification_client).to receive(:send_email)
end

it 'increments a StatsD exhaustion metric, logs to the Rails logger and updates the job status' do
Timecop.freeze(exhaustion_time) do
described_class.within_sidekiq_retries_exhausted_block(retry_params) do
expect(Rails.logger).to receive(:warn).with(
'Form0781DocumentUploadFailureEmail retries exhausted',
{
job_id: 123,
error_class: 'JennyNotFound',
error_message: 'I tried to call you before but I lost my nerve',
timestamp: exhaustion_time,
form526_submission_id: form526_submission.id
}
).and_call_original
expect(StatsD).to receive(:increment).with(
'api.form_526.veteran_notifications.form0781_upload_failure_email.exhausted'
)
end

expect(form526_job_status.reload.status).to eq(Form526JobStatus::STATUS[:exhausted])
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,53 @@

it 'updates a StatsD counter and updates the status on an exhaustion event' do
subject.within_sidekiq_retries_exhausted_block({ 'jid' => form526_job_status.job_id }) do
# Will receieve increment for failure mailer metric
allow(StatsD).to receive(:increment).with(
'shared.sidekiq.default.EVSS_DisabilityCompensationForm_Form0781DocumentUploadFailureEmail.enqueue'
)
freeheeling marked this conversation as resolved.
Show resolved Hide resolved

expect(StatsD).to receive(:increment).with("#{subject::STATSD_KEY_PREFIX}.exhausted")
expect(Rails).to receive(:logger).and_call_original
end
form526_job_status.reload
expect(form526_job_status.status).to eq(Form526JobStatus::STATUS[:exhausted])
end

context 'when the form526_send_0781_failure_notification Flipper is enabled' do
before do
Flipper.enable(:form526_send_0781_failure_notification)
end

it 'enqueues a failure notification mailer to send to the veteran' do
subject.within_sidekiq_retries_exhausted_block(
{
'jid' => form526_job_status.job_id,
'args' => [form526_submission.id]
}
) do
expect(EVSS::DisabilityCompensationForm::Form0781DocumentUploadFailureEmail)
.to receive(:perform_async).with(form526_submission.id)
end
end
end

context 'when the form526_send_0781_failure_notification Flipper is disabled' do
before do
Flipper.disable(:form526_send_0781_failure_notification)
end

it 'does not enqueue a failure notification mailer to send to the veteran' do
subject.within_sidekiq_retries_exhausted_block(
{
'jid' => form526_job_status.job_id,
'args' => [form526_submission.id]
}
) do
expect(EVSS::DisabilityCompensationForm::Form0781DocumentUploadFailureEmail)
.not_to receive(:perform_async)
end
end
end
end
end
end
Loading