diff --git a/app/assets/javascripts/invite_members/components/import_project_members_modal.vue b/app/assets/javascripts/invite_members/components/import_project_members_modal.vue index 5599ad276f07745ab41f767e0510b5f0ccab94f8..887527bab236ba935a59f6cf5dec8a430327d950 100644 --- a/app/assets/javascripts/invite_members/components/import_project_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/import_project_members_modal.vue @@ -177,8 +177,10 @@ export default { } else { this.onInviteSuccess(); } - } catch { - this.showErrorAlert(); + } catch (error) { + const message = error.response.data?.message; + + this.showErrorAlert(message); } finally { this.isLoading = false; this.projectToBeImported = {}; @@ -201,8 +203,8 @@ export default { this.$toast.show(this.$options.i18n.successMessage, this.$options.toastOptions); this.closeModal(); }, - showErrorAlert() { - this.invalidFeedbackMessage = this.$options.i18n.defaultError; + showErrorAlert(message) { + this.invalidFeedbackMessage = message || this.$options.i18n.defaultError; }, onCancel() { this.track('click_cancel'); diff --git a/app/services/members/import_project_team_service.rb b/app/services/members/import_project_team_service.rb index ef43d8206a97fafcc88207aa2ac0b630b3c728ac..97b64d6e700ee0d187e06246899d167b353c5c50 100644 --- a/app/services/members/import_project_team_service.rb +++ b/app/services/members/import_project_team_service.rb @@ -3,6 +3,7 @@ module Members class ImportProjectTeamService < BaseService ImportProjectTeamForbiddenError = Class.new(StandardError) + SeatLimitExceededError = Class.new(StandardError) def initialize(*args) super @@ -13,12 +14,13 @@ def initialize(*args) def execute check_target_and_source_projects_exist! check_user_permissions! + check_seats! import_project_team process_import_result result - rescue ArgumentError, ImportProjectTeamForbiddenError => e + rescue ArgumentError, ImportProjectTeamForbiddenError, SeatLimitExceededError => e ServiceResponse.error(message: e.message, reason: :unprocessable_entity) end @@ -44,6 +46,10 @@ def check_target_and_source_projects_exist! end end + def check_seats! + # Overridden in EE + end + def check_user_permissions! return if can?(current_user, :read_project_member, source_project) && can?(current_user, :import_project_members_from_another_project, target_project) @@ -82,3 +88,5 @@ def source_project end end end + +Members::ImportProjectTeamService.prepend_mod_with('Members::ImportProjectTeamService') diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 475277ad7f6ac6c95d97861ba1b280aa3223c5e8..8e0323c7125c4ba74337bd53383ebbf371e817ad 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -555,7 +555,7 @@ def seats_available_for?(invites) billable_ids = billed_user_ids[:user_ids].map(&:to_s) - new_invites = invites - billable_ids + new_invites = invites.map(&:to_s) - billable_ids gitlab_subscription.seats >= (billable_ids.count + new_invites.count) end diff --git a/ee/app/services/ee/members/import_project_team_service.rb b/ee/app/services/ee/members/import_project_team_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..6308d1b4133d0f45d864cb075dda82819bd9f8f2 --- /dev/null +++ b/ee/app/services/ee/members/import_project_team_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module EE + module Members + module ImportProjectTeamService + extend ::Gitlab::Utils::Override + + override :check_seats! + def check_seats! + root_namespace = target_project.root_ancestor + invited_user_ids = source_project.project_members.pluck_user_ids + + return unless root_namespace.block_seat_overages? && !root_namespace.seats_available_for?(invited_user_ids) + + raise ::Members::ImportProjectTeamService::SeatLimitExceededError, error_message + end + + private + + def error_message + messages = [ + s_('AddMember|There are not enough available seats to invite this many users.') + ] + + unless can?(current_user, :owner_access, target_project.root_ancestor) + messages << s_('AddMember|Ask a user with the Owner role to purchase more seats.') + end + + messages.join(" ") + end + end + end +end diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 60c24cd6495cb4f6e8362364de7d5d3c66c1917c..9db1c13e9993303818fd8363db9079a6ef17ca37 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -3755,6 +3755,12 @@ def webhook_headers expect(group.seats_available_for?([])).to eq(true) end + + it 'accepts an array of integers' do + user_ids = [1, 2, 3] + + expect(group.seats_available_for?(user_ids)).to eq(true) + end end context 'without a subscription' do diff --git a/ee/spec/services/ee/members/import_project_team_service_spec.rb b/ee/spec/services/ee/members/import_project_team_service_spec.rb index 44da33a4f3ece08bb290d88f953ca31abb253a83..a0d891d5582f53638c8c891f73157a5c044031a6 100644 --- a/ee/spec/services/ee/members/import_project_team_service_spec.rb +++ b/ee/spec/services/ee/members/import_project_team_service_spec.rb @@ -4,8 +4,9 @@ RSpec.describe Members::ImportProjectTeamService, feature_category: :groups_and_projects do describe '#execute' do + let_it_be(:group) { create(:group) } let_it_be(:source_project) { create(:project) } - let_it_be(:target_project) { create(:project, group: create(:group)) } + let_it_be(:target_project, refind: true) { create(:project, group: group) } let_it_be(:user) { create(:user) } let(:source_project_id) { source_project.id } @@ -51,5 +52,112 @@ end end end + + context 'when block seat overages is enabled', :saas, :use_clean_rails_memory_store_caching do + let_it_be(:subscription) { create(:gitlab_subscription, :premium, namespace: group, seats: 2) } + + let(:owner_message) { s_('AddMember|There are not enough available seats to invite this many users.') } + let(:maintainer_message) do + s_('AddMember|There are not enough available seats to invite this many users. ' \ + 'Ask a user with the Owner role to purchase more seats.') + end + + before do + stub_feature_flags(block_seat_overages: group) + end + + context 'when there are no seats left in the subscription' do + before_all do + group.add_developer(create(:user)) + end + + it 'rejects the additional members' do + result = import.execute + + expect(result.error?).to be(true) + expect(result.message).to eq(maintainer_message) + expect(result.reason).to eq(:unprocessable_entity) + expect(target_project.reload.members.map(&:user_id)).to contain_exactly(user.id) + end + + context 'when the user is a group owner' do + before_all do + group.add_owner(user) + end + + it 'rejects the additional members with a message for the owner' do + result = import.execute + + expect(result.error?).to be(true) + expect(result.message).to eq(owner_message) + expect(result.reason).to eq(:unprocessable_entity) + expect(target_project.reload.members.map(&:user_id)).to contain_exactly(user.id) + end + end + end + + context 'when there are seats left in the subscription' do + it 'imports the additional members' do + result = import.execute + + expect(result.success?).to be(true) + expect(result.message).to eq('Successfully imported') + expect(target_project.reload.members.map(&:user_id)).to contain_exactly(user.id, source_project.owner.id) + end + + context 'when importing more members than there are seats remaining' do + before_all do + source_project.add_developer(create(:user)) + end + + it 'rejects all the members' do + result = import.execute + + expect(result.error?).to be(true) + expect(result.message).to eq(maintainer_message) + expect(result.reason).to eq(:unprocessable_entity) + expect(target_project.reload.members.map(&:user_id)).to contain_exactly(user.id) + end + end + end + + context 'with a target project in a subgroup' do + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:subgroup_project) { create(:project, group: subgroup) } + + let(:target_project_id) { subgroup_project.id } + + before_all do + subgroup_project.add_maintainer(user) + end + + it 'rejects the additional members when there are not enough seats left in the subscription' do + group.add_developer(create(:user)) + + result = import.execute + + expect(result.error?).to be(true) + expect(result.message).to eq(maintainer_message) + expect(result.reason).to eq(:unprocessable_entity) + expect(subgroup_project.reload.members.map(&:user_id)).to contain_exactly(user.id) + end + end + end + + context 'when block seat overages is disabled' do + before do + stub_feature_flags(block_seat_overages: false) + end + + it 'imports the additional members even if there are no seats left in the subscription' do + group.add_developer(create(:user)) + + result = import.execute + + expect(result.success?).to be(true) + expect(result.message).to eq('Successfully imported') + expect(target_project.reload.members.map(&:user_id)).to contain_exactly(user.id, source_project.owner.id) + end + end end end diff --git a/spec/features/projects/members/import_project_members_spec.rb b/spec/features/projects/members/import_project_members_spec.rb index 20cf42cd1358d1ee9afe9bd9653ad50a1f571f43..890ab5ab8baba89a95b818509d37e61f75f12a13 100644 --- a/spec/features/projects/members/import_project_members_spec.rb +++ b/spec/features/projects/members/import_project_members_spec.rb @@ -41,7 +41,7 @@ submit_import { project2.destroy! } within import_project_members_modal_selector do - expect(page).to have_content('Unable to import project members') + expect(page).to have_content('404 Project Not Found') end end diff --git a/spec/frontend/invite_members/components/import_project_members_modal_spec.js b/spec/frontend/invite_members/components/import_project_members_modal_spec.js index 4fac21f7a576cda4e85bac88120eccb771d2d3d1..79ba58bbb3dfc3854b902b23f5377fe0e9a6037f 100644 --- a/spec/frontend/invite_members/components/import_project_members_modal_spec.js +++ b/spec/frontend/invite_members/components/import_project_members_modal_spec.js @@ -291,6 +291,26 @@ describe('ImportProjectMembersModal', () => { }); }); + describe('when the api error includes an error message', () => { + beforeEach(async () => { + createComponent(); + + findProjectSelect().vm.$emit('input', projectToBeImported); + + jest.spyOn(ProjectsApi, 'importProjectMembers').mockRejectedValue({ + response: { data: { success: false, message: 'Failure message' } }, + }); + + clickImportButton(); + await waitForPromises(); + }); + + it('displays the error message from the api', () => { + expect(formGroupInvalidFeedback()).toBe('Failure message'); + expect(formGroupErrorState()).toBe(false); + }); + }); + describe('when the import fails with member import errors', () => { const mockInvitationsApi = (code, data) => { mock.onPost(IMPORT_PROJECT_MEMBERS_PATH).reply(code, data);