From e8033139cfead37b68a73710ebd28a1191b5abaa Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:54:32 -0400 Subject: [PATCH 1/4] Bump redis from 5.1.0 to 5.2.0 (#16354) Bumps [redis](https://github.com/redis/redis-rb) from 5.1.0 to 5.2.0. - [Changelog](https://github.com/redis/redis-rb/blob/master/CHANGELOG.md) - [Commits](https://github.com/redis/redis-rb/compare/v5.1.0...v5.2.0) --- updated-dependencies: - dependency-name: redis dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rebecca Tolmach <10993987+rmtolmach@users.noreply.github.com> --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index be296954a8a..af57e7cd71b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -813,9 +813,9 @@ GEM rchardet (1.8.0) rdoc (6.6.3.1) psych (>= 4.0.0) - redis (5.1.0) - redis-client (>= 0.17.0) - redis-client (0.20.0) + redis (5.2.0) + redis-client (>= 0.22.0) + redis-client (0.22.0) connection_pool redis-namespace (1.11.0) redis (>= 4) From c04424a30dd50b0e0a9061ec28ae41be5123cb96 Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Wed, 17 Apr 2024 07:05:13 -0700 Subject: [PATCH 2/4] 80792 remove redis client refactor feature flag (#16353) --- config/features.yml | 4 -- .../app/services/travel_claim/redis_client.rb | 40 +++---------------- .../travel_claim/redis_client_spec.rb | 21 +--------- 3 files changed, 6 insertions(+), 59 deletions(-) diff --git a/config/features.yml b/config/features.yml index 7011dffe9f1..a97c1e93cb7 100644 --- a/config/features.yml +++ b/config/features.yml @@ -152,10 +152,6 @@ features: actor_type: user description: Enables the unified experience version of the landing page. enable_in_development: true - check_in_experience_travel_claim_redis_client_refactor: - actor_type: user - description: Uses the refactored code for Travel Claim Redis client to fetch attributes - enable_in_development: true claim_letters_access: actor_type: user description: Enables users to access the claim letters page diff --git a/modules/check_in/app/services/travel_claim/redis_client.rb b/modules/check_in/app/services/travel_claim/redis_client.rb index 0625182f995..4df98536115 100644 --- a/modules/check_in/app/services/travel_claim/redis_client.rb +++ b/modules/check_in/app/services/travel_claim/redis_client.rb @@ -33,53 +33,23 @@ def save_token(token:) end def icn(uuid:) - if Flipper.enabled?('check_in_experience_travel_claim_redis_client_refactor') - return fetch_attribute(uuid:, attribute: :icn) - end - - return nil if appointment_identifiers(uuid:).nil? - - Oj.load(appointment_identifiers(uuid:)).with_indifferent_access.dig(:data, :attributes, :icn) + fetch_attribute(uuid:, attribute: :icn) end def mobile_phone(uuid:) - if Flipper.enabled?('check_in_experience_travel_claim_redis_client_refactor') - return fetch_attribute(uuid:, attribute: :mobilePhone) - end - - return nil if appointment_identifiers(uuid:).nil? - - Oj.load(appointment_identifiers(uuid:)).with_indifferent_access.dig(:data, :attributes, :mobilePhone) + fetch_attribute(uuid:, attribute: :mobilePhone) end def patient_cell_phone(uuid:) - if Flipper.enabled?('check_in_experience_travel_claim_redis_client_refactor') - return fetch_attribute(uuid:, attribute: :patientCellPhone) - end - - return nil if appointment_identifiers(uuid:).nil? - - Oj.load(appointment_identifiers(uuid:)).with_indifferent_access.dig(:data, :attributes, :patientCellPhone) + fetch_attribute(uuid:, attribute: :patientCellPhone) end def station_number(uuid:) - if Flipper.enabled?('check_in_experience_travel_claim_redis_client_refactor') - return fetch_attribute(uuid:, attribute: :stationNo) - end - - return nil if appointment_identifiers(uuid:).nil? - - Oj.load(appointment_identifiers(uuid:)).with_indifferent_access.dig(:data, :attributes, :stationNo) + fetch_attribute(uuid:, attribute: :stationNo) end def facility_type(uuid:) - if Flipper.enabled?('check_in_experience_travel_claim_redis_client_refactor') - return fetch_attribute(uuid:, attribute: :facilityType) - end - - return nil if appointment_identifiers(uuid:).nil? - - Oj.load(appointment_identifiers(uuid:)).with_indifferent_access.dig(:data, :attributes, :facilityType) + fetch_attribute(uuid:, attribute: :facilityType) end def fetch_attribute(uuid:, attribute:) diff --git a/modules/check_in/spec/services/travel_claim/redis_client_spec.rb b/modules/check_in/spec/services/travel_claim/redis_client_spec.rb index 53a1b5a4d75..9763f2ed55c 100644 --- a/modules/check_in/spec/services/travel_claim/redis_client_spec.rb +++ b/modules/check_in/spec/services/travel_claim/redis_client_spec.rb @@ -30,9 +30,6 @@ before do allow(Rails).to receive(:cache).and_return(memory_store) - allow(Flipper).to receive(:enabled?).with('check_in_experience_travel_claim_redis_client_refactor') - .and_return(false) - Rails.cache.clear end @@ -201,23 +198,7 @@ ) end - context 'when cache exists and refactor feature flag is off' do - before do - allow(Flipper).to receive(:enabled?).with('check_in_experience_travel_claim_redis_client_refactor') - .and_return(false) - end - - it 'returns the cached value' do - expect(redis_client.facility_type(uuid:)).to eq(facility_type) - end - end - - context 'when cache exists and refactor feature flag is on' do - before do - allow(Flipper).to receive(:enabled?).with('check_in_experience_travel_claim_redis_client_refactor') - .and_return(true) - end - + context 'when cache exists' do it 'returns the cached value' do expect(redis_client.facility_type(uuid:)).to eq(facility_type) end From 88ec761393103d8a471894492e198db1ac327ae9 Mon Sep 17 00:00:00 2001 From: Eric Tillberg Date: Wed, 17 Apr 2024 10:43:33 -0400 Subject: [PATCH 3/4] Fix 20-10207 mapping and date stamp bug (#16376) --- .../simple_forms_api/app/form_mappings/vba_20_10207.json.erb | 5 ++--- .../app/services/simple_forms_api/pdf_stamper.rb | 3 +-- .../spec/fixtures/form_json/vba_20_10207-veteran.json | 3 ++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/simple_forms_api/app/form_mappings/vba_20_10207.json.erb b/modules/simple_forms_api/app/form_mappings/vba_20_10207.json.erb index dcbb0119b2e..776c194bfc3 100644 --- a/modules/simple_forms_api/app/form_mappings/vba_20_10207.json.erb +++ b/modules/simple_forms_api/app/form_mappings/vba_20_10207.json.erb @@ -49,6 +49,7 @@ "form1[0].#subform[3].CheckBox1[1]": "<%= nil %>", "form1[0].#subform[3].Email_Address[3]": "<%= nil %>", "form1[0].#subform[3].VA_File_Number_If_Applicable[1]": "<%= form.data.dig('veteran_id', 'va_file_number') %>", + "form1[0].#subform[3].CurrentlyHomeless[0]": "<%= form.data.dig('living_situation', 'none') ? 1 : 0 %>", "form1[0].#subform[4].I_Live_Or_Sleep_In_A_Place_That_Is_Not_Meant_For_Regular_Sleeping[0]": "<%= form.data.dig('living_situation', 'overnight') ? 1 : 0 %>", "form1[0].#subform[4].I_Live_In_A_Shelter[0]": "<%= form.data.dig('living_situation', 'shelter') ? 1 : 0 %>", @@ -57,10 +58,8 @@ "form1[0].#subform[4].IN_THE_NEXT_30_DAYS_I_WILL_LOSE_MY_HOME[0]": "<%= form.data.dig('living_situation', 'losing_home') ? 1 : 0 %>", "form1[0].#subform[4].NONE_OF_THESE_SITUATIONS_APPLY_TO_ME[0]": "<%= form.data.dig('living_situation', 'none') ? 1 : 0 %>", "form1[0].#subform[4].OTHER_Specify[0]": "<%= form.data.dig('living_situation', 'other_risk') ? 1 : 0 %>", - - "form1[0].#subform[3].CurrentlyHomeless[0]": "<%= form.data.dig('living_situation', 'none') ? 1 : 0 %>", + "form1[0].#subform[4].Other1[0]": "<%= form.data['other_housing_risks'] %>", - "form1[0].#subform[3].Other1[0]": "<%= form.data['other_housing_risks'] %>", "form1[0].#subform[4].Veterans_SocialSecurityNumber_LastFourNumbers[1]": "<%= form.data.dig('veteran_id', 'ssn')&.[](5..8) %>", "form1[0].#subform[4].Veterans_SocialSecurityNumber_SecondTwoNumbers[1]": "<%= form.data.dig('veteran_id', 'ssn')&.[](3..4) %>", "form1[0].#subform[4].Veterans_SocialSecurityNumber_FirstThreeNumbers[1]": "<%= form.data.dig('veteran_id', 'ssn')&.[](0..2) %>", diff --git a/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb b/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb index c2e708d80cc..02988c59632 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/pdf_stamper.rb @@ -37,8 +37,7 @@ def self.stamp_auth_text(stamped_template_path, current_loa) end coords = [10, 10] text = SUBMISSION_TEXT + current_time - page = 0 - desired_stamp = { coords:, text:, page: } + desired_stamp = { coords:, text: } verify(stamped_template_path) do stamp(desired_stamp, stamped_template_path, append_to_stamp: auth_text, text_only: false) end diff --git a/modules/simple_forms_api/spec/fixtures/form_json/vba_20_10207-veteran.json b/modules/simple_forms_api/spec/fixtures/form_json/vba_20_10207-veteran.json index 05ec5f3be90..dde9552118f 100644 --- a/modules/simple_forms_api/spec/fixtures/form_json/vba_20_10207-veteran.json +++ b/modules/simple_forms_api/spec/fixtures/form_json/vba_20_10207-veteran.json @@ -11,7 +11,8 @@ }, "living_situation": { "overnight": true, - "losing_home": true + "losing_home": true, + "other_risk": true }, "other_housing_risks": "Other housing risks", "mailing_address_yes_no": true, From 3bc601f3e755b8a701a5571d1b3deda8173b0ca4 Mon Sep 17 00:00:00 2001 From: stevenjcumming <134282106+stevenjcumming@users.noreply.github.com> Date: Wed, 17 Apr 2024 11:07:31 -0400 Subject: [PATCH 4/4] Remove DOB and SSN from veteran_representatives (#16369) * remove dob and ssn from veteran_representatives * rubocop formatting * rubocop formatting * add data back to personal_information_log_schema * move migration to another PR * reset schema to master * reset schema to master * add ignored columns to vet rep * remove ignore_columns --- .../models/veteran/service/representative.rb | 58 ++++--------------- .../veteran/service/representative_spec.rb | 55 ++++++------------ 2 files changed, 29 insertions(+), 84 deletions(-) diff --git a/modules/veteran/app/models/veteran/service/representative.rb b/modules/veteran/app/models/veteran/service/representative.rb index 4cfc2da6ad8..ff777fef6d4 100644 --- a/modules/veteran/app/models/veteran/service/representative.rb +++ b/modules/veteran/app/models/veteran/service/representative.rb @@ -9,8 +9,6 @@ class Representative < ApplicationRecord BASE_URL = 'https://www.va.gov/ogc/apps/accreditation/' self.primary_key = :representative_id - has_kms_key - has_encrypted :dob, :ssn, key: :kms_key, **lockbox_options scope :attorneys, -> { where(user_types: ['attorney']) } scope :veteran_service_officers, -> { where(user_types: ['veteran_service_officer']) } @@ -24,62 +22,28 @@ class Representative < ApplicationRecord # Find all representatives that matches the provided search criteria # @param first_name: [String] First name to search for, ignoring case # @param last_name: [String] Last name to search for, ignoring case - # @param ssn: nil [String] SSN to search for - # @param dob: nil [String] Date of birth to search for # @param middle_initial: nil [String] Middle initial to search for # @param poa_code: nil [String] filter to reps working this POA code # # @return [Array(Veteran::Service::Representative)] All representatives found using the submitted search criteria - def self.all_for_user(first_name:, last_name:, ssn: nil, dob: nil, middle_initial: nil, poa_code: nil) # rubocop:disable Metrics/ParameterLists - reps = where('lower(first_name) = ? AND lower(last_name) = ?', first_name.downcase, last_name.downcase) - reps = reps.where('? = ANY(poa_codes)', poa_code) if poa_code - - reps.select do |rep| - matching_ssn(rep, ssn) && - matching_date_of_birth(rep, dob) && - matching_middle_initial(rep, middle_initial) - end + def self.all_for_user(first_name:, last_name:, middle_initial: nil, poa_code: nil) + representatives = where('lower(first_name) = ? AND lower(last_name) = ?', first_name.downcase, + last_name.downcase) + representatives = representatives.where('? = ANY(poa_codes)', poa_code) if poa_code + representatives.select { |rep| matching_middle_initial(rep, middle_initial) } end # # Find first representative that matches the provided search criteria # @param first_name: [String] First name to search for, ignoring case # @param last_name: [String] Last name to search for, ignoring case - # @param ssn: nil [String] SSN to search for - # @param dob: nil [String] Date of birth to search for # # @return [Veteran::Service::Representative] First representative record found using the submitted search criteria - def self.for_user(first_name:, last_name:, ssn: nil, dob: nil) - reps = all_for_user(first_name:, last_name:, ssn:, dob:) - return nil if reps.blank? - - reps.first - end - - # - # Determine if representative ssn matches submitted ssn search query - # @note Assumes that the consumer did not submit an ssn value if the value is blank - # @param rep [Veteran::Service::Representative] Representative to match soon with - # @param ssn [String] Submitted ssn to match against representative - # - # @return [Boolean] True if matches, false if not - def self.matching_ssn(rep, ssn) - return true if ssn.blank? - - rep.ssn.present? && rep.ssn == ssn - end - - # - # Determine if representative dob matches submitted birth_date search query - # @note Assumes that the consumer did not submit a birth_date value if the value is blank - # @param rep [Veteran::Service::Representative] Representative to match soon with - # @param birth_date [String] Submitted birth_date to match against representative - # - # @return [Boolean] True if matches, false if not - def self.matching_date_of_birth(rep, birth_date) - return true if birth_date.blank? + def self.for_user(first_name:, last_name:) + representatives = all_for_user(first_name:, last_name:) + return nil if representatives.blank? - rep.dob.present? && rep.dob == birth_date + representatives.first end # @@ -89,10 +53,10 @@ def self.matching_date_of_birth(rep, birth_date) # @param middle_initial [String] Submitted middle_initial to match against representative # # @return [Boolean] True if matches, false if not - def self.matching_middle_initial(rep, middle_initial) + def self.matching_middle_initial(representative, middle_initial) return true if middle_initial.blank? - rep.middle_initial.present? && rep.middle_initial == middle_initial + representative.middle_initial.present? && representative.middle_initial == middle_initial end # diff --git a/modules/veteran/spec/models/veteran/service/representative_spec.rb b/modules/veteran/spec/models/veteran/service/representative_spec.rb index 0ed3407192a..a8f4d72f625 100644 --- a/modules/veteran/spec/models/veteran/service/representative_spec.rb +++ b/modules/veteran/spec/models/veteran/service/representative_spec.rb @@ -25,50 +25,31 @@ def basic_attributes end describe 'finding by identity' do - let(:rep) do + let(:representative) do FactoryBot.create(:representative, - basic_attributes.merge!(ssn: identity.ssn, dob: identity.birth_date)) + basic_attributes) end before do identity - rep + representative end - describe 'finding by all fields' do - it 'finds a user by name, ssn, and dob' do + describe 'finding by the name' do + it 'finds a user' do expect(Veteran::Service::Representative.for_user( first_name: identity.first_name, - last_name: identity.last_name, - dob: identity.birth_date, - ssn: identity.ssn - ).id).to eq(rep.id) + last_name: identity.last_name + ).id).to eq(representative.id) end it 'finds right user when 2 with the same name exist' do FactoryBot.create(:representative, - basic_attributes.merge!(ssn: '123-45-6789', dob: '1929-10-01')) - expect(Veteran::Service::Representative.for_user( - first_name: identity.first_name, - last_name: identity.last_name, - dob: identity.birth_date, - ssn: identity.ssn - ).id).to eq(rep.id) - end - end - - describe 'finding by the name only' do - it 'finds a user by name fields' do - rep = FactoryBot.create(:representative, first_name: 'Bob', last_name: 'Smith') - identity = FactoryBot.create(:user_identity, first_name: rep.first_name, last_name: rep.last_name) - Veteran::Service::Representative.for_user( - first_name: identity.first_name, - last_name: identity.last_name - ) + basic_attributes) expect(Veteran::Service::Representative.for_user( first_name: identity.first_name, last_name: identity.last_name - ).id).to eq(rep.id) + ).id).to eq(representative.id) end end end @@ -118,26 +99,26 @@ def basic_attributes describe '#set_full_name' do context 'creating a new representative' do it 'sets the full_name attribute as first_name + last_name' do - rep = described_class.new(representative_id: 'abc', poa_codes: ['123'], first_name: 'Joe', - last_name: 'Smith') + representative = described_class.new(representative_id: 'abc', poa_codes: ['123'], first_name: 'Joe', + last_name: 'Smith') - expect(rep.full_name).to be_nil + expect(representative.full_name).to be_nil - rep.save! + representative.save! - expect(rep.reload.full_name).to eq('Joe Smith') + expect(representative.reload.full_name).to eq('Joe Smith') end end context 'updating an existing representative' do it 'sets the full_name attribute as first_name + last_name' do - rep = create(:representative, first_name: 'Joe', last_name: 'Smith') + representative = create(:representative, first_name: 'Joe', last_name: 'Smith') - expect(rep.full_name).to eq('Joe Smith') + expect(representative.full_name).to eq('Joe Smith') - rep.update(first_name: 'Bob') + representative.update(first_name: 'Bob') - expect(rep.reload.full_name).to eq('Bob Smith') + expect(representative.reload.full_name).to eq('Bob Smith') end end end