Display error if remove invalid billable member

Some billable members can be associated via group invites, which makes
them invalid for removal from the group being viewed. This will now
trigger an error modal to inform the customer

Changelog: changed

ee/app/assets/javascripts/billings/seat_usage/components/remove_billable_member_modal.vue

@@ -86,7 +86,7 @@ export default {
     data-qa-selector="remove_billable_member_modal"
     :ok-disabled="!canSubmit"
     @primary="removeBillableMember"
-    @canceled="setBillableMemberToRemove(null)"
+    @hide="setBillableMemberToRemove(null)"
   >
     <p>
       <gl-sprintf :message="modalText">

ee/app/assets/javascripts/billings/seat_usage/components/subscription_seats.vue

@@ -6,6 +6,7 @@ import {
   GlButton,
   GlDropdown,
   GlDropdownItem,
+  GlModal,
   GlModalDirective,
   GlIcon,
   GlPagination,
@@ -20,6 +21,9 @@ import {
   AVATAR_SIZE,
   SEARCH_DEBOUNCE_MS,
   REMOVE_BILLABLE_MEMBER_MODAL_ID,
+  CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_ID,
+  CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_TITLE,
+  CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_CONTENT,
 } from 'ee/billings/seat_usage/constants';
 import { s__ } from '~/locale';
 import RemoveBillableMemberModal from './remove_billable_member_modal.vue';
@@ -37,6 +41,7 @@ export default {
     GlButton,
     GlDropdown,
     GlDropdownItem,
+    GlModal,
     GlIcon,
     GlPagination,
     GlSearchBoxByType,
@@ -118,6 +123,13 @@ export default {
         this.resetBillableMembers();
       }
     },
+    displayRemoveMemberModal(user) {
+      if (user.removable) {
+        this.setBillableMemberToRemove(user);
+      } else {
+        this.$refs.cannotRemoveModal.show();
+      }
+    },
   },
   i18n: {
     emailNotVisibleTooltipText: s__(
@@ -127,6 +139,9 @@ export default {
   avatarSize: AVATAR_SIZE,
   fields: FIELDS,
   removeBillableMemberModalId: REMOVE_BILLABLE_MEMBER_MODAL_ID,
+  cannotRemoveModalId: CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_ID,
+  cannotRemoveModalTitle: CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_TITLE,
+  cannotRemoveModalText: CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_CONTENT,
 };
 </script>
 
@@ -218,7 +233,8 @@ export default {
         <gl-dropdown icon="ellipsis_h" right data-testid="user-actions">
           <gl-dropdown-item
             v-gl-modal="$options.removeBillableMemberModalId"
-            @click="setBillableMemberToRemove(data.item.user)"
+            data-testid="remove-user"
+            @click="displayRemoveMemberModal(data.item.user)"
           >
             {{ __('Remove user') }}
           </gl-dropdown-item>
@@ -243,5 +259,17 @@ export default {
       v-if="billableMemberToRemove"
       :modal-id="$options.removeBillableMemberModalId"
     />
+
+    <gl-modal
+      ref="cannotRemoveModal"
+      :modal-id="$options.cannotRemoveModalId"
+      :title="$options.cannotRemoveModalTitle"
+      :action-primary="{ text: __('Okay') }"
+      static
+    >
+      <p>
+        {{ $options.cannotRemoveModalText }}
+      </p>
+    </gl-modal>
   </section>
 </template>

ee/app/assets/javascripts/billings/seat_usage/constants.js

@@ -35,6 +35,12 @@ export const DETAILS_FIELDS = [
   { key: 'role', label: __('Role'), thClass: thWidthClass(40) },
 ];
 
+export const CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_ID = 'cannot-remove-member-modal';
+export const CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_TITLE = s__('Billing|Cannot remove user');
+export const CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_CONTENT = s__(
+  `Billing|Members who were invited via a group invitation cannot be removed.
+  You can either remove the entire group, or ask an Owner of the invited group to remove the member.`,
+);
 export const REMOVE_BILLABLE_MEMBER_MODAL_ID = 'billable-member-remove-modal';
 export const REMOVE_BILLABLE_MEMBER_MODAL_CONTENT_TEXT_TEMPLATE = s__(
   `Billing|You are about to remove user %{username} from your subscription.

ee/app/assets/javascripts/billings/seat_usage/store/getters.js

 export const tableItems = (state) => {
-  if (state.members.length) {
-    return state.members.map(
-      ({ id, name, username, avatar_url, web_url, email, last_activity_on, membership_type }) => {
-        const formattedUserName = `@${username}`;
-
-        return {
-          user: {
-            id,
-            name,
-            username: formattedUserName,
-            avatar_url,
-            web_url,
-            last_activity_on,
-            membership_type,
-          },
-          email,
-        };
-      },
-    );
-  }
-  return [];
+  return (state.members ?? []).map(({ email, ...member }) => ({
+    user: {
+      ...member,
+      username: `@${member.username}`,
+    },
+    email,
+  }));
 };
 
 export const membershipsById = (state) => (memberId) => {

ee/changelogs/unreleased/vij-prevent-invited-group-member-removal.yml

+---
+title: Add a new error modal for billable member removal
+merge_request: 61509
+author:
+type: changed

ee/spec/features/groups/billing/seat_usage_spec.rb

@@ -8,6 +8,8 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
   let_it_be(:sub_group) { create(:group, parent: group) }
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:user_from_sub_group) { create(:user) }
+  let_it_be(:shared_group) { create(:group) }
+  let_it_be(:shared_group_developer) { create(:user) }
 
   before do
     allow(Gitlab).to receive(:com?).and_return(true)
@@ -18,6 +20,9 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
 
     sub_group.add_maintainer(user_from_sub_group)
 
+    shared_group.add_developer(shared_group_developer)
+    create(:group_group_link, { shared_with_group: shared_group, shared_group: group })
+
     sign_in(user)
 
     visit group_seat_usage_path(group)
@@ -27,7 +32,7 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
   context 'seat usage table' do
     it 'displays correct number of users' do
       within '[data-testid="table"]' do
-        expect(all('tbody tr').count).to eq(3)
+        expect(all('tbody tr').count).to eq(4)
       end
     end
 
@@ -92,6 +97,10 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
           expect(page).to have_button('Remove user', disabled: true)
         end
       end
+
+      it 'does not display the error modal' do
+        expect(page).not_to have_content('Cannot remove user')
+      end
     end
 
     context 'removing the user' do
@@ -113,7 +122,7 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
         wait_for_all_requests
 
         within '[data-testid="table"]' do
-          expect(all('tbody tr').count).to eq(2)
+          expect(all('tbody tr').count).to eq(3)
         end
 
         expect(page.find('.flash-container')).to have_content('User was successfully removed')
@@ -122,7 +131,7 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
       context 'removing the user from a sub-group' do
         it 'updates the seat table of the parent group' do
           within '[data-testid="table"]' do
-            expect(all('tbody tr').count).to eq(3)
+            expect(all('tbody tr').count).to eq(4)
           end
 
           visit group_group_members_path(sub_group)
@@ -140,10 +149,27 @@ RSpec.describe 'Groups > Billing > Seat Usage', :js do
           wait_for_all_requests
 
           within '[data-testid="table"]' do
-            expect(all('tbody tr').count).to eq(2)
+            expect(all('tbody tr').count).to eq(3)
           end
         end
       end
     end
+
+    context 'when cannot remove the user' do
+      let(:shared_user_row) do
+        within '[data-testid="table"]' do
+          find('tr', text: shared_group_developer.name)
+        end
+      end
+
+      it 'displays an error modal' do
+        within shared_user_row do
+          find('[data-testid="user-actions"]').click
+          click_button 'Remove user'
+        end
+
+        expect(page).to have_content('Cannot remove user')
+      end
+    end
   end
 end

ee/spec/frontend/billings/mock_data.js

@@ -76,6 +76,7 @@ export const mockDataSeats = {
       email: 'administrator@email.com',
       last_activity_on: '2020-03-01',
       membership_type: 'group_member',
+      removable: true,
     },
     {
       id: 3,
@@ -86,6 +87,7 @@ export const mockDataSeats = {
       email: 'agustin_walker@email.com',
       last_activity_on: '2020-03-01',
       membership_type: 'group_member',
+      removable: true,
     },
     {
       id: 4,
@@ -96,6 +98,7 @@ export const mockDataSeats = {
       last_activity_on: null,
       email: null,
       membership_type: 'group_invite',
+      removable: false,
     },
   ],
   headers: {
@@ -127,6 +130,7 @@ export const mockTableItems = [
       web_url: 'path/to/administrator',
       last_activity_on: '2020-03-01',
       membership_type: 'group_member',
+      removable: true,
     },
   },
   {
@@ -139,6 +143,7 @@ export const mockTableItems = [
       web_url: 'path/to/agustin_walker',
       last_activity_on: '2020-03-01',
       membership_type: 'group_member',
+      removable: true,
     },
   },
   {
@@ -151,6 +156,7 @@ export const mockTableItems = [
       web_url: 'path/to/joella_miller',
       last_activity_on: null,
       membership_type: 'group_invite',
+      removable: false,
     },
   },
 ];

ee/spec/frontend/billings/seat_usage/components/subscription_seats_spec.js

@@ -6,11 +6,14 @@ import {
   GlAvatarLabeled,
   GlSearchBoxByType,
   GlBadge,
+  GlModal,
 } from '@gitlab/ui';
 import { mount, shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import SubscriptionSeats from 'ee/billings/seat_usage/components/subscription_seats.vue';
+import { CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_CONTENT } from 'ee/billings/seat_usage/constants';
 import { mockDataSeats, mockTableItems } from 'ee_jest/billings/mock_data';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -18,6 +21,7 @@ localVue.use(Vuex);
 const actionSpies = {
   fetchBillableMembersList: jest.fn(),
   resetBillableMembers: jest.fn(),
+  setBillableMemberToRemove: jest.fn(),
 };
 
 const providedFields = {
@@ -53,10 +57,12 @@ describe('Subscription Seats', () => {
     mountFn = shallowMount,
     initialGetters = {},
   } = {}) => {
-    return mountFn(SubscriptionSeats, {
-      store: fakeStore({ initialState, initialGetters }),
-      localVue,
-    });
+    return extendedWrapper(
+      mountFn(SubscriptionSeats, {
+        store: fakeStore({ initialState, initialGetters }),
+        localVue,
+      }),
+    );
   };
 
   const findTable = () => wrapper.find(GlTable);
@@ -69,6 +75,9 @@ describe('Subscription Seats', () => {
   const findSearchBox = () => wrapper.find(GlSearchBoxByType);
   const findPagination = () => wrapper.find(GlPagination);
 
+  const findAllRemoveUserItems = () => wrapper.findAllByTestId('remove-user');
+  const findErrorModal = () => wrapper.findComponent(GlModal);
+
   const serializeUser = (rowWrapper) => {
     const avatarLink = rowWrapper.find(GlAvatarLink);
     const avatarLabeled = rowWrapper.find(GlAvatarLabeled);
@@ -152,6 +161,20 @@ describe('Subscription Seats', () => {
         totalItems: 300,
       });
     });
+
+    describe('with error modal', () => {
+      it('does not render the model if the user is not removable', async () => {
+        await findAllRemoveUserItems().at(0).trigger('click');
+
+        expect(findErrorModal().html()).toBe('');
+      });
+
+      it('renders the error modal if the user is removable', async () => {
+        await findAllRemoveUserItems().at(2).trigger('click');
+
+        expect(findErrorModal().text()).toContain(CANNOT_REMOVE_BILLABLE_MEMBER_MODAL_CONTENT);
+      });
+    });
   });
 
   describe('pagination', () => {

locale/gitlab.pot

@@ -5129,6 +5129,9 @@ msgstr ""
 msgid "Billing|An error occurred while removing a billable member"
 msgstr ""
 
+msgid "Billing|Cannot remove user"
+msgstr ""
+
 msgid "Billing|Direct memberships"
 msgstr ""
 
@@ -5141,6 +5144,9 @@ msgstr ""
 msgid "Billing|Group invite"
 msgstr ""
 
+msgid "Billing|Members who were invited via a group invitation cannot be removed. You can either remove the entire group, or ask an Owner of the invited group to remove the member."
+msgstr ""
+
 msgid "Billing|No users to display."
 msgstr ""
 
@@ -22744,6 +22750,9 @@ msgstr ""
 msgid "Ok, let's go"
 msgstr ""
 
+msgid "Okay"
+msgstr ""
+
 msgid "Oldest first"
 msgstr ""