Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace datamask with key from new key value api #17680

Merged
merged 32 commits into from
Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5655338
afirst stage to ccheck to get initial datamask
pkdotson Dec 7, 2021
94db318
clean up code and update typescript
pkdotson Dec 8, 2021
38461bb
remove consoles
pkdotson Dec 8, 2021
b08a472
fix ts and update copy dashboard url
pkdotson Dec 8, 2021
6420292
use key when one doesn't exists
pkdotson Dec 8, 2021
3ce45d1
lint clean up
pkdotson Dec 9, 2021
dfb050f
fix errors
pkdotson Dec 10, 2021
e7ccfb7
add suggested changes
pkdotson Dec 14, 2021
e4b3830
remove line
pkdotson Dec 14, 2021
548a5bb
add tests and add changes for copydashboard
pkdotson Dec 15, 2021
abd4a86
fix lint
pkdotson Dec 15, 2021
cac4bea
fix lint
pkdotson Dec 15, 2021
cc443ef
fix lint
pkdotson Dec 15, 2021
0ce2bae
Update superset-frontend/src/dashboard/components/Header/index.jsx
pkdotson Dec 15, 2021
d87db08
Merge branch 'master' of https://github.com/preset-io/superset into f…
pkdotson Dec 15, 2021
cc3dd9c
add timeout
pkdotson Dec 15, 2021
d12b370
fix test
pkdotson Dec 16, 2021
ee2982c
Merge branch 'master' of https://github.com/preset-io/superset into f…
pkdotson Dec 16, 2021
2af1863
fix test, add qs to cypress and add suggestions
pkdotson Dec 17, 2021
ba64b39
add suggestions
pkdotson Dec 17, 2021
b3db570
fix lint
pkdotson Dec 17, 2021
9c58035
more suggested changes for backwards comapat
pkdotson Dec 17, 2021
63672e8
fix lint
pkdotson Dec 17, 2021
67193e1
cleanup naming and add qs parse to tests
pkdotson Dec 17, 2021
957fe20
Update superset-frontend/src/dashboard/components/menu/ShareMenuItems…
pkdotson Dec 17, 2021
a9dfb14
Update superset-frontend/src/dashboard/components/menu/ShareMenuItems…
pkdotson Dec 17, 2021
fc7ef7b
more changes and fix lint
pkdotson Dec 17, 2021
c74b8d5
Merge branch 'frontend-dashboard-longurl-2' of https://github.com/pre…
pkdotson Dec 17, 2021
a683b2d
remove nativefiler param
pkdotson Dec 17, 2021
0c9163a
fix path
pkdotson Dec 17, 2021
8e19e18
remove con
pkdotson Dec 17, 2021
40d7343
simplify logic
pkdotson Dec 20, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const URL_PARAMS = {
},
nativeFilters: {
name: 'native_filters',
type: 'rison',
type: 'rison | string',
},
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
filterSet: {
name: 'filter_set',
Expand Down
7 changes: 4 additions & 3 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ export const hydrateDashboard =
dashboardData,
chartData,
filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP,
dataMaskApplied,
) =>
(dispatch, getState) => {
const { user, common } = getState();

pkdotson marked this conversation as resolved.
Show resolved Hide resolved
const { metadata } = dashboardData;
const regularUrlParams = extractUrlParams('regular');
const reservedUrlParams = extractUrlParams('reserved');
Expand Down Expand Up @@ -378,10 +378,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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
);

const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID];
const rootChildId = dashboardRoot.children[0];
const rootChildId = dashboardRoot?.children[0];
const topLevelTabs =
rootChildId !== DASHBOARD_GRID_ID
? dashboardLayout[rootChildId]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
28 changes: 15 additions & 13 deletions superset-frontend/src/dashboard/components/Header/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
this.props.fetchUISpecificReport(
user.userId,
'dashboard_id',
'dashboards',
dashboardInfo.id,
user.email,
);
}
}
}

Expand Down Expand Up @@ -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,
);
}
}
Expand Down Expand Up @@ -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 (
<StyledDashboardHeader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
/* eslint-disable no-param-reassign */
import { DataMask, HandlerFunction, styled, t } from '@superset-ui/core';
import React, { useEffect, useState, useCallback, useMemo } from 'react';
import { useDispatch } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';
import cx from 'classnames';
import Icons from 'src/components/Icons';
import { Tabs } from 'src/common/components';
Expand All @@ -40,6 +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 replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull';
import { checkIsApplyDisabled, TabIds } from './utils';
import FilterSets from './FilterSets';
Expand All @@ -50,6 +51,7 @@ import {
useFilterUpdates,
useInitialization,
} from './state';
import { createFilterKey, updateFilterKey } from './keyValue';
import EditSection from './FilterSets/EditSection';
import Header from './Header';
import FilterControls from './FilterControls/FilterControls';
Expand Down Expand Up @@ -160,6 +162,9 @@ const FilterBar: React.FC<FiltersBarProps> = ({
const filters = useFilters();
const previousFilters = usePrevious(filters);
const filterValues = Object.values<Filter>(filters);
const dashboardId = useSelector<any, string>(
({ dashboardInfo }) => dashboardInfo?.id,
);

const handleFilterSelectionChange = useCallback(
(
Expand All @@ -186,23 +191,47 @@ const FilterBar: React.FC<FiltersBarProps> = ({
[dataMaskSelected, dispatch, setDataMaskSelected, tab],
);

const getFilterKeyForUrl = async (value: string) => {
let key;
try {
key = await createFilterKey(dashboardId, value);
} catch (err) {
return null;
}
return key;
};

pkdotson marked this conversation as resolved.
Show resolved Hide resolved
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 = '';
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
previousParams.forEach((value, key) => {
if (key !== URL_PARAMS.nativeFilters.name) {
newParams.append(key, value);
}
});

newParams.set(
URL_PARAMS.nativeFilters.name,
rison.encode(replaceUndefinedByNull(dataMaskSelected)),
);
const getParam = getUrlParam(URL_PARAMS.nativeFilters);
const dataMask = JSON.stringify(dataMaskSelected);

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 {}
}
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
newParams.set(URL_PARAMS.nativeFilters.name, dataMaskKey);

history.replace({
search: newParams.toString(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { SupersetClient } from '@superset-ui/core';
pkdotson marked this conversation as resolved.
Show resolved Hide resolved

export const updateFilterKey = (dashId: string, value: string, key: string) =>
SupersetClient.put({
endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`,
jsonPayload: { value },
})
.then(r => r.json.message)
.catch(err => err);
pkdotson marked this conversation as resolved.
Show resolved Hide resolved

export const createFilterKey = (dashId: string, value: string) =>
SupersetClient.post({
endpoint: `api/v1/dashboard/${dashId}/filter_state`,
jsonPayload: { value },
})
.then(r => r.json.key)
.catch(err => console.log('err', err));
pkdotson marked this conversation as resolved.
Show resolved Hide resolved

export const getFilterValue = (
dashId: string | number | undefined,
key: string,
) =>
SupersetClient.get({
endpoint: `api/v1/dashboard/${dashId}/filter_state/${key}/`,
})
.then(({ json }) => JSON.parse(json.value))
.catch(err => err);
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export const useFilterUpdates = (
) => {
const filters = useFilters();
const dataMaskApplied = useNativeFiltersDataMask();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove the unnecessary change?

useEffect(() => {
// Remove deleted filters from local state
Object.keys(dataMaskSelected).forEach(selectedId => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export const checkIsApplyDisabled = (
) => {
const dataSelectedValues = Object.values(dataMaskSelected);
const dataAppliedValues = Object.values(dataMaskApplied);

pkdotson marked this conversation as resolved.
Show resolved Hide resolved
return (
areObjectsEqual(
getOnlyExtraFormData(dataMaskSelected),
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/dashboard/containers/Dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 32 additions & 7 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { canUserEditDashboard } from 'src/dashboard/util/findPermission';
import { getFilterSets } from '../actions/nativeFilters';
import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue';

export const MigrationContext = React.createContext(
FILTER_BOX_MIGRATION_STATES.NOOP,
Expand Down Expand Up @@ -153,16 +154,40 @@ const DashboardPage: FC = () => {
}, [readyToRender]);

useEffect(() => {
if (readyToRender) {
if (!isDashboardHydrated.current) {
isDashboardHydrated.current = true;
if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET)) {
// only initialize filterset once
dispatch(getFilterSets());
// eslint-disable-next-line consistent-return
async function getDataMaskApplied() {
const nativeFilterValue = getUrlParam(URL_PARAMS.nativeFilters);
let dataMaskFromUrl = nativeFilterValue || {};

if (typeof nativeFilterValue === 'string') {
try {
dataMaskFromUrl = await getFilterValue(
dashboard?.id,
nativeFilterValue,
);
} catch (err) {
return null;
}
}
dispatch(hydrateDashboard(dashboard, charts, filterboxMigrationState));
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,
),
);
}
}
if (dashboard?.id) getDataMaskApplied();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [readyToRender, filterboxMigrationState]);

Expand Down
15 changes: 4 additions & 11 deletions superset-frontend/src/dashboard/util/getDashboardUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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}` : '';
Expand Down
6 changes: 6 additions & 0 deletions superset-frontend/src/dataMask/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down