From 7383e4348b1800b033b8f828a4af4c24286bf455 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Wed, 22 Jan 2025 15:04:20 +0300 Subject: [PATCH 01/18] fix(timezoneselector): Correct the order to match names first (#31941) --- .../TimezoneSelector.test.tsx | 5 ++--- .../src/components/TimezoneSelector/index.tsx | 21 +++++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx index 0f9309b5a8be3..311c7e56d0fdf 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx @@ -124,13 +124,12 @@ test('can update props and rerender with different values', async () => { timezone="Asia/Dubai" />, ); - expect(screen.getByTitle('GMT +04:00 (Asia/Baku)')).toBeInTheDocument(); + expect(screen.getByTitle('GMT +04:00 (Asia/Dubai)')).toBeInTheDocument(); rerender( , ); - expect(screen.getByTitle('GMT +08:00 (Asia/Brunei)')).toBeInTheDocument(); - expect(onTimezoneChange).toHaveBeenCalledTimes(2); + expect(screen.getByTitle('GMT +08:00 (Australia/Perth)')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/components/TimezoneSelector/index.tsx b/superset-frontend/src/components/TimezoneSelector/index.tsx index 0584655b85c16..329ad3461882b 100644 --- a/superset-frontend/src/components/TimezoneSelector/index.tsx +++ b/superset-frontend/src/components/TimezoneSelector/index.tsx @@ -112,10 +112,23 @@ export default function TimezoneSelector({ // pre-sort timezone options by time offset TIMEZONE_OPTIONS.sort(TIMEZONE_OPTIONS_SORT_COMPARATOR); - const matchTimezoneToOptions = (timezone: string) => - TIMEZONE_OPTIONS.find( - option => option.offsets === getOffsetKey(timezone), - )?.value || DEFAULT_TIMEZONE.value; + const matchTimezoneToOptions = (timezone: string) => { + const offsetKey = getOffsetKey(timezone); + let fallbackValue: string | undefined; + + for (const option of TIMEZONE_OPTIONS) { + if ( + option.offsets === offsetKey && + option.timezoneName === timezone + ) { + return option.value; + } + if (!fallbackValue && option.offsets === offsetKey) { + fallbackValue = option.value; + } + } + return fallbackValue || DEFAULT_TIMEZONE.value; + }; const validTimezone = matchTimezoneToOptions( timezone || extendedDayjs.tz.guess(), From b74da7963b94850b0b0a123066c4921a79499aab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90=E1=BB=97=20Tr=E1=BB=8Dng=20H=E1=BA=A3i?= <41283691+hainenber@users.noreply.github.com> Date: Wed, 22 Jan 2025 21:13:49 +0700 Subject: [PATCH 02/18] chore(fe): migrate 6 Enzyme-based unit tests to RTL (#31819) Signed-off-by: hainenber Co-authored-by: JUST.in DO IT --- .../components/QueryTable/QueryTable.test.tsx | 16 ++--- ...geLoader.test.jsx => ImageLoader.test.tsx} | 45 ++++++++----- .../components/ListViewCard/ImageLoader.tsx | 1 + ...iewCard.test.jsx => ListViewCard.test.tsx} | 25 +++----- .../gridComponents/Divider.test.jsx | 44 +++++++------ .../controls/ViewportControl.test.jsx | 32 ++++------ .../RowLevelSecurityList.test.tsx | 64 ++++++------------- 7 files changed, 104 insertions(+), 123 deletions(-) rename superset-frontend/src/components/ListViewCard/{ImageLoader.test.jsx => ImageLoader.test.tsx} (65%) rename superset-frontend/src/components/ListViewCard/{ListViewCard.test.jsx => ListViewCard.test.tsx} (70%) diff --git a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx index 4be4c995452c5..d10d926b11131 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx @@ -19,12 +19,10 @@ import { isValidElement } from 'react'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; -import { styledMount as mount } from 'spec/helpers/theming'; import QueryTable from 'src/SqlLab/components/QueryTable'; -import TableView from 'src/components/TableView'; -import TableCollection from 'src/components/TableCollection'; import { Provider } from 'react-redux'; import { runningQuery, successfulQuery, user } from 'src/SqlLab/fixtures'; +import { render, screen } from 'spec/helpers/testing-library'; const mockedProps = { queries: [runningQuery, successfulQuery], @@ -43,15 +41,15 @@ test('renders a proper table', () => { user, }); - const wrapper = mount( + const { container } = render( , ); - const tableWrapper = wrapper.find(TableView).find(TableCollection); - expect(wrapper.find(TableView)).toExist(); - expect(tableWrapper.find('table')).toExist(); - expect(tableWrapper.find('table').find('thead').find('tr')).toHaveLength(1); - expect(tableWrapper.find('table').find('tbody').find('tr')).toHaveLength(2); + expect(screen.getByTestId('listview-table')).toBeVisible(); // Presence of TableCollection + expect(screen.getByRole('table')).toBeVisible(); + expect(container.querySelector('.table-condensed')).toBeVisible(); // Presence of TableView signature class + expect(container.querySelectorAll('table > thead > tr')).toHaveLength(1); + expect(container.querySelectorAll('table > tbody > tr')).toHaveLength(2); }); diff --git a/superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx b/superset-frontend/src/components/ListViewCard/ImageLoader.test.tsx similarity index 65% rename from superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx rename to superset-frontend/src/components/ListViewCard/ImageLoader.test.tsx index cdb03caa55288..2449b838f28a0 100644 --- a/superset-frontend/src/components/ListViewCard/ImageLoader.test.jsx +++ b/superset-frontend/src/components/ListViewCard/ImageLoader.test.tsx @@ -16,11 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { styledMount as mount } from 'spec/helpers/theming'; import fetchMock from 'fetch-mock'; -import ImageLoader from 'src/components/ListViewCard/ImageLoader'; -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; +import ImageLoader, { + BackgroundPosition, +} from 'src/components/ListViewCard/ImageLoader'; +import { render, screen } from 'spec/helpers/testing-library'; global.URL.createObjectURL = jest.fn(() => '/local_url'); const blob = new Blob([], { type: 'image/png' }); @@ -37,36 +38,46 @@ describe('ImageLoader', () => { const defaultProps = { src: '/thumbnail', fallback: '/fallback', + position: 'top' as BackgroundPosition, }; - const factory = (extraProps = {}) => { + const setup = (extraProps = {}) => { const props = { ...defaultProps, ...extraProps }; - return mount(); + return render(); }; - afterEach(fetchMock.resetHistory); + afterEach(() => fetchMock.resetHistory()); it('is a valid element', async () => { - const wrapper = factory(); - await waitForComponentToPaint(wrapper); - expect(wrapper.find(ImageLoader)).toExist(); + setup(); + expect(await screen.findByTestId('image-loader')).toBeVisible(); }); it('fetches loads the image in the background', async () => { - const wrapper = factory(); - expect(wrapper.find('div').props().src).toBe('/fallback'); - await waitForComponentToPaint(wrapper); + setup(); + expect(screen.getByTestId('image-loader')).toHaveAttribute( + 'src', + '/fallback', + ); expect(fetchMock.calls(/thumbnail/)).toHaveLength(1); expect(global.URL.createObjectURL).toHaveBeenCalled(); - expect(wrapper.find('div').props().src).toBe('/local_url'); + expect(await screen.findByTestId('image-loader')).toHaveAttribute( + 'src', + '/local_url', + ); }); it('displays fallback image when response is not an image', async () => { fetchMock.once('/thumbnail2', {}); - const wrapper = factory({ src: '/thumbnail2' }); - expect(wrapper.find('div').props().src).toBe('/fallback'); - await waitForComponentToPaint(wrapper); + setup({ src: '/thumbnail2' }); + expect(screen.getByTestId('image-loader')).toHaveAttribute( + 'src', + '/fallback', + ); expect(fetchMock.calls(/thumbnail2/)).toHaveLength(1); - expect(wrapper.find('div').props().src).toBe('/fallback'); + expect(await screen.findByTestId('image-loader')).toHaveAttribute( + 'src', + '/fallback', + ); }); }); diff --git a/superset-frontend/src/components/ListViewCard/ImageLoader.tsx b/superset-frontend/src/components/ListViewCard/ImageLoader.tsx index 86d115365e16f..ae4c429843129 100644 --- a/superset-frontend/src/components/ListViewCard/ImageLoader.tsx +++ b/superset-frontend/src/components/ListViewCard/ImageLoader.tsx @@ -77,6 +77,7 @@ export default function ImageLoader({ return ( '/local_url'); fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false }); @@ -44,25 +42,22 @@ describe('ListViewCard', () => { ), }; - let wrapper; - const factory = (extraProps = {}) => { - const props = { ...defaultProps, ...extraProps }; - return mount(); - }; - beforeEach(async () => { - wrapper = factory(); - await waitForComponentToPaint(wrapper); + beforeEach(() => { + const props = { ...defaultProps }; + render(); }); it('is a valid element', () => { - expect(wrapper.find(ListViewCard)).toExist(); + expect(screen.getByTestId('styled-card')).toBeInTheDocument(); }); it('renders Actions', () => { - expect(wrapper.find(ListViewCard.Actions)).toExist(); + expect(screen.getByTestId('card-actions')).toBeVisible(); + expect(screen.getByText('Action 1')).toBeVisible(); + expect(screen.getByText('Action 2')).toBeVisible(); }); - it('renders and ImageLoader', () => { - expect(wrapper.find(ImageLoader)).toExist(); + it('renders an ImageLoader', () => { + expect(screen.getByTestId('image-loader')).toBeVisible(); }); }); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Divider.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Divider.test.jsx index 02d55da08aaf9..76bd0dfb7da09 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Divider.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Divider.test.jsx @@ -16,20 +16,18 @@ * specific language governing permissions and limitations * under the License. */ -import { styledMount as mount } from 'spec/helpers/theming'; import sinon from 'sinon'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; -import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; -import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; -import { Draggable } from 'src/dashboard/components/dnd/DragDroppable'; import Divider from 'src/dashboard/components/gridComponents/Divider'; import newComponentFactory from 'src/dashboard/util/newComponentFactory'; import { DIVIDER_TYPE, DASHBOARD_GRID_TYPE, } from 'src/dashboard/util/componentTypes'; +import { screen, render } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; describe('Divider', () => { const props = { @@ -44,42 +42,48 @@ describe('Divider', () => { deleteComponent() {}, }; - function setup(overrideProps) { + const setup = overrideProps => // We have to wrap provide DragDropContext for the underlying DragDroppable // otherwise we cannot assert on DragDroppable children - const wrapper = mount( + render( , + { + useDnd: true, + }, ); - return wrapper; - } it('should render a Draggable', () => { - const wrapper = setup(); - expect(wrapper.find(Draggable)).toExist(); + setup(); + expect(screen.getByTestId('dragdroppable-object')).toBeInTheDocument(); }); it('should render a div with class "dashboard-component-divider"', () => { - const wrapper = setup(); - expect(wrapper.find('.dashboard-component-divider')).toExist(); + const { container } = setup(); + expect( + container.querySelector('.dashboard-component-divider'), + ).toBeInTheDocument(); }); it('should render a HoverMenu with DeleteComponentButton in editMode', () => { - let wrapper = setup(); - expect(wrapper.find(HoverMenu)).not.toExist(); - expect(wrapper.find(DeleteComponentButton)).not.toExist(); + setup(); + expect(screen.queryByTestId('hover-menu')).not.toBeInTheDocument(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); // we cannot set props on the Divider because of the WithDragDropContext wrapper - wrapper = setup({ editMode: true }); - expect(wrapper.find(HoverMenu)).toExist(); - expect(wrapper.find(DeleteComponentButton)).toExist(); + setup({ editMode: true }); + expect(screen.getByTestId('hover-menu')).toBeInTheDocument(); + expect(screen.getByRole('button').firstChild).toHaveAttribute( + 'aria-label', + 'trash', + ); }); it('should call deleteComponent when deleted', () => { const deleteComponent = sinon.spy(); - const wrapper = setup({ editMode: true, deleteComponent }); - wrapper.find(DeleteComponentButton).simulate('click'); + setup({ editMode: true, deleteComponent }); + userEvent.click(screen.getByRole('button')); expect(deleteComponent.callCount).toBe(1); }); }); diff --git a/superset-frontend/src/explore/components/controls/ViewportControl.test.jsx b/superset-frontend/src/explore/components/controls/ViewportControl.test.jsx index 282e39f6b1ec1..b1b958d6dab89 100644 --- a/superset-frontend/src/explore/components/controls/ViewportControl.test.jsx +++ b/superset-frontend/src/explore/components/controls/ViewportControl.test.jsx @@ -16,13 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { styledMount as mount } from 'spec/helpers/theming'; -import Popover from 'src/components/Popover'; - -import Label from 'src/components/Label'; import ViewportControl from 'src/explore/components/controls/ViewportControl'; -import TextControl from 'src/explore/components/controls/TextControl'; -import ControlHeader from 'src/explore/components/ControlHeader'; +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; const defaultProps = { value: { @@ -33,29 +29,27 @@ const defaultProps = { pitch: 0, }, name: 'foo', + label: 'bar', }; +const renderedCoordinate = '6° 51\' 8.50" | 31° 13\' 21.56"'; describe('ViewportControl', () => { - let wrapper; - let inst; beforeEach(() => { - wrapper = mount(); - inst = wrapper.instance(); + render(); }); - it('renders a OverlayTrigger', () => { - const controlHeader = wrapper.find(ControlHeader); - expect(controlHeader).toHaveLength(1); - expect(wrapper.find(Popover)).toExist(); + it('renders a OverlayTrigger if clicked', () => { + expect(screen.getByTestId('foo-header')).toBeInTheDocument(); // Presence of ControlHeader + userEvent.click(screen.getByText(renderedCoordinate)); + expect(screen.getByText('Viewport')).toBeInTheDocument(); // Presence of Popover }); - it('renders a Popover with 5 TextControl', () => { - const popOver = mount(inst.renderPopover()); - expect(popOver.find(TextControl)).toHaveLength(5); + it('renders a Popover with 5 TextControl if clicked', () => { + userEvent.click(screen.getByText(renderedCoordinate)); + expect(screen.queryAllByTestId('inline-name')).toHaveLength(5); }); it('renders a summary in the label', () => { - const label = wrapper.find(Label).first(); - expect(label.render().text()).toBe('6° 51\' 8.50" | 31° 13\' 21.56"'); + expect(screen.getByText(renderedCoordinate)).toBeInTheDocument(); }); }); diff --git a/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx b/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx index 0cbf349491797..7565877f40517 100644 --- a/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx +++ b/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx @@ -21,12 +21,6 @@ import { render, screen, within } from 'spec/helpers/testing-library'; import { act } from 'react-dom/test-utils'; import { MemoryRouter } from 'react-router-dom'; import { QueryParamProvider } from 'use-query-params'; -import { styledMount as mount } from 'spec/helpers/theming'; -import { Provider } from 'react-redux'; -import configureStore from 'redux-mock-store'; -import thunk from 'redux-thunk'; -import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; -import ListView from 'src/components/ListView/ListView'; import userEvent from '@testing-library/user-event'; import RowLevelSecurityList from '.'; @@ -101,43 +95,6 @@ const mockUser = { userId: 1, }; -const mockedProps = {}; - -const mockStore = configureStore([thunk]); -const store = mockStore({}); - -describe('RulesList Enzyme', () => { - let wrapper: any; - - beforeAll(async () => { - fetchMock.resetHistory(); - wrapper = mount( - - - - - , - ); - - await waitForComponentToPaint(wrapper); - }); - - it('renders', () => { - expect(wrapper.find(RowLevelSecurityList)).toExist(); - }); - it('renders a ListView', () => { - expect(wrapper.find(ListView)).toExist(); - }); - it('fetched data', () => { - // wrapper.update(); - const apiCalls = fetchMock.calls(/rowlevelsecurity\/\?q/); - expect(apiCalls).toHaveLength(1); - expect(apiCalls[0][0]).toMatchInlineSnapshot( - `"http://localhost/api/v1/rowlevelsecurity/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)"`, - ); - }); -}); - describe('RuleList RTL', () => { async function renderAndWait() { const mounted = act(async () => { @@ -154,6 +111,27 @@ describe('RuleList RTL', () => { return mounted; } + it('renders', async () => { + await renderAndWait(); + expect(screen.getByText('Row Level Security')).toBeVisible(); + }); + + it('renders a ListView', async () => { + await renderAndWait(); + expect(screen.getByTestId('rls-list-view')).toBeInTheDocument(); + }); + + it('fetched data', async () => { + fetchMock.resetHistory(); + await renderAndWait(); + const apiCalls = fetchMock.calls(/rowlevelsecurity\/\?q/); + expect(apiCalls).toHaveLength(1); + expect(apiCalls[0][0]).toMatchInlineSnapshot( + `"http://localhost/api/v1/rowlevelsecurity/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)"`, + ); + fetchMock.resetHistory(); + }); + it('renders add rule button on empty state', async () => { fetchMock.get( ruleListEndpoint, From e4e07eef5ae27c8507cd374a08121b4b547c6aec Mon Sep 17 00:00:00 2001 From: Alexandru Soare <37236580+alexandrusoare@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:54:29 +0200 Subject: [PATCH 03/18] feat(CalendarFrame): adding previous calendar quarter (#31889) Co-authored-by: Diego Pucci --- superset-frontend/src/GlobalStyles.tsx | 9 +- .../tests/CalendarFrame.test.tsx | 90 +++++++++++++++++++ .../controls/DateFilterControl/types.ts | 2 + .../DateFilterControl/utils/constants.ts | 3 + superset/utils/date_parser.py | 9 ++ tests/unit_tests/utils/date_parser_tests.py | 28 ++++++ 6 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 superset-frontend/src/explore/components/controls/DateFilterControl/tests/CalendarFrame.test.tsx diff --git a/superset-frontend/src/GlobalStyles.tsx b/superset-frontend/src/GlobalStyles.tsx index c305fb214a0a8..7829f2ff58859 100644 --- a/superset-frontend/src/GlobalStyles.tsx +++ b/superset-frontend/src/GlobalStyles.tsx @@ -39,11 +39,16 @@ export const GlobalStyles = () => ( .echarts-tooltip[style*='visibility: hidden'] { display: none !important; } + // Ant Design is applying inline z-index styles causing troubles + // TODO: Remove z-indexes when Ant Design is fully upgraded to v5 + // Prefer vanilla Ant Design z-indexes that should work out of the box .ant-popover, .antd5-dropdown, .ant-dropdown, - .ant-select-dropdown { - z-index: ${theme.zIndex.max}; + .ant-select-dropdown, + .antd5-modal-wrap, + .antd5-modal-mask { + z-index: ${theme.zIndex.max} !important; } // TODO: Remove when buttons have been upgraded to Ant Design 5. diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/tests/CalendarFrame.test.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/tests/CalendarFrame.test.tsx new file mode 100644 index 0000000000000..163ce4caf1a4a --- /dev/null +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/tests/CalendarFrame.test.tsx @@ -0,0 +1,90 @@ +/** + * 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 { render, screen, fireEvent } from 'spec/helpers/testing-library'; +import { CalendarFrame } from '../components/CalendarFrame'; +import { PreviousCalendarWeek, PreviousCalendarQuarter } from '../types'; +import { CALENDAR_RANGE_OPTIONS } from '../utils/constants'; + +describe('CalendarFrame', () => { + it('calls onChange with PreviousCalendarWeek if value is not in CALENDAR_RANGE_SET', () => { + const mockOnChange = jest.fn(); + render(); + + expect(mockOnChange).toHaveBeenCalledWith(PreviousCalendarWeek); + }); + + it('renders null if value is not in CALENDAR_RANGE_SET', () => { + render(); + expect( + screen.queryByText('Configure Time Range: Previous...'), + ).not.toBeInTheDocument(); + }); + + it('renders the correct number of radio options', () => { + render(); + const radios = screen.getAllByRole('radio'); + expect(radios).toHaveLength(CALENDAR_RANGE_OPTIONS.length); + CALENDAR_RANGE_OPTIONS.forEach(option => { + expect(screen.getByText(option.label)).toBeInTheDocument(); + }); + }); + + it('calls onChange with the correct value when a radio button is selected', () => { + const mockOnChange = jest.fn(); + render( + , + ); + + const secondOption = CALENDAR_RANGE_OPTIONS[1]; + const radio = screen.getByLabelText(secondOption.label); + fireEvent.click(radio); + + expect(mockOnChange).toHaveBeenCalledWith(secondOption.value); + }); + + it('renders the section title correctly', () => { + render( + , + ); + expect( + screen.getByText('Configure Time Range: Previous...'), + ).toBeInTheDocument(); + }); + + it('ensures the third option is PreviousCalendarQuarter', () => { + render( + , + ); + + const thirdOption = CALENDAR_RANGE_OPTIONS[2]; + expect(thirdOption.value).toBe(PreviousCalendarQuarter); + + expect(screen.getByLabelText(thirdOption.label)).toBeInTheDocument(); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/types.ts b/superset-frontend/src/explore/components/controls/DateFilterControl/types.ts index 0d0fbb9724d73..c1aa843448085 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/types.ts +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/types.ts @@ -80,10 +80,12 @@ export type CommonRangeType = export const PreviousCalendarWeek = 'previous calendar week'; export const PreviousCalendarMonth = 'previous calendar month'; +export const PreviousCalendarQuarter = 'previous calendar quarter'; export const PreviousCalendarYear = 'previous calendar year'; export type CalendarRangeType = | typeof PreviousCalendarWeek | typeof PreviousCalendarMonth + | typeof PreviousCalendarQuarter | typeof PreviousCalendarYear; export const CurrentDay = 'Current day'; diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts b/superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts index 4a116fb65e444..16b24d7a0b322 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts @@ -21,6 +21,7 @@ import { SelectOptionType, PreviousCalendarWeek, PreviousCalendarMonth, + PreviousCalendarQuarter, PreviousCalendarYear, CommonRangeType, CalendarRangeType, @@ -56,6 +57,7 @@ export const COMMON_RANGE_VALUES_SET = new Set( export const CALENDAR_RANGE_OPTIONS: SelectOptionType[] = [ { value: PreviousCalendarWeek, label: t('previous calendar week') }, { value: PreviousCalendarMonth, label: t('previous calendar month') }, + { value: PreviousCalendarQuarter, label: t('previous calendar quarter') }, { value: PreviousCalendarYear, label: t('previous calendar year') }, ]; export const CALENDAR_RANGE_VALUES_SET = new Set( @@ -119,6 +121,7 @@ export const COMMON_RANGE_SET: Set = new Set([ export const CALENDAR_RANGE_SET: Set = new Set([ PreviousCalendarWeek, PreviousCalendarMonth, + PreviousCalendarQuarter, PreviousCalendarYear, ]); diff --git a/superset/utils/date_parser.py b/superset/utils/date_parser.py index cd404a3fb4733..585c837f9d06e 100644 --- a/superset/utils/date_parser.py +++ b/superset/utils/date_parser.py @@ -369,6 +369,15 @@ def get_since_until( # pylint: disable=too-many-arguments,too-many-locals,too-m and separator not in time_range ): time_range = "DATETRUNC(DATEADD(DATETIME('today'), -1, MONTH), MONTH) : DATETRUNC(DATETIME('today'), MONTH)" # pylint: disable=line-too-long,useless-suppression # noqa: E501 + if ( + time_range + and time_range.startswith("previous calendar quarter") + and separator not in time_range + ): + time_range = ( + "DATETRUNC(DATEADD(DATETIME('today'), -1, QUARTER), QUARTER) : " + "DATETRUNC(DATETIME('today'), QUARTER)" # pylint: disable=line-too-long,useless-suppression # noqa: E501 + ) if ( time_range and time_range.startswith("previous calendar year") diff --git a/tests/unit_tests/utils/date_parser_tests.py b/tests/unit_tests/utils/date_parser_tests.py index 61121a28a9d63..824b21d167431 100644 --- a/tests/unit_tests/utils/date_parser_tests.py +++ b/tests/unit_tests/utils/date_parser_tests.py @@ -19,6 +19,7 @@ from typing import Optional from unittest.mock import Mock, patch +import freezegun import pytest from dateutil.relativedelta import relativedelta @@ -316,6 +317,33 @@ def test_get_since_until_instant_time_comparison_enabled() -> None: assert result == expected +def test_previous_calendar_quarter(): + with freezegun.freeze_time("2023-01-15"): + result = get_since_until("previous calendar quarter") + expected = (datetime(2022, 10, 1), datetime(2023, 1, 1)) + assert result == expected + + with freezegun.freeze_time("2023, 4, 15"): + result = get_since_until("previous calendar quarter") + expected = (datetime(2023, 1, 1), datetime(2023, 4, 1)) + assert result == expected + + with freezegun.freeze_time("2023, 8, 15"): + result = get_since_until("previous calendar quarter") + expected = (datetime(2023, 4, 1), datetime(2023, 7, 1)) + assert result == expected + + with freezegun.freeze_time("2023, 10, 15"): + result = get_since_until("previous calendar quarter") + expected = (datetime(2023, 7, 1), datetime(2023, 10, 1)) + assert result == expected + + with freezegun.freeze_time("2024, 1, 1"): + result = get_since_until("previous calendar quarter") + expected = (datetime(2023, 10, 1), datetime(2024, 1, 1)) + assert result == expected + + @patch("superset.utils.date_parser.parse_human_datetime", mock_parse_human_datetime) def test_datetime_eval() -> None: result = datetime_eval("datetime('now')") From f5fff5eaadeb19067283e475b78ec87e60b41556 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:38:44 -0300 Subject: [PATCH 04/18] refactor: Removes legacy CSS template endpoint (#31942) --- .../components/CssEditor/CssEditor.test.tsx | 2 +- .../dashboard/components/CssEditor/index.tsx | 5 ++-- superset/initialization/__init__.py | 6 +---- superset/views/css_templates.py | 26 ++----------------- 4 files changed, 7 insertions(+), 32 deletions(-) diff --git a/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx b/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx index 9bfe43ece51b8..f66ca1702788a 100644 --- a/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx +++ b/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx @@ -38,7 +38,7 @@ const templates = [ { template_name: 'Template C', css: 'background-color: yellow;' }, ]; -fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', { +fetchMock.get('glob:*/api/v1/css_template*', { result: templates, }); diff --git a/superset-frontend/src/dashboard/components/CssEditor/index.tsx b/superset-frontend/src/dashboard/components/CssEditor/index.tsx index 996afcca4407b..e2748c8b0b1d7 100644 --- a/superset-frontend/src/dashboard/components/CssEditor/index.tsx +++ b/superset-frontend/src/dashboard/components/CssEditor/index.tsx @@ -17,6 +17,7 @@ * under the License. */ import { Key, ReactNode, PureComponent } from 'react'; +import rison from 'rison'; import { AntdDropdown } from 'src/components'; import { Menu } from 'src/components/Menu'; import Button from 'src/components/Button'; @@ -73,8 +74,8 @@ class CssEditor extends PureComponent { componentDidMount() { AceCssEditor.preload(); - - SupersetClient.get({ endpoint: '/csstemplateasyncmodelview/api/read' }) + const query = rison.encode({ columns: ['template_name', 'css'] }); + SupersetClient.get({ endpoint: `/api/v1/css_template/?q=${query}` }) .then(({ json }) => { const templates = json.result.map( (row: { template_name: string; css: string }) => ({ diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index a57325c359076..2fe230355905e 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -165,10 +165,7 @@ def init_views(self) -> None: from superset.views.api import Api from superset.views.chart.views import SliceAsync, SliceModelView from superset.views.core import Superset - from superset.views.css_templates import ( - CssTemplateAsyncModelView, - CssTemplateModelView, - ) + from superset.views.css_templates import CssTemplateModelView from superset.views.dashboard.views import ( Dashboard, DashboardModelView, @@ -297,7 +294,6 @@ def init_views(self) -> None: # Setup views with no menu # appbuilder.add_view_no_menu(Api) - appbuilder.add_view_no_menu(CssTemplateAsyncModelView) appbuilder.add_view_no_menu(Dashboard) appbuilder.add_view_no_menu(DashboardModelViewAsync) appbuilder.add_view_no_menu(Datasource) diff --git a/superset/views/css_templates.py b/superset/views/css_templates.py index d629fbfcc56d5..b0bb5e97f3070 100644 --- a/superset/views/css_templates.py +++ b/superset/views/css_templates.py @@ -15,18 +15,13 @@ # specific language governing permissions and limitations # under the License. from flask_appbuilder.api import expose -from flask_appbuilder.baseviews import expose_api from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.security.decorators import ( - has_access, - has_access_api, - permission_name, -) +from flask_appbuilder.security.decorators import has_access from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models import core as models from superset.superset_typing import FlaskResponse -from superset.views.base import DeleteMixin, deprecated, SupersetModelView +from superset.views.base import DeleteMixin, SupersetModelView class CssTemplateModelView( # pylint: disable=too-many-ancestors @@ -43,20 +38,3 @@ class CssTemplateModelView( # pylint: disable=too-many-ancestors @has_access def list(self) -> FlaskResponse: return super().render_app_template() - - -class CssTemplateAsyncModelView( # pylint: disable=too-many-ancestors - CssTemplateModelView -): - include_route_methods = RouteMethod.API_READ - class_permission_name = "CssTemplate" - method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP - - list_columns = ["template_name", "css"] - - @expose_api(name="read", url="/api/read", methods=["GET"]) - @has_access_api - @permission_name("list") - @deprecated(eol_version="5.0.0") - def api_read(self) -> FlaskResponse: - return super().api_read() From 7cf726708520c6485b12dab2c5416b13bd83670b Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:39:04 -0300 Subject: [PATCH 05/18] refactor: Removes legacy dashboard endpoints (#31943) --- superset/initialization/__init__.py | 2 - superset/views/dashboard/views.py | 75 ++--------------------------- 2 files changed, 3 insertions(+), 74 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 2fe230355905e..67426b2de3568 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -169,7 +169,6 @@ def init_views(self) -> None: from superset.views.dashboard.views import ( Dashboard, DashboardModelView, - DashboardModelViewAsync, ) from superset.views.database.views import DatabaseView from superset.views.datasource.views import DatasetEditor, Datasource @@ -295,7 +294,6 @@ def init_views(self) -> None: # appbuilder.add_view_no_menu(Api) appbuilder.add_view_no_menu(Dashboard) - appbuilder.add_view_no_menu(DashboardModelViewAsync) appbuilder.add_view_no_menu(Datasource) appbuilder.add_view_no_menu(DatasetEditor) appbuilder.add_view_no_menu(EmbeddedView) diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 8a419fcb26f97..fb70c7b396394 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -17,17 +17,12 @@ import builtins from typing import Callable, Union -from flask import g, redirect, request, Response +from flask import g, redirect, Response from flask_appbuilder import expose from flask_appbuilder.actions import action -from flask_appbuilder.baseviews import expose_api from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.security.decorators import ( - has_access, - has_access_api, - permission_name, -) -from flask_babel import gettext as __, lazy_gettext as _ +from flask_appbuilder.security.decorators import has_access +from flask_babel import gettext as __ from flask_login import AnonymousUserMixin, login_user from superset import db, event_logger, is_feature_enabled @@ -39,8 +34,6 @@ BaseSupersetView, common_bootstrap_payload, DeleteMixin, - deprecated, - generate_download_headers, SupersetModelView, ) from superset.views.dashboard.mixin import DashboardMixin @@ -61,20 +54,6 @@ class DashboardModelView(DashboardMixin, SupersetModelView, DeleteMixin): # pyl "download_dashboards", } - @expose_api(name="read", url="/api/read", methods=["GET"]) - @has_access_api - @permission_name("list") - @deprecated(eol_version="5.0.0") - def api_read(self) -> FlaskResponse: - return super().api_read() - - @expose_api(name="delete", url="/api/delete/", methods=["DELETE"]) - @has_access_api - @permission_name("delete") - @deprecated(eol_version="5.0.0") - def api_delete(self, pk: int) -> FlaskResponse: - return super().delete(pk) - @has_access @expose("/list/") def list(self) -> FlaskResponse: @@ -90,22 +69,6 @@ def mulexport( ids = "".join(f"&id={d.id}" for d in items) return redirect(f"/dashboard/export_dashboards_form?{ids[1:]}") - @event_logger.log_this - @has_access - @expose("/export_dashboards_form") - @deprecated(eol_version="5.0.0") - def download_dashboards(self) -> FlaskResponse: - if request.args.get("action") == "go": - ids = set(request.args.getlist("id")) - return Response( - DashboardModel.export_dashboards(ids), - headers=generate_download_headers("json"), - mimetype="application/text", - ) - return self.render_template( - "superset/export_dashboards.html", dashboards_url="/dashboard/list" - ) - class Dashboard(BaseSupersetView): """The base views for Superset!""" @@ -163,35 +126,3 @@ def embedded( bootstrap_data, default=json.pessimistic_json_iso_dttm_ser ), ) - - -class DashboardModelViewAsync(DashboardModelView): # pylint: disable=too-many-ancestors - route_base = "/dashboardasync" - class_permission_name = "Dashboard" - method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP - - include_route_methods = {RouteMethod.API_READ} - - list_columns = [ - "id", - "dashboard_link", - "creator", - "modified", - "dashboard_title", - "changed_on", - "url", - "changed_by_name", - ] - label_columns = { - "dashboard_link": _("Dashboard"), - "dashboard_title": _("Title"), - "creator": _("Creator"), - "modified": _("Modified"), - } - - @expose_api(name="read", url="/api/read", methods=["GET"]) - @has_access_api - @permission_name("list") - @deprecated(eol_version="5.0.0") - def api_read(self) -> FlaskResponse: - return super().api_read() From 1d6423e71f926f89980640ddf917f4abdc2b37a3 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Wed, 22 Jan 2025 11:18:52 -0800 Subject: [PATCH 06/18] fix(sqllab): Missing allowHTML props in ResultTableExtension (#31960) --- .../packages/superset-ui-core/src/ui-overrides/types.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts index 775e2c129ad15..3f0ff82b2fcb5 100644 --- a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts @@ -134,13 +134,14 @@ export interface SQLFormExtensionProps { startQuery: (ctasArg?: any, ctas_method?: any) => void; } -export interface SQLResultTableExtentionProps { +export interface SQLResultTableExtensionProps { queryId: string; orderedColumnKeys: string[]; data: Record[]; height: number; filterText?: string; expandedColumns?: string[]; + allowHTML?: boolean; } /** @@ -223,7 +224,7 @@ export type Extensions = Partial<{ 'database.delete.related': ComponentType; 'dataset.delete.related': ComponentType; 'sqleditor.extension.form': ComponentType; - 'sqleditor.extension.resultTable': ComponentType; + 'sqleditor.extension.resultTable': ComponentType; 'dashboard.slice.header': ComponentType; 'sqleditor.extension.customAutocomplete': ( args: CustomAutoCompleteArgs, From 7482b20f7b3ab99d4ec0ac3566810b4a20e53307 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:46:06 -0800 Subject: [PATCH 07/18] chore: replace selenium user with fixed user (#31844) --- UPDATING.md | 1 + docs/docs/configuration/alerts-reports.mdx | 5 +- docs/docs/configuration/cache.mdx | 7 +- .../src/components/ListViewCard/index.tsx | 2 +- .../components/AddSliceCard/AddSliceCard.tsx | 2 +- superset-frontend/src/types/Dashboard.ts | 2 +- superset/commands/report/alert.py | 2 +- superset/commands/report/execute.py | 8 +- superset/config.py | 41 +++++--- superset/dashboards/schemas.py | 2 +- superset/models/dashboard.py | 9 +- superset/models/slice.py | 9 +- superset/tasks/cache.py | 96 ++++++++++++------- superset/tasks/exceptions.py | 4 + superset/tasks/thumbnails.py | 6 +- superset/tasks/types.py | 20 +++- superset/tasks/utils.py | 58 +++++------ superset/thumbnails/digest.py | 40 ++++---- superset/utils/screenshots.py | 13 ++- .../integration_tests/reports/alert_tests.py | 12 +-- .../reports/commands_tests.py | 2 +- tests/integration_tests/strategy_tests.py | 31 +++--- tests/integration_tests/thumbnails_tests.py | 52 ++++------ tests/unit_tests/tasks/test_utils.py | 59 +++++++----- tests/unit_tests/thumbnails/test_digest.py | 36 +++++-- 25 files changed, 304 insertions(+), 215 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 663df68cd9b69..2a9e8992d85ec 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [31844](https://github.com/apache/superset/pull/31844) The `ALERT_REPORTS_EXECUTE_AS` and `THUMBNAILS_EXECUTE_AS` config parameters have been renamed to `ALERT_REPORTS_EXECUTORS` and `THUMBNAILS_EXECUTORS` respectively. A new config flag `CACHE_WARMUP_EXECUTORS` has also been introduced to be able to control which user is used to execute cache warmup tasks. Finally, the config flag `THUMBNAILS_SELENIUM_USER` has been removed. To use a fixed executor for async tasks, use the new `FixedExecutor` class. See the config and docs for more info on setting up different executor profiles. - [31894](https://github.com/apache/superset/pull/31894) Domain sharding is deprecated in favor of HTTP2. The `SUPERSET_WEBSERVER_DOMAINS` configuration will be removed in the next major version (6.0) - [31774](https://github.com/apache/superset/pull/31774): Fixes the spelling of the `USE-ANALAGOUS-COLORS` feature flag. Please update any scripts/configuration item to use the new/corrected `USE-ANALOGOUS-COLORS` flag spelling. - [31582](https://github.com/apache/superset/pull/31582) Removed the legacy Area, Bar, Event Flow, Heatmap, Histogram, Line, Sankey, and Sankey Loop charts. They were all automatically migrated to their ECharts counterparts with the exception of the Event Flow and Sankey Loop charts which were removed as they were not actively maintained and not widely used. If you were using the Event Flow or Sankey Loop charts, you will need to find an alternative solution. diff --git a/docs/docs/configuration/alerts-reports.mdx b/docs/docs/configuration/alerts-reports.mdx index 293ed3f71ff11..5ff1ef4b819ef 100644 --- a/docs/docs/configuration/alerts-reports.mdx +++ b/docs/docs/configuration/alerts-reports.mdx @@ -177,10 +177,9 @@ By default, Alerts and Reports are executed as the owner of the alert/report obj just change the config as follows (`admin` in this example): ```python -from superset.tasks.types import ExecutorType +from superset.tasks.types import FixedExecutor -THUMBNAIL_SELENIUM_USER = 'admin' -ALERT_REPORTS_EXECUTE_AS = [ExecutorType.SELENIUM] +ALERT_REPORTS_EXECUTORS = [FixedExecutor("admin")] ``` Please refer to `ExecutorType` in the codebase for other executor types. diff --git a/docs/docs/configuration/cache.mdx b/docs/docs/configuration/cache.mdx index 6d761c56b7113..c0eadca95bfaf 100644 --- a/docs/docs/configuration/cache.mdx +++ b/docs/docs/configuration/cache.mdx @@ -94,10 +94,9 @@ By default thumbnails are rendered per user, and will fall back to the Selenium To always render thumbnails as a fixed user (`admin` in this example), use the following configuration: ```python -from superset.tasks.types import ExecutorType +from superset.tasks.types import FixedExecutor -THUMBNAIL_SELENIUM_USER = "admin" -THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] +THUMBNAIL_EXECUTORS = [FixedExecutor("admin")] ``` @@ -130,8 +129,6 @@ def init_thumbnail_cache(app: Flask) -> S3Cache: THUMBNAIL_CACHE_CONFIG = init_thumbnail_cache -# Async selenium thumbnail task will use the following user -THUMBNAIL_SELENIUM_USER = "Admin" ``` Using the above example cache keys for dashboards will be `superset_thumb__dashboard__{ID}`. You can diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx index 8d897d5bd32c0..cba0a089792eb 100644 --- a/superset-frontend/src/components/ListViewCard/index.tsx +++ b/superset-frontend/src/components/ListViewCard/index.tsx @@ -153,7 +153,7 @@ interface CardProps { subtitle?: ReactNode; url?: string; linkComponent?: ComponentType; - imgURL?: string; + imgURL?: string | null; imgFallbackURL?: string; imgPosition?: BackgroundPosition; description: string; diff --git a/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx b/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx index 0ab150e529a68..42852fef4d900 100644 --- a/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx +++ b/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx @@ -174,7 +174,7 @@ const AddSliceCard: FC<{ lastModified?: string; sliceName: string; style?: CSSProperties; - thumbnailUrl?: string; + thumbnailUrl?: string | null; visType: string; }> = ({ datasourceUrl, diff --git a/superset-frontend/src/types/Dashboard.ts b/superset-frontend/src/types/Dashboard.ts index faecc0bc4acf7..38e5e0fe52f0b 100644 --- a/superset-frontend/src/types/Dashboard.ts +++ b/superset-frontend/src/types/Dashboard.ts @@ -24,7 +24,7 @@ export interface Dashboard { slug?: string | null; url: string; dashboard_title: string; - thumbnail_url: string; + thumbnail_url: string | null; published: boolean; css?: string | null; json_metadata?: string | null; diff --git a/superset/commands/report/alert.py b/superset/commands/report/alert.py index d713c45811021..458f78fd3c667 100644 --- a/superset/commands/report/alert.py +++ b/superset/commands/report/alert.py @@ -170,7 +170,7 @@ def _execute_query(self) -> pd.DataFrame: ) executor, username = get_executor( # pylint: disable=unused-variable - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index 54a2890a96f91..9e69258650dad 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -295,7 +295,7 @@ def _get_screenshots(self) -> list[bytes]: :raises: ReportScheduleScreenshotFailedError """ _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -360,7 +360,7 @@ def _get_pdf(self) -> bytes: def _get_csv_data(self) -> bytes: url = self._get_url(result_format=ChartDataResultFormat.CSV) _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -389,7 +389,7 @@ def _get_embedded_data(self) -> pd.DataFrame: """ url = self._get_url(result_format=ChartDataResultFormat.JSON) _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -859,7 +859,7 @@ def run(self) -> None: if not self._model: raise ReportScheduleExecuteUnexpectedError() _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._model, ) user = security_manager.find_user(username) diff --git a/superset/config.py b/superset/config.py index ae944f3ca11ef..89fbdf35a05a3 100644 --- a/superset/config.py +++ b/superset/config.py @@ -689,6 +689,15 @@ class D3TimeFormat(TypedDict, total=False): # This is merely a default EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = [] +# User used to execute cache warmup tasks +# By default, the cache is warmed up using the primary owner. To fall back to using +# a fixed user (admin in this example), use the following configuration: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")] +CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER] + # --------------------------------------------------- # Thumbnail config (behind feature flag) # --------------------------------------------------- @@ -696,25 +705,30 @@ class D3TimeFormat(TypedDict, total=False): # user for anonymous users. Similar to Alerts & Reports, thumbnails # can be configured to always be rendered as a fixed user. See # `superset.tasks.types.ExecutorType` for a full list of executor options. -# To always use a fixed user account, use the following configuration: -# THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] -THUMBNAIL_SELENIUM_USER: str | None = "admin" -THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER, ExecutorType.SELENIUM] +# To always use a fixed user account (admin in this example, use the following +# configuration: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# THUMBNAIL_EXECUTORS = [FixedExecutor("admin")] +THUMBNAIL_EXECUTORS = [ExecutorType.CURRENT_USER] # By default, thumbnail digests are calculated based on various parameters in the # chart/dashboard metadata, and in the case of user-specific thumbnails, the # username. To specify a custom digest function, use the following config parameters # to define callbacks that receive # 1. the model (dashboard or chart) -# 2. the executor type (e.g. ExecutorType.SELENIUM) +# 2. the executor type (e.g. ExecutorType.FIXED_USER) # 3. the executor's username (note, this is the executor as defined by -# `THUMBNAIL_EXECUTE_AS`; the executor is only equal to the currently logged in +# `THUMBNAIL_EXECUTORS`; the executor is only equal to the currently logged in # user if the executor type is equal to `ExecutorType.CURRENT_USER`) # and return the final digest string: THUMBNAIL_DASHBOARD_DIGEST_FUNC: ( - None | (Callable[[Dashboard, ExecutorType, str], str]) + Callable[[Dashboard, ExecutorType, str], str | None] | None ) = None -THUMBNAIL_CHART_DIGEST_FUNC: Callable[[Slice, ExecutorType, str], str] | None = None +THUMBNAIL_CHART_DIGEST_FUNC: Callable[[Slice, ExecutorType, str], str | None] | None = ( + None +) THUMBNAIL_CACHE_CONFIG: CacheConfig = { "CACHE_TYPE": "NullCache", @@ -1421,16 +1435,19 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # noq # # To first try to execute as the creator in the owners list (if present), then fall # back to the creator, then the last modifier in the owners list (if present), then the -# last modifier, then an owner and finally `THUMBNAIL_SELENIUM_USER`, set as follows: -# ALERT_REPORTS_EXECUTE_AS = [ +# last modifier, then an owner and finally the "admin" user, set as follows: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# ALERT_REPORTS_EXECUTORS = [ # ExecutorType.CREATOR_OWNER, # ExecutorType.CREATOR, # ExecutorType.MODIFIER_OWNER, # ExecutorType.MODIFIER, # ExecutorType.OWNER, -# ExecutorType.SELENIUM, +# FixedExecutor("admin"), # ] -ALERT_REPORTS_EXECUTE_AS: list[ExecutorType] = [ExecutorType.OWNER] +ALERT_REPORTS_EXECUTORS: list[ExecutorType] = [ExecutorType.OWNER] # if ALERT_REPORTS_WORKING_TIME_OUT_KILL is True, set a celery hard timeout # Equal to working timeout + ALERT_REPORTS_WORKING_TIME_OUT_LAG ALERT_REPORTS_WORKING_TIME_OUT_LAG = int(timedelta(seconds=10).total_seconds()) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 1295f0b206538..5b18e856c93b4 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -212,7 +212,7 @@ class DashboardGetResponseSchema(Schema): dashboard_title = fields.String( metadata={"description": dashboard_title_description} ) - thumbnail_url = fields.String() + thumbnail_url = fields.String(allow_none=True) published = fields.Boolean() css = fields.String(metadata={"description": css_description}) json_metadata = fields.String(metadata={"description": json_metadata_description}) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 3af3a63ab7668..5b8675a6a0881 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -225,16 +225,19 @@ def dashboard_link(self) -> Markup: return Markup(f'{title}') @property - def digest(self) -> str: + def digest(self) -> str | None: return get_dashboard_digest(self) @property - def thumbnail_url(self) -> str: + def thumbnail_url(self) -> str | None: """ Returns a thumbnail URL with a HEX digest. We want to avoid browser cache if the dashboard has changed """ - return f"/api/v1/dashboard/{self.id}/thumbnail/{self.digest}/" + if digest := self.digest: + return f"/api/v1/dashboard/{self.id}/thumbnail/{digest}/" + + return None @property def changed_by_name(self) -> str: diff --git a/superset/models/slice.py b/superset/models/slice.py index 1e6daa8321112..8795d186ef45f 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -247,16 +247,19 @@ def data(self) -> dict[str, Any]: } @property - def digest(self) -> str: + def digest(self) -> str | None: return get_chart_digest(self) @property - def thumbnail_url(self) -> str: + def thumbnail_url(self) -> str | None: """ Returns a thumbnail URL with a HEX digest. We want to avoid browser cache if the dashboard has changed """ - return f"/api/v1/chart/{self.id}/thumbnail/{self.digest}/" + if digest := self.digest: + return f"/api/v1/chart/{self.id}/thumbnail/{digest}/" + + return None @property def json_data(self) -> str: diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index b1eaff58bd6c4..1c8012ff839a0 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -14,22 +14,26 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import logging -from typing import Any, Optional, Union +from typing import Any, Optional, TypedDict, Union from urllib import request from urllib.error import URLError from celery.beat import SchedulingError from celery.utils.log import get_task_logger +from flask import current_app from sqlalchemy import and_, func -from superset import app, db, security_manager +from superset import db, security_manager from superset.extensions import celery_app from superset.models.core import Log from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.tags.models import Tag, TaggedObject -from superset.tasks.utils import fetch_csrf_token +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.utils import fetch_csrf_token, get_executor from superset.utils import json from superset.utils.date_parser import parse_human_datetime from superset.utils.machine_auth import MachineAuthProvider @@ -39,19 +43,38 @@ logger.setLevel(logging.INFO) -def get_payload(chart: Slice, dashboard: Optional[Dashboard] = None) -> dict[str, int]: - """Return payload for warming up a given chart/table cache.""" - payload = {"chart_id": chart.id} +class CacheWarmupPayload(TypedDict, total=False): + chart_id: int + dashboard_id: int | None + + +class CacheWarmupTask(TypedDict): + payload: CacheWarmupPayload + username: str | None + + +def get_task(chart: Slice, dashboard: Optional[Dashboard] = None) -> CacheWarmupTask: + """Return task for warming up a given chart/table cache.""" + executors = current_app.config["CACHE_WARMUP_EXECUTORS"] + payload: CacheWarmupPayload = {"chart_id": chart.id} if dashboard: payload["dashboard_id"] = dashboard.id - return payload + + username: str | None + try: + executor = get_executor(executors, chart) + username = executor[1] + except (ExecutorNotFoundError, InvalidExecutorError): + username = None + + return {"payload": payload, "username": username} class Strategy: # pylint: disable=too-few-public-methods """ A cache warm up strategy. - Each strategy defines a `get_payloads` method that returns a list of payloads to + Each strategy defines a `get_tasks` method that returns a list of tasks to send to the `/api/v1/chart/warm_up_cache` endpoint. Strategies can be configured in `superset/config.py`: @@ -73,8 +96,8 @@ class Strategy: # pylint: disable=too-few-public-methods def __init__(self) -> None: pass - def get_payloads(self) -> list[dict[str, int]]: - raise NotImplementedError("Subclasses must implement get_payloads!") + def get_tasks(self) -> list[CacheWarmupTask]: + raise NotImplementedError("Subclasses must implement get_tasks!") class DummyStrategy(Strategy): # pylint: disable=too-few-public-methods @@ -95,8 +118,8 @@ class DummyStrategy(Strategy): # pylint: disable=too-few-public-methods name = "dummy" - def get_payloads(self) -> list[dict[str, int]]: - return [get_payload(chart) for chart in db.session.query(Slice).all()] + def get_tasks(self) -> list[CacheWarmupTask]: + return [get_task(chart) for chart in db.session.query(Slice).all()] class TopNDashboardsStrategy(Strategy): # pylint: disable=too-few-public-methods @@ -124,7 +147,7 @@ def __init__(self, top_n: int = 5, since: str = "7 days ago") -> None: self.top_n = top_n self.since = parse_human_datetime(since) if since else None - def get_payloads(self) -> list[dict[str, int]]: + def get_tasks(self) -> list[CacheWarmupTask]: records = ( db.session.query(Log.dashboard_id, func.count(Log.dashboard_id)) .filter(and_(Log.dashboard_id.isnot(None), Log.dttm >= self.since)) @@ -139,7 +162,7 @@ def get_payloads(self) -> list[dict[str, int]]: ) return [ - get_payload(chart, dashboard) + get_task(chart, dashboard) for dashboard in dashboards for chart in dashboard.slices ] @@ -167,8 +190,8 @@ def __init__(self, tags: Optional[list[str]] = None) -> None: super().__init__() self.tags = tags or [] - def get_payloads(self) -> list[dict[str, int]]: - payloads = [] + def get_tasks(self) -> list[CacheWarmupTask]: + tasks = [] tags = db.session.query(Tag).filter(Tag.name.in_(self.tags)).all() tag_ids = [tag.id for tag in tags] @@ -189,7 +212,7 @@ def get_payloads(self) -> list[dict[str, int]]: ) for dashboard in tagged_dashboards: for chart in dashboard.slices: - payloads.append(get_payload(chart)) + tasks.append(get_task(chart)) # add charts that are tagged tagged_objects = ( @@ -205,9 +228,9 @@ def get_payloads(self) -> list[dict[str, int]]: chart_ids = [tagged_object.object_id for tagged_object in tagged_objects] tagged_charts = db.session.query(Slice).filter(Slice.id.in_(chart_ids)) for chart in tagged_charts: - payloads.append(get_payload(chart)) + tasks.append(get_task(chart)) - return payloads + return tasks strategies = [DummyStrategy, TopNDashboardsStrategy, DashboardTagsStrategy] @@ -284,22 +307,25 @@ def cache_warmup( logger.exception(message) return message - user = security_manager.get_user_by_username(app.config["THUMBNAIL_SELENIUM_USER"]) - cookies = MachineAuthProvider.get_auth_cookies(user) - headers = { - "Cookie": f"session={cookies.get('session', '')}", - "Content-Type": "application/json", - } - results: dict[str, list[str]] = {"scheduled": [], "errors": []} - for payload in strategy.get_payloads(): - try: - payload = json.dumps(payload) - logger.info("Scheduling %s", payload) - fetch_url.delay(payload, headers) - results["scheduled"].append(payload) - except SchedulingError: - logger.exception("Error scheduling fetch_url for payload: %s", payload) - results["errors"].append(payload) + for task in strategy.get_tasks(): + username = task["username"] + payload = json.dumps(task["payload"]) + if username: + try: + user = security_manager.get_user_by_username(username) + cookies = MachineAuthProvider.get_auth_cookies(user) + headers = { + "Cookie": f"session={cookies.get('session', '')}", + "Content-Type": "application/json", + } + logger.info("Scheduling %s", payload) + fetch_url.delay(payload, headers) + results["scheduled"].append(payload) + except SchedulingError: + logger.exception("Error scheduling fetch_url for payload: %s", payload) + results["errors"].append(payload) + else: + logger.warn("Executor not found for %s", payload) return results diff --git a/superset/tasks/exceptions.py b/superset/tasks/exceptions.py index 6698661754e5e..19a97ea658b81 100644 --- a/superset/tasks/exceptions.py +++ b/superset/tasks/exceptions.py @@ -22,3 +22,7 @@ class ExecutorNotFoundError(SupersetException): message = _("Scheduled task executor not found") + + +class InvalidExecutorError(SupersetException): + message = _("Invalid executor type") diff --git a/superset/tasks/thumbnails.py b/superset/tasks/thumbnails.py index dd9b5065dce34..3b0b47dbb368f 100644 --- a/superset/tasks/thumbnails.py +++ b/superset/tasks/thumbnails.py @@ -55,7 +55,7 @@ def cache_chart_thumbnail( url = get_url_path("Superset.slice", slice_id=chart.id) logger.info("Caching chart: %s", url) _, username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=chart, current_user=current_user, ) @@ -92,7 +92,7 @@ def cache_dashboard_thumbnail( logger.info("Caching dashboard: %s", url) _, username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=dashboard, current_user=current_user, ) @@ -135,7 +135,7 @@ def cache_dashboard_screenshot( # pylint: disable=too-many-arguments current_user = security_manager.get_guest_user_from_token(guest_token) else: _, exec_username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=dashboard, current_user=username, ) diff --git a/superset/tasks/types.py b/superset/tasks/types.py index 84a3e7b01f26c..8f2f76b40528f 100644 --- a/superset/tasks/types.py +++ b/superset/tasks/types.py @@ -14,18 +14,25 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import NamedTuple + from superset.utils.backports import StrEnum +class FixedExecutor(NamedTuple): + username: str + + class ExecutorType(StrEnum): """ - Which user should scheduled tasks be executed as. Used as follows: + Which user should async tasks be executed as. Used as follows: For Alerts & Reports: the "model" refers to the AlertSchedule object For Thumbnails: The "model" refers to the Slice or Dashboard object """ - # See the THUMBNAIL_SELENIUM_USER config parameter - SELENIUM = "selenium" + # A fixed user account. Note that for assigning a fixed user you should use the + # FixedExecutor class. + FIXED_USER = "fixed_user" # The creator of the model CREATOR = "creator" # The creator of the model, if found in the owners list @@ -41,3 +48,10 @@ class ExecutorType(StrEnum): # user. If the modifier is not found, returns the creator if found in the owners # list. Finally, if neither are present, returns the first user in the owners list. OWNER = "owner" + + +Executor = FixedExecutor | ExecutorType + + +# Alias type to represent the executor that was chosen from a list of Executors +ChosenExecutor = tuple[ExecutorType, str] diff --git a/superset/tasks/utils.py b/superset/tasks/utils.py index 4815b70343f57..845ea2b5fc906 100644 --- a/superset/tasks/utils.py +++ b/superset/tasks/utils.py @@ -23,10 +23,10 @@ from urllib import request from celery.utils.log import get_task_logger -from flask import current_app, g +from flask import g -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.types import ChosenExecutor, Executor, ExecutorType, FixedExecutor from superset.utils import json from superset.utils.urls import get_url_path @@ -42,56 +42,60 @@ # pylint: disable=too-many-branches def get_executor( # noqa: C901 - executor_types: list[ExecutorType], + executors: list[Executor], model: Dashboard | ReportSchedule | Slice, current_user: str | None = None, -) -> tuple[ExecutorType, str]: +) -> ChosenExecutor: """ Extract the user that should be used to execute a scheduled task. Certain executor types extract the user from the underlying object (e.g. CREATOR), the constant Selenium user (SELENIUM), or the user that initiated the request. - :param executor_types: The requested executor type in descending order. When the + :param executors: The requested executor in descending order. When the first user is found it is returned. :param model: The underlying object :param current_user: The username of the user that initiated the task. For thumbnails this is the user that requested the thumbnail, while for alerts and reports this is None (=initiated by Celery). - :return: User to execute the report as - :raises ScheduledTaskExecutorNotFoundError: If no users were found in after - iterating through all entries in `executor_types` + :return: User to execute the execute the async task as. The first element of the + tuple represents the type of the executor, and the second represents the + username of the executor. + :raises ExecutorNotFoundError: If no users were found in after + iterating through all entries in `executors` """ owners = model.owners owner_dict = {owner.id: owner for owner in owners} - for executor_type in executor_types: - if executor_type == ExecutorType.SELENIUM: - return executor_type, current_app.config["THUMBNAIL_SELENIUM_USER"] - if executor_type == ExecutorType.CURRENT_USER and current_user: - return executor_type, current_user - if executor_type == ExecutorType.CREATOR_OWNER: + for executor in executors: + if isinstance(executor, FixedExecutor): + return ExecutorType.FIXED_USER, executor.username + if executor == ExecutorType.FIXED_USER: + raise InvalidExecutorError() + if executor == ExecutorType.CURRENT_USER and current_user: + return executor, current_user + if executor == ExecutorType.CREATOR_OWNER: if (user := model.created_by) and (owner := owner_dict.get(user.id)): - return executor_type, owner.username - if executor_type == ExecutorType.CREATOR: + return executor, owner.username + if executor == ExecutorType.CREATOR: if user := model.created_by: - return executor_type, user.username - if executor_type == ExecutorType.MODIFIER_OWNER: + return executor, user.username + if executor == ExecutorType.MODIFIER_OWNER: if (user := model.changed_by) and (owner := owner_dict.get(user.id)): - return executor_type, owner.username - if executor_type == ExecutorType.MODIFIER: + return executor, owner.username + if executor == ExecutorType.MODIFIER: if user := model.changed_by: - return executor_type, user.username - if executor_type == ExecutorType.OWNER: + return executor, user.username + if executor == ExecutorType.OWNER: owners = model.owners if len(owners) == 1: - return executor_type, owners[0].username + return executor, owners[0].username if len(owners) > 1: if modifier := model.changed_by: if modifier and (user := owner_dict.get(modifier.id)): - return executor_type, user.username + return executor, user.username if creator := model.created_by: if creator and (user := owner_dict.get(creator.id)): - return executor_type, user.username - return executor_type, owners[0].username + return executor, user.username + return executor, owners[0].username raise ExecutorNotFoundError() diff --git a/superset/thumbnails/digest.py b/superset/thumbnails/digest.py index c10e4330cb458..446d06b20d902 100644 --- a/superset/thumbnails/digest.py +++ b/superset/thumbnails/digest.py @@ -23,6 +23,7 @@ from flask import current_app from superset import security_manager +from superset.tasks.exceptions import ExecutorNotFoundError from superset.tasks.types import ExecutorType from superset.tasks.utils import get_current_user, get_executor from superset.utils.core import override_user @@ -89,14 +90,17 @@ def _adjust_string_with_rls( return unique_string -def get_dashboard_digest(dashboard: Dashboard) -> str: +def get_dashboard_digest(dashboard: Dashboard) -> str | None: config = current_app.config - datasources = dashboard.datasources - executor_type, executor = get_executor( - executor_types=config["THUMBNAIL_EXECUTE_AS"], - model=dashboard, - current_user=get_current_user(), - ) + try: + executor_type, executor = get_executor( + executors=config["THUMBNAIL_EXECUTORS"], + model=dashboard, + current_user=get_current_user(), + ) + except ExecutorNotFoundError: + return None + if func := config["THUMBNAIL_DASHBOARD_DIGEST_FUNC"]: return func(dashboard, executor_type, executor) @@ -106,25 +110,29 @@ def get_dashboard_digest(dashboard: Dashboard) -> str: ) unique_string = _adjust_string_for_executor(unique_string, executor_type, executor) - unique_string = _adjust_string_with_rls(unique_string, datasources, executor) + unique_string = _adjust_string_with_rls( + unique_string, dashboard.datasources, executor + ) return md5_sha_from_str(unique_string) -def get_chart_digest(chart: Slice) -> str: +def get_chart_digest(chart: Slice) -> str | None: config = current_app.config - datasource = chart.datasource - executor_type, executor = get_executor( - executor_types=config["THUMBNAIL_EXECUTE_AS"], - model=chart, - current_user=get_current_user(), - ) + try: + executor_type, executor = get_executor( + executors=config["THUMBNAIL_EXECUTORS"], + model=chart, + current_user=get_current_user(), + ) + except ExecutorNotFoundError: + return None if func := config["THUMBNAIL_CHART_DIGEST_FUNC"]: return func(chart, executor_type, executor) unique_string = f"{chart.params or ''}.{executor}" unique_string = _adjust_string_for_executor(unique_string, executor_type, executor) - unique_string = _adjust_string_with_rls(unique_string, [datasource], executor) + unique_string = _adjust_string_with_rls(unique_string, [chart.datasource], executor) return md5_sha_from_str(unique_string) diff --git a/superset/utils/screenshots.py b/superset/utils/screenshots.py index 96c0f40d6da51..1557bc283379b 100644 --- a/superset/utils/screenshots.py +++ b/superset/utils/screenshots.py @@ -56,15 +56,18 @@ class BaseScreenshot: driver_type = current_app.config["WEBDRIVER_TYPE"] + url: str + digest: str | None + screenshot: bytes | None thumbnail_type: str = "" element: str = "" window_size: WindowSize = DEFAULT_SCREENSHOT_WINDOW_SIZE thumb_size: WindowSize = DEFAULT_SCREENSHOT_THUMBNAIL_SIZE - def __init__(self, url: str, digest: str): - self.digest: str = digest + def __init__(self, url: str, digest: str | None): + self.digest = digest self.url = url - self.screenshot: bytes | None = None + self.screenshot = None def driver(self, window_size: WindowSize | None = None) -> WebDriver: window_size = window_size or self.window_size @@ -227,7 +230,7 @@ class ChartScreenshot(BaseScreenshot): def __init__( self, url: str, - digest: str, + digest: str | None, window_size: WindowSize | None = None, thumb_size: WindowSize | None = None, ): @@ -248,7 +251,7 @@ class DashboardScreenshot(BaseScreenshot): def __init__( self, url: str, - digest: str, + digest: str | None, window_size: WindowSize | None = None, thumb_size: WindowSize | None = None, ): diff --git a/tests/integration_tests/reports/alert_tests.py b/tests/integration_tests/reports/alert_tests.py index 16ce8f3fed34b..0e1b21569362b 100644 --- a/tests/integration_tests/reports/alert_tests.py +++ b/tests/integration_tests/reports/alert_tests.py @@ -26,7 +26,7 @@ from superset.commands.report.exceptions import AlertQueryError from superset.reports.models import ReportCreationMethod, ReportScheduleType -from superset.tasks.types import ExecutorType +from superset.tasks.types import ExecutorType, FixedExecutor from superset.utils.database import get_example_database from tests.integration_tests.test_app import app @@ -34,7 +34,7 @@ @pytest.mark.parametrize( "owner_names,creator_name,config,expected_result", [ - (["gamma"], None, [ExecutorType.SELENIUM], "admin"), + (["gamma"], None, [FixedExecutor("admin")], "admin"), (["gamma"], None, [ExecutorType.OWNER], "gamma"), ( ["alpha", "gamma"], @@ -69,8 +69,8 @@ def test_execute_query_as_report_executor( from superset.commands.report.alert import AlertCommand from superset.reports.models import ReportSchedule - original_config = app.config["ALERT_REPORTS_EXECUTE_AS"] - app.config["ALERT_REPORTS_EXECUTE_AS"] = config + original_config = app.config["ALERT_REPORTS_EXECUTORS"] + app.config["ALERT_REPORTS_EXECUTORS"] = config owners = [get_user(owner_name) for owner_name in owner_names] report_schedule = ReportSchedule( created_by=get_user(creator_name) if creator_name else None, @@ -96,7 +96,7 @@ def test_execute_query_as_report_executor( command.run() assert override_user_mock.call_args[0][0].username == expected_result - app.config["ALERT_REPORTS_EXECUTE_AS"] = original_config + app.config["ALERT_REPORTS_EXECUTORS"] = original_config def test_execute_query_mutate_query_enabled( @@ -278,7 +278,7 @@ def test_get_alert_metadata_from_object( from superset.commands.report.alert import AlertCommand from superset.reports.models import ReportSchedule - app.config["ALERT_REPORTS_EXECUTE_AS"] = [ExecutorType.OWNER] + app.config["ALERT_REPORTS_EXECUTORS"] = [ExecutorType.OWNER] mock_database = mocker.MagicMock() mock_exec_id = uuid.uuid4() diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index a3f105a419536..7b22b38fc0cfc 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -773,7 +773,7 @@ def test_email_chart_report_schedule_alpha_owner( ExecuteReport Command: Test chart email report schedule with screenshot executed as the chart owner """ - config_key = "ALERT_REPORTS_EXECUTE_AS" + config_key = "ALERT_REPORTS_EXECUTORS" original_config_value = app.config[config_key] app.config[config_key] = [ExecutorType.OWNER] diff --git a/tests/integration_tests/strategy_tests.py b/tests/integration_tests/strategy_tests.py index e5901b5b82cb7..6dc99f501fee6 100644 --- a/tests/integration_tests/strategy_tests.py +++ b/tests/integration_tests/strategy_tests.py @@ -82,11 +82,15 @@ def test_top_n_dashboards_strategy(self): self.client.get(f"/superset/dashboard/{dash.id}/") strategy = TopNDashboardsStrategy(1) - result = strategy.get_payloads() + result = strategy.get_tasks() expected = [ - {"chart_id": chart.id, "dashboard_id": dash.id} for chart in dash.slices + { + "payload": {"chart_id": chart.id, "dashboard_id": dash.id}, + "username": "admin", + } + for chart in dash.slices ] - self.assertCountEqual(result, expected) # noqa: PT009 + assert len(result) == len(expected) def reset_tag(self, tag): """Remove associated object from tag, used to reset tests""" @@ -104,34 +108,30 @@ def test_dashboard_tags_strategy(self): self.reset_tag(tag1) strategy = DashboardTagsStrategy(["tag1"]) - result = strategy.get_payloads() - expected = [] - assert result == expected + assert strategy.get_tasks() == [] # tag dashboard 'births' with `tag1` tag1 = get_tag("tag1", db.session, TagType.custom) dash = self.get_dash_by_slug("births") - tag1_urls = [{"chart_id": chart.id} for chart in dash.slices] + tag1_payloads = [{"chart_id": chart.id} for chart in dash.slices] tagged_object = TaggedObject( tag_id=tag1.id, object_id=dash.id, object_type=ObjectType.dashboard ) db.session.add(tagged_object) db.session.commit() - self.assertCountEqual(strategy.get_payloads(), tag1_urls) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag1_payloads) strategy = DashboardTagsStrategy(["tag2"]) tag2 = get_tag("tag2", db.session, TagType.custom) self.reset_tag(tag2) - result = strategy.get_payloads() - expected = [] - assert result == expected + assert strategy.get_tasks() == [] # tag first slice dash = self.get_dash_by_slug("unicode-test") chart = dash.slices[0] - tag2_urls = [{"chart_id": chart.id}] + tag2_payloads = [{"chart_id": chart.id}] object_id = chart.id tagged_object = TaggedObject( tag_id=tag2.id, object_id=object_id, object_type=ObjectType.chart @@ -139,11 +139,8 @@ def test_dashboard_tags_strategy(self): db.session.add(tagged_object) db.session.commit() - result = strategy.get_payloads() - self.assertCountEqual(result, tag2_urls) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag2_payloads) strategy = DashboardTagsStrategy(["tag1", "tag2"]) - result = strategy.get_payloads() - expected = tag1_urls + tag2_urls - self.assertCountEqual(result, expected) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag1_payloads + tag2_payloads) diff --git a/tests/integration_tests/thumbnails_tests.py b/tests/integration_tests/thumbnails_tests.py index ccb9a9c734c13..e808858fb306a 100644 --- a/tests/integration_tests/thumbnails_tests.py +++ b/tests/integration_tests/thumbnails_tests.py @@ -30,7 +30,7 @@ from superset.extensions import machine_auth_provider_factory from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.tasks.types import ExecutorType +from superset.tasks.types import ExecutorType, FixedExecutor from superset.utils import json from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot from superset.utils.urls import get_url_path @@ -53,8 +53,8 @@ def create_app(self): return app def url_open_auth(self, username: str, url: str): - admin_user = security_manager.find_user(username=username) - cookies = machine_auth_provider_factory.instance.get_auth_cookies(admin_user) + user = security_manager.find_user(username=username) + cookies = machine_auth_provider_factory.instance.get_auth_cookies(user) opener = urllib.request.build_opener() opener.addheaders.append(("Cookie", f"session={cookies['session']}")) return opener.open(f"{self.get_server_url()}/{url}") @@ -70,7 +70,7 @@ def test_get_async_dashboard_screenshot(self): thumbnail_url = resp["result"][0]["thumbnail_url"] response = self.url_open_auth( - "admin", + ADMIN_USERNAME, thumbnail_url, ) assert response.getcode() == 202 @@ -84,9 +84,7 @@ def test_not_call_find_unexpected_errors_if_feature_disabled( self, mock_find_unexpected_errors, mock_firefox, mock_webdriver_wait ): webdriver_proxy = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1) webdriver_proxy.get_screenshot(url, "grid-container", user=user) @@ -100,9 +98,7 @@ def test_call_find_unexpected_errors_if_feature_enabled( ): app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"] = True webdriver_proxy = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1) webdriver_proxy.get_screenshot(url, "grid-container", user=user) @@ -149,9 +145,7 @@ def test_screenshot_selenium_headstart( self, mock_sleep, mock_webdriver, mock_webdriver_wait ): webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") app.config["SCREENSHOT_SELENIUM_HEADSTART"] = 5 webdriver.get_screenshot(url, "chart-container", user=user) @@ -162,9 +156,7 @@ def test_screenshot_selenium_headstart( def test_screenshot_selenium_locate_wait(self, mock_webdriver, mock_webdriver_wait): app.config["SCREENSHOT_LOCATE_WAIT"] = 15 webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") webdriver.get_screenshot(url, "chart-container", user=user) assert mock_webdriver_wait.call_args_list[0] == call(ANY, 15) @@ -174,9 +166,7 @@ def test_screenshot_selenium_locate_wait(self, mock_webdriver, mock_webdriver_wa def test_screenshot_selenium_load_wait(self, mock_webdriver, mock_webdriver_wait): app.config["SCREENSHOT_LOAD_WAIT"] = 15 webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") webdriver.get_screenshot(url, "chart-container", user=user) assert mock_webdriver_wait.call_args_list[2] == call(ANY, 15) @@ -188,9 +178,7 @@ def test_screenshot_selenium_animation_wait( self, mock_sleep, mock_webdriver, mock_webdriver_wait ): webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") app.config["SCREENSHOT_SELENIUM_ANIMATION_WAIT"] = 4 webdriver.get_screenshot(url, "chart-container", user=user) @@ -232,7 +220,7 @@ def test_chart_thumbnail_disabled(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(THUMBNAILS=True) - def test_get_async_dashboard_screenshot_as_selenium(self): + def test_get_async_dashboard_screenshot_as_fixed_user(self): """ Thumbnails: Simple get async dashboard screenshot as selenium user """ @@ -241,7 +229,7 @@ def test_get_async_dashboard_screenshot_as_selenium(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -251,8 +239,8 @@ def test_get_async_dashboard_screenshot_as_selenium(self): mock_adjust_string.return_value = self.digest_return_value _, thumbnail_url = self._get_id_and_thumbnail_url(DASHBOARD_URL) assert self.digest_hash in thumbnail_url - assert mock_adjust_string.call_args[0][1] == ExecutorType.SELENIUM - assert mock_adjust_string.call_args[0][2] == "admin" + assert mock_adjust_string.call_args[0][1] == ExecutorType.FIXED_USER + assert mock_adjust_string.call_args[0][2] == ADMIN_USERNAME rv = self.client.get(thumbnail_url) assert rv.status_code == 202 @@ -269,7 +257,7 @@ def test_get_async_dashboard_screenshot_as_current_user(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.CURRENT_USER], + "THUMBNAIL_EXECUTORS": [ExecutorType.CURRENT_USER], }, ), patch( @@ -310,7 +298,7 @@ def test_get_async_dashboard_not_allowed(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(THUMBNAILS=True) - def test_get_async_chart_screenshot_as_selenium(self): + def test_get_async_chart_screenshot_as_fixed_user(self): """ Thumbnails: Simple get async chart screenshot as selenium user """ @@ -319,7 +307,7 @@ def test_get_async_chart_screenshot_as_selenium(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -329,8 +317,8 @@ def test_get_async_chart_screenshot_as_selenium(self): mock_adjust_string.return_value = self.digest_return_value _, thumbnail_url = self._get_id_and_thumbnail_url(CHART_URL) assert self.digest_hash in thumbnail_url - assert mock_adjust_string.call_args[0][1] == ExecutorType.SELENIUM - assert mock_adjust_string.call_args[0][2] == "admin" + assert mock_adjust_string.call_args[0][1] == ExecutorType.FIXED_USER + assert mock_adjust_string.call_args[0][2] == ADMIN_USERNAME rv = self.client.get(thumbnail_url) assert rv.status_code == 202 @@ -347,7 +335,7 @@ def test_get_async_chart_screenshot_as_current_user(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.CURRENT_USER], + "THUMBNAIL_EXECUTORS": [ExecutorType.CURRENT_USER], }, ), patch( diff --git a/tests/unit_tests/tasks/test_utils.py b/tests/unit_tests/tasks/test_utils.py index 2d85a1c05678c..d4b24c66661b6 100644 --- a/tests/unit_tests/tasks/test_utils.py +++ b/tests/unit_tests/tasks/test_utils.py @@ -23,11 +23,11 @@ import pytest from flask_appbuilder.security.sqla.models import User -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.types import Executor, ExecutorType, FixedExecutor -SELENIUM_USER_ID = 1234 -SELENIUM_USERNAME = "admin" +FIXED_USER_ID = 1234 +FIXED_USERNAME = "admin" def _get_users( @@ -54,18 +54,18 @@ class ModelType(int, Enum): @pytest.mark.parametrize( - "model_type,executor_types,model_config,current_user,expected_result", + "model_type,executors,model_config,current_user,expected_result", [ ( ModelType.REPORT_SCHEDULE, - [ExecutorType.SELENIUM], + [FixedExecutor(FIXED_USERNAME)], ModelConfig( owners=[1, 2], creator=3, modifier=4, ), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.REPORT_SCHEDULE, @@ -75,11 +75,11 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[]), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.REPORT_SCHEDULE, @@ -89,7 +89,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[], modifier=1), None, @@ -103,7 +103,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[2], modifier=1), None, @@ -117,7 +117,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[2], creator=3, modifier=1), None, @@ -198,11 +198,11 @@ class ModelType(int, Enum): ( ModelType.DASHBOARD, [ - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), 4, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.DASHBOARD, @@ -219,11 +219,11 @@ class ModelType(int, Enum): ExecutorType.CREATOR_OWNER, ExecutorType.MODIFIER_OWNER, ExecutorType.CURRENT_USER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.CHART, @@ -237,11 +237,11 @@ class ModelType(int, Enum): ( ModelType.CHART, [ - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), 4, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.CHART, @@ -252,26 +252,35 @@ class ModelType(int, Enum): None, ExecutorNotFoundError(), ), + ( + ModelType.CHART, + [ + ExecutorType.FIXED_USER, + ], + ModelConfig(owners=[]), + None, + InvalidExecutorError(), + ), ( ModelType.CHART, [ ExecutorType.CREATOR_OWNER, ExecutorType.MODIFIER_OWNER, ExecutorType.CURRENT_USER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ], ) def test_get_executor( model_type: ModelType, - executor_types: list[ExecutorType], + executors: list[Executor], model_config: ModelConfig, current_user: Optional[int], - expected_result: tuple[int, ExecutorNotFoundError], + expected_result: tuple[ExecutorType, int] | Exception, ) -> None: from superset.models.dashboard import Dashboard from superset.models.slice import Slice @@ -308,14 +317,14 @@ def test_get_executor( cm = nullcontext() expected_executor_type = expected_result[0] expected_executor = ( - SELENIUM_USERNAME - if expected_executor_type == ExecutorType.SELENIUM + FIXED_USERNAME + if expected_executor_type == ExecutorType.FIXED_USER else str(expected_result[1]) ) with cm: executor_type, executor = get_executor( - executor_types=executor_types, + executors=executors, model=obj, current_user=str(current_user) if current_user else None, ) diff --git a/tests/unit_tests/thumbnails/test_digest.py b/tests/unit_tests/thumbnails/test_digest.py index b08b896918fbc..aa5d8d08aea12 100644 --- a/tests/unit_tests/thumbnails/test_digest.py +++ b/tests/unit_tests/thumbnails/test_digest.py @@ -24,8 +24,8 @@ from flask_appbuilder.security.sqla.models import User from superset.connectors.sqla.models import BaseDatasource, SqlaTable -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import InvalidExecutorError +from superset.tasks.types import Executor, ExecutorType, FixedExecutor from superset.utils.core import DatasourceType, override_user if TYPE_CHECKING: @@ -81,7 +81,7 @@ def prepare_datasource_mock( [ ( None, - [ExecutorType.SELENIUM], + [FixedExecutor("admin")], False, False, [], @@ -214,13 +214,21 @@ def prepare_datasource_mock( False, False, [], - ExecutorNotFoundError(), + None, + ), + ( + None, + [ExecutorType.FIXED_USER], + False, + False, + [], + InvalidExecutorError(), ), ], ) def test_dashboard_digest( dashboard_overrides: dict[str, Any] | None, - execute_as: list[ExecutorType], + execute_as: list[Executor], has_current_user: bool, use_custom_digest: bool, rls_datasources: list[dict[str, Any]], @@ -255,7 +263,7 @@ def test_dashboard_digest( patch.dict( app.config, { - "THUMBNAIL_EXECUTE_AS": execute_as, + "THUMBNAIL_EXECUTORS": execute_as, "THUMBNAIL_DASHBOARD_DIGEST_FUNC": func, }, ), @@ -282,7 +290,7 @@ def test_dashboard_digest( [ ( None, - [ExecutorType.SELENIUM], + [FixedExecutor("admin")], False, False, None, @@ -345,13 +353,21 @@ def test_dashboard_digest( False, False, None, - ExecutorNotFoundError(), + None, + ), + ( + None, + [ExecutorType.FIXED_USER], + False, + False, + None, + InvalidExecutorError(), ), ], ) def test_chart_digest( chart_overrides: dict[str, Any] | None, - execute_as: list[ExecutorType], + execute_as: list[Executor], has_current_user: bool, use_custom_digest: bool, rls_datasource: dict[str, Any] | None, @@ -383,7 +399,7 @@ def test_chart_digest( patch.dict( app.config, { - "THUMBNAIL_EXECUTE_AS": execute_as, + "THUMBNAIL_EXECUTORS": execute_as, "THUMBNAIL_CHART_DIGEST_FUNC": func, }, ), From fcd166149cfb896a2a389a5c9075b24eb2cf1ade Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Wed, 22 Jan 2025 13:20:38 -0800 Subject: [PATCH 08/18] chore: Empty state refactor (#31860) --- superset-frontend/package-lock.json | 118 ++------ .../SqlLab/components/QueryHistory/index.tsx | 5 +- .../SqlLab/components/SouthPane/Results.tsx | 4 +- .../src/SqlLab/components/SqlEditor/index.tsx | 5 +- .../components/SqlEditorLeftBar/index.tsx | 6 +- .../components/TabbedSqlEditors/index.tsx | 5 +- superset-frontend/src/assets/images/chart.svg | 6 +- .../src/assets/images/document.svg | 4 +- .../src/assets/images/empty-charts.svg | 22 +- .../src/assets/images/empty-dashboard.svg | 14 +- .../src/assets/images/empty-dataset.svg | 39 +-- .../src/assets/images/empty-queries.svg | 16 +- .../src/assets/images/empty-query.svg | 12 +- .../src/assets/images/empty-table.svg | 6 +- superset-frontend/src/assets/images/empty.svg | 6 +- .../src/assets/images/empty_sql_chart.svg | 6 +- .../src/assets/images/filter-results.svg | 67 +++-- .../src/assets/images/filter.svg | 52 +++- .../src/assets/images/star-circle.svg | 6 +- superset-frontend/src/assets/images/union.svg | 6 +- .../src/assets/images/vector.svg | 4 +- .../src/components/Chart/Chart.tsx | 8 +- .../src/components/Chart/ChartRenderer.jsx | 7 +- .../Chart/DrillDetail/DrillDetailPane.tsx | 4 +- .../DatabaseSelector.test.tsx | 4 +- .../EmptyState/EmptyState.stories.tsx | 99 ++++--- .../src/components/EmptyState/index.tsx | 251 +++++++----------- .../src/components/ListView/ListView.tsx | 8 +- .../src/components/Table/index.tsx | 59 ++-- .../DashboardBuilder/DashboardBuilder.tsx | 5 +- .../dashboard/components/DashboardGrid.jsx | 11 +- .../gridComponents/ChartHolder.test.tsx | 2 +- .../components/gridComponents/Tab.jsx | 4 +- .../components/gridComponents/Tab.test.tsx | 4 +- .../nativeFilters/FilterBar/Vertical.tsx | 5 +- .../DataTablesPane/components/SamplesPane.tsx | 4 +- .../components/useResultsPane.tsx | 6 +- .../AnnotationLayer.jsx | 5 +- .../ColumnSelectPopover.tsx | 11 +- .../AdhocMetricEditPopover/index.jsx | 8 +- .../features/allEntities/AllEntitiesTable.tsx | 5 +- .../DatasetPanel/MessageContent.tsx | 7 +- .../AddDataset/EditDataset/UsageTab/index.tsx | 7 +- .../datasets/AddDataset/LeftPanel/index.tsx | 21 +- .../src/features/home/ActivityTable.test.tsx | 6 +- .../src/features/home/ChartTable.test.tsx | 4 +- .../src/features/home/EmptyState.test.tsx | 14 +- .../src/features/home/EmptyState.tsx | 206 +++++--------- 48 files changed, 528 insertions(+), 656 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index a7ac01fa4143b..1b06e003e47f0 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -1714,15 +1714,18 @@ } }, "node_modules/@babel/helper-string-parser": { - "version": "7.24.8", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.24.8.tgz", - "integrity": "sha512-pO9KhhRcuUyGnJWwyEgnRJTSIZHiT+vMD0kPeD+so0l7mxkMT19g3pjY9GTnHySck/hDzq+dtW/4VgnMkippsQ==", + "version": "7.25.9", + "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", + "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==", + "license": "MIT", "engines": { "node": ">=6.9.0" } }, "node_modules/@babel/helper-validator-identifier": { - "version": "7.24.7", + "version": "7.25.9", + "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", + "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==", "license": "MIT", "engines": { "node": ">=6.9.0" @@ -3496,13 +3499,13 @@ "license": "MIT" }, "node_modules/@babel/types": { - "version": "7.25.2", - "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.25.2.tgz", - "integrity": "sha512-YTnYtra7W9e6/oAZEHj0bJehPRUlLH9/fbpT5LfB0NhQXyALCRkRs3zH9v07IYhkgpqX6Z78FnuccZr/l4Fs4Q==", + "version": "7.26.5", + "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.26.5.tgz", + "integrity": "sha512-L6mZmwFDK6Cjh1nRCLXpa6no13ZIioJDz7mdkzHv399pThrTa/k0nUlNaenOeh2kWu/iaOQYElEpKPUswUa9Vg==", + "license": "MIT", "dependencies": { - "@babel/helper-string-parser": "^7.24.8", - "@babel/helper-validator-identifier": "^7.24.7", - "to-fast-properties": "^2.0.0" + "@babel/helper-string-parser": "^7.25.9", + "@babel/helper-validator-identifier": "^7.25.9" }, "engines": { "node": ">=6.9.0" @@ -50919,13 +50922,6 @@ "dev": true, "license": "BSD-3-Clause" }, - "node_modules/to-fast-properties": { - "version": "2.0.0", - "license": "MIT", - "engines": { - "node": ">=4" - } - }, "node_modules/to-object-path": { "version": "0.3.0", "dev": true, @@ -56297,26 +56293,6 @@ "node": ">=6.9.0" } }, - "packages/superset-ui-demo/node_modules/@babel/helper-string-parser": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", - "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6.9.0" - } - }, - "packages/superset-ui-demo/node_modules/@babel/helper-validator-identifier": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", - "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6.9.0" - } - }, "packages/superset-ui-demo/node_modules/@babel/helper-validator-option": { "version": "7.25.9", "resolved": "https://registry.npmjs.org/@babel/helper-validator-option/-/helper-validator-option-7.25.9.tgz", @@ -58309,26 +58285,6 @@ "react-dom": "^16.13.1" } }, - "plugins/plugin-chart-pivot-table/node_modules/@babel/helper-string-parser": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", - "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6.9.0" - } - }, - "plugins/plugin-chart-pivot-table/node_modules/@babel/helper-validator-identifier": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", - "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6.9.0" - } - }, "plugins/plugin-chart-pivot-table/node_modules/@babel/types": { "version": "7.26.3", "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.26.3.tgz", @@ -59449,12 +59405,14 @@ } }, "@babel/helper-string-parser": { - "version": "7.24.8", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.24.8.tgz", - "integrity": "sha512-pO9KhhRcuUyGnJWwyEgnRJTSIZHiT+vMD0kPeD+so0l7mxkMT19g3pjY9GTnHySck/hDzq+dtW/4VgnMkippsQ==" + "version": "7.25.9", + "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", + "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==" }, "@babel/helper-validator-identifier": { - "version": "7.24.7" + "version": "7.25.9", + "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", + "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==" }, "@babel/helper-validator-option": { "version": "7.24.7", @@ -60507,13 +60465,12 @@ } }, "@babel/types": { - "version": "7.25.2", - "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.25.2.tgz", - "integrity": "sha512-YTnYtra7W9e6/oAZEHj0bJehPRUlLH9/fbpT5LfB0NhQXyALCRkRs3zH9v07IYhkgpqX6Z78FnuccZr/l4Fs4Q==", + "version": "7.26.5", + "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.26.5.tgz", + "integrity": "sha512-L6mZmwFDK6Cjh1nRCLXpa6no13ZIioJDz7mdkzHv399pThrTa/k0nUlNaenOeh2kWu/iaOQYElEpKPUswUa9Vg==", "requires": { - "@babel/helper-string-parser": "^7.24.8", - "@babel/helper-validator-identifier": "^7.24.7", - "to-fast-properties": "^2.0.0" + "@babel/helper-string-parser": "^7.25.9", + "@babel/helper-validator-identifier": "^7.25.9" } }, "@base2/pretty-print-object": { @@ -66764,18 +66721,6 @@ "integrity": "sha512-kSMlyUVdWe25rEsRGviIgOWnoT/nfABVWlqt9N19/dIPWViAOW2s9wznP5tURbs/IDuNk4gPy3YdYRgH3uxhBw==", "dev": true }, - "@babel/helper-string-parser": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", - "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==", - "dev": true - }, - "@babel/helper-validator-identifier": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", - "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==", - "dev": true - }, "@babel/helper-validator-option": { "version": "7.25.9", "resolved": "https://registry.npmjs.org/@babel/helper-validator-option/-/helper-validator-option-7.25.9.tgz", @@ -68904,18 +68849,6 @@ "jest": "^29.7.0" }, "dependencies": { - "@babel/helper-string-parser": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.25.9.tgz", - "integrity": "sha512-4A/SCr/2KLd5jrtOMFzaKjVtAei3+2r/NChoBNoZ3EyP/+GlhoaEGoWOZUmFmoITP7zOJyHIMm+DYRd8o3PvHA==", - "dev": true - }, - "@babel/helper-validator-identifier": { - "version": "7.25.9", - "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz", - "integrity": "sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ==", - "dev": true - }, "@babel/types": { "version": "7.26.3", "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.26.3.tgz", @@ -93957,9 +93890,6 @@ "version": "1.0.5", "dev": true }, - "to-fast-properties": { - "version": "2.0.0" - }, "to-object-path": { "version": "0.3.0", "dev": true, diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index 30103dafa566c..6916e1e19b1a5 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -20,7 +20,7 @@ import { useEffect, useMemo, useState } from 'react'; import { shallowEqual, useSelector } from 'react-redux'; import { useInView } from 'react-intersection-observer'; import { omit } from 'lodash'; -import { EmptyStateMedium } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { t, styled, @@ -143,8 +143,9 @@ const QueryHistory = ({ ) : ( - diff --git a/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx b/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx index 3dfec40bdc037..6a2a60831bf74 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx @@ -19,7 +19,7 @@ import { FC } from 'react'; import { shallowEqual, useSelector } from 'react-redux'; import Alert from 'src/components/Alert'; -import { EmptyStateMedium } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { FeatureFlag, styled, t, isFeatureEnabled } from '@superset-ui/core'; import { SqlLabRootState } from 'src/SqlLab/types'; @@ -67,7 +67,7 @@ const Results: FC = ({ ) { return ( - diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index 4d1aded5be29f..09a2fa94e409f 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -100,7 +100,7 @@ import { LocalStorageKeys, setItem, } from 'src/utils/localStorageHelpers'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import Alert from 'src/components/Alert'; import getBootstrapData from 'src/utils/getBootstrapData'; import useLogAction from 'src/logger/useLogAction'; @@ -968,8 +968,9 @@ const SqlEditor: FC = ({ ) : showEmptyState && !hasSqlStatement ? ( - ( null, ); @@ -249,7 +249,7 @@ const SqlEditorLeftBar = ({ } database={userSelectedDb} getDbList={handleDbList} handleError={handleError} diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx index 3105aa94bda53..a9360ad655e32 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx @@ -27,7 +27,7 @@ import { Logger } from 'src/logger/LogUtils'; import { Tooltip } from 'src/components/Tooltip'; import { detectOS } from 'src/utils/common'; import * as Actions from 'src/SqlLab/actions/sqlLab'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import getBootstrapData from 'src/utils/getBootstrapData'; import { locationContext } from 'src/pages/SqlLab/LocationContext'; import SqlEditor from '../SqlEditor'; @@ -259,8 +259,9 @@ class TabbedSqlEditors extends PureComponent { tab={emptyTab} closable={false} > - diff --git a/superset-frontend/src/assets/images/chart.svg b/superset-frontend/src/assets/images/chart.svg index 2267342bccd1f..34e45631e08d8 100644 --- a/superset-frontend/src/assets/images/chart.svg +++ b/superset-frontend/src/assets/images/chart.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/document.svg b/superset-frontend/src/assets/images/document.svg index e3d1bfe1beb28..552fc8d220b34 100644 --- a/superset-frontend/src/assets/images/document.svg +++ b/superset-frontend/src/assets/images/document.svg @@ -17,6 +17,6 @@ specific language governing permissions and limitations under the License. --> - - + + diff --git a/superset-frontend/src/assets/images/empty-charts.svg b/superset-frontend/src/assets/images/empty-charts.svg index b4cdd99086c76..8c7b205d2a904 100644 --- a/superset-frontend/src/assets/images/empty-charts.svg +++ b/superset-frontend/src/assets/images/empty-charts.svg @@ -16,15 +16,15 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - - - - - - + + + + + + + diff --git a/superset-frontend/src/assets/images/empty-dashboard.svg b/superset-frontend/src/assets/images/empty-dashboard.svg index c76eca0c30aaa..2acf05c801186 100644 --- a/superset-frontend/src/assets/images/empty-dashboard.svg +++ b/superset-frontend/src/assets/images/empty-dashboard.svg @@ -16,11 +16,11 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - - + + + + + + + diff --git a/superset-frontend/src/assets/images/empty-dataset.svg b/superset-frontend/src/assets/images/empty-dataset.svg index 5ce1752545b29..9a0299d3b422d 100644 --- a/superset-frontend/src/assets/images/empty-dataset.svg +++ b/superset-frontend/src/assets/images/empty-dataset.svg @@ -16,23 +16,24 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + diff --git a/superset-frontend/src/assets/images/empty-queries.svg b/superset-frontend/src/assets/images/empty-queries.svg index 2239c0ae8e920..1d2d8b00f229f 100644 --- a/superset-frontend/src/assets/images/empty-queries.svg +++ b/superset-frontend/src/assets/images/empty-queries.svg @@ -16,20 +16,20 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - + + + + - + - - - + + + diff --git a/superset-frontend/src/assets/images/empty-query.svg b/superset-frontend/src/assets/images/empty-query.svg index be72857d8e5c4..a19fc73eebfa9 100644 --- a/superset-frontend/src/assets/images/empty-query.svg +++ b/superset-frontend/src/assets/images/empty-query.svg @@ -16,10 +16,10 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - + + + + + + diff --git a/superset-frontend/src/assets/images/empty-table.svg b/superset-frontend/src/assets/images/empty-table.svg index c1062502f39dc..9079b07114cab 100644 --- a/superset-frontend/src/assets/images/empty-table.svg +++ b/superset-frontend/src/assets/images/empty-table.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/empty.svg b/superset-frontend/src/assets/images/empty.svg index e2c78339ce64c..b503324f6764f 100644 --- a/superset-frontend/src/assets/images/empty.svg +++ b/superset-frontend/src/assets/images/empty.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/empty_sql_chart.svg b/superset-frontend/src/assets/images/empty_sql_chart.svg index 6ab969575c9db..b729b471a6562 100644 --- a/superset-frontend/src/assets/images/empty_sql_chart.svg +++ b/superset-frontend/src/assets/images/empty_sql_chart.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/filter-results.svg b/superset-frontend/src/assets/images/filter-results.svg index 770a54b34f37f..7244f5538b262 100644 --- a/superset-frontend/src/assets/images/filter-results.svg +++ b/superset-frontend/src/assets/images/filter-results.svg @@ -16,19 +16,56 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - - - - - - - - - - + + + + + + diff --git a/superset-frontend/src/assets/images/filter.svg b/superset-frontend/src/assets/images/filter.svg index 0e1f6b41efc3d..2e8c5e0f0991a 100644 --- a/superset-frontend/src/assets/images/filter.svg +++ b/superset-frontend/src/assets/images/filter.svg @@ -16,17 +16,43 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - - - - - - - - - - - + + + + + + + diff --git a/superset-frontend/src/assets/images/star-circle.svg b/superset-frontend/src/assets/images/star-circle.svg index a46a1dd0fb81c..d9e6c77e2c5c0 100644 --- a/superset-frontend/src/assets/images/star-circle.svg +++ b/superset-frontend/src/assets/images/star-circle.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/union.svg b/superset-frontend/src/assets/images/union.svg index 6ac0e0f01621b..9ccf437b225a8 100644 --- a/superset-frontend/src/assets/images/union.svg +++ b/superset-frontend/src/assets/images/union.svg @@ -16,7 +16,7 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - - + + + diff --git a/superset-frontend/src/assets/images/vector.svg b/superset-frontend/src/assets/images/vector.svg index 0bf9c39c6ccb0..d2f946e8107a7 100644 --- a/superset-frontend/src/assets/images/vector.svg +++ b/superset-frontend/src/assets/images/vector.svg @@ -16,6 +16,6 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> - - + + diff --git a/superset-frontend/src/components/Chart/Chart.tsx b/superset-frontend/src/components/Chart/Chart.tsx index 3526a26b3f9ce..fd7b5fcaabe0d 100644 --- a/superset-frontend/src/components/Chart/Chart.tsx +++ b/superset-frontend/src/components/Chart/Chart.tsx @@ -31,7 +31,7 @@ import { } from '@superset-ui/core'; import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants'; import Loading from 'src/components/Loading'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import ErrorBoundary from 'src/components/ErrorBoundary'; import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils'; import { URL_PARAMS } from 'src/constants'; @@ -337,7 +337,8 @@ class Chart extends PureComponent { if (errorMessage && ensureIsArray(queriesResponse).length === 0) { return ( - { ensureIsArray(queriesResponse).length === 0 ) { return ( - diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx index d93e30a107b13..dd456b0db43c3 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.jsx @@ -30,7 +30,7 @@ import { VizType, } from '@superset-ui/core'; import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils'; -import { EmptyStateBig, EmptyStateSmall } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { ChartSource } from 'src/types/ChartSource'; import ChartContextMenu from './ChartContextMenu/ChartContextMenu'; @@ -308,7 +308,8 @@ class ChartRenderer extends Component { const noResultImage = 'chart.svg'; if (width > BIG_NO_RESULT_MIN_WIDTH && height > BIG_NO_RESULT_MIN_HEIGHT) { noResultsComponent = ( - + ); } diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx index 9cf0102b5ef91..bcab733f48c00 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx +++ b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx @@ -41,7 +41,7 @@ import Loading from 'src/components/Loading'; import BooleanCell from 'src/components/Table/cell-renderers/BooleanCell'; import NullCell from 'src/components/Table/cell-renderers/NullCell'; import TimeCell from 'src/components/Table/cell-renderers/TimeCell'; -import { EmptyStateMedium } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { getDatasourceSamples } from 'src/components/Chart/chartAction'; import Table, { ColumnsType, TableSize } from 'src/components/Table'; import HeaderWithRadioGroup from 'src/components/Table/header-renderers/HeaderWithRadioGroup'; @@ -285,7 +285,7 @@ export default function DrillDetailPane({ } else if (resultsPage?.total === 0) { // Render empty state if no results are returned for page const title = t('No rows were returned for this dataset'); - tableContent = ; + tableContent = ; } else { // Render table if at least one page has successfully loaded tableContent = ( diff --git a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx index 44d0bff0e04bd..ee17f2d801d49 100644 --- a/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx +++ b/superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx @@ -28,7 +28,7 @@ import { import userEvent from '@testing-library/user-event'; import { api } from 'src/hooks/apiResources/queryApi'; import DatabaseSelector, { DatabaseSelectorProps } from '.'; -import { EmptyStateSmall } from '../EmptyState'; +import { EmptyState } from '../EmptyState'; const createProps = (): DatabaseSelectorProps => ({ db: { @@ -307,7 +307,7 @@ test('should show empty state if there are no options', async () => { } + emptyState={} />, { useRedux: true, store }, ); diff --git a/superset-frontend/src/components/EmptyState/EmptyState.stories.tsx b/superset-frontend/src/components/EmptyState/EmptyState.stories.tsx index 9d6f56c52d002..dbc3a0af9cc86 100644 --- a/superset-frontend/src/components/EmptyState/EmptyState.stories.tsx +++ b/superset-frontend/src/components/EmptyState/EmptyState.stories.tsx @@ -9,64 +9,59 @@ * * 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 + * 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 { Meta, StoryFn } from '@storybook/react'; +import { Row, Col } from 'antd'; +import { EmptyState, imageMap } from '.'; -import EmptyImage from 'src/assets/images/empty.svg'; -import ChartImage from 'src/assets/images/chart.svg'; -import FilterImage from 'src/assets/images/filter.svg'; -import { EmptyStateBig, EmptyStateMedium, EmptyStateSmall } from '.'; +const emptyStates = [ + { title: null, description: null, image: undefined }, + ...Object.keys(imageMap).map(key => ({ + image: key, + title: `Empty State with image ${key}`, + description: 'This is the default empty state.', + })), +]; export default { - title: 'Empty state', - component: EmptyStateMedium, -}; - -export const SmallEmptyState = () => ( - } - title="Small empty state" - description="This is an example of a small empty state" - /> -); - -export const MediumEmptyState = () => ( - } - title="Medium empty state" - description="This is an example of a medium empty state" - /> -); + title: 'Empty State Gallery', + component: EmptyState, + argTypes: { + size: { + control: { type: 'select' }, + options: ['small', 'medium', 'large'], + defaultValue: 'medium', + description: 'Size of the Empty State components', + }, + }, +} as Meta; -export const MediumEmptyStateWithButton = () => ( - } - title="Medium empty state" - description="This is an example of a medium empty state with a button" - buttonAction={() => {}} - buttonText="Click!" - /> +export const Gallery: StoryFn<{ size: 'small' | 'medium' | 'large' }> = ({ + size, +}) => ( + + {emptyStates.map((state, index) => ( + + + Childrens render here. + + + ))} + ); -export const BigEmptyState = () => ( - } - title="Big empty state" - description="This is an example of a big empty state" - /> -); - -export const BigEmptyStateWithButton = () => ( - } - title="Big empty state" - description="This is an example of a big empty state with a button" - buttonText="Click!" - buttonAction={() => {}} - /> -); +Gallery.args = { + size: 'medium', +}; diff --git a/superset-frontend/src/components/EmptyState/index.tsx b/superset-frontend/src/components/EmptyState/index.tsx index edc13e622fa34..a083b6cdcfaa8 100644 --- a/superset-frontend/src/components/EmptyState/index.tsx +++ b/superset-frontend/src/components/EmptyState/index.tsx @@ -16,38 +16,57 @@ * specific language governing permissions and limitations * under the License. */ - -import { - ReactNode, - SyntheticEvent, - MouseEventHandler as ReactMouseEventHandler, -} from 'react'; +import { ReactNode, SyntheticEvent } from 'react'; import { styled, css, SupersetTheme, t } from '@superset-ui/core'; import Button from 'src/components/Button'; + +// Importing svg images +import FilterResultsImage from 'src/assets/images/filter-results.svg'; +import ChartImage from 'src/assets/images/chart.svg'; +import FilterImage from 'src/assets/images/filter.svg'; +import EmptyChartsImage from 'src/assets/images/empty-charts.svg'; +import EmptyDashboardImage from 'src/assets/images/empty-dashboard.svg'; +import UnionImage from 'src/assets/images/union.svg'; +import EmptyQueriesImage from 'src/assets/images/empty-queries.svg'; +import StarCircleImage from 'src/assets/images/star-circle.svg'; +import VectorImage from 'src/assets/images/vector.svg'; +import DocumentImage from 'src/assets/images/document.svg'; +import DatasetImage from 'src/assets/images/empty-dataset.svg'; +import EmptySqlChartImage from 'src/assets/images/empty_sql_chart.svg'; +import EmptyQueryImage from 'src/assets/images/empty-query.svg'; +import EmptyTableImage from 'src/assets/images/empty-table.svg'; +import EmptyImage from 'src/assets/images/empty.svg'; import { Empty } from './Empty'; -export enum EmptyStateSize { - Small, - Medium, - Big, -} +export const imageMap = { + 'chart.svg': , + 'document.svg': , + 'empty-charts.svg': , + 'empty-dashboard.svg': , + 'empty-dataset.svg': , + 'empty-queries.svg': , + 'empty-query.svg': , + 'empty-table.svg': , + 'empty.svg': , + 'empty_sql_chart.svg': , + 'filter-results.svg': , + 'filter.svg': , + 'star-circle.svg': , + 'union.svg': , + 'vector.svg': , +}; -export interface EmptyStateSmallProps { +type EmptyStateSize = 'small' | 'medium' | 'large'; + +export type EmptyStateProps = { title?: ReactNode; description?: ReactNode; - image?: ReactNode; -} - -export interface EmptyStateProps extends EmptyStateSmallProps { + image?: ReactNode | string; buttonText?: ReactNode; - buttonAction?: ReactMouseEventHandler; - className?: string; -} - -export interface ImageContainerProps { - image: ReactNode; - size: EmptyStateSize; -} + buttonAction?: (event: SyntheticEvent) => void; + size?: EmptyStateSize; + children?: ReactNode; +}; const EmptyStateContainer = styled.div` ${({ theme }) => css` @@ -55,6 +74,7 @@ const EmptyStateContainer = styled.div` flex-direction: column; width: 100%; height: 100%; + color: ${theme.colors.grayscale.light2}; align-items: center; justify-content: center; padding: ${theme.gridUnit * 4}px; @@ -75,43 +95,24 @@ const EmptyStateContainer = styled.div` `} `; -const TextContainer = styled.div``; - -const Title = styled.p` - ${({ theme }) => css` - font-size: ${theme.typography.sizes.m}px; +const Title = styled.p<{ size: EmptyStateSize }>` + ${({ theme, size }) => css` + font-size: ${size === 'large' + ? theme.typography.sizes.l + : theme.typography.sizes.m}px; color: ${theme.colors.grayscale.light1}; - margin: ${theme.gridUnit * 2}px 0 0 0; + margin-top: ${size === 'large' ? theme.gridUnit * 4 : theme.gridUnit * 2}px; font-weight: ${theme.typography.weights.bold}; `} `; -const BigTitle = styled(Title)` - ${({ theme }) => css` - font-size: ${theme.typography.sizes.l}px; - color: ${theme.colors.grayscale.light1}; - margin-top: ${theme.gridUnit * 4}px; - `} -`; - -const Description = styled.p` - ${({ theme }) => css` - font-size: ${theme.typography.sizes.s}px; +const Description = styled.p<{ size: EmptyStateSize }>` + ${({ theme, size }) => css` + font-size: ${size === 'large' + ? theme.typography.sizes.m + : theme.typography.sizes.s}px; color: ${theme.colors.grayscale.light1}; - margin: ${theme.gridUnit * 2}px 0 0 0; - `} -`; - -const BigDescription = styled(Description)` - ${({ theme }) => css` - font-size: ${theme.typography.sizes.m}px; - `} -`; - -const SmallDescription = styled(Description)` - ${({ theme }) => css` - margin-top: ${theme.gridUnit}px; - line-height: 1.2; + margin-top: ${theme.gridUnit * 2}px; `} `; @@ -122,81 +123,68 @@ const ActionButton = styled(Button)` `} `; -const getImage = (image: string | ReactNode) => - typeof image === 'string' ? `/static/assets/images/${image}` : image; - const getImageHeight = (size: EmptyStateSize) => { switch (size) { - case EmptyStateSize.Small: + case 'small': return { height: '50px' }; - case EmptyStateSize.Medium: + case 'medium': return { height: '80px' }; - case EmptyStateSize.Big: + case 'large': return { height: '150px' }; default: - return { height: '50px' }; + return { height: '80px' }; } }; -const ImageContainer = ({ image, size }: ImageContainerProps) => ( - -); +const ImageContainer = ({ + image, + size, +}: { + image?: ReactNode | string; + size: EmptyStateSize; +}) => { + if (!image) return null; + const mappedImage = typeof image === 'string' ? imageMap[image] : image; + return ( +
+ +
+ ); +}; const handleMouseDown = (e: SyntheticEvent) => { e.preventDefault(); e.stopPropagation(); }; -export const EmptyStateBig = ({ - title, - image, - description, - buttonAction, +export const EmptyState: React.FC = ({ + title = t('No results'), + description = t('There is currently no information to display.'), + image = 'empty.svg', buttonText, - className, -}: EmptyStateProps) => ( - - {image && } - css` - max-width: ${theme.gridUnit * 150}px; - `} - > - {title} - {description && {description}} - {buttonAction && buttonText && ( - - {buttonText} - - )} - - -); - -export const EmptyStateMedium = ({ - title, - image, - description, buttonAction, - buttonText, -}: EmptyStateProps) => ( + size = 'medium', + children, +}) => ( - {image && } - } +
css` - max-width: ${theme.gridUnit * 100}px; + max-width: ${size === 'large' + ? theme.gridUnit * 150 + : theme.gridUnit * 100}px; `} > - {title} - {description && {description}} + {title && {title}} + {description && ( + + {description} + + )} {buttonText && buttonAction && ( )} - - -); - -export const EmptyStateSmall = ({ - title, - image, - description, -}: EmptyStateSmallProps) => ( - - {image && } - css` - max-width: ${theme.gridUnit * 75}px; - `} - > - {title} - {description && {description}} - + {children} +
); - -const TRANSLATIONS = { - NO_DATABASES_MATCH_TITLE: t('No databases match your search'), - NO_DATABASES_AVAILABLE_TITLE: t('There are no databases available'), - MANAGE_YOUR_DATABASES_TEXT: t('Manage your databases'), - HERE_TEXT: t('here'), -}; - -export const emptyStateComponent = (emptyResultsWithSearch: boolean) => ( - - {TRANSLATIONS.MANAGE_YOUR_DATABASES_TEXT}{' '} - {TRANSLATIONS.HERE_TEXT} -

- } - /> -); diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index 30bcb99aa3b98..cfa060f11da63 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -37,7 +37,7 @@ import { ViewModeType, } from './types'; import { ListViewError, useListViewState } from './utils'; -import { EmptyStateBig, EmptyStateProps } from '../EmptyState'; +import { EmptyState, EmptyStateProps } from '../EmptyState'; const ListViewStyles = styled.div` text-align: center; @@ -447,17 +447,19 @@ function ListView({ {!loading && rows.length === 0 && ( {query.filters ? ( - handleClearFilterControls()} buttonText={t('clear all filters')} /> ) : ( - )} diff --git a/superset-frontend/src/components/Table/index.tsx b/superset-frontend/src/components/Table/index.tsx index 1495befa5acc9..11a803893b92c 100644 --- a/superset-frontend/src/components/Table/index.tsx +++ b/superset-frontend/src/components/Table/index.tsx @@ -16,13 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect, useRef, ReactElement, Key } from 'react'; +import { useState, useEffect, useRef, Key } from 'react'; import AntTable, { ColumnsType, TableProps as AntTableProps, } from 'antd/lib/table'; -import ConfigProvider from 'antd/lib/config-provider'; import { PaginationProps } from 'antd/lib/pagination'; import { t, useTheme, logging, styled } from '@superset-ui/core'; import Loading from 'src/components/Loading'; @@ -116,10 +115,6 @@ export interface TableProps { * Set table to display no data even if data has been provided */ hideData?: boolean; - /** - * emptyComponent - */ - emptyComponent?: ReactElement; /** * Enables setting the text displayed in various components and tooltips within the Table UI. */ @@ -256,7 +251,6 @@ export function Table( defaultPageSize = 15, pageSizeOptions = ['5', '15', '25', '50', '100'], hideData = false, - emptyComponent, locale, height, virtualize = false, @@ -288,9 +282,6 @@ export function Table( onChange: onSelectChange, }; - const renderEmpty = () => - emptyComponent ??
{mergedLocale.emptyText}
; - // Log use of experimental features useEffect(() => { if (reorderable === true) { @@ -403,31 +394,29 @@ export function Table( }; return ( - -
- {!virtualize && ( - - )} - {virtualize && ( - - )} -
- +
+ {!virtualize && ( + + )} + {virtualize && ( + + )} +
); } diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 29dc12a7c89cd..cdd59b459a091 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -65,7 +65,7 @@ import { } from 'src/dashboard/util/constants'; import FilterBar from 'src/dashboard/components/nativeFilters/FilterBar'; import Loading from 'src/components/Loading'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { useUiConfig } from 'src/components/UiConfigContext'; import ResizableSidebar from 'src/components/ResizableSidebar'; import { @@ -667,8 +667,9 @@ const DashboardBuilder = () => { {!editMode && !topLevelTabs && dashboardLayout[DASHBOARD_GRID_ID]?.children?.length === 0 && ( - @@ -228,8 +229,9 @@ class DashboardGrid extends PureComponent { ); const topLevelTabEmptyState = editMode ? ( - ) : ( - { 'Make sure that the controls are configured properly and the datasource contains data for the selected time range', ), ).not.toBeInTheDocument(); // description should display only in Explore view - expect(screen.getByAltText('empty')).toBeVisible(); + expect(screen.getByRole('img', { name: 'empty' })).toBeVisible(); }); it('should render anchor link when not editing', async () => { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx index 2c4695f8c7285..36a77316413be 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx @@ -22,7 +22,7 @@ import classNames from 'classnames'; import { useDispatch, useSelector } from 'react-redux'; import { styled, t } from '@superset-ui/core'; -import { EmptyStateMedium } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import EditableTitle from 'src/components/EditableTitle'; import { setEditMode } from 'src/dashboard/actions/dashboardState'; import DashboardComponent from 'src/dashboard/containers/DashboardComponent'; @@ -197,7 +197,7 @@ const Tab = props => { )} {shouldDisplayEmptyState && ( - { expect( screen.getByText('There are no components added to this tab'), ).toBeVisible(); - expect(screen.getByAltText('empty')).toBeVisible(); + expect(screen.getByRole('img', { name: 'empty' })).toBeVisible(); expect(screen.queryByText('edit mode')).not.toBeInTheDocument(); }); @@ -397,7 +397,7 @@ test('Render tab content with no children, editMode: true, canEdit: true', () => expect( screen.getByText('Drag and drop components to this tab'), ).toBeVisible(); - expect(screen.getByAltText('empty')).toBeVisible(); + expect(screen.getByRole('img', { name: 'empty' })).toBeVisible(); expect( screen.getByRole('link', { name: 'create a new chart' }), ).toBeVisible(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index b76b65c13395c..d6960ad37027f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -33,7 +33,7 @@ import cx from 'classnames'; import { FeatureFlag, isFeatureEnabled, styled, t } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import Loading from 'src/components/Loading'; -import { EmptyStateSmall } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { getFilterBarTestId } from './utils'; import { VerticalBarProps } from './types'; import Header from './Header'; @@ -168,7 +168,8 @@ const VerticalFilterBar: FC = ({ () => filterValues.length === 0 ? ( - ; + return ; } return ( diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx index fe262fe72078d..e96963d10854d 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx @@ -26,7 +26,7 @@ import { getClientErrorObject, } from '@superset-ui/core'; import Loading from 'src/components/Loading'; -import { EmptyStateMedium } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { getChartDataRequest } from 'src/components/Chart/chartAction'; import { ResultsPaneProps, QueryResultInterface } from '../types'; import { SingleQueryResultPane } from './SingleQueryResultPane'; @@ -110,7 +110,7 @@ export const useResultsPane = ({ if (errorMessage) { const title = t('Run a query to display results'); return Array(queryCount).fill( - , + , ); } @@ -136,7 +136,7 @@ export const useResultsPane = ({ if (resultResp.length === 0) { const title = t('No results were returned for this query'); return Array(queryCount).fill( - , + , ); } return resultResp diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx index 304ec3409d342..905fd04f9ceb5 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx @@ -38,7 +38,7 @@ import TextControl from 'src/explore/components/controls/TextControl'; import CheckboxControl from 'src/explore/components/controls/CheckboxControl'; import PopoverSection from 'src/components/PopoverSection'; import ControlHeader from 'src/explore/components/ControlHeader'; -import { EmptyStateSmall } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { ANNOTATION_SOURCE_TYPES, ANNOTATION_TYPES, @@ -111,8 +111,9 @@ const NotFoundContentWrapper = styled.div` const NotFoundContent = () => ( - {t('Add an annotation layer')}{' '} diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx index 711cad392de2f..6e0a256965bba 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx @@ -43,7 +43,7 @@ import { Select } from 'src/components'; import { Form, FormItem } from 'src/components/Form'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; import { SQLEditor } from 'src/components/AsyncAceEditor'; -import { EmptyStateSmall } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords'; import { StyledColumnOption } from 'src/explore/components/optionRenderers'; import { @@ -334,8 +334,9 @@ const ColumnSelectPopover = ({ /> ) : datasourceType === DatasourceType.Table ? ( - ) : ( - {isTemporal && simpleColumns.length === 0 ? ( - ) : datasource.type === DatasourceType.Table ? ( - ) : ( - diff --git a/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx b/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx index 8ecbf33872185..ae51762d788ae 100644 --- a/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx +++ b/superset-frontend/src/features/allEntities/AllEntitiesTable.tsx @@ -23,7 +23,7 @@ import { TagsList } from 'src/components/Tags'; import FacePile from 'src/components/FacePile'; import Tag from 'src/types/TagType'; import Owner from 'src/types/Owner'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { NumberParam, useQueryParam } from 'use-query-params'; const MAX_TAGS_TO_SHOW = 3; @@ -160,8 +160,9 @@ export default function AllEntitiesTable({ {renderTable('query')} ) : ( - setShowTagModal(true)} buttonText={t('Add tag to entities')} diff --git a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/MessageContent.tsx b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/MessageContent.tsx index 9f57935947897..5aab72027222a 100644 --- a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/MessageContent.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/MessageContent.tsx @@ -18,7 +18,7 @@ */ import { t, styled } from '@superset-ui/core'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { Link } from 'react-router-dom'; const StyledContainer = styled.div` @@ -31,7 +31,7 @@ const StyledContainer = styled.div` height: 100%; `; -const StyledEmptyStateBig = styled(EmptyStateBig)` +const StyledEmptyState = styled(EmptyState)` max-width: 50%; p { @@ -91,8 +91,9 @@ export const MessageContent = (props: MessageContentProps) => { } return ( - diff --git a/superset-frontend/src/features/datasets/AddDataset/EditDataset/UsageTab/index.tsx b/superset-frontend/src/features/datasets/AddDataset/EditDataset/UsageTab/index.tsx index 0e41182096130..289a67f5aa98f 100644 --- a/superset-frontend/src/features/datasets/AddDataset/EditDataset/UsageTab/index.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/EditDataset/UsageTab/index.tsx @@ -32,7 +32,7 @@ import Table, { TableSize, OnChangeFunction, } from 'src/components/Table'; -import { EmptyStateBig } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import ChartImage from 'src/assets/images/chart.svg'; import Icons from 'src/components/Icons'; import { useToasts } from 'src/components/MessageToasts/withToasts'; @@ -147,7 +147,7 @@ const emptyStateButtonText = ( ); -const StyledEmptyStateBig = styled(EmptyStateBig)` +const StyledEmptyState = styled(EmptyState)` margin: ${({ theme }) => 13 * theme.gridUnit}px 0; `; @@ -250,8 +250,9 @@ const DatasetUsage = ({ datasetId }: DatasetUsageProps) => { onChange={onChange} /> {!data.length && !loading ? ( - } + size="large" title={t('No charts')} description={t('This dataset is not used to power any charts.')} buttonText={emptyStateButtonText} diff --git a/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx b/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx index c14109aede56f..adb52234224b9 100644 --- a/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx @@ -20,7 +20,7 @@ import { useEffect, SetStateAction, Dispatch, useCallback } from 'react'; import { styled, t } from '@superset-ui/core'; import TableSelector, { TableOption } from 'src/components/TableSelector'; import { DatabaseObject } from 'src/components/DatabaseSelector'; -import { emptyStateComponent } from 'src/components/EmptyState'; +import { EmptyState } from 'src/components/EmptyState'; import { useToasts } from 'src/components/MessageToasts/withToasts'; import { LocalStorageKeys, getItem } from 'src/utils/localStorageHelpers'; import { @@ -178,13 +178,30 @@ export default function LeftPanel({ ), [datasetNames], ); + const getDatabaseEmptyState = (emptyResultsWithSearch: boolean) => ( + + {t('Manage your databases')}{' '} + {t('here')} + + } + size="small" + /> + ); return ( { }); test('show empty state if there is no data', () => { renderActivityTable(emptyActivityProps); - expect( - screen.getByText( - /recently created charts, dashboards, and saved queries will appear here/i, - ), - ).toBeInTheDocument(); + expect(screen.getByText(/nothing here yet/i)).toBeInTheDocument(); }); diff --git a/superset-frontend/src/features/home/ChartTable.test.tsx b/superset-frontend/src/features/home/ChartTable.test.tsx index f03a705245e81..0e19f8c059ad4 100644 --- a/superset-frontend/src/features/home/ChartTable.test.tsx +++ b/superset-frontend/src/features/home/ChartTable.test.tsx @@ -87,9 +87,7 @@ const renderChartTable = (props: any) => test('renders with EmptyState if no data present', async () => { await renderChartTable(mockedProps); expect(screen.getAllByRole('tab')).toHaveLength(3); - expect( - screen.getByText(/other charts will appear here/i), - ).toBeInTheDocument(); + expect(screen.getByText(/nothing here yet/i)).toBeInTheDocument(); }); test('fetches chart favorites and renders chart cards', async () => { diff --git a/superset-frontend/src/features/home/EmptyState.test.tsx b/superset-frontend/src/features/home/EmptyState.test.tsx index fe633bc788632..0be86559dcf63 100644 --- a/superset-frontend/src/features/home/EmptyState.test.tsx +++ b/superset-frontend/src/features/home/EmptyState.test.tsx @@ -70,15 +70,7 @@ describe('EmptyState', () => { // Select the first description node const textContainer = wrapper.find('.ant-empty-description').at(0); - expect(textContainer.text()).toEqual( - variant.tab === TableTab.Favorite - ? "You don't have any favorites yet!" - : `No ${ - variant.tableName === WelcomeTable.SavedQueries - ? 'saved queries' - : variant.tableName.toLowerCase() - } yet`, - ); + expect(textContainer.text()).toEqual('Nothing here yet'); expect(wrapper.find('button')).toHaveLength(1); }); }); @@ -95,9 +87,7 @@ describe('EmptyState', () => { expect(wrapper.find('.ant-empty-image').children()).toHaveLength(1); // Check the correct text is displayed - expect(textContainer.text()).toContain( - `Recently ${recent.tab?.toLowerCase()} charts, dashboards, and saved queries will appear here`, - ); + expect(textContainer.text()).toContain(`Nothing here yet`); }); }); }); diff --git a/superset-frontend/src/features/home/EmptyState.tsx b/superset-frontend/src/features/home/EmptyState.tsx index cbcede55ecad2..7a2ba4d62ba61 100644 --- a/superset-frontend/src/features/home/EmptyState.tsx +++ b/superset-frontend/src/features/home/EmptyState.tsx @@ -16,172 +16,96 @@ * specific language governing permissions and limitations * under the License. */ -import { Link } from 'react-router-dom'; import Button from 'src/components/Button'; -import { Empty } from 'src/components/EmptyState/Empty'; +import { EmptyState as EmptyStateComponent } from 'src/components/EmptyState'; import { TableTab } from 'src/views/CRUD/types'; import { styled, t } from '@superset-ui/core'; import { WelcomeTable } from './types'; -const welcomeTableLabels: Record = { - [WelcomeTable.Charts]: t('charts'), - [WelcomeTable.Dashboards]: t('dashboards'), - [WelcomeTable.Recents]: t('recents'), - [WelcomeTable.SavedQueries]: t('saved queries'), -}; - -const welcomeTableEmpty: Record = { - [WelcomeTable.Charts]: t('No charts yet'), - [WelcomeTable.Dashboards]: t('No dashboards yet'), - [WelcomeTable.Recents]: t('No recents yet'), - [WelcomeTable.SavedQueries]: t('No saved queries yet'), -}; - -const welcomeTableWillAppear: Record string> = - { - [WelcomeTable.Charts]: (other: string) => - t('%(other)s charts will appear here', { other }), - [WelcomeTable.Dashboards]: (other: string) => - t('%(other)s dashboards will appear here', { other }), - [WelcomeTable.Recents]: (other: string) => - t('%(other)s recents will appear here', { other }), - [WelcomeTable.SavedQueries]: (other: string) => - t('%(other)s saved queries will appear here', { other }), - }; - -export interface EmptyStateProps { - tableName: WelcomeTable; - tab?: string; - otherTabTitle?: string; -} const EmptyContainer = styled.div` min-height: 200px; display: flex; + color: ${({ theme }) => theme.colors.grayscale.light2}; flex-direction: column; justify-content: space-around; `; -const ButtonContainer = styled.div` - Button { - svg { - color: ${({ theme }) => theme.colors.grayscale.light5}; - } - } -`; -type Redirects = Record< - WelcomeTable.Charts | WelcomeTable.Dashboards | WelcomeTable.SavedQueries, - string ->; +const ICONS = { + [WelcomeTable.Charts]: 'empty-charts.svg', + [WelcomeTable.Dashboards]: 'empty-dashboard.svg', + [WelcomeTable.Recents]: 'union.svg', + [WelcomeTable.SavedQueries]: 'empty-queries.svg', +} as const; -export default function EmptyState({ - tableName, - tab, - otherTabTitle, -}: EmptyStateProps) { - const mineRedirects: Redirects = { +const REDIRECTS = { + create: { [WelcomeTable.Charts]: '/chart/add', [WelcomeTable.Dashboards]: '/dashboard/new', [WelcomeTable.SavedQueries]: '/sqllab?new=true', - }; - const favRedirects: Redirects = { + }, + viewAll: { [WelcomeTable.Charts]: '/chart/list', [WelcomeTable.Dashboards]: '/dashboard/list/', [WelcomeTable.SavedQueries]: '/savedqueryview/list/', - }; - const tableIcon: Record = { - [WelcomeTable.Charts]: 'empty-charts.svg', - [WelcomeTable.Dashboards]: 'empty-dashboard.svg', - [WelcomeTable.Recents]: 'union.svg', - [WelcomeTable.SavedQueries]: 'empty-queries.svg', - }; - const mine = {welcomeTableEmpty[tableName]}; - const recent = ( - - {(() => { - if (tab === TableTab.Viewed) { - return t( - `Recently viewed charts, dashboards, and saved queries will appear here`, - ); - } - if (tab === TableTab.Created) { - return t( - 'Recently created charts, dashboards, and saved queries will appear here', - ); - } - if (tab === TableTab.Other) { - const other = otherTabTitle || t('Other'); - return welcomeTableWillAppear[tableName](other); - } - if (tab === TableTab.Edited) { - return t( - `Recently edited charts, dashboards, and saved queries will appear here`, - ); - } - return null; - })()} - - ); + }, +} as const; + +export interface EmptyStateProps { + tableName: WelcomeTable; + tab?: string; + otherTabTitle?: string; +} + +export default function EmptyState({ + tableName, + tab, + otherTabTitle, +}: EmptyStateProps) { + const getActionButton = () => { + if (tableName === WelcomeTable.Recents) { + return null; + } + + const isFavorite = tab === TableTab.Favorite; + const buttonText = + tableName === WelcomeTable.SavedQueries + ? isFavorite + ? t('SQL Lab queries') + : t('SQL query') + : isFavorite + ? t(tableName.toLowerCase()) + : tableName.slice(0, -1); + + const url = isFavorite + ? REDIRECTS.viewAll[tableName] + : REDIRECTS.create[tableName]; - // Mine and Recent Activity(all tabs) tab empty state - if ( - tab === TableTab.Mine || - tableName === WelcomeTable.Recents || - tab === TableTab.Other - ) { return ( - - - {tableName !== WelcomeTable.Recents && ( - - - - - - )} - - + ); - } - // Favorite tab empty state + }; + + const image = + tab === TableTab.Favorite ? 'star-circle.svg' : ICONS[tableName]; + return ( - - {t("You don't have any favorites yet!")} - - } + - - + {getActionButton()} + ); } From 6d117ffbb564f611df21fcb9a19c15b9567f35d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90=E1=BB=97=20Tr=E1=BB=8Dng=20H=E1=BA=A3i?= <41283691+hainenber@users.noreply.github.com> Date: Thu, 23 Jan 2025 18:54:40 +0700 Subject: [PATCH 09/18] chore: fix `tsc` errors (#31965) Signed-off-by: hainenber --- .../superset-ui-core/src/models/Registry.ts | 4 ++-- .../src/components/ModalTrigger/index.tsx | 2 +- .../src/components/Select/utils.tsx | 17 ++++++++++++----- .../controls/VizTypeControl/index.tsx | 10 +--------- .../components/controls/VizTypeControl/types.ts | 10 ++++++++++ superset-frontend/src/views/routes.tsx | 6 +++--- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/models/Registry.ts b/superset-frontend/packages/superset-ui-core/src/models/Registry.ts index 53aff08c4a415..7488ab82459fc 100644 --- a/superset-frontend/packages/superset-ui-core/src/models/Registry.ts +++ b/superset-frontend/packages/superset-ui-core/src/models/Registry.ts @@ -22,11 +22,11 @@ export enum OverwritePolicy { Warn = 'WARN', } -interface ItemWithValue { +export interface ItemWithValue { value: T; } -interface ItemWithLoader { +export interface ItemWithLoader { loader: () => T; } diff --git a/superset-frontend/src/components/ModalTrigger/index.tsx b/superset-frontend/src/components/ModalTrigger/index.tsx index 58c19347e626d..d0de9db1d77e6 100644 --- a/superset-frontend/src/components/ModalTrigger/index.tsx +++ b/superset-frontend/src/components/ModalTrigger/index.tsx @@ -21,7 +21,7 @@ import { forwardRef, useState, ReactNode, MouseEvent } from 'react'; import Modal from 'src/components/Modal'; import Button from 'src/components/Button'; -interface ModalTriggerProps { +export interface ModalTriggerProps { dialogClassName?: string; triggerNode: ReactNode; modalTitle?: string; diff --git a/superset-frontend/src/components/Select/utils.tsx b/superset-frontend/src/components/Select/utils.tsx index 56483367328f1..b4d77204b53cc 100644 --- a/superset-frontend/src/components/Select/utils.tsx +++ b/superset-frontend/src/components/Select/utils.tsx @@ -186,8 +186,10 @@ export const handleFilterOptionHelper = ( const searchValue = search.trim().toLowerCase(); if (optionFilterProps?.length) { return optionFilterProps.some(prop => { - const optionProp = option?.[prop] - ? String(option[prop]).trim().toLowerCase() + const optionProp = option?.[prop as keyof LabeledValue] + ? String(option[prop as keyof LabeledValue]) + .trim() + .toLowerCase() : ''; return optionProp.includes(searchValue); }); @@ -200,7 +202,9 @@ export const handleFilterOptionHelper = ( export const hasCustomLabels = (options: SelectOptionsType) => options?.some(opt => !!opt?.customLabel); -export const renderSelectOptions = (options: SelectOptionsType) => +export const renderSelectOptions = ( + options: SelectOptionsType, +): JSX.Element[] => options.map(opt => { const isOptObject = typeof opt === 'object'; const label = isOptObject ? opt?.label || opt.value : opt; @@ -213,7 +217,10 @@ export const renderSelectOptions = (options: SelectOptionsType) => ); }); -export const mapValues = (values: SelectOptionsType, labelInValue: boolean) => +export const mapValues = ( + values: SelectOptionsType, + labelInValue: boolean, +): (Record | any)[] => labelInValue ? values.map(opt => ({ key: opt.value, @@ -222,7 +229,7 @@ export const mapValues = (values: SelectOptionsType, labelInValue: boolean) => })) : values.map(opt => opt.value); -export const mapOptions = (values: SelectOptionsType) => +export const mapOptions = (values: SelectOptionsType): Record[] => values.map(opt => ({ children: opt.label, key: opt.value, diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl/index.tsx b/superset-frontend/src/explore/components/controls/VizTypeControl/index.tsx index 1adf60eee8bde..96754950cee2e 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl/index.tsx +++ b/superset-frontend/src/explore/components/controls/VizTypeControl/index.tsx @@ -33,15 +33,7 @@ import VizTypeGallery, { MAX_ADVISABLE_VIZ_GALLERY_WIDTH, } from './VizTypeGallery'; import { FastVizSwitcher } from './FastVizSwitcher'; - -interface VizTypeControlProps { - description?: string; - label?: string; - name: string; - onChange: (vizType: string | null) => void; - value: string | null; - isModalOpenInit?: boolean; -} +import { VizTypeControlProps } from './types'; const bootstrapData = getBootstrapData(); const denyList: string[] = ( diff --git a/superset-frontend/src/explore/components/controls/VizTypeControl/types.ts b/superset-frontend/src/explore/components/controls/VizTypeControl/types.ts index 2fe40ba633c47..7d38a4b5e3ba4 100644 --- a/superset-frontend/src/explore/components/controls/VizTypeControl/types.ts +++ b/superset-frontend/src/explore/components/controls/VizTypeControl/types.ts @@ -27,9 +27,19 @@ export interface FastVizSwitcherProps { onChange: (vizName: string) => void; currentSelection: string | null; } + export interface VizTileProps { vizMeta: VizMeta; isActive: boolean; isRendered: boolean; onTileClick: (vizType: string) => void; } + +export interface VizTypeControlProps { + description?: string; + label?: string; + name: string; + onChange: (vizType: string | null) => void; + value: string | null; + isModalOpenInit?: boolean; +} diff --git a/superset-frontend/src/views/routes.tsx b/superset-frontend/src/views/routes.tsx index 7044e8940af34..30ce65f069537 100644 --- a/superset-frontend/src/views/routes.tsx +++ b/superset-frontend/src/views/routes.tsx @@ -238,7 +238,7 @@ if (isFeatureEnabled(FeatureFlag.TaggingSystem)) { }); } -const frontEndRoutes = routes +const frontEndRoutes: Record = routes .map(r => r.path) .reduce( (acc, curr) => ({ @@ -248,10 +248,10 @@ const frontEndRoutes = routes {}, ); -export function isFrontendRoute(path?: string) { +export const isFrontendRoute = (path?: string): boolean => { if (path) { const basePath = path.split(/[?#]/)[0]; // strip out query params and link bookmarks return !!frontEndRoutes[basePath]; } return false; -} +}; From 14f798afec47c75ac6129d87cbfa04787261143b Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 23 Jan 2025 15:59:45 -0500 Subject: [PATCH 10/18] fix: proper URL building (#31962) --- superset/dashboards/permalink/api.py | 5 ++--- superset/explore/permalink/api.py | 5 ++--- superset/sqllab/permalink/api.py | 9 ++++----- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/superset/dashboards/permalink/api.py b/superset/dashboards/permalink/api.py index a6ae2910f42a1..208cda2a1f2e9 100644 --- a/superset/dashboards/permalink/api.py +++ b/superset/dashboards/permalink/api.py @@ -16,7 +16,7 @@ # under the License. import logging -from flask import request, Response +from flask import request, Response, url_for from flask_appbuilder.api import expose, protect, safe from marshmallow import ValidationError @@ -98,8 +98,7 @@ def post(self, pk: str) -> Response: dashboard_id=pk, state=state, ).run() - http_origin = request.headers.environ.get("HTTP_ORIGIN") - url = f"{http_origin}/superset/dashboard/p/{key}/" + url = url_for("Superset.dashboard_permalink", key=key, _external=True) return self.response(201, key=key, url=url) except (ValidationError, DashboardPermalinkInvalidStateError) as ex: return self.response(400, message=str(ex)) diff --git a/superset/explore/permalink/api.py b/superset/explore/permalink/api.py index bc9bd1cf67a21..a554225719580 100644 --- a/superset/explore/permalink/api.py +++ b/superset/explore/permalink/api.py @@ -16,7 +16,7 @@ # under the License. import logging -from flask import request, Response +from flask import request, Response, url_for from flask_appbuilder.api import expose, protect, safe from marshmallow import ValidationError @@ -95,8 +95,7 @@ def post(self) -> Response: try: state = self.add_model_schema.load(request.json) key = CreateExplorePermalinkCommand(state=state).run() - http_origin = request.headers.environ.get("HTTP_ORIGIN") - url = f"{http_origin}/superset/explore/p/{key}/" + url = url_for("ExplorePermalinkView.permalink", key=key, _external=True) return self.response(201, key=key, url=url) except ValidationError as ex: return self.response(400, message=ex.messages) diff --git a/superset/sqllab/permalink/api.py b/superset/sqllab/permalink/api.py index c86fb99a5edb3..35eae9f9062f1 100644 --- a/superset/sqllab/permalink/api.py +++ b/superset/sqllab/permalink/api.py @@ -16,7 +16,7 @@ # under the License. import logging -from flask import request, Response +from flask import request, Response, url_for from flask_appbuilder.api import expose, protect, safe from marshmallow import ValidationError @@ -87,8 +87,7 @@ def post(self) -> Response: try: state = self.add_model_schema.load(request.json) key = CreateSqlLabPermalinkCommand(state=state).run() - http_origin = request.headers.environ.get("HTTP_ORIGIN") - url = f"{http_origin}/sqllab/p/{key}" + url = url_for("SqllabView.root", key=key, _external=True) return self.response(201, key=key, url=url) except ValidationError as ex: return self.response(400, message=ex.messages) @@ -104,10 +103,10 @@ def post(self) -> Response: log_to_statsd=False, ) def get(self, key: str) -> Response: - """Get chart's permanent link state. + """Get permanent link state for SQLLab editor. --- get: - summary: Get chart's permanent link state + summary: Get permanent link state for SQLLab editor. parameters: - in: path schema: From fc5dde15fe7f8135effaa3757f146530e1267412 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 23 Jan 2025 16:12:04 -0700 Subject: [PATCH 11/18] chore(build): enforce eslint rule banning antd imports outside of core Superset components (#31963) Co-authored-by: Maxime Beauchemin --- superset-frontend/.eslintrc.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index d2ed09e69e75b..5f5acaa6d64fb 100644 --- a/superset-frontend/.eslintrc.js +++ b/superset-frontend/.eslintrc.js @@ -331,7 +331,7 @@ module.exports = { 'no-prototype-builtins': 0, 'no-restricted-properties': 0, 'no-restricted-imports': [ - 'warn', + 'error', { paths: [ { @@ -339,6 +339,11 @@ module.exports = { message: 'Please import Ant components from the index of src/components', }, + { + name: 'antd-v5', + message: + 'Please import Ant v5 components from the index of src/components', + }, { name: '@superset-ui/core', importNames: ['supersetTheme'], From 51e090d67a914bf6692d8256de2d232dd7882666 Mon Sep 17 00:00:00 2001 From: mcdogg17 <63260972+mcdogg17@users.noreply.github.com> Date: Fri, 24 Jan 2025 04:14:12 +0500 Subject: [PATCH 12/18] fix(verbose map): Correct raw metrics handling in verbose map (#29417) --- superset/connectors/sqla/models.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 2acc7b12b6647..7d0570b92b81d 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -359,12 +359,15 @@ def order_by_choices(self) -> list[tuple[str, str]]: @property def verbose_map(self) -> dict[str, str]: verb_map = {"__timestamp": "Time"} - verb_map.update( - {o.metric_name: o.verbose_name or o.metric_name for o in self.metrics} - ) - verb_map.update( - {o.column_name: o.verbose_name or o.column_name for o in self.columns} - ) + + for o in self.metrics: + if o.metric_name not in verb_map: + verb_map[o.metric_name] = o.verbose_name or o.metric_name + + for o in self.columns: + if o.column_name not in verb_map: + verb_map[o.column_name] = o.verbose_name or o.column_name + return verb_map @property From 5fe6ef268ec5b136f911ccfe05fb8bdd7fc7a79f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 23 Jan 2025 18:31:45 -0700 Subject: [PATCH 13/18] chore(deps): bump react-transition-group and @types/react-transition-group in /superset-frontend (#31547) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- superset-frontend/package-lock.json | 59 +++++++++++++++++------------ superset-frontend/package.json | 4 +- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 1b06e003e47f0..f8878539a0670 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -125,7 +125,7 @@ "react-split": "^2.0.9", "react-syntax-highlighter": "^15.4.5", "react-table": "^7.8.0", - "react-transition-group": "^2.5.3", + "react-transition-group": "^4.4.5", "react-ultimate-pagination": "^1.3.2", "react-virtualized-auto-sizer": "^1.0.25", "react-window": "^1.8.10", @@ -203,7 +203,7 @@ "@types/react-router-dom": "^5.3.3", "@types/react-syntax-highlighter": "^15.5.13", "@types/react-table": "^7.7.20", - "@types/react-transition-group": "^4.4.10", + "@types/react-transition-group": "^4.4.12", "@types/react-ultimate-pagination": "^1.2.4", "@types/react-virtualized-auto-sizer": "^1.0.4", "@types/react-window": "^1.8.8", @@ -13817,10 +13817,11 @@ } }, "node_modules/@types/react-transition-group": { - "version": "4.4.10", + "version": "4.4.12", + "resolved": "https://registry.npmjs.org/@types/react-transition-group/-/react-transition-group-4.4.12.tgz", + "integrity": "sha512-8TV6R3h2j7a91c+1DXdJi3Syo69zzIZbz7Lg5tORM5LEJG7X/E6a1V3drRyBRZq7/utz7A+c4OgYLiLcYGHG6w==", "dev": true, - "license": "MIT", - "dependencies": { + "peerDependencies": { "@types/react": "*" } }, @@ -22498,10 +22499,12 @@ } }, "node_modules/dom-helpers": { - "version": "3.4.0", - "license": "MIT", + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.2.1.tgz", + "integrity": "sha512-nRCa7CK3VTrM2NmGkIy4cbK7IZlgBE/PYMn55rrXefr5xXDP0LdtfPnblFDoVdcAfslJ7or6iqAUnx0CCGIWQA==", "dependencies": { - "@babel/runtime": "^7.1.2" + "@babel/runtime": "^7.8.7", + "csstype": "^3.0.2" } }, "node_modules/dom-serializer": { @@ -45822,17 +45825,18 @@ } }, "node_modules/react-transition-group": { - "version": "2.5.3", - "license": "BSD-3-Clause", + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/react-transition-group/-/react-transition-group-4.4.5.tgz", + "integrity": "sha512-pZcd1MCJoiKiBR2NRxeCRg13uCXbydPnmB4EOeRrY7480qNWO8IIgQG6zlDkm6uRMsURXPuKq0GWtiM59a5Q6g==", "dependencies": { - "dom-helpers": "^3.3.1", + "@babel/runtime": "^7.5.5", + "dom-helpers": "^5.0.1", "loose-envify": "^1.4.0", - "prop-types": "^15.6.2", - "react-lifecycles-compat": "^3.0.4" + "prop-types": "^15.6.2" }, "peerDependencies": { - "react": ">=15.0.0", - "react-dom": ">=15.0.0" + "react": ">=16.6.0", + "react-dom": ">=16.6.0" } }, "node_modules/react-ultimate-pagination": { @@ -70024,11 +70028,11 @@ } }, "@types/react-transition-group": { - "version": "4.4.10", + "version": "4.4.12", + "resolved": "https://registry.npmjs.org/@types/react-transition-group/-/react-transition-group-4.4.12.tgz", + "integrity": "sha512-8TV6R3h2j7a91c+1DXdJi3Syo69zzIZbz7Lg5tORM5LEJG7X/E6a1V3drRyBRZq7/utz7A+c4OgYLiLcYGHG6w==", "dev": true, - "requires": { - "@types/react": "^16.9.53" - } + "requires": {} }, "@types/react-ultimate-pagination": { "version": "1.2.4", @@ -75959,9 +75963,12 @@ } }, "dom-helpers": { - "version": "3.4.0", + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.2.1.tgz", + "integrity": "sha512-nRCa7CK3VTrM2NmGkIy4cbK7IZlgBE/PYMn55rrXefr5xXDP0LdtfPnblFDoVdcAfslJ7or6iqAUnx0CCGIWQA==", "requires": { - "@babel/runtime": "^7.1.2" + "@babel/runtime": "^7.8.7", + "csstype": "^3.0.2" } }, "dom-serializer": { @@ -90556,12 +90563,14 @@ } }, "react-transition-group": { - "version": "2.5.3", + "version": "4.4.5", + "resolved": "https://registry.npmjs.org/react-transition-group/-/react-transition-group-4.4.5.tgz", + "integrity": "sha512-pZcd1MCJoiKiBR2NRxeCRg13uCXbydPnmB4EOeRrY7480qNWO8IIgQG6zlDkm6uRMsURXPuKq0GWtiM59a5Q6g==", "requires": { - "dom-helpers": "^3.3.1", + "@babel/runtime": "^7.5.5", + "dom-helpers": "^5.0.1", "loose-envify": "^1.4.0", - "prop-types": "^15.6.2", - "react-lifecycles-compat": "^3.0.4" + "prop-types": "^15.6.2" } }, "react-ultimate-pagination": { diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a71e64ac80f60..7b8aee659123d 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -192,7 +192,7 @@ "react-split": "^2.0.9", "react-syntax-highlighter": "^15.4.5", "react-table": "^7.8.0", - "react-transition-group": "^2.5.3", + "react-transition-group": "^4.4.5", "react-ultimate-pagination": "^1.3.2", "react-virtualized-auto-sizer": "^1.0.25", "react-window": "^1.8.10", @@ -270,7 +270,7 @@ "@types/react-router-dom": "^5.3.3", "@types/react-syntax-highlighter": "^15.5.13", "@types/react-table": "^7.7.20", - "@types/react-transition-group": "^4.4.10", + "@types/react-transition-group": "^4.4.12", "@types/react-ultimate-pagination": "^1.2.4", "@types/react-virtualized-auto-sizer": "^1.0.4", "@types/react-window": "^1.8.8", From 6eb87e04c0c0bfec5539f7459925b66790edab7f Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 24 Jan 2025 08:39:09 -0800 Subject: [PATCH 14/18] chore: refactor Alert-related components (#31858) --- .github/workflows/superset-e2e.yml | 2 +- .../cypress/e2e/dashboard/drillby.test.ts | 2 +- .../e2e/dashboard/drilltodetail.test.ts | 8 +- .../e2e/dashboard/horizontalFilterBar.test.ts | 6 +- .../cypress/e2e/dashboard_list/list.test.ts | 2 +- .../cypress/e2e/explore/annotations.test.ts | 2 +- .../cypress/e2e/sqllab/query.test.ts | 4 +- .../cypress-base/cypress/support/e2e.ts | 2 +- .../superset-ui-core/src/style/index.tsx | 7 - .../src/ReactCalendar.jsx | 8 +- .../src/components/Alert/Alert.stories.tsx | 11 - .../src/components/Alert/Alert.test.tsx | 20 +- .../src/components/Alert/index.tsx | 50 +-- .../src/components/AlteredSliceTag/index.tsx | 2 +- .../src/components/Chart/Chart.tsx | 20 +- .../components/Chart/ChartErrorMessage.tsx | 7 +- .../src/components/ErrorBoundary/index.tsx | 22 +- .../ErrorMessage/DatabaseErrorMessage.tsx | 28 +- .../DatasetNotFoundErrorMessage.tsx | 12 +- .../ErrorMessage/ErrorAlert.stories.tsx | 151 ++++++++ .../ErrorMessage/ErrorAlert.test.tsx | 231 ++++-------- .../components/ErrorMessage/ErrorAlert.tsx | 341 ++++++------------ .../ErrorMessageWithStackTrace.test.tsx | 2 +- .../ErrorMessageWithStackTrace.tsx | 37 +- .../FrontendNetworkErrorMessage.tsx | 33 ++ .../InvalidSQLErrorMessage.test.tsx | 135 +++---- .../ErrorMessage/InvalidSQLErrorMessage.tsx | 16 +- .../ErrorMessage/OAuth2RedirectMessage.tsx | 9 +- .../ParameterErrorMessage.test.tsx | 2 +- .../ErrorMessage/ParameterErrorMessage.tsx | 15 +- .../ErrorMessage/TimeoutErrorMessage.tsx | 15 +- .../src/components/Label/Label.stories.tsx | 1 - .../src/components/Label/index.tsx | 15 +- .../src/components/ListViewCard/index.tsx | 1 + .../WarningIconWithTooltip/index.tsx | 2 +- superset-frontend/src/components/index.ts | 1 + .../DashboardBuilder/DashboardWrapper.tsx | 2 +- .../src/explore/components/ControlHeader.tsx | 9 +- .../components/ControlPanelsContainer.tsx | 6 +- .../src/explore/components/ExploreAlert.tsx | 8 +- .../controls/ColorSchemeControl/index.tsx | 2 +- .../FormattingPopoverContent.tsx | 4 +- .../DatasourceControl.test.tsx | 2 +- .../controls/DatasourceControl/index.jsx | 44 +-- .../alerts/components/AlertStatusIcon.tsx | 4 +- .../src/features/dashboards/DashboardCard.tsx | 6 +- .../databases/DatabaseModal/index.test.tsx | 8 +- .../databases/DatabaseModal/index.tsx | 5 +- .../src/setup/setupErrorMessages.ts | 5 + superset-frontend/src/theme/index.ts | 9 - superset-frontend/src/views/App.tsx | 11 +- 51 files changed, 592 insertions(+), 755 deletions(-) create mode 100644 superset-frontend/src/components/ErrorMessage/ErrorAlert.stories.tsx create mode 100644 superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx diff --git a/.github/workflows/superset-e2e.yml b/.github/workflows/superset-e2e.yml index cbf9728f2381d..85a22bf11e917 100644 --- a/.github/workflows/superset-e2e.yml +++ b/.github/workflows/superset-e2e.yml @@ -141,4 +141,4 @@ jobs: if: failure() with: path: ${{ github.workspace }}/superset-frontend/cypress-base/cypress/screenshots - name: cypress-artifact-${{ github.run_id }}-${{ github.job }} + name: cypress-artifact-${{ github.run_id }}-${{ github.job }}-${{ matrix.browser }}-${{ matrix.parallel_id }} diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts index c4c5ed47665fb..e471d1da8caa2 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts @@ -599,7 +599,7 @@ describe('Drill by modal', () => { ]); }); - it('Radar Chart', () => { + it.skip('Radar Chart', () => { testEchart('radar', 'Radar Chart', [ [182, 49], [423, 91], diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/drilltodetail.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/drilltodetail.test.ts index f11aac445446b..4ebd64dd6e501 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/drilltodetail.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/drilltodetail.test.ts @@ -335,7 +335,7 @@ describe('Drill to detail modal', () => { }); }); - describe('Bar Chart', () => { + describe.skip('Bar Chart', () => { it('opens the modal with the correct filters', () => { interceptSamples(); @@ -373,7 +373,7 @@ describe('Drill to detail modal', () => { }); }); - describe('Area Chart', () => { + describe.skip('Area Chart', () => { it('opens the modal with the correct filters', () => { testTimeChart('echarts_area'); }); @@ -407,7 +407,7 @@ describe('Drill to detail modal', () => { }); }); - describe('World Map', () => { + describe.skip('World Map', () => { it('opens the modal with the correct filters', () => { interceptSamples(); @@ -567,7 +567,7 @@ describe('Drill to detail modal', () => { }); }); - describe('Radar Chart', () => { + describe.skip('Radar Chart', () => { it('opens the modal with the correct filters', () => { interceptSamples(); diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts index f1bfa9617e1c3..bcacae8a36dbc 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts @@ -176,7 +176,7 @@ describe('Horizontal FilterBar', () => { validateFilterNameOnDashboard(testItems.topTenChart.filterColumn); }); - it('should spot changes in "more filters" and apply their values', () => { + it.skip('should spot changes in "more filters" and apply their values', () => { cy.intercept(`/api/v1/chart/data?form_data=**`).as('chart'); prepareDashboardFilters([ { name: 'test_1', column: 'country_name', datasetId: 2 }, @@ -204,7 +204,7 @@ describe('Horizontal FilterBar', () => { ); }); - it('should focus filter and open "more filters" programmatically', () => { + it.skip('should focus filter and open "more filters" programmatically', () => { prepareDashboardFilters([ { name: 'test_1', column: 'country_name', datasetId: 2 }, { name: 'test_2', column: 'country_code', datasetId: 2 }, @@ -231,7 +231,7 @@ describe('Horizontal FilterBar', () => { cy.get('.ant-select-focused').should('be.visible'); }); - it('should show tag count and one plain tag on focus and only count on blur in select ', () => { + it.skip('should show tag count and one plain tag on focus and only count on blur in select ', () => { prepareDashboardFilters([ { name: 'test_1', column: 'country_name', datasetId: 2 }, ]); diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts index 77d0953edb667..c887ae0e6c2c3 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts @@ -206,7 +206,7 @@ describe('Dashboards list', () => { .should('not.contain', '4 - Sample dashboard'); }); - it('should delete correctly in list mode', () => { + it.skip('should delete correctly in list mode', () => { // deletes in list-view setGridMode('list'); diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/annotations.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/annotations.test.ts index ec1596e932008..b4f31723ab5fb 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/annotations.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/explore/annotations.test.ts @@ -18,7 +18,7 @@ */ import { interceptChart } from 'cypress/utils'; -describe('Annotations', () => { +describe.skip('Annotations', () => { beforeEach(() => { interceptChart({ legacy: false }).as('chartData'); }); diff --git a/superset-frontend/cypress-base/cypress/e2e/sqllab/query.test.ts b/superset-frontend/cypress-base/cypress/e2e/sqllab/query.test.ts index be758ed6dd2bd..a5898e27f3dcd 100644 --- a/superset-frontend/cypress-base/cypress/e2e/sqllab/query.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/sqllab/query.test.ts @@ -142,10 +142,11 @@ describe('SqlLab query panel', () => { }); }); - it('Create a chart from a query', () => { + it.skip('Create a chart from a query', () => { cy.intercept('/api/v1/sqllab/execute/').as('queryFinished'); cy.intercept('**/api/v1/explore/**').as('explore'); cy.intercept('**/api/v1/chart/**').as('chart'); + cy.intercept('**/tabstateview/**').as('tabstateview'); // cypress doesn't handle opening a new tab, override window.open to open in the same tab cy.window().then(win => { @@ -154,6 +155,7 @@ describe('SqlLab query panel', () => { win.location.href = url; }); }); + cy.wait('@tabstateview'); const query = 'SELECT gender, name FROM birth_names'; diff --git a/superset-frontend/cypress-base/cypress/support/e2e.ts b/superset-frontend/cypress-base/cypress/support/e2e.ts index 4a471c87d1b05..87229278b7ee5 100644 --- a/superset-frontend/cypress-base/cypress/support/e2e.ts +++ b/superset-frontend/cypress-base/cypress/support/e2e.ts @@ -169,7 +169,7 @@ Cypress.Commands.add('login', () => { }).then(response => { if (response.status === 302) { // If there's a redirect, follow it manually - const redirectUrl = response.headers['location']; + const redirectUrl = response.headers.location; cy.request({ method: 'GET', url: redirectUrl, diff --git a/superset-frontend/packages/superset-ui-core/src/style/index.tsx b/superset-frontend/packages/superset-ui-core/src/style/index.tsx index ee0b6e10ac419..c4964172be622 100644 --- a/superset-frontend/packages/superset-ui-core/src/style/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/style/index.tsx @@ -98,13 +98,6 @@ const defaultTheme = { light2: '#FAEDEE', }, warning: { - base: '#FF7F44', - dark1: '#BF5E33', - dark2: '#7F3F21', - light1: '#FEC0A1', - light2: '#FFF2EC', - }, - alert: { base: '#FCC700', dark1: '#BC9501', dark2: '#7D6300', diff --git a/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx b/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx index f3fcc807d8a04..46cb1de852ada 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx +++ b/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx @@ -157,13 +157,13 @@ export default styled(Calendar)` } .cal-heatmap-container .q1 { - background-color: ${theme.colors.alert.light2}; - fill: ${theme.colors.alert.light2}; + background-color: ${theme.colors.warning.light2}; + fill: ${theme.colors.warning.light2}; } .cal-heatmap-container .q2 { - background-color: ${theme.colors.alert.light1}; - fill: ${theme.colors.alert.light1}; + background-color: ${theme.colors.warning.light1}; + fill: ${theme.colors.warning.light1}; } .cal-heatmap-container .q3 { diff --git a/superset-frontend/src/components/Alert/Alert.stories.tsx b/superset-frontend/src/components/Alert/Alert.stories.tsx index 9aff2afee617b..649abaa16a95d 100644 --- a/superset-frontend/src/components/Alert/Alert.stories.tsx +++ b/superset-frontend/src/components/Alert/Alert.stories.tsx @@ -46,17 +46,6 @@ export const AlertGallery = () => ( message={smallText} description={bigText} closable - closeIcon={ - - x - - } /> diff --git a/superset-frontend/src/components/Alert/Alert.test.tsx b/superset-frontend/src/components/Alert/Alert.test.tsx index 89f221e0c5eb1..f7cb342a75d54 100644 --- a/superset-frontend/src/components/Alert/Alert.test.tsx +++ b/superset-frontend/src/components/Alert/Alert.test.tsx @@ -27,19 +27,17 @@ test('renders with default props', async () => { render(); expect(screen.getByRole('alert')).toHaveTextContent('Message'); - expect(await screen.findByLabelText('info icon')).toBeInTheDocument(); - expect(await screen.findByLabelText('close icon')).toBeInTheDocument(); + expect(screen.getByRole('img', { name: 'info-circle' })).toBeInTheDocument(); }); -test('renders each type', async () => { +test('renders message for each alert type', () => { const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success']; - await Promise.all( - types.map(async type => { - render(); - expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument(); - }), - ); + types.forEach(type => { + const { rerender } = render(); + expect(screen.getByText('Test message')).toBeInTheDocument(); + rerender(<>); // Clean up between renders + }); }); test('renders without close button', async () => { @@ -51,7 +49,7 @@ test('renders without close button', async () => { test('disappear when closed', async () => { render(); - userEvent.click(screen.getByLabelText('close icon')); + userEvent.click(screen.getByRole('img', { name: 'close' })); await waitFor(() => { expect(screen.queryByRole('alert')).not.toBeInTheDocument(); }); @@ -74,6 +72,6 @@ test('renders message and description', async () => { test('calls onClose callback when closed', () => { const onCloseMock = jest.fn(); render(); - userEvent.click(screen.getByLabelText('close icon')); + userEvent.click(screen.getByRole('img', { name: 'close' })); expect(onCloseMock).toHaveBeenCalledTimes(1); }); diff --git a/superset-frontend/src/components/Alert/index.tsx b/superset-frontend/src/components/Alert/index.tsx index 6a85739950fa3..b8fb872b6a24d 100644 --- a/superset-frontend/src/components/Alert/index.tsx +++ b/superset-frontend/src/components/Alert/index.tsx @@ -19,8 +19,6 @@ import { PropsWithChildren } from 'react'; import { Alert as AntdAlert } from 'antd-v5'; import { AlertProps as AntdAlertProps } from 'antd-v5/lib/alert'; -import { css, useTheme } from '@superset-ui/core'; -import Icons from 'src/components/Icons'; export type AlertProps = PropsWithChildren< Omit & { roomBelow?: boolean } @@ -32,60 +30,20 @@ export default function Alert(props: AlertProps) { description, showIcon = true, closable = true, - roomBelow = false, children, + ...rest } = props; - const theme = useTheme(); - const { colors } = theme; - const { alert: alertColor, error, info, success } = colors; - - let baseColor = info; - let AlertIcon = Icons.InfoSolid; - if (type === 'error') { - baseColor = error; - AlertIcon = Icons.ErrorSolid; - } else if (type === 'warning') { - baseColor = alertColor; - AlertIcon = Icons.AlertSolid; - } else if (type === 'success') { - baseColor = success; - AlertIcon = Icons.CircleCheckSolid; - } - return ( - - - ) - } - closeIcon={closable && } + closable={closable} message={children || 'Default message'} description={description} - css={css` - margin-bottom: ${roomBelow ? theme.gridUnit * 4 : 0}px; - a { - text-decoration: underline; - } - .antd5-alert-message { - font-weight: ${description - ? theme.typography.weights.bold - : 'inherit'}; - } - `} - {...props} + {...rest} /> ); } diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 9aca6b46b84df..46fc58b3ffef8 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -221,7 +221,7 @@ const AlteredSliceTag: FC = props => {