LG-11235: Rename double address verification as ipp_enrollment_in_progress by JackRyan1989 · Pull Request #9390 · 18F/identity-idp

JackRyan1989 · 2023-10-16T14:35:41Z

🎫 LG-11235

Link to the relevant ticket. (I don't have Jira access ATM, so you'll have to manually look this up :P )

🛠 Summary of changes

Removed double_address_verification and replaced with ipp_enrollment_in_progress. The motivation for this naming change is that DAV used to check if a user's enrollment was either pending or establishing. We don't want to alter the logic of the proofer at the moment, so we want to keep this check while renaming it to something more informative.

📜 Testing Plan

Run the normal test suite and all should pass without issue. No new tests or function changes to existing tests.

…rogress

svalexander · 2023-10-16T15:14:01Z

app/controllers/concerns/idv/verify_info_concern.rb

    private

-    def double_address_verification
+    def ipp_enrollment_in_progress?


i think this name makes sense.

svalexander · 2023-10-16T15:37:42Z

app/controllers/concerns/idv/verify_info_concern.rb


-    def double_address_verification
+    def ipp_enrollment_in_progress?
      # If in person return true else return false. This is temporary until we add a feature flag


i think we can remove these comments in this pr as well

…5-use-ipp-instead-of-dav

…lect reality

svalexander · 2023-10-16T17:48:10Z

spec/controllers/idv/in_person/verify_info_controller_spec.rb

      end

-      it 'disables double address verification for the user' do
+      it 'indicates to the IDV agent that ipp_enrollment_in_progress is disabled' do


i think rather than disabled/enabled we should change the wording to true/false or something more in line with the fact that the user is going through ipp.

Ok, yeah I guess the flow isn't being altered anymore, right? We're just monitoring whether their in the middle of an enrollment, right?

…controller

zachmargolis · 2023-10-16T22:59:36Z

app/jobs/resolution_proofing_job.rb

    trace_id:,
    should_proof_state_id:,
-    double_address_verification: false,
+    ipp_enrollment_in_progress: false,


renaming a job argument for a background job needs to be done in multiple steps because of the 50-50 state during deploys.

add the new name as with a default value to the job

add the new name

remove the old name

Ah yeah, thanks for the reminder @zachmargolis!

@zachmargolis maybe this is a conversation for slack, but I'm unsure why I need to only account for the 50/50 state in the job code? Does the application code get provisioned differently than the jobs do?

On the same box, all code will be the same version, so in-memory code doesn't need to worry about the 50-50 state.

Background jobs get written to the database, so during the 50-50 state, they can be picked up by either old or new boxes. So since we don't know which version the box will write the job is and which version box will perform the job is, we need to be more flexible.

app/services/idv/agent.rb

app/services/proofing/resolution/result_adjudicator.rb

app/services/idv/agent.rb

Adding proper input to job_arguments hash. Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

svalexander · 2023-10-20T12:59:58Z

app/services/proofing/resolution/progressive_proofer.rb

      # @param [Hash] applicant_pii keys are symbols and values are strings, confidential user info
      # @param [Boolean] double_address_verification flag that indicates if user will have
+      #   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


can we update the comment to include the number for the ticket that will address this? (the new one you added)

Yep! I made the ticket last night. I'll get that noted in this comment now. Great idea @svalexander

…F/identity-idp into jryan/lg-11235-use-ipp-instead-of-dav

svalexander · 2023-10-20T17:46:40Z

spec/services/proofing/resolution/result_adjudicator_spec.rb

      end
+
+      # rubocop:disable Layout/LineLength
+      context 'Confirm adjudication works for either double_address_verification or ipp_enrollment_in_progress' do


svalexander · 2023-10-20T17:49:27Z

spec/services/proofing/resolution/progressive_proofer_spec.rb


        expect(result.same_address_as_id).to eq('true')
-        expect(result.double_address_verification).to eq(true)
+        expect(result.ipp_enrollment_in_progress).to eq(true)


let's add dav check back in

svalexander · 2023-10-20T17:55:49Z

app/services/proofing/resolution/result_adjudicator.rb

-        if !residential_resolution_result.success? &&
-           same_address_as_id == 'false' && double_address_verification == true
+        if !residential_resolution_result.success? && same_address_as_id == 'false' &&
+           (ipp_enrollment_in_progress == true || double_address_verification)


remove == ||

* LG-11083: Enable USPS Public Endpoint (#9355) * changelog: Internal, In-Person Proofing, Enable public USPS post office search * Use EnrollmentHelper to switch between mock/real thing * Try behaves_like * Revert shared examples for now * Use full name * Update report mailer preview to be more realistic (#9419) **How**: stubs CloudwatchClient changelog: Internal, Reporting, Updates report preview to use live code * Add analytics section to frontend documentation (#9421) * Add analytics section to frontend documentation changelog: Internal, Documentation, Add analytics frontend documentation * link to correct javascript package * LG-11101: Support multiple valid MFA to satisfy authentication request (#9335) changelog: User-Facing Improvements, MFA, Avoid prompting for MFA in some scenarios where a recent MFA satisfies the requirement * LG-11148 | Adds monthly report on total verified users (#9376) changelog: Internal, Reporting, Monthly report now includes total verified users Also incorporates LG-11150 Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> * Remove second MFA prompt exception for strict MFA requirement (#9422) changelog: User-Facing Improvements, MFA Setup, Add second MFA reminder screen for single-MFA accounts when signing in at AAL2 * LG-11126 Update Start over verifying your identity screen (#9313) * change text for start over verify screen * add translations for page * add changelog changelog: User-Facing Improvements, IdV By Mail, update text in start over verifying identity screen * remove unused i18n * create new translation with question mark added * current step indicator for user not in gpo flow yet * a missing period * Restore deleted translations, and rename start_over to start_over_new_address Co-authored-by: Doug Price <douglas.price@gsa.gov> * New template for confirm start over from request_letter Add source param to indicate whether referer is request_letter * Update specs to check for correct template Co-authored-by: Doug Price <douglas.price@gsa.gov> * Add before_letter route for new screen, don't use it yet And analytics * Lint, unused arg in analytics_events * alphabetization lint * Add suggested comment Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> * lints --------- Co-authored-by: Douglas Price <douglas.price@gsa.gov> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> * LG-11198: Update address text (#9420) Update address text changelog: User-Facing Improvements, IdV, Update text for address * LG-10922: Display new headings for Hybrid Handoff page on AB test (#9316) * changelog: User-Facing Improvements, Doc Auth, Display new headings for Hybrid Handoff page on AB test Adds: * Conditional headers depending on which flag is on * Hybrid handoff show view test * Translations * LG-11235: Rename double address verification as ipp_enrollment_in_progress (#9390) * Removed double address verification replaced with ipp_enrollment_in_progress * changelog: Internal, In-person Proofing, change DAV references to reflect reality * Change test description to be closer to what is being changed in the controller * Addressing 50/50 state concerns in proofer and adjudicator * Addressing linter issues * Set missing initial value for dav * Moving arg with default value to end of list * Apply suggestions from code review Adding proper input to job_arguments hash. Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Adding note about existing ticket for work post 50/50 state * Resolving Shannon's comments * Adding back in test for dav, need reader on adjudicator * Adding back in test for dav, need reader on adjudicator --------- Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Add --deflate option to data-pull and action-account scripts (#9424) changelog: Internal, Scripts, Add --deflate option to data-pull and action-account scripts --------- Co-authored-by: Matt Gardner <wilburnforce@gmail.com> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Matt Wagner <mattwagner@navapbc.com> Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov> Co-authored-by: Douglas Price <douglas.price@gsa.gov> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Brittany Greaner <35475380+night-jellyfish@users.noreply.github.com> Co-authored-by: Jack Ryan <jackryan@navapbc.com> Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>

Removed double address verification replaced with ipp_enrollment_in_p…

e46a305

…rogress

JackRyan1989 self-assigned this Oct 16, 2023

JackRyan1989 requested review from sheldon-b and svalexander October 16, 2023 14:35

svalexander reviewed Oct 16, 2023

View reviewed changes

JackRyan1989 added 2 commits October 16, 2023 13:43

Merge branch 'main' of github.com:18F/identity-idp into jryan/lg-1123…

ad1e48e

…5-use-ipp-instead-of-dav

changelog: Internal, In-person Proofing, change DAV references to ref…

5e5feac

…lect reality

svalexander reviewed Oct 16, 2023

View reviewed changes

Change test description to be closer to what is being changed in the …

1f692ef

…controller

JackRyan1989 changed the title ~~Removed double address verification replaced with ipp_enrollment_in_progress~~ LG-11235: Rename double address verification as ipp_enrollment_in_progress Oct 16, 2023

JackRyan1989 marked this pull request as ready for review October 16, 2023 18:42

zachmargolis reviewed Oct 16, 2023

View reviewed changes

JackRyan1989 added 2 commits October 18, 2023 15:27

Addressing 50/50 state concerns in proofer and adjudicator

a849bff

Addressing linter issues

da60f00

zachmargolis reviewed Oct 18, 2023

View reviewed changes

app/services/idv/agent.rb Show resolved Hide resolved

JackRyan1989 added 2 commits October 18, 2023 17:05

Set missing initial value for dav

2ae1d96

Moving arg with default value to end of list

9c4014a

gina-yamada reviewed Oct 19, 2023

View reviewed changes

app/services/proofing/resolution/result_adjudicator.rb Show resolved Hide resolved

zachmargolis reviewed Oct 19, 2023

View reviewed changes

app/services/idv/agent.rb Outdated Show resolved Hide resolved

Apply suggestions from code review

023e489

Adding proper input to job_arguments hash. Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

JackRyan1989 requested a review from a team October 19, 2023 19:02

svalexander reviewed Oct 20, 2023

View reviewed changes

JackRyan1989 added 2 commits October 20, 2023 09:34

Adding note about existing ticket for work post 50/50 state

a3553dc

Merge branch 'jryan/lg-11235-use-ipp-instead-of-dav' of github.com:18…

e0b933b

…F/identity-idp into jryan/lg-11235-use-ipp-instead-of-dav

zachmargolis approved these changes Oct 20, 2023

View reviewed changes

svalexander reviewed Oct 20, 2023

View reviewed changes

svalexander approved these changes Oct 20, 2023

View reviewed changes

JackRyan1989 added 3 commits October 20, 2023 14:04

Resolving Shannon's comments

33b15f0

Adding back in test for dav, need reader on adjudicator

6383928

Adding back in test for dav, need reader on adjudicator

bfc6c0f

JackRyan1989 merged commit a941eb6 into main Oct 20, 2023

JackRyan1989 deleted the jryan/lg-11235-use-ipp-instead-of-dav branch October 20, 2023 19:28

mdiarra3 mentioned this pull request Oct 23, 2023

Deploy RC 326 to Prod #9432

Merged

svalexander mentioned this pull request Jan 2, 2024

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

Closed

Conversation

JackRyan1989 commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 LG-11235

🛠 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!

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!

Uh oh!

Uh oh!

Uh oh!

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JackRyan1989 commented Oct 16, 2023 •

edited

Loading

svalexander Oct 20, 2023 •

edited

Loading