sadpandajoe commented on code in PR #38368:
URL: https://github.com/apache/superset/pull/38368#discussion_r2881379523
##########
superset-frontend/src/pages/DashboardList/DashboardList.test.tsx:
##########
@@ -16,233 +16,290 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
-import { MemoryRouter } from 'react-router-dom';
import fetchMock from 'fetch-mock';
import { isFeatureEnabled } from '@superset-ui/core';
import {
- render,
screen,
+ selectOption,
waitFor,
fireEvent,
} from 'spec/helpers/testing-library';
-import { QueryParamProvider } from 'use-query-params';
-import { ReactRouter5Adapter } from 'use-query-params/adapters/react-router-5';
-
-import DashboardListComponent from 'src/pages/DashboardList';
-
-// Cast to accept partial mock props in tests
-const DashboardList = DashboardListComponent as unknown as React.FC<
- Record<string, any>
->;
+import {
+ mockDashboards,
+ mockAdminUser,
+ setupMocks,
+ renderDashboardList,
+ API_ENDPOINTS,
+ getLatestDashboardApiCall,
+} from './DashboardList.testHelpers';
-const dashboardsInfoEndpoint = 'glob:*/api/v1/dashboard/_info*';
-const dashboardOwnersEndpoint = 'glob:*/api/v1/dashboard/related/owners*';
-const dashboardCreatedByEndpoint =
- 'glob:*/api/v1/dashboard/related/created_by*';
-const dashboardFavoriteStatusEndpoint =
- 'glob:*/api/v1/dashboard/favorite_status*';
-const dashboardsEndpoint = 'glob:*/api/v1/dashboard/?*';
-const dashboardEndpoint = 'glob:*/api/v1/dashboard/*';
+jest.setTimeout(30000);
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-const mockDashboards = Array.from({ length: 3 }, (_, i) => ({
- id: i,
- url: 'url',
- dashboard_title: `title ${i}`,
- changed_by_name: 'user',
- changed_by_fk: 1,
- published: true,
- changed_on_utc: new Date().toISOString(),
- changed_on_delta_humanized: '5 minutes ago',
- owners: [{ id: 1, first_name: 'admin', last_name: 'admin_user' }],
- roles: [{ id: 1, name: 'adminUser' }],
- thumbnail_url: '/thumbnail',
+jest.mock('src/utils/export', () => ({
+ __esModule: true,
+ default: jest.fn(),
}));
-const mockUser = {
- userId: 1,
-};
+const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
+ typeof isFeatureEnabled
+>;
-fetchMock.get(dashboardsInfoEndpoint, {
- permissions: ['can_read', 'can_write'],
+beforeEach(() => {
+ setupMocks();
+ mockIsFeatureEnabled.mockImplementation(
+ (feature: string) => feature === 'LISTVIEWS_DEFAULT_CARD_VIEW',
+ );
});
-fetchMock.get(dashboardOwnersEndpoint, {
- result: [],
+
+afterEach(() => {
+ fetchMock.clearHistory().removeRoutes();
+ mockIsFeatureEnabled.mockReset();
});
-fetchMock.get(dashboardCreatedByEndpoint, {
- result: [],
+
+test('renders', async () => {
+ renderDashboardList(mockAdminUser);
+ expect(await screen.findByText('Dashboards')).toBeInTheDocument();
});
-fetchMock.get(dashboardFavoriteStatusEndpoint, {
- result: [],
+
+test('renders a ListView', async () => {
+ renderDashboardList(mockAdminUser);
+ expect(await screen.findByTestId('dashboard-list-view')).toBeInTheDocument();
});
-fetchMock.get(dashboardsEndpoint, {
- result: mockDashboards,
- dashboard_count: 3,
+
+test('fetches info', async () => {
+ renderDashboardList(mockAdminUser);
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls(/dashboard\/_info/);
+ expect(calls).toHaveLength(1);
+ });
});
-fetchMock.get(dashboardEndpoint, {
- result: mockDashboards[0],
+
+test('fetches data', async () => {
+ renderDashboardList(mockAdminUser);
+ await waitFor(() => {
+ const calls = fetchMock.callHistory.calls(/dashboard\/\?q/);
+ expect(calls).toHaveLength(1);
+ });
+
+ const calls = fetchMock.callHistory.calls(/dashboard\/\?q/);
+ expect(calls[0].url).toMatchInlineSnapshot(
+
`"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25,select_columns:!(id,dashboard_title,published,url,slug,changed_by,changed_by.id,changed_by.first_name,changed_by.last_name,changed_on_delta_humanized,owners,owners.id,owners.first_name,owners.last_name,tags.id,tags.name,tags.type,status,certified_by,certification_details,changed_on))"`,
+ );
});
-global.URL.createObjectURL = jest.fn();
-fetchMock.get('/thumbnail', { body: new Blob(), sendAsJson: false });
-
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DashboardList', () => {
- const renderDashboardList = (props = {}, userProp = mockUser) =>
- render(
- <MemoryRouter>
- <QueryParamProvider adapter={ReactRouter5Adapter}>
- <DashboardList {...props} user={userProp} />
- </QueryParamProvider>
- </MemoryRouter>,
- { useRedux: true },
- );
+test('switches between card and table view', async () => {
+ renderDashboardList(mockAdminUser);
- beforeEach(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(
- (feature: string) => feature === 'LISTVIEWS_DEFAULT_CARD_VIEW',
- );
- fetchMock.clearHistory();
- });
+ // Wait for the list to load
+ await screen.findByTestId('dashboard-list-view');
- afterEach(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
- });
+ // Initially in card view (no table)
+ expect(screen.queryByTestId('listview-table')).not.toBeInTheDocument();
- test('renders', async () => {
- renderDashboardList();
- expect(await screen.findByText('Dashboards')).toBeInTheDocument();
- });
+ // Switch to table view via the list icon
+ const listViewIcon = screen.getByRole('img', { name: 'unordered-list' });
+ const listViewButton = listViewIcon.closest('[role="button"]')!;
+ fireEvent.click(listViewButton);
- test('renders a ListView', async () => {
- renderDashboardList();
- expect(
- await screen.findByTestId('dashboard-list-view'),
- ).toBeInTheDocument();
+ await waitFor(() => {
+ expect(screen.getByTestId('listview-table')).toBeInTheDocument();
});
- test('fetches info', async () => {
- renderDashboardList();
- await waitFor(() => {
- const calls = fetchMock.callHistory.calls(/dashboard\/_info/);
- expect(calls).toHaveLength(1);
- });
+ // Switch back to card view
+ const cardViewIcon = screen.getByRole('img', { name: 'appstore' });
+ const cardViewButton = cardViewIcon.closest('[role="button"]')!;
+ fireEvent.click(cardViewButton);
+
+ await waitFor(() => {
+ expect(screen.queryByTestId('listview-table')).not.toBeInTheDocument();
});
+});
- test('fetches data', async () => {
- renderDashboardList();
- await waitFor(() => {
- const calls = fetchMock.callHistory.calls(/dashboard\/\?q/);
- expect(calls).toHaveLength(1);
- });
+test('shows edit modal', async () => {
+ renderDashboardList(mockAdminUser);
- const calls = fetchMock.callHistory.calls(/dashboard\/\?q/);
- expect(calls[0].url).toMatchInlineSnapshot(
-
`"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25,select_columns:!(id,dashboard_title,published,url,slug,changed_by,changed_by.id,changed_by.first_name,changed_by.last_name,changed_on_delta_humanized,owners,owners.id,owners.first_name,owners.last_name,tags.id,tags.name,tags.type,status,certified_by,certification_details,changed_on))"`,
- );
+ // Wait for data to load
+ await screen.findByText(mockDashboards[0].dashboard_title);
+
+ // Find and click the first more options button
+ const moreIcons = await screen.findAllByRole('img', {
+ name: 'more',
});
+ fireEvent.click(moreIcons[0]);
- test('switches between card and table view', async () => {
- renderDashboardList();
+ // Click edit from the dropdown
+ const editButton = await screen.findByTestId(
+ 'dashboard-card-option-edit-button',
+ );
+ fireEvent.click(editButton);
- // Wait for the list to load
- await screen.findByTestId('dashboard-list-view');
+ // Check for modal
+ expect(await screen.findByRole('dialog')).toBeInTheDocument();
+});
- // Initially in card view
- const cardViewIcon = screen.getByRole('img', { name: 'appstore' });
- expect(cardViewIcon).toBeInTheDocument();
+test('shows delete confirmation', async () => {
+ renderDashboardList(mockAdminUser);
- // Switch to table view
- const listViewIcon = screen.getByRole('img', { name: 'appstore' });
- const listViewButton = listViewIcon.closest('[role="button"]')!;
- fireEvent.click(listViewButton);
+ // Wait for data to load
+ await screen.findByText(mockDashboards[0].dashboard_title);
- // Switch back to card view
- const cardViewButton = cardViewIcon.closest('[role="button"]')!;
- fireEvent.click(cardViewButton);
+ // Find and click the first more options button
+ const moreIcons = await screen.findAllByRole('img', {
+ name: 'more',
});
+ fireEvent.click(moreIcons[0]);
+
+ // Click delete from the dropdown
+ const deleteButton = await screen.findByTestId(
+ 'dashboard-card-option-delete-button',
+ );
+ fireEvent.click(deleteButton);
+
+ // Check for confirmation dialog
+ expect(
+ await screen.findByText(/Are you sure you want to delete/i),
+ ).toBeInTheDocument();
+});
- test('shows edit modal', async () => {
- renderDashboardList();
-
- // Wait for data to load
- await screen.findByText('title 0');
+test('renders an "Import Dashboard" tooltip', async () => {
+ renderDashboardList(mockAdminUser);
- // Find and click the first more options button
- const moreIcons = await screen.findAllByRole('img', {
- name: 'more',
- });
- fireEvent.click(moreIcons[0]);
+ const importButton = await screen.findByTestId('import-button');
+ fireEvent.mouseOver(importButton);
- // Click edit from the dropdown
- const editButton = await screen.findByTestId(
- 'dashboard-card-option-edit-button',
- );
- fireEvent.click(editButton);
+ expect(
+ await screen.findByRole('tooltip', {
+ name: 'Import dashboards',
+ }),
+ ).toBeInTheDocument();
Review Comment:
This test passes as-is — antd Tooltip does expose its content as the
accessible name via `aria-label` on the tooltip element. Verified by running
the test: `npx jest --testNamePattern="Import Dashboard.*tooltip"` → PASS.
No change needed.
##########
superset-frontend/src/pages/DashboardList/DashboardList.testHelpers.tsx:
##########
@@ -0,0 +1,360 @@
+/**
+ * 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.
+ */
+// eslint-disable-next-line import/no-extraneous-dependencies
+import fetchMock from 'fetch-mock';
+import rison from 'rison';
+import { render, screen } from 'spec/helpers/testing-library';
+import { Provider } from 'react-redux';
+import { MemoryRouter } from 'react-router-dom';
+import { configureStore } from '@reduxjs/toolkit';
+import { QueryParamProvider } from 'use-query-params';
+import { ReactRouter5Adapter } from 'use-query-params/adapters/react-router-5';
+import DashboardListComponent from 'src/pages/DashboardList';
+import handleResourceExport from 'src/utils/export';
+
+// Cast to accept partial mock props in tests
+const DashboardList = DashboardListComponent as unknown as React.FC<
+ Record<string, any>
+>;
+
+export const mockHandleResourceExport =
+ handleResourceExport as jest.MockedFunction<typeof handleResourceExport>;
+
+export const mockDashboards = [
+ {
+ id: 1,
+ url: '/superset/dashboard/1/',
+ dashboard_title: 'Sales Dashboard',
+ published: true,
+ changed_by_name: 'admin',
+ changed_by_fk: 1,
+ changed_by: {
+ first_name: 'Admin',
+ last_name: 'User',
+ id: 1,
+ },
+ changed_on_utc: new Date().toISOString(),
+ changed_on_delta_humanized: '1 day ago',
+ owners: [{ id: 1, first_name: 'Admin', last_name: 'User' }],
+ roles: [{ id: 1, name: 'Admin' }],
+ tags: [{ id: 1, name: 'production', type: 'TagTypes.custom' }],
+ thumbnail_url: '/thumbnail',
+ certified_by: 'Data Team',
+ certification_details: 'Approved for production use',
+ status: 'published',
+ },
+ {
+ id: 2,
+ url: '/superset/dashboard/2/',
+ dashboard_title: 'Analytics Dashboard',
+ published: false,
+ changed_by_name: 'analyst',
+ changed_by_fk: 2,
+ changed_by: {
+ first_name: 'Data',
+ last_name: 'Analyst',
+ id: 2,
+ },
+ changed_on_utc: new Date().toISOString(),
+ changed_on_delta_humanized: '2 days ago',
+ owners: [
+ { id: 1, first_name: 'Admin', last_name: 'User' },
+ { id: 2, first_name: 'Data', last_name: 'Analyst' },
+ ],
+ roles: [],
+ tags: [],
+ thumbnail_url: '/thumbnail',
+ certified_by: null,
+ certification_details: null,
+ status: 'draft',
+ },
+ {
+ id: 3,
+ url: '/superset/dashboard/3/',
+ dashboard_title: 'Executive Overview',
+ published: true,
+ changed_by_name: 'admin',
+ changed_by_fk: 1,
+ changed_by: {
+ first_name: 'Admin',
+ last_name: 'User',
+ id: 1,
+ },
+ changed_on_utc: new Date().toISOString(),
+ changed_on_delta_humanized: '3 days ago',
+ owners: [],
+ roles: [{ id: 2, name: 'Alpha' }],
+ tags: [
+ { id: 2, name: 'executive', type: 'TagTypes.custom' },
+ { id: 3, name: 'quarterly', type: 'TagTypes.custom' },
+ ],
+ thumbnail_url: '/thumbnail',
+ certified_by: 'QA Team',
+ certification_details: 'Verified for executive use',
+ status: 'published',
+ },
+ {
+ id: 4,
+ url: '/superset/dashboard/4/',
+ dashboard_title: 'Marketing Metrics',
+ published: false,
+ changed_by_name: 'marketing',
+ changed_by_fk: 3,
+ changed_by: {
+ first_name: 'Marketing',
+ last_name: 'Lead',
+ id: 3,
+ },
+ changed_on_utc: new Date().toISOString(),
+ changed_on_delta_humanized: '5 days ago',
+ owners: [{ id: 3, first_name: 'Marketing', last_name: 'Lead' }],
+ roles: [],
+ tags: [],
+ thumbnail_url: '/thumbnail',
+ certified_by: null,
+ certification_details: null,
+ status: 'draft',
+ },
+ {
+ id: 5,
+ url: '/superset/dashboard/5/',
+ dashboard_title: 'Ops Monitor',
+ published: true,
+ changed_by_name: 'ops',
+ changed_by_fk: 4,
+ changed_by: {
+ first_name: 'Ops',
+ last_name: 'Engineer',
+ id: 4,
+ },
+ changed_on_utc: new Date().toISOString(),
+ changed_on_delta_humanized: '1 week ago',
+ owners: [
+ { id: 4, first_name: 'Ops', last_name: 'Engineer' },
+ { id: 1, first_name: 'Admin', last_name: 'User' },
+ ],
+ roles: [],
+ tags: [{ id: 4, name: 'monitoring', type: 'TagTypes.custom' }],
+ thumbnail_url: '/thumbnail',
+ certified_by: null,
+ certification_details: null,
+ status: 'published',
+ },
+];
+
+// Mock users with various permission levels
+export const mockAdminUser = {
+ userId: 1,
+ firstName: 'Admin',
+ lastName: 'User',
+ roles: {
+ Admin: [
+ ['can_write', 'Dashboard'],
+ ['can_export', 'Dashboard'],
+ ['can_read', 'Tag'],
+ ],
+ },
+};
+
+export const mockReadOnlyUser = {
+ userId: 10,
+ firstName: 'Read',
+ lastName: 'Only',
+ roles: {
+ Gamma: [['can_read', 'Dashboard']],
+ },
+};
+
+export const mockExportOnlyUser = {
+ userId: 11,
+ firstName: 'Export',
+ lastName: 'User',
+ roles: {
+ Gamma: [
+ ['can_read', 'Dashboard'],
+ ['can_export', 'Dashboard'],
+ ],
+ },
+};
+
+// API endpoint constants
+export const API_ENDPOINTS = {
+ DASHBOARDS_INFO: 'glob:*/api/v1/dashboard/_info*',
+ DASHBOARDS: 'glob:*/api/v1/dashboard/?*',
+ DASHBOARD_GET: 'glob:*/api/v1/dashboard/*',
+ DASHBOARD_FAVORITE_STATUS: 'glob:*/api/v1/dashboard/favorite_status*',
+ DASHBOARD_RELATED_OWNERS: 'glob:*/api/v1/dashboard/related/owners*',
+ DASHBOARD_RELATED_CHANGED_BY: 'glob:*/api/v1/dashboard/related/changed_by*',
+ THUMBNAIL: '/thumbnail',
+ CATCH_ALL: 'glob:*',
+};
+
+interface StoreState {
+ user?: any;
+ common?: {
+ conf?: {
+ SUPERSET_WEBSERVER_TIMEOUT?: number;
+ };
+ };
+ dashboards?: {
+ dashboardList?: typeof mockDashboards;
+ };
+}
+
+export const createMockStore = (initialState: Partial<StoreState> = {}) =>
+ configureStore({
+ reducer: {
+ user: (state = initialState.user || {}) => state,
+ common: (state = initialState.common || {}) => state,
+ dashboards: (state = initialState.dashboards || {}) => state,
+ },
+ preloadedState: initialState,
+ middleware: getDefaultMiddleware =>
+ getDefaultMiddleware({
+ serializableCheck: false,
+ immutableCheck: false,
+ }),
+ });
+
+export const createDefaultStoreState = (user: any): StoreState => ({
+ user,
+ common: {
+ conf: {
+ SUPERSET_WEBSERVER_TIMEOUT: 60000,
+ },
+ },
+ dashboards: {
+ dashboardList: mockDashboards,
+ },
+});
+
+export const renderDashboardList = (
+ user: any,
+ props: Record<string, any> = {},
+ storeState: Partial<StoreState> = {},
+) => {
+ const defaultStoreState = createDefaultStoreState(user);
+ const storeStateWithUser = {
+ ...defaultStoreState,
+ user,
+ ...storeState,
+ };
+
+ const store = createMockStore(storeStateWithUser);
+
+ return render(
+ <Provider store={store}>
+ <MemoryRouter>
+ <QueryParamProvider adapter={ReactRouter5Adapter}>
+ <DashboardList user={user} {...props} />
+ </QueryParamProvider>
+ </MemoryRouter>
+ </Provider>,
+ );
+};
+
+/**
+ * Helper to wait for the DashboardList page to be ready
+ * Waits for the "Dashboards" heading to appear, indicating initial render is
complete
+ */
+export const waitForDashboardsPageReady = async () => {
+ await screen.findByText('Dashboards');
+};
+
+export const setupMocks = (
+ payloadMap: Record<string, string[]> = {
+ [API_ENDPOINTS.DASHBOARDS_INFO]: ['can_read', 'can_write', 'can_export'],
+ },
+) => {
+ fetchMock.get(
+ API_ENDPOINTS.DASHBOARDS_INFO,
+ {
+ permissions: payloadMap[API_ENDPOINTS.DASHBOARDS_INFO],
+ },
+ { name: API_ENDPOINTS.DASHBOARDS_INFO },
+ );
+
+ fetchMock.get(
+ API_ENDPOINTS.DASHBOARDS,
+ {
+ result: mockDashboards,
+ dashboard_count: mockDashboards.length,
+ },
+ { name: API_ENDPOINTS.DASHBOARDS },
+ );
+
+ fetchMock.get(
+ API_ENDPOINTS.DASHBOARD_FAVORITE_STATUS,
+ { result: [] },
+ { name: API_ENDPOINTS.DASHBOARD_FAVORITE_STATUS },
+ );
+
+ fetchMock.get(
+ API_ENDPOINTS.DASHBOARD_RELATED_OWNERS,
+ { result: [], count: 0 },
+ { name: API_ENDPOINTS.DASHBOARD_RELATED_OWNERS },
+ );
+
+ fetchMock.get(
+ API_ENDPOINTS.DASHBOARD_RELATED_CHANGED_BY,
+ { result: [], count: 0 },
+ { name: API_ENDPOINTS.DASHBOARD_RELATED_CHANGED_BY },
+ );
+
+ global.URL.createObjectURL = jest.fn();
+ fetchMock.get(
+ API_ENDPOINTS.THUMBNAIL,
+ { body: new Blob(), sendAsJson: false },
+ { name: API_ENDPOINTS.THUMBNAIL },
+ );
+
+ fetchMock.get(
+ API_ENDPOINTS.CATCH_ALL,
+ (callLog: any) => {
+ const reqUrl =
+ typeof callLog === 'string' ? callLog : callLog?.url || callLog;
+ throw new Error(`[fetchMock catch-all] Unmatched GET: ${reqUrl}`);
+ },
+ { name: API_ENDPOINTS.CATCH_ALL },
+ );
+};
+
+/**
+ * Parse the rison-encoded `q` query parameter from a fetch-mock call URL.
+ * Returns the decoded object, or null if parsing fails.
+ */
+export const parseQueryFromUrl = (url: string): Record<string, any> | null => {
+ const match = url.match(/[?&]q=(.+?)(?:&|$)/);
+ if (!match) return null;
+ return rison.decode(decodeURIComponent(match[1]));
Review Comment:
This is a test-only helper called exclusively with URLs from fetch-mock call
history — always valid rison produced by the component's own API calls. If
`rison.decode` throws, that's a useful test failure signal (means the component
sent a malformed query). Adding try/catch would silently mask bad data.
The "null if parsing fails" docstring refers to the regex not matching (no
`q` param), not decode errors. No change needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
