From 5d84913c4a1dd87f19348602f82dff08185f173d Mon Sep 17 00:00:00 2001 From: takenory Date: Mon, 3 Aug 2026 18:15:53 +0900 Subject: [PATCH 1/2] Add pagination to the project members management list (#43355) --- app/controllers/application_controller.rb | 1 + app/controllers/members_controller.rb | 9 ++- app/helpers/members_helper.rb | 16 ++++ app/views/members/_edit.html.erb | 2 +- app/views/members/_new_modal.html.erb | 2 +- app/views/projects/settings/_members.html.erb | 17 +++-- test/functional/members_controller_test.rb | 32 ++++++++ test/functional/projects_controller_test.rb | 74 +++++++++++++++++++ test/helpers/members_helper_test.rb | 27 +++++++ 9 files changed, 172 insertions(+), 8 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6dc17d26b5..b573334647 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -682,6 +682,7 @@ class ApplicationController < ActionController::Base end per_page end + helper_method :per_page_option # Returns offset and limit used to retrieve objects # for an API response based on offset, limit and page parameters diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 84be32d42c..8508fdb40d 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -138,6 +138,13 @@ class MembersController < ApplicationController private def redirect_to_settings_in_projects - redirect_to settings_project_path(@project, :tab => 'members') + redirect_to settings_project_path(@project, members_settings_url_params) + end + + def members_settings_url_params + query = {:tab => 'members'} + query[:members_page] = params[:members_page] if params[:members_page].present? + query[:per_page] = params[:per_page] if params[:per_page].present? + query end end diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index 195f8c0c0c..f1791a8073 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -48,6 +48,22 @@ module MembersHelper s + content_tag('span', links, :class => 'pagination') end + # limit/offset on Member.sorted would paginate role join rows, not members + def paginate_members(project) + ordered_ids = + project.memberships. + left_joins(:member_roles => :role).joins(:principal). + reorder("#{Role.table_name}.position"). + order(Principal.fields_for_order_statement). + pluck("#{Member.table_name}.id").uniq + member_count = ordered_ids.size + member_pages = Redmine::Pagination::Paginator.new(member_count, per_page_option, params['members_page'], 'members_page') + page_ids = ordered_ids[member_pages.offset, member_pages.per_page] || [] + members_by_id = project.memberships.where(:id => page_ids).preload(:project, :principal, :roles).index_by(&:id) + members = page_ids.filter_map {|id| members_by_id[id]} + [members, member_pages, member_count] + end + # Returns inheritance information for an inherited member role def render_role_inheritance(member, role) content = member.role_inheritance(role).filter_map do |h| diff --git a/app/views/members/_edit.html.erb b/app/views/members/_edit.html.erb index 8b05ae92f6..09369dc76a 100644 --- a/app/views/members/_edit.html.erb +++ b/app/views/members/_edit.html.erb @@ -1,4 +1,4 @@ -<%= form_for(@member, :url => membership_path(@member), +<%= form_for(@member, :url => membership_path(@member, :members_page => params[:members_page]), :as => :membership, :remote => request.xhr?, :method => :put) do |f| %> diff --git a/app/views/members/_new_modal.html.erb b/app/views/members/_new_modal.html.erb index 3cb3360f5e..9719a254df 100644 --- a/app/views/members/_new_modal.html.erb +++ b/app/views/members/_new_modal.html.erb @@ -1,6 +1,6 @@

<%= l(:label_member_new) %>

-<%= form_for @member, :as => :membership, :url => project_memberships_path(@project), :remote => true, :method => :post do |f| %> +<%= form_for @member, :as => :membership, :url => project_memberships_path(@project, :members_page => params[:members_page]), :remote => true, :method => :post do |f| %> <%= render :partial => 'new_form' %>

<%= submit_tag l(:button_add), :id => 'member-add-submit' %> diff --git a/app/views/projects/settings/_members.html.erb b/app/views/projects/settings/_members.html.erb index b046bf55f4..3840fa6db4 100644 --- a/app/views/projects/settings/_members.html.erb +++ b/app/views/projects/settings/_members.html.erb @@ -1,11 +1,12 @@ -<% members = @project.memberships.preload(:project).sorted.to_a %> +<% members, member_pages, member_count = paginate_members(@project) %> <% if User.current.admin? %>

<%= link_to sprite_icon('settings', l(:label_administration)), users_path, :class => "icon icon-settings" %>
<% end %> -

<%= link_to sprite_icon('add', l(:label_member_new)), new_project_membership_path(@project), :remote => true, :class => "icon icon-add" %>

+

<%= 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" %>

-<% if members.any? %> +<% if member_count > 0 %> +
@@ -32,10 +33,10 @@ @@ -44,6 +45,12 @@ <% end %>
<%= link_to sprite_icon('edit', l(:button_edit)), - edit_membership_path(member), + edit_membership_path(member, :members_page => params[:members_page]), :remote => true, :class => 'icon icon-edit' %> - <%= remove_link membership_path(member), + <%= remove_link membership_path(member, :members_page => params[:members_page]), :remote => true, :data => (!User.current.admin? && member.include?(User.current) ? {:confirm => l(:text_own_membership_delete_confirmation)} : {}) if member.deletable? %>
+
+ + <%= pagination_links_full(member_pages, member_count) do |text, parameters, options| + link_to text, settings_project_path(@project, 'members', request.query_parameters.merge(parameters)), options + end %> + <% other_formats_links do |f| %> <%= f.link_to_with_query_parameters "CSV", {}, :onclick => "showModal('csv-export-options', '330px'); return false;" %> <% end %> diff --git a/test/functional/members_controller_test.rb b/test/functional/members_controller_test.rb index 1d9b00f363..afd29d96c9 100644 --- a/test/functional/members_controller_test.rb +++ b/test/functional/members_controller_test.rb @@ -224,6 +224,38 @@ class MembersControllerTest < Redmine::ControllerTest assert_redirected_to '/projects/ecookbook/settings/members' end + def test_update_xhr_should_keep_members_tab_in_pagination_links + project = Project.find(1) + 26.times { User.add_to_project(User.generate!, project) } + @request.session[:user_id] = 2 + with_settings :per_page_options => '25,50,100' do + put( + :update, + :params => { + :id => 2, + :membership => {:role_ids => [1]} + }, + :xhr => true + ) + end + assert_response :success + assert_match %r{settings/members\?members_page=}, response.body + end + + def test_edit_xhr_should_keep_current_page_in_form_action + @request.session[:user_id] = 2 + get(:edit, :params => {:id => 2, :members_page => 2}, :xhr => true) + assert_response :success + assert_match %r{/memberships/2\?members_page=2}, response.body + end + + def test_new_xhr_should_keep_current_page_in_form_action + @request.session[:user_id] = 2 + get(:new, :params => {:project_id => 1, :members_page => 2}, :xhr => true) + assert_response :success + assert_match %r{/projects/ecookbook/memberships\?members_page=2}, response.body + end + def test_update_locked_member_should_be_allowed User.find(3).lock! diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index d3ed25a2fd..82ba3107fd 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -1041,6 +1041,80 @@ class ProjectsControllerTest < Redmine::ControllerTest assert_select "tr#member-#{group_member.id} td.name a[href=?]", '/groups/10', :text => 'A Team' end + def test_settings_members_should_be_paginated + project = Project.find(1) + per_page = 25 + (per_page + 5).times { User.add_to_project(User.generate!, project) } + @request.session[:user_id] = 2 + with_settings :per_page_options => '25,50,100' do + get( + :settings, + :params => {:id => 'ecookbook', :tab => 'members'} + ) + end + assert_response :success + assert_select 'div#tab-content-members table.list.members tbody tr.member', :count => per_page + assert_select 'div#tab-content-members span.pagination' + assert_select 'div#tab-content-members span.pagination a[href*=?]', 'settings/members?members_page=' + end + + def test_settings_members_with_multiple_roles_should_not_appear_on_two_pages + project = Project.find(1) + 30.times { User.add_to_project(User.generate!, project) } + Member.where(:project_id => project.id).first.update!(:role_ids => [1, 2]) + @request.session[:user_id] = 2 + page1 = page2 = nil + with_settings :per_page_options => '25,50,100' do + get(:settings, :params => {:id => 'ecookbook', :tab => 'members'}) + page1 = css_select('div#tab-content-members tr.member').pluck('id') + get(:settings, :params => {:id => 'ecookbook', :tab => 'members', :members_page => 2}) + page2 = css_select('div#tab-content-members tr.member').pluck('id') + end + assert_not page1.intersect?(page2), 'a member must not appear on more than one page' + end + + def test_settings_members_should_show_requested_page + project = Project.find(1) + per_page = 25 + (per_page + 5).times { User.add_to_project(User.generate!, project) } + @request.session[:user_id] = 2 + with_settings :per_page_options => '25,50,100' do + get( + :settings, + :params => {:id => 'ecookbook', :tab => 'members', :members_page => 2} + ) + end + assert_response :success + assert_select 'div#tab-content-members table.list.members tbody tr.member' + assert_select 'div#tab-content-members span.pagination' + end + + def test_settings_members_with_out_of_range_page_should_keep_pagination + project = Project.find(1) + per_page = 25 + (per_page + 5).times { User.add_to_project(User.generate!, project) } + @request.session[:user_id] = 2 + with_settings :per_page_options => '25,50,100' do + get( + :settings, + :params => {:id => 'ecookbook', :tab => 'members', :members_page => 99} + ) + end + assert_response :success + assert_select 'div#tab-content-members p.nodata', :count => 0 + assert_select 'div#tab-content-members table.list.members' + assert_select 'div#tab-content-members span.pagination a[href*=?]', 'settings/members?members_page=' + end + + def test_settings_members_without_members_should_show_no_data + project = Project.generate! + @request.session[:user_id] = 1 + get(:settings, :params => {:id => project.id, :tab => 'members'}) + assert_response :success + assert_select 'div#tab-content-members p.nodata' + assert_select 'div#tab-content-members table.list.members', :count => 0 + end + def test_settings_should_show_tabs_depending_on_permission @request.session[:user_id] = 3 project = Project.find(1) diff --git a/test/helpers/members_helper_test.rb b/test/helpers/members_helper_test.rb index 645ec99796..76b94e3bd3 100644 --- a/test/helpers/members_helper_test.rb +++ b/test/helpers/members_helper_test.rb @@ -39,4 +39,31 @@ class MembersHelperTest < Redmine::HelperTest assert_select_in result, 'span.pagination li.current span', :text => '1' assert_select_in result, 'a[href=?]', "/projects/#{project.identifier}/memberships/autocomplete.js?page=2", :text => '2' end + + def test_paginate_members_returns_only_the_requested_page + # per_page_option is provided by ApplicationController in the running app + stubs(:per_page_option).returns(3) + project = Project.generate! + 5.times { User.add_to_project(User.generate!, project) } + + members, member_pages, member_count = paginate_members(project) + + assert_equal 3, members.size + assert_equal 3, member_pages.per_page + assert_equal project.memberships.count, member_count + end + + def test_paginate_members_lists_a_member_with_several_roles_once + stubs(:per_page_option).returns(3) + project = Project.generate! + 3.times { User.add_to_project(User.generate!, project) } + Member.where(:project_id => project.id).first.update!(:role_ids => [1, 2]) + + members, _member_pages, member_count = paginate_members(project) + + member_ids = members.map(&:id) + assert_equal member_ids.uniq, member_ids + assert_equal member_count, members.size + assert_equal project.memberships.count, member_count + end end -- 2.30.0