Commit 900905ab authored by Pavel Shutsin's avatar Pavel Shutsin

Merge branch 'dz/345486-enhance-users-overage-modal' into 'master'

Add users overage check

See merge request gitlab-org/gitlab!78287
parents 5dad1cf7 5749edb1
...@@ -44,6 +44,10 @@ export default { ...@@ -44,6 +44,10 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
rootId: {
type: String,
required: true,
},
isProject: { isProject: {
type: Boolean, type: Boolean,
required: true, required: true,
...@@ -290,6 +294,8 @@ export default { ...@@ -290,6 +294,8 @@ export default {
:submit-disabled="inviteDisabled" :submit-disabled="inviteDisabled"
:invalid-feedback-message="invalidFeedbackMessage" :invalid-feedback-message="invalidFeedbackMessage"
:is-loading="isLoading" :is-loading="isLoading"
:new-users-to-invite="newUsersToInvite"
:root-group-id="rootId"
@reset="resetFields" @reset="resetFields"
@submit="sendInvite" @submit="sendInvite"
@access-level="onAccessLevelUpdate" @access-level="onAccessLevelUpdate"
......
...@@ -5,15 +5,11 @@ import { parseBoolean } from '~/lib/utils/common_utils'; ...@@ -5,15 +5,11 @@ import { parseBoolean } from '~/lib/utils/common_utils';
Vue.use(GlToast); Vue.use(GlToast);
let initedInviteMembersModal; export default (function initInviteMembersModal() {
let inviteMembersModal;
export default function initInviteMembersModal() {
if (initedInviteMembersModal) {
// if we already loaded this in another part of the dom, we don't want to do it again
// else we will stack the modals
return false;
}
return () => {
if (!inviteMembersModal) {
// https://gitlab.com/gitlab-org/gitlab/-/issues/344955 // https://gitlab.com/gitlab-org/gitlab/-/issues/344955
// bug lying in wait here for someone to put group and project invite in same screen // bug lying in wait here for someone to put group and project invite in same screen
// once that happens we'll need to mount these differently, perhaps split // once that happens we'll need to mount these differently, perhaps split
...@@ -24,9 +20,7 @@ export default function initInviteMembersModal() { ...@@ -24,9 +20,7 @@ export default function initInviteMembersModal() {
return false; return false;
} }
initedInviteMembersModal = true; inviteMembersModal = new Vue({
return new Vue({
el, el,
name: 'InviteMembersModalRoot', name: 'InviteMembersModalRoot',
provide: { provide: {
...@@ -46,4 +40,7 @@ export default function initInviteMembersModal() { ...@@ -46,4 +40,7 @@ export default function initInviteMembersModal() {
}, },
}), }),
}); });
} }
return inviteMembersModal;
};
})();
...@@ -48,6 +48,7 @@ module InviteMembersHelper ...@@ -48,6 +48,7 @@ module InviteMembersHelper
def common_invite_modal_dataset(source) def common_invite_modal_dataset(source)
dataset = { dataset = {
id: source.id, id: source.id,
root_id: source&.root_ancestor&.id,
name: source.name, name: source.name,
default_access_level: Gitlab::Access::GUEST default_access_level: Gitlab::Access::GUEST
} }
......
import { difference } from 'lodash';
/**
* This method checks if adding given users (by ids or email) or a group id
* will trigger an overage.
*
* Returns the boolean flag if overage is present and a total count of users
* to be billed in case of overage
*
* @param {Object} subscriptionData Data from related subscription
* @param {Number} subscriptionSeats
* @param {Number} maxSeatsUsed Maximum of seats being used for this subscription
* @param {Number} seatsInUse Amount of seats currently in use
* @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
*
* @returns {Object}
*/
export const checkOverage = (
{ subscriptionSeats, maxSeatsUsed, seatsInUse, billedUserIds, billedUserEmails, isFreeGroup },
usersToAddById,
usersToInviteByEmail,
) => {
// overage only possible on paid plans
if (isFreeGroup) {
return { usersOverage: null, hasOverage: false };
}
// we could add a user to already overfilled group
const initialUserCount = subscriptionSeats < maxSeatsUsed ? maxSeatsUsed : subscriptionSeats;
const addedByIdUsersCount = difference(usersToAddById, billedUserIds).length;
const addedByEmailUsersCount = difference(usersToInviteByEmail, billedUserEmails).length;
const totalUserCount = seatsInUse + addedByIdUsersCount + addedByEmailUsersCount;
return {
usersOverage: totalUserCount,
hasOverage: initialUserCount < totalUserCount,
};
};
<script> <script>
import { GlLink, GlButton } from '@gitlab/ui'; import { GlLink, GlButton } from '@gitlab/ui';
import { partition, isString } from 'lodash';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import InviteModalBase from '~/invite_members/components/invite_modal_base.vue'; import InviteModalBase from '~/invite_members/components/invite_modal_base.vue';
import { import {
...@@ -11,6 +12,8 @@ import { ...@@ -11,6 +12,8 @@ import {
overageModalInfoText, overageModalInfoText,
overageModalInfoWarning, overageModalInfoWarning,
} from '../constants'; } from '../constants';
import { checkOverage } from '../check_overage';
import { fetchSubscription } from '../get_subscription_data';
const OVERAGE_CONTENT_SLOT = 'overage-content'; const OVERAGE_CONTENT_SLOT = 'overage-content';
const EXTRA_SLOTS = [ const EXTRA_SLOTS = [
...@@ -40,16 +43,23 @@ export default { ...@@ -40,16 +43,23 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
subscriptionSeats: { rootGroupId: {
type: Number, type: String,
required: false,
default: '',
},
newUsersToInvite: {
type: Array,
required: false, required: false,
default: 10, // TODO: pass data from backend https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78287 default: () => [],
}, },
}, },
data() { data() {
return { return {
hasOverage: false, hasOverage: false,
totalUserCount: null, totalUserCount: null,
subscriptionSeats: 0,
namespaceId: parseInt(this.rootGroupId, 10),
}; };
}, },
computed: { computed: {
...@@ -96,18 +106,37 @@ export default { ...@@ -96,18 +106,37 @@ export default {
return listeners; return listeners;
}, },
onReset(...args) { onReset() {
// don't reopen the overage modal // don't reopen the overage modal
this.hasOverage = false; this.hasOverage = false;
this.$emit('reset', ...args); this.$emit('reset');
}, },
onSubmit(...args) { onSubmit(args) {
if (this.enabledOverageCheck && !this.hasOverage) { if (this.enabledOverageCheck && !this.hasOverage) {
this.totalUserCount = 1; this.checkAndSubmit(args);
this.hasOverage = true; } else {
this.$emit('submit', { accessLevel: args.accessLevel, expiresAt: args.expiresAt });
}
},
async checkAndSubmit(args) {
this.isLoading = true;
const [usersToInviteByEmail, usersToAddById] = this.partitionNewUsersToInvite();
const subscriptionData = await fetchSubscription(this.namespaceId);
this.subscriptionSeats = subscriptionData.subscriptionSeats;
const { hasOverage, usersOverage } = checkOverage(
subscriptionData,
usersToAddById,
usersToInviteByEmail,
);
this.isLoading = false;
this.hasOverage = hasOverage;
if (hasOverage) {
this.totalUserCount = usersOverage;
} else { } else {
this.$emit('submit', ...args); this.$emit('submit', { accessLevel: args.accessLevel, expiresAt: args.expiresAt });
} }
}, },
handleBack() { handleBack() {
...@@ -116,6 +145,14 @@ export default { ...@@ -116,6 +145,14 @@ export default {
passthroughSlotNames() { passthroughSlotNames() {
return Object.keys(this.$scopedSlots || {}); return Object.keys(this.$scopedSlots || {});
}, },
partitionNewUsersToInvite() {
const [usersToInviteByEmail, usersToAddById] = partition(
this.newUsersToInvite,
({ id }) => isString(id) && id.includes('user-defined-token'),
);
return [usersToInviteByEmail.map(({ name }) => name), usersToAddById.map(({ id }) => id)];
},
}, },
i18n: { i18n: {
OVERAGE_MODAL_TITLE, OVERAGE_MODAL_TITLE,
......
import { memoize } from 'lodash';
import Api from 'ee/api';
const isFreeGroup = (plan) => ['free', null].includes(plan.code);
const fetchSubscriptionData = memoize((id) =>
Api.userSubscription(id).then((response) => response.data),
);
export const fetchSubscription = async (namespaceId) => {
const data = await fetchSubscriptionData(namespaceId);
return {
subscriptionSeats: data.usage.seats_in_subscription,
// Fetch data in https://gitlab.com/gitlab-org/gitlab/-/issues/354768
billedUserIds: [],
billedUserEmails: [],
maxSeatsUsed: data.usage.max_seats_used,
seatsInUse: data.usage.seats_in_use,
isFreeGroup: isFreeGroup(data.plan),
};
};
...@@ -2,48 +2,91 @@ ...@@ -2,48 +2,91 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Groups > Members > Manage members' do RSpec.describe 'Groups > Members > Manage members', :saas, :js do
include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::Features::MembersHelpers
include Spec::Support::Helpers::Features::InviteMembersModalHelper include Spec::Support::Helpers::Features::InviteMembersModalHelper
include Spec::Support::Helpers::ModalHelpers include Spec::Support::Helpers::ModalHelpers
let_it_be(:user1) { create(:user, name: 'John Doe') } let_it_be(:user1) { create(:user, name: 'John Doe') }
let_it_be(:user2) { create(:user, name: 'Mary Jane') } let_it_be(:user2) { create(:user, name: 'Mary Jane') }
let_it_be(:user3) { create(:user, name: 'Peter Parker') }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:premium_plan) { create(:premium_plan) }
shared_examples "adding one user doesn't trigger an overage modal" do
it do
group.add_owner(user1)
add_user_by_name(user2.name, 'Developer')
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('Developer')
end
end
end
before do before do
sign_in(user1) sign_in(user1)
stub_application_setting(check_namespace_plan: true)
end end
context 'when adding a member to a group triggers an overage', :js, :aggregate_failures do context 'adding a member to a free group' do
before do before do
allow(Gitlab).to receive(:com?) { true } create(:gitlab_subscription, namespace: group, hosted_plan: nil)
create(:gitlab_subscription, namespace: group)
stub_application_setting(check_namespace_plan: true)
end end
it 'show an overage modal and invites a member to a group if confirmed' do it_behaves_like "adding one user doesn't trigger an overage modal"
group.add_owner(user1) end
context 'when adding a member to a premium group' do
context 'when there is one free space in the subscription' do
before do
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 'adding two users triggers overage modal', :aggregate_failures do
group.add_owner(user1)
visit group_group_members_path(group) visit group_group_members_path(group)
click_on 'Invite members' click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do page.within '[data-testid="invite-modal"]' do
find('[data-testid="members-token-select-input"]').set(user2.name) add_user_to_input(user2.name)
add_user_to_input(user3.name)
wait_for_requests
click_button user2.name
choose_options('Developer', nil) choose_options('Developer', nil)
click_button 'Invite' click_button 'Invite'
end
expect(page).to have_content("Your subscription includes 10 seats. If you continue, the #{group.name} group will have 1 seat in use and will be billed for the overage. Learn more.") expect(page).to have_content("You are about to incur additional charges")
expect(page).to have_content("Your subscription includes 2 seats. If you continue, the #{group.name} group will have 3 seats in use and will be billed for the overage. Learn more.")
end
end
context 'when modal is shown' do
before do
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' click_button 'Continue'
page.refresh page.refresh
end
page.within(second_row) do page.within(second_row) do
expect(page).to have_content(user2.name) expect(page).to have_content(user2.name)
...@@ -51,26 +94,14 @@ RSpec.describe 'Groups > Members > Manage members' do ...@@ -51,26 +94,14 @@ RSpec.describe 'Groups > Members > Manage members' do
end end
end end
it 'show an overage modal and get back to initial modal if not confirmed' do it 'get back to initial modal if not confirmed', :aggregate_failures do
group.add_owner(user1) group.add_owner(user1)
add_user_by_name(user2.name, 'Developer')
visit group_group_members_path(group) 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_on 'Invite members'
page.within '[data-testid="invite-modal"]' do
find('[data-testid="members-token-select-input"]').set(user2.name)
wait_for_requests
click_button user2.name
choose_options('Developer', nil)
click_button 'Invite'
expect(page).to have_content("Your subscription includes 10 seats. If you continue, the #{group.name} group will have 1 seat in use and will be billed for the overage. Learn more.")
click_button 'Back' click_button 'Back'
end
expect(page).to have_content("You're inviting members to the #{group.name} group.") expect(page).to have_content("You're inviting members to the #{group.name} group.")
...@@ -80,4 +111,25 @@ RSpec.describe 'Groups > Members > Manage members' do ...@@ -80,4 +111,25 @@ RSpec.describe 'Groups > Members > Manage members' do
expect(page).not_to have_button('Developer') expect(page).not_to have_button('Developer')
end end
end end
end
def add_user_by_name(name, role)
visit group_group_members_path(group)
click_on 'Invite members'
page.within '[data-testid="invite-modal"]' do
add_user_to_input(name)
choose_options(role, nil)
click_button 'Invite'
end
end
def add_user_to_input(name)
find('[data-testid="members-token-select-input"]').set(name)
wait_for_requests
click_button name
end
end end
import { checkOverage } from 'ee/invite_members/check_overage';
import {
freePlanSubsciption,
oneFreeSeatSubscription,
noFreePlacesSubscription,
subscriptionWithOverage,
} from './mock_data';
describe('overage check', () => {
it('returns no overage on free plans', () => {
const result = checkOverage(freePlanSubsciption, [], []);
expect(result.hasOverage).toBe(false);
});
it('returns no overage when there is one free seat', () => {
const result = checkOverage(oneFreeSeatSubscription, [], ['new_user@email.com']);
expect(result.hasOverage).toBe(false);
});
it('returns overage when new user added by id', () => {
const result = checkOverage(noFreePlacesSubscription, [2], []);
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']);
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], []);
expect(result.hasOverage).toBe(true);
expect(result.usersOverage).toBe(3);
});
it('returns overage for only overlapping users added by emails', () => {
const result = checkOverage(
noFreePlacesSubscription,
[],
['test@example', 'test2@example', 'test3@example'],
);
expect(result.hasOverage).toBe(true);
expect(result.usersOverage).toBe(3);
});
it('returns overage for only overlapping users added by ids and emails', () => {
const result = checkOverage(
noFreePlacesSubscription,
[1, 2],
['test@example', 'test2@example'],
);
expect(result.hasOverage).toBe(true);
expect(result.usersOverage).toBe(3);
});
it('returns no overage if adding a user does not increase seats owed', () => {
const result = checkOverage(subscriptionWithOverage, [2], []);
expect(result.hasOverage).toBe(false);
});
});
...@@ -11,6 +11,15 @@ import { ...@@ -11,6 +11,15 @@ import {
OVERAGE_MODAL_BACK_BUTTON, OVERAGE_MODAL_BACK_BUTTON,
} from 'ee/invite_members/constants'; } from 'ee/invite_members/constants';
import { propsData } from 'jest/invite_members/mock_data/modal_base'; import { propsData } from 'jest/invite_members/mock_data/modal_base';
import { noFreePlacesSubscription as mockSubscription } from '../mock_data';
jest.mock('ee/invite_members/check_overage', () => ({
checkOverage: jest.fn().mockImplementation(() => ({ hasOverage: true, usersOverage: 2 })),
}));
jest.mock('ee/invite_members/get_subscription_data', () => ({
fetchSubscription: jest.fn().mockImplementation(() => mockSubscription),
}));
describe('EEInviteModalBase', () => { describe('EEInviteModalBase', () => {
let wrapper; let wrapper;
...@@ -135,7 +144,7 @@ describe('EEInviteModalBase', () => { ...@@ -135,7 +144,7 @@ describe('EEInviteModalBase', () => {
it('shows the info text', () => { it('shows the info text', () => {
expect(wrapper.findComponent(GlModal).text()).toContain( expect(wrapper.findComponent(GlModal).text()).toContain(
'If you continue, the _name_ group will have 1 seat in use and will be billed for the overage.', 'Your subscription includes 1 seat. If you continue, the _name_ group will have 2 seats in use and will be billed for the overage.',
); );
}); });
......
import { mockDataSubscription } from 'ee_jest/billings/mock_data';
import { fetchSubscription } from 'ee/invite_members/get_subscription_data';
import Api from 'ee/api';
jest.mock('ee/api.js', () => ({
userSubscription: jest
.fn()
.mockResolvedValueOnce({ data: mockDataSubscription.gold })
.mockResolvedValueOnce({ data: mockDataSubscription.free }),
}));
describe('fetchUserIdsFromGroup', () => {
it('caches the response for the same input', async () => {
await fetchSubscription(1);
await fetchSubscription(1);
expect(Api.userSubscription).toHaveBeenCalledTimes(1);
});
it('returns correct subscription data for paid group', async () => {
const result = await fetchSubscription(1);
expect(result).toEqual({
billedUserEmails: [],
billedUserIds: [],
isFreeGroup: false,
maxSeatsUsed: 104,
seatsInUse: 98,
subscriptionSeats: 100,
});
});
it('returns correct subscription data for free group', async () => {
const result = await fetchSubscription(2);
expect(result).toEqual({
billedUserEmails: [],
billedUserIds: [],
isFreeGroup: true,
maxSeatsUsed: 5,
seatsInUse: 0,
subscriptionSeats: 0,
});
});
});
const generateSubscriptionData = ({
isFreeGroup = false,
subscriptionSeats = 1,
maxSeatsUsed = 0,
seatsInUse = 0,
billedUserIds = [],
billedUserEmails = [],
} = {}) => ({
isFreeGroup,
subscriptionSeats,
maxSeatsUsed,
seatsInUse,
billedUserIds,
billedUserEmails,
});
export const freePlanSubsciption = generateSubscriptionData({ isFreeGroup: true });
export const oneFreeSeatSubscription = generateSubscriptionData();
export const noFreePlacesSubscription = generateSubscriptionData({
maxSeatsUsed: 1,
seatsInUse: 1,
billedUserIds: [1],
billedUserEmails: ['test@example'],
});
export const subscriptionWithOverage = generateSubscriptionData({
maxSeatsUsed: 2,
seatsInUse: 1,
billedUserIds: [1],
billedUserEmails: ['test@example'],
});
export const propsData = { export const propsData = {
id: '1', id: '1',
rootId: '1',
name: 'test name', name: 'test name',
isProject: false, isProject: false,
accessLevels: { Guest: 10, Reporter: 20, Developer: 30, Maintainer: 40, Owner: 50 }, accessLevels: { Guest: 10, Reporter: 20, Developer: 30, Maintainer: 40, Owner: 50 },
......
...@@ -35,6 +35,7 @@ RSpec.describe InviteMembersHelper do ...@@ -35,6 +35,7 @@ RSpec.describe InviteMembersHelper do
it 'has expected common attributes' do it 'has expected common attributes' do
attributes = { attributes = {
id: project.id, id: project.id,
root_id: project.root_ancestor.id,
name: project.name, name: project.name,
default_access_level: Gitlab::Access::GUEST default_access_level: Gitlab::Access::GUEST
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment