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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
