fix: update Permissions for right nav #19051

AAfghahi · 2022-03-07T23:47:43Z

SUMMARY

Updating the permissions in the right nav bar to check that a user can upload csv and excel files to a database before showing them the options. Makes it so that non-Admin users only see menu items when they have a shared database.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After:
as Admin

Screen.Recording.2022-03-09.at.1.35.55.PM.mov

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

pkdotson · 2022-03-09T18:58:51Z

superset-frontend/src/views/components/MenuRight.tsx

@@ -105,19 +129,22 @@ const RightMenu = ({
          label: t('Upload CSV to database'),
          name: 'Upload a CSV',
          url: '/csvtodatabaseview/form',
-          perm: CSV_EXTENSIONS,
+          perm: CSV_EXTENSIONS && showUploads,
+          upload: true,


would these need to be variable for the upload param?

codecov · 2022-03-09T19:19:26Z

Codecov Report

Merging #19051 (5538994) into master (d693f4e) will increase coverage by 0.17%.
The diff coverage is 81.42%.

❗ Current head 5538994 differs from pull request most recent head 0d2ec22. Consider uploading reports for the commit 0d2ec22 to get more accurate results

@@            Coverage Diff             @@
##           master   #19051      +/-   ##
==========================================
+ Coverage   66.30%   66.47%   +0.17%     
==========================================
  Files        1681     1681              
  Lines       64408    64465      +57     
  Branches     6593     6607      +14     
==========================================
+ Hits        42704    42856     +152     
+ Misses      20020    19915     -105     
- Partials     1684     1694      +10

Flag	Coverage Δ
javascript	`51.15% <69.04%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/views/components/Menu.tsx	`60.00% <ø> (ø)`
superset-frontend/src/views/components/SubMenu.tsx	`85.96% <50.00%> (-1.54%)`	⬇️
...perset-frontend/src/views/components/MenuRight.tsx	`67.81% <66.66%> (+0.12%)`	⬆️
...tend/src/views/CRUD/data/database/DatabaseList.tsx	`69.23% <73.33%> (+1.28%)`	⬆️
...rset-frontend/src/dashboard/util/findPermission.ts	`100.00% <100.00%> (ø)`
superset/databases/api.py	`94.03% <100.00%> (+0.04%)`	⬆️
superset/databases/filters.py	`100.00% <100.00%> (ø)`
superset-frontend/src/views/CRUD/utils.tsx	`64.51% <0.00%> (-1.62%)`	⬇️
superset/utils/core.py	`89.83% <0.00%> (+0.11%)`	⬆️
superset/models/core.py	`89.07% <0.00%> (+0.23%)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d693f4e...0d2ec22. Read the comment docs.

pkdotson · 2022-03-09T19:30:16Z

superset-frontend/src/views/components/MenuRight.tsx

@@ -105,19 +129,19 @@ const RightMenu = ({
          label: t('Upload CSV to database'),
          name: 'Upload a CSV',
          url: '/csvtodatabaseview/form',
-          perm: CSV_EXTENSIONS,
+          perm: CSV_EXTENSIONS && showUploads,


Nice like the dual perms checking!

pkdotson · 2022-03-09T19:31:00Z

/testenv up

github-actions · 2022-03-09T19:31:39Z

@pkdotson Container image not yet published for this PR. Please try again when build is complete.

github-actions · 2022-03-09T19:31:40Z

@pkdotson Ephemeral environment creation failed. Please check the Actions logs for details.

hughhhh · 2022-03-09T19:48:57Z

superset/databases/api.py

+              $ref: '#/components/responses/500'
+        """
+        dbs_with_uploads = (
+            db.session.query(Database).filter_by(allow_file_upload=True).all()


What happens if the ORM query fails? Can we add try catch to manage what we'd return to the user?

You could prolly just wrap it in a try/catch with a SupersetException then return a SIP-40 compliant error

What would a SIP-40 compliant error be? Also it is currently failing a test with:
No fallback response defined for GET to http://localhost/api/v1/database/upload_enabled".]

seems to be in a fetchMock. Are those related?

eschutho · 2022-03-09T21:13:25Z

superset-frontend/src/views/components/MenuRight.tsx

@@ -63,6 +79,9 @@ const StyledAnchor = styled.a`

 const { SubMenu } = Menu;

+// stole this from findPermission.ts
+const ADMIN_ROLE_NAME = 'admin';


do you want to export this from findPermission.ts so that if it changes there, it will change here, too?

superset/databases/api.py

lyndsiWilliams

Found a couple of nits, nothing blocking

lyndsiWilliams · 2022-03-10T03:12:16Z

superset-frontend/src/views/components/MenuRight.tsx

+  useEffect(() => {
+    hasFileUploadEnabled();
+  }, []);


Suggested change

useEffect(() => {

hasFileUploadEnabled();

}, []);

useEffect(() => hasFileUploadEnabled(), []);

Small nit, not blocking, but this can be written on one line.

lyndsiWilliams · 2022-03-10T03:12:52Z

superset-frontend/src/views/components/MenuRight.tsx

@@ -165,14 +201,39 @@ const RightMenu = ({
    setShowModal(false);
  };

+  const isDisabled = isUserAdmin && !allowUploads;
+
+  const tooltipText = "Enable 'Allow data upload' in any database's settings";


Should this string be t() translated?

yes! Thank you.

eschutho · 2022-03-12T00:08:27Z

superset-frontend/src/views/components/MenuRight.tsx

+        selectable={false}
+        mode="horizontal"
+        onClick={handleMenuSelection}
+        onOpenChange={() => hasFileUploadEnabled()}


nit, this can just be

onOpenChange={hasFileUploadEnabled}

but also when testing it looks like this is also triggering on close, which you prob don't need.

yousoph · 2022-04-08T17:56:34Z

/testenv up

github-actions · 2022-04-08T18:00:40Z

@yousoph Ephemeral environment spinning up at http://34.217.3.104:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

…erset into ch36353_upload_menus

github-actions · 2022-04-08T19:49:14Z

Ephemeral environment shutdown and build artifacts deleted.

AAfghahi · 2022-04-08T20:11:34Z

/testenv up

github-actions · 2022-04-08T20:13:28Z

@AAfghahi Ephemeral environment spinning up at http://54.245.4.253:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

pkdotson · 2022-04-11T17:02:44Z

@AAfghahi I think we need to add these perms to the database crud view dropdown as well.

AAfghahi · 2022-04-11T17:03:55Z

@pkdotson

Screen.Recording.2022-04-11.at.12.21.14.PM.mov

pkdotson

lgtm!

github-actions · 2022-04-11T20:51:22Z

Ephemeral environment shutdown and build artifacts deleted.

* draft pr * finished styling * add filter * added testing * added tests * added permissions tests * Empty-Commit * new test * Update superset-frontend/src/views/components/MenuRight.tsx Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> * revisions * added to CRUD view Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>

superset-github-bot bot added the Superset-Community-Partners Preset community partner program participants label Mar 7, 2022

pull-request-size bot added the size/L label Mar 7, 2022

AAfghahi force-pushed the ch36353_upload_menus branch 2 times, most recently from 7d8416b to fea9336 Compare March 9, 2022 18:41

AAfghahi marked this pull request as ready for review March 9, 2022 18:43

AAfghahi force-pushed the ch36353_upload_menus branch 2 times, most recently from 36f736c to fe33eaf Compare March 9, 2022 18:49

pkdotson reviewed Mar 9, 2022

View reviewed changes

AAfghahi force-pushed the ch36353_upload_menus branch 2 times, most recently from 3f476c0 to 611b14b Compare March 9, 2022 19:18

pkdotson reviewed Mar 9, 2022

View reviewed changes

hughhhh reviewed Mar 9, 2022

View reviewed changes

eschutho reviewed Mar 9, 2022

View reviewed changes

superset/databases/api.py Outdated Show resolved Hide resolved

lyndsiWilliams reviewed Mar 10, 2022

View reviewed changes

AAfghahi force-pushed the ch36353_upload_menus branch 2 times, most recently from 5eb27e4 to 7527656 Compare March 10, 2022 16:54

lyndsiWilliams removed the Superset-Community-Partners Preset community partner program participants label Mar 10, 2022

AAfghahi force-pushed the ch36353_upload_menus branch 7 times, most recently from 2945ef1 to be027d0 Compare March 11, 2022 17:29

eschutho reviewed Mar 12, 2022

View reviewed changes

AAfghahi force-pushed the ch36353_upload_menus branch from b0a6d45 to 5d53bc8 Compare April 8, 2022 18:01

Merge branch 'ch36353_upload_menus' of ssh://github.com/preset-io/sup…

f5cea78

…erset into ch36353_upload_menus

AAfghahi force-pushed the ch36353_upload_menus branch from 5d53bc8 to f5cea78 Compare April 8, 2022 18:06

yousoph closed this Apr 8, 2022

yousoph reopened this Apr 8, 2022

AAfghahi force-pushed the ch36353_upload_menus branch from c1d1e00 to e886b54 Compare April 11, 2022 16:54

pkdotson approved these changes Apr 11, 2022

View reviewed changes

AAfghahi force-pushed the ch36353_upload_menus branch from 733d5d6 to d27a8c5 Compare April 11, 2022 17:11

added to CRUD view

75e6ee7

AAfghahi force-pushed the ch36353_upload_menus branch 3 times, most recently from b485d0d to 7bfbe43 Compare April 11, 2022 19:22

Merge branch 'master' into ch36353_upload_menus

0d2ec22

AAfghahi force-pushed the ch36353_upload_menus branch from 7bfbe43 to 0d2ec22 Compare April 11, 2022 20:08

eschutho approved these changes Apr 11, 2022

View reviewed changes

AAfghahi merged commit 4bf4d58 into apache:master Apr 11, 2022

This was referenced Apr 12, 2022

fix(database-api): allow search for all columns #19662

Merged

fix: login button does not render #19685

Merged

ktmud mentioned this pull request Apr 14, 2022

fix(nav): infinite redirect and upload dataset nav permissions #19708

Merged

9 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update Permissions for right nav #19051

fix: update Permissions for right nav #19051

AAfghahi commented Mar 7, 2022 •

edited

Loading

pkdotson Mar 9, 2022

codecov bot commented Mar 9, 2022 •

edited

Loading

pkdotson Mar 9, 2022

pkdotson commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

hughhhh Mar 9, 2022

hughhhh Mar 9, 2022

AAfghahi Mar 9, 2022

eschutho Mar 9, 2022

lyndsiWilliams left a comment

lyndsiWilliams Mar 10, 2022

lyndsiWilliams Mar 10, 2022

AAfghahi Mar 10, 2022

eschutho Mar 12, 2022

yousoph commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

AAfghahi commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

pkdotson commented Apr 11, 2022

AAfghahi commented Apr 11, 2022

pkdotson left a comment

github-actions bot commented Apr 11, 2022

fix: update Permissions for right nav #19051

fix: update Permissions for right nav #19051

Conversation

AAfghahi commented Mar 7, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

codecov bot commented Mar 9, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

pkdotson commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

github-actions bot commented Mar 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lyndsiWilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yousoph commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

AAfghahi commented Apr 8, 2022

github-actions bot commented Apr 8, 2022

pkdotson commented Apr 11, 2022

AAfghahi commented Apr 11, 2022

pkdotson left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 11, 2022

AAfghahi commented Mar 7, 2022 •

edited

Loading

codecov bot commented Mar 9, 2022 •

edited

Loading