Adds pagination to forum messages (#4664).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3373 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/messages_controller.rb

@@ -29,10 +29,24 @@ class MessagesController < ApplicationController
   helper :attachments
   include AttachmentsHelper   
 
+  REPLIES_PER_PAGE = 25 unless const_defined?(:REPLIES_PER_PAGE)
+  
   # Show a topic and its replies
   def show
-    @replies = @topic.children.find(:all, :include => [:author, :attachments, {:board => :project}])
-    @replies.reverse! if User.current.wants_comments_in_reverse_order?
+    page = params[:page]
+    # Find the page of the requested reply
+    if params[:r] && page.nil?
+      offset = @topic.children.count(:conditions => ["#{Message.table_name}.id < ?", params[:r].to_i])
+      page = 1 + offset / REPLIES_PER_PAGE
+    end
+    
+    @reply_count = @topic.children.count
+    @reply_pages = Paginator.new self, @reply_count, REPLIES_PER_PAGE, page
+    @replies =  @topic.children.find(:all, :include => [:author, :attachments, {:board => :project}],
+                                           :order => "#{Message.table_name}.created_on ASC",
+                                           :limit => @reply_pages.items_per_page,
+                                           :offset => @reply_pages.current.offset)
+    
     @reply = Message.new(:subject => "RE: #{@message.subject}")
     render :action => "show", :layout => false if request.xhr?
   end
@@ -63,7 +77,7 @@ class MessagesController < ApplicationController
       call_hook(:controller_messages_reply_after_save, { :params => params, :message => @reply})
       attach_files(@reply, params[:attachments])
     end
-    redirect_to :action => 'show', :id => @topic
+    redirect_to :action => 'show', :id => @topic, :r => @reply
   end
 
   # Edit a message
@@ -77,7 +91,7 @@ class MessagesController < ApplicationController
       attach_files(@message, params[:attachments])
       flash[:notice] = l(:notice_successful_update)
       @message.reload
-      redirect_to :action => 'show', :board_id => @message.board, :id => @message.root
+      redirect_to :action => 'show', :board_id => @message.board, :id => @message.root, :r => (@message.parent_id && @message.id)
     end
   end
   
@@ -87,7 +101,7 @@ class MessagesController < ApplicationController
     @message.destroy
     redirect_to @message.parent.nil? ?
       { :controller => 'boards', :action => 'show', :project_id => @project, :id => @board } :
-      { :action => 'show', :id => @message.parent }
+      { :action => 'show', :id => @message.parent, :r => @message }
   end
   
   def quote

app/helpers/application_helper.rb

@@ -289,6 +289,7 @@ module ApplicationHelper
 
   def pagination_links_full(paginator, count=nil, options={})
     page_param = options.delete(:page_param) || :page
+    per_page_links = options.delete(:per_page_links)
     url_param = params.dup
     # don't reuse query params if filters are present
     url_param.merge!(:fields => nil, :values => nil, :operators => nil) if url_param.delete(:set_filter)
@@ -307,10 +308,10 @@ module ApplicationHelper
     end
 
     unless count.nil?
-      html << [
-        " (#{paginator.current.first_item}-#{paginator.current.last_item}/#{count})",
-        per_page_links(paginator.items_per_page)
-      ].compact.join(' | ')
+      html << " (#{paginator.current.first_item}-#{paginator.current.last_item}/#{count})"
+      if per_page_links != false && links = per_page_links(paginator.items_per_page)
+	      html << " | #{links}"
+      end
     end
 
     html

app/helpers/messages_helper.rb

@@ -23,6 +23,7 @@ module MessagesHelper
                                            :action => 'show',
                                            :board_id => message.board_id,
                                            :id => message.root,
+                                           :r => (message.parent_id && message.id),
                                            :anchor => (message.parent_id ? "message-#{message.id}" : nil)
   end
 end

app/models/message.rb

@@ -30,7 +30,7 @@ class Message < ActiveRecord::Base
                 :description => :content,
                 :type => Proc.new {|o| o.parent_id.nil? ? 'message' : 'reply'},
                 :url => Proc.new {|o| {:controller => 'messages', :action => 'show', :board_id => o.board_id}.merge(o.parent_id.nil? ? {:id => o.id} : 
-                                                                                                                                       {:id => o.parent_id, :anchor => "message-#{o.id}"})}
+                                                                                                                                       {:id => o.parent_id, :r => o.id, :anchor => "message-#{o.id}"})}
 
   acts_as_activity_provider :find_options => {:include => [{:board => :project}, :author]},
                             :author_key => :author_id

app/views/messages/show.rhtml

@@ -20,7 +20,7 @@
 <br />
 
 <% unless @replies.empty? %>
-<h3 class="comments"><%= l(:label_reply_plural) %></h3>
+<h3 class="comments"><%= l(:label_reply_plural) %> (<%= @reply_count %>)</h3>
 <% @replies.each do |message| %>
   <div class="message reply" id="<%= "message-#{message.id}" %>">
     <div class="contextual">
@@ -38,6 +38,7 @@
   <%= link_to_attachments message, :author => false %>
   </div>
 <% end %>
+<p class="pagination"><%= pagination_links_full @reply_pages, @reply_count, :per_page_links => false %></p>
 <% end %>
 
 <% if !@topic.locked? && authorize_for('messages', 'reply') %>

test/functional/messages_controller_test.rb

@@ -47,6 +47,22 @@ class MessagesControllerTest < ActionController::TestCase
     assert_not_nil assigns(:topic)
   end
   
+  def test_show_with_pagination
+    message = Message.find(1)
+    assert_difference 'Message.count', 30 do
+      30.times do 
+        message.children << Message.new(:subject => 'Reply', :content => 'Reply body', :author_id => 2, :board_id => 1)
+      end
+    end
+    get :show, :board_id => 1, :id => 1, :r => message.children.last(:order => 'id').id
+    assert_response :success
+    assert_template 'show'
+    replies = assigns(:replies)
+    assert_not_nil replies
+    assert !replies.include?(message.children.first(:order => 'id'))
+    assert replies.include?(message.children.last(:order => 'id'))
+  end
+  
   def test_show_with_reply_permission
     @request.session[:user_id] = 2
     get :show, :board_id => 1, :id => 1
@@ -143,7 +159,8 @@ class MessagesControllerTest < ActionController::TestCase
   def test_reply
     @request.session[:user_id] = 2
     post :reply, :board_id => 1, :id => 1, :reply => { :content => 'This is a test reply', :subject => 'Test reply' }
-    assert_redirected_to 'boards/1/topics/1'
+    reply = Message.find(:first, :order => 'id DESC')
+    assert_redirected_to "boards/1/topics/1?r=#{reply.id}"
     assert Message.find_by_subject('Test reply')
   end