From 9acc7f96d7ecf844ce79b5357f9da5fc48759f28 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 20 Aug 2020 17:02:45 -0700 Subject: [PATCH 01/13] refactor: convert DashboardList to a function component --- .../views/CRUD/dashboard/DashboardList.tsx | 659 +++++++++--------- .../views/CRUD/data/dataset/DatasetList.tsx | 2 +- superset-frontend/src/views/CRUD/utils.tsx | 7 +- 3 files changed, 341 insertions(+), 327 deletions(-) diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 437edbbc34cc6..bac7b05370164 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -18,8 +18,7 @@ */ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useState, useEffect, useCallback } from 'react'; import rison from 'rison'; import { createFetchRelated, @@ -47,7 +46,7 @@ import { Dropdown, Menu } from 'src/common/components'; const PAGE_SIZE = 25; const FAVESTAR_BASE_URL = '/superset/favstar/Dashboard'; -interface Props { +interface DashboardListProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; } @@ -76,12 +75,8 @@ interface Dashboard { owners: Owner[]; } -class DashboardList extends React.PureComponent { - static propTypes = { - addDangerToast: PropTypes.func.isRequired, - }; - - state: State = { +function DashboardList(props: DashboardListProps) { + const [state, setState] = useState({ bulkSelectEnabled: false, dashboardCount: 0, dashboards: [], @@ -90,56 +85,202 @@ class DashboardList extends React.PureComponent { lastFetchDataConfig: null, loading: true, permissions: [], - }; + }); + + function updateState(update: Partial) { + setState({ ...state, ...update }); + } - componentDidMount() { + useEffect(() => { SupersetClient.get({ endpoint: `/api/v1/dashboard/_info`, }).then( ({ json: infoJson = {} }) => { - this.setState({ + updateState({ permissions: infoJson.permissions, }); }, createErrorHandler(errMsg => - this.props.addDangerToast( + props.addDangerToast( t('An error occurred while fetching Dashboards: %s, %s', errMsg), ), ), ); - } + }, []); - get canEdit() { - return this.hasPerm('can_edit'); - } + function hasPerm(perm: string) { + if (!state.permissions.length) { + return false; + } - get canDelete() { - return this.hasPerm('can_delete'); + return Boolean(state.permissions.find(p => p === perm)); } - get canExport() { - return this.hasPerm('can_mulexport'); - } + const canEdit = hasPerm('can_edit'); - initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; + const canDelete = hasPerm('can_delete'); - fetchMethods = createFaveStarHandlers( + const canExport = hasPerm('can_mulexport'); + + const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; + + const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - this, + { + state, + setState: updateState, + }, (message: string) => { - this.props.addDangerToast(message); + props.addDangerToast(message); }, ); - columns = [ + const fetchData = useCallback( + ({ + pageIndex, + pageSize, + sortBy, + filters: filterValues, + }: FetchDataConfig) => { + // set loading state, cache the last config for fetching data in this component. + updateState({ + lastFetchDataConfig: { + filters: filterValues, + pageIndex, + pageSize, + sortBy, + }, + loading: true, + }); + const filterExps = filterValues.map( + ({ id: col, operator: opr, value }) => ({ + col, + opr, + value, + }), + ); + + const queryParams = rison.encode({ + order_column: sortBy[0].id, + order_direction: sortBy[0].desc ? 'desc' : 'asc', + page: pageIndex, + page_size: pageSize, + ...(filterExps.length ? { filters: filterExps } : {}), + }); + + return SupersetClient.get({ + endpoint: `/api/v1/dashboard/?q=${queryParams}`, + }) + .then( + ({ json = {} }) => { + updateState({ + dashboards: json.result, + dashboardCount: json.count, + }); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('An error occurred while fetching dashboards: %s', errMsg), + ), + ), + ) + .finally(() => { + updateState({ loading: false }); + }); + }, + [], + ); + + function openDashboardEditModal(dashboard: Dashboard) { + updateState({ + dashboardToEdit: dashboard, + }); + } + + function handleDashboardEdit(edits: any) { + updateState({ loading: true }); + return SupersetClient.get({ + endpoint: `/api/v1/dashboard/${edits.id}`, + }).then( + ({ json = {} }) => { + updateState({ + dashboards: state.dashboards.map(dashboard => { + if (dashboard.id === json.id) { + return json.result; + } + return dashboard; + }), + loading: false, + }); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('An error occurred while fetching dashboards: %s', errMsg), + ), + ), + ); + } + + function handleDashboardDelete({ + id, + dashboard_title: dashboardTitle, + }: Dashboard) { + return SupersetClient.delete({ + endpoint: `/api/v1/dashboard/${id}`, + }).then( + () => { + const { lastFetchDataConfig } = state; + if (lastFetchDataConfig) { + fetchData(lastFetchDataConfig); + } + props.addSuccessToast(t('Deleted: %s', dashboardTitle)); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('There was an issue deleting %s: %s', dashboardTitle, errMsg), + ), + ), + ); + } + + function handleBulkDashboardDelete(dashboards: Dashboard[]) { + return SupersetClient.delete({ + endpoint: `/api/v1/dashboard/?q=${rison.encode( + dashboards.map(({ id }) => id), + )}`, + }).then( + ({ json = {} }) => { + const { lastFetchDataConfig } = state; + if (lastFetchDataConfig) { + fetchData(lastFetchDataConfig); + } + props.addSuccessToast(json.message); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('There was an issue deleting the selected dashboards: ', errMsg), + ), + ), + ); + } + + function handleBulkDashboardExport(dashboards: Dashboard[]) { + return window.location.assign( + `/api/v1/dashboard/export/?q=${rison.encode( + dashboards.map(({ id }) => id), + )}`, + ); + } + + const columns = [ { Cell: ({ row: { original } }: any) => { return ( ); @@ -215,18 +356,72 @@ class DashboardList extends React.PureComponent { disableSortBy: true, }, { - Cell: ({ row: { original } }: any) => this.renderActions(original), + Cell: ({ row: { original } }: any) => { + const handleDelete = () => handleDashboardDelete(original); + const handleEdit = () => openDashboardEditModal(original); + const handleExport = () => handleBulkDashboardExport([original]); + if (!canEdit && !canDelete && !canExport) { + return null; + } + return ( + + {canDelete && ( + + {t('Are you sure you want to delete')}{' '} + {original.dashboard_title}? + + } + onConfirm={handleDelete} + > + {confirmDelete => ( + + + + )} + + )} + {canExport && ( + + + + )} + {canEdit && ( + + + + )} + + ); + }, Header: t('Actions'), id: 'actions', disableSortBy: true, }, ]; - toggleBulkSelect = () => { - this.setState({ bulkSelectEnabled: !this.state.bulkSelectEnabled }); - }; + function toggleBulkSelect() { + updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); + } - filters: Filters = [ + const filters: Filters = [ { Header: 'Owner', id: 'owners', @@ -237,7 +432,7 @@ class DashboardList extends React.PureComponent { 'dashboard', 'owners', createErrorHandler(errMsg => - this.props.addDangerToast( + props.addDangerToast( t( 'An error occurred while fetching chart owner values: %s', errMsg, @@ -266,7 +461,7 @@ class DashboardList extends React.PureComponent { }, ]; - sortTypes = [ + const sortTypes = [ { desc: false, id: 'dashboard_title', @@ -287,210 +482,20 @@ class DashboardList extends React.PureComponent { }, ]; - hasPerm = (perm: string) => { - if (!this.state.permissions.length) { - return false; - } - - return Boolean(this.state.permissions.find(p => p === perm)); - }; - - openDashboardEditModal = (dashboard: Dashboard) => { - this.setState({ - dashboardToEdit: dashboard, - }); - }; - - handleDashboardEdit = (edits: any) => { - this.setState({ loading: true }); - return SupersetClient.get({ - endpoint: `/api/v1/dashboard/${edits.id}`, - }).then( - ({ json = {} }) => { - this.setState({ - dashboards: this.state.dashboards.map(dashboard => { - if (dashboard.id === json.id) { - return json.result; - } - return dashboard; - }), - loading: false, - }); - }, - createErrorHandler(errMsg => - this.props.addDangerToast( - t('An error occurred while fetching dashboards: %s', errMsg), - ), - ), - ); - }; - - handleDashboardDelete = ({ - id, - dashboard_title: dashboardTitle, - }: Dashboard) => - SupersetClient.delete({ - endpoint: `/api/v1/dashboard/${id}`, - }).then( - () => { - const { lastFetchDataConfig } = this.state; - if (lastFetchDataConfig) { - this.fetchData(lastFetchDataConfig); - } - this.props.addSuccessToast(t('Deleted: %s', dashboardTitle)); - }, - createErrorHandler(errMsg => - this.props.addDangerToast( - t('There was an issue deleting %s: %s', dashboardTitle, errMsg), - ), - ), - ); - - handleBulkDashboardDelete = (dashboards: Dashboard[]) => { - SupersetClient.delete({ - endpoint: `/api/v1/dashboard/?q=${rison.encode( - dashboards.map(({ id }) => id), - )}`, - }).then( - ({ json = {} }) => { - const { lastFetchDataConfig } = this.state; - if (lastFetchDataConfig) { - this.fetchData(lastFetchDataConfig); - } - this.props.addSuccessToast(json.message); - }, - createErrorHandler(errMsg => - this.props.addDangerToast( - t('There was an issue deleting the selected dashboards: ', errMsg), - ), - ), - ); - }; - - handleBulkDashboardExport = (dashboards: Dashboard[]) => { - return window.location.assign( - `/api/v1/dashboard/export/?q=${rison.encode( - dashboards.map(({ id }) => id), - )}`, - ); - }; - - fetchData = ({ pageIndex, pageSize, sortBy, filters }: FetchDataConfig) => { - // set loading state, cache the last config for fetching data in this component. - this.setState({ - lastFetchDataConfig: { - filters, - pageIndex, - pageSize, - sortBy, - }, - loading: true, - }); - const filterExps = filters.map(({ id: col, operator: opr, value }) => ({ - col, - opr, - value, - })); - - const queryParams = rison.encode({ - order_column: sortBy[0].id, - order_direction: sortBy[0].desc ? 'desc' : 'asc', - page: pageIndex, - page_size: pageSize, - ...(filterExps.length ? { filters: filterExps } : {}), - }); - - return SupersetClient.get({ - endpoint: `/api/v1/dashboard/?q=${queryParams}`, - }) - .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 }); - }); - }; - - renderActions(original: Dashboard) { - const handleDelete = () => this.handleDashboardDelete(original); - const handleEdit = () => this.openDashboardEditModal(original); - const handleExport = () => this.handleBulkDashboardExport([original]); - if (!this.canEdit && !this.canDelete && !this.canExport) { - return null; - } - return ( - - {this.canDelete && ( - - {t('Are you sure you want to delete')}{' '} - {original.dashboard_title}? - - } - onConfirm={handleDelete} - > - {confirmDelete => ( - - - - )} - - )} - {this.canExport && ( - - - - )} - {this.canEdit && ( - - - - )} - - ); - } - - renderCard = (props: Dashboard & { loading: boolean }) => { + function renderCard(dashboard: Dashboard & { loading: boolean }) { const menu = ( - {this.canDelete && ( + {canDelete && ( {t('Are you sure you want to delete')}{' '} - {props.dashboard_title}? + {dashboard.dashboard_title}? } - onConfirm={() => this.handleDashboardDelete(props)} + onConfirm={() => handleDashboardDelete(dashboard)} > {confirmDelete => (
{ )} - {this.canExport && ( + {canExport && ( this.handleBulkDashboardExport([props])} + onClick={() => handleBulkDashboardExport([dashboard])} > Export )} - {this.canEdit && ( + {canEdit && ( this.openDashboardEditModal(props)} + onClick={() => openDashboardEditModal(dashboard)} > Edit @@ -528,17 +533,22 @@ class DashboardList extends React.PureComponent { return ( {props.published ? 'published' : 'draft'}} - url={this.state.bulkSelectEnabled ? undefined : props.url} - imgURL={props.thumbnail_url} + loading={dashboard.loading} + title={dashboard.dashboard_title} + titleRight={ + + } + url={state.bulkSelectEnabled ? undefined : dashboard.url} + imgURL={dashboard.thumbnail_url} imgFallbackURL="/static/assets/images/dashboard-card-fallback.png" - description={t('Last modified %s', props.changed_on_delta_humanized)} - coverLeft={(props.owners || []).slice(0, 5).map(owner => ( + description={t( + 'Last modified %s', + dashboard.changed_on_delta_humanized, + )} + coverLeft={dashboard.owners.slice(0, 5).map(owner => ( { actions={ @@ -562,87 +572,86 @@ class DashboardList extends React.PureComponent { } /> ); - }; + } - render() { - const { - bulkSelectEnabled, - dashboards, - dashboardCount, - loading, - dashboardToEdit, - } = this.state; - return ( - <> - + + + {confirmDelete => { + const bulkActions: ListViewProps['bulkActions'] = []; + if (canDelete) { + bulkActions.push({ + key: 'delete', + name: t('Delete'), + type: 'danger', + onSelect: confirmDelete, + }); } - /> - - {confirmDelete => { - const bulkActions: ListViewProps['bulkActions'] = []; - if (this.canDelete) { - bulkActions.push({ - key: 'delete', - name: t('Delete'), - type: 'danger', - onSelect: confirmDelete, - }); - } - if (this.canExport) { - bulkActions.push({ - key: 'export', - name: t('Export'), - type: 'primary', - onSelect: this.handleBulkDashboardExport, - }); - } - return ( - <> - {dashboardToEdit && ( - this.setState({ dashboardToEdit: null })} - onSubmit={this.handleDashboardEdit} - /> - )} - + {dashboardToEdit && ( + updateState({ dashboardToEdit: null })} + onSubmit={handleDashboardEdit} /> - - ); - }} - - - ); - } + )} + + + ); + }} + + + ); } export default withToasts(DashboardList); diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index f7a5ab04f5aff..1e9af703364fe 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -105,6 +105,7 @@ const DatasetList: FunctionComponent = ({ addSuccessToast, }) => { const [datasetCount, setDatasetCount] = useState(0); + const [datasets, setDatasets] = useState([]); const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState< (Dataset & { chart_count: number; dashboard_count: number }) | null >(null); @@ -112,7 +113,6 @@ const DatasetList: FunctionComponent = ({ datasetCurrentlyEditing, setDatasetCurrentlyEditing, ] = useState(null); - const [datasets, setDatasets] = useState([]); const [ lastFetchDataConfig, setLastFetchDataConfig, diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 5aea548a78a82..1286aa1b4aed9 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -62,9 +62,14 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { }; } +interface FaveStarContext { + state: { favoriteStatus: object }; + setState: (update: any) => void; +} + export function createFaveStarHandlers( baseURL: string, - context: any, + context: FaveStarContext, handleErrorFunc: (message: string) => void, ) { const fetchFaveStar = (id: number) => { From 33e16a65f6c5476125c71295607e98c3585fe0f2 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 20 Aug 2020 17:22:05 -0700 Subject: [PATCH 02/13] refactor: convert ChartList to a function component --- .../src/views/CRUD/chart/ChartList.tsx | 493 +++++++++--------- .../views/CRUD/dashboard/DashboardList.tsx | 8 +- 2 files changed, 252 insertions(+), 249 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index fb8d905540c2c..3818261655dc2 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,8 +19,7 @@ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import { getChartMetadataRegistry } from '@superset-ui/chart'; -import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useState, useCallback, useEffect } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; import { @@ -49,7 +48,7 @@ import { Dropdown, Menu } from 'src/common/components'; const PAGE_SIZE = 25; const FAVESTAR_BASE_URL = '/superset/favstar/slice'; -interface Props { +interface ChartListProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; } @@ -102,12 +101,8 @@ const createFetchDatasets = (handleError: (err: Response) => void) => async ( } return []; }; -class ChartList extends React.PureComponent { - static propTypes = { - addDangerToast: PropTypes.func.isRequired, - }; - - state: State = { +function ChartList(props: ChartListProps) { + const [state, setState] = useState({ bulkSelectEnabled: false, chartCount: 0, charts: [], @@ -116,52 +111,182 @@ class ChartList extends React.PureComponent { loading: true, permissions: [], sliceCurrentlyEditing: null, - }; + }); + + function updateState(update: Partial) { + setState({ ...state, ...update }); + } - componentDidMount() { + useEffect(() => { SupersetClient.get({ endpoint: `/api/v1/chart/_info`, }).then( ({ json: infoJson = {} }) => { - this.setState({ + updateState({ permissions: infoJson.permissions, }); }, createErrorHandler(errMsg => - this.props.addDangerToast( + props.addDangerToast( t('An error occurred while fetching chart info: %s', errMsg), ), ), ); - } + }, []); - get canEdit() { - return this.hasPerm('can_edit'); - } + function hasPerm(perm: string) { + if (!state.permissions.length) { + return false; + } - get canDelete() { - return this.hasPerm('can_delete'); + return Boolean(state.permissions.find(p => p === perm)); } - initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; + const canEdit = hasPerm('can_edit'); + + const canDelete = hasPerm('can_delete'); + + const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - fetchMethods = createFaveStarHandlers( + const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - this, + { + state, + setState: updateState, + }, (message: string) => { - this.props.addDangerToast(message); + props.addDangerToast(message); }, ); - columns = [ + const fetchData = useCallback( + ({ + pageIndex, + pageSize, + sortBy, + filters: filterValues, + }: FetchDataConfig) => { + // set loading state, cache the last config for fetching data in this component. + updateState({ + lastFetchDataConfig: { + filters: filterValues, + pageIndex, + pageSize, + sortBy, + }, + loading: true, + }); + + const filterExps = filterValues.map( + ({ id: col, operator: opr, value }) => ({ + col, + opr, + value, + }), + ); + + const queryParams = rison.encode({ + order_column: sortBy[0].id, + order_direction: sortBy[0].desc ? 'desc' : 'asc', + page: pageIndex, + page_size: pageSize, + ...(filterExps.length ? { filters: filterExps } : {}), + }); + + return SupersetClient.get({ + endpoint: `/api/v1/chart/?q=${queryParams}`, + }) + .then( + ({ json = {} }) => { + updateState({ charts: json.result, chartCount: json.count }); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('An error occurred while fetching charts: %s', errMsg), + ), + ), + ) + .finally(() => { + updateState({ loading: false }); + }); + }, + [], + ); + + function toggleBulkSelect() { + updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); + } + + function openChartEditModal(chart: Chart) { + updateState({ + sliceCurrentlyEditing: { + slice_id: chart.id, + slice_name: chart.slice_name, + description: chart.description, + cache_timeout: chart.cache_timeout, + }, + }); + } + + function closeChartEditModal() { + updateState({ sliceCurrentlyEditing: null }); + } + + function handleChartUpdated(edits: Chart) { + // update the chart in our state with the edited info + const newCharts = state.charts.map(chart => + chart.id === edits.id ? { ...chart, ...edits } : chart, + ); + updateState({ + charts: newCharts, + }); + } + + function handleChartDelete({ id, slice_name: sliceName }: Chart) { + SupersetClient.delete({ + endpoint: `/api/v1/chart/${id}`, + }).then( + () => { + const { lastFetchDataConfig } = state; + if (lastFetchDataConfig) { + fetchData(lastFetchDataConfig); + } + props.addSuccessToast(t('Deleted: %s', sliceName)); + }, + () => { + props.addDangerToast(t('There was an issue deleting: %s', sliceName)); + }, + ); + } + + function handleBulkChartDelete(charts: Chart[]) { + SupersetClient.delete({ + endpoint: `/api/v1/chart/?q=${rison.encode(charts.map(({ id }) => id))}`, + }).then( + ({ json = {} }) => { + const { lastFetchDataConfig } = state; + if (lastFetchDataConfig) { + fetchData(lastFetchDataConfig); + } + props.addSuccessToast(json.message); + }, + createErrorHandler(errMsg => + props.addDangerToast( + t('There was an issue deleting the selected charts: %s', errMsg), + ), + ), + ); + } + + const columns = [ { Cell: ({ row: { original } }: any) => { return ( ); @@ -235,15 +360,15 @@ class ChartList extends React.PureComponent { }, { Cell: ({ row: { original } }: any) => { - const handleDelete = () => this.handleChartDelete(original); - const openEditModal = () => this.openChartEditModal(original); - if (!this.canEdit && !this.canDelete) { + const handleDelete = () => handleChartDelete(original); + const openEditModal = () => openChartEditModal(original); + if (!canEdit && !canDelete) { return null; } return ( - {this.canDelete && ( + {canDelete && ( { )} )} - {this.canEdit && ( + {canEdit && ( { }, ]; - filters: Filters = [ + const filters: Filters = [ { Header: t('Owner'), id: 'owners', @@ -296,7 +421,7 @@ class ChartList extends React.PureComponent { 'chart', 'owners', createErrorHandler(errMsg => - this.props.addDangerToast( + props.addDangerToast( t( 'An error occurred while fetching chart dataset values: %s', errMsg, @@ -324,7 +449,7 @@ class ChartList extends React.PureComponent { unfilteredLabel: 'All', fetchSelects: createFetchDatasets( createErrorHandler(errMsg => - this.props.addDangerToast( + props.addDangerToast( t( 'An error occurred while fetching chart dataset values: %s', errMsg, @@ -342,7 +467,7 @@ class ChartList extends React.PureComponent { }, ]; - sortTypes = [ + const sortTypes = [ { desc: false, id: 'slice_name', @@ -363,139 +488,20 @@ class ChartList extends React.PureComponent { }, ]; - hasPerm = (perm: string) => { - if (!this.state.permissions.length) { - return false; - } - - return this.state.permissions.some(p => p === perm); - }; - - toggleBulkSelect = () => { - this.setState({ bulkSelectEnabled: !this.state.bulkSelectEnabled }); - }; - - openChartEditModal = (chart: Chart) => { - this.setState({ - sliceCurrentlyEditing: { - slice_id: chart.id, - slice_name: chart.slice_name, - description: chart.description, - cache_timeout: chart.cache_timeout, - }, - }); - }; - - closeChartEditModal = () => { - this.setState({ sliceCurrentlyEditing: null }); - }; - - handleChartUpdated = (edits: Chart) => { - // update the chart in our state with the edited info - const newCharts = this.state.charts.map(chart => - chart.id === edits.id ? { ...chart, ...edits } : chart, - ); - this.setState({ - charts: newCharts, - }); - }; - - handleChartDelete = ({ id, slice_name: sliceName }: Chart) => { - SupersetClient.delete({ - endpoint: `/api/v1/chart/${id}`, - }).then( - () => { - const { lastFetchDataConfig } = this.state; - if (lastFetchDataConfig) { - this.fetchData(lastFetchDataConfig); - } - this.props.addSuccessToast(t('Deleted: %s', sliceName)); - }, - () => { - this.props.addDangerToast( - t('There was an issue deleting: %s', sliceName), - ); - }, - ); - }; - - handleBulkChartDelete = (charts: Chart[]) => { - SupersetClient.delete({ - endpoint: `/api/v1/chart/?q=${rison.encode(charts.map(({ id }) => id))}`, - }).then( - ({ json = {} }) => { - const { lastFetchDataConfig } = this.state; - if (lastFetchDataConfig) { - this.fetchData(lastFetchDataConfig); - } - this.props.addSuccessToast(json.message); - }, - createErrorHandler(errMsg => - this.props.addDangerToast( - t('There was an issue deleting the selected charts: %s', errMsg), - ), - ), - ); - }; - - fetchData = ({ pageIndex, pageSize, sortBy, filters }: FetchDataConfig) => { - // set loading state, cache the last config for fetching data in this component. - this.setState({ - lastFetchDataConfig: { - filters, - pageIndex, - pageSize, - sortBy, - }, - loading: true, - }); - - const filterExps = filters.map(({ id: col, operator: opr, value }) => ({ - col, - opr, - value, - })); - - const queryParams = rison.encode({ - order_column: sortBy[0].id, - order_direction: sortBy[0].desc ? 'desc' : 'asc', - page: pageIndex, - page_size: pageSize, - ...(filterExps.length ? { filters: filterExps } : {}), - }); - - return SupersetClient.get({ - endpoint: `/api/v1/chart/?q=${queryParams}`, - }) - .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 }); - }); - }; - - renderCard = (props: Chart & { loading: boolean }) => { + function renderCard(chart: Chart & { loading: boolean }) { const menu = ( - {this.canDelete && ( + {canDelete && ( {t('Are you sure you want to delete')}{' '} - {props.slice_name}? + {chart.slice_name}? } - onConfirm={() => this.handleChartDelete(props)} + onConfirm={() => handleChartDelete(chart)} > {confirmDelete => (
{ )} - {this.canEdit && ( + {canEdit && ( this.openChartEditModal(props)} + onClick={() => openChartEditModal(chart)} > Edit @@ -524,16 +530,16 @@ class ChartList extends React.PureComponent { return ( ( + description={t('Last modified %s', chart.changed_on_delta_humanized)} + coverLeft={(chart.owners || []).slice(0, 5).map(owner => ( { /> ))} coverRight={ - + } actions={ @@ -560,79 +566,76 @@ class ChartList extends React.PureComponent { } /> ); - }; - - render() { - const { - bulkSelectEnabled, - charts, - chartCount, - loading, - sliceCurrentlyEditing, - } = this.state; - return ( - <> - - {sliceCurrentlyEditing && ( - - )} - - {confirmDelete => { - const bulkActions: ListViewProps['bulkActions'] = this.canDelete - ? [ - { - key: 'delete', - name: t('Delete'), - onSelect: confirmDelete, - type: 'danger', - }, - ] - : []; - - return ( - - ); - }} - - - ); } + + const { + bulkSelectEnabled, + charts, + chartCount, + loading, + sliceCurrentlyEditing, + } = state; + + return ( + <> + + {sliceCurrentlyEditing && ( + + )} + + {confirmDelete => { + const bulkActions: ListViewProps['bulkActions'] = canDelete + ? [ + { + key: 'delete', + name: t('Delete'), + onSelect: confirmDelete, + type: 'danger', + }, + ] + : []; + + return ( + + ); + }} + + + ); } export default withToasts(ChartList); diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index bac7b05370164..80452ea338d6b 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -191,6 +191,10 @@ function DashboardList(props: DashboardListProps) { [], ); + function toggleBulkSelect() { + updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); + } + function openDashboardEditModal(dashboard: Dashboard) { updateState({ dashboardToEdit: dashboard, @@ -417,10 +421,6 @@ function DashboardList(props: DashboardListProps) { }, ]; - function toggleBulkSelect() { - updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); - } - const filters: Filters = [ { Header: 'Owner', From 555cc19ee34ae79955a75307faf279da6c836181 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 20 Aug 2020 19:33:45 -0700 Subject: [PATCH 03/13] refactor: convert DatasetList to a function component --- .../views/CRUD/data/dataset/DatasetList.tsx | 265 +++++++++--------- 1 file changed, 136 insertions(+), 129 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 1e9af703364fe..1606677f04074 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -66,6 +66,20 @@ interface DatasetListProps { addSuccessToast: (msg: string) => void; } +interface DatasetListState { + bulkSelectEnabled: boolean; + datasetCount: number; + datasets: Dataset[]; + lastFetchDataConfig: FetchDataConfig | null; + loading: boolean; + permissions: string[]; + datasetAddModalOpen: boolean; + datasetCurrentlyDeleting: + | (Dataset & { chart_count: number; dashboard_count: number }) + | null; + datasetCurrentlyEditing: Dataset | null; +} + export const createFetchSchemas = ( handleError: (error: Response) => void, ) => async (filterValue = '', pageIndex?: number, pageSize?: number) => { @@ -104,26 +118,21 @@ const DatasetList: FunctionComponent = ({ addDangerToast, addSuccessToast, }) => { - const [datasetCount, setDatasetCount] = useState(0); - const [datasets, setDatasets] = useState([]); - const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState< - (Dataset & { chart_count: number; dashboard_count: number }) | null - >(null); - const [ - datasetCurrentlyEditing, - setDatasetCurrentlyEditing, - ] = useState(null); - const [ - lastFetchDataConfig, - setLastFetchDataConfig, - ] = useState(null); - const [loading, setLoading] = useState(true); - const [permissions, setPermissions] = useState([]); - - const [datasetAddModalOpen, setDatasetAddModalOpen] = useState( - false, - ); - const [bulkSelectEnabled, setBulkSelectEnabled] = useState(false); + const [state, setState] = useState({ + bulkSelectEnabled: false, + datasetCount: 0, + datasets: [], + lastFetchDataConfig: null, + loading: true, + permissions: [], + datasetCurrentlyDeleting: null, + datasetCurrentlyEditing: null, + datasetAddModalOpen: false, + }); + + function updateState(update: Partial) { + setState(currentState => ({ ...currentState, ...update })); + } const filterTypes: Filters = [ { @@ -197,7 +206,9 @@ const DatasetList: FunctionComponent = ({ endpoint: `/api/v1/dataset/_info`, }).then( ({ json: infoJson = {} }) => { - setPermissions(infoJson.permissions); + updateState({ + permissions: infoJson.permissions, + }); }, createErrorHandler(errMsg => addDangerToast(t('An error occurred while fetching datasets', errMsg)), @@ -210,11 +221,11 @@ const DatasetList: FunctionComponent = ({ }, []); const hasPerm = (perm: string) => { - if (!permissions.length) { + if (!state.permissions.length) { return false; } - return Boolean(permissions.find(p => p === perm)); + return Boolean(state.permissions.find(p => p === perm)); }; const canEdit = hasPerm('can_edit'); @@ -229,7 +240,7 @@ const DatasetList: FunctionComponent = ({ }) .then(({ json = {} }) => { const owners = json.result.owners.map((owner: any) => owner.id); - setDatasetCurrentlyEditing({ ...json.result, owners }); + updateState({ datasetCurrentlyEditing: { ...json.result, owners } }); }) .catch(() => { addDangerToast( @@ -243,10 +254,12 @@ const DatasetList: FunctionComponent = ({ endpoint: `/api/v1/dataset/${dataset.id}/related_objects`, }) .then(({ json = {} }) => { - setDatasetCurrentlyDeleting({ - ...dataset, - chart_count: json.charts.count, - dashboard_count: json.dashboards.count, + updateState({ + datasetCurrentlyDeleting: { + ...dataset, + chart_count: json.charts.count, + dashboard_count: json.dashboards.count, + }, }); }) .catch( @@ -453,33 +466,38 @@ const DatasetList: FunctionComponent = ({ {t('Dataset')}{' '} ), - onClick: () => setDatasetAddModalOpen(true), + onClick: () => updateState({ datasetAddModalOpen: true }), }; } if (canDelete) { menuData.secondaryButton = { name: t('Bulk Select'), - onClick: () => setBulkSelectEnabled(!bulkSelectEnabled), + onClick: () => + updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }), }; } const closeDatasetDeleteModal = () => { - setDatasetCurrentlyDeleting(null); + updateState({ datasetCurrentlyDeleting: null }); }; - const closeDatasetEditModal = () => setDatasetCurrentlyEditing(null); + const closeDatasetEditModal = () => + updateState({ datasetCurrentlyEditing: null }); const fetchData = useCallback( ({ pageIndex, pageSize, sortBy, filters }: FetchDataConfig) => { // set loading state, cache the last config for fetching data in this component. - setLoading(true); - setLastFetchDataConfig({ - filters, - pageIndex, - pageSize, - sortBy, + updateState({ + lastFetchDataConfig: { + filters, + pageIndex, + pageSize, + sortBy, + }, + loading: true, }); + const filterExps = filters.map(({ id: col, operator: opr, value }) => ({ col, opr, @@ -499,9 +517,10 @@ const DatasetList: FunctionComponent = ({ }) .then( ({ json }) => { - setLoading(false); - setDatasets(json.result); - setDatasetCount(json.count); + updateState({ + datasets: json.result, + datasetCount: json.count, + }); }, createErrorHandler(errMsg => addDangerToast( @@ -509,7 +528,9 @@ const DatasetList: FunctionComponent = ({ ), ), ) - .finally(() => setLoading(false)); + .finally(() => { + updateState({ loading: false }); + }); }, [], ); @@ -519,10 +540,10 @@ const DatasetList: FunctionComponent = ({ endpoint: `/api/v1/dataset/${id}`, }).then( () => { - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); + if (state.lastFetchDataConfig) { + fetchData(state.lastFetchDataConfig); } - setDatasetCurrentlyDeleting(null); + updateState({ datasetCurrentlyDeleting: null }); addSuccessToast(t('Deleted: %s', tableName)); }, createErrorHandler(errMsg => @@ -536,12 +557,12 @@ const DatasetList: FunctionComponent = ({ const handleBulkDatasetDelete = () => { SupersetClient.delete({ endpoint: `/api/v1/dataset/?q=${rison.encode( - datasets.map(({ id }) => id), + state.datasets.map(({ id }) => id), )}`, }).then( ({ json = {} }) => { - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); + if (state.lastFetchDataConfig) { + fetchData(state.lastFetchDataConfig); } addSuccessToast(json.message); }, @@ -554,8 +575,8 @@ const DatasetList: FunctionComponent = ({ }; const handleUpdateDataset = () => { - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); + if (state.lastFetchDataConfig) { + fetchData(state.lastFetchDataConfig); } }; @@ -563,26 +584,38 @@ const DatasetList: FunctionComponent = ({ <> setDatasetAddModalOpen(false)} + show={state.datasetAddModalOpen} + onHide={() => updateState({ datasetAddModalOpen: false })} onDatasetAdd={() => { - if (lastFetchDataConfig) fetchData(lastFetchDataConfig); + if (state.lastFetchDataConfig) fetchData(state.lastFetchDataConfig); }} /> - {datasetCurrentlyDeleting && ( + {state.datasetCurrentlyDeleting && ( handleDatasetDelete(datasetCurrentlyDeleting)} + onConfirm={() => { + if (state.datasetCurrentlyDeleting) { + handleDatasetDelete(state.datasetCurrentlyDeleting); + } + }} onHide={closeDatasetDeleteModal} open title={t('Delete Dataset?')} /> )} + {state.datasetCurrentlyEditing && ( + + )} = ({ : []; return ( - <> - {datasetCurrentlyDeleting && ( - - handleDatasetDelete(datasetCurrentlyDeleting) - } - onHide={closeDatasetDeleteModal} - open - title={t('Delete Dataset?')} - /> - )} - {datasetCurrentlyEditing && ( - - )} - setBulkSelectEnabled(false)} - renderBulkSelectCopy={selected => { - const { virtualCount, physicalCount } = selected.reduce( - (acc, e) => { - if (e.original.kind === 'physical') - acc.physicalCount += 1; - else if (e.original.kind === 'virtual') - acc.virtualCount += 1; - return acc; - }, - { virtualCount: 0, physicalCount: 0 }, + + updateState({ bulkSelectEnabled: false }) + } + renderBulkSelectCopy={selected => { + const { virtualCount, physicalCount } = selected.reduce( + (acc, e) => { + if (e.original.kind === 'physical') acc.physicalCount += 1; + else if (e.original.kind === 'virtual') + acc.virtualCount += 1; + return acc; + }, + { virtualCount: 0, physicalCount: 0 }, + ); + + if (!selected.length) { + return t('0 Selected'); + } else if (virtualCount && !physicalCount) { + return t( + '%s Selected (Virtual)', + selected.length, + virtualCount, ); - - if (!selected.length) { - return t('0 Selected'); - } else if (virtualCount && !physicalCount) { - return t( - '%s Selected (Virtual)', - selected.length, - virtualCount, - ); - } else if (physicalCount && !virtualCount) { - return t( - '%s Selected (Physical)', - selected.length, - physicalCount, - ); - } - + } else if (physicalCount && !virtualCount) { return t( - '%s Selected (%s Physical, %s Virtual)', + '%s Selected (Physical)', selected.length, physicalCount, - virtualCount, ); - }} - /> - + } + + return t( + '%s Selected (%s Physical, %s Virtual)', + selected.length, + physicalCount, + virtualCount, + ); + }} + /> ); }} From 912203e920e7be920e4a495e55c3a8f5a9b9bdbf Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 20 Aug 2020 19:34:16 -0700 Subject: [PATCH 04/13] fix: use functional state update, optimize favestar fetching --- superset-frontend/src/components/FaveStar.tsx | 11 ++++++++--- superset-frontend/src/views/CRUD/chart/ChartList.tsx | 11 +++++++---- .../src/views/CRUD/dashboard/DashboardList.tsx | 4 ++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/components/FaveStar.tsx b/superset-frontend/src/components/FaveStar.tsx index dc1f6e847aae0..81d7b1b5fb7f1 100644 --- a/superset-frontend/src/components/FaveStar.tsx +++ b/superset-frontend/src/components/FaveStar.tsx @@ -25,7 +25,7 @@ interface FaveStarProps { itemId: number; fetchFaveStar(id: number): any; saveFaveStar(id: number, isStarred: boolean): any; - isStarred: boolean; + isStarred?: boolean; width?: number; height?: number; showTooltip?: boolean; @@ -33,12 +33,17 @@ interface FaveStarProps { export default class FaveStar extends React.PureComponent { componentDidMount() { - this.props.fetchFaveStar(this.props.itemId); + // only fetch if we don't know the current status + if (typeof this.props.isStarred === 'undefined') { + this.props.fetchFaveStar(this.props.itemId); + } } onClick(e: React.MouseEvent) { e.preventDefault(); - this.props.saveFaveStar(this.props.itemId, this.props.isStarred); + if (typeof this.props.isStarred !== 'undefined') { + this.props.saveFaveStar(this.props.itemId, this.props.isStarred); + } } render() { diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 3818261655dc2..d36997af37028 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -114,7 +114,7 @@ function ChartList(props: ChartListProps) { }); function updateState(update: Partial) { - setState({ ...state, ...update }); + setState(currentState => ({ ...currentState, ...update })); } useEffect(() => { @@ -198,7 +198,10 @@ function ChartList(props: ChartListProps) { }) .then( ({ json = {} }) => { - updateState({ charts: json.result, chartCount: json.count }); + updateState({ + charts: json.result, + chartCount: json.count, + }); }, createErrorHandler(errMsg => props.addDangerToast( @@ -286,7 +289,7 @@ function ChartList(props: ChartListProps) { itemId={original.id} fetchFaveStar={fetchFaveStarMethods.fetchFaveStar} saveFaveStar={fetchFaveStarMethods.saveFaveStar} - isStarred={!!state.favoriteStatus[original.id]} + isStarred={state.favoriteStatus[original.id]} height={20} /> ); @@ -555,7 +558,7 @@ function ChartList(props: ChartListProps) { itemId={chart.id} fetchFaveStar={fetchFaveStarMethods.fetchFaveStar} saveFaveStar={fetchFaveStarMethods.saveFaveStar} - isStarred={!!state.favoriteStatus[chart.id]} + isStarred={state.favoriteStatus[chart.id]} width={20} height={20} /> diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 80452ea338d6b..d607607b50cd4 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -88,7 +88,7 @@ function DashboardList(props: DashboardListProps) { }); function updateState(update: Partial) { - setState({ ...state, ...update }); + setState(currentState => ({ ...currentState, ...update })); } useEffect(() => { @@ -176,6 +176,7 @@ function DashboardList(props: DashboardListProps) { updateState({ dashboards: json.result, dashboardCount: json.count, + loading: false, }); }, createErrorHandler(errMsg => @@ -214,7 +215,6 @@ function DashboardList(props: DashboardListProps) { } return dashboard; }), - loading: false, }); }, createErrorHandler(errMsg => From 72473ee448e5232cbda5387f4c1528999a874468 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Fri, 21 Aug 2020 11:40:23 -0700 Subject: [PATCH 05/13] guard against empty owners --- superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index d607607b50cd4..9635ba46f3219 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -545,7 +545,7 @@ function DashboardList(props: DashboardListProps) { 'Last modified %s', dashboard.changed_on_delta_humanized, )} - coverLeft={dashboard.owners.slice(0, 5).map(owner => ( + coverLeft={(dashboard.owners || []).slice(0, 5).map(owner => ( Date: Tue, 25 Aug 2020 22:18:38 -0700 Subject: [PATCH 06/13] refactor: convert ChartList to use new ListViewResource hook --- .../src/views/CRUD/chart/ChartList.tsx | 560 +++++++----------- .../views/CRUD/dashboard/DashboardList.tsx | 6 +- superset-frontend/src/views/CRUD/hooks.ts | 168 ++++++ superset-frontend/src/views/CRUD/utils.tsx | 20 +- 4 files changed, 405 insertions(+), 349 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/hooks.ts diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index d36997af37028..d3329b6194b70 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,7 +19,7 @@ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import { getChartMetadataRegistry } from '@superset-ui/chart'; -import React, { useState, useCallback, useEffect } from 'react'; +import React, { useState, useMemo } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; import { @@ -27,6 +27,7 @@ import { createErrorHandler, createFaveStarHandlers, } from 'src/views/CRUD/utils'; +import { useListViewResource } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import AvatarIcon from 'src/components/AvatarIcon'; @@ -34,7 +35,6 @@ import Icon from 'src/components/Icon'; import FaveStar from 'src/components/FaveStar'; import ListView, { ListViewProps, - FetchDataConfig, Filters, SelectOption, } from 'src/components/ListView'; @@ -48,23 +48,6 @@ import { Dropdown, Menu } from 'src/common/components'; const PAGE_SIZE = 25; const FAVESTAR_BASE_URL = '/superset/favstar/slice'; -interface ChartListProps { - addDangerToast: (msg: string) => void; - addSuccessToast: (msg: string) => void; -} - -interface State { - bulkSelectEnabled: boolean; - chartCount: number; - charts: Chart[]; - favoriteStatus: object; - 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 ( filterValue = '', pageIndex?: number, @@ -101,46 +84,31 @@ const createFetchDatasets = (handleError: (err: Response) => void) => async ( } return []; }; -function ChartList(props: ChartListProps) { - const [state, setState] = useState({ - bulkSelectEnabled: false, - chartCount: 0, - charts: [], - favoriteStatus: {}, // Hash mapping dashboard id to 'isStarred' status - lastFetchDataConfig: null, - loading: true, - permissions: [], - sliceCurrentlyEditing: null, - }); - - function updateState(update: Partial) { - setState(currentState => ({ ...currentState, ...update })); - } - - useEffect(() => { - SupersetClient.get({ - endpoint: `/api/v1/chart/_info`, - }).then( - ({ json: infoJson = {} }) => { - updateState({ - permissions: infoJson.permissions, - }); - }, - createErrorHandler(errMsg => - props.addDangerToast( - t('An error occurred while fetching chart info: %s', errMsg), - ), - ), - ); - }, []); - function hasPerm(perm: string) { - if (!state.permissions.length) { - return false; - } +interface ChartListProps { + addDangerToast: (msg: string) => void; + addSuccessToast: (msg: string) => void; +} - return Boolean(state.permissions.find(p => p === perm)); - } +function ChartList(props: ChartListProps) { + const { + state: { + loading, + resourceCount: chartCount, + resourceCollection: charts, + bulkSelectEnabled, + }, + setResourceCollection: setCharts, + hasPerm, + fetchData, + toggleBulkSelect, + refreshData, + } = useListViewResource('chart', t('chart'), props.addDangerToast); + const [favoriteStatus, setFavoriteStatus] = useState({}); + const [ + sliceCurrentlyEditing, + setSliceCurrentlyEditing, + ] = useState(null); const canEdit = hasPerm('can_edit'); @@ -150,99 +118,32 @@ function ChartList(props: ChartListProps) { const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - { - state, - setState: updateState, - }, + favoriteStatus, + setFavoriteStatus, (message: string) => { props.addDangerToast(message); }, ); - const fetchData = useCallback( - ({ - pageIndex, - pageSize, - sortBy, - filters: filterValues, - }: FetchDataConfig) => { - // set loading state, cache the last config for fetching data in this component. - updateState({ - lastFetchDataConfig: { - filters: filterValues, - pageIndex, - pageSize, - sortBy, - }, - loading: true, - }); - - const filterExps = filterValues.map( - ({ id: col, operator: opr, value }) => ({ - col, - opr, - value, - }), - ); - - const queryParams = rison.encode({ - order_column: sortBy[0].id, - order_direction: sortBy[0].desc ? 'desc' : 'asc', - page: pageIndex, - page_size: pageSize, - ...(filterExps.length ? { filters: filterExps } : {}), - }); - - return SupersetClient.get({ - endpoint: `/api/v1/chart/?q=${queryParams}`, - }) - .then( - ({ json = {} }) => { - updateState({ - charts: json.result, - chartCount: json.count, - }); - }, - createErrorHandler(errMsg => - props.addDangerToast( - t('An error occurred while fetching charts: %s', errMsg), - ), - ), - ) - .finally(() => { - updateState({ loading: false }); - }); - }, - [], - ); - - function toggleBulkSelect() { - updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); - } - function openChartEditModal(chart: Chart) { - updateState({ - sliceCurrentlyEditing: { - slice_id: chart.id, - slice_name: chart.slice_name, - description: chart.description, - cache_timeout: chart.cache_timeout, - }, + setSliceCurrentlyEditing({ + slice_id: chart.id, + slice_name: chart.slice_name, + description: chart.description, + cache_timeout: chart.cache_timeout, }); } function closeChartEditModal() { - updateState({ sliceCurrentlyEditing: null }); + setSliceCurrentlyEditing(null); } function handleChartUpdated(edits: Chart) { // update the chart in our state with the edited info - const newCharts = state.charts.map(chart => + const newCharts = charts.map(chart => chart.id === edits.id ? { ...chart, ...edits } : chart, ); - updateState({ - charts: newCharts, - }); + setCharts(newCharts); } function handleChartDelete({ id, slice_name: sliceName }: Chart) { @@ -250,10 +151,7 @@ function ChartList(props: ChartListProps) { endpoint: `/api/v1/chart/${id}`, }).then( () => { - const { lastFetchDataConfig } = state; - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); - } + refreshData(); props.addSuccessToast(t('Deleted: %s', sliceName)); }, () => { @@ -262,15 +160,14 @@ function ChartList(props: ChartListProps) { ); } - function handleBulkChartDelete(charts: Chart[]) { + function handleBulkChartDelete(chartsToDelete: Chart[]) { SupersetClient.delete({ - endpoint: `/api/v1/chart/?q=${rison.encode(charts.map(({ id }) => id))}`, + endpoint: `/api/v1/chart/?q=${rison.encode( + chartsToDelete.map(({ id }) => id), + )}`, }).then( ({ json = {} }) => { - const { lastFetchDataConfig } = state; - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); - } + refreshData(); props.addSuccessToast(json.message); }, createErrorHandler(errMsg => @@ -281,194 +178,203 @@ function ChartList(props: ChartListProps) { ); } - const columns = [ - { - Cell: ({ row: { original } }: any) => { - return ( - - ); - }, - Header: '', - id: 'favorite', - disableSortBy: true, - }, - { - Cell: ({ - row: { - original: { url, slice_name: sliceName }, - }, - }: any) => {sliceName}, - Header: t('Chart'), - accessor: 'slice_name', - }, - { - Cell: ({ - row: { - original: { viz_type: vizType }, - }, - }: any) => vizType, - Header: t('Visualization Type'), - accessor: 'viz_type', - }, - { - Cell: ({ - row: { - original: { datasource_name_text: dsNameTxt, datasource_url: dsUrl }, + const columns = useMemo( + () => [ + { + Cell: ({ row: { original } }: any) => { + return ( + + ); }, - }: any) => {dsNameTxt}, - Header: t('Datasource'), - accessor: 'datasource_name', - }, - { - Cell: ({ - row: { - original: { - changed_by_name: changedByName, - changed_by_url: changedByUrl, + Header: '', + id: 'favorite', + disableSortBy: true, + }, + { + Cell: ({ + row: { + original: { url, slice_name: sliceName }, }, - }, - }: any) => {changedByName}, - Header: t('Modified By'), - accessor: 'changed_by.first_name', - }, - { - Cell: ({ - row: { - original: { changed_on_delta_humanized: changedOn }, - }, - }: any) => {changedOn}, - Header: t('Last Modified'), - accessor: 'changed_on_delta_humanized', - }, - { - accessor: 'description', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'owners', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'datasource_id', - hidden: true, - disableSortBy: true, - }, - { - Cell: ({ row: { original } }: any) => { - const handleDelete = () => handleChartDelete(original); - const openEditModal = () => openChartEditModal(original); - if (!canEdit && !canDelete) { - return null; - } + }: any) => {sliceName}, + Header: t('Chart'), + accessor: 'slice_name', + }, + { + Cell: ({ + row: { + original: { viz_type: vizType }, + }, + }: any) => vizType, + Header: t('Visualization Type'), + accessor: 'viz_type', + }, + { + Cell: ({ + row: { + original: { + datasource_name_text: dsNameTxt, + datasource_url: dsUrl, + }, + }, + }: any) => {dsNameTxt}, + Header: t('Datasource'), + accessor: 'datasource_name', + }, + { + Cell: ({ + row: { + original: { + changed_by_name: changedByName, + changed_by_url: changedByUrl, + }, + }, + }: any) => {changedByName}, + Header: t('Modified By'), + accessor: 'changed_by.first_name', + }, + { + Cell: ({ + row: { + original: { changed_on_delta_humanized: changedOn }, + }, + }: any) => {changedOn}, + Header: t('Last Modified'), + accessor: 'changed_on_delta_humanized', + }, + { + accessor: 'description', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'owners', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'datasource_id', + hidden: true, + disableSortBy: true, + }, + { + Cell: ({ row: { original } }: any) => { + const handleDelete = () => handleChartDelete(original); + const openEditModal = () => openChartEditModal(original); + if (!canEdit && !canDelete) { + return null; + } - return ( - - {canDelete && ( - - {t('Are you sure you want to delete')}{' '} - {original.slice_name}? - - } - onConfirm={handleDelete} - > - {confirmDelete => ( - - - - )} - - )} - {canEdit && ( - - - - )} - - ); + return ( + + {canDelete && ( + + {t('Are you sure you want to delete')}{' '} + {original.slice_name}? + + } + onConfirm={handleDelete} + > + {confirmDelete => ( + + + + )} + + )} + {canEdit && ( + + + + )} + + ); + }, + Header: t('Actions'), + id: 'actions', + disableSortBy: true, }, - Header: t('Actions'), - id: 'actions', - disableSortBy: true, - }, - ]; + ], + [], + ); - const filters: Filters = [ - { - Header: t('Owner'), - id: 'owners', - input: 'select', - operator: 'rel_m_m', - unfilteredLabel: 'All', - fetchSelects: createFetchRelated( - 'chart', - 'owners', - createErrorHandler(errMsg => - props.addDangerToast( - t( - 'An error occurred while fetching chart dataset values: %s', - errMsg, + const filters: Filters = useMemo( + () => [ + { + Header: t('Owner'), + id: 'owners', + input: 'select', + operator: 'rel_m_m', + unfilteredLabel: 'All', + fetchSelects: createFetchRelated( + 'chart', + 'owners', + createErrorHandler(errMsg => + 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('Datasource'), - id: 'datasource_id', - input: 'select', - operator: 'eq', - unfilteredLabel: 'All', - fetchSelects: createFetchDatasets( - createErrorHandler(errMsg => - 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('Datasource'), + id: 'datasource_id', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + fetchSelects: createFetchDatasets( + createErrorHandler(errMsg => + 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', - }, - ]; + paginate: false, + }, + { + Header: t('Search'), + id: 'slice_name', + input: 'search', + operator: 'name_or_description', + }, + ], + [], + ); const sortTypes = [ { @@ -535,7 +441,7 @@ function ChartList(props: ChartListProps) { @@ -571,14 +477,6 @@ function ChartList(props: ChartListProps) { ); } - const { - bulkSelectEnabled, - charts, - chartCount, - loading, - sliceCurrentlyEditing, - } = state; - return ( <> updateState({ favoriteStatus }), (message: string) => { props.addDangerToast(message); }, diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts new file mode 100644 index 0000000000000..0b48efa0b291f --- /dev/null +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -0,0 +1,168 @@ +/** + * 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 rison from 'rison'; +import { useState, useEffect, useCallback } from 'react'; +import { SupersetClient } from '@superset-ui/connection'; +import { t } from '@superset-ui/translation'; + +import { createErrorHandler } from 'src/views/CRUD/utils'; +import { FetchDataConfig } from 'src/components/ListView'; + +interface ListViewResourceState { + loading: boolean; + collection: D[]; + count: number; + permissions: string[]; + lastFetchDataConfig: FetchDataConfig | null; + bulkSelectEnabled: boolean; +} + +export function useListViewResource( + resource: string, + resourceLabel: string, // resourceLabel for translations + handleErrorMsg: (errorMsg: string) => void, +) { + const [state, setState] = useState>({ + count: 0, + collection: [], + loading: true, + lastFetchDataConfig: null, + permissions: [], + bulkSelectEnabled: false, + }); + + function updateState(update: Partial>) { + setState(currentState => ({ ...currentState, ...update })); + } + + function toggleBulkSelect() { + updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); + } + + useEffect(() => { + SupersetClient.get({ + endpoint: `/api/v1/${resource}/_info`, + }).then( + ({ json: infoJson = {} }) => { + updateState({ + permissions: infoJson.permissions, + }); + }, + createErrorHandler(errMsg => + handleErrorMsg( + t( + 'An error occurred while fetching %s info: %s', + resourceLabel, + errMsg, + ), + ), + ), + ); + }, []); + + function hasPerm(perm: string) { + if (!state.permissions.length) { + return false; + } + + return Boolean(state.permissions.find(p => p === perm)); + } + + const fetchData = useCallback( + ({ + pageIndex, + pageSize, + sortBy, + filters: filterValues, + }: FetchDataConfig) => { + // set loading state, cache the last config for refreshing data. + updateState({ + lastFetchDataConfig: { + filters: filterValues, + pageIndex, + pageSize, + sortBy, + }, + loading: true, + }); + + const filterExps = filterValues.map( + ({ id: col, operator: opr, value }) => ({ + col, + opr, + value, + }), + ); + + const queryParams = rison.encode({ + order_column: sortBy[0].id, + order_direction: sortBy[0].desc ? 'desc' : 'asc', + page: pageIndex, + page_size: pageSize, + ...(filterExps.length ? { filters: filterExps } : {}), + }); + + return SupersetClient.get({ + endpoint: `/api/v1/chart/?q=${queryParams}`, + }) + .then( + ({ json = {} }) => { + updateState({ + collection: json.result, + count: json.count, + }); + }, + createErrorHandler(errMsg => + handleErrorMsg( + t( + 'An error occurred while fetching %s: %s', + resourceLabel, + errMsg, + ), + ), + ), + ) + .finally(() => { + updateState({ loading: false }); + }); + }, + [], + ); + + return { + state: { + loading: state.loading, + resourceCount: state.count, + resourceCollection: state.collection, + bulkSelectEnabled: state.bulkSelectEnabled, + }, + setResourceCollection: (update: D[]) => + updateState({ + collection: update, + }), + hasPerm, + fetchData, + toggleBulkSelect, + refreshData: () => { + if (state.lastFetchDataConfig) { + fetchData(state.lastFetchDataConfig); + } + }, + }; +} diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 1286aa1b4aed9..6ef6849d558c3 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -62,14 +62,10 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { }; } -interface FaveStarContext { - state: { favoriteStatus: object }; - setState: (update: any) => void; -} - export function createFaveStarHandlers( baseURL: string, - context: FaveStarContext, + favoriteStatus: object, + setFavoriteStatus: (update: any) => void, handleErrorFunc: (message: string) => void, ) { const fetchFaveStar = (id: number) => { @@ -78,14 +74,12 @@ export function createFaveStarHandlers( }) .then(({ json }) => { const faves = { - ...context.state.favoriteStatus, + ...favoriteStatus, }; faves[id] = json.count > 0; - context.setState({ - favoriteStatus: faves, - }); + setFavoriteStatus(faves); }) .catch(() => handleErrorFunc(t('There was an error fetching the favorite status')), @@ -100,14 +94,12 @@ export function createFaveStarHandlers( }) .then(() => { const faves = { - ...context.state.favoriteStatus, + ...favoriteStatus, }; faves[id] = !isStarred; - context.setState({ - favoriteStatus: faves, - }); + setFavoriteStatus(faves); }) .catch(() => handleErrorFunc(t('There was an error saving the favorite status')), From c7b68a9a8780772b0008986ed1ec2f4592259408 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Tue, 25 Aug 2020 23:21:55 -0700 Subject: [PATCH 07/13] refactor: convert DatasetList and ChartList to useListViewResource --- .../src/views/CRUD/chart/ChartList.tsx | 2 - .../views/CRUD/dashboard/DashboardList.tsx | 190 ++++----------- .../views/CRUD/data/dataset/DatasetList.tsx | 225 +++++------------- superset-frontend/src/views/CRUD/hooks.ts | 2 +- 4 files changed, 101 insertions(+), 318 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index d3329b6194b70..c22fd3a07e40b 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -111,9 +111,7 @@ function ChartList(props: ChartListProps) { ] = useState(null); const canEdit = hasPerm('can_edit'); - const canDelete = hasPerm('can_delete'); - const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; const fetchFaveStarMethods = createFaveStarHandlers( diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index bba54c9df9494..e3e0b198e03fe 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -18,21 +18,18 @@ */ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState } from 'react'; import rison from 'rison'; import { createFetchRelated, createErrorHandler, createFaveStarHandlers, } from 'src/views/CRUD/utils'; +import { useListViewResource } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import AvatarIcon from 'src/components/AvatarIcon'; -import ListView, { - ListViewProps, - FetchDataConfig, - Filters, -} from 'src/components/ListView'; +import ListView, { ListViewProps, Filters } from 'src/components/ListView'; import ExpandableList from 'src/components/ExpandableList'; import Owner from 'src/types/Owner'; import withToasts from 'src/messageToasts/enhancers/withToasts'; @@ -51,17 +48,6 @@ interface DashboardListProps { addSuccessToast: (msg: string) => void; } -interface State { - bulkSelectEnabled: boolean; - dashboardCount: number; - dashboards: Dashboard[]; - favoriteStatus: object; - dashboardToEdit: Dashboard | null; - lastFetchDataConfig: FetchDataConfig | null; - loading: boolean; - permissions: string[]; -} - interface Dashboard { changed_by_name: string; changed_by_url: string; @@ -76,45 +62,28 @@ interface Dashboard { } function DashboardList(props: DashboardListProps) { - const [state, setState] = useState({ - bulkSelectEnabled: false, - dashboardCount: 0, - dashboards: [], - favoriteStatus: {}, // Hash mapping dashboard id to 'isStarred' status - dashboardToEdit: null, - lastFetchDataConfig: null, - loading: true, - permissions: [], - }); - - function updateState(update: Partial) { - setState(currentState => ({ ...currentState, ...update })); - } - - useEffect(() => { - SupersetClient.get({ - endpoint: `/api/v1/dashboard/_info`, - }).then( - ({ json: infoJson = {} }) => { - updateState({ - permissions: infoJson.permissions, - }); - }, - createErrorHandler(errMsg => - props.addDangerToast( - t('An error occurred while fetching Dashboards: %s, %s', errMsg), - ), - ), - ); - }, []); - - function hasPerm(perm: string) { - if (!state.permissions.length) { - return false; - } + const { + state: { + loading, + resourceCount: dashboardCount, + resourceCollection: dashboards, + bulkSelectEnabled, + }, + setResourceCollection: setDashboards, + hasPerm, + fetchData, + toggleBulkSelect, + refreshData, + } = useListViewResource( + 'dashboard', + t('dashboard'), + props.addDangerToast, + ); + const [favoriteStatus, setFavoriteStatus] = useState({}); - return Boolean(state.permissions.find(p => p === perm)); - } + const [dashboardToEdit, setDashboardToEdit] = useState( + null, + ); const canEdit = hasPerm('can_edit'); @@ -126,94 +95,30 @@ function DashboardList(props: DashboardListProps) { const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - state.favoriteStatus, - favoriteStatus => updateState({ favoriteStatus }), + favoriteStatus, + setFavoriteStatus, (message: string) => { props.addDangerToast(message); }, ); - const fetchData = useCallback( - ({ - pageIndex, - pageSize, - sortBy, - filters: filterValues, - }: FetchDataConfig) => { - // set loading state, cache the last config for fetching data in this component. - updateState({ - lastFetchDataConfig: { - filters: filterValues, - pageIndex, - pageSize, - sortBy, - }, - loading: true, - }); - const filterExps = filterValues.map( - ({ id: col, operator: opr, value }) => ({ - col, - opr, - value, - }), - ); - - const queryParams = rison.encode({ - order_column: sortBy[0].id, - order_direction: sortBy[0].desc ? 'desc' : 'asc', - page: pageIndex, - page_size: pageSize, - ...(filterExps.length ? { filters: filterExps } : {}), - }); - - return SupersetClient.get({ - endpoint: `/api/v1/dashboard/?q=${queryParams}`, - }) - .then( - ({ json = {} }) => { - updateState({ - dashboards: json.result, - dashboardCount: json.count, - loading: false, - }); - }, - createErrorHandler(errMsg => - props.addDangerToast( - t('An error occurred while fetching dashboards: %s', errMsg), - ), - ), - ) - .finally(() => { - updateState({ loading: false }); - }); - }, - [], - ); - - function toggleBulkSelect() { - updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }); - } - function openDashboardEditModal(dashboard: Dashboard) { - updateState({ - dashboardToEdit: dashboard, - }); + setDashboardToEdit(dashboard); } - function handleDashboardEdit(edits: any) { - updateState({ loading: true }); + function handleDashboardEdit(edits: Dashboard) { return SupersetClient.get({ endpoint: `/api/v1/dashboard/${edits.id}`, }).then( ({ json = {} }) => { - updateState({ - dashboards: state.dashboards.map(dashboard => { + setDashboards( + dashboards.map(dashboard => { if (dashboard.id === json.id) { return json.result; } return dashboard; }), - }); + ); }, createErrorHandler(errMsg => props.addDangerToast( @@ -231,10 +136,7 @@ function DashboardList(props: DashboardListProps) { endpoint: `/api/v1/dashboard/${id}`, }).then( () => { - const { lastFetchDataConfig } = state; - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); - } + refreshData(); props.addSuccessToast(t('Deleted: %s', dashboardTitle)); }, createErrorHandler(errMsg => @@ -245,17 +147,13 @@ function DashboardList(props: DashboardListProps) { ); } - function handleBulkDashboardDelete(dashboards: Dashboard[]) { + function handleBulkDashboardDelete(dashboardsToDelete: Dashboard[]) { return SupersetClient.delete({ endpoint: `/api/v1/dashboard/?q=${rison.encode( - dashboards.map(({ id }) => id), + dashboardsToDelete.map(({ id }) => id), )}`, }).then( ({ json = {} }) => { - const { lastFetchDataConfig } = state; - if (lastFetchDataConfig) { - fetchData(lastFetchDataConfig); - } props.addSuccessToast(json.message); }, createErrorHandler(errMsg => @@ -266,10 +164,10 @@ function DashboardList(props: DashboardListProps) { ); } - function handleBulkDashboardExport(dashboards: Dashboard[]) { + function handleBulkDashboardExport(dashboardsToExport: Dashboard[]) { return window.location.assign( `/api/v1/dashboard/export/?q=${rison.encode( - dashboards.map(({ id }) => id), + dashboardsToExport.map(({ id }) => id), )}`, ); } @@ -282,7 +180,7 @@ function DashboardList(props: DashboardListProps) { itemId={original.id} fetchFaveStar={fetchFaveStarMethods.fetchFaveStar} saveFaveStar={fetchFaveStarMethods.saveFaveStar} - isStarred={!!state.favoriteStatus[original.id]} + isStarred={!!favoriteStatus[original.id]} height={20} /> ); @@ -536,7 +434,7 @@ function DashboardList(props: DashboardListProps) { titleRight={ } - url={state.bulkSelectEnabled ? undefined : dashboard.url} + url={bulkSelectEnabled ? undefined : dashboard.url} imgURL={dashboard.thumbnail_url} imgFallbackURL="/static/assets/images/dashboard-card-fallback.png" description={t( @@ -559,7 +457,7 @@ function DashboardList(props: DashboardListProps) { itemId={dashboard.id} fetchFaveStar={fetchFaveStarMethods.fetchFaveStar} saveFaveStar={fetchFaveStarMethods.saveFaveStar} - isStarred={!!state.favoriteStatus[dashboard.id]} + isStarred={!!favoriteStatus[dashboard.id]} width={20} height={20} /> @@ -572,14 +470,6 @@ function DashboardList(props: DashboardListProps) { ); } - const { - bulkSelectEnabled, - dashboards, - dashboardCount, - loading, - dashboardToEdit, - } = state; - return ( <> updateState({ dashboardToEdit: null })} + onHide={() => setDashboardToEdit(null)} onSubmit={handleDashboardEdit} /> )} diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 1606677f04074..04279c6df9dd9 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -18,22 +18,14 @@ */ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import React, { - FunctionComponent, - useCallback, - useEffect, - useState, -} from 'react'; +import React, { FunctionComponent, useState } from 'react'; import rison from 'rison'; import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; +import { useListViewResource } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import DatasourceModal from 'src/datasource/DatasourceModal'; import DeleteModal from 'src/components/DeleteModal'; -import ListView, { - ListViewProps, - FetchDataConfig, - Filters, -} from 'src/components/ListView'; +import ListView, { ListViewProps, Filters } from 'src/components/ListView'; import SubMenu, { SubMenuProps } from 'src/components/Menu/SubMenu'; import { commonMenuData } from 'src/views/CRUD/data/common'; import AvatarIcon from 'src/components/AvatarIcon'; @@ -54,6 +46,7 @@ type Dataset = { id: string; database_name: string; }; + kind: string; explore_url: string; id: number; owners: Array; @@ -66,20 +59,6 @@ interface DatasetListProps { addSuccessToast: (msg: string) => void; } -interface DatasetListState { - bulkSelectEnabled: boolean; - datasetCount: number; - datasets: Dataset[]; - lastFetchDataConfig: FetchDataConfig | null; - loading: boolean; - permissions: string[]; - datasetAddModalOpen: boolean; - datasetCurrentlyDeleting: - | (Dataset & { chart_count: number; dashboard_count: number }) - | null; - datasetCurrentlyEditing: Dataset | null; -} - export const createFetchSchemas = ( handleError: (error: Response) => void, ) => async (filterValue = '', pageIndex?: number, pageSize?: number) => { @@ -118,21 +97,31 @@ const DatasetList: FunctionComponent = ({ addDangerToast, addSuccessToast, }) => { - const [state, setState] = useState({ - bulkSelectEnabled: false, - datasetCount: 0, - datasets: [], - lastFetchDataConfig: null, - loading: true, - permissions: [], - datasetCurrentlyDeleting: null, - datasetCurrentlyEditing: null, - datasetAddModalOpen: false, - }); + const { + state: { + loading, + resourceCount: datasetCount, + resourceCollection: datasets, + bulkSelectEnabled, + }, + hasPerm, + fetchData, + toggleBulkSelect, + refreshData, + } = useListViewResource('dataset', t('dataset'), addDangerToast); + + const [datasetAddModalOpen, setDatasetAddModalOpen] = useState( + false, + ); - function updateState(update: Partial) { - setState(currentState => ({ ...currentState, ...update })); - } + const [datasetCurrentlyDeleting, setDatasetCurrentlyDeleting] = useState< + (Dataset & { chart_count: number; dashboard_count: number }) | null + >(null); + + const [ + datasetCurrentlyEditing, + setDatasetCurrentlyEditing, + ] = useState(null); const filterTypes: Filters = [ { @@ -201,33 +190,6 @@ const DatasetList: FunctionComponent = ({ }, ]; - const fetchDatasetInfo = () => { - SupersetClient.get({ - endpoint: `/api/v1/dataset/_info`, - }).then( - ({ json: infoJson = {} }) => { - updateState({ - permissions: infoJson.permissions, - }); - }, - createErrorHandler(errMsg => - addDangerToast(t('An error occurred while fetching datasets', errMsg)), - ), - ); - }; - - useEffect(() => { - fetchDatasetInfo(); - }, []); - - const hasPerm = (perm: string) => { - if (!state.permissions.length) { - return false; - } - - return Boolean(state.permissions.find(p => p === perm)); - }; - const canEdit = hasPerm('can_edit'); const canDelete = hasPerm('can_delete'); const canCreate = hasPerm('can_add'); @@ -240,7 +202,7 @@ const DatasetList: FunctionComponent = ({ }) .then(({ json = {} }) => { const owners = json.result.owners.map((owner: any) => owner.id); - updateState({ datasetCurrentlyEditing: { ...json.result, owners } }); + setDatasetCurrentlyEditing({ ...json.result, owners }); }) .catch(() => { addDangerToast( @@ -254,12 +216,10 @@ const DatasetList: FunctionComponent = ({ endpoint: `/api/v1/dataset/${dataset.id}/related_objects`, }) .then(({ json = {} }) => { - updateState({ - datasetCurrentlyDeleting: { - ...dataset, - chart_count: json.charts.count, - dashboard_count: json.dashboards.count, - }, + setDatasetCurrentlyDeleting({ + ...dataset, + chart_count: json.charts.count, + dashboard_count: json.dashboards.count, }); }) .catch( @@ -466,84 +426,32 @@ const DatasetList: FunctionComponent = ({ {t('Dataset')}{' '} ), - onClick: () => updateState({ datasetAddModalOpen: true }), + onClick: () => setDatasetAddModalOpen(true), }; } if (canDelete) { menuData.secondaryButton = { name: t('Bulk Select'), - onClick: () => - updateState({ bulkSelectEnabled: !state.bulkSelectEnabled }), + onClick: toggleBulkSelect, }; } const closeDatasetDeleteModal = () => { - updateState({ datasetCurrentlyDeleting: null }); + setDatasetCurrentlyDeleting(null); }; - const closeDatasetEditModal = () => - updateState({ datasetCurrentlyEditing: null }); - - const fetchData = useCallback( - ({ pageIndex, pageSize, sortBy, filters }: FetchDataConfig) => { - // set loading state, cache the last config for fetching data in this component. - updateState({ - lastFetchDataConfig: { - filters, - pageIndex, - pageSize, - sortBy, - }, - loading: true, - }); - - const filterExps = filters.map(({ id: col, operator: opr, value }) => ({ - col, - opr, - value, - })); - - const queryParams = rison.encode({ - order_column: sortBy[0].id, - order_direction: sortBy[0].desc ? 'desc' : 'asc', - page: pageIndex, - page_size: pageSize, - ...(filterExps.length ? { filters: filterExps } : {}), - }); - - return SupersetClient.get({ - endpoint: `/api/v1/dataset/?q=${queryParams}`, - }) - .then( - ({ json }) => { - updateState({ - datasets: json.result, - datasetCount: json.count, - }); - }, - createErrorHandler(errMsg => - addDangerToast( - t('An error occurred while fetching datasets: %s', errMsg), - ), - ), - ) - .finally(() => { - updateState({ loading: false }); - }); - }, - [], - ); + const closeDatasetEditModal = () => { + setDatasetCurrentlyEditing(null); + }; const handleDatasetDelete = ({ id, table_name: tableName }: Dataset) => { SupersetClient.delete({ endpoint: `/api/v1/dataset/${id}`, }).then( () => { - if (state.lastFetchDataConfig) { - fetchData(state.lastFetchDataConfig); - } - updateState({ datasetCurrentlyDeleting: null }); + refreshData(); + setDatasetCurrentlyDeleting(null); addSuccessToast(t('Deleted: %s', tableName)); }, createErrorHandler(errMsg => @@ -554,16 +462,13 @@ const DatasetList: FunctionComponent = ({ ); }; - const handleBulkDatasetDelete = () => { + const handleBulkDatasetDelete = (datasetsToDelete: Dataset[]) => { SupersetClient.delete({ endpoint: `/api/v1/dataset/?q=${rison.encode( - state.datasets.map(({ id }) => id), + datasetsToDelete.map(({ id }) => id), )}`, }).then( ({ json = {} }) => { - if (state.lastFetchDataConfig) { - fetchData(state.lastFetchDataConfig); - } addSuccessToast(json.message); }, createErrorHandler(errMsg => @@ -574,33 +479,25 @@ const DatasetList: FunctionComponent = ({ ); }; - const handleUpdateDataset = () => { - if (state.lastFetchDataConfig) { - fetchData(state.lastFetchDataConfig); - } - }; - return ( <> updateState({ datasetAddModalOpen: false })} - onDatasetAdd={() => { - if (state.lastFetchDataConfig) fetchData(state.lastFetchDataConfig); - }} + show={datasetAddModalOpen} + onHide={() => setDatasetAddModalOpen(false)} + onDatasetAdd={refreshData} /> - {state.datasetCurrentlyDeleting && ( + {datasetCurrentlyDeleting && ( { - if (state.datasetCurrentlyDeleting) { - handleDatasetDelete(state.datasetCurrentlyDeleting); + if (datasetCurrentlyDeleting) { + handleDatasetDelete(datasetCurrentlyDeleting); } }} onHide={closeDatasetDeleteModal} @@ -608,10 +505,10 @@ const DatasetList: FunctionComponent = ({ title={t('Delete Dataset?')} /> )} - {state.datasetCurrentlyEditing && ( + {datasetCurrentlyEditing && ( @@ -639,18 +536,16 @@ const DatasetList: FunctionComponent = ({ - updateState({ bulkSelectEnabled: false }) - } + bulkSelectEnabled={bulkSelectEnabled} + disableBulkSelect={toggleBulkSelect} renderBulkSelectCopy={selected => { const { virtualCount, physicalCount } = selected.reduce( (acc, e) => { diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 0b48efa0b291f..61de42a127d96 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -119,7 +119,7 @@ export function useListViewResource( }); return SupersetClient.get({ - endpoint: `/api/v1/chart/?q=${queryParams}`, + endpoint: `/api/v1/${resource}/?q=${queryParams}`, }) .then( ({ json = {} }) => { From 6a1afec04761443c3e75ea8255375a158bfb8084 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Tue, 25 Aug 2020 23:43:44 -0700 Subject: [PATCH 08/13] fix specs, tidy up --- .../views/CRUD/chart/ChartList_spec.jsx | 12 +- .../CRUD/dashboard/DashboardList_spec.jsx | 11 +- .../src/views/CRUD/chart/ChartList.tsx | 363 +++++++++--------- .../views/CRUD/dashboard/DashboardList.tsx | 2 - .../views/CRUD/data/dataset/DatasetList.tsx | 135 +++---- 5 files changed, 259 insertions(+), 264 deletions(-) diff --git a/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx index d6401164af202..73dd9eea335bf 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/chart/ChartList_spec.jsx @@ -17,18 +17,18 @@ * under the License. */ import React from 'react'; -import { mount } from 'enzyme'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; -import { supersetTheme, ThemeProvider } from '@superset-ui/style'; + +import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { styledMount as mount } from 'spec/helpers/theming'; import ChartList from 'src/views/CRUD/chart/ChartList'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import ListView from 'src/components/ListView'; import PropertiesModal from 'src/explore/components/PropertiesModal'; import ListViewCard from 'src/components/ListViewCard'; - // store needed for withToasts(ChartTable) const mockStore = configureStore([thunk]); const store = mockStore({}); @@ -78,8 +78,10 @@ describe('ChartList', () => { const mockedProps = {}; const wrapper = mount(, { context: { store }, - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, + }); + + beforeAll(async () => { + await waitForComponentToPaint(wrapper); }); it('renders', () => { diff --git a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx index 7eb634c46fc00..fd824147ff553 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx @@ -17,11 +17,12 @@ * under the License. */ import React from 'react'; -import { mount } from 'enzyme'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import fetchMock from 'fetch-mock'; -import { supersetTheme, ThemeProvider } from '@superset-ui/style'; + +import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import { styledMount as mount } from 'spec/helpers/theming'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import DashboardList from 'src/views/CRUD/dashboard/DashboardList'; @@ -69,8 +70,10 @@ describe('DashboardList', () => { const mockedProps = {}; const wrapper = mount(, { context: { store }, - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, + }); + + beforeAll(async () => { + await waitForComponentToPaint(wrapper); }); it('renders', () => { diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index c22fd3a07e40b..bcea91e9a18d9 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -176,203 +176,194 @@ function ChartList(props: ChartListProps) { ); } - const columns = useMemo( - () => [ - { - Cell: ({ row: { original } }: any) => { - return ( - - ); - }, - Header: '', - id: 'favorite', - disableSortBy: true, - }, - { - Cell: ({ - row: { - original: { url, slice_name: sliceName }, - }, - }: any) => {sliceName}, - Header: t('Chart'), - accessor: 'slice_name', - }, - { - Cell: ({ - row: { - original: { viz_type: vizType }, - }, - }: any) => vizType, - Header: t('Visualization Type'), - accessor: 'viz_type', - }, - { - Cell: ({ - row: { - original: { - datasource_name_text: dsNameTxt, - datasource_url: dsUrl, - }, - }, - }: any) => {dsNameTxt}, - Header: t('Datasource'), - accessor: 'datasource_name', - }, - { - Cell: ({ - row: { - original: { - changed_by_name: changedByName, - changed_by_url: changedByUrl, - }, - }, - }: any) => {changedByName}, - Header: t('Modified By'), - accessor: 'changed_by.first_name', + const columns = [ + { + Cell: ({ row: { original } }: any) => { + return ( + + ); }, - { - Cell: ({ - row: { - original: { changed_on_delta_humanized: changedOn }, + Header: '', + id: 'favorite', + disableSortBy: true, + }, + { + Cell: ({ + row: { + original: { url, slice_name: sliceName }, + }, + }: any) => {sliceName}, + Header: t('Chart'), + accessor: 'slice_name', + }, + { + Cell: ({ + row: { + original: { viz_type: vizType }, + }, + }: any) => vizType, + Header: t('Visualization Type'), + accessor: 'viz_type', + }, + { + Cell: ({ + row: { + original: { datasource_name_text: dsNameTxt, datasource_url: dsUrl }, + }, + }: any) => {dsNameTxt}, + Header: t('Datasource'), + accessor: 'datasource_name', + }, + { + Cell: ({ + row: { + original: { + changed_by_name: changedByName, + changed_by_url: changedByUrl, }, - }: any) => {changedOn}, - Header: t('Last Modified'), - accessor: 'changed_on_delta_humanized', - }, - { - accessor: 'description', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'owners', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'datasource_id', - hidden: true, - disableSortBy: true, - }, - { - Cell: ({ row: { original } }: any) => { - const handleDelete = () => handleChartDelete(original); - const openEditModal = () => openChartEditModal(original); - if (!canEdit && !canDelete) { - return null; - } - - return ( - - {canDelete && ( - - {t('Are you sure you want to delete')}{' '} - {original.slice_name}? - - } - onConfirm={handleDelete} - > - {confirmDelete => ( - - - - )} - - )} - {canEdit && ( - - - - )} - - ); }, - Header: t('Actions'), - id: 'actions', - disableSortBy: true, + }: any) => {changedByName}, + Header: t('Modified By'), + accessor: 'changed_by.first_name', + }, + { + Cell: ({ + row: { + original: { changed_on_delta_humanized: changedOn }, + }, + }: any) => {changedOn}, + Header: t('Last Modified'), + accessor: 'changed_on_delta_humanized', + }, + { + accessor: 'description', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'owners', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'datasource_id', + hidden: true, + disableSortBy: true, + }, + { + Cell: ({ row: { original } }: any) => { + const handleDelete = () => handleChartDelete(original); + const openEditModal = () => openChartEditModal(original); + if (!canEdit && !canDelete) { + return null; + } + + return ( + + {canDelete && ( + + {t('Are you sure you want to delete')}{' '} + {original.slice_name}? + + } + onConfirm={handleDelete} + > + {confirmDelete => ( + + + + )} + + )} + {canEdit && ( + + + + )} + + ); }, - ], - [], - ); + Header: t('Actions'), + id: 'actions', + disableSortBy: true, + }, + ]; - const filters: Filters = useMemo( - () => [ - { - Header: t('Owner'), - id: 'owners', - input: 'select', - operator: 'rel_m_m', - unfilteredLabel: 'All', - fetchSelects: createFetchRelated( - 'chart', - 'owners', - createErrorHandler(errMsg => - props.addDangerToast( - t( - 'An error occurred while fetching chart dataset values: %s', - errMsg, - ), + const filters: Filters = [ + { + Header: t('Owner'), + id: 'owners', + input: 'select', + operator: 'rel_m_m', + unfilteredLabel: 'All', + fetchSelects: createFetchRelated( + 'chart', + 'owners', + createErrorHandler(errMsg => + 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('Datasource'), - id: 'datasource_id', - input: 'select', - operator: 'eq', - unfilteredLabel: 'All', - fetchSelects: createFetchDatasets( - createErrorHandler(errMsg => - 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('Datasource'), + id: 'datasource_id', + input: 'select', + operator: 'eq', + unfilteredLabel: 'All', + fetchSelects: createFetchDatasets( + createErrorHandler(errMsg => + 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', - }, - ], - [], - ); + ), + paginate: false, + }, + { + Header: t('Search'), + id: 'slice_name', + input: 'search', + operator: 'name_or_description', + }, + ]; const sortTypes = [ { diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index e3e0b198e03fe..52946bc5d15e1 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -86,9 +86,7 @@ function DashboardList(props: DashboardListProps) { ); const canEdit = hasPerm('can_edit'); - const canDelete = hasPerm('can_delete'); - const canExport = hasPerm('can_mulexport'); const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 04279c6df9dd9..00e63ebdc8373 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -123,73 +123,6 @@ const DatasetList: FunctionComponent = ({ setDatasetCurrentlyEditing, ] = useState(null); - 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: createFetchRelated( - 'dataset', - 'database', - createErrorHandler(errMsg => - t( - 'An error occurred while fetching dataset 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 canEdit = hasPerm('can_edit'); const canDelete = hasPerm('can_delete'); const canCreate = hasPerm('can_add'); @@ -413,6 +346,73 @@ const DatasetList: FunctionComponent = ({ }, ]; + 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: createFetchRelated( + 'dataset', + 'database', + createErrorHandler(errMsg => + t( + 'An error occurred while fetching dataset 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 menuData: SubMenuProps = { activeChild: 'Datasets', ...commonMenuData, @@ -469,6 +469,7 @@ const DatasetList: FunctionComponent = ({ )}`, }).then( ({ json = {} }) => { + refreshData(); addSuccessToast(json.message); }, createErrorHandler(errMsg => From ad2d88db1268ca3acde94cd19b27bb383006ccc4 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Tue, 25 Aug 2020 23:58:04 -0700 Subject: [PATCH 09/13] fix plural --- superset-frontend/src/views/CRUD/chart/ChartList.tsx | 2 +- superset-frontend/src/views/CRUD/hooks.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index bcea91e9a18d9..0a003e81da33f 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,7 +19,7 @@ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import { getChartMetadataRegistry } from '@superset-ui/chart'; -import React, { useState, useMemo } from 'react'; +import React, { useState } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; import { diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 61de42a127d96..773051960085e 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -67,7 +67,7 @@ export function useListViewResource( createErrorHandler(errMsg => handleErrorMsg( t( - 'An error occurred while fetching %s info: %s', + 'An error occurred while fetching %ss info: %s', resourceLabel, errMsg, ), @@ -131,7 +131,7 @@ export function useListViewResource( createErrorHandler(errMsg => handleErrorMsg( t( - 'An error occurred while fetching %s: %s', + 'An error occurred while fetching %ss: %s', resourceLabel, errMsg, ), From 8e6e211d136819f72428dfbe269b1fc2c44a39d5 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Wed, 26 Aug 2020 12:18:23 -0700 Subject: [PATCH 10/13] fix FaveStar issues with infinite remounting --- superset-frontend/src/components/FaveStar.tsx | 11 +- .../src/views/CRUD/chart/ChartList.tsx | 284 +++++------ .../views/CRUD/dashboard/DashboardList.tsx | 305 ++++++------ .../views/CRUD/data/dataset/DatasetList.tsx | 456 +++++++++--------- superset-frontend/src/views/CRUD/hooks.ts | 19 +- superset-frontend/src/views/CRUD/types.ts | 22 + superset-frontend/src/views/CRUD/utils.tsx | 21 +- 7 files changed, 578 insertions(+), 540 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/types.ts diff --git a/superset-frontend/src/components/FaveStar.tsx b/superset-frontend/src/components/FaveStar.tsx index 81d7b1b5fb7f1..dc1f6e847aae0 100644 --- a/superset-frontend/src/components/FaveStar.tsx +++ b/superset-frontend/src/components/FaveStar.tsx @@ -25,7 +25,7 @@ interface FaveStarProps { itemId: number; fetchFaveStar(id: number): any; saveFaveStar(id: number, isStarred: boolean): any; - isStarred?: boolean; + isStarred: boolean; width?: number; height?: number; showTooltip?: boolean; @@ -33,17 +33,12 @@ interface FaveStarProps { export default class FaveStar extends React.PureComponent { componentDidMount() { - // only fetch if we don't know the current status - if (typeof this.props.isStarred === 'undefined') { - this.props.fetchFaveStar(this.props.itemId); - } + this.props.fetchFaveStar(this.props.itemId); } onClick(e: React.MouseEvent) { e.preventDefault(); - if (typeof this.props.isStarred !== 'undefined') { - this.props.saveFaveStar(this.props.itemId, this.props.isStarred); - } + this.props.saveFaveStar(this.props.itemId, this.props.isStarred); } render() { diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 0a003e81da33f..c43b98a637eec 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,7 +19,7 @@ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import { getChartMetadataRegistry } from '@superset-ui/chart'; -import React, { useState } from 'react'; +import React, { useState, useMemo } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; import { @@ -27,7 +27,7 @@ import { createErrorHandler, createFaveStarHandlers, } from 'src/views/CRUD/utils'; -import { useListViewResource } from 'src/views/CRUD/hooks'; +import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import AvatarIcon from 'src/components/AvatarIcon'; @@ -104,7 +104,7 @@ function ChartList(props: ChartListProps) { toggleBulkSelect, refreshData, } = useListViewResource('chart', t('chart'), props.addDangerToast); - const [favoriteStatus, setFavoriteStatus] = useState({}); + const [favoriteStatusRef, setFavoriteStatus] = useFavoriteStatus({}); const [ sliceCurrentlyEditing, setSliceCurrentlyEditing, @@ -116,7 +116,7 @@ function ChartList(props: ChartListProps) { const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - favoriteStatus, + favoriteStatusRef.current, setFavoriteStatus, (message: string) => { props.addDangerToast(message); @@ -176,137 +176,150 @@ function ChartList(props: ChartListProps) { ); } - const columns = [ - { - Cell: ({ row: { original } }: any) => { - return ( - - ); + function renderFaveStar(id: number) { + return ( + + ); + } + + const columns = useMemo( + () => [ + { + Cell: ({ + row: { + original: { id }, + }, + }: any) => renderFaveStar(id), + Header: '', + id: 'favorite', + disableSortBy: true, }, - Header: '', - id: 'favorite', - disableSortBy: true, - }, - { - Cell: ({ - row: { - original: { url, slice_name: sliceName }, - }, - }: any) => {sliceName}, - Header: t('Chart'), - accessor: 'slice_name', - }, - { - Cell: ({ - row: { - original: { viz_type: vizType }, - }, - }: any) => vizType, - Header: t('Visualization Type'), - accessor: 'viz_type', - }, - { - Cell: ({ - row: { - original: { datasource_name_text: dsNameTxt, datasource_url: dsUrl }, - }, - }: any) => {dsNameTxt}, - Header: t('Datasource'), - accessor: 'datasource_name', - }, - { - Cell: ({ - row: { - original: { - changed_by_name: changedByName, - changed_by_url: changedByUrl, + { + Cell: ({ + row: { + original: { url, slice_name: sliceName }, }, - }, - }: any) => {changedByName}, - Header: t('Modified By'), - accessor: 'changed_by.first_name', - }, - { - Cell: ({ - row: { - original: { changed_on_delta_humanized: changedOn }, - }, - }: any) => {changedOn}, - Header: t('Last Modified'), - accessor: 'changed_on_delta_humanized', - }, - { - accessor: 'description', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'owners', - hidden: true, - disableSortBy: true, - }, - { - accessor: 'datasource_id', - hidden: true, - disableSortBy: true, - }, - { - Cell: ({ row: { original } }: any) => { - const handleDelete = () => handleChartDelete(original); - const openEditModal = () => openChartEditModal(original); - if (!canEdit && !canDelete) { - return null; - } + }: any) => {sliceName}, + Header: t('Chart'), + accessor: 'slice_name', + }, + { + Cell: ({ + row: { + original: { viz_type: vizType }, + }, + }: any) => vizType, + Header: t('Visualization Type'), + accessor: 'viz_type', + }, + { + Cell: ({ + row: { + original: { + datasource_name_text: dsNameTxt, + datasource_url: dsUrl, + }, + }, + }: any) => {dsNameTxt}, + Header: t('Datasource'), + accessor: 'datasource_name', + }, + { + Cell: ({ + row: { + original: { + changed_by_name: changedByName, + changed_by_url: changedByUrl, + }, + }, + }: any) => {changedByName}, + Header: t('Modified By'), + accessor: 'changed_by.first_name', + }, + { + Cell: ({ + row: { + original: { changed_on_delta_humanized: changedOn }, + }, + }: any) => {changedOn}, + Header: t('Last Modified'), + accessor: 'changed_on_delta_humanized', + }, + { + accessor: 'description', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'owners', + hidden: true, + disableSortBy: true, + }, + { + accessor: 'datasource_id', + hidden: true, + disableSortBy: true, + }, + { + Cell: ({ row: { original } }: any) => { + const handleDelete = () => handleChartDelete(original); + const openEditModal = () => openChartEditModal(original); + if (!canEdit && !canDelete) { + return null; + } - return ( - - {canDelete && ( - - {t('Are you sure you want to delete')}{' '} - {original.slice_name}? - - } - onConfirm={handleDelete} - > - {confirmDelete => ( - - - - )} - - )} - {canEdit && ( - - - - )} - - ); + return ( + + {canDelete && ( + + {t('Are you sure you want to delete')}{' '} + {original.slice_name}? + + } + onConfirm={handleDelete} + > + {confirmDelete => ( + + + + )} + + )} + {canEdit && ( + + + + )} + + ); + }, + Header: t('Actions'), + id: 'actions', + disableSortBy: true, }, - Header: t('Actions'), - id: 'actions', - disableSortBy: true, - }, - ]; + ], + [canEdit, canDelete, favoriteStatusRef], + ); const filters: Filters = [ { @@ -449,14 +462,7 @@ function ChartList(props: ChartListProps) { } actions={ - + {renderFaveStar(chart.id)} diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 52946bc5d15e1..1037beee0c386 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -18,14 +18,14 @@ */ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import React, { useState } from 'react'; +import React, { useState, useMemo } from 'react'; import rison from 'rison'; import { createFetchRelated, createErrorHandler, createFaveStarHandlers, } from 'src/views/CRUD/utils'; -import { useListViewResource } from 'src/views/CRUD/hooks'; +import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; import AvatarIcon from 'src/components/AvatarIcon'; @@ -79,7 +79,7 @@ function DashboardList(props: DashboardListProps) { t('dashboard'), props.addDangerToast, ); - const [favoriteStatus, setFavoriteStatus] = useState({}); + const [favoriteStatusRef, setFavoriteStatus] = useFavoriteStatus({}); const [dashboardToEdit, setDashboardToEdit] = useState( null, @@ -93,7 +93,7 @@ function DashboardList(props: DashboardListProps) { const fetchFaveStarMethods = createFaveStarHandlers( FAVESTAR_BASE_URL, - favoriteStatus, + favoriteStatusRef.current, setFavoriteStatus, (message: string) => { props.addDangerToast(message); @@ -170,150 +170,160 @@ function DashboardList(props: DashboardListProps) { ); } - const columns = [ - { - Cell: ({ row: { original } }: any) => { - return ( - - ); + function renderFaveStar(id: number) { + return ( + + ); + } + + const columns = useMemo( + () => [ + { + Cell: ({ + row: { + original: { id }, + }, + }: any) => renderFaveStar(id), + Header: '', + id: 'favorite', + disableSortBy: true, }, - Header: '', - id: 'favorite', - disableSortBy: true, - }, - { - Cell: ({ - row: { - original: { url, dashboard_title: dashboardTitle }, - }, - }: any) => {dashboardTitle}, - Header: t('Title'), - accessor: 'dashboard_title', - }, - { - Cell: ({ - row: { - original: { owners }, - }, - }: any) => ( - - `${firstName} ${lastName}`, - )} - display={2} - /> - ), - Header: t('Owners'), - accessor: 'owners', - disableSortBy: true, - }, - { - Cell: ({ - row: { - original: { - changed_by_name: changedByName, - changed_by_url: changedByUrl, + { + Cell: ({ + row: { + original: { url, dashboard_title: dashboardTitle }, }, - }, - }: any) => {changedByName}, - Header: t('Modified By'), - accessor: 'changed_by.first_name', - }, - { - Cell: ({ - row: { - original: { published }, - }, - }: any) => ( - - {published ? : ''} - - ), - Header: t('Published'), - accessor: 'published', - }, - { - Cell: ({ - row: { - original: { changed_on_delta_humanized: changedOn }, - }, - }: any) => {changedOn}, - Header: t('Modified'), - accessor: 'changed_on_delta_humanized', - }, - { - accessor: 'slug', - hidden: true, - disableSortBy: true, - }, - { - Cell: ({ row: { original } }: any) => { - const handleDelete = () => handleDashboardDelete(original); - const handleEdit = () => openDashboardEditModal(original); - const handleExport = () => handleBulkDashboardExport([original]); - if (!canEdit && !canDelete && !canExport) { - return null; - } - return ( - - {canDelete && ( - - {t('Are you sure you want to delete')}{' '} - {original.dashboard_title}? - - } - onConfirm={handleDelete} - > - {confirmDelete => ( - - - - )} - - )} - {canExport && ( - - - - )} - {canEdit && ( - - - + }: any) => {dashboardTitle}, + Header: t('Title'), + accessor: 'dashboard_title', + }, + { + Cell: ({ + row: { + original: { owners }, + }, + }: any) => ( + + `${firstName} ${lastName}`, )} + display={2} + /> + ), + Header: t('Owners'), + accessor: 'owners', + disableSortBy: true, + }, + { + Cell: ({ + row: { + original: { + changed_by_name: changedByName, + changed_by_url: changedByUrl, + }, + }, + }: any) => {changedByName}, + Header: t('Modified By'), + accessor: 'changed_by.first_name', + }, + { + Cell: ({ + row: { + original: { published }, + }, + }: any) => ( + + {published ? : ''} - ); + ), + Header: t('Published'), + accessor: 'published', }, - Header: t('Actions'), - id: 'actions', - disableSortBy: true, - }, - ]; + { + Cell: ({ + row: { + original: { changed_on_delta_humanized: changedOn }, + }, + }: any) => {changedOn}, + Header: t('Modified'), + accessor: 'changed_on_delta_humanized', + }, + { + accessor: 'slug', + hidden: true, + disableSortBy: true, + }, + { + Cell: ({ row: { original } }: any) => { + const handleDelete = () => handleDashboardDelete(original); + const handleEdit = () => openDashboardEditModal(original); + const handleExport = () => handleBulkDashboardExport([original]); + if (!canEdit && !canDelete && !canExport) { + return null; + } + return ( + + {canDelete && ( + + {t('Are you sure you want to delete')}{' '} + {original.dashboard_title}? + + } + onConfirm={handleDelete} + > + {confirmDelete => ( + + + + )} + + )} + {canExport && ( + + + + )} + {canEdit && ( + + + + )} + + ); + }, + Header: t('Actions'), + id: 'actions', + disableSortBy: true, + }, + ], + [canEdit, canDelete, canExport], + ); const filters: Filters = [ { @@ -451,14 +461,7 @@ function DashboardList(props: DashboardListProps) { ))} actions={ - + {renderFaveStar(dashboard.id)} diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 00e63ebdc8373..668739c5f72f6 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -18,7 +18,7 @@ */ import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import React, { FunctionComponent, useState } from 'react'; +import React, { FunctionComponent, useState, useMemo } from 'react'; import rison from 'rison'; import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import { useListViewResource } from 'src/views/CRUD/hooks'; @@ -164,254 +164,260 @@ const DatasetList: FunctionComponent = ({ ), ); - const columns = [ - { - Cell: ({ - row: { - original: { kind }, - }, - }: any) => { - if (kind === 'physical') + const columns = useMemo( + () => [ + { + Cell: ({ + row: { + original: { kind }, + }, + }: any) => { + if (kind === 'physical') + return ( + + + + ); + return ( - + ); - - return ( - - - - ); - }, - accessor: 'kind_icon', - disableSortBy: true, - size: 'xs', - }, - { - Cell: ({ - row: { - original: { table_name: datasetTitle }, }, - }: any) => datasetTitle, - Header: t('Name'), - accessor: 'table_name', - }, - { - Cell: ({ - row: { - original: { kind }, - }, - }: any) => kind[0]?.toUpperCase() + kind.slice(1), - Header: t('Type'), - accessor: 'kind', - disableSortBy: true, - size: 'md', - }, - { - Header: t('Source'), - accessor: 'database.database_name', - size: 'lg', - }, - { - Header: t('Schema'), - accessor: 'schema', - size: 'lg', - }, - { - Cell: ({ - row: { - original: { changed_on_delta_humanized: changedOn }, - }, - }: any) => {changedOn}, - Header: t('Modified'), - accessor: 'changed_on_delta_humanized', - size: 'xl', - }, - { - Cell: ({ - row: { - original: { changed_by_name: changedByName }, - }, - }: any) => changedByName, - Header: t('Modified By'), - accessor: 'changed_by.first_name', - size: 'xl', - }, - { - accessor: 'database', - disableSortBy: true, - hidden: true, - }, - { - Cell: ({ - row: { - original: { owners, table_name: tableName }, + accessor: 'kind_icon', + disableSortBy: true, + size: 'xs', + }, + { + Cell: ({ + row: { + original: { table_name: datasetTitle }, + }, + }: any) => datasetTitle, + Header: t('Name'), + accessor: 'table_name', + }, + { + Cell: ({ + row: { + original: { kind }, + }, + }: any) => kind[0]?.toUpperCase() + kind.slice(1), + Header: t('Type'), + accessor: 'kind', + disableSortBy: true, + size: 'md', + }, + { + Header: t('Source'), + accessor: 'database.database_name', + size: 'lg', + }, + { + Header: t('Schema'), + accessor: 'schema', + size: 'lg', + }, + { + Cell: ({ + row: { + original: { changed_on_delta_humanized: changedOn }, + }, + }: any) => {changedOn}, + Header: t('Modified'), + accessor: 'changed_on_delta_humanized', + size: 'xl', + }, + { + Cell: ({ + row: { + original: { changed_by_name: changedByName }, + }, + }: any) => changedByName, + Header: t('Modified By'), + accessor: 'changed_by.first_name', + size: 'xl', + }, + { + accessor: 'database', + disableSortBy: true, + hidden: true, + }, + { + Cell: ({ + row: { + original: { owners, table_name: tableName }, + }, + }: any) => { + if (!owners) { + return null; + } + return owners + .slice(0, 5) + .map((owner: Owner) => ( + + )); }, - }: any) => { - if (!owners) { - return null; - } - return owners - .slice(0, 5) - .map((owner: Owner) => ( - - )); + Header: t('Owners'), + id: 'owners', + disableSortBy: true, + size: 'lg', }, - Header: t('Owners'), - id: 'owners', - disableSortBy: true, - size: 'lg', - }, - { - accessor: 'is_sqllab_view', - hidden: true, - disableSortBy: true, - }, - { - Cell: ({ row: { original } }: any) => { - const handleEdit = () => openDatasetEditModal(original); - const handleDelete = () => openDatasetDeleteModal(original); - if (!canEdit && !canDelete) { - return null; - } - return ( - - - - - - - {canDelete && ( + { + accessor: 'is_sqllab_view', + hidden: true, + disableSortBy: true, + }, + { + Cell: ({ row: { original } }: any) => { + const handleEdit = () => openDatasetEditModal(original); + const handleDelete = () => openDatasetDeleteModal(original); + if (!canEdit && !canDelete) { + return null; + } + return ( + - - - + + - )} + {canDelete && ( + + + + + + )} - {canEdit && ( - - - - - - )} - - ); + + + + + )} + + ); + }, + Header: t('Actions'), + id: 'actions', + disableSortBy: true, }, - Header: t('Actions'), - id: 'actions', - disableSortBy: true, - }, - ]; + ], + [canCreate, canEdit, canDelete], + ); - 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, + const filterTypes: Filters = useMemo( + () => [ + { + 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: createFetchRelated( - 'dataset', - 'database', - createErrorHandler(errMsg => - t( - 'An error occurred while fetching dataset datasource values: %s', - errMsg, + paginate: true, + }, + { + Header: t('Datasource'), + id: 'database', + input: 'select', + operator: 'rel_o_m', + unfilteredLabel: 'All', + fetchSelects: createFetchRelated( + 'dataset', + 'database', + createErrorHandler(errMsg => + t( + 'An error occurred while fetching dataset 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', - }, - ]; + 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 menuData: SubMenuProps = { activeChild: 'Datasets', diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 773051960085e..17012872b049b 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -17,12 +17,13 @@ * under the License. */ import rison from 'rison'; -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect, useCallback, useRef } from 'react'; import { SupersetClient } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; import { createErrorHandler } from 'src/views/CRUD/utils'; import { FetchDataConfig } from 'src/components/ListView'; +import { FavoriteStatus } from './types'; interface ListViewResourceState { loading: boolean; @@ -166,3 +167,19 @@ export function useListViewResource( }, }; } + +export function useFavoriteStatus(initialState: FavoriteStatus) { + const [favoriteStatus, setFavoriteStatus] = useState( + initialState, + ); + const favoriteStatusRef = useRef(favoriteStatus); + useEffect(() => { + favoriteStatusRef.current = favoriteStatus; + }); + + return [ + favoriteStatusRef, + (update: FavoriteStatus) => + setFavoriteStatus(currentState => ({ ...currentState, ...update })), + ] as const; +} diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts new file mode 100644 index 0000000000000..91d88a375e547 --- /dev/null +++ b/superset-frontend/src/views/CRUD/types.ts @@ -0,0 +1,22 @@ +/** + * 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. + */ + +export type FavoriteStatus = { + [id: number]: boolean; +}; diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 6ef6849d558c3..73bee32dae4ad 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -24,6 +24,7 @@ import { t } from '@superset-ui/translation'; import rison from 'rison'; import getClientErrorObject from 'src/utils/getClientErrorObject'; import { logging } from '@superset-ui/core'; +import { FavoriteStatus } from './types'; export const createFetchRelated = ( resource: string, @@ -64,8 +65,8 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { export function createFaveStarHandlers( baseURL: string, - favoriteStatus: object, - setFavoriteStatus: (update: any) => void, + favoriteStatus: FavoriteStatus, + setFavoriteStatus: (update: Partial) => void, handleErrorFunc: (message: string) => void, ) { const fetchFaveStar = (id: number) => { @@ -73,13 +74,7 @@ export function createFaveStarHandlers( endpoint: `${baseURL}/${id}/count/`, }) .then(({ json }) => { - const faves = { - ...favoriteStatus, - }; - - faves[id] = json.count > 0; - - setFavoriteStatus(faves); + setFavoriteStatus({ [id]: json.count > 0 }); }) .catch(() => handleErrorFunc(t('There was an error fetching the favorite status')), @@ -93,13 +88,7 @@ export function createFaveStarHandlers( endpoint: `${baseURL}/${id}/${urlSuffix}/`, }) .then(() => { - const faves = { - ...favoriteStatus, - }; - - faves[id] = !isStarred; - - setFavoriteStatus(faves); + setFavoriteStatus({ [id]: !isStarred }); }) .catch(() => handleErrorFunc(t('There was an error saving the favorite status')), From a4b0fc3a1999e102509b5dc8e27340550e576f17 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Wed, 26 Aug 2020 13:17:12 -0700 Subject: [PATCH 11/13] move fetchFaveStar methods into useFaveStar hook --- .../src/views/CRUD/chart/ChartList.tsx | 27 +++++------ .../views/CRUD/dashboard/DashboardList.tsx | 29 +++++------- superset-frontend/src/views/CRUD/hooks.ts | 45 ++++++++++++++++--- superset-frontend/src/views/CRUD/utils.tsx | 38 ---------------- 4 files changed, 60 insertions(+), 79 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index c43b98a637eec..17349c012c9db 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -22,11 +22,7 @@ import { getChartMetadataRegistry } from '@superset-ui/chart'; import React, { useState, useMemo } from 'react'; import rison from 'rison'; import { uniqBy } from 'lodash'; -import { - createFetchRelated, - createErrorHandler, - createFaveStarHandlers, -} from 'src/views/CRUD/utils'; +import { createFetchRelated, createErrorHandler } from 'src/views/CRUD/utils'; import { useListViewResource, useFavoriteStatus } from 'src/views/CRUD/hooks'; import ConfirmStatusChange from 'src/components/ConfirmStatusChange'; import SubMenu from 'src/components/Menu/SubMenu'; @@ -104,7 +100,13 @@ function ChartList(props: ChartListProps) { toggleBulkSelect, refreshData, } = useListViewResource('chart', t('chart'), props.addDangerToast); - const [favoriteStatusRef, setFavoriteStatus] = useFavoriteStatus({}); + const [favoriteStatusRef, fetchFaveStar, saveFaveStar] = useFavoriteStatus( + {}, + FAVESTAR_BASE_URL, + (message: string) => { + props.addDangerToast(message); + }, + ); const [ sliceCurrentlyEditing, setSliceCurrentlyEditing, @@ -114,15 +116,6 @@ function ChartList(props: ChartListProps) { const canDelete = hasPerm('can_delete'); const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - const fetchFaveStarMethods = createFaveStarHandlers( - FAVESTAR_BASE_URL, - favoriteStatusRef.current, - setFavoriteStatus, - (message: string) => { - props.addDangerToast(message); - }, - ); - function openChartEditModal(chart: Chart) { setSliceCurrentlyEditing({ slice_id: chart.id, @@ -180,8 +173,8 @@ function ChartList(props: ChartListProps) { return ( { + props.addDangerToast(message); + }, + ); const [dashboardToEdit, setDashboardToEdit] = useState( null, @@ -91,15 +93,6 @@ function DashboardList(props: DashboardListProps) { const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - const fetchFaveStarMethods = createFaveStarHandlers( - FAVESTAR_BASE_URL, - favoriteStatusRef.current, - setFavoriteStatus, - (message: string) => { - props.addDangerToast(message); - }, - ); - function openDashboardEditModal(dashboard: Dashboard) { setDashboardToEdit(dashboard); } @@ -174,8 +167,8 @@ function DashboardList(props: DashboardListProps) { return ( ( }; } -export function useFavoriteStatus(initialState: FavoriteStatus) { +// there hooks api has some known limitations around stale state in closures. +// See https://github.com/reactjs/rfcs/blob/master/text/0068-react-hooks.md#drawbacks +// the useRef hook is a way of getting around these limitations by having a consistent ref +// that points to the latest value. +export function useFavoriteStatus( + initialState: FavoriteStatus, + baseURL: string, + handleErrorFunc: (message: string) => void, +) { const [favoriteStatus, setFavoriteStatus] = useState( initialState, ); @@ -177,9 +185,34 @@ export function useFavoriteStatus(initialState: FavoriteStatus) { favoriteStatusRef.current = favoriteStatus; }); - return [ - favoriteStatusRef, - (update: FavoriteStatus) => - setFavoriteStatus(currentState => ({ ...currentState, ...update })), - ] as const; + const updateFavoriteStatus = (update: FavoriteStatus) => + setFavoriteStatus(currentState => ({ ...currentState, ...update })); + + const fetchFaveStar = (id: number) => { + SupersetClient.get({ + endpoint: `${baseURL}/${id}/count/`, + }) + .then(({ json }) => { + updateFavoriteStatus({ [id]: json.count > 0 }); + }) + .catch(() => + handleErrorFunc(t('There was an error fetching the favorite status')), + ); + }; + + const saveFaveStar = (id: number, isStarred: boolean) => { + const urlSuffix = isStarred ? 'unselect' : 'select'; + + SupersetClient.get({ + endpoint: `${baseURL}/${id}/${urlSuffix}/`, + }) + .then(() => { + updateFavoriteStatus({ [id]: !isStarred }); + }) + .catch(() => + handleErrorFunc(t('There was an error saving the favorite status')), + ); + }; + + return [favoriteStatusRef, fetchFaveStar, saveFaveStar] as const; } diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 73bee32dae4ad..a5e7789791ff0 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -62,41 +62,3 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { handleErrorFunc(parsedError.message); }; } - -export function createFaveStarHandlers( - baseURL: string, - favoriteStatus: FavoriteStatus, - setFavoriteStatus: (update: Partial) => void, - handleErrorFunc: (message: string) => void, -) { - const fetchFaveStar = (id: number) => { - SupersetClient.get({ - endpoint: `${baseURL}/${id}/count/`, - }) - .then(({ json }) => { - setFavoriteStatus({ [id]: json.count > 0 }); - }) - .catch(() => - handleErrorFunc(t('There was an error fetching the favorite status')), - ); - }; - - const saveFaveStar = (id: number, isStarred: boolean) => { - const urlSuffix = isStarred ? 'unselect' : 'select'; - - SupersetClient.get({ - endpoint: `${baseURL}/${id}/${urlSuffix}/`, - }) - .then(() => { - setFavoriteStatus({ [id]: !isStarred }); - }) - .catch(() => - handleErrorFunc(t('There was an error saving the favorite status')), - ); - }; - - return { - fetchFaveStar, - saveFaveStar, - }; -} From 260b22a889af4c2fbfc33d8b0f86de3de569ffcb Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Wed, 26 Aug 2020 13:44:56 -0700 Subject: [PATCH 12/13] fix: createErrorHandler function using wrong key on error object --- .../src/views/CRUD/chart/ChartList.tsx | 4 +-- .../views/CRUD/dashboard/DashboardList.tsx | 4 +-- superset-frontend/src/views/CRUD/hooks.ts | 32 +++++++++++-------- superset-frontend/src/views/CRUD/utils.tsx | 4 +-- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 17349c012c9db..514a93a70b2bb 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -103,9 +103,7 @@ function ChartList(props: ChartListProps) { const [favoriteStatusRef, fetchFaveStar, saveFaveStar] = useFavoriteStatus( {}, FAVESTAR_BASE_URL, - (message: string) => { - props.addDangerToast(message); - }, + props.addDangerToast, ); const [ sliceCurrentlyEditing, diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index eb0a9cdce3bf7..fded93a1bc10f 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -78,9 +78,7 @@ function DashboardList(props: DashboardListProps) { const [favoriteStatusRef, fetchFaveStar, saveFaveStar] = useFavoriteStatus( {}, FAVESTAR_BASE_URL, - (message: string) => { - props.addDangerToast(message); - }, + props.addDangerToast, ); const [dashboardToEdit, setDashboardToEdit] = useState( diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index c68574f64011f..1840d0784b965 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -175,7 +175,7 @@ export function useListViewResource( export function useFavoriteStatus( initialState: FavoriteStatus, baseURL: string, - handleErrorFunc: (message: string) => void, + handleErrorMsg: (message: string) => void, ) { const [favoriteStatus, setFavoriteStatus] = useState( initialState, @@ -191,13 +191,16 @@ export function useFavoriteStatus( const fetchFaveStar = (id: number) => { SupersetClient.get({ endpoint: `${baseURL}/${id}/count/`, - }) - .then(({ json }) => { + }).then( + ({ json }) => { updateFavoriteStatus({ [id]: json.count > 0 }); - }) - .catch(() => - handleErrorFunc(t('There was an error fetching the favorite status')), - ); + }, + createErrorHandler(errMsg => + handleErrorMsg( + t('There was an error fetching the favorite status: %s', errMsg), + ), + ), + ); }; const saveFaveStar = (id: number, isStarred: boolean) => { @@ -205,13 +208,16 @@ export function useFavoriteStatus( SupersetClient.get({ endpoint: `${baseURL}/${id}/${urlSuffix}/`, - }) - .then(() => { + }).then( + () => { updateFavoriteStatus({ [id]: !isStarred }); - }) - .catch(() => - handleErrorFunc(t('There was an error saving the favorite status')), - ); + }, + createErrorHandler(errMsg => + handleErrorMsg( + t('There was an error saving the favorite status: %s', errMsg), + ), + ), + ); }; return [favoriteStatusRef, fetchFaveStar, saveFaveStar] as const; diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index a5e7789791ff0..2bced467bb3c3 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -20,11 +20,9 @@ import { SupersetClient, SupersetClientResponse, } from '@superset-ui/connection'; -import { t } from '@superset-ui/translation'; import rison from 'rison'; import getClientErrorObject from 'src/utils/getClientErrorObject'; import { logging } from '@superset-ui/core'; -import { FavoriteStatus } from './types'; export const createFetchRelated = ( resource: string, @@ -59,6 +57,6 @@ export function createErrorHandler(handleErrorFunc: (errMsg?: string) => void) { return async (e: SupersetClientResponse | string) => { const parsedError = await getClientErrorObject(e); logging.error(e); - handleErrorFunc(parsedError.message); + handleErrorFunc(parsedError.error); }; } From 1319fdcdb4c889ae5dbdbdec554914fb6e9517a8 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Wed, 26 Aug 2020 14:42:24 -0700 Subject: [PATCH 13/13] fix comment --- superset-frontend/src/views/CRUD/hooks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 1840d0784b965..56e5c56443039 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -168,10 +168,10 @@ export function useListViewResource( }; } -// there hooks api has some known limitations around stale state in closures. +// the hooks api has some known limitations around stale state in closures. // See https://github.com/reactjs/rfcs/blob/master/text/0068-react-hooks.md#drawbacks // the useRef hook is a way of getting around these limitations by having a consistent ref -// that points to the latest value. +// that points to the most recent value. export function useFavoriteStatus( initialState: FavoriteStatus, baseURL: string,