From b7a0559aaf5ff4266baf5069b93379fbecfb4a00 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Thu, 17 Mar 2022 01:15:52 +0200 Subject: [PATCH] feat: add permalink to dashboard and explore (#19078) * 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 --- UPDATING.md | 3 +- .../components/AnchorLink/AnchorLink.test.jsx | 8 +- .../src/components/AnchorLink/index.jsx | 11 +- .../URLShortLinkButton.test.tsx | 52 ++++-- .../components/URLShortLinkButton/index.jsx | 27 ++- superset-frontend/src/constants.ts | 16 ++ .../HeaderActionsDropdown.test.tsx | 4 +- .../Header/HeaderActionsDropdown/index.jsx | 4 +- .../components/SliceHeaderControls/index.tsx | 7 +- .../components/gridComponents/Tab.jsx | 2 + .../ShareMenuItems/ShareMenuItems.test.tsx | 16 +- .../components/menu/ShareMenuItems/index.tsx | 66 ++----- .../nativeFilters/FilterBar/index.tsx | 13 +- .../nativeFilters/FilterBar/keyValue.tsx | 13 +- .../dashboard/containers/DashboardPage.tsx | 14 +- superset-frontend/src/dashboard/types.ts | 8 + .../explore/components/EmbedCodeButton.jsx | 25 ++- .../components/EmbedCodeButton.test.jsx | 10 +- .../components/ExploreActionButtons.tsx | 21 ++- .../components/ExploreViewContainer/index.jsx | 10 +- .../src/hooks/useUrlShortener.ts | 39 ---- superset-frontend/src/utils/urlUtils.ts | 87 ++++++++- superset/config.py | 3 + superset/dashboards/filter_state/api.py | 8 +- .../filter_state/commands/create.py | 10 +- .../filter_state/commands/delete.py | 14 +- .../dashboards/filter_state/commands/get.py | 8 +- .../filter_state/commands/update.py | 14 +- .../permalink}/__init__.py | 0 superset/dashboards/permalink/api.py | 171 +++++++++++++++++ .../permalink/commands}/__init__.py | 0 .../dashboards/permalink/commands/base.py | 23 +++ .../dashboards/permalink/commands/create.py | 61 ++++++ superset/dashboards/permalink/commands/get.py | 65 +++++++ superset/dashboards/permalink/exceptions.py | 31 ++++ superset/dashboards/permalink/schemas.py | 40 ++++ .../permalink/types.py} | 17 +- superset/explore/form_data/api.py | 10 +- superset/explore/form_data/commands/create.py | 13 +- superset/explore/form_data/commands/delete.py | 14 +- superset/explore/form_data/commands/get.py | 6 +- superset/explore/form_data/commands/update.py | 19 +- superset/explore/permalink/__init__.py | 16 ++ superset/explore/permalink/api.py | 174 ++++++++++++++++++ .../explore/permalink/commands/__init__.py | 16 ++ superset/explore/permalink/commands/base.py | 23 +++ superset/explore/permalink/commands/create.py | 60 ++++++ superset/explore/permalink/commands/get.py | 66 +++++++ superset/explore/permalink/exceptions.py | 31 ++++ superset/explore/permalink/schemas.py | 37 ++++ superset/explore/permalink/types.py | 29 +++ superset/explore/{form_data => }/utils.py | 0 superset/initialization/__init__.py | 4 + superset/key_value/commands/create.py | 60 +++++- superset/key_value/commands/delete.py | 51 ++++- superset/key_value/commands/get.py | 51 +++-- superset/key_value/commands/update.py | 70 +++++-- .../key_value/{commands => }/exceptions.py | 5 + superset/key_value/models.py | 38 ++++ superset/key_value/types.py | 34 ++++ superset/key_value/utils.py | 43 ++++- .../6766938c6065_add_key_value_store.py | 61 ++++++ superset/temporary_cache/__init__.py | 16 ++ .../{key_value => temporary_cache}/api.py | 27 +-- superset/temporary_cache/commands/__init__.py | 16 ++ superset/temporary_cache/commands/create.py | 45 +++++ superset/temporary_cache/commands/delete.py | 45 +++++ .../commands/entry.py | 0 .../temporary_cache/commands/exceptions.py | 45 +++++ superset/temporary_cache/commands/get.py | 46 +++++ .../commands/parameters.py | 0 superset/temporary_cache/commands/update.py | 48 +++++ superset/temporary_cache/schemas.py | 37 ++++ superset/temporary_cache/utils.py | 28 +++ superset/views/core.py | 55 +++++- superset/views/redirects.py | 21 +-- tests/integration_tests/core_tests.py | 24 --- .../dashboards/filter_state/api_tests.py | 101 +++++----- .../dashboards/permalink/__init__.py | 16 ++ .../dashboards/permalink/api_tests.py | 90 +++++++++ .../explore/form_data/api_tests.py | 114 +++++++----- .../explore/permalink/__init__.py | 16 ++ .../explore/permalink/api_tests.py | 117 ++++++++++++ tests/integration_tests/fixtures/client.py | 26 +++ tests/integration_tests/key_value/__init__.py | 16 ++ .../key_value/commands/__init__.py | 16 ++ .../key_value/commands/create_test.py | 64 +++++++ .../key_value/commands/delete_test.py | 91 +++++++++ .../key_value/commands/fixtures.py | 62 +++++++ .../key_value/commands/get_test.py | 100 ++++++++++ .../key_value/commands/update_test.py | 87 +++++++++ .../explore/{form_data => }/utils_test.py | 26 +-- tests/unit_tests/key_value/__init__.py | 16 ++ tests/unit_tests/key_value/utils_test.py | 117 ++++++++++++ 94 files changed, 2942 insertions(+), 438 deletions(-) delete mode 100644 superset-frontend/src/hooks/useUrlShortener.ts rename superset/{key_value/commands => dashboards/permalink}/__init__.py (100%) create mode 100644 superset/dashboards/permalink/api.py rename {tests/unit_tests/explore/form_data => superset/dashboards/permalink/commands}/__init__.py (100%) create mode 100644 superset/dashboards/permalink/commands/base.py create mode 100644 superset/dashboards/permalink/commands/create.py create mode 100644 superset/dashboards/permalink/commands/get.py create mode 100644 superset/dashboards/permalink/exceptions.py create mode 100644 superset/dashboards/permalink/schemas.py rename superset/{key_value/schemas.py => dashboards/permalink/types.py} (69%) create mode 100644 superset/explore/permalink/__init__.py create mode 100644 superset/explore/permalink/api.py create mode 100644 superset/explore/permalink/commands/__init__.py create mode 100644 superset/explore/permalink/commands/base.py create mode 100644 superset/explore/permalink/commands/create.py create mode 100644 superset/explore/permalink/commands/get.py create mode 100644 superset/explore/permalink/exceptions.py create mode 100644 superset/explore/permalink/schemas.py create mode 100644 superset/explore/permalink/types.py rename superset/explore/{form_data => }/utils.py (100%) rename superset/key_value/{commands => }/exceptions.py (90%) create mode 100644 superset/key_value/models.py create mode 100644 superset/key_value/types.py create mode 100644 superset/migrations/versions/6766938c6065_add_key_value_store.py create mode 100644 superset/temporary_cache/__init__.py rename superset/{key_value => temporary_cache}/api.py (87%) create mode 100644 superset/temporary_cache/commands/__init__.py create mode 100644 superset/temporary_cache/commands/create.py create mode 100644 superset/temporary_cache/commands/delete.py rename superset/{key_value => temporary_cache}/commands/entry.py (100%) create mode 100644 superset/temporary_cache/commands/exceptions.py create mode 100644 superset/temporary_cache/commands/get.py rename superset/{key_value => temporary_cache}/commands/parameters.py (100%) create mode 100644 superset/temporary_cache/commands/update.py create mode 100644 superset/temporary_cache/schemas.py create mode 100644 superset/temporary_cache/utils.py create mode 100644 tests/integration_tests/dashboards/permalink/__init__.py create mode 100644 tests/integration_tests/dashboards/permalink/api_tests.py create mode 100644 tests/integration_tests/explore/permalink/__init__.py create mode 100644 tests/integration_tests/explore/permalink/api_tests.py create mode 100644 tests/integration_tests/fixtures/client.py create mode 100644 tests/integration_tests/key_value/__init__.py create mode 100644 tests/integration_tests/key_value/commands/__init__.py create mode 100644 tests/integration_tests/key_value/commands/create_test.py create mode 100644 tests/integration_tests/key_value/commands/delete_test.py create mode 100644 tests/integration_tests/key_value/commands/fixtures.py create mode 100644 tests/integration_tests/key_value/commands/get_test.py create mode 100644 tests/integration_tests/key_value/commands/update_test.py rename tests/unit_tests/explore/{form_data => }/utils_test.py (88%) create mode 100644 tests/unit_tests/key_value/__init__.py create mode 100644 tests/unit_tests/key_value/utils_test.py diff --git a/UPDATING.md b/UPDATING.md index ea9f02094a52..854e18074d9e 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -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/). diff --git a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx index 9f0c05a8eb87..3f05416b1c0c 100644 --- a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx +++ b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx @@ -25,6 +25,7 @@ import URLShortLinkButton from 'src/components/URLShortLinkButton'; describe('AnchorLink', () => { const props = { anchorLinkId: 'CHART-123', + dashboardId: 10, }; const globalLocation = window.location; @@ -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); }); }); diff --git a/superset-frontend/src/components/AnchorLink/index.jsx b/superset-frontend/src/components/AnchorLink/index.jsx index 743cb3a3c649..71ba76dff7a0 100644 --- a/superset-frontend/src/components/AnchorLink/index.jsx +++ b/superset-frontend/src/components/AnchorLink/index.jsx @@ -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, @@ -70,17 +70,14 @@ class AnchorLink extends React.PureComponent { } render() { - const { anchorLinkId, filters, showShortLinkButton, placement } = + const { anchorLinkId, dashboardId, showShortLinkButton, placement } = this.props; return ( {showShortLinkButton && ( { - render(, { useRedux: true }); + render(, { useRedux: true }); expect(screen.getByRole('button')).toBeInTheDocument(); }); test('renders overlay on click', async () => { - render(, { useRedux: true }); + render(, { useRedux: true }); userEvent.click(screen.getByRole('button')); expect(await screen.findByRole('tooltip')).toBeInTheDocument(); }); test('obtains short url', async () => { - render(, { useRedux: true }); + render(, { 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(, { - useRedux: true, - }); + render( + , + { + 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( <> - + , { useRedux: true }, diff --git a/superset-frontend/src/components/URLShortLinkButton/index.jsx b/superset-frontend/src/components/URLShortLinkButton/index.jsx index 1678471b61f7..35795f81a11f 100644 --- a/superset-frontend/src/components/URLShortLinkButton/index.jsx +++ b/superset-frontend/src/components/URLShortLinkButton/index.jsx @@ -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']), }; @@ -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() { @@ -96,7 +110,6 @@ class URLShortLinkButton extends React.Component { } URLShortLinkButton.defaultProps = { - url: window.location.href.substring(window.location.origin.length), placement: 'left', emailSubject: '', emailContent: '', diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index b54fc1173c28..777d5f2a4e43 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -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. */ diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx index 899664507947..d1f87ec999e0 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx @@ -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 () => { diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index b7d368ec32db..9375c684af90 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -257,8 +257,8 @@ class HeaderActionsDropdown extends React.PureComponent { {userCanShare && ( ; onExploreChart: () => void; forceRefresh: (sliceId: number, dashboardId: number) => void; @@ -309,8 +310,8 @@ class SliceHeaderControls extends React.PureComponent< {supersetCanShare && ( = 5 ? 'left' : 'right'} 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 da7d196bd8b5..579f9d4b6907 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/${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', @@ -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, }, ); }); @@ -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); @@ -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( @@ -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 }, ); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index cb31503ac861..c70e47dc3d01 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -17,19 +17,16 @@ * under the License. */ import React from 'react'; -import { useUrlShortener } from 'src/hooks/useUrlShortener'; import copyTextToClipboard from 'src/utils/copy'; -import { t, logging } from '@superset-ui/core'; +import { t, logging, QueryFormData } from '@superset-ui/core'; import { Menu } from 'src/components/Menu'; -import { getUrlParam } from 'src/utils/urlUtils'; -import { postFormData } from 'src/explore/exploreUtils/formData'; -import { useTabId } from 'src/hooks/useTabId'; -import { URL_PARAMS } from 'src/constants'; -import { mountExploreUrl } from 'src/explore/exploreUtils'; import { - createFilterKey, - getFilterValue, -} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; + getChartPermalink, + getDashboardPermalink, + getUrlParam, +} from 'src/utils/urlUtils'; +import { RESERVED_DASHBOARD_URL_PARAMS, URL_PARAMS } from 'src/constants'; +import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; interface ShareMenuItemProps { url?: string; @@ -40,12 +37,11 @@ interface ShareMenuItemProps { addDangerToast: Function; addSuccessToast: Function; dashboardId?: string; - formData?: { slice_id: number; datasource: string }; + formData?: Pick; } const ShareMenuItems = (props: ShareMenuItemProps) => { const { - url, copyMenuItemTitle, emailMenuItemTitle, emailSubject, @@ -57,47 +53,25 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { ...rest } = props; - const tabId = useTabId(); - - const getShortUrl = useUrlShortener(url || ''); - - async function getCopyUrl() { - const risonObj = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof risonObj === 'object' || !dashboardId) return null; - const prevData = await getFilterValue( - dashboardId, - getUrlParam(URL_PARAMS.nativeFiltersKey), - ); - const newDataMaskKey = await createFilterKey( - dashboardId, - JSON.stringify(prevData), - tabId, - ); - const newUrl = new URL(`${window.location.origin}${url}`); - newUrl.searchParams.set(URL_PARAMS.nativeFilters.name, newDataMaskKey); - return `${newUrl.pathname}${newUrl.search}`; - } - async function generateUrl() { + // chart if (formData) { - const key = await postFormData( - parseInt(formData.datasource.split('_')[0], 10), - formData, - formData.slice_id, - tabId, - ); - return `${window.location.origin}${mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - [URL_PARAMS.sliceId.name]: formData.slice_id, - })}`; + // we need to remove reserved dashboard url params + return getChartPermalink(formData, RESERVED_DASHBOARD_URL_PARAMS); + } + // dashboard + const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + let filterState = {}; + if (nativeFiltersKey && dashboardId) { + filterState = await getFilterValue(dashboardId, nativeFiltersKey); } - const copyUrl = await getCopyUrl(); - return getShortUrl(copyUrl); + return getDashboardPermalink(String(dashboardId), filterState); } async function onCopyLink() { try { - await copyTextToClipboard(await generateUrl()); + const url = await generateUrl(); + await copyTextToClipboard(url); addSuccessToast(t('Copied to clipboard!')); } catch (error) { logging.error(error); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 73a589312acc..309d75dac9a8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -165,6 +165,11 @@ export interface FiltersBarProps { offset: number; } +const EXCLUDED_URL_PARAMS: string[] = [ + URL_PARAMS.nativeFilters.name, + URL_PARAMS.permalinkKey.name, +]; + const publishDataMask = debounce( async ( history, @@ -177,9 +182,9 @@ const publishDataMask = debounce( const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey: string; + let dataMaskKey: string | null; previousParams.forEach((value, key) => { - if (key !== URL_PARAMS.nativeFilters.name) { + if (!EXCLUDED_URL_PARAMS.includes(key)) { newParams.append(key, value); } }); @@ -200,7 +205,9 @@ const publishDataMask = debounce( } else { dataMaskKey = await createFilterKey(dashboardId, dataMask, tabId); } - newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + if (dataMaskKey) { + newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + } // pathname could be updated somewhere else through window.history // keep react router history in sync with window history diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx index 9682fdb7b8f0..ec9735f09169 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -17,6 +17,7 @@ * under the License. */ import { SupersetClient, logging } from '@superset-ui/core'; +import { DashboardPermalinkValue } from 'src/dashboard/types'; const assembleEndpoint = ( dashId: string | number, @@ -58,7 +59,7 @@ export const createFilterKey = ( endpoint: assembleEndpoint(dashId, undefined, tabId), jsonPayload: { value }, }) - .then(r => r.json.key) + .then(r => r.json.key as string) .catch(err => { logging.error(err); return null; @@ -73,3 +74,13 @@ export const getFilterValue = (dashId: string | number, key: string) => logging.error(err); return null; }); + +export const getPermalinkValue = (key: string) => + SupersetClient.get({ + endpoint: `/api/v1/dashboard/permalink/${key}`, + }) + .then(({ json }) => json as DashboardPermalinkValue) + .catch(err => { + logging.error(err); + return null; + }); diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 878f3d68695a..e5fff328724d 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -49,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 { + getFilterValue, + getPermalinkValue, +} from '../components/nativeFilters/FilterBar/keyValue'; import { filterCardPopoverStyle } from '../styles'; export const MigrationContext = React.createContext( @@ -161,12 +164,17 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { + const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey); const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey); let dataMaskFromUrl = nativeFilterKeyValue || {}; const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - // check if key from key_value api and get datamask - if (nativeFilterKeyValue) { + if (permalinkKey) { + const permalinkValue = await getPermalinkValue(permalinkKey); + if (permalinkValue) { + dataMaskFromUrl = permalinkValue.state.filterState; + } + } else if (nativeFilterKeyValue) { dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue); } if (isOldRison) { diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index fbdf362eea70..dffbd9fbe0be 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -144,3 +144,11 @@ type ActiveFilter = { export type ActiveFilters = { [key: string]: ActiveFilter; }; + +export type DashboardPermalinkValue = { + dashboardId: string; + state: { + filterState: DataMaskStateWithId; + hash: string; + }; +}; diff --git a/superset-frontend/src/explore/components/EmbedCodeButton.jsx b/superset-frontend/src/explore/components/EmbedCodeButton.jsx index 57e6d30de453..71f77a4621fa 100644 --- a/superset-frontend/src/explore/components/EmbedCodeButton.jsx +++ b/superset-frontend/src/explore/components/EmbedCodeButton.jsx @@ -25,6 +25,7 @@ import Icons from 'src/components/Icons'; import { Tooltip } from 'src/components/Tooltip'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { URL_PARAMS } from 'src/constants'; +import { getChartPermalink } from 'src/utils/urlUtils'; export default class EmbedCodeButton extends React.Component { constructor(props) { @@ -32,8 +33,11 @@ export default class EmbedCodeButton extends React.Component { this.state = { height: '400', width: '600', + url: '', + errorMessage: '', }; this.handleInputChange = this.handleInputChange.bind(this); + this.updateUrl = this.updateUrl.bind(this); } handleInputChange(e) { @@ -43,8 +47,21 @@ export default class EmbedCodeButton extends React.Component { this.setState(data); } + updateUrl() { + this.setState({ url: '' }); + getChartPermalink(this.props.formData) + .then(url => this.setState({ errorMessage: '', url })) + .catch(() => { + this.setState({ errorMessage: t('Error') }); + this.props.addDangerToast( + t('Sorry, something went wrong. Try again later.'), + ); + }); + } + generateEmbedHTML() { - const srcLink = `${window.location.href}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; + if (!this.state.url) return ''; + const srcLink = `${this.state.url}?${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; return ( '
@@ -67,7 +86,8 @@ export default class EmbedCodeButton extends React.Component {