feat(dashboard_rbac): manage roles for dashboard #13145
Conversation
|
@riahk it would be great if you could take a look at this PR! |
junlincc
added
the
dashboard:security:access
|
@simcha90 I found an issue when trying to remove all roles & save then it failed to save remove_failed.mov |
|
I've added a few developers that are more familiar with this code. These add/edit modals become quite complex over time and are frequent sources of bugs in the codebase. Would it be possible to add tests for the new functionality to help reduce the likelihood of future regressions? |
| ); | ||
| } | ||
|
|
||
| getRowsWithRoles() { |
There was a problem hiding this comment.
getRowsWithoutRoles and getRowsWithRoles are very similar. Maybe the two functions could combine into one and use DASHBOARD_RBAC feature flag to determine AsyncSelect with role show or hidden.
There was a problem hiding this comment.
I did it first... but except of add AsyncSelect it also require changes in layout, because of it I moved to separate functions only code that requires changes of layout
…ions � Conflicts: � superset-frontend/src/featureFlags.ts
…-superset into role_permissions � Conflicts: � superset-frontend/src/featureFlags.ts
|
@amitmiran137 fixed, @willbarrett added |
Codecov Report
@@ Coverage Diff @@
## master #13145 +/- ##
==========================================
+ Coverage 53.06% 58.56% +5.50%
==========================================
Files 489 478 -11
Lines 17314 16005 -1309
Branches 4482 4127 -355
==========================================
+ Hits 9187 9374 +187
+ Misses 8127 6631 -1496
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@simcha90 This change is causing the test to fail in Node 15 since in this version of Node they changed the way to handle promise rejections. In previous versions of Node, promise rejections are handled as warnings, so the test pass but that doesn't mean that the test is correct. I don't know if we support Node 15 but I'm using it in development for security reasons. Can you check what's causing the promise rejection?
|
Fixed in #13548 |
* feat(dashboard_rbac): manage roles for dashboard * test: fix tests * fix: fix empty roles Co-authored-by: Amit Miran <47772523+amitmiran137@users.noreply.github.com>
mistercrunch
added
🏷️ bot
SUMMARY
This PR add roles management to DashboardProperties modal under Feature flag
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2021-02-16.at.8.52.10.mov
TEST PLAN
ADDITIONAL INFORMATION