fix(Features): add remove option to feature settings panel - #8203
fix(Features): add remove option to feature settings panel#8203bardock-2393 wants to merge 2 commits into
Conversation
The Settings tab only exposed an Archived toggle, which looks like the way to delete a feature but isn't. Add a "Remove Feature" action that reuses the existing ConfirmRemoveFeature flow already used from the features list.
|
@bardock-2393 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
front-end
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 66a505cc-eacf-4bec-bc95-d131aa92cd27
📒 Files selected for processing (1)
frontend/web/components/modals/create-feature/tabs/FeatureSettingsTab.tsx
The tab returned early behind the CREATE_FEATURE permission check before ever rendering the Remove Feature button added in the previous commit, so a user granted only DELETE_FEATURE (a distinct, independently grantable project permission) could never reach it. Broaden the gate to render for either permission, keeping the editable settings themselves scoped to CREATE_FEATURE as before.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/web/components/modals/create-feature/tabs/FeatureSettingsTab.tsx (1)
136-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the repeated permission
InfoMessageblock.The same
InfoMessagewithdangerouslySetInnerHTML: Constants.projectPermissions(ProjectPermission.CREATE_FEATURE)now appears twice: once as the top-level early return (Lines 140-146) and once inline for the!createFeaturebranch (Lines 158-164). Extract this block into a small helper or local component to avoid divergence between the two copies.♻️ Proposed refactor
+const CreateFeaturePermissionMessage: FC = () => ( + <InfoMessage> + <div + dangerouslySetInnerHTML={{ + __html: Constants.projectPermissions(ProjectPermission.CREATE_FEATURE), + }} + /> + </InfoMessage> +) + const FeatureSettingsTab: FC<FeatureSettingsTabProps> = ({ ... }) => { ... if (!createFeature && !deleteFeature) { - return ( - <InfoMessage> - <div - dangerouslySetInnerHTML={{ - __html: Constants.projectPermissions( - ProjectPermission.CREATE_FEATURE, - ), - }} - /> - </InfoMessage> - ) + return <CreateFeaturePermissionMessage /> } ... return ( <div className={`${identity ? 'mx-3' : ''}`}> - {!createFeature && ( - <InfoMessage> - <div - dangerouslySetInnerHTML={{ - __html: Constants.projectPermissions( - ProjectPermission.CREATE_FEATURE, - ), - }} - /> - </InfoMessage> - )} + {!createFeature && <CreateFeaturePermissionMessage />}Also applies to: 156-166
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c2cfe433-d874-4433-82a9-30476e92eb64
📒 Files selected for processing (1)
frontend/web/components/modals/create-feature/tabs/FeatureSettingsTab.tsx
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #7896
The Settings tab of the create/edit-feature modal only exposed an "Archived" toggle, which looks like the way to delete a feature but isn't — deletion previously only existed in the features list's kebab menu.
Added a "Remove Feature" action to the Settings tab, gated behind the existing
DELETE_FEATUREproject permission, that reuses the sameConfirmRemoveFeatureconfirmation modal and removal mutation already used from the features list, closing the enclosing modal on success.How did you test this code?
No existing Jest/RTL test coverage exists for this component or its siblings in
components/modals/create-feature/, so I didn't introduce new test scaffolding for it in isolation. Rannpx eslint --fixon the touched file (clean) andnpm run typecheck, confirming an identical error count with and without this change (979, all pre-existing/unrelated) — so this introduces zero new type errors.