Skip to content

LG-8084 Hybrid Handoff SMS#7499

Merged
theabrad merged 11 commits intomainfrom
abrad-lg-8084-hybrid-sms
Dec 16, 2022
Merged

LG-8084 Hybrid Handoff SMS#7499
theabrad merged 11 commits intomainfrom
abrad-lg-8084-hybrid-sms

Conversation

@theabrad
Copy link
Contributor

🛠 Summary of changes

Updated Hybrid Handoff SMS to new text which includes which Service Provider the user is verifying from.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Sign up for an account.
  • Go through IdV flow
  • At the verify id step select 'Use your phone'
  • Enter phone number and check the text message that was received.

👀 Screenshots

New SMS when verifying from Login.gov

Screen Shot 2022-12-15 at 1 54 44 PM

New SMS when verifying from a Service Provider

Screen Shot 2022-12-15 at 2 47 46 PM

@theabrad theabrad requested a review from a team December 15, 2022 19:51
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.

LGTM 👍

end

def sp_name
service_provider ? service_provider&.friendly_name : APP_NAME
Copy link
Contributor

@aduth aduth Dec 16, 2022

Choose a reason for hiding this comment

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

We should be able to access current_sp here (source), as an alternative to implementing the other new helper method.

Suggested change
service_provider ? service_provider&.friendly_name : APP_NAME
current_sp&.friendly_name || APP_NAME

doc_auth_link: "%{link} You've requested to verify your identity on a mobile
phone. Please take a photo of your state issued ID."
doc_auth_link: |-
%{app_name}: %{link} You're verifying your identity to access %{service_provider}. Take a photo of your ID to continue.
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen a few other takes on naming the interpolated value to clarify that APP_NAME is possibly inserted, like sp_or_app_name or service_provider_or_app_name. Do you think it'd be worth doing something similar here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Good suggestion!

@theabrad theabrad merged commit 6c5d13c into main Dec 16, 2022
@theabrad theabrad deleted the abrad-lg-8084-hybrid-sms branch December 16, 2022 20:04
@aduth aduth mentioned this pull request Dec 22, 2022
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.

2 participants