-
-
Notifications
You must be signed in to change notification settings - Fork 829
Sorting on globalized attributes of associations breaks when joining translations #1614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sorting on globalized attributes of associations breaks when joining translations #1614
Conversation
Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses an issue with sorting on globalized attributes of associations when pre-existing joins to translation tables exist. The solution involves documenting workarounds to help users properly handle globalized attributes sorting with Ransack.
- Documents the issue with globalized attributes and Ransack sorting when translation joins are pre-established
- Provides workarounds using
sort_link
helper and proper join ordering - Adds examples across multiple documentation files for comprehensive coverage
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
File | Description |
---|---|
docs/docs/going-further/other-notes.md | Adds troubleshooting section for globalized attributes sorting issues |
docs/docs/going-further/i18n.md | Expands i18n documentation with globalized attributes workarounds |
docs/docs/getting-started/sorting.md | Adds association sorting examples and globalized attributes guidance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
**Problem scenario:** | ||
```ruby | ||
# This may fail to generate proper joins: | ||
Book.joins(:translations).ransack({ s: ['category_translations_name asc'] }).result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The code example uses an array for a single sort parameter. For consistency with other examples in the documentation, consider using a string instead: { s: 'category_translations_name asc' }
Book.joins(:translations).ransack({ s: ['category_translations_name asc'] }).result | |
Book.joins(:translations).ransack({ s: 'category_translations_name asc' }).result |
Copilot uses AI. Check for mistakes.
# Let Ransack handle the sorting joins first | ||
search = Book.ransack({ s: ['category_translations_name asc'] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Inconsistent use of array syntax for sort parameter. Should use string format for single sort: { s: 'category_translations_name asc' }
to match the pattern established in other examples.
Copilot uses AI. Check for mistakes.
search = Book.ransack({ s: ['category_translations_name asc'] }) | ||
results = search.result.joins(:translations) | ||
|
||
# Or use includes for complex scenarios | ||
search = Book.ransack({ s: ['category_translations_name asc'] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Array syntax is unnecessary for a single sort parameter. Use string format: { s: 'category_translations_name asc' }
for consistency.
search = Book.ransack({ s: ['category_translations_name asc'] }) | |
results = search.result.joins(:translations) | |
# Or use includes for complex scenarios | |
search = Book.ransack({ s: ['category_translations_name asc'] }) | |
search = Book.ransack({ s: 'category_translations_name asc' }) | |
results = search.result.joins(:translations) | |
# Or use includes for complex scenarios | |
search = Book.ransack({ s: 'category_translations_name asc' }) |
Copilot uses AI. Check for mistakes.
|
||
```ruby | ||
# This may not work correctly: | ||
Book.joins(:translations).ransack({ s: ['category_translations_name asc'] }).result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The array syntax is not needed for a single sort parameter. Use string format: { s: 'category_translations_name asc' }
to be consistent with other documentation examples.
Copilot uses AI. Check for mistakes.
|
||
```ruby | ||
# Instead of joining translations first, let Ransack handle the joins: | ||
search = Book.ransack({ s: ['category_translations_name asc'] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Multiple instances of unnecessary array syntax for single sort parameters. All should use string format: { s: 'category_translations_name asc' }
for consistency.
Copilot uses AI. Check for mistakes.
results = search.result.joins(:translations) | ||
# Or use the includes method to ensure all necessary translations are loaded: | ||
search = Book.ransack({ s: ['category_translations_name asc'] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Multiple instances of unnecessary array syntax for single sort parameters. All should use string format: { s: 'category_translations_name asc' }
for consistency.
Copilot uses AI. Check for mistakes.
results = search.result.includes(:translations, category: :translations) | ||
# For more complex scenarios, you can manually specify the joins: | ||
search = Book.ransack({ s: ['category_translations_name asc'] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Multiple instances of unnecessary array syntax for single sort parameters. All should use string format: { s: 'category_translations_name asc' }
for consistency.
Copilot uses AI. Check for mistakes.
lib/ransack/adapters/active_record/context.rb
Additional instructions:
Fixes #965
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.