From f424ad3ddd897aea6ffaee853fc6fd3f1e804578 Mon Sep 17 00:00:00 2001 From: Nicolas Dular Date: Tue, 5 Apr 2022 14:28:44 +0200 Subject: [PATCH 1/5] Add API to set membership state This API will be used for the Usage Quota page to toggle the membership state on free groups. It changes the membership states for all related memberships of a user to the given group to either `awaiting` or `active`. Changelog: added EE: true --- doc/api/members.md | 18 +++++ ee/lib/ee/api/members.rb | 24 +++++++ ee/spec/requests/api/members_spec.rb | 103 +++++++++++++++++++++++++++ 3 files changed, 145 insertions(+) diff --git a/doc/api/members.md b/doc/api/members.md index 21ed6db724590c..c2b335566ad9cb 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -483,6 +483,24 @@ DELETE /groups/:id/billable_members/:user_id curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/:id/billable_members/:user_id" ``` +## Change membership state of a user from a group + +Changes the state of all memberships of a user from a group and its subgroups and projects. + +```plaintext +PUT /groups/:id/members/:user_id/state +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | +| `user_id` | integer | yes | The user ID of the member | +| `state` | String | yes | The new state for the user. State is either `awaiting` or `active` | + +```shell +curl --request PUT --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/:id/members/:user_id/state?state=active" +``` + ## Add a member to a group or project Adds a member to a group or project. diff --git a/ee/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb index cdebb7c9f3dac0..732e3fb0999e5b 100644 --- a/ee/lib/ee/api/members.rb +++ b/ee/lib/ee/api/members.rb @@ -142,6 +142,30 @@ module Members awaiting_user_ids: result[:awaiting_user_ids] end + desc 'Deactivates the the memberships of a user in the group' + params do + requires :user_id, type: Integer, desc: 'The user ID of the user' + requires :state, type: String, values: %w(awaiting active), desc: 'The new state for the memberships of the user' + end + put ":id/members/:user_id/state", feature_category: :user_management do + user = ::User.find(params[:user_id]) + bad_request! unless user + + group = find_group!(params[:id]) + bad_request! unless group.root? + bad_request! unless can?(current_user, :admin_group_member, group) + + service = params[:state] == 'active' ? ::Members::ActivateService : ::Members::AwaitService + + result = service.new(group, current_user: current_user, user: user).execute + + if result[:status] == :success + { success: true } + else + bad_request!(result[:message]) + end + end + desc 'Get the memberships of a billable user of a root group.' do success ::EE::API::Entities::BillableMembership end diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index bb342e598ec01f..c5df5ad9a4f497 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -649,6 +649,109 @@ end end + describe 'PUT /groups/:id/members/:user_id/state', :saas do + let(:url) { "/groups/#{group.id}/members/#{user.id}/state" } + let(:state) { 'active' } + let(:params) { { state: state } } + + let_it_be(:user) { create(:user) } + + subject(:change_state) do + put api(url, current_user), params: params + + json_response + end + + context 'when the current user has insufficient rights' do + let(:current_user) { create(:user) } + + it 'returns 400' do + change_state + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when authenticated as an owner' do + let(:current_user) { owner } + + context 'when setting the user to be active' do + let(:state) { 'active' } + + it 'is successful' do + member = create(:group_member, :awaiting, group: group, user: user) + + change_state + + expect(response).to have_gitlab_http_status(:success) + expect(member.reload).to be_active + end + end + + context 'when setting the user to be awaiting' do + let(:state) { 'awaiting' } + + it 'is successful' do + member = create(:group_member, :active, group: group, user: user) + + change_state + + expect(response).to have_gitlab_http_status(:success) + expect(member.reload).to be_awaiting + end + end + + it 'forwards the error from the service' do + change_state + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq "400 Bad request - No memberships found" + end + + context 'with invalid parameters' do + let(:state) { 'non-existing-state' } + + it 'returns a relevant error message' do + change_state + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'state does not have a valid value' + end + end + + context 'with a group that does not exist' do + let(:url) { "/groups/foo/members/#{user.id}/state" } + + it 'returns a relevant error message' do + change_state + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq '404 Group Not Found' + end + end + + context 'with a group that is a sub-group' do + let(:url) { "/groups/#{nested_group.id}/members/#{user.id}/state" } + + it 'returns a relevant error message' do + change_state + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'with a user that does not exist' do + let(:url) { "/groups/#{group.id}/members/foo/state" } + + it 'returns a relevant error message' do + change_state + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + end + describe 'DELETE /groups/:id/billable_members/:user_id' do context 'when the current user has insufficient rights' do it 'returns 400' do -- GitLab From b9178c75fc32d0d4a0305cfad32aea9e3f0b0686 Mon Sep 17 00:00:00 2001 From: Nicolas Dular Date: Thu, 5 May 2022 20:40:02 +0200 Subject: [PATCH 2/5] Enable toggling membership state This connects the toggles to the API to change the membership state of a user and set them to active or awaiting. --- ee/app/assets/javascripts/api/groups_api.js | 9 ++ .../seats/components/subscription_seats.vue | 11 ++- .../usage_quotas/seats/store/actions.js | 27 ++++- .../seats/store/mutation_types.js | 3 + .../usage_quotas/seats/store/mutations.js | 10 ++ ee/spec/frontend/api/groups_api_spec.js | 24 +++++ .../components/subscription_seats_spec.js | 35 ++++++- .../usage_quotas/seats/store/actions_spec.js | 98 +++++++++++++++++++ 8 files changed, 213 insertions(+), 4 deletions(-) diff --git a/ee/app/assets/javascripts/api/groups_api.js b/ee/app/assets/javascripts/api/groups_api.js index 8457995dd55c8a..635b04bdbf3a14 100644 --- a/ee/app/assets/javascripts/api/groups_api.js +++ b/ee/app/assets/javascripts/api/groups_api.js @@ -6,6 +6,7 @@ const GROUPS_BILLABLE_MEMBERS_SINGLE_PATH = '/api/:version/groups/:group_id/bill const GROUPS_BILLABLE_MEMBERS_PATH = '/api/:version/groups/:id/billable_members'; const GROUPS_BILLABLE_MEMBERS_SINGLE_MEMBERSHIPS_PATH = '/api/:version/groups/:group_id/billable_members/:member_id/memberships'; +const GROUPS_MEMBERS_CHANGE_STATE_PATH = '/api/:version/groups/:group_id/members/:user_id/state'; export const fetchBillableGroupMembersList = (namespaceId, options = {}) => { const url = buildApiUrl(GROUPS_BILLABLE_MEMBERS_PATH).replace(':id', namespaceId); @@ -38,6 +39,14 @@ export const removeBillableMemberFromGroup = (groupId, memberId) => { return axios.delete(url); }; +export const changeMembershipState = (groupId, userId, state) => { + const url = buildApiUrl(GROUPS_MEMBERS_CHANGE_STATE_PATH) + .replace(':group_id', groupId) + .replace(':user_id', userId); + + return axios.put(url, { state }); +}; + const GROUPS_PENDING_MEMBERS_PATH = '/api/:version/groups/:id/pending_members'; const GROUPS_PENDING_MEMBERS_STATE = 'awaiting'; diff --git a/ee/app/assets/javascripts/usage_quotas/seats/components/subscription_seats.vue b/ee/app/assets/javascripts/usage_quotas/seats/components/subscription_seats.vue index 0472afc305b6a7..5ed9e19816c9a7 100644 --- a/ee/app/assets/javascripts/usage_quotas/seats/components/subscription_seats.vue +++ b/ee/app/assets/javascripts/usage_quotas/seats/components/subscription_seats.vue @@ -157,6 +157,7 @@ export default { 'fetchGitlabSubscription', 'resetBillableMembers', 'setBillableMemberToRemove', + 'changeMembershipState', 'setSearchQuery', 'setCurrentPage', 'setSortOption', @@ -191,10 +192,15 @@ export default { visitUrl(this.pendingMembersPagePath); }, isToggleDisabled(user) { - return this.hasReachedFreePlanLimit && user.membership_state === MEMBER_AWAITING_STATE; + return this.isLoading || this.restrictActivatingUser(user); }, toggleTooltipText(user) { - return this.isToggleDisabled(user) ? this.$options.i18n.activateMemberRestrictedText : null; + return this.restrictActivatingUser(user) + ? this.$options.i18n.activateMemberRestrictedText + : ''; + }, + restrictActivatingUser(user) { + return this.hasReachedFreePlanLimit && user.membership_state === MEMBER_AWAITING_STATE; }, membershipStateToggleValue(user) { return user.membership_state === MEMBER_ACTIVE_STATE; @@ -357,6 +363,7 @@ export default { :value="membershipStateToggleValue(user)" :disabled="isToggleDisabled(user)" :title="toggleTooltipText(user)" + @change="changeMembershipState(user)" /> diff --git a/ee/app/assets/javascripts/usage_quotas/seats/store/actions.js b/ee/app/assets/javascripts/usage_quotas/seats/store/actions.js index c38c17958b6c20..a4c0c8552225ba 100644 --- a/ee/app/assets/javascripts/usage_quotas/seats/store/actions.js +++ b/ee/app/assets/javascripts/usage_quotas/seats/store/actions.js @@ -1,7 +1,8 @@ import * as GroupsApi from 'ee/api/groups_api'; import Api from 'ee/api'; import { createAlert, VARIANT_SUCCESS } from '~/flash'; -import { s__ } from '~/locale'; +import { s__, __ } from '~/locale'; +import { MEMBER_ACTIVE_STATE, MEMBER_AWAITING_STATE } from '../constants'; import * as types from './mutation_types'; export const fetchBillableMembersList = ({ commit, dispatch, state }) => { @@ -55,6 +56,30 @@ export const setBillableMemberToRemove = ({ commit }, member) => { commit(types.SET_BILLABLE_MEMBER_TO_REMOVE, member); }; +export const changeMembershipState = async ({ commit, dispatch, state }, user) => { + commit(types.CHANGE_MEMBERSHIP_STATE); + + try { + const newState = + user.membership_state === MEMBER_ACTIVE_STATE ? MEMBER_AWAITING_STATE : MEMBER_ACTIVE_STATE; + + await GroupsApi.changeMembershipState(state.namespaceId, user.id, newState); + + await dispatch('fetchBillableMembersList'); + await dispatch('fetchGitlabSubscription'); + } catch { + dispatch('changeMembershipStateError'); + } +}; + +export const changeMembershipStateError = ({ commit }) => { + createAlert({ + message: __('Something went wrong. Please try again.'), + }); + + commit(types.CHANGE_MEMBERSHIP_STATE_ERROR); +}; + export const removeBillableMember = ({ dispatch, state }) => { return GroupsApi.removeBillableMemberFromGroup(state.namespaceId, state.billableMemberToRemove.id) .then(() => dispatch('removeBillableMemberSuccess')) diff --git a/ee/app/assets/javascripts/usage_quotas/seats/store/mutation_types.js b/ee/app/assets/javascripts/usage_quotas/seats/store/mutation_types.js index 23e719b38bd877..e3afa1f65db64a 100644 --- a/ee/app/assets/javascripts/usage_quotas/seats/store/mutation_types.js +++ b/ee/app/assets/javascripts/usage_quotas/seats/store/mutation_types.js @@ -19,3 +19,6 @@ export const SET_BILLABLE_MEMBER_TO_REMOVE = 'SET_BILLABLE_MEMBER_TO_REMOVE'; export const FETCH_BILLABLE_MEMBER_DETAILS = 'FETCH_BILLABLE_MEMBER_DETAILS'; export const FETCH_BILLABLE_MEMBER_DETAILS_SUCCESS = 'FETCH_BILLABLE_MEMBER_DETAILS_SUCCESS'; export const FETCH_BILLABLE_MEMBER_DETAILS_ERROR = 'FETCH_BILLABLE_MEMBER_DETAILS_ERROR'; + +export const CHANGE_MEMBERSHIP_STATE = 'CHANGE_MEMBERSHIP_STATE'; +export const CHANGE_MEMBERSHIP_STATE_ERROR = 'CHANGE_MEMBERSHIP_STATE_ERROR'; diff --git a/ee/app/assets/javascripts/usage_quotas/seats/store/mutations.js b/ee/app/assets/javascripts/usage_quotas/seats/store/mutations.js index 119b6ba41197eb..00f4b3689a53a1 100644 --- a/ee/app/assets/javascripts/usage_quotas/seats/store/mutations.js +++ b/ee/app/assets/javascripts/usage_quotas/seats/store/mutations.js @@ -87,6 +87,16 @@ export default { } }, + [types.CHANGE_MEMBERSHIP_STATE](state) { + state.isLoading = true; + state.hasError = false; + }, + + [types.CHANGE_MEMBERSHIP_STATE_ERROR](state) { + state.isLoading = false; + state.hasError = true; + }, + [types.REMOVE_BILLABLE_MEMBER](state) { state.isLoading = true; state.hasError = false; diff --git a/ee/spec/frontend/api/groups_api_spec.js b/ee/spec/frontend/api/groups_api_spec.js index 6946f2217edb02..6ea2574150e0c7 100644 --- a/ee/spec/frontend/api/groups_api_spec.js +++ b/ee/spec/frontend/api/groups_api_spec.js @@ -103,4 +103,28 @@ describe('GroupsApi', () => { expect(axios.put).toHaveBeenCalledWith(expectedUrl); }); }); + + describe('changeMembershipState', () => { + const userId = 3; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${namespaceId}/members/${userId}/state`; + + beforeEach(() => { + jest.spyOn(axios, 'put'); + mock.onPut(expectedUrl).replyOnce(httpStatus.OK, []); + }); + + it('changes the state for the member to awaiting', async () => { + const { data } = await GroupsApi.changeMembershipState(namespaceId, userId, 'awaiting'); + + expect(data).toEqual([]); + expect(axios.put).toHaveBeenCalledWith(expectedUrl, { state: 'awaiting' }); + }); + + it('changes the state for the member to active', async () => { + const { data } = await GroupsApi.changeMembershipState(namespaceId, userId, 'active'); + + expect(data).toEqual([]); + expect(axios.put).toHaveBeenCalledWith(expectedUrl, { state: 'active' }); + }); + }); }); diff --git a/ee/spec/frontend/usage_quotas/seats/components/subscription_seats_spec.js b/ee/spec/frontend/usage_quotas/seats/components/subscription_seats_spec.js index a08bdb18d9c380..54b4ce43387122 100644 --- a/ee/spec/frontend/usage_quotas/seats/components/subscription_seats_spec.js +++ b/ee/spec/frontend/usage_quotas/seats/components/subscription_seats_spec.js @@ -29,6 +29,7 @@ const actionSpies = { resetBillableMembers: jest.fn(), setBillableMemberToRemove: jest.fn(), setSearchQuery: jest.fn(), + changeMembershipState: jest.fn(), }; const providedFields = { @@ -233,7 +234,7 @@ describe('Subscription Seats', () => { if (currentMember.user.membership_state === 'active') { expect(serializedRow.toggle.disabled).toBe(false); - expect(serializedRow.toggle.title).toBeUndefined(); + expect(serializedRow.toggle.title).toBe(''); expect(serializedRow.toggle.value).toBe(true); } }); @@ -254,6 +255,38 @@ describe('Subscription Seats', () => { } }); }); + + it('disables the toggles when isLoading=true', () => { + wrapper = createComponent({ + mountFn: mount, + initialGetters: { + tableItems: () => mockTableItems, + }, + initialState: { + isLoading: true, + hasNoSubscription: true, + hasLimitedFreePlan: true, + hasReachedFreePlanLimit: true, + }, + }); + + serializedTable = findSerializedTable(findTable()); + + serializedTable.forEach((serializedRow) => { + expect(serializedRow.toggle.disabled).toBe(true); + }); + }); + + it('calls the changeMembershipState action when clicking the toggle', () => { + const toggles = findTable().findComponent(GlToggle); + + toggles.vm.$emit('change', false); + + expect(actionSpies.changeMembershipState).toHaveBeenCalledWith( + expect.any(Object), + mockTableItems[0].user, + ); + }); }); }); }); diff --git a/ee/spec/frontend/usage_quotas/seats/store/actions_spec.js b/ee/spec/frontend/usage_quotas/seats/store/actions_spec.js index 3b460dd349986d..01c7b176597af3 100644 --- a/ee/spec/frontend/usage_quotas/seats/store/actions_spec.js +++ b/ee/spec/frontend/usage_quotas/seats/store/actions_spec.js @@ -2,6 +2,7 @@ import MockAdapter from 'axios-mock-adapter'; import * as GroupsApi from 'ee/api/groups_api'; import Api from 'ee/api'; import * as actions from 'ee/usage_quotas/seats/store/actions'; +import { MEMBER_ACTIVE_STATE, MEMBER_AWAITING_STATE } from 'ee/usage_quotas/seats/constants'; import * as types from 'ee/usage_quotas/seats/store/mutation_types'; import State from 'ee/usage_quotas/seats/store/state'; import { @@ -297,6 +298,103 @@ describe('seats actions', () => { }); }); + describe('changeMembershipState', () => { + let user; + let expectedActions; + let expectedMutations; + + beforeEach(() => { + state.namespaceId = 1; + user = { id: 2, membership_state: MEMBER_ACTIVE_STATE }; + }); + + afterEach(() => { + mock.reset(); + }); + + describe('on success', () => { + beforeEach(() => { + mock + .onPut('/api/v4/groups/1/members/2/state') + .replyOnce(httpStatusCodes.OK, { success: true }); + + expectedActions = [ + { type: 'fetchBillableMembersList' }, + { type: 'fetchGitlabSubscription' }, + ]; + + expectedMutations = [{ type: types.CHANGE_MEMBERSHIP_STATE }]; + }); + + describe('for an active user', () => { + it('passes correct arguments to Api call for an active user', () => { + const spy = jest.spyOn(GroupsApi, 'changeMembershipState'); + + jest.spyOn(GroupsApi, 'fetchBillableGroupMembersList'); + jest.spyOn(Api, 'userSubscription'); + + testAction({ + action: actions.changeMembershipState, + payload: user, + state, + expectedMutations, + expectedActions, + }); + + expect(spy).toBeCalledWith(state.namespaceId, user.id, MEMBER_AWAITING_STATE); + }); + }); + + describe('for an awaiting user', () => { + it('passes correct arguments to Api call for an active user', () => { + const spy = jest.spyOn(GroupsApi, 'changeMembershipState'); + + testAction({ + action: actions.changeMembershipState, + payload: { ...user, membership_state: MEMBER_AWAITING_STATE }, + state, + expectedMutations, + expectedActions, + }); + + expect(spy).toBeCalledWith(state.namespaceId, user.id, MEMBER_ACTIVE_STATE); + }); + }); + }); + + describe('on error', () => { + beforeEach(() => { + mock + .onPut('/api/v4/groups/1/members/2/state') + .replyOnce(httpStatusCodes.UNPROCESSABLE_ENTITY, {}); + }); + + it('should dispatch the request and error actions', async () => { + await testAction({ + action: actions.changeMembershipState, + payload: user, + state, + expectedMutations: [{ type: types.CHANGE_MEMBERSHIP_STATE }], + expectedActions: [{ type: 'changeMembershipStateError' }], + }); + }); + }); + }); + + describe('changeMembershipStateError', () => { + it('ccommits mutation and calls createAlert', async () => { + await testAction({ + action: actions.changeMembershipStateError, + state, + expectedMutations: [{ type: types.CHANGE_MEMBERSHIP_STATE_ERROR }], + }); + + expect(createAlert).toHaveBeenCalledWith({ + message: 'Something went wrong. Please try again.', + }); + }); + }); + describe('fetchBillableMemberDetails', () => { const member = mockDataSeats.data[0]; -- GitLab From 2477562e6b875ec8db9cc55a4271146d66f32cd4 Mon Sep 17 00:00:00 2001 From: Nicolas Dular Date: Tue, 10 May 2022 12:12:47 +0200 Subject: [PATCH 3/5] Implement BE review --- ee/lib/ee/api/members.rb | 8 ++++---- ee/spec/requests/api/members_spec.rb | 25 ++++++++++++++++--------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/ee/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb index 732e3fb0999e5b..766af932c40674 100644 --- a/ee/lib/ee/api/members.rb +++ b/ee/lib/ee/api/members.rb @@ -142,14 +142,14 @@ module Members awaiting_user_ids: result[:awaiting_user_ids] end - desc 'Deactivates the the memberships of a user in the group' + desc 'Changes the state of the memberships of a user in the group' params do requires :user_id, type: Integer, desc: 'The user ID of the user' requires :state, type: String, values: %w(awaiting active), desc: 'The new state for the memberships of the user' end put ":id/members/:user_id/state", feature_category: :user_management do - user = ::User.find(params[:user_id]) - bad_request! unless user + user = find_user(params[:user_id]) + not_found! unless user group = find_group!(params[:id]) bad_request! unless group.root? @@ -162,7 +162,7 @@ module Members if result[:status] == :success { success: true } else - bad_request!(result[:message]) + unprocessable_entity!(result[:message]) end end diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index c5df5ad9a4f497..e0aa3fd9952ba4 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -656,11 +656,7 @@ let_it_be(:user) { create(:user) } - subject(:change_state) do - put api(url, current_user), params: params - - json_response - end + subject(:change_state) { put api(url, current_user), params: params } context 'when the current user has insufficient rights' do let(:current_user) { create(:user) } @@ -704,8 +700,8 @@ it 'forwards the error from the service' do change_state - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq "400 Bad request - No memberships found" + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq "No memberships found" end context 'with invalid parameters' do @@ -741,12 +737,23 @@ end context 'with a user that does not exist' do - let(:url) { "/groups/#{group.id}/members/foo/state" } + let(:url) { "/groups/#{group.id}/members/0/state" } it 'returns a relevant error message' do change_state - expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with a user that is not a member of the group' do + it 'returns a relevant error message' do + create(:group_member, :awaiting, group: create(:group), user: user) + + change_state + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq "No memberships found" end end end -- GitLab From bcd970f8032cd0af3dae84a5199b80e209676a72 Mon Sep 17 00:00:00 2001 From: Nicolas Dular Date: Tue, 10 May 2022 22:59:20 +0200 Subject: [PATCH 4/5] Consistency when checking group and user --- ee/lib/ee/api/members.rb | 5 +++-- ee/spec/requests/api/members_spec.rb | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb index 766af932c40674..f4ec644d1ca9bc 100644 --- a/ee/lib/ee/api/members.rb +++ b/ee/lib/ee/api/members.rb @@ -149,9 +149,10 @@ module Members end put ":id/members/:user_id/state", feature_category: :user_management do user = find_user(params[:user_id]) - not_found! unless user + not_found!('User') unless user - group = find_group!(params[:id]) + group = find_group(params[:id]) + not_found!('Group') unless group bad_request! unless group.root? bad_request! unless can?(current_user, :admin_group_member, group) diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index e0aa3fd9952ba4..233027045e4be6 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -743,6 +743,7 @@ change_state expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq '404 User Not Found' end end -- GitLab From 1e80cb2151ea87c047fd57e6355cf2faa980b26c Mon Sep 17 00:00:00 2001 From: Nicolas Dular Date: Wed, 11 May 2022 09:10:01 +0200 Subject: [PATCH 5/5] Implement doc suggestions --- doc/api/members.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/doc/api/members.md b/doc/api/members.md index c2b335566ad9cb..1db9714bfd1d36 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -483,9 +483,12 @@ DELETE /groups/:id/billable_members/:user_id curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/:id/billable_members/:user_id" ``` -## Change membership state of a user from a group +## Change membership state of a user in a group -Changes the state of all memberships of a user from a group and its subgroups and projects. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86705) in GitLab 15.0. + +Changes the membership state of a user in a group. The state is applied to +all subgroups and projects. ```plaintext PUT /groups/:id/members/:user_id/state @@ -493,14 +496,22 @@ PUT /groups/:id/members/:user_id/state | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user | -| `user_id` | integer | yes | The user ID of the member | -| `state` | String | yes | The new state for the user. State is either `awaiting` or `active` | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) owned by the authenticated user. | +| `user_id` | integer | yes | The user ID of the member. | +| `state` | string | yes | The new state for the user. State is either `awaiting` or `active`. | ```shell curl --request PUT --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/:id/members/:user_id/state?state=active" ``` +Example response: + +```json +{ + "success":true +} +``` + ## Add a member to a group or project Adds a member to a group or project. -- GitLab