[FSSDK-11552] Add holdout support and refactor decision logic #422
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![prisma-cloud-devsecops[bot]](https://avatars.githubusercontent.com/in/135857?s=60&v=4){kind=small}
…ecision-logic-fssdk11552
- Changed loop from 'for _, holdout := range holdouts' to 'for i := range holdouts' - Created proper pointer 'holdout := &holdouts[i]' to avoid address-of-iteration-variable issue - This fixes the security warning where all iterations would point to the same memory location - All tests passing Resolves Prisma Cloud security scan: Incorrect access of indexable resource
- Implemented holdout parsing from datafile with global/specific/exclusion logic - Added MapHoldouts() to process holdout relationships with feature flags - Implemented GetHoldoutsForFlag() to retrieve applicable holdouts per flag - Fixed bucketer initialization to use pointer (interface value) - Created comprehensive unit tests for HoldoutService (11 test cases) - Added integration test with real bucketer and evaluator - Added GetHoldoutsForFlag() mock method to helpers_test.go All tests passing (decision and config packages) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Added 3 unit tests for GetHoldoutsForFlag() in config_test.go - Created holdout_test.go with 7 comprehensive mapper tests: - Empty holdouts - Global holdouts with exclusions - Specific holdouts with inclusions - Non-running holdouts filtering - Mixed global and specific holdouts - Audience conditions mapping - Variations and traffic allocation mapping - Fixed govet shadow linting error in holdout_service.go line 105 All tests passing. Improves code coverage for new holdout functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
junaed-optimizely
requested changes
Nov 18, 2025
Comment on lines
+73
to
+86
| // Add specifically included holdouts | ||
| if included, exists := includedHoldouts[flagID]; exists { | ||
| applicableHoldouts = append(applicableHoldouts, included...) | ||
| } | ||
|
|
||
| // Add global holdouts (if not excluded) | ||
| isExcluded := false | ||
| if _, exists := excludedHoldouts[flagID]; exists { | ||
| isExcluded = true | ||
| } | ||
|
|
||
| if !isExcluded { | ||
| applicableHoldouts = append(applicableHoldouts, globalHoldouts...) | ||
| } |
There was a problem hiding this comment.
Global holdouts will take precedence over local holdouts. This seems opposite of that. Also can we add one test case that covers the precedence scenario ?
There was a problem hiding this comment.
We can add two more tests here
- Global vs local holdout priority test
- Excluded flag should not appear in
flagHoldoutsMap
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per jira ticket:
Changes:
Jira ticket:
https://optimizely-ext.atlassian.net/browse/FSSDK-11552