-
Notifications
You must be signed in to change notification settings - Fork 466
fix(features): Fix multivariate weights in change requests and segment overrides #6509
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
Closed
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
1e00a8e
fix: explicitely-set-id-in-mv-options
Zaimwa9 c685b14
fix: extracted-mv-mapping-into-utils
Zaimwa9 fd21b09
fix(features): filter out MV variations without IDs in mapMvOptionsTo…
talissoncosta dc0722a
fix(nav): rename duplicate scheduled-changes-link ID in sidebar
talissoncosta cc6501d
test(e2e): add data-test id for 4_EYES toggle in environment settings
talissoncosta f7e8139
test(e2e): add change request e2e tests for MV features
talissoncosta 797f242
fix(utils): handle segment override MV options in mapMvOptionsToState…
talissoncosta e02d94b
fix(diff): pass projectFlag to DiffVariations in segment overrides
talissoncosta 4af571b
test(e2e): add assertions for segment override MV weights in CR diff
talissoncosta a343aef
fix(diff): correct array concatenation in getVariationDiff
talissoncosta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Unused parameter causes incorrect feature state value IDs
Medium Severity
The
mapMvOptionsToStateValuesfunction accepts_existingMvFeatureStateValuesas a second parameter but never uses it. The function returnsMultivariateFeatureStateValueobjects withidset tomvOption.id(the multivariate option ID). However,MultivariateFeatureStateValue.idshould be the feature state value ID, not the option ID. The_existingMvFeatureStateValuesparameter appears intended to look up correct feature state value IDs but is ignored. This could cause API issues when updating existing feature states, as the incorrect ID might prevent proper matching of existing records. The similar functionbuildSegmentOverrideMvValuescorrectly omits theidfield entirely.