Merge branch 'group-members-in-project-members-view' into 'master'

Shows group members in the project members list

Closes #24122

See merge request !7899

4 files changed, 124 insertions, 6 deletions

diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index 699a56ae2f8..3fb8bba3cd0 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -10,14 +10,38 @@ class Projects::ProjectMembersController < Projects::ApplicationController
     @project_members = @project.project_members
     @project_members = @project_members.non_invite unless can?(current_user, :admin_project, @project)
 
+    group = @project.group
+
+    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
+      # FIXME: This whole logic should be moved to a finder!
+      non_null_user_ids = @project_members.where.not(user_id: nil).select(:user_id)
+      group_members = group.group_members.where.not(user_id: non_null_user_ids)
+      group_members = group_members.non_invite unless can?(current_user, :admin_group, @group)
+    end
+
     if params[:search].present?
-      users = @project.users.search(params[:search]).to_a
-      @project_members = @project_members.where(user_id: users)
+      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))
     end
 
-    @project_members = @project_members.order(access_level: :desc).page(params[:page])
+    member_ids = @project_members.pluck(:id)
+
+    if group_members
+      member_ids += group_members.pluck(:id)
+    end
+
+    @project_members = Member.where(id: member_ids).order(access_level: :desc).page(params[:page])
 
     @requesters = AccessRequestsFinder.new(@project).execute(current_user)
 
diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml
index 432047a1c4e..e67f7d5a352 100644
--- a/app/views/shared/members/_member.html.haml
+++ b/app/views/shared/members/_member.html.haml
@@ -20,8 +20,8 @@
           %strong Blocked
 
       - if source.instance_of?(Group) && !@group
-        = link_to source, class: "member-group-link prepend-left-5" do
-          = "· #{source.name}"
+        &middot;
+        = link_to source.name, source, class: "member-group-link"
 
       .hidden-xs.cgray
         - if member.request?
@@ -45,7 +45,7 @@
         = time_ago_with_tooltip(member.created_at)
   - if show_roles
     .controls.member-controls
-      - if show_controls
+      - if show_controls && (member.respond_to?(:group) && @group) || (member.respond_to?(:project) && @project)
         - if user != current_user
           = form_for member, remote: true, html: { class: 'form-horizontal js-edit-member-form' } do |f|
             = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control member-form-control append-right-5 js-member-update-control', id: "member_access_level_#{member.id}", disabled: !can_admin_member
diff --git a/changelogs/unreleased/group-members-in-project-members-view.yml b/changelogs/unreleased/group-members-in-project-members-view.yml
new file mode 100644
index 00000000000..415e2b6b1e2
--- /dev/null
+++ b/changelogs/unreleased/group-members-in-project-members-view.yml
@@ -0,0 +1,4 @@
+---
+title: Shows group members in project members list
+merge_request: 
+author: 
diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb
new file mode 100644
index 00000000000..7d0065ee2c4
--- /dev/null
+++ b/spec/features/projects/members/group_members_spec.rb
@@ -0,0 +1,90 @@
+require 'spec_helper'
+
+feature 'Projects members', feature: true do
+  let(:user) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:group) { create(:group, :public, :access_requestable) }
+  let(:project) { create(:empty_project, :public, :access_requestable, creator: user, group: group) }
+  let(:project_invitee) { create(:project_member, project: project, invite_token: '123', invite_email: 'test1@abc.com', user: nil) }
+  let(:group_invitee) { create(:group_member, group: group, invite_token: '123', invite_email: 'test2@abc.com', user: nil) }
+  let(:project_requester) { create(:user) }
+  let(:group_requester) { create(:user) }
+
+  background do
+    project.team << [developer, :developer]
+    group.add_owner(user)
+    login_as(user)
+  end
+
+  context 'with a group invitee' do
+    before do
+      group_invitee
+      visit namespace_project_project_members_path(project.namespace, project)
+    end
+
+    scenario 'does not appear in the project members page' do
+      page.within first('.content-list') do
+        expect(page).not_to have_content('test2@abc.com')
+      end
+    end
+  end
+
+  context 'with a group and a project invitee' do
+    before do
+      group_invitee
+      project_invitee
+      visit namespace_project_project_members_path(project.namespace, project)
+    end
+
+    scenario 'shows the project invitee, the project developer, and the group owner' do
+      page.within first('.content-list') do
+        expect(page).to have_content('test1@abc.com')
+        expect(page).not_to have_content('test2@abc.com')
+
+        # Project developer
+        expect(page).to have_content(developer.name)
+
+        # Group owner
+        expect(page).to have_content(user.name)
+        expect(page).to have_content(group.name)
+      end
+    end
+  end
+
+  context 'with a group requester' do
+    before do
+      group.request_access(group_requester)
+      visit namespace_project_project_members_path(project.namespace, project)
+    end
+
+    scenario 'does not appear in the project members page' do
+      page.within first('.content-list') do
+        expect(page).not_to have_content(group_requester.name)
+      end
+    end
+  end
+
+  context 'with a group and a project requesters' do
+    before do
+      group.request_access(group_requester)
+      project.request_access(project_requester)
+      visit namespace_project_project_members_path(project.namespace, project)
+    end
+
+    scenario 'shows the project requester, the project developer, and the group owner' do
+      page.within first('.content-list') do
+        expect(page).to have_content(project_requester.name)
+        expect(page).not_to have_content(group_requester.name)
+      end
+
+      page.within all('.content-list').last do
+        # Project developer
+        expect(page).to have_content(developer.name)
+
+        # Group owner
+        expect(page).to have_content(user.name)
+        expect(page).to have_content(group.name)
+      end
+    end
+  end
+end