Merge branch '289911-feature-flag-rollout-of-group_members_filtered_search' into 'master'

Remove `group_members_filtered_search` FF

See merge request gitlab-org/gitlab!51406

app/assets/javascripts/groups/members/components/app.vue

@@ -5,12 +5,10 @@ import MembersTable from '~/members/components/table/members_table.vue';
 import FilterSortContainer from '~/members/components/filter_sort/filter_sort_container.vue';
 import { scrollToElement } from '~/lib/utils/common_utils';
 import { HIDE_ERROR } from '~/members/store/mutation_types';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
   name: 'GroupMembersApp',
   components: { MembersTable, FilterSortContainer, GlAlert },
-  mixins: [glFeatureFlagsMixin()],
   computed: {
     ...mapState(['showError', 'errorMessage']),
   },
@@ -36,7 +34,7 @@ export default {
     <gl-alert v-if="showError" ref="errorAlert" variant="danger" @dismiss="hideError">{{
       errorMessage
     }}</gl-alert>
-    <filter-sort-container v-if="glFeatures.groupMembersFilteredSearch" />
+    <filter-sort-container />
     <members-table />
   </div>
 </template>

app/controllers/groups/group_members_controller.rb

@@ -14,10 +14,6 @@ class Groups::GroupMembersController < Groups::ApplicationController
   # Authorize
   before_action :authorize_admin_group_member!, except: admin_not_required_endpoints
 
-  before_action do
-    push_frontend_feature_flag(:group_members_filtered_search, @group, default_enabled: true)
-  end
-
   skip_before_action :check_two_factor_requirement, only: :leave
   skip_cross_project_access_check :index, :create, :update, :destroy, :request_access,
                                   :approve_access_request, :leave, :resend_invite,

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

@@ -3,8 +3,6 @@
 - show_invited_members = can_manage_members && @invited_members.exists?
 - show_access_requests = can_manage_members && @requesters.exists?
 - invited_active = params[:search_invited].present? || params[:invited_members_page].present?
-- filtered_search_enabled = Feature.enabled?(:group_members_filtered_search, @group, default_enabled: true)
-- form_item_label_css_class = 'label-bold gl-mr-2 gl-mb-0 gl-py-2 align-self-md-center'
 
 .js-remove-member-modal
 .project-members-page.gl-mt-3
@@ -51,52 +49,23 @@
           %span.badge.badge-pill= @requesters.count
   .tab-content
     #tab-members.tab-pane{ class: ('active' unless invited_active) }
-      - unless filtered_search_enabled
-        = render 'shared/members/tab_pane/header' do
-          = render 'shared/members/tab_pane/title' do
-            = 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', data: { testid: 'user-search-form' } do
-            .gl-px-3.gl-py-2
-              .search-control-wrap.gl-relative
-                = render 'shared/members/search_field'
-            - if can_manage_members
-              = render 'shared/members/tab_pane/form_item' do
-                = label_tag '2fa', _('2FA'), class: form_item_label_css_class
-                = render 'shared/members/filter_2fa_dropdown'
-            = render 'shared/members/tab_pane/form_item' do
-              = label_tag :sort_by, _('Sort by'), class: form_item_label_css_class
-              = render 'shared/members/sort_dropdown'
       .js-group-members-list{ data: group_members_list_data_attributes(@group, @members) }
         .loading
           .spinner.spinner-md
       = paginate @members, theme: 'gitlab', params: { invited_members_page: nil, search_invited: nil }
     - if @group.shared_with_group_links.any?
       #tab-groups.tab-pane
-        - unless filtered_search_enabled
-          = render 'shared/members/tab_pane/header' do
-            = render 'shared/members/tab_pane/title' do
-              = 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 }
         .js-group-linked-list{ data: linked_groups_list_data_attributes(@group) }
           .loading
             .spinner.spinner-md
     - if show_invited_members
       #tab-invited-members.tab-pane{ class: ('active' if invited_active) }
-        - unless filtered_search_enabled
-          = render 'shared/members/tab_pane/header' do
-            = render 'shared/members/tab_pane/title' do
-              = html_escape(_('Members invited 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', data: { testid: 'user-search-form' } do
-              = render 'shared/members/search_field', name: 'search_invited'
         .js-group-invited-members-list{ data: group_members_list_data_attributes(@group, @invited_members) }
           .loading
             .spinner.spinner-md
         = paginate @invited_members, param_name: 'invited_members_page', theme: 'gitlab', params: { page: nil }
     - if show_access_requests
       #tab-access-requests.tab-pane
-        - unless filtered_search_enabled
-          = render 'shared/members/tab_pane/header' do
-            = render 'shared/members/tab_pane/title' do
-              = 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 }
         .js-group-access-requests-list{ data: group_members_list_data_attributes(@group, @requesters) }
           .loading
             .spinner.spinner-md

config/feature_flags/development/group_members_filtered_search.yml

----
-name: group_members_filtered_search
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48272
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/289911
-milestone: '13.7'
-type: development
-group: group::access
-default_enabled: true

doc/user/group/index.md

@@ -215,10 +215,7 @@ To remove a member from a group:
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21727) in GitLab 12.6.
 > - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/228675) in GitLab 13.7.
-> - Improvements are [deployed behind a feature flag](../feature_flags.md), enabled by default.
-> - Improvements are enabled on GitLab.com.
-> - Improvements are recommended for production use.
-> - For GitLab self-managed instances, GitLab administrators can opt to [disable improvements](#enable-or-disable-improvements-to-the-ability-to-filter-and-sort-group-members). **(CORE ONLY)**
+> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/289911) in GitLab 13.8.
 
 The following sections illustrate how you can filter and sort members in a group. To view these options,
 navigate to your desired group, go to **Members**, and include the noted search terms.
@@ -269,30 +266,6 @@ You can sort members by **Account**, **Access granted**, **Max role**, or **Last
 
 ![Group members sort](img/group_members_sort_13_7.png)
 
-### Enable or disable improvements to the ability to filter and sort group members **(CORE ONLY)**
-
-Group member filtering and sorting improvements are deployed behind a feature flag that is **enabled by default**.
-[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
-can opt to disable the improvements.
-
-To disable them:
-
-```ruby
-# For the instance
-Feature.disable(:group_members_filtered_search)
-# For a single group
-Feature.disable(:group_members_filtered_search, Group.find(<group id>))
-```
-
-To enable them:
-
-```ruby
-# For the instance
-Feature.enable(:group_members_filtered_search)
-# For a single group
-Feature.enable(:group_members_filtered_search, Group.find(<group id>))
-```
-
 ## Changing the default branch protection of a group
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7583) in GitLab 12.9.

locale/gitlab.pot

@@ -14114,9 +14114,6 @@ msgstr ""
 msgid "Groups with access to %{strong_open}%{project_name}%{strong_close}"
 msgstr ""
 
-msgid "Groups with access to %{strong_start}%{group_name}%{strong_end}"
-msgstr ""
-
 msgid "GroupsDropdown|Frequently visited"
 msgstr ""
 
@@ -17367,9 +17364,6 @@ msgstr ""
 msgid "Members can be added by project %{i_open}Maintainers%{i_close} or %{i_open}Owners%{i_close}"
 msgstr ""
 
-msgid "Members invited to %{strong_start}%{group_name}%{strong_end}"
-msgstr ""
-
 msgid "Members invited to %{strong_start}%{project_name}%{strong_end}"
 msgstr ""
 
@@ -17388,9 +17382,6 @@ msgstr ""
 msgid "Members of a group may only view projects they have permission to access"
 msgstr ""
 
-msgid "Members with access to %{strong_start}%{group_name}%{strong_end}"
-msgstr ""
-
 msgid "Members|%{time} by %{user}"
 msgstr ""
 
@@ -30911,9 +30902,6 @@ msgstr ""
 msgid "Users requesting access to"
 msgstr ""
 
-msgid "Users requesting access to %{strong_start}%{group_name}%{strong_end}"
-msgstr ""
-
 msgid "Users requesting access to %{strong_start}%{project_name}%{strong_end}"
 msgstr ""
 

spec/features/groups/members/sort_members_spec.rb

@@ -16,174 +16,92 @@ RSpec.describe 'Groups > Members > Sort members', :js do
     sign_in(owner)
   end
 
-  context 'when `group_members_filtered_search` feature flag is enabled' do
-    def expect_sort_by(text, sort_direction)
-      within('[data-testid="members-sort-dropdown"]') do
-        expect(page).to have_css('button[aria-haspopup="true"]', text: text)
-        expect(page).to have_button("Sorting Direction: #{sort_direction == :asc ? 'Ascending' : 'Descending'}")
-      end
-    end
-
-    it 'sorts by account by default' do
-      visit_members_list(sort: nil)
-
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-
-      expect_sort_by('Account', :asc)
-    end
-
-    it 'sorts by max role ascending' do
-      visit_members_list(sort: :access_level_asc)
-
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-
-      expect_sort_by('Max role', :asc)
-    end
-
-    it 'sorts by max role descending' do
-      visit_members_list(sort: :access_level_desc)
-
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-
-      expect_sort_by('Max role', :desc)
-    end
-
-    it 'sorts by access granted ascending' do
-      visit_members_list(sort: :last_joined)
-
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-
-      expect_sort_by('Access granted', :asc)
-    end
-
-    it 'sorts by access granted descending' do
-      visit_members_list(sort: :oldest_joined)
-
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-
-      expect_sort_by('Access granted', :desc)
-    end
-
-    it 'sorts by account ascending' do
-      visit_members_list(sort: :name_asc)
-
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-
-      expect_sort_by('Account', :asc)
+  def expect_sort_by(text, sort_direction)
+    within('[data-testid="members-sort-dropdown"]') do
+      expect(page).to have_css('button[aria-haspopup="true"]', text: text)
+      expect(page).to have_button("Sorting Direction: #{sort_direction == :asc ? 'Ascending' : 'Descending'}")
     end
+  end
 
-    it 'sorts by account descending' do
-      visit_members_list(sort: :name_desc)
+  it 'sorts by account by default' do
+    visit_members_list(sort: nil)
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
+    expect(first_row.text).to include(owner.name)
+    expect(second_row.text).to include(developer.name)
 
-      expect_sort_by('Account', :desc)
-    end
+    expect_sort_by('Account', :asc)
+  end
 
-    it 'sorts by last sign-in ascending', :clean_gitlab_redis_shared_state do
-      visit_members_list(sort: :recent_sign_in)
+  it 'sorts by max role ascending' do
+    visit_members_list(sort: :access_level_asc)
 
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
+    expect(first_row.text).to include(developer.name)
+    expect(second_row.text).to include(owner.name)
 
-      expect_sort_by('Last sign-in', :asc)
-    end
+    expect_sort_by('Max role', :asc)
+  end
 
-    it 'sorts by last sign-in descending', :clean_gitlab_redis_shared_state do
-      visit_members_list(sort: :oldest_sign_in)
+  it 'sorts by max role descending' do
+    visit_members_list(sort: :access_level_desc)
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
+    expect(first_row.text).to include(owner.name)
+    expect(second_row.text).to include(developer.name)
 
-      expect_sort_by('Last sign-in', :desc)
-    end
+    expect_sort_by('Max role', :desc)
   end
 
-  context 'when `group_members_filtered_search` feature flag is disabled' do
-    dropdown_toggle_selector = '[data-testid="user-sort-dropdown"] [data-testid="dropdown-toggle"]'
+  it 'sorts by access granted ascending' do
+    visit_members_list(sort: :last_joined)
 
-    before do
-      stub_feature_flags(group_members_filtered_search: false)
-    end
-
-    it 'sorts alphabetically by default' do
-      visit_members_list(sort: nil)
-
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Name, ascending')
-    end
+    expect(first_row.text).to include(developer.name)
+    expect(second_row.text).to include(owner.name)
 
-    it 'sorts by access level ascending' do
-      visit_members_list(sort: :access_level_asc)
+    expect_sort_by('Access granted', :asc)
+  end
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Access level, ascending')
-    end
+  it 'sorts by access granted descending' do
+    visit_members_list(sort: :oldest_joined)
 
-    it 'sorts by access level descending' do
-      visit_members_list(sort: :access_level_desc)
+    expect(first_row.text).to include(owner.name)
+    expect(second_row.text).to include(developer.name)
 
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Access level, descending')
-    end
+    expect_sort_by('Access granted', :desc)
+  end
 
-    it 'sorts by last joined' do
-      visit_members_list(sort: :last_joined)
+  it 'sorts by account ascending' do
+    visit_members_list(sort: :name_asc)
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Last joined')
-    end
+    expect(first_row.text).to include(owner.name)
+    expect(second_row.text).to include(developer.name)
 
-    it 'sorts by oldest joined' do
-      visit_members_list(sort: :oldest_joined)
+    expect_sort_by('Account', :asc)
+  end
 
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Oldest joined')
-    end
+  it 'sorts by account descending' do
+    visit_members_list(sort: :name_desc)
 
-    it 'sorts by name ascending' do
-      visit_members_list(sort: :name_asc)
+    expect(first_row.text).to include(developer.name)
+    expect(second_row.text).to include(owner.name)
 
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Name, ascending')
-    end
+    expect_sort_by('Account', :desc)
+  end
 
-    it 'sorts by name descending' do
-      visit_members_list(sort: :name_desc)
+  it 'sorts by last sign-in ascending', :clean_gitlab_redis_shared_state do
+    visit_members_list(sort: :recent_sign_in)
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Name, descending')
-    end
+    expect(first_row.text).to include(owner.name)
+    expect(second_row.text).to include(developer.name)
 
-    it 'sorts by recent sign in', :clean_gitlab_redis_shared_state do
-      visit_members_list(sort: :recent_sign_in)
+    expect_sort_by('Last sign-in', :asc)
+  end
 
-      expect(first_row.text).to include(owner.name)
-      expect(second_row.text).to include(developer.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Recent sign in')
-    end
+  it 'sorts by last sign-in descending', :clean_gitlab_redis_shared_state do
+    visit_members_list(sort: :oldest_sign_in)
 
-    it 'sorts by oldest sign in', :clean_gitlab_redis_shared_state do
-      visit_members_list(sort: :oldest_sign_in)
+    expect(first_row.text).to include(developer.name)
+    expect(second_row.text).to include(owner.name)
 
-      expect(first_row.text).to include(developer.name)
-      expect(second_row.text).to include(owner.name)
-      expect(page).to have_css(dropdown_toggle_selector, text: 'Oldest sign in')
-    end
+    expect_sort_by('Last sign-in', :desc)
   end
 
   def visit_members_list(sort:)

spec/frontend/groups/members/components/app_spec.js

@@ -87,21 +87,9 @@ describe('GroupMembersApp', () => {
     });
   });
 
-  describe.each`
-    featureFlagValue | exists
-    ${true}          | ${true}
-    ${false}         | ${false}
-  `(
-    'when `group_members_filtered_search` feature flag is $featureFlagValue',
-    ({ featureFlagValue, exists }) => {
-      it(`${exists ? 'renders' : 'does not render'} FilterSortContainer`, () => {
-        createComponent(
-          {},
-          { provide: { glFeatures: { groupMembersFilteredSearch: featureFlagValue } } },
-        );
-
-        expect(findFilterSortContainer().exists()).toBe(exists);
-      });
-    },
-  );
+  it('renders `FilterSortContainer`', () => {
+    createComponent();
+
+    expect(findFilterSortContainer().exists()).toBe(true);
+  });
 });