-
Notifications
You must be signed in to change notification settings - Fork 18
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
Use the shortcode pool for Notify SMS #2213
Conversation
if template_id is not None and str(template_id) in self.current_app.config["AWS_PINPOINT_SC_TEMPLATE_IDS"]: | ||
use_shortcode_pool = ( | ||
str(template_id) in self.current_app.config["AWS_PINPOINT_SC_TEMPLATE_IDS"] | ||
or str(service_id) == self.current_app.config["NOTIFY_SERVICE_ID"] |
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.
Why don't we just put the notify service id in the AWS_PINPOINT_SC_TEMPLATE_IDS?
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.
We could but we plan to move away from this env var entirely and have the pool choice be more dynamic. The NOTIFY_SERVICE_ID
variable already exists so this way we don't need to change AWS_PINPOINT_SC_TEMPLATE_IDS
and can eventually remove 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.
LGTM
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.
LGTM!
Summary | Résumé
Use the shortcode pool for GC Notify's SMS.
Related Issues | Cartes liées
Test instructions | Instructions pour tester la modification
Send SMS from the GC Notify service. Should be from the shortcode pool.
Release Instructions | Instructions pour le déploiement
None.
Reviewer checklist | Liste de vérification du réviseur