-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix feature that allows marking events as non-interactive #6001
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
| :revenue_reporting_amount, | ||
| :revenue_reporting_currency | ||
| :revenue_reporting_currency, | ||
| :interactive? |
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.
This here was the root cause. We correctly derived what the user wanted to set the interactive? value to, but we didn't allow it to pass to the event we built. It was ignored by ClickhouseEventV2.new/1 and the event was built with the default true value. This meant that logic to handle non-interactive events down the line never kicked in.
|
b2adc14 to
c7c5da0
Compare
zoldar
left a comment
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.
LGTM. Nice work 👍
c7c5da0 to
6ff67e6
Compare
Changes
After this PR, when an event is built, it allows interactive?: false value to be set if it is set in the API request.
After this PR, we will quietly ignore attempts to override any system event to be non-interactive. This is to avoid unexpected behavior. To do so, this PR refactors and expands
SystemGoalsmodule toSystemEvents.Before this PR, the event request interactive value was ignored completely, so events built from API requests with
interactive: falsewere treated asinteractive: true.Existing tests didn't catch it, because session
is_bouncerelated tests create events with the event factory, rather than the ingestion pipeline.Tests
Changelog
Documentation
Dark mode