Edit: The previous plugin started its menu_order at 0 (zero), so I suspect the is_already_sequential check returned false and therefore caused a reindexed.
I suspect this is what @martinsauter was reporting here https://wordpress.org/support/topic/please-respect-existing-order/
Maybe on the initial re-indexing, ordering by menu_order first to preserve the “current order” could work? If the menu_order has never been set it will be 0 (zero) or NULL and so the other fields included in the query will be used for the sort.
Hi Jamie,
You’re right, and your follow-up diagnosis was spot on. Thank you — this one had been reported before and your detail is what made it obvious.
The plugin seeded a newly enabled post type by title (for pages) or date (for everything else), ignoring menu_order completely. So the values your previous plugins left behind were simply overwritten. And it was worse than “on enable”: that code runs for every enabled type on every settings save, so merely revisiting the settings screen could flatten the order again.
Your suggestion — order by menu_order first and let the other fields break the tie — is exactly the right shape, and that’s essentially what 2.8.8 does. One refinement: rather than always sorting on menu_order first, it checks whether there is anything to preserve at all. If every row of the type shares a single value (an untouched site, everything at 0) it seeds from scratch as before; otherwise it follows the existing sequence.
The reason for that distinction is a second bug your report led me to. refresh(), which tidies the numbering when you open a list screen, breaks ties on ID. The seeding used a title/date tie-break. Both renumber the same rows, and a site hits whichever runs first — so two items sharing a menu_order could settle one way after a settings save and the other way after simply opening the list. Both now break ties on ID.
Worth flagging honestly: 2.8.7 briefly made the original bug easier to hit. Tightening the “is this already sequential?” check meant more sites reached the seeding code — which was fine in itself, except the seeding was destructive. That’s fixed at the root now rather than by narrowing the check again.
2.8.8 is out. Thanks again — three of your reports are in the last two releases.
Thank you so much for taking the time to respond and fix the issue so quickly. I just tested this on a website with a zero-based index and after saving the settings page the plugin re-indexed one-based and preserved the existing order.
I’m curious if this code simplification would have worked:
$seed_order = 'menu_order ASC, ' . ( 'page' === $object ? 'post_title ASC, ID ASC' : 'post_date DESC, ID DESC' );
Thanks again.
Hi Jamie,
Great to hear it held up on a real zero-based site – thanks for testing so quickly.
Your one-liner is genuinely elegant, and in the two common cases it produces exactly the same result as the shipped code. On an untouched site every row shares one menu_order, so the leading menu_order ASC is a constant and the title/date tie-breakers decide everything — identical to the seed-from-scratch branch. And when an existing order has distinct values, menu_order ASC decides everything on its own — identical to the preserve branch.
The two versions only diverge when the existing values contain duplicates, and that’s the one case I wanted pinned down. Duplicates really do occur in the wild – imports, direct SQL, and until 2.8.7 any post in a status registered by another plugin was silently skipped by the renumbering, which manufactured them (that was the “dragging two posts does nothing” thread). For a tied pair, your composite breaks the tie on title/date, while refresh() – the tidy-up that runs when a list screen loads – breaks it on ID. Both paths repair the same rows and a site simply hits whichever runs first, so with the composite a tied pair would land in title order if the settings save got there first and in ID order if a list screen did. It wouldn’t flip back and forth (once repaired, both paths skip the type), but the final order would depend on which admin screen happened to load first, and that path-dependence is the exact class of bug 2.8.8 was meant to close. With the two branches, the preserve branch’s ORDER BY is verbatim the same as refresh()‘s, so the two paths provably agree whatever the input.
The other direction – putting your composite into refresh() too, so they’d agree that way – would work, but then merely opening a list screen could reorder tied rows by title/date, and refresh() would need the same per-type conditional. ID is boring but stable (titles and dates can be edited), so I kept ID as the shared tie-break and wore the extra branch as the cost of making the invariant obvious.
So: considered, and it was a near thing – the branch survives purely for the duplicates case. Thanks again for the testing and for thinking about the code itself; it’s rare and very welcome.
Thank you for taking the time to explain and indulge my curiosity.
Hands down the best replies I’ve ever seen from any support thread. That and the code quality gives me confidence that I chose well including this plugin in my stack.
