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 01/24] 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 1013eeee2d3b..7a25fe86208d 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 aad547ca2aa5..0bfae7777e7d 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 035cff842c9e..79830fd82092 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 02/24] 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 000000000000..782b14783918 --- /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 e2d0453bb297..7cf9d6ba657d 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 dbf81cfcf282..1786a6cf313a 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 f7efc04f3472..3fb0f9c5261e 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 03/24] 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 d9d21c8565d1..c2ae0d8cbed1 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 068cad8e363d..77a074f71b6c 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 531ed5384734..ab5b5d8d82a7 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 a026fd6f3f3d..fcda931fcd88 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 04/24] 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 8e023d8a1a24..f9729be1c3c9 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 ba27bf30163a..24b6ff0aa7a6 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 cf9d1d6e730e..6bf0d438daca 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 79830fd82092..19c713adf4f1 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 05/24] =?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 93cea6d08d65..06bf187e1185 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 43f6c5d8a7d6..b54936b691ef 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 06/24] 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 35f70a82eab6..9b3a06c47358 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 000000000000..13a83393a912 --- /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 2b54de87852e..dd4ff3bc5717 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 a0467972d5fe..87408bab3770 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 134bda580c7e..9e8cb7e37442 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 4e3843a0daee..be9210a06c66 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 ca2cfe5de967..e209ae8ad2fd 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 bf4fe4fe79bd..ec205b6a6abe 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 7c7a2046f059..ae18c741583e 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 c90550ed6990..59fcf39a8672 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 af22d9319b27..784b1f19e39d 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 f90924ba0e66..bd9041915542 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 07/24] feat: add permalink to dashboard and explore (#19078) * rename key_value to temporary_cache * add migration * create new key_value package * add commands * lots of new stuff * fix schema reference * remove redundant filter state from bootstrap data * add missing license headers * fix pylint * fix dashboard permalink access * use valid json mocks for filter state tests * fix temporary cache tests * add anchors to dashboard state * lint * fix util test * fix url shortlink button tests * remove legacy shortner * remove unused imports * fix js tests * fix test * add native filter state to anchor link * add UPDATING.md section * address comments * address comments * lint * fix test * add utils tests + other test stubs * add key_value integration tests * add filter box state to permalink state * fully support persisting url parameters * lint, add redirects and a few integration tests * fix test + clean up trailing comma * fix anchor bug * change value to LargeBinary to support persisting binary values * fix urlParams type and simplify urlencode * lint * add optional entry expiration * fix incorrect chart id + add test --- UPDATING.md | 3 +- .../components/AnchorLink/AnchorLink.test.jsx | 8 +- .../src/components/AnchorLink/index.jsx | 11 +- .../URLShortLinkButton.test.tsx | 52 ++++-- .../components/URLShortLinkButton/index.jsx | 27 ++- superset-frontend/src/constants.ts | 16 ++ .../HeaderActionsDropdown.test.tsx | 4 +- .../Header/HeaderActionsDropdown/index.jsx | 4 +- .../components/SliceHeaderControls/index.tsx | 7 +- .../components/gridComponents/Tab.jsx | 2 + .../ShareMenuItems/ShareMenuItems.test.tsx | 16 +- .../components/menu/ShareMenuItems/index.tsx | 66 ++----- .../nativeFilters/FilterBar/index.tsx | 13 +- .../nativeFilters/FilterBar/keyValue.tsx | 13 +- .../dashboard/containers/DashboardPage.tsx | 14 +- superset-frontend/src/dashboard/types.ts | 8 + .../explore/components/EmbedCodeButton.jsx | 25 ++- .../components/EmbedCodeButton.test.jsx | 10 +- .../components/ExploreActionButtons.tsx | 21 ++- .../components/ExploreViewContainer/index.jsx | 10 +- .../src/hooks/useUrlShortener.ts | 39 ---- superset-frontend/src/utils/urlUtils.ts | 87 ++++++++- superset/config.py | 3 + superset/dashboards/filter_state/api.py | 8 +- .../filter_state/commands/create.py | 10 +- .../filter_state/commands/delete.py | 14 +- .../dashboards/filter_state/commands/get.py | 8 +- .../filter_state/commands/update.py | 14 +- .../permalink}/__init__.py | 0 superset/dashboards/permalink/api.py | 171 +++++++++++++++++ .../permalink/commands}/__init__.py | 0 .../dashboards/permalink/commands/base.py | 23 +++ .../dashboards/permalink/commands/create.py | 61 ++++++ superset/dashboards/permalink/commands/get.py | 65 +++++++ superset/dashboards/permalink/exceptions.py | 31 ++++ superset/dashboards/permalink/schemas.py | 40 ++++ .../permalink/types.py} | 17 +- superset/explore/form_data/api.py | 10 +- superset/explore/form_data/commands/create.py | 13 +- superset/explore/form_data/commands/delete.py | 14 +- superset/explore/form_data/commands/get.py | 6 +- superset/explore/form_data/commands/update.py | 19 +- superset/explore/permalink/__init__.py | 16 ++ superset/explore/permalink/api.py | 174 ++++++++++++++++++ .../explore/permalink/commands/__init__.py | 16 ++ superset/explore/permalink/commands/base.py | 23 +++ superset/explore/permalink/commands/create.py | 60 ++++++ superset/explore/permalink/commands/get.py | 66 +++++++ superset/explore/permalink/exceptions.py | 31 ++++ superset/explore/permalink/schemas.py | 37 ++++ superset/explore/permalink/types.py | 29 +++ superset/explore/{form_data => }/utils.py | 0 superset/initialization/__init__.py | 4 + superset/key_value/commands/create.py | 60 +++++- superset/key_value/commands/delete.py | 51 ++++- superset/key_value/commands/get.py | 51 +++-- superset/key_value/commands/update.py | 70 +++++-- .../key_value/{commands => }/exceptions.py | 5 + superset/key_value/models.py | 38 ++++ superset/key_value/types.py | 34 ++++ superset/key_value/utils.py | 43 ++++- .../6766938c6065_add_key_value_store.py | 61 ++++++ superset/temporary_cache/__init__.py | 16 ++ .../{key_value => temporary_cache}/api.py | 27 +-- superset/temporary_cache/commands/__init__.py | 16 ++ superset/temporary_cache/commands/create.py | 45 +++++ superset/temporary_cache/commands/delete.py | 45 +++++ .../commands/entry.py | 0 .../temporary_cache/commands/exceptions.py | 45 +++++ superset/temporary_cache/commands/get.py | 46 +++++ .../commands/parameters.py | 0 superset/temporary_cache/commands/update.py | 48 +++++ superset/temporary_cache/schemas.py | 37 ++++ superset/temporary_cache/utils.py | 28 +++ superset/views/core.py | 55 +++++- superset/views/redirects.py | 21 +-- tests/integration_tests/core_tests.py | 24 --- .../dashboards/filter_state/api_tests.py | 101 +++++----- .../dashboards/permalink/__init__.py | 16 ++ .../dashboards/permalink/api_tests.py | 90 +++++++++ .../explore/form_data/api_tests.py | 114 +++++++----- .../explore/permalink/__init__.py | 16 ++ .../explore/permalink/api_tests.py | 117 ++++++++++++ tests/integration_tests/fixtures/client.py | 26 +++ tests/integration_tests/key_value/__init__.py | 16 ++ .../key_value/commands/__init__.py | 16 ++ .../key_value/commands/create_test.py | 64 +++++++ .../key_value/commands/delete_test.py | 91 +++++++++ .../key_value/commands/fixtures.py | 62 +++++++ .../key_value/commands/get_test.py | 100 ++++++++++ .../key_value/commands/update_test.py | 87 +++++++++ .../explore/{form_data => }/utils_test.py | 26 +-- tests/unit_tests/key_value/__init__.py | 16 ++ tests/unit_tests/key_value/utils_test.py | 117 ++++++++++++ 94 files changed, 2942 insertions(+), 438 deletions(-) delete mode 100644 superset-frontend/src/hooks/useUrlShortener.ts rename superset/{key_value/commands => dashboards/permalink}/__init__.py (100%) create mode 100644 superset/dashboards/permalink/api.py rename {tests/unit_tests/explore/form_data => superset/dashboards/permalink/commands}/__init__.py (100%) create mode 100644 superset/dashboards/permalink/commands/base.py create mode 100644 superset/dashboards/permalink/commands/create.py create mode 100644 superset/dashboards/permalink/commands/get.py create mode 100644 superset/dashboards/permalink/exceptions.py create mode 100644 superset/dashboards/permalink/schemas.py rename superset/{key_value/schemas.py => dashboards/permalink/types.py} (69%) create mode 100644 superset/explore/permalink/__init__.py create mode 100644 superset/explore/permalink/api.py create mode 100644 superset/explore/permalink/commands/__init__.py create mode 100644 superset/explore/permalink/commands/base.py create mode 100644 superset/explore/permalink/commands/create.py create mode 100644 superset/explore/permalink/commands/get.py create mode 100644 superset/explore/permalink/exceptions.py create mode 100644 superset/explore/permalink/schemas.py create mode 100644 superset/explore/permalink/types.py rename superset/explore/{form_data => }/utils.py (100%) rename superset/key_value/{commands => }/exceptions.py (90%) create mode 100644 superset/key_value/models.py create mode 100644 superset/key_value/types.py create mode 100644 superset/migrations/versions/6766938c6065_add_key_value_store.py create mode 100644 superset/temporary_cache/__init__.py rename superset/{key_value => temporary_cache}/api.py (87%) create mode 100644 superset/temporary_cache/commands/__init__.py create mode 100644 superset/temporary_cache/commands/create.py create mode 100644 superset/temporary_cache/commands/delete.py rename superset/{key_value => temporary_cache}/commands/entry.py (100%) create mode 100644 superset/temporary_cache/commands/exceptions.py create mode 100644 superset/temporary_cache/commands/get.py rename superset/{key_value => temporary_cache}/commands/parameters.py (100%) create mode 100644 superset/temporary_cache/commands/update.py create mode 100644 superset/temporary_cache/schemas.py create mode 100644 superset/temporary_cache/utils.py create mode 100644 tests/integration_tests/dashboards/permalink/__init__.py create mode 100644 tests/integration_tests/dashboards/permalink/api_tests.py create mode 100644 tests/integration_tests/explore/permalink/__init__.py create mode 100644 tests/integration_tests/explore/permalink/api_tests.py create mode 100644 tests/integration_tests/fixtures/client.py create mode 100644 tests/integration_tests/key_value/__init__.py create mode 100644 tests/integration_tests/key_value/commands/__init__.py create mode 100644 tests/integration_tests/key_value/commands/create_test.py create mode 100644 tests/integration_tests/key_value/commands/delete_test.py create mode 100644 tests/integration_tests/key_value/commands/fixtures.py create mode 100644 tests/integration_tests/key_value/commands/get_test.py create mode 100644 tests/integration_tests/key_value/commands/update_test.py rename tests/unit_tests/explore/{form_data => }/utils_test.py (88%) create mode 100644 tests/unit_tests/key_value/__init__.py create mode 100644 tests/unit_tests/key_value/utils_test.py diff --git a/UPDATING.md b/UPDATING.md index ea9f02094a52..854e18074d9e 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -49,11 +49,12 @@ flag for the legacy datasource editor (DISABLE_LEGACY_DATASOURCE_EDITOR) in conf ### Deprecations +- [19078](https://github.com/apache/superset/pull/19078): Creation of old shorturl links has been deprecated in favor of a new permalink feature that solves the long url problem (old shorturls will still work, though!). By default, new permalinks use UUID4 as the key. However, to use serial ids similar to the old shorturls, add the following to your `superset_config.py`: `PERMALINK_KEY_TYPE = "id"`. - [18960](https://github.com/apache/superset/pull/18960): Persisting URL params in chart metadata is no longer supported. To set a default value for URL params in Jinja code, use the optional second argument: `url_param("my-param", "my-default-value")`. ### Other -- [17589](https://github.com/apache/incubator-superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager. +- [17589](https://github.com/apache/superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager. - [17536](https://github.com/apache/superset/pull/17536): introduced a key-value endpoint to store dashboard filter state. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `FILTER_STATE_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). - [17882](https://github.com/apache/superset/pull/17882): introduced a key-value endpoint to store Explore form data. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `EXPLORE_FORM_DATA_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). diff --git a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx index 9f0c05a8eb87..3f05416b1c0c 100644 --- a/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx +++ b/superset-frontend/src/components/AnchorLink/AnchorLink.test.jsx @@ -25,6 +25,7 @@ import URLShortLinkButton from 'src/components/URLShortLinkButton'; describe('AnchorLink', () => { const props = { anchorLinkId: 'CHART-123', + dashboardId: 10, }; const globalLocation = window.location; @@ -64,8 +65,9 @@ describe('AnchorLink', () => { expect(wrapper.find(URLShortLinkButton)).toExist(); expect(wrapper.find(URLShortLinkButton)).toHaveProp({ placement: 'right' }); - const targetUrl = wrapper.find(URLShortLinkButton).prop('url'); - const hash = targetUrl.slice(targetUrl.indexOf('#') + 1); - expect(hash).toBe(props.anchorLinkId); + const anchorLinkId = wrapper.find(URLShortLinkButton).prop('anchorLinkId'); + const dashboardId = wrapper.find(URLShortLinkButton).prop('dashboardId'); + expect(anchorLinkId).toBe(props.anchorLinkId); + expect(dashboardId).toBe(props.dashboardId); }); }); diff --git a/superset-frontend/src/components/AnchorLink/index.jsx b/superset-frontend/src/components/AnchorLink/index.jsx index 743cb3a3c649..71ba76dff7a0 100644 --- a/superset-frontend/src/components/AnchorLink/index.jsx +++ b/superset-frontend/src/components/AnchorLink/index.jsx @@ -21,11 +21,11 @@ import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import URLShortLinkButton from 'src/components/URLShortLinkButton'; -import getDashboardUrl from 'src/dashboard/util/getDashboardUrl'; import getLocationHash from 'src/dashboard/util/getLocationHash'; const propTypes = { anchorLinkId: PropTypes.string.isRequired, + dashboardId: PropTypes.number, filters: PropTypes.object, showShortLinkButton: PropTypes.bool, inFocus: PropTypes.bool, @@ -70,17 +70,14 @@ class AnchorLink extends React.PureComponent { } render() { - const { anchorLinkId, filters, showShortLinkButton, placement } = + const { anchorLinkId, dashboardId, showShortLinkButton, placement } = this.props; return ( {showShortLinkButton && ( { - render(, { useRedux: true }); + render(, { useRedux: true }); expect(screen.getByRole('button')).toBeInTheDocument(); }); test('renders overlay on click', async () => { - render(, { useRedux: true }); + render(, { useRedux: true }); userEvent.click(screen.getByRole('button')); expect(await screen.findByRole('tooltip')).toBeInTheDocument(); }); test('obtains short url', async () => { - render(, { useRedux: true }); + render(, { useRedux: true }); userEvent.click(screen.getByRole('button')); - expect(await screen.findByRole('tooltip')).toHaveTextContent(fakeUrl); + expect(await screen.findByRole('tooltip')).toHaveTextContent( + PERMALINK_PAYLOAD.url, + ); }); test('creates email anchor', async () => { const subject = 'Subject'; const content = 'Content'; - render(, { - useRedux: true, - }); + render( + , + { + useRedux: true, + }, + ); - const href = `mailto:?Subject=${subject}%20&Body=${content}${fakeUrl}`; + const href = `mailto:?Subject=${subject}%20&Body=${content}${PERMALINK_PAYLOAD.url}`; userEvent.click(screen.getByRole('button')); expect(await screen.findByRole('link')).toHaveAttribute('href', href); }); test('renders error message on short url error', async () => { - fetchMock.mock('glob:*/r/shortner/', 500, { + fetchMock.mock(`glob:*/api/v1/dashboard/${DASHBOARD_ID}/permalink`, 500, { overwriteRoutes: true, }); render( <> - + , { useRedux: true }, diff --git a/superset-frontend/src/components/URLShortLinkButton/index.jsx b/superset-frontend/src/components/URLShortLinkButton/index.jsx index 1678471b61f7..35795f81a11f 100644 --- a/superset-frontend/src/components/URLShortLinkButton/index.jsx +++ b/superset-frontend/src/components/URLShortLinkButton/index.jsx @@ -21,14 +21,17 @@ import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import Popover from 'src/components/Popover'; import CopyToClipboard from 'src/components/CopyToClipboard'; -import { getShortUrl } from 'src/utils/urlUtils'; +import { getDashboardPermalink, getUrlParam } from 'src/utils/urlUtils'; import withToasts from 'src/components/MessageToasts/withToasts'; +import { URL_PARAMS } from 'src/constants'; +import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; const propTypes = { - url: PropTypes.string, + addDangerToast: PropTypes.func.isRequired, + anchorLinkId: PropTypes.string, + dashboardId: PropTypes.number, emailSubject: PropTypes.string, emailContent: PropTypes.string, - addDangerToast: PropTypes.func.isRequired, placement: PropTypes.oneOf(['right', 'left', 'top', 'bottom']), }; @@ -50,9 +53,20 @@ class URLShortLinkButton extends React.Component { getCopyUrl(e) { e.stopPropagation(); - getShortUrl(this.props.url) - .then(this.onShortUrlSuccess) - .catch(this.props.addDangerToast); + const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + if (this.props.dashboardId) { + getFilterValue(this.props.dashboardId, nativeFiltersKey) + .then(filterState => + getDashboardPermalink( + String(this.props.dashboardId), + filterState, + this.props.anchorLinkId, + ) + .then(this.onShortUrlSuccess) + .catch(this.props.addDangerToast), + ) + .catch(this.props.addDangerToast); + } } renderPopover() { @@ -96,7 +110,6 @@ class URLShortLinkButton extends React.Component { } URLShortLinkButton.defaultProps = { - url: window.location.href.substring(window.location.origin.length), placement: 'left', emailSubject: '', emailContent: '', diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index b54fc1173c28..777d5f2a4e43 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -71,8 +71,24 @@ export const URL_PARAMS = { name: 'force', type: 'boolean', }, + permalinkKey: { + name: 'permalink_key', + type: 'string', + }, } as const; +export const RESERVED_CHART_URL_PARAMS: string[] = [ + URL_PARAMS.formDataKey.name, + URL_PARAMS.sliceId.name, + URL_PARAMS.datasetId.name, +]; +export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [ + URL_PARAMS.nativeFilters.name, + URL_PARAMS.nativeFiltersKey.name, + URL_PARAMS.permalinkKey.name, + URL_PARAMS.preselectFilters.name, +]; + /** * Faster debounce delay for inputs without expensive operation. */ diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx index 899664507947..d1f87ec999e0 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx @@ -135,8 +135,8 @@ test('should show the share actions', async () => { }; render(setup(canShareProps)); await openDropdown(); - expect(screen.getByText('Copy dashboard URL')).toBeInTheDocument(); - expect(screen.getByText('Share dashboard by email')).toBeInTheDocument(); + expect(screen.getByText('Copy permalink to clipboard')).toBeInTheDocument(); + expect(screen.getByText('Share permalink by email')).toBeInTheDocument(); }); test('should render the "Save Modal" when user can save', async () => { diff --git a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx index b7d368ec32db..9375c684af90 100644 --- a/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx @@ -257,8 +257,8 @@ class HeaderActionsDropdown extends React.PureComponent { {userCanShare && ( ; onExploreChart: () => void; forceRefresh: (sliceId: number, dashboardId: number) => void; @@ -309,8 +310,8 @@ class SliceHeaderControls extends React.PureComponent< {supersetCanShare && ( = 5 ? 'left' : 'right'} diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx index da7d196bd8b5..579f9d4b6907 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx @@ -31,7 +31,7 @@ const DASHBOARD_ID = '26'; const createProps = () => ({ addDangerToast: jest.fn(), addSuccessToast: jest.fn(), - url: `/superset/dashboard/${DASHBOARD_ID}/?preselect_filters=%7B%7D`, + url: `/superset/dashboard/${DASHBOARD_ID}`, copyMenuItemTitle: 'Copy dashboard URL', emailMenuItemTitle: 'Share dashboard by email', emailSubject: 'Superset dashboard COVID Vaccine Dashboard', @@ -45,10 +45,10 @@ beforeAll((): void => { // @ts-ignore delete window.location; fetchMock.post( - 'http://localhost/r/shortner/', - { body: 'http://localhost:8088/r/3' }, + `http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`, + { key: '123', url: 'http://localhost/superset/dashboard/p/123/' }, { - sendAsJson: false, + sendAsJson: true, }, ); }); @@ -104,7 +104,7 @@ test('Click on "Copy dashboard URL" and succeed', async () => { await waitFor(() => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost:8088/r/3'); + expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(1); expect(props.addSuccessToast).toBeCalledWith('Copied to clipboard!'); expect(props.addDangerToast).toBeCalledTimes(0); @@ -130,7 +130,7 @@ test('Click on "Copy dashboard URL" and fail', async () => { await waitFor(() => { expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith('http://localhost:8088/r/3'); + expect(spy).toBeCalledWith('http://localhost/superset/dashboard/p/123/'); expect(props.addSuccessToast).toBeCalledTimes(0); expect(props.addDangerToast).toBeCalledTimes(1); expect(props.addDangerToast).toBeCalledWith( @@ -159,14 +159,14 @@ test('Click on "Share dashboard by email" and succeed', async () => { await waitFor(() => { expect(props.addDangerToast).toBeCalledTimes(0); expect(window.location.href).toBe( - 'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%3A8088%2Fr%2F3', + 'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%2Fsuperset%2Fdashboard%2Fp%2F123%2F', ); }); }); test('Click on "Share dashboard by email" and fail', async () => { fetchMock.post( - 'http://localhost/r/shortner/', + `http://localhost/api/v1/dashboard/${DASHBOARD_ID}/permalink`, { status: 404 }, { overwriteRoutes: true }, ); diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index cb31503ac861..c70e47dc3d01 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -17,19 +17,16 @@ * under the License. */ import React from 'react'; -import { useUrlShortener } from 'src/hooks/useUrlShortener'; import copyTextToClipboard from 'src/utils/copy'; -import { t, logging } from '@superset-ui/core'; +import { t, logging, QueryFormData } from '@superset-ui/core'; import { Menu } from 'src/components/Menu'; -import { getUrlParam } from 'src/utils/urlUtils'; -import { postFormData } from 'src/explore/exploreUtils/formData'; -import { useTabId } from 'src/hooks/useTabId'; -import { URL_PARAMS } from 'src/constants'; -import { mountExploreUrl } from 'src/explore/exploreUtils'; import { - createFilterKey, - getFilterValue, -} from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; + getChartPermalink, + getDashboardPermalink, + getUrlParam, +} from 'src/utils/urlUtils'; +import { RESERVED_DASHBOARD_URL_PARAMS, URL_PARAMS } from 'src/constants'; +import { getFilterValue } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue'; interface ShareMenuItemProps { url?: string; @@ -40,12 +37,11 @@ interface ShareMenuItemProps { addDangerToast: Function; addSuccessToast: Function; dashboardId?: string; - formData?: { slice_id: number; datasource: string }; + formData?: Pick; } const ShareMenuItems = (props: ShareMenuItemProps) => { const { - url, copyMenuItemTitle, emailMenuItemTitle, emailSubject, @@ -57,47 +53,25 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { ...rest } = props; - const tabId = useTabId(); - - const getShortUrl = useUrlShortener(url || ''); - - async function getCopyUrl() { - const risonObj = getUrlParam(URL_PARAMS.nativeFilters); - if (typeof risonObj === 'object' || !dashboardId) return null; - const prevData = await getFilterValue( - dashboardId, - getUrlParam(URL_PARAMS.nativeFiltersKey), - ); - const newDataMaskKey = await createFilterKey( - dashboardId, - JSON.stringify(prevData), - tabId, - ); - const newUrl = new URL(`${window.location.origin}${url}`); - newUrl.searchParams.set(URL_PARAMS.nativeFilters.name, newDataMaskKey); - return `${newUrl.pathname}${newUrl.search}`; - } - async function generateUrl() { + // chart if (formData) { - const key = await postFormData( - parseInt(formData.datasource.split('_')[0], 10), - formData, - formData.slice_id, - tabId, - ); - return `${window.location.origin}${mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - [URL_PARAMS.sliceId.name]: formData.slice_id, - })}`; + // we need to remove reserved dashboard url params + return getChartPermalink(formData, RESERVED_DASHBOARD_URL_PARAMS); + } + // dashboard + const nativeFiltersKey = getUrlParam(URL_PARAMS.nativeFiltersKey); + let filterState = {}; + if (nativeFiltersKey && dashboardId) { + filterState = await getFilterValue(dashboardId, nativeFiltersKey); } - const copyUrl = await getCopyUrl(); - return getShortUrl(copyUrl); + return getDashboardPermalink(String(dashboardId), filterState); } async function onCopyLink() { try { - await copyTextToClipboard(await generateUrl()); + const url = await generateUrl(); + await copyTextToClipboard(url); addSuccessToast(t('Copied to clipboard!')); } catch (error) { logging.error(error); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 73a589312acc..309d75dac9a8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -165,6 +165,11 @@ export interface FiltersBarProps { offset: number; } +const EXCLUDED_URL_PARAMS: string[] = [ + URL_PARAMS.nativeFilters.name, + URL_PARAMS.permalinkKey.name, +]; + const publishDataMask = debounce( async ( history, @@ -177,9 +182,9 @@ const publishDataMask = debounce( const { search } = location; const previousParams = new URLSearchParams(search); const newParams = new URLSearchParams(); - let dataMaskKey: string; + let dataMaskKey: string | null; previousParams.forEach((value, key) => { - if (key !== URL_PARAMS.nativeFilters.name) { + if (!EXCLUDED_URL_PARAMS.includes(key)) { newParams.append(key, value); } }); @@ -200,7 +205,9 @@ const publishDataMask = debounce( } else { dataMaskKey = await createFilterKey(dashboardId, dataMask, tabId); } - newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + if (dataMaskKey) { + newParams.set(URL_PARAMS.nativeFiltersKey.name, dataMaskKey); + } // pathname could be updated somewhere else through window.history // keep react router history in sync with window history diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx index 9682fdb7b8f0..ec9735f09169 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/keyValue.tsx @@ -17,6 +17,7 @@ * under the License. */ import { SupersetClient, logging } from '@superset-ui/core'; +import { DashboardPermalinkValue } from 'src/dashboard/types'; const assembleEndpoint = ( dashId: string | number, @@ -58,7 +59,7 @@ export const createFilterKey = ( endpoint: assembleEndpoint(dashId, undefined, tabId), jsonPayload: { value }, }) - .then(r => r.json.key) + .then(r => r.json.key as string) .catch(err => { logging.error(err); return null; @@ -73,3 +74,13 @@ export const getFilterValue = (dashId: string | number, key: string) => logging.error(err); return null; }); + +export const getPermalinkValue = (key: string) => + SupersetClient.get({ + endpoint: `/api/v1/dashboard/permalink/${key}`, + }) + .then(({ json }) => json as DashboardPermalinkValue) + .catch(err => { + logging.error(err); + return null; + }); diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index 878f3d68695a..e5fff328724d 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -49,7 +49,10 @@ import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { canUserEditDashboard } from 'src/dashboard/util/findPermission'; import { getFilterSets } from '../actions/nativeFilters'; -import { getFilterValue } from '../components/nativeFilters/FilterBar/keyValue'; +import { + getFilterValue, + getPermalinkValue, +} from '../components/nativeFilters/FilterBar/keyValue'; import { filterCardPopoverStyle } from '../styles'; export const MigrationContext = React.createContext( @@ -161,12 +164,17 @@ const DashboardPage: FC = () => { useEffect(() => { // eslint-disable-next-line consistent-return async function getDataMaskApplied() { + const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey); const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey); let dataMaskFromUrl = nativeFilterKeyValue || {}; const isOldRison = getUrlParam(URL_PARAMS.nativeFilters); - // check if key from key_value api and get datamask - if (nativeFilterKeyValue) { + if (permalinkKey) { + const permalinkValue = await getPermalinkValue(permalinkKey); + if (permalinkValue) { + dataMaskFromUrl = permalinkValue.state.filterState; + } + } else if (nativeFilterKeyValue) { dataMaskFromUrl = await getFilterValue(id, nativeFilterKeyValue); } if (isOldRison) { diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index fbdf362eea70..dffbd9fbe0be 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -144,3 +144,11 @@ type ActiveFilter = { export type ActiveFilters = { [key: string]: ActiveFilter; }; + +export type DashboardPermalinkValue = { + dashboardId: string; + state: { + filterState: DataMaskStateWithId; + hash: string; + }; +}; diff --git a/superset-frontend/src/explore/components/EmbedCodeButton.jsx b/superset-frontend/src/explore/components/EmbedCodeButton.jsx index 57e6d30de453..71f77a4621fa 100644 --- a/superset-frontend/src/explore/components/EmbedCodeButton.jsx +++ b/superset-frontend/src/explore/components/EmbedCodeButton.jsx @@ -25,6 +25,7 @@ import Icons from 'src/components/Icons'; import { Tooltip } from 'src/components/Tooltip'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { URL_PARAMS } from 'src/constants'; +import { getChartPermalink } from 'src/utils/urlUtils'; export default class EmbedCodeButton extends React.Component { constructor(props) { @@ -32,8 +33,11 @@ export default class EmbedCodeButton extends React.Component { this.state = { height: '400', width: '600', + url: '', + errorMessage: '', }; this.handleInputChange = this.handleInputChange.bind(this); + this.updateUrl = this.updateUrl.bind(this); } handleInputChange(e) { @@ -43,8 +47,21 @@ export default class EmbedCodeButton extends React.Component { this.setState(data); } + updateUrl() { + this.setState({ url: '' }); + getChartPermalink(this.props.formData) + .then(url => this.setState({ errorMessage: '', url })) + .catch(() => { + this.setState({ errorMessage: t('Error') }); + this.props.addDangerToast( + t('Sorry, something went wrong. Try again later.'), + ); + }); + } + generateEmbedHTML() { - const srcLink = `${window.location.href}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; + if (!this.state.url) return ''; + const srcLink = `${this.state.url}?${URL_PARAMS.standalone.name}=1&height=${this.state.height}`; return ( '
@@ -67,7 +86,8 @@ export default class EmbedCodeButton extends React.Component {