From be1dbac02e59a7b95df0212e83e43034b89a298f Mon Sep 17 00:00:00 2001
From: Claude <claude@anthropic.com>
Date: Tue, 12 May 2026 13:12:51 -0700
Subject: [PATCH] test(SaveModal,CopyToClipboard): drop placeholder asserts;
 add string/number copyNode guard
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two changes in response to review feedback on the FC conversion:

1. Drop three `expect(true).toBe(true)` placeholder tests in
   `SaveModal.test.tsx` (`dispatches removeChartState ... - placeholder`,
   `onDashboardChange triggers tabs load ... - placeholder`,
   `onTabChange correctly updates selectedTab - placeholder`). They were
   left behind from the class-component version and assert nothing —
   net-negative as regression guards. They'll be re-added in a follow-up
   once the FC-shaped versions are wired up.

2. Add a regression test in `CopyToClipboard.test.tsx` that renders the
   component with a string `copyNode` and with a number `copyNode`,
   covering the `cloneElement`-on-non-element crash that prompted the
   `isValidElement` guard in `src/components/CopyToClipboard/index.tsx`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../CopyToClipboard/CopyToClipboard.test.tsx  | 14 ++++++++
 .../src/explore/components/SaveModal.test.tsx | 34 -------------------
 2 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/superset-frontend/src/components/CopyToClipboard/CopyToClipboard.test.tsx b/superset-frontend/src/components/CopyToClipboard/CopyToClipboard.test.tsx
index 1ebb158dead..73a0d930850 100644
--- a/superset-frontend/src/components/CopyToClipboard/CopyToClipboard.test.tsx
+++ b/superset-frontend/src/components/CopyToClipboard/CopyToClipboard.test.tsx
@@ -37,6 +37,20 @@ test('renders with custom copy node', () => {
   expect(screen.getByRole('link')).toBeInTheDocument();
 });
 
+// Regression guard: passing a non-element copyNode (string or number) used to
+// crash because cloneElement only accepts React elements. The render path now
+// gates the cloneElement call behind isValidElement and falls back to a span
+// wrapper, so plain primitives should render without throwing.
+test('renders with string copyNode without crashing', () => {
+  render(<CopyToClipboard copyNode="just text" />, { useRedux: true });
+  expect(screen.getByRole('button')).toHaveTextContent('just text');
+});
+
+test('renders with number copyNode without crashing', () => {
+  render(<CopyToClipboard copyNode={42} />, { useRedux: true });
+  expect(screen.getByRole('button')).toHaveTextContent('42');
+});
+
 test('renders without text showing', () => {
   const text = 'Text';
   render(<CopyToClipboard text={text} shouldShowText={false} />, {
diff --git a/superset-frontend/src/explore/components/SaveModal.test.tsx b/superset-frontend/src/explore/components/SaveModal.test.tsx
index ebf042db5d9..fa5a9b59547 100644
--- a/superset-frontend/src/explore/components/SaveModal.test.tsx
+++ b/superset-frontend/src/explore/components/SaveModal.test.tsx
@@ -410,18 +410,6 @@ test('createRedirectParams removes form_data_key from URL parameters', () => {
   expect(result.get('save_action')).toEqual('overwrite');
 });
 
-/**
- * TODO: This test was written for the class component version of SaveModal.
- * Since SaveModal has been converted to a function component, this test
- * needs to be rewritten to test through component rendering and user interaction.
- * The test should verify that clicking "Save & go to dashboard" dispatches
- * removeChartState with the correct chart ID.
- */
-test('dispatches removeChartState when saving and going to dashboard - placeholder', () => {
-  // See TODO comment above
-  expect(true).toBe(true);
-});
-
 test('disables tab selector when no dashboard selected', () => {
   const { getByRole, getByTestId } = setup();
   fireEvent.click(getByRole('radio', { name: 'Save as...' }));
@@ -440,28 +428,6 @@ test('renders tab selector when saving as', async () => {
   expect(tabSelector).toBeDisabled();
 });
 
-/**
- * TODO: This test was written for the class component version of SaveModal.
- * Since SaveModal has been converted to a function component, this test
- * needs to be rewritten to test through component rendering and user interaction.
- * The test should verify that selecting a dashboard triggers tab loading.
- */
-test('onDashboardChange triggers tabs load for existing dashboard - placeholder', () => {
-  // See TODO comment above
-  expect(true).toBe(true);
-});
-
-/**
- * TODO: This test was written for the class component version of SaveModal.
- * Since SaveModal has been converted to a function component, this test
- * needs to be rewritten to test through component rendering and user interaction.
- * The test should verify that changing the tab selection updates the component state.
- */
-test('onTabChange correctly updates selectedTab - placeholder', () => {
-  // See TODO comment above
-  expect(true).toBe(true);
-});
-
 test('chart placement logic finds row with available space', () => {
   // Test case 1: Row has space (8 + 4 = 12 <= 12)
   const positionJson1 = {