Merge pull request #1197 from nyaruka/template_translation_matching

Rework template translation finding

assets/static/template.go

@@ -38,15 +38,15 @@ func (t *Template) Translations() []assets.TemplateTranslation {
 
 // TemplateTranslation represents a single template translation
 type TemplateTranslation struct {
-	Channel_       assets.ChannelReference `json:"channel"         validate:"required"`
-	Content_       string                  `json:"content"         validate:"required"`
-	Locale_        i18n.Locale             `json:"locale"          validate:"required"`
-	Namespace_     string                  `json:"namespace"`
-	VariableCount_ int                     `json:"variable_count"`
+	Channel_       *assets.ChannelReference `json:"channel"         validate:"required"`
+	Content_       string                   `json:"content"         validate:"required"`
+	Locale_        i18n.Locale              `json:"locale"          validate:"required"`
+	Namespace_     string                   `json:"namespace"`
+	VariableCount_ int                      `json:"variable_count"`
 }
 
 // NewTemplateTranslation creates a new template translation
-func NewTemplateTranslation(channel assets.ChannelReference, locale i18n.Locale, content string, variableCount int, namespace string) *TemplateTranslation {
+func NewTemplateTranslation(channel *assets.ChannelReference, locale i18n.Locale, content string, variableCount int, namespace string) *TemplateTranslation {
 	return &TemplateTranslation{
 		Channel_:       channel,
 		Content_:       content,
@@ -69,4 +69,4 @@ func (t *TemplateTranslation) Locale() i18n.Locale { return t.Locale_ }
 func (t *TemplateTranslation) VariableCount() int { return t.VariableCount_ }
 
 // Channel returns the channel this template translation is for
-func (t *TemplateTranslation) Channel() assets.ChannelReference { return t.Channel_ }
+func (t *TemplateTranslation) Channel() *assets.ChannelReference { return t.Channel_ }

assets/static/template_test.go

@@ -10,10 +10,7 @@ import (
 )
 
 func TestTemplate(t *testing.T) {
-	channel := assets.ChannelReference{
-		Name: "Test Channel",
-		UUID: assets.ChannelUUID("ffffffff-9b24-92e1-ffff-ffffb207cdb4"),
-	}
+	channel := assets.NewChannelReference("Test Channel", "ffffffff-9b24-92e1-ffff-ffffb207cdb4")
 
 	translation := NewTemplateTranslation(channel, i18n.Locale("eng-US"), "Hello {{1}}", 1, "0162a7f4_dfe4_4c96_be07_854d5dba3b2b")
 	assert.Equal(t, channel, translation.Channel())

assets/template.go

@@ -48,7 +48,7 @@ type TemplateTranslation interface {
 	Locale() i18n.Locale
 	Namespace() string
 	VariableCount() int
-	Channel() ChannelReference
+	Channel() *ChannelReference
 }
 
 // TemplateReference is used to reference a Template

flows/actions/send_msg.go

@@ -88,21 +88,26 @@ func (a *SendMsgAction) Execute(run flows.Run, step flows.Step, logModifier flow
 
 	sa := run.Session().Assets()
 
+	var template *flows.Template
+	if a.Templating != nil {
+		template = sa.Templates().Get(a.Templating.Template.UUID)
+	}
+
 	// create a new message for each URN+channel destination
 	for _, dest := range destinations {
 		urn := dest.URN.URN()
 		channelRef := assets.NewChannelReference(dest.Channel.UUID(), dest.Channel.Name())
 
 		// do we have a template defined?
 		var templating *flows.MsgTemplating
-		if a.Templating != nil {
+		if template != nil {
 			// looks for a translation in the contact locale or environment default
 			locales := []i18n.Locale{
 				run.Session().MergedEnvironment().DefaultLocale(),
 				run.Session().Environment().DefaultLocale(),
 			}
 
-			translation := sa.Templates().FindTranslation(a.Templating.Template.UUID, channelRef, locales)
+			translation := template.FindTranslation(dest.Channel, locales)
 			if translation != nil {
 				localizedVariables, _ := run.GetTextArray(uuids.UUID(a.Templating.UUID), "variables", a.Templating.Variables, nil)
 

flows/template.go

@@ -31,38 +31,22 @@ func (t *Template) Reference() *assets.TemplateReference {
 }
 
 // FindTranslation finds the matching translation for the passed in channel and languages (in priority order)
-func (t *Template) FindTranslation(channel assets.ChannelUUID, locales []i18n.Locale) *TemplateTranslation {
-	// first iterate through and find all translations that are for this channel
-	candidatesByLocale := make(map[i18n.Locale]*TemplateTranslation)
-	candidatesByLang := make(map[i18n.Language]*TemplateTranslation)
+func (t *Template) FindTranslation(channel *Channel, locales []i18n.Locale) *TemplateTranslation {
+	// find all translations for this channel
+	candidates := make(map[string]*TemplateTranslation)
+	candidateLocales := make([]string, 0, 5)
 	for _, tr := range t.Template.Translations() {
-		if tr.Channel().UUID == channel {
-			tt := NewTemplateTranslation(tr)
-			lang, _ := tt.Locale().Split()
-
-			candidatesByLocale[tt.Locale()] = tt
-			candidatesByLang[lang] = tt
-		}
-	}
-
-	// first look for exact locale match
-	for _, locale := range locales {
-		tt := candidatesByLocale[locale]
-		if tt != nil {
-			return tt
+		if tr.Channel().UUID == channel.UUID() {
+			candidates[string(tr.Locale())] = NewTemplateTranslation(tr)
+			candidateLocales = append(candidateLocales, string(tr.Locale()))
 		}
 	}
-
-	// if that fails look for language match
-	for _, locale := range locales {
-		lang, _ := locale.Split()
-		tt := candidatesByLang[lang]
-		if tt != nil {
-			return tt
-		}
+	if len(candidates) == 0 {
+		return nil
 	}
 
-	return nil
+	match := i18n.NewBCP47Matcher(candidateLocales...).ForLocales(locales...)
+	return candidates[match]
 }
 
 // TemplateTranslation represents a single translation for a template
@@ -119,22 +103,3 @@ func NewTemplateAssets(ts []assets.Template) *TemplateAssets {
 func (a *TemplateAssets) Get(uuid assets.TemplateUUID) *Template {
 	return a.byUUID[uuid]
 }
-
-// FindTranslation looks through our list of templates to find the template matching the passed in uuid
-// If no template or translation is found then empty string is returned
-func (a *TemplateAssets) FindTranslation(uuid assets.TemplateUUID, channel *assets.ChannelReference, locales []i18n.Locale) *TemplateTranslation {
-	// no channel, can't match to a template
-	if channel == nil {
-		return nil
-	}
-
-	template := a.byUUID[uuid]
-
-	// not found, no template
-	if template == nil {
-		return nil
-	}
-
-	// look through our translations looking for a match by both channel and translation
-	return template.FindTranslation(channel.UUID, locales)
-}

flows/template_test.go

@@ -1,11 +1,14 @@
-package flows
+package flows_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/nyaruka/gocommon/i18n"
 	"github.com/nyaruka/goflow/assets"
 	"github.com/nyaruka/goflow/assets/static"
+	"github.com/nyaruka/goflow/flows"
+	"github.com/nyaruka/goflow/test"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -23,85 +26,85 @@ func TestTemplateTranslation(t *testing.T) {
 	channel := assets.NewChannelReference("0bce5fd3-c215-45a0-bcb8-2386eb194175", "Test Channel")
 
 	for i, tc := range tcs {
-		tt := NewTemplateTranslation(static.NewTemplateTranslation(*channel, i18n.Locale("eng-US"), tc.Content, len(tc.Variables), "a6a8863e_7879_4487_ad24_5e2ea429027c"))
+		tt := flows.NewTemplateTranslation(static.NewTemplateTranslation(channel, i18n.Locale("eng-US"), tc.Content, len(tc.Variables), "a6a8863e_7879_4487_ad24_5e2ea429027c"))
 		result := tt.Substitute(tc.Variables)
 		assert.Equal(t, tc.Expected, result, "%d: unexpected template substitution", i)
 	}
 }
 
-func TestTemplates(t *testing.T) {
-	channel1 := assets.NewChannelReference("0bce5fd3-c215-45a0-bcb8-2386eb194175", "Test Channel")
-	tt1 := static.NewTemplateTranslation(*channel1, i18n.Locale("eng"), "Hello {{1}}", 1, "")
-	tt2 := static.NewTemplateTranslation(*channel1, i18n.Locale("spa-EC"), "Que tal {{1}}", 1, "")
-	tt3 := static.NewTemplateTranslation(*channel1, i18n.Locale("spa-ES"), "Hola {{1}}", 1, "")
-	template := NewTemplate(static.NewTemplate("c520cbda-e118-440f-aaf6-c0485088384f", "greeting", []*static.TemplateTranslation{tt1, tt2, tt3}))
+func TestTemplate(t *testing.T) {
+	channel1 := test.NewChannel("WhatsApp 1", "+12345", []string{"whatsapp"}, []assets.ChannelRole{}, nil)
+	channel2 := test.NewChannel("WhatsApp 2", "+23456", []string{"whatsapp"}, []assets.ChannelRole{}, nil)
+	channel3 := test.NewChannel("WhatsApp 3", "+34567", []string{"whatsapp"}, []assets.ChannelRole{}, nil)
+	channel1Ref := assets.NewChannelReference(channel1.UUID(), channel1.Name())
+	channel2Ref := assets.NewChannelReference(channel2.UUID(), channel2.Name())
 
-	tas := NewTemplateAssets([]assets.Template{template})
+	tt1 := static.NewTemplateTranslation(channel1Ref, i18n.Locale("eng"), "Hello {{1}}", 1, "")
+	tt2 := static.NewTemplateTranslation(channel1Ref, i18n.Locale("spa-EC"), "Que tal {{1}}", 1, "")
+	tt3 := static.NewTemplateTranslation(channel1Ref, i18n.Locale("spa-ES"), "Hola {{1}}", 1, "")
+	tt4 := static.NewTemplateTranslation(channel2Ref, i18n.Locale("en"), "Hello {{1}}", 1, "")
+	template := flows.NewTemplate(static.NewTemplate("c520cbda-e118-440f-aaf6-c0485088384f", "greeting", []*static.TemplateTranslation{tt1, tt2, tt3, tt4}))
+
+	tas := flows.NewTemplateAssets([]assets.Template{template})
 
 	tcs := []struct {
-		UUID      assets.TemplateUUID
-		Channel   *assets.ChannelReference
-		Locales   []i18n.Locale
-		Variables []string
-		Expected  string
+		channel   *flows.Channel
+		locales   []i18n.Locale
+		variables []string
+		expected  string
 	}{
 		{
-			"c520cbda-e118-440f-aaf6-c0485088384f",
 			channel1,
 			[]i18n.Locale{"eng-US", "spa-CO"},
 			[]string{"Chef"},
 			"Hello Chef",
 		},
 		{
-			"c520cbda-e118-440f-aaf6-c0485088384f",
 			channel1,
 			[]i18n.Locale{"eng", "spa-CO"},
 			[]string{"Chef"},
 			"Hello Chef",
 		},
 		{
-			"c520cbda-e118-440f-aaf6-c0485088384f",
 			channel1,
 			[]i18n.Locale{"deu-DE", "spa-ES"},
 			[]string{"Chef"},
 			"Hola Chef",
 		},
 		{
-			"c520cbda-e118-440f-aaf6-c0485088384f",
-			nil,
-			[]i18n.Locale{"deu-DE", "spa-ES"},
+			channel1,
+			[]i18n.Locale{"deu-DE"},
 			[]string{"Chef"},
-			"",
+			"Hello Chef",
 		},
 		{
-			"c520cbda-e118-440f-aaf6-c0485088384f",
-			channel1,
-			[]i18n.Locale{"deu-DE"},
+			channel2,
+			[]i18n.Locale{"eng-US", "spa-ES"},
 			[]string{"Chef"},
-			"",
+			"Hello Chef",
 		},
 		{
-			"8c5d4910-114a-4521-ba1d-bde8b024865a",
-			channel1,
+			channel3,
 			[]i18n.Locale{"eng-US", "spa-ES"},
 			[]string{"Chef"},
 			"",
 		},
 	}
 
 	for _, tc := range tcs {
-		tr := tas.FindTranslation(tc.UUID, tc.Channel, tc.Locales)
-		if tr == nil {
-			assert.Equal(t, "", tc.Expected)
-			continue
+		testID := fmt.Sprintf("channel '%s' and locales %v", tc.channel.Name(), tc.locales)
+		tr := template.FindTranslation(tc.channel, tc.locales)
+		if tc.expected == "" {
+			assert.Nil(t, tr, "unexpected translation found for %s", testID)
+		} else {
+			if assert.NotNil(t, tr, "expected translation to be found for %s", testID) {
+				assert.Equal(t, tc.expected, tr.Substitute(tc.variables), "substition mismatch for %s", testID)
+			}
 		}
-		assert.NotNil(t, tr.Asset())
-
-		assert.Equal(t, tc.Expected, tr.Substitute(tc.Variables))
 	}
 
 	template = tas.Get(assets.TemplateUUID("c520cbda-e118-440f-aaf6-c0485088384f"))
 	assert.NotNil(t, template)
 	assert.Equal(t, assets.NewTemplateReference("c520cbda-e118-440f-aaf6-c0485088384f", "greeting"), template.Reference())
-	assert.Equal(t, (*assets.TemplateReference)(nil), (*Template)(nil).Reference())
+	assert.Equal(t, (*assets.TemplateReference)(nil), (*flows.Template)(nil).Reference())
 }