This is an automated email from the ASF dual-hosted git repository. tai pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push: new 6f56cd5 feat(listviews): SIP-34 filters for charts, dashboards, datasets (#10335) 6f56cd5 is described below commit 6f56cd5e9d18ab8c89f282551b349ccf8da99dc1 Author: ʈᵃᵢ <tdupree...@gmail.com> AuthorDate: Mon Jul 27 10:14:11 2020 -0700 feat(listviews): SIP-34 filters for charts, dashboards, datasets (#10335) --- .../components/ListView/ListView_spec.jsx | 233 ++++++---------- .../{chartList => CRUD/chart}/ChartList_spec.jsx | 14 +- .../dashboard}/DashboardList_spec.jsx | 13 +- .../dataset}/DatasetList_spec.jsx | 14 +- .../ErrorMessage/TimeoutErrorMessage.tsx | 2 +- .../src/components/ListView/LegacyFilters.tsx | 204 -------------- .../src/components/ListView/ListView.tsx | 35 +-- superset-frontend/src/components/ListView/types.ts | 30 +- superset-frontend/src/components/ListView/utils.ts | 23 -- superset-frontend/src/components/Modal.tsx | 2 +- superset-frontend/src/featureFlags.ts | 1 - .../views/{chartList => CRUD/chart}/ChartList.tsx | 303 +++++++-------------- .../dashboard}/DashboardList.tsx | 271 ++++++------------ .../dataset}/AddDatasetModal.tsx | 12 +- .../views/{datasetList => CRUD/dataset}/Button.tsx | 0 .../{datasetList => CRUD/dataset}/DatasetList.tsx | 302 +++++++++++--------- superset-frontend/src/views/CRUD/utils.tsx | 61 +++++ superset-frontend/src/welcome/App.tsx | 6 +- superset/config.py | 1 - superset/databases/api.py | 90 +++++- superset/databases/schemas.py | 10 + tests/database_api_tests.py | 39 +++ 22 files changed, 678 insertions(+), 988 deletions(-) diff --git a/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx b/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx index de0d179..cd1f823 100644 --- a/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx +++ b/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx @@ -19,7 +19,6 @@ import React from 'react'; import { mount, shallow } from 'enzyme'; import { act } from 'react-dom/test-utils'; -import { MenuItem } from 'react-bootstrap'; import { QueryParamProvider } from 'use-query-params'; import { supersetTheme, ThemeProvider } from '@superset-ui/style'; @@ -42,6 +41,7 @@ function makeMockLocation(query) { }; } +const fetchSelectsMock = jest.fn(() => []); const mockedProps = { title: 'Data Table', columns: [ @@ -61,9 +61,25 @@ const mockedProps = { ], filters: [ { + Header: 'ID', + id: 'id', + input: 'select', + selects: [{ label: 'foo', value: 'bar' }], + operator: 'eq', + }, + { Header: 'Name', id: 'name', - operators: [{ label: 'Starts With', value: 'sw' }], + input: 'search', + operator: 'ct', + }, + { + Header: 'Age', + id: 'age', + input: 'select', + fetchSelects: fetchSelectsMock, + paginate: true, + operator: 'eq', }, ], data: [ @@ -145,59 +161,6 @@ describe('ListView', () => { `); }); - it('calls fetchData on filter', () => { - act(() => { - wrapper - .find('.dropdown-toggle') - .children('button') - .at(0) - .props() - .onClick(); - - wrapper - .find(MenuItem) - .at(0) - .props() - .onSelect({ id: 'name', Header: 'name' }); - }); - wrapper.update(); - - act(() => { - wrapper.find('.filter-inputs input[type="text"]').prop('onChange')({ - persist() {}, - currentTarget: { value: 'foo' }, - }); - }); - wrapper.update(); - - act(() => { - wrapper.find('[data-test="apply-filters"]').last().prop('onClick')(); - }); - wrapper.update(); - - expect(mockedProps.fetchData.mock.calls[0]).toMatchInlineSnapshot(` -Array [ - Object { - "filters": Array [ - Object { - "id": "name", - "operator": "sw", - "value": "foo", - }, - ], - "pageIndex": 0, - "pageSize": 1, - "sortBy": Array [ - Object { - "desc": false, - "id": "id", - }, - ], - }, -] -`); - }); - it('renders pagination controls', () => { expect(wrapper.find(Pagination).exists()).toBe(true); expect(wrapper.find(Pagination.Prev).exists()).toBe(true); @@ -212,26 +175,20 @@ Array [ wrapper.update(); expect(mockedProps.fetchData.mock.calls[0]).toMatchInlineSnapshot(` -Array [ - Object { - "filters": Array [ - Object { - "id": "name", - "operator": "sw", - "value": "foo", - }, - ], - "pageIndex": 1, - "pageSize": 1, - "sortBy": Array [ - Object { - "desc": false, - "id": "id", - }, - ], - }, -] -`); + Array [ + Object { + "filters": Array [], + "pageIndex": 1, + "pageSize": 1, + "sortBy": Array [ + Object { + "desc": false, + "id": "id", + }, + ], + }, + ] + `); }); it('handles bulk actions on 1 row', () => { @@ -339,46 +296,6 @@ Array [ '"Invalid filter config, some_column is not present in columns"', ); }); -}); - -describe('ListView with new UI filters', () => { - const fetchSelectsMock = jest.fn(() => []); - const newFiltersProps = { - ...mockedProps, - isSIP34FilterUIEnabled: true, - filters: [ - { - Header: 'ID', - id: 'id', - input: 'select', - selects: [{ label: 'foo', value: 'bar' }], - operator: 'eq', - }, - { - Header: 'Name', - id: 'name', - input: 'search', - operator: 'ct', - }, - { - Header: 'Age', - id: 'age', - input: 'select', - fetchSelects: fetchSelectsMock, - paginate: true, - operator: 'eq', - }, - ], - }; - - const wrapper = factory(newFiltersProps); - - afterEach(() => { - mockedProps.fetchData.mockClear(); - mockedProps.bulkActions.forEach(ba => { - ba.onSelect.mockClear(); - }); - }); it('renders UI filters', () => { expect(wrapper.find(ListViewFilters)).toHaveLength(1); @@ -407,43 +324,53 @@ describe('ListView with new UI filters', () => { wrapper.find('[data-test="search-input"]').last().props().onBlur(); }); - expect(newFiltersProps.fetchData.mock.calls[0]).toMatchInlineSnapshot(` -Array [ - Object { - "filters": Array [ - Object { - "id": "id", - "operator": "eq", - "value": "bar", - }, - ], - "pageIndex": 0, - "pageSize": 1, - "sortBy": Array [], - }, -] -`); - - expect(newFiltersProps.fetchData.mock.calls[1]).toMatchInlineSnapshot(` -Array [ - Object { - "filters": Array [ - Object { - "id": "id", - "operator": "eq", - "value": "bar", - }, - Object { - "id": "name", - "operator": "ct", - "value": "something", - }, - ], - "pageIndex": 0, - "pageSize": 1, - "sortBy": Array [], - }, -] -`); + expect(mockedProps.fetchData.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "filters": Array [ + Object { + "id": "id", + "operator": "eq", + "value": "bar", + }, + ], + "pageIndex": 0, + "pageSize": 1, + "sortBy": Array [ + Object { + "desc": false, + "id": "id", + }, + ], + }, + ] + `); + + expect(mockedProps.fetchData.mock.calls[1]).toMatchInlineSnapshot(` + Array [ + Object { + "filters": Array [ + Object { + "id": "id", + "operator": "eq", + "value": "bar", + }, + Object { + "id": "name", + "operator": "ct", + "value": "something", + }, + ], + "pageIndex": 0, + "pageSize": 1, + "sortBy": Array [ + Object { + "desc": false, + "id": "id", + }, + ], + }, + ] + `); }); }); diff --git a/superset-frontend/spec/javascripts/views/chartList/ChartList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx similarity index 90% rename from superset-frontend/spec/javascripts/views/chartList/ChartList_spec.jsx rename to superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx index 695b74e..ba6c464 100644 --- a/superset-frontend/spec/javascripts/views/chartList/ChartList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx @@ -23,7 +23,7 @@ import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import { supersetTheme, ThemeProvider } from '@superset-ui/style'; -import ChartList from 'src/views/chartList/ChartList'; +import ChartList from 'src/views/CRUD/chart/ChartList'; import ListView from 'src/components/ListView/ListView'; // store needed for withToasts(ChartTable) @@ -48,13 +48,6 @@ const mockCharts = [...new Array(3)].map((_, i) => ({ fetchMock.get(chartsInfoEndpoint, { permissions: ['can_list', 'can_edit'], - filters: { - slice_name: [], - description: [], - viz_type: [], - datasource_name: [], - owners: [], - }, }); fetchMock.get(chartssOwnersEndpoint, { result: [], @@ -95,11 +88,6 @@ describe('ChartList', () => { expect(callsI).toHaveLength(1); }); - it('fetches owners', () => { - const callsO = fetchMock.calls(/chart\/related\/owners/); - expect(callsO).toHaveLength(1); - }); - it('fetches data', () => { wrapper.update(); const callsD = fetchMock.calls(/chart\/\?q/); diff --git a/superset-frontend/spec/javascripts/views/dashboardList/DashboardList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx similarity index 91% rename from superset-frontend/spec/javascripts/views/dashboardList/DashboardList_spec.jsx rename to superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx index cfaaeca..c847b17 100644 --- a/superset-frontend/spec/javascripts/views/dashboardList/DashboardList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx @@ -23,7 +23,7 @@ import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import { supersetTheme, ThemeProvider } from '@superset-ui/style'; -import DashboardList from 'src/views/dashboardList/DashboardList'; +import DashboardList from 'src/views/CRUD/dashboard/DashboardList'; import ListView from 'src/components/ListView/ListView'; import PropertiesModal from 'src/dashboard/components/PropertiesModal'; @@ -50,12 +50,6 @@ const mockDashboards = [...new Array(3)].map((_, i) => ({ fetchMock.get(dashboardsInfoEndpoint, { permissions: ['can_list', 'can_edit'], - filters: { - dashboard_title: [], - slug: [], - owners: [], - published: [], - }, }); fetchMock.get(dashboardOwnersEndpoint, { result: [], @@ -86,11 +80,6 @@ describe('DashboardList', () => { expect(callsI).toHaveLength(1); }); - it('fetches owners', () => { - const callsO = fetchMock.calls(/dashboard\/related\/owners/); - expect(callsO).toHaveLength(1); - }); - it('fetches data', () => { wrapper.update(); const callsD = fetchMock.calls(/dashboard\/\?q/); diff --git a/superset-frontend/spec/javascripts/views/datasetList/DatasetList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/dataset/DatasetList_spec.jsx similarity index 94% rename from superset-frontend/spec/javascripts/views/datasetList/DatasetList_spec.jsx rename to superset-frontend/spec/javascripts/views/CRUD/dataset/DatasetList_spec.jsx index 58b5ef0..9f7c285 100644 --- a/superset-frontend/spec/javascripts/views/datasetList/DatasetList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/dataset/DatasetList_spec.jsx @@ -23,7 +23,7 @@ import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; import { supersetTheme, ThemeProvider } from '@superset-ui/style'; -import DatasetList from 'src/views/datasetList/DatasetList'; +import DatasetList from 'src/views/CRUD/dataset/DatasetList'; import ListView from 'src/components/ListView/ListView'; import Button from 'src/components/Button'; import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox'; @@ -54,13 +54,6 @@ const mockdatasets = [...new Array(3)].map((_, i) => ({ fetchMock.get(datasetsInfoEndpoint, { permissions: ['can_list', 'can_edit', 'can_add', 'can_delete'], - filters: { - database: [], - schema: [], - table_name: [], - owners: [], - is_sqllab_view: [], - }, }); fetchMock.get(datasetsOwnersEndpoint, { result: [], @@ -105,11 +98,6 @@ describe('DatasetList', () => { expect(callsI).toHaveLength(1); }); - it('fetches owners', () => { - const callsO = fetchMock.calls(/dataset\/related\/owners/); - expect(callsO).toHaveLength(1); - }); - it('fetches data', () => { const callsD = fetchMock.calls(/dataset\/\?q/); expect(callsD).toHaveLength(1); diff --git a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx index 2b7ff01..029f1e0 100644 --- a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx @@ -22,8 +22,8 @@ import { styled, supersetTheme } from '@superset-ui/style'; import { t, tn } from '@superset-ui/translation'; import { noOp } from 'src/utils/common'; +import Button from 'src/views/CRUD/dataset/Button'; import Icon from '../Icon'; -import Button from '../../views/datasetList/Button'; import { ErrorMessageComponentProps } from './types'; import CopyToClipboard from '../CopyToClipboard'; import IssueCode from './IssueCode'; diff --git a/superset-frontend/src/components/ListView/LegacyFilters.tsx b/superset-frontend/src/components/ListView/LegacyFilters.tsx deleted file mode 100644 index 111af4b..0000000 --- a/superset-frontend/src/components/ListView/LegacyFilters.tsx +++ /dev/null @@ -1,204 +0,0 @@ -/** - * 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 { t } from '@superset-ui/translation'; -import React, { Dispatch, SetStateAction } from 'react'; -import { - Button, - Col, - DropdownButton, - FormControl, - MenuItem, - Row, -} from 'react-bootstrap'; -import { Select } from 'src/components/Select'; -import { Filters, InternalFilter, SelectOption } from './types'; -import { extractInputValue, getDefaultFilterOperator } from './utils'; - -const styleWidth100p = { width: '100%' }; - -export const FilterMenu = ({ - filters, - internalFilters, - setInternalFilters, -}: { - filters: Filters; - internalFilters: InternalFilter[]; - setInternalFilters: Dispatch<SetStateAction<InternalFilter[]>>; -}) => ( - <div className="filter-dropdown"> - <DropdownButton - id="filter-picker" - bsSize="small" - bsStyle={'default'} - noCaret - title={ - <> - <i className="fa fa-filter text-primary" /> - {' '} - {t('Filter')} - </> - } - > - {filters - .map(({ id, Header }) => ({ - Header, - id, - value: undefined, - })) - .map(ft => ( - <MenuItem - key={ft.id} - eventKey={ft} - // @ts-ignore - onSelect={(fltr: typeof ft) => { - setInternalFilters([...internalFilters, fltr]); - }} - > - {ft.Header} - </MenuItem> - ))} - </DropdownButton> - </div> -); - -export const FilterInputs = ({ - internalFilters, - filters, - updateInternalFilter, - removeFilterAndApply, - filtersApplied, - applyFilters, -}: { - internalFilters: InternalFilter[]; - filters: Filters; - updateInternalFilter: (i: number, f: object) => void; - removeFilterAndApply: (i: number) => void; - filtersApplied: boolean; - applyFilters: () => void; -}) => ( - <> - {internalFilters.map((ft, i) => { - const filter = filters.find(f => f.id === ft.id); - if (!filter) { - // eslint-disable-next-line no-console - console.error(`could not find filter for ${ft.id}`); - return null; - } - return ( - <div key={`${ft.Header}-${i}`} className="filter-inputs"> - <Row> - <Col className="text-center filter-column" md={2}> - <span>{ft.Header}</span> - </Col> - <Col md={2}> - <FormControl - componentClass="select" - bsSize="small" - value={ft.operator} - placeholder={filter ? getDefaultFilterOperator(filter) : ''} - // @ts-ignore - onChange={(e: React.ChangeEvent<HTMLInputElement>) => { - updateInternalFilter(i, { - operator: e.currentTarget.value, - }); - }} - > - {(filter.operators || []).map( - ({ label, value }: SelectOption) => ( - <option key={label} value={value}> - {label} - </option> - ), - )} - </FormControl> - </Col> - <Col md={1} /> - <Col md={4}> - {filter.input === 'select' && ( - <Select - autoFocus - multi - searchable - name={`filter-${filter.id}-select`} - options={filter.selects} - placeholder="Select Value" - value={ft.value as SelectOption['value'][] | undefined} - onChange={(e: SelectOption[] | null) => { - updateInternalFilter(i, { - operator: ft.operator || getDefaultFilterOperator(filter), - value: e ? e.map(s => s.value) : e, - }); - }} - /> - )} - {filter.input !== 'select' && ( - // @ts-ignore - <FormControl - type={filter.input ? filter.input : 'text'} - bsSize="small" - value={String(ft.value || '')} - checked={Boolean(ft.value)} - // @ts-ignore - onChange={(e: React.ChangeEvent<HTMLInputElement>) => { - e.persist(); - updateInternalFilter(i, { - operator: ft.operator || getDefaultFilterOperator(filter), - value: extractInputValue(filter.input, e), - }); - }} - /> - )} - </Col> - <Col md={1}> - <div - className="filter-close" - role="button" - tabIndex={0} - onClick={() => removeFilterAndApply(i)} - > - <i className="fa fa-close text-primary" /> - </div> - </Col> - </Row> - <br /> - </div> - ); - })} - {internalFilters.length > 0 && ( - <> - <Row> - <Col md={11} /> - <Col md={1}> - <Button - data-test="apply-filters" - disabled={!!filtersApplied} - bsStyle="primary" - style={styleWidth100p} - onClick={applyFilters} - bsSize="small" - > - {t('Apply')} - </Button> - </Col> - </Row> - <br /> - </> - )} - </> -); diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index 7a7216f..970658a 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -26,7 +26,6 @@ import Loading from 'src/components/Loading'; import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox'; import TableCollection from './TableCollection'; import Pagination from './Pagination'; -import { FilterMenu, FilterInputs } from './LegacyFilters'; import FilterControls from './Filters'; import { FetchDataConfig, Filters, SortColumn } from './types'; import { ListViewError, useListViewState } from './utils'; @@ -198,7 +197,6 @@ export interface ListViewProps { onSelect: (rows: any[]) => any; type?: 'primary' | 'secondary' | 'danger'; }>; - isSIP34FilterUIEnabled?: boolean; bulkSelectEnabled?: boolean; disableBulkSelect?: () => void; renderBulkSelectCopy?: (selects: any[]) => React.ReactNode; @@ -263,7 +261,6 @@ const ListView: FunctionComponent<ListViewProps> = ({ className = '', filters = [], bulkActions = [], - isSIP34FilterUIEnabled = false, bulkSelectEnabled = false, disableBulkSelect = () => {}, renderBulkSelectCopy = selected => t('%s Selected', selected.length), @@ -276,12 +273,7 @@ const ListView: FunctionComponent<ListViewProps> = ({ prepareRow, pageCount = 1, gotoPage, - removeFilterAndApply, - setInternalFilters, - updateInternalFilter, applyFilterValue, - applyFilters, - filtersApplied, selectedFlatRows, toggleAllRowsSelected, state: { pageIndex, pageSize, internalFilters }, @@ -294,7 +286,7 @@ const ListView: FunctionComponent<ListViewProps> = ({ fetchData, initialPageSize, initialSort, - initialFilters: isSIP34FilterUIEnabled ? filters : [], + initialFilters: filters, }); const filterable = Boolean(filters.length); if (filterable) { @@ -317,30 +309,7 @@ const ListView: FunctionComponent<ListViewProps> = ({ <ListViewStyles> <div className={`superset-list-view ${className}`}> <div className="header"> - {!isSIP34FilterUIEnabled && filterable && ( - <> - <Row> - <Col md={10} /> - <Col md={2}> - <FilterMenu - filters={filters} - internalFilters={internalFilters} - setInternalFilters={setInternalFilters} - /> - </Col> - </Row> - <hr /> - <FilterInputs - internalFilters={internalFilters} - filters={filters} - updateInternalFilter={updateInternalFilter} - removeFilterAndApply={removeFilterAndApply} - filtersApplied={filtersApplied} - applyFilters={applyFilters} - /> - </> - )} - {isSIP34FilterUIEnabled && filterable && ( + {filterable && ( <FilterControls filters={filters} internalFilters={internalFilters} diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index dbb283b..8a8b4f7 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -32,7 +32,19 @@ export interface Filter { Header: string; id: string; operators?: SelectOption[]; - operator?: string; + operator?: + | 'sw' + | 'ew' + | 'ct' + | 'eq' + | 'nsw' + | 'new' + | 'nct' + | 'neq' + | 'rel_m_m' + | 'rel_o_m' + | 'title_or_slug' + | 'name_or_description'; input?: 'text' | 'textarea' | 'select' | 'checkbox' | 'search'; unfilteredLabel?: string; selects?: SelectOption[]; @@ -63,19 +75,3 @@ export interface FetchDataConfig { export interface InternalFilter extends FilterValue { Header?: string; } - -export interface FilterOperatorMap { - [columnId: string]: Array<{ - name: string; - operator: - | 'sw' - | 'ew' - | 'ct' - | 'eq' - | 'nsw' - | 'new' - | 'nct' - | 'neq' - | 'rel_m_m'; - }>; -} diff --git a/superset-frontend/src/components/ListView/utils.ts b/superset-frontend/src/components/ListView/utils.ts index f3a6975..b73d8ad 100644 --- a/superset-frontend/src/components/ListView/utils.ts +++ b/superset-frontend/src/components/ListView/utils.ts @@ -225,18 +225,6 @@ export function useListViewState({ } }, [query]); - const filtersApplied = internalFilters.every( - ({ id, value, operator }, index) => - id && - filters[index]?.id === id && - filters[index]?.value === value && - // @ts-ignore - filters[index]?.operator === operator, - ); - - const updateInternalFilter = (index: number, update: object) => - setInternalFilters(updateInList(internalFilters, index, update)); - const applyFilterValue = (index: number, value: any) => { // skip redunundant updates if (internalFilters[index].value === value) { @@ -249,18 +237,9 @@ export function useListViewState({ gotoPage(0); // clear pagination on filter }; - const removeFilterAndApply = (index: number) => { - const updated = removeFromList(internalFilters, index); - setInternalFilters(updated); - setAllFilters(convertFilters(updated)); - }; - return { - applyFilters: () => setAllFilters(convertFilters(internalFilters)), - removeFilterAndApply, canNextPage, canPreviousPage, - filtersApplied, getTableBodyProps, getTableProps, gotoPage, @@ -270,10 +249,8 @@ export function useListViewState({ rows, selectedFlatRows, setAllFilters, - setInternalFilters, state: { pageIndex, pageSize, sortBy, filters, internalFilters }, toggleAllRowsSelected, - updateInternalFilter, applyFilterValue, }; } diff --git a/superset-frontend/src/components/Modal.tsx b/superset-frontend/src/components/Modal.tsx index 1c67ffe..f9d2fae 100644 --- a/superset-frontend/src/components/Modal.tsx +++ b/superset-frontend/src/components/Modal.tsx @@ -20,7 +20,7 @@ import React from 'react'; import styled from '@superset-ui/style'; import { Modal as BaseModal } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; -import Button from '../views/datasetList/Button'; +import Button from 'src/views/CRUD/dataset/Button'; interface ModalProps { children: React.ReactNode; diff --git a/superset-frontend/src/featureFlags.ts b/superset-frontend/src/featureFlags.ts index e157603..0853578 100644 --- a/superset-frontend/src/featureFlags.ts +++ b/superset-frontend/src/featureFlags.ts @@ -26,7 +26,6 @@ export enum FeatureFlag { ESTIMATE_QUERY_COST = 'ESTIMATE_QUERY_COST', SHARE_QUERIES_VIA_KV_STORE = 'SHARE_QUERIES_VIA_KV_STORE', SQLLAB_BACKEND_PERSISTENCE = 'SQLLAB_BACKEND_PERSISTENCE', - LIST_VIEWS_SIP34_FILTER_UI = 'LIST_VIEWS_SIP34_FILTER_UI', } export type FeatureFlagMap = { diff --git a/superset-frontend/src/views/chartList/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx similarity index 68% rename from superset-frontend/src/views/chartList/ChartList.tsx rename to superset-frontend/src/views/CRUD/chart/ChartList.tsx index d7a4336..30649bc 100644 --- a/superset-frontend/src/views/chartList/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -22,21 +22,15 @@ import { getChartMetadataRegistry } from '@superset-ui/chart'; import PropTypes from 'prop-types'; import React from 'react'; import rison from 'rison'; -// @ts-ignore -import { Panel } from 'react-bootstrap'; +import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import Icon from 'src/components/Icon'; import ListView, { ListViewProps } from 'src/components/ListView/ListView'; -import { - FetchDataConfig, - FilterOperatorMap, - Filters, -} from 'src/components/ListView/types'; +import { FetchDataConfig, Filters } from 'src/components/ListView/types'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import PropertiesModal, { Slice } from 'src/explore/components/PropertiesModal'; import Chart from 'src/types/Chart'; -import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; const PAGE_SIZE = 25; @@ -49,8 +43,6 @@ interface State { bulkSelectEnabled: boolean; chartCount: number; charts: any[]; - filterOperators: FilterOperatorMap; - filters: Filters; lastFetchDataConfig: FetchDataConfig | null; loading: boolean; permissions: string[]; @@ -58,7 +50,23 @@ interface State { // In future it would be better to have a unified Chart entity. sliceCurrentlyEditing: Slice | null; } +const createFetchDatasets = ( + handleError: (err: Response) => void, +) => async () => { + try { + const { json = {} } = await SupersetClient.get({ + endpoint: '/api/v1/chart/datasources', + }); + return json?.result?.map((ds: { label: string; value: any }) => ({ + ...ds, + value: JSON.stringify(ds.value), + })); + } catch (e) { + handleError(e); + } + return []; +}; class ChartList extends React.PureComponent<Props, State> { static propTypes = { addDangerToast: PropTypes.func.isRequired, @@ -68,8 +76,6 @@ class ChartList extends React.PureComponent<Props, State> { bulkSelectEnabled: false, chartCount: 0, charts: [], - filterOperators: {}, - filters: [], lastFetchDataConfig: null, loading: true, permissions: [], @@ -81,20 +87,15 @@ class ChartList extends React.PureComponent<Props, State> { endpoint: `/api/v1/chart/_info`, }).then( ({ json: infoJson = {} }) => { - this.setState( - { - filterOperators: infoJson.filters, - permissions: infoJson.permissions, - }, - this.updateFilters, - ); + this.setState({ + permissions: infoJson.permissions, + }); }, - e => { + createErrorHandler(errMsg => this.props.addDangerToast( - t('An error occurred while fetching charts: %s', e.statusText), - ); - console.error(e); - }, + t('An error occurred while fetching chart info: %s', errMsg), + ), + ), ); } @@ -106,10 +107,6 @@ class ChartList extends React.PureComponent<Props, State> { return this.hasPerm('can_delete'); } - get isSIP34FilterUIEnabled() { - return isFeatureEnabled(FeatureFlag.LIST_VIEWS_SIP34_FILTER_UI); - } - initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; columns = [ @@ -228,6 +225,63 @@ class ChartList extends React.PureComponent<Props, State> { }, ]; + filters: Filters = [ + { + Header: t('Owner'), + id: 'owners', + input: 'select', + operator: 'rel_m_m', + unfilteredLabel: 'All', + fetchSelects: createFetchRelated( + 'chart', + 'owners', + createErrorHandler(errMsg => + this.props.addDangerToast( + t( + 'An error occurred while fetching chart dataset values: %s', + errMsg, + ), + ), + ), + ), + paginate: true, + }, + { + Header: t('Viz Type'), + id: 'viz_type', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + selects: getChartMetadataRegistry() + .keys() + .map(k => ({ label: k, value: k })), + }, + { + Header: t('Dataset'), + id: 'datasource', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + fetchSelects: createFetchDatasets( + createErrorHandler(errMsg => + this.props.addDangerToast( + t( + 'An error occurred while fetching chart dataset values: %s', + errMsg, + ), + ), + ), + ), + paginate: false, + }, + { + Header: t('Search'), + id: 'slice_name', + input: 'search', + operator: 'name_or_description', + }, + ]; + hasPerm = (perm: string) => { if (!this.state.permissions.length) { return false; @@ -295,15 +349,11 @@ class ChartList extends React.PureComponent<Props, State> { } this.props.addSuccessToast(json.message); }, - (err: any) => { - console.error(err); + createErrorHandler(errMsg => this.props.addDangerToast( - t( - 'There was an issue deleting the selected charts: %s', - err.statusText, - ), - ); - }, + t('There was an issue deleting the selected charts: %s', errMsg), + ), + ), ); }; @@ -354,171 +404,27 @@ class ChartList extends React.PureComponent<Props, State> { return SupersetClient.get({ endpoint: `/api/v1/chart/?q=${queryParams}`, }) - .then(({ json = {} }) => { - this.setState({ charts: json.result, chartCount: json.count }); - }) - .catch(e => { - console.log(e.body); - this.props.addDangerToast( - t('An error occurred while fetching charts: %s', e.statusText), - ); - }) + .then( + ({ json = {} }) => { + this.setState({ charts: json.result, chartCount: json.count }); + }, + createErrorHandler(errMsg => + this.props.addDangerToast( + t('An error occurred while fetching charts: %s', errMsg), + ), + ), + ) .finally(() => { this.setState({ loading: false }); }); }; - fetchOwners = async ( - filterValue = '', - pageIndex?: number, - pageSize?: number, - ) => { - const resource = '/api/v1/chart/related/owners'; - - try { - const queryParams = rison.encode({ - ...(pageIndex ? { page: pageIndex } : {}), - ...(pageSize ? { page_ize: pageSize } : {}), - ...(filterValue ? { filter: filterValue } : {}), - }); - const { json = {} } = await SupersetClient.get({ - endpoint: `${resource}?q=${queryParams}`, - }); - - return json?.result?.map( - ({ text: label, value }: { text: string; value: any }) => ({ - label, - value, - }), - ); - } catch (e) { - console.error(e); - this.props.addDangerToast( - t( - 'An error occurred while fetching chart owner values: %s', - e.statusText, - ), - ); - } - return []; - }; - - fetchDatasets = async () => { - const resource = '/api/v1/chart/datasources'; - try { - const { json = {} } = await SupersetClient.get({ - endpoint: `${resource}`, - }); - - return json?.result?.map((ds: { label: string; value: any }) => ({ - ...ds, - value: JSON.stringify(ds.value), - })); - } catch (e) { - this.props.addDangerToast( - t( - 'An error occurred while fetching chart dataset values: %s', - e.statusText, - ), - ); - } - return []; - }; - - updateFilters = async () => { - const { filterOperators } = this.state; - - if (this.isSIP34FilterUIEnabled) { - this.setState({ - filters: [ - { - Header: 'Owner', - id: 'owners', - input: 'select', - operator: 'rel_m_m', - unfilteredLabel: 'All', - fetchSelects: this.fetchOwners, - paginate: true, - }, - { - Header: 'Viz Type', - id: 'viz_type', - input: 'select', - operator: 'eq', - unfilteredLabel: 'All', - selects: getChartMetadataRegistry() - .keys() - .map(k => ({ label: k, value: k })), - }, - { - Header: 'Dataset', - id: 'datasource', - input: 'select', - operator: 'eq', - unfilteredLabel: 'All', - fetchSelects: this.fetchDatasets, - paginate: false, - }, - { - Header: 'Search', - id: 'slice_name', - input: 'search', - operator: 'name_or_description', - }, - ], - }); - return; - } - - const convertFilter = ({ - name: label, - operator, - }: { - name: string; - operator: string; - }) => ({ label, value: operator }); - - const owners = await this.fetchOwners(); - this.setState({ - filters: [ - { - Header: 'Chart', - id: 'slice_name', - operators: filterOperators.slice_name.map(convertFilter), - }, - { - Header: 'Description', - id: 'description', - operators: filterOperators.slice_name.map(convertFilter), - }, - { - Header: 'Visualization Type', - id: 'viz_type', - operators: filterOperators.viz_type.map(convertFilter), - }, - { - Header: 'Datasource Name', - id: 'datasource_name', - operators: filterOperators.datasource_name.map(convertFilter), - }, - { - Header: 'Owners', - id: 'owners', - input: 'select', - operators: filterOperators.owners.map(convertFilter), - selects: owners, - }, - ], - }); - }; - render() { const { bulkSelectEnabled, charts, chartCount, loading, - filters, sliceCurrentlyEditing, } = this.state; return ( @@ -536,9 +442,9 @@ class ChartList extends React.PureComponent<Props, State> { /> {sliceCurrentlyEditing && ( <PropertiesModal - show onHide={this.closeChartEditModal} onSave={this.handleChartUpdated} + show slice={sliceCurrentlyEditing} /> )} @@ -563,19 +469,18 @@ class ChartList extends React.PureComponent<Props, State> { return ( <ListView + bulkActions={bulkActions} + bulkSelectEnabled={bulkSelectEnabled} className="chart-list-view" columns={this.columns} - data={charts} count={chartCount} - pageSize={PAGE_SIZE} + data={charts} + disableBulkSelect={this.toggleBulkSelect} fetchData={this.fetchData} - loading={loading} + filters={this.filters} initialSort={this.initialSort} - filters={filters} - bulkActions={bulkActions} - bulkSelectEnabled={bulkSelectEnabled} - disableBulkSelect={this.toggleBulkSelect} - isSIP34FilterUIEnabled={this.isSIP34FilterUIEnabled} + loading={loading} + pageSize={PAGE_SIZE} /> ); }} diff --git a/superset-frontend/src/views/dashboardList/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx similarity index 73% rename from superset-frontend/src/views/dashboardList/DashboardList.tsx rename to superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index de23ee8..1489a58 100644 --- a/superset-frontend/src/views/dashboardList/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -21,21 +21,15 @@ import { t } from '@superset-ui/translation'; import PropTypes from 'prop-types'; import React from 'react'; import rison from 'rison'; -// @ts-ignore -import { Panel } from 'react-bootstrap'; +import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import ListView, { ListViewProps } from 'src/components/ListView/ListView'; import ExpandableList from 'src/components/ExpandableList'; -import { - FetchDataConfig, - FilterOperatorMap, - Filters, -} from 'src/components/ListView/types'; +import { FetchDataConfig, Filters } from 'src/components/ListView/types'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import Icon from 'src/components/Icon'; import PropertiesModal from 'src/dashboard/components/PropertiesModal'; -import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; const PAGE_SIZE = 25; @@ -49,8 +43,6 @@ interface State { dashboardCount: number; dashboards: any[]; dashboardToEdit: Dashboard | null; - filterOperators: FilterOperatorMap; - filters: Filters; lastFetchDataConfig: FetchDataConfig | null; loading: boolean; permissions: string[]; @@ -77,8 +69,6 @@ class DashboardList extends React.PureComponent<Props, State> { dashboardCount: 0, dashboards: [], dashboardToEdit: null, - filterOperators: {}, - filters: [], lastFetchDataConfig: null, loading: true, permissions: [], @@ -89,23 +79,15 @@ class DashboardList extends React.PureComponent<Props, State> { endpoint: `/api/v1/dashboard/_info`, }).then( ({ json: infoJson = {} }) => { - this.setState( - { - filterOperators: infoJson.filters, - permissions: infoJson.permissions, - }, - this.updateFilters, - ); + this.setState({ + permissions: infoJson.permissions, + }); }, - e => { + createErrorHandler(errMsg => this.props.addDangerToast( - t( - 'An error occurred while fetching Dashboards: %s, %s', - e.statusText, - ), - ); - console.error(e); - }, + t('An error occurred while fetching Dashboards: %s, %s', errMsg), + ), + ), ); } @@ -121,10 +103,6 @@ class DashboardList extends React.PureComponent<Props, State> { return this.hasPerm('can_mulexport'); } - get isSIP34FilterUIEnabled() { - return isFeatureEnabled(FeatureFlag.LIST_VIEWS_SIP34_FILTER_UI); - } - initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; columns = [ @@ -260,6 +238,46 @@ class DashboardList extends React.PureComponent<Props, State> { this.setState({ bulkSelectEnabled: !this.state.bulkSelectEnabled }); }; + filters: Filters = [ + { + Header: 'Owner', + id: 'owners', + input: 'select', + operator: 'rel_m_m', + unfilteredLabel: 'All', + fetchSelects: createFetchRelated( + 'dashboard', + 'owners', + createErrorHandler(errMsg => + this.props.addDangerToast( + t( + 'An error occurred while fetching chart owner values: %s', + errMsg, + ), + ), + ), + ), + paginate: true, + }, + { + Header: 'Published', + id: 'published', + input: 'select', + operator: 'eq', + unfilteredLabel: 'Any', + selects: [ + { label: 'Published', value: true }, + { label: 'Unpublished', value: false }, + ], + }, + { + Header: 'Search', + id: 'dashboard_title', + input: 'search', + operator: 'title_or_slug', + }, + ]; + hasPerm = (perm: string) => { if (!this.state.permissions.length) { return false; @@ -278,8 +296,8 @@ class DashboardList extends React.PureComponent<Props, State> { this.setState({ loading: true }); return SupersetClient.get({ endpoint: `/api/v1/dashboard/${edits.id}`, - }) - .then(({ json = {} }) => { + }).then( + ({ json = {} }) => { this.setState({ dashboards: this.state.dashboards.map(dashboard => { if (dashboard.id === json.id) { @@ -289,12 +307,13 @@ class DashboardList extends React.PureComponent<Props, State> { }), loading: false, }); - }) - .catch(e => { + }, + createErrorHandler(errMsg => this.props.addDangerToast( - t('An error occurred while fetching dashboards: %s', e.statusText), - ); - }); + t('An error occurred while fetching dashboards: %s', errMsg), + ), + ), + ); }; handleDashboardDelete = ({ @@ -311,12 +330,11 @@ class DashboardList extends React.PureComponent<Props, State> { } this.props.addSuccessToast(t('Deleted: %s', dashboardTitle)); }, - (err: any) => { - console.error(err); + createErrorHandler(errMsg => this.props.addDangerToast( - t('There was an issue deleting %s', dashboardTitle), - ); - }, + t('There was an issue deleting %s: %s', dashboardTitle, errMsg), + ), + ), ); handleBulkDashboardDelete = (dashboards: Dashboard[]) => { @@ -332,15 +350,11 @@ class DashboardList extends React.PureComponent<Props, State> { } this.props.addSuccessToast(json.message); }, - (err: any) => { - console.error(err); + createErrorHandler(errMsg => this.props.addDangerToast( - t( - 'There was an issue deleting the selected dashboards: ', - err.statusText, - ), - ); - }, + t('There was an issue deleting the selected dashboards: ', errMsg), + ), + ), ); }; @@ -380,137 +394,31 @@ class DashboardList extends React.PureComponent<Props, State> { return SupersetClient.get({ endpoint: `/api/v1/dashboard/?q=${queryParams}`, }) - .then(({ json = {} }) => { - this.setState({ dashboards: json.result, dashboardCount: json.count }); - }) - .catch(e => { - this.props.addDangerToast( - t('An error occurred while fetching dashboards: %s', e.statusText), - ); - }) + .then( + ({ json = {} }) => { + this.setState({ + dashboards: json.result, + dashboardCount: json.count, + }); + }, + createErrorHandler(errMsg => + this.props.addDangerToast( + t('An error occurred while fetching dashboards: %s', errMsg), + ), + ), + ) .finally(() => { this.setState({ loading: false }); }); }; - fetchOwners = async ( - filterValue = '', - pageIndex?: number, - pageSize?: number, - ) => { - const resource = '/api/v1/dashboard/related/owners'; - - try { - const queryParams = rison.encode({ - ...(pageIndex ? { page: pageIndex } : {}), - ...(pageSize ? { page_ize: pageSize } : {}), - ...(filterValue ? { filter: filterValue } : {}), - }); - const { json = {} } = await SupersetClient.get({ - endpoint: `${resource}?q=${queryParams}`, - }); - - return json?.result?.map( - ({ text: label, value }: { text: string; value: any }) => ({ - label, - value, - }), - ); - } catch (e) { - console.error(e); - this.props.addDangerToast( - t( - 'An error occurred while fetching chart owner values: %s', - e.statusText, - ), - ); - } - return []; - }; - - updateFilters = async () => { - const { filterOperators } = this.state; - - if (this.isSIP34FilterUIEnabled) { - return this.setState({ - filters: [ - { - Header: 'Owner', - id: 'owners', - input: 'select', - operator: 'rel_m_m', - unfilteredLabel: 'All', - fetchSelects: this.fetchOwners, - paginate: true, - }, - { - Header: 'Published', - id: 'published', - input: 'select', - operator: 'eq', - unfilteredLabel: 'Any', - selects: [ - { label: 'Published', value: true }, - { label: 'Unpublished', value: false }, - ], - }, - { - Header: 'Search', - id: 'dashboard_title', - input: 'search', - operator: 'title_or_slug', - }, - ], - }); - } - - const convertFilter = ({ - name: label, - operator, - }: { - name: string; - operator: string; - }) => ({ label, value: operator }); - - const owners = await this.fetchOwners(); - - return this.setState({ - filters: [ - { - Header: 'Dashboard', - id: 'dashboard_title', - operators: filterOperators.dashboard_title.map(convertFilter), - }, - { - Header: 'Slug', - id: 'slug', - operators: filterOperators.slug.map(convertFilter), - }, - { - Header: 'Owners', - id: 'owners', - input: 'select', - operators: filterOperators.owners.map(convertFilter), - selects: owners, - }, - { - Header: 'Published', - id: 'published', - input: 'checkbox', - operators: filterOperators.published.map(convertFilter), - }, - ], - }); - }; - render() { const { bulkSelectEnabled, - dashboardCount, dashboards, - dashboardToEdit, - filters, + dashboardCount, loading, + dashboardToEdit, } = this.state; return ( <> @@ -554,26 +462,25 @@ class DashboardList extends React.PureComponent<Props, State> { <> {dashboardToEdit && ( <PropertiesModal - show dashboardId={dashboardToEdit.id} - onHide={() => this.setState({ dashboardToEdit: null })} onDashboardSave={this.handleDashboardEdit} + onHide={() => this.setState({ dashboardToEdit: null })} + show /> )} <ListView + bulkActions={bulkActions} + bulkSelectEnabled={bulkSelectEnabled} className="dashboard-list-view" columns={this.columns} - data={dashboards} count={dashboardCount} - pageSize={PAGE_SIZE} + data={dashboards} + disableBulkSelect={this.toggleBulkSelect} fetchData={this.fetchData} - loading={loading} + filters={this.filters} initialSort={this.initialSort} - filters={filters} - bulkActions={bulkActions} - bulkSelectEnabled={bulkSelectEnabled} - disableBulkSelect={this.toggleBulkSelect} - isSIP34FilterUIEnabled={this.isSIP34FilterUIEnabled} + loading={loading} + pageSize={PAGE_SIZE} /> </> ); diff --git a/superset-frontend/src/views/datasetList/AddDatasetModal.tsx b/superset-frontend/src/views/CRUD/dataset/AddDatasetModal.tsx similarity index 93% rename from superset-frontend/src/views/datasetList/AddDatasetModal.tsx rename to superset-frontend/src/views/CRUD/dataset/AddDatasetModal.tsx index c699520..5182cc8 100644 --- a/superset-frontend/src/views/datasetList/AddDatasetModal.tsx +++ b/superset-frontend/src/views/CRUD/dataset/AddDatasetModal.tsx @@ -24,7 +24,8 @@ import { t } from '@superset-ui/translation'; import Icon from 'src/components/Icon'; import Modal from 'src/components/Modal'; import TableSelector from 'src/components/TableSelector'; -import withToasts from '../../messageToasts/enhancers/withToasts'; +import withToasts from 'src/messageToasts/enhancers/withToasts'; +import { createErrorHandler } from 'src/views/CRUD/utils'; type DatasetAddObject = { id: number; @@ -95,10 +96,11 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({ addSuccessToast(t('The dataset has been saved')); onHide(); }) - .catch(e => { - addDangerToast(t('Error while saving dataset')); - console.error(e); - }); + .catch( + createErrorHandler(errMsg => + addDangerToast(t('Error while saving dataset: %s', errMsg)), + ), + ); }; return ( diff --git a/superset-frontend/src/views/datasetList/Button.tsx b/superset-frontend/src/views/CRUD/dataset/Button.tsx similarity index 100% rename from superset-frontend/src/views/datasetList/Button.tsx rename to superset-frontend/src/views/CRUD/dataset/Button.tsx diff --git a/superset-frontend/src/views/datasetList/DatasetList.tsx b/superset-frontend/src/views/CRUD/dataset/DatasetList.tsx similarity index 75% rename from superset-frontend/src/views/datasetList/DatasetList.tsx rename to superset-frontend/src/views/CRUD/dataset/DatasetList.tsx index a0fcfd6..4e3dda6 100644 --- a/superset-frontend/src/views/datasetList/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/dataset/DatasetList.tsx @@ -25,16 +25,13 @@ import React, { useState, } from 'react'; import rison from 'rison'; +import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import DeleteModal from 'src/components/DeleteModal'; import ListView, { ListViewProps } from 'src/components/ListView/ListView'; import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu'; import AvatarIcon from 'src/components/AvatarIcon'; -import { - FetchDataConfig, - FilterOperatorMap, - Filters, -} from 'src/components/ListView/types'; +import { FetchDataConfig, Filters } from 'src/components/ListView/types'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import TooltipWrapper from 'src/components/TooltipWrapper'; import Icon from 'src/components/Icon'; @@ -69,29 +66,92 @@ 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, +) => { + try { + const queryParams = rison.encode({ + columns: ['database_name', 'id'], + keys: ['none'], + ...(pageIndex ? { page: pageIndex } : {}), + ...(pageSize ? { page_size: pageSize } : {}), + ...(filterValue ? { filter: filterValue } : {}), + }); + const { json = {} } = await SupersetClient.get({ + endpoint: `/api/v1/database/?q=${queryParams}`, + }); + + return json?.result?.map( + ({ database_name: label, id: value }: Database) => ({ + label, + value, + }), + ); + } catch (e) { + handleError(e); + } + return []; +}; +export const createFetchSchemas = ( + handleError: (error: Response) => void, +) => async (filterValue = '', pageIndex?: number, pageSize?: number) => { + try { + const queryParams = rison.encode({ + ...(pageIndex ? { page: pageIndex } : {}), + ...(pageSize ? { page_size: pageSize } : {}), + ...(filterValue ? { filter: filterValue } : {}), + }); + const { json = {} } = await SupersetClient.get({ + endpoint: `/api/v1/database/schemas/?q=${queryParams}`, + }); + + return json?.result?.map( + ({ text: label, value }: { text: string; value: any }) => ({ + label, + value, + }), + ); + } catch (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>( @@ -99,98 +159,85 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ ); 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: createFetchRelated( + 'dataset', + 'owners', + createErrorHandler(errMsg => + t( + 'An error occurred while fetching dataset owner values: %s', + errMsg, + ), + ), + ), + paginate: true, + }, + { + Header: t('Datasource'), + id: 'database', + input: 'select', + operator: 'rel_o_m', + unfilteredLabel: 'All', + fetchSelects: createFetchDatabases( + createErrorHandler(errMsg => + t('An error occurred while fetching datasource values: %s', errMsg), + ), + ), + paginate: true, + }, + { + Header: t('Schema'), + id: 'schema', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + fetchSelects: createFetchSchemas(errMsg => + t('An error occurred while fetching schema values: %s', errMsg), + ), + paginate: true, + }, + { + Header: t('Type'), + id: 'is_sqllab_view', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + selects: [ + { label: 'Virtual', value: true }, + { label: 'Physical', value: false }, + ], + }, + { + Header: t('Search'), + id: 'table_name', + input: 'search', + operator: 'ct', + }, + ]; - const fetchDataset = () => - Promise.all([ - SupersetClient.get({ - endpoint: `/api/v1/dataset/_info`, - }), - SupersetClient.get({ - endpoint: `/api/v1/dataset/related/owners`, - }), - SupersetClient.get({ - endpoint: `/api/v1/dataset/related/database`, - }), - ]) - .then( - ([ - { json: infoJson = {} }, - { json: ownersJson = {} }, - { json: databasesJson = {} }, - ]) => { - setCurrentOwners(ownersJson.result); - setDatabases(databasesJson.result); - setPermissions(infoJson.permissions); - setFilterOperators(infoJson.filters); - }, - ) - .catch(([e1, e2]) => { - addDangerToast(t('An error occurred while fetching datasets')); - if (e1) { - console.error(e1); - } - if (e2) { - console.error(e2); - } - }); + const fetchDatasetInfo = () => { + SupersetClient.get({ + endpoint: `/api/v1/dataset/_info`, + }).then( + ({ json: infoJson = {} }) => { + setPermissions(infoJson.permissions); + }, + createErrorHandler(errMsg => + addDangerToast(t('An error occurred while fetching datasets', errMsg)), + ), + ); + }; useEffect(() => { - fetchDataset(); + fetchDatasetInfo(); }, []); - useEffect(() => { - updateFilters(); - }, [databases, currentOwners, permissions, filterOperators]); - const hasPerm = (perm: string) => { if (!permissions.length) { return false; @@ -220,11 +267,14 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ dashboard_count: json.dashboards.count, }); }) - .catch(() => { - addDangerToast( - t('An error occurred while fetching dataset related data'), - ); - }); + .catch( + createErrorHandler(errMsg => + t( + 'An error occurred while fetching dataset related data: %s', + errMsg, + ), + ), + ); const columns = [ { @@ -477,15 +527,19 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ return SupersetClient.get({ endpoint: `/api/v1/dataset/?q=${queryParams}`, }) - .then(({ json }) => { - setLoading(false); - setDatasets(json.result); - setDatasetCount(json.count); - }) - .catch(() => { - addDangerToast(t('An error occurred while fetching datasets')); - setLoading(false); - }); + .then( + ({ json }) => { + setLoading(false); + setDatasets(json.result); + setDatasetCount(json.count); + }, + createErrorHandler(errMsg => + addDangerToast( + t('An error occurred while fetching datasets: %s', errMsg), + ), + ), + ) + .finally(() => setLoading(false)); }, [], ); @@ -501,10 +555,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ setDatasetCurrentlyDeleting(null); addSuccessToast(t('Deleted: %s', tableName)); }, - (err: any) => { - console.error(err); - addDangerToast(t('There was an issue deleting %s', tableName)); - }, + createErrorHandler(errMsg => + addDangerToast( + t('There was an issue deleting %s: %s', tableName, errMsg), + ), + ), ); }; @@ -520,10 +575,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ } addSuccessToast(json.message); }, - (err: any) => { - console.error(err); - addDangerToast(t('There was an issue deleting the selected datasets')); - }, + createErrorHandler(errMsg => + addDangerToast( + t('There was an issue deleting the selected datasets: %s', errMsg), + ), + ), ); }; @@ -578,9 +634,9 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ count={datasetCount} pageSize={PAGE_SIZE} fetchData={fetchData} + filters={filterTypes} loading={loading} initialSort={initialSort} - filters={currentFilters} bulkActions={bulkActions} bulkSelectEnabled={bulkSelectEnabled} disableBulkSelect={() => setBulkSelectEnabled(false)} diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx new file mode 100644 index 0000000..1873542 --- /dev/null +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -0,0 +1,61 @@ +/** + * 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, + SupersetClientResponse, +} from '@superset-ui/connection'; +import rison from 'rison'; +import getClientErrorObject from 'src/utils/getClientErrorObject'; + +export const createFetchRelated = ( + resource: string, + relation: string, + handleError: (error: Response) => void, +) => async (filterValue = '', pageIndex?: number, pageSize?: number) => { + const resourceEndpoint = `/api/v1/${resource}/related/${relation}`; + + 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) { + handleError(e); + } + return []; +}; + +export const createErrorHandler = ( + handleError: (errMsg?: string) => void, +) => async (e: SupersetClientResponse | string) => { + const parsedError = await getClientErrorObject(e); + console.error(e); // eslint-disable-line no-console + handleError(parsedError.message); +}; diff --git a/superset-frontend/src/welcome/App.tsx b/superset-frontend/src/welcome/App.tsx index 267dd5a..4df1488 100644 --- a/superset-frontend/src/welcome/App.tsx +++ b/superset-frontend/src/welcome/App.tsx @@ -29,9 +29,9 @@ import { supersetTheme, ThemeProvider } from '@superset-ui/style'; import ErrorBoundary from 'src/components/ErrorBoundary'; import Menu from 'src/components/Menu/Menu'; import FlashProvider from 'src/components/FlashProvider'; -import DashboardList from 'src/views/dashboardList/DashboardList'; -import ChartList from 'src/views/chartList/ChartList'; -import DatasetList from 'src/views/datasetList/DatasetList'; +import DashboardList from 'src/views/CRUD/dashboard/DashboardList'; +import ChartList from 'src/views/CRUD/chart/ChartList'; +import DatasetList from 'src/views/CRUD/dataset/DatasetList'; import messageToastReducer from '../messageToasts/reducers'; import { initEnhancer } from '../reduxUtils'; diff --git a/superset/config.py b/superset/config.py index d22fd80..2c2b180 100644 --- a/superset/config.py +++ b/superset/config.py @@ -307,7 +307,6 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = { "SIP_38_VIZ_REARCHITECTURE": False, "TAGGING_SYSTEM": False, "SQLLAB_BACKEND_PERSISTENCE": False, - "LIST_VIEWS_SIP34_FILTER_UI": False, } # This is merely a default. diff --git a/superset/databases/api.py b/superset/databases/api.py index 0b8321e..34607a1 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -16,23 +16,33 @@ # under the License. from typing import Any, Dict, List, Optional -from flask_appbuilder.api import expose, protect, safe +from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from sqlalchemy.exc import NoSuchTableError, SQLAlchemyError -from superset import event_logger +from superset import event_logger, security_manager from superset.databases.decorators import check_datasource_access from superset.databases.schemas import ( + DatabaseSchemaResponseSchema, SelectStarResponseSchema, TableMetadataResponseSchema, ) from superset.models.core import Database from superset.typing import FlaskResponse from superset.utils.core import error_msg_from_exception -from superset.views.base_api import BaseSupersetModelRestApi +from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics from superset.views.database.filters import DatabaseFilter from superset.views.database.validators import sqlalchemy_uri_validator +get_schemas_schema = { + "type": "object", + "properties": { + "page_size": {"type": "integer"}, + "page": {"type": "integer"}, + "filter": {"type": "string"}, + }, +} + def get_foreign_keys_metadata( database: Database, table_name: str, schema_name: Optional[str] @@ -115,12 +125,13 @@ def get_table_metadata( class DatabaseRestApi(BaseSupersetModelRestApi): datamodel = SQLAInterface(Database) - include_route_methods = {"get_list", "table_metadata", "select_star"} + include_route_methods = {"get_list", "table_metadata", "select_star", "schemas"} class_permission_name = "DatabaseView" method_permission_name = { "get_list": "list", "table_metadata": "list", "select_star": "list", + "schemas": "list", } resource_name = "database" allow_browser_login = True @@ -148,7 +159,11 @@ class DatabaseRestApi(BaseSupersetModelRestApi): validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator} openapi_spec_tag = "Database" + apispec_parameter_schemas = { + "get_schemas_schema": get_schemas_schema, + } openapi_spec_component_schemas = ( + DatabaseSchemaResponseSchema, TableMetadataResponseSchema, SelectStarResponseSchema, ) @@ -265,3 +280,70 @@ class DatabaseRestApi(BaseSupersetModelRestApi): return self.response(404, message="Table not found on the database") self.incr_stats("success", self.select_star.__name__) return self.response(200, result=result) + + @expose("/schemas/", methods=["GET"]) + @protect() + @safe + @statsd_metrics + @rison(get_schemas_schema) + def schemas(self, **kwargs: Any) -> FlaskResponse: + """Get all schemas + --- + get: + parameters: + - in: query + name: q + content: + application/json: + schema: + $ref: '#/components/schemas/get_schemas_schema' + responses: + 200: + description: Related column data + content: + application/json: + schema: + $ref: "#/components/schemas/DatabaseSchemaResponseSchema" + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + args = kwargs.get("rison", {}) + # handle pagination + page, page_size = self._handle_page_args(args) + filter_ = args.get("filter", "") + + _, databases = self.datamodel.query(page=page, page_size=page_size) + result = [] + count = 0 + if databases: + for database in databases: + try: + schemas = database.get_all_schema_names( + cache=database.schema_cache_enabled, + cache_timeout=database.schema_cache_timeout, + force=False, + ) + except SQLAlchemyError: + self.incr_stats("error", self.schemas.__name__) + continue + + schemas = security_manager.get_schemas_accessible_by_user( + database, schemas + ) + count += len(schemas) + for schema in schemas: + if filter_: + if schema.startswith(filter_): + result.append({"text": schema, "value": schema}) + else: + result.append({"text": schema, "value": schema}) + + return self.response(200, count=count, result=result) diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 8b0a93e..db2fab6 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -77,3 +77,13 @@ class TableMetadataResponseSchema(Schema): class SelectStarResponseSchema(Schema): result = fields.String(description="SQL select star") + + +class DatabaseSchemaObjectResponseSchema(Schema): + value = fields.String(description="Schema name") + text = fields.String(description="Schema display name") + + +class DatabaseSchemaResponseSchema(Schema): + count = fields.Integer(description="The total number of schemas") + result = fields.Nested(DatabaseSchemaObjectResponseSchema) diff --git a/tests/database_api_tests.py b/tests/database_api_tests.py index 2cb2d43..9e869ba 100644 --- a/tests/database_api_tests.py +++ b/tests/database_api_tests.py @@ -17,6 +17,7 @@ # isort:skip_file """Unit tests for Superset""" import json +from unittest import mock import prison from sqlalchemy.sql import func @@ -220,3 +221,41 @@ class TestDatabaseApi(SupersetTestCase): uri = f"api/v1/database/{example_db.id}/select_star/table_does_not_exist/" rv = self.client.get(uri) self.assertEqual(rv.status_code, 404) + + def test_schemas(self): + self.login("admin") + dbs = db.session.query(Database).all() + schemas = [] + for database in dbs: + schemas += database.get_all_schema_names() + + rv = self.client.get("api/v1/database/schemas/") + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(len(schemas), response["count"]) + self.assertEqual(schemas[0], response["result"][0]["value"]) + + rv = self.client.get( + f"api/v1/database/schemas/?q={prison.dumps({'filter': 'foo'})}" + ) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(0, len(response["result"])) + + rv = self.client.get( + f"api/v1/database/schemas/?q={prison.dumps({'page': 0, 'page_size': 25})}" + ) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(len(schemas), len(response["result"])) + + rv = self.client.get( + f"api/v1/database/schemas/?q={prison.dumps({'page': 1, 'page_size': 25})}" + ) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(0, len(response["result"])) + + @mock.patch("superset.security_manager.get_schemas_accessible_by_user") + def test_schemas_no_access(self, mock_get_schemas_accessible_by_user): + mock_get_schemas_accessible_by_user.return_value = [] + self.login("admin") + rv = self.client.get("api/v1/database/schemas/") + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(0, response["count"])