-
Notifications
You must be signed in to change notification settings - Fork 83
[FSSDK-11529] Impression event logic adjustment for holdouts #1076
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
[FSSDK-11529] Impression event logic adjustment for holdouts #1076
Conversation
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.
Pull Request Overview
This PR adjusts the impression event logic to properly handle holdout decisions. Holdouts are now treated similarly to feature tests for impression event dispatching, ensuring that impression events are sent when users are assigned to holdout experiments.
- Added holdout decision source checks to impression event dispatch conditions
- Updated layer ID logic to exclude holdouts from layer ID lookup
- Enabled the holdout feature toggle
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
lib/optimizely/index.ts | Added HOLDOUT decision source checks to impression event conditions |
lib/optimizely/index.spec.ts | Added comprehensive test coverage for holdout impression events |
lib/feature_toggle.ts | Enabled the holdout feature flag |
lib/event_processor/event_builder/user_event.ts | Modified layer ID logic to exclude holdouts |
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.
Looks good. A small fix requested.
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
Summary
Event sending logic has been adjusted to accommodate holdouts
Test plan
New test cases have been added
Issues