Merge branch '329940-fix-remove-user-in-seat-usage' into 'master'

Make remove user buttons consistent in admin and groups dashboards

See merge request gitlab-org/gitlab!75497

app/assets/javascripts/admin/users/components/user_actions.vue

@@ -112,7 +112,7 @@ export default {
         right
         :text="$options.i18n.userAdministration"
         :text-sr-only="!showButtonLabels"
-        icon="settings"
+        icon="ellipsis_h"
         data-qa-selector="user_actions_dropdown_toggle"
         :data-qa-username="user.username"
       >

app/assets/javascripts/members/components/action_buttons/access_request_action_buttons.vue

@@ -53,6 +53,7 @@ export default {
         :title="s__('Member|Deny access')"
         :is-access-request="true"
         icon="close"
+        button-category="primary"
       />
     </div>
   </action-button-group>

app/assets/javascripts/members/components/action_buttons/invite_action_buttons.vue

@@ -41,6 +41,8 @@ export default {
       <remove-member-button
         :member-id="member.id"
         :message="message"
+        icon="remove"
+        button-category="primary"
         :title="s__('Member|Revoke invite')"
         is-invite
       />

app/assets/javascripts/members/components/action_buttons/remove_member_button.vue

@@ -30,7 +30,17 @@ export default {
     icon: {
       type: String,
       required: false,
-      default: 'remove',
+      default: undefined,
+    },
+    buttonText: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    buttonCategory: {
+      type: String,
+      required: false,
+      default: 'secondary',
     },
     isAccessRequest: {
       type: Boolean,
@@ -79,10 +89,12 @@ export default {
   <gl-button
     v-gl-tooltip
     variant="danger"
+    :category="buttonCategory"
     :title="title"
     :aria-label="title"
     :icon="icon"
     data-qa-selector="delete_member_button"
     @click="showRemoveMemberModal(modalData)"
-  />
+    ><template v-if="buttonText">{{ buttonText }}</template></gl-button
+  >
 </template>

app/assets/javascripts/members/components/action_buttons/user_action_buttons.vue

 <script>
-import { s__, sprintf } from '~/locale';
+import { __, s__, sprintf } from '~/locale';
 import { parseUserDeletionObstacles } from '~/vue_shared/components/user_deletion_obstacles/utils';
 import ActionButtonGroup from './action_button_group.vue';
 import LeaveButton from './leave_button.vue';
@@ -23,6 +23,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    isInvitedUser: {
+      type: Boolean,
+      required: true,
+    },
     permissions: {
       type: Object,
       required: true,
@@ -56,6 +60,15 @@ export default {
         obstacles: parseUserDeletionObstacles(this.member.user),
       };
     },
+    removeMemberButtonText() {
+      return this.isInvitedUser ? null : __('Remove user');
+    },
+    removeMemberButtonIcon() {
+      return this.isInvitedUser ? 'remove' : '';
+    },
+    removeMemberButtonCategory() {
+      return this.isInvitedUser ? 'primary' : 'secondary';
+    },
   },
 };
 </script>
@@ -70,6 +83,9 @@ export default {
         :member-type="member.type"
         :user-deletion-obstacles="userDeletionObstaclesUserData"
         :message="message"
+        :icon="removeMemberButtonIcon"
+        :button-text="removeMemberButtonText"
+        :button-category="removeMemberButtonCategory"
         :title="s__('Member|Remove member')"
       />
     </div>

app/assets/javascripts/members/components/table/member_action_buttons.vue

@@ -30,6 +30,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    isInvitedUser: {
+      type: Boolean,
+      required: true,
+    },
   },
   computed: {
     actionButtonComponent() {
@@ -53,5 +57,6 @@ export default {
     :member="member"
     :permissions="permissions"
     :is-current-user="isCurrentUser"
+    :is-invited-user="isInvitedUser"
   />
 </template>

app/assets/javascripts/members/components/table/members_table.vue

@@ -8,6 +8,7 @@ import initUserPopovers from '~/user_popovers';
 import {
   FIELDS,
   ACTIVE_TAB_QUERY_PARAM_NAME,
+  TAB_QUERY_PARAM_VALUES,
   MEMBER_STATE_AWAITING,
   USER_STATE_BLOCKED_PENDING_APPROVAL,
   BADGE_LABELS_PENDING_OWNER_APPROVAL,
@@ -82,6 +83,9 @@ export default {
 
       return paramName && currentPage && perPage && totalItems;
     },
+    isInvitedUser() {
+      return this.tabQueryParamValue === TAB_QUERY_PARAM_VALUES.invite;
+    },
   },
   mounted() {
     initUserPopovers(this.$el.querySelectorAll('.js-user-link'));
@@ -275,6 +279,7 @@ export default {
           <member-action-buttons
             :member-type="memberType"
             :is-current-user="isCurrentUser"
+            :is-invited-user="isInvitedUser"
             :permissions="permissions"
             :member="member"
           />

app/views/admin/users/projects.html.haml

@@ -17,7 +17,7 @@
             %span.light.vertical-align-middle= group_member.human_access
             - unless group_member.owner?
               = link_to group_group_member_path(group, group_member), data: { confirm: remove_member_message(group_member), testid: 'remove-user' }, method: :delete, remote: true, class: "btn btn-sm btn-danger gl-button btn-icon gl-ml-3", title: _('Remove user from group') do
-                = sprite_icon('close', size: 16, css_class: 'gl-icon')
+                = sprite_icon('remove', size: 16, css_class: 'gl-icon')
 
 .row
   .col-md-6
@@ -47,6 +47,6 @@
 
                   - if member.respond_to? :project
                     = link_to project_project_member_path(project, member), data: { confirm: remove_member_message(member) }, remote: true, method: :delete, class: "btn btn-sm btn-danger gl-button btn-icon gl-ml-3", title: _('Remove user from project') do
-                      = sprite_icon('close', size: 16, css_class: 'gl-icon')
+                      = sprite_icon('remove', size: 16, css_class: 'gl-icon')
 
 = render partial: 'admin/users/modals'

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

@@ -5,8 +5,6 @@ import {
   GlAvatarLink,
   GlBadge,
   GlButton,
-  GlDropdown,
-  GlDropdownItem,
   GlModal,
   GlModalDirective,
   GlIcon,
@@ -41,8 +39,6 @@ export default {
     GlAvatarLink,
     GlBadge,
     GlButton,
-    GlDropdown,
-    GlDropdownItem,
     GlModal,
     GlIcon,
     GlPagination,
@@ -265,15 +261,15 @@ export default {
       </template>
 
       <template #cell(actions)="data">
-        <gl-dropdown icon="ellipsis_h" right data-testid="user-actions">
-          <gl-dropdown-item
-            v-gl-modal="$options.removeBillableMemberModalId"
-            data-testid="remove-user"
-            @click="displayRemoveMemberModal(data.item.user)"
-          >
-            {{ __('Remove user') }}
-          </gl-dropdown-item>
-        </gl-dropdown>
+        <gl-button
+          v-gl-modal="$options.removeBillableMemberModalId"
+          category="secondary"
+          variant="danger"
+          data-testid="remove-user"
+          @click="displayRemoveMemberModal(data.item.user)"
+        >
+          {{ __('Remove user') }}
+        </gl-button>
       </template>
 
       <template #row-details="{ item }">

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

@@ -69,7 +69,6 @@ RSpec.describe 'Groups > Usage Quotas > Seat Usage', :js do
     context 'with a modal to confirm removal' do
       before do
         within user_to_remove_row do
-          find('[data-testid="user-actions"]').click
           click_button 'Remove user'
         end
       end
@@ -106,7 +105,6 @@ RSpec.describe 'Groups > Usage Quotas > Seat Usage', :js do
     context 'removing the user' do
       before do
         within user_to_remove_row do
-          find('[data-testid="user-actions"]').click
           click_button 'Remove user'
         end
       end
@@ -164,7 +162,6 @@ RSpec.describe 'Groups > Usage Quotas > Seat Usage', :js do
 
       it 'displays an error modal' do
         within shared_user_row do
-          find('[data-testid="user-actions"]').click
           click_button 'Remove user'
         end
 

ee/spec/frontend/members/components/action_buttons/user_action_buttons_spec.js

@@ -16,6 +16,7 @@ describe('UserActionButtons', () => {
       propsData: {
         member,
         isCurrentUser: false,
+        isInvitedUser: false,
         ...propsData,
       },
     });

ee/spec/frontend/seat_usage/components/__snapshots__/subscription_seats_spec.js.snap

@@ -3,8 +3,8 @@
 exports[`Subscription Seats renders table content renders the correct data 1`] = `
 Array [
   Object {
-    "dropdownExists": true,
     "email": "administrator@email.com",
+    "removeUserButtonExists": true,
     "tooltip": undefined,
     "user": Object {
       "avatarLabeled": Object {
@@ -19,8 +19,8 @@ Array [
     },
   },
   Object {
-    "dropdownExists": true,
     "email": "agustin_walker@email.com",
+    "removeUserButtonExists": true,
     "tooltip": undefined,
     "user": Object {
       "avatarLabeled": Object {
@@ -35,8 +35,8 @@ Array [
     },
   },
   Object {
-    "dropdownExists": true,
     "email": "Private",
+    "removeUserButtonExists": true,
     "tooltip": "An email address is only visible for users with public emails.",
     "user": Object {
       "avatarLabeled": Object {
@@ -53,8 +53,8 @@ Array [
     },
   },
   Object {
-    "dropdownExists": true,
     "email": "jdoe@email.com",
+    "removeUserButtonExists": true,
     "tooltip": undefined,
     "user": Object {
       "avatarLabeled": Object {

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

 import {
   GlAlert,
   GlPagination,
-  GlDropdown,
+  GlButton,
   GlTable,
   GlAvatarLink,
   GlAvatarLabeled,
@@ -107,7 +107,7 @@ describe('Subscription Seats', () => {
       user: serializeUser(rowWrapper),
       email: emailWrapper.text(),
       tooltip: emailWrapper.find('span').attributes('title'),
-      dropdownExists: rowWrapper.findComponent(GlDropdown).exists(),
+      removeUserButtonExists: rowWrapper.findComponent(GlButton).exists(),
     };
   };
 

spec/frontend/members/components/action_buttons/remove_member_button_spec.js

@@ -54,6 +54,8 @@ describe('RemoveMemberButton', () => {
     });
   };
 
+  const findButton = () => wrapper.findComponent(GlButton);
+
   beforeEach(() => {
     createComponent();
   });
@@ -66,7 +68,6 @@ describe('RemoveMemberButton', () => {
     expect(wrapper.attributes()).toMatchObject({
       'aria-label': 'Remove member',
       title: 'Remove member',
-      icon: 'remove',
     });
   });
 
@@ -75,8 +76,22 @@ describe('RemoveMemberButton', () => {
   });
 
   it('calls Vuex action to show `remove member` modal when clicked', () => {
-    wrapper.findComponent(GlButton).vm.$emit('click');
+    findButton().vm.$emit('click');
 
     expect(actions.showRemoveMemberModal).toHaveBeenCalledWith(expect.any(Object), modalData);
   });
+
+  describe('button optional properties', () => {
+    it('has default value for category and text', () => {
+      createComponent();
+      expect(findButton().props('category')).toBe('secondary');
+      expect(findButton().text()).toBe('');
+    });
+
+    it('allow changing value of button category and text', () => {
+      createComponent({ buttonCategory: 'primary', buttonText: 'Decline request' });
+      expect(findButton().props('category')).toBe('primary');
+      expect(findButton().text()).toBe('Decline request');
+    });
+  });
 });

spec/frontend/members/components/action_buttons/user_action_buttons_spec.js

@@ -13,6 +13,7 @@ describe('UserActionButtons', () => {
       propsData: {
         member,
         isCurrentUser: false,
+        isInvitedUser: false,
         ...propsData,
       },
     });
@@ -45,7 +46,9 @@ describe('UserActionButtons', () => {
         title: 'Remove member',
         isAccessRequest: false,
         isInvite: false,
-        icon: 'remove',
+        icon: '',
+        buttonCategory: 'secondary',
+        buttonText: 'Remove user',
         userDeletionObstacles: {
           name: member.user.name,
           obstacles: parseUserDeletionObstacles(member.user),
@@ -129,4 +132,30 @@ describe('UserActionButtons', () => {
       expect(findRemoveMemberButton().props().memberType).toBe('ProjectMember');
     });
   });
+
+  describe('isInvitedUser', () => {
+    it.each`
+      isInvitedUser | icon        | buttonText       | buttonCategory
+      ${true}       | ${'remove'} | ${null}          | ${'primary'}
+      ${false}      | ${''}       | ${'Remove user'} | ${'secondary'}
+    `(
+      'passes the correct props to remove-member-button when isInvitedUser is $isInvitedUser',
+      ({ isInvitedUser, icon, buttonText, buttonCategory }) => {
+        createComponent({
+          isInvitedUser,
+          permissions: {
+            canRemove: true,
+          },
+        });
+
+        expect(findRemoveMemberButton().props()).toEqual(
+          expect.objectContaining({
+            icon,
+            buttonText,
+            buttonCategory,
+          }),
+        );
+      },
+    );
+  });
 });

spec/frontend/members/components/table/member_action_buttons_spec.js

@@ -14,6 +14,7 @@ describe('MemberActionButtons', () => {
     wrapper = shallowMount(MemberActionButtons, {
       propsData: {
         isCurrentUser: false,
+        isInvitedUser: false,
         permissions: {
           canRemove: true,
         },