Skip to content

Commit

Permalink
feat: add permalink to dashboard and explore (#19078)
Browse files Browse the repository at this point in the history
* rename key_value to temporary_cache

* add migration

* create new key_value package

* add commands

* lots of new stuff

* fix schema reference

* remove redundant filter state from bootstrap data

* add missing license headers

* fix pylint

* fix dashboard permalink access

* use valid json mocks for filter state tests

* fix temporary cache tests

* add anchors to dashboard state

* lint

* fix util test

* fix url shortlink button tests

* remove legacy shortner

* remove unused imports

* fix js tests

* fix test

* add native filter state to anchor link

* add UPDATING.md section

* address comments

* address comments

* lint

* fix test

* add utils tests + other test stubs

* add key_value integration tests

* add filter box state to permalink state

* fully support persisting url parameters

* lint, add redirects and a few integration tests

* fix test + clean up trailing comma

* fix anchor bug

* change value to LargeBinary to support persisting binary values

* fix urlParams type and simplify urlencode

* lint

* add optional entry expiration

* fix incorrect chart id + add test
  • Loading branch information
villebro committed Mar 16, 2022
1 parent d01fdad commit b7a0559
Show file tree
Hide file tree
Showing 94 changed files with 2,942 additions and 438 deletions.
3 changes: 2 additions & 1 deletion UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in conf

### Deprecations

- [19078](https://github.com/apache/superset/pull/19078): Creation of old shorturl links has been deprecated in favor of a new permalink feature that solves the long url problem (old shorturls will still work, though!). By default, new permalinks use UUID4 as the key. However, to use serial ids similar to the old shorturls, add the following to your `superset_config.py`: `PERMALINK_KEY_TYPE = "id"`.
- [18960](https://github.com/apache/superset/pull/18960): Persisting URL params in chart metadata is no longer supported. To set a default value for URL params in Jinja code, use the optional second argument: `url_param("my-param", "my-default-value")`.

### Other

- [17589](https://github.com/apache/incubator-superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager.
- [17589](https://github.com/apache/superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager.
- [17536](https://github.com/apache/superset/pull/17536): introduced a key-value endpoint to store dashboard filter state. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `FILTER_STATE_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/).
- [17882](https://github.com/apache/superset/pull/17882): introduced a key-value endpoint to store Explore form data. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `EXPLORE_FORM_DATA_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import URLShortLinkButton from 'src/components/URLShortLinkButton';
describe('AnchorLink', () => {
const props = {
anchorLinkId: 'CHART-123',
dashboardId: 10,
};

const globalLocation = window.location;
Expand Down Expand Up @@ -64,8 +65,9 @@ describe('AnchorLink', () => {
expect(wrapper.find(URLShortLinkButton)).toExist();
expect(wrapper.find(URLShortLinkButton)).toHaveProp({ placement: 'right' });

const targetUrl = wrapper.find(URLShortLinkButton).prop('url');
const hash = targetUrl.slice(targetUrl.indexOf('#') + 1);
expect(hash).toBe(props.anchorLinkId);
const anchorLinkId = wrapper.find(URLShortLinkButton).prop('anchorLinkId');
const dashboardId = wrapper.find(URLShortLinkButton).prop('dashboardId');
expect(anchorLinkId).toBe(props.anchorLinkId);
expect(dashboardId).toBe(props.dashboardId);
});
});
11 changes: 4 additions & 7 deletions superset-frontend/src/components/AnchorLink/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import PropTypes from 'prop-types';
import { t } from '@superset-ui/core';

import URLShortLinkButton from 'src/components/URLShortLinkButton';
import getDashboardUrl from 'src/dashboard/util/getDashboardUrl';
import getLocationHash from 'src/dashboard/util/getLocationHash';

const propTypes = {
anchorLinkId: PropTypes.string.isRequired,
dashboardId: PropTypes.number,
filters: PropTypes.object,
showShortLinkButton: PropTypes.bool,
inFocus: PropTypes.bool,
Expand Down Expand Up @@ -70,17 +70,14 @@ class AnchorLink extends React.PureComponent {
}

render() {
const { anchorLinkId, filters, showShortLinkButton, placement } =
const { anchorLinkId, dashboardId, showShortLinkButton, placement } =
this.props;
return (
<span className="anchor-link-container" id={anchorLinkId}>
{showShortLinkButton && (
<URLShortLinkButton
url={getDashboardUrl({
pathname: window.location.pathname,
filters,
hash: anchorLinkId,
})}
anchorLinkId={anchorLinkId}
dashboardId={dashboardId}
emailSubject={t('Superset chart')}
emailContent={t('Check out this chart in dashboard:')}
placement={placement}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,48 +23,76 @@ import fetchMock from 'fetch-mock';
import URLShortLinkButton from 'src/components/URLShortLinkButton';
import ToastContainer from 'src/components/MessageToasts/ToastContainer';

const fakeUrl = 'http://fakeurl.com';
const DASHBOARD_ID = 10;
const PERMALINK_PAYLOAD = {
key: '123',
url: 'http://fakeurl.com/123',
};
const FILTER_STATE_PAYLOAD = {
value: '{}',
};

fetchMock.post('glob:*/r/shortner/', fakeUrl);
const props = {
dashboardId: DASHBOARD_ID,
};

fetchMock.get(
`glob:*/api/v1/dashboard/${DASHBOARD_ID}/filter_state*`,
FILTER_STATE_PAYLOAD,
);

fetchMock.post(
`glob:*/api/v1/dashboard/${DASHBOARD_ID}/permalink`,
PERMALINK_PAYLOAD,
);

test('renders with default props', () => {
render(<URLShortLinkButton />, { useRedux: true });
render(<URLShortLinkButton {...props} />, { useRedux: true });
expect(screen.getByRole('button')).toBeInTheDocument();
});

test('renders overlay on click', async () => {
render(<URLShortLinkButton />, { useRedux: true });
render(<URLShortLinkButton {...props} />, { useRedux: true });
userEvent.click(screen.getByRole('button'));
expect(await screen.findByRole('tooltip')).toBeInTheDocument();
});

test('obtains short url', async () => {
render(<URLShortLinkButton />, { useRedux: true });
render(<URLShortLinkButton {...props} />, { useRedux: true });
userEvent.click(screen.getByRole('button'));
expect(await screen.findByRole('tooltip')).toHaveTextContent(fakeUrl);
expect(await screen.findByRole('tooltip')).toHaveTextContent(
PERMALINK_PAYLOAD.url,
);
});

test('creates email anchor', async () => {
const subject = 'Subject';
const content = 'Content';

render(<URLShortLinkButton emailSubject={subject} emailContent={content} />, {
useRedux: true,
});
render(
<URLShortLinkButton
{...props}
emailSubject={subject}
emailContent={content}
/>,
{
useRedux: true,
},
);

const href = `mailto:?Subject=${subject}%20&Body=${content}${fakeUrl}`;
const href = `mailto:?Subject=${subject}%20&Body=${content}${PERMALINK_PAYLOAD.url}`;
userEvent.click(screen.getByRole('button'));
expect(await screen.findByRole('link')).toHaveAttribute('href', href);
});

test('renders error message on short url error', async () => {
fetchMock.mock('glob:*/r/shortner/', 500, {
fetchMock.mock(`glob:*/api/v1/dashboard/${DASHBOARD_ID}/permalink`, 500, {
overwriteRoutes: true,
});

render(
<>
<URLShortLinkButton />
<URLShortLinkButton {...props} />
<ToastContainer />
</>,
{ useRedux: true },
Expand Down
27 changes: 20 additions & 7 deletions superset-frontend/src/components/URLShortLinkButton/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ import PropTypes from 'prop-types';
import { t } from '@superset-ui/core';
import Popover from 'src/components/Popover';
import CopyToClipboard from 'src/components/CopyToClipboard';
import { getShortUrl } from 'src/utils/urlUtils';
import { getDashboardPermalink, getUrlParam } from 'src/utils/urlUtils';
import withToasts from 'src/components/MessageToasts/withToasts';
import { URL_PARAMS } from 'src/constants';
import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';

const propTypes = {
url: PropTypes.string,
addDangerToast: PropTypes.func.isRequired,
anchorLinkId: PropTypes.string,
dashboardId: PropTypes.number,
emailSubject: PropTypes.string,
emailContent: PropTypes.string,
addDangerToast: PropTypes.func.isRequired,
placement: PropTypes.oneOf(['right', 'left', 'top', 'bottom']),
};

Expand All @@ -50,9 +53,20 @@ class URLShortLinkButton extends React.Component {

getCopyUrl(e) {
e.stopPropagation();
getShortUrl(this.props.url)
.then(this.onShortUrlSuccess)
.catch(this.props.addDangerToast);
const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey);
if (this.props.dashboardId) {
getFilterValue(this.props.dashboardId, nativeFiltersKey)
.then(filterState =>
getDashboardPermalink(
String(this.props.dashboardId),
filterState,
this.props.anchorLinkId,
)
.then(this.onShortUrlSuccess)
.catch(this.props.addDangerToast),
)
.catch(this.props.addDangerToast);
}
}

renderPopover() {
Expand Down Expand Up @@ -96,7 +110,6 @@ class URLShortLinkButton extends React.Component {
}

URLShortLinkButton.defaultProps = {
url: window.location.href.substring(window.location.origin.length),
placement: 'left',
emailSubject: '',
emailContent: '',
Expand Down
16 changes: 16 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,24 @@ export const URL_PARAMS = {
name: 'force',
type: 'boolean',
},
permalinkKey: {
name: 'permalink_key',
type: 'string',
},
} as const;

export const RESERVED_CHART_URL_PARAMS: string[] = [
URL_PARAMS.formDataKey.name,
URL_PARAMS.sliceId.name,
URL_PARAMS.datasetId.name,
];
export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [
URL_PARAMS.nativeFilters.name,
URL_PARAMS.nativeFiltersKey.name,
URL_PARAMS.permalinkKey.name,
URL_PARAMS.preselectFilters.name,
];

/**
* Faster debounce delay for inputs without expensive operation.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ test('should show the share actions', async () => {
};
render(setup(canShareProps));
await openDropdown();
expect(screen.getByText('Copy dashboard URL')).toBeInTheDocument();
expect(screen.getByText('Share dashboard by email')).toBeInTheDocument();
expect(screen.getByText('Copy permalink to clipboard')).toBeInTheDocument();
expect(screen.getByText('Share permalink by email')).toBeInTheDocument();
});

test('should render the "Save Modal" when user can save', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ class HeaderActionsDropdown extends React.PureComponent {
{userCanShare && (
<ShareMenuItems
url={url}
copyMenuItemTitle={t('Copy dashboard URL')}
emailMenuItemTitle={t('Share dashboard by email')}
copyMenuItemTitle={t('Copy permalink to clipboard')}
emailMenuItemTitle={t('Share permalink by email')}
emailSubject={emailSubject}
emailBody={emailBody}
addSuccessToast={addSuccessToast}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import moment from 'moment';
import {
Behavior,
getChartMetadataRegistry,
QueryFormData,
styled,
t,
} from '@superset-ui/core';
Expand Down Expand Up @@ -98,7 +99,7 @@ export interface SliceHeaderControlsProps {
isExpanded?: boolean;
updatedDttm: number | null;
isFullSize?: boolean;
formData: { slice_id: number; datasource: string };
formData: Pick<QueryFormData, 'slice_id' | 'datasource'>;
onExploreChart: () => void;

forceRefresh: (sliceId: number, dashboardId: number) => void;
Expand Down Expand Up @@ -309,8 +310,8 @@ class SliceHeaderControls extends React.PureComponent<

{supersetCanShare && (
<ShareMenuItems
copyMenuItemTitle={t('Copy chart URL')}
emailMenuItemTitle={t('Share chart by email')}
copyMenuItemTitle={t('Copy permalink to clipboard')}
emailMenuItemTitle={t('Share permalink by email')}
emailSubject={t('Superset chart')}
emailBody={t('Check out this chart: ')}
addSuccessToast={addSuccessToast}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const RENDER_TAB = 'RENDER_TAB';
export const RENDER_TAB_CONTENT = 'RENDER_TAB_CONTENT';

const propTypes = {
dashboardId: PropTypes.number.isRequired,
id: PropTypes.string.isRequired,
parentId: PropTypes.string.isRequired,
component: componentShape.isRequired,
Expand Down Expand Up @@ -237,6 +238,7 @@ export default class Tab extends React.PureComponent {
{!editMode && (
<AnchorLink
anchorLinkId={component.id}
dashboardId={this.props.dashboardId}
filters={filters}
showShortLinkButton
placement={index >= 5 ? 'left' : 'right'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const DASHBOARD_ID = '26';
const createProps = () => ({
addDangerToast: jest.fn(),
addSuccessToast: jest.fn(),
url: `/superset/dashboard/${DASHBOARD_ID}/?preselect_filters=%7B%7D`,
url: `/superset/dashboard/${DASHBOARD_ID}`,
copyMenuItemTitle: 'Copy dashboard URL',
emailMenuItemTitle: 'Share dashboard by email',
emailSubject: 'Superset dashboard COVID Vaccine Dashboard',
Expand All @@ -45,10 +45,10 @@ beforeAll((): void => {
// @ts-ignore
delete window.location;
fetchMock.post(
'http://localhost/r/shortner/',
{ body: 'http://localhost:8088/r/3' },
`http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`,
{ key: '123', url: 'http://localhost/superset/dashboard/p/123/' },
{
sendAsJson: false,
sendAsJson: true,
},
);
});
Expand Down Expand Up @@ -104,7 +104,7 @@ test('Click on "Copy dashboard URL" and succeed', async () => {

await waitFor(() => {
expect(spy).toBeCalledTimes(1);
expect(spy).toBeCalledWith('http://localhost:8088/r/3');
expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/');
expect(props.addSuccessToast).toBeCalledTimes(1);
expect(props.addSuccessToast).toBeCalledWith('Copied to clipboard!');
expect(props.addDangerToast).toBeCalledTimes(0);
Expand All @@ -130,7 +130,7 @@ test('Click on "Copy dashboard URL" and fail', async () => {

await waitFor(() => {
expect(spy).toBeCalledTimes(1);
expect(spy).toBeCalledWith('http://localhost:8088/r/3');
expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/');
expect(props.addSuccessToast).toBeCalledTimes(0);
expect(props.addDangerToast).toBeCalledTimes(1);
expect(props.addDangerToast).toBeCalledWith(
Expand Down Expand Up @@ -159,14 +159,14 @@ test('Click on "Share dashboard by email" and succeed', async () => {
await waitFor(() => {
expect(props.addDangerToast).toBeCalledTimes(0);
expect(window.location.href).toBe(
'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%3A8088%2Fr%2F3',
'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%2Fsuperset%2Fdashboard%2Fp%2F123%2F',
);
});
});

test('Click on "Share dashboard by email" and fail', async () => {
fetchMock.post(
'http://localhost/r/shortner/',
`http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`,
{ status: 404 },
{ overwriteRoutes: true },
);
Expand Down

0 comments on commit b7a0559

Please sign in to comment.