LG-9139: Support double address verification on address page by eileen-nava · Pull Request #8041 · 18F/identity-idp

eileen-nava · 2023-03-21T20:32:55Z

🎫 Ticket

LG-9139: Updates to Address page for Double Address Verification

🛠 Summary of changes

For all users...

Display the heading "Enter your current residential address"
~~Display beginning implementation of hint text for Puerto Rico users~~ covered by LG-9200 (out of scope)

When the feature flag is true....

Remove descriptive text at the top of the page
Relocate state dropdown to the top of the page
Remove the radio button yes/no question from the page's bottom
Remove the "optional" text from Address line 2 (but field remains optional)
Submit a new logging event: "IdV: in person proofing residential_address submitted"

Note about technical changes:

This PR allows the analytics event functions to be sourced from BaseStep instances in addition to the singleton methods on the classes. This supports validation based on user attributes, and one of the included feature tests will fail if the validation is instead added/removed statically.

📜 Testing Plan

High-level steps

Run automated tests
Compare the updated address page to the designs
Check translations
Confirm users can still proceed through the flow when the feature flag is off

Detailed testing steps

Feature Flag - Off

Feature Flag - On

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

English:

Spanish:

French:

app/javascript/packs/puerto-rico-hint.ts

svalexander · 2023-03-21T21:31:34Z

app/services/idv/steps/in_person/address_step.rb


        def call
          Idv::InPerson::AddressForm::ATTRIBUTES.each do |attr|
+            if attr == :same_address_as_id && IdentityConfig.store.


checking my understanding - here we're saying if the current residential address is the same as the one on the user's id don't get attributes from flow_params

No, we're choosing whether to assign the same_address_as_id field to the session here based on the feature flag. #call here is accepting data from the address form submission.

app/javascript/packs/puerto-rico-hint.ts

app/validators/idv/in_person/form_address_validator.rb

app/views/idv/in_person/address.html.erb

NavaTim · 2023-03-21T21:57:34Z

I reran the failed spec to make sure, and the same test failed twice. The failed test suggests that this PR may have introduced a regression or test issue related to analytics.

app/javascript/packs/puerto-rico-hint.ts

svalexander · 2023-03-21T22:34:18Z

spec/features/idv/in_person_spec.rb

+        search_for_post_office
+
+        # location page
+        location = page.find_all('.location-collection-item')[1]


test is failing here

Test has been updated to use a more stable version of this step.

config/locales/in_person_proofing/fr.yml

kellular

LGTM Eileen!

…39-dav-address-page

This reverts commit 99b3a86.

svalexander · 2023-03-23T14:41:30Z

app/services/flow/flow_state_machine.rb

      increment_step_name_counts
      analytics.public_send(
-        flow.step_handler(step).analytics_submitted_event,
+        flow.step_handler_instance(step).analytics_submitted_event,


we should probably add a ticket to revisit this when the flow state machine is removed

Good point. 👍🏻

svalexander

LGTM. There was a failing test ( didn't seem related to these changes though) but when I re-ran it all the specs passed.

zachmargolis · 2023-03-23T20:58:20Z

app/forms/idv/in_person/address_form.rb


      attr_accessor(*ATTRIBUTES)

+      attr_reader :capture_secondary_id_enabled


WDYT of removing this and making a nice ? method instead?

Suggested change

attr_reader :capture_secondary_id_enabled

def capture_secondary_id_enabled?

@capture_secondary_id_enabled

end

Alternatively, we use this pattern in a few places to achieve the same effect:

identity-idp/app/presenters/session_timeout_modal_presenter.rb

Lines 16 to 17 in fb439d8

attr_reader :user_fully_authenticated

alias_method :user_fully_authenticated?, :user_fully_authenticated

eileen-nava · 2023-03-24T14:57:28Z

app/services/flow/flow_state_machine.rb

      increment_step_name_counts
      analytics.public_send(
-        flow.step_handler(step).analytics_submitted_event,
+        flow.step_handler_instance(step).analytics_submitted_event,


Good point. 👍🏻

eileen-nava · 2023-03-24T15:18:45Z

spec/features/idv/analytics_spec.rb

    before do
      allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true)
-      allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(false)
+      allow(IdentityConfig.store).to receive(:in_person_capture_secondary_id_enabled).


Thank you for catching this! 🙏🏻

eileen-nava added 4 commits March 20, 2023 14:49

update address page to support DAV

06cf5e2

WIP need to bypass address form validation

76803ab

changelog: Upcoming Features, In-person proofing, update address page

25e7eaa

fix build

c5818cc

eileen-nava requested review from a team, kellular and racingspider and removed request for a team March 21, 2023 20:33

svalexander reviewed Mar 21, 2023

View reviewed changes

app/javascript/packs/puerto-rico-hint.ts Outdated Show resolved Hide resolved

svalexander reviewed Mar 21, 2023

View reviewed changes

NavaTim reviewed Mar 21, 2023

View reviewed changes

svalexander reviewed Mar 21, 2023

View reviewed changes

app/views/idv/in_person/address.html.erb Outdated Show resolved Hide resolved

zachmargolis reviewed Mar 21, 2023

View reviewed changes

app/javascript/packs/puerto-rico-hint.ts Outdated Show resolved Hide resolved

svalexander reviewed Mar 21, 2023

View reviewed changes

NavaTim mentioned this pull request Mar 22, 2023

LG-9006: Double Address Verification Feature Toggle #8045

Merged

14 tasks

kellular reviewed Mar 22, 2023

View reviewed changes

config/locales/in_person_proofing/fr.yml Show resolved Hide resolved

kellular approved these changes Mar 22, 2023

View reviewed changes

NavaTim added 5 commits March 22, 2023 15:06

Merge branch 'main' of https://github.com/18F/identity-idp into em/91…

7ebdf24

…39-dav-address-page

LG-9139: Remove Puerto Rico hint text code for other story

c3e93c7

LG-9139: Use enrollment-based feature flag; update tests

54bc83c

LG-9139: Update address line 2 French translation

99b3a86

LG-9139: Write test to verify same_address_as_id bug; fix bug

595a601

NavaTim requested review from kellular, svalexander and zachmargolis March 23, 2023 01:01

NavaTim added 2 commits March 22, 2023 18:47

LG-9139: Fix analytics spec

08f813f

Revert "LG-9139: Update address line 2 French translation"

9e09eca

This reverts commit 99b3a86.

svalexander reviewed Mar 23, 2023

View reviewed changes

svalexander approved these changes Mar 23, 2023

View reviewed changes

NavaTim requested a review from jmhooper March 23, 2023 18:37

zachmargolis reviewed Mar 23, 2023

View reviewed changes

eileen-nava commented Mar 24, 2023

View reviewed changes

refactor method that returns a boolean to end w/ ?

987ccdb

eileen-nava merged commit 7e7b99f into main Mar 24, 2023

eileen-nava deleted the em/9139-dav-address-page branch March 24, 2023 21:19

aduth mentioned this pull request Mar 27, 2023

Deploy RC 266 to Prod #8080

Merged


		attr_accessor(*ATTRIBUTES)

		attr_reader :capture_secondary_id_enabled

	attr_reader :user_fully_authenticated
	alias_method :user_fully_authenticated?, :user_fully_authenticated

Conversation

eileen-nava commented Mar 21, 2023 • edited by NavaTim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

High-level steps

Detailed testing steps

Feature Flag - Off

Feature Flag - On

👀 Screenshots

Uh oh!

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!

Uh oh!

Uh oh!

NavaTim commented Mar 21, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kellular left a comment

Choose a reason for hiding this comment

Uh oh!

svalexander Mar 23, 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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eileen-nava commented Mar 21, 2023 •

edited by NavaTim

Loading

svalexander Mar 23, 2023 •

edited

Loading