Fixes SMS not sending for country codes not supporting alphanumeric sender IDs#5969
Fixes SMS not sending for country codes not supporting alphanumeric sender IDs#5969CarinaWolli merged 12 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| ? reminder.booking?.attendees[0].timeZone | ||
| : reminder.booking?.user?.timeZone; | ||
|
|
||
| const isAlphanumericSenderIdSupported = !noAlphanumericSenderIdSupport.find( |
There was a problem hiding this comment.
If country code is not in list, then the alphanumeric sender id is supported
| @@ -0,0 +1,75 @@ | |||
| export const noAlphanumericSenderIdSupport = [ | |||
There was a problem hiding this comment.
List of country codes that don't support alphanumeric sender IDs:
Supported Countries
Country codes from here: Country Codes
There was a problem hiding this comment.
do we throw an error if the country is not supported?
There was a problem hiding this comment.
@PeerRich We don't throw an error, but if alphanumeric sender ids are not supported SMS are still sent with our twilio phone number (instead of the sender id)
pumfleet
left a comment
pumfleet
left a comment
There was a problem hiding this comment.
Great PR! I've left a couple of comments about a file name (and subsequently it's imports) being misspelt as well as a hardcoded phone number, but otherwise all good :)
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
- const isAlphanumericSenderIdSupported = !noAlphanumericSenderIdSupport.find(
- (code) => code === reminderPhone?.substring(0, code.length)
- );
- const senderID = isAlphanumericSenderIdSupported ? sender : "";
+ getSenderId(reminderPhone)
// or
+ getSenderId(sendTo) // returns process.env.TWILIO_SENDER_ID = "Cal" || ""getSenderIdShould be the function exposed through the library - this way you can reduce the duplicate code.- The SENDER_ID should be an environment variable (optional) - defaulting to Cal - allowing self hosters to easily update the SENDER_ID. Perhaps should be non-twilio-specific.
| @@ -0,0 +1,87 @@ | |||
| import { SENDER_ID } from "@calcom/lib/constants"; | |||
|
|
|||
| export function getSenderId(phoneNumber?: string | null, sender?: string | null) { | |||
There was a problem hiding this comment.
@emrysal added the getSenderId function and also added a new constant SENDER_ID which is the new NEXT_PUBLIC_SENDER_ID env variable (if empty, "Cal" as a fallback)
6 participants
What does this PR do?
Not all countries support alphanumeric sender IDs for SMS sending. Here is the list of countries supporting/not supporting it: Supported Countries
This caused that SMS sending didn't work for some countries. With this PR we now check if the country code supports alphanumeric sender Ids and then won't use them if it's not supported.
Fixes #5943
Environment: Staging(main branch)
Type of change
How should this be tested?