Add support for sending Twilio Template #751
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #751 +/- ##
==========================================
+ Coverage 75.81% 75.84% +0.03%
==========================================
Files 111 111
Lines 10316 10363 +47
==========================================
+ Hits 7821 7860 +39
- Misses 1781 1785 +4
- Partials 714 718 +4 ☔ View full report in Codecov by Sentry. |
handlers/twiml/handlers.go
Outdated
| // do we have a template and support whatsapp scheme? | ||
| if msg.Templating() != nil && channel.IsScheme(urns.WhatsApp) { | ||
| if msg.Templating().ExternalID == "" { | ||
| return courier.ErrFailedWithReason("", "template missing contentSID") |
There was a problem hiding this comment.
again ErrFailedWithReason is for error codes from the channel side that we can expose on the RP side
If the scenario above happens, it's because we've messed up syncing templates or creating messages in mailroom. Therefore it's something that courier shouldn't retry and should log. In that way it's similar to ErrChannelConfig but how about we add a new err type for this....
// ErrMessageInvalid should be returned by a handler send method when the message it has received is invalid
var ErrMessageInvalid error = &SendError{
msg: "message invalid",
retryable: false,
loggable: true,
clogCode: "message_invalid",
clogMsg: "Message is missing required values.",
}
handlers/twiml/handlers.go
Outdated
| form.Add("MediaUrl", a.URL) | ||
| } | ||
| } | ||
| contentVariables := make(map[string]string) |
There was a problem hiding this comment.
make(map[string]string, len(msg.Templating().Variables))
handlers/twiml/handlers.go
Outdated
| if channel.IsScheme(urns.WhatsApp) { | ||
| form["To"][0] = fmt.Sprintf("%s:+%s", urns.WhatsApp.Prefix, form["To"][0]) | ||
| form["From"][0] = fmt.Sprintf("%s:%s", urns.WhatsApp.Prefix, form["From"][0]) | ||
| contentVariablesJson, _ := json.Marshal(contentVariables) |
There was a problem hiding this comment.
if you're not checking the error, and there's no reason to here because there's nothing we're marshaling here that could fail to marshal, use jsonx.MustMarshal
handlers/twiml/handlers.go
Outdated
| sort.Strings(varNames) | ||
| for _, varName := range varNames { | ||
| contentVariables[varName] = msg.Templating().Variables[comp.Variables[varName]].Value | ||
| } |
There was a problem hiding this comment.
can't this be a bit simpler like
for _, comp := range msg.Templating().Components {
for varVey, varIndex := range comp.Variables {
contentVariables[varKey] = msg.Templating().Variables[varIndex]
}
}I'm not getting why you need a sort...
| // do we have a template and support whatsapp scheme? | ||
| if msg.Templating() != nil && channel.IsScheme(urns.WhatsApp) { | ||
| if msg.Templating().ExternalID == "" { | ||
| return courier.ErrMessageInvalid |
There was a problem hiding this comment.
@norkans7 please have a think about places in other handlers where we should use this
No description provided.