diff --git a/doc/api/members.md b/doc/api/members.md index 21ed6db724590c3f155bcde6ed5a0d6e5fd454fa..1db9714bfd1d36f99030e65bd6e2163cee25b185 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -483,6 +483,35 @@ 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 in a group + +> [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 +``` + +| 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" +``` + +Example response: + +```json +{ + "success":true +} +``` + ## Add a member to a group or project Adds a member to a group or project. diff --git a/ee/app/assets/javascripts/api/groups_api.js b/ee/app/assets/javascripts/api/groups_api.js index 8457995dd55c8a36c33ea390bffedd1ef3720af0..635b04bdbf3a14e875dff5110d35d470c24670f0 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 0472afc305b6a7a840d64e0335f4bdccfdd0a433..5ed9e19816c9a752bfbd1306a0605df1cc60af84 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 c38c17958b6c2029298666ec82045008d558c7fb..a4c0c8552225babf4880a5fc1cbfeed5810216a3 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 23e719b38bd87729ce7f5789a689903e829bd694..e3afa1f65db64a4ed46e8a55a543981611bf9bce 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 119b6ba41197eb77a5aee4c67abf93316b4e3ce4..00f4b3689a53a1f8f8db65f121efd25bf8e51a9a 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/lib/ee/api/members.rb b/ee/lib/ee/api/members.rb index cdebb7c9f3dac0244915ffb0a6a704e2b8cd45d8..f4ec644d1ca9bcccd7d47a366b67b66e4dcdc4eb 100644 --- a/ee/lib/ee/api/members.rb +++ b/ee/lib/ee/api/members.rb @@ -142,6 +142,31 @@ module Members awaiting_user_ids: result[:awaiting_user_ids] end + 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 = find_user(params[:user_id]) + not_found!('User') unless user + + 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) + + 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 + unprocessable_entity!(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/frontend/api/groups_api_spec.js b/ee/spec/frontend/api/groups_api_spec.js index 6946f2217edb021ad5582e26eaabb465e3733b77..6ea2574150e0c74cf6973ca231a09baa3f981cae 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 a08bdb18d9c3801a64426b65265ab2881b616fa9..54b4ce43387122adba56a9ec1de6f6898864869b 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 3b460dd349986d9903edd8d2e26e5815d9dc566e..01c7b176597af361cbf300264c7f795adafd5a1c 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]; diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index bb342e598ec01f0cca248104e31ee8a05ed3f989..233027045e4be6b2b05ed4c8c1b8754cd4f29c74 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -649,6 +649,117 @@ 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) { put api(url, current_user), params: params } + + 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(:unprocessable_entity) + expect(json_response['message']).to eq "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/0/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 User 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 + end + describe 'DELETE /groups/:id/billable_members/:user_id' do context 'when the current user has insufficient rights' do it 'returns 400' do