From 6f56cd5e9d18ab8c89f282551b349ccf8da99dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CA=88=E1=B5=83=E1=B5=A2?= Date: Mon, 27 Jul 2020 10:14:11 -0700 Subject: [PATCH] feat(listviews): SIP-34 filters for charts, dashboards, datasets (#10335) --- .../components/ListView/ListView_spec.jsx | 231 +++++-------- .../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 +- .../src/components/ListView/types.ts | 30 +- .../src/components/ListView/utils.ts | 23 -- superset-frontend/src/components/Modal.tsx | 2 +- superset-frontend/src/featureFlags.ts | 1 - .../{chartList => CRUD/chart}/ChartList.tsx | 307 ++++++------------ .../dashboard}/DashboardList.tsx | 275 ++++++---------- .../dataset}/AddDatasetModal.tsx | 12 +- .../{datasetList => CRUD/dataset}/Button.tsx | 0 .../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, 681 insertions(+), 991 deletions(-) rename superset-frontend/spec/javascripts/views/{chartList => CRUD/chart}/ChartList_spec.jsx (90%) rename superset-frontend/spec/javascripts/views/{dashboardList => CRUD/dashboard}/DashboardList_spec.jsx (91%) rename superset-frontend/spec/javascripts/views/{datasetList => CRUD/dataset}/DatasetList_spec.jsx (94%) delete mode 100644 superset-frontend/src/components/ListView/LegacyFilters.tsx rename superset-frontend/src/views/{chartList => CRUD/chart}/ChartList.tsx (68%) rename superset-frontend/src/views/{dashboardList => CRUD/dashboard}/DashboardList.tsx (73%) rename superset-frontend/src/views/{datasetList => CRUD/dataset}/AddDatasetModal.tsx (93%) rename superset-frontend/src/views/{datasetList => CRUD/dataset}/Button.tsx (100%) rename superset-frontend/src/views/{datasetList => CRUD/dataset}/DatasetList.tsx (75%) create mode 100644 superset-frontend/src/views/CRUD/utils.tsx diff --git a/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx b/superset-frontend/spec/javascripts/components/ListView/ListView_spec.jsx index de0d179df2..cd1f823382 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: [ @@ -60,10 +60,26 @@ 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(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(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[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 695b74eb9d..ba6c464f12 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 cfaaeca8fb..c847b17159 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 58b5ef0f57..9f7c285c9a 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 2b7ff01bb8..029f1e0522 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 111af4b0a1..0000000000 --- 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>; -}) => ( -
- - - {' '} - {t('Filter')} - - } - > - {filters - .map(({ id, Header }) => ({ - Header, - id, - value: undefined, - })) - .map(ft => ( - { - setInternalFilters([...internalFilters, fltr]); - }} - > - {ft.Header} - - ))} - -
-); - -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 ( -
- - - {ft.Header} - - - ) => { - updateInternalFilter(i, { - operator: e.currentTarget.value, - }); - }} - > - {(filter.operators || []).map( - ({ label, value }: SelectOption) => ( - - ), - )} - - - - - {filter.input === 'select' && ( -