LG-10530: Improve Verify by Mail controller & route names by matthinz · Pull Request #9136 · 18F/identity-idp

matthinz · 2023-09-01T23:58:37Z

🎫 Ticket

🛠 Summary of changes

This PR is the first in a series that will update GPO-related routes, analytics events, and controllers to better reflect what they do.

In this one, we:

Add new routes where we eventually want the "real" GPO routes to point (e.g. /verify/by_mail/request_letter)
Rename GPO-related analytics events to be more consistent
Rename and move GPO-related controllers into Idv::ByMail
Update naming of GPO-related url & path helpers

A future PR will finalize moving GPO routes to the new locations and add redirects from the old paths to the new (to support the 50/50 state during deploy).

Routes

Old	New
`/verify/usps`	`/verify/by_mail/request_letter`
`/verify/come_back_later`	`/verify/by_mail/letter_enqueued`
`/verify/by_mail`	`/verify/by_mail/enter_code`

Analytics events

Old	New
`IdV: USPS address visited`	`IdV: request letter visited`
`IdV: come back later visited`	`IdV: letter enqueued visited`
`IdV: GPO verification visited`	`IdV: enter verify by mail code visited`
`IdV: GPO verification submitted`	`IdV: enter verify by mail code submitted`

Controllers

Old	New
`Idv::ComeBackLaterController`	`Idv::ByMail::LetterEnqueuedController`
`Idv::GpoController`	`Idv::ByMail::RequestLetterController`
`Idv::GpoVerifyController`	`Idv::ByMail::EnterCodeController`

Helpers

Old	New
`idv_gpo_url`	`idv_request_letter_url`
`idv_come_back_later_url`	`idv_letter_enqueued_url`
`idv_gpo_verify_url`	`idv_verify_by_mail_enter_code_url`

soniaconnolly

It definitely reads better this way! Suggestion for one more thing to rename.

app/controllers/idv/by_mail/request_letter_controller.rb

config/routes.rb

app/services/analytics_events.rb

spec/controllers/idv/by_mail/request_letter_controller_spec.rb

app/controllers/concerns/idv_step_concern.rb

config/routes.rb

[skip changelog]

Match structure in the gpo_verification_enabled? check

Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>

- Add handlers at the new route locations (for the GET routes--the others will PUT/POST to the old paths still) - Don't do any 302 redirecting yet. Actual route changes will come in a future deploy

soniaconnolly · 2023-09-08T19:37:20Z

config/routes.rb

+      get '/by_mail/request_letter' => 'by_mail/request_letter#index'
+      get '/by_mail/letter_enqueued' => 'by_mail/letter_enqueued#show'
+      get '/by_mail/enter_code' => 'by_mail/enter_code#index'


I think you need the put routes here too.

The actual forms all set their action attributes at the old path names (for example, this one) , so no PUT routes are needed here. We WILL need to do a second set of PUT routes when we ultimately rename the routes, but that'll be a different PR

solipet · 2023-09-08T21:13:26Z

Are you going to update the dashboard (cloudwatch-idv-letter-flow.tf) that uses the old event names?

matthinz · 2023-09-08T22:01:47Z

@solipet Yeah I'll do that as part of this ticket. I believe there are a couple of dashboards to update

soniaconnolly

LGTM! Thanks for all this work!

We renamed the code that is used to verify by mail in #9136. This replaced "GPO" with more descriptive names. This commit removes a few lingering references to GPO. [skip changelog]

matthinz added the status - work in progress label Sep 1, 2023

matthinz force-pushed the matthinz/10530-nobody-knows-what-gpo-is branch 3 times, most recently from 214fd31 to 97afce2 Compare September 6, 2023 16:45

matthinz removed the status - work in progress label Sep 6, 2023

matthinz force-pushed the matthinz/10530-nobody-knows-what-gpo-is branch from f4d6aa2 to 7b4d60a Compare September 6, 2023 20:58

matthinz changed the title ~~WIP: LG-10530: Improve Verify by Mail controller & route names~~ LG-10530: Improve Verify by Mail controller & route names Sep 6, 2023

matthinz marked this pull request as ready for review September 6, 2023 22:13

matthinz requested a review from a team September 6, 2023 22:13

soniaconnolly reviewed Sep 6, 2023

View reviewed changes

app/controllers/idv/by_mail/request_letter_controller.rb Outdated Show resolved Hide resolved

config/routes.rb Outdated Show resolved Hide resolved

soniaconnolly reviewed Sep 6, 2023

View reviewed changes

app/services/analytics_events.rb Outdated Show resolved Hide resolved

spec/controllers/idv/by_mail/request_letter_controller_spec.rb Outdated Show resolved Hide resolved

matthinz force-pushed the matthinz/10530-nobody-knows-what-gpo-is branch from 0f1202d to 7282981 Compare September 7, 2023 16:49

soniaconnolly reviewed Sep 8, 2023

View reviewed changes

app/controllers/concerns/idv_step_concern.rb Outdated Show resolved Hide resolved

config/routes.rb Outdated Show resolved Hide resolved

aduth mentioned this pull request Sep 8, 2023

LG-10681: Remove ArcGIS API Usage #9154

Merged

2 tasks

matthinz added 16 commits September 8, 2023 12:18

Move GPO controllers into by_mail and rename

5f905aa

[skip changelog]

Move GPO controller specs into by_mail and rename

4cadcbe

Get RequestLetterController spec passing

e7b66ad

Get LetterEnqueuedController spec passing

55162c8

Get EnterCodeController spec passing

14cc998

come_back_later -> letter_enqueued

f411f8d

Update view specs

f8341f8

Analytics event naming

94ee59a

Rename some routes and add temporary redirects

7a261bc

'come back later visited' -> 'letter enqueued visited'

7475623

idv_letter_enqueued -> idv_gpo_letter_enqueued

621061a

/by_mail -> /by_mail/enter_code

25d1ca5

Fix GpoPresenter spec

789a2be

'USPS address visited' -> 'gpo request letter visited'

c03f7f9

letter enqueued visited -> gpo letter enqueued visited'

1fdca1f

IdV: GPO verification visited -> IdV: gpo enter code visited

5c1064e

matthinz and others added 14 commits September 8, 2023 12:18

'GPO verification submitted' -> 'enter verify by mail code submitted'

91b2f41

Remove GPO from a couple of analytics event names

e8b8ceb

Clarify temporary redirects

12c97dd

Match structure in the gpo_verification_enabled? check

Update app/services/analytics_events.rb

6e11451

Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>

idv_gpo_url -> idv_request_letter_url

969874c

idv_gpo_letter_enqueued_url -> idv_letter_enqueued_url

47224a7

idv_gpo_verify_url -> idv_enter_verify_by_mail_code_path

59b1f2b

idv_gpo_url fixup

f65218c

fixup idv_gpo_verify_url

8a75ad6

Idv::GpoPresenter -> Idv::ByMail::RequestLetterPresenter

49f82b4

A couple of analytics methods

198616a

Fix test

93c3216

idv_enter_verify_by_mail_code_url -> idv_verify_by_mail_enter_code_url

7301fa3

Don't change routes yet

5d2f930

- Add handlers at the new route locations (for the GET routes--the others will PUT/POST to the old paths still) - Don't do any 302 redirecting yet. Actual route changes will come in a future deploy

matthinz force-pushed the matthinz/10530-nobody-knows-what-gpo-is branch from 2447276 to 5d2f930 Compare September 8, 2023 19:18

soniaconnolly reviewed Sep 8, 2023

View reviewed changes

Fix presenter spec

8602e6b

matthinz added 2 commits September 8, 2023 15:41

Update analytics method name

d10f004

Add PUT/POST routes for new GPO locations

562373e

soniaconnolly approved these changes Sep 8, 2023

View reviewed changes

matthinz mentioned this pull request Sep 8, 2023

LG-10530: Officially turn on new GPO routes #9184

Merged

matthinz merged commit a24506e into main Sep 8, 2023

matthinz deleted the matthinz/10530-nobody-knows-what-gpo-is branch September 8, 2023 23:09

aduth mentioned this pull request Sep 11, 2023

Deploy RC 313 to Prod #9188

Merged

matthinz mentioned this pull request Sep 15, 2023

LG-10530: Add redirects for select old GPO routes #9220

Merged

jmhooper mentioned this pull request Oct 13, 2023

Remove trailing references to GPO in IdV #9378

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-10530: Improve Verify by Mail controller & route names#9136

LG-10530: Improve Verify by Mail controller & route names#9136
matthinz merged 34 commits intomainfrom
matthinz/10530-nobody-knows-what-gpo-is

matthinz commented Sep 1, 2023 •

edited

Loading

Uh oh!

soniaconnolly left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

soniaconnolly Sep 8, 2023

Uh oh!

matthinz Sep 8, 2023

Uh oh!

solipet commented Sep 8, 2023

Uh oh!

matthinz commented Sep 8, 2023 •

edited

Loading

Uh oh!

soniaconnolly left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

matthinz commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

Routes

Analytics events

Controllers

Helpers

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

soniaconnolly Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

matthinz Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

solipet commented Sep 8, 2023

Uh oh!

matthinz commented Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matthinz commented Sep 1, 2023 •

edited

Loading

matthinz commented Sep 8, 2023 •

edited

Loading