Reorder function

[Jianbinzhu]

Updated reorder function to account for when there are duplicate Sort values of elements within an ElementalArea

[ScopeyNZ]

What are we trying to fix here? From what I'm reading, the existing code already supports the situation where the current sort order matches the intended sort order, as it will decrement the sort of every element with a sort less than or equal to it.

I really don't like code that introduces an arbitrary number of queries - and this code has been specifically written to avoid more than one.

[Jianbinzhu]

@ScopeyNZ I came across an issue where elements under the same ParentID have duplicate Sort value, and I can't seem to replicate that anymore. To fix existing records, I've created a sort all elements function. Please advise if there's a better way of doing this, thanks.

[ScopeyNZ]

I don't really like the idea of a side effect of somebody reordering their elements is a long running "fix the whole thing", that also doesn't have any guarantee of respecting the order they chose.

If there's a bug in this code which will mess up the sort order then we should address that, but other than that, I'm thinking we shouldn't have the code be concerned with garbage data.

[brynwhyman]

This just came up in a different context, possibly a bit more information about the bug at play: #849 (comment)

[chrispenny]

Just adding my $0.02.

As Bryn mentioned, I experienced this issue here #849 (comment)

I think we could avoid a lot of these instances simply by making sure that we don't set a Sort value on our blocks until they actually have a Parent.

There could be some other instances where a block gets a duplicate Sort, but so far, it's always been Sort: 1, and it's because the Block model was saved before the Parent was set.

[Jianbinzhu]

@brynwhyman @chrispenny Just found out that this solution is only partially working 😂 , it doesn't work when when have 2 elements with sort value of 1 and then trying to drag the second element to the top. $elementToBeAfterID on ReorderElements:L72 will be 0 as the element to be after will be none. Which means that $sortAfterPosition will stay 0 and then we will be comparing $sortAfterPosition 0 to $currentPosition 1 and it won't hit that else condition. I've created a task to fix the broken elements instead for now.

Another thing I found is that the duplicated sort value is caused by when an Elemental Block is archived and then restored

[chrispenny]

@Jianbinzhu good find with the archive restoration. Based on @ScopeyNZ's comment, this would be an area where we would want to fix the "restoration process" itself.

EG: Maybe we have it set the Sort to the max Sort value + 1 from that Parent (put it down the bottom) any time we bring an archived block back to life.

[ScopeyNZ]

In regards to the sort order on archive/publish. I think it would make sense to clear the sort order value when a block is archived. I'll raise an issue for sort being broken as elements are restored from the archive.

WRT this PR, I'm going to close it as I don't think it's the solution we're looking for. Thanks for the help getting to the bottom if this issue!