-
Notifications
You must be signed in to change notification settings - Fork 76
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
feat: enabled undeploy for users by removing feature flag #14640
Conversation
📝 WalkthroughWalkthroughThis change removes unnecessary UI elements and test cases related to deployment environment display. Two tests in the log list component were removed, and the UI was simplified by deleting a heading wrapper and a secondary feature flag condition. Additionally, an unused localization entry for deployed version history was eliminated and the "Undeploy" feature flag was removed from the enum. No public or exported entity declarations were altered. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14640 +/- ##
==========================================
- Coverage 95.75% 95.75% -0.01%
==========================================
Files 1912 1912
Lines 24918 24917 -1
Branches 2848 2848
==========================================
- Hits 23861 23860 -1
Misses 799 799
Partials 258 258 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/app-development/features/appPublish/components/DeploymentEnvironmentLogList.tsx (1)
49-49
: Consider using undefined instead of empty string for spinnerTitle.While the current implementation works, using
undefined
instead of an empty string forspinnerTitle
would be more semantically correct since you're explicitly hiding the title withshowSpinnerTitle={false}
.- return <StudioSpinner size='small' spinnerTitle='' showSpinnerTitle={false} />; + return <StudioSpinner size='small' spinnerTitle={undefined} showSpinnerTitle={false} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/app-development/features/appPublish/components/DeploymentEnvironmentLogList.test.tsx
(0 hunks)frontend/app-development/features/appPublish/components/DeploymentEnvironmentLogList.tsx
(3 hunks)frontend/app-development/features/appPublish/components/DeploymentEnvironmentStatus.tsx
(1 hunks)frontend/language/src/nb.json
(0 hunks)frontend/packages/shared/src/utils/featureToggleUtils.ts
(0 hunks)
💤 Files with no reviewable changes (3)
- frontend/packages/shared/src/utils/featureToggleUtils.ts
- frontend/app-development/features/appPublish/components/DeploymentEnvironmentLogList.test.tsx
- frontend/language/src/nb.json
🔇 Additional comments (4)
frontend/app-development/features/appPublish/components/DeploymentEnvironmentLogList.tsx (3)
3-3
: LGTM!The import statement is correctly formatted and the imported components are used appropriately in the code.
60-128
: LGTM! Well-structured table implementation.The table implementation follows best practices with:
- Proper semantic markup using Table components
- Consistent styling through CSS classes
- Accessible header cells and data cells
135-141
: LGTM! Clean and focused helper function.The function follows good practices:
- Single responsibility principle
- Clear conditional logic
- Proper string interpolation for i18n keys
frontend/app-development/features/appPublish/components/DeploymentEnvironmentStatus.tsx (1)
51-53
: LGTM! Please verify the feature flag removal.The code changes look good and align with the PR objective to enable undeploy for users. The conditional rendering is safe with the optional chaining operator.
Please verify:
- The removal of the
Undeploy
feature flag was intentional across all environments.- There are no environment-specific restrictions for the undeploy feature.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining references to the Undeploy feature flag. # Search for any remaining references to the Undeploy feature flag rg -i "undeploy.*feature" -g "!{*.md,*.json}" # Search for any environment-specific configuration related to undeploy rg -i "undeploy.*env" -g "!{*.md,*.json}"Length of output: 6092
Feature Flag Removal Verification
The removal of the feature flag check in the
DeploymentEnvironmentStatus.tsx
file is consistent with the PR’s objective to enable undeploy for users. There are no leftover conditional checks in that file, and our search did not reveal any environment-specific restrictions for the undeploy feature in this context.
- The code now renders the
DeployMoreOptionsMenu
based solely on the existence of a deployment version.- No feature flag references guarding the undeploy functionality remain in this file.
- Other parts of the code (tests and backend services) reference undeploy functionality, confirming that the feature itself is intact and likely governed elsewhere if needed.
frontend/app-development/features/appPublish/components/DeploymentEnvironmentStatus.tsx
Show resolved
Hide resolved
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.
🥳
Description
Removed feature-flag for un-deploy
Related Issue(s)
Verification
Documentation