From e3e03d202db959d708acbfe8a25a50dddc85fa7f Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Tue, 15 Mar 2022 06:29:18 +1300 Subject: [PATCH 01/17] chore: Deprecate Python 3.7 (#19017) * chore: Deprecate Python 3.7 and add support for Python 3.10 * Update local-backend.mdx * Update UPDATING.md Co-authored-by: John Bodley --- Makefile | 6 +++--- UPDATING.md | 1 + docs/docs/contributing/local-backend.mdx | 2 +- setup.py | 3 +-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 47d0f5b55824a..8dc7ffa8f569e 100644 --- a/Makefile +++ b/Makefile @@ -15,8 +15,8 @@ # limitations under the License. # -# Python version installed; we need 3.7-3.9 -PYTHON=`command -v python3.9 || command -v python3.8 || command -v python3.7` +# Python version installed; we need 3.8-3.9 +PYTHON=`command -v python3.9 || command -v python3.8` .PHONY: install superset venv pre-commit @@ -70,7 +70,7 @@ update-js: venv: # Create a virtual environment and activate it (recommended) - if ! [ -x "${PYTHON}" ]; then echo "You need Python 3.7, 3.8 or 3.9 installed"; exit 1; fi + if ! [ -x "${PYTHON}" ]; then echo "You need Python 3.8 or 3.9 installed"; exit 1; fi test -d venv || ${PYTHON} -m venv venv # setup a python3 virtualenv . venv/bin/activate diff --git a/UPDATING.md b/UPDATING.md index 977a63ac54b16..ea9f02094a52c 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -36,6 +36,7 @@ assists people when migrating to a new version. - [17539](https://github.com/apache/superset/pull/17539): all Superset CLI commands (init, load_examples and etc) require setting the FLASK_APP environment variable (which is set by default when `.flaskenv` is loaded) - [18970](https://github.com/apache/superset/pull/18970): Changes feature flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in config.py to True, thus disabling the feature from being shown in the client. +- [19017](https://github.com/apache/superset/pull/19017): Removes Python 3.7 support. ### Potential Downtime diff --git a/docs/docs/contributing/local-backend.mdx b/docs/docs/contributing/local-backend.mdx index 7c03c0dc50db4..1da5b8b3e1ba4 100644 --- a/docs/docs/contributing/local-backend.mdx +++ b/docs/docs/contributing/local-backend.mdx @@ -12,7 +12,7 @@ version: 1 Make sure your machine meets the [OS dependencies](https://superset.apache.org/docs/installation/installing-superset-from-scratch#os-dependencies) before following these steps. You also need to install MySQL or [MariaDB](https://mariadb.com/downloads). -Ensure that you are using Python version 3.7 or 3.8, then proceed with: +Ensure that you are using Python version 3.8 or 3.9, then proceed with: ````bash # Create a virtual environment and activate it (recommended) diff --git a/setup.py b/setup.py index 9899daf4009fa..b091cef5fa8e3 100644 --- a/setup.py +++ b/setup.py @@ -167,13 +167,12 @@ def get_git_sha() -> str: "vertica": ["sqlalchemy-vertica-python>=0.5.9, < 0.6"], "netezza": ["nzalchemy>=11.0.2"], }, - python_requires="~=3.7", + python_requires="~=3.8", author="Apache Software Foundation", author_email="dev@superset.apache.org", url="https://superset.apache.org/", download_url="https://www.apache.org/dist/superset/" + version_string, classifiers=[ - "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", ], From 981f09b5db77fc2348a508da49cdddf1fcfcb3c6 Mon Sep 17 00:00:00 2001 From: Jinghuayao <81597121+jinghua-qa@users.noreply.github.com> Date: Tue, 15 Mar 2022 08:16:06 -0700 Subject: [PATCH 02/17] test(native filter): add new native filter test (#19115) * add new native filter test --- .../dashboard/nativeFilters.test.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) 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 e0eccfa520150..433b4a65e33cb 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -529,6 +529,42 @@ describe('Nativefilters Sanity test', () => { .contains('year') .should('be.visible'); }); + it('User can create a value filter', () => { + cy.get(nativeFilters.filterFromDashboardView.expand).click({ force: true }); + cy.get(nativeFilters.filterFromDashboardView.createFilterButton) + .should('be.visible') + .click(); + cy.get(nativeFilters.modal.container).should('be.visible'); + cy.get('body').type('{home}'); + + cy.get(nativeFilters.filtersPanel.filterTypeInput) + .click({ scrollBehavior: false }) + .type('{home}Value{enter}', { scrollBehavior: false }); + cy.get(nativeFilters.filtersPanel.filterTypeInput) + .find(nativeFilters.filtersPanel.filterTypeItem) + .should('have.text', 'Value'); + cy.get(nativeFilters.modal.container) + .find(nativeFilters.filtersPanel.filterName) + .click({ scrollBehavior: false }) + .clear() + .type('country_name', { scrollBehavior: false }); + + cy.get(nativeFilters.silentLoading).should('not.exist'); + cy.get(nativeFilters.filtersPanel.filterInfoInput) + .last() + .should('be.visible') + .click({ force: true }); + cy.get(nativeFilters.filtersPanel.filterInfoInput) + .last() + .type('country_name {enter}'); + cy.get(nativeFilters.modal.footer) + .find(nativeFilters.modal.saveButton) + .should('be.visible') + .click({ force: true }); + cy.get(nativeFilters.filterFromDashboardView.filterName) + .should('be.visible', { timeout: 40000 }) + .contains('country_name'); + }); }); xdescribe('Nativefilters', () => { From 74910f99d8e1fe0c054780848927b4b54554cec9 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 16 Mar 2022 01:01:21 -0400 Subject: [PATCH 03/17] fix(dashboard): scrolling table viz overlaps next chart (#19121) --- .../components/gridComponents/Chart.jsx | 20 +++++++++++++++---- .../components/gridComponents/Chart.test.jsx | 8 ++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 7e7c8c5222980..239bb508deb06 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -128,6 +128,8 @@ export default class Chart extends React.Component { this.resize = this.resize.bind(this); this.setDescriptionRef = this.setDescriptionRef.bind(this); this.setHeaderRef = this.setHeaderRef.bind(this); + this.getChartHeight = this.getChartHeight.bind(this); + this.getDescriptionHeight = this.getDescriptionHeight.bind(this); } shouldComponentUpdate(nextProps, nextState) { @@ -178,21 +180,31 @@ export default class Chart extends React.Component { return this.props.cacheBusterProp !== nextProps.cacheBusterProp; } + componentDidMount() { + if (this.props.isExpanded) { + const descriptionHeight = this.getDescriptionHeight(); + this.setState({ descriptionHeight }); + } + } + componentWillUnmount() { clearTimeout(this.resizeTimeout); } componentDidUpdate(prevProps) { if (this.props.isExpanded !== prevProps.isExpanded) { - const descriptionHeight = - this.props.isExpanded && this.descriptionRef - ? this.descriptionRef.offsetHeight - : 0; + const descriptionHeight = this.getDescriptionHeight(); // eslint-disable-next-line react/no-did-update-set-state this.setState({ descriptionHeight }); } } + getDescriptionHeight() { + return this.props.isExpanded && this.descriptionRef + ? this.descriptionRef.offsetHeight + : 0; + } + getChartHeight() { const headerHeight = this.getHeaderHeight(); return this.state.height - headerHeight - this.state.descriptionHeight; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx index 153838271ad2e..59cbce2090354 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx @@ -93,6 +93,14 @@ describe('Chart', () => { expect(wrapper.find('.slice_description')).toExist(); }); + it('should calculate the description height if it has one and isExpanded=true', () => { + const spy = jest.spyOn(Chart.prototype, 'getDescriptionHeight'); + const wrapper = setup({ isExpanded: true }); + + expect(wrapper.find('.slice_description')).toExist(); + expect(spy).toHaveBeenCalled(); + }); + it('should call refreshChart when SliceHeader calls forceRefresh', () => { const refreshChart = sinon.spy(); const wrapper = setup({ refreshChart }); From 8e7af79aa9eadafe5def8dd7bac593f0b5f55776 Mon Sep 17 00:00:00 2001 From: smileydev <47900232+prosdev0107@users.noreply.github.com> Date: Wed, 16 Mar 2022 01:09:56 -0400 Subject: [PATCH 04/17] fix(dbs-dropdown): make to allow the search in supported db dropdown (#19125) --- .../src/views/CRUD/data/database/DatabaseModal/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 813ace469c160..81ae065b23de7 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -688,6 +688,7 @@ const DatabaseModal: FunctionComponent = ({ className="available-select" onChange={setDatabaseModel} placeholder={t('Choose a database...')} + showSearch > {[...(availableDbs?.databases || [])] ?.sort((a: DatabaseForm, b: DatabaseForm) => From 30c97ad5bfe1dc9b074efc294c647c32fdd05fe0 Mon Sep 17 00:00:00 2001 From: smileydev <47900232+prosdev0107@users.noreply.github.com> Date: Wed, 16 Mar 2022 01:10:55 -0400 Subject: [PATCH 05/17] fix(dashboard-css): make to stay custom css when reload (#19084) * fix(dashboard-css): make to stay custome css when reload * fix(dashboard-css): make to add injectCustomCSS into HeaderActionsDropdown.test.tsx --- .../HeaderActionsDropdown/HeaderActionsDropdown.test.tsx | 3 +++ .../components/Header/HeaderActionsDropdown/index.jsx | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) 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 8779f8ef2f064..8996645079472 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx @@ -21,6 +21,7 @@ import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; import { HeaderDropdownProps } from 'src/dashboard/components/Header/types'; +import injectCustomCss from 'src/dashboard/util/injectCustomCss'; import HeaderActionsDropdown from '.'; const createProps = () => ({ @@ -180,7 +181,9 @@ test('should NOT render the "Refresh dashboard" menu item as disabled', async () test('should render with custom css', () => { const mockedProps = createProps(); + const { customCss } = mockedProps; render(setup(mockedProps)); + injectCustomCss(customCss); expect(screen.getByRole('button')).toHaveStyle('margin-left: 100px'); }); diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index b86af3da8a89d..b7d368ec32db2 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -117,8 +117,6 @@ class HeaderActionsDropdown extends React.PureComponent { } UNSAFE_componentWillMount() { - injectCustomCss(this.state.css); - SupersetClient.get({ endpoint: '/csstemplateasyncmodelview/api/read' }) .then(({ json }) => { const cssTemplates = json.result.map(row => ({ From 3d66912d89851f03c38803b29128a45d66b34cb6 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Wed, 16 Mar 2022 09:59:56 -0700 Subject: [PATCH 06/17] fix: clean up chart metadata config (#19143) * fix: clean up chart metadata config * missed a spot * missed another spot * fix failing time-specific test * can't call translation functions here * Revert "fix failing time-specific test" This reverts commit 3eeb8ab9cc5db155a30d08b81fd697af1fcaf6ea. * skip problematic test * extra import --- .../src/chart/models/ChartMetadata.ts | 22 ++++++++----- .../superset-ui-core/src/chart/types/Base.ts | 13 +++++--- .../TimezoneSelector.test.tsx | 2 +- .../VizTypeControl/VizTypeGallery.tsx | 32 +++++++++++-------- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts index 1013eeee2d3b3..7a25fe86208d1 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts @@ -43,10 +43,11 @@ export interface ChartMetadataConfig { exampleGallery?: ExampleImage[]; tags?: string[]; category?: string | null; - label?: { - name?: ChartLabel; - description?: string; - } | null; + // deprecated: true hides a chart from all viz picker interactions. + deprecated?: boolean; + // label: ChartLabel.DEPRECATED which will display a "deprecated" label on the chart. + label?: ChartLabel | null; + labelExplanation?: string | null; } export default class ChartMetadata { @@ -80,10 +81,11 @@ export default class ChartMetadata { category: string | null; - label?: { - name?: ChartLabel; - description?: string; - } | null; + deprecated?: boolean; + + label?: ChartLabel | null; + + labelExplanation?: string | null; constructor(config: ChartMetadataConfig) { const { @@ -101,7 +103,9 @@ export default class ChartMetadata { exampleGallery = [], tags = [], category = null, + deprecated = false, label = null, + labelExplanation = null, } = config; this.name = name; @@ -127,7 +131,9 @@ export default class ChartMetadata { this.exampleGallery = exampleGallery; this.tags = tags; this.category = category; + this.deprecated = deprecated; this.label = label; + this.labelExplanation = labelExplanation; } canBeAnnotationType(type: string): boolean { diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts index aad547ca2aa5d..0bfae7777e7df 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts @@ -53,18 +53,21 @@ export interface PlainObject { } export enum ChartLabel { - VERIFIED = 'VERIFIED', DEPRECATED = 'DEPRECATED', FEATURED = 'FEATURED', } -export const ChartLabelWeight = { +export const chartLabelExplanations: Record = { + [ChartLabel.DEPRECATED]: + 'This chart uses features or modules which are no longer actively maintained. It will eventually be replaced or removed.', + [ChartLabel.FEATURED]: + 'This chart was tested and verified, so the overall experience should be stable.', +}; + +export const chartLabelWeight: Record = { [ChartLabel.DEPRECATED]: { weight: -0.1, }, - [ChartLabel.VERIFIED]: { - weight: 0.2, - }, [ChartLabel.FEATURED]: { weight: 0.1, }, diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx index 035cff842c9e2..79830fd820921 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx @@ -55,7 +55,7 @@ it('use the default timezone when an invalid timezone is provided', async () => expect(onTimezoneChange).toHaveBeenLastCalledWith('Africa/Abidjan'); }); -it('can select a timezone values and returns canonical value', async () => { +it.skip('can select a timezone values and returns canonical value', async () => { const onTimezoneChange = jest.fn(); render( = ({ > {type.name} - {type.label?.name && ( + {type.label && ( -
{t(type.label?.name)}
+
{t(type.label)}
)} @@ -503,8 +503,7 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) { .map(([key, value]) => ({ key, value })) .filter( ({ value }) => - nativeFilterGate(value.behaviors || []) && - value.label?.name !== ChartLabel.DEPRECATED, + nativeFilterGate(value.behaviors || []) && !value.deprecated, ); result.sort((a, b) => vizSortFactor(a) - vizSortFactor(b)); return result; @@ -593,12 +592,16 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) { .search(searchInputValue) .map(result => result.item) .sort((a, b) => { - const aName = a.value?.label?.name; - const bName = b.value?.label?.name; + const aLabel = a.value?.label; + const bLabel = b.value?.label; const aOrder = - aName && ChartLabelWeight[aName] ? ChartLabelWeight[aName].weight : 0; + aLabel && chartLabelWeight[aLabel] + ? chartLabelWeight[aLabel].weight + : 0; const bOrder = - bName && ChartLabelWeight[bName] ? ChartLabelWeight[bName].weight : 0; + bLabel && chartLabelWeight[bLabel] + ? chartLabelWeight[bLabel].weight + : 0; return bOrder - aOrder; }); }, [searchInputValue, fuse]); @@ -798,15 +801,18 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) { `} > {selectedVizMetadata?.name} - {selectedVizMetadata?.label?.name && ( + {selectedVizMetadata?.label && ( -
{t(selectedVizMetadata.label?.name)}
+
{t(selectedVizMetadata.label)}
From bb618a47ff1e1747cf66bffa8bceee133a5c9064 Mon Sep 17 00:00:00 2001 From: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Date: Wed, 16 Mar 2022 14:09:19 -0500 Subject: [PATCH 07/17] fix(sqllab): Updated blank states for query results and query history (#19111) * Empty states updated on result tab and query history tab * Testing on query history blank state * Testing on result tab with empty state * Forgot to remove a comment * Corrected empty state image size and centered with drag bar * Centered blank states vertically --- .../QueryHistory/QueryHistory.test.tsx | 50 +++++++++++++++++++ .../SqlLab/components/QueryHistory/index.tsx | 22 ++++++-- .../components/SouthPane/SouthPane.test.jsx | 50 +++++++++++++++++-- .../src/SqlLab/components/SouthPane/index.tsx | 19 ++++++- 4 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx new file mode 100644 index 0000000000000..782b147839186 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx @@ -0,0 +1,50 @@ +/** + * 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 React from 'react'; +import { render, screen } from 'spec/helpers/testing-library'; +import QueryHistory from 'src/SqlLab/components/QueryHistory'; + +const NOOP = () => {}; +const mockedProps = { + queries: [], + actions: { + queryEditorSetSql: NOOP, + cloneQueryToNewTab: NOOP, + fetchQueryResults: NOOP, + clearQueryResults: NOOP, + removeQuery: NOOP, + }, + displayLimit: 1000, +}; + +const setup = (overrides = {}) => ( + +); + +describe('QueryHistory', () => { + it('Renders an empty state for query history', () => { + render(setup()); + + const emptyStateText = screen.getByText( + /run a query to display query history/i, + ); + + expect(emptyStateText).toBeVisible(); + }); +}); diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index e2d0453bb297a..7cf9d6ba657dd 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -17,8 +17,8 @@ * under the License. */ import React from 'react'; -import Alert from 'src/components/Alert'; -import { t } from '@superset-ui/core'; +import { EmptyStateMedium } from 'src/components/EmptyState'; +import { t, styled } from '@superset-ui/core'; import { Query } from 'src/SqlLab/types'; import QueryTable from 'src/SqlLab/components/QueryTable'; @@ -34,6 +34,17 @@ interface QueryHistoryProps { displayLimit: number; } +const StyledEmptyStateWrapper = styled.div` + height: 100%; + .ant-empty-image img { + margin-right: 28px; + } + + p { + margin-right: 28px; + } +`; + const QueryHistory = ({ queries, actions, displayLimit }: QueryHistoryProps) => queries.length > 0 ? ( displayLimit={displayLimit} /> ) : ( - + + + ); export default QueryHistory; diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx index dbf81cfcf282d..1786a6cf313a6 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx @@ -20,11 +20,15 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { styledShallow as shallow } from 'spec/helpers/theming'; +import { render, screen, act } from 'spec/helpers/testing-library'; import SouthPaneContainer from 'src/SqlLab/components/SouthPane/state'; import ResultSet from 'src/SqlLab/components/ResultSet'; import '@testing-library/jest-dom/extend-expect'; import { STATUS_OPTIONS } from 'src/SqlLab/constants'; import { initialState } from 'src/SqlLab/fixtures'; +import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; + +const NOOP = () => {}; const mockedProps = { editorQueries: [ @@ -71,13 +75,36 @@ const mockedProps = { offline: false, }; +const mockedEmptyProps = { + editorQueries: [], + latestQueryId: '', + dataPreviewQueries: [], + actions: { + queryEditorSetSql: NOOP, + cloneQueryToNewTab: NOOP, + fetchQueryResults: NOOP, + clearQueryResults: NOOP, + removeQuery: NOOP, + setActiveSouthPaneTab: NOOP, + }, + activeSouthPaneTab: '', + height: 100, + databases: '', + offline: false, + displayLimit: 100, + user: UserWithPermissionsAndRoles, + defaultQueryLimit: 100, +}; + const middlewares = [thunk]; const mockStore = configureStore(middlewares); const store = mockStore(initialState); +const setup = (overrides = {}) => ( + +); -describe('SouthPane', () => { - const getWrapper = () => - shallow().dive(); +describe('SouthPane - Enzyme', () => { + const getWrapper = () => shallow(setup()).dive(); let wrapper; @@ -95,3 +122,20 @@ describe('SouthPane', () => { ); }); }); + +describe('SouthPane - RTL', () => { + const renderAndWait = overrides => { + const mounted = act(async () => { + render(setup(overrides)); + }); + + return mounted; + }; + it('Renders an empty state for results', async () => { + await renderAndWait(mockedEmptyProps); + + const emptyStateText = screen.getByText(/run a query to display results/i); + + expect(emptyStateText).toBeVisible(); + }); +}); diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index f7efc04f34725..3fb0f9c5261e4 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -20,6 +20,7 @@ import React, { createRef } from 'react'; import shortid from 'shortid'; import Alert from 'src/components/Alert'; import Tabs from 'src/components/Tabs'; +import { EmptyStateMedium } from 'src/components/EmptyState'; import { t, styled } from '@superset-ui/core'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; @@ -93,6 +94,17 @@ const StyledPane = styled.div` } `; +const StyledEmptyStateWrapper = styled.div` + height: 100%; + .ant-empty-image img { + margin-right: 28px; + } + + p { + margin-right: 28px; + } +`; + export default function SouthPane({ editorQueries, latestQueryId, @@ -161,7 +173,12 @@ export default function SouthPane({ } } else { results = ( - + + + ); } return results; From 181ecf450990c5102c1e9a077dfe7455073fb70d Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Wed, 16 Mar 2022 14:39:08 -0700 Subject: [PATCH 08/17] fix: Logic for showing extension in Global Nav (#19158) * fix logic for checking extensions * add specific types * fix lint --- superset-frontend/src/views/CRUD/utils.tsx | 7 +++---- superset-frontend/src/views/components/Menu.tsx | 2 +- .../src/views/components/MenuRight.tsx | 17 ++++++++++------- superset/utils/async_query_manager.py | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index d9d21c8565d17..c2ae0d8cbed13 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -412,12 +412,11 @@ export const hasTerminalValidation = (errors: Record[]) => ); export const checkUploadExtensions = ( - perm: Array | string | undefined | boolean, - cons: Array, + perm: Array, + cons: Array, ) => { if (perm !== undefined) { - if (typeof perm === 'boolean') return perm; - return intersection(perm, cons).length; + return intersection(perm, cons).length > 0; } return false; }; diff --git a/superset-frontend/src/views/components/Menu.tsx b/superset-frontend/src/views/components/Menu.tsx index 068cad8e363d1..77a074f71b6c7 100644 --- a/superset-frontend/src/views/components/Menu.tsx +++ b/superset-frontend/src/views/components/Menu.tsx @@ -74,7 +74,7 @@ interface MenuObjectChildProps { index?: number; url?: string; isFrontendRoute?: boolean; - perm?: string | Array | boolean; + perm?: string | boolean; view?: string; } diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx index 531ed53847346..ab5b5d8d82a7e 100644 --- a/superset-frontend/src/views/components/MenuRight.tsx +++ b/superset-frontend/src/views/components/MenuRight.tsx @@ -79,7 +79,6 @@ const RightMenu = ({ ALLOWED_EXTENSIONS, HAS_GSHEETS_INSTALLED, } = useSelector(state => state.common.conf); - const [showModal, setShowModal] = useState(false); const [engine, setEngine] = useState(''); const canSql = findPermission('can_sqllab', 'Superset', roles); @@ -124,19 +123,25 @@ const RightMenu = ({ label: t('Upload CSV to database'), name: 'Upload a CSV', url: '/csvtodatabaseview/form', - perm: CSV_EXTENSIONS && canUploadCSV, + perm: + checkUploadExtensions(CSV_EXTENSIONS, ALLOWED_EXTENSIONS) && + canUploadCSV, }, { label: t('Upload columnar file to database'), name: 'Upload a Columnar file', url: '/columnartodatabaseview/form', - perm: COLUMNAR_EXTENSIONS && canUploadColumnar, + perm: + checkUploadExtensions(COLUMNAR_EXTENSIONS, ALLOWED_EXTENSIONS) && + canUploadColumnar, }, { label: t('Upload Excel file to database'), name: 'Upload Excel', url: '/exceltodatabaseview/form', - perm: EXCEL_EXTENSIONS && canUploadExcel, + perm: + checkUploadExtensions(EXCEL_EXTENSIONS, ALLOWED_EXTENSIONS) && + canUploadExcel, }, ], }, @@ -209,9 +214,7 @@ const RightMenu = ({ title={menuIconAndLabel(menu)} > {menu.childs.map((item, idx) => - typeof item !== 'string' && - item.name && - checkUploadExtensions(item.perm, ALLOWED_EXTENSIONS) ? ( + typeof item !== 'string' && item.name && item.perm ? ( <> {idx === 2 && } diff --git a/superset/utils/async_query_manager.py b/superset/utils/async_query_manager.py index a026fd6f3f3d7..fcda931fcd880 100644 --- a/superset/utils/async_query_manager.py +++ b/superset/utils/async_query_manager.py @@ -71,7 +71,7 @@ class AsyncQueryManager: def __init__(self) -> None: super().__init__() - self._redis: redis.Redis # type: ignore + self._redis: redis.Redis self._stream_prefix: str = "" self._stream_limit: Optional[int] self._stream_limit_firehose: Optional[int] From 8d53db1db6c3211d6be6905bd1e4cd11043e8219 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Wed, 16 Mar 2022 14:47:41 -0700 Subject: [PATCH 09/17] test: fix TimezoneSelector tests on daylight saving time (#19156) --- superset-frontend/cypress-base/cypress.json | 2 +- .../integration/dashboard/key_value.test.ts | 14 ++- .../TimezoneSelector.stories.tsx | 4 +- .../TimezoneSelector.test.tsx | 94 +++++++++++++------ .../src/components/TimezoneSelector/index.tsx | 19 ++-- 5 files changed, 89 insertions(+), 44 deletions(-) diff --git a/superset-frontend/cypress-base/cypress.json b/superset-frontend/cypress-base/cypress.json index 8e023d8a1a24b..f9729be1c3c91 100644 --- a/superset-frontend/cypress-base/cypress.json +++ b/superset-frontend/cypress-base/cypress.json @@ -1,7 +1,7 @@ { "baseUrl": "http://localhost:8088", "chromeWebSecurity": false, - "defaultCommandTimeout": 5000, + "defaultCommandTimeout": 8000, "numTestsKeptInMemory": 0, "experimentalFetchPolyfill": true, "requestTimeout": 10000, 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 ba27bf30163a2..24b6ff0aa7a62 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 @@ -27,16 +27,19 @@ interface QueryString { native_filters_key: string; } -describe('nativefiler url param key', () => { +xdescribe('nativefiler url param key', () => { // const urlParams = { param1: '123', param2: 'abc' }; before(() => { cy.login(); - cy.visit(WORLD_HEALTH_DASHBOARD); - WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); - cy.wait(1000); // wait for key to be published (debounced) }); + let initialFilterKey: string; it('should have cachekey in nativefilter param', () => { + // things in `before` will not retry and the `waitForChartLoad` check is + // especically flaky and may need more retries + cy.visit(WORLD_HEALTH_DASHBOARD); + WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); + cy.wait(1000); // wait for key to be published (debounced) cy.location().then(loc => { const queryParams = qs.parse(loc.search) as QueryString; expect(typeof queryParams.native_filters_key).eq('string'); @@ -44,6 +47,9 @@ describe('nativefiler url param key', () => { }); it('should have different key when page reloads', () => { + cy.visit(WORLD_HEALTH_DASHBOARD); + WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); + cy.wait(1000); // wait for key to be published (debounced) cy.location().then(loc => { const queryParams = qs.parse(loc.search) as QueryString; expect(queryParams.native_filters_key).not.equal(initialFilterKey); diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx index cf9d1d6e730ed..6bf0d438daca6 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.stories.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; import { useArgs } from '@storybook/client-api'; -import TimezoneSelector, { TimezoneProps } from './index'; +import TimezoneSelector, { TimezoneSelectorProps } from './index'; export default { title: 'TimezoneSelector', @@ -26,7 +26,7 @@ export default { }; // eslint-disable-next-line @typescript-eslint/no-unused-vars -export const InteractiveTimezoneSelector = (args: TimezoneProps) => { +export const InteractiveTimezoneSelector = (args: TimezoneSelectorProps) => { const [{ timezone }, updateArgs] = useArgs(); const onTimezoneChange = (value: string) => { updateArgs({ timezone: value }); diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx index 79830fd820921..19c713adf4f13 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx @@ -20,21 +20,42 @@ import React from 'react'; import moment from 'moment-timezone'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import TimezoneSelector from './index'; +import type { TimezoneSelectorProps } from './index'; -jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); +const loadComponent = (mockCurrentTime?: string) => { + if (mockCurrentTime) { + jest.useFakeTimers('modern'); + jest.setSystemTime(new Date(mockCurrentTime)); + } + return new Promise>(resolve => { + jest.isolateModules(() => { + const { default: TimezoneSelector } = module.require('./index'); + resolve(TimezoneSelector); + jest.useRealTimers(); + }); + }); +}; const getSelectOptions = () => waitFor(() => document.querySelectorAll('.ant-select-item-option-content')); -it('use the timezone from `moment` if no timezone provided', () => { +const openSelectMenu = async () => { + const searchInput = screen.getByRole('combobox'); + userEvent.click(searchInput); +}; + +jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); + +test('use the timezone from `moment` if no timezone provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render(); expect(onTimezoneChange).toHaveBeenCalledTimes(1); expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau'); }); -it('update to closest deduped timezone when timezone is provided', async () => { +test('update to closest deduped timezone when timezone is provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( { expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Vancouver'); }); -it('use the default timezone when an invalid timezone is provided', async () => { +test('use the default timezone when an invalid timezone is provided', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( , @@ -55,7 +77,8 @@ it('use the default timezone when an invalid timezone is provided', async () => expect(onTimezoneChange).toHaveBeenLastCalledWith('Africa/Abidjan'); }); -it.skip('can select a timezone values and returns canonical value', async () => { +test('render timezones in correct oder for standard time', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); render( timezone="America/Nassau" />, ); - - const searchInput = screen.getByRole('combobox', { - name: 'Timezone selector', - }); - expect(searchInput).toBeInTheDocument(); - userEvent.click(searchInput); - const isDaylight = moment(moment.now()).isDST(); - - const selectedTimezone = isDaylight - ? 'GMT -04:00 (Eastern Daylight Time)' - : 'GMT -05:00 (Eastern Standard Time)'; - - // selected option ranks first + await openSelectMenu(); const options = await getSelectOptions(); - expect(options[0]).toHaveTextContent(selectedTimezone); - - // others are ranked by offset + expect(options[0]).toHaveTextContent('GMT -05:00 (Eastern Standard Time)'); expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)'); expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)'); expect(options[3]).toHaveTextContent('GMT -10:00 (America/Adak)'); +}); + +test('render timezones in correct order for daylight saving time', async () => { + const TimezoneSelector = await loadComponent('2022-07-01'); + const onTimezoneChange = jest.fn(); + render( + , + ); + await openSelectMenu(); + const options = await getSelectOptions(); + // first option is always current timezone + expect(options[0]).toHaveTextContent('GMT -04:00 (Eastern Daylight Time)'); + expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)'); + expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)'); + expect(options[3]).toHaveTextContent('GMT -09:30 (Pacific/Marquesas)'); +}); +test('can select a timezone values and returns canonical timezone name', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); + const onTimezoneChange = jest.fn(); + render( + , + ); + + await openSelectMenu(); + + const searchInput = screen.getByRole('combobox'); // search for mountain time await userEvent.type(searchInput, 'mou', { delay: 10 }); - - const findTitle = isDaylight - ? 'GMT -06:00 (Mountain Daylight Time)' - : 'GMT -07:00 (Mountain Standard Time)'; + const findTitle = 'GMT -07:00 (Mountain Standard Time)'; const selectOption = await screen.findByTitle(findTitle); - expect(selectOption).toBeInTheDocument(); userEvent.click(selectOption); expect(onTimezoneChange).toHaveBeenCalledTimes(1); expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Cambridge_Bay'); }); -it('can update props and rerender with different values', async () => { +test('can update props and rerender with different values', async () => { + const TimezoneSelector = await loadComponent('2022-01-01'); const onTimezoneChange = jest.fn(); const { rerender } = render( { ); }; -export interface TimezoneProps { - onTimezoneChange: (value: string) => void; - timezone?: string | null; -} - const ALL_ZONES = moment.tz .countries() .map(country => moment.tz.zonesForCountry(country, true)) @@ -106,7 +101,15 @@ const matchTimezoneToOptions = (timezone: string) => TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone)) ?.value || DEFAULT_TIMEZONE.value; -const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { +export type TimezoneSelectorProps = { + onTimezoneChange: (value: string) => void; + timezone?: string | null; +}; + +export default function TimezoneSelector({ + onTimezoneChange, + timezone, +}: TimezoneSelectorProps) { const validTimezone = useMemo( () => matchTimezoneToOptions(timezone || moment.tz.guess()), [timezone], @@ -129,6 +132,4 @@ const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { sortComparator={TIMEZONE_OPTIONS_SORT_COMPARATOR} /> ); -}; - -export default TimezoneSelector; +} From fc8721800b00ea8a4a627ec54adb5852857f6d3c Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Wed, 16 Mar 2022 15:57:35 -0700 Subject: [PATCH 10/17] =?UTF-8?q?fix:=20Revert=20"refactor:=20converted=20?= =?UTF-8?q?QueryAutoRefresh=20to=20functional=20component=20=E2=80=A6=20(#?= =?UTF-8?q?19226)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Revert "refactor: converted QueryAutoRefresh to functional component (#18179)" This reverts commit f497c1900e896a45daac66cf35776d9ad343f69e. * lint --- .../QueryAutoRefresh.test.jsx | 40 ++++----- .../components/QueryAutoRefresh/index.jsx | 86 ++++++++++--------- 2 files changed, 67 insertions(+), 59 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx index 93cea6d08d65f..06bf187e1185a 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.jsx @@ -17,14 +17,12 @@ * under the License. */ import React from 'react'; -import { render } from 'spec/helpers/testing-library'; -import { ThemeProvider, supersetTheme } from '@superset-ui/core'; +import { shallow } from 'enzyme'; +import sinon from 'sinon'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; import QueryAutoRefresh from 'src/SqlLab/components/QueryAutoRefresh'; import { initialState, runningQuery } from 'src/SqlLab/fixtures'; -import fetchMock from 'fetch-mock'; -import * as actions from 'src/SqlLab/actions/sqlLab'; describe('QueryAutoRefresh', () => { const middlewares = [thunk]; @@ -40,29 +38,31 @@ describe('QueryAutoRefresh', () => { sqlLab, }; const store = mockStore(state); - const setup = (overrides = {}) => ( - - - - ); - - const mockFetch = fetchMock.get('glob:*/superset/queries/*', {}); + const getWrapper = () => + shallow() + .dive() + .dive(); + let wrapper; it('shouldCheckForQueries', () => { - render(setup(), { - useRedux: true, - }); - - expect(mockFetch.called()).toBe(true); + wrapper = getWrapper(); + expect(wrapper.instance().shouldCheckForQueries()).toBe(true); }); it('setUserOffline', () => { - const spy = jest.spyOn(actions, 'setUserOffline'); + wrapper = getWrapper(); + const spy = sinon.spy(wrapper.instance().props.actions, 'setUserOffline'); - render(setup(), { - useRedux: true, + // state not changed + wrapper.setState({ + offline: false, }); + expect(spy.called).toBe(false); - expect(spy).toHaveBeenCalled(); + // state is changed + wrapper.setState({ + offline: true, + }); + expect(spy.callCount).toBe(1); }); }); diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx index 43f6c5d8a7d6e..b54936b691efe 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect } from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; @@ -28,12 +28,31 @@ const QUERY_UPDATE_BUFFER_MS = 5000; const MAX_QUERY_AGE_TO_POLL = 21600000; const QUERY_TIMEOUT_LIMIT = 10000; -function QueryAutoRefresh({ offline, queries, queriesLastUpdate, actions }) { - const [offlineState, setOfflineState] = useState(offline); - let timer = null; +class QueryAutoRefresh extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + offline: props.offline, + }; + } + + UNSAFE_componentWillMount() { + this.startTimer(); + } + + componentDidUpdate(prevProps) { + if (prevProps.offline !== this.state.offline) { + this.props.actions.setUserOffline(this.state.offline); + } + } + + componentWillUnmount() { + this.stopTimer(); + } - const shouldCheckForQueries = () => { + shouldCheckForQueries() { // if there are started or running queries, this method should return true + const { queries } = this.props; const now = new Date().getTime(); const isQueryRunning = q => ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0; @@ -41,57 +60,46 @@ function QueryAutoRefresh({ offline, queries, queriesLastUpdate, actions }) { return Object.values(queries).some( q => isQueryRunning(q) && now - q.startDttm < MAX_QUERY_AGE_TO_POLL, ); - }; + } + + startTimer() { + if (!this.timer) { + this.timer = setInterval(this.stopwatch.bind(this), QUERY_UPDATE_FREQ); + } + } - const stopwatch = () => { + stopTimer() { + clearInterval(this.timer); + this.timer = null; + } + + stopwatch() { // only poll /superset/queries/ if there are started or running queries - if (shouldCheckForQueries()) { + if (this.shouldCheckForQueries()) { SupersetClient.get({ endpoint: `/superset/queries/${ - queriesLastUpdate - QUERY_UPDATE_BUFFER_MS + this.props.queriesLastUpdate - QUERY_UPDATE_BUFFER_MS }`, timeout: QUERY_TIMEOUT_LIMIT, }) .then(({ json }) => { if (Object.keys(json).length > 0) { - actions.refreshQueries(json); + this.props.actions.refreshQueries(json); } - - setOfflineState(false); + this.setState({ offline: false }); }) .catch(() => { - setOfflineState(true); + this.setState({ offline: true }); }); } else { - setOfflineState(false); + this.setState({ offline: false }); } - }; - - const startTimer = () => { - if (!timer) { - timer = setInterval(stopwatch(), QUERY_UPDATE_FREQ); - } - }; - - const stopTimer = () => { - clearInterval(timer); - timer = null; - }; - - useEffect(() => { - startTimer(); - return () => { - stopTimer(); - }; - }, []); + } - useEffect(() => { - actions.setUserOffline(offlineState); - }, [offlineState]); - - return null; + render() { + return null; + } } - QueryAutoRefresh.propTypes = { offline: PropTypes.bool.isRequired, queries: PropTypes.object.isRequired, From d01fdad1d8da740af95e32adf2c9fc4bd1da7db5 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 16 Mar 2022 16:03:06 -0700 Subject: [PATCH 11/17] feat: add export_related flag (#19215) * feat: add export_related flag * Fix lint --- superset/charts/commands/export.py | 6 +-- superset/commands/export/__init__.py | 16 +++++++ .../commands/{export.py => export/models.py} | 11 +++-- superset/dashboards/commands/export.py | 15 ++++--- superset/databases/commands/export.py | 37 ++++++++-------- superset/datasets/commands/export.py | 43 ++++++++++--------- .../queries/saved_queries/commands/export.py | 41 ++++++++++-------- .../charts/commands_tests.py | 20 +++++++++ .../dashboards/commands_tests.py | 22 ++++++++++ .../databases/commands_tests.py | 20 +++++++++ .../datasets/commands_tests.py | 20 +++++++++ .../queries/saved_queries/commands_tests.py | 18 ++++++++ 12 files changed, 199 insertions(+), 70 deletions(-) create mode 100644 superset/commands/export/__init__.py rename superset/commands/{export.py => export/models.py} (86%) diff --git a/superset/charts/commands/export.py b/superset/charts/commands/export.py index 35f70a82eab65..9b3a06c473585 100644 --- a/superset/charts/commands/export.py +++ b/superset/charts/commands/export.py @@ -26,7 +26,7 @@ from superset.charts.commands.exceptions import ChartNotFoundError from superset.charts.dao import ChartDAO from superset.datasets.commands.export import ExportDatasetsCommand -from superset.commands.export import ExportModelsCommand +from superset.commands.export.models import ExportModelsCommand from superset.models.slice import Slice from superset.utils.dict_import_export import EXPORT_VERSION @@ -43,7 +43,7 @@ class ExportChartsCommand(ExportModelsCommand): not_found = ChartNotFoundError @staticmethod - def _export(model: Slice) -> Iterator[Tuple[str, str]]: + def _export(model: Slice, export_related: bool = True) -> Iterator[Tuple[str, str]]: chart_slug = secure_filename(model.slice_name) file_name = f"charts/{chart_slug}_{model.id}.yaml" @@ -72,5 +72,5 @@ def _export(model: Slice) -> Iterator[Tuple[str, str]]: file_content = yaml.safe_dump(payload, sort_keys=False) yield file_name, file_content - if model.table: + if model.table and export_related: yield from ExportDatasetsCommand([model.table.id]).run() diff --git a/superset/commands/export/__init__.py b/superset/commands/export/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/superset/commands/export/__init__.py @@ -0,0 +1,16 @@ +# 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. diff --git a/superset/commands/export.py b/superset/commands/export/models.py similarity index 86% rename from superset/commands/export.py rename to superset/commands/export/models.py index 2b54de87852e9..dd4ff3bc57172 100644 --- a/superset/commands/export.py +++ b/superset/commands/export/models.py @@ -14,10 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# isort:skip_file -from datetime import datetime -from datetime import timezone +from datetime import datetime, timezone from typing import Iterator, List, Tuple, Type import yaml @@ -36,14 +34,15 @@ class ExportModelsCommand(BaseCommand): dao: Type[BaseDAO] = BaseDAO not_found: Type[CommandException] = CommandException - def __init__(self, model_ids: List[int]): + def __init__(self, model_ids: List[int], export_related: bool = True): self.model_ids = model_ids + self.export_related = export_related # this will be set when calling validate() self._models: List[Model] = [] @staticmethod - def _export(model: Model) -> Iterator[Tuple[str, str]]: + def _export(model: Model, export_related: bool = True) -> Iterator[Tuple[str, str]]: raise NotImplementedError("Subclasses MUST implement _export") def run(self) -> Iterator[Tuple[str, str]]: @@ -58,7 +57,7 @@ def run(self) -> Iterator[Tuple[str, str]]: seen = {METADATA_FILE_NAME} for model in self._models: - for file_name, file_content in self._export(model): + for file_name, file_content in self._export(model, self.export_related): if file_name not in seen: yield file_name, file_content seen.add(file_name) diff --git a/superset/dashboards/commands/export.py b/superset/dashboards/commands/export.py index a0467972d5feb..87408bab37706 100644 --- a/superset/dashboards/commands/export.py +++ b/superset/dashboards/commands/export.py @@ -29,7 +29,7 @@ from superset.dashboards.commands.exceptions import DashboardNotFoundError from superset.dashboards.commands.importers.v1.utils import find_chart_uuids from superset.dashboards.dao import DashboardDAO -from superset.commands.export import ExportModelsCommand +from superset.commands.export.models import ExportModelsCommand from superset.datasets.commands.export import ExportDatasetsCommand from superset.datasets.dao import DatasetDAO from superset.models.dashboard import Dashboard @@ -106,8 +106,11 @@ class ExportDashboardsCommand(ExportModelsCommand): dao = DashboardDAO not_found = DashboardNotFoundError + # pylint: disable=too-many-locals @staticmethod - def _export(model: Dashboard) -> Iterator[Tuple[str, str]]: + def _export( + model: Dashboard, export_related: bool = True + ) -> Iterator[Tuple[str, str]]: dashboard_slug = secure_filename(model.dashboard_title) file_name = f"dashboards/{dashboard_slug}.yaml" @@ -138,7 +141,8 @@ def _export(model: Dashboard) -> Iterator[Tuple[str, str]]: if dataset_id is not None: dataset = DatasetDAO.find_by_id(dataset_id) target["datasetUuid"] = str(dataset.uuid) - yield from ExportDatasetsCommand([dataset_id]).run() + if export_related: + yield from ExportDatasetsCommand([dataset_id]).run() # the mapping between dashboard -> charts is inferred from the position # attribute, so if it's not present we need to add a default config @@ -160,5 +164,6 @@ def _export(model: Dashboard) -> Iterator[Tuple[str, str]]: file_content = yaml.safe_dump(payload, sort_keys=False) yield file_name, file_content - chart_ids = [chart.id for chart in model.slices] - yield from ExportChartsCommand(chart_ids).run() + if export_related: + chart_ids = [chart.id for chart in model.slices] + yield from ExportChartsCommand(chart_ids).run() diff --git a/superset/databases/commands/export.py b/superset/databases/commands/export.py index 134bda580c7e5..9e8cb7e374426 100644 --- a/superset/databases/commands/export.py +++ b/superset/databases/commands/export.py @@ -25,7 +25,7 @@ from superset.databases.commands.exceptions import DatabaseNotFoundError from superset.databases.dao import DatabaseDAO -from superset.commands.export import ExportModelsCommand +from superset.commands.export.models import ExportModelsCommand from superset.models.core import Database from superset.utils.dict_import_export import EXPORT_VERSION @@ -55,7 +55,9 @@ class ExportDatabasesCommand(ExportModelsCommand): not_found = DatabaseNotFoundError @staticmethod - def _export(model: Database) -> Iterator[Tuple[str, str]]: + def _export( + model: Database, export_related: bool = True + ) -> Iterator[Tuple[str, str]]: database_slug = secure_filename(model.database_name) file_name = f"databases/{database_slug}.yaml" @@ -90,18 +92,19 @@ def _export(model: Database) -> Iterator[Tuple[str, str]]: file_content = yaml.safe_dump(payload, sort_keys=False) yield file_name, file_content - for dataset in model.tables: - dataset_slug = secure_filename(dataset.table_name) - file_name = f"datasets/{database_slug}/{dataset_slug}.yaml" - - payload = dataset.export_to_dict( - recursive=True, - include_parent_ref=False, - include_defaults=True, - export_uuids=True, - ) - payload["version"] = EXPORT_VERSION - payload["database_uuid"] = str(model.uuid) - - file_content = yaml.safe_dump(payload, sort_keys=False) - yield file_name, file_content + if export_related: + for dataset in model.tables: + dataset_slug = secure_filename(dataset.table_name) + file_name = f"datasets/{database_slug}/{dataset_slug}.yaml" + + payload = dataset.export_to_dict( + recursive=True, + include_parent_ref=False, + include_defaults=True, + export_uuids=True, + ) + payload["version"] = EXPORT_VERSION + payload["database_uuid"] = str(model.uuid) + + file_content = yaml.safe_dump(payload, sort_keys=False) + yield file_name, file_content diff --git a/superset/datasets/commands/export.py b/superset/datasets/commands/export.py index 4e3843a0daee9..be9210a06c669 100644 --- a/superset/datasets/commands/export.py +++ b/superset/datasets/commands/export.py @@ -23,7 +23,7 @@ import yaml from werkzeug.utils import secure_filename -from superset.commands.export import ExportModelsCommand +from superset.commands.export.models import ExportModelsCommand from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.exceptions import DatasetNotFoundError from superset.datasets.dao import DatasetDAO @@ -40,7 +40,9 @@ class ExportDatasetsCommand(ExportModelsCommand): not_found = DatasetNotFoundError @staticmethod - def _export(model: SqlaTable) -> Iterator[Tuple[str, str]]: + def _export( + model: SqlaTable, export_related: bool = True + ) -> Iterator[Tuple[str, str]]: database_slug = secure_filename(model.database.database_name) dataset_slug = secure_filename(model.table_name) file_name = f"datasets/{database_slug}/{dataset_slug}.yaml" @@ -76,23 +78,24 @@ def _export(model: SqlaTable) -> Iterator[Tuple[str, str]]: yield file_name, file_content # include database as well - file_name = f"databases/{database_slug}.yaml" - - payload = model.database.export_to_dict( - recursive=False, - include_parent_ref=False, - include_defaults=True, - export_uuids=True, - ) - # TODO (betodealmeida): move this logic to export_to_dict once this - # becomes the default export endpoint - if payload.get("extra"): - try: - payload["extra"] = json.loads(payload["extra"]) - except json.decoder.JSONDecodeError: - logger.info("Unable to decode `extra` field: %s", payload["extra"]) + if export_related: + file_name = f"databases/{database_slug}.yaml" + + payload = model.database.export_to_dict( + recursive=False, + include_parent_ref=False, + include_defaults=True, + export_uuids=True, + ) + # TODO (betodealmeida): move this logic to export_to_dict once this + # becomes the default export endpoint + if payload.get("extra"): + try: + payload["extra"] = json.loads(payload["extra"]) + except json.decoder.JSONDecodeError: + logger.info("Unable to decode `extra` field: %s", payload["extra"]) - payload["version"] = EXPORT_VERSION + payload["version"] = EXPORT_VERSION - file_content = yaml.safe_dump(payload, sort_keys=False) - yield file_name, file_content + file_content = yaml.safe_dump(payload, sort_keys=False) + yield file_name, file_content diff --git a/superset/queries/saved_queries/commands/export.py b/superset/queries/saved_queries/commands/export.py index ca2cfe5de9679..e209ae8ad2fd8 100644 --- a/superset/queries/saved_queries/commands/export.py +++ b/superset/queries/saved_queries/commands/export.py @@ -23,7 +23,7 @@ import yaml from werkzeug.utils import secure_filename -from superset.commands.export import ExportModelsCommand +from superset.commands.export.models import ExportModelsCommand from superset.models.sql_lab import SavedQuery from superset.queries.saved_queries.commands.exceptions import SavedQueryNotFoundError from superset.queries.saved_queries.dao import SavedQueryDAO @@ -38,7 +38,9 @@ class ExportSavedQueriesCommand(ExportModelsCommand): not_found = SavedQueryNotFoundError @staticmethod - def _export(model: SavedQuery) -> Iterator[Tuple[str, str]]: + def _export( + model: SavedQuery, export_related: bool = True + ) -> Iterator[Tuple[str, str]]: # build filename based on database, optional schema, and label database_slug = secure_filename(model.database.database_name) schema_slug = secure_filename(model.schema) @@ -58,23 +60,24 @@ def _export(model: SavedQuery) -> Iterator[Tuple[str, str]]: yield file_name, file_content # include database as well - file_name = f"databases/{database_slug}.yaml" + if export_related: + file_name = f"databases/{database_slug}.yaml" - payload = model.database.export_to_dict( - recursive=False, - include_parent_ref=False, - include_defaults=True, - export_uuids=True, - ) - # TODO (betodealmeida): move this logic to export_to_dict once this - # becomes the default export endpoint - if "extra" in payload: - try: - payload["extra"] = json.loads(payload["extra"]) - except json.decoder.JSONDecodeError: - logger.info("Unable to decode `extra` field: %s", payload["extra"]) + payload = model.database.export_to_dict( + recursive=False, + include_parent_ref=False, + include_defaults=True, + export_uuids=True, + ) + # TODO (betodealmeida): move this logic to export_to_dict once this + # becomes the default export endpoint + if "extra" in payload: + try: + payload["extra"] = json.loads(payload["extra"]) + except json.decoder.JSONDecodeError: + logger.info("Unable to decode `extra` field: %s", payload["extra"]) - payload["version"] = EXPORT_VERSION + payload["version"] = EXPORT_VERSION - file_content = yaml.safe_dump(payload, sort_keys=False) - yield file_name, file_content + file_content = yaml.safe_dump(payload, sort_keys=False) + yield file_name, file_content diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index bf4fe4fe79bd4..ec205b6a6abee 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -176,6 +176,26 @@ def test_export_chart_command_key_order(self, mock_g): "dataset_uuid", ] + @patch("superset.security.manager.g") + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_export_chart_command_no_related(self, mock_g): + """ + Test that only the chart is exported when export_related=False. + """ + mock_g.user = security_manager.find_user("admin") + + example_chart = ( + db.session.query(Slice).filter_by(slice_name="Energy Sankey").one() + ) + command = ExportChartsCommand([example_chart.id], export_related=False) + contents = dict(command.run()) + + expected = [ + "metadata.yaml", + f"charts/Energy_Sankey_{example_chart.id}.yaml", + ] + assert expected == list(contents.keys()) + class TestImportChartsCommand(SupersetTestCase): @patch("superset.charts.commands.importers.v1.utils.g") diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py index 7c7a2046f0592..ae18c741583e7 100644 --- a/tests/integration_tests/dashboards/commands_tests.py +++ b/tests/integration_tests/dashboards/commands_tests.py @@ -423,6 +423,28 @@ def test_append_charts(self, mock_suffix): "DASHBOARD_VERSION_KEY": "v2", } + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @patch("superset.security.manager.g") + @patch("superset.views.base.g") + def test_export_dashboard_command_no_related(self, mock_g1, mock_g2): + """ + Test that only the dashboard is exported when export_related=False. + """ + mock_g1.user = security_manager.find_user("admin") + mock_g2.user = security_manager.find_user("admin") + + example_dashboard = ( + db.session.query(Dashboard).filter_by(slug="world_health").one() + ) + command = ExportDashboardsCommand([example_dashboard.id], export_related=False) + contents = dict(command.run()) + + expected_paths = { + "metadata.yaml", + "dashboards/World_Banks_Data.yaml", + } + assert expected_paths == set(contents.keys()) + class TestImportDashboardsCommand(SupersetTestCase): def test_import_v0_dashboard_cli_export(self): diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py index c90550ed69903..59fcf39a86727 100644 --- a/tests/integration_tests/databases/commands_tests.py +++ b/tests/integration_tests/databases/commands_tests.py @@ -358,6 +358,26 @@ def test_export_database_command_key_order(self, mock_g): "version", ] + @patch("superset.security.manager.g") + @pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", "load_energy_table_with_slice" + ) + def test_export_database_command_no_related(self, mock_g): + """ + Test that only databases are exported when export_related=False. + """ + mock_g.user = security_manager.find_user("admin") + + example_db = get_example_database() + db_uuid = example_db.uuid + + command = ExportDatabasesCommand([example_db.id], export_related=False) + contents = dict(command.run()) + prefixes = {path.split("/")[0] for path in contents} + assert "metadata.yaml" in prefixes + assert "databases" in prefixes + assert "datasets" not in prefixes + class TestImportDatabasesCommand(SupersetTestCase): def test_import_v1_database(self): diff --git a/tests/integration_tests/datasets/commands_tests.py b/tests/integration_tests/datasets/commands_tests.py index af22d9319b27c..784b1f19e39d9 100644 --- a/tests/integration_tests/datasets/commands_tests.py +++ b/tests/integration_tests/datasets/commands_tests.py @@ -219,6 +219,26 @@ def test_export_dataset_command_key_order(self, mock_g): "database_uuid", ] + @patch("superset.security.manager.g") + @pytest.mark.usefixtures("load_energy_table_with_slice") + def test_export_dataset_command_no_related(self, mock_g): + """ + Test that only datasets are exported when export_related=False. + """ + mock_g.user = security_manager.find_user("admin") + + example_db = get_example_database() + example_dataset = _get_table_from_list_by_name( + "energy_usage", example_db.tables + ) + command = ExportDatasetsCommand([example_dataset.id], export_related=False) + contents = dict(command.run()) + + assert list(contents.keys()) == [ + "metadata.yaml", + "datasets/examples/energy_usage.yaml", + ] + class TestImportDatasetsCommand(SupersetTestCase): @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") diff --git a/tests/integration_tests/queries/saved_queries/commands_tests.py b/tests/integration_tests/queries/saved_queries/commands_tests.py index f90924ba0e66a..bd90419155422 100644 --- a/tests/integration_tests/queries/saved_queries/commands_tests.py +++ b/tests/integration_tests/queries/saved_queries/commands_tests.py @@ -83,6 +83,24 @@ def test_export_query_command(self, mock_g): "database_uuid": str(self.example_database.uuid), } + @patch("superset.queries.saved_queries.filters.g") + def test_export_query_command_no_related(self, mock_g): + """ + Test that only the query is exported when export_related=False. + """ + mock_g.user = security_manager.find_user("admin") + + command = ExportSavedQueriesCommand( + [self.example_query.id], export_related=False + ) + contents = dict(command.run()) + + expected = [ + "metadata.yaml", + "queries/examples/schema1/The_answer.yaml", + ] + assert expected == list(contents.keys()) + @patch("superset.queries.saved_queries.filters.g") def test_export_query_command_no_access(self, mock_g): """Test that users can't export datasets they don't have access to""" 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 12/17] 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 ea9f02094a52c..854e18074d9e0 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 9f0c05a8eb87a..3f05416b1c0c5 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 743cb3a3c6493..71ba76dff7a07 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 1678471b61f79..35795f81a11fa 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 b54fc1173c28f..777d5f2a4e434 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 8996645079472..d1f87ec999e0c 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 b7d368ec32db2..9375c684af90a 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 da7d196bd8b50..579f9d4b69077 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 cb31503ac8611..c70e47dc3d01d 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 73a589312acc6..309d75dac9a80 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 9682fdb7b8f0e..ec9735f091690 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 878f3d68695ae..e5fff328724d5 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 fbdf362eea709..dffbd9fbe0be8 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 57e6d30de4532..71f77a4621fa2 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 {