From 0fd3e4cd2b2b1b31304a922dc65284d5363f78b6 Mon Sep 17 00:00:00 2001
From: Mark Catley <mark@nexx.co.nz>
Date: Sat, 21 Jun 2008 23:41:30 +1200
Subject: [PATCH] Fix column collision with named_scope and :joins.  [#46
 state:resolved]

---
 activerecord/CHANGELOG                        |  2 ++
 activerecord/lib/active_record/base.rb        |  2 +-
 .../test/cases/method_scoping_test.rb         | 10 +++++++++
 activerecord/test/cases/named_scope_test.rb   | 21 ++++++++++++++++++-
 activerecord/test/models/post.rb              |  7 ++++++-
 5 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 0ac5ce960c..aaa7c3a0d1 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Edge*
 
+* Fix column collision with named_scope and :joins.  #46 [Duncan Beevers, Mark Catley]
+
 * db:migrate:down and :up update schema_migrations.  #369 [Michael Raidel, RaceCondition]
 
 * PostgreSQL: support :conditions => [':foo::integer', { :foo => 1 }] without treating the ::integer typecast as a bind variable.  [Tarmo Tänav]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 8d5ea271a7..e6ab87e8fc 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1479,7 +1479,7 @@ def type_name_with_module(type_name)
 
         def construct_finder_sql(options)
           scope = scope(:find)
-          sql  = "SELECT #{options[:select] || (scope && scope[:select]) || (options[:joins] && quoted_table_name + '.*') || '*'} "
+          sql  = "SELECT #{options[:select] || (scope && scope[:select]) || ((options[:joins] || (scope && scope[:joins])) && quoted_table_name + '.*') || '*'} "
           sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "
 
           add_joins!(sql, options, scope)
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index 1a9a875730..d6b3e341df 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -87,6 +87,16 @@ def test_scoped_find_include
     assert_equal 1, scoped_developers.size
   end
 
+  def test_scoped_find_joins
+    scoped_developers = Developer.with_scope(:find => { :joins => 'JOIN developers_projects ON id = developer_id' } ) do
+      Developer.find(:all, :conditions => 'developers_projects.project_id = 2')
+    end
+    assert scoped_developers.include?(developers(:david))
+    assert !scoped_developers.include?(developers(:jamis))
+    assert_equal 1, scoped_developers.size
+    assert_equal developers(:david).attributes, scoped_developers.first.attributes
+  end
+
   def test_scoped_count_include
     # with the include, will retrieve only developers for the given project
     Developer.with_scope(:find => { :include => :projects }) do
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 73673e0f0e..393ba086c9 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -6,7 +6,7 @@
 require 'models/author'
 
 class NamedScopeTest < ActiveRecord::TestCase
-  fixtures :posts, :authors, :topics, :comments
+  fixtures :posts, :authors, :topics, :comments, :author_addresses
 
   def test_implements_enumerable
     assert !Topic.find(:all).empty?
@@ -83,6 +83,25 @@ def test_procedural_scopes
     assert_equal topics_written_before_the_second, Topic.written_before(topics(:second).written_on)
   end
 
+  def test_scopes_with_joins
+    address = author_addresses(:david_address)
+    posts_with_authors_at_address = Post.find(
+      :all, :joins => 'JOIN authors ON authors.id = posts.author_id',
+      :conditions => [ 'authors.author_address_id = ?', address.id ]
+    )
+    assert_equal posts_with_authors_at_address, Post.with_authors_at_address(address)
+  end
+
+  def test_scopes_with_joins_respects_custom_select
+    address = author_addresses(:david_address)
+    posts_with_authors_at_address_titles = Post.find(:all,
+      :select => 'title',
+      :joins => 'JOIN authors ON authors.id = posts.author_id',
+      :conditions => [ 'authors.author_address_id = ?', address.id ]
+    )
+    assert_equal posts_with_authors_at_address_titles, Post.with_authors_at_address(address).find(:all, :select => 'title')
+  end
+
   def test_extensions
     assert_equal 1, Topic.anonymous_extension.one
     assert_equal 2, Topic.named_extension.two
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index d9101706b5..3adbc0ce1f 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -1,6 +1,11 @@
 class Post < ActiveRecord::Base
   named_scope :containing_the_letter_a, :conditions => "body LIKE '%a%'"
-  
+  named_scope :with_authors_at_address, lambda { |address| {
+      :conditions => [ 'authors.author_address_id = ?', address.id ],
+      :joins => 'JOIN authors ON authors.id = posts.author_id'
+    }
+  }
+
   belongs_to :author do
     def greeting
       "hello"
-- 
GitLab