Commit d0f42601 authored by peterhegman's avatar peterhegman

Hide "Actions" field when there are no buttons to display

Part of a larger initiative to convert the group members view from
HAML to Vue
parent 8f1dc265
...@@ -51,6 +51,7 @@ export const FIELDS = [ ...@@ -51,6 +51,7 @@ export const FIELDS = [
key: 'actions', key: 'actions',
thClass: 'col-actions', thClass: 'col-actions',
tdClass: 'col-actions', tdClass: 'col-actions',
showFunction: 'showActionsField',
}, },
]; ];
......
...@@ -2,6 +2,12 @@ ...@@ -2,6 +2,12 @@
import { mapState } from 'vuex'; import { mapState } from 'vuex';
import { GlTable, GlBadge } from '@gitlab/ui'; import { GlTable, GlBadge } from '@gitlab/ui';
import MembersTableCell from 'ee_else_ce/vue_shared/components/members/table/members_table_cell.vue'; 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 { FIELDS } from '../constants';
import initUserPopovers from '~/user_popovers'; import initUserPopovers from '~/user_popovers';
import MemberAvatar from './member_avatar.vue'; import MemberAvatar from './member_avatar.vue';
...@@ -33,14 +39,40 @@ export default { ...@@ -33,14 +39,40 @@ export default {
), ),
}, },
computed: { computed: {
...mapState(['members', 'tableFields']), ...mapState(['members', 'tableFields', 'currentUserId', 'sourceId']),
filteredFields() { 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() { mounted() {
initUserPopovers(this.$el.querySelectorAll('.js-user-link')); 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> </script>
......
<script> <script>
import { mapState } from 'vuex'; import { mapState } from 'vuex';
import { MEMBER_TYPES } from '../constants'; import { MEMBER_TYPES } from '../constants';
import { isGroup, isDirectMember, isCurrentUser, canRemove, canResend, canUpdate } from '../utils';
export default { export default {
name: 'MembersTableCell', name: 'MembersTableCell',
...@@ -13,7 +14,7 @@ export default { ...@@ -13,7 +14,7 @@ export default {
computed: { computed: {
...mapState(['sourceId', 'currentUserId']), ...mapState(['sourceId', 'currentUserId']),
isGroup() { isGroup() {
return Boolean(this.member.sharedWithGroup); return isGroup(this.member);
}, },
isInvite() { isInvite() {
return Boolean(this.member.invite); return Boolean(this.member.invite);
...@@ -33,19 +34,19 @@ export default { ...@@ -33,19 +34,19 @@ export default {
return MEMBER_TYPES.user; return MEMBER_TYPES.user;
}, },
isDirectMember() { isDirectMember() {
return this.isGroup || this.member.source?.id === this.sourceId; return isDirectMember(this.member, this.sourceId);
}, },
isCurrentUser() { isCurrentUser() {
return this.member.user?.id === this.currentUserId; return isCurrentUser(this.member, this.currentUserId);
}, },
canRemove() { canRemove() {
return this.isDirectMember && this.member.canRemove; return canRemove(this.member, this.sourceId);
}, },
canResend() { canResend() {
return Boolean(this.member.invite?.canResend); return canResend(this.member);
}, },
canUpdate() { canUpdate() {
return !this.isCurrentUser && this.isDirectMember && this.member.canUpdate; return canUpdate(this.member, this.currentUserId, this.sourceId);
}, },
}, },
render() { render() {
......
...@@ -17,3 +17,32 @@ export const generateBadges = (member, isCurrentUser) => [ ...@@ -17,3 +17,32 @@ export const generateBadges = (member, isCurrentUser) => [
variant: 'info', 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;
<script> <script>
import CEMembersTableCell from '~/vue_shared/components/members/table/members_table_cell.vue'; import CEMembersTableCell from '~/vue_shared/components/members/table/members_table_cell.vue';
import { canOverride } from '../utils';
export default { export default {
name: 'MembersTableCell', name: 'MembersTableCell',
...@@ -11,7 +12,7 @@ export default { ...@@ -11,7 +12,7 @@ export default {
}, },
computed: { computed: {
canOverride() { canOverride() {
return this.member.canOverride; return canOverride(this.member);
}, },
}, },
render(createElement) { render(createElement) {
......
import { __ } from '~/locale'; import { __ } from '~/locale';
import { generateBadges as CEGenerateBadges } from '~/vue_shared/components/members/utils'; 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) => [ export const generateBadges = (member, isCurrentUser) => [
...CEGenerateBadges(member, isCurrentUser), ...CEGenerateBadges(member, isCurrentUser),
{ {
...@@ -24,3 +33,5 @@ export const generateBadges = (member, isCurrentUser) => [ ...@@ -24,3 +33,5 @@ export const generateBadges = (member, isCurrentUser) => [
variant: 'info', variant: 'info',
}, },
]; ];
export const canOverride = member => member.canOverride;
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);
});
});
});
});
});
import { member as memberMock } from 'jest/vue_shared/components/members/mock_data'; 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('Members Utils', () => {
describe('generateBadges', () => { describe('generateBadges', () => {
...@@ -27,4 +27,14 @@ describe('Members Utils', () => { ...@@ -27,4 +27,14 @@ describe('Members Utils', () => {
expect(generateBadges(member, true)).toContainEqual(expect.objectContaining(expected)); 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);
});
});
}); });
...@@ -3,6 +3,7 @@ import Vuex from 'vuex'; ...@@ -3,6 +3,7 @@ import Vuex from 'vuex';
import { import {
getByText as getByTextHelper, getByText as getByTextHelper,
getByTestId as getByTestIdHelper, getByTestId as getByTestIdHelper,
within,
} from '@testing-library/dom'; } from '@testing-library/dom';
import { GlBadge } from '@gitlab/ui'; import { GlBadge } from '@gitlab/ui';
import MembersTable from '~/vue_shared/components/members/table/members_table.vue'; import MembersTable from '~/vue_shared/components/members/table/members_table.vue';
...@@ -28,6 +29,7 @@ describe('MemberList', () => { ...@@ -28,6 +29,7 @@ describe('MemberList', () => {
members: [], members: [],
tableFields: [], tableFields: [],
sourceId: 1, sourceId: 1,
currentUserId: 1,
...state, ...state,
}, },
}); });
...@@ -62,12 +64,16 @@ describe('MemberList', () => { ...@@ -62,12 +64,16 @@ describe('MemberList', () => {
}); });
describe('fields', () => { describe('fields', () => {
const memberCanUpdate = { const directMember = {
...memberMock, ...memberMock,
canUpdate: true,
source: { ...memberMock.source, id: 1 }, source: { ...memberMock.source, id: 1 },
}; };
const memberCanUpdate = {
...directMember,
canUpdate: true,
};
it.each` it.each`
field | label | member | expectedComponent field | label | member | expectedComponent
${'account'} | ${'Account'} | ${memberMock} | ${MemberAvatar} ${'account'} | ${'Account'} | ${memberMock} | ${MemberAvatar}
...@@ -96,8 +102,9 @@ describe('MemberList', () => { ...@@ -96,8 +102,9 @@ describe('MemberList', () => {
} }
}); });
describe('"Actions" field', () => {
it('renders "Actions" field for screen readers', () => { it('renders "Actions" field for screen readers', () => {
createComponent({ members: [memberMock], tableFields: ['actions'] }); createComponent({ members: [memberCanUpdate], tableFields: ['actions'] });
const actionField = getByTestId('col-actions'); const actionField = getByTestId('col-actions');
...@@ -110,6 +117,46 @@ describe('MemberList', () => { ...@@ -110,6 +117,46 @@ describe('MemberList', () => {
.exists(), .exists(),
).toBe(true); ).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);
});
});
});
}); });
describe('when `members` is an empty array', () => { describe('when `members` is an empty array', () => {
......
import { generateBadges } from '~/vue_shared/components/members/utils'; import {
import { member as memberMock } from './mock_data'; 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('Members Utils', () => {
describe('generateBadges', () => { describe('generateBadges', () => {
...@@ -26,4 +40,83 @@ describe('Members Utils', () => { ...@@ -26,4 +40,83 @@ describe('Members Utils', () => {
expect(generateBadges(member, true)).toContainEqual(expect.objectContaining(expected)); 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);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment