diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index d54d264bc5d2cd662e97100693d0389c53acc057..45408c9ab3cef8dcb398bad84aa5dbdb721816bf 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -67,7 +67,7 @@ position: absolute; right: 4px; top: 0; - height: 35px; + height: $input-height; padding-left: 10px; padding-right: 10px; color: $gray-darkest; diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 5144640f6fb87faae30dbddbaabd4eb055cbc0fc..1b1416a72d77e8d19c58f2f81edd7684e5d008b5 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -5,6 +5,8 @@ class Groups::GroupMembersController < Groups::ApplicationController include MembersPresentation include SortingHelper + MEMBER_PER_PAGE_LIMIT = 50 + def self.admin_not_required_endpoints %i[index leave request_access] end @@ -24,7 +26,14 @@ def index @project = @group.projects.find(params[:project_id]) if params[:project_id] @members = GroupMembersFinder.new(@group).execute - @members = @members.non_invite unless can_manage_members + + if can_manage_members + @invited_members = @members.invite + @invited_members = @invited_members.search_invite_email(params[:search_invited]) if params[:search_invited].present? + @invited_members = present_members(@invited_members.page(params[:invited_members_page]).per(MEMBER_PER_PAGE_LIMIT)) + end + + @members = @members.non_invite @members = @members.search(params[:search]) if params[:search].present? @members = @members.sort_by_attribute(@sort) @@ -32,7 +41,7 @@ def index @members = @members.filter_by_2fa(params[:two_factor]) end - @members = @members.page(params[:page]).per(50) + @members = @members.page(params[:page]).per(MEMBER_PER_PAGE_LIMIT) @members = present_members(@members) @requesters = present_members( diff --git a/app/models/member.rb b/app/models/member.rb index 695a28d37b571e8a4b907177481fd8651fd8d467..d503490d881c11cca09e8a09d82a1ffd2619bd8c 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -107,6 +107,10 @@ def search(query) joins(:user).merge(User.search(query)) end + def search_invite_email(query) + invite.where(['invite_email ILIKE ?', "%#{query}%"]) + end + def filter_by_2fa(value) case value when 'enabled' diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 2b8c9f65d43eccd1ea525c5b56bd89a9921db338..882fcc794217298b7a0673d7dc0bfae2d5735437 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -1,39 +1,67 @@ -- page_title "Members" +- page_title _("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? .project-members-page.prepend-top-default %h4 - Members + = _("Members") %hr - if can_manage_members .project-members-new.append-bottom-default %p.clearfix - Add new member to - %strong= @group.name + = _("Add new member to %{strong_start}%{group_name}%{strong_end}").html_safe % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe } = render "new_group_member" = render 'shared/members/requests', membership_source: @group, requesters: @requesters = render_if_exists 'groups/group_members/ldap_sync' - .clearfix - %h5.member.existing-title - Existing members - .card - .card-header.flex-project-members-panel - %span.flex-project-title - Members with access to - %strong= @group.name - %span.badge.badge-pill= @members.total_count - = form_tag group_group_members_path(@group), method: :get, class: 'form-inline user-search-form flex-users-form' do - .form-group - .position-relative.append-right-8 - = 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") - - if can_manage_members - = render 'shared/members/filter_2fa_dropdown' - = render 'shared/members/sort_dropdown' - %ul.content-list.members-list - = render partial: 'shared/members/member', collection: @members, as: :member - = paginate @members, theme: 'gitlab' + %ul.nav-links.mobile-separator.nav.nav-tabs.clearfix + %li.nav-item + = link_to "#existing_members", class: ["nav-link", ("active" unless pending_active)] , 'data-toggle' => 'tab' do + %span + = _("Existing") + %span.badge.badge-pill= @members.total_count + - if show_invited_members + %li.nav-item + = link_to "#invited_members", class: ["nav-link", ("active" if pending_active)], 'data-toggle' => 'tab' do + %span + = _("Pending") + %span.badge.badge-pill= @invited_members.total_count + + .tab-content + #existing_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 + = 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' + = render 'shared/members/filter_2fa_dropdown' + = render 'shared/members/sort_dropdown' + %ul.content-list.members-list + = render partial: 'shared/members/member', collection: @members, as: :member + = paginate @members, theme: 'gitlab' + + - if show_invited_members + #invited_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") + %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' diff --git a/app/views/shared/members/_filter_2fa_dropdown.html.haml b/app/views/shared/members/_filter_2fa_dropdown.html.haml index 3e98587aeaa5dd31f20d06da001d994db464cc8e..44ea844028ed300a120a52681a0933c30a0280d9 100644 --- a/app/views/shared/members/_filter_2fa_dropdown.html.haml +++ b/app/views/shared/members/_filter_2fa_dropdown.html.haml @@ -1,7 +1,7 @@ - filter = params[:two_factor] || 'everyone' - filter_options = { 'everyone' => _('Everyone'), 'enabled' => _('Enabled'), 'disabled' => _('Disabled') } -.dropdown.inline.member-filter-2fa-dropdown - = dropdown_toggle('2FA: ' + filter_options[filter], { toggle: 'dropdown' }) +.dropdown.inline.member-filter-2fa-dropdown.pr-md-2 + = dropdown_toggle(filter_options[filter], { toggle: 'dropdown' }) %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable %li.dropdown-header = _("Filter by two-factor authentication") diff --git a/app/views/shared/members/_sort_dropdown.html.haml b/app/views/shared/members/_sort_dropdown.html.haml index f5ebab035dbe490a68052649c282042011fbaf15..feca109dadeee1beb1b9a1cc3a0718238ba60757 100644 --- a/app/views/shared/members/_sort_dropdown.html.haml +++ b/app/views/shared/members/_sort_dropdown.html.haml @@ -1,4 +1,5 @@ -.dropdown.inline.user-sort-dropdown += label_tag :sort_by, 'Sort by', class: 'col-form-label label-bold pr-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 %li.dropdown-header diff --git a/changelogs/unreleased/sh-break-out-invited-group-members.yml b/changelogs/unreleased/sh-break-out-invited-group-members.yml new file mode 100644 index 0000000000000000000000000000000000000000..091f1d48843d41d51998854d2fa5cb04991e3a9e --- /dev/null +++ b/changelogs/unreleased/sh-break-out-invited-group-members.yml @@ -0,0 +1,5 @@ +--- +title: Make it easier to find invited group members +merge_request: 28436 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5516306a46b5b8edfec7c696d112e3dfe37b1ef1..d15f3b156f28986574b4fd8c9d7e6dfe0eed9045 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -896,6 +896,9 @@ msgstr "" msgid "Add new directory" msgstr "" +msgid "Add new member to %{strong_start}%{group_name}%{strong_end}" +msgstr "" + msgid "Add or subtract spent time" msgstr "" @@ -5867,6 +5870,9 @@ msgstr "" msgid "Except policy:" msgstr "" +msgid "Existing" +msgstr "" + msgid "Existing members and groups" msgstr "" @@ -8937,6 +8943,12 @@ msgstr "" msgid "Members of <strong>%{project_name}</strong>" msgstr "" +msgid "Members with access to %{strong_start}%{group_name}%{strong_end}" +msgstr "" + +msgid "Members with pending access to %{strong_start}%{group_name}%{strong_end}" +msgstr "" + msgid "Merge" msgstr "" diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 413598ddde02d08dd9cbdb7c896e660d2ab82404..908c564e7617c818731a6b9deb16f4cd48b0dd8f 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -16,6 +16,39 @@ expect(response).to have_gitlab_http_status(200) expect(response).to render_template(:index) end + + context 'user with owner access' do + let!(:invited) { create_list(:group_member, 3, :invited, group: group) } + + before do + group.add_owner(user) + sign_in(user) + end + + it 'assigns invited members' do + get :index, params: { group_id: group } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.map(&:invite_email)) + end + + it 'restricts search to one email' do + get :index, params: { group_id: group, search_invited: invited.first.invite_email } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.first.invite_email) + end + + it 'paginates invited list' do + stub_const('Groups::GroupMembersController::MEMBER_PER_PAGE_LIMIT', 2) + + get :index, params: { group_id: group, invited_members_page: 1 } + + expect(assigns(:invited_members).count).to eq(2) + + get :index, params: { group_id: group, invited_members_page: 2 } + + expect(assigns(:invited_members).count).to eq(1) + end + end end describe 'POST create' do diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 3bf9cdef2535b7cee31199378d45fc107716848d..8dab6c71b06a34d06c5709a624097a6eb1909b79 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -16,7 +16,9 @@ trait(:invited) do user_id nil invite_token 'xxx' - invite_email 'email@email.com' + sequence :invite_email do |n| + "email#{n}@email.com" + end end end end diff --git a/spec/features/groups/members/filter_members_spec.rb b/spec/features/groups/members/filter_members_spec.rb index 7aad66ec08ffcce74cf785592edd0e1ce048df1c..fc62c92db4ef11dad1d361e1be1bc2cf584e8900 100644 --- a/spec/features/groups/members/filter_members_spec.rb +++ b/spec/features/groups/members/filter_members_spec.rb @@ -19,7 +19,7 @@ expect(first_member).to include(user.name) expect(second_member).to include(user_with_2fa.name) - expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: '2FA: Everyone') + expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Everyone') end it 'shows only 2FA members' do @@ -27,7 +27,7 @@ expect(first_member).to include(user_with_2fa.name) expect(members_list.size).to eq(1) - expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: '2FA: Enabled') + expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Enabled') end it 'shows only non 2FA members' do @@ -35,7 +35,7 @@ expect(first_member).to include(user.name) expect(members_list.size).to eq(1) - expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: '2FA: Disabled') + expect(page).to have_css('.member-filter-2fa-dropdown .dropdown-toggle-text', text: 'Disabled') end def visit_members_list(options = {}) diff --git a/spec/features/groups/members/manage_members_spec.rb b/spec/features/groups/members/manage_members_spec.rb index 779fa74501a4b4c1fd7b29b02e982f08be5f5eda..cdd16ae9441ba08e0f1cad59330ed6e848734dbd 100644 --- a/spec/features/groups/members/manage_members_spec.rb +++ b/spec/features/groups/members/manage_members_spec.rb @@ -98,7 +98,9 @@ add_user('test@example.com', 'Reporter') - page.within(second_row) do + click_link('Pending') + + page.within('.content-list.members-list') do expect(page).to have_content('test@example.com') expect(page).to have_content('Invited') expect(page).to have_button('Reporter') diff --git a/spec/features/groups/members/sort_members_spec.rb b/spec/features/groups/members/sort_members_spec.rb index 48b0136227ea466881ba7e54f3a0211e0b5f9155..76709199942824c01261c2b5319f25be1238faf8 100644 --- a/spec/features/groups/members/sort_members_spec.rb +++ b/spec/features/groups/members/sort_members_spec.rb @@ -19,7 +19,7 @@ expect(first_member).to include(owner.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') end it 'sorts by access level ascending' do @@ -27,7 +27,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(owner.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Access level, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Access level, ascending') end it 'sorts by access level descending' do @@ -35,7 +35,7 @@ expect(first_member).to include(owner.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Access level, descending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Access level, descending') end it 'sorts by last joined' do @@ -43,7 +43,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(owner.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Last joined') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Last joined') end it 'sorts by oldest joined' do @@ -51,7 +51,7 @@ expect(first_member).to include(owner.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined') end it 'sorts by name ascending' do @@ -59,7 +59,7 @@ expect(first_member).to include(owner.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') end it 'sorts by name descending' do @@ -67,7 +67,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(owner.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') end it 'sorts by recent sign in', :clean_gitlab_redis_shared_state do @@ -75,7 +75,7 @@ expect(first_member).to include(owner.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') end it 'sorts by oldest sign in', :clean_gitlab_redis_shared_state do @@ -83,7 +83,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(owner.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Oldest sign in') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Oldest sign in') end def visit_members_list(sort:) diff --git a/spec/features/projects/members/sorting_spec.rb b/spec/features/projects/members/sorting_spec.rb index 88240fbbedc2af1ad20ec6e7216bbd8d4ea3c3dd..12f485317d809c82ebd35f8f002e9bea3ece4ba7 100644 --- a/spec/features/projects/members/sorting_spec.rb +++ b/spec/features/projects/members/sorting_spec.rb @@ -18,7 +18,7 @@ expect(first_member).to include(maintainer.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') end it 'sorts by access level ascending' do @@ -26,7 +26,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(maintainer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Access level, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Access level, ascending') end it 'sorts by access level descending' do @@ -34,7 +34,7 @@ expect(first_member).to include(maintainer.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Access level, descending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Access level, descending') end it 'sorts by last joined' do @@ -42,7 +42,7 @@ expect(first_member).to include(maintainer.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Last joined') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Last joined') end it 'sorts by oldest joined' do @@ -50,7 +50,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(maintainer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Oldest joined') end it 'sorts by name ascending' do @@ -58,7 +58,7 @@ expect(first_member).to include(maintainer.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, ascending') end it 'sorts by name descending' do @@ -66,7 +66,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(maintainer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') end it 'sorts by recent sign in', :clean_gitlab_redis_shared_state do @@ -74,7 +74,7 @@ expect(first_member).to include(maintainer.name) expect(second_member).to include(developer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') end it 'sorts by oldest sign in', :clean_gitlab_redis_shared_state do @@ -82,7 +82,7 @@ expect(first_member).to include(developer.name) expect(second_member).to include(maintainer.name) - expect(page).to have_css('.user-sort-dropdown .dropdown-toggle-text', text: 'Oldest sign in') + expect(page).to have_css('.qa-user-sort-dropdown .dropdown-toggle-text', text: 'Oldest sign in') end def visit_members_list(sort:) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 782a84f922b386601852b9df0429ed24685139e2..2cb4f222ea4399f20c847dcc21f682255d7f3d76 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -172,6 +172,19 @@ it { expect(described_class.non_request).to include @accepted_request_member } end + describe '.search_invite_email' do + it 'returns only members the matching e-mail' do + create(:group_member, :invited) + + invited = described_class.search_invite_email(@invited_member.invite_email) + + expect(invited.count).to eq(1) + expect(invited.first).to eq(@invited_member) + + expect(described_class.search_invite_email('bad-email@example.com').count).to eq(0) + end + end + describe '.developers' do subject { described_class.developers.to_a }