Patch #26030 » reply_display_order_with_pre_page_2.patch
| app/controllers/messages_controller.rb (working copy) | ||
|---|---|---|
| 30 | 30 |
helper :attachments |
| 31 | 31 |
include AttachmentsHelper |
| 32 | 32 | |
| 33 |
REPLIES_PER_PAGE = 25 unless const_defined?(:REPLIES_PER_PAGE) |
|
| 34 | ||
| 35 | 33 |
# Show a topic and its replies |
| 36 | 34 |
def show |
| 37 | 35 |
page = params[:page] |
| 38 | 36 |
# Find the page of the requested reply |
| 37 |
replies_order = User.current.wants_comments_in_reverse_order? ? 'DESC' : 'ASC' |
|
| 38 |
@replies = @topic.children. |
|
| 39 |
reorder("#{Message.table_name}.created_on #{replies_order}, #{Message.table_name}.id #{replies_order}")
|
|
| 40 | ||
| 39 | 41 |
if params[:r] && page.nil? |
| 40 |
offset = @topic.children.where("#{Message.table_name}.id < ?", params[:r].to_i).count
|
|
| 41 |
page = 1 + offset / REPLIES_PER_PAGE
|
|
| 42 |
offset = @replies.pluck(:id).index(params[:r].to_i)
|
|
| 43 |
page = 1 + offset / per_page_option
|
|
| 42 | 44 |
end |
| 43 | 45 | |
| 44 |
@reply_count = @topic.children.count
|
|
| 45 |
@reply_pages = Paginator.new @reply_count, REPLIES_PER_PAGE, page
|
|
| 46 |
@replies = @topic.children.
|
|
| 46 |
@reply_count = @replies.count
|
|
| 47 |
@reply_pages = Paginator.new @reply_count, per_page_option, page
|
|
| 48 |
@replies = @replies.
|
|
| 47 | 49 |
includes(:author, :attachments, {:board => :project}).
|
| 48 |
reorder("#{Message.table_name}.created_on ASC, #{Message.table_name}.id ASC").
|
|
| 49 | 50 |
limit(@reply_pages.per_page). |
| 50 | 51 |
offset(@reply_pages.offset). |
| 51 | 52 |
to_a |
| ... | ... | |
| 107 | 108 |
@message.destroy |
| 108 | 109 |
flash[:notice] = l(:notice_successful_delete) |
| 109 | 110 |
if @message.parent |
| 110 |
redirect_to board_message_path(@board, @message.parent, :r => r)
|
|
| 111 |
redirect_to board_message_path(@board, @message.parent) |
|
| 111 | 112 |
else |
| 112 | 113 |
redirect_to project_board_path(@project, @board) |
| 113 | 114 |
end |
| app/views/messages/_reply_form.html.erb (working copy) | ||
|---|---|---|
| 1 |
<% has_form ||= false %> |
|
| 2 |
<% if !@topic.locked? && authorize_for('messages', 'reply') %>
|
|
| 3 |
<% if has_form %> |
|
| 4 |
<p><%= toggle_link l(:button_reply), "reply", :focus => 'message_content' %></p> |
|
| 5 |
<div id="reply" style="display:none;"> |
|
| 6 |
<%= form_for @reply, :as => :reply, :url => {:action => 'reply', :id => @topic}, :html => {:multipart => true, :id => 'message-form'} do |f| %>
|
|
| 7 |
<%= render :partial => 'form', :locals => {:f => f, :replying => true} %>
|
|
| 8 |
<%= submit_tag l(:button_submit) %> |
|
| 9 |
<% end %> |
|
| 10 |
</div> |
|
| 11 |
<% else %> |
|
| 12 |
<p><%= toggle_link l(:button_reply), "reply", :focus => 'message_content', :scroll => 'message_content' %></p> |
|
| 13 |
<% end %> |
|
| 14 |
<% end %> |
|
| app/views/messages/show.html.erb (working copy) | ||
|---|---|---|
| 36 | 36 |
<% unless @replies.empty? %> |
| 37 | 37 |
<div id="replies"> |
| 38 | 38 |
<h3 class="comments icon icon-comments"><%= l(:label_reply_plural) %> (<%= @reply_count %>)</h3> |
| 39 |
<% if !@topic.locked? && authorize_for('messages', 'reply') && @replies.size >= 3 %>
|
|
| 40 |
<p><%= toggle_link l(:button_reply), "reply", :focus => 'message_content', :scroll => "message_content" %></p> |
|
| 41 |
<% end %> |
|
| 39 |
<%= render :partial => 'reply_form', :locals => {:has_form => User.current.wants_comments_in_reverse_order? } %>
|
|
| 42 | 40 |
<% @replies.each do |message| %> |
| 43 | 41 |
<div class="message reply" id="<%= "message-#{message.id}" %>">
|
| 44 | 42 |
<div class="contextual"> |
| ... | ... | |
| 76 | 74 |
</div> |
| 77 | 75 |
<% end %> |
| 78 | 76 |
</div> |
| 79 |
<span class="pagination"><%= pagination_links_full @reply_pages, @reply_count, :per_page_links => false %></span>
|
|
| 77 |
<span class="pagination"><%= pagination_links_full @reply_pages, @reply_count %></span> |
|
| 80 | 78 |
<% end %> |
| 81 | 79 | |
| 82 |
<% if !@topic.locked? && authorize_for('messages', 'reply') %>
|
|
| 83 |
<p><%= toggle_link l(:button_reply), "reply", :focus => 'message_content' %></p> |
|
| 84 |
<div id="reply" style="display:none;"> |
|
| 85 |
<%= form_for @reply, :as => :reply, :url => {:action => 'reply', :id => @topic}, :html => {:multipart => true, :id => 'message-form'} do |f| %>
|
|
| 86 |
<%= render :partial => 'form', :locals => {:f => f, :replying => true} %>
|
|
| 87 |
<%= submit_tag l(:button_submit) %> |
|
| 88 |
<% end %> |
|
| 89 |
</div> |
|
| 90 |
<% end %> |
|
| 80 |
<%= render :partial => 'reply_form', :locals => {:has_form => @replies.empty? || !User.current.wants_comments_in_reverse_order? } %>
|
|
| 91 | 81 | |
| 92 | 82 |
<% html_title @topic.subject %> |
| test/functional/messages_controller_test.rb (working copy) | ||
|---|---|---|
| 252 | 252 |
:id => 2 |
| 253 | 253 |
} |
| 254 | 254 |
end |
| 255 |
assert_redirected_to '/boards/1/topics/1?r=2'
|
|
| 255 |
assert_redirected_to '/boards/1/topics/1' |
|
| 256 | 256 |
assert_equal I18n.t(:notice_successful_delete), flash[:notice] |
| 257 | 257 |
assert_nil Message.find_by_id(2) |
| 258 | 258 |
end |
| test/integration/messages_test.rb (working copy) | ||
|---|---|---|
| 1 |
# frozen_string_literal: true |
|
| 2 | ||
| 3 |
# Redmine - project management software |
|
| 4 |
# Copyright (C) 2006-2019 Jean-Philippe Lang |
|
| 5 |
# |
|
| 6 |
# This program is free software; you can redistribute it and/or |
|
| 7 |
# modify it under the terms of the GNU General Public License |
|
| 8 |
# as published by the Free Software Foundation; either version 2 |
|
| 9 |
# of the License, or (at your option) any later version. |
|
| 10 |
# |
|
| 11 |
# This program is distributed in the hope that it will be useful, |
|
| 12 |
# but WITHOUT ANY WARRANTY; without even the implied warranty of |
|
| 13 |
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
|
| 14 |
# GNU General Public License for more details. |
|
| 15 |
# |
|
| 16 |
# You should have received a copy of the GNU General Public License |
|
| 17 |
# along with this program; if not, write to the Free Software |
|
| 18 |
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. |
|
| 19 | ||
| 20 |
require File.expand_path('../../test_helper', __FILE__)
|
|
| 21 | ||
| 22 |
class MessagesTest < Redmine::IntegrationTest |
|
| 23 |
fixtures :projects, :users, :email_addresses, :user_preferences, :members, |
|
| 24 |
:member_roles, :roles, :boards, :messages, :enabled_modules |
|
| 25 | ||
| 26 |
def setup |
|
| 27 |
message = Message.find(1) |
|
| 28 |
assert_difference 'Message.count', 60 do |
|
| 29 |
60.times do |
|
| 30 |
message.children << Message.new( |
|
| 31 |
:subject => 'Reply', |
|
| 32 |
:content => 'Reply body', |
|
| 33 |
:author_id => 2, |
|
| 34 |
:board_id => 1) |
|
| 35 |
end |
|
| 36 |
end |
|
| 37 |
@reply_ids = message.children.map(&:id).sort |
|
| 38 |
@per_page = (@reply_ids.count - 1).to_s |
|
| 39 | ||
| 40 |
log_user('jsmith', 'jsmith')
|
|
| 41 |
end |
|
| 42 | ||
| 43 |
def test_show_with_pagination_should_ascending_order |
|
| 44 |
with_settings :per_page_options => @per_page do |
|
| 45 |
put '/my/account', :params => { :pref => { :comments_sorting => 'asc' }}
|
|
| 46 |
assert !User.current.wants_comments_in_reverse_order? |
|
| 47 | ||
| 48 |
get '/boards/1/topics/1', :params => { :r => @reply_ids.last }
|
|
| 49 |
assert_select 'span.pagination > ul.pages > li.current > span', text: '2' |
|
| 50 |
end |
|
| 51 |
end |
|
| 52 | ||
| 53 |
def test_show_with_pagination_should_descending_order |
|
| 54 |
with_settings :per_page_options => @per_page do |
|
| 55 |
put '/my/account', :params => { :pref => { :comments_sorting => 'desc' }}
|
|
| 56 |
assert User.current.wants_comments_in_reverse_order? |
|
| 57 | ||
| 58 |
get '/boards/1/topics/1', :params => { :r => @reply_ids.last }
|
|
| 59 |
assert_select 'span.pagination > ul.pages > li.current > span', text: '1' |
|
| 60 |
end |
|
| 61 |
end |
|
| 62 | ||
| 63 |
end |
|
| 64 |
require File.expand_path('../../test_helper', __FILE__)
|
|
| 65 | ||
| 66 |
class MessagesTest < Redmine::IntegrationTest |
|
| 67 |
fixtures :projects, :users, :email_addresses, :user_preferences, :members, |
|
| 68 |
:member_roles, :roles, :boards, :messages, :enabled_modules |
|
| 69 | ||
| 70 |
def setup |
|
| 71 |
message = Message.find(1) |
|
| 72 |
assert_difference 'Message.count', 60 do |
|
| 73 |
60.times do |
|
| 74 |
message.children << Message.new( |
|
| 75 |
:subject => 'Reply', |
|
| 76 |
:content => 'Reply body', |
|
| 77 |
:author_id => 2, |
|
| 78 |
:board_id => 1) |
|
| 79 |
end |
|
| 80 |
end |
|
| 81 |
@reply_ids = message.children.map(&:id).sort |
|
| 82 |
@per_page = (@reply_ids.count - 1).to_s |
|
| 83 | ||
| 84 |
log_user('jsmith', 'jsmith')
|
|
| 85 |
end |
|
| 86 | ||
| 87 |
def test_show_with_pagination_should_ascending_order |
|
| 88 |
with_settings :per_page_options => @per_page do |
|
| 89 |
put '/my/account', :params => { :pref => { :comments_sorting => 'asc' }}
|
|
| 90 |
assert !User.current.wants_comments_in_reverse_order? |
|
| 91 | ||
| 92 |
get '/boards/1/topics/1', :params => { :r => @reply_ids.last }
|
|
| 93 |
assert_select 'span.pagination > ul.pages > li.current > span', text: '2' |
|
| 94 |
end |
|
| 95 |
end |
|
| 96 | ||
| 97 |
def test_show_with_pagination_should_descending_order |
|
| 98 |
with_settings :per_page_options => @per_page do |
|
| 99 |
put '/my/account', :params => { :pref => { :comments_sorting => 'desc' }}
|
|
| 100 |
assert User.current.wants_comments_in_reverse_order? |
|
| 101 | ||
| 102 |
get '/boards/1/topics/1', :params => { :r => @reply_ids.last }
|
|
| 103 |
assert_select 'span.pagination > ul.pages > li.current > span', text: '1' |
|
| 104 |
end |
|
| 105 |
end |
|
| 106 | ||
| 107 |
end |
|