Search by an attribute path instead of a table-qualified string

searchable_columns built "table.column" strings, which is why the Mongoid
adapter had to take them apart again in parse_collection_name and guess whether
the prefix named a collection or an embedded path -- the clearest case of a
field deciding how a query gets built.

Return a Criteria::Path wherever the configuration can be read as one, and let
each adapter resolve it: ActiveRecord qualifies with the associated table and
reads that half back out for the relation's references, Mongoid keeps grouping
conditions by collection and still reaches embedded documents by field path.

Configurations naming a table or a model class stay strings and are handed
through, since their prefix is not an association name:

  searchable 'teams.name'
  searchable({leagues: :name})
  searchable(League => :name)

searchable_columns remains the option adapters read, so overriding it with
strings keeps working.

Field specs now assert the path, which is what the field owes; what each of them
compiles to is pinned in the ActiveRecord adapter spec, where the previous
strings are reproduced exactly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: mshibuya

lib/rails_admin/abstract_model.rb

@@ -111,6 +111,13 @@ def sort_expression(order)
       order.to_s
     end
 
+    # Render a search target as the column reference the store understands.
+    # Overridden where reaching an associated attribute takes more than the
+    # dotted path itself.
+    def search_column(target)
+      target.to_s
+    end
+
     def to_s
       model.to_s
     end

lib/rails_admin/adapters/active_record.rb

@@ -154,20 +154,36 @@ def attribute_enum_values(name)
       def sort_expression(order)
         return order.to_s unless order.is_a?(RailsAdmin::Criteria::Path)
 
-        "#{sort_table_name(order)}.#{quote_column_name(order.attribute)}"
+        "#{quoted_path_table_name(order)}.#{quote_column_name(order.attribute)}"
+      end
+
+      # Unlike sorting, search statements are assembled unquoted, and the table
+      # half is read back out to build the relation's references.
+      def search_column(target)
+        return target.to_s unless target.is_a?(RailsAdmin::Criteria::Path)
+
+        "#{path_table_name(target)}.#{target.attribute}"
       end
 
     private
 
-      # The table the path's attribute lives on. Only a single association hop
+      # The model the path's attribute lives on. Only a single association hop
       # is resolvable, which is all the field configuration can express.
-      def sort_table_name(path)
-        return quoted_table_name if path.root?
+      def path_abstract_model(path)
+        return self if path.root?
 
         association = associations.detect { |a| a.name == path.associations.first }
-        raise ArgumentError.new("Unknown association in sort path: #{path}") unless association
+        raise ArgumentError.new("Unknown association in path: #{path}") unless association
+
+        RailsAdmin::AbstractModel.new(association.klass)
+      end
+
+      def path_table_name(path)
+        path_abstract_model(path).table_name
+      end
 
-        RailsAdmin::AbstractModel.new(association.klass).quoted_table_name
+      def quoted_path_table_name(path)
+        path_abstract_model(path).quoted_table_name
       end
 
       def primary_key_scope(scope, id)
@@ -202,21 +218,23 @@ def sort_scope(scope, options)
       end
 
       class WhereBuilder
-        def initialize(scope)
+        def initialize(scope, abstract_model)
           @statements = []
           @values = []
           @tables = []
           @scope = scope
+          @abstract_model = abstract_model
         end
 
         def add(field, value, operator)
           field.searchable_columns.flatten.each do |column_infos|
-            statement, value1, value2 = StatementBuilder.new(column_infos[:column], column_infos[:type], value, operator, @scope.connection.adapter_name).to_statement
+            column = @abstract_model.search_column(column_infos[:column])
+            statement, value1, value2 = StatementBuilder.new(column, column_infos[:type], value, operator, @scope.connection.adapter_name).to_statement
             @statements << statement if statement.present?
             @values << value1 unless value1.nil?
             @values << value2 unless value2.nil?
-            table, column = column_infos[:column].split('.')
-            @tables.push(table) if column
+            table, qualified = column.split('.')
+            @tables.push(table) if qualified
           end
         end
 
@@ -231,7 +249,7 @@ def query_scope(scope, query, fields = config.list.fields.select(&:queryable?))
         if config.list.search_by
           scope.send(config.list.search_by, query)
         else
-          wb = WhereBuilder.new(scope)
+          wb = WhereBuilder.new(scope, self)
           fields.each do |field|
             value = parse_field_value(field, query)
             wb.add(field, value, field.search_operator)
@@ -246,7 +264,7 @@ def query_scope(scope, query, fields = config.list.fields.select(&:queryable?))
       def filter_scope(scope, filters, fields = config.list.fields.select(&:filterable?))
         filters.each_pair do |field_name, filters_dump|
           filters_dump.each_value do |filter_dump|
-            wb = WhereBuilder.new(scope)
+            wb = WhereBuilder.new(scope, self)
             field = fields.detect { |f| f.name.to_s == field_name }
             value = parse_field_value(field, filter_dump[:v])
 

lib/rails_admin/adapters/mongoid.rb

@@ -175,7 +175,26 @@ def filter_scope(scope, filters, fields = config.list.fields.select(&:filterable
         scope.where(statements.any? ? {'$and' => statements} : {})
       end
 
-      def parse_collection_name(column)
+      # Where a search target lives, as [collection, field] -- the collection is
+      # only used to group conditions and to tell apart what can be asked of this
+      # collection directly from what needs a second lookup.
+      def parse_collection_name(target)
+        return parse_collection_name_from_string(target.to_s) unless target.is_a?(RailsAdmin::Criteria::Path)
+
+        return [table_name, target.attribute.to_s] if target.root?
+
+        association = associations.detect { |a| a.name == target.associations.first }
+        if association.try(:embeds?)
+          # Embedded documents are reached by their field path, not by collection.
+          [table_name, target.to_s]
+        elsif association
+          [RailsAdmin::AbstractModel.new(association.klass).table_name, target.attribute.to_s]
+        else
+          [target.associations.first.to_s, target.attribute.to_s]
+        end
+      end
+
+      def parse_collection_name_from_string(column)
         collection_name, column_name = column.split('.')
         if associations.detect { |a| a.name == collection_name.to_sym }.try(:embeds?)
           [table_name, column]

lib/rails_admin/config/fields/base.rb

@@ -129,36 +129,39 @@ def filter_options
           false
         end
 
-        # list of columns I should search for that field [{ column: 'table_name.column', type: field.type }, {..}]
+        # What to search for this field: [{column: <target>, type: <field type>}, ..]
+        #
+        # A target is a Criteria::Path wherever the configuration can be read as
+        # one, so that the adapter decides how to reach the attribute. The
+        # table-qualified forms cannot be read that way -- their prefix names a
+        # table, not an association -- so they stay strings and are handed to the
+        # store as they are. Overriding this option with strings keeps working.
         register_instance_option :searchable_columns do
           @searchable_columns ||=
             case searchable
             when true
-              [{column: "#{abstract_model.table_name}.#{name}", type: type}]
+              [{column: RailsAdmin::Criteria::Path[name], type: type}]
             when false
               []
             when :all # valid only for associations
-              table_name = associated_model_config.abstract_model.table_name
-              associated_model_config.list.fields.collect { |f| {column: "#{table_name}.#{f.name}", type: f.type} }
+              associated_model_config.list.fields.collect do |f|
+                {column: RailsAdmin::Criteria::Path[name, f.name], type: f.type}
+              end
             else
               [searchable].flatten.collect do |f|
-                if f.is_a?(String) && f.include?('.')                            #  table_name.column
-                  table_name, column = f.split '.'
-                  type = nil
-                elsif f.is_a?(Hash)                                              #  <Model|table_name> => <attribute|column>
+                if f.is_a?(String) && f.include?('.')                            #  table_name.column, passed through
+                  {column: f, type: :string}
+                elsif f.is_a?(Hash)                                              #  <Model|table_name> => <attribute|column>, passed through
                   am = AbstractModel.new(f.keys.first) if f.keys.first.is_a?(Class)
                   table_name = am&.table_name || f.keys.first
-                  column = f.values.first
                   property = am&.properties&.detect { |p| p.name == f.values.first.to_sym }
-                  type = property&.type
-                else                                                             #  <attribute|column>
+                  {column: "#{table_name}.#{f.values.first}", type: (property&.type || :string)}
+                else                                                             #  <attribute>
                   am = (association? ? associated_model_config.abstract_model : abstract_model)
-                  table_name = am.table_name
-                  column = f
                   property = am.properties.detect { |p| p.name == f.to_sym }
-                  type = property&.type
+                  path = association? ? RailsAdmin::Criteria::Path[name, f] : RailsAdmin::Criteria::Path[f]
+                  {column: path, type: (property&.type || :string)}
                 end
-                {column: "#{table_name}.#{column}", type: (type || :string)}
               end
             end
         end

spec/rails_admin/adapters/active_record_spec.rb

@@ -174,6 +174,23 @@ class PlayerWithDefaultScope < Player
     end
   end
 
+  describe '#search_column' do
+    let(:abstract_model) { RailsAdmin::AbstractModel.new('Team') }
+
+    # Search statements are assembled unquoted, unlike sort expressions.
+    it 'qualifies a local attribute with its own table' do
+      expect(abstract_model.search_column(RailsAdmin::Criteria::Path[:name])).to eq 'teams.name'
+    end
+
+    it 'qualifies an associated attribute with the associated table' do
+      expect(abstract_model.search_column(RailsAdmin::Criteria::Path[:division, :name])).to eq 'divisions.name'
+    end
+
+    it 'hands table-qualified strings through untouched' do
+      expect(abstract_model.search_column('leagues.name')).to eq 'leagues.name'
+    end
+  end
+
   describe '#query_scope' do
     let(:abstract_model) { RailsAdmin::AbstractModel.new('Team') }
     let!(:teams) do

spec/rails_admin/config/fields/base_spec.rb

@@ -348,10 +348,15 @@ class CommentReversed < Tableless
     end
   end
 
+  # Targets that can be read as an attribute reached through associations come
+  # back as a Criteria::Path, leaving the adapter to decide what that means for
+  # its store. Table-qualified configurations stay strings; see the adapter specs
+  # for what each of these compiles to.
   describe '#searchable_columns' do
     describe 'for belongs_to fields' do
       it 'finds label method on the opposite side for belongs_to associations by default' do
-        expect(RailsAdmin.config(Team).fields.detect { |f| f.name == :division }.searchable_columns.collect { |c| c[:column] }).to eq(['divisions.name', 'teams.division_id'])
+        expect(RailsAdmin.config(Team).fields.detect { |f| f.name == :division }.searchable_columns.collect { |c| c[:column] }).
+          to eq([RailsAdmin::Criteria::Path[:division, :name], 'teams.division_id'])
       end
 
       it 'searches on opposite table for belongs_to' do
@@ -360,7 +365,8 @@ class CommentReversed < Tableless
             searchable :custom_id
           end
         end
-        expect(RailsAdmin.config(Team).fields.detect { |f| f.name == :division }.searchable_columns.collect { |c| c[:column] }).to eq(['divisions.custom_id'])
+        expect(RailsAdmin.config(Team).fields.detect { |f| f.name == :division }.searchable_columns.collect { |c| c[:column] }).
+          to eq([RailsAdmin::Criteria::Path[:division, :custom_id]])
       end
 
       it 'searches on asked table with model name' do
@@ -383,9 +389,9 @@ class CommentReversed < Tableless
     end
 
     describe 'for basic type fields' do
-      it 'uses base table and find correct column type' do
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :text_field }.searchable_columns).to eq([{column: 'field_tests.text_field', type: :text}])
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :integer_field }.searchable_columns).to eq([{column: 'field_tests.integer_field', type: :integer}])
+      it 'uses the attribute itself and finds the correct column type' do
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :text_field }.searchable_columns).to eq([{column: RailsAdmin::Criteria::Path[:text_field], type: :text}])
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :integer_field }.searchable_columns).to eq([{column: RailsAdmin::Criteria::Path[:integer_field], type: :integer}])
       end
 
       it 'is customizable to another field on the same table' do
@@ -394,7 +400,7 @@ class CommentReversed < Tableless
             searchable :date_field
           end
         end
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :time_field }.searchable_columns).to eq([{column: 'field_tests.date_field', type: :date}])
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :time_field }.searchable_columns).to eq([{column: RailsAdmin::Criteria::Path[:date_field], type: :date}])
       end
 
       it 'is customizable to another field on another table with :table_name' do
@@ -418,15 +424,18 @@ class CommentReversed < Tableless
 
     describe 'for mapped fields' do
       it 'of paperclip should find the underlying column on the base table' do
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :paperclip_asset }.searchable_columns.collect { |c| c[:column] }).to eq(['field_tests.paperclip_asset_file_name'])
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :paperclip_asset }.searchable_columns.collect { |c| c[:column] }).
+          to eq([RailsAdmin::Criteria::Path[:paperclip_asset_file_name]])
       end
 
       it 'of dragonfly should find the underlying column on the base table' do
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :dragonfly_asset }.searchable_columns.collect { |c| c[:column] }).to eq(['field_tests.dragonfly_asset_name'])
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :dragonfly_asset }.searchable_columns.collect { |c| c[:column] }).
+          to eq([RailsAdmin::Criteria::Path[:dragonfly_asset_name]])
       end
 
       it 'of carrierwave should find the underlying column on the base table' do
-        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :carrierwave_asset }.searchable_columns.collect { |c| c[:column] }).to eq(['field_tests.carrierwave_asset'])
+        expect(RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :carrierwave_asset }.searchable_columns.collect { |c| c[:column] }).
+          to eq([RailsAdmin::Criteria::Path[:carrierwave_asset]])
       end
     end
   end