Repository: primefaces/primefaces
Pull request #8692 (2022-04-20 12:40:57)

Tree: fix dragdrop on server side model
If you move an item in tree downwards, then the frontend shows it correctly but the model at server side has wrong order.

Initial order
```
Node 1
Node 2
Node 3
Node 4
```
move an item
```
Node 2
<-- Node 1
Node 3
Node 4
```
order at server side (vs client side)
```
Node 2 (Node 2)
Node 3 (Node 1)
Node 1 (Node 3)
Node 4 (Node 4)
```

melloware: @stonewoodman can you open a GitHub issue ticket describing the issue so I can link it to this PR?

stonewoodman: @melloware i created an issue #8693

Patch:
diff --git a/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java b/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
index 9fa05402c8..cde6552537 100644
--- a/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
+++ b/primefaces/src/main/java/org/primefaces/model/TreeNodeChildren.java
@@ -67,6 +67,16 @@ public class TreeNodeChildren<T> extends TreeNodeList<T> {
             throw new IndexOutOfBoundsException();
         }
 
+        // check if the movement is on the same list
+        if (node.getParent() != null && node.getParent().getChildren() == this) {
+
+            // check if the movement is downwards then correct the index
+            int removedIndex = super.indexOf(node);
+            if (removedIndex > -1 && index > removedIndex) {
+                index--;
+            }
+        }
+
         eraseParent(node);
         super.add(index, node);
         node.setParent(parent);


Test patch:
diff --git a/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java b/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java
index f4f43a8cbe..af0afb87ee 100644
--- a/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java
+++ b/primefaces/src/test/java/org/primefaces/model/TreeNodeChildrenTest.java
@@ -59,4 +59,185 @@ public class TreeNodeChildrenTest {
         assertEquals(root, values.get(1).getParent());
     }
 
+    @Test
+    public void moveNodeSameListDown() {
+        // Arrange
+        TreeNode<String> root = new DefaultTreeNode<>("Node R", null);
+        TreeNode<String> node1 = new DefaultTreeNode<>("Node 1", root);
+        TreeNode<String> node2 = new DefaultTreeNode<>("Node 2", root);
+        TreeNode<String> node3 = new DefaultTreeNode<>("Node 3", root);
+        TreeNode<String> node4 = new DefaultTreeNode<>("Node 4", root);
+        TreeNode<String> node5 = new DefaultTreeNode<>("Node 5", root);
+
+        // Pre-assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node4, root.getChildren().get(3));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+
+        // Act (simulate Drag&Drop)
+        root.getChildren().add(2, root.getChildren().get(0));
+
+        // Assert
+        assertEquals(node2, root.getChildren().get(0));
+        assertEquals(root, node2.getParent());
+        assertEquals(node1, root.getChildren().get(1));
+        assertEquals(root, node1.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node4, root.getChildren().get(3));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+    }
+
+    @Test
+    public void moveNodeSameListUp() {
+        // Arrange
+        TreeNode<String> root = new DefaultTreeNode<>("Node R", null);
+        TreeNode<String> node1 = new DefaultTreeNode<>("Node 1", root);
+        TreeNode<String> node2 = new DefaultTreeNode<>("Node 2", root);
+        TreeNode<String> node3 = new DefaultTreeNode<>("Node 3", root);
+        TreeNode<String> node4 = new DefaultTreeNode<>("Node 4", root);
+        TreeNode<String> node5 = new DefaultTreeNode<>("Node 5", root);
+
+        // Pre-assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node4, root.getChildren().get(3));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+
+        // Act (simulate Drag&Drop)
+        root.getChildren().add(2, root.getChildren().get(3));
+
+        // Assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node4, root.getChildren().get(2));
+        assertEquals(root, node4.getParent());
+        assertEquals(node3, root.getChildren().get(3));
+        assertEquals(root, node3.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+    }
+
+    @Test
+    public void moveNodeToChild() {
+        // Arrange
+        TreeNode<String> root = new DefaultTreeNode<>("Node R", null);
+        TreeNode<String> node1 = new DefaultTreeNode<>("Node 1", root);
+        TreeNode<String> node2 = new DefaultTreeNode<>("Node 2", root);
+        TreeNode<String> node3 = new DefaultTreeNode<>("Node 3", root);
+        TreeNode<String> node31 = new DefaultTreeNode<>("Node 3.1", node3);
+        TreeNode<String> node32 = new DefaultTreeNode<>("Node 3.2", node3);
+        TreeNode<String> node33 = new DefaultTreeNode<>("Node 3.3", node3);
+        TreeNode<String> node4 = new DefaultTreeNode<>("Node 4", root);
+        TreeNode<String> node5 = new DefaultTreeNode<>("Node 5", root);
+
+        // Pre-assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node31, root.getChildren().get(2).getChildren().get(0));
+        assertEquals(node3, node31.getParent());
+        assertEquals(node32, root.getChildren().get(2).getChildren().get(1));
+        assertEquals(node3, node32.getParent());
+        assertEquals(node33, root.getChildren().get(2).getChildren().get(2));
+        assertEquals(node3, node33.getParent());
+        assertEquals(node4, root.getChildren().get(3));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+
+        // Act (simulate Drag&Drop)
+        root.getChildren().get(2).getChildren().add(1, root.getChildren().get(3));
+
+        // Assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node31, root.getChildren().get(2).getChildren().get(0));
+        assertEquals(node3, node31.getParent());
+        assertEquals(node4, root.getChildren().get(2).getChildren().get(1));
+        assertEquals(node3, node4.getParent());
+        assertEquals(node32, root.getChildren().get(2).getChildren().get(2));
+        assertEquals(node3, node32.getParent());
+        assertEquals(node33, root.getChildren().get(2).getChildren().get(3));
+        assertEquals(node3, node33.getParent());
+        assertEquals(node5, root.getChildren().get(3));
+        assertEquals(root, node5.getParent());
+    }
+
+    @Test
+    public void moveNodeToParent() {
+        // Arrange
+        TreeNode<String> root = new DefaultTreeNode<>("Node R", null);
+        TreeNode<String> node1 = new DefaultTreeNode<>("Node 1", root);
+        TreeNode<String> node2 = new DefaultTreeNode<>("Node 2", root);
+        TreeNode<String> node3 = new DefaultTreeNode<>("Node 3", root);
+        TreeNode<String> node31 = new DefaultTreeNode<>("Node 3.1", node3);
+        TreeNode<String> node32 = new DefaultTreeNode<>("Node 3.2", node3);
+        TreeNode<String> node33 = new DefaultTreeNode<>("Node 3.3", node3);
+        TreeNode<String> node4 = new DefaultTreeNode<>("Node 4", root);
+        TreeNode<String> node5 = new DefaultTreeNode<>("Node 5", root);
+
+        // Pre-assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node3, root.getChildren().get(2));
+        assertEquals(root, node3.getParent());
+        assertEquals(node31, root.getChildren().get(2).getChildren().get(0));
+        assertEquals(node3, node31.getParent());
+        assertEquals(node32, root.getChildren().get(2).getChildren().get(1));
+        assertEquals(node3, node32.getParent());
+        assertEquals(node33, root.getChildren().get(2).getChildren().get(2));
+        assertEquals(node3, node33.getParent());
+        assertEquals(node4, root.getChildren().get(3));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(4));
+        assertEquals(root, node5.getParent());
+
+        // Act (simulate Drag&Drop)
+        root.getChildren().add(2, root.getChildren().get(2).getChildren().get(1));
+
+        // Assert
+        assertEquals(node1, root.getChildren().get(0));
+        assertEquals(root, node1.getParent());
+        assertEquals(node2, root.getChildren().get(1));
+        assertEquals(root, node2.getParent());
+        assertEquals(node32, root.getChildren().get(2));
+        assertEquals(root, node32.getParent());
+        assertEquals(node3, root.getChildren().get(3));
+        assertEquals(root, node3.getParent());
+        assertEquals(node31, root.getChildren().get(3).getChildren().get(0));
+        assertEquals(node3, node31.getParent());
+        assertEquals(node33, root.getChildren().get(3).getChildren().get(1));
+        assertEquals(node3, node33.getParent());
+        assertEquals(node4, root.getChildren().get(4));
+        assertEquals(root, node4.getParent());
+        assertEquals(node5, root.getChildren().get(5));
+        assertEquals(root, node5.getParent());
+    }
 }