Skip to content

LG-8577: Prefill user's mobile number for hybrid docauth#7762

Merged
matthinz merged 5 commits intomainfrom
matthinz/8577-prefill-mobile
Feb 3, 2023
Merged

LG-8577: Prefill user's mobile number for hybrid docauth#7762
matthinz merged 5 commits intomainfrom
matthinz/8577-prefill-mobile

Conversation

@matthinz
Copy link
Contributor

@matthinz matthinz commented Feb 3, 2023

If the user has a mobile number associated with their account, prefill that when showing the handoff screen for hybrid docauth.

🎫 Ticket

LG-8577

🛠 Summary of changes

If the user has a phone number associated with their account (from 2fa), use that to prefill the phone number field on the doc auth hybrid handoff screen.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Create an account and use SMS as your 2nd factor
  • Start IDV and click on the big "Use your phone" button on the Verify your ID step.
  • Verify your 2fa phone number shows up as the default value

👀 Screenshots

screenshot-en

If the user has a mobile number associated with their account, prefill that when showing the handoff screen for hybrid docauth.

changelog: User-Facing Improvements, Identity Verification, Prefill mobile number for hybrid doc auth
@matthinz matthinz requested a review from a team February 3, 2023 00:18
Comment on lines +31 to +35
def extra_view_variables
{
idv_phone_form: create_form,
}
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels kind of yucky, but I think is the way to do this until we pull the hybrid flow out of the FSM?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think if we were invested in improving FSM, it could be nice to somehow provide a form object through the FSM machinery, but it seems like we're moving away from that, and this is the otherwise standard way to make values available to the view, so seems fine to me.

@aduth
Copy link
Contributor

aduth commented Feb 3, 2023

In testing, I noticed that this didn't prefill anything for me when I have two phones configured to my account. Is that expected?

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love using the form object this way! I've been wishing we'd do more of this.

Comment on lines +31 to +35
def extra_view_variables
{
idv_phone_form: create_form,
}
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think if we were invested in improving FSM, it could be nice to somehow provide a form object through the FSM machinery, but it seems like we're moving away from that, and this is the otherwise standard way to make values available to the view, so seems fine to me.

Comment on lines +74 to +76
{
idv_phone_form: be_an_instance_of(Idv::PhoneForm),
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the curly braces could be optional, though I don't have a strong preference in case you think they help readability.

Suggested change
{
idv_phone_form: be_an_instance_of(Idv::PhoneForm),
},
idv_phone_form: be_an_instance_of(Idv::PhoneForm),

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

<p><%= t('doc_auth.instructions.send_sms') %></p>
<%= simple_form_for(
:doc_auth,
idv_phone_form,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to check: the only reason we're building the form is so we can pass it something that responds to #phone right? Could we pass a different object or like a simple PORO/Struct that has a #phone method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I based this usage on Idv::PhoneController which also uses Idv::PhoneForm. The form has some logic already built into it to pre-fill the user's MFA phone number.

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@matthinz
Copy link
Contributor Author

matthinz commented Feb 3, 2023

In testing, I noticed that this didn't prefill anything for me when I have two phones configured to my account. Is that expected?

@aduth Yeah, it looks like this has been this form's behavior for a couple years now. I'm not sure why that would be?

@matthinz matthinz merged commit 416e22b into main Feb 3, 2023
@matthinz matthinz deleted the matthinz/8577-prefill-mobile branch February 3, 2023 19:40
@aduth aduth mentioned this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants