-
Notifications
You must be signed in to change notification settings - Fork 71
Update payloads for tasks queued to mailroom #723
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.
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
Conversation
@@ -189,7 +189,7 @@ func (ts *BackendTestSuite) TestDeleteMsgByExternalID() { | |||
err = ts.b.DeleteMsgByExternalID(ctx, knChannel, "ext2") | |||
ts.Nil(err) | |||
|
|||
ts.assertQueuedContactTask(ContactID(100), "msg_deleted", map[string]any{"org_id": float64(1), "msg_id": float64(10002)}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to repeat org_id
or contact_id
in the task payload... mailroom gets that from the queue
"org_id": e.OrgID_, | ||
"contact_id": e.ContactID_, | ||
"event_id": e.ID_, | ||
"event_type": e.EventType_, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to switch these to all be a single channel_event
task type, so step one is putting event_type
in the payload
@@ -32,15 +29,18 @@ func queueMsgHandling(rc redis.Conn, c *Contact, m *Msg) error { | |||
|
|||
func queueChannelEvent(rc redis.Conn, c *Contact, e *ChannelEvent) error { | |||
body := map[string]any{ | |||
"org_id": e.OrgID_, | |||
"contact_id": e.ContactID_, | |||
"event_id": e.ID_, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we should some day model whether channel events are handled.. i.e. have mailroom update their status like we do for messages.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #723 +/- ##
=======================================
Coverage 74.42% 74.43%
=======================================
Files 110 110
Lines 13331 13330 -1
=======================================
Hits 9922 9922
+ Misses 2689 2688 -1
Partials 720 720 ☔ View full report in Codecov by Sentry. |
"urn_id": e.ContactURNID_, | ||
"channel_id": e.ChannelID_, | ||
"extra": e.Extra(), | ||
"new_contact": c.IsNew_, | ||
"occurred_on": e.OccurredOn_, | ||
"created_on": e.CreatedOn_, | ||
} | ||
if e.OptInID_ != 0 { | ||
body["optin_id"] = e.OptInID_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
best I can tell we're not doing optin triggers correctly in the engine because this isn't being passed - the triggers work but @optin
is exposed in expressions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.