From e617ef453a4364e64107dc8e02e4ef1fe7bb5f20 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Fri, 25 May 2018 14:44:47 -0500 Subject: [PATCH 1/2] Handle legacy flows where things which are supposed to be translation dicts aren't --- legacy/definition.go | 48 +++++++++++++++++++++++++++++---------- legacy/definition_test.go | 37 ++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/legacy/definition.go b/legacy/definition.go index cbd27fdc2..f5c207460 100644 --- a/legacy/definition.go +++ b/legacy/definition.go @@ -51,6 +51,30 @@ type Flow struct { Entry flows.NodeUUID `json:"entry" validate:"required,uuid4"` } +// Translations is an inline translation map used for localization +type Translations map[utils.Language]string + +// UnmarshalJSON unmarshals legacy translations from the given JSON +func (t *Translations) UnmarshalJSON(data []byte) error { + // sometimes legacy flows have a single string instead of a map + if data[0] == '"' { + var asString string + if err := json.Unmarshal(data, &asString); err != nil { + return err + } + *t = Translations{"base": asString} + return nil + } + + asMap := make(map[utils.Language]string) + if err := json.Unmarshal(data, &asMap); err != nil { + return err + } + + *t = asMap + return nil +} + // Note is a legacy sticky note type Note struct { X decimal.Decimal `json:"x"` @@ -82,11 +106,11 @@ type Metadata struct { } type Rule struct { - UUID flows.ExitUUID `json:"uuid" validate:"required,uuid4"` - Destination flows.NodeUUID `json:"destination" validate:"omitempty,uuid4"` - DestinationType string `json:"destination_type" validate:"eq=A|eq=R"` - Test utils.TypedEnvelope `json:"test"` - Category map[utils.Language]string `json:"category"` + UUID flows.ExitUUID `json:"uuid" validate:"required,uuid4"` + Destination flows.NodeUUID `json:"destination" validate:"omitempty,uuid4"` + DestinationType string `json:"destination_type" validate:"eq=A|eq=R"` + Test utils.TypedEnvelope `json:"test"` + Category Translations `json:"category"` } type RuleSet struct { @@ -276,7 +300,7 @@ type webhookTest struct { } type localizedStringTest struct { - Test map[utils.Language]string `json:"test"` + Test Translations `json:"test"` } type stringTest struct { @@ -305,7 +329,7 @@ type wardTest struct { District string `json:"district"` } -func addTranslationMap(baseLanguage utils.Language, localization flows.Localization, mapped map[utils.Language]string, uuid utils.UUID, property string) string { +func addTranslationMap(baseLanguage utils.Language, localization flows.Localization, mapped Translations, uuid utils.UUID, property string) string { var inBaseLanguage string for language, item := range mapped { expression, _ := expressions.MigrateTemplate(item, expressions.ExtraAsFunction) @@ -340,7 +364,7 @@ func addTranslationMultiMap(baseLanguage utils.Language, localization flows.Loca // // [{"eng": "yes", "fra": "oui"}, {"eng": "no", "fra": "non"}] becomes {"eng": ["yes", "no"], "fra": ["oui", "non"]} // -func TransformTranslations(items []map[utils.Language]string) map[utils.Language][]string { +func TransformTranslations(items []Translations) map[utils.Language][]string { // re-organize into a map of arrays transformed := make(map[utils.Language][]string) @@ -465,8 +489,8 @@ func migrateAction(baseLanguage utils.Language, a Action, localization flows.Loc CreateContact: createContact, }, nil case "reply", "send": - msg := make(map[utils.Language]string) - media := make(map[utils.Language]string) + msg := make(Translations) + media := make(Translations) var quickReplies map[utils.Language][]string err := json.Unmarshal(a.Msg, &msg) @@ -481,7 +505,7 @@ func migrateAction(baseLanguage utils.Language, a Action, localization flows.Loc } } if a.QuickReplies != nil { - legacyQuickReplies := make([]map[utils.Language]string, 0) + legacyQuickReplies := make([]Translations, 0) err := json.Unmarshal(a.QuickReplies, &legacyQuickReplies) if err != nil { @@ -741,7 +765,7 @@ func migrateRule(baseLanguage utils.Language, exitMap map[string]flows.Exit, r R type categoryName struct { uuid flows.ExitUUID destination flows.NodeUUID - translations map[utils.Language]string + translations Translations order int } diff --git a/legacy/definition_test.go b/legacy/definition_test.go index 496bf84a4..a67f0c03b 100644 --- a/legacy/definition_test.go +++ b/legacy/definition_test.go @@ -148,8 +148,6 @@ func TestFlowMigration(t *testing.T) { migratedFlowJSON, _ := utils.JSONMarshalPretty(migratedFlow) expectedFlowJSON, _ := utils.JSONMarshalPretty(test.Expected) - fmt.Println(string(migratedFlowJSON)) - assert.Equal(t, string(expectedFlowJSON), string(migratedFlowJSON)) } } @@ -172,8 +170,8 @@ func TestActionMigration(t *testing.T) { migratedAction := migratedFlow.Nodes()[0].Actions()[0] migratedActionEnvelope, _ := utils.EnvelopeFromTyped(migratedAction) - migratedActionJSON, _ := utils.JSONMarshalPretty(migratedActionEnvelope) - expectedActionJSON, _ := utils.JSONMarshalPretty(test.ExpectedAction) + migratedActionJSON, _ := utils.JSONMarshal(migratedActionEnvelope) + expectedActionJSON, _ := utils.JSONMarshal(test.ExpectedAction) assert.Equal(t, string(expectedActionJSON), string(migratedActionJSON)) @@ -207,8 +205,8 @@ func TestTestMigration(t *testing.T) { t.Errorf("Got no migrated case from legacy test:\n%s\n\n", string(test.LegacyTest)) } else { migratedCase := migratedRouter.Cases[0] - migratedCaseJSON, _ := utils.JSONMarshalPretty(migratedCase) - expectedCaseJSON, _ := utils.JSONMarshalPretty(test.ExpectedCase) + migratedCaseJSON, _ := utils.JSONMarshal(migratedCase) + expectedCaseJSON, _ := utils.JSONMarshal(test.ExpectedCase) assert.Equal(t, string(expectedCaseJSON), string(migratedCaseJSON)) @@ -249,8 +247,8 @@ func TestRuleSetMigration(t *testing.T) { } } - migratedNodeJSON, _ := utils.JSONMarshalPretty(migratedNode) - expectedNodeJSON, _ := utils.JSONMarshalPretty(test.ExpectedNode) + migratedNodeJSON, _ := utils.JSONMarshal(migratedNode) + expectedNodeJSON, _ := utils.JSONMarshal(test.ExpectedNode) assert.Equal(t, string(expectedNodeJSON), string(migratedNodeJSON)) @@ -266,14 +264,29 @@ func readLegacyTestFlows(flowsJSON string) ([]*legacy.Flow, error) { } func checkFlowLocalization(t *testing.T, flow flows.Flow, expectedLocalizationRaw json.RawMessage) { - actualLocalizationJSON, _ := utils.JSONMarshalPretty(flow.Localization()) - expectedLocalizationJSON, _ := utils.JSONMarshalPretty(expectedLocalizationRaw) + actualLocalizationJSON, _ := utils.JSONMarshal(flow.Localization()) + expectedLocalizationJSON, _ := utils.JSONMarshal(expectedLocalizationRaw) assert.Equal(t, string(expectedLocalizationJSON), string(actualLocalizationJSON)) } func TestTranslations(t *testing.T) { - translations := []map[utils.Language]string{ + // can unmarshall from a single string + translations := make(legacy.Translations) + json.Unmarshal([]byte(`"hello"`), &translations) + assert.Equal(t, legacy.Translations{"base": "hello"}, translations) + + // or a map + translations = make(legacy.Translations) + json.Unmarshal([]byte(`{"eng": "hello", "fra": "bonjour"}`), &translations) + assert.Equal(t, legacy.Translations{"eng": "hello", "fra": "bonjour"}, translations) + + // and back to JSON + data, err := json.Marshal(translations) + require.NoError(t, err) + assert.Equal(t, []byte(`{"eng":"hello","fra":"bonjour"}`), data) + + translationSet := []legacy.Translations{ {"eng": "Yes", "fra": "Oui"}, {"eng": "No", "fra": "Non"}, {"eng": "Maybe"}, @@ -282,5 +295,5 @@ func TestTranslations(t *testing.T) { assert.Equal(t, map[utils.Language][]string{ "eng": {"Yes", "No", "Maybe", "Never"}, "fra": {"Oui", "Non", "", "Jamas"}, - }, legacy.TransformTranslations(translations)) + }, legacy.TransformTranslations(translationSet)) } From 16d7f54a686d561c7e9193852f3b156fb400c68d Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Fri, 25 May 2018 15:09:38 -0500 Subject: [PATCH 2/2] Map legacy translations of "base" to the actual base language --- legacy/definition.go | 53 +++++------------------------------- legacy/definition_test.go | 28 ------------------- legacy/testdata/flows.json | 11 ++------ legacy/testdata/tests.json | 13 +++++++++ legacy/utils.go | 55 ++++++++++++++++++++++++++++++++++++++ legacy/utils_test.go | 50 ++++++++++++++++++++++++++++++++++ 6 files changed, 127 insertions(+), 83 deletions(-) create mode 100644 legacy/utils.go create mode 100644 legacy/utils_test.go diff --git a/legacy/definition.go b/legacy/definition.go index f5c207460..6ec65a066 100644 --- a/legacy/definition.go +++ b/legacy/definition.go @@ -17,21 +17,6 @@ import ( "github.com/shopspring/decimal" ) -// represents a decimal value which may be provided as a string or floating point value -type decimalString string - -// UnmarshalJSON unmarshals a decimal string from the given JSON -func (s *decimalString) UnmarshalJSON(data []byte) error { - if data[0] == '"' { - // data is a quoted string - *s = decimalString(data[1 : len(data)-1]) - } else { - // data is JSON float - *s = decimalString(data) - } - return nil -} - var legacyWebhookBody = `{ "contact": {"uuid": "@contact.uuid", "name": "@contact.name", "urn": @(json(if(default(run.input.urn, default(contact.urns.0, null)), text(default(run.input.urn, default(contact.urns.0, null))), null)))}, "flow": @(json(run.flow)), @@ -51,30 +36,6 @@ type Flow struct { Entry flows.NodeUUID `json:"entry" validate:"required,uuid4"` } -// Translations is an inline translation map used for localization -type Translations map[utils.Language]string - -// UnmarshalJSON unmarshals legacy translations from the given JSON -func (t *Translations) UnmarshalJSON(data []byte) error { - // sometimes legacy flows have a single string instead of a map - if data[0] == '"' { - var asString string - if err := json.Unmarshal(data, &asString); err != nil { - return err - } - *t = Translations{"base": asString} - return nil - } - - asMap := make(map[utils.Language]string) - if err := json.Unmarshal(data, &asMap); err != nil { - return err - } - - *t = asMap - return nil -} - // Note is a legacy sticky note type Note struct { X decimal.Decimal `json:"x"` @@ -308,7 +269,7 @@ type stringTest struct { } type numericTest struct { - Test decimalString `json:"test"` + Test DecimalString `json:"test"` } type betweenTest struct { @@ -333,7 +294,7 @@ func addTranslationMap(baseLanguage utils.Language, localization flows.Localizat var inBaseLanguage string for language, item := range mapped { expression, _ := expressions.MigrateTemplate(item, expressions.ExtraAsFunction) - if language != baseLanguage { + if language != baseLanguage && language != "base" { localization.AddItemTranslation(language, uuid, property, []string{expression}) } else { inBaseLanguage = expression @@ -644,7 +605,7 @@ func migrateAction(baseLanguage utils.Language, a Action, localization flows.Loc // migrates the given legacy rule to a router case func migrateRule(baseLanguage utils.Language, exitMap map[string]flows.Exit, r Rule, localization flows.Localization) (routers.Case, error) { - category := r.Category[baseLanguage] + category := r.Category.Base(baseLanguage) newType, _ := testTypeMappings[r.Test.Type] var omitOperand bool @@ -686,7 +647,7 @@ func migrateRule(baseLanguage utils.Language, exitMap map[string]flows.Exit, r R case "contains", "contains_any", "contains_phrase", "contains_only_phrase", "regex", "starts": test := localizedStringTest{} err = json.Unmarshal(r.Test.Data, &test) - arguments = []string{test.Test[baseLanguage]} + arguments = []string{test.Test.Base(baseLanguage)} addTranslationMap(baseLanguage, localization, test.Test, caseUUID, "arguments") @@ -775,7 +736,7 @@ func parseRules(baseLanguage utils.Language, r RuleSet, localization flows.Local categoryMap := make(map[string]categoryName) order := 0 for i := range r.Rules { - category := r.Rules[i].Category[baseLanguage] + category := r.Rules[i].Category.Base(baseLanguage) _, ok := categoryMap[category] if !ok { categoryMap[category] = categoryName{ @@ -806,7 +767,7 @@ func parseRules(baseLanguage utils.Language, r RuleSet, localization flows.Local if r.Rules[i].Test.Type == "true" { // take the first true rule as our default exit if defaultExit == "" { - defaultExit = exitMap[r.Rules[i].Category[baseLanguage]].UUID() + defaultExit = exitMap[r.Rules[i].Category.Base(baseLanguage)].UUID() } continue } @@ -820,7 +781,7 @@ func parseRules(baseLanguage utils.Language, r RuleSet, localization flows.Local if r.Rules[i].Test.Type == "webhook_status" { // webhook failures don't have a case, instead they are the default - defaultExit = exitMap[r.Rules[i].Category[baseLanguage]].UUID() + defaultExit = exitMap[r.Rules[i].Category.Base(baseLanguage)].UUID() } } diff --git a/legacy/definition_test.go b/legacy/definition_test.go index a67f0c03b..6be32e55d 100644 --- a/legacy/definition_test.go +++ b/legacy/definition_test.go @@ -269,31 +269,3 @@ func checkFlowLocalization(t *testing.T, flow flows.Flow, expectedLocalizationRa assert.Equal(t, string(expectedLocalizationJSON), string(actualLocalizationJSON)) } - -func TestTranslations(t *testing.T) { - // can unmarshall from a single string - translations := make(legacy.Translations) - json.Unmarshal([]byte(`"hello"`), &translations) - assert.Equal(t, legacy.Translations{"base": "hello"}, translations) - - // or a map - translations = make(legacy.Translations) - json.Unmarshal([]byte(`{"eng": "hello", "fra": "bonjour"}`), &translations) - assert.Equal(t, legacy.Translations{"eng": "hello", "fra": "bonjour"}, translations) - - // and back to JSON - data, err := json.Marshal(translations) - require.NoError(t, err) - assert.Equal(t, []byte(`{"eng":"hello","fra":"bonjour"}`), data) - - translationSet := []legacy.Translations{ - {"eng": "Yes", "fra": "Oui"}, - {"eng": "No", "fra": "Non"}, - {"eng": "Maybe"}, - {"eng": "Never", "fra": "Jamas"}, - } - assert.Equal(t, map[utils.Language][]string{ - "eng": {"Yes", "No", "Maybe", "Never"}, - "fra": {"Oui", "Non", "", "Jamas"}, - }, legacy.TransformTranslations(translationSet)) -} diff --git a/legacy/testdata/flows.json b/legacy/testdata/flows.json index 2ff4b7ac2..ed0e8e108 100644 --- a/legacy/testdata/flows.json +++ b/legacy/testdata/flows.json @@ -26,7 +26,7 @@ "actions": [ { "msg": { - "base": "Hello", + "eng": "Hello", "fre": "Bonjour" }, "media": {}, @@ -46,13 +46,6 @@ "language": "eng", "expire_after_minutes": 0, "localization": { - "base": { - "98388930-7a0f-4eb8-9a0a-09be2f006420": { - "text": [ - "Hello" - ] - } - }, "fre": { "98388930-7a0f-4eb8-9a0a-09be2f006420": { "text": [ @@ -68,7 +61,7 @@ { "type": "send_msg", "uuid": "98388930-7a0f-4eb8-9a0a-09be2f006420", - "text": "", + "text": "Hello", "attachments": [] } ], diff --git a/legacy/testdata/tests.json b/legacy/testdata/tests.json index 9472d25b8..ae3edb935 100644 --- a/legacy/testdata/tests.json +++ b/legacy/testdata/tests.json @@ -57,6 +57,19 @@ } } }, + { + "legacy_test": { + "type": "contains_any", + "test": "oops not a dict" + }, + "expected_case": { + "uuid": "d2f852ec-7b4e-457f-ae7f-f8b243c49ff5", + "type": "has_any_word", + "arguments": ["oops not a dict"], + "exit_uuid": "c072ecb5-0686-40ea-8ed3-898dc1349783" + }, + "expected_localization": {} + }, { "legacy_test": { "type": "contains_only_phrase", diff --git a/legacy/utils.go b/legacy/utils.go new file mode 100644 index 000000000..7190e009e --- /dev/null +++ b/legacy/utils.go @@ -0,0 +1,55 @@ +package legacy + +import ( + "encoding/json" + + "github.com/nyaruka/goflow/utils" +) + +// Translations is an inline translation map used for localization +type Translations map[utils.Language]string + +// Base looks up the translation in the given base language, or "base" +func (t Translations) Base(baseLanguage utils.Language) string { + val, exists := t[baseLanguage] + if exists { + return val + } + return t["base"] +} + +// UnmarshalJSON unmarshals legacy translations from the given JSON +func (t *Translations) UnmarshalJSON(data []byte) error { + // sometimes legacy flows have a single string instead of a map + if data[0] == '"' { + var asString string + if err := json.Unmarshal(data, &asString); err != nil { + return err + } + *t = Translations{"base": asString} + return nil + } + + asMap := make(map[utils.Language]string) + if err := json.Unmarshal(data, &asMap); err != nil { + return err + } + + *t = asMap + return nil +} + +// DecimalString represents a decimal value which may be provided as a string or floating point value +type DecimalString string + +// UnmarshalJSON unmarshals a decimal string from the given JSON +func (s *DecimalString) UnmarshalJSON(data []byte) error { + if data[0] == '"' { + // data is a quoted string + *s = DecimalString(data[1 : len(data)-1]) + } else { + // data is JSON float + *s = DecimalString(data) + } + return nil +} diff --git a/legacy/utils_test.go b/legacy/utils_test.go new file mode 100644 index 000000000..beb748604 --- /dev/null +++ b/legacy/utils_test.go @@ -0,0 +1,50 @@ +package legacy_test + +import ( + "encoding/json" + "testing" + + "github.com/nyaruka/goflow/legacy" + "github.com/nyaruka/goflow/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslations(t *testing.T) { + // can unmarshall from a single string + translations := make(legacy.Translations) + json.Unmarshal([]byte(`"hello"`), &translations) + assert.Equal(t, legacy.Translations{"base": "hello"}, translations) + + // or a map + translations = make(legacy.Translations) + json.Unmarshal([]byte(`{"eng": "hello", "fra": "bonjour"}`), &translations) + assert.Equal(t, legacy.Translations{"eng": "hello", "fra": "bonjour"}, translations) + + // and back to JSON + data, err := json.Marshal(translations) + require.NoError(t, err) + assert.Equal(t, []byte(`{"eng":"hello","fra":"bonjour"}`), data) + + translationSet := []legacy.Translations{ + {"eng": "Yes", "fra": "Oui"}, + {"eng": "No", "fra": "Non"}, + {"eng": "Maybe"}, + {"eng": "Never", "fra": "Jamas"}, + } + assert.Equal(t, map[utils.Language][]string{ + "eng": {"Yes", "No", "Maybe", "Never"}, + "fra": {"Oui", "Non", "", "Jamas"}, + }, legacy.TransformTranslations(translationSet)) +} + +func TestDecimalString(t *testing.T) { + // can unmarshall from a string + var decimal legacy.DecimalString + json.Unmarshal([]byte(`"123.45"`), &decimal) + assert.Equal(t, legacy.DecimalString("123.45"), decimal) + + // or a floating point (JSON number type) + json.Unmarshal([]byte(`567.89`), &decimal) + assert.Equal(t, legacy.DecimalString("567.89"), decimal) +}