diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 44cd6e9926fa10bfc4b33d0a309f99560766bd69..458eaec4e2e0d5165c54d17b9b8e27385bf58c13 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -18,15 +18,7 @@ def execute(noteable) end def project_members - @project_members ||= sorted(get_project_members) - end - - def get_project_members - members = Member.from_union([project_members_through_ancestral_groups, - project_members_through_invited_groups, - individual_project_members]) - - User.id_in(members.select(:user_id)) + @project_members ||= sorted(project.authorized_users) end def all_members @@ -34,33 +26,5 @@ def all_members [{ username: "all", name: "All Project and Group Members", count: project_members.count }] end - - private - - def project_members_through_invited_groups - GroupMember - .active_without_invites_and_requests - .with_source_id(visible_groups.self_and_ancestors.pluck_primary_key) - .select(*GroupMember.cached_column_list) - end - - def visible_groups - visible_groups = project.invited_groups - - unless project.team.member?(current_user) - visible_groups = visible_groups.public_or_visible_to_user(current_user) - end - - visible_groups - end - - def project_members_through_ancestral_groups - members = project.group.present? ? project.group.members_with_parents : Member.none - members.select(*GroupMember.cached_column_list) - end - - def individual_project_members - project.project_members.select(*GroupMember.cached_column_list) - end end end diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index d0bfbeae78f02b86ba29ac020db84cd82ba3b73a..1745dfe3af0171362360315807f7cbaf77f8345d 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -196,23 +196,28 @@ def members_by_username(username) end end - shared_examples 'only public members are returned for public project' do + shared_examples 'returns all members of public project' do before do stub_feature_flags(disable_all_mention: false) end - it 'only returns public members' do + it 'returns members including those from invited private groups' do get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } expect(members_by_username('all').symbolize_keys).to include( username: 'all', name: 'All Project and Group Members', - count: 1) + count: 2) expect(members_by_username(user.username).symbolize_keys).to include( type: user.class.name, name: user.name, avatar_url: user.avatar_url) + + expect(members_by_username(invited_private_member.username).symbolize_keys).to include( + type: invited_private_member.class.name, + name: invited_private_member.name, + avatar_url: invited_private_member.avatar_url) end context 'when `disable_all_mention` FF is enabled' do @@ -234,7 +239,7 @@ def members_by_username(username) let(:issuable_type) { private_issue.class.name } end - it_behaves_like 'only public members are returned for public project' do + it_behaves_like 'returns all members of public project' do let(:issuable_type) { issue.class.name } end end @@ -244,7 +249,7 @@ def members_by_username(username) let(:issuable_type) { private_work_item.class.name } end - it_behaves_like 'only public members are returned for public project' do + it_behaves_like 'returns all members of public project' do let(:issuable_type) { work_item.class.name } end end diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index 04c43dff2dcefad7f9f08cf960758103fef82c4b..b01e64439ec4b1a9f126ed4eb09cc64b3c8a43f5 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -120,120 +120,88 @@ def run_service describe '#project_members' do subject(:usernames) { service.project_members.map { |member| member[:username] } } - shared_examples 'return project members' do - context 'when there is a project in group namespace' do - let_it_be(:public_group) { create(:group, :public) } - let_it_be(:public_project) { create(:project, :public, namespace: public_group) } + context 'when there is a project in group namespace' do + let_it_be(:public_group) { create(:group, :public) } + let_it_be(:public_project, reload: true) { create(:project, :public, namespace: public_group) } - let_it_be(:public_group_owner) { create(:user) } + let_it_be(:public_group_owner) { create(:user) } - let(:service) { described_class.new(public_project, create(:user)) } - - before do - public_group.add_owner(public_group_owner) - end + let(:service) { described_class.new(public_project, create(:user)) } - it 'returns members of a group' do - expect(usernames).to include(public_group_owner.username) - end + before do + public_group.add_owner(public_group_owner) end - context 'when there is a private group and a public project' do - let_it_be(:public_group) { create(:group, :public) } - let_it_be(:private_group) { create(:group, :private, :nested) } - let_it_be(:public_project) { create(:project, :public, namespace: public_group) } - - let_it_be(:project_issue) { create(:issue, project: public_project) } - - let_it_be(:public_group_owner) { create(:user) } - let_it_be(:private_group_member) { create(:user) } - let_it_be(:public_project_maintainer) { create(:user) } - let_it_be(:private_group_owner) { create(:user) } - - let_it_be(:group_ancestor_owner) { create(:user) } - - before_all do - public_group.add_owner public_group_owner - private_group.add_developer private_group_member - public_project.add_maintainer public_project_maintainer - - private_group.add_owner private_group_owner - private_group.parent.add_owner group_ancestor_owner - end + it 'returns members of a group' do + expect(usernames).to include(public_group_owner.username) + end + end - context 'when the private group is invited to the public project' do - before_all do - create(:project_group_link, group: private_group, project: public_project) - end + context 'when there is a private group and a public project' do + let_it_be(:public_group) { create(:group, :public) } + let_it_be(:private_group) { create(:group, :private, :nested) } + let_it_be(:public_project, reload: true) { create(:project, :public, namespace: public_group) } - context 'when a user who is outside the public project and the private group is signed in' do - let(:service) { described_class.new(public_project, create(:user)) } + let_it_be(:project_issue) { create(:issue, project: public_project) } - it 'does not return the private group' do - expect(usernames).not_to include(private_group.name) - end + let_it_be(:public_group_owner) { create(:user) } + let_it_be(:private_group_member) { create(:user) } + let_it_be(:public_project_maintainer) { create(:user) } + let_it_be(:private_group_owner) { create(:user) } - it 'does not return private group members' do - expect(usernames).not_to include(private_group_member.username) - end + let_it_be(:group_ancestor_owner) { create(:user) } - it 'returns the project maintainer' do - expect(usernames).to include(public_project_maintainer.username) - end + before_all do + public_group.add_owner public_group_owner + private_group.add_developer private_group_member + public_project.add_maintainer public_project_maintainer - it 'returns project members from an invited public group' do - invited_public_group = create(:group, :public) - invited_public_group.add_owner create(:user) + private_group.add_owner private_group_owner + private_group.parent.add_owner group_ancestor_owner + end - create(:project_group_link, group: invited_public_group, project: public_project) + context 'when the private group is invited to the public project' do + before_all do + create(:project_group_link, group: private_group, project: public_project) + end - expect(usernames).to include(invited_public_group.users.first.username) - end + let(:service) { described_class.new(public_project, create(:user)) } - it 'does not return ancestors of the private group' do - expect(usernames).not_to include(group_ancestor_owner.username) - end - end + it 'does not return the private group' do + expect(usernames).not_to include(private_group.name) + end - context 'when public project maintainer is signed in' do - let(:service) { described_class.new(public_project, public_project_maintainer) } + it 'returns private group members' do + expect(usernames).to include(private_group_member.username) + end - it 'returns private group members' do - expect(usernames).to include(private_group_member.username) - end + it 'returns the project maintainer' do + expect(usernames).to include(public_project_maintainer.username) + end - it 'returns members of the ancestral groups of the private group' do - expect(usernames).to include(group_ancestor_owner.username) - end - end + it 'returns project members from an invited public group' do + invited_public_group = create(:group, :public) + invited_public_group.add_owner create(:user) - context 'when private group owner is signed in' do - let(:service) { described_class.new(public_project, private_group_owner) } + create(:project_group_link, group: invited_public_group, project: public_project) - it 'returns private group members' do - expect(usernames).to include(private_group_member.username) - end + expect(usernames).to include(invited_public_group.users.first.username) + end - it 'returns ancestors of the the private group' do - expect(usernames).to include(group_ancestor_owner.username) - end - end + it 'returns members of the ancestral groups of the private group' do + expect(usernames).to include(group_ancestor_owner.username) + end - context 'when the namespace owner of the public project is signed in' do - let(:service) { described_class.new(public_project, public_group_owner) } + it 'returns invited group members of the private group' do + invited_group = create(:group, :public) + create(:group_group_link, shared_group: private_group, shared_with_group: invited_group) - it 'returns private group members' do - expect(usernames).to include(private_group_member.username) - end + other_user = create(:user) + invited_group.add_guest(other_user) - it 'does not return members of the ancestral groups of the private group' do - expect(usernames).to include(group_ancestor_owner.username) - end - end + expect(usernames).to include(other_user.username) end end end - - it_behaves_like 'return project members' end end