Skip to content

Add primary key tiebreaker for stable sorting - #2886

Merged
pablobm merged 2 commits into
thoughtbot:mainfrom
goosys:stable-sort-with-primary-key-tiebreaker
Nov 29, 2025
Merged

pablobm merged 2 commits into
thoughtbot:mainfrom
goosys:stable-sort-with-primary-key-tiebreaker

Conversation

@goosys

@goosys goosys commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

This pull request improves the stability of sorting in Administrate by introducing a tiebreaker using the primary key.

When ordering by a column, the primary key is now used as a secondary sort key to ensure stable and predictable results.
If the relation does not have a primary key, the tiebreaker is skipped.

Changes:

  • Add the primary key as a tiebreaker when ordering by a column
  • Skip the tiebreaker if the relation has no primary key
  • Update and add specs to cover these scenarios

Motivation:

Without a tiebreaker, sorting by a non-unique column can result in unpredictable order for records with the same value.
This can cause issues such as the order changing on every page load, or the same record appearing on multiple pages when paginating.
By using the primary key as a tiebreaker, we guarantee a stable sort order and prevent these problems.

Please review!

@goosys
goosys force-pushed the stable-sort-with-primary-key-tiebreaker branch from bef286f to 7cd3015 Compare September 19, 2025 03:23
@nickcharlton

Copy link
Copy Markdown
Member

Oh, fun! Could you add something to the docs on this as well?

@goosys
goosys force-pushed the stable-sort-with-primary-key-tiebreaker branch from 0bd8a6b to 9d728cc Compare October 8, 2025 11:24
@goosys

goosys commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

I've added the docs! How does this look?

@goosys

goosys commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

By the way, do you think the tiebreak logic should live inside apply, or would it be better to split it out into its own method?

@nickcharlton nickcharlton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The docs look good to me. I just have a preference for capitalising the keywords on SQL statements!

Comment thread docs/guides/stable_sorting.md Outdated

```sql
-- When toggling the name attribute
select * from users order by name desc, id desc;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
select * from users order by name desc, id desc;
SELECT * FROM users ORDER BY name DESC, id DESC;

Comment thread docs/guides/stable_sorting.md Outdated
select * from users order by name desc, id desc;

-- When toggling the name attribute again
select * from users order by name asc, id asc;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
select * from users order by name asc, id asc;
SELECT * FROM users ORDER BY name ASC, id ASC;

Comment thread docs/guides/stable_sorting.md Outdated

```sql
-- When toggling the name attribute
select * from users order by name desc;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
select * from users order by name desc;
SELECT * FROM users ORDER BY name DESC;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed the changes!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@nickcharlton

Copy link
Copy Markdown
Member

@pablobm, do you have any thoughts on this one? I think you might've spent more time in this part of the code base than I have.

@goosys
goosys force-pushed the stable-sort-with-primary-key-tiebreaker branch from 9d728cc to be1cf4d Compare October 14, 2025 12:44
@pablobm

pablobm commented Oct 17, 2025

Copy link
Copy Markdown
Collaborator

Always tricky with these things, but... this looks good to me 🙂

The one thing I'm not sure is if the guides is the right place to put this documentation. In my mind the guides is a place to put "recipes" that require configuration or code, while here we are just explaining how the UI works. Having said that, I don't know what a good place would be 🤔 Thoughts?

@pablobm

pablobm commented Oct 20, 2025

Copy link
Copy Markdown
Collaborator

Crazy idea, re: where to put this documentation - Perhaps it should be a comment in the controller template. Similar to the existing ones. Perhaps we should add comments for default_sorting_attribute/default_sorting_direction and add this documentation there.

Just a thought! Not 100% convinced myself.

@goosys

goosys commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for your comment!
As you mentioned, I was also wondering if it would be appropriate to put this content in the guides/.
Writing it in the controller's template also seems like a good idea. Hmm.

I've previously read comments from everyone wanting to enrich the documentation, so I'm keen to write more in the docs/.
With the current hierarchical structure, there isn't a place to put it, but perhaps we could create a hierarchical level specifically for this kind of documentation.
Everyone would want documentation explaining the implicit aspects of Administrate.

@pablobm

pablobm commented Nov 7, 2025

Copy link
Copy Markdown
Collaborator

@goosys - Sounds good 🙂 I think it makes sense to create a new doc under docs/ to be linked from the main docs sidebar. Something like "Using Administrate" or similar.

@goosys
goosys force-pushed the stable-sort-with-primary-key-tiebreaker branch from be1cf4d to 1ebc6ce Compare November 27, 2025 13:51
@goosys
goosys force-pushed the stable-sort-with-primary-key-tiebreaker branch from 1ebc6ce to d35f964 Compare November 27, 2025 14:02
@goosys

goosys commented Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

Added new item to sidebar.

image

@pablobm pablobm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with this.

@pablobm
pablobm merged commit 4ffbef3 into thoughtbot:main Nov 29, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants