-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[WIP] feat: add feature flag for CSS Templates #33675
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: master
Are you sure you want to change the base?
[WIP] feat: add feature flag for CSS Templates #33675
Conversation
This is a rough first draft at this feature, doesn't have tests or screenshots yet, but I'll try to add those this week. Would definitely appreciate feedback from the team! |
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.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
Category | Issue | Status |
---|---|---|
Hard to read type annotation ▹ view | 🧠 Not in scope | |
Redundant Feature Flag Checks ▹ view | 🧠 Not in standard | |
Missing Permission Check for CSS Editor ▹ view | 🧠 Not in standard | |
Incorrect HTTP status code for disabled feature ▹ view | 🧠 Incorrect |
Files scanned
File Path | Reviewed |
---|---|
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts | ✅ |
superset/css_templates/api.py | ✅ |
superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx | ✅ |
superset/initialization/init.py | ✅ |
superset/config.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
@before_request() | ||
def ensure_css_templates_enabled(self) -> Optional[Response]: | ||
css_templates_enabled = is_feature_enabled("ENABLE_CSS_TEMPLATES") | ||
if not css_templates_enabled: | ||
return self.response_404("CSS templates are not enabled.") | ||
return None |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@before_request() | ||
def ensure_css_templates_enabled(self) -> Optional[Response]: | ||
css_templates_enabled = is_feature_enabled("ENABLE_CSS_TEMPLATES") | ||
if not css_templates_enabled: | ||
return self.response_404("CSS templates are not enabled.") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
SQLLAB_CTAS_SCHEMA_NAME_FUNC: None | ( | ||
Callable[[Database, models.User, str, str], str] | ||
) = None |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@@ -192,7 +192,7 @@ export const useHeaderActionsMenu = ({ | |||
{t('Edit properties')} | |||
</Menu.Item> | |||
)} | |||
{editMode && ( | |||
{editMode && isFeatureEnabled(FeatureFlag.EnableCssTemplates) && ( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #33675 +/- ##
===========================================
+ Coverage 60.48% 83.04% +22.56%
===========================================
Files 1931 558 -1373
Lines 76236 40996 -35240
Branches 8568 0 -8568
===========================================
- Hits 46114 34047 -12067
+ Misses 28017 6949 -21068
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Since this is a WIP still, going to convert this to a draft until it's ready. |
Sounds good to me! @dpgaspar may want to assess the security implications. |
SUMMARY
At Yahoo, one of our security personnel flagged this feature as a potential security issue:
Additionally, there was a discussion around this feature: #296
I added a feature flag (
ENABLE_CSS_TEMPLATES
) that will allow users to disable CSS templates. The feature remains enabled by default.There is a flag check in the Menu builder, a
@before_request()
hook in front of the CssTemplates API endpoints, and flag checks inuseHeaderActionsDropdownMenu
.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
(In Progress)
TESTING INSTRUCTIONS
(In Progress)
ADDITIONAL INFORMATION
ENABLE_CSS_TEMPLATES