From cfd50eb7cd022dcb7e136e9b515fef7eca1f3ca5 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 21 Dec 2021 09:08:48 -0800 Subject: [PATCH] fix: replace datamask with key from new key value api (#17680) * afirst stage to ccheck to get initial datamask * clean up code and update typescript * remove consoles * fix ts and update copy dashboard url * use key when one doesn't exists * lint clean up * fix errors * add suggested changes * remove line * add tests and add changes for copydashboard * fix lint * fix lint * fix lint * Update superset-frontend/src/dashboard/components/Header/index.jsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * add timeout * fix test * fix test, add qs to cypress and add suggestions * add suggestions * fix lint * more suggested changes for backwards comapat * fix lint * cleanup naming and add qs parse to tests * Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * more changes and fix lint * remove nativefiler param * fix path * remove con * simplify logic Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- .../integration/dashboard/key_value.test.ts | 54 +++++++++++++++++++ .../dashboard/nativeFilters.test.ts | 19 ++++++- .../cypress-base/package-lock.json | 24 +++++++++ superset-frontend/cypress-base/package.json | 2 + .../dashboard/util/getDashboardUrl_spec.js | 26 ++++----- .../src/common/hooks/useUrlShortener.ts | 4 +- superset-frontend/src/constants.ts | 4 ++ .../src/dashboard/actions/hydrate.js | 6 ++- .../DashboardBuilder/DashboardBuilder.tsx | 2 +- .../components/DashboardBuilder/utils.ts | 2 +- .../Header/HeaderActionsDropdown/index.jsx | 3 +- .../src/dashboard/components/Header/index.jsx | 30 ++++++----- .../ShareMenuItems/ShareMenuItems.test.tsx | 4 +- .../components/menu/ShareMenuItems/index.tsx | 30 ++++++++++- .../nativeFilters/FilterBar/index.tsx | 33 ++++++++---- .../nativeFilters/FilterBar/keyValue.tsx | 54 +++++++++++++++++++ .../nativeFilters/FilterBar/state.ts | 1 - .../nativeFilters/FilterBar/utils.ts | 1 - .../src/dashboard/containers/Dashboard.ts | 4 +- .../dashboard/containers/DashboardPage.tsx | 39 +++++++++++--- .../src/dashboard/util/getDashboardUrl.ts | 14 ++--- superset-frontend/src/dataMask/actions.ts | 6 +++ superset-frontend/src/dataMask/reducer.ts | 13 +++-- superset-frontend/src/utils/urlUtils.ts | 5 +- 24 files changed, 303 insertions(+), 77 deletions(-) create mode 100644 superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts new file mode 100644 index 0000000000000..6bd34373f2acf --- /dev/null +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -0,0 +1,54 @@ +/** + * 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 qs from 'querystringify'; +import { + WORLD_HEALTH_DASHBOARD, + WORLD_HEALTH_CHARTS, + waitForChartLoad, +} from './dashboard.helper'; + +interface QueryString { + native_filters_key: string; +} + +describe('nativefiler url param key', () => { + // const urlParams = { param1: '123', param2: 'abc' }; + before(() => { + cy.login(); + cy.visit(WORLD_HEALTH_DASHBOARD); + WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); + }); + beforeEach(() => { + cy.login(); + }); + let initialFilterKey: string; + it('should have cachekey in nativefilter param', () => { + cy.location().then(loc => { + const queryParams = qs.parse(loc.search) as QueryString; + expect(typeof queryParams.native_filters_key).eq('string'); + }); + }); + + it('should have different key when page reloads', () => { + cy.location().then(loc => { + const queryParams = qs.parse(loc.search) as QueryString; + expect(queryParams.native_filters_key).not.equal(initialFilterKey); + }); + }); +}); diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts index e241c386d03b2..da893f4c528f7 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import qs from 'querystring'; import { dashboardView, nativeFilters } from 'cypress/support/directories'; import { testItems } from './dashboard.helper'; import { DASHBOARD_LIST } from '../dashboard_list/dashboard_list.helper'; @@ -93,6 +94,15 @@ describe('Nativefilters Sanity test', () => { cy.get(nativeFilters.modal.container).should('be.visible'); }); it('User can add a new native filter', () => { + let filterKey: string; + const removeFirstChar = (search: string) => + search.split('').slice(1, search.length).join(''); + cy.wait(3000); + cy.location().then(loc => { + const queryParams = qs.parse(removeFirstChar(loc.search)); + filterKey = queryParams.native_filters_key as string; + expect(typeof filterKey).eq('string'); + }); cy.get(nativeFilters.filterFromDashboardView.expand).click({ force: true }); cy.get(nativeFilters.createFilterButton).should('be.visible').click(); cy.get(nativeFilters.modal.container) @@ -115,7 +125,7 @@ describe('Nativefilters Sanity test', () => { cy.wait(5000); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() - .should('be.visible') + .should('be.visible', { timeout: 30000 }) .click({ force: true }); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() @@ -128,6 +138,13 @@ describe('Nativefilters Sanity test', () => { .contains('Save') .should('be.visible') .click(); + cy.wait(3000); + cy.location().then(loc => { + const queryParams = qs.parse(removeFirstChar(loc.search)); + const newfilterKey = queryParams.native_filters_key; + expect(newfilterKey).not.eq(filterKey); + }); + cy.wait(3000); cy.get(nativeFilters.modal.container).should('not.exist'); }); it('User can delete a native filter', () => { diff --git a/superset-frontend/cypress-base/package-lock.json b/superset-frontend/cypress-base/package-lock.json index cd457f61eb224..8b040d1527a55 100644 --- a/superset-frontend/cypress-base/package-lock.json +++ b/superset-frontend/cypress-base/package-lock.json @@ -11,11 +11,13 @@ "dependencies": { "@cypress/code-coverage": "^3.9.11", "@superset-ui/core": "^0.18.8", + "querystringify": "^2.2.0", "react-dom": "^16.13.0", "rison": "^0.1.1", "shortid": "^2.2.15" }, "devDependencies": { + "@types/querystringify": "^2.0.0", "cypress": "^7.0.0", "eslint-plugin-cypress": "^2.12.1" } @@ -1413,6 +1415,12 @@ "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.3.tgz", "integrity": "sha512-KfRL3PuHmqQLOG+2tGpRO26Ctg+Cq1E01D2DMriKEATHgWLfeNDmq9e29Q9WIky0dQ3NPkd1mzYH8Lm936Z9qw==" }, + "node_modules/@types/querystringify": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@types/querystringify/-/querystringify-2.0.0.tgz", + "integrity": "sha512-9WgEGTevECrXJC2LSWPqiPYWq8BRmeaOyZn47js/3V6UF0PWtcVfvvR43YjeO8BzBsthTz98jMczujOwTw+WYg==", + "dev": true + }, "node_modules/@types/react": { "version": "17.0.3", "resolved": "https://registry.npmjs.org/@types/react/-/react-17.0.3.tgz", @@ -6682,6 +6690,11 @@ "node": ">=0.4.x" } }, + "node_modules/querystringify": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/querystringify/-/querystringify-2.2.0.tgz", + "integrity": "sha512-FIqgj2EUvTa7R50u0rGsyTftzjYmv/a3hO345bZNrqabNqjtgiDMgmo4mkUjd+nzU5oF3dClKqFIPUKybUyqoQ==" + }, "node_modules/queue-microtask": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz", @@ -9741,6 +9754,12 @@ "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.3.tgz", "integrity": "sha512-KfRL3PuHmqQLOG+2tGpRO26Ctg+Cq1E01D2DMriKEATHgWLfeNDmq9e29Q9WIky0dQ3NPkd1mzYH8Lm936Z9qw==" }, + "@types/querystringify": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@types/querystringify/-/querystringify-2.0.0.tgz", + "integrity": "sha512-9WgEGTevECrXJC2LSWPqiPYWq8BRmeaOyZn47js/3V6UF0PWtcVfvvR43YjeO8BzBsthTz98jMczujOwTw+WYg==", + "dev": true + }, "@types/react": { "version": "17.0.3", "resolved": "https://registry.npmjs.org/@types/react/-/react-17.0.3.tgz", @@ -13992,6 +14011,11 @@ "resolved": "https://registry.npmjs.org/querystring-es3/-/querystring-es3-0.2.1.tgz", "integrity": "sha1-nsYfeQSYdXB9aUFFlv2Qek1xHnM=" }, + "querystringify": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/querystringify/-/querystringify-2.2.0.tgz", + "integrity": "sha512-FIqgj2EUvTa7R50u0rGsyTftzjYmv/a3hO345bZNrqabNqjtgiDMgmo4mkUjd+nzU5oF3dClKqFIPUKybUyqoQ==" + }, "queue-microtask": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz", diff --git a/superset-frontend/cypress-base/package.json b/superset-frontend/cypress-base/package.json index 825e16fa2a230..4208556572b4d 100644 --- a/superset-frontend/cypress-base/package.json +++ b/superset-frontend/cypress-base/package.json @@ -12,11 +12,13 @@ "dependencies": { "@cypress/code-coverage": "^3.9.11", "@superset-ui/core": "^0.18.8", + "querystringify": "^2.2.0", "react-dom": "^16.13.0", "rison": "^0.1.1", "shortid": "^2.2.15" }, "devDependencies": { + "@types/querystringify": "^2.0.0", "cypress": "^7.0.0", "eslint-plugin-cypress": "^2.12.1" }, diff --git a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index 8ceac1ab296eb..53904e1c6bf25 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js @@ -73,25 +73,21 @@ describe('getChartIdsFromLayout', () => { ); }); - it('should encode native filters', () => { + it('should process native filters key', () => { + const windowSpy = jest.spyOn(window, 'window', 'get'); + windowSpy.mockImplementation(() => ({ + location: { + origin: 'https://localhost', + search: + '?preselect_filters=%7B%7D&native_filters_key=024380498jdkjf-2094838', + }, + })); + const urlWithNativeFilters = getDashboardUrl({ pathname: 'path', - dataMask: { - 'NATIVE_FILTER-foo123': { - filterState: { - label: 'custom label', - value: ['a', 'b'], - }, - }, - 'NATIVE_FILTER-bar456': { - filterState: { - value: undefined, - }, - }, - }, }); expect(urlWithNativeFilters).toBe( - 'path?preselect_filters=%7B%7D&native_filters=%28NATIVE_FILTER-bar456%3A%28filterState%3A%28value%3A%21n%29%29%2CNATIVE_FILTER-foo123%3A%28filterState%3A%28label%3A%27custom+label%27%2Cvalue%3A%21%28a%2Cb%29%29%29%29', + 'path?preselect_filters=%7B%7D&native_filters_key=024380498jdkjf-2094838', ); }); }); diff --git a/superset-frontend/src/common/hooks/useUrlShortener.ts b/superset-frontend/src/common/hooks/useUrlShortener.ts index f8d9f81511115..33cb636b4527c 100644 --- a/superset-frontend/src/common/hooks/useUrlShortener.ts +++ b/superset-frontend/src/common/hooks/useUrlShortener.ts @@ -23,9 +23,9 @@ export function useUrlShortener(url: string): Function { const [update, setUpdate] = useState(false); const [shortUrl, setShortUrl] = useState(''); - async function getShortUrl() { + async function getShortUrl(urlOverride?: string) { if (update) { - const newShortUrl = await getShortUrlUtil(url); + const newShortUrl = await getShortUrlUtil(urlOverride || url); setShortUrl(newShortUrl); setUpdate(false); return newShortUrl; diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index ab0fb9da84f7e..a57951af12c32 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -39,6 +39,10 @@ export const URL_PARAMS = { name: 'native_filters', type: 'rison', }, + nativeFiltersKey: { + name: 'native_filters_key', + type: 'string', + }, filterSet: { name: 'filter_set', type: 'string', diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 80a5a47ce00eb..e78f46bbf5a40 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -67,6 +67,7 @@ export const hydrateDashboard = dashboardData, chartData, filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP, + dataMaskApplied, ) => (dispatch, getState) => { const { user, common } = getState(); @@ -378,10 +379,11 @@ export const hydrateDashboard = slice_can_edit: findPermission('can_slice', 'Superset', roles), common: { // legacy, please use state.common instead - flash_messages: common.flash_messages, - conf: common.conf, + flash_messages: common?.flash_messages, + conf: common?.conf, }, }, + dataMask: dataMaskApplied, dashboardFilters, nativeFilters, dashboardState: { diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 7d583d8e1aa9e..818ed9ed6c76d 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -235,7 +235,7 @@ const DashboardBuilder: FC = () => { ); const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID]; - const rootChildId = dashboardRoot.children[0]; + const rootChildId = dashboardRoot?.children[0]; const topLevelTabs = rootChildId !== DASHBOARD_GRID_ID ? dashboardLayout[rootChildId] diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts index f2aa381136b45..50aa989c68610 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts @@ -25,7 +25,7 @@ import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponen export const getRootLevelTabsComponent = (dashboardLayout: DashboardLayout) => { const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID]; - const rootChildId = dashboardRoot.children[0]; + const rootChildId = dashboardRoot?.children[0]; return rootChildId === DASHBOARD_GRID_ID ? dashboardLayout[DASHBOARD_ROOT_ID] : dashboardLayout[rootChildId]; diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index 9ca63842d8352..77dd5211c75b6 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -193,7 +193,6 @@ class HeaderActionsDropdown extends React.PureComponent { dashboardTitle, dashboardId, dashboardInfo, - dataMask, refreshFrequency, shouldPersistRefreshFrequency, editMode, @@ -220,7 +219,6 @@ class HeaderActionsDropdown extends React.PureComponent { const emailBody = t('Check out this dashboard: '); const url = getDashboardUrl({ - dataMask, pathname: window.location.pathname, filters: getActiveFilters(), hash: window.location.hash, @@ -266,6 +264,7 @@ class HeaderActionsDropdown extends React.PureComponent { emailBody={emailBody} addSuccessToast={addSuccessToast} addDangerToast={addDangerToast} + dashboardId={dashboardId} /> )} { const { dashboardInfoChanged, dashboardTitleChanged } = this.props; @@ -529,7 +531,7 @@ class Header extends React.PureComponent { canEdit={userCanEdit} canSave={userCanSaveAs} /> - {user?.userId && ( + {user?.userId && dashboardInfo?.id && ( ({ addDangerToast: jest.fn(), addSuccessToast: jest.fn(), - url: '/superset/dashboard/26/?preselect_filters=%7B%7D', + url: `/superset/dashboard/${DASHBOARD_ID}/?preselect_filters=%7B%7D`, copyMenuItemTitle: 'Copy dashboard URL', emailMenuItemTitle: 'Share dashboard by email', emailSubject: 'Superset dashboard COVID Vaccine Dashboard', emailBody: 'Check out this dashboard: ', + dashboardId: DASHBOARD_ID, }); const { location } = window; diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 2152dc75357df..56e9a60566e35 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -21,6 +21,12 @@ import { useUrlShortener } from 'src/common/hooks/useUrlShortener'; import copyTextToClipboard from 'src/utils/copy'; import { t } from '@superset-ui/core'; import { Menu } from 'src/common/components'; +import { getUrlParam } from 'src/utils/urlUtils'; +import { URL_PARAMS } from 'src/constants'; +import { + createFilterKey, + getFilterValue, +} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; interface ShareMenuItemProps { url: string; @@ -30,6 +36,7 @@ interface ShareMenuItemProps { emailBody: string; addDangerToast: Function; addSuccessToast: Function; + dashboardId?: string; } const ShareMenuItems = (props: ShareMenuItemProps) => { @@ -41,14 +48,32 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { emailBody, addDangerToast, addSuccessToast, + dashboardId, ...rest } = props; const getShortUrl = useUrlShortener(url); + async function getCopyUrl() { + const risonObj = getUrlParam(URL_PARAMS.nativeFilters); + if (typeof risonObj === 'object' || !dashboardId) return null; + const prevData = await getFilterValue( + dashboardId, + getUrlParam(URL_PARAMS.nativeFiltersKey), + ); + const newDataMaskKey = await createFilterKey( + dashboardId, + JSON.stringify(prevData), + ); + const newUrl = new URL(`${window.location.origin}${url}`); + newUrl.searchParams.set(URL_PARAMS.nativeFilters.name, newDataMaskKey); + return `${newUrl.pathname}${newUrl.search}`; + } + async function onCopyLink() { try { - const shortUrl = await getShortUrl(); + const copyUrl = await getCopyUrl(); + const shortUrl = await getShortUrl(copyUrl); await copyTextToClipboard(shortUrl); addSuccessToast(t('Copied to clipboard!')); } catch (error) { @@ -58,7 +83,8 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { async function onShareByEmail() { try { - const shortUrl = await getShortUrl(); + const copyUrl = await getCopyUrl(); + const shortUrl = await getShortUrl(copyUrl); const bodyWithLink = `${emailBody}${shortUrl}`; window.location.href = `mailto:?Subject=${emailSubject}%20&Body=${bodyWithLink}`; } catch (error) { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 2427821d41a7c..4f252e0de3ed1 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -20,13 +20,12 @@ /* eslint-disable no-param-reassign */ import { DataMask, HandlerFunction, styled, t } from '@superset-ui/core'; import React, { useEffect, useState, useCallback, useMemo } from 'react'; -import { useDispatch } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import cx from 'classnames'; import Icons from 'src/components/Icons'; import { Tabs } from 'src/common/components'; import { useHistory } from 'react-router-dom'; import { usePrevious } from 'src/common/hooks/usePrevious'; -import rison from 'rison'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { updateDataMask, clearDataMask } from 'src/dataMask/actions'; import { DataMaskStateWithId, DataMaskWithId } from 'src/dataMask/types'; @@ -40,7 +39,7 @@ import { import Loading from 'src/components/Loading'; import { getInitialDataMask } from 'src/dataMask/reducer'; import { URL_PARAMS } from 'src/constants'; -import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; +import { getUrlParam } from 'src/utils/urlUtils'; import { checkIsApplyDisabled, TabIds } from './utils'; import FilterSets from './FilterSets'; import { @@ -50,6 +49,7 @@ import { useFilterUpdates, useInitialization, } from './state'; +import { createFilterKey, updateFilterKey } from './keyValue'; import EditSection from './FilterSets/EditSection'; import Header from './Header'; import FilterControls from './FilterControls/FilterControls'; @@ -154,12 +154,16 @@ const FilterBar: React.FC = ({ const [dataMaskSelected, setDataMaskSelected] = useImmer(dataMaskApplied); const dispatch = useDispatch(); + const [updateKey, setUpdateKey] = useState(0); const filterSets = useFilterSets(); const filterSetFilterValues = Object.values(filterSets); const [tab, setTab] = useState(TabIds.AllFilters); const filters = useFilters(); const previousFilters = usePrevious(filters); const filterValues = Object.values(filters); + const dashboardId = useSelector( + ({ dashboardInfo }) => dashboardInfo?.id, + ); const handleFilterSelectionChange = useCallback( ( @@ -187,28 +191,36 @@ const FilterBar: React.FC = ({ ); const publishDataMask = useCallback( - (dataMaskSelected: DataMaskStateWithId) => { + async (dataMaskSelected: DataMaskStateWithId) => { const { location } = history; const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - + let dataMaskKey = ''; previousParams.forEach((value, key) => { if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); } }); - newParams.set( - URL_PARAMS.nativeFilters.name, - rison.encode(replaceUndefinedByNull(dataMaskSelected)), - ); + const nativeFiltersCacheKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + const dataMask = JSON.stringify(dataMaskSelected); + if ( + updateKey && + nativeFiltersCacheKey && + (await updateFilterKey(dashboardId, dataMask, nativeFiltersCacheKey)) + ) { + dataMaskKey = nativeFiltersCacheKey; + } else { + dataMaskKey = await createFilterKey(dashboardId, dataMask); + } + newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); history.replace({ search: newParams.toString(), }); }, - [history], + [history, updateKey], ); useEffect(() => { @@ -250,6 +262,7 @@ const FilterBar: React.FC = ({ const handleApply = useCallback(() => { const filterIds = Object.keys(dataMaskSelected); + setUpdateKey(1); filterIds.forEach(filterId => { if (dataMaskSelected[filterId]) { dispatch(updateDataMask(filterId, dataMaskSelected[filterId])); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx new file mode 100644 index 0000000000000..ef8cd68875321 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -0,0 +1,54 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { SupersetClient, logging } from '@superset-ui/core'; + +export const updateFilterKey = (dashId: string, value: string, key: string) => + SupersetClient.put({ + endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`, + jsonPayload: { value }, + }) + .then(r => r.json.message) + .catch(err => { + logging.error(err); + return null; + }); + +export const createFilterKey = (dashId: string | number, value: string) => + SupersetClient.post({ + endpoint: `api/v1/dashboard/${dashId}/filter_state`, + jsonPayload: { value }, + }) + .then(r => r.json.key) + .catch(err => { + logging.error(err); + return null; + }); + +export const getFilterValue = ( + dashId: string | number | undefined, + key: string, +) => + SupersetClient.get({ + endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`, + }) + .then(({ json }) => JSON.parse(json.value)) + .catch(err => { + logging.error(err); + return null; + }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts index 03f1232378b73..8e7022f070b02 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts @@ -88,7 +88,6 @@ export const useFilterUpdates = ( ) => { const filters = useFilters(); const dataMaskApplied = useNativeFiltersDataMask(); - useEffect(() => { // Remove deleted filters from local state Object.keys(dataMaskSelected).forEach(selectedId => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index 2ea9f6ea09ff7..a2bc7caa04792 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -69,7 +69,6 @@ export const checkIsApplyDisabled = ( ) => { const dataSelectedValues = Object.values(dataMaskSelected); const dataAppliedValues = Object.values(dataMaskApplied); - return ( areObjectsEqual( getOnlyExtraFormData(dataMaskSelected), diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index 60db7b4c00aef..f2e6f25dca389 100644 --- a/superset-frontend/src/dashboard/containers/Dashboard.ts +++ b/superset-frontend/src/dashboard/containers/Dashboard.ts @@ -49,8 +49,8 @@ function mapStateToProps(state: RootState) { } = state; return { - initMessages: dashboardInfo.common.flash_messages, - timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, + initMessages: dashboardInfo.common?.flash_messages, + timeout: dashboardInfo.common?.conf?.SUPERSET_WEBSERVER_TIMEOUT, userId: dashboardInfo.userId, dashboardInfo, dashboardState, diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 1f28ecad74e27..b43262eb30326 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -48,6 +48,7 @@ import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { canUserEditDashboard } from 'src/dashboard/util/findPermission'; import { getFilterSets } from '../actions/nativeFilters'; +import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, @@ -155,16 +156,40 @@ const DashboardPage: FC = () => { }, [readyToRender]); useEffect(() => { - if (readyToRender) { - if (!isDashboardHydrated.current) { - isDashboardHydrated.current = true; - if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET)) { - // only initialize filterset once - dispatch(getFilterSets(id)); + // eslint-disable-next-line consistent-return + async function getDataMaskApplied() { + const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey); + let dataMaskFromUrl = nativeFilterKeyValue || {}; + + const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); + // check if key from key_value api and get datamask + if (nativeFilterKeyValue) { + dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue); + } + if (isOldRison) { + dataMaskFromUrl = isOldRison; + } + + if (readyToRender) { + if (!isDashboardHydrated.current) { + isDashboardHydrated.current = true; + if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET)) { + // only initialize filterset once + dispatch(getFilterSets(id)); + } } + dispatch( + hydrateDashboard( + dashboard, + charts, + filterboxMigrationState, + dataMaskFromUrl, + ), + ); } - dispatch(hydrateDashboard(dashboard, charts, filterboxMigrationState)); + return null; } + if (id) getDataMaskApplied(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [readyToRender, filterboxMigrationState]); diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.ts b/superset-frontend/src/dashboard/util/getDashboardUrl.ts index c3cc696c23f35..9c261e721db80 100644 --- a/superset-frontend/src/dashboard/util/getDashboardUrl.ts +++ b/superset-frontend/src/dashboard/util/getDashboardUrl.ts @@ -16,25 +16,21 @@ * specific language governing permissions and limitations * under the License. */ -import rison from 'rison'; import { JsonObject } from '@superset-ui/core'; import { URL_PARAMS } from 'src/constants'; -import replaceUndefinedByNull from './replaceUndefinedByNull'; +import { getUrlParam } from 'src/utils/urlUtils'; import serializeActiveFilterValues from './serializeActiveFilterValues'; -import { DataMaskState } from '../../dataMask/types'; export default function getDashboardUrl({ pathname, filters = {}, hash = '', standalone, - dataMask, }: { pathname: string; filters: JsonObject; hash: string; standalone?: number | null; - dataMask?: DataMaskState; }) { const newSearchParams = new URLSearchParams(); @@ -48,11 +44,11 @@ export default function getDashboardUrl({ if (standalone) { newSearchParams.set(URL_PARAMS.standalone.name, standalone.toString()); } - - if (dataMask) { + const dataMaskKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + if (dataMaskKey) { newSearchParams.set( - URL_PARAMS.nativeFilters.name, - rison.encode(replaceUndefinedByNull(dataMask)), + URL_PARAMS.nativeFiltersKey.name, + dataMaskKey as string, ); } diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts index b2f8c58dd59df..b98f36ebd5f25 100644 --- a/superset-frontend/src/dataMask/actions.ts +++ b/superset-frontend/src/dataMask/actions.ts @@ -34,6 +34,12 @@ export interface UpdateDataMask { dataMask: DataMask; } +export const INIT_DATAMASK = 'INIT_DATAMASK'; +export interface INITDATAMASK { + type: typeof INIT_DATAMASK; + dataMask: DataMask; +} + export const SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE = 'SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE'; diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 792677509c105..d87a5442b58c7 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -24,8 +24,6 @@ import { DataMask, FeatureFlag } from '@superset-ui/core'; import { NATIVE_FILTER_PREFIX } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/utils'; import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate'; import { isFeatureEnabled } from 'src/featureFlags'; -import { getUrlParam } from 'src/utils/urlUtils'; -import { URL_PARAMS } from 'src/constants'; import { DataMaskStateWithId, DataMaskWithId } from './types'; import { AnyDataMaskAction, @@ -63,18 +61,19 @@ export function getInitialDataMask( } as DataMaskWithId; } -function fillNativeFilters( +async function fillNativeFilters( filterConfig: FilterConfiguration, mergedDataMask: DataMaskStateWithId, draftDataMask: DataMaskStateWithId, + initialDataMask?: DataMaskStateWithId, currentFilters?: Filters, ) { - const dataMaskFromUrl = getUrlParam(URL_PARAMS.nativeFilters) || {}; filterConfig.forEach((filter: Filter) => { + const dataMask = initialDataMask || {}; mergedDataMask[filter.id] = { ...getInitialDataMask(filter.id), // take initial data ...filter.defaultDataMask, // if something new came from BE - take it - ...dataMaskFromUrl[filter.id], + ...dataMask[filter.id], }; if ( currentFilters && @@ -131,6 +130,8 @@ const dataMaskReducer = produce( [], cleanState, draft, + // @ts-ignore + action.data.dataMask, ); return cleanState; case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE: @@ -138,6 +139,8 @@ const dataMaskReducer = produce( action.filterConfig ?? [], cleanState, draft, + // @ts-ignore + action.data.dataMask, action.filters, ); return cleanState; diff --git a/superset-frontend/src/utils/urlUtils.ts b/superset-frontend/src/utils/urlUtils.ts index f0d9fa8e6ba6e..00633b0843b9c 100644 --- a/superset-frontend/src/utils/urlUtils.ts +++ b/superset-frontend/src/utils/urlUtils.ts @@ -28,6 +28,9 @@ export function getUrlParam(param: UrlParam & { type: 'number' }): number; export function getUrlParam(param: UrlParam & { type: 'boolean' }): boolean; export function getUrlParam(param: UrlParam & { type: 'object' }): object; export function getUrlParam(param: UrlParam & { type: 'rison' }): object; +export function getUrlParam( + param: UrlParam & { type: 'rison | string' }, +): string | object; export function getUrlParam({ name, type }: UrlParam): unknown { const urlParam = new URLSearchParams(window.location.search).get(name); switch (type) { @@ -62,7 +65,7 @@ export function getUrlParam({ name, type }: UrlParam): unknown { try { return rison.decode(urlParam); } catch { - return null; + return urlParam; } default: return urlParam;