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.

Sign up for GitHub

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

Prefill with VAProfile Contact Information V2 instead of the PCIU #18729

Merged
merged 6 commits into from
Jan 2, 2025
Merged

Prefill with VAProfile Contact Information V2 instead of the PCIU #18729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
edc71e1
Prefill with VAProfile Contact Information V2 instead of the PCIU
RachalCassity Dec 17, 2024
7f8503e
Fixed merge conflicts
RachalCassity Dec 17, 2024
57aac8f
Fixed merge conficts
RachalCassity Dec 17, 2024
c9e8603
check if icn is present
RachalCassity Dec 17, 2024
a8ef84a
Added specs for remove_pciu flipper in the user model
RachalCassity Jan 2, 2025
2993d55
Fixed linting
RachalCassity Jan 2, 2025
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,7 @@ spec/models/form526_submission_spec.rb @department-of-veterans-affairs/Disabilit
spec/models/form526_submission_remediation_spec.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/form_attachment_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/form_profile_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/form_profile_v2_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/form_submission_spec.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/form_submission_attempt_spec.rb @department-of-veterans-affairs/platform-va-product-forms @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
spec/models/gi_bill_feedback_spec.rb @department-of-veterans-affairs/my-education-benefits @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
Expand Down
24 changes: 17 additions & 7 deletions app/models/form_profile.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ def prefill
@contact_information = initialize_contact_information
@military_information = initialize_military_information
form = form_id == '1010EZ' ? '1010ez' : form_id

if FormProfile.prefill_enabled_forms.include?(form)
mappings = self.class.mappings_for_form(form_id)

Expand Down Expand Up @@ -282,18 +283,21 @@ def vets360_contact_info_hash
def initialize_contact_information
opt = {}
opt.merge!(vets360_contact_info_hash) if vet360_contact_info
Rails.logger.info("User Vet360 Contact Info, Address? #{opt[:address].present?}
Email? #{opt[:email].present?}, Phone? #{opt[:home_phone].present?}")

if Flipper.enabled?(:remove_pciu, user)
# Monitor logs to validate the presence of Contact Information V2 user data
Rails.logger.info("VAProfile Contact Info: Address? #{opt[:address].present?},
Email? #{opt[:email].present?}, Phone? #{opt[:home_phone].present?}")
end
opt[:address] ||= user_address_hash

# The following pciu lines need to removed when tearing down the EVSS PCIU service.
opt[:email] ||= extract_pciu_data(:pciu_email)
if opt[:home_phone].nil?
opt[:home_phone] = pciu_primary_phone
opt[:us_phone] = pciu_us_phone
end

format_for_schema_compatibility(opt)

FormContactInformation.new(opt)
end

Expand All @@ -302,7 +306,9 @@ def vet360_contact_info
return @vet360_contact_info if @vet360_contact_info_retrieved

@vet360_contact_info_retrieved = true
if VAProfile::Configuration::SETTINGS.prefill && user.vet360_id.present?
if Flipper.enabled?(:remove_pciu, user) && user.icn.present?
@vet360_contact_info = VAProfileRedis::V2::ContactInformation.for_user(user)
elsif VAProfile::Configuration::SETTINGS.prefill && user.vet360_id.present?
@vet360_contact_info = VAProfileRedis::ContactInformation.for_user(user)
else
Rails.logger.info('Vet360 Contact Info Null')
Expand Down Expand Up @@ -330,7 +336,6 @@ def format_for_schema_compatibility(opt)
opt[:address][:street2] = apt[1]
opt[:address][:street] = opt[:address][:street].gsub(/\W?\s+#{apt[1]}/, '').strip
end

%i[home_phone us_phone mobile_phone].each do |phone|
opt[phone] = opt[phone].gsub(/\D/, '') if opt[phone]
end
Expand Down Expand Up @@ -362,6 +367,11 @@ def phone_object
home = vet360_contact_info&.home_phone
return home if home&.area_code && home.phone_number

if Flipper.enabled?(:remove_pciu, user)
# Track precense of home and mobile
Rails.logger.info("VAProfile Phone Object: Home? #{home.present?}, Mobile? #{mobile.present?}")
end

phone_struct = Struct.new(:area_code, :phone_number)

return phone_struct.new(pciu_us_phone.first(3), pciu_us_phone.last(7)) if pciu_us_phone&.length == 10
Expand Down Expand Up @@ -424,7 +434,7 @@ def clean!(value)
end

def clean_hash!(hash)
hash.deep_transform_keys! { |k| k.camelize(:lower) }
hash.deep_transform_keys! { |k| k.to_s.camelize(:lower) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure symbols are converted to string before running camelize.

hash.each { |k, v| hash[k] = clean!(v) }
hash.delete_if { |_k, v| v.blank? }
end
Expand Down
8 changes: 7 additions & 1 deletion app/models/form_profiles/va_686c674.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ def metadata
private

def prefill_form_address
mailing_address = VAProfileRedis::ContactInformation.for_user(user).mailing_address if user.vet360_id.present?
replace_pciu = Flipper.enabled?(:remove_pciu, user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can some of this be extracted to reduce duplication? Something like...

module ContactInformationVersioning
  extend ActiveSupport::Concern

  # Determine whether to use the new VAProfile V2 system
  def use_v2_service?(user)
    Flipper.enabled?(:remove_pciu, user)
  end

  # Retrieve the user's contact information
  def contact_information_from(user)
    if use_v2_service?(user)
      VAProfileRedis::V2::ContactInformation.for_user(user)
    else
      VAProfileRedis::ContactInformation.for_user(user)
    end
  end

  # Retrieve mailing address based on conditions
  def mailing_address_from(user)
    redis_prefill = contact_information_from(user)
    use_v2_service = use_v2_service?(user)
    # Only assign mailing address if the feature flag is enabled or the user has a vet360_id
    redis_prefill&.mailing_address if use_v2_service || user.vet360_id.present?
  end
end

Just a suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree to split this out if there were not steps after this to remove all the Contact Info V1 references. I believe the new version will be replaced soon after being pushed to prod. This would require more files to be removed when we remove PCIU and Contact Information in the later steps of the PCIU migration epic. Both Contact Information V1 and PCIU need to removed after this integration has been tested.

Copy link
Contributor

@tpharrison tpharrison Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking along the same lines and thought having a module/concern in place could make future upgrades more convenient. But, you're probably right - keeping it as-is for now should simplify clean up.

redis_prefill = if replace_pciu
VAProfileRedis::V2::ContactInformation.for_user(user)
else
VAProfileRedis::ContactInformation.for_user(user)
end
mailing_address = redis_prefill&.mailing_address if replace_pciu || user.vet360_id.present?
return if mailing_address.blank?

@form_address = FormAddress.new(
Expand Down
6 changes: 5 additions & 1 deletion app/models/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,11 @@ def onboarding
def vet360_contact_info
return nil unless VAProfile::Configuration::SETTINGS.contact_information.enabled && vet360_id.present?

@vet360_contact_info ||= VAProfileRedis::ContactInformation.for_user(self)
@vet360_contact_info ||= if Flipper.enabled?(:remove_pciu, self)
VAProfileRedis::V2::ContactInformation.for_user(self)
else
VAProfileRedis::ContactInformation.for_user(self)
end
end
Copy link
Member Author

@RachalCassity RachalCassity Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bosawt These lines are the only changes to the user model. The previous code, quetionable code, was removed. :)


def va_profile_email
Expand Down
3 changes: 0 additions & 3 deletions config/features.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,6 @@ features:
communication_preferences:
actor_type: user
description: Allow user to access backend communication_preferences API
contact_info_change_email:
actor_type: user
description: Send user a notification email when their contact info changes.
covid_vaccine_registration:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer used.

actor_type: user
description: Toggles availability of covid vaccine form API.
Expand Down
4 changes: 0 additions & 4 deletions lib/va_profile/contact_information/service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,6 @@ def get_email_personalisation(type)
end

def send_contact_change_notification(transaction_status, personalisation)
return unless Flipper.enabled?(:contact_info_change_email, @user)

transaction = transaction_status.transaction

if transaction.completed_success?
Expand All @@ -270,8 +268,6 @@ def send_contact_change_notification(transaction_status, personalisation)
end

def send_email_change_notification(transaction_status)
return unless Flipper.enabled?(:contact_info_change_email, @user)

transaction = transaction_status.transaction

if transaction.completed_success?
Expand Down
12 changes: 6 additions & 6 deletions modules/claims_api/app/swagger/claims_api/description/v2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## Background

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not modify the file. This came in from the merge conflict and extra spaces were removed.

The Benefits Claims API Version 2 lets internal consumers:
The Benefits Claims API Version 2 lets internal consumers:

- Retrieve existing claim information, including status, by claim ID.
- Automatically establish an Intent To File (21-0966) in VBMS.
Expand All @@ -11,7 +11,7 @@ The Benefits Claims API Version 2 lets internal consumers:
- Automatically establish a power of attorney appointment in VBMS for an accredited individual (VA Form 21-22a).

You should use the [Benefits Claims API Version 1](https://developer.va.gov/explore/benefits/docs/claims?version=current) if you are a consumer outside of VA and do not have the necessary VA agreements to use this API.

## Appointing an accredited representative for dependents

Dependents of Veterans, such as spouses, children (biological and step), and parents (biological and foster) may be eligible for VA benefits and can request representation by an accredited representative.
Expand All @@ -28,14 +28,14 @@ End-to-end claims tracking provides the status of claims as they move through th

### Claim statuses

After you submit a disability compensation claim with the `POST /veterans/{veteranId}/526/synchronous` endpoint, it is then established in Veterans Benefits Management System (VBMS). A `202` response means that the claim was successfully submitted by the API. However, it does not mean VA has received the required 526EZ PDF.
After you submit a disability compensation claim with the `POST /veterans/{veteranId}/526/synchronous` endpoint, it is then established in Veterans Benefits Management System (VBMS). A `202` response means that the claim was successfully submitted by the API. However, it does not mean VA has received the required 526EZ PDF.

To confirm the status of your submission, use the `GET /veterans/{veteranId}/claims/{id}` endpoint and the ID returned with your submission response. Statuses are:
To confirm the status of your submission, use the `GET /veterans/{veteranId}/claims/{id}` endpoint and the ID returned with your submission response. Statuses are:

* **Pending**: The claim is successfully submitted for processing
* **Errored**: The submission encountered upstream errors
* **Canceled**: The claim was identified as a duplicate, or another issue caused the claim to be canceled.
* For duplicate claims, the claim's progress is tracked under a different Claim ID than the one returned in your submission response.
* **Canceled**: The claim was identified as a duplicate, or another issue caused the claim to be canceled.
* For duplicate claims, the claim's progress is tracked under a different Claim ID than the one returned in your submission response.
* **Claim received**: The claim was received, but hasn't been assigned to a reviewer yet.
* **Initial review**: The claim has been assigned to a reviewer, who will determine if more information is needed.
* **Evidence gathering, review, and decision**: VA is gathering evidence to make a decision from health care providers, government agencies, and other sources.
Expand Down
2 changes: 1 addition & 1 deletion modules/mobile/app/services/mobile/v0/profile/sync_update_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def save!(http_method, resource_type, params)
end

def build_record(type, params)
if type == :address && Flipper.enabled?(:va_v3_contact_information_service, @user)
if type == :address && Flipper.enabled?(:mobile_v2_contact_info, @user)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mobile owns the Contact Information V2 migration for all mobile requests. Replaced va_v3_contact_information_service with mobile_v2_contact_info to prevent the newly updated Contact Information service to be used.

'VAProfile::Models::V3::Address'
.constantize
.new(params)
Expand Down
1 change: 0 additions & 1 deletion modules/mobile/spec/requests/mobile/v0/claims/pre_need_burial_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
include CommitteeHelper

describe 'POST /mobile/v0/claims/pre-need-burial' do
Flipper.disable(:va_v3_contact_information_service)
let!(:user) { sis_user(icn: '1012846043V576341') }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer needed.

let(:params) do
{ application: attributes_for(:burial_form) }
Expand Down
12 changes: 8 additions & 4 deletions modules/mobile/spec/requests/mobile/v0/user/addresses_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@

let!(:user) { sis_user }

describe 'update endpoints' do
Flipper.disable(:va_v3_contact_information_service)
before do
allow(Flipper).to receive(:enabled?).with(:mobile_v2_contact_info, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:remove_pciu, instance_of(User)).and_return(false)
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preventing flaky tests.

describe 'update endpoints', :skip_va_profile_user do
let(:address) do
address = build(:va_profile_address, vet360_id: user.vet360_id)
# Some domestic addresses are coming in with province of string 'null'.
Expand Down Expand Up @@ -255,8 +260,7 @@
end
end

describe 'POST /mobile/v0/user/addresses/validate' do
Flipper.disable(:va_v3_contact_information_service)
describe 'POST /mobile/v0/user/addresses/validate', :skip_va_profile_user do
let(:address) do
address = build(:va_profile_address, vet360_id: user.vet360_id)
# Some domestic addresses are coming in with province of string 'null'.
Expand Down
9 changes: 6 additions & 3 deletions modules/mobile/spec/requests/mobile/v0/user/contact_info_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

require_relative '../../../../support/helpers/rails_helper'
RSpec.describe 'Mobile::V0::User::ContactInfo', type: :request do
Flipper.disable(:va_v3_contact_information_service)
before do
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:remove_pciu, instance_of(User)).and_return(false)
end

let!(:user) { sis_user }
let(:attributes) { response.parsed_body.dig('data', 'attributes') }
Expand Down Expand Up @@ -86,7 +89,7 @@
}
end

describe 'GET /mobile/v0/user/contact_info with vet360 id' do
describe 'GET /mobile/v0/user/contact_info with vet360 id', :skip_va_profile_user do
context 'valid user' do
before do
get('/mobile/v0/user/contact-info', headers: sis_headers)
Expand All @@ -107,7 +110,7 @@
end
end

describe 'GET /mobile/v0/user/contact_info without vet360 id' do
describe 'GET /mobile/v0/user/contact_info without vet360 id', :skip_va_profile_user do
let!(:user) { sis_user(vet360_id: nil) }

before do
Expand Down
8 changes: 6 additions & 2 deletions modules/mobile/spec/requests/mobile/v0/user/phones_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@
end
let(:telephone) { build(:telephone, vet360_id: user.vet360_id) }

Flipper.disable(:va_v3_contact_information_service)
describe 'POST /mobile/v0/user/phones' do
before do
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:remove_pciu, instance_of(User)).and_return(false)
end

describe 'POST /mobile/v0/user/phones', :skip_va_profile_user do
context 'with a valid phone number' do
before do
telephone.id = 42
Expand Down
10 changes: 7 additions & 3 deletions modules/mobile/spec/requests/mobile/v0/user_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
VAProfile::ContactInformation::Service
end

describe 'GET /mobile/v0/user' do
before do
Flipper.disable(:va_v3_contact_information_service)
Flipper.disable(:remove_pciu)
end

describe 'GET /mobile/v0/user', :skip_va_profile_user do
let!(:user) do
sis_user(
first_name: 'GREG',
Expand All @@ -25,7 +30,6 @@
end

before(:all) do
Flipper.disable(:va_v3_contact_information_service)
Flipper.disable(:mobile_lighthouse_letters)
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer needed.


Expand Down Expand Up @@ -459,7 +463,7 @@
end
end

describe 'POST /mobile/v0/user/logged-in' do
describe 'POST /mobile/v0/user/logged-in', :skip_va_profile_user do
let!(:user) { sis_user }

it 'returns an ok response' do
Expand Down
8 changes: 6 additions & 2 deletions modules/mobile/spec/requests/mobile/v1/user_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@

RSpec.describe 'Mobile::V1::User', type: :request do
include JsonSchemaMatchers
Flipper.disable(:va_v3_contact_information_service)
let(:contact_information_service) do
VAProfile::ContactInformation::Service
end

describe 'GET /mobile/v1/user' do
before do
Flipper.disable(:va_v3_contact_information_service)
Flipper.disable(:remove_pciu)
end

describe 'GET /mobile/v1/user', :skip_va_profile_user do
let!(:user) do
sis_user(
first_name: 'GREG',
Expand Down
12 changes: 7 additions & 5 deletions modules/mobile/spec/services/sync_update_service_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
let(:user) { create(:user, :api_auth) }
let(:service) { Mobile::V0::Profile::SyncUpdateService.new(user) }

# DO THIS
describe '#save_and_await_response' do
before do
Flipper.disable(:va_v3_contact_information_service)
end
before do
allow(Flipper).to receive(:enabled?).with(:mobile_v2_contact_info, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service, instance_of(User)).and_return(false)
allow(Flipper).to receive(:enabled?).with(:remove_pciu, instance_of(User)).and_return(false)
end

# DO THIS
describe '#save_and_await_response', :skip_va_profile_user do
let(:params) { build(:va_profile_address, vet360_id: user.vet360_id, validation_key: nil) }

context 'when it succeeds after one incomplete status check' do
Expand Down
12 changes: 2 additions & 10 deletions modules/veteran/spec/sidekiq/representatives/update_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,11 @@
let(:address_exists) { true }

before do
Flipper.enable(:va_v3_contact_information_service)
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service).and_return(true)
create_flagged_records(flag_type)
allow(VAProfile::V3::AddressValidation::Service).to receive(:new).and_return(double('VAProfile::V3::AddressValidation::Service', candidate: nil)) # rubocop:disable Layout/LineLength
end

after do
Flipper.disable(:va_v3_contact_information_service)
end

it "updates the #{flag_type} and the associated flagged records" do
flagged_records =
RepresentationManagement::FlaggedVeteranRepresentativeContactData
Expand Down Expand Up @@ -731,14 +727,10 @@ def create_flagged_records(flag_type)
end

before do
Flipper.enable(:va_v3_contact_information_service)
allow(Flipper).to receive(:enabled?).with(:va_v3_contact_information_service).and_return(true)
allow_any_instance_of(VAProfile::V3::AddressValidation::Service).to receive(:candidate).and_return(api_response)
end

after do
Flipper.disable(:va_v3_contact_information_service)
end

context 'when JSON parsing fails' do
let(:invalid_json_data) { 'invalid json' }

Expand Down
Loading
Loading