Backend reorder not working correctly

[ericp-mrel]

Winter CMS Build

1.1

PHP Version

8.0

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

There is an issue where sorting from the backend reorder list view does not work when records all contain the same/duplicate sort order value since the winter.reorder.js file does not recalculate the sort order value and re-uses the existing sort order values from the DB and just switches what record that value is applied to. This is a problem if you add sorting functionality to an existing table with records already created and then the migration assigns a default sort order value of 0 to every record and when you create a new record, it will contain the sort order of 0 which will also cause sorting for that record to stop working if it conflicts with another.

Steps to replicate

1. Create records in a table without sorting functionality.
2. Add new sort_order column to table and have the migration assign default sort_order value of 0 to every record.
3. Implement Reorder Controller functionality
4. Try to re-order list of records from backend, but notice the new order of the records is not reflected in the DB / after reloading the page.

Workaround

1. During migration, loop through every record and adjust the sort_order to be the same as the primary key.
2. When creating new records, override default sort_order so it matches the primary key as well so we don't run into duplicates.

[bennothommo]

@ericp-mrel the documentation does state there is an expectation that the developer has to assign sort order values for existing records - see the note at the bottom of the Sortable chapter here: https://wintercms.com/docs/database/traits#sortable

Given that they would need to create a migration in the first place (to add the sort_order column to the model's table), that would be the place to put the query to assign sort orders.

[ericp-mrel]

Yeah, sorry I see it now that you point it out. I still personally feel it would be a bit of a nicer experience to not have to do a migration like that assigning sort order values in order for it to actually work from the backend UI.

I'll let you guys decide on if that's something you'd be willing to consider changing, otherwise you can close this ticket since as you pointed out in the documentation this isn't really a bug, but could be more of an enhancement.

I'm not sure if this would help (or maybe it's just me), but it might make the note more obvious and noticeable to move it closer to the example migration snippet line.

[bennothommo]

@ericp-mrel we'll still consider the PR, I was just making a note that it is documented.

As for the positioning of that notice in the documentation, I think the location is right, it just doesn't stand out enough (like most notes on the current docs). I'm hoping to have these new docs online soon, where it's a bit more apparent ;)