From 9f700ac52d142f2ed2fd1ef5a157f9ebd812b0b7 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 25 Jan 2024 14:26:16 -0500 Subject: [PATCH 1/2] Fix templated message generation --- flows/actions/send_msg.go | 81 ++++---- flows/actions/testdata/_assets.json | 9 +- flows/actions/testdata/request_optin.json | 4 +- flows/actions/testdata/send_msg.json | 219 ++++++++++++++++------ 4 files changed, 206 insertions(+), 107 deletions(-) diff --git a/flows/actions/send_msg.go b/flows/actions/send_msg.go index cf3e35b06..ce46f487e 100644 --- a/flows/actions/send_msg.go +++ b/flows/actions/send_msg.go @@ -2,8 +2,6 @@ package actions import ( "encoding/json" - "strconv" - "strings" "github.com/nyaruka/gocommon/i18n" "github.com/nyaruka/gocommon/urns" @@ -11,7 +9,6 @@ import ( "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/flows/events" - "github.com/nyaruka/goflow/utils" ) func init() { @@ -55,12 +52,15 @@ type SendMsgAction struct { Topic flows.MsgTopic `json:"topic,omitempty" validate:"omitempty,msg_topic"` } -type Params struct { +type TemplateParams struct { UUID uuids.UUID `json:"uuid"` Values map[string][]string } -func (p *Params) MarshalJSON() ([]byte, error) { +// LocalizationUUID gets the UUID which identifies this object for localization +func (p *TemplateParams) LocalizationUUID() uuids.UUID { return uuids.UUID(p.UUID) } + +func (p *TemplateParams) MarshalJSON() ([]byte, error) { if p == nil { return json.Marshal(p) } @@ -73,7 +73,7 @@ func (p *Params) MarshalJSON() ([]byte, error) { return json.Marshal(m) } -func (p *Params) UnmarshalJSON(d []byte) error { +func (p *TemplateParams) UnmarshalJSON(d []byte) error { var m map[string]any if err := json.Unmarshal(d, &m); err != nil { return err @@ -101,7 +101,7 @@ type Templating struct { UUID uuids.UUID `json:"uuid" validate:"required,uuid4"` Template *assets.TemplateReference `json:"template" validate:"required"` Variables []string `json:"variables" engine:"localized,evaluated"` - Params *Params `json:"params,omitempty"` + Params *TemplateParams `json:"params,omitempty"` } // LocalizationUUID gets the UUID which identifies this object for localization @@ -150,7 +150,7 @@ func (a *SendMsgAction) Execute(run flows.Run, step flows.Step, logModifier flow locales := []i18n.Locale{run.Session().MergedEnvironment().DefaultLocale(), run.Session().Environment().DefaultLocale()} templateTranslation := template.FindTranslation(dest.Channel, locales) if templateTranslation != nil { - msg = getTemplateMsg(a, run, urn, channelRef, templateTranslation, evaluatedAttachments, evaluatedQuickReplies, unsendableReason, logEvent) + msg = a.getTemplateMsg(run, urn, channelRef, templateTranslation, unsendableReason, logEvent) } } @@ -173,13 +173,12 @@ func (a *SendMsgAction) Execute(run flows.Run, step flows.Step, logModifier flow // for message actions that specidy a template, this generates the template message where the message content should be // considered just a preview of how the template will be evaluated by the channel -func getTemplateMsg(action *SendMsgAction, run flows.Run, urn urns.URN, channelRef *assets.ChannelReference, templateTranslation *flows.TemplateTranslation, evaluatedAttachments []utils.Attachment, evaluatedQuickReplies []string, unsendableReason flows.UnsendableReason, logEvent flows.EventCallback) *flows.MsgOut { +func (a *SendMsgAction) getTemplateMsg(run flows.Run, urn urns.URN, channelRef *assets.ChannelReference, translation *flows.TemplateTranslation, unsendableReason flows.UnsendableReason, logEvent flows.EventCallback) *flows.MsgOut { evaluatedParams := make(map[string][]string) - templateTranslationParams := templateTranslation.Params() - - if _, ok := templateTranslationParams["body"]; ok { - localizedVariables, _ := run.GetTextArray(uuids.UUID(action.Templating.UUID), "variables", action.Templating.Variables, nil) + // start by localizing and evaluating either the legacy variables or per-component params + if len(a.Templating.Variables) > 0 { + localizedVariables, _ := run.GetTextArray(uuids.UUID(a.Templating.UUID), "variables", a.Templating.Variables, nil) evaluatedVariables := make([]string, len(localizedVariables)) for i, variable := range localizedVariables { @@ -191,52 +190,42 @@ func getTemplateMsg(action *SendMsgAction, run flows.Run, urn urns.URN, channelR } evaluatedParams["body"] = evaluatedVariables - } - if action.Templating.Params != nil { + } else if a.Templating.Params != nil { + for comp, compParams := range a.Templating.Params.Values { + localizedCompParams, _ := run.GetTextArray(uuids.UUID(a.Templating.Params.UUID), comp, compParams, nil) + evaluatedCompParams := make([]string, len(localizedCompParams)) - for compKey, compVars := range action.Templating.Params.Values { - var evaluatedComponentVariables []string - if strings.HasPrefix(compKey, "button.") { - qrIndex, err := strconv.Atoi(strings.TrimPrefix(compKey, "button.")) + for i, variable := range localizedCompParams { + sub, err := run.EvaluateTemplate(variable) if err != nil { logEvent(events.NewError(err)) } - paramValue := evaluatedQuickReplies[qrIndex] - evaluatedComponentVariables = []string{paramValue} - - } else { - - localizedComponentVariables, _ := run.GetTextArray(uuids.UUID(action.Templating.Params.UUID), compKey, compVars, nil) - evaluatedComponentVariables = make([]string, len(localizedComponentVariables)) - for i, variable := range localizedComponentVariables { - sub, err := run.EvaluateTemplate(variable) - if err != nil { - logEvent(events.NewError(err)) - } - evaluatedComponentVariables[i] = sub - } + evaluatedCompParams[i] = sub } - - evaluatedParams[compKey] = evaluatedComponentVariables + evaluatedParams[comp] = evaluatedCompParams } } - // generate a preview of the body text with parameters substituted - evaluatedText := templateTranslation.Substitute(evaluatedParams["body"]) + // next we cross reference with params defined in the template translation to get types + params := make(map[string][]flows.TemplateParam, len(translation.Params())) - params := make(map[string][]flows.TemplateParam, 1) + for comp, compParams := range translation.Params() { + params[comp] = make([]flows.TemplateParam, len(compParams)) - for compKey, compValues := range evaluatedParams { - if len(compValues) > 0 { - params[compKey] = make([]flows.TemplateParam, len(compValues)) - for i, v := range compValues { - params[compKey][i] = flows.TemplateParam{Type: templateTranslationParams[compKey][i].Type(), Value: v} + for i, tp := range compParams { + if i < len(evaluatedParams[comp]) { + params[comp][i] = flows.TemplateParam{Type: tp.Type(), Value: evaluatedParams[comp][i]} + } else { + params[comp][i] = flows.TemplateParam{Type: tp.Type(), Value: ""} } } } - templating := flows.NewMsgTemplating(action.Templating.Template, params, templateTranslation.Namespace()) - locale := templateTranslation.Locale() - return flows.NewMsgOut(urn, channelRef, evaluatedText, evaluatedAttachments, evaluatedQuickReplies, templating, action.Topic, locale, unsendableReason) + // generate a preview of the body text with parameters substituted + evaluatedText := translation.Substitute(evaluatedParams["body"]) + + templating := flows.NewMsgTemplating(a.Templating.Template, params, translation.Namespace()) + locale := translation.Locale() + return flows.NewMsgOut(urn, channelRef, evaluatedText, nil, nil, templating, a.Topic, locale, unsendableReason) } diff --git a/flows/actions/testdata/_assets.json b/flows/actions/testdata/_assets.json index cfc7a194c..0726f974f 100644 --- a/flows/actions/testdata/_assets.json +++ b/flows/actions/testdata/_assets.json @@ -315,6 +315,11 @@ "locale": "eng-US", "content": "Hey {{1}}, your gender is saved as {{2}}.", "params": { + "header": [ + { + "type": "image" + } + ], "body": [ { "type": "text" @@ -323,9 +328,9 @@ "type": "text" } ], - "header": [ + "button.0": [ { - "type": "image" + "type": "text" } ] } diff --git a/flows/actions/testdata/request_optin.json b/flows/actions/testdata/request_optin.json index 697528555..7145a2db0 100644 --- a/flows/actions/testdata/request_optin.json +++ b/flows/actions/testdata/request_optin.json @@ -39,8 +39,8 @@ "created_on": "2018-10-18T14:20:30.000123456Z", "step_uuid": "59d74b86-3e2f-4a93-aece-b05d2fdcde0c", "optin": { - "name": "Joke Of The Day", - "uuid": "248be71d-78e9-4d71-a6c4-9981d369e5cb" + "uuid": "248be71d-78e9-4d71-a6c4-9981d369e5cb", + "name": "Joke Of The Day" }, "channel": { "uuid": "4bb288a0-7fca-4da1-abe8-59a593aff648", diff --git a/flows/actions/testdata/send_msg.json b/flows/actions/testdata/send_msg.json index c4205ac6f..965f9c9f5 100644 --- a/flows/actions/testdata/send_msg.json +++ b/flows/actions/testdata/send_msg.json @@ -419,13 +419,6 @@ "text": "Hi Ryan Lewis, who's a good boy?", "templating": { "uuid": "9c4bf5b5-3aa4-48ec-9bb9-424a9cbc6785", - "params": { - "body": [ - "@contact.name", - "boy" - ], - "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7" - }, "template": { "uuid": "5722e1fd-fe32-4e74-ac78-3cf41a6adb7e", "name": "affirmation" @@ -433,7 +426,14 @@ "variables": [ "@contact.name", "boy" - ] + ], + "params": { + "body": [ + "@contact.name", + "boy" + ], + "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7" + } }, "topic": "account" }, @@ -506,13 +506,6 @@ "text": "Hi Ryan Lewis, who's a good boy?", "templating": { "uuid": "9c4bf5b5-3aa4-48ec-9bb9-424a9cbc6785", - "params": { - "body": [ - "@contact.name", - "boy" - ], - "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7" - }, "template": { "uuid": "5722e1fd-fe32-4e74-ac78-3cf41a6adb7e", "name": "affirmation" @@ -520,7 +513,14 @@ "variables": [ "@contact.name", "boy" - ] + ], + "params": { + "body": [ + "@contact.name", + "boy" + ], + "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7" + } } }, "localization": { @@ -808,7 +808,7 @@ } }, { - "description": "Use template translation with attachments and quick replies and localized", + "description": "Use template translation with component params", "action": { "type": "send_msg", "uuid": "ad154980-7bf7-4ab8-8728-545fd6378912", @@ -822,22 +822,149 @@ ], "templating": { "uuid": "9c4bf5b5-3aa4-48ec-9bb9-424a9cbc6785", + "template": { + "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", + "name": "gender_update" + }, + "variables": null, "params": { - "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7", - "header": [ - "http://example.com/red.jpg" - ], "body": [ "@contact.name", "boy" ], "button.0": [ - "no_used" + "Yeah" ], "button.1": [ - "no_used" + "Nope" + ], + "header": [ + "http://templates.com/red.jpg" + ], + "uuid": "1067f8e2-82f0-4378-9214-0f019365ddb7" + } + } + }, + "localization": { + "spa": { + "ad154980-7bf7-4ab8-8728-545fd6378912": { + "text": [ + "Hola!" + ], + "attachments": [ + "http://example.com/rojo.jpg" + ], + "quick_replies": [ + "Si", + "No" ] }, + "1067f8e2-82f0-4378-9214-0f019365ddb7": { + "header": [ + "http://templates.com/rojo.jpg" + ], + "body": [ + "@contact.name", + "niño" + ], + "button.0": [ + "Sip" + ] + } + } + }, + "events": [ + { + "type": "msg_created", + "created_on": "2018-10-18T14:20:30.000123456Z", + "step_uuid": "59d74b86-3e2f-4a93-aece-b05d2fdcde0c", + "msg": { + "uuid": "9688d21d-95aa-4bed-afc7-f31b35731a3d", + "urn": "tel:+12065551212?channel=57f1078f-88aa-46f4-a59a-948a5739c03d&id=123", + "channel": { + "uuid": "57f1078f-88aa-46f4-a59a-948a5739c03d", + "name": "My Android Phone" + }, + "text": "Hola, Ryan Lewis, tu género está guardado como niño.", + "templating": { + "template": { + "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", + "name": "gender_update" + }, + "params": { + "body": [ + { + "type": "text", + "value": "Ryan Lewis" + }, + { + "type": "text", + "value": "niño" + } + ], + "button.0": [ + { + "type": "text", + "value": "Sip" + } + ], + "button.1": [ + { + "type": "text", + "value": "Nope" + } + ], + "header": [ + { + "type": "image", + "value": "http://templates.com/rojo.jpg" + } + ] + }, + "namespace": "" + }, + "locale": "spa" + } + } + ], + "templates": [ + "Hey Ryan Lewis, your gender is saved as boy.", + "Hola!", + "http://example.com/red.jpg", + "http://example.com/rojo.jpg", + "Yes", + "No", + "Si", + "No" + ], + "localizables": [ + "Hey Ryan Lewis, your gender is saved as boy.", + "http://example.com/red.jpg", + "Yes", + "No" + ], + "inspection": { + "dependencies": [ + { + "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", + "name": "gender_update", + "type": "template" + } + ], + "issues": [], + "results": [], + "waiting_exits": [], + "parent_refs": [] + } + }, + { + "description": "Use template translation with legacy variables", + "action": { + "type": "send_msg", + "uuid": "ad154980-7bf7-4ab8-8728-545fd6378912", + "text": "Hey Ryan Lewis, your gender is saved as boy.", + "templating": { + "uuid": "9c4bf5b5-3aa4-48ec-9bb9-424a9cbc6785", "template": { "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", "name": "gender_update" @@ -867,15 +994,6 @@ "@contact.name", "niño" ] - }, - "1067f8e2-82f0-4378-9214-0f019365ddb7": { - "header": [ - "http://example.com/rojo.jpg" - ], - "body": [ - "@contact.name", - "niño" - ] } } }, @@ -892,14 +1010,11 @@ "name": "My Android Phone" }, "text": "Hola, Ryan Lewis, tu género está guardado como niño.", - "attachments": [ - "http://example.com/rojo.jpg" - ], - "quick_replies": [ - "Si", - "No" - ], "templating": { + "template": { + "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", + "name": "gender_update" + }, "params": { "body": [ { @@ -911,29 +1026,25 @@ "value": "niño" } ], - "header": [ - { - "type": "image", - "value": "http://example.com/rojo.jpg" - } - ], "button.0": [ { "type": "text", - "value": "Si" + "value": "" } ], "button.1": [ { "type": "text", - "value": "No" + "value": "" + } + ], + "header": [ + { + "type": "image", + "value": "" } ] }, - "template": { - "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", - "name": "gender_update" - }, "namespace": "" }, "locale": "spa" @@ -943,10 +1054,7 @@ "templates": [ "Hey Ryan Lewis, your gender is saved as boy.", "Hola!", - "http://example.com/red.jpg", "http://example.com/rojo.jpg", - "Yes", - "No", "Si", "No", "@contact.name", @@ -956,9 +1064,6 @@ ], "localizables": [ "Hey Ryan Lewis, your gender is saved as boy.", - "http://example.com/red.jpg", - "Yes", - "No", "@contact.name", "boy" ], From 83fbbd03d4fc8710a8b83c06f30297151f664bb8 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 25 Jan 2024 14:44:00 -0500 Subject: [PATCH 2/2] Include template params in template (flow def expressions) inspection --- flows/actions/send_msg.go | 18 ++++++++++--- flows/actions/testdata/send_msg.json | 40 +++++++++++++--------------- flows/inspect/templates.go | 26 +++++++++++------- flows/interfaces.go | 4 +++ utils/misc.go | 2 +- 5 files changed, 55 insertions(+), 35 deletions(-) diff --git a/flows/actions/send_msg.go b/flows/actions/send_msg.go index ce46f487e..2dc06c0bc 100644 --- a/flows/actions/send_msg.go +++ b/flows/actions/send_msg.go @@ -9,6 +9,7 @@ import ( "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/flows/events" + "github.com/nyaruka/goflow/utils" ) func init() { @@ -57,8 +58,19 @@ type TemplateParams struct { Values map[string][]string } -// LocalizationUUID gets the UUID which identifies this object for localization -func (p *TemplateParams) LocalizationUUID() uuids.UUID { return uuids.UUID(p.UUID) } +func (p *TemplateParams) EnumerateTemplates(localization flows.Localization, include func(i18n.Language, string)) { + for _, comp := range utils.SortedKeys(p.Values) { + for _, v := range p.Values[comp] { + include(i18n.NilLanguage, v) + } + for _, lang := range localization.Languages() { + lvals := localization.GetItemTranslation(lang, p.UUID, comp) + for _, v := range lvals { + include(lang, v) + } + } + } +} func (p *TemplateParams) MarshalJSON() ([]byte, error) { if p == nil { @@ -100,7 +112,7 @@ func (p *TemplateParams) UnmarshalJSON(d []byte) error { type Templating struct { UUID uuids.UUID `json:"uuid" validate:"required,uuid4"` Template *assets.TemplateReference `json:"template" validate:"required"` - Variables []string `json:"variables" engine:"localized,evaluated"` + Variables []string `json:"variables,omitempty" engine:"localized,evaluated"` Params *TemplateParams `json:"params,omitempty"` } diff --git a/flows/actions/testdata/send_msg.json b/flows/actions/testdata/send_msg.json index 965f9c9f5..78cafdd3e 100644 --- a/flows/actions/testdata/send_msg.json +++ b/flows/actions/testdata/send_msg.json @@ -423,10 +423,6 @@ "uuid": "5722e1fd-fe32-4e74-ac78-3cf41a6adb7e", "name": "affirmation" }, - "variables": [ - "@contact.name", - "boy" - ], "params": { "body": [ "@contact.name", @@ -480,9 +476,7 @@ "boy" ], "localizables": [ - "Hi Ryan Lewis, who's a good boy?", - "@contact.name", - "boy" + "Hi Ryan Lewis, who's a good boy?" ], "inspection": { "dependencies": [ @@ -510,10 +504,6 @@ "uuid": "5722e1fd-fe32-4e74-ac78-3cf41a6adb7e", "name": "affirmation" }, - "variables": [ - "@contact.name", - "boy" - ], "params": { "body": [ "@contact.name", @@ -578,14 +568,14 @@ "templates": [ "Hi Ryan Lewis, who's a good boy?", "@contact.name", + "niño", + "@contact.name", "boy", "@contact.name", "niño" ], "localizables": [ - "Hi Ryan Lewis, who's a good boy?", - "@contact.name", - "boy" + "Hi Ryan Lewis, who's a good boy?" ], "inspection": { "dependencies": [ @@ -602,7 +592,7 @@ } }, { - "description": "Msg with template but no variables", + "description": "Msg with template but no params or variables", "action": { "type": "send_msg", "uuid": "ad154980-7bf7-4ab8-8728-545fd6378912", @@ -612,8 +602,7 @@ "template": { "uuid": "2edc8dfd-aef0-41cf-a900-8a71bdb00900", "name": "wakeup" - }, - "variables": [] + } } }, "events": [ @@ -671,8 +660,7 @@ "template": { "uuid": "2edc8dfd-aef0-41cf-a900-8a71bdb00900", "name": "wakeup" - }, - "variables": null + } } }, "localization": { @@ -808,7 +796,7 @@ } }, { - "description": "Use template translation with component params", + "description": "Use template translation with non body component params", "action": { "type": "send_msg", "uuid": "ad154980-7bf7-4ab8-8728-545fd6378912", @@ -826,7 +814,6 @@ "uuid": "ce00c80e-991a-4c03-b373-3273c23ee042", "name": "gender_update" }, - "variables": null, "params": { "body": [ "@contact.name", @@ -935,7 +922,16 @@ "Yes", "No", "Si", - "No" + "No", + "@contact.name", + "boy", + "@contact.name", + "niño", + "Yeah", + "Sip", + "Nope", + "http://templates.com/red.jpg", + "http://templates.com/rojo.jpg" ], "localizables": [ "Hey Ryan Lewis, your gender is saved as boy.", diff --git a/flows/inspect/templates.go b/flows/inspect/templates.go index d6df275f0..d51f60421 100644 --- a/flows/inspect/templates.go +++ b/flows/inspect/templates.go @@ -17,18 +17,26 @@ func Templates(s any, localization flows.Localization, include func(i18n.Languag } func templateValues(v reflect.Value, localization flows.Localization, include func(i18n.Language, string)) { - walk(v, nil, func(sv reflect.Value, fv reflect.Value, ef *EngineField) { - if ef.Evaluated { - extractTemplates(fv, i18n.NilLanguage, include) + walk(v, + func(v reflect.Value) { + te, ok := v.Interface().(flows.TemplateEnumerator) + if ok { + te.EnumerateTemplates(localization, include) + } + }, + func(sv reflect.Value, fv reflect.Value, ef *EngineField) { + if ef.Evaluated { + extractTemplates(fv, i18n.NilLanguage, include) - // if this field is also localized, each translation is a template and needs to be included - if ef.Localized && localization != nil { - localizable := sv.Interface().(flows.Localizable) + // if this field is also localized, each translation is a template and needs to be included + if ef.Localized && localization != nil { + localizable := sv.Interface().(flows.Localizable) - Translations(localization, localizable.LocalizationUUID(), ef.JSONName, include) + Translations(localization, localizable.LocalizationUUID(), ef.JSONName, include) + } } - } - }) + }, + ) } func Translations(localization flows.Localization, itemUUID uuids.UUID, property string, include func(i18n.Language, string)) { diff --git a/flows/interfaces.go b/flows/interfaces.go index 9a32aeae0..b50881295 100644 --- a/flows/interfaces.go +++ b/flows/interfaces.go @@ -122,6 +122,10 @@ type Localizable interface { LocalizationUUID() uuids.UUID } +type TemplateEnumerator interface { + EnumerateTemplates(Localization, func(i18n.Language, string)) +} + // Flow describes the ordered logic of actions and routers type Flow interface { Contextable diff --git a/utils/misc.go b/utils/misc.go index 736b28cfb..28afeb98a 100644 --- a/utils/misc.go +++ b/utils/misc.go @@ -35,7 +35,7 @@ func Set[K constraints.Ordered](s []K) map[K]bool { } // SortedKeys returns the keys of a set in lexical order -func SortedKeys[K constraints.Ordered](m map[K]bool) []K { +func SortedKeys[K constraints.Ordered, V any](m map[K]V) []K { keys := maps.Keys(m) slices.Sort(keys) return keys