Merge branch 'vij-prevent-invited-group-member-removal' into 'master'

Add a new error modal for billable members

See merge request gitlab-org/gitlab!61509

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 ""