Merge pull request #1926 from naitsirch/sync_positions_after_multiple_deletes

[Sortable] Fix issue with position synchronization after multiple deletes

Author: l3pp4rd

lib/Gedmo/Sortable/SortableListener.php

@@ -413,6 +413,9 @@ public function postFlush(EventArgs $args)
     {
         $ea = $this->getEventAdapter($args);
         $em = $ea->getObjectManager();
+
+        $updatedObjects = [];
+
         foreach ($this->relocations as $hash => $relocation) {
             $config = $this->getConfiguration($em, $relocation['name']);
             foreach ($relocation['deltas'] as $delta) {
@@ -473,12 +476,28 @@ public function postFlush(EventArgs $args)
                             $value = next($relocation['groups']);
                         }
                         if ($matches) {
-                            $this->setFieldValue($ea, $object, $config['position'], $pos, $pos + $delta['delta']);
+                            // We cannot use `$this->setFieldValue()` here, because it will create a change set, that will
+                            // prevent from other relocations being executed on this object.
+                            // We just update the object value and will create the change set later.
+                            if (!isset($updatedObjects[$oid])) {
+                                $updatedObjects[$oid] = array(
+                                    'object' => $object,
+                                    'field' => $config['position'],
+                                    'oldValue' => $pos,
+                                );
+                            }
+                            $updatedObjects[$oid]['newValue'] = $pos + $delta['delta'];
+
+                            $meta->getReflectionProperty($config['position'])->setValue($object, $updatedObjects[$oid]['newValue']);
                         }
                     }
                 }
             }
 
+            foreach ($updatedObjects as $updateData) {
+                $this->setFieldValue($ea, $updateData['object'], $updateData['field'], $updateData['oldValue'], $updateData['newValue']);
+            }
+
             // Clear relocations
             unset($this->relocations[$hash]);
             unset($this->maxPositions[$hash]); // unset only if relocations has been processed

tests/Gedmo/Sortable/SortableTest.php

@@ -74,7 +74,7 @@ public function testMoveLastPosition()
             $node->setPath("/");
             $this->em->persist($node);
         }
-        $this->em->flush();  
+        $this->em->flush();
 
         $repo = $this->em->getRepository(self::NODE);
 
@@ -91,7 +91,7 @@ public function testMoveLastPosition()
         $node = $repo->findOneByPosition(9);
         $this->assertNotNull($node);
         $this->assertEquals('Node1', $node->getName());
-    
+
     }
 
     /**
@@ -248,6 +248,56 @@ public function shouldSyncPositionAfterDelete()
         $this->assertEquals(1, $nodes[1]->getPosition());
     }
 
+    /**
+     * Test if the sorting is correct if multiple items are deleted.
+     *
+     * Example:
+     *     Position | Element | Action | Expected Position
+     *            0 | Node1   |        |                 0
+     *            1 | Node2   | delete |
+     *            2 | Node3   | delete |
+     *            3 | Node4   |        |                 1
+     *
+     * @test
+     */
+    public function shouldSyncPositionAfterMultipleDeletes()
+    {
+        $repo = $this->em->getRepository(self::NODE);
+
+        $node2 = new Node();
+        $node2->setName("Node2");
+        $node2->setPath("/");
+        $this->em->persist($node2);
+
+        $node3 = new Node();
+        $node3->setName("Node3");
+        $node3->setPath("/");
+        $this->em->persist($node3);
+
+        $node4 = new Node();
+        $node4->setName("Node4");
+        $node4->setPath("/");
+        $this->em->persist($node4);
+
+        $this->em->flush();
+
+        $node1 = $repo->findOneByName('Node1');
+        $this->em->remove($node2);
+        $this->em->remove($node3);
+        $this->em->flush();
+
+        // test if synced on objects in memory correctly
+        $this->assertEquals(0, $node1->getPosition());
+        $this->assertEquals(1, $node4->getPosition());
+
+        // test if persisted correctly
+        $this->em->clear();
+        $nodes = $repo->findAll();
+        $this->assertCount(2, $nodes);
+        $this->assertEquals(0, $nodes[0]->getPosition());
+        $this->assertEquals(1, $nodes[1]->getPosition());
+    }
+
     /**
      * This is a test case for issue #1209
      * @test
@@ -792,7 +842,7 @@ public function testSetOutOfBoundsHighPosition()
 
         $this->assertEquals(4, $nodes[4]->getPosition());
     }
-    
+
     /**
      * @test
      */