-
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
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 PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| multivariate_feature_option: mvOption.id, | ||
| percentage_allocation: weight, | ||
| } | ||
| }) |
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 mapMvOptionsToStateValues function accepts _existingMvFeatureStateValues as a second parameter but never uses it. The function returns MultivariateFeatureStateValue objects with id set to mvOption.id (the multivariate option ID). However, MultivariateFeatureStateValue.id should be the feature state value ID, not the option ID. The _existingMvFeatureStateValues parameter 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 function buildSegmentOverrideMvValues correctly omits the id field entirely.
…StateValues New multivariate variations that haven't been saved to the project yet don't have IDs. When creating change requests, these variations cannot be referenced until they are saved. This fix filters out variations without IDs to prevent invalid data being sent to the API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The Scheduling link was using 'change-requests-link' ID which is also used by the Change Requests link. This caused e2e tests to click on the wrong element. Renamed to 'scheduled-changes-link' for uniqueness. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Added data-test='js-feature-change-requests' attribute to the ChangeRequestsSetting component to enable e2e tests to reliably detect and interact with the feature change requests toggle. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive e2e tests for change requests with multivariate features: - TEST 1: Create change request with MV weight changes - TEST 2: Verify values are NOT applied before CR is published - TEST 3: Verify CR diff shows correct weight values - TEST 4: Create segment override with MV weights via change request - TEST 5: Verify segment override CR contains correct values Tests cover both feature state and segment override change requests, ensuring MV weights are correctly stored and displayed in the diff. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…Values The function now handles both MultivariateOption (from project flag) and MultivariateFeatureStateValue (from segment overrides) types. Previously, segment override MV options were filtered out because they have 'multivariate_feature_option' property instead of 'id', causing MV weights to not be stored in change requests for segment overrides. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add projectFlag prop to DiffSegmentOverrides and DiffSegmentOverride components to display variation values in the diff view. Without this, variation values were shown as empty in segment override diffs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Verify that MV weight values (60%, 30%, 10%) are displayed in the change request diff view for segment overrides. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Fix operator precedence issue where || had higher precedence than concat, causing the array to not be properly concatenated. Wrap the left operand in parentheses to ensure correct evaluation order. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
48e6463 to
a343aef
Compare
Summary
This PR fixes issues with multivariate (MV) variation weights when creating change requests and segment overrides.
Bugs Fixed
Segment override MV weights sent as 0% - When creating change requests with segment overrides, the multivariate weights were being sent as 0% instead of the actual user-entered values. This was caused by using
default_percentage_allocation(project-level default) instead ofpercentage_allocation(override weight).Diff view not showing all variations - The change request diff view wasn't showing all project variations, only those that existed in the old/new state arrays. This made it hard to see newly added variations.
React state mutations - Direct array mutations in
SegmentOverrides.jsprevented React from detecting state changes, causing UI inconsistencies.Changes Made
feature-list-store.tsbuildSegmentOverrideMvValuesto build MV state values with correct override weightsSegmentOverrides.jsmapinstead of direct mutation), filter unsaved variations, useupdateVariationWeightdiff-utils.tsprojectMultivariateOptionsparam to show all variations, detect newly added variations for 0% -> X% diffDiffFeature.tsxprojectFlag.multivariate_optionsto diff functionsDiffSegmentOverrides.tsxprojectFlagthrough toDiffVariationsFeatureListProvider.jsmapMvOptionsToStateValuesfor change request creationutils.tsxbuildSegmentOverrideMvValues,updateVariationWeight,mapMvOptionsToStateValuesutilitiesCreateFlag.jsenvironmentVariationsinstead ofidentityVariationsfor change requestsKnown Limitations / Out of Scope
Test plan
🤖 Generated with Claude Code