diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 4f273a8d4f0af583e2cf922a2f39df67546d7bfd..0cbf3eb58a337f91d8dad798733cbdf7f5b4768c 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -9,7 +9,7 @@ class Groups::GroupMembersController < Groups::ApplicationController @sort = params[:sort].presence || sort_value_name @project = @group.projects.find(params[:project_id]) if params[:project_id] - @members = @group.group_members + @members = GroupMembersFinder.new(@group).execute @members = @members.non_invite unless can?(current_user, :admin_group, @group) @members = @members.search(params[:search]) if params[:search].present? @members = @members.sort(@sort) diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..9f2206346ce065711df474c073b2746eb4b9e07d --- /dev/null +++ b/app/finders/group_members_finder.rb @@ -0,0 +1,20 @@ +class GroupMembersFinder < Projects::ApplicationController + def initialize(group) + @group = group + end + + def execute + group_members = @group.members + + return group_members unless @group.parent + + parents_members = GroupMember.non_request. + where(source_id: @group.ancestors.select(:id)). + where.not(user_id: @group.users.select(:id)) + + wheres = ["members.id IN (#{group_members.select(:id).to_sql})"] + wheres << "members.id IN (#{parents_members.select(:id).to_sql})" + + GroupMember.where(wheres.join(' OR ')) + end +end diff --git a/app/models/group.rb b/app/models/group.rb index a5b92283daa1b5caf4b26d4f26ff9533a7764896..cc6624ff4aa9f7ecf37a91b777c37ac0ff923227 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -206,7 +206,7 @@ class Group < Namespace end def members_with_parents - GroupMember.where(requested_at: nil, source_id: ancestors.map(&:id).push(id)) + GroupMember.non_request.where(source_id: ancestors.map(&:id).push(id)) end def users_with_parents diff --git a/app/models/member.rb b/app/models/member.rb index 26a6054e00db2537d6b55c894c3067ea251b734a..d07f270b7570a19b3a11e6e2fe3813bbda584a49 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -47,6 +47,7 @@ class Member < ActiveRecord::Base scope :invite, -> { where.not(invite_token: nil) } scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } + scope :non_request, -> { where(requested_at: nil) } scope :has_access, -> { active.where('access_level > 0') } diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 659d4c905fc9822bec6913659ae2899668bc806f..239387fc9fad06f4acfef435fba5ec75770bc38b 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -19,9 +19,9 @@ %label.label.label-danger %strong Blocked - - if source.instance_of?(Group) && !@group + - if source.instance_of?(Group) && source != @group · - = link_to source.name, source, class: "member-group-link" + = link_to source.full_name, source, class: "member-group-link" .hidden-xs.cgray - if member.request? @@ -44,8 +44,9 @@ = link_to member.created_by.name, user_path(member.created_by) = time_ago_with_tooltip(member.created_at) - if show_roles + - current_resource = @project || @group .controls.member-controls - - if show_controls && (member.respond_to?(:group) && @group) || (member.respond_to?(:project) && @project) + - if show_controls && member.source == current_resource - if user != current_user = form_for member, remote: true, html: { class: 'form-horizontal js-edit-member-form' } do |f| = f.hidden_field :access_level diff --git a/spec/features/groups/members/list_spec.rb b/spec/features/groups/members/list_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..109de39b2dd67b7a4975c857c845c5c711dd448a --- /dev/null +++ b/spec/features/groups/members/list_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +feature 'Groups members list', feature: true do + let(:user1) { create(:user, name: 'John Doe') } + let(:user2) { create(:user, name: 'Mary Jane') } + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + + background do + login_as(user1) + end + + scenario 'show members from current group and parent' do + group.add_developer(user1) + nested_group.add_developer(user2) + + visit group_group_members_path(nested_group) + + expect(first_row.text).to include(user1.name) + expect(second_row.text).to include(user2.name) + end + + scenario 'show user once if member of both current group and parent' do + group.add_developer(user1) + nested_group.add_developer(user1) + + visit group_group_members_path(nested_group) + + expect(first_row.text).to include(user1.name) + expect(second_row).to be_blank + end + + def first_row + page.all('ul.content-list > li')[0] + end + + def second_row + page.all('ul.content-list > li')[1] + end +end diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b762756f9ce2358a5de4fbc5f1b581e396d1d61d --- /dev/null +++ b/spec/finders/group_members_finder_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe GroupMembersFinder, '#execute' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, :access_requestable, parent: group) } + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:user3) { create(:user) } + let(:user4) { create(:user) } + + it 'returns members for top-level group' do + member1 = group.add_master(user1) + member2 = group.add_master(user2) + member3 = group.add_master(user3) + + result = described_class.new(group).execute + + expect(result.to_a).to eq([member3, member2, member1]) + end + + it 'returns members for nested group' do + group.add_master(user2) + nested_group.request_access(user4) + member1 = group.add_master(user1) + member3 = nested_group.add_master(user2) + member4 = nested_group.add_master(user3) + + result = described_class.new(nested_group).execute + + expect(result.to_a).to eq([member4, member3, member1]) + end +end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 16e2144d6a16a653733fa7d798f81ae30fbea990..c720cc9f2c2e3d1e1939590ae3a601b70a79786c 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -129,6 +129,14 @@ describe Member, models: true do it { expect(described_class.request).not_to include @accepted_request_member } end + describe '.non_request' do + it { expect(described_class.non_request).to include @master } + it { expect(described_class.non_request).to include @invited_member } + it { expect(described_class.non_request).to include @accepted_invite_member } + it { expect(described_class.non_request).not_to include @requested_member } + it { expect(described_class.non_request).to include @accepted_request_member } + end + describe '.developers' do subject { described_class.developers.to_a }