Add scope column for categorising feature flags#9
Open
stevethomas wants to merge 3 commits into
Open
Conversation
* add scope column for categorising feature flags Adds a nullable scope string column to the features table, allowing projects to categorise flags however they want (e.g. 'development', 'release'). Supports both simple and rich config formats, with BackedEnum resolution. Scope syncs from config on every deploy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix import ordering in SyncFeaturesActionTest Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * document scope feature and upgrade steps in README Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * rewrite scope README section to focus on practical use case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * add description column, publish-only migrations, and v2 docs - Add nullable `description` column to features table for human-readable flag explanations in admin UIs - Switch to publish-only migrations (no auto-loading from vendor) - Delete redundant alter migration; single create stub is source of truth - Sync scope and description from config on every deploy (state still only set on creation) - Add `getDescription()` to FeatureFlags and facade - Add UPGRADING.md for v1 to v2 migration guide - Rewrite Feature Scopes README section to frame scope as categorisation for UIs, not runtime behaviour - Update config docblock for rich format with description Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
These accessors have no practical use case — scope and description are for bulk filtering via Eloquent, not single-feature lookups. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
||
| return [ | ||
| 'name' => $name, | ||
| 'state' => $alwaysOn ? FeatureState::on() : $config['state'], |
There was a problem hiding this comment.
Bug: The normalise method accesses keys on the $config variable without first validating that it is an array and contains required keys, risking a crash during synchronization.
Severity: HIGH
Suggested Fix
Before accessing array keys on the $config variable within the normalise method, add a validation step. Check that $config is an array and that it contains the required 'state' key. If the validation fails, throw a descriptive InvalidArgumentException to inform the user of the specific configuration error.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/Actions/SyncFeaturesAction.php#L66
Potential issue: In the `SyncFeaturesAction::normalise` method, the `$config` variable,
which comes directly from user configuration, is accessed as an array without prior
validation. If a user provides a rich-format configuration as an array but omits the
required `'state'` key, the access on line 66 (`$config['state']`) evaluates to `null`.
This `null` value is then passed downstream, causing an `InvalidArgumentException` in
the `FeatureStateCast::set` method, which crashes the entire feature flag
synchronization action. A similar fatal error occurs if the config value is not an array
at all (e.g., a string or `null`), as the code will attempt an illegal offset access.
Did we get this right? 👍 / 👎 to inform future reviews.
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
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.
Summary
Re-opened from #6. Originally merged but reverted from main to keep v1.x clean — this is targeted at v2.
scopeanddescriptioncolumns to the features tableFeatureStatevaluesTest plan