Skip to content
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(report): Fix permission check for set up email report on charts/dashboards. Fixes #21559 #21561

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

zhaorui2022
Copy link
Contributor

SUMMARY

set up email report button should be enabled for users who have "menu_access on Manage" permission. However, the previous implementation only checks if the first role associated with the user has the required permission, not the others. For those who were granted with the permission through other roles, they won't be able to see the button.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before
Screen Shot 2022-09-22 at 3 57 51 PM

After
Screen Shot 2022-09-21 at 10 59 51 AM

TESTING INSTRUCTIONS

I have tested with our own superset stack. With the same set of permissions, before the change, the user couldn't see the button. But after the change, the button shows up.

ADDITIONAL INFORMATION

…oards as long as they get required permissions
@mayurnewase
Copy link
Contributor

added test is failing.

@zhaorui2022
Copy link
Contributor Author

zhaorui2022 commented Sep 29, 2022

Yeah I noticed my tests were written based on an old HEAD and the way it fetches user has been changed. Fixed.

@bkyryliuk bkyryliuk merged commit 7f971b4 into apache:master Sep 29, 2022
eschutho pushed a commit to preset-io/superset that referenced this pull request Oct 18, 2022
…ashboards. Fixes apache#21559 (apache#21561)

Co-authored-by: Rui Zhao <zhaorui@dropbox.com>
(cherry picked from commit 7f971b4)
Fahrenheit35 pushed a commit to Fahrenheit35/superset that referenced this pull request Nov 11, 2022
…ashboards. Fixes apache#21559 (apache#21561)

Co-authored-by: Rui Zhao <zhaorui@dropbox.com>
(cherry picked from commit 7f971b4)
@michael-s-molina michael-s-molina added v1.5 and removed v1.5 labels Jan 3, 2023
Fahrenheit35 pushed a commit to Fahrenheit35/superset that referenced this pull request Jul 11, 2023
…ashboards. Fixes apache#21559 (apache#21561)

Co-authored-by: Rui Zhao <zhaorui@dropbox.com>
(cherry picked from commit 7f971b4)
@mistercrunch mistercrunch added 🍒 2.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M v2.0 v2.0.1 🍒 2.0.1 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants