LG-8948: Swap order of form steps "Location" and "Prepare" by allthesignals · Pull Request #8125 · 18F/identity-idp

allthesignals · 2023-04-03T22:45:13Z

🎫 Ticket

🛠 Summary of changes

Before we showed the location selection step, then the "prepare" step.

This PR changes the order to show the "prepare" step first, then the location selection step.

📜 Testing Plan

Currently updating tests to reflect this new order

… “location” and “prepare”

app/javascript/packages/document-capture/components/in-person-prepare-step.tsx

This reverts commit a38d2d3.

app/javascript/packages/document-capture/components/in-person-prepare-step.tsx

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx

aduth · 2023-04-10T19:10:17Z

spec/support/features/in_person_helper.rb

    expect(page).to have_css(
      '.step-indicator__step',
      text: t('step_indicator.flows.idv.find_a_post_office'),
+      wait: 10,
    )


I'm wondering if we could move the wait somewhere a little more precise to the actual user interaction that requires an asynchronous delay. I notice we're also changing logic around click_spinner_button_and_wait. Ideally it'd be good if we could use that helper and avoid adding the wait to these other assertions, particularly on screens where we're not expecting a wait.

eileen-nava

I think this is coming along well. It looks like you still need to complete some AC for changes to the "verify page" and the "find a participating post office" page. Also, I thought Andrew had a good point about the usage of wait in feature tests.

eileen-nava · 2023-04-11T14:39:36Z

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx

            e.target.disabled = false;
            e.target.click();
+
+            e.preventDefault();


Do you think it would be helpful to add some comments to this area of the code?

# Conflicts: # app/javascript/packages/document-capture/components/in-person-call-to-action.tsx # app/javascript/packages/document-capture/components/in-person-prepare-step.tsx # spec/features/idv/in_person_spec.rb

NavaTim · 2023-04-21T23:51:24Z

spec/features/idv/analytics_spec.rb

-      'IdV: in person proofing location visited' => { flow_path: 'standard', in_person_cta_variant: 'in_person_variant_a' },
-      'IdV: in person proofing location submitted' => { flow_path: 'standard', selected_location: '606 E JUNEAU AVE, MILWAUKEE, WI, 53202-9998', in_person_cta_variant: 'in_person_variant_a' },
      'IdV: in person proofing prepare visited' => { flow_path: 'standard' },
      'IdV: in person proofing prepare submitted' => { flow_path: 'standard' },
+      'IdV: in person proofing location visited' => hash_including(flow_path: 'standard'),
+      'IdV: in person proofing location submitted' => { flow_path: 'standard', selected_location: '606 E JUNEAU AVE, MILWAUKEE, WI, 53202-9998', in_person_cta_variant: 'in_person_variant_a' },


After merging in origin/main, this test appears to fail by skipping the search page altogether.

NavaTim · 2023-04-21T23:54:34Z

spec/features/idv/in_person_spec.rb

-
-      expect(page).to have_content(
-        t('in_person_proofing.headings.prepare'), wait: 5
-      )


This test appears to be failing because there is a delay in switching to the address page.

@NavaTim from what I can tell, this test is failing because capture_secondary_id_enabled is being set to true in the state_id_step, which is causing all of the additional fields that are normally hidden behind that flag to be displayed.

State ID step: capture secondary id enabled: true
DAV in the test: false -> this is a default value
Same address in the test: true -> this is a default value

It seems that setting the config value is toggling the value in the User enrollment. I'm not sure how or why this is happening.

The enrollment's value is determined when it's created based on the config value at that time. If you intend to test with DAV disabled, then you need to ensure it's disabled at the time that the location step is completed for that enrollment. This is by design.

That said, based on prior observations I expected that this test might be failing instead because the unload prevention dialog wasn't properly disabled before redirecting from the prepare page.

app/javascript/packages/document-capture/components/document-capture.tsx

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx

NavaTim · 2023-04-24T18:39:41Z

app/javascript/packages/document-capture/components/in-person-prepare-step.tsx


    if (!isSubmitting) {
      setIsSubmitting(true);
-      removeUnloadProtection();


You may need to add this function call to the location step.

…948-move-search

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx

JackRyan1989 · 2023-04-28T21:10:05Z

Closing in favor of #8308

allthesignals added 2 commits April 3, 2023 18:42

changelog: Improvements, In-Person Proofing, Swap order of form steps…

86e6a7d

… “location” and “prepare”

useHistoryParam

a38d2d3

allthesignals force-pushed the 8948-move-search branch from 4656702 to a38d2d3 Compare April 3, 2023 23:40

JackRyan1989 reviewed Apr 4, 2023

View reviewed changes

app/javascript/packages/document-capture/components/in-person-prepare-step.tsx Outdated Show resolved Hide resolved

allthesignals added 2 commits April 4, 2023 09:57

Revert "useHistoryParam"

171b173

This reverts commit a38d2d3.

Update back button test

ed58cfe

allthesignals marked this pull request as ready for review April 5, 2023 15:53

allthesignals added 5 commits April 5, 2023 12:01

Update analytics context

1cd5c26

Swap steps; add wait

c3cf9a9

Lintfix

812c2b6

Swap test logic

b131d4e

Update mobile doc capture test

3a67874

aduth reviewed Apr 10, 2023

View reviewed changes

app/javascript/packages/document-capture/components/in-person-prepare-step.tsx Outdated Show resolved Hide resolved

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx Outdated Show resolved Hide resolved

aduth reviewed Apr 10, 2023

View reviewed changes

eileen-nava reviewed Apr 11, 2023

View reviewed changes

allthesignals added 2 commits April 11, 2023 12:33

Merge branch 'main' into 8948-move-search

e8dab19

# Conflicts: # app/javascript/packages/document-capture/components/in-person-call-to-action.tsx # app/javascript/packages/document-capture/components/in-person-prepare-step.tsx # spec/features/idv/in_person_spec.rb

Fix a few more tests

5dc5e79

allthesignals force-pushed the 8948-move-search branch from 4218150 to 5dc5e79 Compare April 12, 2023 19:17

allthesignals added 6 commits April 14, 2023 14:08

Update verify step spec

2d04f31

JS tests pass

03ce743

Bump location in test sequence

4d52c85

Merge branch 'main' into 8948-move-search

1a422f8

Debug test behavior

c7fe5c2

Merge branch 'main' into 8948-move-search

6028749

allthesignals closed this Apr 18, 2023

allthesignals reopened this Apr 18, 2023

Tomas Apodaca added 4 commits April 18, 2023 09:39

Merge remote-tracking branch 'origin/main' into 8948-move-search

8dd176d

revert changes from merge conflict resolution

28e68b2

Merge remote-tracking branch 'origin/main' into 8948-move-search

7c03a81

navigate by setting step name instead of href

a272d0b

Tomas Apodaca added 4 commits April 21, 2023 10:18

log variant, don't track event, move href set

4b2a7e4

use Button, not SpinnerButton

7619a0e

move expectations inside helper functions

0b61b8e

properly use onContinue in the prepare step

ab477db

NavaTim reviewed Apr 21, 2023

View reviewed changes

NavaTim reviewed Apr 24, 2023

View reviewed changes

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx Outdated Show resolved Hide resolved

NavaTim reviewed Apr 24, 2023

View reviewed changes

Tomas Apodaca and others added 8 commits April 24, 2023 13:29

onContinue doesn't need to be async

0ee24b9

Merge remote-tracking branch 'origin/main' into 8948-move-search

a288ff1

View and content changes along with passing analytics test

672d873

Merge branch 'main' of github.com:18F/identity-idp into 8948-move-search

8579096

super small js refactor and in person spec

663ccfb

Merge branch 'main' into 8948-move-search

bb2ea49

put redirect in correct location and fix test

e4f02ae

Merge branch '8948-move-search' of github.com:18F/identity-idp into 8…

44d7b6d

…948-move-search

NavaTim reviewed Apr 27, 2023

View reviewed changes

...vascript/packages/document-capture/components/in-person-location-post-office-search-step.tsx Outdated Show resolved Hide resolved

JackRyan1989 closed this Apr 28, 2023

tomas-nava deleted the 8948-move-search branch May 4, 2023 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-8948: Swap order of form steps "Location" and "Prepare"#8125

LG-8948: Swap order of form steps "Location" and "Prepare"#8125
allthesignals wants to merge 33 commits intomainfrom
8948-move-search

allthesignals commented Apr 3, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth Apr 10, 2023

Uh oh!

eileen-nava left a comment

Uh oh!

eileen-nava Apr 11, 2023

Uh oh!

NavaTim Apr 21, 2023

Uh oh!

NavaTim Apr 21, 2023

Uh oh!

JackRyan1989 Apr 26, 2023

Uh oh!

NavaTim Apr 27, 2023

Uh oh!

Uh oh!

Uh oh!

NavaTim Apr 24, 2023

Uh oh!

Uh oh!

JackRyan1989 commented Apr 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

allthesignals commented Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth Apr 10, 2023

Choose a reason for hiding this comment

Uh oh!

eileen-nava left a comment

Choose a reason for hiding this comment

Uh oh!

eileen-nava Apr 11, 2023

Choose a reason for hiding this comment

Uh oh!

NavaTim Apr 21, 2023

Choose a reason for hiding this comment

Uh oh!

NavaTim Apr 21, 2023

Choose a reason for hiding this comment

Uh oh!

JackRyan1989 Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

NavaTim Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NavaTim Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JackRyan1989 commented Apr 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

allthesignals commented Apr 3, 2023 •

edited

Loading