From d789201f150ac9fbcde2b3faa5f53a76d3ae59f4 Mon Sep 17 00:00:00 2001 From: Bogdan Denkovych Date: Thu, 15 May 2025 17:28:18 +0300 Subject: [PATCH] Clean up `new_implementation_of_invite_members_search` FF and old code Related to https://gitlab.com/gitlab-org/gitlab/-/issues/540306 Changelog: changed --- .../components/invite_members_modal.vue | 13 ---- .../components/members_token_select.vue | 30 +------- .../javascripts/invite_members/constants.js | 2 - .../init_invite_members_modal.js | 2 - app/helpers/invite_members_helper.rb | 5 -- .../groups/_invite_members_modal.html.haml | 2 +- .../projects/_invite_members_modal.html.haml | 2 +- ...mplementation_of_invite_members_search.yml | 10 --- ee/app/helpers/ee/invite_members_helper.rb | 9 --- .../groups/members/list_members_spec.rb | 25 ------- .../helpers/ee/invite_members_helper_spec.rb | 45 ------------ lib/gitlab/gon_helper.rb | 1 - .../components/members_token_select_spec.js | 70 +++---------------- 13 files changed, 11 insertions(+), 205 deletions(-) delete mode 100644 config/feature_flags/gitlab_com_derisk/new_implementation_of_invite_members_search.yml diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue index a96646057030af..2dc72f3bf7d74d 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -16,7 +16,6 @@ import { BLOCKED_SEAT_OVERAGES_BODY, BLOCKED_SEAT_OVERAGES_CTA, BLOCKED_SEAT_OVERAGES_CTA_DOCS, - USERS_FILTER_ALL, MEMBER_MODAL_LABELS, INVITE_MEMBER_MODAL_TRACKING_CATEGORY, } from '../constants'; @@ -91,16 +90,6 @@ export default { type: String, required: true, }, - usersFilter: { - type: String, - required: false, - default: USERS_FILTER_ALL, - }, - filterId: { - type: Number, - required: false, - default: null, - }, fullPath: { type: String, required: true, @@ -495,8 +484,6 @@ export default { aria-labelledby="empty-invites-alert" :input-id="inputId" :exception-state="exceptionState" - :users-filter="usersFilter" - :filter-id="filterId" :users-with-warning="usersWithWarning" :invalid-members="invalidMembers" @clear="clearValidation" diff --git a/app/assets/javascripts/invite_members/components/members_token_select.vue b/app/assets/javascripts/invite_members/components/members_token_select.vue index 9adc67e22f598d..cb6a2aac8c2f75 100644 --- a/app/assets/javascripts/invite_members/components/members_token_select.vue +++ b/app/assets/javascripts/invite_members/components/members_token_select.vue @@ -2,14 +2,10 @@ import { GlTokenSelector, GlAvatar, GlAvatarLabeled, GlIcon, GlSprintf } from '@gitlab/ui'; import { debounce, isEmpty } from 'lodash'; import { __ } from '~/locale'; -import { getUsers } from '~/rest_api'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { memberName, searchUsers } from '../utils/member_utils'; import { SEARCH_DELAY, - USERS_FILTER_ALL, - USERS_FILTER_SAML_PROVIDER_ID, VALID_TOKEN_BACKGROUND, WARNING_TOKEN_BACKGROUND, INVALID_TOKEN_BACKGROUND, @@ -23,7 +19,6 @@ export default { GlIcon, GlSprintf, }, - mixins: [glFeatureFlagsMixin()], inject: ['searchUrl'], props: { placeholder: { @@ -40,16 +35,6 @@ export default { required: false, default: false, }, - usersFilter: { - type: String, - required: false, - default: USERS_FILTER_ALL, - }, - filterId: { - type: Number, - required: false, - default: null, - }, usersWithWarning: { type: Object, required: false, @@ -86,15 +71,6 @@ export default { } return ''; }, - queryOptions() { - if (this.usersFilter === USERS_FILTER_SAML_PROVIDER_ID) { - return { - saml_provider_id: this.filterId, - ...this.$options.defaultQueryOptions, - }; - } - return this.$options.defaultQueryOptions; - }, hasErrorOrWarning() { return !isEmpty(this.invalidMembers) || !isEmpty(this.usersWithWarning); }, @@ -136,10 +112,7 @@ export default { })); }, retrieveUsersRequest() { - if (this.glFeatures.newImplementationOfInviteMembersSearch) { - return searchUsers(this.searchUrl, this.query); - } - return getUsers(this.query, this.queryOptions); + return searchUsers(this.searchUrl, this.query); }, retrieveUsers: debounce(async function debouncedRetrieveUsers() { try { @@ -198,7 +171,6 @@ export default { return Object.prototype.hasOwnProperty.call(this.invalidMembers, memberName(token)); }, }, - defaultQueryOptions: { without_project_bots: true, active: true }, i18n: { inviteTextMessage: __('Invite "%{email}" by email'), }, diff --git a/app/assets/javascripts/invite_members/constants.js b/app/assets/javascripts/invite_members/constants.js index 6d4f410cabcbdd..70cf0ef1ec61b4 100644 --- a/app/assets/javascripts/invite_members/constants.js +++ b/app/assets/javascripts/invite_members/constants.js @@ -14,8 +14,6 @@ export const GROUP_FILTERS = { DESCENDANT_GROUPS: 'descendant_groups', }; -export const USERS_FILTER_ALL = 'all'; -export const USERS_FILTER_SAML_PROVIDER_ID = 'saml_provider_id'; export const TRIGGER_ELEMENT_BUTTON = 'button'; export const TOP_NAV_INVITE_MEMBERS_COMPONENT = 'invite_members'; export const TRIGGER_ELEMENT_WITH_EMOJI = 'text-emoji'; diff --git a/app/assets/javascripts/invite_members/init_invite_members_modal.js b/app/assets/javascripts/invite_members/init_invite_members_modal.js index e55d8aa930d0ff..3f7bd18f80d6c8 100644 --- a/app/assets/javascripts/invite_members/init_invite_members_modal.js +++ b/app/assets/javascripts/invite_members/init_invite_members_modal.js @@ -39,8 +39,6 @@ export default (function initInviteMembersModal() { accessLevels: JSON.parse(el.dataset.accessLevels), defaultAccessLevel: parseInt(el.dataset.defaultAccessLevel, 10), defaultMemberRoleId: parseInt(el.dataset.defaultMemberRoleId, 10) || null, - usersFilter: el.dataset.usersFilter, - filterId: parseInt(el.dataset.filterId, 10), usersLimitDataset: convertObjectPropsToCamelCase( JSON.parse(el.dataset.usersLimitDataset || '{}'), ), diff --git a/app/helpers/invite_members_helper.rb b/app/helpers/invite_members_helper.rb index 7852b9b202f3aa..888abb20d2fad5 100644 --- a/app/helpers/invite_members_helper.rb +++ b/app/helpers/invite_members_helper.rb @@ -53,11 +53,6 @@ def group_select_data(source) {} end end - - # Overridden in EE - def users_filter_data(group) - {} - end end InviteMembersHelper.prepend_mod_with('InviteMembersHelper') diff --git a/app/views/groups/_invite_members_modal.html.haml b/app/views/groups/_invite_members_modal.html.haml index 97f64063eff9da..6a5c084a60a3b3 100644 --- a/app/views/groups/_invite_members_modal.html.haml +++ b/app/views/groups/_invite_members_modal.html.haml @@ -4,4 +4,4 @@ access_levels: access_level_roles_user_can_assign(group, group.access_level_roles).to_json, reload_page_on_submit: content_for(:reload_on_member_invite_success).present?.to_s, search_url: invite_search_group_group_members_url(group, format: :json), - help_link: help_page_url('user/permissions.md') }.merge(common_invite_modal_dataset(group)).merge(users_filter_data(group)) } + help_link: help_page_url('user/permissions.md') }.merge(common_invite_modal_dataset(group)) } diff --git a/app/views/projects/_invite_members_modal.html.haml b/app/views/projects/_invite_members_modal.html.haml index 41bf23165522cf..9032a9478edc24 100644 --- a/app/views/projects/_invite_members_modal.html.haml +++ b/app/views/projects/_invite_members_modal.html.haml @@ -4,4 +4,4 @@ access_levels: ProjectMember.permissible_access_level_roles(current_user, project).to_json, reload_page_on_submit: content_for(:reload_on_member_invite_success).present?.to_s, search_url: invite_search_namespace_project_project_members_url(namespace_id: project.namespace, project_id: project, format: :json), - help_link: help_page_url('user/permissions.md') }.merge(common_invite_modal_dataset(project)).merge(users_filter_data(project.group)) } + help_link: help_page_url('user/permissions.md') }.merge(common_invite_modal_dataset(project)) } diff --git a/config/feature_flags/gitlab_com_derisk/new_implementation_of_invite_members_search.yml b/config/feature_flags/gitlab_com_derisk/new_implementation_of_invite_members_search.yml deleted file mode 100644 index f5c92e35477136..00000000000000 --- a/config/feature_flags/gitlab_com_derisk/new_implementation_of_invite_members_search.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -name: new_implementation_of_invite_members_search -description: New implementation of "Invite Members" search -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/460261 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/190070 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/540306 -milestone: '18.1' -group: group::authentication -type: gitlab_com_derisk -default_enabled: false diff --git a/ee/app/helpers/ee/invite_members_helper.rb b/ee/app/helpers/ee/invite_members_helper.rb index 07de221dfd5d7a..5aea8d876e1e3b 100644 --- a/ee/app/helpers/ee/invite_members_helper.rb +++ b/ee/app/helpers/ee/invite_members_helper.rb @@ -79,15 +79,6 @@ def users_limit_dataset(source, free_user_cap) } end - override :users_filter_data - def users_filter_data(group) - root_group = group&.root_ancestor - - return {} unless root_group&.enforced_sso? && root_group.saml_provider&.id - - { users_filter: 'saml_provider_id', filter_id: root_group.saml_provider.id } - end - def overage_members_modal_available ::Gitlab::Saas.feature_available?(:overage_members_modal) end diff --git a/ee/spec/features/groups/members/list_members_spec.rb b/ee/spec/features/groups/members/list_members_spec.rb index 1801f807d55058..bd3f4c75560e88 100644 --- a/ee/spec/features/groups/members/list_members_spec.rb +++ b/ee/spec/features/groups/members/list_members_spec.rb @@ -122,31 +122,6 @@ expect(page).not_to have_content(user4.name) end end - - context 'when new_implementation_of_invite_members_search FF is disabled' do - before do - stub_feature_flags(new_implementation_of_invite_members_search: false) - end - - it 'returns only users with SAML in autocomplete', :js do - visit group_group_members_path(group) - - click_on 'Invite members' - - page.within invite_modal_selector do - field = find(member_dropdown_selector) - field.native.send_keys :tab - field.click - - wait_for_requests - - expect(page).to have_content(user1.name) - expect(page).to have_content(user2.name) - expect(page).not_to have_content(user3.name) - expect(page).not_to have_content(user4.name) - end - end - end end context 'when over free user limit', :saas do diff --git a/ee/spec/helpers/ee/invite_members_helper_spec.rb b/ee/spec/helpers/ee/invite_members_helper_spec.rb index 39de716a0a9106..46f40b4397c024 100644 --- a/ee/spec/helpers/ee/invite_members_helper_spec.rb +++ b/ee/spec/helpers/ee/invite_members_helper_spec.rb @@ -227,51 +227,6 @@ end end - describe '#users_filter_data' do - let_it_be(:group) { create(:group, :private) } - let_it_be(:saml_provider) { create(:saml_provider, group: group) } - - let!(:group2) { create(:group) } - - context 'when the group has enforced sso' do - before do - allow(group).to receive(:enforced_sso?).and_return(true) - end - - context 'when there is a group with a saml provider' do - it 'returns user filter data' do - expected = { users_filter: 'saml_provider_id', filter_id: saml_provider.id } - - expect(helper.users_filter_data(group)).to eq expected - end - end - - context 'when there is a group without a saml provider' do - it 'does not return user filter data' do - expect(helper.users_filter_data(group2)).to eq({}) - end - end - end - - context 'when group has enforced sso disabled' do - before do - allow(group).to receive(:enforced_sso?).and_return(false) - end - - context 'when there is a group with a saml provider' do - it 'does not return user filter data' do - expect(helper.users_filter_data(group)).to eq({}) - end - end - - context 'when there is a group without a saml provider' do - it 'does not return user filter data' do - expect(helper.users_filter_data(group2)).to eq({}) - end - end - end - end - describe '#overage_members_modal_available' do context 'when SaaS' do before do diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 82ed9e136a8415..b418c075a39e94 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -93,7 +93,6 @@ def add_gon_variables push_frontend_feature_flag(:new_project_creation_form, current_user, type: :wip) push_frontend_feature_flag(:work_items_client_side_boards, current_user) push_frontend_feature_flag(:glql_work_items, current_user, type: :wip) - push_frontend_feature_flag(:new_implementation_of_invite_members_search) end # Exposes the state of a feature flag to the frontend code. diff --git a/spec/frontend/invite_members/components/members_token_select_spec.js b/spec/frontend/invite_members/components/members_token_select_spec.js index 31ae2aae0fcdfb..c2af5bc8ab55ba 100644 --- a/spec/frontend/invite_members/components/members_token_select_spec.js +++ b/spec/frontend/invite_members/components/members_token_select_spec.js @@ -2,7 +2,6 @@ import { GlTokenSelector } from '@gitlab/ui'; import { nextTick } from 'vue'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import * as UserApi from '~/api/user_api'; import MembersTokenSelect from '~/invite_members/components/members_token_select.vue'; import { VALID_TOKEN_BACKGROUND, INVALID_TOKEN_BACKGROUND } from '~/invite_members/constants'; import * as MembersUtils from '~/invite_members/utils/member_utils'; @@ -19,7 +18,7 @@ const handleEnterSpy = jest.fn(); let wrapper; const searchUrl = 'https://example.com/gitlab/groups/mygroup/-/group_members/invite_search.json'; -const createComponent = ({ props = {}, glFeatures = {} } = {}) => { +const createComponent = ({ props = {} } = {}) => { wrapper = mountExtended(MembersTokenSelect, { propsData: { ariaLabelledby: label, @@ -27,7 +26,7 @@ const createComponent = ({ props = {}, glFeatures = {} } = {}) => { placeholder, ...props, }, - provide: { glFeatures, searchUrl }, + provide: { searchUrl }, }); }; @@ -92,34 +91,16 @@ describe('MembersTokenSelect', () => { }); describe('users', () => { - describe('when `newImplementationOfInviteMembersSearch` is enabled', () => { - let tokenSelector; - - beforeEach(() => { - jest.spyOn(MembersUtils, 'searchUsers').mockResolvedValue({ data: allUsers }); - createComponent({ glFeatures: { newImplementationOfInviteMembersSearch: true } }); - tokenSelector = findTokenSelector(); - }); - - it('calls the API with search parameter with whitespaces and is trimmed', async () => { - tokenSelector.vm.$emit('text-input', ' foo@bar.com '); - - await waitForPromises(); - - expect(MembersUtils.searchUsers).toHaveBeenCalledWith(searchUrl, 'foo@bar.com'); - expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); - }); - }); + let tokenSelector; beforeEach(() => { - jest.spyOn(UserApi, 'getUsers').mockResolvedValue({ data: allUsers }); + jest.spyOn(MembersUtils, 'searchUsers').mockResolvedValue({ data: allUsers }); createComponent(); + tokenSelector = findTokenSelector(); }); describe('when input is manually focused', () => { it('calls the API and sets dropdown items as request result', async () => { - const tokenSelector = findTokenSelector(); - tokenSelector.vm.$emit('focus'); await waitForPromises(); @@ -130,12 +111,6 @@ describe('MembersTokenSelect', () => { }); describe('when text input is typed in', () => { - let tokenSelector; - - beforeEach(() => { - tokenSelector = findTokenSelector(); - }); - it('calls the API with search parameter', async () => { const searchParam = 'One'; @@ -143,10 +118,7 @@ describe('MembersTokenSelect', () => { await waitForPromises(); - expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { - active: true, - without_project_bots: true, - }); + expect(MembersUtils.searchUsers).toHaveBeenCalledWith(searchUrl, searchParam); expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); }); @@ -155,10 +127,7 @@ describe('MembersTokenSelect', () => { await waitForPromises(); - expect(UserApi.getUsers).toHaveBeenCalledWith('foo@bar.com', { - active: true, - without_project_bots: true, - }); + expect(MembersUtils.searchUsers).toHaveBeenCalledWith(searchUrl, 'foo@bar.com'); expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); }); @@ -182,7 +151,7 @@ describe('MembersTokenSelect', () => { describe('when API search fails', () => { beforeEach(() => { jest.spyOn(Sentry, 'captureException'); - jest.spyOn(UserApi, 'getUsers').mockRejectedValue('error'); + jest.spyOn(MembersUtils, 'searchUsers').mockRejectedValue('error'); }); it('reports to sentry', async () => { @@ -242,27 +211,4 @@ describe('MembersTokenSelect', () => { expect(tokenSelector.props('hideDropdownWithNoItems')).toBe(false); }); }); - - describe('when component is mounted for a group using a SAML provider', () => { - const searchParam = 'name'; - const samlProviderId = 123; - - beforeEach(() => { - jest.spyOn(UserApi, 'getUsers').mockResolvedValue({ data: allUsers }); - - createComponent({ - props: { filterId: samlProviderId, usersFilter: 'saml_provider_id' }, - }); - - findTokenSelector().vm.$emit('text-input', searchParam); - }); - - it('calls the API with the saml provider ID param', () => { - expect(UserApi.getUsers).toHaveBeenCalledWith(searchParam, { - active: true, - without_project_bots: true, - saml_provider_id: samlProviderId, - }); - }); - }); }); -- GitLab