Merge branch...

Merge branch '235603-convert-group-members-list-view-from-haml-to-vue-role-dropdown-component' into 'master'

Group members Vue conversion - role dropdown

See merge request gitlab-org/gitlab!44651

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

@@ -38,8 +38,8 @@ export const FIELDS = [
   {
     key: 'maxRole',
     label: __('Max role'),
-    thClass: 'col-meta',
-    tdClass: 'col-meta',
+    thClass: 'col-max-role',
+    tdClass: 'col-max-role',
   },
   {
     key: 'expiration',

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

 <script>
 import { mapState } from 'vuex';
-import { GlTable } from '@gitlab/ui';
+import { GlTable, GlBadge } from '@gitlab/ui';
 import { FIELDS } from '../constants';
 import initUserPopovers from '~/user_popovers';
 import MemberAvatar from './member_avatar.vue';
@@ -9,17 +9,20 @@ import CreatedAt from './created_at.vue';
 import ExpiresAt from './expires_at.vue';
 import MemberActionButtons from './member_action_buttons.vue';
 import MembersTableCell from './members_table_cell.vue';
+import RoleDropdown from './role_dropdown.vue';
 
 export default {
   name: 'MembersTable',
   components: {
     GlTable,
+    GlBadge,
     MemberAvatar,
     CreatedAt,
     ExpiresAt,
     MembersTableCell,
     MemberSource,
     MemberActionButtons,
+    RoleDropdown,
   },
   computed: {
     ...mapState(['members', 'tableFields']),
@@ -77,6 +80,13 @@ export default {
       <expires-at :date="expiresAt" />
     </template>
 
+    <template #cell(maxRole)="{ item: member }">
+      <members-table-cell #default="{ permissions }" :member="member">
+        <role-dropdown v-if="permissions.canUpdate" :member="member" />
+        <gl-badge v-else>{{ member.accessLevel.stringValue }}</gl-badge>
+      </members-table-cell>
+    </template>
+
     <template #cell(actions)="{ item: member }">
       <members-table-cell #default="{ memberType, isCurrentUser, permissions }" :member="member">
         <member-action-buttons

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

@@ -33,7 +33,7 @@ export default {
       return MEMBER_TYPES.user;
     },
     isDirectMember() {
-      return this.member.source?.id === this.sourceId;
+      return this.isGroup || this.member.source?.id === this.sourceId;
     },
     isCurrentUser() {
       return this.member.user?.id === this.currentUserId;
@@ -44,6 +44,9 @@ export default {
     canResend() {
       return Boolean(this.member.invite?.canResend);
     },
+    canUpdate() {
+      return !this.isCurrentUser && this.isDirectMember && this.member.canUpdate;
+    },
   },
   render() {
     return this.$scopedSlots.default({
@@ -53,6 +56,7 @@ export default {
       permissions: {
         canRemove: this.canRemove,
         canResend: this.canResend,
+        canUpdate: this.canUpdate,
       },
     });
   },

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

+<script>
+import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
+import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
+
+export default {
+  name: 'RoleDropdown',
+  components: {
+    GlDropdown,
+    GlDropdownItem,
+  },
+  props: {
+    member: {
+      type: Object,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      isDesktop: false,
+    };
+  },
+  mounted() {
+    this.isDesktop = bp.isDesktop();
+  },
+  methods: {
+    handleSelect() {
+      // Vuex action will be called here to make API request and update `member.accessLevel`
+    },
+  },
+};
+</script>
+
+<template>
+  <gl-dropdown
+    :right="!isDesktop"
+    :text="member.accessLevel.stringValue"
+    :header-text="__('Change permissions')"
+  >
+    <gl-dropdown-item
+      v-for="(value, name) in member.validRoles"
+      :key="value"
+      is-check-item
+      :is-checked="value === member.accessLevel.integerValue"
+      @click="handleSelect"
+    >
+      {{ name }}
+    </gl-dropdown-item>
+  </gl-dropdown>
+</template>

app/assets/stylesheets/pages/members.scss

@@ -216,6 +216,10 @@
       width: px-to-rem(150px);
     }
 
+    .col-max-role {
+      width: px-to-rem(175px);
+    }
+
     .col-expiration {
       width: px-to-rem(200px);
     }

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

@@ -24,6 +24,14 @@ export const member = {
   usingLicense: false,
   groupSso: false,
   groupManagedAccount: false,
+  validRoles: {
+    Guest: 10,
+    Reporter: 20,
+    Developer: 30,
+    Maintainer: 40,
+    Owner: 50,
+    'Minimal Access': 5,
+  },
 };
 
 export const group = {
@@ -39,6 +47,7 @@ export const group = {
   id: 3,
   createdAt: '2020-08-06T15:31:07.662Z',
   expiresAt: null,
+  validRoles: { Guest: 10, Reporter: 20, Developer: 30, Maintainer: 40, Owner: 50 },
 };
 
 const { user, ...memberNoUser } = member;

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

@@ -65,6 +65,14 @@ describe('MemberList', () => {
 
   const findWrappedComponent = () => wrapper.find(WrappedComponent);
 
+  const memberCurrentUser = {
+    ...memberMock,
+    user: {
+      ...memberMock.user,
+      id: 1,
+    },
+  };
+
   const createComponentWithDirectMember = (member = {}) => {
     createComponent({
       member: {
@@ -115,18 +123,20 @@ describe('MemberList', () => {
 
       expect(findWrappedComponent().props('isDirectMember')).toBe(false);
     });
+
+    it('returns `true` for linked groups', () => {
+      createComponent({
+        member: group,
+      });
+
+      expect(findWrappedComponent().props('isDirectMember')).toBe(true);
+    });
   });
 
   describe('isCurrentUser', () => {
     it('returns `true` when `member.user` has the same ID as `currentUserId`', () => {
       createComponent({
-        member: {
-          ...memberMock,
-          user: {
-            ...memberMock.user,
-            id: 1,
-          },
-        },
+        member: memberCurrentUser,
       });
 
       expect(findWrappedComponent().props('isCurrentUser')).toBe(true);
@@ -203,5 +213,39 @@ describe('MemberList', () => {
         });
       });
     });
+
+    describe('canUpdate', () => {
+      describe('for a direct member', () => {
+        it('returns `true` when `canUpdate` is `true`', () => {
+          createComponentWithDirectMember({
+            canUpdate: true,
+          });
+
+          expect(findWrappedComponent().props('permissions').canUpdate).toBe(true);
+        });
+
+        it('returns `false` when `canUpdate` is `false`', () => {
+          createComponentWithDirectMember({
+            canUpdate: false,
+          });
+
+          expect(findWrappedComponent().props('permissions').canUpdate).toBe(false);
+        });
+
+        it('returns `false` for current user', () => {
+          createComponentWithDirectMember(memberCurrentUser);
+
+          expect(findWrappedComponent().props('permissions').canUpdate).toBe(false);
+        });
+      });
+
+      describe('for an inherited member', () => {
+        it('returns `false`', () => {
+          createComponentWithInheritedMember();
+
+          expect(findWrappedComponent().props('permissions').canUpdate).toBe(false);
+        });
+      });
+    });
   });
 });

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

@@ -4,11 +4,13 @@ import {
   getByText as getByTextHelper,
   getByTestId as getByTestIdHelper,
 } from '@testing-library/dom';
+import { GlBadge } from '@gitlab/ui';
 import MembersTable from '~/vue_shared/components/members/table/members_table.vue';
 import MemberAvatar from '~/vue_shared/components/members/table/member_avatar.vue';
 import MemberSource from '~/vue_shared/components/members/table/member_source.vue';
 import ExpiresAt from '~/vue_shared/components/members/table/expires_at.vue';
 import CreatedAt from '~/vue_shared/components/members/table/created_at.vue';
+import RoleDropdown from '~/vue_shared/components/members/table/role_dropdown.vue';
 import MemberActionButtons from '~/vue_shared/components/members/table/member_action_buttons.vue';
 import * as initUserPopovers from '~/user_popovers';
 import { member as memberMock, invite, accessRequest } from '../mock_data';
@@ -24,6 +26,7 @@ describe('MemberList', () => {
       state: {
         members: [],
         tableFields: [],
+        sourceId: 1,
         ...state,
       },
     });
@@ -39,6 +42,7 @@ describe('MemberList', () => {
         'expires-at',
         'created-at',
         'member-action-buttons',
+        'role-dropdown',
       ],
     });
   };
@@ -55,6 +59,12 @@ describe('MemberList', () => {
   });
 
   describe('fields', () => {
+    const memberCanUpdate = {
+      ...memberMock,
+      canUpdate: true,
+      source: { ...memberMock.source, id: 1 },
+    };
+
     it.each`
       field           | label               | member             | expectedComponent
       ${'account'}    | ${'Account'}        | ${memberMock}      | ${MemberAvatar}
@@ -63,7 +73,7 @@ describe('MemberList', () => {
       ${'invited'}    | ${'Invited'}        | ${invite}          | ${CreatedAt}
       ${'requested'}  | ${'Requested'}      | ${accessRequest}   | ${CreatedAt}
       ${'expires'}    | ${'Access expires'} | ${memberMock}      | ${ExpiresAt}
-      ${'maxRole'}    | ${'Max role'}       | ${memberMock}    | ${null}
+      ${'maxRole'}    | ${'Max role'}       | ${memberCanUpdate} | ${RoleDropdown}
       ${'expiration'} | ${'Expiration'}     | ${memberMock}      | ${null}
     `('renders the $label field', ({ field, label, member, expectedComponent }) => {
       createComponent({
@@ -107,6 +117,19 @@ describe('MemberList', () => {
     });
   });
 
+  describe('when member can not be updated', () => {
+    it('renders badge in "Max role" field', () => {
+      createComponent({ members: [memberMock], tableFields: ['maxRole'] });
+
+      expect(
+        wrapper
+          .find(`[data-label="Max role"][role="cell"]`)
+          .find(GlBadge)
+          .text(),
+      ).toBe(memberMock.accessLevel.stringValue);
+    });
+  });
+
   it('initializes user popovers when mounted', () => {
     const initUserPopoversMock = jest.spyOn(initUserPopovers, 'default');
 

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

+import { mount, createWrapper } from '@vue/test-utils';
+import { nextTick } from 'vue';
+import { within } from '@testing-library/dom';
+import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
+import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
+import RoleDropdown from '~/vue_shared/components/members/table/role_dropdown.vue';
+import { member } from '../mock_data';
+
+describe('RoleDropdown', () => {
+  let wrapper;
+
+  const createComponent = (propsData = {}) => {
+    wrapper = mount(RoleDropdown, {
+      propsData: {
+        member,
+        ...propsData,
+      },
+    });
+  };
+
+  const getDropdownMenu = () => within(wrapper.element).getByRole('menu');
+  const getByTextInDropdownMenu = (text, options = {}) =>
+    createWrapper(within(getDropdownMenu()).getByText(text, options));
+  const getDropdownItemByText = text =>
+    getByTextInDropdownMenu(text, { selector: '[role="menuitem"] p' });
+  const getCheckedDropdownItem = () =>
+    wrapper
+      .findAll(GlDropdownItem)
+      .wrappers.find(dropdownItemWrapper => dropdownItemWrapper.props('isChecked'));
+
+  const findDropdownToggle = () => wrapper.find('button[aria-haspopup="true"]');
+  const findDropdown = () => wrapper.find(GlDropdown);
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('when dropdown is open', () => {
+    beforeEach(done => {
+      createComponent();
+
+      findDropdownToggle().trigger('click');
+      wrapper.vm.$root.$on('bv::dropdown::shown', () => {
+        done();
+      });
+    });
+
+    it('renders all valid roles', () => {
+      Object.keys(member.validRoles).forEach(role => {
+        expect(getDropdownItemByText(role).exists()).toBe(true);
+      });
+    });
+
+    it('renders dropdown header', () => {
+      expect(getByTextInDropdownMenu('Change permissions').exists()).toBe(true);
+    });
+
+    it('sets dropdown toggle and checks selected role', async () => {
+      expect(findDropdownToggle().text()).toBe('Owner');
+      expect(getCheckedDropdownItem().text()).toBe('Owner');
+    });
+  });
+
+  it("sets initial dropdown toggle value to member's role", () => {
+    createComponent();
+
+    expect(findDropdownToggle().text()).toBe('Owner');
+  });
+
+  it('sets the dropdown alignment to right on mobile', async () => {
+    jest.spyOn(bp, 'isDesktop').mockReturnValue(false);
+    createComponent();
+
+    await nextTick();
+
+    expect(findDropdown().attributes('right')).toBe('true');
+  });
+
+  it('sets the dropdown alignment to left on desktop', async () => {
+    jest.spyOn(bp, 'isDesktop').mockReturnValue(true);
+    createComponent();
+
+    await nextTick();
+
+    expect(findDropdown().attributes('right')).toBeUndefined();
+  });
+});