From b474d5a2fb16807566b19e905578c22ed632ffd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= Date: Thu, 15 Feb 2024 13:00:26 +0100 Subject: [PATCH] Use group/project ID for invite user modal We are replacing the query to invite users when the SAML is enabled. We have decided to use either a group query or project query depending of the context. See https://gitlab.com/gitlab-org/gitlab/-/issues/434464#note_1764747685. These API queries are not yet in use because the feature flag, `group_users_saml` is currently off in all environments. So it is safe to deploy. --- app/assets/javascripts/api/user_api.js | 11 ++-- .../components/invite_members_modal.vue | 3 +- .../components/members_token_select.vue | 8 ++- spec/frontend/api/user_api_spec.js | 15 ++++- .../components/members_token_select_spec.js | 58 ++++++++++++++++--- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/api/user_api.js b/app/assets/javascripts/api/user_api.js index 302de9760800c1..b3925e654fbf26 100644 --- a/app/assets/javascripts/api/user_api.js +++ b/app/assets/javascripts/api/user_api.js @@ -4,7 +4,8 @@ import { buildApiUrl } from './api_utils'; const USER_COUNTS_PATH = '/api/:version/user_counts'; const USERS_PATH = '/api/:version/users.json'; -const USERS_SAML_PATH = '/api/:version/groups/:id/users.json'; +const USERS_SAML_GROUP_PATH = '/api/:version/groups/:id/users.json'; +const USERS_SAML_PROJECT_PATH = '/api/:version/projects/:id/group_users.json'; const USER_PATH = '/api/:version/users/:id'; const USER_STATUS_PATH = '/api/:version/users/:id/status'; const USER_PROJECTS_PATH = '/api/:version/users/:id/projects'; @@ -31,11 +32,13 @@ export function getUsers(query, options) { * If the query string is less than 3 characters it returns an empty list. * * @param {string} query - query string to search - * @param {string} groupId -- top-level group id + * @param {boolean} isProject + * @param {string} id -- group or project id * @param {object} options */ -export function getGroupUsers(query, groupId, options) { - const url = buildApiUrl(USERS_SAML_PATH).replace(':id', groupId); +export function getGroupUsers(query, isProject, id, options) { + const path = isProject ? USERS_SAML_PROJECT_PATH : USERS_SAML_GROUP_PATH; + const url = buildApiUrl(path).replace(':id', id); return axios.get(url, { params: { search: query, 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 5125f6f70ac46c..ddf8396333bf62 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -456,7 +456,8 @@ export default { :exception-state="exceptionState" :users-filter="usersFilter" :filter-id="filterId" - :group-id="id" + :group-or-project-id="id" + :is-project="isProject" :invalid-members="invalidMembers" @clear="clearValidation" @token-remove="removeToken" 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 0f5b437bb569f8..b8a3f540a92353 100644 --- a/app/assets/javascripts/invite_members/components/members_token_select.vue +++ b/app/assets/javascripts/invite_members/components/members_token_select.vue @@ -57,10 +57,14 @@ export default { required: false, default: '', }, - groupId: { + groupOrProjectId: { type: String, required: true, }, + isProject: { + type: Boolean, + required: true, + }, }, data() { return { @@ -140,7 +144,7 @@ export default { }, retrieveUsersRequest() { if (this.usersFilter === USERS_FILTER_SAML_PROVIDER_ID && this.glFeatures.groupUserSaml) { - return getGroupUsers(this.query, this.groupId, this.queryOptions); + return getGroupUsers(this.query, this.isProject, this.groupOrProjectId, this.queryOptions); } return getUsers(this.query, this.queryOptions); diff --git a/spec/frontend/api/user_api_spec.js b/spec/frontend/api/user_api_spec.js index aeddf6b9ae135f..4263137509c631 100644 --- a/spec/frontend/api/user_api_spec.js +++ b/spec/frontend/api/user_api_spec.js @@ -52,11 +52,22 @@ describe('~/api/user_api', () => { }); describe('getSAMLUsers', () => { - it('calls correct URL with expected query parameters', async () => { + it('calls the group URL with expected query parameters', async () => { const expectedUrl = '/api/v4/groups/34/users.json'; axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK); - await getGroupUsers('den', '34', { include_service_accounts: true }); + await getGroupUsers('den', false, '34', { include_service_accounts: true }); + + const { url, params } = axiosMock.history.get[0]; + expect(url).toBe(expectedUrl); + expect(params).toMatchObject({ search: 'den', include_service_accounts: true }); + }); + + it('calls the project URL with expected query parameters', async () => { + const expectedUrl = '/api/v4/projects/34/group_users.json'; + axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK); + + await getGroupUsers('den', true, '34', { include_service_accounts: true }); const { url, params } = axiosMock.history.get[0]; expect(url).toBe(expectedUrl); 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 44f62d5904d2b9..622219c73d41e8 100644 --- a/spec/frontend/invite_members/components/members_token_select_spec.js +++ b/spec/frontend/invite_members/components/members_token_select_spec.js @@ -10,7 +10,7 @@ import * as Sentry from '~/sentry/sentry_browser_wrapper'; const label = 'testgroup'; const placeholder = 'Search for a member'; -const groupId = '31'; +const groupOrProjectId = '31'; const user1 = { id: 1, name: 'John Smith', username: 'one_1', avatar_url: '' }; const user2 = { id: 2, name: 'Jane Doe', username: 'two_2', avatar_url: '' }; const allUsers = [user1, user2]; @@ -22,7 +22,8 @@ const createComponent = (props = {}, glFeatures = {}) => { ariaLabelledby: label, invalidMembers: {}, placeholder, - groupId, + isProject: false, + groupOrProjectId, ...props, }, provide: { glFeatures }, @@ -222,20 +223,59 @@ describe('MembersTokenSelect', () => { describe('when component is mounted for a group using a SAML provider', () => { const searchParam = 'name'; + let isProject; beforeEach(() => { jest.spyOn(UserApi, 'getGroupUsers').mockResolvedValue({ data: allUsers }); + }); + + describe('when used on projects', () => { + beforeEach(() => { + isProject = true; + wrapper = createComponent( + { usersFilter: 'saml_provider_id', isProject }, + { groupUserSaml: true }, + ); - wrapper = createComponent({ usersFilter: 'saml_provider_id' }, { groupUserSaml: true }); + findTokenSelector().vm.$emit('text-input', searchParam); + }); - findTokenSelector().vm.$emit('text-input', searchParam); + it('calls the group API with correct parameters', () => { + expect(UserApi.getGroupUsers).toHaveBeenCalledWith( + searchParam, + isProject, + groupOrProjectId, + { + active: true, + include_saml_users: true, + include_service_accounts: true, + }, + ); + }); }); - it('calls the group API with correct parameters', () => { - expect(UserApi.getGroupUsers).toHaveBeenCalledWith(searchParam, groupId, { - active: true, - include_saml_users: true, - include_service_accounts: true, + describe('when used on groups', () => { + beforeEach(() => { + isProject = false; + wrapper = createComponent( + { usersFilter: 'saml_provider_id', isProject }, + { groupUserSaml: true }, + ); + + findTokenSelector().vm.$emit('text-input', searchParam); + }); + + it('calls the group API with correct parameters', () => { + expect(UserApi.getGroupUsers).toHaveBeenCalledWith( + searchParam, + isProject, + groupOrProjectId, + { + active: true, + include_saml_users: true, + include_service_accounts: true, + }, + ); }); }); }); -- GitLab