diff options
| -rw-r--r-- | app/controllers/messages_controller.rb | 24 | ||||
| -rw-r--r-- | app/helpers/application_helper.rb | 9 | ||||
| -rw-r--r-- | app/helpers/messages_helper.rb | 1 | ||||
| -rw-r--r-- | app/models/message.rb | 2 | ||||
| -rw-r--r-- | app/views/messages/show.rhtml | 3 | ||||
| -rw-r--r-- | test/functional/messages_controller_test.rb | 19 |
6 files changed, 46 insertions, 12 deletions
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index fe5451944..bab0e1e19 100644 --- a/app/controllers/messages_controller.rb +++ b/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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 02db2caf5..fd7d821d0 100644 --- a/app/helpers/application_helper.rb +++ b/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 diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index 034d1ad31..ed58ad796 100644 --- a/app/helpers/messages_helper.rb +++ b/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 diff --git a/app/models/message.rb b/app/models/message.rb index 1e59719dd..3744c239b 100644 --- a/app/models/message.rb +++ b/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 diff --git a/app/views/messages/show.rhtml b/app/views/messages/show.rhtml index 079b9887e..ec6fd4fa9 100644 --- a/app/views/messages/show.rhtml +++ b/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') %> diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb index 2522f0a87..34614a9ef 100644 --- a/test/functional/messages_controller_test.rb +++ b/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 |