LG-8693: Verify residential address with LexisNexis#8343
Conversation
...when the user is going through double address verification and has a different address listed on their state ID than where they reside. changelog: Upcoming Features, Double address verification, Begin proofing state ID address
This was necessary with how the previous version was structured but it's not anymore
…mposition-exploration
| ) | ||
| end | ||
|
|
||
| # TK: confirm vendor name with Team Ada |
There was a problem hiding this comment.
I will follow up with Team Ada in Slack about this.
There was a problem hiding this comment.
I followed up with Team Ada. They confirmed the decision to
put the reason for not calling the vendor, rather than putting the name of the vendor.
|
|
||
| # todo(LG-8693): Begin verifying both the user's residential address and identity document | ||
| # address | ||
| residential_instant_verify_proof = proof_residential_address_if_needed( |
There was a problem hiding this comment.
Hi @jmhooper, I wanted to seek out your opinion on how SOLID principles apply to this file.
When @night-jellyfish was synchronously reviewing this PR with me, she pointed out that the methods proof_with_threatmetrix_if_needed, proof_residential_address_if_needed, proof_id_address_with_lexis_nexis_if_needed and proof_id_with_aamva_if_needed, violate single responsibility principles because they A) determine if the proofing is necessary then B) do the actual proofing. I saw that the proof_with_threatmetrix_if_needed method existed in resolution proofing job before Team Joy introduced progressive proofer and I wanted to seek out your thoughts on this pattern. Thanks!
There was a problem hiding this comment.
The only complicated logic/precondition part is proof_id_with_aamva_if_needed, since it depends on some control flags and previous step's result. I think we only need refactor this part.
The whole progressive proofer feels like a facade/chain of command pattern.
There was a problem hiding this comment.
The whole progressive proofer feels like a facade/chain of command pattern.
@dawei-nava I agree with this part of your comment, seems like the chain of command pattern particularly.
| end | ||
| end | ||
|
|
||
| def residential_address_unnecessary_result |
There was a problem hiding this comment.
is this for the case where a user has not been presented DAV and so they only have their state id address?
There was a problem hiding this comment.
You're correct this is for the case where a user is not going through the double address verification flow and only one address is verified. Relevant code here.
I wouldn't say that "they only have their state id address," because before DAV, someone could enter their address on the pre-DAV address page and check that their current address does not appear on their state-issued id. Link to figma here.
There was a problem hiding this comment.
i think it might be helpful to include a short comment above each of these functions that briefly describes the use case
There was a problem hiding this comment.
I think that progressive_proofer.rb retains too much of the complexity of this process & needs some of the logic extracted into modules. One clear point of separation would be using a separate module as a results factory that could contain all of the customized calls to Proofing::AddressResult.new.
NavaTim
left a comment
NavaTim
left a comment
There was a problem hiding this comment.
I think the latest changes get us where we need to be, but I still highly recommend splitting up the progressive proofer more before merging this.
tomas-nava
changed the title
7 participants
🎫 Ticket
LG-8693: Verify current address with LexisNexis
🛠 Summary of changes
OR
📜 Testing Plan
Provide a checklist of steps to confirm the changes.