[Automated] Merged master into target k8s

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

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

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:)

spec/lib/lighthouse/direct_deposit/control_information_spec.rb

@@ -10,23 +10,50 @@
     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)
 
         expect(info.account_updatable?).to be(false)
         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