[BUGFIX] sorting with nested containers

Fixes: #425
b13 · Aug 15, 2023 · f3d45ee · f3d45ee
1 parent b30aa88
commit f3d45ee
Show file tree

Hide file tree

Showing 14 changed files with 156 additions and 17 deletions.
diff --git a/Classes/Hooks/Datahandler/CommandMapBeforeStartHook.php b/Classes/Hooks/Datahandler/CommandMapBeforeStartHook.php
@@ -122,21 +122,22 @@ protected function rewriteCommandMapTargetForAfterContainer(array $cmdmap): arra
                     if (in_array($operation, ['copy', 'move'], true) === false) {
                         continue;
                     }
-                    if (
-                        (!isset($value['update']['tx_container_parent']) || (int)$value['update']['tx_container_parent'] === 0) &&
-                        ((is_array($value) && $value['target'] < 0) || (int)$value < 0)
-                    ) {
+                    if ((is_array($value) && $value['target'] < 0) || (int)$value < 0) {
                         if (is_array($value)) {
                             $target = -(int)$value['target'];
                         } else {
                             // simple command
                             $target = -(int)$value;
                         }
-                        $record = $this->database->fetchOneRecord($target);
-                        if ($record === null || $record['tx_container_parent'] > 0) {
+                        if ($target === (int)$value['update']['tx_container_parent']) {
                             // elements in container have already correct target
                             continue;
                         }
+                        $record = $this->database->fetchOneRecord($target);
+                        if ($record === null) {
+                            // should not happen
+                            continue;
+                        }
                         if (!$this->tcaRegistry->isContainerElement($record['CType'])) {
                             continue;
                         }

diff --git a/Classes/Hooks/Datahandler/DatamapPreProcessFieldArrayHook.php b/Classes/Hooks/Datahandler/DatamapPreProcessFieldArrayHook.php
@@ -55,14 +55,17 @@ public function __construct(
 
     protected function newElementAfterContainer(array $incomingFieldArray): array
     {
-        if (isset($incomingFieldArray['tx_container_parent']) && (int)$incomingFieldArray['tx_container_parent'] > 0) {
-            return $incomingFieldArray;
-        }
         $record = $this->database->fetchOneRecord(-(int)$incomingFieldArray['pid']);
-        if ($record === null || $record['tx_container_parent'] > 0) {
+        if ($record === null) {
             // new elements in container have already correct target
             return $incomingFieldArray;
         }
+        if (
+            (int)$record['uid'] === (int)($incomingFieldArray['tx_container_parent'] ?? 0) ||
+            (isset($record['t3_origuid']) && $record['t3_origuid'] > 0 && (int)$record['t3_origuid'] === (int)($incomingFieldArray['tx_container_parent'] ?? 0))
+        ) {
+            return $incomingFieldArray;
+        }
         if (!$this->tcaRegistry->isContainerElement($record['CType'])) {
             return $incomingFieldArray;
         }

diff --git a/Classes/Integrity/Database.php b/Classes/Integrity/Database.php
@@ -207,4 +207,28 @@ public function getContainerChildRecords(): array
         }
         return $rows;
     }
+
+    public function getSortingByUid(int $uid): ?int
+    {
+        $queryBuilder = $this->getQueryBuilder();
+        $stm = $queryBuilder
+            ->select('sorting')
+            ->from('tt_content')
+            ->where(
+                $queryBuilder->expr()->eq(
+                    'uid',
+                    $queryBuilder->createNamedParameter($uid, Connection::PARAM_INT)
+                )
+            )
+            ->execute();
+        if ((GeneralUtility::makeInstance(Typo3Version::class))->getMajorVersion() === 10) {
+            $row = $stm->fetch();
+        } else {
+            $row = $stm->fetchAssociative();
+        }
+        if ($row === false || !isset($row['sorting'])) {
+            return null;
+        }
+        return $row['sorting'];
+    }
 }
diff --git a/Classes/Integrity/Sorting.php b/Classes/Integrity/Sorting.php
@@ -15,6 +15,7 @@
 use B13\Container\Domain\Factory\ContainerFactory;
 use B13\Container\Domain\Factory\Exception;
 use B13\Container\Domain\Model\Container;
+use B13\Container\Domain\Service\ContainerService;
 use B13\Container\Tca\Registry;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
 use TYPO3\CMS\Core\SingletonInterface;
@@ -37,13 +38,19 @@ class Sorting implements SingletonInterface
      */
     protected $containerFactory;
 
+    /**
+     * @var ContainerService
+     */
+    protected $containerService;
+
     protected $errors = [];
 
-    public function __construct(Database $database, Registry $tcaRegistry, ContainerFactory $containerFactory)
+    public function __construct(Database $database, Registry $tcaRegistry, ContainerFactory $containerFactory, ContainerService $containerService)
     {
         $this->database = $database;
         $this->tcaRegistry = $tcaRegistry;
         $this->containerFactory = $containerFactory;
+        $this->containerService = $containerService;
     }
 
     public function run(bool $dryRun = true): array
@@ -89,6 +96,16 @@ protected function fixChildrenSortingUpdateRequired(Container $container, array
                     return true;
                 }
                 $prevSorting = $child['sorting'];
+                if ($this->tcaRegistry->isContainerElement($child['CType'])) {
+                    $container = $this->containerFactory->buildContainer((int)$child['uid']);
+                    $targetUid = (-1) * $this->containerService->getAfterContainerElementTarget($container);
+                    if ($container->getUid() !== $targetUid) {
+                        $sorting = $this->database->getSortingByUid($targetUid);
+                        if ($child['sorting'] <= $sorting) {
+                            $prevSorting = $sorting;
+                        }
+                    }
+                }
             }
         }
         return false;

diff --git a/Tests/Functional/Datahandler/DefaultLanguage/ContainerTest.php b/Tests/Functional/Datahandler/DefaultLanguage/ContainerTest.php
@@ -302,7 +302,7 @@ public function copyContainerKeepsSortingOfChildren(): void
         $secondChild = $this->fetchOneRecord('t3_origuid', 5);
         self::assertTrue($child['sorting'] < $secondChild['sorting']);
         $container = $this->fetchOneRecord('uid', 1);
-        self::assertTrue($child['sorting'] > $container['sorting'], 'moved child is sorted before container');
+        self::assertTrue($child['sorting'] > $container['sorting'], 'copied child is sorted before container');
     }
 
     /**

diff --git a/...unctional/Datahandler/DefaultLanguage/Fixtures/Container/copy_container_keeps_sorting.csv b/...unctional/Datahandler/DefaultLanguage/Fixtures/Container/copy_container_keeps_sorting.csv
@@ -4,6 +4,6 @@
 ,3,0,"page-2","/page-2"
 "tt_content"
 ,"uid","pid","CType","header","sorting","sys_language_uid","colPos","tx_container_parent"
-,1,1,"b13-2cols-with-header-container","container-default",256,0,,
+,1,1,"b13-2cols-with-header-container","container-default",56,0,,
 ,2,1,"header","header-default",128,0,200,1
 ,5,1,"header","second element",256,0,200,1
diff --git a/Tests/Functional/Datahandler/DefaultLanguage/Fixtures/MoveElement/nested_container.csv b/Tests/Functional/Datahandler/DefaultLanguage/Fixtures/MoveElement/nested_container.csv
@@ -0,0 +1,10 @@
+"pages"
+,"uid","pid","title","slug"
+,1,0,"page-1","/"
+"tt_content"
+,"uid","pid","CType","header","sorting","sys_language_uid","colPos","tx_container_parent"
+,1,1,"header","outside",128,0,0,
+,2,1,"b13-2cols-with-header-container","container",256,0,,
+,3,1,"b13-2cols-with-header-container","nested-container",512,0,201,2
+,4,1,"header","element-in-nested-container",768,0,201,3
+# move 1 target -3 shoukd sort 1 after 4
diff --git a/Tests/Functional/Datahandler/DefaultLanguage/Fixtures/NewElement/nested_container.csv b/Tests/Functional/Datahandler/DefaultLanguage/Fixtures/NewElement/nested_container.csv
@@ -0,0 +1,10 @@
+"pages"
+,"uid","pid","title","slug"
+,1,0,"page-1","/"
+"tt_content"
+,"uid","pid","CType","header","sorting","sys_language_uid","colPos","tx_container_parent"
+,2,1,"b13-2cols-with-header-container","container",256,0,,
+,3,1,"b13-2cols-with-header-container","nested-container",512,0,201,2
+,4,1,"header","element-in-nested-container",768,0,201,3
+# create new  target -3 ->  sorted after 4
+# nochmal eins in ersten container reinmachen -> test
diff --git a/Tests/Functional/Datahandler/DefaultLanguage/MoveElementOtherPageTest.php b/Tests/Functional/Datahandler/DefaultLanguage/MoveElementOtherPageTest.php
@@ -260,8 +260,8 @@ public function moveElementAfterContainerSortElementAfterLastContainerChild(): v
         $row = $this->fetchOneRecord('uid', 4);
         $lastChild = $this->fetchOneRecord('uid', 13);
         $nextElement = $this->fetchOneRecord('uid', 14);
-        self::assertTrue($row['sorting'] > $lastChild['sorting'], 'copied element is not sorted after last child container');
-        self::assertTrue($row['sorting'] < $nextElement['sorting'], 'copied element is not sorted before containers next element');
+        self::assertTrue($row['sorting'] > $lastChild['sorting'], 'moved element is not sorted after last child container');
+        self::assertTrue($row['sorting'] < $nextElement['sorting'], 'moved element is not sorted before containers next element');
     }
 
     /**

diff --git a/Tests/Functional/Datahandler/DefaultLanguage/MoveElementTest.php b/Tests/Functional/Datahandler/DefaultLanguage/MoveElementTest.php
@@ -254,4 +254,25 @@ public function moveContainerIntoItSelfs(): void
         self::assertSame(0, (int)$row['colPos']);
         self::assertNotEmpty($this->dataHandler->errorLog, 'dataHander error log is empty');
     }
+
+    /**
+     * @test
+     */
+    public function moveElementAfterNestedContainerHasCorrectSorting(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/MoveElement/nested_container.csv');
+        $cmdmap = [
+            'tt_content' => [
+                1 => [
+                    'move' => -3,
+                ],
+            ],
+        ];
+        $this->dataHandler->start([], $cmdmap, $this->backendUser);
+        $this->dataHandler->process_datamap();
+        $this->dataHandler->process_cmdmap();
+        $movedElementRow = $this->fetchOneRecord('uid', 1);
+        $elementInNestedContainerRow = $this->fetchOneRecord('uid', 4);
+        self::assertTrue($movedElementRow['sorting'] > $elementInNestedContainerRow['sorting'], 'moved element is not sorted after element in nested container');
+    }
 }
diff --git a/Tests/Functional/Datahandler/DefaultLanguage/NewElementTest.php b/Tests/Functional/Datahandler/DefaultLanguage/NewElementTest.php
@@ -39,4 +39,29 @@ public function newElementAfterContainerSortElementAfterLastChild(): void
         $lastChildInContainer = $this->fetchOneRecord('uid', 2);
         self::assertTrue($newRecord['sorting'] > $lastChildInContainer['sorting'], 'new element is not sorted after last child in container');
     }
+
+    /**
+     * @test
+     */
+    public function newElementAfterNestedContainerSortElementAfterLastChild(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/NewElement/nested_container.csv');
+        $newId = StringUtility::getUniqueId('NEW');
+        $datamap = [
+            'tt_content' => [
+                $newId => [
+                    'pid' => -3,
+                    'colPos' => 201,
+                    'tx_container_parent' => 2,
+                ],
+            ],
+        ];
+        $this->dataHandler->start($datamap, [], $this->backendUser);
+        $this->dataHandler->process_datamap();
+        $this->dataHandler->process_cmdmap();
+
+        $newRecord = $this->fetchOneRecord('uid', 5);
+        $lastChildInContainer = $this->fetchOneRecord('uid', 4);
+        self::assertTrue($newRecord['sorting'] > $lastChildInContainer['sorting'], 'new element is not sorted after last child in container');
+    }
 }
diff --git a/Tests/Functional/Integrity/Fixtures/Sorting/nested_container.csv b/Tests/Functional/Integrity/Fixtures/Sorting/nested_container.csv
@@ -0,0 +1,10 @@
+"pages"
+,"uid","pid","title","slug"
+,1,0,"page-1","/"
+"tt_content"
+,"uid","pid","CType","header","sorting","sys_language_uid","colPos","tx_container_parent"
+,2,1,"b13-2cols-with-header-container","container",256,0,,
+,3,1,"b13-2cols-with-header-container","nested-container",512,0,201,2
+,1,1,"header","wrong-sorting",768,0,201,2,
+,4,1,"header","element-in-nested-container",1024,0,201,3
+# 1 should sort after 4
diff --git a/Tests/Functional/Integrity/SortingTest.php b/Tests/Functional/Integrity/SortingTest.php
@@ -13,6 +13,7 @@
  */
 
 use B13\Container\Domain\Factory\ContainerFactory;
+use B13\Container\Domain\Service\ContainerService;
 use B13\Container\Integrity\Database;
 use B13\Container\Integrity\Sorting;
 use B13\Container\Tca\Registry;
@@ -48,7 +49,8 @@ protected function setUp(): void
         $sortingDatabase = GeneralUtility::makeInstance(Database::class);
         $factoryDatabase = GeneralUtility::makeInstance(\B13\Container\Domain\Factory\Database::class, $context);
         $containerFactory = GeneralUtility::makeInstance(ContainerFactory::class, $factoryDatabase, $containerRegistry, $context);
-        $this->sorting = GeneralUtility::makeInstance(Sorting::class, $sortingDatabase, $containerRegistry, $containerFactory);
+        $containerService = GeneralUtility::makeInstance(ContainerService::class, $containerRegistry, $containerFactory);
+        $this->sorting = GeneralUtility::makeInstance(Sorting::class, $sortingDatabase, $containerRegistry, $containerFactory, $containerService);
     }
 
     /**
@@ -63,6 +65,20 @@ public function childBeforeContainerIsSortedAfterContainer(): void
         self::assertTrue($rows[1]['sorting'] < $rows[2]['sorting'], 'child should be sorted after container');
     }
 
+    /**
+     * @test
+     */
+    public function nestedContainer(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/Sorting/nested_container.csv');
+        $errors = $this->sorting->run(false);
+        self::assertTrue(count($errors) === 1, 'should get one error');
+        $rows = $this->getContentsByUid();
+        self::assertTrue($rows[1]['sorting'] > $rows[4]['sorting'], 'child should be sorted after container');
+        $errors = $this->sorting->run();
+        self::assertTrue(count($errors) === 1, 'no error is added error');
+    }
+
     /**
      * @test
      */

diff --git a/Tests/Functional/Integrity/SortingWithContentDefenderTest.php b/Tests/Functional/Integrity/SortingWithContentDefenderTest.php
@@ -13,6 +13,7 @@
  */
 
 use B13\Container\Domain\Factory\ContainerFactory;
+use B13\Container\Domain\Service\ContainerService;
 use B13\Container\Integrity\Database;
 use B13\Container\Integrity\Sorting;
 use B13\Container\Tca\Registry;
@@ -49,7 +50,8 @@ protected function setUp(): void
         $sortingDatabase = GeneralUtility::makeInstance(Database::class);
         $factoryDatabase = GeneralUtility::makeInstance(\B13\Container\Domain\Factory\Database::class, $context);
         $containerFactory = GeneralUtility::makeInstance(ContainerFactory::class, $factoryDatabase, $containerRegistry, $context);
-        $this->sorting = GeneralUtility::makeInstance(Sorting::class, $sortingDatabase, $containerRegistry, $containerFactory);
+        $containerService = GeneralUtility::makeInstance(ContainerService::class, $containerRegistry, $containerFactory);
+        $this->sorting = GeneralUtility::makeInstance(Sorting::class, $sortingDatabase, $containerRegistry, $containerFactory, $containerService);
     }
 
     /**