<%= role %>: <%= @principals_by_role[role].sort.collect{|p| link_to_principal(p, :class => p.is_a?(Group) ? 'icon icon-group' : nil)}.join(", ").html_safe %>
+ <% @roles_with_active_members.each do |role| %> + <%= render :partial => 'members_box_role', :locals => {:role => role} %> <% end %>+ <%= role %>: + <%= render :partial => 'members_box_principals', :locals => {:principals => principals} %><% if more %> <%= 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'}) %><% end %> +
diff --git a/app/views/projects/show_all_members.html.erb b/app/views/projects/show_all_members.html.erb new file mode 100644 index 0000000000..437c38c1eb --- /dev/null +++ b/app/views/projects/show_all_members.html.erb @@ -0,0 +1 @@ +<% if @principals.any? %>, <%= render :partial => 'members_box_principals', :locals => {:principals => @principals} %><% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 6ebe23d466..f94763dc59 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -792,6 +792,7 @@ en: label_nobody: nobody label_next: Next label_previous: Previous + label_show_all: Show all label_used_by: Used by label_details: Details label_add_note: Add a note diff --git a/config/locales/ja.yml b/config/locales/ja.yml index afa98fcaf1..d59bac5d24 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -602,6 +602,7 @@ ja: label_nobody: 無記名 label_next: 次 label_previous: 前 + label_show_all: すべて表示 label_used_by: 使用中 label_details: 詳細 label_add_note: コメントを追加 diff --git a/config/routes.rb b/config/routes.rb index ce15da4205..3f4679daf8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -154,6 +154,7 @@ Rails.application.routes.draw do match 'reopen', :via => [:post, :put] match 'copy', :via => [:get, :post] match 'bookmark', :via => [:delete, :post] + get 'show_all_members' end shallow do diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index 946fbb1538..13154174ed 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -34,7 +34,7 @@ module Redmine # Permissions AccessControl.map do |map| - map.permission :view_project, {:projects => [:show, :bookmark], :activities => [:index]}, :public => true, :read => true + map.permission :view_project, {:projects => [:show, :bookmark, :show_all_members], :activities => [:index]}, :public => true, :read => true map.permission :search_project, {:search => :index}, :public => true, :read => true map.permission :add_project, {:projects => [:new, :create]}, :require => :loggedin map.permission :edit_project, {:projects => [:settings, :edit, :update]}, :require => :member diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 496ab7dee9..6e358c6c2b 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -906,6 +906,105 @@ class ProjectsControllerTest < Redmine::ControllerTest end end + def test_show_should_display_members_box_grouped_by_role + get(:show, :params => {:id => 'ecookbook'}) + assert_response :success + assert_select 'div.members.box' do + assert_select 'p.member-role', :count => 2 + assert_select 'p.member-role:nth-of-type(1) span.label', :text => 'Manager:' + assert_select 'p.member-role:nth-of-type(1) a', :text => 'John Smith' + assert_select 'p.member-role:nth-of-type(2) span.label', :text => 'Developer:' + end + end + + def test_show_should_not_display_locked_users_in_members_box + get(:show, :params => {:id => 'ecookbook'}) + assert_response :success + # User 5 is a locked member with the Developer role (see test/fixtures/members.yml) + assert_select 'div.members.box a', :text => User.find(5).name, :count => 0 + end + + def test_show_should_not_display_more_link_when_role_has_few_members + project = Project.find(1) + role = Role.find(1) # Manager, already has 1 active member (John Smith) + 5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + get(:show, :params => {:id => 'ecookbook'}) + assert_response :success + assert_select 'div.members.box p.member-role:nth-of-type(1)' do + assert_select 'a[href^=?]', '/users/', :count => 6 + assert_select 'span.show-all-members-trigger', :count => 0 + end + end + + def test_show_should_limit_members_box_principals_per_role_and_display_more_link + project = Project.find(1) + role = Role.find(1) # Manager, already has 1 active member (John Smith) + (Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE + 5).times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + get(:show, :params => {:id => 'ecookbook'}) + assert_response :success + assert_select 'div.members.box p.member-role:nth-of-type(1)' do + assert_select 'a[href^=?]', '/users/', :count => Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE + assert_select 'span.show-all-members-trigger a[data-action=?][href*=?]', + 'lazy-load#loadAll', "offset=#{Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE}", + :text => 'Show all' + end + end + + def test_show_all_members_should_return_every_member_from_the_given_offset + project = Project.find(1) + role = Role.find(1) + 5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + get(:show_all_members, :params => {:id => 'ecookbook', :role_id => role.id, :offset => 1}, :xhr => true) + assert_response :success + assert_select 'a[href^=?]', '/users/', :count => 5 + assert_select 'span.show-all-members-trigger', :count => 0 + end + + def test_show_all_members_should_return_every_remaining_member_without_a_further_trigger + project = Project.find(1) + role = Role.find(1) + (Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE + 5).times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + # Clicking "Show all" requests everything past what the initial page already showed + get( + :show_all_members, + :params => {:id => 'ecookbook', :role_id => role.id, :offset => Project::MEMBERS_BOX_PRINCIPALS_PER_ROLE}, + :xhr => true + ) + assert_response :success + # 1 pre-existing manager (John Smith) + (PER_ROLE + 5) new - PER_ROLE already shown = 6 remaining + assert_select 'a[href^=?]', '/users/', :count => 6 + assert_select 'span.show-all-members-trigger', :count => 0 + end + + def test_show_all_members_should_not_include_locked_users + get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 2, :offset => 0}, :xhr => true) + assert_response :success + assert_select 'a', :text => User.find(5).name, :count => 0 + end + + def test_show_all_members_should_not_render_a_stray_comma_when_nothing_remains + # Role 1 (Manager) on project 1 only has 1 active member (John Smith), so + # an offset past it must not leave a dangling ", " where the members + # would otherwise have been inserted. + get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 1, :offset => 999}, :xhr => true) + assert_response :success + assert_equal '', response.body.strip + end + + def test_show_all_members_should_respond_with_not_found_for_a_role_without_active_members + get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 999}, :xhr => true) + assert_response :not_found + end + + def test_show_all_members_should_not_require_login_for_a_public_project + get(:show_all_members, :params => {:id => 'ecookbook', :role_id => 1}, :xhr => true) + assert_response :success + end + def test_settings @request.session[:user_id] = 2 # manager get(:settings, :params => {:id => 1}) diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb index 7a9cf7dc6d..b7aea7fa17 100644 --- a/test/unit/project_test.rb +++ b/test/unit/project_test.rb @@ -480,6 +480,94 @@ class ProjectTest < ActiveSupport::TestCase assert_not principals_by_role.values.flatten.include?(locked_user) end + def test_roles_with_active_members + # Project 1 has active members with role 1 (Manager) and role 2 (Developer) + assert_equal [Role.find(1), Role.find(2)], Project.find(1).roles_with_active_members.to_a + end + + def test_roles_with_active_members_should_not_include_roles_without_members + project = Project.generate! + assert_equal [], project.roles_with_active_members.to_a + end + + def test_roles_with_active_members_should_not_include_roles_with_only_locked_members + project = Project.find(1) + role = Role.find(2) + # Role 2 (Developer) only has active member 3 besides the locked member 5 + Member.find_by(:project_id => 1, :user_id => 3).destroy + assert_not project.reload.roles_with_active_members.to_a.include?(role) + end + + def test_principals_for_role + principals, more = Project.find(1).principals_for_role(Role.find(1)) + assert_kind_of Array, principals + assert principals.include?(User.find(2)) + assert_not more + end + + def test_principals_for_role_should_only_return_active_users + # Role 2 (Developer) on project 1 has active member 3 and locked member 5 + principals, _more = Project.find(1).principals_for_role(Role.find(2)) + assert_equal [User.find(3)], principals + end + + def test_principals_for_role_should_include_groups + group = Group.find(10) + Member.create!(:principal => group, :project_id => 1, :role_ids => [1]) + + principals, _more = Project.find(1).principals_for_role(Role.find(1)) + assert principals.include?(group) + end + + def test_principals_for_role_should_not_duplicate_a_user_with_an_inherited_role_matching_a_direct_role + project = Project.find(1) + role = Role.find(1) + user = User.generate! + User.add_to_project(user, project, role) + group = Group.generate! + group.users << user + # Also grants role via the group: the user now has 2 member_roles for the + # same role (one direct, one inherited, see MemberRole#add_role_to_group_users) + User.add_to_project(group, project, role) + + principals, _more = project.principals_for_role(role, :limit => 100) + assert_equal 1, principals.count {|p| p == user} + end + + def test_principals_for_role_should_paginate + project = Project.find(1) + role = Role.find(1) + 5.times {|i| Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + principals, more = project.principals_for_role(role, :limit => 2) + assert_equal 2, principals.size + assert more + + more_principals, more = project.principals_for_role(role, :offset => 2, :limit => 2) + assert_equal 2, more_principals.size + assert more + assert_empty principals & more_principals + + rest, more = project.principals_for_role(role, :offset => 4, :limit => 2) + assert_equal 2, rest.size + assert_not more + end + + def test_principals_for_role_with_a_nil_limit_should_return_every_remaining_member + project = Project.find(1) + role = Role.find(1) + 5.times {Member.create!(:principal => User.generate!, :project => project, :role_ids => [role.id])} + + first_page, more = project.principals_for_role(role, :limit => 2) + assert_equal 2, first_page.size + assert more + + rest, more = project.principals_for_role(role, :offset => first_page.size, :limit => nil) + assert_equal 4, rest.size + assert_not more + assert_empty first_page & rest + end + def test_rolled_up_trackers parent = Project.find(1) parent.trackers = Tracker.find([1, 2])