Add guests check for overage

Adding guests doesn't trigger overage
for ultimate-like plans

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 };
   }
 

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;
 

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,
   };
 };

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
 

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)
 

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',

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);
+    });
+  });
 });

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,

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,
     });
   });
 });

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,
+});