From 565533805ffd843f583d29b910cc0550caff1217 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 7 Dec 2021 12:18:13 -0800 Subject: [PATCH 01/29] afirst stage to ccheck to get initial datamask --- superset-frontend/src/constants.ts | 2 +- .../src/dashboard/actions/hydrate.js | 12 +++-- .../DashboardBuilder/DashboardBuilder.tsx | 2 +- .../components/DashboardBuilder/utils.ts | 2 +- .../src/dashboard/components/Header/index.jsx | 28 +++++----- .../nativeFilters/FilterBar/index.tsx | 41 ++++++++++++--- .../nativeFilters/FilterBar/keyValue.tsx | 51 +++++++++++++++++++ .../nativeFilters/FilterBar/state.ts | 2 +- .../nativeFilters/FilterBar/utils.ts | 9 +++- .../src/dashboard/containers/Dashboard.ts | 4 +- .../dashboard/containers/DashboardPage.tsx | 44 +++++++++++++--- superset-frontend/src/dataMask/reducer.ts | 31 +++++++++-- superset-frontend/src/utils/urlUtils.ts | 15 ++++-- 13 files changed, 199 insertions(+), 44 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index ab0fb9da84f7..ef7bb395fda5 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -37,7 +37,7 @@ export const URL_PARAMS = { }, nativeFilters: { name: 'native_filters', - type: 'rison', + type: 'rison | string', }, filterSet: { name: 'filter_set', diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 80a5a47ce00e..2a057356f07b 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -59,6 +59,7 @@ import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; import { FeatureFlag, isFeatureEnabled } from '../../featureFlags'; import extractUrlParams from '../util/extractUrlParams'; import getNativeFilterConfig from '../util/filterboxMigrationHelper'; +import { css } from 'jquery'; export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD'; @@ -67,10 +68,11 @@ export const hydrateDashboard = dashboardData, chartData, filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP, + dataMaskApplied, ) => (dispatch, getState) => { const { user, common } = getState(); - + console.log('common', common) const { metadata } = dashboardData; const regularUrlParams = extractUrlParams('regular'); const reservedUrlParams = extractUrlParams('reserved'); @@ -347,6 +349,9 @@ export const hydrateDashboard = const { roles } = user; const canEdit = canUserEditDashboard(dashboardData, user); + console.log('<***** dataMaskApplied dispatch *****>'); + console.log('dataMaskApplied dispatch *****>', dataMaskApplied); + console.log('<***** dataMaskApplied End *****>'); return dispatch({ type: HYDRATE_DASHBOARD, data: { @@ -378,10 +383,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 7d583d8e1aa9..818ed9ed6c76 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 f2aa381136b4..50aa989c6861 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/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index f43480d5e6a0..bbe96fa91ac0 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -174,13 +174,15 @@ class Header extends React.PureComponent { this.startPeriodicRender(refreshFrequency * 1000); if (this.canAddReports()) { // this is in case there is an anonymous user. - this.props.fetchUISpecificReport( - user.userId, - 'dashboard_id', - 'dashboards', - dashboardInfo.id, - user.email, - ); + if(Object.entries(dashboardInfo).length !== 0) { + this.props.fetchUISpecificReport( + user.userId, + 'dashboard_id', + 'dashboards', + dashboardInfo.id, + user.email, + ); + } } } @@ -212,11 +214,11 @@ class Header extends React.PureComponent { ) { // this is in case there is an anonymous user. this.props.fetchUISpecificReport( - user.userId, + user?.userId, 'dashboard_id', 'dashboards', - nextProps.dashboardInfo.id, - user.email, + nextProps?.dashboardInfo?.id, + user?.email, ); } } @@ -487,10 +489,10 @@ class Header extends React.PureComponent { filterboxMigrationState !== FILTER_BOX_MIGRATION_STATES.REVIEWING; const shouldShowReport = !editMode && this.canAddReports(); const refreshLimit = - dashboardInfo.common.conf.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; + dashboardInfo.common?.conf?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; const refreshWarning = - dashboardInfo.common.conf - .SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE; + dashboardInfo.common?.conf + ?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE; return ( = ({ const filters = useFilters(); const previousFilters = usePrevious(filters); const filterValues = Object.values(filters); + const dashboardId = useSelector( + ({ dashboardInfo }) => dashboardInfo?.id, + ); const handleFilterSelectionChange = useCallback( ( @@ -186,27 +191,44 @@ const FilterBar: React.FC = ({ [dataMaskSelected, dispatch, setDataMaskSelected, tab], ); + const getFilterKeyForUrl = async (value: string) => { + let key; + try { + key = await createFilterKey(dashboardId, value); + console.log('key', key) + } catch(err) { + console.log('err in key') + console.log(err); + return null; + } + return key; + }; + 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; + console.log('datamas', dataMaskSelected); previousParams.forEach((value, key) => { if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); } }); + + const createKey = JSON.stringify(dataMaskSelected); + //const getFilterKey = await getFilterKeyForUrl(createKey); - newParams.set( + /*newParams.set( URL_PARAMS.nativeFilters.name, - rison.encode(replaceUndefinedByNull(dataMaskSelected)), - ); + getFilterKey // rison.encode(replaceUndefinedByNull(dataMaskSelected)), + );*/ - history.replace({ + /*history.replace({ search: newParams.toString(), - }); + });*/ }, [history], ); @@ -277,6 +299,7 @@ const FilterBar: React.FC = ({ dataMaskApplied, filterValues, ); + console.log('isApplyDisabled', isApplyDisabled) const isInitialized = useInitialization(); const tabPaneStyle = useMemo(() => ({ overflow: 'auto', height }), [height]); @@ -284,6 +307,8 @@ const FilterBar: React.FC = ({ filterValue => filterValue.type === NativeFilterType.NATIVE_FILTER, ).length; + console.log('dataMaskSlected', dataMaskSelected); + // console.log('userFilterUpdates', useFilterUpdates); return ( + SupersetClient.put({ + endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}`, + body: value, + }) + .then(r => { + console.log('RESPONSE --->', r); + }) + .catch(err => err); + +export const createFilterKey = (dashId: string, value: DataMaskStateWithId) => + SupersetClient.post({ + endpoint: `api/v1/dashboard/${dashId}/filter_state`, + jsonPayload: { value }, + }) + .then(r => { + console.log('createfilterkey', r.json.key); + return r.json.key; + }) + .catch(err => console.log('err', err)); + +export const getFilterValue = (dashId: string, key: string) => + SupersetClient.get({ + endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`, + }) + .then(({json}) => JSON.parse(json.value)) + .catch(err => err); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts index 03f1232378b7..81be1e634375 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts @@ -88,7 +88,7 @@ export const useFilterUpdates = ( ) => { const filters = useFilters(); const dataMaskApplied = useNativeFiltersDataMask(); - + console.log('filter updated') 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 2ea9f6ea09ff..ea56406c2a4a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -56,6 +56,9 @@ export const checkIsMissingRequiredValue = ( ) => { const value = filterState?.value; // TODO: this property should be unhardcoded + console.log('checkIsMissingRequired', filter.controlValues?.enableEmptyFilter && + (value === null || value === undefined), + ); return ( filter.controlValues?.enableEmptyFilter && (value === null || value === undefined) @@ -69,7 +72,11 @@ export const checkIsApplyDisabled = ( ) => { const dataSelectedValues = Object.values(dataMaskSelected); const dataAppliedValues = Object.values(dataMaskApplied); - + console.log('dataMaskSelected', dataMaskSelected); + console.log('dataMaskSelectedVAlue', dataSelectedValues.length); + console.log('dataMaskApplied', dataMaskApplied); + console.log('dataMaskAppliedValue', dataAppliedValues.length); + // console.log('filters') return ( areObjectsEqual( getOnlyExtraFormData(dataMaskSelected), diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index 60db7b4c00ae..f2e6f25dca38 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 35968886b845..6ba3ad1fac72 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 { createFilterKey, getFilterValue, } from '../components/nativeFilters/FilterBar/keyValue'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, @@ -153,16 +154,45 @@ 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()); + // eslint-disable-next-line consistent-return + async function getDataMaskApplied() { + // + console.log('DASHBOARD', dashboard); + // const isLongUrl = search.indexOf('NATIVE_FILTER'); + const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters); + // console.log('key in fill native filters', nativeFilterValue) + let dataMaskFromUrl = nativeFilterValue || {}; + // if nativefiltervalue is string mean key + // console.log('nativeFilterValue', typeof nativeFilterValue) + console.log({ nativeFilterValue }); + if (typeof nativeFilterValue === 'string') { + try { + dataMaskFromUrl = await getFilterValue( + dashboard?.id, + nativeFilterValue, + ); + + console.log('-------datamaskfromurl--------'); + console.log('getfiltervalue datamask', dataMaskFromUrl); + console.log('-------datamaskfromurl end--------'); + console.log('GET filterkey datamask', dataMaskFromUrl); + } catch (err) { + console.log(err); + return null; + } + } + if (readyToRender) { + if (!isDashboardHydrated.current) { + isDashboardHydrated.current = true; + if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET)) { + // only initialize filterset once + dispatch(getFilterSets()); + } } + dispatch(hydrateDashboard(dashboard, charts, filterboxMigrationState, dataMaskFromUrl)); } - dispatch(hydrateDashboard(dashboard, charts, filterboxMigrationState)); } + if (dashboard?.id) getDataMaskApplied(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [readyToRender, filterboxMigrationState]); diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 792677509c10..2a907333a2e8 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -26,6 +26,7 @@ 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 { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; import { DataMaskStateWithId, DataMaskWithId } from './types'; import { AnyDataMaskAction, @@ -63,18 +64,37 @@ 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) || {}; + + /* const isLongUrl = search.indexOf('NATIVE_FILTER'); + const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters); + // console.log('key in fill native filters', nativeFilterValue) + let dataMaskFromUrl = nativeFilterValue || {}; + // if nativefiltervalue is string mean key + // console.log('nativeFilterValue', typeof nativeFilterValue) + console.log({nativeFilterValue}); + if (typeof nativeFilterValue === 'string') { + try { + console.log('hello are you working in try catch****') + dataMaskFromUrl = await getFilterValue(dashboardId, nativeFilterValue); + console.log({ dataMaskFromUrl }); + // console.log('GET filterkey datamask', dataMaskFromUrlTest) + } catch (err) { + console.log(err); + } + } + */ filterConfig.forEach((filter: Filter) => { mergedDataMask[filter.id] = { ...getInitialDataMask(filter.id), // take initial data ...filter.defaultDataMask, // if something new came from BE - take it - ...dataMaskFromUrl[filter.id], + ...initialDataMask[filter.id], }; if ( currentFilters && @@ -131,13 +151,18 @@ const dataMaskReducer = produce( [], cleanState, draft, + // @ts-ignore + action.data.dataMask, ); + console.log("hydrating dataMask.....", cleanState); return cleanState; case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE: fillNativeFilters( 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 f0d9fa8e6ba6..74caf00a8f03 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' }, +): object; export function getUrlParam({ name, type }: UrlParam): unknown { const urlParam = new URLSearchParams(window.location.search).get(name); switch (type) { @@ -55,14 +58,20 @@ export function getUrlParam({ name, type }: UrlParam): unknown { return null; } return urlParam !== 'false' && urlParam !== '0'; - case 'rison': + case 'rison | string': if (!urlParam) { return null; } try { - return rison.decode(urlParam); + console.log('urlParam', urlParam); + const parsedRison = rison.decode(urlParam); + console.log('parsedRison', parsedRison, typeof parsedRison); + return parsedRison; + // console.log('trydeconde', tryDecode); + // return tryDecode; } catch { - return null; + console.log('are you returning urlParam for rison | string') + return urlParam; } default: return urlParam; From 94db318d3b9faee273fb0a1c18ea4f96766f0bd5 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 7 Dec 2021 19:09:36 -0800 Subject: [PATCH 02/29] clean up code and update typescript --- .../src/dashboard/actions/hydrate.js | 4 -- .../nativeFilters/FilterBar/index.tsx | 47 ++++++++++--------- .../nativeFilters/FilterBar/keyValue.tsx | 28 +++++------ .../dashboard/containers/DashboardPage.tsx | 24 ++++------ superset-frontend/src/dataMask/actions.ts | 6 +++ superset-frontend/src/dataMask/reducer.ts | 20 -------- 6 files changed, 53 insertions(+), 76 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 2a057356f07b..c30f73dc3bef 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -72,7 +72,6 @@ export const hydrateDashboard = ) => (dispatch, getState) => { const { user, common } = getState(); - console.log('common', common) const { metadata } = dashboardData; const regularUrlParams = extractUrlParams('regular'); const reservedUrlParams = extractUrlParams('reserved'); @@ -349,9 +348,6 @@ export const hydrateDashboard = const { roles } = user; const canEdit = canUserEditDashboard(dashboardData, user); - console.log('<***** dataMaskApplied dispatch *****>'); - console.log('dataMaskApplied dispatch *****>', dataMaskApplied); - console.log('<***** dataMaskApplied End *****>'); return dispatch({ type: HYDRATE_DASHBOARD, data: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index fa8045728e84..6a9a339f3b1e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -40,7 +40,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,11 +50,11 @@ import { useFilterUpdates, useInitialization, } from './state'; -import { createFilterKey } from './keyValue'; +import { createFilterKey, updateFilterKey } from './keyValue'; import EditSection from './FilterSets/EditSection'; import Header from './Header'; import FilterControls from './FilterControls/FilterControls'; -import { getUrlParam } from 'src/utils/urlUtils'; +import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; export const FILTER_BAR_TEST_ID = 'filter-bar'; export const getFilterBarTestId = testWithId(FILTER_BAR_TEST_ID); @@ -195,10 +195,7 @@ const FilterBar: React.FC = ({ let key; try { key = await createFilterKey(dashboardId, value); - console.log('key', key) - } catch(err) { - console.log('err in key') - console.log(err); + } catch (err) { return null; } return key; @@ -210,25 +207,36 @@ const FilterBar: React.FC = ({ const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey; - console.log('datamas', dataMaskSelected); + let dataMaskKey = null; previousParams.forEach((value, key) => { if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); } }); - - const createKey = JSON.stringify(dataMaskSelected); - //const getFilterKey = await getFilterKeyForUrl(createKey); - /*newParams.set( - URL_PARAMS.nativeFilters.name, - getFilterKey // rison.encode(replaceUndefinedByNull(dataMaskSelected)), - );*/ + const getParam = getUrlParam(URL_PARAMS.nativeFilters); + const dataMask = JSON.stringify(dataMaskSelected); - /*history.replace({ + if (typeof getParam === 'object') { + const res = await getFilterKeyForUrl(rison.encode(getParam)); + if (res === null) { + dataMaskKey = rison.encode(replaceUndefinedByNull(dataMaskSelected)); + } + } else if (typeof getParam === 'string') { + try { + const res = await updateFilterKey(dashboardId, dataMask, getParam); + if (res === 'Value updated successfully.') { + dataMaskKey = getParam; + } + // eslint-disable-next-line no-empty + } catch {} + } + + newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); + + history.replace({ search: newParams.toString(), - });*/ + }); }, [history], ); @@ -299,7 +307,6 @@ const FilterBar: React.FC = ({ dataMaskApplied, filterValues, ); - console.log('isApplyDisabled', isApplyDisabled) const isInitialized = useInitialization(); const tabPaneStyle = useMemo(() => ({ overflow: 'auto', height }), [height]); @@ -307,8 +314,6 @@ const FilterBar: React.FC = ({ filterValue => filterValue.type === NativeFilterType.NATIVE_FILTER, ).length; - console.log('dataMaskSlected', dataMaskSelected); - // console.log('userFilterUpdates', useFilterUpdates); return ( +export const updateFilterKey = (dashId: string, value: string, key: string) => SupersetClient.put({ - endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}`, - body: value, + endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`, + jsonPayload: { value }, }) - .then(r => { - console.log('RESPONSE --->', r); - }) + .then(r => r.json.message) .catch(err => err); -export const createFilterKey = (dashId: string, value: DataMaskStateWithId) => +export const createFilterKey = (dashId: string, value: string) => SupersetClient.post({ endpoint: `api/v1/dashboard/${dashId}/filter_state`, jsonPayload: { value }, }) - .then(r => { - console.log('createfilterkey', r.json.key); - return r.json.key; - }) + .then(r => r.json.key) .catch(err => console.log('err', err)); -export const getFilterValue = (dashId: string, key: string) => +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)) + .then(({ json }) => JSON.parse(json.value)) .catch(err => err); diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 6ba3ad1fac72..5ea0b10ea6e1 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -48,7 +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 { createFilterKey, getFilterValue, } from '../components/nativeFilters/FilterBar/keyValue'; +import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, @@ -156,26 +156,15 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { - // - console.log('DASHBOARD', dashboard); - // const isLongUrl = search.indexOf('NATIVE_FILTER'); const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters); - // console.log('key in fill native filters', nativeFilterValue) let dataMaskFromUrl = nativeFilterValue || {}; - // if nativefiltervalue is string mean key - // console.log('nativeFilterValue', typeof nativeFilterValue) - console.log({ nativeFilterValue }); + if (typeof nativeFilterValue === 'string') { try { dataMaskFromUrl = await getFilterValue( dashboard?.id, nativeFilterValue, ); - - console.log('-------datamaskfromurl--------'); - console.log('getfiltervalue datamask', dataMaskFromUrl); - console.log('-------datamaskfromurl end--------'); - console.log('GET filterkey datamask', dataMaskFromUrl); } catch (err) { console.log(err); return null; @@ -189,7 +178,14 @@ const DashboardPage: FC = () => { dispatch(getFilterSets()); } } - dispatch(hydrateDashboard(dashboard, charts, filterboxMigrationState, dataMaskFromUrl)); + dispatch( + hydrateDashboard( + dashboard, + charts, + filterboxMigrationState, + dataMaskFromUrl, + ), + ); } } if (dashboard?.id) getDataMaskApplied(); diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts index b2f8c58dd59d..b98f36ebd5f2 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 2a907333a2e8..6ec3e69c8953 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -71,25 +71,6 @@ async function fillNativeFilters( initialDataMask?: DataMaskStateWithId, currentFilters?: Filters, ) { - - /* const isLongUrl = search.indexOf('NATIVE_FILTER'); - const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters); - // console.log('key in fill native filters', nativeFilterValue) - let dataMaskFromUrl = nativeFilterValue || {}; - // if nativefiltervalue is string mean key - // console.log('nativeFilterValue', typeof nativeFilterValue) - console.log({nativeFilterValue}); - if (typeof nativeFilterValue === 'string') { - try { - console.log('hello are you working in try catch****') - dataMaskFromUrl = await getFilterValue(dashboardId, nativeFilterValue); - console.log({ dataMaskFromUrl }); - // console.log('GET filterkey datamask', dataMaskFromUrlTest) - } catch (err) { - console.log(err); - } - } - */ filterConfig.forEach((filter: Filter) => { mergedDataMask[filter.id] = { ...getInitialDataMask(filter.id), // take initial data @@ -154,7 +135,6 @@ const dataMaskReducer = produce( // @ts-ignore action.data.dataMask, ); - console.log("hydrating dataMask.....", cleanState); return cleanState; case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE: fillNativeFilters( From 38461bb86411c89c423956185eb4b3cd61c7a86f Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 7 Dec 2021 20:45:44 -0800 Subject: [PATCH 03/29] remove consoles --- .../dashboard/components/nativeFilters/FilterBar/utils.ts | 8 -------- superset-frontend/src/utils/urlUtils.ts | 5 ----- 2 files changed, 13 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index ea56406c2a4a..a2bc7caa0479 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -56,9 +56,6 @@ export const checkIsMissingRequiredValue = ( ) => { const value = filterState?.value; // TODO: this property should be unhardcoded - console.log('checkIsMissingRequired', filter.controlValues?.enableEmptyFilter && - (value === null || value === undefined), - ); return ( filter.controlValues?.enableEmptyFilter && (value === null || value === undefined) @@ -72,11 +69,6 @@ export const checkIsApplyDisabled = ( ) => { const dataSelectedValues = Object.values(dataMaskSelected); const dataAppliedValues = Object.values(dataMaskApplied); - console.log('dataMaskSelected', dataMaskSelected); - console.log('dataMaskSelectedVAlue', dataSelectedValues.length); - console.log('dataMaskApplied', dataMaskApplied); - console.log('dataMaskAppliedValue', dataAppliedValues.length); - // console.log('filters') return ( areObjectsEqual( getOnlyExtraFormData(dataMaskSelected), diff --git a/superset-frontend/src/utils/urlUtils.ts b/superset-frontend/src/utils/urlUtils.ts index 74caf00a8f03..7c35f7257086 100644 --- a/superset-frontend/src/utils/urlUtils.ts +++ b/superset-frontend/src/utils/urlUtils.ts @@ -63,14 +63,9 @@ export function getUrlParam({ name, type }: UrlParam): unknown { return null; } try { - console.log('urlParam', urlParam); const parsedRison = rison.decode(urlParam); - console.log('parsedRison', parsedRison, typeof parsedRison); return parsedRison; - // console.log('trydeconde', tryDecode); - // return tryDecode; } catch { - console.log('are you returning urlParam for rison | string') return urlParam; } default: From b08a47267e509b42702419ef8a02d493193525c9 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 7 Dec 2021 22:24:45 -0800 Subject: [PATCH 04/29] fix ts and update copy dashboard url --- .../components/nativeFilters/FilterBar/index.tsx | 2 +- .../src/dashboard/containers/DashboardPage.tsx | 1 - .../src/dashboard/util/getDashboardUrl.ts | 15 ++++----------- superset-frontend/src/dataMask/reducer.ts | 3 ++- superset-frontend/src/utils/urlUtils.ts | 2 +- 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 6a9a339f3b1e..bfdc035304b4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -207,7 +207,7 @@ const FilterBar: React.FC = ({ const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey = null; + let dataMaskKey = ""; previousParams.forEach((value, key) => { if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 5ea0b10ea6e1..d01fd2e8cac3 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -166,7 +166,6 @@ const DashboardPage: FC = () => { nativeFilterValue, ); } catch (err) { - console.log(err); return null; } } diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.ts b/superset-frontend/src/dashboard/util/getDashboardUrl.ts index c3cc696c23f3..bf0d6e7090c7 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,12 +44,9 @@ export default function getDashboardUrl({ if (standalone) { newSearchParams.set(URL_PARAMS.standalone.name, standalone.toString()); } - - if (dataMask) { - newSearchParams.set( - URL_PARAMS.nativeFilters.name, - rison.encode(replaceUndefinedByNull(dataMask)), - ); + const dataMaskKey = getUrlParam(URL_PARAMS.nativeFilters); + if (dataMaskKey) { + newSearchParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey as string); } const hashSection = hash ? `#${hash}` : ''; diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 6ec3e69c8953..a1fc07d54d73 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -72,10 +72,11 @@ async function fillNativeFilters( currentFilters?: Filters, ) { 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 - ...initialDataMask[filter.id], + ...dataMask[filter.id], }; if ( currentFilters && diff --git a/superset-frontend/src/utils/urlUtils.ts b/superset-frontend/src/utils/urlUtils.ts index 7c35f7257086..3c44031a40c4 100644 --- a/superset-frontend/src/utils/urlUtils.ts +++ b/superset-frontend/src/utils/urlUtils.ts @@ -30,7 +30,7 @@ export function getUrlParam(param: UrlParam & { type: 'object' }): object; export function getUrlParam(param: UrlParam & { type: 'rison' }): object; export function getUrlParam( param: UrlParam & { type: 'rison | string' }, -): object; +): string | object; export function getUrlParam({ name, type }: UrlParam): unknown { const urlParam = new URLSearchParams(window.location.search).get(name); switch (type) { From 6420292420be0d815525451932931301ef5859b7 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Wed, 8 Dec 2021 15:09:53 -0800 Subject: [PATCH 05/29] use key when one doesn't exists --- .../dashboard/components/nativeFilters/FilterBar/index.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index bfdc035304b4..8bf71f027347 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -41,6 +41,7 @@ import Loading from 'src/components/Loading'; import { getInitialDataMask } from 'src/dataMask/reducer'; import { URL_PARAMS } from 'src/constants'; import { getUrlParam} from 'src/utils/urlUtils'; +import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; import { checkIsApplyDisabled, TabIds } from './utils'; import FilterSets from './FilterSets'; import { @@ -54,7 +55,6 @@ import { createFilterKey, updateFilterKey } from './keyValue'; import EditSection from './FilterSets/EditSection'; import Header from './Header'; import FilterControls from './FilterControls/FilterControls'; -import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; export const FILTER_BAR_TEST_ID = 'filter-bar'; export const getFilterBarTestId = testWithId(FILTER_BAR_TEST_ID); @@ -207,7 +207,7 @@ const FilterBar: React.FC = ({ const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey = ""; + let dataMaskKey = ''; previousParams.forEach((value, key) => { if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); @@ -221,7 +221,7 @@ const FilterBar: React.FC = ({ const res = await getFilterKeyForUrl(rison.encode(getParam)); if (res === null) { dataMaskKey = rison.encode(replaceUndefinedByNull(dataMaskSelected)); - } + } else dataMaskKey = res; } else if (typeof getParam === 'string') { try { const res = await updateFilterKey(dashboardId, dataMask, getParam); @@ -231,7 +231,6 @@ const FilterBar: React.FC = ({ // eslint-disable-next-line no-empty } catch {} } - newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); history.replace({ From 3ce45d14340bfc672fd40a4491a8ff71ffb98085 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Wed, 8 Dec 2021 17:47:24 -0800 Subject: [PATCH 06/29] lint clean up --- superset-frontend/src/dashboard/actions/hydrate.js | 1 - superset-frontend/src/dashboard/components/Header/index.jsx | 2 +- .../dashboard/components/nativeFilters/FilterBar/index.tsx | 4 ++-- .../src/dashboard/components/nativeFilters/FilterBar/state.ts | 1 - 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index c30f73dc3bef..bd7f10c3650e 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -59,7 +59,6 @@ import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants'; import { FeatureFlag, isFeatureEnabled } from '../../featureFlags'; import extractUrlParams from '../util/extractUrlParams'; import getNativeFilterConfig from '../util/filterboxMigrationHelper'; -import { css } from 'jquery'; export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD'; diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index bbe96fa91ac0..e01ad4d54411 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -174,7 +174,7 @@ class Header extends React.PureComponent { this.startPeriodicRender(refreshFrequency * 1000); if (this.canAddReports()) { // this is in case there is an anonymous user. - if(Object.entries(dashboardInfo).length !== 0) { + if (Object.entries(dashboardInfo).length !== 0) { this.props.fetchUISpecificReport( user.userId, 'dashboard_id', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 8bf71f027347..53c5b110b208 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -40,7 +40,7 @@ import { import Loading from 'src/components/Loading'; import { getInitialDataMask } from 'src/dataMask/reducer'; import { URL_PARAMS } from 'src/constants'; -import { getUrlParam} from 'src/utils/urlUtils'; +import { getUrlParam } from 'src/utils/urlUtils'; import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; import { checkIsApplyDisabled, TabIds } from './utils'; import FilterSets from './FilterSets'; @@ -228,7 +228,7 @@ const FilterBar: React.FC = ({ if (res === 'Value updated successfully.') { dataMaskKey = getParam; } - // eslint-disable-next-line no-empty + // eslint-disable-next-line no-empty } catch {} } newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts index 81be1e634375..8e7022f070b0 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(); - console.log('filter updated') useEffect(() => { // Remove deleted filters from local state Object.keys(dataMaskSelected).forEach(selectedId => { From dfb050fef0d579b759b4be45e38fe310d8ad6508 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 9 Dec 2021 19:07:13 -0800 Subject: [PATCH 07/29] fix errors --- superset-frontend/src/dashboard/containers/DashboardPage.tsx | 1 + superset-frontend/src/dataMask/reducer.ts | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index d01fd2e8cac3..95ed194131d5 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -186,6 +186,7 @@ const DashboardPage: FC = () => { ), ); } + return null; } if (dashboard?.id) getDataMaskApplied(); // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index a1fc07d54d73..d87a5442b58c 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -24,9 +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 { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; import { DataMaskStateWithId, DataMaskWithId } from './types'; import { AnyDataMaskAction, From e7ccfb777a82b39912c3d22cd5acd6c2ce9da510 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Mon, 13 Dec 2021 16:29:19 -0800 Subject: [PATCH 08/29] add suggested changes --- superset-frontend/src/constants.ts | 6 ++- .../nativeFilters/FilterBar/index.tsx | 46 ++++++++----------- .../nativeFilters/FilterBar/keyValue.tsx | 17 +++++-- superset-frontend/src/utils/urlUtils.ts | 5 +- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index ef7bb395fda5..d32b7faaff33 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -37,7 +37,11 @@ export const URL_PARAMS = { }, nativeFilters: { name: 'native_filters', - type: 'rison | string', + type: 'rison', + }, + nativeFiltersByCacheKey: { + name: 'native_filters', + type: 'string', }, filterSet: { name: 'filter_set', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 53c5b110b208..5a5d91b69f24 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -41,7 +41,7 @@ import Loading from 'src/components/Loading'; import { getInitialDataMask } from 'src/dataMask/reducer'; import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; -import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; +// import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull'; import { checkIsApplyDisabled, TabIds } from './utils'; import FilterSets from './FilterSets'; import { @@ -156,6 +156,7 @@ 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); @@ -191,16 +192,6 @@ const FilterBar: React.FC = ({ [dataMaskSelected, dispatch, setDataMaskSelected, tab], ); - const getFilterKeyForUrl = async (value: string) => { - let key; - try { - key = await createFilterKey(dashboardId, value); - } catch (err) { - return null; - } - return key; - }; - const publishDataMask = useCallback( async (dataMaskSelected: DataMaskStateWithId) => { const { location } = history; @@ -214,22 +205,24 @@ const FilterBar: React.FC = ({ } }); - const getParam = getUrlParam(URL_PARAMS.nativeFilters); + const nativeFiltersCacheKey = getUrlParam( + URL_PARAMS.nativeFiltersByCacheKey, + ); const dataMask = JSON.stringify(dataMaskSelected); + if ( + updateKey && + nativeFiltersCacheKey && + (await updateFilterKey(dashboardId, dataMask, nativeFiltersCacheKey)) + ) { + dataMaskKey = nativeFiltersCacheKey; + } else { + let filterType; + const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); + if (typeof isOldRison === 'object') { + filterType = rison.encode(isOldRison); + } else filterType = getUrlParam(URL_PARAMS.nativeFiltersByCacheKey); - if (typeof getParam === 'object') { - const res = await getFilterKeyForUrl(rison.encode(getParam)); - if (res === null) { - dataMaskKey = rison.encode(replaceUndefinedByNull(dataMaskSelected)); - } else dataMaskKey = res; - } else if (typeof getParam === 'string') { - try { - const res = await updateFilterKey(dashboardId, dataMask, getParam); - if (res === 'Value updated successfully.') { - dataMaskKey = getParam; - } - // eslint-disable-next-line no-empty - } catch {} + dataMaskKey = await createFilterKey(dashboardId, filterType); } newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); @@ -237,7 +230,7 @@ const FilterBar: React.FC = ({ search: newParams.toString(), }); }, - [history], + [history, updateKey], ); useEffect(() => { @@ -279,6 +272,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 index 489798d95a9e..5da6cba0b3c7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { SupersetClient } from '@superset-ui/core'; +import { SupersetClient, logging } from '@superset-ui/core'; export const updateFilterKey = (dashId: string, value: string, key: string) => SupersetClient.put({ @@ -24,7 +24,10 @@ export const updateFilterKey = (dashId: string, value: string, key: string) => jsonPayload: { value }, }) .then(r => r.json.message) - .catch(err => err); + .catch(err => { + logging.error(err); + return null; + }); export const createFilterKey = (dashId: string, value: string) => SupersetClient.post({ @@ -32,7 +35,10 @@ export const createFilterKey = (dashId: string, value: string) => jsonPayload: { value }, }) .then(r => r.json.key) - .catch(err => console.log('err', err)); + .catch(err => { + logging.error(err); + return null; + }); export const getFilterValue = ( dashId: string | number | undefined, @@ -42,4 +48,7 @@ export const getFilterValue = ( endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`, }) .then(({ json }) => JSON.parse(json.value)) - .catch(err => err); + .catch(err => { + logging.error(err); + return null; + }); diff --git a/superset-frontend/src/utils/urlUtils.ts b/superset-frontend/src/utils/urlUtils.ts index 3c44031a40c4..00633b0843b9 100644 --- a/superset-frontend/src/utils/urlUtils.ts +++ b/superset-frontend/src/utils/urlUtils.ts @@ -58,13 +58,12 @@ export function getUrlParam({ name, type }: UrlParam): unknown { return null; } return urlParam !== 'false' && urlParam !== '0'; - case 'rison | string': + case 'rison': if (!urlParam) { return null; } try { - const parsedRison = rison.decode(urlParam); - return parsedRison; + return rison.decode(urlParam); } catch { return urlParam; } From e4b383012510fa7168b8c9b9a81b21142790e51f Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Mon, 13 Dec 2021 16:44:42 -0800 Subject: [PATCH 09/29] remove line --- superset-frontend/src/dashboard/actions/hydrate.js | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index bd7f10c3650e..e78f46bbf5a4 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -71,6 +71,7 @@ export const hydrateDashboard = ) => (dispatch, getState) => { const { user, common } = getState(); + const { metadata } = dashboardData; const regularUrlParams = extractUrlParams('regular'); const reservedUrlParams = extractUrlParams('reserved'); From 548a5bb2abd2020668bcaab902bc590e70cf5b0a Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 14 Dec 2021 19:45:39 -0800 Subject: [PATCH 10/29] add tests and add changes for copydashboard --- .../integration/dashboard/key_value.test.ts | 49 +++++++++++++++++++ .../dashboard/nativeFilters.test.ts | 11 +++++ .../dashboard/util/getDashboardUrl_spec.js | 25 ++++------ .../src/common/hooks/useUrlShortener.ts | 4 +- .../Header/HeaderActionsDropdown/index.jsx | 2 +- .../components/menu/ShareMenuItems/index.tsx | 31 +++++++++++- .../nativeFilters/FilterBar/index.tsx | 3 +- .../src/dashboard/util/getDashboardUrl.ts | 2 +- 8 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts 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 000000000000..571bbac45f12 --- /dev/null +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -0,0 +1,49 @@ +/** + * 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 { + WORLD_HEALTH_DASHBOARD, + WORLD_HEALTH_CHARTS, + waitForChartLoad, +} from './dashboard.helper'; + +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 in nativefilter param', () => { + cy.location().then(loc => { + initialFilterKey = loc.search.split('=')[1]; + expect(typeof initialFilterKey).eq('string'); + }); + }); + + it('should have different key when pages reloads', () => { + cy.location().then(loc => { + const newFilterKey = loc.search.split('='); + expect(newFilterKey).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 e241c386d03b..6812b91f3b43 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -93,6 +93,12 @@ describe('Nativefilters Sanity test', () => { cy.get(nativeFilters.modal.container).should('be.visible'); }); it('User can add a new native filter', () => { + let cacheKey: string; + cy.wait(3000); + cy.location().then(loc => { + cacheKey = loc.search.split('=')[1]; + expect(typeof cacheKey).eq('string'); + }); cy.get(nativeFilters.filterFromDashboardView.expand).click({ force: true }); cy.get(nativeFilters.createFilterButton).should('be.visible').click(); cy.get(nativeFilters.modal.container) @@ -129,6 +135,11 @@ describe('Nativefilters Sanity test', () => { .should('be.visible') .click(); cy.get(nativeFilters.modal.container).should('not.exist'); + cy.wait(3000); + cy.location().then(loc => { + const sameCacheKey: string = loc.search.split('=')[1]; + expect(sameCacheKey).eq(cacheKey); + }); }); it('User can delete a native filter', () => { cy.get(nativeFilters.createFilterButton).click({ force: true }); diff --git a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index 8ceac1ab296e..ec947a9f06ca 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js @@ -73,25 +73,20 @@ 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=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=024380498jdkjf-2094838', ); }); }); diff --git a/superset-frontend/src/common/hooks/useUrlShortener.ts b/superset-frontend/src/common/hooks/useUrlShortener.ts index f8d9f8151111..f36ad7a20f9d 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/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index 9ca63842d835..b7b400e4ed7e 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -220,7 +220,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 +265,7 @@ class HeaderActionsDropdown extends React.PureComponent { emailBody={emailBody} addSuccessToast={addSuccessToast} addDangerToast={addDangerToast} + dashboardId={dashboardId} /> )} { @@ -41,24 +48,44 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { emailBody, addDangerToast, addSuccessToast, + dashboardId, ...rest } = props; const getShortUrl = useUrlShortener(url); + async function getCopyUrl() { + const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); + if (typeof isOldRison === 'object') return null; + const getPrevData = await getFilterValue( + dashboardId, + getUrlParam(URL_PARAMS.nativeFiltersByCacheKey), + ); + const newDataMaskKey = await createFilterKey( + dashboardId, + JSON.stringify(getPrevData), + ); + 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) { + console.error(error); addDangerToast(t('Sorry, your browser does not support copying.')); } } 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 5a5d91b69f24..4d197515734a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -220,9 +220,10 @@ const FilterBar: React.FC = ({ const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); if (typeof isOldRison === 'object') { filterType = rison.encode(isOldRison); - } else filterType = getUrlParam(URL_PARAMS.nativeFiltersByCacheKey); + } else filterType = dataMask; dataMaskKey = await createFilterKey(dashboardId, filterType); + console.log('dataMask', dataMaskKey); } newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.ts b/superset-frontend/src/dashboard/util/getDashboardUrl.ts index bf0d6e7090c7..33218eb4cb06 100644 --- a/superset-frontend/src/dashboard/util/getDashboardUrl.ts +++ b/superset-frontend/src/dashboard/util/getDashboardUrl.ts @@ -44,7 +44,7 @@ export default function getDashboardUrl({ if (standalone) { newSearchParams.set(URL_PARAMS.standalone.name, standalone.toString()); } - const dataMaskKey = getUrlParam(URL_PARAMS.nativeFilters); + const dataMaskKey = getUrlParam(URL_PARAMS.nativeFiltersByCacheKey); if (dataMaskKey) { newSearchParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey as string); } From abd4a86fedfc9605e37f04841b07df54e10f9788 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 14 Dec 2021 20:07:55 -0800 Subject: [PATCH 11/29] fix lint --- .../javascripts/dashboard/util/getDashboardUrl_spec.js | 9 +++++---- .../components/Header/HeaderActionsDropdown/index.jsx | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index ec947a9f06ca..48b2e6d00920 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js @@ -74,12 +74,13 @@ describe('getChartIdsFromLayout', () => { }); it('should process native filters key', () => { - const windowSpy = jest.spyOn(window, "window", "get"); + const windowSpy = jest.spyOn(window, 'window', 'get'); windowSpy.mockImplementation(() => ({ location: { - origin: "https://localhost", - search: "?preselect_filters=%7B%7D&native_filters=024380498jdkjf-2094838" - } + origin: 'https://localhost', + search: + '?preselect_filters=%7B%7D&native_filters=024380498jdkjf-2094838', + }, })); const urlWithNativeFilters = getDashboardUrl({ diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index b7b400e4ed7e..77dd5211c75b 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, From cac4bea7ec6b6faa5d8d421cba9338293491263c Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 14 Dec 2021 21:08:31 -0800 Subject: [PATCH 12/29] fix lint --- .../components/menu/ShareMenuItems/ShareMenuItems.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index f4c65ce67dfa..fb264f53d777 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -35,6 +35,7 @@ const createProps = () => ({ emailMenuItemTitle: 'Share dashboard by email', emailSubject: 'Superset dashboard COVID Vaccine Dashboard', emailBody: 'Check out this dashboard: ', + dashboardId: '26', }); const { location } = window; From cc443efad0440a75e37089219c09788d94a59b0e Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 14 Dec 2021 22:49:56 -0800 Subject: [PATCH 13/29] fix lint --- .../src/dashboard/components/menu/ShareMenuItems/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 4051b3eb010c..491e6d757dc6 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -36,7 +36,7 @@ interface ShareMenuItemProps { emailBody: string; addDangerToast: Function; addSuccessToast: Function; - dashboardId: string; + dashboardId?: string; } const ShareMenuItems = (props: ShareMenuItemProps) => { @@ -56,7 +56,7 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { async function getCopyUrl() { const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof isOldRison === 'object') return null; + if (typeof isOldRison === 'object' || !dashboardId) return null; const getPrevData = await getFilterValue( dashboardId, getUrlParam(URL_PARAMS.nativeFiltersByCacheKey), From 0ce2baed608c3d66f5d9ce31746be69b40474a03 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Tue, 14 Dec 2021 22:52:52 -0800 Subject: [PATCH 14/29] Update superset-frontend/src/dashboard/components/Header/index.jsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- superset-frontend/src/dashboard/components/Header/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index e01ad4d54411..73c70959e3b0 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -174,7 +174,7 @@ class Header extends React.PureComponent { this.startPeriodicRender(refreshFrequency * 1000); if (this.canAddReports()) { // this is in case there is an anonymous user. - if (Object.entries(dashboardInfo).length !== 0) { + if (Object.entries(dashboardInfo).length) { this.props.fetchUISpecificReport( user.userId, 'dashboard_id', From cc3dd9c5a2a557e26191d747d114b8273b1fcc63 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Wed, 15 Dec 2021 15:14:00 -0800 Subject: [PATCH 15/29] add timeout --- .../cypress/integration/dashboard/nativeFilters.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6812b91f3b43..8783dd5e389c 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -121,7 +121,7 @@ describe('Nativefilters Sanity test', () => { cy.wait(5000); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() - .should('be.visible') + .should('be.visible', { timeout: 20000 }) .click({ force: true }); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() From d12b370725e7d74fcfbb6c68eb1d6541a81bc4b9 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Wed, 15 Dec 2021 16:19:31 -0800 Subject: [PATCH 16/29] fix test --- .../cypress/integration/dashboard/nativeFilters.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 8783dd5e389c..de420b1f9899 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -121,7 +121,7 @@ describe('Nativefilters Sanity test', () => { cy.wait(5000); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() - .should('be.visible', { timeout: 20000 }) + .should('be.visible', { timeout: 30000 }) .click({ force: true }); cy.get(nativeFilters.filtersPanel.filterInfoInput) .last() @@ -134,12 +134,13 @@ describe('Nativefilters Sanity test', () => { .contains('Save') .should('be.visible') .click(); - cy.get(nativeFilters.modal.container).should('not.exist'); cy.wait(3000); cy.location().then(loc => { const sameCacheKey: string = loc.search.split('=')[1]; - expect(sameCacheKey).eq(cacheKey); + expect(sameCacheKey).not.eq(cacheKey); }); + cy.wait(3000); + cy.get(nativeFilters.modal.container).should('not.exist'); }); it('User can delete a native filter', () => { cy.get(nativeFilters.createFilterButton).click({ force: true }); From 2af1863780e5dc55de36c8f180e0ed44a2ae94ac Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 16 Dec 2021 16:06:27 -0800 Subject: [PATCH 17/29] fix test, add qs to cypress and add suggestions --- .../integration/dashboard/key_value.test.ts | 14 +++++++---- .../cypress-base/package-lock.json | 24 +++++++++++++++++++ superset-frontend/cypress-base/package.json | 2 ++ .../src/components/FaveStar/FaveStar.test.tsx | 2 +- .../src/dashboard/components/Header/index.jsx | 2 +- .../nativeFilters/FilterBar/index.tsx | 2 -- 6 files changed, 38 insertions(+), 8 deletions(-) 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 index 571bbac45f12..710d574536df 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -16,11 +16,17 @@ * specific language governing permissions and limitations * under the License. */ +import qs from 'querystringify'; import { WORLD_HEALTH_DASHBOARD, WORLD_HEALTH_CHARTS, waitForChartLoad, } from './dashboard.helper'; +// import { URL_PARAMS } from 'superset-frontend/src/constants.ts'; + +interface QueryString { + native_filters: string; +} describe('nativefiler url param key', () => { // const urlParams = { param1: '123', param2: 'abc' }; @@ -33,14 +39,14 @@ describe('nativefiler url param key', () => { cy.login(); }); let initialFilterKey: string; - it('should have cachekey in in nativefilter param', () => { + it('should have cachekey in nativefilter param', () => { cy.location().then(loc => { - initialFilterKey = loc.search.split('=')[1]; - expect(typeof initialFilterKey).eq('string'); + const check = qs.parse(loc.search) as QueryString; + expect(typeof check.native_filters).eq('string'); }); }); - it('should have different key when pages reloads', () => { + it('should have different key when page reloads', () => { cy.location().then(loc => { const newFilterKey = loc.search.split('='); expect(newFilterKey).not.equal(initialFilterKey); diff --git a/superset-frontend/cypress-base/package-lock.json b/superset-frontend/cypress-base/package-lock.json index cd457f61eb22..8b040d1527a5 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 825e16fa2a23..4208556572b4 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/src/components/FaveStar/FaveStar.test.tsx b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx index 68433db96ea4..24dc0114b572 100644 --- a/superset-frontend/src/components/FaveStar/FaveStar.test.tsx +++ b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx @@ -85,7 +85,7 @@ test('Call fetchFaveStar only on the first render', () => { }; const { rerender } = render(); - expect(props.fetchFaveStar).toBeCalledTimes(1); + expect(props.).toBeCalledTimes(1); expect(props.fetchFaveStar).toBeCalledWith(props.itemId); rerender(); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index a4eaa3e96ac6..a67061832dda 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -531,7 +531,7 @@ class Header extends React.PureComponent { canEdit={userCanEdit} canSave={userCanSaveAs} /> - {user?.userId && ( + {user?.userId && dashboardInfo?.id && ( = ({ } else filterType = dataMask; dataMaskKey = await createFilterKey(dashboardId, filterType); - console.log('dataMask', dataMaskKey); } newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); From ba64b3923017af540a83ce991c65837c1959fe36 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 16 Dec 2021 16:27:12 -0800 Subject: [PATCH 18/29] add suggestions --- superset-frontend/src/common/hooks/useUrlShortener.ts | 2 +- .../components/menu/ShareMenuItems/ShareMenuItems.test.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/common/hooks/useUrlShortener.ts b/superset-frontend/src/common/hooks/useUrlShortener.ts index f36ad7a20f9d..33cb636b4527 100644 --- a/superset-frontend/src/common/hooks/useUrlShortener.ts +++ b/superset-frontend/src/common/hooks/useUrlShortener.ts @@ -23,7 +23,7 @@ export function useUrlShortener(url: string): Function { const [update, setUpdate] = useState(false); const [shortUrl, setShortUrl] = useState(''); - async function getShortUrl(urlOverride: string) { + async function getShortUrl(urlOverride?: string) { if (update) { const newShortUrl = await getShortUrlUtil(urlOverride || url); setShortUrl(newShortUrl); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index fb264f53d777..f7b4a78cfd1f 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -27,6 +27,7 @@ import ShareMenuItems from '.'; const spy = jest.spyOn(copyTextToClipboard, 'default'); +const DASHBOARD_ID = 26; const createProps = () => ({ addDangerToast: jest.fn(), addSuccessToast: jest.fn(), @@ -35,7 +36,7 @@ const createProps = () => ({ emailMenuItemTitle: 'Share dashboard by email', emailSubject: 'Superset dashboard COVID Vaccine Dashboard', emailBody: 'Check out this dashboard: ', - dashboardId: '26', + dashboardId: DASHBOARD_ID, }); const { location } = window; From b3db57050ac2f5901b09f9d9f8d89aefd52fee64 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 16 Dec 2021 16:48:02 -0800 Subject: [PATCH 19/29] fix lint --- .../components/menu/ShareMenuItems/ShareMenuItems.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index f7b4a78cfd1f..243d8e489e79 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -27,7 +27,7 @@ import ShareMenuItems from '.'; const spy = jest.spyOn(copyTextToClipboard, 'default'); -const DASHBOARD_ID = 26; +const DASHBOARD_ID = '26'; const createProps = () => ({ addDangerToast: jest.fn(), addSuccessToast: jest.fn(), From 9c580357310213a740747e50fac68680fe315f17 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 16 Dec 2021 23:37:40 -0800 Subject: [PATCH 20/29] more suggested changes for backwards comapat --- .../integration/dashboard/key_value.test.ts | 5 ++--- .../src/components/FaveStar/FaveStar.test.tsx | 2 +- superset-frontend/src/constants.ts | 2 +- .../nativeFilters/FilterBar/index.tsx | 11 +++------- .../nativeFilters/FilterBar/keyValue.tsx | 2 +- .../dashboard/containers/DashboardPage.tsx | 22 ++++++++++++++----- 6 files changed, 25 insertions(+), 19 deletions(-) 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 index 710d574536df..2e7dd6152354 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -22,10 +22,9 @@ import { WORLD_HEALTH_CHARTS, waitForChartLoad, } from './dashboard.helper'; -// import { URL_PARAMS } from 'superset-frontend/src/constants.ts'; interface QueryString { - native_filters: string; + native_filters_key: string; } describe('nativefiler url param key', () => { @@ -42,7 +41,7 @@ describe('nativefiler url param key', () => { it('should have cachekey in nativefilter param', () => { cy.location().then(loc => { const check = qs.parse(loc.search) as QueryString; - expect(typeof check.native_filters).eq('string'); + expect(typeof check.native_filters_key).eq('string'); }); }); diff --git a/superset-frontend/src/components/FaveStar/FaveStar.test.tsx b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx index 24dc0114b572..68433db96ea4 100644 --- a/superset-frontend/src/components/FaveStar/FaveStar.test.tsx +++ b/superset-frontend/src/components/FaveStar/FaveStar.test.tsx @@ -85,7 +85,7 @@ test('Call fetchFaveStar only on the first render', () => { }; const { rerender } = render(); - expect(props.).toBeCalledTimes(1); + expect(props.fetchFaveStar).toBeCalledTimes(1); expect(props.fetchFaveStar).toBeCalledWith(props.itemId); rerender(); diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index d32b7faaff33..5e30390d637b 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -40,7 +40,7 @@ export const URL_PARAMS = { type: 'rison', }, nativeFiltersByCacheKey: { - name: 'native_filters', + name: 'native_filters_key', type: 'string', }, filterSet: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index ee873067a2a3..95eb5dcd6643 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -215,15 +215,10 @@ const FilterBar: React.FC = ({ ) { dataMaskKey = nativeFiltersCacheKey; } else { - let filterType; - const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof isOldRison === 'object') { - filterType = rison.encode(isOldRison); - } else filterType = dataMask; - - dataMaskKey = await createFilterKey(dashboardId, filterType); + dataMaskKey = await createFilterKey(dashboardId, dataMask); } - newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey); + + newParams.set(URL_PARAMS.nativeFiltersByCacheKey.name, dataMaskKey); history.replace({ search: newParams.toString(), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx index 5da6cba0b3c7..ef8cd6887532 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -29,7 +29,7 @@ export const updateFilterKey = (dashId: string, value: string, key: string) => return null; }); -export const createFilterKey = (dashId: string, value: string) => +export const createFilterKey = (dashId: string | number, value: string) => SupersetClient.post({ endpoint: `api/v1/dashboard/${dashId}/filter_state`, jsonPayload: { value }, diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 3870a0fc6a8c..1646d8856e57 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -20,6 +20,7 @@ import React, { FC, useRef, useEffect, useState } from 'react'; import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; import { useDispatch, useSelector } from 'react-redux'; import { useParams } from 'react-router-dom'; +import rison from 'rison'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import Loading from 'src/components/Loading'; import FilterBoxMigrationModal from 'src/dashboard/components/FilterBoxMigrationModal'; @@ -48,7 +49,10 @@ 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'; +import { + createFilterKey, + getFilterValue, +} from '../components/nativeFilters/FilterBar/keyValue'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, @@ -156,19 +160,27 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { - const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters); - let dataMaskFromUrl = nativeFilterValue || {}; + const nativeFilterKeyValue = getUrlParam( + URL_PARAMS.nativeFiltersByCacheKey, + ); + let dataMaskFromUrl = nativeFilterKeyValue || {}; - if (typeof nativeFilterValue === 'string') { + const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); + // check if key from key_value api and get datamask + if (nativeFilterKeyValue) { try { dataMaskFromUrl = await getFilterValue( dashboard?.id, - nativeFilterValue, + nativeFilterKeyValue, ); } catch (err) { return null; } + // else get old old rison string and set as datamask + } else if (isOldRison && dashboard?.id) { + dataMaskFromUrl = isOldRison; } + if (readyToRender) { if (!isDashboardHydrated.current) { isDashboardHydrated.current = true; From 63672e817a6ee0542a3d6dcf66fffd018e509f87 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Thu, 16 Dec 2021 23:59:33 -0800 Subject: [PATCH 21/29] fix lint --- .../dashboard/components/nativeFilters/FilterBar/index.tsx | 1 - .../src/dashboard/containers/DashboardPage.tsx | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 95eb5dcd6643..20a0e1ed5ba7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -26,7 +26,6 @@ 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'; diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 1646d8856e57..80e5ca6dbf09 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -20,7 +20,6 @@ import React, { FC, useRef, useEffect, useState } from 'react'; import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; import { useDispatch, useSelector } from 'react-redux'; import { useParams } from 'react-router-dom'; -import rison from 'rison'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import Loading from 'src/components/Loading'; import FilterBoxMigrationModal from 'src/dashboard/components/FilterBoxMigrationModal'; @@ -49,10 +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 { - createFilterKey, - getFilterValue, -} from '../components/nativeFilters/FilterBar/keyValue'; +import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue'; export const MigrationContext = React.createContext( FILTER_BOX_MIGRATION_STATES.NOOP, From 67193e1672b6de6ad34605cdeefe0dddeff44c64 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 11:57:53 -0800 Subject: [PATCH 22/29] cleanup naming and add qs parse to tests --- .../integration/dashboard/key_value.test.ts | 8 ++++---- .../integration/dashboard/nativeFilters.test.ts | 15 ++++++++++----- .../cypress-base/cypress/support/index.ts | 2 +- .../dashboard/util/getDashboardUrl_spec.js | 4 ++-- superset-frontend/src/constants.ts | 2 +- .../src/dashboard/containers/DashboardPage.tsx | 4 +--- .../src/dashboard/util/getDashboardUrl.ts | 7 +++++-- 7 files changed, 24 insertions(+), 18 deletions(-) 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 index 2e7dd6152354..6bd34373f2ac 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/key_value.test.ts @@ -40,15 +40,15 @@ describe('nativefiler url param key', () => { let initialFilterKey: string; it('should have cachekey in nativefilter param', () => { cy.location().then(loc => { - const check = qs.parse(loc.search) as QueryString; - expect(typeof check.native_filters_key).eq('string'); + 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 newFilterKey = loc.search.split('='); - expect(newFilterKey).not.equal(initialFilterKey); + 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 de420b1f9899..da893f4c528f 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,11 +94,14 @@ describe('Nativefilters Sanity test', () => { cy.get(nativeFilters.modal.container).should('be.visible'); }); it('User can add a new native filter', () => { - let cacheKey: string; + let filterKey: string; + const removeFirstChar = (search: string) => + search.split('').slice(1, search.length).join(''); cy.wait(3000); cy.location().then(loc => { - cacheKey = loc.search.split('=')[1]; - expect(typeof cacheKey).eq('string'); + 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(); @@ -136,8 +140,9 @@ describe('Nativefilters Sanity test', () => { .click(); cy.wait(3000); cy.location().then(loc => { - const sameCacheKey: string = loc.search.split('=')[1]; - expect(sameCacheKey).not.eq(cacheKey); + 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'); diff --git a/superset-frontend/cypress-base/cypress/support/index.ts b/superset-frontend/cypress-base/cypress/support/index.ts index e22f69975e96..07265ada67ce 100644 --- a/superset-frontend/cypress-base/cypress/support/index.ts +++ b/superset-frontend/cypress-base/cypress/support/index.ts @@ -35,7 +35,7 @@ Cypress.Commands.add('login', () => { cy.request({ method: 'POST', url: '/login/', - body: { username: 'admin', password: 'general' }, + body: { username: 'admin', password: 'admin' }, }).then(response => { expect(response.status).to.eq(200); }); diff --git a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index 48b2e6d00920..53904e1c6bf2 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js @@ -79,7 +79,7 @@ describe('getChartIdsFromLayout', () => { location: { origin: 'https://localhost', search: - '?preselect_filters=%7B%7D&native_filters=024380498jdkjf-2094838', + '?preselect_filters=%7B%7D&native_filters_key=024380498jdkjf-2094838', }, })); @@ -87,7 +87,7 @@ describe('getChartIdsFromLayout', () => { pathname: 'path', }); expect(urlWithNativeFilters).toBe( - 'path?preselect_filters=%7B%7D&native_filters=024380498jdkjf-2094838', + 'path?preselect_filters=%7B%7D&native_filters_key=024380498jdkjf-2094838', ); }); }); diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index 5e30390d637b..a57951af12c3 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -39,7 +39,7 @@ export const URL_PARAMS = { name: 'native_filters', type: 'rison', }, - nativeFiltersByCacheKey: { + nativeFiltersKey: { name: 'native_filters_key', type: 'string', }, diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 80e5ca6dbf09..7f30979a1ac6 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -156,9 +156,7 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { - const nativeFilterKeyValue = getUrlParam( - URL_PARAMS.nativeFiltersByCacheKey, - ); + const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey); let dataMaskFromUrl = nativeFilterKeyValue || {}; const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); diff --git a/superset-frontend/src/dashboard/util/getDashboardUrl.ts b/superset-frontend/src/dashboard/util/getDashboardUrl.ts index 33218eb4cb06..9c261e721db8 100644 --- a/superset-frontend/src/dashboard/util/getDashboardUrl.ts +++ b/superset-frontend/src/dashboard/util/getDashboardUrl.ts @@ -44,9 +44,12 @@ export default function getDashboardUrl({ if (standalone) { newSearchParams.set(URL_PARAMS.standalone.name, standalone.toString()); } - const dataMaskKey = getUrlParam(URL_PARAMS.nativeFiltersByCacheKey); + const dataMaskKey = getUrlParam(URL_PARAMS.nativeFiltersKey); if (dataMaskKey) { - newSearchParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey as string); + newSearchParams.set( + URL_PARAMS.nativeFiltersKey.name, + dataMaskKey as string, + ); } const hashSection = hash ? `#${hash}` : ''; From 957fe20a4a4f54807cc3d1f530bd849b008b8a16 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 11:58:45 -0800 Subject: [PATCH 23/29] Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- .../dashboard/components/menu/ShareMenuItems/index.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 491e6d757dc6..a2321bc7bac2 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -55,15 +55,15 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { const getShortUrl = useUrlShortener(url); async function getCopyUrl() { - const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof isOldRison === 'object' || !dashboardId) return null; - const getPrevData = await getFilterValue( + const risonObj = getUrlParam(URL_PARAMS.nativeFilters); + if (typeof risonObj === 'object' || !dashboardId) return null; + const prevData = await getFilterValue( dashboardId, getUrlParam(URL_PARAMS.nativeFiltersByCacheKey), ); const newDataMaskKey = await createFilterKey( dashboardId, - JSON.stringify(getPrevData), + JSON.stringify(prevData), ); const newUrl = new URL(`${window.location.origin}${url}`); newUrl.searchParams.set(URL_PARAMS.nativeFilters.name, newDataMaskKey); From a9dfb1496e943316d1e216518c9f7d122983f17f Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 11:58:52 -0800 Subject: [PATCH 24/29] Update superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- .../src/dashboard/components/menu/ShareMenuItems/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index a2321bc7bac2..2afcdd20d5bd 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -77,7 +77,6 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { await copyTextToClipboard(shortUrl); addSuccessToast(t('Copied to clipboard!')); } catch (error) { - console.error(error); addDangerToast(t('Sorry, your browser does not support copying.')); } } From fc7ef7bf9b2ef5a6ac6340f08497136a598c3c7f Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 12:14:41 -0800 Subject: [PATCH 25/29] more changes and fix lint --- superset-frontend/cypress-base/cypress/support/index.ts | 2 +- .../menu/ShareMenuItems/ShareMenuItems.test.tsx | 2 +- .../dashboard/components/menu/ShareMenuItems/index.tsx | 2 +- .../components/nativeFilters/FilterBar/index.tsx | 9 +++------ 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/support/index.ts b/superset-frontend/cypress-base/cypress/support/index.ts index 07265ada67ce..e22f69975e96 100644 --- a/superset-frontend/cypress-base/cypress/support/index.ts +++ b/superset-frontend/cypress-base/cypress/support/index.ts @@ -35,7 +35,7 @@ Cypress.Commands.add('login', () => { cy.request({ method: 'POST', url: '/login/', - body: { username: 'admin', password: 'admin' }, + body: { username: 'admin', password: 'general' }, }).then(response => { expect(response.status).to.eq(200); }); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index 243d8e489e79..9e47441fec67 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -31,7 +31,7 @@ const DASHBOARD_ID = '26'; const createProps = () => ({ 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', diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 491e6d757dc6..5728a8ccaacb 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -59,7 +59,7 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { if (typeof isOldRison === 'object' || !dashboardId) return null; const getPrevData = await getFilterValue( dashboardId, - getUrlParam(URL_PARAMS.nativeFiltersByCacheKey), + getUrlParam(URL_PARAMS.nativeFiltersKey), ); const newDataMaskKey = await createFilterKey( dashboardId, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 20a0e1ed5ba7..c56710a83cca 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -198,14 +198,12 @@ const FilterBar: React.FC = ({ const newParams = new URLSearchParams(); let dataMaskKey = ''; previousParams.forEach((value, key) => { - if (key !== URL_PARAMS.nativeFilters.name) { + if (key !== URL_PARAMS.nativeFiltersKey.name) { newParams.append(key, value); } }); - const nativeFiltersCacheKey = getUrlParam( - URL_PARAMS.nativeFiltersByCacheKey, - ); + const nativeFiltersCacheKey = getUrlParam(URL_PARAMS.nativeFiltersKey); const dataMask = JSON.stringify(dataMaskSelected); if ( updateKey && @@ -216,8 +214,7 @@ const FilterBar: React.FC = ({ } else { dataMaskKey = await createFilterKey(dashboardId, dataMask); } - - newParams.set(URL_PARAMS.nativeFiltersByCacheKey.name, dataMaskKey); + newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); history.replace({ search: newParams.toString(), From a683b2daed87887b2424c60f0cfd7fed5a54b91f Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 14:59:36 -0800 Subject: [PATCH 26/29] remove nativefiler param --- .../src/dashboard/components/nativeFilters/FilterBar/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index c56710a83cca..8be9f257361a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -198,7 +198,7 @@ const FilterBar: React.FC = ({ const newParams = new URLSearchParams(); let dataMaskKey = ''; previousParams.forEach((value, key) => { - if (key !== URL_PARAMS.nativeFiltersKey.name) { + if (key !== URL_PARAMS.nativeFilters.name) { newParams.append(key, value); } }); @@ -256,6 +256,7 @@ const FilterBar: React.FC = ({ const dataMaskAppliedText = JSON.stringify(dataMaskApplied); useEffect(() => { + console.log(' publish datamask initial') publishDataMask(dataMaskApplied); // eslint-disable-next-line react-hooks/exhaustive-deps }, [dataMaskAppliedText, publishDataMask]); From 0c9163a9529749eae3326f5593fcfff7e9158f72 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 15:02:39 -0800 Subject: [PATCH 27/29] fix path --- .../src/dashboard/components/menu/ShareMenuItems/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index f8d8b8948a62..56e9a60566e3 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -26,7 +26,7 @@ import { URL_PARAMS } from 'src/constants'; import { createFilterKey, getFilterValue, -} from '../../nativeFilters/FilterBar/keyValue'; +} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; interface ShareMenuItemProps { url: string; From 8e19e18829ce6e3e251096b040c99146814e5eaf Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Fri, 17 Dec 2021 15:04:16 -0800 Subject: [PATCH 28/29] remove con --- .../src/dashboard/components/nativeFilters/FilterBar/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 8be9f257361a..4f252e0de3ed 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -256,7 +256,6 @@ const FilterBar: React.FC = ({ const dataMaskAppliedText = JSON.stringify(dataMaskApplied); useEffect(() => { - console.log(' publish datamask initial') publishDataMask(dataMaskApplied); // eslint-disable-next-line react-hooks/exhaustive-deps }, [dataMaskAppliedText, publishDataMask]); From 40d73433ef6b1947f872d4ab45ddda3dab485b61 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Mon, 20 Dec 2021 13:08:43 -0800 Subject: [PATCH 29/29] simplify logic --- .../src/dashboard/containers/DashboardPage.tsx | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 7f30979a1ac6..b14b1e0fdd79 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -162,16 +162,9 @@ const DashboardPage: FC = () => { const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); // check if key from key_value api and get datamask if (nativeFilterKeyValue) { - try { - dataMaskFromUrl = await getFilterValue( - dashboard?.id, - nativeFilterKeyValue, - ); - } catch (err) { - return null; - } - // else get old old rison string and set as datamask - } else if (isOldRison && dashboard?.id) { + dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue); + } + if (isOldRison) { dataMaskFromUrl = isOldRison; } @@ -194,7 +187,7 @@ const DashboardPage: FC = () => { } return null; } - if (dashboard?.id) getDataMaskApplied(); + if (id) getDataMaskApplied(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [readyToRender, filterboxMigrationState]);