Fix destination resolution algo

x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/drag_and_drop_tree_provider.tsx

@@ -3,9 +3,16 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import React, { createContext, useState, FunctionComponent, useContext, useCallback } from 'react';
+import React, {
+  createContext,
+  useState,
+  FunctionComponent,
+  useContext,
+  useCallback,
+  useRef,
+} from 'react';
 import { EuiDragDropContext } from '@elastic/eui';
-import { mapSelectorToDragLocation, resolveDestinationLocation } from './utils';
+import { DragDirection, mapSelectorToDragLocation, resolveDestinationLocation } from './utils';
 import { DraggableLocation, ProcessorSelector } from '../../types';
 
 interface PrivateMapTreeEntry {
@@ -38,6 +45,10 @@ interface Props {
 }
 
 export const DragAndDropTreeProvider: FunctionComponent<Props> = ({ children, onDragEnd }) => {
+  const dragDirectionRef = useRef<{
+    start?: { index: number; treeId: string };
+    current?: { index: number; treeId: string };
+  }>({});
   const [currentDragSelector, setCurrentDragSelector] = useState<string | undefined>();
   const [privateTreeItemMap] = useState(() => new Map<string, PrivateMapTreeEntry>());
 
@@ -68,8 +79,45 @@ export const DragAndDropTreeProvider: FunctionComponent<Props> = ({ children, on
         onBeforeCapture={({ draggableId: serializedSelector }) => {
           setCurrentDragSelector(serializedSelector);
         }}
+        onDragStart={arg => {
+          dragDirectionRef.current = {
+            current: {
+              index: arg.source.index,
+              treeId: arg.source.droppableId,
+            },
+            start: {
+              index: arg.source.index,
+              treeId: arg.source.droppableId,
+            },
+          };
+        }}
+        onDragUpdate={arg => {
+          // We are dragging outside of the tree, clear tracking
+          if (!arg.destination && !arg.combine) {
+            dragDirectionRef.current = {};
+            return;
+          }
+
+          if (
+            arg.destination &&
+            arg.source &&
+            (arg.destination.index !== dragDirectionRef.current.current?.index ||
+              arg.destination.droppableId !== dragDirectionRef.current.current?.treeId)
+          ) {
+            dragDirectionRef.current = {
+              current: { index: arg.destination.index, treeId: arg.destination.droppableId },
+              start:
+                // Reset start if we have gone x-tree
+                arg.destination.droppableId !== dragDirectionRef.current.current?.treeId
+                  ? { index: arg.destination.index, treeId: arg.destination.droppableId }
+                  : dragDirectionRef.current.start,
+            };
+          }
+        }}
         onDragEnd={arg => {
           setCurrentDragSelector(undefined);
+          const dragPointers = dragDirectionRef.current;
+          dragDirectionRef.current = {};
 
           const { source, destination, combine } = arg;
           if (source && combine) {
@@ -90,19 +138,34 @@ export const DragAndDropTreeProvider: FunctionComponent<Props> = ({ children, on
           }
 
           if (source && destination) {
+            let dragDirection: DragDirection;
+            const { current, start } = dragPointers;
+            if (!current || !start) {
+              dragDirection = 'none';
+            } else if (current.index < start.index) {
+              dragDirection = 'up';
+            } else {
+              dragDirection = 'down';
+            }
+
             const sourceTree = privateTreeItemMap.get(source.droppableId)!;
-            const destinationTree =
-              source.droppableId !== destination.droppableId
-                ? privateTreeItemMap.get(destination.droppableId)!
-                : sourceTree;
+            const xTreeDrag = source.droppableId !== destination.droppableId;
+            const destinationTree = xTreeDrag
+              ? privateTreeItemMap.get(destination.droppableId)!
+              : sourceTree;
             const sourceSelector = sourceTree.selectors[source.index];
+
+            const destinationLocation = resolveDestinationLocation(
+              destinationTree.selectors,
+              destination.index,
+              destinationTree.baseSelector,
+              xTreeDrag ? 'none' : dragDirection,
+              sourceSelector.length <= 2
+            );
+
             onDragEnd({
               source: mapSelectorToDragLocation(sourceSelector),
-              destination: resolveDestinationLocation(
-                destinationTree.selectors,
-                destination.index,
-                destinationTree.baseSelector
-              ),
+              destination: destinationLocation,
             });
           }
         }}

x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/utils.test.ts

@@ -14,28 +14,56 @@ describe('Resolve destination location', () => {
       ['processors', '0'],
       ['processors', '0', 'onFailure', '0'],
     ];
-    const result = resolveDestinationLocation(testItems, 0, testBaseSelector);
+    const result = resolveDestinationLocation(testItems, 0, testBaseSelector, 'up');
     expect(result).toEqual({ selector: ['processors'], index: 0 });
   });
 
-  it('resolves to root level when dragged to bottom', () => {
+  it('resolves to root level when dragged to bottom if source comes from root', () => {
     const testItems = [
       ['processors', '0'],
-      ['processors', '0', 'onFailure', '0'],
+      ['processors', '1'],
+      ['processors', '1', 'onFailure', '0'],
     ];
-    const result = resolveDestinationLocation(testItems, testItems.length - 1, testBaseSelector);
-    expect(result).toEqual({ selector: ['processors'], index: testItems.length - 1 });
+    const result1 = resolveDestinationLocation(
+      testItems,
+      testItems.length - 1,
+      testBaseSelector,
+      'down',
+      testItems[0].length === 2
+    );
+    expect(result1).toEqual({ selector: ['processors'], index: testItems.length - 1 });
+  });
+
+  it('nests an element at the position it was placed', () => {
+    const testItems = [
+      ['processors', '0'],
+      ['processors', '1'],
+      ['processors', '1', 'onFailure', '0'],
+    ];
+    const result2 = resolveDestinationLocation(
+      testItems,
+      testItems.length - 1,
+      testBaseSelector,
+      'down',
+      false
+    );
+    expect(result2).toEqual({ selector: ['processors', '1', 'onFailure'], index: 1 });
   });
 
   it('sets the base selector if there are no items', () => {
     const testItems: ProcessorSelector[] = [];
-    const result = resolveDestinationLocation(testItems, testItems.length - 1, testBaseSelector);
-    expect(result).toEqual({ selector: testBaseSelector, index: testItems.length - 1 });
+    const result = resolveDestinationLocation(
+      testItems,
+      testItems.length - 1,
+      testBaseSelector,
+      'none'
+    );
+    expect(result).toEqual({ selector: testBaseSelector, index: 0 });
   });
 
   it('displaces the current item if surrounded by items at same level', () => {
     const testItems = [['0'], ['0', 'onFailure', '0'], ['0', 'onFailure', '1']];
-    const result = resolveDestinationLocation(testItems, 1, testBaseSelector);
+    const result = resolveDestinationLocation(testItems, 1, testBaseSelector, 'up');
     expect(result).toEqual({ selector: ['0', 'onFailure'], index: 0 });
   });
 
@@ -48,7 +76,7 @@ describe('Resolve destination location', () => {
       ['0', 'onFailure', '1', 'onFailure', '1'],
     ];
 
-    const result1 = resolveDestinationLocation(testItems1, 3, testBaseSelector);
+    const result1 = resolveDestinationLocation(testItems1, 3, testBaseSelector, 'down');
     expect(result1).toEqual({
       selector: ['0', 'onFailure', '1', 'onFailure'],
       index: 0,
@@ -63,7 +91,7 @@ describe('Resolve destination location', () => {
       ['0', 'onFailure', '3'],
     ];
 
-    const result2 = resolveDestinationLocation(testItems2, 4, testBaseSelector);
+    const result2 = resolveDestinationLocation(testItems2, 4, testBaseSelector, 'down');
     expect(result2).toEqual({ selector: ['0', 'onFailure'], index: 2 });
   });
 
@@ -73,7 +101,7 @@ describe('Resolve destination location', () => {
       ['0', 'onFailure' /* should end in a number! */],
       ['0', 'onFailure' /* should end in a number! */],
     ];
-    expect(() => resolveDestinationLocation(testItems, 1, testBaseSelector)).toThrow(
+    expect(() => resolveDestinationLocation(testItems, 1, testBaseSelector, 'down')).toThrow(
       'Expected an integer but received "onFailure"'
     );
   });

x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/drag_and_drop_tree/utils.ts

@@ -17,29 +17,50 @@ export const mapSelectorToDragLocation = (selector: ProcessorSelector): Draggabl
   };
 };
 
+export type DragDirection = 'up' | 'down' | 'none';
+
+/**
+ *
+ * @param items
+ * @param destinationIndex
+ * @param baseDestinationSelector
+ * @param dragDirection
+ * @param isSourceAtRootLevel
+ */
 export const resolveDestinationLocation = (
   items: ProcessorSelector[],
   destinationIndex: number,
-  baseSelector: ProcessorSelector
+  baseDestinationSelector: ProcessorSelector,
+  dragDirection: DragDirection,
+  isSourceAtRootLevel = false
 ): DraggableLocation => {
   // Dragged to top, place at root level
-  if (destinationIndex === 0) {
-    const destinationSelector = items[destinationIndex] ?? baseSelector;
+  if (destinationIndex <= 0) {
+    const destinationSelector = items[destinationIndex] ?? baseDestinationSelector;
     return { selector: destinationSelector.slice(0, 1), index: 0 };
   }
 
-  // Dragged to bottom, place at root level
-  if (destinationIndex === items.length - 1 || destinationIndex === items.length) {
-    const destinationSelector = items[destinationIndex] ?? baseSelector;
+  // Dragged to bottom, place at root level if source is already at root
+  if (destinationIndex === items.length - 1 && isSourceAtRootLevel) {
+    const destinationSelector = items[destinationIndex] ?? baseDestinationSelector;
     return { selector: destinationSelector.slice(0, 1), index: items.length - 1 };
   }
 
-  const above: ProcessorSelector = items[destinationIndex - 1];
-  const below: ProcessorSelector = items[destinationIndex]; // This is the processor we are displacing
+  // This can happen when dragging across trees
+  if (destinationIndex > items.length - 1) {
+    return { selector: baseDestinationSelector.slice(0), index: destinationIndex };
+  }
 
-  if (above.length !== below.length) {
-    return mapSelectorToDragLocation(below);
+  const displacing: ProcessorSelector = items[destinationIndex];
+
+  if (dragDirection === 'none') {
+    return mapSelectorToDragLocation(displacing);
   }
 
-  return mapSelectorToDragLocation(items[destinationIndex]);
+  if (dragDirection === 'down') {
+    const below: ProcessorSelector = items[destinationIndex + 1];
+    return mapSelectorToDragLocation(below);
+  } else {
+    return mapSelectorToDragLocation(displacing);
+  }
 };