diff options
author | Jacopo <beschi.jacopo@gmail.com> | 2019-01-29 21:10:37 +0300 |
---|---|---|
committer | Jacopo <beschi.jacopo@gmail.com> | 2019-05-27 16:40:56 +0300 |
commit | a9827e0e18b532fb5cc3f227ce6c6bddaf7a960b (patch) | |
tree | 1cd3b68ccff5d664a2a31a3fed44626fa3ac23ad /app | |
parent | f9a2c4034f690e4ab9803522110c063d78da8f4d (diff) |
Removes duplicated members from api/projects/:id/members/all
When using the members/all api the same user was returned multiple times
when he was a member of the project/group and also of one of the
ancestor groups.
Now the member is returned only once giving priority to the membership
on the project and maintaining the same behaviour of the members UI.
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')) } |