Skip to content

Commit

Permalink
Merge pull request #1291 from nyaruka/run_result_changed_only_when_ch…
Browse files Browse the repository at this point in the history 
…anged

Only generate `run_result_changed` event when value or category changes
  • Loading branch information
@rowanseymour
rowanseymour authored Dec 2, 2024
2 parents a23f38d + df8ea10 commit 951316e
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 15 deletions.
6 changes: 4 additions & 2 deletions flows/actions/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions flows/events/run_result_changed.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -53,6 +64,6 @@ func NewRunResultChanged(result, prev *flows.Result) *RunResultChangedEvent {
CategoryLocalized: result.CategoryLocalized,
Input: result.Input,
Extra: result.Extra,
Previous: prev,
Previous: p,
}
}
2 changes: 1 addition & 1 deletion flows/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions flows/results.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions flows/routers/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion flows/runs/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
19 changes: 16 additions & 3 deletions flows/runs/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 8 additions & 0 deletions test/testdata/runner/legacy_registration.test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions test/testdata/runner/legacy_subflow.test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
20 changes: 20 additions & 0 deletions test/testdata/runner/number_quiz.exceed_limit.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions test/testdata/runner/phone_number.test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
24 changes: 24 additions & 0 deletions test/testdata/runner/subflow_other.test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 951316e

Please sign in to comment.