diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index bd38316ba..927126632 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -238,6 +238,7 @@ "exports-loader": "^0.7.0", "fetch-mock": "^7.7.3", "fork-ts-checker-webpack-plugin": "^6.3.3", + "history": "^4.10.1", "ignore-styles": "^5.0.1", "imports-loader": "^3.0.0", "jest": "^26.6.3", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 32b129146..36d092c95 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -299,6 +299,7 @@ "exports-loader": "^0.7.0", "fetch-mock": "^7.7.3", "fork-ts-checker-webpack-plugin": "^6.3.3", + "history": "^4.10.1", "ignore-styles": "^5.0.1", "imports-loader": "^3.0.0", "jest": "^26.6.3", diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts index 10bee2dfc..c86c1cfbe 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts @@ -116,6 +116,10 @@ export type Filters = { [filterId: string]: Filter | Divider; }; +export type PartialFilters = { + [filterId: string]: Partial; +}; + export type NativeFiltersState = { filters: Filters; filterSets: FilterSets; diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 3cb4925e9..dea5cfb8f 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -145,6 +145,7 @@ export const singleNativeFiltersState = { inverseSelection: false, allowsMultipleValues: false, isRequired: false, + chartsInScope: [230], }, }, }; diff --git a/superset-frontend/src/components/EditableTitle/index.tsx b/superset-frontend/src/components/EditableTitle/index.tsx index 447188f4b..eebb0c714 100644 --- a/superset-frontend/src/components/EditableTitle/index.tsx +++ b/superset-frontend/src/components/EditableTitle/index.tsx @@ -16,9 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import React, { MouseEvent, useEffect, useState, useRef } from 'react'; +import React, { useEffect, useState, useRef } from 'react'; +import { Link } from 'react-router-dom'; import cx from 'classnames'; -import { css, styled, t } from '@superset-ui/core'; +import { css, styled, SupersetTheme, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import CertifiedBadge from '../CertifiedBadge'; @@ -37,7 +38,7 @@ export interface EditableTitleProps { placeholder?: string; certifiedBy?: string; certificationDetails?: string; - onClickTitle?: (event: MouseEvent) => void; + url?: string; } const StyledCertifiedBadge = styled(CertifiedBadge)` @@ -58,7 +59,7 @@ export default function EditableTitle({ placeholder = '', certifiedBy, certificationDetails, - onClickTitle, + url, // rest is related to title tooltip ...rest }: EditableTitleProps) { @@ -218,20 +219,20 @@ export default function EditableTitle({ } if (!canEdit) { // don't actually want an input in this case - titleComponent = onClickTitle ? ( - css` + color: ${theme.colors.grayscale.dark1}; + text-decoration: none; :hover { text-decoration: underline; } `} > {value} - + ) : ( {value} ); diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index a41450895..2f0aeff01 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -65,7 +65,7 @@ export const URL_PARAMS = { }, sliceId: { name: 'slice_id', - type: 'string', + type: 'number', }, datasourceId: { name: 'datasource_id', @@ -103,6 +103,10 @@ export const URL_PARAMS = { name: 'save_action', type: 'string', }, + dashboardPageId: { + name: 'dashboard_page_id', + type: 'string', + }, } as const; export const RESERVED_CHART_URL_PARAMS: string[] = [ diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 7f393f3bc..94001767f 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -55,6 +55,8 @@ import { FeatureFlag, isFeatureEnabled } from '../../featureFlags'; import extractUrlParams from '../util/extractUrlParams'; import getNativeFilterConfig from '../util/filterboxMigrationHelper'; import { updateColorSchema } from './dashboardInfo'; +import { getChartIdsInFilterScope } from '../util/getChartIdsInFilterScope'; +import updateComponentParentsList from '../util/updateComponentParentsList'; export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD'; @@ -256,6 +258,19 @@ export const hydrateDashboard = layout[layoutId].meta.sliceName = slice.slice_name; } }); + + // make sure that parents tree is built + if ( + Object.values(layout).some( + element => element.id !== DASHBOARD_ROOT_ID && !element.parents, + ) + ) { + updateComponentParentsList({ + currentComponent: layout[DASHBOARD_ROOT_ID], + layout, + }); + } + buildActiveFilters({ dashboardFilters, components: layout, @@ -333,9 +348,21 @@ export const hydrateDashboard = rootPath: [DASHBOARD_ROOT_ID], excluded: [chartId], // By default it doesn't affects itself }, + chartsInScope: Array.from(sliceIds), }, }; } + if ( + behaviors.includes(Behavior.INTERACTIVE_CHART) && + !metadata.chart_configuration[chartId].crossFilters?.chartsInScope + ) { + metadata.chart_configuration[chartId].crossFilters.chartsInScope = + getChartIdsInFilterScope( + metadata.chart_configuration[chartId].crossFilters.scope, + charts, + dashboardLayout.present, + ); + } }); } diff --git a/superset-frontend/src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal.tsx b/superset-frontend/src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal.tsx index f860f65b3..cdd9364b4 100644 --- a/superset-frontend/src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal.tsx +++ b/superset-frontend/src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal.tsx @@ -24,6 +24,8 @@ import Button from 'src/components/Button'; import { AntdForm } from 'src/components'; import { setChartConfiguration } from 'src/dashboard/actions/dashboardInfo'; import { ChartConfiguration } from 'src/dashboard/reducers/types'; +import { ChartsState, Layout, RootState } from 'src/dashboard/types'; +import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; import CrossFilterScopingForm from './CrossFilterScopingForm'; import { CrossFilterScopingFormType } from './types'; import { StyledForm } from '../nativeFilters/FiltersConfigModal/FiltersConfigModal'; @@ -44,14 +46,24 @@ const CrossFilterScopingModal: FC = ({ const chartConfig = useSelector( ({ dashboardInfo }) => dashboardInfo?.metadata?.chart_configuration, ); + const charts = useSelector(state => state.charts); + const layout = useSelector( + state => state.dashboardLayout.present, + ); const scope = chartConfig?.[chartId]?.crossFilters?.scope; const handleSave = () => { + const chartsInScope = getChartIdsInFilterScope( + form.getFieldValue('scope'), + charts, + layout, + ); + dispatch( setChartConfiguration({ ...chartConfig, [chartId]: { id: chartId, - crossFilters: { scope: form.getFieldValue('scope') }, + crossFilters: { scope: form.getFieldValue('scope'), chartsInScope }, }, }), ); diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index a881d0cda..56a696f91 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -31,7 +31,6 @@ import datasources from 'spec/fixtures/mockDatasource'; import { extraFormData, NATIVE_FILTER_ID, - layoutForSingleNativeFilter, singleNativeFiltersState, dataMaskWith1Filter, } from 'spec/fixtures/mockNativeFilters'; @@ -157,7 +156,7 @@ describe('Dashboard', () => { ...getAllActiveFilters({ dataMask: dataMaskWith1Filter, nativeFilters: singleNativeFiltersState.filters, - layout: layoutForSingleNativeFilter, + allSliceIds: [227, 229, 230], }), }, }); diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 6d9ab6fe9..880a6d33e 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -28,7 +28,6 @@ import { } from '@superset-ui/core'; import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants'; import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters'; -import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import { ChartConfiguration } from 'src/dashboard/reducers/types'; import { Layout } from 'src/dashboard/types'; import { areObjectsEqual } from 'src/reduxUtils'; @@ -293,18 +292,9 @@ export const selectNativeIndicatorsForChart = ( let crossFilterIndicators: any = []; if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { const dashboardLayoutValues = Object.values(dashboardLayout); - const chartLayoutItem = dashboardLayoutValues.find( - layoutItem => layoutItem?.meta?.chartId === chartId, - ); crossFilterIndicators = Object.values(chartConfiguration) - .filter( - chartConfig => - !chartConfig.crossFilters.scope.excluded.includes(chartId) && - chartConfig.crossFilters.scope.rootPath.some( - elementId => - chartLayoutItem?.type === CHART_TYPE && - chartLayoutItem?.parents?.includes(elementId), - ), + .filter(chartConfig => + chartConfig.crossFilters.chartsInScope.includes(chartId), ) .map(chartConfig => { const filterState = dataMask[chartConfig.id]?.filterState; diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx index 4113e114a..bc4054adc 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx @@ -17,6 +17,8 @@ * under the License. */ import React from 'react'; +import { Router } from 'react-router-dom'; +import { createMemoryHistory } from 'history'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import SliceHeader from '.'; @@ -155,7 +157,6 @@ const createProps = (overrides: any = {}) => ({ forceRefresh: jest.fn(), logExploreChart: jest.fn(), exportCSV: jest.fn(), - onExploreChart: jest.fn(), formData: { slice_id: 1, datasource: '58__table' }, width: 100, height: 100, @@ -206,7 +207,7 @@ test('Should render - default props', () => { // @ts-ignore delete props.sliceCanEdit; - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect(screen.getByTestId('slice-header')).toBeInTheDocument(); }); @@ -248,7 +249,7 @@ test('Should render default props and "call" actions', () => { // @ts-ignore delete props.sliceCanEdit; - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); userEvent.click(screen.getByTestId('toggleExpandSlice')); userEvent.click(screen.getByTestId('forceRefresh')); userEvent.click(screen.getByTestId('exploreChart')); @@ -261,13 +262,21 @@ test('Should render default props and "call" actions', () => { test('Should render title', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect(screen.getByText('Vaccine Candidates per Phase')).toBeInTheDocument(); }); test('Should render click to edit prompt and run onExploreChart on click', async () => { const props = createProps(); - render(, { useRedux: true }); + const history = createMemoryHistory({ + initialEntries: ['/superset/dashboard/1/'], + }); + render( + + + , + { useRedux: true }, + ); userEvent.hover(screen.getByText('Vaccine Candidates per Phase')); expect( await screen.findByText('Click to edit Vaccine Candidates per Phase.'), @@ -277,13 +286,13 @@ test('Should render click to edit prompt and run onExploreChart on click', async ).toBeInTheDocument(); userEvent.click(screen.getByText('Vaccine Candidates per Phase')); - expect(props.onExploreChart).toHaveBeenCalled(); + expect(history.location.pathname).toMatch('/explore'); }); test('Display cmd button in tooltip if running on MacOS', async () => { jest.spyOn(window.navigator, 'appVersion', 'get').mockReturnValue('Mac'); const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); userEvent.hover(screen.getByText('Vaccine Candidates per Phase')); expect( await screen.findByText('Click to edit Vaccine Candidates per Phase.'), @@ -296,7 +305,7 @@ test('Display cmd button in tooltip if running on MacOS', async () => { test('Display correct tooltip when DASHBOARD_EDIT_CHART_IN_NEW_TAB is enabled', async () => { window.featureFlags.DASHBOARD_EDIT_CHART_IN_NEW_TAB = true; const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); userEvent.hover(screen.getByText('Vaccine Candidates per Phase')); expect( await screen.findByText( @@ -307,7 +316,15 @@ test('Display correct tooltip when DASHBOARD_EDIT_CHART_IN_NEW_TAB is enabled', test('Should not render click to edit prompt and run onExploreChart on click if supersetCanExplore=false', () => { const props = createProps({ supersetCanExplore: false }); - render(, { useRedux: true }); + const history = createMemoryHistory({ + initialEntries: ['/superset/dashboard/1/'], + }); + render( + + + , + { useRedux: true }, + ); userEvent.hover(screen.getByText('Vaccine Candidates per Phase')); expect( screen.queryByText( @@ -316,12 +333,20 @@ test('Should not render click to edit prompt and run onExploreChart on click if ).not.toBeInTheDocument(); userEvent.click(screen.getByText('Vaccine Candidates per Phase')); - expect(props.onExploreChart).not.toHaveBeenCalled(); + expect(history.location.pathname).toMatch('/superset/dashboard'); }); test('Should not render click to edit prompt and run onExploreChart on click if in edit mode', () => { const props = createProps({ editMode: true }); - render(, { useRedux: true }); + const history = createMemoryHistory({ + initialEntries: ['/superset/dashboard/1/'], + }); + render( + + + , + { useRedux: true }, + ); userEvent.hover(screen.getByText('Vaccine Candidates per Phase')); expect( screen.queryByText( @@ -330,12 +355,12 @@ test('Should not render click to edit prompt and run onExploreChart on click if ).not.toBeInTheDocument(); userEvent.click(screen.getByText('Vaccine Candidates per Phase')); - expect(props.onExploreChart).not.toHaveBeenCalled(); + expect(history.location.pathname).toMatch('/superset/dashboard'); }); test('Should render "annotationsLoading"', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect( screen.getByRole('img', { name: 'Annotation layers are still loading.', @@ -345,7 +370,7 @@ test('Should render "annotationsLoading"', () => { test('Should render "annotationsError"', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect( screen.getByRole('img', { name: 'One ore more annotation layers failed loading.', @@ -357,7 +382,7 @@ test('Should not render "annotationsError" and "annotationsLoading"', () => { const props = createProps(); props.annotationQuery = {}; props.annotationError = {}; - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect( screen.queryByRole('img', { name: 'One ore more annotation layers failed loading.', @@ -372,7 +397,7 @@ test('Should not render "annotationsError" and "annotationsLoading"', () => { test('Correct props to "FiltersBadge"', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect(screen.getByTestId('FiltersBadge')).toHaveAttribute( 'data-chart-id', '312', @@ -381,7 +406,7 @@ test('Correct props to "FiltersBadge"', () => { test('Correct props to "SliceHeaderControls"', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect(screen.getByTestId('SliceHeaderControls')).toHaveAttribute( 'data-cached-dttm', '', @@ -438,7 +463,7 @@ test('Correct props to "SliceHeaderControls"', () => { test('Correct actions to "SliceHeaderControls"', () => { const props = createProps(); - render(, { useRedux: true }); + render(, { useRedux: true, useRouter: true }); expect(props.toggleExpandSlice).toBeCalledTimes(0); userEvent.click(screen.getByTestId('toggleExpandSlice')); diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index d4b516669..587446dfc 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -19,6 +19,7 @@ import React, { FC, ReactNode, + useContext, useEffect, useMemo, useRef, @@ -37,6 +38,7 @@ import Icons from 'src/components/Icons'; import { RootState } from 'src/dashboard/types'; import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator'; import { getSliceHeaderTooltip } from 'src/dashboard/util/getSliceHeaderTooltip'; +import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; import { clearDataMask } from 'src/dataMask/actions'; type SliceHeaderProps = SliceHeaderControlsProps & { @@ -68,7 +70,6 @@ const SliceHeader: FC = ({ updateSliceName = () => ({}), toggleExpandSlice = () => ({}), logExploreChart = () => ({}), - onExploreChart, exportCSV = () => ({}), editMode = false, annotationQuery = {}, @@ -97,6 +98,7 @@ const SliceHeader: FC = ({ }) => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); + const dashboardPageId = useContext(DashboardPageIdContext); const [headerTooltip, setHeaderTooltip] = useState(null); const headerRef = useRef(null); // TODO: change to indicator field after it will be implemented @@ -112,12 +114,11 @@ const SliceHeader: FC = ({ [crossFilterValue], ); - const handleClickTitle = - !editMode && supersetCanExplore ? onExploreChart : undefined; + const canExplore = !editMode && supersetCanExplore; useEffect(() => { const headerElement = headerRef.current; - if (handleClickTitle) { + if (canExplore) { setHeaderTooltip(getSliceHeaderTooltip(sliceName)); } else if ( headerElement && @@ -128,7 +129,9 @@ const SliceHeader: FC = ({ } else { setHeaderTooltip(null); } - }, [sliceName, width, height, handleClickTitle]); + }, [sliceName, width, height, canExplore]); + + const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`; return (
@@ -145,7 +148,7 @@ const SliceHeader: FC = ({ emptyText="" onSaveTitle={updateSliceName} showTooltip={false} - onClickTitle={handleClickTitle} + url={canExplore ? exploreUrl : undefined} /> {!!Object.values(annotationQuery).length && ( @@ -206,7 +209,6 @@ const SliceHeader: FC = ({ toggleExpandSlice={toggleExpandSlice} forceRefresh={forceRefresh} logExploreChart={logExploreChart} - onExploreChart={onExploreChart} exportCSV={exportCSV} exportFullCSV={exportFullCSV} supersetCanExplore={supersetCanExplore} @@ -222,6 +224,7 @@ const SliceHeader: FC = ({ isDescriptionExpanded={isExpanded} chartStatus={chartStatus} formData={formData} + exploreUrl={exploreUrl} /> )} diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx index 234029407..514ff4fc2 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx @@ -21,7 +21,7 @@ import userEvent from '@testing-library/user-event'; import React from 'react'; import { render, screen } from 'spec/helpers/testing-library'; import { FeatureFlag } from 'src/featureFlags'; -import SliceHeaderControls from '.'; +import SliceHeaderControls, { SliceHeaderControlsProps } from '.'; jest.mock('src/components/Dropdown', () => { const original = jest.requireActual('src/components/Dropdown'); @@ -36,66 +36,74 @@ jest.mock('src/components/Dropdown', () => { }; }); -const createProps = (viz_type = 'sunburst') => ({ - addDangerToast: jest.fn(), - addSuccessToast: jest.fn(), - exploreChart: jest.fn(), - exportCSV: jest.fn(), - exportFullCSV: jest.fn(), - forceRefresh: jest.fn(), - handleToggleFullSize: jest.fn(), - toggleExpandSlice: jest.fn(), - onExploreChart: jest.fn(), - slice: { - slice_id: 371, - slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20371%7D', - slice_name: 'Vaccine Candidates per Country & Stage', - slice_description: 'Table of vaccine candidates for 100 countries', - form_data: { - adhoc_filters: [], - color_scheme: 'supersetColors', - datasource: '58__table', - groupby: ['product_category', 'clinical_stage'], - linear_color_scheme: 'schemeYlOrBr', - metric: 'count', - queryFields: { - groupby: 'groupby', - metric: 'metrics', - secondary_metric: 'metrics', - }, - row_limit: 10000, +const createProps = (viz_type = 'sunburst') => + ({ + addDangerToast: jest.fn(), + addSuccessToast: jest.fn(), + exploreChart: jest.fn(), + exportCSV: jest.fn(), + exportFullCSV: jest.fn(), + forceRefresh: jest.fn(), + handleToggleFullSize: jest.fn(), + toggleExpandSlice: jest.fn(), + slice: { slice_id: 371, - time_range: 'No filter', - url_params: {}, + slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20371%7D', + slice_name: 'Vaccine Candidates per Country & Stage', + slice_description: 'Table of vaccine candidates for 100 countries', + form_data: { + adhoc_filters: [], + color_scheme: 'supersetColors', + datasource: '58__table', + groupby: ['product_category', 'clinical_stage'], + linear_color_scheme: 'schemeYlOrBr', + metric: 'count', + queryFields: { + groupby: 'groupby', + metric: 'metrics', + secondary_metric: 'metrics', + }, + row_limit: 10000, + slice_id: 371, + time_range: 'No filter', + url_params: {}, + viz_type, + }, viz_type, + datasource: '58__table', + description: 'test-description', + description_markeddown: '', + owners: [], + modified: '22 hours ago', + changed_on: 1617143411523, }, - viz_type, - datasource: '58__table', - description: 'test-description', - description_markeddown: '', - owners: [], - modified: '22 hours ago', - changed_on: 1617143411523, - }, - isCached: [false], - isExpanded: false, - cachedDttm: [''], - updatedDttm: 1617213803803, - supersetCanExplore: true, - supersetCanCSV: true, - sliceCanEdit: false, - componentId: 'CHART-fYo7IyvKZQ', - dashboardId: 26, - isFullSize: false, - chartStatus: 'rendered', - showControls: true, - supersetCanShare: true, - formData: { slice_id: 1, datasource: '58__table', viz_type: 'sunburst' }, -}); + isCached: [false], + isExpanded: false, + cachedDttm: [''], + updatedDttm: 1617213803803, + supersetCanExplore: true, + supersetCanCSV: true, + sliceCanEdit: false, + componentId: 'CHART-fYo7IyvKZQ', + dashboardId: 26, + isFullSize: false, + chartStatus: 'rendered', + showControls: true, + supersetCanShare: true, + formData: { slice_id: 1, datasource: '58__table', viz_type: 'sunburst' }, + exploreUrl: '/explore', + } as SliceHeaderControlsProps); + +const renderWrapper = (overrideProps?: SliceHeaderControlsProps) => { + const props = overrideProps || createProps(); + return render(, { + useRedux: true, + useRouter: true, + }); +}; test('Should render', () => { - const props = createProps(); - render(, { useRedux: true }); + renderWrapper(); expect( screen.getByRole('button', { name: 'More Options' }), ).toBeInTheDocument(); @@ -124,7 +132,7 @@ test('Should render default props', () => { // @ts-ignore delete props.sliceCanEdit; - render(, { useRedux: true }); + renderWrapper(props); expect( screen.getByRole('menuitem', { name: 'Enter fullscreen' }), ).toBeInTheDocument(); @@ -150,8 +158,7 @@ test('Should render default props', () => { test('Should "export to CSV"', async () => { const props = createProps(); - render(, { useRedux: true }); - + renderWrapper(props); expect(props.exportCSV).toBeCalledTimes(0); userEvent.hover(screen.getByText('Download')); userEvent.click(await screen.findByText('Export to .CSV')); @@ -161,7 +168,7 @@ test('Should "export to CSV"', async () => { test('Should not show "Download" if slice is filter box', () => { const props = createProps('filter_box'); - render(, { useRedux: true }); + renderWrapper(props); expect(screen.queryByText('Download')).not.toBeInTheDocument(); }); @@ -171,7 +178,7 @@ test('Export full CSV is under featureflag', async () => { [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: false, }; const props = createProps('table'); - render(, { useRedux: true }); + renderWrapper(props); userEvent.hover(screen.getByText('Download')); expect(await screen.findByText('Export to .CSV')).toBeInTheDocument(); expect(screen.queryByText('Export to full .CSV')).not.toBeInTheDocument(); @@ -183,7 +190,7 @@ test('Should "export full CSV"', async () => { [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true, }; const props = createProps('table'); - render(, { useRedux: true }); + renderWrapper(props); expect(props.exportFullCSV).toBeCalledTimes(0); userEvent.hover(screen.getByText('Download')); userEvent.click(await screen.findByText('Export to full .CSV')); @@ -196,8 +203,7 @@ test('Should not show export full CSV if report is not table', async () => { global.featureFlags = { [FeatureFlag.ALLOW_FULL_CSV_EXPORT]: true, }; - const props = createProps(); - render(, { useRedux: true }); + renderWrapper(); userEvent.hover(screen.getByText('Download')); expect(await screen.findByText('Export to .CSV')).toBeInTheDocument(); expect(screen.queryByText('Export to full .CSV')).not.toBeInTheDocument(); @@ -205,8 +211,7 @@ test('Should not show export full CSV if report is not table', async () => { test('Should "Show chart description"', () => { const props = createProps(); - render(, { useRedux: true }); - + renderWrapper(props); expect(props.toggleExpandSlice).toBeCalledTimes(0); userEvent.click(screen.getByText('Show chart description')); expect(props.toggleExpandSlice).toBeCalledTimes(1); @@ -215,8 +220,7 @@ test('Should "Show chart description"', () => { test('Should "Force refresh"', () => { const props = createProps(); - render(, { useRedux: true }); - + renderWrapper(props); expect(props.forceRefresh).toBeCalledTimes(0); userEvent.click(screen.getByText('Force refresh')); expect(props.forceRefresh).toBeCalledTimes(1); @@ -226,7 +230,7 @@ test('Should "Force refresh"', () => { test('Should "Enter fullscreen"', () => { const props = createProps(); - render(, { useRedux: true }); + renderWrapper(props); expect(props.handleToggleFullSize).toBeCalledTimes(0); userEvent.click(screen.getByText('Enter fullscreen')); diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index ba748fc9c..8673a0384 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { MouseEvent, Key } from 'react'; +import { Link, RouteComponentProps, withRouter } from 'react-router-dom'; import moment from 'moment'; import { Behavior, @@ -108,7 +109,7 @@ export interface SliceHeaderControlsProps { isFullSize?: boolean; isDescriptionExpanded?: boolean; formData: QueryFormData; - onExploreChart: (event: MouseEvent) => void; + exploreUrl: string; forceRefresh: (sliceId: number, dashboardId: number) => void; logExploreChart?: (sliceId: number) => void; @@ -125,6 +126,8 @@ export interface SliceHeaderControlsProps { supersetCanCSV?: boolean; sliceCanEdit?: boolean; } +type SliceHeaderControlsPropsWithRouter = SliceHeaderControlsProps & + RouteComponentProps; interface State { showControls: boolean; showCrossFilterScopingModal: boolean; @@ -138,10 +141,10 @@ const dropdownIconsStyles = css` `; class SliceHeaderControls extends React.PureComponent< - SliceHeaderControlsProps, + SliceHeaderControlsPropsWithRouter, State > { - constructor(props: SliceHeaderControlsProps) { + constructor(props: SliceHeaderControlsPropsWithRouter) { super(props); this.toggleControls = this.toggleControls.bind(this); this.refreshChart = this.refreshChart.bind(this); @@ -306,13 +309,14 @@ class SliceHeaderControls extends React.PureComponent< )} {this.props.supersetCanExplore && ( - this.props.onExploreChart(domEvent)} - > - - {t('Edit chart')} - + + + + {t('Edit chart')} + + )} @@ -355,7 +359,7 @@ class SliceHeaderControls extends React.PureComponent< @@ -463,4 +467,4 @@ class SliceHeaderControls extends React.PureComponent< } } -export default SliceHeaderControls; +export default withRouter(SliceHeaderControls); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index ee7d0fa6a..f28c5d47c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -211,10 +211,14 @@ const publishDataMask = debounce( // pathname could be updated somewhere else through window.history // keep react router history in sync with window history - history.location.pathname = window.location.pathname; - history.replace({ - search: newParams.toString(), - }); + // replace params only when current page is /superset/dashboard + // this prevents a race condition between updating filters and navigating to Explore + if (window.location.pathname.includes('/superset/dashboard')) { + history.location.pathname = window.location.pathname; + history.replace({ + search: newParams.toString(), + }); + } }, SLOW_DEBOUNCE, ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 868205714..9a21eb0e8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -28,7 +28,7 @@ import { getChartMetadataRegistry, QueryFormData, } from '@superset-ui/core'; -import { Charts, DashboardLayout } from 'src/dashboard/types'; +import { DashboardLayout } from 'src/dashboard/types'; import extractUrlParams from 'src/dashboard/util/extractUrlParams'; import { isFeatureEnabled } from 'src/featureFlags'; import { CHART_TYPE, TAB_TYPE } from '../../util/componentTypes'; @@ -122,7 +122,6 @@ export function isCrossFilter(vizType: string) { export function getExtraFormData( dataMask: DataMaskStateWithId, - charts: Charts, filterIdsAppliedOnChart: string[], ): ExtraFormData { let extraFormData: ExtraFormData = {}; diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index 81d06b856..337b5fc01 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -46,7 +46,6 @@ function mapStateToProps( charts: chartQueries, dashboardInfo, dashboardState, - dashboardLayout, dataMask, datasources, sliceEntities, @@ -65,16 +64,15 @@ function mapStateToProps( const sharedLabelColors = dashboardInfo?.metadata?.shared_label_colors || {}; // note: this method caches filters if possible to prevent render cascades const formData = getFormDataWithExtraFilters({ - layout: dashboardLayout.present, chart, - // eslint-disable-next-line camelcase chartConfiguration: dashboardInfo.metadata?.chart_configuration, charts: chartQueries, filters: getAppliedFilterValues(id), colorScheme, colorNamespace, sliceId: id, - nativeFilters, + nativeFilters: nativeFilters?.filters, + allSliceIds: dashboardState.sliceIds, dataMask, extraControls, labelColors, diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index 647d774f4..50e42fef2 100644 --- a/superset-frontend/src/dashboard/containers/Dashboard.ts +++ b/superset-frontend/src/dashboard/containers/Dashboard.ts @@ -68,7 +68,7 @@ function mapStateToProps(state: RootState) { chartConfiguration: dashboardInfo.metadata?.chart_configuration, nativeFilters: nativeFilters.filters, dataMask, - layout: dashboardLayout.present, + allSliceIds: dashboardState.sliceIds, }), }, chartConfiguration: dashboardInfo.metadata?.chart_configuration, diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 27e17e0f5..410bf4ab5 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FC, useRef, useEffect, useState } from 'react'; +import React, { FC, useEffect, useMemo, useRef, useState } from 'react'; import { CategoricalColorNamespace, FeatureFlag, @@ -25,6 +25,7 @@ import { t, useTheme, } from '@superset-ui/core'; +import pick from 'lodash/pick'; import { useDispatch, useSelector } from 'react-redux'; import { Global } from '@emotion/react'; import { useToasts } from 'src/components/MessageToasts/withToasts'; @@ -44,8 +45,8 @@ import { addWarningToast } from 'src/components/MessageToasts/actions'; import { getItem, - setItem, LocalStorageKeys, + setItem, } from 'src/utils/localStorageHelpers'; import { FILTER_BOX_MIGRATION_STATES, @@ -61,11 +62,17 @@ import { getPermalinkValue, } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; import { filterCardPopoverStyle } from 'src/dashboard/styles'; +import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore'; +import shortid from 'shortid'; +import { RootState } from '../types'; +import { getActiveFilters } from '../util/activeDashboardFilters'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, ); +export const DashboardPageIdContext = React.createContext(''); + setupPlugins(); const DashboardContainer = React.lazy( () => @@ -82,12 +89,76 @@ type PageProps = { idOrSlug: string; }; +const getDashboardContextLocalStorage = () => { + const dashboardsContexts = getItem( + LocalStorageKeys.dashboard__explore_context, + {}, + ); + // A new dashboard tab id is generated on each dashboard page opening. + // We mark ids as redundant when user leaves the dashboard, because they won't be reused. + // Then we remove redundant dashboard contexts from local storage in order not to clutter it + return Object.fromEntries( + Object.entries(dashboardsContexts).filter( + ([, value]) => !value.isRedundant, + ), + ); +}; + +const updateDashboardTabLocalStorage = ( + dashboardPageId: string, + dashboardContext: DashboardContextForExplore, +) => { + const dashboardsContexts = getDashboardContextLocalStorage(); + setItem(LocalStorageKeys.dashboard__explore_context, { + ...dashboardsContexts, + [dashboardPageId]: dashboardContext, + }); +}; + +const useSyncDashboardStateWithLocalStorage = () => { + const dashboardPageId = useMemo(() => shortid.generate(), []); + const dashboardContextForExplore = useSelector< + RootState, + DashboardContextForExplore + >(({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({ + labelColors: dashboardInfo.metadata?.label_colors || {}, + sharedLabelColors: dashboardInfo.metadata?.shared_label_colors || {}, + colorScheme: dashboardState?.colorScheme, + chartConfiguration: dashboardInfo.metadata?.chart_configuration || {}, + nativeFilters: Object.entries(nativeFilters.filters).reduce( + (acc, [key, filterValue]) => ({ + ...acc, + [key]: pick(filterValue, ['chartsInScope']), + }), + {}, + ), + dataMask, + dashboardId: dashboardInfo.id, + filterBoxFilters: getActiveFilters(), + dashboardPageId, + })); + + useEffect(() => { + updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore); + return () => { + // mark tab id as redundant when dashboard unmounts - case when user opens + // Explore in the same tab + updateDashboardTabLocalStorage(dashboardPageId, { + ...dashboardContextForExplore, + isRedundant: true, + }); + }; + }, [dashboardContextForExplore, dashboardPageId]); + return dashboardPageId; +}; + export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const dispatch = useDispatch(); const theme = useTheme(); const user = useSelector( state => state.user, ); + const dashboardPageId = useSyncDashboardStateWithLocalStorage(); const { addDangerToast } = useToasts(); const { result: dashboard, error: dashboardApiError } = useDashboard(idOrSlug); @@ -113,6 +184,25 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { migrationStateParam || FILTER_BOX_MIGRATION_STATES.NOOP, ); + useEffect(() => { + // mark tab id as redundant when user closes browser tab - a new id will be + // generated next time user opens a dashboard and the old one won't be reused + const handleTabClose = () => { + const dashboardsContexts = getDashboardContextLocalStorage(); + setItem(LocalStorageKeys.dashboard__explore_context, { + ...dashboardsContexts, + [dashboardPageId]: { + ...dashboardsContexts[dashboardPageId], + isRedundant: true, + }, + }); + }; + window.addEventListener('beforeunload', handleTabClose); + return () => { + window.removeEventListener('beforeunload', handleTabClose); + }; + }, [dashboardPageId]); + useEffect(() => { dispatch(setDatasetsStatus(status)); }, [dispatch, status]); @@ -295,7 +385,9 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { /> - + + + ); diff --git a/superset-frontend/src/dashboard/reducers/types.ts b/superset-frontend/src/dashboard/reducers/types.ts index 7b3aec252..9e9b4e8ca 100644 --- a/superset-frontend/src/dashboard/reducers/types.ts +++ b/superset-frontend/src/dashboard/reducers/types.ts @@ -30,6 +30,7 @@ export type ChartConfiguration = { id: number; crossFilters: { scope: NativeFilterScope; + chartsInScope: number[]; }; }; }; diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 160d62564..52d110599 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -65,6 +65,8 @@ export type DashboardState = { isRefreshing: boolean; isFiltersRefreshing: boolean; hasUnsavedChanges: boolean; + colorScheme: string; + sliceIds: number[]; }; export type DashboardInfo = { id: number; @@ -79,6 +81,8 @@ export type DashboardInfo = { native_filter_configuration: JsonObject; show_native_filters: boolean; chart_configuration: JsonObject; + label_colors: JsonObject; + shared_label_colors: JsonObject; }; }; diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts index f0a9b709e..14ae2d4ea 100644 --- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts +++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts @@ -18,61 +18,11 @@ */ import { DataMaskStateWithId, - Filters, + PartialFilters, JsonObject, - NativeFilterScope, } from '@superset-ui/core'; -import { CHART_TYPE } from './componentTypes'; -import { ActiveFilters, Layout, LayoutItem } from '../types'; +import { ActiveFilters } from '../types'; import { ChartConfiguration } from '../reducers/types'; -import { DASHBOARD_ROOT_ID } from './constants'; - -// Looking for affected chart scopes and values -export const findAffectedCharts = ({ - child, - layout, - scope, - activeFilters, - filterId, - extraFormData, -}: { - child: string; - layout: { [key: string]: LayoutItem }; - scope: NativeFilterScope; - activeFilters: ActiveFilters; - filterId: string; - extraFormData: any; -}) => { - const chartId = layout[child]?.meta?.chartId; - if (layout[child].type === CHART_TYPE) { - // Ignore excluded charts - if (scope.excluded.includes(chartId)) { - return; - } - if (!activeFilters[filterId]) { - // Small mutation but simplify logic - // eslint-disable-next-line no-param-reassign - activeFilters[filterId] = { - scope: [], - values: extraFormData, - }; - } - // Add not excluded chart scopes(to know what charts refresh) and values(refresh only if its value changed) - activeFilters[filterId].scope.push(chartId); - return; - } - // If child is not chart, recursive iterate over its children - layout[child].children.forEach((child: string) => - findAffectedCharts({ - child, - layout, - scope, - activeFilters, - filterId, - extraFormData, - }), - ); -}; export const getRelevantDataMask = ( dataMask: DataMaskStateWithId, @@ -89,36 +39,27 @@ export const getAllActiveFilters = ({ chartConfiguration, nativeFilters, dataMask, - layout, + allSliceIds, }: { chartConfiguration: ChartConfiguration; dataMask: DataMaskStateWithId; - nativeFilters: Filters; - layout: Layout; + nativeFilters: PartialFilters; + allSliceIds: number[]; }): ActiveFilters => { const activeFilters = {}; // Combine native filters with cross filters, because they have similar logic Object.values(dataMask).forEach(({ id: filterId, extraFormData }) => { - const scope = nativeFilters?.[filterId]?.scope ?? - chartConfiguration?.[filterId]?.crossFilters?.scope ?? { - rootPath: [DASHBOARD_ROOT_ID], - excluded: [filterId], - }; + const scope = + nativeFilters?.[filterId]?.chartsInScope ?? + chartConfiguration?.[filterId]?.crossFilters?.chartsInScope ?? + allSliceIds ?? + []; // Iterate over all roots to find all affected charts - scope.rootPath.forEach((layoutItemId: string | number) => { - layout[layoutItemId]?.children?.forEach((child: string) => { - // Need exclude from affected charts, charts that located in scope `excluded` - findAffectedCharts({ - child, - layout, - scope, - activeFilters, - filterId, - extraFormData, - }); - }); - }); + activeFilters[filterId] = { + scope, + values: extraFormData, + }; }); return activeFilters; }; diff --git a/superset-frontend/src/dashboard/util/activeDashboardFilters.js b/superset-frontend/src/dashboard/util/activeDashboardFilters.js index 41871f7c0..3369dc1c5 100644 --- a/superset-frontend/src/dashboard/util/activeDashboardFilters.js +++ b/superset-frontend/src/dashboard/util/activeDashboardFilters.js @@ -45,10 +45,10 @@ export function isFilterBox(chartId) { // this function is to find all filter values applied to a chart, // it goes through all active filters and their scopes. // return: { [column]: array of selected values } -export function getAppliedFilterValues(chartId) { +export function getAppliedFilterValues(chartId, filters) { // use cached data if possible if (!(chartId in appliedFilterValuesByChart)) { - const applicableFilters = Object.entries(activeFilters).filter( + const applicableFilters = Object.entries(filters || activeFilters).filter( ([, { scope: chartIds }]) => chartIds.includes(chartId), ); appliedFilterValuesByChart[chartId] = flow( diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 3ca003fd0..cbed55755 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -20,9 +20,9 @@ import { DataMaskStateWithId, DataRecordFilters, JsonObject, - NativeFiltersState, + PartialFilters, } from '@superset-ui/core'; -import { ChartQueryPayload, Charts, LayoutItem } from 'src/dashboard/types'; +import { ChartQueryPayload } from 'src/dashboard/types'; import { getExtraFormData } from 'src/dashboard/components/nativeFilters/utils'; import { areObjectsEqual } from 'src/reduxUtils'; import getEffectiveExtraFilters from './getEffectiveExtraFilters'; @@ -37,17 +37,16 @@ const cachedFormdataByChart = {}; export interface GetFormDataWithExtraFiltersArguments { chartConfiguration: ChartConfiguration; chart: ChartQueryPayload; - charts: Charts; filters: DataRecordFilters; - layout: { [key: string]: LayoutItem }; colorScheme?: string; colorNamespace?: string; sliceId: number; dataMask: DataMaskStateWithId; - nativeFilters: NativeFiltersState; + nativeFilters: PartialFilters; extraControls: Record; labelColors?: Record; sharedLabelColors?: Record; + allSliceIds: number[]; } // this function merge chart's formData with dashboard filters value, @@ -55,18 +54,17 @@ export interface GetFormDataWithExtraFiltersArguments { // filters param only contains those applicable to this chart. export default function getFormDataWithExtraFilters({ chart, - charts, filters, nativeFilters, chartConfiguration, colorScheme, colorNamespace, sliceId, - layout, dataMask, extraControls, labelColors, sharedLabelColors, + allSliceIds, }: GetFormDataWithExtraFiltersArguments) { // if dashboard metadata + filters have not changed, use cache if possible const cachedFormData = cachedFormdataByChart[sliceId]; @@ -99,19 +97,15 @@ export default function getFormDataWithExtraFilters({ const activeFilters = getAllActiveFilters({ chartConfiguration, dataMask, - layout, - nativeFilters: nativeFilters.filters, + nativeFilters, + allSliceIds, }); const filterIdsAppliedOnChart = Object.entries(activeFilters) .filter(([, { scope }]) => scope.includes(chart.id)) .map(([filterId]) => filterId); if (filterIdsAppliedOnChart.length) { extraData = { - extra_form_data: getExtraFormData( - dataMask, - charts, - filterIdsAppliedOnChart, - ), + extra_form_data: getExtraFormData(dataMask, filterIdsAppliedOnChart), }; } diff --git a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts index ce2cff688..34cbe4b1b 100644 --- a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts +++ b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts @@ -50,19 +50,13 @@ describe('getFormDataWithExtraFilters', () => { }; const mockArgs: GetFormDataWithExtraFiltersArguments = { chartConfiguration: {}, - charts: { - [chartId as number]: mockChart, - }, chart: mockChart, filters: { region: ['Spain'], color: ['pink', 'purple'], }, sliceId: chartId, - nativeFilters: { - filters: {}, - filterSets: {}, - }, + nativeFilters: {}, dataMask: { [filterId]: { id: filterId, @@ -71,10 +65,10 @@ describe('getFormDataWithExtraFilters', () => { ownState: {}, }, }, - layout: {}, extraControls: { stack: 'Stacked', }, + allSliceIds: [chartId], }; it('should include filters from the passed filters', () => { diff --git a/superset-frontend/src/explore/ExplorePage.tsx b/superset-frontend/src/explore/ExplorePage.tsx index 6425f4c49..06e443ae5 100644 --- a/superset-frontend/src/explore/ExplorePage.tsx +++ b/superset-frontend/src/explore/ExplorePage.tsx @@ -19,17 +19,21 @@ import React, { useEffect, useRef, useState } from 'react'; import { useDispatch } from 'react-redux'; import { useLocation } from 'react-router-dom'; -import { makeApi, t, isDefined, JsonObject } from '@superset-ui/core'; +import { isDefined, JsonObject, makeApi, t } from '@superset-ui/core'; import Loading from 'src/components/Loading'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { getUrlParam } from 'src/utils/urlUtils'; import { URL_PARAMS } from 'src/constants'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; +import { getAppliedFilterValues } from 'src/dashboard/util/activeDashboardFilters'; import { getParsedExploreURLParams } from './exploreUtils/getParsedExploreURLParams'; import { hydrateExplore } from './actions/hydrateExplore'; import ExploreViewContainer from './components/ExploreViewContainer'; import { ExploreResponsePayload } from './types'; import { fallbackExploreInitialData } from './fixtures'; +import { getItem, LocalStorageKeys } from '../utils/localStorageHelpers'; +import { getFormDataWithDashboardContext } from './controlUtils/getFormDataWithDashboardContext'; const isResult = (rv: JsonObject): rv is ExploreResponsePayload => rv?.result?.form_data && @@ -57,6 +61,43 @@ const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { } }; +const getDashboardContextFormData = () => { + const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId); + const sliceId = getUrlParam(URL_PARAMS.sliceId) || 0; + let dashboardContextWithFilters = {}; + if (dashboardPageId) { + const { + labelColors, + sharedLabelColors, + colorScheme, + chartConfiguration, + nativeFilters, + filterBoxFilters, + dataMask, + dashboardId, + } = + getItem(LocalStorageKeys.dashboard__explore_context, {})[ + dashboardPageId + ] || {}; + dashboardContextWithFilters = getFormDataWithExtraFilters({ + chart: { id: sliceId }, + filters: getAppliedFilterValues(sliceId, filterBoxFilters), + nativeFilters, + chartConfiguration, + colorScheme, + dataMask, + labelColors, + sharedLabelColors, + sliceId, + allSliceIds: [sliceId], + extraControls: {}, + }); + Object.assign(dashboardContextWithFilters, { dashboardId }); + return dashboardContextWithFilters; + } + return {}; +}; + export default function ExplorePage() { const [isLoaded, setIsLoaded] = useState(false); const isExploreInitialized = useRef(false); @@ -66,10 +107,20 @@ export default function ExplorePage() { useEffect(() => { const exploreUrlParams = getParsedExploreURLParams(location); const isSaveAction = !!getUrlParam(URL_PARAMS.saveAction); + const dashboardContextFormData = getDashboardContextFormData(); if (!isExploreInitialized.current || isSaveAction) { fetchExploreData(exploreUrlParams) .then(({ result }) => { - dispatch(hydrateExplore(result)); + const formData = getFormDataWithDashboardContext( + result.form_data, + dashboardContextFormData, + ); + dispatch( + hydrateExplore({ + ...result, + form_data: formData, + }), + ); }) .catch(err => { dispatch(hydrateExplore(fallbackExploreInitialData)); diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 02050eaec..8c0dd2a15 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -212,7 +212,7 @@ class SaveModal extends React.Component { return; } - const searchParams = new URLSearchParams(this.props.location.search); + const searchParams = new URLSearchParams(window.location.search); searchParams.set('save_action', this.state.action); searchParams.delete('form_data_key'); if (this.state.action === 'saveas') { diff --git a/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts b/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts new file mode 100644 index 000000000..ab14c8cf0 --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts @@ -0,0 +1,212 @@ +/** + * 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 { JsonObject } from '@superset-ui/core'; +import { getFormDataWithDashboardContext } from './getFormDataWithDashboardContext'; + +const getExploreFormData = (overrides: JsonObject = {}) => ({ + adhoc_filters: [ + { + clause: 'WHERE' as const, + expressionType: 'SIMPLE' as const, + operator: 'IN' as const, + subject: 'gender', + comparator: ['boys'], + filterOptionName: '123', + }, + ], + applied_time_extras: {}, + color_scheme: 'supersetColors', + datasource: '2__table', + granularity_sqla: 'ds', + groupby: ['gender'], + metric: { + aggregate: 'SUM', + column: { + column_name: 'num', + type: 'BIGINT', + }, + expressionType: 'SIMPLE', + label: 'Births', + }, + slice_id: 46, + time_range: '100 years ago : now', + viz_type: 'pie', + ...overrides, +}); + +const getDashboardFormData = (overrides: JsonObject = {}) => ({ + label_colors: { + Girls: '#FF69B4', + Boys: '#ADD8E6', + girl: '#FF69B4', + boy: '#ADD8E6', + }, + shared_label_colors: { + boy: '#ADD8E6', + girl: '#FF69B4', + }, + color_scheme: 'd3Category20b', + extra_filters: [ + { + col: '__time_range', + op: '==', + val: 'No filter', + }, + { + col: '__time_grain', + op: '==', + val: 'P1D', + }, + { + col: '__time_col', + op: '==', + val: 'ds', + }, + ], + extra_form_data: { + filters: [ + { + col: 'name', + op: 'IN', + val: ['Aaron'], + }, + { + col: 'num_boys', + op: '<=', + val: 10000, + }, + ], + granularity_sqla: 'ds', + time_range: 'Last month', + time_grain_sqla: 'PT1S', + }, + dashboardId: 2, + ...overrides, +}); + +const getExpectedResultFormData = (overrides: JsonObject = {}) => ({ + adhoc_filters: [ + { + clause: 'WHERE', + expressionType: 'SIMPLE', + operator: 'IN', + subject: 'gender', + comparator: ['boys'], + filterOptionName: '123', + }, + { + clause: 'WHERE', + expressionType: 'SIMPLE', + operator: 'IN', + subject: 'name', + comparator: ['Aaron'], + isExtra: true, + filterOptionName: expect.any(String), + }, + { + clause: 'WHERE', + expressionType: 'SIMPLE', + operator: '<=', + subject: 'num_boys', + comparator: 10000, + isExtra: true, + filterOptionName: expect.any(String), + }, + ], + applied_time_extras: { + __time_grain: 'P1D', + __time_col: 'ds', + }, + color_scheme: 'd3Category20b', + datasource: '2__table', + granularity_sqla: 'ds', + groupby: ['gender'], + metric: { + aggregate: 'SUM', + column: { + column_name: 'num', + type: 'BIGINT', + }, + expressionType: 'SIMPLE', + label: 'Births', + }, + slice_id: 46, + time_range: 'Last month', + viz_type: 'pie', + label_colors: { + Girls: '#FF69B4', + Boys: '#ADD8E6', + girl: '#FF69B4', + boy: '#ADD8E6', + }, + shared_label_colors: { + boy: '#ADD8E6', + girl: '#FF69B4', + }, + extra_filters: [ + { + col: '__time_range', + op: '==', + val: 'No filter', + }, + { + col: '__time_grain', + op: '==', + val: 'P1D', + }, + { + col: '__time_col', + op: '==', + val: 'ds', + }, + ], + extra_form_data: { + filters: [ + { + col: 'name', + op: 'IN', + val: ['Aaron'], + }, + { + col: 'num_boys', + op: '<=', + val: 10000, + }, + ], + granularity_sqla: 'ds', + time_range: 'Last month', + time_grain_sqla: 'PT1S', + }, + dashboardId: 2, + time_grain_sqla: 'PT1S', + granularity: 'ds', + extras: { + time_grain_sqla: 'PT1S', + }, + ...overrides, +}); + +it('merges dashboard context form data with explore form data', () => { + const fullFormData = getFormDataWithDashboardContext( + getExploreFormData(), + getDashboardFormData(), + ); + expect(fullFormData).toEqual(getExpectedResultFormData()); +}); diff --git a/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts b/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts new file mode 100644 index 000000000..f3b604ba8 --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts @@ -0,0 +1,191 @@ +/** + * 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 isEqual from 'lodash/isEqual'; +import { + EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS, + EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS, + isDefined, + JsonObject, + ensureIsArray, + QueryObjectFilterClause, + SimpleAdhocFilter, + QueryFormData, +} from '@superset-ui/core'; +import { NO_TIME_RANGE } from '../constants'; + +const simpleFilterToAdhoc = ( + filterClause: QueryObjectFilterClause, + clause = 'where', +) => { + const result = { + clause: clause.toUpperCase(), + expressionType: 'SIMPLE', + operator: filterClause.op, + subject: filterClause.col, + comparator: 'val' in filterClause ? filterClause.val : undefined, + } as SimpleAdhocFilter; + if (filterClause.isExtra) { + Object.assign(result, { + isExtra: true, + filterOptionName: `filter_${Math.random() + .toString(36) + .substring(2, 15)}_${Math.random().toString(36).substring(2, 15)}`, + }); + } + return result; +}; + +const removeAdhocFilterDuplicates = (filters: SimpleAdhocFilter[]) => { + const isDuplicate = ( + adhocFilter: SimpleAdhocFilter, + existingFilters: SimpleAdhocFilter[], + ) => + existingFilters.some( + (existingFilter: SimpleAdhocFilter) => + existingFilter.operator === adhocFilter.operator && + existingFilter.subject === adhocFilter.subject && + ((!('comparator' in existingFilter) && + !('comparator' in adhocFilter)) || + ('comparator' in existingFilter && + 'comparator' in adhocFilter && + isEqual(existingFilter.comparator, adhocFilter.comparator))), + ); + + return filters.reduce((acc, filter) => { + if (!isDuplicate(filter, acc)) { + acc.push(filter); + } + return acc; + }, [] as SimpleAdhocFilter[]); +}; + +const mergeFilterBoxToFormData = ( + exploreFormData: QueryFormData, + dashboardFormData: JsonObject, +) => { + const dateColumns = { + __time_range: 'time_range', + __time_col: 'granularity_sqla', + __time_grain: 'time_grain_sqla', + __granularity: 'granularity', + }; + const appliedTimeExtras = {}; + + const filterBoxData: JsonObject = {}; + ensureIsArray(dashboardFormData.extra_filters).forEach(filter => { + if (dateColumns[filter.col]) { + if (filter.val !== NO_TIME_RANGE) { + filterBoxData[dateColumns[filter.col]] = filter.val; + appliedTimeExtras[filter.col] = filter.val; + } + } else { + const adhocFilter = simpleFilterToAdhoc({ + ...(filter as QueryObjectFilterClause), + isExtra: true, + }); + filterBoxData.adhoc_filters = [ + ...ensureIsArray(filterBoxData.adhoc_filters), + adhocFilter, + ]; + } + }); + filterBoxData.applied_time_extras = appliedTimeExtras; + return filterBoxData; +}; + +const mergeNativeFiltersToFormData = ( + exploreFormData: QueryFormData, + dashboardFormData: JsonObject, +) => { + const nativeFiltersData: JsonObject = {}; + const extraFormData = dashboardFormData.extra_form_data || {}; + + Object.entries(EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS).forEach( + ([srcKey, targetKey]) => { + const val = extraFormData[srcKey]; + if (isDefined(val)) { + nativeFiltersData[targetKey] = val; + } + }, + ); + + if ('time_grain_sqla' in extraFormData) { + nativeFiltersData.time_grain_sqla = extraFormData.time_grain_sqla; + } + if ('granularity_sqla' in extraFormData) { + nativeFiltersData.granularity_sqla = extraFormData.granularity_sqla; + } + + const extras = dashboardFormData.extras || {}; + EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS.forEach(key => { + const val = extraFormData[key]; + if (isDefined(val)) { + extras[key] = val; + } + }); + if (Object.keys(extras).length) { + nativeFiltersData.extras = extras; + } + + nativeFiltersData.adhoc_filters = ensureIsArray( + extraFormData.adhoc_filters, + ).map(filter => ({ + ...filter, + isExtra: true, + })); + + const appendFilters = ensureIsArray(extraFormData.filters).map(extraFilter => + simpleFilterToAdhoc({ ...extraFilter, isExtra: true }), + ); + Object.keys(exploreFormData).forEach(key => { + if (key.match(/adhoc_filter.*/)) { + nativeFiltersData[key] = [ + ...ensureIsArray(nativeFiltersData[key]), + ...appendFilters, + ]; + } + }); + return nativeFiltersData; +}; + +export const getFormDataWithDashboardContext = ( + exploreFormData: QueryFormData, + dashboardContextFormData: JsonObject, +) => { + const filterBoxData = mergeFilterBoxToFormData( + exploreFormData, + dashboardContextFormData, + ); + const nativeFiltersData = mergeNativeFiltersToFormData( + exploreFormData, + dashboardContextFormData, + ); + const adhocFilters = removeAdhocFilterDuplicates([ + ...ensureIsArray(exploreFormData.adhoc_filters), + ...ensureIsArray(filterBoxData.adhoc_filters), + ...ensureIsArray(nativeFiltersData.adhoc_filters), + ]); + return { + ...exploreFormData, + ...dashboardContextFormData, + ...filterBoxData, + ...nativeFiltersData, + adhoc_filters: adhocFilters, + }; +}; diff --git a/superset-frontend/src/types/DashboardContextForExplore.ts b/superset-frontend/src/types/DashboardContextForExplore.ts new file mode 100644 index 000000000..b69ec3ca7 --- /dev/null +++ b/superset-frontend/src/types/DashboardContextForExplore.ts @@ -0,0 +1,43 @@ +/** + * 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 { + DataMaskStateWithId, + DataRecordValue, + PartialFilters, +} from '@superset-ui/core'; +import { ChartConfiguration } from 'src/dashboard/reducers/types'; + +export interface DashboardContextForExplore { + labelColors: Record; + sharedLabelColors: Record; + colorScheme: string; + chartConfiguration: ChartConfiguration; + nativeFilters: PartialFilters; + dataMask: DataMaskStateWithId; + dashboardId: number; + filterBoxFilters: + | { + [key: string]: { + scope: number[]; + values: DataRecordValue[]; + }; + } + | {}; + isRedundant?: boolean; +} diff --git a/superset-frontend/src/utils/localStorageHelpers.ts b/superset-frontend/src/utils/localStorageHelpers.ts index 686a9a249..dfb0d7337 100644 --- a/superset-frontend/src/utils/localStorageHelpers.ts +++ b/superset-frontend/src/utils/localStorageHelpers.ts @@ -19,6 +19,7 @@ import { TableTabTypes } from 'src/views/CRUD/types'; import { SetTabType } from 'src/views/CRUD/welcome/ActivityTable'; +import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore'; export enum LocalStorageKeys { /** @@ -52,6 +53,7 @@ export enum LocalStorageKeys { sqllab__is_autocomplete_enabled = 'sqllab__is_autocomplete_enabled', explore__data_table_original_formatted_time_columns = 'explore__data_table_original_formatted_time_columns', dashboard__custom_filter_bar_widths = 'dashboard__custom_filter_bar_widths', + dashboard__explore_context = 'dashboard__explore_context', } export type LocalStorageValues = { @@ -68,6 +70,7 @@ export type LocalStorageValues = { sqllab__is_autocomplete_enabled: boolean; explore__data_table_original_formatted_time_columns: Record; dashboard__custom_filter_bar_widths: Record; + dashboard__explore_context: Record; }; /*