[BUGFIX] Fix drop-at-top sorting inversion (#738)

Classes/Hooks/Datahandler/CommandMapBeforeStartHook.php

@@ -15,6 +15,7 @@
 use B13\Container\Domain\Factory\ContainerFactory;
 use B13\Container\Domain\Factory\Exception;
 use B13\Container\Domain\Service\ContainerService;
+use B13\Container\Integrity\Sorting;
 use B13\Container\Tca\Registry;
 use Symfony\Component\DependencyInjection\Attribute\Autoconfigure;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
@@ -26,7 +27,8 @@ public function __construct(
         protected ContainerFactory $containerFactory,
         protected Registry $tcaRegistry,
         protected Database $database,
-        protected ContainerService $containerService
+        protected ContainerService $containerService,
+        protected Sorting $sorting
     ) {
     }
 
@@ -162,6 +164,7 @@ protected function rewriteCommandMapTargetForTopAtContainer(array $cmdmap): arra
                         continue;
                     }
 
+                    // Path 1: drop-at-top rewritten from a positive page-uid target.
                     if (
                         isset($value['update']) &&
                         isset($value['update']['tx_container_parent']) &&
@@ -173,11 +176,40 @@ protected function rewriteCommandMapTargetForTopAtContainer(array $cmdmap): arra
                         try {
                             $container = $this->containerFactory->buildContainer((int)$value['update']['tx_container_parent']);
                             $target = $this->containerService->getNewContentElementAtTopTargetInColumn($container, (int)$value['update']['colPos']);
+                            if ($target === -$container->getUid()) {
+                                // Fallback anchor active. Ensure the invariant
+                                // container.sorting < min(child.sorting) so DataHandler
+                                // computes the new sorting within the children's sort space
+                                // instead of around the container record itself (see #738).
+                                $this->sorting->normalizeChildrenSortingForContainer($container);
+                            }
                             $cmd[$operation]['target'] = $target;
                         } catch (Exception $e) {
                             // not a container
                         }
                     }
+
+                    // Path 2: command that already arrives with target = -container_uid.
+                    // The Path 1 branch skips this case (target > 0 gate), so the
+                    // -container_uid anchor reaches DataHandler unchanged. Ensure the
+                    // invariant here as well (see #738).
+                    if (
+                        isset($value['update']) &&
+                        isset($value['update']['tx_container_parent']) &&
+                        $value['update']['tx_container_parent'] > 0 &&
+                        isset($value['update']['colPos']) &&
+                        $value['update']['colPos'] > 0 &&
+                        isset($value['target']) &&
+                        (int)$value['target'] < 0 &&
+                        abs((int)$value['target']) === (int)$value['update']['tx_container_parent']
+                    ) {
+                        try {
+                            $container = $this->containerFactory->buildContainer((int)$value['update']['tx_container_parent']);
+                            $this->sorting->normalizeChildrenSortingForContainer($container);
+                        } catch (Exception $e) {
+                            // not a container
+                        }
+                    }
                 }
             }
         }

Classes/Integrity/Sorting.php

@@ -17,11 +17,20 @@
 use B13\Container\Domain\Model\Container;
 use B13\Container\Domain\Service\ContainerService;
 use B13\Container\Tca\Registry;
+use TYPO3\CMS\Core\Database\Connection;
+use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 class Sorting
 {
+    /**
+     * Default sort interval used when renumbering children. Matches
+     * DataHandler::$sortIntervals (which is instance state, not a constant,
+     * so we mirror it here).
+     */
+    public const SORT_INTERVAL = 256;
+
     protected array $errors = [];
 
     public function __construct(
@@ -65,6 +74,86 @@ protected function unsetContentDefenderConfiguration(string $cType): void
         }
     }
 
+    /**
+     * Renumber the container's children so that all sorting values sit strictly
+     * above the container's own sorting, preserving the configured container
+     * column order (`tcaRegistry` order) and the within-column sorting order.
+     *
+     * Intended for the drop-at-top path in `CommandMapBeforeStartHook`: when
+     * the `-container_uid` fallback anchor is used, the invariant
+     * `container.sorting < min(child.sorting)` must hold, otherwise
+     * DataHandler computes the moved record's new sorting from the container's
+     * own page-level sorting instead of the children's sort space
+     * (see #738).
+     *
+     * The read side reuses the `Container` model for language and configured
+     * column-order scoping.
+     *
+     * Writes are direct `Connection::update()`s of `sorting` and `tstamp` —
+     * intentionally not routed through DataHandler to avoid re-entering the
+     * hook stack from within a hook. Only touches the container's own direct
+     * children.
+     *
+     * Workspaces are intentionally out of scope for this initial fix: the
+     * method short-circuits when the current BE user is not in workspace 0.
+     * The reason is that in a non-live workspace `ContainerFactory` can
+     * return live records for children that have no workspace overlay yet;
+     * a direct SQL update would then silently mutate live rows from a
+     * workspace DataHandler run. The workspace path deserves its own,
+     * overlay-aware follow-up.
+     */
+    public function normalizeChildrenSortingForContainer(Container $container): void
+    {
+        $workspace = (int)($GLOBALS['BE_USER']->workspace ?? 0);
+        if ($workspace !== 0) {
+            // Non-live workspace: skip. See method docblock for rationale.
+            return;
+        }
+
+        $containerRecord = $container->getContainerRecord();
+        $containerSorting = (int)$containerRecord['sorting'];
+        $cType = $container->getCType();
+
+        $orderedChildren = [];
+        foreach ($this->tcaRegistry->getAvailableColumns($cType) as $column) {
+            $children = $container->getChildrenByColPos((int)$column['colPos']);
+            foreach ($children as $child) {
+                $orderedChildren[] = $child;
+            }
+        }
+
+        if ($orderedChildren === []) {
+            return;
+        }
+
+        $minChildSorting = null;
+        foreach ($orderedChildren as $child) {
+            $sorting = (int)$child['sorting'];
+            if ($minChildSorting === null || $sorting < $minChildSorting) {
+                $minChildSorting = $sorting;
+            }
+        }
+        if ($minChildSorting !== null && $minChildSorting > $containerSorting) {
+            return;
+        }
+
+        $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('tt_content');
+        $newSorting = $containerSorting + self::SORT_INTERVAL;
+        $tstamp = time();
+        foreach ($orderedChildren as $child) {
+            $childUid = (int)$child['uid'];
+            if ((int)$child['sorting'] !== $newSorting) {
+                $connection->update(
+                    'tt_content',
+                    ['sorting' => $newSorting, 'tstamp' => $tstamp],
+                    ['uid' => $childUid],
+                    ['sorting' => Connection::PARAM_INT, 'tstamp' => Connection::PARAM_INT, 'uid' => Connection::PARAM_INT]
+                );
+            }
+            $newSorting += self::SORT_INTERVAL;
+        }
+    }
+
     protected function fixChildrenSortingUpdateRequired(Container $container, array $colPosByCType): bool
     {
         $containerRecord = $container->getContainerRecord();

Tests/Functional/Datahandler/DefaultLanguage/Fixtures/MoveElementSortingInverted/MoveIntoInvertedContainerAtTopResult.csv

@@ -0,0 +1,9 @@
+"pages"
+,"uid","pid","title"
+,1,0,"page-1"
+"tt_content"
+,"uid","pid","CType","header","sorting","sys_language_uid","colPos","tx_container_parent"
+,1,1,"b13-2cols-with-header-container","container",10000,0,0,0
+,2,1,"header","child A",10256,0,200,1
+,3,1,"header","child B",10512,0,200,1
+,4,1,"header","test subject",10128,0,200,1

Tests/Functional/Datahandler/DefaultLanguage/Fixtures/MoveElementSortingInverted/setup.csv

@@ -0,0 +1,9 @@
+"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,"b13-2cols-with-header-container","container",10000,0,0,0
+,2,1,"header","child A",100,0,200,1
+,3,1,"header","child B",200,0,200,1
+,4,1,"header","test subject",10500,0,0,0

Tests/Functional/Datahandler/DefaultLanguage/MoveElementSortingInvertedTest.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace B13\Container\Tests\Functional\Datahandler\DefaultLanguage;
+
+/*
+ * This file is part of TYPO3 CMS-based extension "container" by b13.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ */
+
+use B13\Container\Tests\Functional\Datahandler\AbstractDatahandler;
+use PHPUnit\Framework\Attributes\Test;
+
+/**
+ * Regression tests for #738 — drop-at-top of a container child column places
+ * the moved element outside the children's sort space when
+ * container.sorting > min(child.sorting). The hook now normalises the
+ * children's sorting before the -container_uid fallback anchor is used, so
+ * the moved element lands at the top of the children as intended.
+ */
+class MoveElementSortingInvertedTest extends AbstractDatahandler
+{
+    #[Test]
+    public function moveIntoInvertedContainerAtTopViaNegativeTarget(): void
+    {
+        // Path 2: cmd already carries target = -container_uid.
+        // Upstream, the sorting rewrite skipped this branch because of the
+        // `target > 0` gate, so the anchor reached DataHandler unchanged.
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/MoveElementSortingInverted/setup.csv');
+        $cmdmap = [
+            'tt_content' => [
+                4 => [
+                    'move' => [
+                        'action' => 'paste',
+                        'target' => -1,
+                        'update' => [
+                            'colPos' => 200,
+                            'tx_container_parent' => 1,
+                            'sys_language_uid' => 0,
+                        ],
+                    ],
+                ],
+            ],
+        ];
+        $this->dataHandler->start([], $cmdmap, $this->backendUser);
+        $this->dataHandler->process_datamap();
+        $this->dataHandler->process_cmdmap();
+        self::assertCSVDataSet(__DIR__ . '/Fixtures/MoveElementSortingInverted/MoveIntoInvertedContainerAtTopResult.csv');
+    }
+
+    #[Test]
+    public function moveIntoInvertedContainerAtTopViaPositivePageTarget(): void
+    {
+        // Path 1: cmd arrives with target = positive page uid + update carries
+        // tx_container_parent + colPos. The rewrite path resolves this via
+        // ContainerService::getNewContentElementAtTopTargetInColumn(), which
+        // falls back to -container_uid for the first configured column when
+        // no preceding column has records.
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/MoveElementSortingInverted/setup.csv');
+        $cmdmap = [
+            'tt_content' => [
+                4 => [
+                    'move' => [
+                        'action' => 'paste',
+                        'target' => 1,
+                        'update' => [
+                            'colPos' => 200,
+                            'tx_container_parent' => 1,
+                            'sys_language_uid' => 0,
+                        ],
+                    ],
+                ],
+            ],
+        ];
+        $this->dataHandler->start([], $cmdmap, $this->backendUser);
+        $this->dataHandler->process_datamap();
+        $this->dataHandler->process_cmdmap();
+        self::assertCSVDataSet(__DIR__ . '/Fixtures/MoveElementSortingInverted/MoveIntoInvertedContainerAtTopResult.csv');
+    }
+}