Skip to content

Commit

Permalink
feat: save active tabs in dashboard permalink (#19983)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ktmud
ktmud committed Jun 29, 2022
1 parent c5d3678 commit cadd259
Show file tree
Hide file tree
Showing 17 changed files with 274 additions and 103 deletions.
44 changes: 21 additions & 23 deletions superset-frontend/src/dashboard/actions/hydrate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,23 @@ import { updateColorSchema } from './dashboardInfo';
export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';

export const hydrateDashboard =
(
dashboardData,
chartData,
({
dashboard,
charts,
filterboxMigrationState = FILTER_BOX_MIGRATION_STATES.NOOP,
dataMaskApplied,
) =>
dataMask,
activeTabs,
}) =>
(dispatch, getState) => {
const { user, common, dashboardState } = getState();

const { metadata } = dashboardData;
const { metadata, position_data: positionData } = dashboard;
const regularUrlParams = extractUrlParams('regular');
const reservedUrlParams = extractUrlParams('reserved');
const editMode = reservedUrlParams.edit === 'true';

let preselectFilters = {};

chartData.forEach(chart => {
charts.forEach(chart => {
// eslint-disable-next-line no-param-reassign
chart.slice_id = chart.form_data.slice_id;
});
Expand All @@ -98,12 +98,10 @@ export const hydrateDashboard =
updateColorSchema(metadata, metadata?.label_colors);
}

// dashboard layout
const { position_data } = dashboardData;
// new dash: position_json could be {} or null
const layout =
position_data && Object.keys(position_data).length > 0
? position_data
positionData && Object.keys(positionData).length > 0
? positionData
: getEmptyLayout();

// create a lookup to sync layout names with slice names
Expand All @@ -128,7 +126,7 @@ export const hydrateDashboard =
const sliceIds = new Set();
const slicesFromExploreCount = new Map();

chartData.forEach(slice => {
charts.forEach(slice => {
const key = slice.slice_id;
const form_data = {
...slice.form_data,
Expand Down Expand Up @@ -269,7 +267,7 @@ export const hydrateDashboard =
id: DASHBOARD_HEADER_ID,
type: DASHBOARD_HEADER_TYPE,
meta: {
text: dashboardData.dashboard_title,
text: dashboard.dashboard_title,
},
};

Expand All @@ -291,7 +289,7 @@ export const hydrateDashboard =
let filterConfig = metadata?.native_filter_configuration || [];
if (filterboxMigrationState === FILTER_BOX_MIGRATION_STATES.REVIEWING) {
filterConfig = getNativeFilterConfig(
chartData,
charts,
filterScopes,
preselectFilters,
);
Expand All @@ -302,7 +300,7 @@ export const hydrateDashboard =
filterConfig,
});
metadata.show_native_filters =
dashboardData?.metadata?.show_native_filters ??
dashboard?.metadata?.show_native_filters ??
(isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&
[
FILTER_BOX_MIGRATION_STATES.CONVERTED,
Expand Down Expand Up @@ -343,7 +341,7 @@ export const hydrateDashboard =
}

const { roles } = user;
const canEdit = canUserEditDashboard(dashboardData, user);
const canEdit = canUserEditDashboard(dashboard, user);

return dispatch({
type: HYDRATE_DASHBOARD,
Expand All @@ -352,7 +350,7 @@ export const hydrateDashboard =
charts: chartQueries,
// read-only data
dashboardInfo: {
...dashboardData,
...dashboard,
metadata,
userId: user.userId ? String(user.userId) : null, // legacy, please use state.user instead
dash_edit_perm: canEdit,
Expand Down Expand Up @@ -380,7 +378,7 @@ export const hydrateDashboard =
conf: common?.conf,
},
},
dataMask: dataMaskApplied,
dataMask,
dashboardFilters,
nativeFilters,
dashboardState: {
Expand All @@ -394,17 +392,17 @@ export const hydrateDashboard =
// dashboard viewers can set refresh frequency for the current visit,
// only persistent refreshFrequency will be saved to backend
shouldPersistRefreshFrequency: false,
css: dashboardData.css || '',
css: dashboard.css || '',
colorNamespace: metadata?.color_namespace || null,
colorScheme: metadata?.color_scheme || null,
editMode: canEdit && editMode,
isPublished: dashboardData.published,
isPublished: dashboard.published,
hasUnsavedChanges: false,
maxUndoHistoryExceeded: false,
lastModifiedTime: dashboardData.changed_on,
lastModifiedTime: dashboard.changed_on,
isRefreshing: false,
isFiltersRefreshing: false,
activeTabs: dashboardState?.activeTabs || [],
activeTabs: activeTabs || dashboardState?.activeTabs || [],
filterboxMigrationState,
datasetsStatus: ResourceStatus.LOADING,
},
Expand Down
34 changes: 25 additions & 9 deletions superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ import React, { useState } from 'react';
import { t } from '@superset-ui/core';
import Popover, { PopoverProps } from 'src/components/Popover';
import CopyToClipboard from 'src/components/CopyToClipboard';
import { getDashboardPermalink, getUrlParam } from 'src/utils/urlUtils';
import { getDashboardPermalink } from 'src/utils/urlUtils';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { URL_PARAMS } from 'src/constants';
import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';
import { useSelector } from 'react-redux';
import { RootState } from 'src/dashboard/types';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';

export type URLShortLinkButtonProps = {
dashboardId: number;
Expand All @@ -42,19 +43,27 @@ export default function URLShortLinkButton({
}: URLShortLinkButtonProps) {
const [shortUrl, setShortUrl] = useState('');
const { addDangerToast } = useToasts();
const { dataMask, activeTabs } = useSelector((state: RootState) => ({
dataMask: state.dataMask,
activeTabs: state.dashboardState.activeTabs,
}));

const getCopyUrl = async () => {
const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey);
try {
const filterState = await getFilterValue(dashboardId, nativeFiltersKey);
const url = await getDashboardPermalink({
dashboardId,
filterState,
hash: anchorLinkId,
dataMask,
activeTabs,
anchor: anchorLinkId,
});
setShortUrl(url);
} catch (error) {
addDangerToast(error);
if (error) {
addDangerToast(
(await getClientErrorObject(error)).error ||
t('Something went wrong.'),
);
}
}
};

Expand All @@ -66,7 +75,14 @@ export default function URLShortLinkButton({
trigger="click"
placement={placement}
content={
<div id="shorturl-popover" data-test="shorturl-popover">
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<div
id="shorturl-popover"
data-test="shorturl-popover"
onClick={e => {
e.stopPropagation();
}}
>
<CopyToClipboard
text={shortUrl}
copyNode={
Expand Down
12 changes: 11 additions & 1 deletion superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const propTypes = {
editMode: PropTypes.bool.isRequired,
renderHoverMenu: PropTypes.bool,
directPathToChild: PropTypes.arrayOf(PropTypes.string),
activeTabs: PropTypes.arrayOf(PropTypes.string),
filterboxMigrationState: FILTER_BOX_MIGRATION_STATES,

// actions (from DashboardComponent.jsx)
Expand All @@ -74,6 +75,7 @@ const defaultProps = {
renderHoverMenu: true,
availableColumnCount: 0,
columnWidth: 0,
activeTabs: [],
directPathToChild: [],
filterboxMigrationState: FILTER_BOX_MIGRATION_STATES.NOOP,
setActiveTabs() {},
Expand Down Expand Up @@ -112,13 +114,20 @@ const StyledTabsContainer = styled.div`
export class Tabs extends React.PureComponent {
constructor(props) {
super(props);
const tabIndex = Math.max(
let tabIndex = Math.max(
0,
findTabIndexByComponentId({
currentComponent: props.component,
directPathToChild: props.directPathToChild,
}),
);
if (tabIndex === 0 && props.activeTabs?.length) {
props.component.children.forEach((tabId, index) => {
if (tabIndex === 0 && props.activeTabs.includes(tabId)) {
tabIndex = index;
}
});
}
const { children: tabIds } = props.component;
const activeKey = tabIds[tabIndex];

Expand Down Expand Up @@ -408,6 +417,7 @@ Tabs.defaultProps = defaultProps;
function mapStateToProps(state) {
return {
nativeFilters: state.nativeFilters,
activeTabs: state.dashboardState.activeTabs,
directPathToChild: state.dashboardState.directPathToChild,
filterboxMigrationState: state.dashboardState.filterboxMigrationState,
};
Expand Down
5 changes: 5 additions & 0 deletions superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ test('Should render menu items', () => {
<Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
<ShareMenuItems {...props} />
</Menu>,
{ useRedux: true },
);
expect(
screen.getByRole('menuitem', { name: 'Copy dashboard URL' }),
Expand All @@ -92,6 +93,7 @@ test('Click on "Copy dashboard URL" and succeed', async () => {
<Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
<ShareMenuItems {...props} />
</Menu>,
{ useRedux: true },
);

await waitFor(() => {
Expand Down Expand Up @@ -119,6 +121,7 @@ test('Click on "Copy dashboard URL" and fail', async () => {
<Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
<ShareMenuItems {...props} />
</Menu>,
{ useRedux: true },
);

await waitFor(() => {
Expand Down Expand Up @@ -147,6 +150,7 @@ test('Click on "Share dashboard by email" and succeed', async () => {
<Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
<ShareMenuItems {...props} />
</Menu>,
{ useRedux: true },
);

await waitFor(() => {
Expand Down Expand Up @@ -177,6 +181,7 @@ test('Click on "Share dashboard by email" and fail', async () => {
<Menu onClick={jest.fn()} selectable={false} data-test="main-menu">
<ShareMenuItems {...props} />
</Menu>,
{ useRedux: true },
);

await waitFor(() => {
Expand Down
20 changes: 10 additions & 10 deletions superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import React from 'react';
import copyTextToClipboard from 'src/utils/copy';
import { t, logging } from '@superset-ui/core';
import { Menu } from 'src/components/Menu';
import { getDashboardPermalink, getUrlParam } from 'src/utils/urlUtils';
import { URL_PARAMS } from 'src/constants';
import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';
import { getDashboardPermalink } from 'src/utils/urlUtils';
import { RootState } from 'src/dashboard/types';
import { useSelector } from 'react-redux';

interface ShareMenuItemProps {
url?: string;
Expand All @@ -48,17 +48,17 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
dashboardComponentId,
...rest
} = props;
const { dataMask, activeTabs } = useSelector((state: RootState) => ({
dataMask: state.dataMask,
activeTabs: state.dashboardState.activeTabs,
}));

async function generateUrl() {
const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey);
let filterState = {};
if (nativeFiltersKey && dashboardId) {
filterState = await getFilterValue(dashboardId, nativeFiltersKey);
}
return getDashboardPermalink({
dashboardId,
filterState,
hash: dashboardComponentId,
dataMask,
activeTabs,
anchor: dashboardComponentId,
});
}

Expand Down
26 changes: 14 additions & 12 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ import {
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { canUserEditDashboard } from 'src/dashboard/util/permissionUtils';
import { getFilterSets } from '../actions/nativeFilters';
import { setDatasetsStatus } from '../actions/dashboardState';
import { getFilterSets } from 'src/dashboard/actions/nativeFilters';
import { setDatasetsStatus } from 'src/dashboard/actions/dashboardState';
import {
getFilterValue,
getPermalinkValue,
} from '../components/nativeFilters/FilterBar/keyValue';
import { filterCardPopoverStyle } from '../styles';
} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';
import { filterCardPopoverStyle } from 'src/dashboard/styles';

export const MigrationContext = React.createContext(
FILTER_BOX_MIGRATION_STATES.NOOP,
Expand Down Expand Up @@ -183,19 +183,20 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
async function getDataMaskApplied() {
const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey);
const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey);
let dataMaskFromUrl = nativeFilterKeyValue || {};

const isOldRison = getUrlParam(URL_PARAMS.nativeFilters);

let dataMask = nativeFilterKeyValue || {};
let activeTabs: string[] | undefined = [];
if (permalinkKey) {
const permalinkValue = await getPermalinkValue(permalinkKey);
if (permalinkValue) {
dataMaskFromUrl = permalinkValue.state.filterState;
({ dataMask, activeTabs } = permalinkValue.state);
}
} else if (nativeFilterKeyValue) {
dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue);
dataMask = await getFilterValue(id, nativeFilterKeyValue);
}
if (isOldRison) {
dataMaskFromUrl = isOldRison;
dataMask = isOldRison;
}

if (readyToRender) {
Expand All @@ -207,12 +208,13 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
}
}
dispatch(
hydrateDashboard(
hydrateDashboard({
dashboard,
charts,
activeTabs,
filterboxMigrationState,
dataMaskFromUrl,
),
dataMask,
}),
);
}
return null;
Expand Down

0 comments on commit cadd259

Please sign in to comment.