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 2020/07/24 22:09:33 UTC
[GitHub] [incubator-superset] rusackas commented on a change in pull request #10335: feat(listviews): SIP-34 filters for charts, dashboards, datasets
rusackas commented on a change in pull request #10335:
URL: https://github.com/apache/incubator-superset/pull/10335#discussion_r460288698
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ database_name: label, id: value }: Database) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
Review comment:
In cases like this, I'd be A-OK with a one-line `eslint-ignore` here so we stop getting the warnings about completely intentional console errors.
##########
File path: superset-frontend/src/views/CRUD/chart/ChartList.tsx
##########
@@ -49,16 +43,31 @@ interface State {
bulkSelectEnabled: boolean;
chartCount: number;
charts: any[];
- filterOperators: FilterOperatorMap;
- filters: Filters;
lastFetchDataConfig: FetchDataConfig | null;
loading: boolean;
permissions: string[];
// for now we need to use the Slice type defined in PropertiesModal.
// In future it would be better to have a unified Chart entity.
sliceCurrentlyEditing: Slice | null;
}
+const createFetchDatasets = (
+ handleError: (err: Response) => void,
+) => async () => {
+ const resource = '/api/v1/chart/datasources';
+ try {
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resource}`,
Review comment:
```suggestion
endpoint: resource,
```
Actually, you might not even need the variable, unless you like it for cleanliness. Could just be
```
endpoint: '/api/v1/chart/datasources',
```
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
Review comment:
Same totally optional nit, but you could simplify with
```
endpoint: `/api/v1/database/?q=${queryParams}`,
```
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
Review comment:
I still get a little excited when I see or use these operators.
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ database_name: label, id: value }: Database) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
+ }
+ return [];
+};
+export const createFetchSchemas = (
+ handleError: (error: Response) => void,
+) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ const resourceEndpoint = `/api/v1/database/schemas/`;
+
+ try {
+ const queryParams = rison.encode({
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ text: label, value }: { text: string; value: any }) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
+ }
+ return [];
+};
const DatasetList: FunctionComponent<DatasetListProps> = ({
addDangerToast,
addSuccessToast,
}) => {
- const [databases, setDatabases] = useState<{ text: string; value: number }[]>(
- [],
- );
const [datasetCount, setDatasetCount] = useState(0);
const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState<
(Dataset & { chart_count: number; dashboard_count: number }) | null
>(null);
const [datasets, setDatasets] = useState<any[]>([]);
- const [currentFilters, setCurrentFilters] = useState<Filters>([]);
- const [filterOperators, setFilterOperators] = useState<FilterOperatorMap>();
const [
lastFetchDataConfig,
setLastFetchDataConfig,
] = useState<FetchDataConfig | null>(null);
const [loading, setLoading] = useState(true);
- const [currentOwners, setCurrentOwners] = useState<
- { text: string; value: number }[]
- >([]);
const [permissions, setPermissions] = useState<string[]>([]);
const [datasetAddModalOpen, setDatasetAddModalOpen] = useState<boolean>(
false,
);
const [bulkSelectEnabled, setBulkSelectEnabled] = useState<boolean>(false);
- const updateFilters = () => {
- const convertFilter = ({
- name: label,
- operator,
- }: {
- name: string;
- operator: string;
- }) => ({ label, value: operator });
- if (filterOperators) {
- setCurrentFilters([
- {
- Header: 'Database',
- id: 'database',
- input: 'select',
- operators: filterOperators.database.map(convertFilter),
- selects: databases.map(({ text: label, value }) => ({
- label,
- value,
- })),
- },
- {
- Header: 'Schema',
- id: 'schema',
- operators: filterOperators.schema.map(convertFilter),
- },
- {
- Header: 'Table Name',
- id: 'table_name',
- operators: filterOperators.table_name.map(convertFilter),
- },
- {
- Header: 'Owners',
- id: 'owners',
- input: 'select',
- operators: filterOperators.owners.map(convertFilter),
- selects: currentOwners.map(({ text: label, value }) => ({
- label,
- value,
- })),
- },
- {
- Header: 'SQL Lab View',
- id: 'is_sqllab_view',
- input: 'checkbox',
- operators: filterOperators.is_sqllab_view.map(convertFilter),
- },
- ]);
- }
- };
+ const filterTypes: Filters = [
+ {
+ Header: t('Owner'),
+ id: 'owners',
+ input: 'select',
+ operator: 'rel_m_m',
+ unfilteredLabel: 'All',
+ fetchSelects: createFetchOwners('dataset', e =>
+ addDangerToast(
+ t(
+ 'An error occurred while fetching databaser owner values: %s',
Review comment:
```suggestion
'An error occurred while fetching database owner values: %s',
```
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ database_name: label, id: value }: Database) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
+ }
+ return [];
+};
+export const createFetchSchemas = (
+ handleError: (error: Response) => void,
+) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ const resourceEndpoint = `/api/v1/database/schemas/`;
+
+ try {
+ const queryParams = rison.encode({
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ text: label, value }: { text: string; value: any }) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
+ }
+ return [];
+};
const DatasetList: FunctionComponent<DatasetListProps> = ({
addDangerToast,
addSuccessToast,
}) => {
- const [databases, setDatabases] = useState<{ text: string; value: number }[]>(
- [],
- );
const [datasetCount, setDatasetCount] = useState(0);
const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState<
(Dataset & { chart_count: number; dashboard_count: number }) | null
>(null);
const [datasets, setDatasets] = useState<any[]>([]);
- const [currentFilters, setCurrentFilters] = useState<Filters>([]);
- const [filterOperators, setFilterOperators] = useState<FilterOperatorMap>();
const [
lastFetchDataConfig,
setLastFetchDataConfig,
] = useState<FetchDataConfig | null>(null);
const [loading, setLoading] = useState(true);
- const [currentOwners, setCurrentOwners] = useState<
- { text: string; value: number }[]
- >([]);
const [permissions, setPermissions] = useState<string[]>([]);
const [datasetAddModalOpen, setDatasetAddModalOpen] = useState<boolean>(
false,
);
const [bulkSelectEnabled, setBulkSelectEnabled] = useState<boolean>(false);
- const updateFilters = () => {
- const convertFilter = ({
- name: label,
- operator,
- }: {
- name: string;
- operator: string;
- }) => ({ label, value: operator });
- if (filterOperators) {
- setCurrentFilters([
- {
- Header: 'Database',
- id: 'database',
- input: 'select',
- operators: filterOperators.database.map(convertFilter),
- selects: databases.map(({ text: label, value }) => ({
- label,
- value,
- })),
- },
- {
- Header: 'Schema',
- id: 'schema',
- operators: filterOperators.schema.map(convertFilter),
- },
- {
- Header: 'Table Name',
- id: 'table_name',
- operators: filterOperators.table_name.map(convertFilter),
- },
- {
- Header: 'Owners',
- id: 'owners',
- input: 'select',
- operators: filterOperators.owners.map(convertFilter),
- selects: currentOwners.map(({ text: label, value }) => ({
- label,
- value,
- })),
- },
- {
- Header: 'SQL Lab View',
- id: 'is_sqllab_view',
- input: 'checkbox',
- operators: filterOperators.is_sqllab_view.map(convertFilter),
- },
- ]);
- }
- };
+ const filterTypes: Filters = [
+ {
+ Header: t('Owner'),
+ id: 'owners',
+ input: 'select',
+ operator: 'rel_m_m',
+ unfilteredLabel: 'All',
+ fetchSelects: createFetchOwners('dataset', e =>
+ addDangerToast(
+ t(
+ 'An error occurred while fetching databaser owner values: %s',
Review comment:
```suggestion
'An error occurred while fetching database owners: %s',
```
##########
File path: superset-frontend/src/views/CRUD/utils.tsx
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { SupersetClient } from '@superset-ui/connection';
+import rison from 'rison';
+
+export const createFetchOwners = (
+ resource: string,
+ handleError: (error: Response) => void,
+) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ const resourceEndpoint = `/api/v1/${resource}/related/owners`;
Review comment:
Just curious if this is worth abstracting for any foreseeable future use, e.g. `createFetchRelatedModel(resource, relation)`
...
```const resourceEndpoint = `/api/v1/${resource}/related/${relation}/`;```
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ database_name: label, id: value }: Database) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
Review comment:
...and/or having the handleError method do that for you.
##########
File path: superset-frontend/src/views/CRUD/dataset/DatasetList.tsx
##########
@@ -69,128 +66,191 @@ interface DatasetListProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
}
+interface Database {
+ allow_csv_upload: boolean;
+ allow_ctas: boolean;
+ allow_cvas: null | boolean;
+ allow_dml: boolean;
+ allow_multi_schema_metadata_fetch: boolean;
+ allow_run_async: boolean;
+ allows_cost_estimate: boolean;
+ allows_subquery: boolean;
+ allows_virtual_table_explore: boolean;
+ backend: string;
+ database_name: string;
+ explore_database_id: number;
+ expose_in_sqllab: boolean;
+ force_ctas_schema: null | boolean;
+ function_names: string[];
+ id: number;
+}
+
+const createFetchDatabases = (handleError: (err: Response) => void) => async (
+ filterValue = '',
+ pageIndex?: number,
+ pageSize?: number,
+) => {
+ const resourceEndpoint = `/api/v1/database/`;
+
+ try {
+ const queryParams = rison.encode({
+ columns: ['database_name', 'id'],
+ keys: ['none'],
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ database_name: label, id: value }: Database) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
+ }
+ return [];
+};
+export const createFetchSchemas = (
+ handleError: (error: Response) => void,
+) => async (filterValue = '', pageIndex?: number, pageSize?: number) => {
+ const resourceEndpoint = `/api/v1/database/schemas/`;
+
+ try {
+ const queryParams = rison.encode({
+ ...(pageIndex ? { page: pageIndex } : {}),
+ ...(pageSize ? { page_ize: pageSize } : {}),
+ ...(filterValue ? { filter: filterValue } : {}),
+ });
+ const { json = {} } = await SupersetClient.get({
+ endpoint: `${resourceEndpoint}?q=${queryParams}`,
+ });
+
+ return json?.result?.map(
+ ({ text: label, value }: { text: string; value: any }) => ({
+ label,
+ value,
+ }),
+ );
+ } catch (e) {
+ console.error(e);
+ handleError(e);
Review comment:
Again, the handleError method could spit out the console error rather than just swallowing the error.
----------------------------------------------------------------
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.
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