From d50b4bc2ea10fab58d22aa322f3d67382b881639 Mon Sep 17 00:00:00 2001 From: Anton Kalmykov Date: Sun, 22 Sep 2024 18:38:26 +0300 Subject: [PATCH] Allow to select multiple groups with the same name in MR approval rules Closes: https://gitlab.com/gitlab-org/gitlab/-/issues/481446 Changelog: changed --- .../components/list_selector/api.js | 22 +++++++++++-------- .../components/list_selector/constants.js | 2 +- .../components/list_selector/api_spec.js | 12 +++++----- .../components/list_selector/index_spec.js | 21 +++++++++++------- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/list_selector/api.js b/app/assets/javascripts/vue_shared/components/list_selector/api.js index 4f8ba790e4d9c5..ff474e7619fced 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/api.js +++ b/app/assets/javascripts/vue_shared/components/list_selector/api.js @@ -16,7 +16,7 @@ export const fetchProjectGroups = (projectPath, search) => { }).then((data) => data?.map((group) => ({ text: group.full_name, - value: group.name, + value: group.id, ...convertObjectPropsToCamelCase(group), })), ); @@ -29,13 +29,17 @@ export const fetchAllGroups = async (apollo, search) => { variables: { search }, }) .then(({ data }) => - data?.groups.nodes.map((group) => ({ - text: group.fullName, - value: group.name, - ...group, - id: getIdFromGraphQLId(group.id), - type: 'group', - })), + data?.groups.nodes.map((group) => { + const groupId = getIdFromGraphQLId(group.id); + + return { + text: group.fullName, + value: groupId, + ...group, + id: groupId, + type: 'group', + }; + }), ); }; @@ -51,7 +55,7 @@ export const fetchGroupsWithProjectAccess = (projectId, search) => { .then(({ data }) => data.map((group) => ({ text: group.name, - value: group.name, + value: group.id, ...convertObjectPropsToCamelCase(group), })), ); 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 2e26a2ef449068..17c16f00192d66 100644 --- a/app/assets/javascripts/vue_shared/components/list_selector/constants.js +++ b/app/assets/javascripts/vue_shared/components/list_selector/constants.js @@ -19,7 +19,7 @@ export const CONFIG = { groups: { title: __('Groups'), icon: 'group', - filterKey: 'name', + filterKey: 'id', showNamespaceDropdown: true, component: GroupItem, }, diff --git a/spec/frontend/vue_shared/components/list_selector/api_spec.js b/spec/frontend/vue_shared/components/list_selector/api_spec.js index 3fadff705f9966..5358c6cb5f7cc7 100644 --- a/spec/frontend/vue_shared/components/list_selector/api_spec.js +++ b/spec/frontend/vue_shared/components/list_selector/api_spec.js @@ -47,8 +47,8 @@ describe('List Selector Utils', () => { const result = await fetchProjectGroups(mockProjectPath, mockGroupSearch); expect(result).toEqual([ - { text: 'Group 1', value: 'group1', id: 1, fullName: 'Group 1', name: 'group1' }, - { text: 'Group 2', value: 'group2', id: 2, fullName: 'Group 2', name: 'group2' }, + { text: 'Group 1', value: 1, id: 1, fullName: 'Group 1', name: 'group1' }, + { text: 'Group 2', value: 2, id: 2, fullName: 'Group 2', name: 'group2' }, ]); }); }); @@ -88,7 +88,7 @@ describe('List Selector Utils', () => { expect(result).toEqual([ { text: 'Group 1', - value: 'group1', + value: 1, id: 1, fullName: 'Group 1', name: 'group1', @@ -96,7 +96,7 @@ describe('List Selector Utils', () => { }, { text: 'Group 2', - value: 'group2', + value: 2, id: 2, fullName: 'Group 2', name: 'group2', @@ -138,8 +138,8 @@ describe('List Selector Utils', () => { const result = await fetchGroupsWithProjectAccess(mockProjectId, mockGroupSearch); expect(result).toEqual([ - { text: 'group1', value: 'group1', id: 1, avatarUrl: null, name: 'group1' }, - { text: 'group2', value: 'group2', id: 2, avatarUrl: null, name: 'group2' }, + { text: 'group1', value: 1, id: 1, avatarUrl: null, name: 'group1' }, + { text: 'group2', value: 2, id: 2, avatarUrl: null, name: 'group2' }, ]); }); }); 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 16c566b3bc4306..d75aaa6b0a0afb 100644 --- a/spec/frontend/vue_shared/components/list_selector/index_spec.js +++ b/spec/frontend/vue_shared/components/list_selector/index_spec.js @@ -198,10 +198,15 @@ describe('List Selector spec', () => { describe('searching', () => { describe('for default all groups', () => { - const searchResponse = GROUPS_RESPONSE_MOCK.data.groups.nodes.map((group) => ({ - ...group, - id: getIdFromGraphQLId(group.id), - })); + const searchResponse = GROUPS_RESPONSE_MOCK.data.groups.nodes.map((group) => { + const groupId = getIdFromGraphQLId(group.id); + + return { + ...group, + id: groupId, + value: groupId, + }; + }); const emitSearchInput = async () => { findSearchBox().vm.$emit('input', search); @@ -231,7 +236,7 @@ describe('List Selector spec', () => { it('emits an event when a search result is selected', () => { const firstSearchResult = searchResponse[0]; - findSearchResultsDropdown().vm.$emit('select', firstSearchResult.name); + findSearchResultsDropdown().vm.$emit('select', firstSearchResult.id); expect(wrapper.emitted('select')).toEqual([ [ @@ -242,7 +247,7 @@ describe('List Selector spec', () => { id: 33, name: 'Flightjs', text: 'Flightjs', - value: 'Flightjs', + value: 33, type: 'group', }, ], @@ -329,10 +334,10 @@ describe('List Selector spec', () => { it('emits an event when a search result is selected', () => { const firstSearchResult = searchResponse[0]; - findSearchResultsDropdown().vm.$emit('select', firstSearchResult.name); + findSearchResultsDropdown().vm.$emit('select', firstSearchResult.id); expect(wrapper.emitted('select')).toMatchObject([ - [{ ...firstSearchResult, value: 'Flightjs' }], + [{ ...firstSearchResult, value: firstSearchResult.id }], ]); }); }); -- GitLab