Commit fc935ff4 authored by Sheldon Led's avatar Sheldon Led Committed by Miguel Rincon

Add “Pending owner approval” badge for members

parent 0d3f6da1
...@@ -5,7 +5,13 @@ import MembersTableCell from 'ee_else_ce/members/components/table/members_table_ ...@@ -5,7 +5,13 @@ import MembersTableCell from 'ee_else_ce/members/components/table/members_table_
import { canOverride, canRemove, canResend, canUpdate } from 'ee_else_ce/members/utils'; import { canOverride, canRemove, canResend, canUpdate } from 'ee_else_ce/members/utils';
import { mergeUrlParams } from '~/lib/utils/url_utility'; import { mergeUrlParams } from '~/lib/utils/url_utility';
import initUserPopovers from '~/user_popovers'; import initUserPopovers from '~/user_popovers';
import { FIELDS, ACTIVE_TAB_QUERY_PARAM_NAME } from '../../constants'; import {
FIELDS,
ACTIVE_TAB_QUERY_PARAM_NAME,
MEMBER_STATE_AWAITING,
USER_STATE_BLOCKED_PENDING_APPROVAL,
BADGE_LABELS_PENDING_OWNER_APPROVAL,
} from '../../constants';
import RemoveGroupLinkModal from '../modals/remove_group_link_modal.vue'; import RemoveGroupLinkModal from '../modals/remove_group_link_modal.vue';
import RemoveMemberModal from '../modals/remove_member_modal.vue'; import RemoveMemberModal from '../modals/remove_member_modal.vue';
import CreatedAt from './created_at.vue'; import CreatedAt from './created_at.vue';
...@@ -129,6 +135,74 @@ export default { ...@@ -129,6 +135,74 @@ export default {
window.location.href, window.location.href,
); );
}, },
/**
* Returns whether it's a new or existing user
*
* If memberInviteMetadata doesn't exist, it means we're adding an existing user
* to the Group/Project, so `isNewUser` should be false.
* If memberInviteMetadata exists but `userState` has content,
* the user has registered but is awaiting root approval
*
* @param {object} memberInviteMetadata - MemberEntity.invite
* @see {@link ~/app/serializers/member_entity.rb}
* @returns {boolean}
*/
isNewUser(memberInviteMetadata) {
return memberInviteMetadata && !memberInviteMetadata.userState;
},
/**
* Returns whether the user is awaiting root approval
*
* This checks User.state exposed via MemberEntity
*
* @param {object} memberInviteMetadata - MemberEntity.invite
* @see {@link ~/app/serializers/member_entity.rb}
* @returns {boolean}
*/
isUserPendingRootApproval(memberInviteMetadata) {
return memberInviteMetadata?.userState === USER_STATE_BLOCKED_PENDING_APPROVAL;
},
/**
* Returns whether the member is awaiting owner approval
*
* This checks Member.state exposed via MemberEntity
*
* @param {Number} memberState - Member.state exposed via MemberEntity.state
* @see {@link ~/ee/app/models/ee/member.rb}
* @see {@link ~/app/serializers/member_entity.rb}
* @returns {boolean}
*/
isMemberPendingOwnerApproval(memberState) {
return memberState === MEMBER_STATE_AWAITING;
},
isUserAwaiting(memberInviteMetadata, memberState) {
return (
this.isUserPendingRootApproval(memberInviteMetadata) ||
this.isMemberPendingOwnerApproval(memberState)
);
},
shouldAddPendingOwnerApprovalBadge(memberInviteMetadata, memberState) {
return (
this.isUserAwaiting(memberInviteMetadata, memberState) &&
!this.isNewUser(memberInviteMetadata)
);
},
/**
* Returns the string to be used in the invite badge
*
* @param {object} memberInviteMetadata - MemberEntity.invite
* @see {@link ~/app/serializers/member_entity.rb}
* @param {Number} memberState - Member.state exposed via MemberEntity.state
* @see {@link ~/ee/app/models/ee/member.rb}
* @returns {string}
*/
inviteBadge(memberInviteMetadata, memberState) {
if (this.shouldAddPendingOwnerApprovalBadge(memberInviteMetadata, memberState)) {
return BADGE_LABELS_PENDING_OWNER_APPROVAL;
}
return '';
},
}, },
}; };
</script> </script>
...@@ -172,8 +246,11 @@ export default { ...@@ -172,8 +246,11 @@ export default {
<created-at :date="createdAt" :created-by="createdBy" /> <created-at :date="createdAt" :created-by="createdBy" />
</template> </template>
<template #cell(invited)="{ item: { createdAt, createdBy } }"> <template #cell(invited)="{ item: { createdAt, createdBy, invite, state } }">
<created-at :date="createdAt" :created-by="createdBy" /> <created-at :date="createdAt" :created-by="createdBy" />
<gl-badge v-if="inviteBadge(invite, state)" data-testid="invited-badge">{{
inviteBadge(invite, state)
}}</gl-badge>
</template> </template>
<template #cell(requested)="{ item: { createdAt } }"> <template #cell(requested)="{ item: { createdAt } }">
......
...@@ -89,6 +89,22 @@ export const TAB_QUERY_PARAM_VALUES = { ...@@ -89,6 +89,22 @@ export const TAB_QUERY_PARAM_VALUES = {
accessRequest: 'access_requests', accessRequest: 'access_requests',
}; };
/**
* This user state value comes from the User model
* see the state machine in app/models/user.rb
*/
export const USER_STATE_BLOCKED_PENDING_APPROVAL = 'blocked_pending_approval';
/**
* This and following member state constants' values
* come from ee/app/models/ee/member.rb
*/
export const MEMBER_STATE_CREATED = 0;
export const MEMBER_STATE_AWAITING = 1;
export const MEMBER_STATE_ACTIVE = 2;
export const BADGE_LABELS_PENDING_OWNER_APPROVAL = __('Pending owner approval');
export const DAYS_TO_EXPIRE_SOON = 7; export const DAYS_TO_EXPIRE_SOON = 7;
export const LEAVE_MODAL_ID = 'member-leave-modal'; export const LEAVE_MODAL_ID = 'member-leave-modal';
......
...@@ -54,7 +54,7 @@ class Groups::GroupMembersController < Groups::ApplicationController ...@@ -54,7 +54,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
end end
def invited_members def invited_members
group_members.invite group_members.invite.with_invited_user_state
end end
def non_invited_members def non_invited_members
......
...@@ -58,7 +58,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -58,7 +58,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
end end
def invited_members def invited_members
members.invite members.invite.with_invited_user_state
end end
def non_invited_members def non_invited_members
......
...@@ -50,6 +50,11 @@ class Member < ApplicationRecord ...@@ -50,6 +50,11 @@ class Member < ApplicationRecord
}, },
if: :project_bot? if: :project_bot?
scope :with_invited_user_state, -> do
joins('LEFT JOIN users as invited_user ON invited_user.email = members.invite_email')
.select('members.*', 'invited_user.state as invited_user_state')
end
scope :in_hierarchy, ->(source) do scope :in_hierarchy, ->(source) do
groups = source.root_ancestor.self_and_descendants groups = source.root_ancestor.self_and_descendants
group_members = Member.default_scoped.where(source: groups) group_members = Member.default_scoped.where(source: groups)
......
...@@ -44,6 +44,8 @@ class MemberEntity < Grape::Entity ...@@ -44,6 +44,8 @@ class MemberEntity < Grape::Entity
MemberUserEntity.represent(member.user, source: options[:source]) MemberUserEntity.represent(member.user, source: options[:source])
end end
expose :state
expose :invite, if: -> (member) { member.invite? } do expose :invite, if: -> (member) { member.invite? } do
expose :email do |member| expose :email do |member|
member.invite_email member.invite_email
...@@ -56,6 +58,10 @@ class MemberEntity < Grape::Entity ...@@ -56,6 +58,10 @@ class MemberEntity < Grape::Entity
expose :can_resend do |member| expose :can_resend do |member|
member.can_resend_invite? member.can_resend_invite?
end end
expose :user_state do |member|
member.respond_to?(:invited_user_state) ? member.invited_user_state : ""
end
end end
end end
......
...@@ -55,7 +55,7 @@ module EE ...@@ -55,7 +55,7 @@ module EE
override :invited_members override :invited_members
def invited_members def invited_members
super.or(group_members.awaiting) super.or(group_members.awaiting.with_invited_user_state)
end end
override :non_invited_members override :non_invited_members
......
...@@ -18,7 +18,7 @@ module EE ...@@ -18,7 +18,7 @@ module EE
override :invited_members override :invited_members
def invited_members def invited_members
super.or(members.awaiting) super.or(members.awaiting.with_invited_user_state)
end end
override :non_invited_members override :non_invited_members
......
...@@ -24743,6 +24743,9 @@ msgstr "" ...@@ -24743,6 +24743,9 @@ msgstr ""
msgid "Pending comments" msgid "Pending comments"
msgstr "" msgstr ""
msgid "Pending owner approval"
msgstr ""
msgid "Pending sync…" msgid "Pending sync…"
msgstr "" msgstr ""
......
...@@ -56,13 +56,15 @@ ...@@ -56,13 +56,15 @@
{ "$ref": "member_user.json" } { "$ref": "member_user.json" }
] ]
}, },
"state": { "type": "integer" },
"invite": { "invite": {
"type": "object", "type": "object",
"required": ["email", "avatar_url", "can_resend"], "required": ["email", "avatar_url", "can_resend", "user_state"],
"properties": { "properties": {
"email": { "type": "string" }, "email": { "type": "string" },
"avatar_url": { "type": "string" }, "avatar_url": { "type": "string" },
"can_resend": { "type": "boolean" } "can_resend": { "type": "boolean" },
"user_state": { "type": "string" }
}, },
"additionalProperties": false "additionalProperties": false
} }
......
import { GlBadge, GlPagination, GlTable } from '@gitlab/ui'; import { GlBadge, GlPagination, GlTable } from '@gitlab/ui';
import { import { createLocalVue } from '@vue/test-utils';
getByText as getByTextHelper,
getByTestId as getByTestIdHelper,
within,
} from '@testing-library/dom';
import { mount, createLocalVue, createWrapper } from '@vue/test-utils';
import Vuex from 'vuex'; import Vuex from 'vuex';
import setWindowLocation from 'helpers/set_window_location_helper'; import setWindowLocation from 'helpers/set_window_location_helper';
import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { mountExtended, extendedWrapper } from 'helpers/vue_test_utils_helper';
import CreatedAt from '~/members/components/table/created_at.vue'; import CreatedAt from '~/members/components/table/created_at.vue';
import ExpirationDatepicker from '~/members/components/table/expiration_datepicker.vue'; import ExpirationDatepicker from '~/members/components/table/expiration_datepicker.vue';
import MemberActionButtons from '~/members/components/table/member_action_buttons.vue'; import MemberActionButtons from '~/members/components/table/member_action_buttons.vue';
...@@ -15,7 +10,15 @@ import MemberAvatar from '~/members/components/table/member_avatar.vue'; ...@@ -15,7 +10,15 @@ import MemberAvatar from '~/members/components/table/member_avatar.vue';
import MemberSource from '~/members/components/table/member_source.vue'; import MemberSource from '~/members/components/table/member_source.vue';
import MembersTable from '~/members/components/table/members_table.vue'; import MembersTable from '~/members/components/table/members_table.vue';
import RoleDropdown from '~/members/components/table/role_dropdown.vue'; import RoleDropdown from '~/members/components/table/role_dropdown.vue';
import { MEMBER_TYPES, TAB_QUERY_PARAM_VALUES } from '~/members/constants'; import {
MEMBER_TYPES,
MEMBER_STATE_CREATED,
MEMBER_STATE_AWAITING,
MEMBER_STATE_ACTIVE,
USER_STATE_BLOCKED_PENDING_APPROVAL,
BADGE_LABELS_PENDING_OWNER_APPROVAL,
TAB_QUERY_PARAM_VALUES,
} from '~/members/constants';
import * as initUserPopovers from '~/user_popovers'; import * as initUserPopovers from '~/user_popovers';
import { import {
member as memberMock, member as memberMock,
...@@ -52,7 +55,7 @@ describe('MembersTable', () => { ...@@ -52,7 +55,7 @@ describe('MembersTable', () => {
}; };
const createComponent = (state, provide = {}) => { const createComponent = (state, provide = {}) => {
wrapper = mount(MembersTable, { wrapper = mountExtended(MembersTable, {
localVue, localVue,
propsData: { propsData: {
tabQueryParamValue: TAB_QUERY_PARAM_VALUES.invite, tabQueryParamValue: TAB_QUERY_PARAM_VALUES.invite,
...@@ -79,17 +82,11 @@ describe('MembersTable', () => { ...@@ -79,17 +82,11 @@ describe('MembersTable', () => {
const url = 'https://localhost/foo-bar/-/project_members?tab=invited'; const url = 'https://localhost/foo-bar/-/project_members?tab=invited';
const getByText = (text, options) =>
createWrapper(getByTextHelper(wrapper.element, text, options));
const getByTestId = (id, options) =>
createWrapper(getByTestIdHelper(wrapper.element, id, options));
const findTable = () => wrapper.find(GlTable); const findTable = () => wrapper.find(GlTable);
const findTableCellByMemberId = (tableCellLabel, memberId) => const findTableCellByMemberId = (tableCellLabel, memberId) =>
getByTestId(`members-table-row-${memberId}`).find( wrapper
`[data-label="${tableCellLabel}"][role="cell"]`, .findByTestId(`members-table-row-${memberId}`)
); .find(`[data-label="${tableCellLabel}"][role="cell"]`);
const findPagination = () => extendedWrapper(wrapper.find(GlPagination)); const findPagination = () => extendedWrapper(wrapper.find(GlPagination));
...@@ -101,7 +98,6 @@ describe('MembersTable', () => { ...@@ -101,7 +98,6 @@ describe('MembersTable', () => {
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
wrapper = null;
}); });
describe('fields', () => { describe('fields', () => {
...@@ -125,7 +121,7 @@ describe('MembersTable', () => { ...@@ -125,7 +121,7 @@ describe('MembersTable', () => {
tableFields: [field], tableFields: [field],
}); });
expect(getByText(label, { selector: '[role="columnheader"]' }).exists()).toBe(true); expect(wrapper.findByText(label, { selector: '[role="columnheader"]' }).exists()).toBe(true);
if (expectedComponent) { if (expectedComponent) {
expect( expect(
...@@ -134,11 +130,50 @@ describe('MembersTable', () => { ...@@ -134,11 +130,50 @@ describe('MembersTable', () => {
} }
}); });
describe('Invited column', () => {
describe.each`
state | userState | expectedBadgeLabel
${MEMBER_STATE_CREATED} | ${null} | ${''}
${MEMBER_STATE_CREATED} | ${USER_STATE_BLOCKED_PENDING_APPROVAL} | ${BADGE_LABELS_PENDING_OWNER_APPROVAL}
${MEMBER_STATE_AWAITING} | ${''} | ${''}
${MEMBER_STATE_AWAITING} | ${USER_STATE_BLOCKED_PENDING_APPROVAL} | ${BADGE_LABELS_PENDING_OWNER_APPROVAL}
${MEMBER_STATE_AWAITING} | ${'something_else'} | ${BADGE_LABELS_PENDING_OWNER_APPROVAL}
${MEMBER_STATE_ACTIVE} | ${null} | ${''}
${MEMBER_STATE_ACTIVE} | ${'something_else'} | ${''}
`('Invited Badge', ({ state, userState, expectedBadgeLabel }) => {
it(`${
expectedBadgeLabel ? 'shows' : 'hides'
} invited badge if user status: '${userState}' and member state: '${state}'`, () => {
createComponent({
members: [
{
...invite,
state,
invite: {
...invite.invite,
userState,
},
},
],
tableFields: ['invited'],
});
const invitedTab = wrapper.findByTestId('invited-badge');
if (expectedBadgeLabel) {
expect(invitedTab.text()).toBe(expectedBadgeLabel);
} else {
expect(invitedTab.exists()).toBe(false);
}
});
});
});
describe('"Actions" field', () => { describe('"Actions" field', () => {
it('renders "Actions" field for screen readers', () => { it('renders "Actions" field for screen readers', () => {
createComponent({ members: [memberCanUpdate], tableFields: ['actions'] }); createComponent({ members: [memberCanUpdate], tableFields: ['actions'] });
const actionField = getByTestId('col-actions'); const actionField = wrapper.findByTestId('col-actions');
expect(actionField.exists()).toBe(true); expect(actionField.exists()).toBe(true);
expect(actionField.classes('gl-sr-only')).toBe(true); expect(actionField.classes('gl-sr-only')).toBe(true);
...@@ -151,7 +186,7 @@ describe('MembersTable', () => { ...@@ -151,7 +186,7 @@ describe('MembersTable', () => {
it('does not render the "Actions" field', () => { it('does not render the "Actions" field', () => {
createComponent({ tableFields: ['actions'] }, { currentUserId: null }); createComponent({ tableFields: ['actions'] }, { currentUserId: null });
expect(within(wrapper.element).queryByTestId('col-actions')).toBe(null); expect(wrapper.findByTestId('col-actions').exists()).toBe(false);
}); });
}); });
...@@ -174,7 +209,7 @@ describe('MembersTable', () => { ...@@ -174,7 +209,7 @@ describe('MembersTable', () => {
it('renders the "Actions" field', () => { it('renders the "Actions" field', () => {
createComponent({ members, tableFields: ['actions'] }); createComponent({ members, tableFields: ['actions'] });
expect(getByTestId('col-actions').exists()).toBe(true); expect(wrapper.findByTestId('col-actions').exists()).toBe(true);
expect(findTableCellByMemberId('Actions', members[0].id).classes()).toStrictEqual([ expect(findTableCellByMemberId('Actions', members[0].id).classes()).toStrictEqual([
'col-actions', 'col-actions',
...@@ -196,7 +231,7 @@ describe('MembersTable', () => { ...@@ -196,7 +231,7 @@ describe('MembersTable', () => {
it('does not render the "Actions" field', () => { it('does not render the "Actions" field', () => {
createComponent({ members, tableFields: ['actions'] }); createComponent({ members, tableFields: ['actions'] });
expect(within(wrapper.element).queryByTestId('col-actions')).toBe(null); expect(wrapper.findByTestId('col-actions').exists()).toBe(false);
}); });
}); });
}); });
...@@ -206,7 +241,7 @@ describe('MembersTable', () => { ...@@ -206,7 +241,7 @@ describe('MembersTable', () => {
it('displays a "No members found" message', () => { it('displays a "No members found" message', () => {
createComponent(); createComponent();
expect(getByText('No members found').exists()).toBe(true); expect(wrapper.findByText('No members found').exists()).toBe(true);
}); });
}); });
......
import { MEMBER_TYPES } from '~/members/constants'; import { MEMBER_TYPES, MEMBER_STATE_CREATED } from '~/members/constants';
export const member = { export const member = {
requestedAt: null, requestedAt: null,
...@@ -14,6 +14,7 @@ export const member = { ...@@ -14,6 +14,7 @@ export const member = {
webUrl: 'https://gitlab.com/groups/foo-bar', webUrl: 'https://gitlab.com/groups/foo-bar',
}, },
type: 'GroupMember', type: 'GroupMember',
state: MEMBER_STATE_CREATED,
user: { user: {
id: 123, id: 123,
name: 'Administrator', name: 'Administrator',
...@@ -70,6 +71,7 @@ export const modalData = { ...@@ -70,6 +71,7 @@ export const modalData = {
const { user, ...memberNoUser } = member; const { user, ...memberNoUser } = member;
export const invite = { export const invite = {
...memberNoUser, ...memberNoUser,
state: MEMBER_STATE_CREATED,
invite: { invite: {
email: 'jewel@hudsonwalter.biz', email: 'jewel@hudsonwalter.biz',
avatarUrl: 'https://www.gravatar.com/avatar/cbab7510da7eec2f60f638261b05436d?s=80&d=identicon', avatarUrl: 'https://www.gravatar.com/avatar/cbab7510da7eec2f60f638261b05436d?s=80&d=identicon',
......
...@@ -169,6 +169,8 @@ RSpec.describe Member do ...@@ -169,6 +169,8 @@ RSpec.describe Member do
describe 'Scopes & finders' do describe 'Scopes & finders' do
let_it_be(:project) { create(:project, :public) } let_it_be(:project) { create(:project, :public) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval ) }
let_it_be(:blocked_pending_approval_project_member) { create(:project_member, :invited, :developer, project: project, invite_email: blocked_pending_approval_user.email) }
before_all do before_all do
@owner_user = create(:user).tap { |u| group.add_owner(u) } @owner_user = create(:user).tap { |u| group.add_owner(u) }
...@@ -538,6 +540,25 @@ RSpec.describe Member do ...@@ -538,6 +540,25 @@ RSpec.describe Member do
it { is_expected.to eq [example_member] } it { is_expected.to eq [example_member] }
end end
end end
describe '.with_invited_user_state' do
subject(:with_invited_user_state) { described_class.with_invited_user_state }
it { is_expected.to include @owner }
it { is_expected.to include @maintainer }
it { is_expected.to include @invited_member }
it { is_expected.to include @accepted_invite_member }
it { is_expected.to include @requested_member }
it { is_expected.to include @accepted_request_member }
context 'with invited pending members' do
it 'includes invited user state' do
invited_pending_members = with_invited_user_state.select { |m| m.invited_user_state.present? }
expect(invited_pending_members.count).to eq 1
expect(invited_pending_members).to include blocked_pending_approval_project_member
end
end
end
end end
describe 'Delegate methods' do describe 'Delegate methods' do
......
...@@ -39,6 +39,10 @@ RSpec.describe MemberEntity do ...@@ -39,6 +39,10 @@ RSpec.describe MemberEntity do
expect(entity_hash[:invite][:can_resend]).to be(true) expect(entity_hash[:invite][:can_resend]).to be(true)
end end
it 'exposes `invite.user_state` as empty string' do
expect(entity_hash[:invite][:user_state]).to eq('')
end
end end
shared_examples 'is_direct_member' do shared_examples 'is_direct_member' do
...@@ -59,6 +63,12 @@ RSpec.describe MemberEntity do ...@@ -59,6 +63,12 @@ RSpec.describe MemberEntity do
end end
end end
shared_examples 'user state is blocked_pending_approval' do
it 'displays proper user state' do
expect(entity_hash[:invite][:user_state]).to eq('blocked_pending_approval')
end
end
context 'group member' do context 'group member' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:source) { group } let(:source) { group }
...@@ -79,6 +89,14 @@ RSpec.describe MemberEntity do ...@@ -79,6 +89,14 @@ RSpec.describe MemberEntity do
it_behaves_like 'is_direct_member' it_behaves_like 'is_direct_member'
end end
context 'new member user state is blocked_pending_approval' do
let(:user) { create(:user, :blocked_pending_approval) }
let(:group_member) { create(:group_member, :invited, group: group, invite_email: user.email) }
let(:member) { GroupMemberPresenter.new(GroupMember.with_invited_user_state.find(group_member.id), current_user: current_user) }
it_behaves_like 'user state is blocked_pending_approval'
end
end end
context 'project member' do context 'project member' do
...@@ -102,5 +120,13 @@ RSpec.describe MemberEntity do ...@@ -102,5 +120,13 @@ RSpec.describe MemberEntity do
it_behaves_like 'is_direct_member' it_behaves_like 'is_direct_member'
end end
context 'new members user state is blocked_pending_approval' do
let(:user) { create(:user, :blocked_pending_approval) }
let(:project_member) { create(:project_member, :invited, project: project, invite_email: user.email) }
let(:member) { ProjectMemberPresenter.new(ProjectMember.with_invited_user_state.find(project_member.id), current_user: current_user) }
it_behaves_like 'user state is blocked_pending_approval'
end
end end
end end
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