Stop by modifier #769
Stop by modifier #769
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how contact stopping is handled by using the flow modifier system instead of direct database operations. The change ensures that channel event stops generate contact_status_changed events like regular flow status changes.
- Replaces direct contact stopping with flow modifier application for consistency
- Adds persistence configuration for
contact_status_changedevents - Updates tests to reflect the new event generation behavior
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/tasks/realtime/ctasks/event_received.go | Replaces direct contact stopping with flow modifier application |
| core/models/event.go | Adds persistence configuration for contact status changed events |
| core/models/contact.go | Removes the direct Contact.Stop method |
| core/tasks/realtime/ctasks/event_received_test.go | Updates test expectations for new event generation |
| core/runner/handlers/contact_status_changed_test.go | Updates test expectations and SQL formatting |
| core/models/contact_test.go | Removes test for deleted Contact.Stop method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if t.EventType == models.EventTypeStopContact { | ||
| if _, err := scene.ApplyModifier(ctx, rt, oa, modifiers.NewStatus(flows.ContactStatusStopped), models.NilUserID); err != nil { | ||
| return nil, fmt.Errorf("error applying stop modifier: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The stop contact handling should occur before trigger processing to maintain consistent event ordering. Consider moving this block before line 110 where trigger processing begins.
Copilot uses AI. Check for mistakes.
| testdb.InsertContactFire(rt, testdb.Org1, testdb.Cathy, models.ContactFireTypeCampaignPoint, fmt.Sprintf("%d:1", testdb.RemindersPoint1.ID), time.Now(), "") | ||
| testdb.InsertContactFire(rt, testdb.Org1, testdb.George, models.ContactFireTypeCampaignPoint, fmt.Sprintf("%d:1", testdb.RemindersPoint1.ID), time.Now(), "") |
There was a problem hiding this comment.
Magic number '1' in the campaign point format string is unexplained. Consider adding a comment to clarify what this number represents or define it as a named constant.
Copilot uses AI. Check for mistakes.
a29ed0a to
f72b105
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #769 +/- ##
=======================================
Coverage 51.03% 51.04%
=======================================
Files 250 250
Lines 15153 15132 -21
=======================================
- Hits 7734 7724 -10
+ Misses 6615 6608 -7
+ Partials 804 800 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
So stops from channel events are handled same as status=stopped changes in flows, and generate
contact_status_changedevents