Skip to content

LG-11352: Remove Double Address Verification from job arguments#9829

Closed
JackRyan1989 wants to merge 9 commits intomainfrom
jryan/lg-11352-remove-dav
Closed

LG-11352: Remove Double Address Verification from job arguments#9829
JackRyan1989 wants to merge 9 commits intomainfrom
jryan/lg-11352-remove-dav

Conversation

@JackRyan1989
Copy link
Contributor

@JackRyan1989 JackRyan1989 commented Dec 22, 2023

🎫 Ticket

Open Question

We are removing the double_address_verification variable and replacing it with ipp_enrollment_in_progress. DAV used to be a configuration value, but since we're doing ipp_enrollment_in_progress that will always be true if a user's enrollment is either pending, or establishing. Which as they are moving through the flow, is always true.

  1. Do we need to check in the proofer, adjudicator and resolution job for cases where this could be false? Could ipp_enrollment_in_progress ever be false when this code is being run?
    Answer: NO. ipp_enrollment_in_progress is serving to distinguish between the remote proofing and ipp flows. Some information is only relevant for IPP or remote, and we want to maintain that separation.

If not, then I could cut out a few things to have them always or never happen.

🛠 Summary of changes

Removed the double_address_verification variable.

📜 Testing Plan

Nothing should change. All specs should continue to pass.
As a test, run through the IPP flow, and on the address step first select:

  1. No my address is different from the one on my ID
  2. Proofing Job should not fail

Create a new enrollment.

  1. Yes my address is the same as the one on my ID
  2. Proofing Job should not fail

@JackRyan1989 JackRyan1989 self-assigned this Dec 22, 2023
@JackRyan1989 JackRyan1989 marked this pull request as ready for review December 26, 2023 21:56
should_proof_state_id:,
double_address_verification: false,
ipp_enrollment_in_progress: false,
ipp_enrollment_in_progress:,
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we need to split this over several deploys to gracefully handle 50/50 states?

Ref: https://handbook.login.gov/articles/manage-50-50-state.html#jobs

Copy link
Contributor

Choose a reason for hiding this comment

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

This pr is a follow up to this one where ipp_enrollment_in_progress was added in order to eventually replace double_address_verification. This should satisfy the 50/50 requirements correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this pull request includes both of Deploy 2 ("Remove the argument in calls to the job") and Deploy 3 ("Remove the argument") from under "Remove an argument from a job #perform method".

Remove the argument in calls to the job: https://github.com/18F/identity-idp/pull/9829/files#diff-b4c1fa063fe7add8eae466de029891f060102bf5c16e4e69e88fe76e5676332fL31

Remove the argument [in the job itself]: https://github.com/18F/identity-idp/pull/9829/files#diff-32ee3d7bfb2561fc0fdef096c00c6533d565fb019e842a72be42b16b9fdaa959L22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah thanks @aduth this PR should be broken into two. @svalexander I didn't quite get it right with the first PR you referenced. That one made sure the jobs are aware of ipp_enrollment_in_progress and that the agent.rb file passes in the correct value to the jobs.

# both state id address and current residential address verified. Note this value is here as
# a placeholder until it can be replaced with ipp_enrollment_in_progress in ticket LG-353:
# https://cm-jira.usa.gov/browse/LG-11353
# @param [Boolean] ipp_enrollment_in_progress flag that indicates if user will have
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably still keep this definition in addition to the newly added info that it's used in place of the dav flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the DAV flag still in place? I thought it was removed from the config?

Copy link
Contributor

@svalexander svalexander Jan 2, 2024

Choose a reason for hiding this comment

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

it's removed - i was thinking the new comment should be more like: ~ # @param [Boolean] ipp_enrollment_in_progress flag is used in place of DAV flag because DAV is # now always true # it indicates if both state id address and current residential address verified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought ipp_enrollment_in_progress indicates that an enrollment is either in the 'establishing' or 'in progress' statuses? Happy to be wrong though!

Copy link
Contributor

Choose a reason for hiding this comment

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

yes but it was also meant as a replacement for double_address_verification . So it's like an in person enrollment exists for the user acting as a stand in for the user is going through the ipp flow.

timer:,
user_email:,
double_address_verification: false
user_email:
Copy link
Contributor

Choose a reason for hiding this comment

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

do we not need ipp_enrollment_in_progress here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Soooo, my open question at the top is more along the lines of "do we need ipp_enrollment_in_progress at all, since I'm pretty sure it's always true? Except when an enrollment is failed or completed it's always in progress or establishing, right?"

@JackRyan1989 JackRyan1989 changed the title LG-11352: Remove Double Address Verification var entirely LG-11352: Remove Double Address Verification from job arguments Jan 4, 2024
@JackRyan1989
Copy link
Contributor Author

Closing this in favor of a smaller, less convoluted git history.

@JackRyan1989 JackRyan1989 deleted the jryan/lg-11352-remove-dav branch May 23, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants