-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
fix: Revert "fix(explore): let admin overwrite slice" #16408
Conversation
This reverts commit d13b081.
Note that reverting this will probably bring some of the bugs that it was intended to solve. There were some issues with (Admin) users seeing 403/404 errors when the clicking Save/Overwrite button on charts they did not own. This fix made it actually work, when perhaps the more prudent approach would be to figure out why the action was enabled in the first place, and find a more proper means to disable that Overwrite action. More research is warranted. |
Codecov Report
@@ Coverage Diff @@
## master #16408 +/- ##
==========================================
+ Coverage 76.33% 76.42% +0.09%
==========================================
Files 1000 1000
Lines 53483 53496 +13
Branches 6818 6816 -2
==========================================
+ Hits 40827 40886 +59
+ Misses 12418 12374 -44
+ Partials 238 236 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
I agree that this probably needs to be discussed more before introducing a change such as this (what first appeared to be a bug in hindsight makes sense to restrict admins from accidentally overwriting other people's charts). I'll continue to work on figuring out what may be the root cause of the 403/404 issue.
* upstream/master: (25 commits) chore(ci): bump pylint to 2.10.2 (apache#16463) fix: prevent page crash when chart can't render (apache#16464) chore: fixed slack invite link (apache#16466) fix(native-filters): handle null values in value filter (apache#16460) feat: add function list to auto-complete to Clickhouse datasource (apache#16234) refactor(explore): improve typing for Dnd controls (apache#16362) fix(explore): update overwrite button on perm change (apache#16437) feat: Draggable and Resizable Modal (apache#16394) refactor: sql_json view endpoint (apache#16441) fix(dashboard): undo and redo buttons weird alignment (apache#16417) fix: setupPlugin in chart list page (apache#16413) fix: Disable Slack notification method if no api token (apache#16367) feat: add Shillelagh DB engine spec (apache#16416) fix: copy to Clipboard order (apache#16299) docs: make FEATURE_FLAGS.md reference a link (apache#16415) chore(viz): bump superset-ui to 0.17.87 (apache#16420) feat: add activate command (apache#16404) Revert "fix(explore): let admin overwrite slice (apache#16290)" (apache#16408) fix(explore): retain chart ownership on query context update (apache#16419) chore: Removes the TODOs and uses the default page size (apache#16422) ...
Reverts #16290
This PR gives Admin users a capability they did not have before, to meddle with another users' work. I think we need to discuss this a little more broadly and gain some degree of consensus before merging it, that we want to allow Admin users to have these added capabilities/privileges.