Uses projects_authorizations.access_level in MembersFinder62284-follow-up-from-resolve-api-to-get-all-project-group-members-returns-duplicates

3 files changed, 69 insertions, 15 deletions

diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb
index f730b015c0a..e8c7f9622a9 100644
--- a/app/finders/members_finder.rb
+++ b/app/finders/members_finder.rb
@@ -60,15 +60,32 @@ class MembersFinder
     # 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.
     <<~SQL
-        SELECT DISTINCT ON (user_id, invite_email) member_union.*
-        FROM (#{union.to_sql}) AS member_union
-        ORDER BY user_id,
-          invite_email,
-          CASE
-            WHEN type = 'ProjectMember' THEN 1
-            WHEN type = 'GroupMember' THEN 2
-            ELSE 3
-          END
+          SELECT DISTINCT ON (user_id, invite_email) #{member_columns}
+          FROM (#{union.to_sql}) AS #{member_union_table}
+          LEFT JOIN users on users.id = member_union.user_id
+          LEFT JOIN project_authorizations on project_authorizations.user_id = users.id
+               AND
+               project_authorizations.project_id = #{project.id}
+          ORDER BY user_id,
+            invite_email,
+            CASE
+              WHEN type = 'ProjectMember' THEN 1
+              WHEN type = 'GroupMember' THEN 2
+              ELSE 3
+            END
     SQL
   end
+
+  def member_union_table
+    'member_union'
+  end
+
+  def member_columns
+    Member.column_names.map do |column_name|
+      # fallback to members.access_level when project_authorizations.access_level is missing
+      next "COALESCE(#{ProjectAuthorization.table_name}.access_level, #{member_union_table}.access_level) access_level" if column_name == 'access_level'
+
+      "#{member_union_table}.#{column_name}"
+    end.join(',')
+  end
 end
diff --git a/changelogs/unreleased/62284-follow-up-from-resolve-api-to-get-all-project-group-members-returns-duplicates.yml b/changelogs/unreleased/62284-follow-up-from-resolve-api-to-get-all-project-group-members-returns-duplicates.yml
new file mode 100644
index 00000000000..0c73f73c297
--- /dev/null
+++ b/changelogs/unreleased/62284-follow-up-from-resolve-api-to-get-all-project-group-members-returns-duplicates.yml
@@ -0,0 +1,5 @@
+---
+title: Uses projects_authorizations.access_level in MembersFinder
+merge_request: 28887
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/spec/finders/members_finder_spec.rb b/spec/finders/members_finder_spec.rb
index 4203f58fe81..6920fb4e572 100644
--- a/spec/finders/members_finder_spec.rb
+++ b/spec/finders/members_finder_spec.rb
@@ -17,11 +17,10 @@ describe MembersFinder, '#execute' do
 
     result = described_class.new(project, user2).execute
 
-    expect(result.to_a).to match_array([member1, member2, member3])
+    expect(result).to contain_exactly(member1, member2, member3)
   end
 
-  it 'includes nested group members if asked' do
-    project = create(:project, namespace: group)
+  it 'includes nested group members if asked', :nested_groups do
     nested_group.request_access(user1)
     member1 = group.add_maintainer(user2)
     member2 = nested_group.add_maintainer(user3)
@@ -29,7 +28,28 @@ describe MembersFinder, '#execute' do
 
     result = described_class.new(project, user2).execute(include_descendants: true)
 
-    expect(result.to_a).to match_array([member1, member2, member3])
+    expect(result).to contain_exactly(member1, member2, member3)
+  end
+
+  it 'returns the members.access_level when the user is invited', :nested_groups do
+    member_invite = create(:project_member, :invited, project: project, invite_email: create(:user).email)
+    member1 = group.add_maintainer(user2)
+
+    result = described_class.new(project, user2).execute(include_descendants: true)
+
+    expect(result).to contain_exactly(member1, member_invite)
+    expect(result.last.access_level).to eq(member_invite.access_level)
+  end
+
+  it 'returns the highest access_level for the user', :nested_groups do
+    member1 = project.add_guest(user1)
+    group.add_developer(user1)
+    nested_group.add_reporter(user1)
+
+    result = described_class.new(project, user1).execute(include_descendants: true)
+
+    expect(result).to contain_exactly(member1)
+    expect(result.first.access_level).to eq(Gitlab::Access::DEVELOPER)
   end
 
   context 'when include_invited_groups_members == true' do
@@ -37,8 +57,8 @@ describe MembersFinder, '#execute' do
 
     set(:linked_group) { create(:group, :public, :access_requestable) }
     set(:nested_linked_group) { create(:group, parent: linked_group) }
-    set(:linked_group_member) { linked_group.add_developer(user1) }
-    set(:nested_linked_group_member) { nested_linked_group.add_developer(user2) }
+    set(:linked_group_member) { linked_group.add_guest(user1) }
+    set(:nested_linked_group_member) { nested_linked_group.add_guest(user2) }
 
     it 'includes all the invited_groups members including members inherited from ancestor groups' do
       create(:project_group_link, project: project, group: nested_linked_group)
@@ -60,5 +80,17 @@ describe MembersFinder, '#execute' do
 
       expect(subject).to contain_exactly(linked_group_member)
     end
+
+    context 'when the user is a member of invited group and ancestor groups' do
+      it 'returns the highest access_level for the user limited by project_group_link.group_access', :nested_groups do
+        create(:project_group_link, project: project, group: nested_linked_group, group_access: Gitlab::Access::REPORTER)
+        nested_linked_group.add_developer(user1)
+
+        result = subject
+
+        expect(result).to contain_exactly(linked_group_member, nested_linked_group_member)
+        expect(result.first.access_level).to eq(Gitlab::Access::REPORTER)
+      end
+    end
   end
 end