-
Notifications
You must be signed in to change notification settings - Fork 467
chore: Create flag migration #6356
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Zaimwa9 <wadii.zaim@flagsmith.com>
# Conflicts: # frontend/web/components/pages/UserPage.tsx
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 is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 10
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.
frontend/web/components/modals/create-feature/FeatureLimitAlert.tsx
Outdated
Show resolved
Hide resolved
| <FeatureValue | ||
| error={error} | ||
| createFeature={createFeature} | ||
| hideValue={false} |
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.
Project setting prevent_flag_defaults no longer respected
Medium Severity
The hideValue prop is hardcoded to false, but the old code passed project.prevent_flag_defaults && !identity to conditionally hide the value section. When a project has "Prevent Flag Defaults" enabled, users should not see the value/enabled controls when creating features - they should only set values per-environment after creation. This project setting is now being ignored, allowing users to set default values when the project administrator specifically disabled that capability.
| // Set settingsChanged flag unless it's only metadata changing | ||
| if (changes.metadata === undefined) { | ||
| updates.settingsChanged = true | ||
| } |
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.
Settings tab changes never mark settingsChanged flag
Medium Severity
The condition changes.metadata === undefined will always be false because FeatureSettings spreads the entire projectFlag object on every change (e.g., onChange({ ...projectFlag, tags })). Since projectFlag always contains a metadata property (initialized as []), changes.metadata is never undefined. This means settingsChanged is never set to true, so the "unsaved changes" indicator won't appear on the Settings tab and users won't see the discard confirmation prompt when closing the modal after making settings changes.
Additional Locations (1)
| onRemoveMultivariateOption={ | ||
| this.props.removeMultivariateOption | ||
| } | ||
| /> |
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.
Value tab missing FeatureInPipelineGuard protection
High Severity
The FeatureValueTab component is rendered without being wrapped in FeatureInPipelineGuard. The old code wrapped the Value tab content in this guard to prevent editing feature values when the feature is part of a release pipeline, showing a fallback message "This feature is in X release pipeline and its value cannot be changed." The Segment Overrides tab still has this guard (line 943), but the Value tab lost it during the migration. This allows users to bypass release pipeline protection and edit values that should be locked.
| onProjectFlagChange({ | ||
| multivariate_options: [...multivariate_options, newVariation], | ||
| }) | ||
| } |
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.
Adding/removing variations doesn't set valueChanged flag
Medium Severity
The addVariation and handleRemoveVariation functions only call onProjectFlagChange, which in the parent component does not set valueChanged: true. In contrast, onEnvironmentFlagChange does set this flag. This means adding or removing multivariate variations won't trigger the "unsaved changes" indicator or the discard confirmation when closing the modal. The old code set valueChanged: true directly in these operations.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
How did you test this code?
E2E covers regression testing on everything this affects