Feature #43355 » 0001-members-pagination.patch
| app/controllers/application_controller.rb | ||
|---|---|---|
| 669 | 669 |
end |
| 670 | 670 |
per_page |
| 671 | 671 |
end |
| 672 |
helper_method :per_page_option |
|
| 672 | 673 | |
| 673 | 674 |
# Returns offset and limit used to retrieve objects |
| 674 | 675 |
# for an API response based on offset, limit and page parameters |
| app/controllers/members_controller.rb | ||
|---|---|---|
| 138 | 138 |
private |
| 139 | 139 | |
| 140 | 140 |
def redirect_to_settings_in_projects |
| 141 |
redirect_to settings_project_path(@project, :tab => 'members') |
|
| 141 |
redirect_to settings_project_path(@project, members_settings_query) |
|
| 142 |
end |
|
| 143 | ||
| 144 |
# Keeps the members tab and its pagination when redirecting after add/edit/remove |
|
| 145 |
def members_settings_query |
|
| 146 |
query = {:tab => 'members'}
|
|
| 147 |
query[:members_page] = params[:members_page] if params[:members_page].present? |
|
| 148 |
query[:per_page] = params[:per_page] if params[:per_page].present? |
|
| 149 |
query |
|
| 142 | 150 |
end |
| 143 | 151 |
end |
| app/helpers/members_helper.rb | ||
|---|---|---|
| 48 | 48 |
s + content_tag('span', links, :class => 'pagination')
|
| 49 | 49 |
end |
| 50 | 50 | |
| 51 |
# Returns the requested page of the project's members together with its |
|
| 52 |
# paginator and the total member count. Member.sorted joins roles (a member |
|
| 53 |
# may have several roles), so the sorted ids are collected first (duplicates |
|
| 54 |
# removed, keeping the lowest role position) and only the current page is |
|
| 55 |
# loaded; this paginates members rather than join rows. |
|
| 56 |
def paginate_members(project) |
|
| 57 |
ordered_ids = |
|
| 58 |
project.memberships. |
|
| 59 |
left_joins(:member_roles => :role).joins(:principal). |
|
| 60 |
reorder("#{Role.table_name}.position").
|
|
| 61 |
order(Principal.fields_for_order_statement). |
|
| 62 |
pluck("#{Member.table_name}.id").uniq
|
|
| 63 |
member_pages = Redmine::Pagination::Paginator.new(ordered_ids.size, per_page_option, params['members_page'], 'members_page') |
|
| 64 |
page_ids = ordered_ids[member_pages.offset, member_pages.per_page] || [] |
|
| 65 |
members_by_id = project.memberships.where(:id => page_ids).preload(:project, :principal, :roles).index_by(&:id) |
|
| 66 |
members = page_ids.filter_map {|id| members_by_id[id]}
|
|
| 67 |
[members, member_pages, ordered_ids.size] |
|
| 68 |
end |
|
| 69 | ||
| 51 | 70 |
# Returns inheritance information for an inherited member role |
| 52 | 71 |
def render_role_inheritance(member, role) |
| 53 | 72 |
content = member.role_inheritance(role).filter_map do |h| |
| app/views/members/_edit.html.erb | ||
|---|---|---|
| 1 |
<%= form_for(@member, :url => membership_path(@member), |
|
| 1 |
<%= form_for(@member, :url => membership_path(@member, :members_page => params[:members_page]),
|
|
| 2 | 2 |
:as => :membership, |
| 3 | 3 |
:remote => request.xhr?, |
| 4 | 4 |
:method => :put) do |f| %> |
| app/views/members/_new_modal.html.erb | ||
|---|---|---|
| 1 | 1 |
<h3 class="title"><%= l(:label_member_new) %></h3> |
| 2 | 2 | |
| 3 |
<%= form_for @member, :as => :membership, :url => project_memberships_path(@project), :remote => true, :method => :post do |f| %> |
|
| 3 |
<%= form_for @member, :as => :membership, :url => project_memberships_path(@project, :members_page => params[:members_page]), :remote => true, :method => :post do |f| %>
|
|
| 4 | 4 |
<%= render :partial => 'new_form' %> |
| 5 | 5 |
<p class="buttons"> |
| 6 | 6 |
<%= submit_tag l(:button_add), :id => 'member-add-submit' %> |
| app/views/projects/settings/_members.html.erb | ||
|---|---|---|
| 1 |
<% members = @project.memberships.preload(:project).sorted.to_a %> |
|
| 1 |
<% |
|
| 2 |
# A dedicated 'members_page' parameter is used so it does not clash with the |
|
| 3 |
# 'page' used by the new-member modal's principal list (render_principals_for_new_members). |
|
| 4 |
members, member_pages, member_count = paginate_members(@project) |
|
| 5 |
%> |
|
| 2 | 6 | |
| 3 | 7 |
<% if User.current.admin? %> |
| 4 | 8 |
<div class="contextual"><%= link_to sprite_icon('settings', l(:label_administration)), users_path, :class => "icon icon-settings" %></div>
|
| 5 | 9 |
<% end %> |
| 6 |
<p><%= link_to sprite_icon('add', l(:label_member_new)), new_project_membership_path(@project), :remote => true, :class => "icon icon-add" %></p>
|
|
| 10 |
<p><%= link_to sprite_icon('add', l(:label_member_new)), new_project_membership_path(@project, :members_page => params[:members_page]), :remote => true, :class => "icon icon-add" %></p>
|
|
| 7 | 11 | |
| 8 | 12 |
<% if members.any? %> |
| 9 | 13 |
<table class="list members"> |
| ... | ... | |
| 32 | 36 |
</td> |
| 33 | 37 |
<td class="buttons"> |
| 34 | 38 |
<%= link_to sprite_icon('edit', l(:button_edit)),
|
| 35 |
edit_membership_path(member), |
|
| 39 |
edit_membership_path(member, :members_page => params[:members_page]),
|
|
| 36 | 40 |
:remote => true, |
| 37 | 41 |
:class => 'icon icon-edit' %> |
| 38 |
<%= remove_link membership_path(member), |
|
| 42 |
<%= remove_link membership_path(member, :members_page => params[:members_page]),
|
|
| 39 | 43 |
:remote => true, |
| 40 | 44 |
:data => (!User.current.admin? && member.include?(User.current) ? {:confirm => l(:text_own_membership_delete_confirmation)} : {}) if member.deletable? %>
|
| 41 | 45 |
</td> |
| ... | ... | |
| 44 | 48 |
<% end %> |
| 45 | 49 |
</tbody> |
| 46 | 50 |
</table> |
| 51 |
<span class="pagination"> |
|
| 52 |
<%= pagination_links_full(member_pages, member_count) do |text, parameters, options| |
|
| 53 |
link_to text, settings_project_path(@project, 'members', request.query_parameters.merge(parameters)), options |
|
| 54 |
end %> |
|
| 55 |
</span> |
|
| 47 | 56 |
<% other_formats_links do |f| %> |
| 48 | 57 |
<%= f.link_to_with_query_parameters "CSV", {}, :onclick => "showModal('csv-export-options', '330px'); return false;" %>
|
| 49 | 58 |
<% end %> |
| test/functional/members_controller_test.rb | ||
|---|---|---|
| 224 | 224 |
assert_redirected_to '/projects/ecookbook/settings/members' |
| 225 | 225 |
end |
| 226 | 226 | |
| 227 |
def test_update_xhr_should_keep_members_tab_in_pagination_links |
|
| 228 |
project = Project.find(1) |
|
| 229 |
26.times { User.add_to_project(User.generate!, project) }
|
|
| 230 |
@request.session[:user_id] = 2 |
|
| 231 |
with_settings :per_page_options => '25,50,100' do |
|
| 232 |
put( |
|
| 233 |
:update, |
|
| 234 |
:params => {
|
|
| 235 |
:id => 2, |
|
| 236 |
:membership => {:role_ids => [1]}
|
|
| 237 |
}, |
|
| 238 |
:xhr => true |
|
| 239 |
) |
|
| 240 |
end |
|
| 241 |
assert_response :success |
|
| 242 |
# The re-rendered members tab keeps its pagination links scoped to the members tab |
|
| 243 |
assert_match %r{settings/members\?members_page=}, response.body
|
|
| 244 |
end |
|
| 245 | ||
| 246 |
def test_edit_xhr_should_keep_current_page_in_form_action |
|
| 247 |
@request.session[:user_id] = 2 |
|
| 248 |
get(:edit, :params => {:id => 2, :members_page => 2}, :xhr => true)
|
|
| 249 |
assert_response :success |
|
| 250 |
# The edit form posts back with the current page so the list stays on it after saving |
|
| 251 |
assert_match %r{/memberships/2\?members_page=2}, response.body
|
|
| 252 |
end |
|
| 253 | ||
| 254 |
def test_new_xhr_should_keep_current_page_in_form_action |
|
| 255 |
@request.session[:user_id] = 2 |
|
| 256 |
get(:new, :params => {:project_id => 1, :members_page => 2}, :xhr => true)
|
|
| 257 |
assert_response :success |
|
| 258 |
# The new member form posts with the members-tab page so the list stays on it after adding |
|
| 259 |
assert_match %r{/projects/ecookbook/memberships\?members_page=2}, response.body
|
|
| 260 |
end |
|
| 261 | ||
| 227 | 262 |
def test_update_locked_member_should_be_allowed |
| 228 | 263 |
User.find(3).lock! |
| 229 | 264 | |
| test/functional/projects_controller_test.rb | ||
|---|---|---|
| 1030 | 1030 |
assert_select "tr#member-#{group_member.id} td.name a[href=?]", '/groups/10', :text => 'A Team'
|
| 1031 | 1031 |
end |
| 1032 | 1032 | |
| 1033 |
def test_settings_members_should_be_paginated |
|
| 1034 |
project = Project.find(1) |
|
| 1035 |
per_page = 25 |
|
| 1036 |
# Ensure there are more members than fit on a single page |
|
| 1037 |
(per_page + 5).times { User.add_to_project(User.generate!, project) }
|
|
| 1038 |
@request.session[:user_id] = 2 |
|
| 1039 |
with_settings :per_page_options => '25,50,100' do |
|
| 1040 |
get( |
|
| 1041 |
:settings, |
|
| 1042 |
:params => {:id => 'ecookbook', :tab => 'members'}
|
|
| 1043 |
) |
|
| 1044 |
end |
|
| 1045 |
assert_response :success |
|
| 1046 |
assert_select 'div#tab-content-members table.list.members tbody tr.member', :count => per_page |
|
| 1047 |
assert_select 'div#tab-content-members span.pagination' |
|
| 1048 |
# Pagination links must keep the members tab in the path and use members_page |
|
| 1049 |
assert_select 'div#tab-content-members span.pagination a[href*=?]', 'settings/members?members_page=' |
|
| 1050 |
end |
|
| 1051 | ||
| 1052 |
def test_settings_members_with_multiple_roles_should_not_appear_on_two_pages |
|
| 1053 |
project = Project.find(1) |
|
| 1054 |
30.times { User.add_to_project(User.generate!, project) }
|
|
| 1055 |
# A member with several roles must still be listed once (no join-row paging) |
|
| 1056 |
Member.where(:project_id => project.id).first.update!(:role_ids => [1, 2]) |
|
| 1057 |
@request.session[:user_id] = 2 |
|
| 1058 |
page1 = page2 = nil |
|
| 1059 |
with_settings :per_page_options => '25,50,100' do |
|
| 1060 |
get(:settings, :params => {:id => 'ecookbook', :tab => 'members'})
|
|
| 1061 |
page1 = css_select('div#tab-content-members tr.member').pluck('id')
|
|
| 1062 |
get(:settings, :params => {:id => 'ecookbook', :tab => 'members', :members_page => 2})
|
|
| 1063 |
page2 = css_select('div#tab-content-members tr.member').pluck('id')
|
|
| 1064 |
end |
|
| 1065 |
assert_not page1.intersect?(page2), 'a member must not appear on more than one page' |
|
| 1066 |
end |
|
| 1067 | ||
| 1068 |
def test_settings_members_should_show_requested_page |
|
| 1069 |
project = Project.find(1) |
|
| 1070 |
per_page = 25 |
|
| 1071 |
(per_page + 5).times { User.add_to_project(User.generate!, project) }
|
|
| 1072 |
@request.session[:user_id] = 2 |
|
| 1073 |
with_settings :per_page_options => '25,50,100' do |
|
| 1074 |
get( |
|
| 1075 |
:settings, |
|
| 1076 |
:params => {:id => 'ecookbook', :tab => 'members', :members_page => 2}
|
|
| 1077 |
) |
|
| 1078 |
end |
|
| 1079 |
assert_response :success |
|
| 1080 |
# The second page holds the remaining members (fewer than a full page) |
|
| 1081 |
assert_select 'div#tab-content-members table.list.members tbody tr.member' |
|
| 1082 |
assert_select 'div#tab-content-members span.pagination' |
|
| 1083 |
end |
|
| 1084 | ||
| 1033 | 1085 |
def test_settings_should_show_tabs_depending_on_permission |
| 1034 | 1086 |
@request.session[:user_id] = 3 |
| 1035 | 1087 |
project = Project.find(1) |
| test/helpers/members_helper_test.rb | ||
|---|---|---|
| 39 | 39 |
assert_select_in result, 'span.pagination li.current span', :text => '1' |
| 40 | 40 |
assert_select_in result, 'a[href=?]', "/projects/#{project.identifier}/memberships/autocomplete.js?page=2", :text => '2'
|
| 41 | 41 |
end |
| 42 | ||
| 43 |
def test_paginate_members_returns_only_the_requested_page |
|
| 44 |
# per_page_option is provided by ApplicationController in the running app |
|
| 45 |
stubs(:per_page_option).returns(3) |
|
| 46 |
project = Project.generate! |
|
| 47 |
5.times { User.add_to_project(User.generate!, project) }
|
|
| 48 | ||
| 49 |
members, member_pages, member_count = paginate_members(project) |
|
| 50 | ||
| 51 |
assert_equal 3, members.size |
|
| 52 |
assert_equal 3, member_pages.per_page |
|
| 53 |
assert_equal project.memberships.count, member_count |
|
| 54 |
end |
|
| 55 | ||
| 56 |
def test_paginate_members_lists_a_member_with_several_roles_once |
|
| 57 |
stubs(:per_page_option).returns(3) |
|
| 58 |
project = Project.generate! |
|
| 59 |
3.times { User.add_to_project(User.generate!, project) }
|
|
| 60 |
# Give one member several roles: Member.sorted would return it on multiple |
|
| 61 |
# join rows, but it must still be listed only once. |
|
| 62 |
Member.where(:project_id => project.id).first.update!(:role_ids => [1, 2]) |
|
| 63 | ||
| 64 |
members, _member_pages, member_count = paginate_members(project) |
|
| 65 | ||
| 66 |
member_ids = members.map(&:id) |
|
| 67 |
assert_equal member_ids.uniq, member_ids |
|
| 68 |
assert_equal member_count, members.size |
|
| 69 |
assert_equal project.memberships.count, member_count |
|
| 70 |
end |
|
| 42 | 71 |
end |