handling for string columns in Rails 8

Author: StephenVNelson

lib/mobility/plugins/active_record/query.rb

@@ -187,21 +187,25 @@ def order(opts, *rest)
 
           if ::ActiveRecord::VERSION::MAJOR >= 8
             # Fix for https://github.com/shioyama/mobility/pull/654#issuecomment-2503479112
-            # TODO: Make this better
+            # When counting with select values that include mobility-translated attributes,
+            # we need to filter out the aliased columns that Mobility adds (e.g. __mobility_title_en__)
+            # to prevent errors in the COUNT SQL generation.
             def select_for_count
               return super unless klass.respond_to?(:mobility_attribute?)
 
-              if select_values.any? { |value| value.right.start_with?(ATTRIBUTE_ALIAS_PREFIX) }
-                filtered_select_values = select_values.map do |value|
-                  value.right.start_with?(ATTRIBUTE_ALIAS_PREFIX) ? value.left : value
-                end
+              # Check if any select values are Mobility attribute aliases
+              has_mobility_aliases = select_values.any? { |value| value.respond_to?(:right) && value.right.start_with?(ATTRIBUTE_ALIAS_PREFIX) }
 
-                # Copied from lib/active_record/relation/calculations.rb
-                with_connection do |conn|
-                  arel_columns(filtered_select_values).map { |column| conn.visitor.compile(column) }.join(", ")
-                end
-              else
-                super
+              return super unless has_mobility_aliases
+
+              # Filter out Mobility aliases, replacing them with the underlying columns
+              filtered_select_values = select_values.map do |value|
+                value.respond_to?(:right) && value.right.start_with?(ATTRIBUTE_ALIAS_PREFIX) ? value.left : value
+              end
+
+              # Copied from lib/active_record/relation/calculations.rb
+              with_connection do |conn|
+                arel_columns(filtered_select_values).map { |column| conn.visitor.compile(column) }.join(", ")
               end
             end
           end

spec/mobility/plugins/active_record/query_spec.rb

@@ -218,4 +218,49 @@
       expect(Article.i18n.where(title: "Title", content: "Content")).to eq([article2])
     end
   end
+
+  describe "regression: select_for_count with mixed select values" do
+    # Test for bug where select_for_count called .right on string values
+    # causing NoMethodError when select_values contained non-Arel nodes
+    if ::ActiveRecord::VERSION::MAJOR >= 8
+      before do
+        stub_const 'Article', Class.new(ActiveRecord::Base)
+        translates Article, :title, backend: :table
+      end
+
+      it "handles count with regular column in select" do
+        Article.create(title: "Article 1")
+        Article.create(title: "Article 2")
+
+        # The original bug: calling .right on string values like "id"
+        # This should not raise NoMethodError: undefined method `right' for "id":String
+        expect { Article.select(:id).count }.not_to raise_error
+        expect(Article.select(:id).count).to eq(2)
+      end
+
+      it "handles count with translated column in select" do
+        Article.create(title: "Article 1")
+        Article.create(title: "Article 2")
+
+        # When selecting translated attributes, Mobility creates Arel::Nodes::As with aliases
+        # This should properly filter out the aliased columns for counting
+        relation = Article.i18n.select(:title)
+
+        expect { relation.count }.not_to raise_error
+        expect(relation.count).to eq(2)
+      end
+
+      it "handles count without select when mobility is present" do
+        Article.create(title: "Article 1")
+        Article.create(title: "Article 2")
+
+        # Ensure normal counting still works on models with mobility
+        expect { Article.count }.not_to raise_error
+        expect(Article.count).to eq(2)
+
+        expect { Article.i18n.count }.not_to raise_error
+        expect(Article.i18n.count).to eq(2)
+      end
+    end
+  end
 end