diff --git a/superset-frontend/cypress-base/cypress/integration/explore/utils.ts b/superset-frontend/cypress-base/cypress/integration/explore/utils.ts index 04cf1f1819986..d5040f867b144 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/utils.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/utils.ts @@ -44,7 +44,10 @@ export function interceptExploreJson() { } export function interceptExploreGet() { - cy.intercept('GET', `/api/v1/explore/?slice_id=**`).as('getExplore'); + cy.intercept({ + method: 'GET', + url: /api\/v1\/explore\/\?(form_data_key|dashboard_page_id|slice_id)=.*/, + }).as('getExplore'); } export function setFilter(filter: string, option: string) { diff --git a/superset-frontend/spec/fixtures/mockDashboardFormData.ts b/superset-frontend/spec/fixtures/mockDashboardFormData.ts new file mode 100644 index 0000000000000..a1adb18a7e722 --- /dev/null +++ b/superset-frontend/spec/fixtures/mockDashboardFormData.ts @@ -0,0 +1,79 @@ +/** + * 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. + */ +/* eslint-disable theme-colors/no-literal-colors */ +import { JsonObject } from '@superset-ui/core'; + +export 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, + }, + { + col: { + sqlExpression: 'totally viable sql expression', + expressionType: 'SQL', + label: 'My column', + }, + op: 'IN', + val: ['Value1', 'Value2'], + }, + ], + granularity_sqla: 'ds', + time_range: 'Last month', + time_grain_sqla: 'PT1S', + }, + dashboardId: 2, + ...overrides, +}); diff --git a/superset-frontend/spec/fixtures/mockExploreFormData.ts b/superset-frontend/spec/fixtures/mockExploreFormData.ts new file mode 100644 index 0000000000000..2ab7303597c50 --- /dev/null +++ b/superset-frontend/spec/fixtures/mockExploreFormData.ts @@ -0,0 +1,87 @@ +/** + * 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'; + +export const getExploreFormData = (overrides: JsonObject = {}) => ({ + adhoc_filters: [ + { + clause: 'WHERE' as const, + expressionType: 'SIMPLE' as const, + operator: 'IN' as const, + subject: 'gender', + comparator: ['boys'], + filterOptionName: '123', + }, + { + clause: 'WHERE' as const, + expressionType: 'SQL' as const, + operator: null, + subject: null, + comparator: null, + sqlExpression: "name = 'John'", + filterOptionName: '456', + }, + { + clause: 'WHERE' as const, + expressionType: 'SQL' as const, + operator: null, + subject: null, + comparator: null, + sqlExpression: "city = 'Warsaw'", + filterOptionName: '567', + }, + { + clause: 'WHERE' as const, + expressionType: 'SIMPLE' as const, + operator: 'TEMPORAL_RANGE' as const, + subject: 'ds', + comparator: 'No filter', + filterOptionName: '678', + }, + ], + adhoc_filters_b: [ + { + clause: 'WHERE' as const, + expressionType: 'SQL' as const, + operator: null, + subject: null, + comparator: null, + sqlExpression: "country = 'Poland'", + filterOptionName: '789', + }, + ], + 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, +}); diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 849baa1417b01..6b4061ed76610 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -266,7 +266,9 @@ class SaveModal extends React.Component { const searchParams = new URLSearchParams(window.location.search); searchParams.set('save_action', this.state.action); - searchParams.delete('form_data_key'); + if (this.state.action !== 'overwrite') { + searchParams.delete('form_data_key'); + } if (this.state.action === 'saveas') { searchParams.set('slice_id', value.id.toString()); } diff --git a/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts b/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts index ca10ae1193c0b..fd120355c8eee 100644 --- a/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts +++ b/superset-frontend/src/explore/controlUtils/getFormDataFromDashboardContext.test.ts @@ -18,135 +18,10 @@ */ import { JsonObject } from '@superset-ui/core'; +import { getExploreFormData } from 'spec/fixtures/mockExploreFormData'; +import { getDashboardFormData } from 'spec/fixtures/mockDashboardFormData'; 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', - }, - { - clause: 'WHERE' as const, - expressionType: 'SQL' as const, - operator: null, - subject: null, - comparator: null, - sqlExpression: "name = 'John'", - filterOptionName: '456', - }, - { - clause: 'WHERE' as const, - expressionType: 'SQL' as const, - operator: null, - subject: null, - comparator: null, - sqlExpression: "city = 'Warsaw'", - filterOptionName: '567', - }, - { - clause: 'WHERE' as const, - expressionType: 'SIMPLE' as const, - operator: 'TEMPORAL_RANGE' as const, - subject: 'ds', - comparator: 'No filter', - filterOptionName: '678', - }, - ], - adhoc_filters_b: [ - { - clause: 'WHERE' as const, - expressionType: 'SQL' as const, - operator: null, - subject: null, - comparator: null, - sqlExpression: "country = 'Poland'", - filterOptionName: '789', - }, - ], - 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, - }, - { - col: { - sqlExpression: 'totally viable sql expression', - expressionType: 'SQL', - label: 'My column', - }, - op: 'IN', - val: ['Value1', 'Value2'], - }, - ], - granularity_sqla: 'ds', - time_range: 'Last month', - time_grain_sqla: 'PT1S', - }, - dashboardId: 2, - ...overrides, -}); - const getExpectedResultFormData = (overrides: JsonObject = {}) => ({ adhoc_filters: [ { diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx new file mode 100644 index 0000000000000..fb4f9a8891698 --- /dev/null +++ b/superset-frontend/src/pages/Chart/Chart.test.tsx @@ -0,0 +1,189 @@ +/** + * 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 React from 'react'; +import fetchMock from 'fetch-mock'; +import { Link } from 'react-router-dom'; +import { + render, + waitFor, + screen, + fireEvent, +} from 'spec/helpers/testing-library'; +import { getExploreFormData } from 'spec/fixtures/mockExploreFormData'; +import { getDashboardFormData } from 'spec/fixtures/mockDashboardFormData'; +import { LocalStorageKeys } from 'src/utils/localStorageHelpers'; +import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; +import { URL_PARAMS } from 'src/constants'; +import { JsonObject } from '@superset-ui/core'; + +import ChartPage from '.'; + +jest.mock('re-resizable', () => ({ + Resizable: () =>
, +})); +jest.mock( + 'src/explore/components/ExploreChartPanel', + () => + ({ exploreState }: { exploreState: JsonObject }) => + ( +
+ {JSON.stringify(exploreState)} +
+ ), +); +jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters'); + +describe('ChartPage', () => { + afterEach(() => { + fetchMock.reset(); + }); + + test('fetches metadata on mount', async () => { + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + const exploreFormData = getExploreFormData({ + viz_type: 'table', + show_cell_bars: true, + }); + fetchMock.get(exploreApiRoute, { + result: { dataset: { id: 1 }, form_data: exploreFormData }, + }); + const { getByTestId } = render(, { + useRouter: true, + useRedux: true, + }); + await waitFor(() => + expect(fetchMock.calls(exploreApiRoute).length).toBe(1), + ); + expect(getByTestId('mock-explore-chart-panel')).toBeInTheDocument(); + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ show_cell_bars: true }).slice(1, -1), + ); + }); + + describe('with dashboardContextFormData', () => { + const dashboardPageId = 'mockPageId'; + + beforeEach(() => { + localStorage.setItem( + LocalStorageKeys.dashboard__explore_context, + JSON.stringify({ + [dashboardPageId]: {}, + }), + ); + }); + + afterEach(() => { + localStorage.clear(); + (getFormDataWithExtraFilters as jest.Mock).mockClear(); + }); + + test('overrides the form_data with dashboardContextFormData', async () => { + const dashboardFormData = getDashboardFormData(); + (getFormDataWithExtraFilters as jest.Mock).mockReturnValue( + dashboardFormData, + ); + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + const exploreFormData = getExploreFormData(); + fetchMock.get(exploreApiRoute, { + result: { dataset: { id: 1 }, form_data: exploreFormData }, + }); + window.history.pushState( + {}, + '', + `/?${URL_PARAMS.dashboardPageId.name}=${dashboardPageId}`, + ); + const { getByTestId } = render(, { + useRouter: true, + useRedux: true, + }); + await waitFor(() => + expect(fetchMock.calls(exploreApiRoute).length).toBe(1), + ); + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ color_scheme: dashboardFormData.color_scheme }).slice( + 1, + -1, + ), + ); + }); + + test('overrides the form_data with exploreFormData when location is updated', async () => { + const dashboardFormData = { + ...getDashboardFormData(), + viz_type: 'table', + show_cell_bars: true, + }; + (getFormDataWithExtraFilters as jest.Mock).mockReturnValue( + dashboardFormData, + ); + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + const exploreFormData = getExploreFormData({ + viz_type: 'table', + show_cell_bars: true, + }); + fetchMock.get(exploreApiRoute, { + result: { dataset: { id: 1 }, form_data: exploreFormData }, + }); + window.history.pushState( + {}, + '', + `/?${URL_PARAMS.dashboardPageId.name}=${dashboardPageId}`, + ); + const { getByTestId } = render( + <> + + Change route + + + , + { + useRouter: true, + useRedux: true, + }, + ); + await waitFor(() => + expect(fetchMock.calls(exploreApiRoute).length).toBe(1), + ); + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ + show_cell_bars: dashboardFormData.show_cell_bars, + }).slice(1, -1), + ); + const updatedExploreFormData = { + ...exploreFormData, + show_cell_bars: false, + }; + fetchMock.reset(); + fetchMock.get(exploreApiRoute, { + result: { dataset: { id: 1 }, form_data: updatedExploreFormData }, + }); + fireEvent.click(screen.getByText('Change route')); + await waitFor(() => + expect(fetchMock.calls(exploreApiRoute).length).toBe(1), + ); + expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent( + JSON.stringify({ + show_cell_bars: updatedExploreFormData.show_cell_bars, + }).slice(1, -1), + ); + }); + }); +}); diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index fae85be59fa24..167de09acc30f 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -129,12 +129,13 @@ export default function ExplorePage() { if (!isExploreInitialized.current || !!saveAction) { fetchExploreData(exploreUrlParams) .then(({ result }) => { - const formData = dashboardContextFormData - ? getFormDataWithDashboardContext( - result.form_data, - dashboardContextFormData, - ) - : result.form_data; + const formData = + !isExploreInitialized.current && dashboardContextFormData + ? getFormDataWithDashboardContext( + result.form_data, + dashboardContextFormData, + ) + : result.form_data; dispatch( hydrateExplore({ ...result,