Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only generate run_result_changed event when value or category changes #1291

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Only generate run_result_changed event when value or category changes #1291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions flows/actions/base.go
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
// 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 @@
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,
}
}

Check warning on line 57 in flows/events/run_result_changed.go

View check run for this annotation

Codecov / codecov/patch

flows/events/run_result_changed.go#L53-L57 

Added lines #L53 - L57 were not covered by tests

return &RunResultChangedEvent{
BaseEvent: NewBaseEvent(TypeRunResultChanged),
Name: result.Name,
Expand All @@ -53,6 +64,6 @@
CategoryLocalized: result.CategoryLocalized,
Input: result.Input,
Extra: result.Extra,
Previous: prev,
Previous: p,
}
}
2 changes: 1 addition & 1 deletion flows/interfaces.go
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,15 @@
}

// 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

Check warning on line 112 in flows/results.go

View check run for this annotation

Codecov / codecov/patch

flows/results.go#L112 

Added line #L112 was not covered by tests
}

// Get returns the result with the given key
Expand Down
6 changes: 4 additions & 2 deletions flows/routers/base.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading