BgsPowerOfAttorney model

.reek.yml

@@ -64,6 +64,7 @@ detectors:
       - Fakes::EndProductStore
       - Fakes::BGSService#get_participant_id_for_user
       - Fakes::BGSServiceRecordMaker
+      - Fakes::BGSService#fetch_poas_by_participant_ids
       - Fakes::PersistentStore#convert_dates_from
       - FetchDocumentsForReaderJob#fetch_for_appeal
       - HearingSerializerBase

app/controllers/idt/api/v1/base_controller.rb

@@ -10,7 +10,7 @@ class Idt::Api::V1::BaseController < ActionController::Base
 
   # :nocov:
   rescue_from StandardError do |error|
-    Raven.capture_exception(error)
+    log_error(error)
     if error.class.method_defined?(:serialize_response)
       render(error.serialize_response)
     else
@@ -72,4 +72,12 @@ def css_id
   def feature_enabled?(feature)
     FeatureToggle.enabled?(feature, user: user)
   end
+
+  private
+
+  def log_error(error)
+    Raven.capture_exception(error)
+    Rails.logger.error(error)
+    Rails.logger.error(error.backtrace.join("\n"))
+  end
 end

app/controllers/idt/api/v1/veterans_controller.rb

@@ -30,15 +30,25 @@ def poa
   end
 
   def fetch_poa_with_address
-    poa = bgs.fetch_poa_by_file_number(veteran[:file_number])
+    return {} unless bgs_poa.found?
 
-    return {} unless poa
+    poa_address = bgs_poa.representative_address
 
-    poa_address = bgs.find_address_by_participant_id(poa[:participant_id])
+    return serializable_poa unless poa_address
 
-    return poa unless poa_address
+    serializable_poa.merge(poa_address)
+  end
+
+  def bgs_poa
+    @bgs_poa ||= BgsPowerOfAttorney.new(file_number: veteran[:file_number])
+  end
 
-    poa.merge(poa_address)
+  def serializable_poa
+    {
+      representative_name: bgs_poa.representative_name,
+      representative_type: bgs_poa.representative_type,
+      participant_id: bgs_poa.poa_participant_id
+    }
   end
 
   def json_veteran_details

app/jobs/warm_bgs_caches_job.rb

@@ -18,6 +18,10 @@ def perform
   private
 
   def warm_poa_caches
+    # We do 2 passes to update our POA cache (bgs_power_of_attorney table).
+    # 1. Look at the 1000 oldest cached records and update them.
+    # 2. Look at Claimants w/o POA associated record and create cached record.
+
     # we average about 10k Claimant rows created a week.
     # we very rarely update the Claimant record after we create it.
     # so we can use the Claimant.updated_at to reflect how recently we've
@@ -28,13 +32,32 @@ def warm_poa_caches
     # we only care about Appeal Claimants because that's all
     # UpdateCachedAppealsAttributesJob cares about.
     # assuming we have 40k open appeals/claimants at any given time,
-    # and we cache each one for 30 days, we want to cache about 1400 a day.
+    # we want to check about 1400 a day to cycle through them all once a month.
+
+    warm_poa_for_oldest_claimants
+    warm_poa_for_oldest_cached_records
+  end
+
+  def warm_poa_for_oldest_cached_records
+    start_time = Time.zone.now
+    oldest_bgs_poa_records.limit(1000).each do |bgs_poa|
+      bgs_poa.update_cached_attributes! if bgs_poa.stale_attributes?
+    end
+    datadog_report_time_segment(segment: "warm_poa_bgs", start_time: start_time)
+  end
+
+  def warm_poa_for_oldest_claimants
     start_time = Time.zone.now
-    oldest_claimants_for_open_appeals.limit(1400).each do |claimant|
-      claimant.representative_name # updates_cache
+    oldest_claimants_with_poa.each do |claimant|
+      bgs_poa = claimant.power_of_attorney
+      bgs_poa.save_with_updated_bgs_record! if bgs_poa.stale_attributes?
       claimant.update!(updated_at: Time.zone.now)
     end
-    datadog_report_time_segment(segment: "warm_poa_caches", start_time: start_time)
+    datadog_report_time_segment(segment: "warm_poa_claimants", start_time: start_time)
+  end
+
+  def oldest_claimants_with_poa
+    oldest_claimants_for_open_appeals.limit(1400).select { |claimant| claimant.power_of_attorney.present? }
   end
 
   def claimants_for_open_appeals
@@ -45,6 +68,10 @@ def oldest_claimants_for_open_appeals
     claimants_for_open_appeals.order(updated_at: :asc)
   end
 
+  def oldest_bgs_poa_records
+    BgsPowerOfAttorney.order(last_synced_at: :asc)
+  end
+
   def open_appeals_from_tasks
     Task.open.where(appeal_type: Appeal.name).pluck(:appeal_id).uniq
   end

app/mappers/power_of_attorney_mapper.rb

@@ -11,21 +11,28 @@ def self.included(base)
     base.extend(PowerOfAttorneyMapper)
   end
 
-  def get_poa_from_bgs_poa(bgs_rep = {})
-    return {} unless bgs_rep
+  def get_poa_from_bgs_poa(bgs_record = {})
+    return {} unless bgs_record.dig(:power_of_attorney)
 
+    bgs_rep = bgs_record[:power_of_attorney]
     bgs_type = bgs_rep[:org_type_nm]
     {
       representative_type: BGS_REP_TYPE_TO_REP_TYPE[bgs_type] || "Other",
       representative_name: bgs_rep[:nm],
       # Used to find the POA address
-      participant_id: bgs_rep[:ptcpnt_id]
+      participant_id: bgs_rep[:ptcpnt_id],
+      # pass through other attrs
+      authzn_change_clmant_addrs_ind: bgs_rep[:authzn_change_clmant_addrs_ind],
+      authzn_poa_access_ind: bgs_rep[:authzn_poa_access_ind],
+      legacy_poa_cd: bgs_rep[:legacy_poa_cd],
+      file_number: bgs_record[:file_number],
+      claimant_participant_id: bgs_record[:ptcpnt_id]
     }
   end
 
   def get_hash_of_poa_from_bgs_poas(bgs_resp)
     [bgs_resp].flatten.each_with_object({}) do |poa, hsh|
-      hsh[poa[:ptcpnt_id]] = get_poa_from_bgs_poa(poa[:power_of_attorney])
+      hsh[poa[:ptcpnt_id]] = get_poa_from_bgs_poa(poa)
     end
   end
 

app/models/bgs_power_of_attorney.rb

@@ -0,0 +1,162 @@
+# frozen_string_literal: true
+
+class BgsPowerOfAttorney < CaseflowRecord
+  include AssociatedBgsRecord
+  include BgsService
+
+  has_many :claimants, primary_key: :claimant_participant_id, foreign_key: :participant_id
+
+  CACHED_BGS_ATTRIBUTES = [
+    :representative_name,
+    :representative_type,
+    :authzn_change_clmant_addrs_ind,
+    :authzn_poa_access_ind,
+    :legacy_poa_cd,
+    :poa_participant_id,
+    :claimant_participant_id,
+    :file_number
+  ].freeze
+
+  delegate :email_address,
+           to: :person, prefix: :representative
+
+  validates :claimant_participant_id, :poa_participant_id, :representative_name, :representative_type, presence: true
+
+  before_save :update_cached_attributes!
+
+  class << self
+    # Neither file_number nor claimant_participant_id is unique by itself,
+    # but we treat them that way for easy lookup. They are unique together in our db.
+    # Since this is a cache we only want to mirror what BGS has and leave the
+    # data integrity to them.
+    def find_or_create_by_file_number(file_number)
+      find_or_create_by!(file_number: file_number)
+    end
+
+    def find_or_create_by_claimant_participant_id(claimant_participant_id)
+      find_or_create_by!(claimant_participant_id: claimant_participant_id)
+    end
+
+    def find_or_load_by_file_number(file_number)
+      find_by(file_number: file_number) || new(file_number: file_number)
+    end
+
+    # In theory, both these BGS calls should return the same thing.
+    # We try both services if necessary mostly for backwards compatability in tests.
+    def fetch_bgs_poa_by_participant_id(pid)
+      [bgs.fetch_poas_by_participant_id(pid)].flatten[0] || bgs.fetch_poas_by_participant_ids([pid])[pid]
+    end
+  end
+
+  def representative_name
+    cached_or_fetched_from_bgs(attr_name: :representative_name)
+  end
+
+  def representative_type
+    cached_or_fetched_from_bgs(attr_name: :representative_type)
+  end
+
+  def authzn_change_clmant_addrs_ind
+    cached_or_fetched_from_bgs(attr_name: :authzn_change_clmant_addrs_ind)
+  end
+
+  def authzn_poa_access_ind
+    cached_or_fetched_from_bgs(attr_name: :authzn_poa_access_ind)
+  end
+
+  def legacy_poa_cd
+    cached_or_fetched_from_bgs(attr_name: :legacy_poa_cd)
+  end
+
+  def representative_address
+    @representative_address ||= load_bgs_address!
+  end
+
+  def poa_participant_id
+    cached_or_fetched_from_bgs(attr_name: :poa_participant_id, bgs_attr: :participant_id)
+  end
+
+  alias participant_id poa_participant_id
+
+  def claimant_participant_id
+    cached_or_fetched_from_bgs(attr_name: :claimant_participant_id)
+  end
+
+  def file_number
+    cached_or_fetched_from_bgs(attr_name: :file_number)
+  end
+
+  def stale_attributes?
+    return false if not_found?
+
+    stale_attributes.any?
+  end
+
+  def update_cached_attributes!
+    stale_attributes.each { |attr| send(attr) }
+    self.last_synced_at = Time.zone.now
+  end
+
+  def save_with_updated_bgs_record!
+    stale_attributes.each do |attr|
+      self[attr] = nil # local object attr empty, should trigger re-fetch of bgs record
+      send(attr)
+    end
+    save!
+  end
+
+  def found?
+    return false if not_found?
+
+    bgs_record.keys.any?
+  end
+
+  private
+
+  def not_found?
+    bgs_record == :not_found
+  end
+
+  def person
+    @person ||= Person.find_or_create_by(participant_id: poa_participant_id)
+  end
+
+  def stale_attributes
+    CACHED_BGS_ATTRIBUTES.select { |attr| self[attr].nil? || self[attr].to_s != bgs_record[attr].to_s }
+  end
+
+  def cached_or_fetched_from_bgs(attr_name:, bgs_attr: nil)
+    bgs_attr ||= attr_name
+    self[attr_name] ||= begin
+      return if bgs_record == :not_found
+
+      bgs_record.dig(bgs_attr)
+    end
+  end
+
+  def fetch_bgs_record
+    if self[:claimant_participant_id]
+      fetch_bgs_record_by_claimant_participant_id
+    elsif self[:file_number]
+      bgs.fetch_poa_by_file_number(self[:file_number])
+    else
+      fail "Must define claimant_participant_id or file_number"
+    end
+  end
+
+  def fetch_bgs_record_by_claimant_participant_id
+    pid = self[:claimant_participant_id]
+    poa = self.class.fetch_bgs_poa_by_participant_id(pid)
+
+    return unless poa
+
+    poa[:claimant_participant_id] ||= pid
+    poa
+  end
+
+  def load_bgs_address!
+    return nil if !participant_id
+
+    BgsAddressService.new(participant_id: poa_participant_id).address
+  end
+end

app/models/claimant.rb

@@ -22,7 +22,7 @@ def self.create_without_intake!(participant_id:, payee_code:)
   end
 
   def power_of_attorney
-    @power_of_attorney ||= BgsPowerOfAttorney.new(claimant_participant_id: participant_id)
+    @power_of_attorney ||= find_power_of_attorney
   end
 
   delegate :representative_name,
@@ -76,6 +76,22 @@ def bgs_record
 
   private
 
+  def find_power_of_attorney
+    find_power_of_attorney_by_pid || find_power_of_attorney_by_file_number
+  end
+
+  def find_power_of_attorney_by_pid
+    BgsPowerOfAttorney.find_or_create_by_claimant_participant_id(participant_id)
+  rescue ActiveRecord::RecordInvalid # not found at BGS by PID
+    nil
+  end
+
+  def find_power_of_attorney_by_file_number
+    BgsPowerOfAttorney.find_or_create_by_file_number(decision_review.veteran_file_number)
+  rescue ActiveRecord::RecordInvalid # not found at BGS
+    nil
+  end
+
   def bgs_address_service
     @bgs_address_service ||= BgsAddressService.new(participant_id: participant_id)
   end

app/models/power_of_attorney.rb

@@ -58,7 +58,7 @@ def update_vacols_rep_info!(appeal:, representative_type:, representative_name:,
   private
 
   def bgs_power_of_attorney
-    @bgs_power_of_attorney ||= BgsPowerOfAttorney.new(file_number: file_number)
+    @bgs_power_of_attorney ||= BgsPowerOfAttorney.find_or_load_by_file_number(file_number)
   end
 
   class << self