* Fix pagination problems when using include

* Introduce Unit Tests for pagination
* Allow count to work with :include by using count distinct.

[Kevin Clark & Jeremy Hopple]


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3553 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

15 files changed, 410 insertions, 50 deletions

diff --git a/actionpack/test/active_record_unit.rb b/actionpack/test/active_record_unit.rb
new file mode 100644
index 0000000000..016f331d77
--- /dev/null
+++ b/actionpack/test/active_record_unit.rb
@@ -0,0 +1,88 @@
+require  File.dirname(__FILE__) + '/abstract_unit'
+
+# Define the essentials
+class ActiveRecordTestConnector
+  cattr_accessor :able_to_connect
+  cattr_accessor :connected
+  
+  # Set our defaults
+  self.connected = false
+  self.able_to_connect = true
+end
+
+# Try to grab AR
+begin
+  PATH_TO_AR = File.dirname(__FILE__) + '/../../activerecord'
+  require "#{PATH_TO_AR}/lib/active_record" unless Object.const_defined?(:ActiveRecord)
+  require "#{PATH_TO_AR}/lib/active_record/fixtures" unless Object.const_defined?(:Fixtures)
+rescue Object => e
+  $stderr.puts "\nSkipping ActiveRecord assertion tests: #{e}"
+  ActiveRecordTestConnector.able_to_connect = false
+end
+
+# Define the rest of the connector
+class ActiveRecordTestConnector  
+  def self.setup
+    unless self.connected || !self.able_to_connect
+      setup_connection
+      load_schema
+      self.connected = true
+    end
+  rescue Object => e
+    $stderr.puts "\nSkipping ActiveRecord assertion tests: #{e}"
+    #$stderr.puts "  #{e.backtrace.join("\n  ")}\n"
+    self.able_to_connect = false
+  end
+  
+  private
+  
+  def self.setup_connection
+    if Object.const_defined?(:ActiveRecord)
+          
+      begin
+        ActiveRecord::Base.establish_connection(:adapter => 'sqlite3', :dbfile => ':memory:')
+        ActiveRecord::Base.connection
+      rescue Object
+        $stderr.puts 'SQLite 3 unavailable; falling to SQLite 2.'
+        ActiveRecord::Base.establish_connection(:adapter => 'sqlite', :dbfile => ':memory:')
+        ActiveRecord::Base.connection
+      end
+    
+      Object.send(:const_set, :QUOTED_TYPE, ActiveRecord::Base.connection.quote_column_name('type')) unless Object.const_defined?(:QUOTED_TYPE)
+    else
+      raise "Couldn't locate ActiveRecord."
+    end
+  end
+  
+  # Load actionpack sqlite tables
+  def self.load_schema
+    File.read(File.dirname(__FILE__) + "/fixtures/db_definitions/sqlite.sql").split(';').each do |sql|
+      ActiveRecord::Base.connection.execute(sql) unless sql.blank?
+    end
+  end
+end
+  
+# Test case for inheiritance  
+class ActiveRecordTestCase < Test::Unit::TestCase
+  # Set our fixture path
+  self.fixture_path = "#{File.dirname(__FILE__)}/fixtures/"
+  
+  def setup
+    abort_tests unless ActiveRecordTestConnector.connected = true
+  end
+  
+  # Default so Test::Unit::TestCase doesn't complain
+  def test_truth
+  end
+  
+  private
+  
+  # If things go wrong, we don't want to run our test cases. We'll just define them to test nothing.
+  def abort_tests
+    self.class.public_instance_methods.grep(/^test./).each do |method|
+      self.class.class_eval { define_method(method.to_sym){} }
+    end
+  end
+end
+
+ActiveRecordTestConnector.setup
\ No newline at end of file
diff --git a/actionpack/test/controller/active_record_assertions_test.rb b/actionpack/test/activerecord/active_record_assertions_test.rb
index b8b6309295..f5661d19cc 100644
--- a/actionpack/test/controller/active_record_assertions_test.rb
+++ b/actionpack/test/activerecord/active_record_assertions_test.rb
@@ -1,45 +1,6 @@
-require "#{File.dirname(__FILE__)}/../abstract_unit"
-
-# Unfurl the safety net.
-path_to_ar = File.dirname(__FILE__) + '/../../../activerecord'
-if Object.const_defined?(:ActiveRecord) || File.exist?(path_to_ar)
-  begin
-
-# These tests require Active Record, so you're going to need AR in a
-# sibling directory to AP and have SQLite installed.
-
-unless Object.const_defined?(:ActiveRecord)
-  require "#{path_to_ar}/lib/active_record"
-end
-
-begin
-  ActiveRecord::Base.establish_connection(:adapter => 'sqlite3', :dbfile => ':memory:')
-  ActiveRecord::Base.connection
-rescue Object
-  $stderr.puts 'SQLite 3 unavailable; falling to SQLite 2.'
-  ActiveRecord::Base.establish_connection(:adapter => 'sqlite', :dbfile => ':memory:')
-  ActiveRecord::Base.connection
-end
-
-# Set up company fixtures.
-$LOAD_PATH << "#{path_to_ar}/test"
-QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type') unless Object.const_defined?(:QUOTED_TYPE)
+require "#{File.dirname(__FILE__)}/../active_record_unit"
 require 'fixtures/company'
-File.read("#{path_to_ar}/test/fixtures/db_definitions/sqlite.sql").split(';').each do |sql|
-  ActiveRecord::Base.connection.execute(sql) unless sql.blank?
-end
-
-# Add some validation rules to trip up the assertions.
-class Company
-  protected
-    def validate
-      errors.add_on_empty('name')
-      errors.add('rating', 'rating should not be 2') if rating == 2
-      errors.add_to_base('oh oh') if rating == 3
-    end  
-end
 
-# A controller to host the assertions.
 class ActiveRecordAssertionsController < ActionController::Base
   self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
 
@@ -76,12 +37,15 @@ class ActiveRecordAssertionsController < ActionController::Base
   # the safety dance......
   def rescue_action(e) raise; end
 end
-                    
-class ActiveRecordAssertionsControllerTest < Test::Unit::TestCase
+                  
+class ActiveRecordAssertionsControllerTest < ActiveRecordTestCase
+  fixtures :companies
+  
   def setup
     @request = ActionController::TestRequest.new
     @response = ActionController::TestResponse.new
     @controller = ActiveRecordAssertionsController.new
+    super
   end
 
   # test for 1 bad apple column
@@ -117,11 +81,4 @@ class ActiveRecordAssertionsControllerTest < Test::Unit::TestCase
     assert_success
     assert_invalid_record 'company'
   end
-end 
-
-# End of safety net.
-  rescue Object => e
-    $stderr.puts "Skipping Active Record assertion tests: #{e}"
-    #$stderr.puts "  #{e.backtrace.join("\n  ")}"
-  end
-end
+end
\ No newline at end of file
diff --git a/actionpack/test/activerecord/pagination_test.rb b/actionpack/test/activerecord/pagination_test.rb
new file mode 100644
index 0000000000..00ae78614f
--- /dev/null
+++ b/actionpack/test/activerecord/pagination_test.rb
@@ -0,0 +1,146 @@
+require File.dirname(__FILE__) + '/../active_record_unit'
+
+require 'fixtures/topic'
+require 'fixtures/reply'
+require 'fixtures/developer'
+require 'fixtures/project'
+
+class PaginationTest < ActiveRecordTestCase
+  fixtures :topics, :replies, :developers, :projects, :developers_projects
+
+  class PaginationController < ActionController::Base
+    self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
+    
+    def simple_paginate
+      @topic_pages, @topics = paginate(:topics)
+      render :nothing => true
+    end
+    
+    def paginate_with_per_page
+      @topic_pages, @topics = paginate(:topics, :per_page => 1)
+      render :nothing => true
+    end
+    
+    def paginate_with_order
+      @topic_pages, @topics = paginate(:topics, :order => 'created_at asc')
+      render :nothing => true
+    end
+    
+    def paginate_with_order_by
+      @topic_pages, @topics = paginate(:topics, :order_by => 'created_at asc')
+      render :nothing => true
+    end
+    
+    def paginate_with_include_and_order
+      @topic_pages, @topics = paginate(:topics, :include => :replies, :order => 'replies.created_at asc, topics.created_at asc')
+      render :nothing => true
+    end
+    
+    def paginate_with_conditions
+      @topic_pages, @topics = paginate(:topics, :conditions => ["created_at > ?", 30.minutes.ago])
+      render :nothing => true
+    end
+    
+    def paginate_with_class_name
+      @developer_pages, @developers = paginate(:developers, :class_name => "DeVeLoPeR")
+      render :nothing => true
+    end
+    
+    def paginate_with_singular_name
+      @developer_pages, @developers = paginate()
+      render :nothing => true
+    end
+    
+    def paginate_with_joins
+      @developer_pages, @developers = paginate(:developers, 
+                                             :joins => 'LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id',
+                                             :conditions => 'project_id=1')        
+      render :nothing => true
+    end
+    
+    def paginate_with_join
+      @developer_pages, @developers = paginate(:developers, 
+                                             :join => 'LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id',
+                                             :conditions => 'project_id=1')        
+      render :nothing => true
+    end
+    
+    def rescue_errors(e) raise e end
+
+    def rescue_action(e) raise end
+    
+  end
+  
+  def setup
+    @controller = PaginationController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    super
+  end
+
+  # Single Action Pagination Tests
+
+  def test_simple_paginate
+    get :simple_paginate
+    assert_equal 1, assigns(:topic_pages).page_count
+    assert_equal 3, assigns(:topics).size
+  end
+  
+  def test_paginate_with_per_page
+    get :paginate_with_per_page
+    assert_equal 1, assigns(:topics).size
+    assert_equal 3, assigns(:topic_pages).page_count
+  end
+  
+  def test_paginate_with_order
+    get :paginate_with_order
+    expected = [topics(:futurama),
+               topics(:harvey_birdman),
+               topics(:rails)]
+    assert_equal expected, assigns(:topics)
+    assert_equal 1, assigns(:topic_pages).page_count
+  end
+  
+  def test_paginate_with_order_by
+    get :paginate_with_order
+    expected = assigns(:topics)
+    get :paginate_with_order_by
+    assert_equal expected, assigns(:topics)  
+    assert_equal 1, assigns(:topic_pages).page_count    
+  end
+  
+  def test_paginate_with_conditions
+    get :paginate_with_conditions
+    expected = [topics(:rails)]
+    assert_equal expected, assigns(:topics)
+    assert_equal 1, assigns(:topic_pages).page_count
+  end
+  
+  def test_paginate_with_class_name
+    get :paginate_with_class_name
+    
+    assert assigns(:developers).size > 0
+    assert_equal DeVeLoPeR, assigns(:developers).first.class
+  end
+      
+  def test_paginate_with_joins
+    get :paginate_with_joins
+    assert_equal 2, assigns(:developers).size
+    developer_names = assigns(:developers).map { |d| d.name }
+    assert developer_names.include?('David')
+    assert developer_names.include?('Jamis')
+  end
+  
+  def test_paginate_with_join_and_conditions
+    get :paginate_with_joins
+    expected = assigns(:topics)
+    get :paginate_with_join
+    assert_equal expected, assigns(:topics)      
+  end
+  
+  def test_paginate_with_include_and_order
+    get :paginate_with_include_and_order
+    expected = Topic.find(:all, :include => 'replies', :order => 'replies.created_at asc, topics.created_at asc', :limit => 10)
+    assert_equal expected, assigns(:topics)
+  end
+end
diff --git a/actionpack/test/fixtures/companies.yml b/actionpack/test/fixtures/companies.yml
new file mode 100644
index 0000000000..707f72abc6
--- /dev/null
+++ b/actionpack/test/fixtures/companies.yml
@@ -0,0 +1,24 @@
+thirty_seven_signals:
+  id: 1
+  name: 37Signals
+  rating: 4
+
+TextDrive:
+  id: 2
+  name: TextDrive
+  rating: 4
+
+PlanetArgon:
+  id: 3
+  name: Planet Argon
+  rating: 4
+
+Google:
+  id: 4
+  name: Google
+  rating: 4
+  
+Ionist:
+  id: 5
+  name: Ioni.st
+  rating: 4
\ No newline at end of file
diff --git a/actionpack/test/fixtures/company.rb b/actionpack/test/fixtures/company.rb
new file mode 100644
index 0000000000..0d1c29b906
--- /dev/null
+++ b/actionpack/test/fixtures/company.rb
@@ -0,0 +1,9 @@
+class Company < ActiveRecord::Base
+  attr_protected :rating
+  set_sequence_name :companies_nonstd_seq
+
+  validates_presence_of :name
+  def validate
+    errors.add('rating', 'rating should not be 2') if rating == 2
+  end  
+end
\ No newline at end of file
diff --git a/actionpack/test/fixtures/db_definitions/sqlite.sql b/actionpack/test/fixtures/db_definitions/sqlite.sql
new file mode 100644
index 0000000000..b4e7539d16
--- /dev/null
+++ b/actionpack/test/fixtures/db_definitions/sqlite.sql
@@ -0,0 +1,42 @@
+CREATE TABLE 'companies' (
+  'id' INTEGER PRIMARY KEY NOT NULL,
+  'name' TEXT DEFAULT NULL,
+  'rating' INTEGER DEFAULT 1
+);
+
+CREATE TABLE 'replies' (
+  'id' INTEGER PRIMARY KEY NOT NULL, 
+  'content' text, 
+  'created_at' datetime, 
+  'updated_at' datetime, 
+  'topic_id' integer
+);
+
+CREATE TABLE 'topics' (
+  'id' INTEGER PRIMARY KEY NOT NULL, 
+  'title' varchar(255), 
+  'subtitle' varchar(255), 
+  'content' text, 
+  'created_at' datetime, 
+  'updated_at' datetime
+);
+
+CREATE TABLE 'developers' (
+  'id' INTEGER PRIMARY KEY NOT NULL,
+  'name' TEXT DEFAULT NULL,
+  'salary' INTEGER DEFAULT 70000,
+  'created_at' DATETIME DEFAULT NULL,
+  'updated_at' DATETIME DEFAULT NULL
+);
+
+CREATE TABLE 'projects' (
+  'id' INTEGER PRIMARY KEY NOT NULL,
+  'name' TEXT DEFAULT NULL
+);
+
+CREATE TABLE 'developers_projects' (
+  'developer_id' INTEGER NOT NULL,
+  'project_id' INTEGER NOT NULL,
+  'joined_on' DATE DEFAULT NULL,
+  'access_level' INTEGER DEFAULT 1
+);
diff --git a/actionpack/test/fixtures/developer.rb b/actionpack/test/fixtures/developer.rb
new file mode 100644
index 0000000000..f5e5b901fd
--- /dev/null
+++ b/actionpack/test/fixtures/developer.rb
@@ -0,0 +1,7 @@
+class Developer < ActiveRecord::Base
+  has_and_belongs_to_many :projects
+end
+
+class DeVeLoPeR < ActiveRecord::Base
+  set_table_name "developers"
+end
diff --git a/actionpack/test/fixtures/developers.yml b/actionpack/test/fixtures/developers.yml
new file mode 100644
index 0000000000..308bf75de2
--- /dev/null
+++ b/actionpack/test/fixtures/developers.yml
@@ -0,0 +1,21 @@
+david:
+  id: 1
+  name: David
+  salary: 80000
+
+jamis:
+  id: 2
+  name: Jamis
+  salary: 150000
+
+<% for digit in 3..10 %>
+dev_<%= digit %>:
+  id: <%= digit %>
+  name: fixture_<%= digit %>
+  salary: 100000
+<% end %>
+
+poor_jamis:
+  id: 11
+  name: Jamis
+  salary: 9000
\ No newline at end of file
diff --git a/actionpack/test/fixtures/developers_projects.yml b/actionpack/test/fixtures/developers_projects.yml
new file mode 100644
index 0000000000..cee359c7cf
--- /dev/null
+++ b/actionpack/test/fixtures/developers_projects.yml
@@ -0,0 +1,13 @@
+david_action_controller:
+  developer_id: 1
+  project_id: 2
+  joined_on: 2004-10-10
+
+david_active_record:
+  developer_id: 1
+  project_id: 1
+  joined_on: 2004-10-10
+
+jamis_active_record:
+  developer_id: 2
+  project_id: 1
\ No newline at end of file
diff --git a/actionpack/test/fixtures/project.rb b/actionpack/test/fixtures/project.rb
new file mode 100644
index 0000000000..2b53d39ed5
--- /dev/null
+++ b/actionpack/test/fixtures/project.rb
@@ -0,0 +1,3 @@
+class Project < ActiveRecord::Base
+  has_and_belongs_to_many :developers, :uniq => true
+end
diff --git a/actionpack/test/fixtures/projects.yml b/actionpack/test/fixtures/projects.yml
new file mode 100644
index 0000000000..02800c7824
--- /dev/null
+++ b/actionpack/test/fixtures/projects.yml
@@ -0,0 +1,7 @@
+action_controller:
+  id: 2
+  name: Active Controller
+
+active_record:
+  id: 1
+  name: Active Record
diff --git a/actionpack/test/fixtures/replies.yml b/actionpack/test/fixtures/replies.yml
new file mode 100644
index 0000000000..284c9c0796
--- /dev/null
+++ b/actionpack/test/fixtures/replies.yml
@@ -0,0 +1,13 @@
+witty_retort:
+  id: 1
+  topic_id: 1
+  content: Birdman is better!
+  created_at: <%= 6.hours.ago.to_s(:db) %>
+  updated_at: nil
+  
+another:
+  id: 2
+  topic_id: 2
+  content: Nuh uh!
+  created_at: <%= 1.hour.ago.to_s(:db) %>
+  updated_at: nil
\ No newline at end of file
diff --git a/actionpack/test/fixtures/reply.rb b/actionpack/test/fixtures/reply.rb
new file mode 100644
index 0000000000..ea84042b9a
--- /dev/null
+++ b/actionpack/test/fixtures/reply.rb
@@ -0,0 +1,5 @@
+class Reply < ActiveRecord::Base
+  belongs_to :topic, :include => [:replies]
+  
+  validates_presence_of :content
+end
diff --git a/actionpack/test/fixtures/topic.rb b/actionpack/test/fixtures/topic.rb
new file mode 100644
index 0000000000..0929de7e08
--- /dev/null
+++ b/actionpack/test/fixtures/topic.rb
@@ -0,0 +1,3 @@
+class Topic < ActiveRecord::Base
+  has_many :replies, :include => [:user], :dependent => true
+end
\ No newline at end of file
diff --git a/actionpack/test/fixtures/topics.yml b/actionpack/test/fixtures/topics.yml
new file mode 100644
index 0000000000..61ea02d76f
--- /dev/null
+++ b/actionpack/test/fixtures/topics.yml
@@ -0,0 +1,22 @@
+futurama:
+  id: 1
+  title: Isnt futurama awesome?
+  subtitle: It really is, isnt it.
+  content: I like futurama
+  created_at: <%= 1.day.ago.to_s(:db) %>
+  updated_at:
+  
+harvey_birdman:
+  id: 2
+  title: Harvey Birdman is the king of all men
+  subtitle: yup
+  content: It really is
+  created_at: <%= 2.hours.ago.to_s(:db) %>
+  updated_at:
+
+rails:
+  id: 3
+  title: Rails is nice
+  subtitle: It makes me happy
+  content: except when I have to hack internals to fix pagination. even then really.
+  created_at: <%= 20.minutes.ago.to_s(:db) %>