-
Notifications
You must be signed in to change notification settings - Fork 71
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
Fix WA button url #715
Fix WA button url #715
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #715 +/- ##
=======================================
Coverage 74.19% 74.19%
=======================================
Files 110 110
Lines 13325 13325
=======================================
Hits 9887 9887
Misses 2722 2722
Partials 716 716 ☔ View full report in Codecov by Sentry. |
component := &Component{Type: "button", Index: strings.TrimPrefix(k, "button."), SubType: "quick_reply"} | ||
component.Params = append(component.Params, &Param{Type: "url", Text: p.Value}) | ||
component.Params = append(component.Params, &Param{Type: "text", Text: p.Value}) |
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.
https://developers.facebook.com/docs/whatsapp/cloud-api/reference/messages#button-parameter-object
type supported are payload and text
handlers/meta/whatsapp/templates.go
Outdated
@@ -52,9 +52,9 @@ func GetTemplatePayload(templating MsgTemplating) *Template { | |||
if strings.HasPrefix(k, "button.") { | |||
|
|||
for _, p := range v { | |||
if strings.HasPrefix(p.Value, "http") { | |||
if p.Type == "url" || strings.HasPrefix(p.Value, "http") { |
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.
I update to have the url type for the param on rapidpro when syncing the templates
nyaruka/rapidpro#5107
and the engine will pass that without changing it https://github.com/nyaruka/goflow/blob/b07f34a76065cea43070b0b0a2c509772a346ad8/flows/actions/send_msg.go#L168
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 do we need || strings.HasPrefix(p.Value, "http")
?
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.
That is what was there, so I did not want to remove that since the docs are not clear for every case,
I think that was added from trials and errors too
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.
But also I doubt that has ever worked since the we used payload and not text
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 can remove that since the type url is an invalid value there
No description provided.