From aea17a97bc29e1c68a816f766dc324a151045adb Mon Sep 17 00:00:00 2001 From: Jacques Date: Thu, 4 Apr 2024 09:56:35 +0200 Subject: [PATCH] Replace approval selectors with User/Group selectors Updates the frontend to use the new approval User/Group selectors Changelog: changed EE: true --- app/assets/javascripts/api.js | 15 + .../components/list_selector/constants.js | 2 +- .../components/list_selector/group_item.vue | 34 ++- .../components/list_selector/index.vue | 42 ++- .../components/list_selector/user_item.vue | 6 +- .../components/approvers/approvers_list.vue | 35 --- .../approvers/approvers_list_empty.vue | 7 - .../approvers/approvers_list_item.vue | 70 ----- .../components/approvers/approvers_select.vue | 191 ------------ .../approvals/components/rules/rule_form.vue | 40 ++- .../approvals/mr_edit/mount_mr_edit.js | 5 + .../mount_project_settings.js | 5 + .../user_edits_approval_rules_mr_spec.rb | 15 +- .../merge_request/user_sets_approvers_spec.rb | 21 +- .../merge_request_approvals_settings_spec.rb | 12 +- .../approvers/approvers_list_item_spec.js | 106 ------- .../approvers/approvers_list_spec.js | 71 ----- .../approvers/approvers_select_spec.js | 274 ------------------ .../components/rules/rule_form_spec.js | 102 ++++--- .../helpers/feature_approval_helper.rb | 11 +- locale/gitlab.pot | 3 - qa/qa/ee/page/merge_request/new.rb | 32 +- qa/qa/page/component/dropdown.rb | 4 +- spec/frontend/api_spec.js | 17 ++ .../list_selector/group_item_spec.js | 15 +- .../components/list_selector/index_spec.js | 138 +++++---- .../list_selector/user_item_spec.js | 2 +- 27 files changed, 330 insertions(+), 945 deletions(-) delete mode 100644 ee/app/assets/javascripts/approvals/components/approvers/approvers_list.vue delete mode 100644 ee/app/assets/javascripts/approvals/components/approvers/approvers_list_empty.vue delete mode 100644 ee/app/assets/javascripts/approvals/components/approvers/approvers_list_item.vue delete mode 100644 ee/app/assets/javascripts/approvals/components/approvers/approvers_select.vue delete mode 100644 ee/spec/frontend/approvals/components/approvers/approvers_list_item_spec.js delete mode 100644 ee/spec/frontend/approvals/components/approvers/approvers_list_spec.js delete mode 100644 ee/spec/frontend/approvals/components/approvers/approvers_select_spec.js diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index b4a263d95a22f0..7ecdd38ca83d64 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -27,6 +27,7 @@ const Api = { projectPackagePath: '/api/:version/projects/:id/packages/:package_id', projectPackageFilePath: '/api/:version/projects/:id/packages/:package_id/package_files/:package_file_id', + projectGroupsPath: '/api/:version/projects/:id/groups.json', groupProjectsPath: '/api/:version/groups/:id/projects.json', groupSharePath: '/api/:version/groups/:id/share', projectsPath: '/api/:version/projects.json', @@ -131,6 +132,20 @@ const Api = { return axios.get(url); }, + projectGroups(id, options) { + const url = Api.buildUrl(this.projectGroupsPath).replace(':id', encodeURIComponent(id)); + + return axios + .get(url, { + params: { + ...options, + }, + }) + .then(({ data }) => { + return data; + }); + }, + deleteProjectPackage(projectId, packageId) { const url = this.buildProjectPackageUrl(projectId, packageId); return axios.delete(url); diff --git a/app/assets/javascripts/vue_shared/components/list_selector/constants.js b/app/assets/javascripts/vue_shared/components/list_selector/constants.js index 5ff5d59ef6b6e2..09cc9a2737beda 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/constants.js +++ b/app/assets/javascripts/vue_shared/components/list_selector/constants.js @@ -9,13 +9,13 @@ export const CONFIG = { title: __('Users'), icon: 'user', filterKey: 'username', - showNamespaceDropdown: true, component: UserItem, }, groups: { title: __('Groups'), icon: 'group', filterKey: 'name', + showNamespaceDropdown: true, component: GroupItem, }, deployKeys: { diff --git a/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue b/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue index fd467eb857673f..1fce74ceaea2da 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue +++ b/app/assets/javascripts/vue_shared/components/list_selector/group_item.vue @@ -7,6 +7,7 @@ export default { components: { GlAvatar, GlButton, + HiddenGroupsItem: () => import('ee_component/approvals/components/hidden_groups_item.vue'), }, props: { data: { @@ -32,31 +33,36 @@ export default { avatarUrl() { return this.data.avatarUrl; }, + isHiddenGroups() { + return this.data.type === 'hidden_groups'; + }, }, }; diff --git a/app/assets/javascripts/vue_shared/components/list_selector/index.vue b/app/assets/javascripts/vue_shared/components/list_selector/index.vue index b53b573e175c05..43474344ad3564 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/index.vue +++ b/app/assets/javascripts/vue_shared/components/list_selector/index.vue @@ -1,8 +1,10 @@ diff --git a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list.vue b/ee/app/assets/javascripts/approvals/components/approvers/approvers_list.vue deleted file mode 100644 index cf92a36bdb7526..00000000000000 --- a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list.vue +++ /dev/null @@ -1,35 +0,0 @@ - - - diff --git a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_empty.vue b/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_empty.vue deleted file mode 100644 index 4f6211a0f3f678..00000000000000 --- a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_empty.vue +++ /dev/null @@ -1,7 +0,0 @@ - diff --git a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_item.vue b/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_item.vue deleted file mode 100644 index f81ed7a8c2a99f..00000000000000 --- a/ee/app/assets/javascripts/approvals/components/approvers/approvers_list_item.vue +++ /dev/null @@ -1,70 +0,0 @@ - - - diff --git a/ee/app/assets/javascripts/approvals/components/approvers/approvers_select.vue b/ee/app/assets/javascripts/approvals/components/approvers/approvers_select.vue deleted file mode 100644 index 5dcf07cbdeb2fc..00000000000000 --- a/ee/app/assets/javascripts/approvals/components/approvers/approvers_select.vue +++ /dev/null @@ -1,191 +0,0 @@ - - - diff --git a/ee/app/assets/javascripts/approvals/components/rules/rule_form.vue b/ee/app/assets/javascripts/approvals/components/rules/rule_form.vue index a3421c6c8d5405..ccfd2e174b6506 100644 --- a/ee/app/assets/javascripts/approvals/components/rules/rule_form.vue +++ b/ee/app/assets/javascripts/approvals/components/rules/rule_form.vue @@ -4,6 +4,7 @@ import { groupBy, isEqual, isNumber } from 'lodash'; // eslint-disable-next-line no-restricted-imports import { mapState, mapActions } from 'vuex'; import ProtectedBranchesSelector from 'ee/vue_shared/components/branches_selector/protected_branches_selector.vue'; +import ListSelector from '~/vue_shared/components/list_selector/index.vue'; import { sprintf } from '~/locale'; import { ALL_BRANCHES, @@ -16,8 +17,6 @@ import { COVERAGE_CHECK_NAME, APPROVAL_DIALOG_I18N, } from '../../constants'; -import ApproversList from '../approvers/approvers_list.vue'; -import ApproversSelect from '../approvers/approvers_select.vue'; const DEFAULT_NAME = 'Default'; @@ -29,8 +28,7 @@ function mapServerResponseToValidationErrors(messages) { export default { components: { - ApproversList, - ApproversSelect, + ListSelector, GlFormGroup, GlFormInput, ProtectedBranchesSelector, @@ -352,6 +350,13 @@ export default { branches, }; }, + handleDeleteApprover(id) { + const approverIndex = this.approvers.findIndex((approver) => approver.id === id); + this.approvers.splice(approverIndex, 1); + }, + handleSelectApprover(approver, type) { + this.approvers.push({ ...approver, type }); + }, }, APPROVAL_DIALOG_I18N, ruleNameInput: 'rule-name-input', @@ -398,6 +403,7 @@ export default { /> - + -
- -
diff --git a/ee/app/assets/javascripts/approvals/mr_edit/mount_mr_edit.js b/ee/app/assets/javascripts/approvals/mr_edit/mount_mr_edit.js index db00652fb7d5fe..56e8fb5ccd0e49 100644 --- a/ee/app/assets/javascripts/approvals/mr_edit/mount_mr_edit.js +++ b/ee/app/assets/javascripts/approvals/mr_edit/mount_mr_edit.js @@ -1,5 +1,7 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; import { parseBoolean } from '~/lib/utils/common_utils'; +import createDefaultClient from '~/lib/graphql'; import createStore from '../stores'; import mrEditModule from '../stores/modules/mr_edit'; import MrEditApp from './app.vue'; @@ -33,6 +35,9 @@ export default function mountApprovalInput(el) { return new Vue({ el, store, + apolloProvider: new VueApollo({ + defaultClient: createDefaultClient(), + }), render(h) { return h(MrEditApp); }, diff --git a/ee/app/assets/javascripts/approvals/project_settings/mount_project_settings.js b/ee/app/assets/javascripts/approvals/project_settings/mount_project_settings.js index b1da1a06292d5e..454327963aca08 100644 --- a/ee/app/assets/javascripts/approvals/project_settings/mount_project_settings.js +++ b/ee/app/assets/javascripts/approvals/project_settings/mount_project_settings.js @@ -1,5 +1,7 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; import { GlToast } from '@gitlab/ui'; +import createDefaultClient from '~/lib/graphql'; import { parseBoolean } from '~/lib/utils/common_utils'; import { mergeRequestApprovalSettingsMappers } from '../mappers'; import createStore from '../stores'; @@ -38,6 +40,9 @@ export default function mountProjectSettingsApprovals(el) { return new Vue({ el, store, + apolloProvider: new VueApollo({ + defaultClient: createDefaultClient(), + }), provide: { coverageCheckHelpPagePath, fullPath, diff --git a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb index 4b23b9789b19ab..f117a92f9f83aa 100644 --- a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb +++ b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb @@ -4,12 +4,15 @@ RSpec.describe 'Merge request > User edits MR with approval rules', :js, feature_category: :code_review_workflow do include ListboxHelpers + include FeatureApprovalHelper include_context 'with project with approval rules' let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:approver) { create(:user) } let_it_be(:mr_rule_names) { %w[foo lorem ipsum] } + let_it_be(:users_testid) { 'users-selector' } + let_it_be(:groups_testid) { 'groups-selector' } let_it_be(:mr_rules) do mr_rule_names.map do |name| create( @@ -53,7 +56,8 @@ def page_rule_names within('.gl-drawer') do fill_in 'Rule name', with: rule_name - select_from_listbox approver.name, from: 'Search users or groups' + search(approver.name, users_testid) + select_listbox_item(approver.name) click_button 'Save changes' end @@ -78,14 +82,17 @@ def page_rule_names end it "with empty search, does not show public group" do - click_button 'Search users or groups' + search(public_group.name, groups_testid) expect_no_listbox_item(public_group.name) end it "with non-empty search, shows public group" do - click_button 'Search users or groups' - send_keys public_group.name + within_testid('groups-selector') do + click_button "Project groups" + find_by_testid("listbox-item-false").click + end + search(public_group.name, groups_testid) expect_listbox_item(public_group.name) end diff --git a/ee/spec/features/merge_request/user_sets_approvers_spec.rb b/ee/spec/features/merge_request/user_sets_approvers_spec.rb index 3adc4db959bc1b..cfd9f4da1764e7 100644 --- a/ee/spec/features/merge_request/user_sets_approvers_spec.rb +++ b/ee/spec/features/merge_request/user_sets_approvers_spec.rb @@ -11,6 +11,8 @@ let(:project) { create(:project, :public, :repository) } let(:config_selector) { '[data-testid="mr-approval-rules"]' } let(:drawer_selector) { '.gl-drawer' } + let_it_be(:users_testid) { 'users-selector' } + let_it_be(:groups_testid) { 'groups-selector' } before do stub_licensed_features(merge_request_approvers: true) @@ -31,7 +33,7 @@ it 'does not allow setting the author as an approver but allows setting the current user as an approver' do click_button('Approval rules') click_button('Add approval rule') - click_button 'Search users or groups' + search(user.name, users_testid) expect_no_listbox_item(author.name) expect_listbox_item(user.name) @@ -54,7 +56,7 @@ it 'allows setting other users as approvers but does not allow setting the current user as an approver, and filters non members from approvers list', :sidekiq_might_not_need_inline do click_button('Approval rules') click_button('Add approval rule') - click_button 'Search users or groups' + search(other_user.name, users_testid) expect_listbox_item(other_user.name) expect_no_listbox_item(non_member.name) @@ -81,7 +83,8 @@ click_button('Approval rules') click_button('Add approval rule') - click_button 'Search users or groups' + + search(group.name, groups_testid) expect_no_listbox_item(group.name) @@ -90,7 +93,13 @@ visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' }) click_button('Approval rules') click_button('Add approval rule') - click_button 'Search users or groups' + + within_testid(groups_testid) do + click_button "Project groups" + find_by_testid("listbox-item-false").click + end + + search(group.name, groups_testid) expect_listbox_item(group.name) @@ -121,7 +130,6 @@ remove_approver(group.name, '.gl-drawer-body') within(drawer_selector) do - expect(page).to have_css('.content-list li', count: 1) click_button 'Save changes' end @@ -155,7 +163,7 @@ click_button('Approval rules') click_button('Add approval rule') - click_button 'Search users or groups' + search(group.name, groups_testid) expect_listbox_item(group.name) @@ -188,7 +196,6 @@ wait_for_requests within(drawer_selector) do - expect(page).to have_css('.content-list li', count: 1) click_button 'Save changes' end diff --git a/ee/spec/features/projects/settings/merge_request_approvals_settings_spec.rb b/ee/spec/features/projects/settings/merge_request_approvals_settings_spec.rb index c4ae7d786cec8f..0d2252855ba80b 100644 --- a/ee/spec/features/projects/settings/merge_request_approvals_settings_spec.rb +++ b/ee/spec/features/projects/settings/merge_request_approvals_settings_spec.rb @@ -13,6 +13,8 @@ let_it_be(:non_member) { create(:user) } let_it_be(:config_selector) { '[data-testid="mr-approval-rules"]' } let_it_be(:modal_selector) { '#project-settings-approvals-create-modal' } + let_it_be(:users_testid) { 'users-selector' } + let_it_be(:groups_testid) { 'groups-selector' } before do sign_in(user) @@ -28,16 +30,16 @@ visit project_settings_merge_requests_path(project) click_button('Add approval rule') - click_button 'Search users or groups' + search(user.name, users_testid) expect_listbox_item(user.name) expect_no_listbox_item(non_member.name) select_listbox_item(user.name) - expect(find('.content-list')).to have_content(user.name) + expect(find_by_testid(users_testid)).to have_content(user.name) - click_button 'Search users or groups' + search(user.name, users_testid) expect_no_listbox_item(user.name) @@ -53,13 +55,13 @@ visit project_settings_merge_requests_path(project) click_button('Add approval rule') - click_button 'Search users or groups' + search(group.name, groups_testid) expect_listbox_item(group.name) select_listbox_item(group.name) - expect(find('.content-list')).to have_content(group.name) + expect(find_by_testid(groups_testid)).to have_content(group.name) within('.gl-drawer') do click_button 'Save changes' diff --git a/ee/spec/frontend/approvals/components/approvers/approvers_list_item_spec.js b/ee/spec/frontend/approvals/components/approvers/approvers_list_item_spec.js deleted file mode 100644 index b4a726bc588072..00000000000000 --- a/ee/spec/frontend/approvals/components/approvers/approvers_list_item_spec.js +++ /dev/null @@ -1,106 +0,0 @@ -import { GlButton, GlAvatarLabeled } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; -import ApproversListItem from 'ee/approvals/components/approvers/approvers_list_item.vue'; -import HiddenGroupsItem from 'ee/approvals/components/hidden_groups_item.vue'; -import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from 'ee/approvals/constants'; -import { AVATAR_SHAPE_OPTION_CIRCLE, AVATAR_SHAPE_OPTION_RECT } from '~/vue_shared/constants'; - -const TEST_USER = { - id: 1, - type: TYPE_USER, - name: 'Lorem Ipsum', - avatar_url: '/asd/1', -}; -const TEST_GROUP = { - id: 1, - type: TYPE_GROUP, - name: 'Lorem Group', - full_path: 'dolar/sit/amit', - avatar_url: '/asd/2', -}; - -describe('Approvals ApproversListItem', () => { - let wrapper; - - const factory = (options = {}) => { - wrapper = shallowMount(ApproversListItem, { - ...options, - }); - }; - - const findAvatar = () => wrapper.findComponent(GlAvatarLabeled); - const findHiddenGroupsItem = () => wrapper.findComponent(HiddenGroupsItem); - - describe('when user', () => { - beforeEach(() => { - factory({ - propsData: { - approver: TEST_USER, - }, - }); - }); - - it('renders GlAvatar for user', () => { - const avatar = findAvatar(); - expect(avatar.exists()).toBe(true); - expect(avatar.attributes()).toMatchObject({ - 'entity-name': TEST_USER.name, - src: TEST_USER.avatar_url, - shape: AVATAR_SHAPE_OPTION_CIRCLE, - alt: TEST_USER.name, - }); - expect(avatar.props('label')).toBe(TEST_USER.name); - }); - - it('when remove clicked, emits remove', async () => { - const button = wrapper.findComponent(GlButton); - await button.vm.$emit('click'); - - expect(wrapper.emitted().remove).toEqual([[TEST_USER]]); - }); - }); - - describe('when group', () => { - beforeEach(() => { - factory({ - propsData: { - approver: TEST_GROUP, - }, - }); - }); - - it('renders ProjectAvatar for group', () => { - const avatar = findAvatar(); - expect(avatar.exists()).toBe(true); - expect(avatar.attributes()).toMatchObject({ - 'entity-name': TEST_GROUP.name, - src: TEST_GROUP.avatar_url, - shape: AVATAR_SHAPE_OPTION_RECT, - alt: TEST_GROUP.name, - }); - expect(avatar.props('label')).toBe(TEST_GROUP.full_path); - }); - - it('does not render hidden-groups-item', () => { - expect(findHiddenGroupsItem().exists()).toBe(false); - }); - }); - - describe('when hidden groups', () => { - beforeEach(() => { - factory({ - propsData: { - approver: { type: TYPE_HIDDEN_GROUPS }, - }, - }); - }); - - it('renders hidden-groups-item', () => { - expect(findHiddenGroupsItem().exists()).toBe(true); - }); - - it('does not render any avatar', () => { - expect(findAvatar().exists()).toBe(false); - }); - }); -}); diff --git a/ee/spec/frontend/approvals/components/approvers/approvers_list_spec.js b/ee/spec/frontend/approvals/components/approvers/approvers_list_spec.js deleted file mode 100644 index 58f1fbfe0d76e3..00000000000000 --- a/ee/spec/frontend/approvals/components/approvers/approvers_list_spec.js +++ /dev/null @@ -1,71 +0,0 @@ -import { shallowMount } from '@vue/test-utils'; -import { nextTick } from 'vue'; -import ApproversList from 'ee/approvals/components/approvers/approvers_list.vue'; -import ApproversListEmpty from 'ee/approvals/components/approvers/approvers_list_empty.vue'; -import ApproversListItem from 'ee/approvals/components/approvers/approvers_list_item.vue'; -import { TYPE_USER, TYPE_GROUP } from 'ee/approvals/constants'; - -const TEST_APPROVERS = [ - { id: 1, type: TYPE_GROUP }, - { id: 1, type: TYPE_USER }, - { id: 2, type: TYPE_USER }, -]; - -describe('ApproversList', () => { - let propsData; - let wrapper; - - const factory = (options = {}) => { - wrapper = shallowMount(ApproversList, { - ...options, - propsData, - }); - }; - - beforeEach(() => { - propsData = {}; - }); - - describe('when empty', () => { - beforeEach(() => { - propsData.value = []; - }); - - it('renders empty', () => { - factory(); - - expect(wrapper.findComponent(ApproversListEmpty).exists()).toBe(true); - expect(wrapper.find('ul').exists()).toBe(false); - }); - }); - - describe('when not empty', () => { - beforeEach(() => { - propsData.value = TEST_APPROVERS; - }); - - it('renders items', () => { - factory(); - - const items = wrapper - .findAllComponents(ApproversListItem) - .wrappers.map((item) => item.props('approver')); - - expect(items).toEqual(TEST_APPROVERS); - }); - - TEST_APPROVERS.forEach((approver, idx) => { - it(`when remove (${idx}), emits new input`, async () => { - factory(); - - const item = wrapper.findAllComponents(ApproversListItem).at(idx); - item.vm.$emit('remove', approver); - - await nextTick(); - const expected = TEST_APPROVERS.filter((x, i) => i !== idx); - - expect(wrapper.emitted().input).toEqual([[expected]]); - }); - }); - }); -}); diff --git a/ee/spec/frontend/approvals/components/approvers/approvers_select_spec.js b/ee/spec/frontend/approvals/components/approvers/approvers_select_spec.js deleted file mode 100644 index 193a65aa8a129f..00000000000000 --- a/ee/spec/frontend/approvals/components/approvers/approvers_select_spec.js +++ /dev/null @@ -1,274 +0,0 @@ -import { GlCollapsibleListbox, GlListboxItem, GlAvatarLabeled, GlFormSelect } from '@gitlab/ui'; -import { nextTick } from 'vue'; -import { cloneDeep } from 'lodash'; -import { mount } from '@vue/test-utils'; -import stubChildren from 'helpers/stub_children'; -import { TEST_HOST } from 'helpers/test_constants'; -import waitForPromises from 'helpers/wait_for_promises'; -import Api from 'ee/api'; -import ApproversSelect from 'ee/approvals/components/approvers/approvers_select.vue'; -import { NAMESPACE_TYPES } from 'ee/security_orchestration/constants'; -import { TYPE_USER, GROUP_OPTIONS } from 'ee/approvals/constants'; - -const TEST_PROJECT_ID = '17'; -const TEST_GROUP_AVATAR = `${TEST_HOST}/group-avatar.png`; -const TEST_USER_AVATAR = `${TEST_HOST}/user-avatar.png`; -const TEST_GROUPS = [ - { id: 1, full_name: 'GitLab Org', full_path: 'gitlab/org', avatar_url: null }, - { - id: 2, - full_name: 'Lorem Ipsum', - full_path: 'lorem-ipsum', - avatar_url: TEST_GROUP_AVATAR, - }, -]; -const TEST_USERS = [ - { id: 1, name: 'Dolar', username: 'dolar', avatar_url: TEST_USER_AVATAR }, - { id: 3, name: 'Sit', username: 'sit', avatar_url: TEST_USER_AVATAR }, -]; - -const TERM = 'lorem'; - -describe('Approvers Selector', () => { - let wrapper; - - const findListbox = () => wrapper.findComponent(GlCollapsibleListbox); - const findAllListboxItems = () => wrapper.findAllComponents(GlListboxItem); - const findAvatar = (index) => findAllListboxItems().at(index).findComponent(GlAvatarLabeled); - const findGroupOptionsDropdown = () => wrapper.findComponent(GlFormSelect); - const findGroupOptions = () => wrapper.findAll('option'); - const search = (searchString) => findListbox().vm.$emit('search', searchString); - - const createComponent = (props = {}) => { - wrapper = mount(ApproversSelect, { - propsData: { - namespaceId: TEST_PROJECT_ID, - ...props, - }, - stubs: { - ...stubChildren(ApproversSelect), - GlCollapsibleListbox, - GlSelect: GlFormSelect, - }, - }); - }; - const openListbox = () => findListbox().vm.$emit('shown'); - - beforeEach(() => { - jest.spyOn(Api, 'groups').mockResolvedValue(TEST_GROUPS); - jest.spyOn(Api, 'projectGroups').mockResolvedValue(TEST_GROUPS); - jest.spyOn(Api, 'projectUsers').mockReturnValue(Promise.resolve(TEST_USERS)); - }); - - describe('Listbox', () => { - it('is rendered', () => { - createComponent(); - expect(findListbox().props()).toMatchObject({ - toggleText: ApproversSelect.i18n.toggleText, - noCaret: true, - searchable: true, - searching: false, - variant: 'default', - category: 'secondary', - }); - }); - - it('variant is set to danger if is invalid', () => { - createComponent({ isInvalid: true }); - - expect(findListbox().props('variant')).toBe('danger'); - }); - - it.each` - name | subtitle | avatarUrl | index - ${TEST_GROUPS[0].full_name} | ${TEST_GROUPS[0].full_path} | ${undefined} | ${0} - ${TEST_GROUPS[1].full_name} | ${TEST_GROUPS[1].full_path} | ${TEST_GROUP_AVATAR} | ${1} - ${TEST_USERS[0].name} | ${`@${TEST_USERS[0].username}`} | ${TEST_USER_AVATAR} | ${2} - ${TEST_USERS[1].name} | ${`@${TEST_USERS[1].username}`} | ${TEST_USER_AVATAR} | ${3} - `( - 'contains avatar with the correct props at index $index', - async ({ name, subtitle, avatarUrl, index }) => { - createComponent(); - openListbox(); - await waitForPromises(); - - expect(findAvatar(index).props()).toMatchObject({ - label: name, - subLabel: subtitle, - }); - - expect(findAvatar(index).attributes('src')).toBe(avatarUrl); - }, - ); - - describe('on show', () => { - it('queries groups and users', async () => { - createComponent(); - openListbox(); - await waitForPromises(); - - expect(Api.groups).toHaveBeenCalledWith('', { - skip_groups: [], - all_available: false, - order_by: 'id', - }); - expect(Api.projectUsers).toHaveBeenCalledWith(TEST_PROJECT_ID, '', { - skip_users: [], - }); - - expect(findListbox().props('items')).toMatchObject([...TEST_GROUPS, ...TEST_USERS]); - }); - - it('sets `searching` to `true` when first opening the dropdown', async () => { - createComponent(); - - expect(findListbox().props('searching')).toBe(false); - - openListbox(); - await nextTick(); - - expect(findListbox().props('searching')).toBe(true); - }); - }); - - describe('group options dropdown', () => { - it('renders dropdown with group options', () => { - createComponent(); - - expect(findGroupOptionsDropdown().exists()).toBe(true); - }); - it('renders expected amount of group options', () => { - createComponent(); - expect(findGroupOptions()).toHaveLength(GROUP_OPTIONS.length); - }); - }); - - describe('on search', () => { - it('sets `searching` to `true` while searching', async () => { - createComponent(); - - expect(findListbox().props('searching')).toBe(false); - - search('foo'); - await nextTick(); - - expect(findListbox().props('searching')).toBe(true); - }); - - it('fetches all groups and users matching the search string', async () => { - const searchString = 'searchString'; - createComponent(); - search(searchString); - await waitForPromises(); - - expect(Api.groups).toHaveBeenCalledWith(searchString, { - skip_groups: [], - all_available: true, - order_by: 'id', - }); - expect(Api.projectUsers).toHaveBeenCalledWith(TEST_PROJECT_ID, searchString, { - skip_users: [], - }); - }); - - describe.each` - namespaceType | api | mockedValue | expectedParams - ${NAMESPACE_TYPES.PROJECT} | ${'projectUsers'} | ${TEST_USERS} | ${[TEST_PROJECT_ID, TERM, { skip_users: [] }]} - ${NAMESPACE_TYPES.GROUP} | ${'groupMembers'} | ${{ data: TEST_USERS }} | ${[TEST_PROJECT_ID, { query: TERM, skip_users: [] }]} - `( - 'with namespaceType: $namespaceType and search term', - ({ namespaceType, api, mockedValue, expectedParams }) => { - beforeEach(async () => { - jest.spyOn(Api, api).mockReturnValue(Promise.resolve(mockedValue)); - - createComponent({ namespaceType }); - await waitForPromises(); - - search(TERM); - await waitForPromises(); - }); - - it('fetches all available groups', () => { - expect(Api.groups).toHaveBeenCalledWith(TERM, { - skip_groups: [], - all_available: true, - order_by: 'id', - }); - }); - - it('fetches users', () => { - expect(Api[api]).toHaveBeenCalledWith(...expectedParams); - }); - }, - ); - - describe('with empty search term and skips', () => { - const skipGroupIds = [7, 8]; - const skipUserIds = [9, 10]; - - beforeEach(async () => { - createComponent({ - skipGroupIds, - skipUserIds, - }); - openListbox(); - await waitForPromises(); - }); - - it('skips groups and does not fetch all available', () => { - expect(Api.groups).toHaveBeenCalledWith('', { - skip_groups: skipGroupIds, - all_available: false, - order_by: 'id', - }); - }); - - it('skips users', () => { - expect(Api.projectUsers).toHaveBeenCalledWith(TEST_PROJECT_ID, '', { - skip_users: skipUserIds, - }); - }); - }); - }); - - describe('on project groups option selection', () => { - it('fetches project groups when the project group option is selected', async () => { - createComponent(); - - const groupOptions = findGroupOptionsDropdown(); - - groupOptions.vm.$emit('input', 'project groups'); - await waitForPromises(); - - expect(Api.projectGroups).toHaveBeenCalledWith('17', { - with_shared: true, - skip_groups: [], - shared_min_access_level: 30, - }); - }); - }); - - describe('on selection', () => { - it('emits input when data changes', async () => { - const selectedUser = TEST_USERS[0]; - const selectedUserValue = `${TYPE_USER}.${selectedUser.id}`; - - createComponent(); - openListbox(); - await waitForPromises(); - - expect(wrapper.emitted().input).toBeUndefined(); - - findListbox().vm.$emit('select', selectedUserValue); - - const expected = { - ...selectedUser, - value: selectedUserValue, - subtitle: `@${selectedUser.username}`, - }; - - expect(cloneDeep(wrapper.emitted().input)).toEqual([[[expected]]]); - }); - }); - }); -}); diff --git a/ee/spec/frontend/approvals/components/rules/rule_form_spec.js b/ee/spec/frontend/approvals/components/rules/rule_form_spec.js index e523dd3f05c3fd..74babde5d66b05 100644 --- a/ee/spec/frontend/approvals/components/rules/rule_form_spec.js +++ b/ee/spec/frontend/approvals/components/rules/rule_form_spec.js @@ -3,8 +3,6 @@ import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; // eslint-disable-next-line no-restricted-imports import Vuex from 'vuex'; -import ApproversList from 'ee/approvals/components/approvers/approvers_list.vue'; -import ApproversSelect from 'ee/approvals/components/approvers/approvers_select.vue'; import RuleForm, { READONLY_NAMES } from 'ee/approvals/components/rules/rule_form.vue'; import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from 'ee/approvals/constants'; import { createStoreOptions } from 'ee/approvals/stores'; @@ -45,8 +43,6 @@ const nameTakenError = { Vue.use(Vuex); -const addType = (type) => (x) => Object.assign(x, { type }); - describe('EE Approvals RuleForm', () => { let wrapper; let store; @@ -76,9 +72,9 @@ describe('EE Approvals RuleForm', () => { const findNameValidation = () => wrapper.findByTestId('name-group'); const findApprovalsRequiredInput = () => wrapper.findByTestId('approvals-required'); const findApprovalsRequiredValidation = () => wrapper.findByTestId('approvals-required-group'); - const findApproversSelect = () => wrapper.findComponent(ApproversSelect); + const findUsersSelector = () => wrapper.findByTestId('users-selector'); + const findGroupsSelector = () => wrapper.findByTestId('groups-selector'); const findApproversValidation = () => wrapper.findByTestId('approvers-group'); - const findApproversList = () => wrapper.findComponent(ApproversList); const findProtectedBranchesSelector = () => wrapper.findComponent(ProtectedBranchesSelector); const findBranchesValidation = () => wrapper.findByTestId('branches-group'); @@ -97,6 +93,18 @@ describe('EE Approvals RuleForm', () => { findBranchesValidation(), ]; + const selectApprovers = () => { + const selectedUser1 = { id: 1, type: TYPE_USER }; + const selectedUser2 = { id: 2, type: TYPE_USER }; + const selectedGroup1 = { id: 2, type: TYPE_GROUP }; + const selectedGroup2 = { id: 3, type: TYPE_GROUP }; + + findUsersSelector().vm.$emit('select', selectedUser1); + findUsersSelector().vm.$emit('select', selectedUser2); + findGroupsSelector().vm.$emit('select', selectedGroup1); + findGroupsSelector().vm.$emit('select', selectedGroup2); + }; + beforeEach(() => { store = createStoreOptions( { approvals: projectSettingsModule() }, @@ -223,11 +231,13 @@ describe('EE Approvals RuleForm', () => { }); const branches = [TEST_PROTECTED_BRANCHES[0]]; - const expected = ruleData({ protectedBranchIds: branches.map((x) => x.id) }); + const expected = ruleData({ + protectedBranchIds: branches.map((x) => x.id), + }); + selectApprovers(); await findNameInput().vm.$emit('input', expected.name); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired); - await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]); await findProtectedBranchesSelector().vm.$emit('input', branches[0]); await findForm().trigger('submit'); @@ -238,12 +248,11 @@ describe('EE Approvals RuleForm', () => { createComponent({ isMrEdit: false, }); - const expected = ruleData(); + selectApprovers(); await findNameInput().vm.$emit('input', expected.name); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired); - await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]); await findProtectedBranchesSelector().vm.$emit('input', ALL_BRANCHES); await findForm().trigger('submit'); @@ -262,9 +271,9 @@ describe('EE Approvals RuleForm', () => { const expected = ruleData({ appliesToAllProtectedBranches: true }); + selectApprovers(); await findNameInput().vm.$emit('input', expected.name); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired); - await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]); await findProtectedBranchesSelector().vm.$emit('input', ALL_PROTECTED_BRANCHES); await findForm().trigger('submit'); @@ -319,7 +328,6 @@ describe('EE Approvals RuleForm', () => { }); it('on submit, shows approvers validation', async () => { - await findApproversList().vm.$emit('input', []); await findForm().trigger('submit'); await nextTick(); @@ -350,7 +358,7 @@ describe('EE Approvals RuleForm', () => { beforeEach(async () => { await findNameInput().vm.$emit('input', expected.name); await findApprovalsRequiredInput().vm.$emit('input', expected.approvalsRequired); - await findApproversList().vm.$emit('input', [...groupRecords, ...userRecords]); + selectApprovers(); await findProtectedBranchesSelector().vm.$emit('input', branches[0]); }); @@ -377,14 +385,22 @@ describe('EE Approvals RuleForm', () => { }); it('adds selected approvers on selection', async () => { - const orig = [{ id: 7, type: TYPE_GROUP }]; - const selected = [{ id: 2, type: TYPE_USER }]; - const expected = [...orig, ...selected]; + const selectedUser1 = { id: 1, type: TYPE_USER }; + const selectedUser2 = { id: 2, type: TYPE_USER }; + const selectedGroup1 = { id: 1, type: TYPE_GROUP }; + const selectedGroup2 = { id: 1, type: TYPE_GROUP }; - await findApproversSelect().vm.$emit('input', orig); - await findApproversSelect().vm.$emit('input', selected); + await findUsersSelector().vm.$emit('select', selectedUser1); + await findUsersSelector().vm.$emit('select', selectedUser2); - expect(findApproversList().props('value')).toEqual(expected); + await findGroupsSelector().vm.$emit('select', selectedGroup1); + await findGroupsSelector().vm.$emit('select', selectedGroup2); + + expect(findUsersSelector().props('selectedItems')).toEqual([selectedUser1, selectedUser2]); + expect(findGroupsSelector().props('selectedItems')).toEqual([ + selectedGroup1, + selectedGroup2, + ]); }); }); @@ -401,11 +417,9 @@ describe('EE Approvals RuleForm', () => { }); it('shows approvers', () => { - const list = findApproversList(); - - expect(list.props('value')).toEqual([ - ...TEST_RULE.groups.map(addType(TYPE_GROUP)), - ...TEST_RULE.users.map(addType(TYPE_USER)), + expect(findGroupsSelector().props('selectedItems')).toEqual([ + { id: 1, type: TYPE_GROUP }, + { id: 2, type: TYPE_GROUP }, ]); }); @@ -454,7 +468,6 @@ describe('EE Approvals RuleForm', () => { findNameInput().vm.$emit('input', ''); findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED); - findApproversList().vm.$emit('input', []); }); describe('with empty name and empty approvers', () => { @@ -494,7 +507,7 @@ describe('EE Approvals RuleForm', () => { describe('with empty name and approvers', () => { beforeEach(() => { - findApproversList().vm.$emit('input', TEST_APPROVERS); + selectApprovers(); findForm().trigger('submit'); }); @@ -509,7 +522,7 @@ describe('EE Approvals RuleForm', () => { describe('with name and approvers', () => { beforeEach(() => { - findApproversList().vm.$emit('input', [{ id: 7, type: TYPE_USER }]); + selectApprovers(); findNameInput().vm.$emit('input', 'Lorem'); findForm().trigger('submit'); }); @@ -535,12 +548,11 @@ describe('EE Approvals RuleForm', () => { }); it('shows approvers and hidden group', () => { - const list = findApproversList(); + const list = findGroupsSelector(); - expect(list.props('value')).toEqual([ - ...TEST_RULE.groups.map(addType(TYPE_GROUP)), - ...TEST_RULE.users.map(addType(TYPE_USER)), - { type: TYPE_HIDDEN_GROUPS }, + expect(list.props('selectedItems')).toEqual([ + { id: 1, type: 'group' }, + { id: 2, type: 'group' }, ]); }); @@ -557,12 +569,7 @@ describe('EE Approvals RuleForm', () => { describe('and hidden groups removed', () => { beforeEach(() => { - findApproversList().vm.$emit( - 'input', - findApproversList() - .props('value') - .filter((x) => x.type !== TYPE_HIDDEN_GROUPS), - ); + findGroupsSelector().vm.$emit('delete', { id: 2, type: TYPE_HIDDEN_GROUPS }); }); it('on submit, removes hidden groups', async () => { @@ -591,8 +598,8 @@ describe('EE Approvals RuleForm', () => { it('does not add hidden groups in approvers', () => { expect( - findApproversList() - .props('value') + findGroupsSelector() + .props('selectedItems') .every((x) => x.type !== TYPE_HIDDEN_GROUPS), ).toBe(true); }); @@ -679,7 +686,7 @@ describe('EE Approvals RuleForm', () => { describe('with approvers selected', () => { beforeEach(() => { - findApproversList().vm.$emit('input', TEST_APPROVERS); + selectApprovers(); findForm().trigger('submit'); }); @@ -689,7 +696,6 @@ describe('EE Approvals RuleForm', () => { expect.objectContaining({ name: expectedNameSubmitted, approvalsRequired: TEST_APPROVALS_REQUIRED, - users: TEST_APPROVERS.map((x) => x.id), }), ); }); @@ -723,7 +729,13 @@ describe('EE Approvals RuleForm', () => { initRule: { ...TEST_RULE, name: '' }, }); findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED); - findApproversList().vm.$emit('input', []); + + // Simulate the user removing all approvers + findGroupsSelector().vm.$emit('delete', 1); + findGroupsSelector().vm.$emit('delete', 2); + findUsersSelector().vm.$emit('delete', 1); + findUsersSelector().vm.$emit('delete', 2); + findUsersSelector().vm.$emit('delete', 3); findForm().trigger('submit'); }); @@ -745,7 +757,7 @@ describe('EE Approvals RuleForm', () => { initRule: { ...TEST_RULE, name: inputName }, }); findApprovalsRequiredInput().vm.$emit('input', TEST_APPROVALS_REQUIRED); - findApproversList().vm.$emit('input', TEST_APPROVERS); + findUsersSelector().vm.$emit('select', TEST_APPROVERS[0]); findForm().trigger('submit'); }); @@ -757,7 +769,7 @@ describe('EE Approvals RuleForm', () => { id: TEST_RULE.id, name: expectedNameSubmitted, approvalsRequired: TEST_APPROVALS_REQUIRED, - users: TEST_APPROVERS.map((x) => x.id), + users: [1, 2, 3, 7], }), ); }); diff --git a/ee/spec/support/helpers/feature_approval_helper.rb b/ee/spec/support/helpers/feature_approval_helper.rb index b5d5b937ab34b6..26a3d60898a99c 100644 --- a/ee/spec/support/helpers/feature_approval_helper.rb +++ b/ee/spec/support/helpers/feature_approval_helper.rb @@ -14,8 +14,15 @@ def open_modal(text: 'Edit', expand: true) end def remove_approver(name, selector = modal_selector) - el = page.find("#{selector} .content-list li", text: /#{name}/i) - el.find('button').click + within(selector) do + find_button("Delete #{name}").click + end + end + + def search(name, testid) + within_testid(testid) do + fill_in 'Search', with: name + end end def expect_avatar(container, users) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d7ea0be2f91621..3f4141d24aeebb 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -59964,9 +59964,6 @@ msgstr "" msgid "You have no permissions" msgstr "" -msgid "You have not added any approvers. Start by adding users or groups." -msgstr "" - msgid "You have set up 2FA for your account! If you lose access to your 2FA device, you can use your recovery codes to access your account. Alternatively, if you upload an SSH key, you can %{anchorOpen}use that key to generate additional recovery codes%{anchorClose}." msgstr "" diff --git a/qa/qa/ee/page/merge_request/new.rb b/qa/qa/ee/page/merge_request/new.rb index c8a208177a9103..acafdafd20d404 100644 --- a/qa/qa/ee/page/merge_request/new.rb +++ b/qa/qa/ee/page/merge_request/new.rb @@ -21,6 +21,8 @@ def self.prepended(base) element 'approvals-required' element 'approvers-group' element 'rule-name-field' + element 'users-selector' + element 'groups-selector' end view 'ee/app/assets/javascripts/approvals/components/rule_drawer/create_rule.vue' do @@ -39,11 +41,13 @@ def add_approval_rules(rules) fill_element('rule-name-field', rule[:name]) fill_element('approvals-required', rule[:approvals_required]) - rule.key?(:users) && rule[:users].each do |user| - select_member(user.username) - end - rule.key?(:groups) && rule[:groups].each do |group| - select_member(group.full_path) + within_element('approvers-group') do + rule.key?(:users) && rule[:users].each do |user| + select_user(user.username) + end + rule.key?(:groups) && rule[:groups].each do |group| + select_group(group.name) + end end click_approvers_modal_ok_button @@ -65,16 +69,18 @@ def click_approvers_modal_ok_button private - def select_member(name) + def select_user(username) retry_until do - within_element('approvers-group') do - click_button('Search users or groups') - search_item(name) + within_element('users-selector') do + search_and_select(username) + end + end + end - # we must send an extra key to trigger the dropdown to filter - # as the filtering does not work correctly with Capybara input - send_keys_to_search(:space) - select_item(name) + def select_group(group_name) + retry_until do + within_element('groups-selector') do + search_and_select(group_name) end end end diff --git a/qa/qa/page/component/dropdown.rb b/qa/qa/page/component/dropdown.rb index 5f60a789900e28..f752d46ffd9795 100644 --- a/qa/qa/page/component/dropdown.rb +++ b/qa/qa/page/component/dropdown.rb @@ -45,12 +45,12 @@ def clear_current_selection_if_present def search_item(item_text) QA::Runtime::Logger.info "Searching in dropdown: \"#{item_text}\"" - find('div.gl-listbox-search input[type="Search"]').set(item_text, rapid: false) + find('input[type="Search"]').set(item_text, rapid: false) wait_for_search_to_complete end def send_keys_to_search(item_text) - find('div.gl-listbox-search input[type="Search"]').send_keys(item_text) + find('input[type="Search"]').send_keys(item_text) wait_for_search_to_complete end diff --git a/spec/frontend/api_spec.js b/spec/frontend/api_spec.js index 345f03a37e6ba3..07475ea99e9f3d 100644 --- a/spec/frontend/api_spec.js +++ b/spec/frontend/api_spec.js @@ -51,6 +51,23 @@ describe('Api', () => { }); }); + describe('projectGroups', () => { + const projectId = '123'; + const options = { search: 'foo' }; + const apiResponse = [{ id: 1, name: 'foo' }]; + + it('fetch all project groups', () => { + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectId}/groups.json`; + jest.spyOn(axios, 'get'); + mock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK, apiResponse); + + return Api.projectGroups(projectId, options).then((data) => { + expect(data).toEqual(apiResponse); + expect(axios.get).toHaveBeenCalledWith(expectedUrl, { params: { ...options } }); + }); + }); + }); + describe('packages', () => { const projectId = 'project_a'; const packageId = 'package_b'; diff --git a/spec/frontend/vue_shared/components/list_selector/group_item_spec.js b/spec/frontend/vue_shared/components/list_selector/group_item_spec.js index 04c68355916c32..2b7d2f5ba07155 100644 --- a/spec/frontend/vue_shared/components/list_selector/group_item_spec.js +++ b/spec/frontend/vue_shared/components/list_selector/group_item_spec.js @@ -1,11 +1,12 @@ import { GlAvatar } from '@gitlab/ui'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import GroupItem from '~/vue_shared/components/list_selector/group_item.vue'; +import HiddenGroupsItem from 'ee_component/approvals/components/hidden_groups_item.vue'; describe('GroupItem spec', () => { let wrapper; - const MOCK_GROUP = { fullName: 'Group 1', name: 'group1', avatarUrl: 'some/avatar.jpg' }; + const MOCK_GROUP = { id: 123, fullName: 'Group 1', name: 'group1', avatarUrl: 'some/avatar.jpg' }; const createComponent = (props) => { wrapper = mountExtended(GroupItem, { @@ -38,6 +39,16 @@ describe('GroupItem spec', () => { expect(findDeleteButton().exists()).toBe(false); }); + describe('hidden groups', () => { + beforeEach(() => createComponent({ data: { ...MOCK_GROUP, type: 'hidden_groups' } })); + + const findHiddenGroupsItem = () => wrapper.findComponent(HiddenGroupsItem); + + it('renders a hidden groups item component', () => { + expect(findHiddenGroupsItem().exists()).toBe(true); + }); + }); + describe('Delete button', () => { beforeEach(() => createComponent({ canDelete: true })); @@ -49,7 +60,7 @@ describe('GroupItem spec', () => { it('emits a delete event if the delete button is clicked', () => { findDeleteButton().trigger('click'); - expect(wrapper.emitted('delete')).toEqual([[MOCK_GROUP.name]]); + expect(wrapper.emitted('delete')).toEqual([[MOCK_GROUP.id]]); }); }); }); diff --git a/spec/frontend/vue_shared/components/list_selector/index_spec.js b/spec/frontend/vue_shared/components/list_selector/index_spec.js index ca1cacdac6f326..f94519b8cab36d 100644 --- a/spec/frontend/vue_shared/components/list_selector/index_spec.js +++ b/spec/frontend/vue_shared/components/list_selector/index_spec.js @@ -4,6 +4,7 @@ import { GlCard, GlIcon, GlCollapsibleListbox, GlSearchBoxByType } from '@gitlab import Api from '~/api'; import RestApi from '~/rest_api'; import { createAlert } from '~/alert'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import ListSelector from '~/vue_shared/components/list_selector/index.vue'; import UserItem from '~/vue_shared/components/list_selector/user_item.vue'; @@ -13,6 +14,7 @@ import DeployKeyItem from '~/vue_shared/components/list_selector/deploy_key_item import groupsAutocompleteQuery from '~/graphql_shared/queries/groups_autocomplete.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { ACCESS_LEVEL_DEVELOPER_INTEGER } from '~/access_level/constants'; import { USERS_RESPONSE_MOCK, GROUPS_RESPONSE_MOCK } from './mock_data'; jest.mock('~/alert'); @@ -79,7 +81,7 @@ describe('List Selector spec', () => { beforeEach(() => { jest.spyOn(Api, 'projectUsers').mockResolvedValue(USERS_RESPONSE_MOCK); - jest.spyOn(Api, 'groupMembers').mockResolvedValue({ data: USERS_RESPONSE_MOCK }); + jest.spyOn(Api, 'projectGroups').mockResolvedValue(GROUPS_RESPONSE_MOCK.data.groups.nodes); }); describe('empty state', () => { @@ -91,8 +93,6 @@ describe('List Selector spec', () => { }); describe('Users type', () => { - const search = 'foo'; - beforeEach(() => createComponent(USERS_MOCK_PROPS)); it('renders a Card component', () => { @@ -112,67 +112,11 @@ describe('List Selector spec', () => { expect(findSearchBox().exists()).toBe(true); }); - it('renders two namespace dropdown items', () => { - expect(findNamespaceDropdown().props('items').length).toBe(2); - }); - it('does not call query when search box has not received an input', () => { expect(Api.projectUsers).not.toHaveBeenCalled(); - expect(Api.groupMembers).not.toHaveBeenCalled(); expect(findAllUserComponents().length).toBe(0); }); - describe.each` - dropdownItemValue | apiMethod | apiParams | searchResponse - ${'false'} | ${'groupMembers'} | ${[USERS_MOCK_PROPS.groupPath, { query: search }]} | ${USERS_RESPONSE_MOCK} - ${'true'} | ${'projectUsers'} | ${[USERS_MOCK_PROPS.projectPath, search]} | ${USERS_RESPONSE_MOCK} - `( - 'searching based on namespace dropdown selection', - ({ dropdownItemValue, apiMethod, apiParams, searchResponse }) => { - const emitSearchInput = async () => { - findSearchBox().vm.$emit('input', search); - await waitForPromises(); - }; - - beforeEach(async () => { - findNamespaceDropdown().vm.$emit('select', dropdownItemValue); - await emitSearchInput(); - }); - - it('shows error alert when API fails', async () => { - jest.spyOn(Api, apiMethod).mockRejectedValueOnce(); - await emitSearchInput(); - - expect(createAlert).toHaveBeenCalledWith({ - message: 'An error occurred while fetching. Please try again.', - }); - }); - - it('calls query with correct variables when Search box receives an input', () => { - expect(Api[apiMethod]).toHaveBeenCalledWith(...apiParams); - }); - - it('renders a List box component with the correct props', () => { - expect(findSearchResultsDropdown().props()).toMatchObject({ - items: searchResponse, - }); - }); - - it('renders a user component for each search result', () => { - expect(findAllUserComponents().length).toBe(searchResponse.length); - }); - - it('emits an event when a search result is selected', () => { - const firstSearchResult = searchResponse[0]; - findSearchResultsDropdown().vm.$emit('select', firstSearchResult.username); - - expect(wrapper.emitted('select')).toEqual([ - [{ ...firstSearchResult, text: 'Administrator', value: 'root' }], - ]); - }); - }, - ); - describe('selected items', () => { const selectedUser = { username: 'root' }; const selectedItems = [selectedUser]; @@ -202,6 +146,7 @@ describe('List Selector spec', () => { describe('Groups type', () => { beforeEach(() => createComponent(GROUPS_MOCK_PROPS)); + const search = 'foo'; it('renders a correct title', () => { expect(findTitle().exists()).toBe(true); @@ -217,16 +162,25 @@ describe('List Selector spec', () => { expect(findAllGroupComponents().length).toBe(0); }); + it('renders two namespace dropdown items', () => { + expect(findNamespaceDropdown().props('items').length).toBe(2); + }); + describe('searching', () => { - const searchResponse = GROUPS_RESPONSE_MOCK.data.groups.nodes; - const search = 'foo'; + const searchResponse = GROUPS_RESPONSE_MOCK.data.groups.nodes.map((group) => ({ + ...group, + id: getIdFromGraphQLId(group.id), + })); const emitSearchInput = async () => { findSearchBox().vm.$emit('input', search); await waitForPromises(); }; - beforeEach(() => emitSearchInput()); + beforeEach(async () => { + findNamespaceDropdown().vm.$emit('select', 'false'); + await emitSearchInput(); + }); it('calls query with correct variables when Search box receives an input', () => { expect(groupsAutocompleteQuerySuccess).toHaveBeenCalledWith({ @@ -249,7 +203,65 @@ describe('List Selector spec', () => { findSearchResultsDropdown().vm.$emit('select', firstSearchResult.name); expect(wrapper.emitted('select')).toEqual([ - [{ ...firstSearchResult, text: 'Flightjs', value: 'Flightjs', type: 'group' }], + [ + { + __typename: 'Group', + avatarUrl: null, + fullName: 'Flightjs', + id: 33, + name: 'Flightjs', + text: 'Flightjs', + value: 'Flightjs', + }, + ], + ]); + }); + }); + + describe('searching based on namespace dropdown selection', () => { + const searchResponse = GROUPS_RESPONSE_MOCK.data.groups.nodes; + + const emitSearchInput = async () => { + findSearchBox().vm.$emit('input', search); + await waitForPromises(); + }; + + beforeEach(async () => { + findNamespaceDropdown().vm.$emit('select', 'true'); + await emitSearchInput(); + }); + + it('shows error alert when API fails', async () => { + jest.spyOn(Api, 'projectGroups').mockRejectedValueOnce(); + await emitSearchInput(); + + expect(createAlert).toHaveBeenCalledWith({ + message: 'An error occurred while fetching. Please try again.', + }); + }); + + it('calls query with correct variables when Search box receives an input', () => { + expect(Api.projectGroups).toHaveBeenCalledWith(USERS_MOCK_PROPS.projectPath, { + search, + shared_min_access_level: ACCESS_LEVEL_DEVELOPER_INTEGER, + with_shared: true, + }); + }); + + it('renders a List box component with the correct props', () => { + expect(findSearchResultsDropdown().props('items')).toMatchObject(searchResponse); + }); + + it('renders a group component for each search result', () => { + expect(findAllGroupComponents().length).toBe(searchResponse.length); + }); + + it('emits an event when a search result is selected', () => { + const firstSearchResult = searchResponse[0]; + findSearchResultsDropdown().vm.$emit('select', firstSearchResult.name); + + expect(wrapper.emitted('select')).toMatchObject([ + [{ ...firstSearchResult, value: 'Flightjs' }], ]); }); }); diff --git a/spec/frontend/vue_shared/components/list_selector/user_item_spec.js b/spec/frontend/vue_shared/components/list_selector/user_item_spec.js index d84a29c67e03df..0a95f77a9db091 100644 --- a/spec/frontend/vue_shared/components/list_selector/user_item_spec.js +++ b/spec/frontend/vue_shared/components/list_selector/user_item_spec.js @@ -49,7 +49,7 @@ describe('UserItem spec', () => { it('emits a delete event if the delete button is clicked', () => { findDeleteButton().trigger('click'); - expect(wrapper.emitted('delete')).toEqual([[MOCK_USER.username]]); + expect(wrapper.emitted('delete')).toEqual([[MOCK_USER.id]]); }); }); }); -- GitLab