From 65e0d54fa52e308e5a9e3c6680a4db46fece3956 Mon Sep 17 00:00:00 2001
From: "JUST.in DO IT" <justin.park@airbnb.com>
Date: Wed, 15 May 2024 04:48:14 -0700
Subject: [PATCH] fix(dashboard): invalid drop item on a tab (#28507)

---
 .../dashboard/components/dnd/handleDrop.js    |  9 +-
 .../components/gridComponents/Tab.jsx         |  6 ++
 .../components/gridComponents/Tab.test.tsx    | 84 ++++++++++++++++++-
 .../src/dashboard/util/getDropPosition.js     |  2 +-
 4 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/dnd/handleDrop.js b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
index 6d50d39c45425..f4b847cd6d410 100644
--- a/superset-frontend/src/dashboard/components/dnd/handleDrop.js
+++ b/superset-frontend/src/dashboard/components/dnd/handleDrop.js
@@ -55,8 +55,11 @@ export default function handleDrop(props, monitor, Component) {
     },
   };
 
+  const shouldAppendToChildren =
+    typeof dropToChild === 'function' ? dropToChild(draggingItem) : dropToChild;
+
   // simplest case, append as child
-  if (dropToChild) {
+  if (shouldAppendToChildren) {
     dropResult.destination = {
       id: component.id,
       type: component.type,
@@ -74,7 +77,9 @@ export default function handleDrop(props, monitor, Component) {
     const sameParent =
       parentComponent && draggingItem.parentId === parentComponent.id;
     const sameParentLowerIndex =
-      sameParent && draggingItem.index < componentIndex;
+      sameParent &&
+      draggingItem.index < componentIndex &&
+      draggingItem.type !== component.type;
 
     const nextIndex = sameParentLowerIndex
       ? componentIndex - 1
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
index b546947e31a4f..bd71afa08167f 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
@@ -32,6 +32,7 @@ import DragDroppable, {
   Droppable,
 } from 'src/dashboard/components/dnd/DragDroppable';
 import { componentShape } from 'src/dashboard/util/propShapes';
+import { TAB_TYPE } from 'src/dashboard/util/componentTypes';
 
 export const RENDER_TAB = 'RENDER_TAB';
 export const RENDER_TAB_CONTENT = 'RENDER_TAB_CONTENT';
@@ -139,6 +140,10 @@ class Tab extends React.PureComponent {
     }
   }
 
+  shouldDropToChild(item) {
+    return item.type !== TAB_TYPE;
+  }
+
   renderTabContent() {
     const {
       component: tabComponent,
@@ -275,6 +280,7 @@ class Tab extends React.PureComponent {
         onDrop={this.handleDrop}
         onHover={this.handleOnHover}
         editMode={editMode}
+        dropToChild={this.shouldDropToChild}
       >
         {({ dropIndicatorProps, dragSourceRef }) => (
           <TabTitleContainer
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
index e19a086d811ce..6534a64f68ca5 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.test.tsx
@@ -19,12 +19,18 @@
 
 import userEvent from '@testing-library/user-event';
 import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+  fireEvent,
+  render,
+  screen,
+  waitFor,
+} from 'spec/helpers/testing-library';
 import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
 import EditableTitle from 'src/components/EditableTitle';
 import { setEditMode } from 'src/dashboard/actions/dashboardState';
 
 import Tab from './Tab';
+import Markdown from './Markdown';
 
 jest.mock('src/dashboard/containers/DashboardComponent', () =>
   jest.fn(() => <div data-test="DashboardComponent" />),
@@ -128,6 +134,82 @@ test('Render tab (no content) editMode:true', () => {
   expect(getByTestId('dragdroppable-object')).toBeInTheDocument();
 });
 
+test('Drop on a tab', async () => {
+  const props = createProps();
+  const mockOnDropOnTab = jest.fn();
+  render(
+    <>
+      <Tab {...props} renderType="RENDER_TAB" editMode />
+      <Tab
+        {...props}
+        renderType="RENDER_TAB"
+        index={2}
+        component={{
+          ...props.component,
+          id: 'TAB-Next-',
+          meta: { text: 'Next Tab' } as any,
+        }}
+        handleComponentDrop={mockOnDropOnTab}
+        editMode
+      />
+      <Markdown
+        id="MARKDOWN-1"
+        parentId="GRID_ID"
+        parentComponent={{
+          id: 'GRID_ID',
+          type: 'GRID',
+          parents: ['ROOT_ID'],
+        }}
+        depth={0}
+        editMode
+        index={1}
+        availableColumnCount={12}
+        columnWidth={120}
+        component={{
+          ...props.component,
+          type: 'MARKDOWN',
+          id: 'MARKDOWN-1',
+          meta: { code: 'Dashboard Component' } as any,
+        }}
+        logEvent={jest.fn()}
+        deleteComponent={jest.fn()}
+        handleComponentDrop={jest.fn()}
+        onResizeStart={jest.fn()}
+        onResize={jest.fn()}
+        onResizeStop={jest.fn()}
+        updateComponents={jest.fn()}
+        addDangerToast={jest.fn()}
+      />
+    </>,
+    {
+      useRedux: true,
+      useDnd: true,
+    },
+  );
+
+  fireEvent.dragStart(screen.getByText('🚀 Aspiring Developers'));
+  fireEvent.drop(screen.getByText('Next Tab'));
+  await waitFor(() => expect(mockOnDropOnTab).toHaveBeenCalled());
+  expect(mockOnDropOnTab).toHaveBeenCalledWith(
+    expect.objectContaining({
+      destination: { id: props.parentComponent.id, index: 2, type: 'TABS' },
+    }),
+  );
+
+  fireEvent.dragStart(screen.getByText('Dashboard Component'));
+  fireEvent.drop(screen.getByText('Next Tab'));
+  await waitFor(() => expect(mockOnDropOnTab).toHaveBeenCalledTimes(2));
+  expect(mockOnDropOnTab).toHaveBeenLastCalledWith(
+    expect.objectContaining({
+      destination: {
+        id: 'TAB-Next-',
+        index: props.component.children.length,
+        type: 'TAB',
+      },
+    }),
+  );
+});
+
 test('Edit table title', () => {
   const props = createProps();
   props.editMode = true;
diff --git a/superset-frontend/src/dashboard/util/getDropPosition.js b/superset-frontend/src/dashboard/util/getDropPosition.js
index c4a21ad113864..9a68808b05dcf 100644
--- a/superset-frontend/src/dashboard/util/getDropPosition.js
+++ b/superset-frontend/src/dashboard/util/getDropPosition.js
@@ -82,7 +82,7 @@ export default function getDropPosition(monitor, Component) {
   const siblingDropOrientation =
     orientation === 'row' ? 'horizontal' : 'vertical';
 
-  if (isDraggingOverShallow && validChild && !validSibling) {
+  if (validChild && !validSibling) {
     // easiest case, insert as child
     if (childDropOrientation === 'vertical') {
       return hasChildren ? DROP_RIGHT : DROP_LEFT;