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

API-45666 Move VANotify job, save harder, and log more #21349

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
4 changes: 1 addition & 3 deletions modules/claims_api/app/sidekiq/claims_api/poa_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ def perform(power_of_attorney_id, rep_id = nil) # rubocop:disable Metrics/Method

ClaimsApi::Logger.log('poa', poa_id: poa_form.id, detail: 'BIRLS Success')

ClaimsApi::VANotifyAcceptedJob.perform_async(poa_form.id, rep_id) if vanotify?(poa_form.auth_headers, rep_id)

ClaimsApi::PoaVBMSUpdater.perform_async(poa_form.id)
ClaimsApi::PoaVBMSUpdater.perform_async(poa_form.id, rep_id)
else
poa_form.status = ClaimsApi::PowerOfAttorney::ERRORED
poa_form.vbms_error_message = "BGS Error: update_birls_record failed with code #{response[:return_code]}"
Expand Down
19 changes: 14 additions & 5 deletions modules/claims_api/app/sidekiq/claims_api/poa_vbms_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PoaVBMSUpdater < ClaimsApi::ServiceBase
LOG_TAG = 'poa_vbms_updater'
sidekiq_options retry_for: 48.hours

def perform(power_of_attorney_id) # rubocop:disable Metrics/MethodLength
def perform(power_of_attorney_id, rep_id = nil) # rubocop:disable Metrics/MethodLength
poa_form = ClaimsApi::PowerOfAttorney.find(power_of_attorney_id)
process = ClaimsApi::Process.find_or_create_by(processable: poa_form, step_type: 'POA_ACCESS_UPDATE')
process.update!(step_status: 'IN_PROGRESS')
Expand All @@ -32,28 +32,37 @@ def perform(power_of_attorney_id) # rubocop:disable Metrics/MethodLength
poa_form.status = ClaimsApi::PowerOfAttorney::UPDATED
process.update!(step_status: 'SUCCESS', error_messages: [], completed_at: Time.zone.now)
poa_form.vbms_error_message = nil if poa_form.vbms_error_message.present?
ClaimsApi::Logger.log('poa_vbms_updater', poa_id: power_of_attorney_id, detail: 'VBMS Success')
ClaimsApi::Logger.log(LOG_TAG, poa_id: power_of_attorney_id, detail: 'VBMS Success')
else
poa_form.status = ClaimsApi::PowerOfAttorney::ERRORED
poa_form.vbms_error_message = 'update_poa_access failed with code ' \
"#{response[:return_code]}: #{response[:return_message]}"
process.update!(step_status: 'FAILED', error_messages: [{ title: 'BGS Error',
detail: poa_form.vbms_error_message }])
ClaimsApi::Logger.log('poa_vbms_updater',
ClaimsApi::Logger.log(LOG_TAG,
poa_id: power_of_attorney_id,
detail: 'VBMS Failed',
error: response[:return_message])
end

poa_form.save
poa_form.save!
ClaimsApi::Logger.log(LOG_TAG, poa_id: poa_form.id, detail: 'POA Saved successfully')

if vanotify?(poa_form.auth_headers, rep_id)
ClaimsApi::Logger.log(LOG_TAG, poa_id: poa_form.id, detail: 'Sending Email')
ClaimsApi::VANotifyAcceptedJob.perform_async(poa_form.id, rep_id)
end
rescue BGS::ShareError => e
poa_form.status = ClaimsApi::PowerOfAttorney::ERRORED
poa_form.vbms_error_message = e.respond_to?(:message) ? e.message : 'BGS::ShareError'
poa_form.save
process.update!(step_status: 'FAILED',
error_messages: [{ title: 'BGS Error',
detail: poa_form.vbms_error_message }])
ClaimsApi::Logger.log('poa', poa_id: poa_form.id, detail: 'BGS Error', error: e)
ClaimsApi::Logger.log(LOG_TAG, poa_id: poa_form.id, detail: 'BGS Error', error: e)
rescue => e
ClaimsApi::Logger.log(LOG_TAG, poa_id: poa_form.id, detail: 'Re-raising Error', error: get_error_message(e))
raise e
end

def update_poa_access(poa_form:, participant_id:, poa_code:)
Expand Down
59 changes: 0 additions & 59 deletions modules/claims_api/spec/sidekiq/poa_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,65 +102,6 @@
end
end

context 'deciding to send a VA Notify email' do
before do
create_mock_lighthouse_service
allow(Flipper).to receive(:enabled?).with(:lighthouse_claims_api_v2_poa_va_notify).and_return true
end

let(:poa) { create_poa }
let(:header_key) { ClaimsApi::V2::Veterans::PowerOfAttorney::BaseController::VA_NOTIFY_KEY }

context 'when the header key and rep are present' do
it 'sends the vanotify job' do
poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

allow_any_instance_of(ClaimsApi::ServiceBase).to receive(:vanotify?).and_return true
expect(ClaimsApi::VANotifyAcceptedJob).to receive(:perform_async)

subject.new.perform(poa.id, 'Rep Data')
end
end

context 'when the flipper is off' do
it 'does not send the vanotify job' do
allow(Flipper).to receive(:enabled?).with(:lighthouse_claims_api_v2_poa_va_notify).and_return false
Flipper.disable(:lighthouse_claims_api_v2_poa_va_notify)

poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, 'Rep Data')
end
end

context 'does not send the va notify job' do
it 'when the rep is not present' do
poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, nil)
end

it 'when the header key is not present' do
expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, 'Rep data')
end
end
end

context 'when an errored job has exhausted its retries' do
it 'logs to the ClaimsApi Logger' do
poa = create_poa
Expand Down
84 changes: 84 additions & 0 deletions modules/claims_api/spec/sidekiq/poa_vbms_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,90 @@
end
end

context 'deciding to send a VA Notify email' do
let(:allow_poa_c_add) { 'Y' }
let(:consent_address_change) { true }

before do
create_mock_lighthouse_service
allow(Flipper).to receive(:enabled?).with(:lighthouse_claims_api_v2_poa_va_notify).and_return true
end

let(:poa) { create_poa }

Check failure on line 122 in modules/claims_api/spec/sidekiq/poa_vbms_updater_spec.rb

View workflow job for this annotation

GitHub Actions / Linting and Security

RSpec/ScatteredLet: Group all let/let! blocks in the example group together.
let(:header_key) { ClaimsApi::V2::Veterans::PowerOfAttorney::BaseController::VA_NOTIFY_KEY }

Check failure on line 123 in modules/claims_api/spec/sidekiq/poa_vbms_updater_spec.rb

View workflow job for this annotation

GitHub Actions / Linting and Security

RSpec/ScatteredLet: Group all let/let! blocks in the example group together.

context 'when the header key and rep are present' do
it 'sends the vanotify job' do
poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

allow_any_instance_of(ClaimsApi::ServiceBase).to receive(:vanotify?).and_return true
expect(ClaimsApi::VANotifyAcceptedJob).to receive(:perform_async)

subject.new.perform(poa.id, 'Rep Data')
end
end

context 'when the flipper is off' do
it 'does not send the vanotify job' do
allow(Flipper).to receive(:enabled?).with(:lighthouse_claims_api_v2_poa_va_notify).and_return false
Flipper.disable(:lighthouse_claims_api_v2_poa_va_notify)

poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, 'Rep Data')
end
end

context 'does not send the va notify job' do
it 'when the rep is not present' do
poa.auth_headers.merge!({
header_key => 'this_value'
})
poa.save!

expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, nil)
end

it 'when the header key is not present' do
expect(ClaimsApi::VANotifyAcceptedJob).not_to receive(:perform_async)

subject.new.perform(poa.id, 'Rep data')
end
end
end

context 'inability to save the poa form' do
let(:allow_poa_c_add) { 'Y' }
let(:consent_address_change) { true }

it 'logs to the ClaimsApi logger & re-raises' do
err_msg = Faker::Lorem.sentence
Sidekiq::Testing.inline! do
poa = create_poa(allow_poa_access: true)
create_mock_lighthouse_service
allow_any_instance_of(ClaimsApi::PowerOfAttorney).to receive(:save!).and_raise StandardError.new(err_msg)
allow(ClaimsApi::Logger).to receive(:log) # Ignore other logger calls we're not testing for, if they exist
expect(ClaimsApi::Logger).to receive(:log).with(
'poa_vbms_updater',
poa_id: poa.id,
detail: 'Re-raising Error',
error: err_msg
)
expect { subject.new.perform(poa.id) }.to raise_error StandardError
end
end
end

context 'when an errored job has exhausted its retries' do
let(:allow_poa_c_add) { 'Y' }
let(:consent_address_change) { true }
Expand Down
Loading