Reorganize group member management into tabs

Move groups, pending, and access requests into tabs

app/assets/stylesheets/pages/users.scss

@@ -25,8 +25,12 @@
   }
 
   .form-control {
-    width: 100%;
     padding-right: 35px;
+  }
+
+  .search-control-wrap,
+  .form-control {
+    width: 100%;
 
     @include media-breakpoint-up(sm) {
       width: 250px;

app/views/groups/group_members/index.html.haml

 - page_title _("Group members")
 - can_manage_members = can?(current_user, :admin_group_member, @group)
 - show_invited_members = can_manage_members && @invited_members.exists?
-- pending_active = params[:search_invited].present?
-- total_count = @members.count + @group.shared_with_group_links.count
+- show_access_requests = can_manage_members && @requesters.exists?
+- pending_active = params[:search_invited].present? || params[:invited_members_page].present?
 
 .js-remove-member-modal
 .project-members-page.gl-mt-3
@@ -21,64 +21,83 @@
       .tab-pane{ id: 'invite-group-pane', role: 'tabpanel' }
         = render 'shared/members/invite_group', submit_url: group_group_links_path(@group), access_levels: GroupMember.access_level_roles, default_access_level: @group_member.access_level, group_link_field: 'shared_with_group_id', group_access_field: 'shared_group_access'
 
-    = render 'shared/members/requests', membership_source: @group, requesters: @requesters
-
   = render_if_exists 'groups/group_members/ldap_sync'
 
-  %ul.nav-links.mobile-separator.nav.nav-tabs.clearfix
+  %ul.nav-links.mobile-separator.nav.nav-tabs
     %li.nav-item
-      = link_to "#existing_shares", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do
+      = link_to "#tab-members", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do
         %span
-          = _("Existing shares")
-        %span.badge.badge-pill= total_count
+          = _("Members")
+        %span.badge.badge-pill= @members.total_count
+    - if @group.shared_with_group_links.any?
+      %li.nav-item
+        = link_to "#tab-groups", class: ["nav-link"] , 'data-toggle' => 'tab' do
+          %span
+            = _("Groups")
+          %span.badge.badge-pill= @group.shared_with_group_links.count
     - if show_invited_members
       %li.nav-item
-        = link_to "#invited_members", class: ["nav-link", ("active" if pending_active)], 'data-toggle' => 'tab' do
+        = link_to "#tab-pending-members", class: ["nav-link", ("active" if pending_active)], 'data-toggle' => 'tab' do
           %span
             = _("Pending")
           %span.badge.badge-pill= @invited_members.total_count
-
+    - if show_access_requests
+      %li.nav-item
+        = link_to "#tab-access-requests", class: "nav-link", 'data-toggle' => 'tab' do
+          %span
+            = _("Access requests")
+          %span.badge.badge-pill= @requesters.count
   .tab-content
-    #existing_shares.tab-pane{ :class => ("active" unless pending_active) }
-      - if @group.shared_with_group_links.any?
-        .card.card-without-border
-          .d-flex.flex-column.flex-md-row.row-content-block.second-block
-            %span.flex-grow-1.align-self-md-center.col-form-label
-              = _("Groups with access to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
-          %ul.content-list.members-list{ data: { qa_selector: "groups_list" } }
-            - can_admin_member = can?(current_user, :admin_group_member, @group)
-            - @group.shared_with_group_links.each do |group_link|
-              = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: group_group_link_path(@group, group_link)
+    #tab-members.tab-pane{ :class => ("active" unless pending_active) }
       .card.card-without-border
-        .d-flex.flex-column.flex-md-row.row-content-block.second-block
-          %span.flex-grow-1.align-self-md-center.col-form-label
-            = _("Members with access to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
-          = form_tag group_group_members_path(@group), method: :get, class: 'form-inline user-search-form' do
-            .form-group.flex-grow
-              .position-relative.mr-md-2
+        .gl-display-flex.gl-md-align-items-center.gl-flex-direction-column.gl-md-flex-direction-row.row-content-block.second-block
+          %span.gl-flex-grow-1.gl-py-3.gl-pr-3
+            = html_escape(_("Members with access to %{strong_start}%{group_name}%{strong_end}")) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
+          = form_tag group_group_members_path(@group), method: :get, class: 'user-search-form gl-display-flex gl-md-align-items-center gl-flex-wrap gl-flex-direction-column gl-md-flex-direction-row gl-mx-n3 gl-my-n3' do
+            .gl-px-3.gl-py-2
+              .search-control-wrap.gl-relative
                 = search_field_tag :search, params[:search], { placeholder: _('Search'), class: 'form-control', spellcheck: false }
-                %button.user-search-btn.border-left{ type: "submit", "aria-label" => _("Submit search") }
-                  = icon("search")
-              - if can_manage_members
-                = label_tag '2fa', '2FA', class: 'col-form-label label-bold pr-md-2'
+                %button.user-search-btn.border-left.gl-display-flex.gl-align-items-center.gl-justify-content-center{ type: "submit", "aria-label" => _("Submit search") }
+                  = sprite_icon("search")
+            - if can_manage_members
+              .gl-px-3.gl-py-3.gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row
+                = label_tag '2fa', _('2FA'), class: 'label-bold gl-mr-2 gl-mb-0 gl-py-2 align-self-md-center'
                 = render 'shared/members/filter_2fa_dropdown'
+            .gl-px-3.gl-py-3.gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row
+              = label_tag :sort_by, _('Sort by'), class: 'label-bold gl-mr-2 gl-mb-0 gl-py-2 align-self-md-center'
               = render 'shared/members/sort_dropdown'
         %ul.content-list.members-list{ data: { qa_selector: "members_list" } }
           = render partial: 'shared/members/member', collection: @members, as: :member
-      = paginate @members, theme: 'gitlab'
-
+      = paginate @members, theme: 'gitlab', params: { invited_members_page: nil, search_invited: nil }
+    - if @group.shared_with_group_links.any?
+      #tab-groups.tab-pane
+        .card.card-without-border
+          .gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row.row-content-block.second-block
+            %span.gl-flex-grow-1.align-self-md-center.gl-py-3
+              = html_escape(_("Groups with access to %{strong_start}%{group_name}%{strong_end}")) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
+          %ul.content-list.members-list{ data: { qa_selector: "groups_list" } }
+            - can_admin_member = can?(current_user, :admin_group_member, @group)
+            - @group.shared_with_group_links.each do |group_link|
+              = render 'shared/members/group', group_link: group_link, can_admin_member: can_admin_member, group_link_path: group_group_link_path(@group, group_link)
     - if show_invited_members
-      #invited_members.tab-pane{ :class => ("active" if pending_active) }
+      #tab-pending-members.tab-pane{ :class => ("active" if pending_active) }
         .card.card-without-border
-          .d-flex.flex-column.flex-md-row.row-content-block.second-block
-            %span.flex-grow-1
-              = _("Members with pending access to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
-            = form_tag group_group_members_path(@group), method: :get, class: 'form-inline user-search-form' do
-              .form-group
-                .position-relative.mr-md-2
-                  = search_field_tag :search_invited, params[:search_invited], { placeholder: _('Search'), class: 'form-control', spellcheck: false }
-                  %button.user-search-btn.border-left{ type: "submit", "aria-label" => _("Submit search") }
-                    = icon("search")
+          .gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row.row-content-block.second-block
+            %span.gl-flex-grow-1.gl-md-align-self-center.gl-py-3
+              = html_escape(_("Members with pending access to %{strong_start}%{group_name}%{strong_end}")) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
+            = form_tag group_group_members_path(@group), method: :get, class: 'user-search-form' do
+              .search-control-wrap.gl-relative
+                = search_field_tag :search_invited, params[:search_invited], { placeholder: _('Search'), class: 'form-control', spellcheck: false }
+                %button.user-search-btn.border-left.gl-display-flex.gl-align-items-center.gl-justify-content-center{ type: "submit", "aria-label" => _("Submit search") }
+                  = sprite_icon("search")
           %ul.content-list.members-list
             = render partial: 'shared/members/member', collection: @invited_members, as: :member
-        = paginate @invited_members, param_name: 'invited_members_page', theme: 'gitlab'
+        = paginate @invited_members, param_name: 'invited_members_page', theme: 'gitlab', params: { page: nil }
+    - if show_access_requests
+      #tab-access-requests.tab-pane
+        .card.card-without-border
+          .gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row.row-content-block.second-block
+            %span.flex-grow-1.align-self-md-center.gl-py-3
+              = html_escape(_("Users requesting access to %{strong_start}%{group_name}%{strong_end}")) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
+          %ul.content-list.members-list
+            = render partial: 'shared/members/member', collection: @requesters, as: :member

app/views/projects/project_members/_team.html.haml

@@ -12,6 +12,7 @@
           = search_field_tag :search, params[:search], { placeholder: _('Find existing members by name'), class: 'form-control', spellcheck: false }
           %button.user-search-btn{ type: "submit", "aria-label" => _("Submit search") }
             = icon("search")
+        = label_tag :sort_by, _('Sort by'), class: 'col-form-label label-bold px-2'
         = render 'shared/members/sort_dropdown'
   %ul.content-list.members-list{ data: { qa_selector: 'members_list' } }
     = render partial: 'shared/members/member', collection: members, as: :member

app/views/shared/members/_filter_2fa_dropdown.html.haml

 - filter = params[:two_factor] || 'everyone'
 - filter_options = { 'everyone' => _('Everyone'), 'enabled' => _('Enabled'), 'disabled' => _('Disabled') }
-.dropdown.inline.member-filter-2fa-dropdown.pr-md-2
+.dropdown.inline.member-filter-2fa-dropdown
   = dropdown_toggle(filter_options[filter], { toggle: 'dropdown' })
   %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable
     %li.dropdown-header

app/views/shared/members/_sort_dropdown.html.haml

-= label_tag :sort_by, 'Sort by', class: 'col-form-label label-bold px-2'
 .dropdown.inline.qa-user-sort-dropdown
   = dropdown_toggle(member_sort_options_hash[@sort], { toggle: 'dropdown' })
   %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable

changelogs/unreleased/229830-move-groups-members-pending-in-to-tabs.yml

+---
+title: Reorganize group member management into tabs
+merge_request: 38344
+author:
+type: changed

locale/gitlab.pot

@@ -1234,6 +1234,9 @@ msgstr ""
 msgid "Access forbidden. Check your access level."
 msgstr ""
 
+msgid "Access requests"
+msgstr ""
+
 msgid "Access to '%{classification_label}' not allowed"
 msgstr ""
 
@@ -9899,9 +9902,6 @@ msgstr ""
 msgid "Existing projects will be able to use expiration policies. Avoid enabling this if an external Container Registry is being used, as there is a performance risk if many images exist on one project."
 msgstr ""
 
-msgid "Existing shares"
-msgstr ""
-
 msgid "Existing sign in methods may be removed"
 msgstr ""
 
@@ -26616,6 +26616,9 @@ msgstr ""
 msgid "Users requesting access to"
 msgstr ""
 
+msgid "Users requesting access to %{strong_start}%{group_name}%{strong_end}"
+msgstr ""
+
 msgid "Users were successfully added."
 msgstr ""
 

spec/features/groups/members/manage_groups_spec.rb

@@ -20,26 +20,28 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
 
     add_group(shared_with_group.id, 'Reporter')
 
+    click_groups_tab
+
     page.within(first_row) do
       expect(page).to have_content(shared_with_group.name)
       expect(page).to have_content('Reporter')
     end
   end
 
-  it 'remove user from group' do
+  it 'remove group from group' do
     create(:group_group_link, shared_group: shared_group,
       shared_with_group: shared_with_group, group_access: ::Gitlab::Access::DEVELOPER)
 
     visit group_group_members_path(shared_group)
 
+    click_groups_tab
+
     expect(page).to have_content(shared_with_group.name)
 
     accept_confirm do
-      find(:css, '#existing_shares li', text: shared_with_group.name).find(:css, 'a.btn-remove').click
+      find(:css, '#tab-groups li', text: shared_with_group.name).find(:css, 'a.btn-remove').click
     end
 
-    wait_for_requests
-
     expect(page).not_to have_content(shared_with_group.name)
   end
 
@@ -49,6 +51,8 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
 
     visit group_group_members_path(shared_group)
 
+    click_groups_tab
+
     page.within(first_row) do
       click_button('Developer')
       click_link('Maintainer')
@@ -67,4 +71,8 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
       click_button "Invite"
     end
   end
+
+  def click_groups_tab
+    click_link "Groups"
+  end
 end

spec/features/groups/members/master_manages_access_requests_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'Groups > Members > Maintainer manages access requests' do
   it_behaves_like 'Maintainer manages access requests' do
+    let(:has_tabs) { true }
     let(:entity) { create(:group, :public) }
     let(:members_page_path) { group_group_members_path(entity) }
   end

spec/features/groups/members/tabs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Groups > Members > Tabs' do
+  using RSpec::Parameterized::TableSyntax
+
+  shared_examples 'active "Members" tab' do
+    it 'displays "Members" tab' do
+      expect(page).to have_selector('.nav-link.active', text: 'Members')
+    end
+  end
+
+  shared_examples 'active "Pending" tab' do
+    it 'displays "Pending" tab' do
+      expect(page).to have_selector('.nav-link.active', text: 'Pending')
+    end
+  end
+
+  let(:owner) { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    stub_const('Groups::GroupMembersController::MEMBER_PER_PAGE_LIMIT', 1)
+    allow_any_instance_of(Member).to receive(:send_request).and_return(true)
+
+    group.add_owner(owner)
+    sign_in(owner)
+
+    create_list(:group_member, 2, group: group)
+    create_list(:group_member, 2, :invited, group: group)
+    create_list(:group_group_link, 2, shared_group: group)
+    create_list(:group_member, 2, :access_request, group: group)
+  end
+
+  where(:tab, :count) do
+    'Members'         | 3
+    'Pending'         | 2
+    'Groups'          | 2
+    'Access requests' | 2
+  end
+
+  with_them do
+    it "renders #{params[:tab]} tab" do
+      visit group_group_members_path(group)
+
+      expect(page).to have_selector('.nav-link', text: "#{tab} #{count}")
+    end
+  end
+
+  context 'displays "Members" tab by default' do
+    before do
+      visit group_group_members_path(group)
+    end
+
+    it_behaves_like 'active "Members" tab'
+  end
+
+  context 'when searching "Pending"', :js do
+    before do
+      visit group_group_members_path(group)
+
+      click_link 'Pending'
+
+      page.within '.user-search-form' do
+        fill_in 'search_invited', with: 'email'
+        find('button[type="submit"]').click
+      end
+    end
+
+    it_behaves_like 'active "Pending" tab'
+
+    context 'and then searching "Members"' do
+      before do
+        click_link 'Members'
+
+        page.within '.user-search-form' do
+          fill_in 'search', with: 'test'
+          find('button[type="submit"]').click
+        end
+      end
+
+      it_behaves_like 'active "Members" tab'
+    end
+  end
+
+  context 'when using "Pending" pagination', :js do
+    before do
+      visit group_group_members_path(group)
+
+      click_link 'Pending'
+
+      page.within '.pagination' do
+        click_link '2'
+      end
+    end
+
+    it_behaves_like 'active "Pending" tab'
+
+    context 'and then using "Members" pagination' do
+      before do
+        click_link 'Members'
+
+        page.within '.pagination' do
+          click_link '2'
+        end
+      end
+
+      it_behaves_like 'active "Members" tab'
+    end
+  end
+end

spec/features/projects/members/master_manages_access_requests_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'Projects > Members > Maintainer manages access requests' do
   it_behaves_like 'Maintainer manages access requests' do
+    let(:has_tabs) { false }
     let(:entity) { create(:project, :public) }
     let(:members_page_path) { project_project_members_path(entity) }
   end

spec/support/shared_examples/features/master_manages_access_requests_shared_example.rb

@@ -8,17 +8,18 @@ RSpec.shared_examples 'Maintainer manages access requests' do
     entity.request_access(user)
     entity.respond_to?(:add_owner) ? entity.add_owner(maintainer) : entity.add_maintainer(maintainer)
     sign_in(maintainer)
-  end
-
-  it 'maintainer can see access requests' do
     visit members_page_path
 
+    if has_tabs
+      click_on 'Access requests'
+    end
+  end
+
+  it 'maintainer can see access requests', :js do
     expect_visible_access_request(entity, user)
   end
 
   it 'maintainer can grant access', :js do
-    visit members_page_path
-
     expect_visible_access_request(entity, user)
 
     click_on 'Grant access'
@@ -31,8 +32,6 @@ RSpec.shared_examples 'Maintainer manages access requests' do
   end
 
   it 'maintainer can deny access', :js do
-    visit members_page_path
-
     expect_visible_access_request(entity, user)
 
     # Open modal
@@ -47,7 +46,13 @@ RSpec.shared_examples 'Maintainer manages access requests' do
   end
 
   def expect_visible_access_request(entity, user)
-    expect(page).to have_content "Users requesting access to #{entity.name} 1"
+    if has_tabs
+      expect(page).to have_content "Access requests 1"
+      expect(page).to have_content "Users requesting access to #{entity.name}"
+    else
+      expect(page).to have_content "Users requesting access to #{entity.name} 1"
+    end
+
     expect(page).to have_content user.name
   end