Defect #44350 » members-box-show-all.patch
| app/controllers/projects_controller.rb | ||
|---|---|---|
| 175 | 175 | |
| 176 | 176 |
respond_to do |format| |
| 177 | 177 |
format.html do |
| 178 |
@principals_by_role = @project.principals_by_role
|
|
| 178 |
@roles_with_active_members = @project.roles_with_active_members
|
|
| 179 | 179 |
@subprojects = @project.leaf? ? [] : @project.children.visible.to_a |
| 180 | 180 |
@news = @project.news.limit(5).includes(:author, :project).reorder("#{News.table_name}.created_on DESC").to_a
|
| 181 | 181 |
with_subprojects = Setting.display_subprojects_issues? |
| ... | ... | |
| 197 | 197 |
end |
| 198 | 198 |
end |
| 199 | 199 | |
| 200 |
# Renders the remaining members of a role for the overview page's members |
|
| 201 |
# box, so that only the members shown on the initial page load there get |
|
| 202 |
# loaded up front (#88737). |
|
| 203 |
def show_all_members |
|
| 204 |
@role = @project.roles_with_active_members.detect {|role| role.id == params[:role_id].to_i}
|
|
| 205 |
unless @role |
|
| 206 |
render_404 |
|
| 207 |
return |
|
| 208 |
end |
|
| 209 | ||
| 210 |
offset = params[:offset].to_i |
|
| 211 |
offset = 0 if offset < 0 |
|
| 212 |
@principals, _more = @project.principals_for_role(@role, :offset => offset, :limit => nil) |
|
| 213 |
render :layout => false |
|
| 214 |
end |
|
| 215 | ||
| 200 | 216 |
def settings |
| 201 | 217 |
@issue_custom_fields = IssueCustomField.sorted.to_a |
| 202 | 218 |
@issue_category ||= IssueCategory.new |
| app/javascript/controllers/lazy_load_controller.js | ||
|---|---|---|
| 1 |
import { Controller } from '@hotwired/stimulus'
|
|
| 2 |
import { get } from '@rails/request.js'
|
|
| 3 | ||
| 4 |
// Fetches a trigger link's href and inserts the response HTML in its place, |
|
| 5 |
// then removes the trigger. Generic: it works with any endpoint that |
|
| 6 |
// returns an HTML fragment to insert before the trigger, so it is safe to |
|
| 7 |
// reuse (rather than copy) for a similar "load on demand" interaction |
|
| 8 |
// elsewhere. |
|
| 9 |
export default class extends Controller {
|
|
| 10 |
static targets = ['trigger'] |
|
| 11 | ||
| 12 |
async loadAll(event) {
|
|
| 13 |
event.preventDefault() |
|
| 14 | ||
| 15 |
// Guard against a second click firing another request (and duplicating |
|
| 16 |
// the inserted content) while the first one is still in flight. Cleared |
|
| 17 |
// in `finally` so a failed or errored request can be retried. |
|
| 18 |
const trigger = this.triggerTarget |
|
| 19 |
if (trigger.dataset.loading) {
|
|
| 20 |
return |
|
| 21 |
} |
|
| 22 |
trigger.dataset.loading = 'true' |
|
| 23 | ||
| 24 |
try {
|
|
| 25 |
const response = await get(event.currentTarget.href, { responseKind: 'html' })
|
|
| 26 |
if (!response.ok) {
|
|
| 27 |
return |
|
| 28 |
} |
|
| 29 | ||
| 30 |
trigger.insertAdjacentHTML('beforebegin', await response.html)
|
|
| 31 |
trigger.remove() |
|
| 32 |
} finally {
|
|
| 33 |
delete trigger.dataset.loading |
|
| 34 |
} |
|
| 35 |
} |
|
| 36 |
} |
|
| app/models/project.rb | ||
|---|---|---|
| 30 | 30 |
# Maximum length for project identifiers |
| 31 | 31 |
IDENTIFIER_MAX_LENGTH = 100 |
| 32 | 32 | |
| 33 |
# Number of principals initially listed per role in the overview page's members box |
|
| 34 |
MEMBERS_BOX_PRINCIPALS_PER_ROLE = 50 unless const_defined?(:MEMBERS_BOX_PRINCIPALS_PER_ROLE) |
|
| 35 | ||
| 33 | 36 |
has_many :memberships, :class_name => 'Member', :inverse_of => :project |
| 34 | 37 |
# Memberships of active users only |
| 35 | 38 |
has_many :members, |
| ... | ... | |
| 565 | 568 |
end |
| 566 | 569 | |
| 567 | 570 |
# Returns a hash of project users/groups grouped by role |
| 571 |
# No longer used by Redmine core (the project overview's members box uses |
|
| 572 |
# #roles_with_active_members and #principals_for_role instead, to avoid |
|
| 573 |
# loading every active member at once). Kept as a public method for |
|
| 574 |
# backward compatibility, since plugins may call it directly. |
|
| 568 | 575 |
def principals_by_role |
| 569 | 576 |
memberships.active.includes(:principal, :roles).inject({}) do |h, m|
|
| 570 | 577 |
m.roles.each do |r| |
| ... | ... | |
| 575 | 582 |
end |
| 576 | 583 |
end |
| 577 | 584 | |
| 585 |
# Returns the roles having at least one active member, sorted like Role#<=>. |
|
| 586 |
# Unlike #principals_by_role, this does not load any principal, so it is |
|
| 587 |
# cheap to call even when the project has a lot of members (project |
|
| 588 |
# overview's members box). |
|
| 589 |
def roles_with_active_members |
|
| 590 |
role_ids = memberships.active.joins(:roles).distinct.pluck("#{Role.table_name}.id")
|
|
| 591 |
Role.where(:id => role_ids).sorted |
|
| 592 |
end |
|
| 593 | ||
| 594 |
# Returns a page of the project's active members having the given role, as |
|
| 595 |
# principals sorted like Principal#<=>, and whether more remain beyond it. |
|
| 596 |
# A nil limit returns everything from offset onwards (more is then false). |
|
| 597 |
# Ids are de-duplicated in Ruby rather than via SQL DISTINCT: a member can |
|
| 598 |
# be joined twice for the same role (e.g. inherited from a group in |
|
| 599 |
# addition to a direct role), and DISTINCT combined with this ORDER BY is |
|
| 600 |
# not portable across all supported databases. |
|
| 601 |
def principals_for_role(role, offset: 0, limit: MEMBERS_BOX_PRINCIPALS_PER_ROLE) |
|
| 602 |
ordered_ids = |
|
| 603 |
memberships.active.joins(:roles).where(:roles => {:id => role.id}).
|
|
| 604 |
order(Principal.fields_for_order_statement). |
|
| 605 |
pluck(:user_id).uniq |
|
| 606 |
ids = (limit ? ordered_ids[offset, limit] : ordered_ids[offset..]) || [] |
|
| 607 |
more = limit ? ordered_ids.size > offset + ids.size : false |
|
| 608 |
principals_by_id = Principal.where(:id => ids).index_by(&:id) |
|
| 609 |
[ids.filter_map {|id| principals_by_id[id]}, more]
|
|
| 610 |
end |
|
| 611 | ||
| 578 | 612 |
# Adds user as a project member with the default role |
| 579 | 613 |
# Used for when a non-admin user creates a project |
| 580 | 614 |
def add_default_member(user) |
| app/views/projects/_members_box.html.erb | ||
|---|---|---|
| 1 |
<% if @principals_by_role.any? %>
|
|
| 1 |
<% if @roles_with_active_members.any? %>
|
|
| 2 | 2 |
<div class="members box"> |
| 3 | 3 |
<h3 class="icon icon-group"><%= sprite_icon('group', l(:label_member_plural)) %></h3>
|
| 4 |
<% @principals_by_role.keys.sort.each do |role| %>
|
|
| 5 |
<p><span class="label"><%= role %>:</span> <%= @principals_by_role[role].sort.collect{|p| link_to_principal(p, :class => p.is_a?(Group) ? 'icon icon-group' : nil)}.join(", ").html_safe %></p>
|
|
| 4 |
<% @roles_with_active_members.each do |role| %>
|
|
| 5 |
<%= render :partial => 'members_box_role', :locals => {:role => role} %>
|
|
| 6 | 6 |
<% end %> |
| 7 | 7 |
</div> |
| 8 | 8 |
<% end %> |
| app/views/projects/_members_box_principals.html.erb | ||
|---|---|---|
| 1 |
<%= principals.collect {|p| link_to_principal(p, :class => p.is_a?(Group) ? 'icon icon-group' : nil)}.join(", ").html_safe %>
|
|
| app/views/projects/_members_box_role.html.erb | ||
|---|---|---|
| 1 |
<% |
|
| 2 |
principals, more = @project.principals_for_role(role) |
|
| 3 |
%> |
|
| 4 |
<p class="member-role" data-controller="lazy-load"> |
|
| 5 |
<span class="label"><%= role %>:</span> |
|
| 6 |
<%= render :partial => 'members_box_principals', :locals => {:principals => principals} %><% if more %> <span class="show-all-members-trigger" data-lazy-load-target="trigger"><%= link_to(sprite_icon('angle-right', l(:label_show_all), :rtl => true), show_all_members_project_path(@project, :role_id => role.id, :offset => principals.size), :class => 'icon icon-angle-right', :data => {:action => 'lazy-load#loadAll'}) %></span><% end %>
|
|
| 7 |
</p> |
|
| app/views/projects/show_all_members.html.erb | ||
|---|---|---|
| 1 |
<% if @principals.any? %>, <%= render :partial => 'members_box_principals', :locals => {:principals => @principals} %><% end %>
|
|
| config/locales/en.yml | ||
|---|---|---|
| 792 | 792 |
label_nobody: nobody |
| 793 | 793 |
label_next: Next |
| 794 | 794 |
label_previous: Previous |
| 795 |
label_show_all: Show all |
|
| 795 | 796 |
label_used_by: Used by |
| 796 | 797 |
label_details: Details |
| 797 | 798 |
label_add_note: Add a note |
| config/locales/ja.yml | ||
|---|---|---|
| 602 | 602 |
label_nobody: 無記名 |
| 603 | 603 |
label_next: 次 |
| 604 | 604 |
label_previous: 前 |
| 605 |
label_show_all: すべて表示 |
|
| 605 | 606 |
label_used_by: 使用中 |
| 606 | 607 |
label_details: 詳細 |
| 607 | 608 |
label_add_note: コメントを追加 |
| config/routes.rb | ||
|---|---|---|
| 154 | 154 |
match 'reopen', :via => [:post, :put] |
| 155 | 155 |
match 'copy', :via => [:get, :post] |
| 156 | 156 |
match 'bookmark', :via => [:delete, :post] |
| 157 |
get 'show_all_members' |
|
| 157 | 158 |
end |
| 158 | 159 | |
| 159 | 160 |
shallow do |
| lib/redmine/preparation.rb | ||
|---|---|---|
| 34 | 34 | |
| 35 | 35 |
# Permissions |
| 36 | 36 |
AccessControl.map do |map| |
| 37 |
map.permission :view_project, {:projects => [:show, :bookmark], :activities => [:index]}, :public => true, :read => true
|
|
| 37 |
map.permission :view_project, {:projects => [:show, :bookmark, :show_all_members], :activities => [:index]}, :public => true, :read => true
|
|
| 38 | 38 |
map.permission :search_project, {:search => :index}, :public => true, :read => true
|
| 39 | 39 |
map.permission :add_project, {:projects => [:new, :create]}, :require => :loggedin
|
| 40 | 40 |
map.permission :edit_project, {:projects => [:settings, :edit, :update]}, :require => :member
|
| test/functional/projects_controller_test.rb | ||
|---|---|---|
| 906 | 906 |
end |
| 907 | 907 |
end |
| 908 | 908 | |
| 909 |
def test_show_should_display_members_box_grouped_by_role |
|
| 910 |
get(:show, :params => {:id => 'ecookbook'})
|
|
| 911 |
assert_response :success |
|
| 912 |
assert_select 'div.members.box' do |
|
| 913 |
assert_select 'p.member-role', :count => 2 |
|
| 914 |
assert_select 'p.member-role:nth-of-type(1) span.label', :text => 'Manager:' |
|
| 915 |
assert_select 'p.member-role:nth-of-type(1) a', :text => 'John Smith' |
|
| 916 |
assert_select 'p.member-role:nth-of-type(2) span.label', :text => 'Developer:' |
|
| 917 |
end |
|
| 918 |
end |
|
| 919 | ||
| 920 |
def test_show_should_not_display_locked_users_in_members_box |
|
| 921 |
get(:show, :params => {:id => 'ecookbook'})
|
|
| 922 |
assert_response :success |
|
| 923 |
# User 5 is a locked member with the Developer role (see test/fixtures/members.yml) |
|
| 924 |
assert_select 'div.members.box a', :text => User.find(5).name, :count => 0 |
|
| 925 |
end |
|
| 926 | ||
| 927 |
def test_show_should_not_display_more_link_when_role_has_few_members |
|
| 928 |
project = Project.find(1) |
|
| 929 |
role = Role.find(1) # Manager, already has 1 active member (John Smith) |
|
| 930 |
5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 931 | ||
| 932 |
get(:show, :params => {:id => 'ecookbook'})
|
|
| 933 |
assert_response :success |
|
| 934 |
assert_select 'div.members.box p.member-role:nth-of-type(1)' do |
|
| 935 |
assert_select 'a[href^=?]', '/users/', :count => 6 |
|
| 936 |
assert_select 'span.show-all-members-trigger', :count => 0 |
|
| 937 |
end |
|
| 938 |
end |
|
| 939 | ||
| 940 |
def test_show_should_limit_members_box_principals_per_role_and_display_more_link |
|
| 941 |
project = Project.find(1) |
|
| 942 |
role = Role.find(1) # Manager, already has 1 active member (John Smith) |
|
| 943 |
(Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE + 5).times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 944 | ||
| 945 |
get(:show, :params => {:id => 'ecookbook'})
|
|
| 946 |
assert_response :success |
|
| 947 |
assert_select 'div.members.box p.member-role:nth-of-type(1)' do |
|
| 948 |
assert_select 'a[href^=?]', '/users/', :count => Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE |
|
| 949 |
assert_select 'span.show-all-members-trigger a[data-action=?][href*=?]', |
|
| 950 |
'lazy-load#loadAll', "offset=#{Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE}",
|
|
| 951 |
:text => 'Show all' |
|
| 952 |
end |
|
| 953 |
end |
|
| 954 | ||
| 955 |
def test_show_all_members_should_return_every_member_from_the_given_offset |
|
| 956 |
project = Project.find(1) |
|
| 957 |
role = Role.find(1) |
|
| 958 |
5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 959 | ||
| 960 |
get(:show_all_members, :params => {:id => 'ecookbook', :role_id => role.id, :offset => 1}, :xhr => true)
|
|
| 961 |
assert_response :success |
|
| 962 |
assert_select 'a[href^=?]', '/users/', :count => 5 |
|
| 963 |
assert_select 'span.show-all-members-trigger', :count => 0 |
|
| 964 |
end |
|
| 965 | ||
| 966 |
def test_show_all_members_should_return_every_remaining_member_without_a_further_trigger |
|
| 967 |
project = Project.find(1) |
|
| 968 |
role = Role.find(1) |
|
| 969 |
(Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE + 5).times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 970 | ||
| 971 |
# Clicking "Show all" requests everything past what the initial page already showed |
|
| 972 |
get( |
|
| 973 |
:show_all_members, |
|
| 974 |
:params => {:id => 'ecookbook', :role_id => role.id, :offset => Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE},
|
|
| 975 |
:xhr => true |
|
| 976 |
) |
|
| 977 |
assert_response :success |
|
| 978 |
# 1 pre-existing manager (John Smith) + (PER_ROLE + 5) new - PER_ROLE already shown = 6 remaining |
|
| 979 |
assert_select 'a[href^=?]', '/users/', :count => 6 |
|
| 980 |
assert_select 'span.show-all-members-trigger', :count => 0 |
|
| 981 |
end |
|
| 982 | ||
| 983 |
def test_show_all_members_should_not_include_locked_users |
|
| 984 |
get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 2, :offset => 0}, :xhr => true)
|
|
| 985 |
assert_response :success |
|
| 986 |
assert_select 'a', :text => User.find(5).name, :count => 0 |
|
| 987 |
end |
|
| 988 | ||
| 989 |
def test_show_all_members_should_not_render_a_stray_comma_when_nothing_remains |
|
| 990 |
# Role 1 (Manager) on project 1 only has 1 active member (John Smith), so |
|
| 991 |
# an offset past it must not leave a dangling ", " where the members |
|
| 992 |
# would otherwise have been inserted. |
|
| 993 |
get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 1, :offset => 999}, :xhr => true)
|
|
| 994 |
assert_response :success |
|
| 995 |
assert_equal '', response.body.strip |
|
| 996 |
end |
|
| 997 | ||
| 998 |
def test_show_all_members_should_respond_with_not_found_for_a_role_without_active_members |
|
| 999 |
get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 999}, :xhr => true)
|
|
| 1000 |
assert_response :not_found |
|
| 1001 |
end |
|
| 1002 | ||
| 1003 |
def test_show_all_members_should_not_require_login_for_a_public_project |
|
| 1004 |
get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 1}, :xhr => true)
|
|
| 1005 |
assert_response :success |
|
| 1006 |
end |
|
| 1007 | ||
| 909 | 1008 |
def test_settings |
| 910 | 1009 |
@request.session[:user_id] = 2 # manager |
| 911 | 1010 |
get(:settings, :params => {:id => 1})
|
| test/unit/project_test.rb | ||
|---|---|---|
| 480 | 480 |
assert_not principals_by_role.values.flatten.include?(locked_user) |
| 481 | 481 |
end |
| 482 | 482 | |
| 483 |
def test_roles_with_active_members |
|
| 484 |
# Project 1 has active members with role 1 (Manager) and role 2 (Developer) |
|
| 485 |
assert_equal [Role.find(1), Role.find(2)], Project.find(1).roles_with_active_members.to_a |
|
| 486 |
end |
|
| 487 | ||
| 488 |
def test_roles_with_active_members_should_not_include_roles_without_members |
|
| 489 |
project = Project.generate! |
|
| 490 |
assert_equal [], project.roles_with_active_members.to_a |
|
| 491 |
end |
|
| 492 | ||
| 493 |
def test_roles_with_active_members_should_not_include_roles_with_only_locked_members |
|
| 494 |
project = Project.find(1) |
|
| 495 |
role = Role.find(2) |
|
| 496 |
# Role 2 (Developer) only has active member 3 besides the locked member 5 |
|
| 497 |
Member.find_by(:project_id => 1, :user_id => 3).destroy |
|
| 498 |
assert_not project.reload.roles_with_active_members.to_a.include?(role) |
|
| 499 |
end |
|
| 500 | ||
| 501 |
def test_principals_for_role |
|
| 502 |
principals, more = Project.find(1).principals_for_role(Role.find(1)) |
|
| 503 |
assert_kind_of Array, principals |
|
| 504 |
assert principals.include?(User.find(2)) |
|
| 505 |
assert_not more |
|
| 506 |
end |
|
| 507 | ||
| 508 |
def test_principals_for_role_should_only_return_active_users |
|
| 509 |
# Role 2 (Developer) on project 1 has active member 3 and locked member 5 |
|
| 510 |
principals, _more = Project.find(1).principals_for_role(Role.find(2)) |
|
| 511 |
assert_equal [User.find(3)], principals |
|
| 512 |
end |
|
| 513 | ||
| 514 |
def test_principals_for_role_should_include_groups |
|
| 515 |
group = Group.find(10) |
|
| 516 |
Member.create!(:principal => group, :project_id => 1, :role_ids => [1]) |
|
| 517 | ||
| 518 |
principals, _more = Project.find(1).principals_for_role(Role.find(1)) |
|
| 519 |
assert principals.include?(group) |
|
| 520 |
end |
|
| 521 | ||
| 522 |
def test_principals_for_role_should_not_duplicate_a_user_with_an_inherited_role_matching_a_direct_role |
|
| 523 |
project = Project.find(1) |
|
| 524 |
role = Role.find(1) |
|
| 525 |
user = User.generate! |
|
| 526 |
User.add_to_project(user, project, role) |
|
| 527 |
group = Group.generate! |
|
| 528 |
group.users << user |
|
| 529 |
# Also grants role via the group: the user now has 2 member_roles for the |
|
| 530 |
# same role (one direct, one inherited, see MemberRole#add_role_to_group_users) |
|
| 531 |
User.add_to_project(group, project, role) |
|
| 532 | ||
| 533 |
principals, _more = project.principals_for_role(role, :limit => 100) |
|
| 534 |
assert_equal 1, principals.count {|p| p == user}
|
|
| 535 |
end |
|
| 536 | ||
| 537 |
def test_principals_for_role_should_paginate |
|
| 538 |
project = Project.find(1) |
|
| 539 |
role = Role.find(1) |
|
| 540 |
5.times {|i| Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 541 | ||
| 542 |
principals, more = project.principals_for_role(role, :limit => 2) |
|
| 543 |
assert_equal 2, principals.size |
|
| 544 |
assert more |
|
| 545 | ||
| 546 |
more_principals, more = project.principals_for_role(role, :offset => 2, :limit => 2) |
|
| 547 |
assert_equal 2, more_principals.size |
|
| 548 |
assert more |
|
| 549 |
assert_empty principals & more_principals |
|
| 550 | ||
| 551 |
rest, more = project.principals_for_role(role, :offset => 4, :limit => 2) |
|
| 552 |
assert_equal 2, rest.size |
|
| 553 |
assert_not more |
|
| 554 |
end |
|
| 555 | ||
| 556 |
def test_principals_for_role_with_a_nil_limit_should_return_every_remaining_member |
|
| 557 |
project = Project.find(1) |
|
| 558 |
role = Role.find(1) |
|
| 559 |
5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])}
|
|
| 560 | ||
| 561 |
first_page, more = project.principals_for_role(role, :limit => 2) |
|
| 562 |
assert_equal 2, first_page.size |
|
| 563 |
assert more |
|
| 564 | ||
| 565 |
rest, more = project.principals_for_role(role, :offset => first_page.size, :limit => nil) |
|
| 566 |
assert_equal 4, rest.size |
|
| 567 |
assert_not more |
|
| 568 |
assert_empty first_page & rest |
|
| 569 |
end |
|
| 570 | ||
| 483 | 571 |
def test_rolled_up_trackers |
| 484 | 572 |
parent = Project.find(1) |
| 485 | 573 |
parent.trackers = Tracker.find([1, 2]) |
- « Previous
- 1
- 2
- Next »