LG 11352 Remove double address verification entirely - deploy second#9857
LG 11352 Remove double address verification entirely - deploy second#9857JackRyan1989 merged 19 commits intomainfrom
Conversation
…n't add dav in agent job call.
| return false unless should_proof_state_id | ||
| # If the user is in double-address-verification and they have changed their address then | ||
| # If the user is in in-person-proofing and they have changed their address then | ||
| # they are not eligible for get-to-yes |
There was a problem hiding this comment.
this question isn't about any update you've made but more about clarifying what this means. I wonder what is meant by "changed their address"? Did we mean if same_address_as_id is false that they aren't eligible for get to yes?
There was a problem hiding this comment.
I think John Hooper left this comment. I'm thinking it's not actually relevant anymore since we do user_can_pass_after_state_id_check? under the if check? Can't user_can_pass_after_state_id_check? help get to yes?
|
one thing i would want to check is that the pii isn't updated just to confirm that everything is as expected |
Just checking, but I assume you mean the pii doesn't change when ipp_enrollment_in_progress is false, but that |
This reverts commit ff6128a. After reviewing the spec these tests are not necessary.
| let(:should_proof_state_id) { true } | ||
| let(:ipp_enrollment_in_progress) { true } | ||
| let(:double_address_verification) { true } | ||
| let(:ipp_enrollment_in_progress) { false } |
There was a problem hiding this comment.
Setting this to false initially will allow us to test the remote flow from the get go.
|
|
||
| it 'returns a ResultAdjudicator' do | ||
| proofing_result = proof | ||
| context 'remote proofing' do |
There was a problem hiding this comment.
The reason this diff is so huge is that I restructured the test a bit. I placed all the initial tests that pertain to remote stuff into a new context block, and then placed all of the existing IPP related tests into an IPP centered context block.
| and_return(true) | ||
| allow(result_that_failed_instant_verify).to receive(:success?). | ||
| and_return(false) | ||
| allow(instance).to receive(:with_state_id_address).and_return(transformed_pii) |
There was a problem hiding this comment.
Mocking the method that does the PII transforming for IPP flow.
svalexander
left a comment
svalexander
left a comment
There was a problem hiding this comment.
My last suggestion is to change the mock applicant used in the "false" case for prog. proofer but it's not blocking. Going to approve but do we want to get another set of eyes to be safe?
Yeah let's grab one more set of 👀. I really want to make sure the 50/50 state is covered as described in the handbook. |
n1zyy
left a comment
n1zyy
left a comment
There was a problem hiding this comment.
Overall this looks awesome.
I have a couple suggestions/mumblings, but none blocking, so calling this Approved.
Stop disabling rubocop Co-authored-by: Matt Wagner <mattwagner@navapbc.com>
4 participants
🎫 Ticket
Link to the relevant ticket
🛠 Summary of changes
Fulfills step three of: https://handbook.login.gov/articles/manage-50-50-state.html#remove-an-argument-from-a-job-perform-method
Note: #9854 needs to be deployed first and past the 50/50 state for this code to be ready to merge, hence the status.
📜 Testing Plan
All tests should pass as normal. When running through IPP and Remote Proofing there should be no changes to the proofing or user flow.