From df8ea10f690082454e0d70b958b94de6fe3bf7ad Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Mon, 2 Dec 2024 19:51:26 +0000 Subject: [PATCH] Only generate run_result_changed event when value or category changes --- flows/actions/base.go | 6 +++-- flows/events/run_result_changed.go | 19 +++++++++++---- flows/interfaces.go | 2 +- flows/results.go | 8 +++++-- flows/routers/base.go | 6 +++-- flows/runs/run.go | 2 +- flows/runs/run_test.go | 19 ++++++++++++--- .../runner/legacy_registration.test.json | 8 +++++++ test/testdata/runner/legacy_subflow.test.json | 8 +++++++ .../runner/number_quiz.exceed_limit.json | 20 ++++++++++++++++ test/testdata/runner/phone_number.test.json | 8 +++++++ test/testdata/runner/subflow_other.test.json | 24 +++++++++++++++++++ 12 files changed, 115 insertions(+), 15 deletions(-) diff --git a/flows/actions/base.go b/flows/actions/base.go index d2e53bfdd..f3acd07c6 100644 --- a/flows/actions/base.go +++ b/flows/actions/base.go @@ -126,8 +126,10 @@ func (a *baseAction) evaluateMessage(run flows.Run, languages []i18n.Language, a // helper to save a run result and log it as an event func (a *baseAction) saveResult(run flows.Run, step flows.Step, name, value, category, categoryLocalized string, input string, extra json.RawMessage, logEvent flows.EventCallback) { result := flows.NewResult(name, value, category, categoryLocalized, step.NodeUUID(), input, extra, dates.Now()) - prev := run.SaveResult(result) - logEvent(events.NewRunResultChanged(result, prev)) + prev, changed := run.SaveResult(result) + if changed { + logEvent(events.NewRunResultChanged(result, prev)) + } } // helper to save a run result based on a webhook call and log it as an event diff --git a/flows/events/run_result_changed.go b/flows/events/run_result_changed.go index 835aca3c0..ce53b172c 100644 --- a/flows/events/run_result_changed.go +++ b/flows/events/run_result_changed.go @@ -13,6 +13,11 @@ func init() { // TypeRunResultChanged is the type of our run result event const TypeRunResultChanged string = "run_result_changed" +type PreviousResult struct { + Value string `json:"value"` + Category string `json:"category"` +} + // RunResultChangedEvent events are created when a run result is saved. They contain not only // the name, value and category of the result, but also the UUID of the node where // the result was generated. @@ -38,13 +43,19 @@ type RunResultChangedEvent struct { CategoryLocalized string `json:"category_localized,omitempty"` Input string `json:"input,omitempty"` Extra json.RawMessage `json:"extra,omitempty"` - - // not included in JSON - used internally to track changes - Previous *flows.Result `json:"-"` + Previous *PreviousResult `json:"previous,omitempty"` } // NewRunResultChanged returns a new save result event for the passed in values func NewRunResultChanged(result, prev *flows.Result) *RunResultChangedEvent { + var p *PreviousResult + if prev != nil { + p = &PreviousResult{ + Value: prev.Value, + Category: prev.Category, + } + } + return &RunResultChangedEvent{ BaseEvent: NewBaseEvent(TypeRunResultChanged), Name: result.Name, @@ -53,6 +64,6 @@ func NewRunResultChanged(result, prev *flows.Result) *RunResultChangedEvent { CategoryLocalized: result.CategoryLocalized, Input: result.Input, Extra: result.Extra, - Previous: prev, + Previous: p, } } diff --git a/flows/interfaces.go b/flows/interfaces.go index 35cd695eb..eac63ce2c 100644 --- a/flows/interfaces.go +++ b/flows/interfaces.go @@ -410,7 +410,7 @@ type Run interface { FlowReference() *assets.FlowReference Session() Session - SaveResult(*Result) *Result + SaveResult(*Result) (*Result, bool) SetStatus(RunStatus) Webhook() *WebhookCall SetWebhook(*WebhookCall) diff --git a/flows/results.go b/flows/results.go index 47883e23b..2ccf63b51 100644 --- a/flows/results.go +++ b/flows/results.go @@ -101,11 +101,15 @@ func (r Results) Clone() Results { } // Save saves a new result in our map using the snakified name as the key. Returns the old result if it existed. -func (r Results) Save(result *Result) *Result { +func (r Results) Save(result *Result) (*Result, bool) { key := utils.Snakify(result.Name) old := r[key] r[key] = result - return old + + if old == nil || (old.Value != result.Value || old.Category != result.Category) { + return old, true + } + return nil, false } // Get returns the result with the given key diff --git a/flows/routers/base.go b/flows/routers/base.go index 361a90592..04878598d 100644 --- a/flows/routers/base.go +++ b/flows/routers/base.go @@ -178,8 +178,10 @@ func (r *baseRouter) routeToCategory(run flows.Run, step flows.Step, categoryUUI extraJSON, _ = jsonx.Marshal(extra) } result := flows.NewResult(r.resultName, match, category.Name(), localizedCategory, step.NodeUUID(), operand, extraJSON, dates.Now()) - prev := run.SaveResult(result) - logEvent(events.NewRunResultChanged(result, prev)) + prev, changed := run.SaveResult(result) + if changed { + logEvent(events.NewRunResultChanged(result, prev)) + } } return category.ExitUUID(), nil diff --git a/flows/runs/run.go b/flows/runs/run.go index 54173769a..f6cc21b0c 100644 --- a/flows/runs/run.go +++ b/flows/runs/run.go @@ -71,7 +71,7 @@ func (r *run) Contact() *flows.Contact { return r.session.Contact() func (r *run) Events() []flows.Event { return r.events } func (r *run) Results() flows.Results { return r.results } -func (r *run) SaveResult(result *flows.Result) *flows.Result { +func (r *run) SaveResult(result *flows.Result) (*flows.Result, bool) { // truncate value if necessary result.Value = stringsx.Truncate(result.Value, r.session.Engine().Options().MaxResultChars) diff --git a/flows/runs/run_test.go b/flows/runs/run_test.go index e49132ae9..845fe83d0 100644 --- a/flows/runs/run_test.go +++ b/flows/runs/run_test.go @@ -292,22 +292,35 @@ func TestSaveResult(t *testing.T) { // no results means empty object with default of empty string test.AssertXEqual(t, types.NewXObject(map[string]types.XValue{"__default__": types.XTextEmpty}), flows.Context(session.Environment(), run.Results())) - run.SaveResult(flows.NewResult("Response 1", "red", "Red", "Rojo", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like red", nil, dates.Now())) + prev, changed := run.SaveResult(flows.NewResult("Response 1", "red", "Red", "Rojo", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like red", nil, dates.Now())) + assert.Nil(t, prev) + assert.True(t, changed) // name is snaked assert.Equal(t, "red", run.Results().Get("response_1").Value) assert.Equal(t, "Red", run.Results().Get("response_1").Category) assert.Equal(t, time.Date(2020, 4, 20, 12, 39, 30, 123456789, time.UTC), run.ModifiedOn()) - run.SaveResult(flows.NewResult("Response 1", "blue", "Blue", "Azul", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like blue", nil, dates.Now())) + prev, changed = run.SaveResult(flows.NewResult("Response 1", "blue", "Blue", "Azul", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like blue", nil, dates.Now())) + if assert.NotNil(t, prev) { + assert.Equal(t, "Red", prev.Category) + } + assert.True(t, changed) // result is overwritten assert.Equal(t, "blue", run.Results().Get("response_1").Value) assert.Equal(t, "Blue", run.Results().Get("response_1").Category) assert.Equal(t, time.Date(2020, 4, 20, 12, 39, 30, 123456789, time.UTC), run.ModifiedOn()) + // try saving new result with same value and category again + prev, changed = run.SaveResult(flows.NewResult("Response 1", "blue", "Blue", "Azul", "6f53c6ae-b66e-44dc-af9e-638e26ad05e9", "blue", nil, dates.Now())) + assert.Nil(t, prev) + assert.False(t, changed) + // long values should truncated - run.SaveResult(flows.NewResult("Response 1", strings.Repeat("創", 700), "Blue", "Azul", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like blue", nil, dates.Now())) + prev, changed = run.SaveResult(flows.NewResult("Response 1", strings.Repeat("創", 700), "Blue", "Azul", "6d35528e-cae3-4e30-b842-8fe6ed7d5c02", "I like blue", nil, dates.Now())) + assert.NotNil(t, prev) + assert.True(t, changed) assert.Equal(t, strings.Repeat("創", 640), run.Results().Get("response_1").Value) } diff --git a/test/testdata/runner/legacy_registration.test.json b/test/testdata/runner/legacy_registration.test.json index 8dc2ead00..da2f0692d 100644 --- a/test/testdata/runner/legacy_registration.test.json +++ b/test/testdata/runner/legacy_registration.test.json @@ -718,6 +718,10 @@ "created_on": "2018-07-06T12:30:51.123456789Z", "input": "18", "name": "Age", + "previous": { + "category": "Other", + "value": "123" + }, "step_uuid": "b504fe9e-d8a8-47fd-af9c-ff2f1faac4db", "type": "run_result_changed", "value": "18" @@ -937,6 +941,10 @@ "created_on": "2018-07-06T12:30:51.123456789Z", "input": "18", "name": "Age", + "previous": { + "category": "Other", + "value": "123" + }, "step_uuid": "b504fe9e-d8a8-47fd-af9c-ff2f1faac4db", "type": "run_result_changed", "value": "18" diff --git a/test/testdata/runner/legacy_subflow.test.json b/test/testdata/runner/legacy_subflow.test.json index 5c76a0ce1..165d71696 100644 --- a/test/testdata/runner/legacy_subflow.test.json +++ b/test/testdata/runner/legacy_subflow.test.json @@ -404,6 +404,10 @@ "created_on": "2018-07-06T12:30:32.123456789Z", "input": "13", "name": "Number", + "previous": { + "category": "Other", + "value": "xx" + }, "step_uuid": "1b5491ec-2b83-445d-bebe-b4a1f677cf4c", "type": "run_result_changed", "value": "13" @@ -520,6 +524,10 @@ "created_on": "2018-07-06T12:30:32.123456789Z", "input": "13", "name": "Number", + "previous": { + "category": "Other", + "value": "xx" + }, "step_uuid": "1b5491ec-2b83-445d-bebe-b4a1f677cf4c", "type": "run_result_changed", "value": "13" diff --git a/test/testdata/runner/number_quiz.exceed_limit.json b/test/testdata/runner/number_quiz.exceed_limit.json index 388b3c201..f013d6e0f 100644 --- a/test/testdata/runner/number_quiz.exceed_limit.json +++ b/test/testdata/runner/number_quiz.exceed_limit.json @@ -438,6 +438,10 @@ "created_on": "2018-07-06T12:30:34.123456789Z", "input": "5", "name": "Result 1", + "previous": { + "category": "Other", + "value": "3" + }, "step_uuid": "a4d15ed4-5b24-407f-b86e-4b881f09a186", "type": "run_result_changed", "value": "5" @@ -607,6 +611,10 @@ "created_on": "2018-07-06T12:30:34.123456789Z", "input": "5", "name": "Result 1", + "previous": { + "category": "Other", + "value": "3" + }, "step_uuid": "a4d15ed4-5b24-407f-b86e-4b881f09a186", "type": "run_result_changed", "value": "5" @@ -762,6 +770,10 @@ "created_on": "2018-07-06T12:30:53.123456789Z", "input": "22", "name": "Result 1", + "previous": { + "category": "Other", + "value": "5" + }, "step_uuid": "44fe8d72-00ed-4736-acca-bbca70987315", "type": "run_result_changed", "value": "22" @@ -918,6 +930,10 @@ "created_on": "2018-07-06T12:30:34.123456789Z", "input": "5", "name": "Result 1", + "previous": { + "category": "Other", + "value": "3" + }, "step_uuid": "a4d15ed4-5b24-407f-b86e-4b881f09a186", "type": "run_result_changed", "value": "5" @@ -958,6 +974,10 @@ "created_on": "2018-07-06T12:30:53.123456789Z", "input": "22", "name": "Result 1", + "previous": { + "category": "Other", + "value": "5" + }, "step_uuid": "44fe8d72-00ed-4736-acca-bbca70987315", "type": "run_result_changed", "value": "22" diff --git a/test/testdata/runner/phone_number.test.json b/test/testdata/runner/phone_number.test.json index 5374e691b..cddf0a4be 100644 --- a/test/testdata/runner/phone_number.test.json +++ b/test/testdata/runner/phone_number.test.json @@ -424,6 +424,10 @@ "created_on": "2018-07-06T12:30:32.123456789Z", "input": "718-456-7890", "name": "Backup Phone", + "previous": { + "category": "Other", + "value": "135532" + }, "step_uuid": "312d3af0-a565-4c96-ba00-bd7f0d08e671", "type": "run_result_changed", "value": "+17184567890" @@ -601,6 +605,10 @@ "created_on": "2018-07-06T12:30:32.123456789Z", "input": "718-456-7890", "name": "Backup Phone", + "previous": { + "category": "Other", + "value": "135532" + }, "step_uuid": "312d3af0-a565-4c96-ba00-bd7f0d08e671", "type": "run_result_changed", "value": "+17184567890" diff --git a/test/testdata/runner/subflow_other.test.json b/test/testdata/runner/subflow_other.test.json index 3fffcfa4a..18a51cbdc 100644 --- a/test/testdata/runner/subflow_other.test.json +++ b/test/testdata/runner/subflow_other.test.json @@ -604,6 +604,10 @@ "created_on": "2018-07-06T12:30:38.123456789Z", "input": "yes", "name": "Answer", + "previous": { + "category": "Other", + "value": "neither" + }, "step_uuid": "4f15f627-b1e2-4851-8dbf-00ecf5d03034", "type": "run_result_changed", "value": "yes" @@ -889,6 +893,10 @@ "created_on": "2018-07-06T12:30:38.123456789Z", "input": "yes", "name": "Answer", + "previous": { + "category": "Other", + "value": "neither" + }, "step_uuid": "4f15f627-b1e2-4851-8dbf-00ecf5d03034", "type": "run_result_changed", "value": "yes" @@ -1360,6 +1368,10 @@ "created_on": "2018-07-06T12:30:38.123456789Z", "input": "yes", "name": "Answer", + "previous": { + "category": "Other", + "value": "neither" + }, "step_uuid": "4f15f627-b1e2-4851-8dbf-00ecf5d03034", "type": "run_result_changed", "value": "yes" @@ -1504,6 +1516,10 @@ "created_on": "2018-07-06T12:31:15.123456789Z", "input": "no", "name": "Answer", + "previous": { + "category": "Other", + "value": "never" + }, "step_uuid": "3ceb7525-c2e1-40b0-bec9-e032f4f9af5f", "type": "run_result_changed", "value": "no" @@ -1696,6 +1712,10 @@ "created_on": "2018-07-06T12:31:15.123456789Z", "input": "no", "name": "Answer", + "previous": { + "category": "Other", + "value": "never" + }, "step_uuid": "3ceb7525-c2e1-40b0-bec9-e032f4f9af5f", "type": "run_result_changed", "value": "no" @@ -1864,6 +1884,10 @@ "created_on": "2018-07-06T12:30:38.123456789Z", "input": "yes", "name": "Answer", + "previous": { + "category": "Other", + "value": "neither" + }, "step_uuid": "4f15f627-b1e2-4851-8dbf-00ecf5d03034", "type": "run_result_changed", "value": "yes"