[pull] master from apache:master

superset-frontend/playwright/components/modals/ConfirmDialog.ts

@@ -22,7 +22,7 @@ import { Modal } from '../core/Modal';
 
 /**
  * Confirm Dialog component for Ant Design Modal.confirm dialogs.
- * These are the "OK" / "Cancel" confirmation dialogs used throughout Superset.
+ * These are the "Confirm" / "Cancel" confirmation dialogs used throughout Superset.
  * Uses getByRole with name to target specific confirm dialogs when multiple are open.
  */
 export class ConfirmDialog extends Modal {
@@ -43,7 +43,7 @@ export class ConfirmDialog extends Modal {
   }
 
   /**
-   * Clicks the OK button to confirm.
+   * Clicks the Confirm button to confirm.
    * @param options.timeout - If provided, silently returns if dialog doesn't appear
    *                          within timeout. If not provided, waits indefinitely (strict mode).
    */
@@ -53,7 +53,7 @@ export class ConfirmDialog extends Modal {
         state: 'visible',
         timeout: options?.timeout,
       });
-      await this.clickFooterButton('OK');
+      await this.clickFooterButton('Confirm');
       await this.waitForHidden();
     } catch (error) {
       // Only swallow TimeoutError when timeout was explicitly provided

superset-frontend/src/SqlLab/components/ResultSet/index.tsx

@@ -390,7 +390,7 @@ const ResultSet = ({
               // provides.
               redirect(getExportCsvUrl(query.id));
             },
-            confirmText: t('OK'),
+            confirmText: t('Confirm'),
             cancelText: t('Close'),
           });
         }

superset-frontend/src/components/Datasource/DatasourceModal/DatasourceModal.test.tsx

@@ -120,7 +120,7 @@ describe('DatasourceModal', () => {
     });
     const saveButton = screen.getByTestId('datasource-modal-save');
     fireEvent.click(saveButton);
-    const okButton = await screen.findByRole('button', { name: 'OK' });
+    const okButton = await screen.findByRole('button', { name: 'Confirm' });
     fireEvent.click(okButton);
     await waitFor(() => {
       expect(onDatasourceSave).toHaveBeenCalled();
@@ -142,7 +142,7 @@ describe('DatasourceModal', () => {
 
     const saveButton = screen.getByTestId('datasource-modal-save');
     fireEvent.click(saveButton);
-    const okButton = await screen.findByRole('button', { name: 'OK' });
+    const okButton = await screen.findByRole('button', { name: 'Confirm' });
     fireEvent.click(okButton);
 
     const errorElements = await screen.findAllByText('Error saving dataset');
@@ -230,7 +230,7 @@ describe('DatasourceModal', () => {
     expect(checkbox).toBeChecked();
 
     // Click OK to submit
-    const okButton = screen.getByRole('button', { name: 'OK' });
+    const okButton = screen.getByRole('button', { name: 'Confirm' });
     fireEvent.click(okButton);
 
     // Verify the PUT request was made with override_columns=true
@@ -297,7 +297,7 @@ describe('DatasourceModal', () => {
     expect(checkbox).not.toBeChecked();
 
     // Click OK to submit
-    const okButton = screen.getByRole('button', { name: 'OK' });
+    const okButton = screen.getByRole('button', { name: 'Confirm' });
     fireEvent.click(okButton);
 
     // Verify the PUT request was made with override_columns=false

superset-frontend/src/components/Datasource/DatasourceModal/index.tsx

@@ -395,7 +395,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
         show={confirmModalOpen}
         onHide={handleConfirmModalClose}
         onHandledPrimaryAction={handleConfirmSave}
-        primaryButtonName={t('OK')}
+        primaryButtonName={t('Confirm')}
         primaryButtonLoading={isSaving}
       >
         {getSaveDialog()}

superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx

@@ -150,7 +150,7 @@ const waitForRender = (props?: any) =>
 test('renders with default props', async () => {
   await waitForRender();
   expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
-  expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
+  expect(screen.getByRole('button', { name: 'Confirm' })).toBeDisabled();
   expect(screen.getByRole('button', { name: 'Cancel' })).toBeEnabled();
 });
 
@@ -188,7 +188,7 @@ test('enables apply and ok buttons', async () => {
 
   await waitFor(() => {
     expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
-    expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
+    expect(screen.getByRole('button', { name: 'Confirm' })).toBeEnabled();
   });
 });
 
@@ -203,7 +203,7 @@ test('triggers addAnnotationLayer and close when ok button is clicked', async ()
   const addAnnotationLayer = jest.fn();
   const close = jest.fn();
   await waitForRender({ name: 'Test', value: '2x', addAnnotationLayer, close });
-  userEvent.click(screen.getByRole('button', { name: 'OK' }));
+  userEvent.click(screen.getByRole('button', { name: 'Confirm' }));
   expect(addAnnotationLayer).toHaveBeenCalled();
   expect(close).toHaveBeenCalled();
 });
@@ -724,7 +724,7 @@ test('Disable apply button if formula is incorrect', async () => {
 
   const formulaInput = screen.getByRole('textbox', { name: 'Formula' });
   const applyButton = screen.getByRole('button', { name: 'Apply' });
-  const okButton = screen.getByRole('button', { name: 'OK' });
+  const okButton = screen.getByRole('button', { name: 'Confirm' });
 
   userEvent.type(formulaInput, 'x+1');
   expect(formulaInput).toHaveValue('x+1');

superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.tsx

@@ -1303,7 +1303,7 @@ function AnnotationLayer({
             disabled={!isValid}
             onClick={submitAnnotation}
           >
-            {t('OK')}
+            {t('Confirm')}
           </Button>
         </div>
       </div>

superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx

@@ -187,7 +187,7 @@ async function openAndSaveChanges(
   await userEvent.click(screen.getByTestId('datasource-menu-trigger'));
   await userEvent.click(await screen.findByTestId('edit-dataset'));
   await userEvent.click(await screen.findByTestId('datasource-modal-save'));
-  await userEvent.click(await screen.findByText('OK'));
+  await userEvent.click(await screen.findByText('Confirm'));
 }
 
 test('Should render', async () => {
@@ -714,10 +714,10 @@ test('should handle metric save confirmation modal', async () => {
   await userEvent.click(await screen.findByTestId('datasource-modal-save'));
 
   // Verify confirmation modal appears
-  expect(await screen.findByText('OK')).toBeInTheDocument();
+  expect(await screen.findByText('Confirm')).toBeInTheDocument();
 
   // Confirm save
-  await userEvent.click(screen.getByText('OK'));
+  await userEvent.click(screen.getByText('Confirm'));
 
   await waitFor(() => {
     expect(props.onDatasourceSave).toHaveBeenCalled();

superset-frontend/src/features/charts/ChartCard.test.tsx

@@ -0,0 +1,108 @@
+/**
+ * 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 { createMemoryHistory, type Update } from 'history';
+import { Router } from 'react-router-dom';
+import { isFeatureEnabled } from '@superset-ui/core';
+import { render, screen, fireEvent } from 'spec/helpers/testing-library';
+import type Chart from 'src/types/Chart';
+import ChartCard from './ChartCard';
+
+jest.mock('@superset-ui/core', () => ({
+  ...jest.requireActual('@superset-ui/core'),
+  isFeatureEnabled: jest.fn(),
+}));
+
+const mockChart = {
+  id: 1,
+  slice_name: 'Sample Chart',
+  url: '/explore/?slice_id=1',
+  changed_on_delta_humanized: '2 days ago',
+  datasource_name_text: 'Sample dataset',
+  thumbnail_url: '/thumbnail.png',
+} as Chart;
+
+const renderCard = (history: ReturnType<typeof createMemoryHistory>) =>
+  render(
+    <Router history={history}>
+      <ChartCard
+        chart={mockChart}
+        hasPerm={() => true}
+        openChartEditModal={jest.fn()}
+        bulkSelectEnabled={false}
+        addDangerToast={jest.fn()}
+        addSuccessToast={jest.fn()}
+        refreshData={jest.fn()}
+        saveFavoriteStatus={jest.fn()}
+        favoriteStatus={false}
+        showThumbnails
+        handleBulkChartExport={jest.fn()}
+      />
+    </Router>,
+  );
+
+const recordNavigations = (
+  history: ReturnType<typeof createMemoryHistory>,
+): string[] => {
+  const navigations: string[] = [];
+  history.listen(({ action, location }: Update) =>
+    navigations.push(`${action} ${location.pathname}${location.search}`),
+  );
+  return navigations;
+};
+
+beforeEach(() => {
+  (isFeatureEnabled as jest.Mock).mockReturnValue(true);
+});
+
+afterEach(() => {
+  (isFeatureEnabled as jest.Mock).mockReset();
+});
+
+test('renders the chart title', () => {
+  renderCard(createMemoryHistory());
+  expect(screen.getByText('Sample Chart')).toBeInTheDocument();
+});
+
+test('clicking the thumbnail navigates to the chart exactly once', () => {
+  // The cover is a router link and the whole card is clickable, so a click on
+  // the cover used to be handled twice and pushed two identical entries. That
+  // left the Back button popping the duplicate instead of returning the user to
+  // the page they came from.
+  const history = createMemoryHistory({
+    initialEntries: ['/superset/welcome/'],
+  });
+  renderCard(history);
+  const navigations = recordNavigations(history);
+
+  fireEvent.click(screen.getByRole('link'));
+
+  expect(navigations).toEqual(['PUSH /explore/?slice_id=1']);
+});
+
+test('clicking the card outside the thumbnail navigates to the chart', () => {
+  const history = createMemoryHistory({
+    initialEntries: ['/superset/welcome/'],
+  });
+  renderCard(history);
+  const navigations = recordNavigations(history);
+
+  fireEvent.click(screen.getByText('Sample Chart'));
+
+  expect(navigations).toEqual(['PUSH /explore/?slice_id=1']);
+});

superset-frontend/src/features/charts/ChartCard.tsx

@@ -32,7 +32,11 @@ import {
 import Chart from 'src/types/Chart';
 import { SubjectPile } from 'src/features/subjects/SubjectPile';
 import { KebabMenuButton } from 'src/components';
-import { handleChartDelete, CardStyles } from 'src/views/CRUD/utils';
+import {
+  handleChartDelete,
+  CardStyles,
+  isNavigationHandledByLink,
+} from 'src/views/CRUD/utils';
 import { assetUrl } from 'src/utils/assetUrl';
 import type { ListViewFetchDataConfig as FetchDataConfig } from 'src/components';
 import { TableTab } from 'src/views/CRUD/types';
@@ -208,8 +212,12 @@ export default function ChartCard({
 
   return (
     <CardStyles
-      onClick={() => {
-        if (!bulkSelectEnabled && chart.url) {
+      onClick={event => {
+        if (
+          !bulkSelectEnabled &&
+          chart.url &&
+          !isNavigationHandledByLink(event)
+        ) {
           history.push(chart.url);
         }
       }}

superset-frontend/src/features/dashboards/DashboardCard.test.tsx

@@ -17,10 +17,16 @@
  * under the License.
  */
 
-import { MemoryRouter } from 'react-router-dom';
+import { createMemoryHistory, type Update } from 'history';
+import { MemoryRouter, Router } from 'react-router-dom';
 import { isFeatureEnabled } from '@superset-ui/core';
 
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+  render,
+  screen,
+  fireEvent,
+  within,
+} from 'spec/helpers/testing-library';
 import { SubjectType } from 'src/types/Subject';
 
 import DashboardCard from './DashboardCard';
@@ -63,6 +69,10 @@ afterAll(() => {
   mockedIsFeatureEnabled.mockClear();
 });
 
+afterEach(() => {
+  jest.restoreAllMocks();
+});
+
 beforeEach(() => {
   render(
     <MemoryRouter>
@@ -101,6 +111,43 @@ test('Renders the modified date', () => {
   expect(modifiedDateElement).toBeInTheDocument();
 });
 
+test('clicking the thumbnail navigates to the dashboard exactly once', () => {
+  // The cover is a router link and the whole card is clickable, so a click on
+  // the cover used to be handled twice and pushed two identical entries, which
+  // left the Back button popping the duplicate rather than returning the user
+  // to the page they came from.
+  jest.spyOn(global, 'fetch').mockResolvedValue({
+    blob: () => Promise.resolve(new Blob([''], { type: 'image/png' })),
+  } as Response);
+  const history = createMemoryHistory({
+    initialEntries: ['/superset/welcome/'],
+  });
+  const { container } = render(
+    <Router history={history}>
+      <DashboardCard
+        dashboard={mockDashboard}
+        hasPerm={mockHasPerm}
+        bulkSelectEnabled={false}
+        loading={false}
+        showThumbnails
+        openDashboardEditModal={mockOpenDashboardEditModal}
+        saveFavoriteStatus={mockSaveFavoriteStatus}
+        favoriteStatus={false}
+        handleBulkDashboardExport={mockHandleBulkDashboardExport}
+        onDelete={mockOnDelete}
+      />
+    </Router>,
+  );
+  const navigations: string[] = [];
+  history.listen(({ action, location }: Update) =>
+    navigations.push(`${action} ${location.pathname}`),
+  );
+
+  fireEvent.click(within(container).getByRole('link'));
+
+  expect(navigations).toEqual(['PUSH /dashboard/1']);
+});
+
 describe('thumbnail URL construction', () => {
   let fetchSpy: jest.SpyInstance;
 

superset-frontend/src/features/dashboards/DashboardCard.tsx

@@ -20,7 +20,7 @@ import { Link, useHistory } from 'react-router-dom';
 import { t } from '@apache-superset/core/translation';
 import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core';
 import { css } from '@apache-superset/core/theme';
-import { CardStyles } from 'src/views/CRUD/utils';
+import { CardStyles, isNavigationHandledByLink } from 'src/views/CRUD/utils';
 import {
   FaveStar,
   Icons,
@@ -169,8 +169,8 @@ function DashboardCard({
 
   return (
     <CardStyles
-      onClick={() => {
-        if (!bulkSelectEnabled) {
+      onClick={event => {
+        if (!bulkSelectEnabled && !isNavigationHandledByLink(event)) {
           history.push(dashboard.url);
         }
       }}