From 1e50563b8ba4a4d4653ca7a7382a1941f7498dc7 Mon Sep 17 00:00:00 2001 From: Tom Harrison Date: Wed, 24 Apr 2024 15:57:29 -0400 Subject: [PATCH] Add temporary logging for direct deposit control information (#16453) --- .../v0/profile/direct_deposits_controller.rb | 30 +++++-- .../direct_deposit/control_information.rb | 80 +++++++++++++------ .../direct_deposits_controller_spec.rb | 44 +++++++++- .../control_information_spec.rb | 61 ++++++++++---- 4 files changed, 168 insertions(+), 47 deletions(-) diff --git a/app/controllers/v0/profile/direct_deposits_controller.rb b/app/controllers/v0/profile/direct_deposits_controller.rb index 942a666f594..ff08ee72400 100644 --- a/app/controllers/v0/profile/direct_deposits_controller.rb +++ b/app/controllers/v0/profile/direct_deposits_controller.rb @@ -11,8 +11,7 @@ module Profile class DirectDepositsController < ApplicationController service_tag 'direct-deposit' before_action { authorize :lighthouse, :direct_deposit_access? } - before_action :payment_account, only: :update - before_action :control_information, only: :update + after_action :log_sso_info, only: :update rescue_from(*Lighthouse::ServiceException::ERROR_MAP.values) do |exception| @@ -28,12 +27,19 @@ class DirectDepositsController < ApplicationController def show response = client.get_payment_info + set_control_information(response.body[:control_information]) + log_control_info(@control_information, 'Show') + render status: response.status, json: response.body, serializer: DisabilityCompensationsSerializer end def update + set_control_information(control_info_params) + set_payment_account(payment_account_params) + + log_control_info(@control_information, 'Update') response = client.update_payment_info(@payment_account) send_confirmation_email @@ -44,6 +50,15 @@ def update private + def log_control_info(control_info, action) + ::Rails.logger.info("Direct Deposit Control Info: #{action}", + { benefit_type: control_info.benefit_type, + updatable: control_info.account_updatable?, + valid: control_info.valid?, + restrictions: control_info.restrictions.join(', '), + errors: control_info.errors.join(', ') }) + end + def single_form_enabled? Flipper.enabled?(:profile_show_direct_deposit_single_form, @current_user) end @@ -56,12 +71,13 @@ def client @client ||= DirectDeposit::Client.new(@current_user.icn) end - def payment_account - @payment_account ||= Lighthouse::DirectDeposit::PaymentAccount.new(payment_account_params) + def set_payment_account(params) + @payment_account ||= Lighthouse::DirectDeposit::PaymentAccount.new(params) end - def control_information - @control_information ||= Lighthouse::DirectDeposit::ControlInformation.new(control_info_params) + def set_control_information(params) + @control_information ||= Lighthouse::DirectDeposit::ControlInformation.new + @control_information.assign_attributes(params) end def payment_account_params @@ -84,7 +100,7 @@ def control_info_params :has_no_fiduciary_assigned, :is_not_deceased, :has_payment_address, - :has_indentity) + :has_identity) end def send_confirmation_email diff --git a/lib/lighthouse/direct_deposit/control_information.rb b/lib/lighthouse/direct_deposit/control_information.rb index 4ed67551aeb..a41dc0c42c9 100644 --- a/lib/lighthouse/direct_deposit/control_information.rb +++ b/lib/lighthouse/direct_deposit/control_information.rb @@ -3,58 +3,88 @@ module Lighthouse module DirectDeposit class ControlInformation - include ActiveModel::Model - - ACTIONS = [:can_update_direct_deposit].freeze - USAGES = %i[is_corp_available is_edu_claim_available].freeze - RESTRICTIONS = %i[ - is_corp_rec_found - has_no_bdn_payments - has_identity - has_index - is_competent - has_mailing_address - has_no_fiduciary_assigned - is_not_deceased - has_payment_address - ].freeze - - attr_accessor(*(ACTIONS + USAGES + RESTRICTIONS)) + include ActiveModel::Attributes + include ActiveModel::AttributeAssignment + + # Actions + attribute :can_update_direct_deposit, :boolean + + # Usage + attribute :is_corp_available, :boolean + attribute :is_edu_claim_available, :boolean + + # Restrictions + attribute :is_corp_rec_found, :boolean + attribute :has_no_bdn_payments, :boolean + attribute :has_identity, :boolean + attribute :has_index, :boolean + attribute :is_competent, :boolean + attribute :has_mailing_address, :boolean + attribute :has_no_fiduciary_assigned, :boolean + attribute :is_not_deceased, :boolean + attribute :has_payment_address, :boolean + attr_reader :errors alias :comp_and_pen? is_corp_available alias :edu_benefits? is_edu_claim_available def account_updatable? - @can_update_direct_deposit && restrictions.size.zero? + can_update_direct_deposit.present? && restrictions.size.zero? end def benefit_type? comp_and_pen? || edu_benefits? end + def benefit_type + return 'both' if comp_and_pen? && edu_benefits? + return 'cnp' if comp_and_pen? + return 'edu' if edu_benefits? + + 'none' + end + def restrictions - RESTRICTIONS.reject { |name| send(name) } + restrictions = [] + restrictions << 'is_corp_rec_found' unless is_corp_rec_found + restrictions << 'has_no_bdn_payments' unless has_no_bdn_payments + restrictions << 'has_identity' unless has_identity + restrictions << 'has_index' unless has_index + restrictions << 'is_competent' unless is_competent + restrictions << 'has_mailing_address' unless has_mailing_address + restrictions << 'has_no_fiduciary_assigned' unless has_no_fiduciary_assigned + restrictions << 'is_not_deceased' unless is_not_deceased + restrictions << 'has_payment_address' unless has_payment_address + restrictions end def clear_restrictions - @can_update_direct_deposit = true - RESTRICTIONS.each { |name| send("#{name}=", true) } + assign_attributes can_update_direct_deposit: true + assign_attributes is_corp_rec_found: true + assign_attributes has_no_bdn_payments: true + assign_attributes has_identity: true + assign_attributes has_index: true + assign_attributes is_competent: true + assign_attributes has_mailing_address: true + assign_attributes has_no_fiduciary_assigned: true + assign_attributes is_not_deceased: true + assign_attributes has_payment_address: true end def valid? @errors = [] error = 'Has restrictions. Account should not be updatable.' - errors << error if @can_update_direct_deposit && restrictions.any? + @errors << error if can_update_direct_deposit && restrictions.any? error = 'Has no restrictions. Account should be updatable.' - errors << error if !@can_update_direct_deposit && restrictions.empty? + @errors << error if !can_update_direct_deposit && restrictions.empty? error = 'Missing benefit type. Must be either CnP or EDU benefits.' - errors << error unless benefit_type? + @errors << error unless benefit_type? - errors.size.zero? + @errors.size.zero? end end end diff --git a/spec/controllers/v0/profile/direct_deposits_controller_spec.rb b/spec/controllers/v0/profile/direct_deposits_controller_spec.rb index 3dd8abc3e91..709a25e4e35 100644 --- a/spec/controllers/v0/profile/direct_deposits_controller_spec.rb +++ b/spec/controllers/v0/profile/direct_deposits_controller_spec.rb @@ -9,11 +9,28 @@ sign_in_as(user) token = 'abcdefghijklmnop' allow_any_instance_of(DirectDeposit::Configuration).to receive(:access_token).and_return(token) + allow(Rails.logger).to receive(:info) Flipper.disable(:profile_show_direct_deposit_single_form) end describe '#show' do context 'when successful' do + it 'logs the control info' do + VCR.use_cassette('lighthouse/direct_deposit/show/200_valid') do + get(:show) + end + + expect(response).to have_http_status(:ok) + expect(Rails.logger) + .to have_received(:info) + .with('Direct Deposit Control Info: Show', + { benefit_type: 'both', + updatable: true, + valid: true, + restrictions: '', + errors: '' }) + end + it 'returns a status of 200' do VCR.use_cassette('lighthouse/direct_deposit/show/200_valid') do get(:show) @@ -154,11 +171,36 @@ control_information: { can_update_direct_deposit: true, is_corp_available: true, - is_edu_claim_available: true + is_edu_claim_available: true, + is_corp_rec_found: true, + has_no_bdn_payments: true, + has_index: true, + is_competent: true, + has_mailing_address: true, + has_no_fiduciary_assigned: true, + is_not_deceased: true, + has_payment_address: true, + has_identity: true } } end + it 'logs the control info' do + VCR.use_cassette('lighthouse/direct_deposit/update/200_valid') do + put(:update, params:) + end + + expect(response).to have_http_status(:ok) + expect(Rails.logger) + .to have_received(:info) + .with('Direct Deposit Control Info: Update', + { benefit_type: 'both', + updatable: true, + valid: true, + restrictions: '', + errors: '' }) + end + it 'returns a status of 200' do VCR.use_cassette('lighthouse/direct_deposit/update/200_valid') do put(:update, params:) diff --git a/spec/lib/lighthouse/direct_deposit/control_information_spec.rb b/spec/lib/lighthouse/direct_deposit/control_information_spec.rb index ab3cdb5db61..252bf2c7296 100644 --- a/spec/lib/lighthouse/direct_deposit/control_information_spec.rb +++ b/spec/lib/lighthouse/direct_deposit/control_information_spec.rb @@ -10,16 +10,32 @@ info.clear_restrictions end + context 'when clear_restrictions is called' do + it 'is updatable' do + expect(info.account_updatable?).to be(true) + end + end + context 'when no restrictions' do it 'is updateable' do expect(info.account_updatable?).to be(true) expect(info.restrictions).to be_empty end + + context 'and can_update_direct_deposit is false' do + it 'is invalid' do + info.is_edu_claim_available = true + info.can_update_direct_deposit = false + + expect(info.valid?).to be(false) + expect(info.errors).to eq(['Has no restrictions. Account should be updatable.']) + end + end end context 'when there is a restriction' do it 'is not updateable' do - Lighthouse::DirectDeposit::ControlInformation::RESTRICTIONS.each do |name| + info.restrictions.each do |name| info.clear_restrictions info.send("#{name}=", false) @@ -27,6 +43,17 @@ expect(info.restrictions).not_to be_empty end end + + context 'and can_update_direct_deposit is true' do + it 'is invalid' do + info.is_corp_available = true + info.has_index = false + info.can_update_direct_deposit = true + + expect(info.valid?).to be(false) + expect(info.errors).to eq(['Has restrictions. Account should not be updatable.']) + end + end end context 'when is_corp_available is true' do @@ -43,30 +70,36 @@ end end - context 'has no benefit type' do + context 'when no benefit type' do it 'is invalid' do expect(info.valid?).to be(false) expect(info.errors).to eq(['Missing benefit type. Must be either CnP or EDU benefits.']) end end - context 'has restrictions' do - it 'account should not be updatable' do - info.is_edu_claim_available = true - info.has_identity = false + context 'benefit type' do + it 'returns a benefit type of cnp' do + info.is_corp_available = true + info.is_edu_claim_available = false + expect(info.benefit_type).to eq('cnp') + end - expect(info.valid?).to be(false) - expect(info.errors).to eq(['Has restrictions. Account should not be updatable.']) + it 'returns a benefit type of edu' do + info.is_corp_available = false + info.is_edu_claim_available = true + expect(info.benefit_type).to eq('edu') end - end - context 'has no restrictions' do - it 'account should be updatable' do + it 'returns a benefit type of both' do info.is_corp_available = true - info.can_update_direct_deposit = false + info.is_edu_claim_available = true + expect(info.benefit_type).to eq('both') + end - expect(info.valid?).to be(false) - expect(info.errors).to eq(['Has no restrictions. Account should be updatable.']) + it 'returns a benefit type of none' do + info.is_corp_available = false + info.is_edu_claim_available = false + expect(info.benefit_type).to eq('none') end end end