diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2019-05-27 14:58:31 +0000 |
---|---|---|
committer | Jan Provaznik <jprovaznik@gitlab.com> | 2019-05-27 14:58:31 +0000 |
commit | 1db09731e77e0869a54faff8bf8494892d4bd640 (patch) | |
tree | e21fc723dd97f34af65e385c7475e1d87db53fba /app | |
parent | 31315850b0108b82b7bb3aa70194dc4dafc9ee7d (diff) | |
parent | a9827e0e18b532fb5cc3f227ce6c6bddaf7a960b (diff) | |
download | gitlab-ce-1db09731e77e0869a54faff8bf8494892d4bd640.tar.gz |
Merge branch '51854-api-to-get-all-project-group-members-returns-duplicates' into 'master'
Resolve "API to get all project/group members returns duplicates"
Closes #51854
See merge request gitlab-org/gitlab-ce!24005
Diffstat (limited to 'app')
-rw-r--r-- | app/finders/members_finder.rb | 43 | ||||
-rw-r--r-- | app/models/member.rb | 2 |
2 files changed, 34 insertions, 11 deletions
diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index f90a7868102..917de249104 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -9,25 +9,18 @@ class MembersFinder @group = project.group end - # rubocop: disable CodeReuse/ActiveRecord - def execute(include_descendants: false) + def execute(include_descendants: false, include_invited_groups_members: false) project_members = project.project_members project_members = project_members.non_invite unless can?(current_user, :admin_project, project) - if group - group_members = GroupMembersFinder.new(group).execute(include_descendants: include_descendants) # rubocop: disable CodeReuse/Finder - group_members = group_members.non_invite + union_members = group_union_members(include_descendants, include_invited_groups_members) - union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false) # rubocop: disable Gitlab/Union - - sql = distinct_on(union) - - Member.includes(:user).from("(#{sql}) AS #{Member.table_name}") + if union_members.any? + distinct_union_of_members(union_members << project_members) else project_members end end - # rubocop: enable CodeReuse/ActiveRecord def can?(*args) Ability.allowed?(*args) @@ -35,6 +28,34 @@ class MembersFinder private + def group_union_members(include_descendants, include_invited_groups_members) + [].tap do |members| + members << direct_group_members(include_descendants) if group + members << project_invited_groups_members if include_invited_groups_members + end + end + + def direct_group_members(include_descendants) + GroupMembersFinder.new(group).execute(include_descendants: include_descendants).non_invite # rubocop: disable CodeReuse/Finder + end + + def project_invited_groups_members + invited_groups_ids_including_ancestors = Gitlab::ObjectHierarchy + .new(project.invited_groups) + .base_and_ancestors + .public_or_visible_to_user(current_user) + .select(:id) + + GroupMember.with_source_id(invited_groups_ids_including_ancestors) + end + + def distinct_union_of_members(union_members) + union = Gitlab::SQL::Union.new(union_members, remove_duplicates: false) # rubocop: disable Gitlab/Union + sql = distinct_on(union) + + Member.includes(:user).from([Arel.sql("(#{sql}) AS #{Member.table_name}")]) # rubocop: disable CodeReuse/ActiveRecord + end + def distinct_on(union) # We're interested in a list of members without duplicates by user_id. # We prefer project members over group members, project members should go first. diff --git a/app/models/member.rb b/app/models/member.rb index 83b4f5b29c4..c7583434148 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -80,6 +80,8 @@ class Member < ApplicationRecord scope :owners_and_masters, -> { owners_and_maintainers } # @deprecated scope :with_user, -> (user) { where(user: user) } + scope :with_source_id, ->(source_id) { where(source_id: source_id) } + scope :order_name_asc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'ASC')) } scope :order_name_desc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'DESC')) } scope :order_recent_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'DESC')) } |