Index: app/controllers/messages_controller.rb =================================================================== --- app/controllers/messages_controller.rb (revision 16559) +++ app/controllers/messages_controller.rb (working copy) @@ -28,22 +28,23 @@ helper :attachments include AttachmentsHelper - REPLIES_PER_PAGE = 25 unless const_defined?(:REPLIES_PER_PAGE) - # Show a topic and its replies def show page = params[:page] # Find the page of the requested reply + replies_order = User.current.wants_comments_in_reverse_order? ? 'DESC' : 'ASC' + @replies = @topic.children. + includes(:author, :attachments, {:board => :project}). + reorder("#{Message.table_name}.created_on #{replies_order}, #{Message.table_name}.id #{replies_order}") + if params[:r] && page.nil? - offset = @topic.children.where("#{Message.table_name}.id < ?", params[:r].to_i).count - page = 1 + offset / REPLIES_PER_PAGE + offset = @replies.pluck(:id).index(params[:r].to_i) + page = 1 + offset / per_page_option end - @reply_count = @topic.children.count - @reply_pages = Paginator.new @reply_count, REPLIES_PER_PAGE, page - @replies = @topic.children. - includes(:author, :attachments, {:board => :project}). - reorder("#{Message.table_name}.created_on ASC, #{Message.table_name}.id ASC"). + @reply_count = @replies.count + @reply_pages = Paginator.new @reply_count, per_page_option, page + @replies = @replies. limit(@reply_pages.per_page). offset(@reply_pages.offset). to_a Index: app/views/messages/show.html.erb =================================================================== --- app/views/messages/show.html.erb (revision 16559) +++ app/views/messages/show.html.erb (working copy) @@ -74,7 +74,7 @@ <%= link_to_attachments message, :author => false, :thumbnails => true %> <% end %> -<%= pagination_links_full @reply_pages, @reply_count, :per_page_links => false %> +<%= pagination_links_full @reply_pages, @reply_count %> <% end %> <% if !@topic.locked? && authorize_for('messages', 'reply') %> Index: test/integration/messages_test.rb =================================================================== --- test/integration/messages_test.rb (nonexistent) +++ test/integration/messages_test.rb (working copy) @@ -0,0 +1,44 @@ +require File.expand_path('../../test_helper', __FILE__) + +class MessagesTest < Redmine::IntegrationTest + fixtures :projects, :users, :email_addresses, :user_preferences, :members, + :member_roles, :roles, :boards, :messages, :enabled_modules + + def setup + message = Message.find(1) + assert_difference 'Message.count', 60 do + 60.times do + message.children << Message.new( + :subject => 'Reply', + :content => 'Reply body', + :author_id => 2, + :board_id => 1) + end + end + @reply_ids = message.children.map(&:id).sort + @per_page = (@reply_ids.count - 1).to_s + + log_user('jsmith', 'jsmith') + end + + def test_show_with_pagination_should_ascending_order + with_settings :per_page_options => @per_page do + post '/my/account', :pref => { :comments_sorting => 'asc' } + assert !User.current.wants_comments_in_reverse_order? + + get '/boards/1/topics/1', :r => @reply_ids.last + assert_select 'span.pagination > ul.pages > li.current > span', text: '2' + end + end + + def test_show_with_pagination_should_descending_order + with_settings :per_page_options => @per_page do + post '/my/account', :pref => { :comments_sorting => 'desc' } + assert User.current.wants_comments_in_reverse_order? + + get '/boards/1/topics/1', :r => @reply_ids.last + assert_select 'span.pagination > ul.pages > li.current > span', text: '1' + end + end + +end