LG-10465 Add update route for address controller #9489

svalexander · 2023-10-31T16:55:19Z

🎫 Ticket

🛠 Summary of changes

Add update route for the new address controller. Also add more before actions to ensure user who manually changes url is routed to the correct page.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

svalexander · 2023-10-31T16:59:30Z

app/controllers/idv/in_person/address_controller.rb

+
+        analytics.idv_in_person_proofing_residential_address_submitted(**form_result.to_h)
+
+        if updating_address?


@soniaconnolly since I'm doing this check here and redirecting to verify_info do I still need the check mentioned in this comment?

You'll have to test it out to make sure, but I believe that you do, because the before_action will redirect away from the Ssn step if the user already has an address in the session, the way they do when they're editing from VerifyInfo. It looks like you did add it below.

svalexander · 2023-10-31T17:03:30Z

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

If i manually change the url to in_person/state_id when on the in_person_proofing/address page it redirects to in_person/address. Any thoughts on how I might fix that @soniaconnolly ?

Re: redirecting to the FSM step from the previous step, let's make a failing spec and look at the redirects and see if we can figure this out. (I think we started on that.)

soniaconnolly · 2023-10-31T18:09:27Z

app/controllers/idv/in_person/address_controller.rb

+
+        analytics.idv_in_person_proofing_residential_address_submitted(**form_result.to_h)
+
+        if updating_address?


You'll have to test it out to make sure, but I believe that you do, because the before_action will redirect away from the Ssn step if the user already has an address in the session, the way they do when they're editing from VerifyInfo. It looks like you did add it below.

soniaconnolly · 2023-10-31T18:10:28Z

app/controllers/idv/in_person/address_controller.rb

-        redirect_to idv_in_person_ssn_url
+        return if request.referer == idv_in_person_verify_info_url
+        redirect_to idv_in_person_verify_info_url


The referer check looks good (although I've been discovering lately that relying on the referer is not ideal). Why are you redirecting to verify_info instead of ssn at the end?

i'm using confirm_ssn_step_needed instead to redirect to ssn. Line 99 will redirect us to verify info in case a user changes the url to in_person_proofing/address after having completed all the steps.

soniaconnolly · 2023-10-31T18:11:17Z

app/controllers/idv/in_person/address_controller.rb

+      def confirm_ssn_step_needed
+        if pii_from_user&.has_key?(:address1) && !idv_session.ssn
+          redirect_to idv_in_person_ssn_url
+        end


Why do you need this check here? I would think the SsnController would do this check if needed.

i added it in to handle someone manually entering in_person_proofing/address on the ssn pg

soniaconnolly · 2023-10-31T18:14:07Z

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

Re: redirecting to the FSM step from the previous step, let's make a failing spec and look at the redirects and see if we can figure this out. (I think we started on that.)

spec/controllers/idv/in_person/address_controller_spec.rb

svalexander · 2023-11-07T21:31:07Z

FormValidation is not working for this page on input fields (other than zipcode)

n1zyy

This all looks like good stuff to me! I left a couple small questions/suggestions below, but certainly nothing blocking.

I think I would prefer to let another teammate approve this, just because I'm new to this flow and it's probably best if someone with more familiarity with it could give this a once-over.

app/controllers/idv/in_person/address_controller.rb

spec/controllers/idv/in_person/address_controller_spec.rb

app/controllers/idv/in_person/address_controller.rb

spec/features/idv/steps/in_person/address_spec.rb

gina-yamada · 2023-11-15T19:55:14Z

@svalexander Everything I have tested is working as I'd expect except for the back button. When I click back I see this page and then get directed to phone. (Cancel- initial page visit is working as I'd expect)

✅ UI Is accurate for update (link/button/header)
✅ Form validation is working for initial submission on also on update (if I clear out fields and submit I can not proceed)
✅ Form submission takes me to Verify on submission (update)
✅ Clicking Update on /verify/in_person/verify_info for Current residential address takes me to new non-FSM controller at /verify/in_person_proofing/address
❌ Back button is not redirecting back to the Verify view at /verify/in_person/verify_info
✅ Redirects correctly when I edit state id address and change same as id from false to true- date also updated, same updates and redirects correctly when I edit residential address (after originally picking that is was the same)- address updates and the radio button on state id address now says they are different
✅ When in_person_full_address_entry_enabled is set to false, existing behavior does not appear to be affected- tested many flows, also observe a 404 when I try to visit non-fsm controller at url /verify/in_person_proofing/address

app/controllers/idv/in_person/address_controller.rb

spec/controllers/idv/in_person/address_controller_spec.rb

gina-yamada · 2023-11-15T22:43:08Z

Worked through code changes & tests with Shannon and Sonia. Back button is now working! Shannon and Sonia are still working through some changes so not going to approve at this time.

soniaconnolly

We reviewed this together in detail along with @gina-yamada, looks great!

zachmargolis · 2023-11-16T00:32:56Z

👋 hi this PR didn't have have a changelog entry in the commit message after it was merged, please make sure to add one next time

svalexander · 2023-11-16T15:45:40Z

👋 hi this PR didn't have have a changelog entry in the commit message after it was merged, please make sure to add one next time

@zachmargolis My mistake. I didn't get a ci/cd error due to the changelog to catch the mistake. How do I make sure the changelog message is present for this pr for the next deploy?

zachmargolis · 2023-11-16T16:55:24Z

@zachmargolis My mistake. I didn't get a ci/cd error due to the changelog to catch the mistake. How do I make sure the changelog message is present for this pr for the next deploy?

When you click "Squash and Merge" just make sure to preserve the changelog: ... line in the commit message

svalexander · 2023-11-16T17:54:17Z

When you click "Squash and Merge" just make sure to preserve the changelog: ... line in the commit message

Looking back at my commit history it looks like i never added a changelog message :-( .

svalexander added 3 commits October 30, 2023 17:48

add update route

8177e14

redirect update btn on verify info pg to controller address pg

eacdf1e

add more before actions to handle routing

c65e1b9

svalexander marked this pull request as draft October 31, 2023 16:55

svalexander commented Oct 31, 2023

View reviewed changes

soniaconnolly reviewed Oct 31, 2023

View reviewed changes

svalexander added 3 commits November 1, 2023 16:54

mark fsm step completed in controller

0e8b4c4

update address controller spec with update tests

d3313c7

update address controller and spec

2d61975

svalexander commented Nov 6, 2023

View reviewed changes

spec/controllers/idv/in_person/address_controller_spec.rb Outdated Show resolved Hide resolved

svalexander commented Nov 6, 2023

View reviewed changes

spec/controllers/idv/in_person/address_controller_spec.rb Outdated Show resolved Hide resolved

svalexander added 2 commits November 6, 2023 16:59

update before action and specs

e38391f

update state id step to fix spec

c80eeda

svalexander added 4 commits November 8, 2023 14:53

allow error to be shown on pg

4a1eba8

add submitted values on success

bd20403

add invalid submission spec

92166f6

update in person spec

96cee93

svalexander changed the title ~~Shannon/lg 10465 update route address~~ LG-10465 Add update route for address controller Nov 9, 2023

svalexander marked this pull request as ready for review November 9, 2023 22:14

svalexander added 2 commits November 9, 2023 17:32

lint fix

6856d27

lint fix

98a5e15

svalexander requested review from a team and gina-yamada November 13, 2023 15:24

n1zyy reviewed Nov 13, 2023

View reviewed changes

svalexander added 2 commits November 13, 2023 12:45

update address controller spec

b097f24

updates based on feedback

012d607

soniaconnolly reviewed Nov 15, 2023

View reviewed changes

svalexander added 7 commits November 15, 2023 15:58

remove before action for ssn step needed and mark address step complete

b1c8d92

fix back link issue

b27db10

remove unused consts

fc4abf9

remove address controller flag from in person spec

8fa74d7

Merge branch 'main' into shannon/lg-10465-update-route-address

b782fde

getting flow path from idv_session for analytics

88c9727

clean up address controller spec

92830d9

update verify info controller spec to get flow path from idv sess

7f3d7e9

soniaconnolly approved these changes Nov 15, 2023

View reviewed changes

svalexander merged commit 8600e4c into main Nov 15, 2023
2 checks passed

svalexander deleted the shannon/lg-10465-update-route-address branch November 15, 2023 22:56

mitchellhenke restored the shannon/lg-10465-update-route-address branch November 16, 2023 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-10465 Add update route for address controller #9489

LG-10465 Add update route for address controller #9489

svalexander commented Oct 31, 2023 •

edited

Loading

svalexander Oct 31, 2023

soniaconnolly Oct 31, 2023

svalexander Oct 31, 2023

soniaconnolly Oct 31, 2023 •

edited

Loading

soniaconnolly Oct 31, 2023

soniaconnolly Oct 31, 2023

svalexander Oct 31, 2023

soniaconnolly Oct 31, 2023

svalexander Oct 31, 2023

soniaconnolly Oct 31, 2023 •

edited

Loading

svalexander commented Nov 7, 2023

n1zyy left a comment

gina-yamada commented Nov 15, 2023 •

edited

Loading

gina-yamada commented Nov 15, 2023 •

edited

Loading

soniaconnolly left a comment

zachmargolis commented Nov 16, 2023

svalexander commented Nov 16, 2023

zachmargolis commented Nov 16, 2023

svalexander commented Nov 16, 2023 •

edited

Loading


		analytics.idv_in_person_proofing_residential_address_submitted(**form_result.to_h)

		if updating_address?

LG-10465 Add update route for address controller #9489

LG-10465 Add update route for address controller #9489

Conversation

svalexander commented Oct 31, 2023 • edited Loading

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soniaconnolly Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soniaconnolly Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

svalexander commented Nov 7, 2023

n1zyy left a comment

Choose a reason for hiding this comment

gina-yamada commented Nov 15, 2023 • edited Loading

gina-yamada commented Nov 15, 2023 • edited Loading

soniaconnolly left a comment

Choose a reason for hiding this comment

zachmargolis commented Nov 16, 2023

svalexander commented Nov 16, 2023

zachmargolis commented Nov 16, 2023

svalexander commented Nov 16, 2023 • edited Loading

svalexander commented Oct 31, 2023 •

edited

Loading

soniaconnolly Oct 31, 2023 •

edited

Loading

soniaconnolly Oct 31, 2023 •

edited

Loading

gina-yamada commented Nov 15, 2023 •

edited

Loading

gina-yamada commented Nov 15, 2023 •

edited

Loading

svalexander commented Nov 16, 2023 •

edited

Loading