Skip to content

Sort nil values last in Relation#order instead of raising - #214

Open
pcbeingused333 wants to merge 1 commit into
Rails-Designer:mainfrom
pcbeingused333:fix/relation-order-nil
Open

pcbeingused333 wants to merge 1 commit into
Rails-Designer:mainfrom
pcbeingused333:fix/relation-order-nil

Conversation

@pcbeingused333

Copy link
Copy Markdown

The bug

Content::Post.all.order(:published_at)
# => ArgumentError: comparison of NilClass with Time failed

Relation#order is sort_by { it.public_send(attribute) }, so one resource with a nil value for the ordering attribute — an optional frontmatter field like a date, a position, or a category — brings the whole query down. #in_order_of already guards this (|| Float::INFINITY in the filter: false branch); #order didn't.

The fix

Partition nil values out of the comparison and append them last, in their original relative order, for both :asc and :desc.

Tests

test/perron/relation_test.rb — a Relation with two nil positions among the values, asserting order for asc and desc. Fails on main (ArgumentError). bundle exec rake is green (402 runs, 0 failures; standardrb clean).

🤖 Generated with Claude Code

`order` did `sort_by { it.public_send(attribute) }`, so a single resource
whose ordering attribute is `nil` — a common case for optional frontmatter
like a date or a position — raised `ArgumentError: comparison of NilClass
with ... failed` and took the whole query down.

`nil` values are now partitioned out of the sort and appended last, in
their original relative order, for both directions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
@pcbeingused333

Copy link
Copy Markdown
Author

CI is on action_required (fork gate). Verified locally with bundle exec rake (Docker, Ruby 3.4): 402 runs, 0 failures, standardrb clean. The new test fails on main with ArgumentError: comparison of Integer with nil failed.

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.

1 participant