Lists: add deterministic ordering to remaining paginated lists - #13214
Draft
ericholscher wants to merge 2 commits into
Draft
Lists: add deterministic ordering to remaining paginated lists#13214ericholscher wants to merge 2 commits into
ericholscher wants to merge 2 commits into
Conversation
Several dashboard lists and API endpoints paginate querysets with no ordering, so rows can appear duplicated or missing across pages (same bug class as the subprojects list). Add Meta.ordering to the models involved, order User-backed lists by username, and add pk tiebreakers to sorts where many rows tie (never-built projects/versions, pending team invites). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB6HHBjvZJsSgYC1aL1N2p
Inherit TimeStampedModel.Meta on EnvironmentVariable and Integration so the new ordering doesn't drop get_latest_by, and update the notifications queryset test for the newest-first ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB6HHBjvZJsSgYC1aL1N2p
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #13210. Auditing every paginated list after that fix showed the subprojects page was one instance of a systemic pattern: the dashboard's shared list template and the APIs paginate whatever queryset a view provides, and several of those querysets have no ordering at all. Paginating an unordered queryset issues a separate
LIMIT/OFFSETquery per page with noORDER BY, so rows can appear duplicated on one page and missing from another while the total count stays correct. DRF'sLimitOffsetPaginationslices the queryset directly, so the API endpoints never even emitUnorderedObjectListWarning.Unordered lists fixed via
Meta.ordering(no-opAlterModelOptionsmigrations, markedSafe.after_deploy()):Notification(-created, newest first) — the five APIv3 notification list endpointsEnvironmentVariable(name) — dashboard list and APIv3 endpointTeam(name,pktiebreaker sincenameis only unique per organization) — dashboard and APIv3 teams endpointsWebHook,EmailHook,Integration,OrganizationOwner(pk) — their dashboard listsUnordered lists fixed in views (models we don't own):
Userhas no default ordering; both now order by the uniqueusernameSocialAccountlists (GitHub App migration page,/api/v2/remote/account/): ordered bypkTie-vulnerable sorts that behave like the full bug in realistic conditions, fixed with a
pktiebreaker:nulls_last— every never-built project/version ties on NULL, so any user with more than one page of never-built items got an undefined-order tailmemberWhy some orderings are
pkand others-pk?Direction never matters for the bug — any deterministic total order fixes pagination — so it's chosen per list for display:
pk= creation order, which also matches the insertion order these lists effectively rendered before, so users see no reshuffle.-pk, matching the primary sort's direction: notifications are-created, -pk(newest first even for same-instant rows), and the dashboard/version latest-build sorts append-pkso a just-created project surfaces at the top of the never-built tail instead of under years-old items.("name", "pk")for teams and environment variables, andpkafter the ascending member sort for team members — duplicate names and pending invites list oldest first, consistent with an A→Z list.The notifications golden response file is reordered because notifications now list newest-first. Remaining tie-vulnerable orderings with only microsecond-collision exposure (builds by
-date, audit logs by-created, versions by-verbose_name, remote repositories/organizations by name) are left as-is, as is a possiblequeryset.orderedguardrail in a shared pagination class. The corporate repo needs the same audit for its own views (temporary-access and SSH-key lists, organizations viewset).🤖 Generated with Claude Code
https://claude.ai/code/session_01BB6HHBjvZJsSgYC1aL1N2p
Generated by Claude Code