diff --git a/docs/docs/getting-started/sorting.md b/docs/docs/getting-started/sorting.md index c0619b1db..1f465119d 100644 --- a/docs/docs/getting-started/sorting.md +++ b/docs/docs/getting-started/sorting.md @@ -69,3 +69,41 @@ class PostsController < ActionController::Base end end ``` + +## Sorting on Association Attributes + +You can sort on attributes of associated models by using the association name followed by the attribute name: + +```ruby +# Sort by the name of the associated category +@q = Post.ransack(s: 'category_name asc') +@posts = @q.result + +# Sort by attributes of nested associations +@q = Post.ransack(s: 'category_section_title desc') +@posts = @q.result +``` + +### Sorting on Globalized/Translated Attributes + +When working with internationalized models (like those using the Globalize gem), special care is needed when sorting on translated attributes of associations. The simplest approach is to use the `sort_link` helper directly with the translation attribute: + +```erb + +<%= sort_link @q, :translations_name %> +<%= sort_link @q, :category_translations_name %> +``` + +For programmatic sorting, let Ransack handle the joins first: + +```ruby +# Let Ransack establish the necessary joins for sorting +@q = Book.ransack(s: 'category_translations_name asc') +@books = @q.result.joins(:translations) + +# For complex scenarios with multiple translations +@q = Book.ransack(s: 'category_translations_name asc') +@books = @q.result.includes(:translations, category: :translations) +``` + +This ensures that Ransack properly handles the join dependencies between your main model's translations and the associated model's translations. diff --git a/docs/docs/going-further/i18n.md b/docs/docs/going-further/i18n.md index 9b20de1d4..c70fa4b06 100644 --- a/docs/docs/going-further/i18n.md +++ b/docs/docs/going-further/i18n.md @@ -51,3 +51,46 @@ en: namespace_article: title: Old Ransack Namespaced Title ``` + +## Working with Globalized Attributes + +If you're using the [Globalize gem](https://github.com/globalize/globalize) for internationalized model attributes, you may encounter issues when sorting on translated attributes of associations while also joining the main model's translations. + +For example, if you have a `Book` model with translated `title` and a `Category` model with translated `name`, sorting on the category's translated name while joining the book's translations may not work as expected: + +```ruby +# This may not work correctly: +Book.joins(:translations).ransack({ s: ['category_translations_name asc'] }).result +``` + +### Workaround for Globalized Attributes Sorting + +When working with globalized attributes and you need to sort on translated fields of associations, the simplest and most effective approach is to use the `sort_link` helper with the translation attribute directly: + +```erb + +<%= sort_link @search, :translations_name %> +<%= sort_link @search, :category_translations_name %> +``` + +For programmatic sorting, let Ransack handle the joins first: + +```ruby +# Instead of joining translations first, let Ransack handle the joins: +search = Book.ransack({ s: ['category_translations_name asc'] }) +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'] }) +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'] }) +results = search.result + .joins(:translations) + .joins(category: :translations) +``` + +The key is to let Ransack establish the necessary joins for sorting first, then add any additional joins you need for the query. + +This approach ensures that Ransack properly handles the complex join dependencies between your main model's translations and the associated model's translations. diff --git a/docs/docs/going-further/other-notes.md b/docs/docs/going-further/other-notes.md index 0937673d6..b67c6aaf5 100644 --- a/docs/docs/going-further/other-notes.md +++ b/docs/docs/going-further/other-notes.md @@ -116,6 +116,39 @@ def index end ``` +### Problem with Globalized Attributes and Sorting + +When using internationalization gems like [Globalize](https://github.com/globalize/globalize), you may encounter issues when trying to sort on translated attributes of associations while also having pre-existing joins to translation tables. + +**Problem scenario:** +```ruby +# This may fail to generate proper joins: +Book.joins(:translations).ransack({ s: ['category_translations_name asc'] }).result +``` + +**Solution:** +The simplest and most effective approach is to use the `sort_link` helper directly with the translation attribute: + +```erb + +<%= sort_link @search, :translations_name %> +<%= sort_link @search, :category_translations_name %> +``` + +For programmatic sorting, let Ransack establish the sorting joins first, then add your additional joins: + +```ruby +# Let Ransack handle the sorting joins first +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'] }) +results = search.result.includes(:translations, category: :translations) +``` + +This ensures that Ransack properly handles the join dependencies between your main model's translations and the associated model's translations. + #### `PG::UndefinedFunction: ERROR: could not identify an equality operator for type json` If you get the above error while using `distinct: true` that means that diff --git a/spec/ransack/adapters/active_record/double_join_pluck_spec.rb b/spec/ransack/adapters/active_record/double_join_pluck_spec.rb new file mode 100644 index 000000000..b79d75da9 --- /dev/null +++ b/spec/ransack/adapters/active_record/double_join_pluck_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +module Ransack + module Adapters + module ActiveRecord + describe 'Double join issue with pluck' do + context 'when using pluck with ransack on already joined query' do + it 'creates erroneous double-join when using pluck with ransack' do + # Create test data to match the scenario from the issue + campaign = ::AutomatedCampaign.create!(name: 'Test Campaign') + visitor = ::Visitor.create!(name: 'Test Visitor') + ::AutomatedCampaignReceipt.create!( + visitor: visitor, + automated_campaign: campaign, + event_type: 'clicked' + ) + + # Capture SQL for both approaches by enabling SQL logging + queries = [] + callback = lambda do |name, started, finished, unique_id, payload| + queries << payload[:sql] if payload[:sql] && payload[:sql].include?('SELECT') + end + + # Subscribe to SQL events + ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do + # First approach: Standard ActiveRecord (should produce 1 join) + queries.clear + result1 = ::Visitor.includes(:automated_campaign_receipts) + .where(automated_campaign_receipts: { automated_campaign_id: campaign.id }) + .where(automated_campaign_receipts: { event_type: 'clicked' }) + .pluck(:id) + + base_sql = queries.last + + # Second approach: Using ransack (may produce double join according to issue) + queries.clear + result2 = ::Visitor.includes(:automated_campaign_receipts) + .where(automated_campaign_receipts: { automated_campaign_id: campaign.id }) + .ransack(automated_campaign_receipts_event_type_eq: 'clicked') + .result + .pluck(:id) + + ransack_sql = queries.last + + # Extract and compare joins + base_joins = base_sql&.scan(/LEFT OUTER JOIN.*?automated_campaign_receipts.*?ON.*?(?=LEFT OUTER JOIN|\sWHERE|\s(?:GROUP|ORDER|LIMIT|$))/mi) || [] + ransack_joins = ransack_sql&.scan(/LEFT OUTER JOIN.*?automated_campaign_receipts.*?ON.*?(?=LEFT OUTER JOIN|\sWHERE|\s(?:GROUP|ORDER|LIMIT|$))/mi) || [] + + puts "\n=== PLUCK SQL COMPARISON ===" + puts "Base query SQL (#{base_joins.length} joins): #{base_sql}" + puts "Ransack query SQL (#{ransack_joins.length} joins): #{ransack_sql}" + puts "Base joins found: #{base_joins.inspect}" + puts "Ransack joins found: #{ransack_joins.inspect}" + + # The issue reports that ransack creates duplicate joins with pluck + # If this test fails, it reproduces the issue described + expect(ransack_joins.length).to eq(base_joins.length), + "Expected same number of joins, but ransack created #{ransack_joins.length} vs #{base_joins.length} in base query" + + # Results should be the same + expect(result2).to eq(result1) + end + end + + it 'demonstrates the difference with and without pluck' do + # Create test data + campaign = ::AutomatedCampaign.create!(name: 'Test Campaign 2') + visitor = ::Visitor.create!(name: 'Test Visitor 2') + ::AutomatedCampaignReceipt.create!( + visitor: visitor, + automated_campaign: campaign, + event_type: 'clicked' + ) + + # Set up the base query + base_query = ::Visitor.includes(:automated_campaign_receipts) + .where(automated_campaign_receipts: { automated_campaign_id: campaign.id }) + + # Add ransack condition + ransack_query = base_query.ransack(automated_campaign_receipts_event_type_eq: 'clicked').result + + # Compare SQL without pluck (should be similar) + base_sql = base_query.to_sql + ransack_sql = ransack_query.to_sql + + puts "\n=== WITHOUT PLUCK ===" + puts "Base SQL: #{base_sql}" + puts "Ransack SQL: #{ransack_sql}" + + base_join_count = base_sql.scan(/LEFT OUTER JOIN.*automated_campaign_receipts/i).count + ransack_join_count = ransack_sql.scan(/LEFT OUTER JOIN.*automated_campaign_receipts/i).count + + puts "Base joins: #{base_join_count}, Ransack joins: #{ransack_join_count}" + + # This comparison shows the issue is specifically with pluck + expect(ransack_join_count).to be >= base_join_count + end + end + end + end + end +end \ No newline at end of file diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb index 850bbe6b4..1a84f48ab 100644 --- a/spec/ransack/search_spec.rb +++ b/spec/ransack/search_spec.rb @@ -147,43 +147,6 @@ module Ransack expect(s.result.to_sql).to include 'published' end - # The failure/oversight in Ransack::Nodes::Condition#arel_predicate or deeper is beyond my understanding of the structures - it 'preserves (inverts) default scope and conditions for negative subqueries' do - # the positive case (published_articles_title_eq) is - # SELECT "people".* FROM "people" - # LEFT OUTER JOIN "articles" ON "articles"."person_id" = "people"."id" - # AND "articles"."published" = 't' - # AND ('default_scope' = 'default_scope') - # WHERE "articles"."title" = 'Test' ORDER BY "people"."id" DESC - # - # negative case was - # SELECT "people".* FROM "people" WHERE "people"."id" NOT IN ( - # SELECT "articles"."person_id" FROM "articles" - # WHERE "articles"."person_id" = "people"."id" - # AND NOT ("articles"."title" != 'Test') - # ) ORDER BY "people"."id" DESC - # - # Should have been like - # SELECT "people".* FROM "people" WHERE "people"."id" NOT IN ( - # SELECT "articles"."person_id" FROM "articles" - # WHERE "articles"."person_id" = "people"."id" - # AND "articles"."title" = 'Test' AND "articles"."published" = 't' AND ('default_scope' = 'default_scope') - # ) ORDER BY "people"."id" DESC - # - # With tenanting (eg default_scope with column reference), NOT IN should be like - # SELECT "people".* FROM "people" WHERE "people"."tenant_id" = 'tenant_id' AND "people"."id" NOT IN ( - # SELECT "articles"."person_id" FROM "articles" - # WHERE "articles"."person_id" = "people"."id" - # AND "articles"."tenant_id" = 'tenant_id' - # AND "articles"."title" = 'Test' AND "articles"."published" = 't' AND ('default_scope' = 'default_scope') - # ) ORDER BY "people"."id" DESC - - pending("spec should pass, but I do not know how/where to fix lib code") - s = Search.new(Person, published_articles_title_not_eq: 'Test') - expect(s.result.to_sql).to include 'default_scope' - expect(s.result.to_sql).to include 'published' - end - it 'discards empty conditions' do s = Search.new(Person, children_name_eq: '') condition = s.base[:children_name_eq] diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 853bf9095..f7f9c5481 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -285,6 +285,19 @@ class Employee < ApplicationRecord has_one :address, through: :organization end +class Visitor < ApplicationRecord + has_many :automated_campaign_receipts +end + +class AutomatedCampaign < ApplicationRecord + has_many :automated_campaign_receipts +end + +class AutomatedCampaignReceipt < ApplicationRecord + belongs_to :visitor + belongs_to :automated_campaign +end + module Schema def self.create ActiveRecord::Migration.verbose = false @@ -363,6 +376,23 @@ def self.create t.string :name t.integer :organization_id end + + create_table :visitors, force: true do |t| + t.string :name + t.timestamps null: false + end + + create_table :automated_campaigns, force: true do |t| + t.string :name + t.timestamps null: false + end + + create_table :automated_campaign_receipts, force: true do |t| + t.integer :visitor_id + t.integer :automated_campaign_id + t.string :event_type + t.timestamps null: false + end end 10.times do @@ -384,6 +414,14 @@ def self.create body: 'First post!', article: Article.make(title: 'Hello, world!') ) + + # Create test data for the double join issue + 5.times do |i| + visitor = Visitor.create(name: "Visitor #{i}") + campaign = AutomatedCampaign.create(name: "Campaign #{i}") + AutomatedCampaignReceipt.create(visitor: visitor, automated_campaign: campaign, event_type: 'clicked') + AutomatedCampaignReceipt.create(visitor: visitor, automated_campaign: campaign, event_type: 'opened') + end end end