summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-03-09 13:54:43 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2017-03-09 13:54:43 +0000
commitca66799267616810f4c1bae75ab0006201c094a4 (patch)
tree51a7452d1805842859efc35654ee67366d62b43b
parent03476ac0bc2a06f19057475d8982055c12a6c0a9 (diff)
parent5b52adcedb78287c4599ead8ffc1d00f84dc4e55 (diff)
downloadgitlab-ce-ca66799267616810f4c1bae75ab0006201c094a4.tar.gz
Merge branch 'dz-nested-groups-members' into 'master'
Show members of parent groups on project members page Closes #28786 and #27962 See merge request !9764
-rw-r--r--app/controllers/projects/settings/members_controller.rb37
-rw-r--r--app/finders/group_members_finder.rb2
-rw-r--r--app/finders/members_finder.rb40
-rw-r--r--changelogs/unreleased/dz-nested-groups-members.yml4
-rw-r--r--lib/gitlab/import_export/project_tree_saver.rb8
-rw-r--r--spec/finders/members_finder_spec.rb22
6 files changed, 69 insertions, 44 deletions
diff --git a/app/controllers/projects/settings/members_controller.rb b/app/controllers/projects/settings/members_controller.rb
index 5735e281f66..cbfa2afa959 100644
--- a/app/controllers/projects/settings/members_controller.rb
+++ b/app/controllers/projects/settings/members_controller.rb
@@ -7,47 +7,18 @@ module Projects
@sort = params[:sort].presence || sort_value_name
@group_links = @project.project_group_links
- @project_members = @project.project_members
- @project_members = @project_members.non_invite unless can?(current_user, :admin_project, @project)
-
- group = @project.group
-
- # group links
- @group_links = @project.project_group_links.all
-
@skip_groups = @group_links.pluck(:group_id)
@skip_groups << @project.namespace_id unless @project.personal?
- if group
- # We need `.where.not(user_id: nil)` here otherwise when a group has an
- # invitee, it would make the following query return 0 rows since a NULL
- # user_id would be present in the subquery
- # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
- group_members = MembersFinder.new(@project_members, group).execute(current_user)
- end
+ @project_members = MembersFinder.new(@project, current_user).execute
if params[:search].present?
- user_ids = @project.users.search(params[:search]).select(:id)
- @project_members = @project_members.where(user_id: user_ids)
-
- if group_members
- user_ids = group.users.search(params[:search]).select(:id)
- group_members = group_members.where(user_id: user_ids)
- end
-
- @group_links = @project.project_group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id))
+ @project_members = @project_members.joins(:user).merge(User.search(params[:search]))
+ @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id))
end
- wheres = ["members.id IN (#{@project_members.select(:id).to_sql})"]
- wheres << "members.id IN (#{group_members.select(:id).to_sql})" if group_members
-
- @project_members = Member.
- where(wheres.join(' OR ')).
- sort(@sort).
- page(params[:page])
-
+ @project_members = @project_members.sort(@sort).page(params[:page])
@requesters = AccessRequestsFinder.new(@project).execute(current_user)
-
@project_member = @project.project_members.new
end
end
diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb
index 9f2206346ce..fce3775f40e 100644
--- a/app/finders/group_members_finder.rb
+++ b/app/finders/group_members_finder.rb
@@ -1,4 +1,4 @@
-class GroupMembersFinder < Projects::ApplicationController
+class GroupMembersFinder
def initialize(group)
@group = group
end
diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb
index 702944404f5..af24045886e 100644
--- a/app/finders/members_finder.rb
+++ b/app/finders/members_finder.rb
@@ -1,13 +1,35 @@
-class MembersFinder < Projects::ApplicationController
- def initialize(project_members, project_group)
- @project_members = project_members
- @project_group = project_group
+class MembersFinder
+ attr_reader :project, :current_user, :group
+
+ def initialize(project, current_user)
+ @project = project
+ @current_user = current_user
+ @group = project.group
+ end
+
+ def execute
+ project_members = project.project_members
+ project_members = project_members.non_invite unless can?(current_user, :admin_project, project)
+ wheres = ["members.id IN (#{project_members.select(:id).to_sql})"]
+
+ if group
+ # We need `.where.not(user_id: nil)` here otherwise when a group has an
+ # invitee, it would make the following query return 0 rows since a NULL
+ # user_id would be present in the subquery
+ # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
+ non_null_user_ids = project_members.where.not(user_id: nil).select(:user_id)
+
+ group_members = GroupMembersFinder.new(group).execute
+ group_members = group_members.where.not(user_id: non_null_user_ids)
+ group_members = group_members.non_invite unless can?(current_user, :admin_group, group)
+
+ wheres << "members.id IN (#{group_members.select(:id).to_sql})"
+ end
+
+ Member.where(wheres.join(' OR '))
end
- def execute(current_user)
- non_null_user_ids = @project_members.where.not(user_id: nil).select(:user_id)
- group_members = @project_group.group_members.where.not(user_id: non_null_user_ids)
- group_members = group_members.non_invite unless can?(current_user, :admin_group, @project_group)
- group_members
+ def can?(*args)
+ Ability.allowed?(*args)
end
end
diff --git a/changelogs/unreleased/dz-nested-groups-members.yml b/changelogs/unreleased/dz-nested-groups-members.yml
new file mode 100644
index 00000000000..bab0c8465c2
--- /dev/null
+++ b/changelogs/unreleased/dz-nested-groups-members.yml
@@ -0,0 +1,4 @@
+---
+title: Show members of parent groups on project members page
+merge_request:
+author:
diff --git a/lib/gitlab/import_export/project_tree_saver.rb b/lib/gitlab/import_export/project_tree_saver.rb
index b79be62245b..3473b466936 100644
--- a/lib/gitlab/import_export/project_tree_saver.rb
+++ b/lib/gitlab/import_export/project_tree_saver.rb
@@ -47,7 +47,13 @@ module Gitlab
def group_members
return [] unless @current_user.can?(:admin_group, @project.group)
- MembersFinder.new(@project.project_members, @project.group).execute(@current_user)
+ # We need `.where.not(user_id: nil)` here otherwise when a group has an
+ # invitee, it would make the following query return 0 rows since a NULL
+ # user_id would be present in the subquery
+ # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
+ non_null_user_ids = @project.project_members.where.not(user_id: nil).select(:user_id)
+
+ GroupMembersFinder.new(@project.group).execute.where.not(user_id: non_null_user_ids)
end
end
end
diff --git a/spec/finders/members_finder_spec.rb b/spec/finders/members_finder_spec.rb
new file mode 100644
index 00000000000..cf691cf684b
--- /dev/null
+++ b/spec/finders/members_finder_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe MembersFinder, '#execute' do
+ let(:group) { create(:group) }
+ let(:nested_group) { create(:group, :access_requestable, parent: group) }
+ let(:project) { create(:project, namespace: nested_group) }
+ let(:user1) { create(:user) }
+ let(:user2) { create(:user) }
+ let(:user3) { create(:user) }
+ let(:user4) { create(:user) }
+
+ it 'returns members for project and parent groups' do
+ nested_group.request_access(user1)
+ member1 = group.add_master(user2)
+ member2 = nested_group.add_master(user3)
+ member3 = project.add_master(user4)
+
+ result = described_class.new(project, user2).execute
+
+ expect(result.to_a).to eq([member3, member2, member1])
+ end
+end