diff --git a/ee/app/assets/javascripts/invite_members/check_overage.js b/ee/app/assets/javascripts/invite_members/check_overage.js index 7cf768a1bbf773f7355107061cf20b09eaf10c9f..d3031825d64df71ca3c36fd3a6764322e69be770 100644 --- a/ee/app/assets/javascripts/invite_members/check_overage.js +++ b/ee/app/assets/javascripts/invite_members/check_overage.js @@ -14,20 +14,32 @@ import { difference } from 'lodash'; * @param {Array} billedUserIds Array of ids of already billed users * @param {Array} billedUserEmails Array of emails of already billed users * @param {Boolean} isFreePlan - * @param {Array} usersToInviteByEmail Array emails of users to be invited by email - * @param {Array} usersToAddById Array ids of users to be invited by id - * @param {Number} groupIdToInvite namespaceId of the added group + * @param {Boolean} excludeGuests Doesn't calculate guests as part of billed users + * @param {Object} invitedMembersData Data of the invited members + * @param {Boolean} isGuestRole Is true if the chosen role is Guest + * @param {Array} usersToInviteByEmail Array emails of users to be invited by email + * @param {Array} usersToAddById Array ids of users to be invited by id * * @returns {Object} */ export const checkOverage = ( - { subscriptionSeats, maxSeatsUsed, seatsInUse, billedUserIds, billedUserEmails, isFreeGroup }, - usersToAddById, - usersToInviteByEmail, + { + subscriptionSeats, + maxSeatsUsed, + seatsInUse, + billedUserIds, + billedUserEmails, + isFreeGroup, + excludeGuests, + }, + { isGuestRole, usersToAddById, usersToInviteByEmail }, ) => { + // overage is not calculate when adding guests to ultimate-like groups + const isExcludingGuests = isGuestRole && excludeGuests; + // overage only possible on paid plans - if (isFreeGroup) { + if (isFreeGroup || isExcludingGuests) { return { usersOverage: null, hasOverage: false }; } diff --git a/ee/app/assets/javascripts/invite_members/components/invite_modal_base.vue b/ee/app/assets/javascripts/invite_members/components/invite_modal_base.vue index 1238800da4cd7a82f1278b67f8769218fa3f285c..b1a37597924ca73326ac106a482d07556f62dc1e 100644 --- a/ee/app/assets/javascripts/invite_members/components/invite_modal_base.vue +++ b/ee/app/assets/javascripts/invite_members/components/invite_modal_base.vue @@ -154,11 +154,13 @@ export default { [usersToInviteByEmail, usersToAddById] = this.partitionNewUsersToInvite(); } - const { hasOverage, usersOverage } = checkOverage( - subscriptionData, + const isGuestRole = args.accessLevel === this.$attrs['access-levels'].Guest; + + const { hasOverage, usersOverage } = checkOverage(subscriptionData, { + isGuestRole, usersToAddById, usersToInviteByEmail, - ); + }); this.isLoading = false; this.hasOverage = hasOverage; diff --git a/ee/app/assets/javascripts/invite_members/get_subscription_data.js b/ee/app/assets/javascripts/invite_members/get_subscription_data.js index 18327f0a1fa32086529955c5446bedda2a70ed85..759cd80bcc50dd2dc7b3e06aaee1fd8cb6161189 100644 --- a/ee/app/assets/javascripts/invite_members/get_subscription_data.js +++ b/ee/app/assets/javascripts/invite_members/get_subscription_data.js @@ -18,5 +18,6 @@ export const fetchSubscription = async (namespaceId) => { maxSeatsUsed: data.usage.max_seats_used, seatsInUse: data.usage.seats_in_use, isFreeGroup: isFreeGroup(data.plan), + excludeGuests: data.plan.exclude_guests || false, }; }; diff --git a/ee/spec/features/groups/members/manage_groups_spec.rb b/ee/spec/features/groups/members/manage_groups_spec.rb index c8c0535930d4158847e202b885e3c706360660fd..002f9d8a83f37bda1f2881cd4465dbe84752cc63 100644 --- a/ee/spec/features/groups/members/manage_groups_spec.rb +++ b/ee/spec/features/groups/members/manage_groups_spec.rb @@ -13,14 +13,15 @@ let_it_be(:group_to_add) { create(:group) } let(:premium_plan) { create(:premium_plan) } + let(:ultimate_plan) { create(:premium_plan) } - shared_examples "adding a group doesn't trigger an overage modal" do + shared_examples "doesn't trigger an overage modal when adding a group with a given role" do |role| it do group.add_owner(user) group_to_add.add_owner(user) visit group_group_members_path(group) - add_group(group_to_add.name, role: 'Reporter') + add_group(group_to_add.name, role) wait_for_requests @@ -30,6 +31,23 @@ click_groups_tab + page.within(first_row) do + expect(page).to have_content(group_to_add.name) + expect(page).to have_content(role) + end + end + end + + shared_examples "triggers an overage modal when adding a group as Reporter" do + it do + add_group_with_one_extra_user + click_button 'Continue' + + wait_for_requests + page.refresh + + click_groups_tab + page.within(first_row) do expect(page).to have_content(group_to_add.name) expect(page).to have_content('Reporter') @@ -47,7 +65,7 @@ allow(group).to receive(:paid?).and_return(false) end - it_behaves_like "adding a group doesn't trigger an overage modal" + it_behaves_like "doesn't trigger an overage modal when adding a group with a given role", 'Reporter' end context 'for a premium group', :aggregate_failures do @@ -55,22 +73,7 @@ create(:gitlab_subscription, namespace: group, hosted_plan: premium_plan, seats: 1, seats_in_use: 0) end - context 'when there is an not yet billed user in the additional group' do - it 'triggers overage modal' do - add_group_with_one_extra_user - click_button 'Continue' - - wait_for_requests - page.refresh - - click_groups_tab - - page.within(first_row) do - expect(page).to have_content(group_to_add.name) - expect(page).to have_content('Reporter') - end - end - end + it_behaves_like "triggers an overage modal when adding a group as Reporter" context 'when overage modal is shown' do it 'goes back to the initial modal if not confirmed' do @@ -86,7 +89,16 @@ end end - def add_group(name, role: 'Guest', expires_at: nil) + context 'for an ultimate group', :aggregate_failures do + before do + create(:gitlab_subscription, namespace: group, hosted_plan: ultimate_plan, seats: 1, seats_in_use: 0) + end + + it_behaves_like "triggers an overage modal when adding a group as Reporter" + it_behaves_like "doesn't trigger an overage modal when adding a group with a given role", 'Guest' + end + + def add_group(name, role, expires_at: nil) click_on 'Invite a group' click_on 'Select a group' @@ -103,7 +115,7 @@ def add_group_with_one_extra_user group_to_add.add_developer(user2) visit group_group_members_path(group) - add_group(group_to_add.name, role: 'Reporter') + add_group(group_to_add.name, 'Reporter') wait_for_requests diff --git a/ee/spec/features/groups/members/manage_members_spec.rb b/ee/spec/features/groups/members/manage_members_spec.rb index 5db2e87a55b6f7b03dcf44495800978305d688e0..41ea606742538aaaf722a1555cb70f61a8d3cf77 100644 --- a/ee/spec/features/groups/members/manage_members_spec.rb +++ b/ee/spec/features/groups/members/manage_members_spec.rb @@ -13,17 +13,37 @@ let_it_be(:group) { create(:group) } let(:premium_plan) { create(:premium_plan) } + let(:ultimate_plan) { create(:ultimate_plan) } - shared_examples "adding one user doesn't trigger an overage modal" do + shared_examples "adding one user with a given role doesn't trigger an overage modal" do |role| it do group.add_owner(user1) - add_user_by_name(user2.name, 'Developer') + add_user_by_name(user2.name, role) expect(page).not_to have_content("You are about to incur additional charges") wait_for_requests page.refresh + page.within(second_row) do + expect(page).to have_content(user2.name) + expect(page).to have_button(role) + end + end + end + + shared_examples "shows an overage for one Developer added and invites them to a group if confirmed" do + it do + group.add_owner(user1) + add_user_by_name(user2.name, 'Developer') + + expect(page).to have_content("You are about to incur additional charges") + expect(page).to have_content("Your subscription includes 1 seat. If you continue, the #{group.name} group will have 2 seats in use and will be billed for the overage. Learn more.") + + click_button 'Continue' + + page.refresh + page.within(second_row) do expect(page).to have_content(user2.name) expect(page).to have_button('Developer') @@ -41,7 +61,7 @@ create(:gitlab_subscription, namespace: group, hosted_plan: nil) end - it_behaves_like "adding one user doesn't trigger an overage modal" + it_behaves_like "adding one user with a given role doesn't trigger an overage modal", 'Developer' end context 'when adding a member to a premium group' do @@ -50,7 +70,7 @@ create(:gitlab_subscription, namespace: group, hosted_plan: premium_plan, seats: 2, seats_in_use: 1) end - it_behaves_like "adding one user doesn't trigger an overage modal" + it_behaves_like "adding one user with a given role doesn't trigger an overage modal", 'Developer' it 'adding two users triggers overage modal', :aggregate_failures do group.add_owner(user1) @@ -77,22 +97,7 @@ create(:gitlab_subscription, namespace: group, hosted_plan: premium_plan, seats: 1, seats_in_use: 1) end - it 'invites a member to a group if confirmed', :aggregate_failures do - group.add_owner(user1) - add_user_by_name(user2.name, 'Developer') - - expect(page).to have_content("You are about to incur additional charges") - expect(page).to have_content("Your subscription includes 1 seat. If you continue, the #{group.name} group will have 2 seats in use and will be billed for the overage. Learn more.") - - click_button 'Continue' - - page.refresh - - page.within(second_row) do - expect(page).to have_content(user2.name) - expect(page).to have_button('Developer') - end - end + it_behaves_like "shows an overage for one Developer added and invites them to a group if confirmed" it 'get back to initial modal if not confirmed', :aggregate_failures do group.add_owner(user1) @@ -113,6 +118,15 @@ end end + context 'when adding a member to a ultimate group with no places left' do + before do + create(:gitlab_subscription, namespace: group, hosted_plan: ultimate_plan, seats: 1, seats_in_use: 1) + end + + it_behaves_like "shows an overage for one Developer added and invites them to a group if confirmed" + it_behaves_like "adding one user with a given role doesn't trigger an overage modal", 'Guest' + end + def add_user_by_name(name, role) visit group_group_members_path(group) diff --git a/ee/spec/frontend/billings/mock_data.js b/ee/spec/frontend/billings/mock_data.js index 514ac827e115a4015f63ac38c9c726afa9153459..708261d2bc1c449176be02d5465a4531169605a0 100644 --- a/ee/spec/frontend/billings/mock_data.js +++ b/ee/spec/frontend/billings/mock_data.js @@ -5,6 +5,7 @@ export const mockDataSubscription = { code: 'gold', trial: false, upgradable: false, + exclude_guests: true, }, usage: { seats_in_subscription: 100, @@ -25,6 +26,7 @@ export const mockDataSubscription = { code: null, trial: null, upgradable: null, + exclude_guests: null, }, usage: { seats_in_subscription: 0, @@ -44,12 +46,14 @@ export const mockDataSubscription = { code: 'gold', trial: true, upgradable: false, + exclude_guests: false, }, usage: { seats_in_subscription: 100, seats_in_use: 1, max_seats_used: 0, seats_owed: 0, + exclude_guests: false, }, billing: { subscription_start_date: '2018-12-13', diff --git a/ee/spec/frontend/invite_members/check_overage_spec.js b/ee/spec/frontend/invite_members/check_overage_spec.js index 3696c717a34baf01395f23092cd3613797a4a3e3..86a13cb4b10de37619f7239c6f98aa9476376c27 100644 --- a/ee/spec/frontend/invite_members/check_overage_spec.js +++ b/ee/spec/frontend/invite_members/check_overage_spec.js @@ -4,37 +4,52 @@ import { oneFreeSeatSubscription, noFreePlacesSubscription, subscriptionWithOverage, + allowGuestsSubscription, + generateInvitedUsersData, } from './mock_data'; +const secondUserAddedById = generateInvitedUsersData({ + usersToAddById: [2], +}); + describe('overage check', () => { it('returns no overage on free plans', () => { - const result = checkOverage(freePlanSubsciption, [], []); + const result = checkOverage(freePlanSubsciption, secondUserAddedById); expect(result.hasOverage).toBe(false); }); it('returns no overage when there is one free seat', () => { - const result = checkOverage(oneFreeSeatSubscription, [], ['new_user@email.com']); + const result = checkOverage( + oneFreeSeatSubscription, + generateInvitedUsersData({ usersToInviteByEmail: ['new_user@email.com'] }), + ); expect(result.hasOverage).toBe(false); }); it('returns overage when new user added by id', () => { - const result = checkOverage(noFreePlacesSubscription, [2], []); + const result = checkOverage(noFreePlacesSubscription, secondUserAddedById); expect(result.hasOverage).toBe(true); expect(result.usersOverage).toBe(2); }); it('returns overage when new user added by email', () => { - const result = checkOverage(noFreePlacesSubscription, [], ['test2@example']); + const result = checkOverage( + noFreePlacesSubscription, + generateInvitedUsersData({ usersToInviteByEmail: ['test2@example'] }), + ); expect(result.hasOverage).toBe(true); expect(result.usersOverage).toBe(2); }); it('returns overage for only overlapping users added by id', () => { - const result = checkOverage(noFreePlacesSubscription, [1, 2, 3], []); + const result = checkOverage( + noFreePlacesSubscription, + generateInvitedUsersData({ usersToAddById: [1, 2, 3] }), + ); expect(result.hasOverage).toBe(true); expect(result.usersOverage).toBe(3); @@ -43,8 +58,9 @@ describe('overage check', () => { it('returns overage for only overlapping users added by emails', () => { const result = checkOverage( noFreePlacesSubscription, - [], - ['test@example', 'test2@example', 'test3@example'], + generateInvitedUsersData({ + usersToInviteByEmail: ['test@example', 'test2@example', 'test3@example'], + }), ); expect(result.hasOverage).toBe(true); @@ -54,8 +70,10 @@ describe('overage check', () => { it('returns overage for only overlapping users added by ids and emails', () => { const result = checkOverage( noFreePlacesSubscription, - [1, 2], - ['test@example', 'test2@example'], + generateInvitedUsersData({ + usersToAddById: [1, 2], + usersToInviteByEmail: ['test@example', 'test2@example'], + }), ); expect(result.hasOverage).toBe(true); @@ -63,8 +81,29 @@ describe('overage check', () => { }); it('returns no overage if adding a user does not increase seats owed', () => { - const result = checkOverage(subscriptionWithOverage, [2], []); + const result = checkOverage(subscriptionWithOverage, secondUserAddedById); expect(result.hasOverage).toBe(false); }); + + describe('for subscriptions that don`\t bill guests', () => { + it('returns overage on adding developers', () => { + const result = checkOverage(allowGuestsSubscription, secondUserAddedById); + + expect(result.hasOverage).toBe(true); + expect(result.usersOverage).toBe(2); + }); + + it('returns no overage on adding guests', () => { + const result = checkOverage( + allowGuestsSubscription, + generateInvitedUsersData({ + isGuestRole: true, + usersToAddById: [2], + }), + ); + + expect(result.hasOverage).toBe(false); + }); + }); }); diff --git a/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js b/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js index 12fe8f421e1f0f5e5d10160fc284b80cfb4a8367..f081ea954c88a29585f84f0f086707300afe7ae0 100644 --- a/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js +++ b/ee/spec/frontend/invite_members/components/invite_modal_base_spec.js @@ -39,6 +39,9 @@ describe('EEInviteModalBase', () => { provide: { ...glFeatures, }, + attrs: { + 'access-levels': propsData.accessLevels, + }, stubs: { GlSprintf, InviteModalBase: CEInviteModalBase, diff --git a/ee/spec/frontend/invite_members/get_subscription_data_spec.js b/ee/spec/frontend/invite_members/get_subscription_data_spec.js index e2e9a4b60d74d54d7152d2f779368137f67222bd..729d330c91b64ff550438b4c9258ec288b59909c 100644 --- a/ee/spec/frontend/invite_members/get_subscription_data_spec.js +++ b/ee/spec/frontend/invite_members/get_subscription_data_spec.js @@ -25,6 +25,7 @@ describe('fetchUserIdsFromGroup', () => { maxSeatsUsed: 104, seatsInUse: 98, subscriptionSeats: 100, + excludeGuests: true, }); }); @@ -37,6 +38,7 @@ describe('fetchUserIdsFromGroup', () => { maxSeatsUsed: 5, seatsInUse: 0, subscriptionSeats: 0, + excludeGuests: false, }); }); }); diff --git a/ee/spec/frontend/invite_members/mock_data.js b/ee/spec/frontend/invite_members/mock_data.js index 1695230c85ec63f66c665ba1368000630c880b20..27e762a0cf311e58ca40cd1e4e9944d48f399706 100644 --- a/ee/spec/frontend/invite_members/mock_data.js +++ b/ee/spec/frontend/invite_members/mock_data.js @@ -5,6 +5,7 @@ const generateSubscriptionData = ({ seatsInUse = 0, billedUserIds = [], billedUserEmails = [], + excludeGuests = false, } = {}) => ({ isFreeGroup, subscriptionSeats, @@ -12,6 +13,17 @@ const generateSubscriptionData = ({ seatsInUse, billedUserIds, billedUserEmails, + excludeGuests, +}); + +export const generateInvitedUsersData = ({ + isGuestRole = false, + usersToInviteByEmail = [], + usersToAddById = [], +} = {}) => ({ + isGuestRole, + usersToInviteByEmail, + usersToAddById, }); export const freePlanSubsciption = generateSubscriptionData({ isFreeGroup: true }); @@ -28,3 +40,10 @@ export const subscriptionWithOverage = generateSubscriptionData({ billedUserIds: [1], billedUserEmails: ['test@example'], }); +export const allowGuestsSubscription = generateSubscriptionData({ + maxSeatsUsed: 1, + seatsInUse: 1, + billedUserIds: [1], + billedUserEmails: ['test@example'], + excludeGuests: true, +});