LG-11202 add untracked costs by svalexander · Pull Request #9753 · 18F/identity-idp

svalexander · 2023-12-13T15:01:11Z

🎫 Ticket

🛠 Summary of changes

Adds cost tracking for second Lexis Nexis Instant Verify check when user's residential address differs from the address on their state id.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

Remote Cost

Proof through remote flow
In terminal open rails console rails c
Check the costs added to the db SpCost.last({num > 4})
Verify that only a cost for threatmetrix and Lexis Nexis exists for the timestamp when you proofed

Ipp Cost - same_address_as_id = true

Proof through ipp flow using an aamva jurisdiction
In terminal open rails console rails c
Check the costs added to the db SpCost.last({num > 4})
Verify that the following costs were added for the timestamp when you proofed: LexisNexis x 1, Aamva x 1, Threatmetrix x 1

Ipp Cost - same_address_as_id = false

Proof through ipp flow using an aamva jurisdiction
In terminal open rails console rails c
Check the costs added to the db SpCost.last({num > 4})
Verify that the following costs were added for the timestamp when you proofed: LexisNexis x 2, Aamva x 1, Threatmetrix x 1

svalexander · 2023-12-13T16:21:04Z

app/controllers/concerns/idv/verify_info_concern.rb

          # transaction_id comes from ConversationId
          add_cost(:lexis_nexis_resolution, transaction_id: hash[:transaction_id])
+        elsif stage == :residential_address
+          next if hash[:vendor_name] == 'ResidentialAddressNotRequired'


if going through remote resident address check is skipped

svalexander · 2023-12-13T16:38:15Z

spec/controllers/idv/in_person/verify_info_controller_spec.rb

+      it 'adds costs' do
+        allow(controller).to receive(:load_async_state).and_return(async_state)
+        allow(async_state).to receive(:done?).and_return(true)
+        allow(async_state).to receive(:result).and_return(adjudicated_result)


adjudicated result reflects result when a user goes through ipp. Stages in result appears to be the same whether or not the user's residential address matches their state id.

Want to check if this is accurate re:Expected costs that should be tracked/added in db

remote, aamva supported state:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 1 (resolution stage)
aamva cost x 1 (state id stage)

remote, aamva unsupported state:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 1 (resolution stage)

ipp, aamva supported state, same_address_as_id is true:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage)
aamva cost x 1 (state id stage)

ipp, aamva unsupported state, same_address_as_id is true:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage)

ipp, aamva supported state, same_address_as_id is false:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage)
aamva cost x 1 (state id stage)

ipp, aamva unsupported state, same_address_as_id is false:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage)

daviddsilvanava · 2023-12-13T17:53:00Z

@svalexander what is resolution stage vs residential stage?

svalexander · 2023-12-13T17:57:00Z

@daviddsilvanava That's a good question. When I look at the result adjudicator what i see is this:

resolution_result:, # InstantVerify
state_id_result:, # AAMVA
residential_resolution_result:, # InstantVerify Residential

So it looks like 2 different instantverify checks?

svalexander · 2023-12-13T18:10:38Z

@eileen-nava what is the difference between the InstantVerify check and the InstantVerify Residential checks mentioned above/found in the result_adjudicator?

daviddsilvanava · 2023-12-13T19:07:36Z

@daviddsilvanava That's a good question. When I look at the result adjudicator what i see is this:

resolution_result:, # InstantVerify state_id_result:, # AAMVA residential_resolution_result:, # InstantVerify Residential

So it looks like 2 different instantverify checks?

I think resolution = state ID and residential = current address, when the user marks they're at a different address than their ID.

ipp, aamva supported state, same_address_as_id is true:
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage) --> I think there is only 1 LN check in this scenario.
aamva cost x 1 (state id stage)

ipp, aamva supported state, same_address_as_id is false: --> this looks correct.
threatmetrix cost x 1 (threatmetrix stage)
lexis nexis cost x 2 (resolution stage, residential stage)
aamva cost x 1 (state id stage)

iirc, the other things to keep in mind is that we run the checks serially. so if they fail one, we abandon the others. i don't remember what order we do them in though.

eileen-nava · 2023-12-14T22:01:06Z

Hi, I agree with what David said above.

If double address verification is enabled, which it is for IPP, and the address on the user’s license matches the address they live at, there will only be one call to LexisNexis InstantVerify.

When a user goes through IPP and the address on their license does not match the address they live at, there will be two calls to LexisNexis InstantVerify.

soniaconnolly

LGTM, with a spec suggestion.

I don't know off-hand what the results should be, which is one reason to add the spec on the remote side and make sure the results are what you expect. I have not run through your test plan - let me know if you'd like me to.

soniaconnolly · 2023-12-19T16:24:21Z

spec/controllers/idv/in_person/verify_info_controller_spec.rb

+        let(:pii) do
+          { same_address_as_id: 'true' }
+        end
+
+        it 'adds costs to database' do
+          allow(subject).to receive(:pii).and_return(pii)


This is overall a great addition to the specs! We could think about adding a similar one to the remote verify_info_controller_spec, not necessarily in this PR. That also brings up thoughts about how the cost calculation should be part of progressive proofer rather than verify info concern, but again, not a problem for this PR.

Let's cut down on the amount of stubbing so we can have more confidence in our specs. You already have a let for pii_from_user at the top, and it's hooked into user_session on line 20. If the suggestion below doesn't work, you might have to specify the whole thing: user_session['idv/in_person'][:pii_from_user]...

Same suggestion for all the specs below to set :same_address_as_id to the desired value.

Suggested change

let(:pii) do

{ same_address_as_id: 'true' }

end

it 'adds costs to database' do

allow(subject).to receive(:pii).and_return(pii)

it 'adds costs to database' do

pii_from_user[:same_address_as_id] = true

I definitely think that's a good call out about moving the file where the costs are added to the db. Matt W. also made that point. It would definitely make sense to move it as it would be a lot more clear what's happening as the cost is occurring in the progressive proofer. I'll add a ticket for that.

I'll update the spec, this is definitely cleaner.

I updated the spec and found I didn't need to update the value of same_address_as_id in the it block

Ticket to move the cost tracking to the progressive proofer

soniaconnolly · 2023-12-19T16:50:29Z

spec/controllers/idv/in_person/verify_info_controller_spec.rb

+                transaction_id: 'resolution-mock-transaction-id-123',
+                vendor_name: 'ResolutionMock',
+              },
+              residential_address: {


Is there a reason this is just called residential_address and not something like resolution_residential_address? I had to go look in progressive proofer to make sure they were parallel. Once again not necessarily for this PR, but a naming change to consider.

the stage is named residential_address currently, i think it was named that way to reflect the stage that already existed named state_id - just the name of what was proofed.

Seems like the whole naming scheme could use some attention at some point, although the 50/50 state makes it a hassle to change.

JackRyan1989 · 2023-12-19T18:16:56Z

@svalexander I am not getting the expected SpCost values from the database when doing a test run through. Can we meet to make sure I'm doing it correctly?

soniaconnolly · 2023-12-19T18:37:44Z

@svalexander I am not getting the expected SpCost values from the database when doing a test run through. Can we meet to make sure I'm doing it correctly?

@JackRyan1989, I was just testing this before standup and Shannon and I realized that the query needs to be SpCost.last(10) and check the timestamps, because SpCost doesn't have the user_id.

svalexander · 2023-12-19T19:19:48Z

@JackRyan1989 i just updated the test instructions

update add costs with residential_address stage

9f7205c

svalexander marked this pull request as draft December 13, 2023 15:01

in person cost spec added to verify info controller spec

aee3efe

svalexander changed the title ~~update add costs with residential_address stage~~ add untracked costs Dec 13, 2023

svalexander commented Dec 13, 2023

View reviewed changes

svalexander added 4 commits December 15, 2023 18:27

skip adding res_address lex nex cost if same_address is true

273ce93

add tests for cost for ipp flow

41baca7

refactor spec

bd0fb80

changelog: Internal, Verify info concern, update cost tracking for ipp

7af367e

svalexander changed the title ~~add untracked costs~~ LG-11202 add untracked costs Dec 16, 2023

svalexander marked this pull request as ready for review December 18, 2023 19:59

svalexander requested review from a team, JackRyan1989 and soniaconnolly December 18, 2023 21:09

Merge branch 'main' into shannon/lg-11202-track-costs

ba82459

soniaconnolly approved these changes Dec 19, 2023

View reviewed changes

soniaconnolly reviewed Dec 19, 2023

View reviewed changes

update spec

e537f2b

svalexander merged commit 4b9bccc into main Dec 20, 2023

svalexander deleted the shannon/lg-11202-track-costs branch December 20, 2023 17:40

jmdembe mentioned this pull request Dec 21, 2023

Deploy RC 342 to Prod #9819

Merged

Conversation

svalexander commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daviddsilvanava commented Dec 13, 2023

Uh oh!

svalexander commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svalexander commented Dec 13, 2023

Uh oh!

daviddsilvanava commented Dec 13, 2023

Uh oh!

eileen-nava commented Dec 14, 2023

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackRyan1989 commented Dec 19, 2023

Uh oh!

soniaconnolly commented Dec 19, 2023

Uh oh!

svalexander commented Dec 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

svalexander commented Dec 13, 2023 •

edited

Loading

svalexander commented Dec 13, 2023 •

edited

Loading