Sort may get (and stay) 'corrupted' by multiple blocks with identical sort values

[micschk]

I don't know exactly how the client managed it, but we have an Elemental based site in which multiple blocks have a sort-order of 1. This breaks the sorting logic as these blocks cannot be moved anywhere else than to the top anymore. I found related issues #849 / #934 / #931 but this one may be slightly different.

The cause seems to be the logic of limiting the reordering operation to blocks between 'current sort position' and 'new sort position' in DNADesign\Elemental\Services\ReorderElements::reorder, which runs an SQL query like:
UPDATE "Element" SET "Sort" = "Element"."Sort" - 1 WHERE ("Element"."Sort" <= 5 AND "Element"."Sort" > 1) AND ("Element"."ParentID" = ?) 74, )>

In case of multiple blocks with an existing sort of 1, they're 'stuck' at one as they're not processed in the reordering (as that's limited to blocks with sorts higher than 1)... And each time you move a block with Sort of 1 to a different position, the block with Sort=2 will get reassigned a Sort of 1.

In one of the related issues, a proposed solution of re-applying a new Sort value to all blocks gets dismissed as sort of being a symptom fix instead of real fix. But there may be various situations in which duplicate Sort values occur which we can not all predict (and these may also be the result of intended actions, eg reassigning a block to a different Elemental Area).

As a more general fix, would it make sense to change the logic in DNADesign\Elemental\Services\ReorderElements::reorder from 'current sort -/+ 1' to an incrementing value? Eg assign a new value to Sort which always increments/decrements instead of relying on the current value +/- 1 (which persists duplicates once there are any). This is basically the solution proposed in #931 but then as a direct SQL query to prevent multiple writes.

This may be a bit less clever/precise than the current logic but any duplicate Sort values would automatically 'heal' and at least the result would match what the user intended.

[micschk]

For anyone looking for a quick solution, I've added a check/fix for this situation to Page::onBeforeWrite;

protected function onBeforeWrite()
    {
        parent::onBeforeWrite();

        // Fix Elemental Sort values in case of duplicate values
        if($this->ElementalAreaID) {
            $Element_ID_Sort_map = BaseElement::get()->filter('ParentID', $this->ElementalAreaID)->sort('Sort', 'asc')->map('ID', 'Sort')->toArray();
            if(count(array_unique($Element_ID_Sort_map)) < count($Element_ID_Sort_map)){

                // Update both the draft and live versions of the records
                $baseTableName = Convert::raw2sql(DataObject::getSchema()->tableName(BaseElement::class));
                $tableNames = [$baseTableName];

                if (BaseElement::has_extension(Versioned::class)) {
                    /** @var BaseElement&Versioned $element */
                    $tableNames[] = DataObject::singleton(BaseElement::class)->element->stageTable($baseTableName, Versioned::LIVE);
                }

                foreach ($tableNames as $tableName) {
                    $sort = 1;
                    foreach (array_keys($Element_ID_Sort_map) as $ElementID) {
                        $query = SQLUpdate::create()
                            ->setTable("$tableName")
                            ->assign('"Sort"', $sort++)
                            ->addWhere(["$tableName.\"ID\"" => $ElementID, "$tableName.\"ParentID\"" => $this->ElementalAreaID]);
                        $query->execute();
                    }
                }
            }
        }
    }