Skip to content
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

feat: use template/text instead of strings.Replace for phone OTP messages #1188

Merged
merged 12 commits into from
Aug 3, 2023

Conversation

J0
Copy link
Contributor

@J0 J0 commented Jul 20, 2023

What kind of change does this PR introduce?

As per microtask, refactors the way in which we generate the sms message template.

Cautionary note: if merged, an error will be thrown if a Key other than Code is placed into the SMS template (e.g. Your code is {{ .Code}} and {{ .SomeOtherKey}}) and an attempt to send SMS is made. While users hopefully do not have such templates we will need to update the FE input to guard agains this

Upd: We did a quick check and found no such entries on the hosted platform

@J0 J0 marked this pull request as ready for review July 20, 2023 20:34
@J0 J0 requested a review from a team as a code owner July 20, 2023 20:34
internal/api/phone.go Outdated Show resolved Hide resolved
internal/api/phone.go Outdated Show resolved Hide resolved
@hf hf changed the title refactor: swap strings.Replace to text/template feat: use template/text instead of strings.Replace for phone OTP messages Jul 25, 2023
internal/conf/configuration.go Outdated Show resolved Hide resolved
@J0 J0 requested a review from hf August 2, 2023 17:02
internal/api/phone.go Outdated Show resolved Hide resolved
@J0 J0 merged commit 5caacc1 into master Aug 3, 2023
1 check passed
@J0 J0 deleted the j0/swap_sms_library_to_text_template branch August 3, 2023 11:47
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

🎉 This PR is included in version 2.91.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants