-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Make OTP template more contextual #7434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@vermakhushboo the PR looks good at first glance. But I dont think it addresses the issues mentioned. The issues mentioned talk about being able to customise the sms message which we plan to do in a separate feature. |
@christyjacob4 I agree, but the PR I have created is for the task that was assigned to me. I think @stnguyen90 added related issues to it. Steven, can we create new tasks for those and assign separately? |
@@ -36,6 +36,7 @@ | |||
"emails.certificate.footer": "Your previous certificate will be valid for 30 days since the first failure. We highly recommend investigating this case, otherwise your domain will end up without a valid SSL communication.", | |||
"emails.certificate.thanks": "Thanks", | |||
"emails.certificate.signature": "{{project}} team", | |||
"sms.verification.body": "{{secret}} is your {{project}} verification code.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think its important for the message to be structured as Your [PROJECT_NAME] verification code is: [CODE]
This will allow the phone operating systems to detect and suggest codes in apps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christyjacob4, @Meldiron and I researched a bit into how other companies do it and we found that most of them follow this structure where code is in the beginning of the message. This would help in 2 things:
- The user can easily read the OTP from the notification without having to open the message
- If the project name is too large, it would be hard to read the OTP/ the message may exceed allowed characters of one message.
We can finalise the approach and I'll make the changes accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I am fine with either messages as long as its detectable by the OS. Can we do a small test to trigger an SMS and check if the phone recognizes it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments.
What does this PR do?
Add more details to the OTP message and generate translations for all languages
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist