Merge branch '18871-check-improve-how-we-display-access-requesters-in-admin-area' into 'master'

Display group/project access requesters separately in admin

## What does this MR do?

It displays the access requesters in a separate list in group & project members pages.

It also harmonize the members counter UI to use `%span.badge` everywhere (in the admin & non-admin members views).

## Are there points in the code the reviewer needs to double check?

No.

## Why was this MR needed?

To not confuse access requesters with actual members.

## What are the relevant issue numbers?

Closes #18871.

## Screenshots

### Group members

| Before | After |
| --------- | ---- |
| ![group-members-before](/uploads/2f15137e073fd3a63bc2cb7b2217cb6c/group-members-before.png) | ![group-members-after](/uploads/5b643974505cfa57783fa0320d3bf8b2/group-members-after.png) |

### Project members

| Before | After |
| --------- | ---- |
| ![project-members-before](/uploads/9c48dcd3736e42de84061b1201ee0b06/project-members-before.png) | ![project-members-after](/uploads/8e04c92ef0bba3de7e2405618632b27d/project-members-after.png) |

### Admin group members

| Before | After |
| --------- | ---- |
| ![admin-group-members-before](/uploads/7fda8c2c94b697bea6655ba892ba45e7/admin-group-members-before.png) | ![admin-group-members-after](/uploads/ea25717001794f75939c679b80308c3a/admin-group-members-after.png) |

### Admin project members

| Before | After |
| --------- | ---- |
| ![admin-project-members-before](/uploads/ba9d3ec52adbda6bb3d45ad9ac5243d3/admin-project-members-before.png) | ![admin-project-members-after](/uploads/3b889a029a9756e9ed2781b45c4dd9cb/admin-project-members-after.png) |

## Does this MR meet the acceptance criteria?

- [x] No CHANGELOG since this is related to the original "request access" MR.
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4798

Signed-off-by: Rémy Coutable <remy@rymai.me>

13 files changed, 39 insertions, 81 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2dca5a2405e..f455a5b39fd 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -7,6 +7,7 @@ v 8.9.1 (unreleased)
   - Add documentation for the export & import features. !4732
   - Add some docs for Docker Registry configuration. !4738
   - Ensure we don't send the "access request declined" email to access requesters on project deletion. !4744
+  - Display group/project access requesters separately in the admin area. !4798
   - Fix MR-auto-close text added to description. !4836
   - Fix typo in export failure email. !4847
 
diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml
index 5b8a0262ea0..50770465f07 100644
--- a/app/views/admin/groups/show.html.haml
+++ b/app/views/admin/groups/show.html.haml
@@ -88,28 +88,17 @@
               = select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2"
             %hr
             = button_tag 'Add users to group', class: "btn btn-create"
+
+    = render 'shared/members/requests', membership_source: @group, members: @members.request
+
     .panel.panel-default
       .panel-heading
-        %h3.panel-title
-          Members
-          %span.badge
-            #{@group.group_members.count}
-      %ul.well-list.group-users-list
-        - @members.each do |member|
-          - user = member.user
-          %li{class: dom_class(member), id: (dom_id(user) if user)}
-            .list-item-name
-              - if user
-                %strong
-                  = link_to user.name, admin_user_path(user)
-              - else
-                %strong
-                  = member.invite_email
-                (invited)
-            %span.pull-right.light
-              = member.human_access
-              - if can?(current_user, :destroy_group_member, member)
-                = link_to group_group_member_path(@group, member), data: { confirm: remove_member_message(member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do
-                  %i.fa.fa-minus.fa-inverse
+        %strong= @group.name
+        group members
+        %span.badge= @group.members.non_request.size
+        .pull-right
+          = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@group, :members]), class: "btn btn-xs"
+      %ul.well-list.group-users-list.content-list
+        = render partial: 'shared/members/member', collection: @members.non_request, as: :member, locals: { show_controls: false }
       .panel-footer
-        = paginate @members, param_name: 'members_page', theme: 'gitlab'
+        = paginate @members.non_request, param_name: 'members_page', theme: 'gitlab'
diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml
index 9e55a562e18..461d588415d 100644
--- a/app/views/admin/projects/show.html.haml
+++ b/app/views/admin/projects/show.html.haml
@@ -135,44 +135,27 @@
     - if @group
       .panel.panel-default
         .panel-heading
-          %strong #{@group.name}
-          group members (#{@group.group_members.count})
+          %strong= @group.name
+          group members
+          %span.badge= @group_members.non_request.size
           .pull-right
             = link_to admin_group_path(@group), class: 'btn btn-xs' do
-              %i.fa.fa-pencil-square-o
-        %ul.well-list
-          - @group_members.each do |member|
-            = render 'shared/members/member', member: member, show_controls: false
+              = icon('pencil-square-o', text: 'Manage Access')
+        %ul.well-list.content-list
+          = render partial: 'shared/members/member', collection: @group_members.non_request, as: :member, locals: { show_controls: false }
         .panel-footer
-          = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab'
+          = paginate @group_members.non_request, param_name: 'group_members_page', theme: 'gitlab'
+
+    = render 'shared/members/requests', membership_source: @project, members: @project_members.request
 
     .panel.panel-default
       .panel-heading
-        Project members
-        %small
-          (#{@project.users.count})
+        %strong= @project.name
+        project members
+        %span.badge= @project.users.size
         .pull-right
-          = link_to namespace_project_project_members_path(@project.namespace, @project), class: "btn btn-xs" do
-            %i.fa.fa-pencil-square-o
-            Manage Access
-      %ul.well-list.project_members
-        - @project_members.each do |project_member|
-          - user = project_member.user
-          %li.project_member
-            .list-item-name
-              - if user
-                %strong
-                  = link_to user.name, admin_user_path(user)
-              - else
-                %strong
-                  = project_member.invite_email
-                (invited)
-            .pull-right
-              - if project_member.owner?
-                %span.light Owner
-              - else
-                %span.light= project_member.human_access
-                = link_to namespace_project_project_member_path(@project.namespace, @project, project_member), data: { confirm: remove_member_message(project_member)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do
-                  %i.fa.fa-times
+          = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@project, :members]), class: "btn btn-xs"
+      %ul.well-list.project_members.content-list
+        = render partial: 'shared/members/member', collection: @project_members.non_request, as: :member, locals: { show_controls: false }
       .panel-footer
-        = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab'
+        = paginate @project_members.non_request, param_name: 'project_members_page', theme: 'gitlab'
diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml
index a36531e095a..d6acade84f1 100644
--- a/app/views/groups/group_members/index.html.haml
+++ b/app/views/groups/group_members/index.html.haml
@@ -17,8 +17,7 @@
     .panel-heading
       %strong #{@group.name}
       group members
-      %small
-        (#{@members.total_count})
+      %span.badge= @members.non_request.size
       .controls
         = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form'  do
           .form-group
diff --git a/app/views/projects/project_members/_group_members.html.haml b/app/views/projects/project_members/_group_members.html.haml
index cb6136c215a..e783d8c72c5 100644
--- a/app/views/projects/project_members/_group_members.html.haml
+++ b/app/views/projects/project_members/_group_members.html.haml
@@ -2,8 +2,7 @@
   .panel-heading
     %strong #{@group.name}
     group members
-    %small
-      (#{members.count})
+    %span.badge= members.size
     - if can?(current_user, :admin_group_member, @group)
       .controls
         = link_to 'Manage group members',
diff --git a/app/views/projects/project_members/_shared_group_members.html.haml b/app/views/projects/project_members/_shared_group_members.html.haml
index 952844acefc..840b57c2e63 100644
--- a/app/views/projects/project_members/_shared_group_members.html.haml
+++ b/app/views/projects/project_members/_shared_group_members.html.haml
@@ -1,6 +1,7 @@
 - @project_group_links.each do |group_links|
   - shared_group = group_links.group
-  - shared_group_users_count = group_links.group.group_members.count
+  - shared_group_members = shared_group.members.non_request
+  - shared_group_users_count = shared_group_members.size
   .panel.panel-default
     .panel-heading
       Shared with
@@ -15,7 +16,7 @@
             Edit group members
     %ul.content-list
       = render partial: 'shared/members/member',
-               collection: shared_group.group_members.order(access_level: :desc).limit(20),
+               collection: shared_group_members.order(access_level: :desc).limit(20),
                as: :member,
                locals: { show_controls: false, show_roles: false }
       - if shared_group_users_count > 20
diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml
index 03207614258..b0bfdd235f7 100644
--- a/app/views/projects/project_members/_team.html.haml
+++ b/app/views/projects/project_members/_team.html.haml
@@ -2,8 +2,7 @@
   .panel-heading
     %strong #{@project.name}
     project members
-    %small
-      (#{members.count})
+    %span.badge= members.size
     .controls
       = form_tag namespace_project_project_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form'  do
         .form-group
diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml
index 357ccccaf1d..a2026c41d01 100644
--- a/app/views/projects/project_members/index.html.haml
+++ b/app/views/projects/project_members/index.html.haml
@@ -18,7 +18,7 @@
   = render 'team', members: @project_members.non_request
 
   - if @group
-    = render "group_members", members: @group_members
+    = render "group_members", members: @group_members.non_request
 
   - if @project_group_links.any? && @project.allowed_to_share_with_group?
     = render "shared_group_members"
diff --git a/app/views/shared/members/_requests.html.haml b/app/views/shared/members/_requests.html.haml
index b5963876034..e4bd2bdc265 100644
--- a/app/views/shared/members/_requests.html.haml
+++ b/app/views/shared/members/_requests.html.haml
@@ -3,6 +3,6 @@
     .panel-heading
       %strong= membership_source.name
       access requests
-      %small= "(#{members.size})"
+      %span.badge= members.size
     %ul.content-list
       = render partial: 'shared/members/member', collection: members, as: :member
diff --git a/features/admin/groups.feature b/features/admin/groups.feature
index ab7de7ac315..657e847cf4a 100644
--- a/features/admin/groups.feature
+++ b/features/admin/groups.feature
@@ -27,13 +27,6 @@ Feature: Admin Groups
     Then I should see project shared with group
 
   @javascript
-  Scenario: Remove user from group
-    Given we have user "John Doe" in group
-    When I visit admin group page
-    And I remove user "John Doe" from group
-    Then I should not see "John Doe" in team list
-
-  @javascript
   Scenario: Invite user to a group by e-mail
     When I visit admin group page
     When I select user "johndoe@gitlab.com" from user list as "Reporter"
diff --git a/features/steps/admin/groups.rb b/features/steps/admin/groups.rb
index e1f1db2872f..8613dc537cc 100644
--- a/features/steps/admin/groups.rb
+++ b/features/steps/admin/groups.rb
@@ -62,7 +62,7 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps
 
   step 'I should see "johndoe@gitlab.com" in team list in every project as "Reporter"' do
     page.within ".group-users-list" do
-      expect(page).to have_content "johndoe@gitlab.com (invited)"
+      expect(page).to have_content "johndoe@gitlab.com – Invited by"
       expect(page).to have_content "Reporter"
     end
   end
@@ -92,12 +92,6 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps
     current_group.add_reporter(user_john)
   end
 
-  step 'I remove user "John Doe" from group' do
-    page.within "#user_#{user_john.id}" do
-      click_link 'Remove user from group'
-    end
-  end
-
   step 'I should not see "John Doe" in team list' do
     page.within ".group-users-list" do
       expect(page).not_to have_content "John Doe"
diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb
index 22525ce530b..321c9bad7d0 100644
--- a/spec/features/groups/members/owner_manages_access_requests_spec.rb
+++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb
@@ -42,7 +42,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
 
   def expect_visible_access_request(group, user)
     expect(group.members.request.exists?(user_id: user)).to be_truthy
-    expect(page).to have_content "#{group.name} access requests (1)"
+    expect(page).to have_content "#{group.name} access requests 1"
     expect(page).to have_content user.name
   end
 end
diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb
index 5fe4caa12f0..aa2d906fa2e 100644
--- a/spec/features/projects/members/master_manages_access_requests_spec.rb
+++ b/spec/features/projects/members/master_manages_access_requests_spec.rb
@@ -41,7 +41,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do
 
   def expect_visible_access_request(project, user)
     expect(project.members.request.exists?(user_id: user)).to be_truthy
-    expect(page).to have_content "#{project.name} access requests (1)"
+    expect(page).to have_content "#{project.name} access requests 1"
     expect(page).to have_content user.name
   end
 end