You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/08 19:49:25 UTC
[GitHub] [superset] AAfghahi opened a new pull request, #19051: fix: update Permissions for right nav
AAfghahi opened a new pull request, #19051:
URL: https://github.com/apache/superset/pull/19051
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
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
<!--- Skip this if not applicable -->
After:
as Admin
https://user-images.githubusercontent.com/48933336/157509633-a4502319-c2e7-41f5-b45a-07a1918d3d6e.mov
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19051:
URL: https://github.com/apache/superset/pull/19051#discussion_r844105026
##########
superset-frontend/src/views/components/MenuRight.tsx:
##########
@@ -175,14 +212,48 @@ const RightMenu = ({
setShowModal(false);
};
+ const isDisabled = isAdmin && !allowUploads;
+
+ const tooltipText = t(
+ "Enable 'Allow data upload' in any database's settings",
+ );
+
+ const buildMenuItem = (item: Record<string, any>) => {
Review Comment:
awesome! so glad you split this out
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] yousoph closed pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
yousoph closed pull request #19051: fix: update Permissions for right nav
URL: https://github.com/apache/superset/pull/19051
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1095553723
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] yousoph commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1093140391
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
pkdotson commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1092272504
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1092275281
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1093315027
@AAfghahi Ephemeral environment spinning up at http://54.245.4.253:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1093313876
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19051:
URL: https://github.com/apache/superset/pull/19051#discussion_r846281850
##########
superset/databases/filters.py:
##########
@@ -51,3 +56,56 @@ def apply(self, query: Query, value: Any) -> Query:
),
)
)
+
+
+class DatabaseUploadEnabledFilter(BaseFilter):
+ """
+ Custom filter for the GET list that filters all databases based on allow_file_upload
+ """
+
+ name = _("Upload Enabled")
+ arg_name = "upload_is_enabled"
+
+ def can_access_databases( # noqa pylint: disable=no-self-use
Review Comment:
this is a dupe from above. Can we can dry this up?
##########
superset-frontend/src/views/components/MenuRight.tsx:
##########
@@ -175,14 +212,48 @@ const RightMenu = ({
setShowModal(false);
};
+ const isDisabled = isAdmin && !allowUploads;
+
+ const tooltipText = t(
+ "Enable 'Allow data upload' in any database's settings",
+ );
+
+ const buildMenuItem = (item: Record<string, any>) => {
+ const disabledText = isDisabled && item.url;
+ return disabledText ? (
+ <Menu.Item key={item.name} css={styledDisabled}>
+ <Tooltip placement="top" title={tooltipText}>
+ {item.label}
+ </Tooltip>
+ </Menu.Item>
+ ) : (
+ <Menu.Item key={item.name}>
+ {item.url ? <a href={item.url}> {item.label} </a> : item.label}
+ </Menu.Item>
+ );
+ };
+
+ const onMenuOpen = (openKeys: string[]) => {
+ if (openKeys.length) {
+ return hasFileUploadEnabled();
+ }
+ return null;
+ };
+
return (
<StyledDiv align={align}>
<DatabaseModal
onHide={handleOnHideModal}
show={showModal}
+ onDatabaseAdd={() => {}}
dbEngine={engine}
/>
- <Menu selectable={false} mode="horizontal" onClick={handleMenuSelection}>
+ <Menu
+ selectable={false}
+ mode="horizontal"
+ onClick={handleMenuSelection}
+ onOpenChange={(openKeys: string[]) => onMenuOpen(openKeys)}
Review Comment:
It looks like you could just pass in the function here, instead of a new function.
```suggestion
onOpenChange={onMenuOpen}
```
##########
superset/databases/filters.py:
##########
@@ -51,3 +56,56 @@ def apply(self, query: Query, value: Any) -> Query:
),
)
)
+
+
+class DatabaseUploadEnabledFilter(BaseFilter):
+ """
+ Custom filter for the GET list that filters all databases based on allow_file_upload
+ """
+
+ name = _("Upload Enabled")
+ arg_name = "upload_is_enabled"
+
+ def can_access_databases( # noqa pylint: disable=no-self-use
+ self,
+ view_menu_name: str,
+ ) -> Set[str]:
+ return {
+ security_manager.unpack_database_and_schema(vm).database
+ for vm in security_manager.user_view_menu_names(view_menu_name)
+ }
+
+ def apply(self, query: Query, value: Any) -> Query:
+
+ filtered_query = query.filter(Database.allow_file_upload)
+
+ database_perms = security_manager.user_view_menu_names("database_access")
+ schema_access_databases = self.can_access_databases("schema_access")
+
+ datasource_access_databases = self.can_access_databases("datasource_access")
+
+ if hasattr(g, "user"):
+ allowed_schemas = [
+ app.config["ALLOWED_USER_CSV_SCHEMA_FUNC"](db, g.user)
+ for db in datasource_access_databases
+ ]
+
+ if len(allowed_schemas):
+ return filtered_query
+
+ schema_filtered_query = filtered_query.filter(
Review Comment:
Reading this in context now, we could probably reuse the filtered_query var here instead of creating a new one. The new var helps explain what's being added, but then it's reused again below in the perm filters, so it's not consistent. Adding some comments could be another option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] pkdotson commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
pkdotson commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1095307611
@AAfghahi I think we need to add these perms to the database crud view dropdown as well.
<img width="273" alt="Screen Shot 2022-04-11 at 10 02 12 AM" src="https://user-images.githubusercontent.com/17326228/162792494-9aba94d2-f8c4-4f15-b4e0-384c9d9124e3.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1093143798
@yousoph Ephemeral environment spinning up at http://34.217.3.104:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1092273412
@pkdotson Ephemeral environment spinning up at http://54.149.44.142:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1095308824
@pkdotson
https://user-images.githubusercontent.com/48933336/162792645-b2a7b4d7-5355-4065-baf2-cdf64c5e4835.mov
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi merged pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
AAfghahi merged PR #19051:
URL: https://github.com/apache/superset/pull/19051
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1092277732
@AAfghahi Ephemeral environment spinning up at http://54.202.210.52:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #19051:
URL: https://github.com/apache/superset/pull/19051#discussion_r846279162
##########
superset-frontend/src/views/components/MenuRight.tsx:
##########
@@ -175,14 +212,48 @@ const RightMenu = ({
setShowModal(false);
};
+ const isDisabled = isAdmin && !allowUploads;
+
+ const tooltipText = t(
+ "Enable 'Allow data upload' in any database's settings",
+ );
+
+ const buildMenuItem = (item: Record<string, any>) => {
+ const disabledText = isDisabled && item.url;
+ return disabledText ? (
+ <Menu.Item key={item.name} css={styledDisabled}>
+ <Tooltip placement="top" title={tooltipText}>
+ {item.label}
+ </Tooltip>
+ </Menu.Item>
+ ) : (
+ <Menu.Item key={item.name}>
+ {item.url ? <a href={item.url}> {item.label} </a> : item.label}
+ </Menu.Item>
+ );
+ };
+
+ const onMenuOpen = (openKeys: string[]) => {
+ if (openKeys.length) {
+ return hasFileUploadEnabled();
+ }
+ return null;
+ };
+
return (
<StyledDiv align={align}>
<DatabaseModal
onHide={handleOnHideModal}
show={showModal}
+ onDatabaseAdd={() => {}}
Review Comment:
can we make this prop optional instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a diff in pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #19051:
URL: https://github.com/apache/superset/pull/19051#discussion_r846355430
##########
superset-frontend/src/views/components/MenuRight.tsx:
##########
@@ -175,14 +212,48 @@ const RightMenu = ({
setShowModal(false);
};
+ const isDisabled = isAdmin && !allowUploads;
+
+ const tooltipText = t(
+ "Enable 'Allow data upload' in any database's settings",
+ );
+
+ const buildMenuItem = (item: Record<string, any>) => {
+ const disabledText = isDisabled && item.url;
+ return disabledText ? (
+ <Menu.Item key={item.name} css={styledDisabled}>
+ <Tooltip placement="top" title={tooltipText}>
+ {item.label}
+ </Tooltip>
+ </Menu.Item>
+ ) : (
+ <Menu.Item key={item.name}>
+ {item.url ? <a href={item.url}> {item.label} </a> : item.label}
+ </Menu.Item>
+ );
+ };
+
+ const onMenuOpen = (openKeys: string[]) => {
+ if (openKeys.length) {
+ return hasFileUploadEnabled();
+ }
+ return null;
+ };
+
return (
<StyledDiv align={align}>
<DatabaseModal
onHide={handleOnHideModal}
show={showModal}
+ onDatabaseAdd={() => {}}
Review Comment:
it already was optional so I just deleted it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #19051: fix: update Permissions for right nav
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19051:
URL: https://github.com/apache/superset/pull/19051#issuecomment-1093299509
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org