moveToStart updates wrong rows

[quelo83]

Hi,
the method "moveToStart" doesn't seem to work well. It increments all rows except the current model and this is not correct.
The current code is:

$this->$orderColumnName = $firstModel->$orderColumnName;
$this->save();

$this->buildSortQuery()->where($this->getKeyName(), '!=', $this->getKey())->increment($orderColumnName);

If I'm not wrong, the query should have another where clause to scope and increment only the previous rows, something like this:

$previousOrder = $this->$orderColumnName;

$this->$orderColumnName = $firstModel->$orderColumnName;
$this->save();

$this->buildSortQuery()
    ->where($this->getKeyName(), '!=', $this->getKey())
    ->where($this->getKeyName(), '<', $previousOrder)
    ->increment($orderColumnName);

Please, let me know if I'm missing something.

Thanks in advance!

[mokhosh]

This makes sense, but it shouldn't break the sort, right? The only issue is you will have a gap like 1,2,3,5,6.

[quelo83]

Yes, you are right, but I think this would make the order data more clean and readable and also will allow users to use this field as a "visible" order number.

I mean, in my case I have a list of courses that have lessons, and the order of the lessons is also the "lesson number". The first lesson must have the number 1, the second the number 2 and so on.
So, it would be great if from backend I could "move" a lesson inside a course and the numbers (orders) would remain as an ordered list.

And at the end, this is what "moveToEnd" method does if you see the code.

$oldOrder = $this->$orderColumnName;

$this->$orderColumnName = $maxOrder;
$this->save();

$this->buildSortQuery()->where($this->getKeyName(), '!=', $this->getKey())
    ->where($orderColumnName, '>', $oldOrder)
    ->decrement($orderColumnName);

What do you think? Is this a possible change?

[mokhosh]

Seems possible. We need to make the change, pass the tests, and make a PR.
The question is, will @freekmurze be willing to merge this change?

[freekmurze]

For me, having gaps in the order is not a problem, and I see solving that problem out of scope for the package.