Commit 4dcf107b authored by Douwe Maan's avatar Douwe Maan

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
parents c11006ac 1f7353ce
...@@ -88,28 +88,17 @@ ...@@ -88,28 +88,17 @@
= select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2" = select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2"
%hr %hr
= button_tag 'Add users to group', class: "btn btn-create" = button_tag 'Add users to group', class: "btn btn-create"
= render 'shared/members/requests', membership_source: @group, members: @members.request
.panel.panel-default .panel.panel-default
.panel-heading .panel-heading
%h3.panel-title %strong= @group.name
Members group members
%span.badge %span.badge= @group.members.non_request.size
#{@group.group_members.count} .pull-right
%ul.well-list.group-users-list = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@group, :members]), class: "btn btn-xs"
- @members.each do |member| %ul.well-list.group-users-list.content-list
- user = member.user = render partial: 'shared/members/member', collection: @members.non_request, as: :member, locals: { show_controls: false }
%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
.panel-footer .panel-footer
= paginate @members, param_name: 'members_page', theme: 'gitlab' = paginate @members.non_request, param_name: 'members_page', theme: 'gitlab'
...@@ -135,44 +135,27 @@ ...@@ -135,44 +135,27 @@
- if @group - if @group
.panel.panel-default .panel.panel-default
.panel-heading .panel-heading
%strong #{@group.name} %strong= @group.name
group members (#{@group.group_members.count}) group members
%span.badge= @group_members.non_request.size
.pull-right .pull-right
= link_to admin_group_path(@group), class: 'btn btn-xs' do = link_to admin_group_path(@group), class: 'btn btn-xs' do
%i.fa.fa-pencil-square-o = icon('pencil-square-o', text: 'Manage Access')
%ul.well-list %ul.well-list.content-list
- @group_members.each do |member| = render partial: 'shared/members/member', collection: @group_members.non_request, as: :member, locals: { show_controls: false }
= render 'shared/members/member', member: member, show_controls: false
.panel-footer .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.panel-default
.panel-heading .panel-heading
Project members %strong= @project.name
%small project members
(#{@project.users.count}) %span.badge= @project.users.size
.pull-right .pull-right
= link_to namespace_project_project_members_path(@project.namespace, @project), class: "btn btn-xs" do = link_to icon('pencil-square-o', text: 'Manage Access'), polymorphic_url([@project, :members]), class: "btn btn-xs"
%i.fa.fa-pencil-square-o %ul.well-list.project_members.content-list
Manage Access = render partial: 'shared/members/member', collection: @project_members.non_request, as: :member, locals: { show_controls: false }
%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
.panel-footer .panel-footer
= paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' = paginate @project_members.non_request, param_name: 'project_members_page', theme: 'gitlab'
...@@ -17,8 +17,7 @@ ...@@ -17,8 +17,7 @@
.panel-heading .panel-heading
%strong #{@group.name} %strong #{@group.name}
group members group members
%small %span.badge= @members.non_request.size
(#{@members.total_count})
.controls .controls
= form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form' do = form_tag group_group_members_path(@group), method: :get, class: 'form-inline member-search-form' do
.form-group .form-group
......
...@@ -2,8 +2,7 @@ ...@@ -2,8 +2,7 @@
.panel-heading .panel-heading
%strong #{@group.name} %strong #{@group.name}
group members group members
%small %span.badge= members.size
(#{members.count})
- if can?(current_user, :admin_group_member, @group) - if can?(current_user, :admin_group_member, @group)
.controls .controls
= link_to 'Manage group members', = link_to 'Manage group members',
......
- @project_group_links.each do |group_links| - @project_group_links.each do |group_links|
- shared_group = group_links.group - 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.panel-default
.panel-heading .panel-heading
Shared with Shared with
...@@ -15,7 +16,7 @@ ...@@ -15,7 +16,7 @@
Edit group members Edit group members
%ul.content-list %ul.content-list
= render partial: 'shared/members/member', = 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, as: :member,
locals: { show_controls: false, show_roles: false } locals: { show_controls: false, show_roles: false }
- if shared_group_users_count > 20 - if shared_group_users_count > 20
......
...@@ -2,8 +2,7 @@ ...@@ -2,8 +2,7 @@
.panel-heading .panel-heading
%strong #{@project.name} %strong #{@project.name}
project members project members
%small %span.badge= members.size
(#{members.count})
.controls .controls
= form_tag namespace_project_project_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form' do = form_tag namespace_project_project_members_path(@project.namespace, @project), method: :get, class: 'form-inline member-search-form' do
.form-group .form-group
......
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
= render 'team', members: @project_members.non_request = render 'team', members: @project_members.non_request
- if @group - 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? - if @project_group_links.any? && @project.allowed_to_share_with_group?
= render "shared_group_members" = render "shared_group_members"
...@@ -3,6 +3,6 @@ ...@@ -3,6 +3,6 @@
.panel-heading .panel-heading
%strong= membership_source.name %strong= membership_source.name
access requests access requests
%small= "(#{members.size})" %span.badge= members.size
%ul.content-list %ul.content-list
= render partial: 'shared/members/member', collection: members, as: :member = render partial: 'shared/members/member', collection: members, as: :member
...@@ -26,13 +26,6 @@ Feature: Admin Groups ...@@ -26,13 +26,6 @@ Feature: Admin Groups
When I visit group page When I visit group page
Then I should see project shared with group 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 @javascript
Scenario: Invite user to a group by e-mail Scenario: Invite user to a group by e-mail
When I visit admin group page When I visit admin group page
......
...@@ -62,7 +62,7 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps ...@@ -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 step 'I should see "johndoe@gitlab.com" in team list in every project as "Reporter"' do
page.within ".group-users-list" 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" expect(page).to have_content "Reporter"
end end
end end
...@@ -92,12 +92,6 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps ...@@ -92,12 +92,6 @@ class Spinach::Features::AdminGroups < Spinach::FeatureSteps
current_group.add_reporter(user_john) current_group.add_reporter(user_john)
end 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 step 'I should not see "John Doe" in team list' do
page.within ".group-users-list" do page.within ".group-users-list" do
expect(page).not_to have_content "John Doe" expect(page).not_to have_content "John Doe"
......
...@@ -42,7 +42,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do ...@@ -42,7 +42,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
def expect_visible_access_request(group, user) def expect_visible_access_request(group, user)
expect(group.members.request.exists?(user_id: user)).to be_truthy 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 expect(page).to have_content user.name
end end
end end
...@@ -41,7 +41,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do ...@@ -41,7 +41,7 @@ feature 'Projects > Members > Master manages access requests', feature: true do
def expect_visible_access_request(project, user) def expect_visible_access_request(project, user)
expect(project.members.request.exists?(user_id: user)).to be_truthy 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 expect(page).to have_content user.name
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment