Merge branch 'peterhegman/hide-actions-field-with-no-buttons' into 'master'

Hide "Actions" field when there are no buttons to display [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!46506

app/assets/javascripts/vue_shared/components/members/constants.js

@@ -51,6 +51,7 @@ export const FIELDS = [
     key: 'actions',
     thClass: 'col-actions',
     tdClass: 'col-actions',
+    showFunction: 'showActionsField',
   },
 ];
 

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

@@ -2,6 +2,12 @@
 import { mapState } from 'vuex';
 import { GlTable, GlBadge } from '@gitlab/ui';
 import MembersTableCell from 'ee_else_ce/vue_shared/components/members/table/members_table_cell.vue';
+import {
+  canOverride,
+  canRemove,
+  canResend,
+  canUpdate,
+} from 'ee_else_ce/vue_shared/components/members/utils';
 import { FIELDS } from '../constants';
 import initUserPopovers from '~/user_popovers';
 import MemberAvatar from './member_avatar.vue';
@@ -33,14 +39,40 @@ export default {
       ),
   },
   computed: {
-    ...mapState(['members', 'tableFields']),
+    ...mapState(['members', 'tableFields', 'currentUserId', 'sourceId']),
     filteredFields() {
-      return FIELDS.filter(field => this.tableFields.includes(field.key));
+      return FIELDS.filter(field => this.tableFields.includes(field.key) && this.showField(field));
+    },
+    userIsLoggedIn() {
+      return this.currentUserId !== null;
     },
   },
   mounted() {
     initUserPopovers(this.$el.querySelectorAll('.js-user-link'));
   },
+  methods: {
+    showField(field) {
+      if (!Object.prototype.hasOwnProperty.call(field, 'showFunction')) {
+        return true;
+      }
+
+      return this[field.showFunction]();
+    },
+    showActionsField() {
+      if (!this.userIsLoggedIn) {
+        return false;
+      }
+
+      return this.members.some(member => {
+        return (
+          canRemove(member, this.sourceId) ||
+          canResend(member) ||
+          canUpdate(member, this.currentUserId, this.sourceId) ||
+          canOverride(member)
+        );
+      });
+    },
+  },
 };
 </script>
 

app/assets/javascripts/vue_shared/components/members/table/members_table_cell.vue

 <script>
 import { mapState } from 'vuex';
 import { MEMBER_TYPES } from '../constants';
+import { isGroup, isDirectMember, isCurrentUser, canRemove, canResend, canUpdate } from '../utils';
 
 export default {
   name: 'MembersTableCell',
@@ -13,7 +14,7 @@ export default {
   computed: {
     ...mapState(['sourceId', 'currentUserId']),
     isGroup() {
-      return Boolean(this.member.sharedWithGroup);
+      return isGroup(this.member);
     },
     isInvite() {
       return Boolean(this.member.invite);
@@ -33,19 +34,19 @@ export default {
       return MEMBER_TYPES.user;
     },
     isDirectMember() {
-      return this.isGroup || this.member.source?.id === this.sourceId;
+      return isDirectMember(this.member, this.sourceId);
     },
     isCurrentUser() {
-      return this.member.user?.id === this.currentUserId;
+      return isCurrentUser(this.member, this.currentUserId);
     },
     canRemove() {
-      return this.isDirectMember && this.member.canRemove;
+      return canRemove(this.member, this.sourceId);
     },
     canResend() {
-      return Boolean(this.member.invite?.canResend);
+      return canResend(this.member);
     },
     canUpdate() {
-      return !this.isCurrentUser && this.isDirectMember && this.member.canUpdate;
+      return canUpdate(this.member, this.currentUserId, this.sourceId);
     },
   },
   render() {

app/assets/javascripts/vue_shared/components/members/utils.js

@@ -17,3 +17,32 @@ export const generateBadges = (member, isCurrentUser) => [
     variant: 'info',
   },
 ];
+
+export const isGroup = member => {
+  return Boolean(member.sharedWithGroup);
+};
+
+export const isDirectMember = (member, sourceId) => {
+  return isGroup(member) || member.source?.id === sourceId;
+};
+
+export const isCurrentUser = (member, currentUserId) => {
+  return member.user?.id === currentUserId;
+};
+
+export const canRemove = (member, sourceId) => {
+  return isDirectMember(member, sourceId) && member.canRemove;
+};
+
+export const canResend = member => {
+  return Boolean(member.invite?.canResend);
+};
+
+export const canUpdate = (member, currentUserId, sourceId) => {
+  return (
+    !isCurrentUser(member, currentUserId) && isDirectMember(member, sourceId) && member.canUpdate
+  );
+};
+
+// Defined in `ee/app/assets/javascripts/vue_shared/components/members/utils.js`
+export const canOverride = () => false;

ee/app/assets/javascripts/vue_shared/components/members/table/members_table_cell.vue

 <script>
 import CEMembersTableCell from '~/vue_shared/components/members/table/members_table_cell.vue';
+import { canOverride } from '../utils';
 
 export default {
   name: 'MembersTableCell',
@@ -11,7 +12,7 @@ export default {
   },
   computed: {
     canOverride() {
-      return this.member.canOverride;
+      return canOverride(this.member);
     },
   },
   render(createElement) {

ee/app/assets/javascripts/vue_shared/components/members/utils.js

 import { __ } from '~/locale';
 import { generateBadges as CEGenerateBadges } from '~/vue_shared/components/members/utils';
 
+export {
+  isGroup,
+  isDirectMember,
+  isCurrentUser,
+  canRemove,
+  canResend,
+  canUpdate,
+} from '~/vue_shared/components/members/utils';
+
 export const generateBadges = (member, isCurrentUser) => [
   ...CEGenerateBadges(member, isCurrentUser),
   {
@@ -24,3 +33,5 @@ export const generateBadges = (member, isCurrentUser) => [
     variant: 'info',
   },
 ];
+
+export const canOverride = member => member.canOverride;

ee/spec/frontend/vue_shared/components/members/table/members_table_spec.js

+import { mount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import { within } from '@testing-library/dom';
+import { member as memberMock, members } from 'jest/vue_shared/components/members/mock_data';
+import MembersTable from '~/vue_shared/components/members/table/members_table.vue';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+describe('MemberList', () => {
+  let wrapper;
+
+  const createStore = (state = {}) => {
+    return new Vuex.Store({
+      state: {
+        members: [],
+        tableFields: [],
+        sourceId: 1,
+        currentUserId: 1,
+        ...state,
+      },
+    });
+  };
+
+  const createComponent = state => {
+    wrapper = mount(MembersTable, {
+      localVue,
+      store: createStore(state),
+      stubs: [
+        'member-avatar',
+        'member-source',
+        'expires-at',
+        'created-at',
+        'member-action-buttons',
+        'role-dropdown',
+        'remove-group-link-modal',
+        'expiration-datepicker',
+      ],
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  describe('fields', () => {
+    describe('"Actions" field', () => {
+      const memberCanOverride = {
+        ...memberMock,
+        source: { ...memberMock.source, id: 1 },
+        canOverride: true,
+      };
+
+      describe('when one of the members has `canOverride` permissions', () => {
+        it('renders the "Actions" field', () => {
+          createComponent({ members: [memberCanOverride], tableFields: ['actions'] });
+
+          expect(within(wrapper.element).queryByTestId('col-actions')).not.toBe(null);
+        });
+      });
+
+      describe('when none of the members have `canOverride` permissions', () => {
+        it('does not render the "Actions" field', () => {
+          createComponent({ members, tableFields: ['actions'] });
+
+          expect(within(wrapper.element).queryByTestId('col-actions')).toBe(null);
+        });
+      });
+    });
+  });
+});

ee/spec/frontend/vue_shared/components/members/utils_spec.js

 import { member as memberMock } from 'jest/vue_shared/components/members/mock_data';
-import { generateBadges } from 'ee/vue_shared/components/members/utils';
+import { generateBadges, canOverride } from 'ee/vue_shared/components/members/utils';
 
 describe('Members Utils', () => {
   describe('generateBadges', () => {
@@ -27,4 +27,14 @@ describe('Members Utils', () => {
       expect(generateBadges(member, true)).toContainEqual(expect.objectContaining(expected));
     });
   });
+
+  describe('canOverride', () => {
+    test.each`
+      member                                  | expected
+      ${{ ...memberMock, canOverride: true }} | ${true}
+      ${memberMock}                           | ${false}
+    `('returns $expected', ({ member, expected }) => {
+      expect(canOverride(member)).toBe(expected);
+    });
+  });
 });

spec/frontend/vue_shared/components/members/table/members_table_spec.js

@@ -3,6 +3,7 @@ import Vuex from 'vuex';
 import {
   getByText as getByTextHelper,
   getByTestId as getByTestIdHelper,
+  within,
 } from '@testing-library/dom';
 import { GlBadge } from '@gitlab/ui';
 import MembersTable from '~/vue_shared/components/members/table/members_table.vue';
@@ -28,6 +29,7 @@ describe('MemberList', () => {
         members: [],
         tableFields: [],
         sourceId: 1,
+        currentUserId: 1,
         ...state,
       },
     });
@@ -62,12 +64,16 @@ describe('MemberList', () => {
   });
 
   describe('fields', () => {
-    const memberCanUpdate = {
+    const directMember = {
       ...memberMock,
-      canUpdate: true,
       source: { ...memberMock.source, id: 1 },
     };
 
+    const memberCanUpdate = {
+      ...directMember,
+      canUpdate: true,
+    };
+
     it.each`
       field           | label               | member             | expectedComponent
       ${'account'}    | ${'Account'}        | ${memberMock}      | ${MemberAvatar}
@@ -96,19 +102,60 @@ describe('MemberList', () => {
       }
     });
 
-    it('renders "Actions" field for screen readers', () => {
-      createComponent({ members: [memberMock], tableFields: ['actions'] });
+    describe('"Actions" field', () => {
+      it('renders "Actions" field for screen readers', () => {
+        createComponent({ members: [memberCanUpdate], tableFields: ['actions'] });
 
-      const actionField = getByTestId('col-actions');
+        const actionField = getByTestId('col-actions');
 
-      expect(actionField.exists()).toBe(true);
-      expect(actionField.classes('gl-sr-only')).toBe(true);
-      expect(
-        wrapper
-          .find(`[data-label="Actions"][role="cell"]`)
-          .find(MemberActionButtons)
-          .exists(),
-      ).toBe(true);
+        expect(actionField.exists()).toBe(true);
+        expect(actionField.classes('gl-sr-only')).toBe(true);
+        expect(
+          wrapper
+            .find(`[data-label="Actions"][role="cell"]`)
+            .find(MemberActionButtons)
+            .exists(),
+        ).toBe(true);
+      });
+
+      describe('when user is not logged in', () => {
+        it('does not render the "Actions" field', () => {
+          createComponent({ currentUserId: null, tableFields: ['actions'] });
+
+          expect(within(wrapper.element).queryByTestId('col-actions')).toBe(null);
+        });
+      });
+
+      const memberCanRemove = {
+        ...directMember,
+        canRemove: true,
+      };
+
+      describe.each`
+        permission     | members
+        ${'canUpdate'} | ${[memberCanUpdate]}
+        ${'canRemove'} | ${[memberCanRemove]}
+        ${'canResend'} | ${[invite]}
+      `('when one of the members has $permission permissions', ({ members }) => {
+        it('renders the "Actions" field', () => {
+          createComponent({ members, tableFields: ['actions'] });
+
+          expect(getByTestId('col-actions').exists()).toBe(true);
+        });
+      });
+
+      describe.each`
+        permission     | members
+        ${'canUpdate'} | ${[memberMock]}
+        ${'canRemove'} | ${[memberMock]}
+        ${'canResend'} | ${[{ ...invite, invite: { ...invite.invite, canResend: false } }]}
+      `('when none of the members have $permission permissions', ({ members }) => {
+        it('does not render the "Actions" field', () => {
+          createComponent({ members, tableFields: ['actions'] });
+
+          expect(within(wrapper.element).queryByTestId('col-actions')).toBe(null);
+        });
+      });
     });
   });
 

spec/frontend/vue_shared/components/members/utils_spec.js

-import { generateBadges } from '~/vue_shared/components/members/utils';
-import { member as memberMock } from './mock_data';
+import {
+  generateBadges,
+  isGroup,
+  isDirectMember,
+  isCurrentUser,
+  canRemove,
+  canResend,
+  canUpdate,
+  canOverride,
+} from '~/vue_shared/components/members/utils';
+import { member as memberMock, group, invite } from './mock_data';
+
+const DIRECT_MEMBER_ID = 178;
+const INHERITED_MEMBER_ID = 179;
+const IS_CURRENT_USER_ID = 123;
+const IS_NOT_CURRENT_USER_ID = 124;
 
 describe('Members Utils', () => {
   describe('generateBadges', () => {
@@ -26,4 +40,83 @@ describe('Members Utils', () => {
       expect(generateBadges(member, true)).toContainEqual(expect.objectContaining(expected));
     });
   });
+
+  describe('isGroup', () => {
+    test.each`
+      member        | expected
+      ${group}      | ${true}
+      ${memberMock} | ${false}
+    `('returns $expected', ({ member, expected }) => {
+      expect(isGroup(member)).toBe(expected);
+    });
+  });
+
+  describe('isDirectMember', () => {
+    test.each`
+      sourceId               | expected
+      ${DIRECT_MEMBER_ID}    | ${true}
+      ${INHERITED_MEMBER_ID} | ${false}
+    `('returns $expected', ({ sourceId, expected }) => {
+      expect(isDirectMember(memberMock, sourceId)).toBe(expected);
+    });
+  });
+
+  describe('isCurrentUser', () => {
+    test.each`
+      currentUserId             | expected
+      ${IS_CURRENT_USER_ID}     | ${true}
+      ${IS_NOT_CURRENT_USER_ID} | ${false}
+    `('returns $expected', ({ currentUserId, expected }) => {
+      expect(isCurrentUser(memberMock, currentUserId)).toBe(expected);
+    });
+  });
+
+  describe('canRemove', () => {
+    const memberCanRemove = {
+      ...memberMock,
+      canRemove: true,
+    };
+
+    test.each`
+      member             | sourceId               | expected
+      ${memberCanRemove} | ${DIRECT_MEMBER_ID}    | ${true}
+      ${memberCanRemove} | ${INHERITED_MEMBER_ID} | ${false}
+      ${memberMock}      | ${INHERITED_MEMBER_ID} | ${false}
+    `('returns $expected', ({ member, sourceId, expected }) => {
+      expect(canRemove(member, sourceId)).toBe(expected);
+    });
+  });
+
+  describe('canResend', () => {
+    test.each`
+      member                                                           | expected
+      ${invite}                                                        | ${true}
+      ${{ ...invite, invite: { ...invite.invite, canResend: false } }} | ${false}
+    `('returns $expected', ({ member, sourceId, expected }) => {
+      expect(canResend(member, sourceId)).toBe(expected);
+    });
+  });
+
+  describe('canUpdate', () => {
+    const memberCanUpdate = {
+      ...memberMock,
+      canUpdate: true,
+    };
+
+    test.each`
+      member             | currentUserId             | sourceId               | expected
+      ${memberCanUpdate} | ${IS_NOT_CURRENT_USER_ID} | ${DIRECT_MEMBER_ID}    | ${true}
+      ${memberCanUpdate} | ${IS_CURRENT_USER_ID}     | ${DIRECT_MEMBER_ID}    | ${false}
+      ${memberCanUpdate} | ${IS_CURRENT_USER_ID}     | ${INHERITED_MEMBER_ID} | ${false}
+      ${memberMock}      | ${IS_NOT_CURRENT_USER_ID} | ${DIRECT_MEMBER_ID}    | ${false}
+    `('returns $expected', ({ member, currentUserId, sourceId, expected }) => {
+      expect(canUpdate(member, currentUserId, sourceId)).toBe(expected);
+    });
+  });
+
+  describe('canOverride', () => {
+    it('returns `false`', () => {
+      expect(canOverride(memberMock)).toBe(false);
+    });
+  });
 });