Merge branch 'led/334292-add-pending-owner-approval-badge' into 'master'

Add “Pending owner approval” badge for members

See merge request gitlab-org/gitlab!70682

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

@@ -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 { mergeUrlParams } from '~/lib/utils/url_utility';
 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 RemoveMemberModal from '../modals/remove_member_modal.vue';
 import CreatedAt from './created_at.vue';
@@ -129,6 +135,74 @@ export default {
         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>
@@ -172,8 +246,11 @@ export default {
         <created-at :date="createdAt" :created-by="createdBy" />
       </template>
 
-      <template #cell(invited)="{ item: { createdAt, createdBy } }">
+      <template #cell(invited)="{ item: { createdAt, createdBy, invite, state } }">
         <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 #cell(requested)="{ item: { createdAt } }">

app/assets/javascripts/members/constants.js

@@ -89,6 +89,22 @@ export const TAB_QUERY_PARAM_VALUES = {
   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 LEAVE_MODAL_ID = 'member-leave-modal';

app/controllers/groups/group_members_controller.rb

@@ -54,7 +54,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def invited_members
-    group_members.invite
+    group_members.invite.with_invited_user_state
   end
 
   def non_invited_members

app/controllers/projects/project_members_controller.rb

@@ -58,7 +58,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def invited_members
-    members.invite
+    members.invite.with_invited_user_state
   end
 
   def non_invited_members

app/models/member.rb

@@ -50,6 +50,11 @@ class Member < ApplicationRecord
     },
     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
     groups = source.root_ancestor.self_and_descendants
     group_members = Member.default_scoped.where(source: groups)

app/serializers/member_entity.rb

@@ -44,6 +44,8 @@ class MemberEntity < Grape::Entity
     MemberUserEntity.represent(member.user, source: options[:source])
   end
 
+  expose :state
+
   expose :invite, if: -> (member) { member.invite? } do
     expose :email do |member|
       member.invite_email
@@ -56,6 +58,10 @@ class MemberEntity < Grape::Entity
     expose :can_resend do |member|
       member.can_resend_invite?
     end
+
+    expose :user_state do |member|
+      member.respond_to?(:invited_user_state) ? member.invited_user_state : ""
+    end
   end
 end
 

ee/app/controllers/ee/groups/group_members_controller.rb

@@ -55,7 +55,7 @@ module EE
 
       override :invited_members
       def invited_members
-        super.or(group_members.awaiting)
+        super.or(group_members.awaiting.with_invited_user_state)
       end
 
       override :non_invited_members

ee/app/controllers/ee/projects/project_members_controller.rb

@@ -18,7 +18,7 @@ module EE
 
       override :invited_members
       def invited_members
-        super.or(members.awaiting)
+        super.or(members.awaiting.with_invited_user_state)
       end
 
       override :non_invited_members

locale/gitlab.pot

@@ -24746,6 +24746,9 @@ msgstr ""
 msgid "Pending comments"
 msgstr ""
 
+msgid "Pending owner approval"
+msgstr ""
+
 msgid "Pending sync…"
 msgstr ""
 

spec/fixtures/api/schemas/entities/member.json

@@ -56,13 +56,15 @@
         { "$ref": "member_user.json" }
       ]
     },
+    "state": { "type": "integer" },
     "invite": {
       "type": "object",
-      "required": ["email", "avatar_url", "can_resend"],
+      "required": ["email", "avatar_url", "can_resend", "user_state"],
       "properties": {
         "email": { "type": "string" },
         "avatar_url": { "type": "string" },
-        "can_resend": { "type": "boolean" }
+        "can_resend": { "type": "boolean" },
+        "user_state": { "type": "string" }
       },
       "additionalProperties": false
     }

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

 import { GlBadge, GlPagination, GlTable } from '@gitlab/ui';
-import {
-  getByText as getByTextHelper,
-  getByTestId as getByTestIdHelper,
-  within,
-} from '@testing-library/dom';
-import { mount, createLocalVue, createWrapper } from '@vue/test-utils';
+import { createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 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 ExpirationDatepicker from '~/members/components/table/expiration_datepicker.vue';
 import MemberActionButtons from '~/members/components/table/member_action_buttons.vue';
@@ -15,7 +10,15 @@ import MemberAvatar from '~/members/components/table/member_avatar.vue';
 import MemberSource from '~/members/components/table/member_source.vue';
 import MembersTable from '~/members/components/table/members_table.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 {
   member as memberMock,
@@ -52,7 +55,7 @@ describe('MembersTable', () => {
   };
 
   const createComponent = (state, provide = {}) => {
-    wrapper = mount(MembersTable, {
+    wrapper = mountExtended(MembersTable, {
       localVue,
       propsData: {
         tabQueryParamValue: TAB_QUERY_PARAM_VALUES.invite,
@@ -79,17 +82,11 @@ describe('MembersTable', () => {
 
   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 findTableCellByMemberId = (tableCellLabel, memberId) =>
-    getByTestId(`members-table-row-${memberId}`).find(
-      `[data-label="${tableCellLabel}"][role="cell"]`,
-    );
+    wrapper
+      .findByTestId(`members-table-row-${memberId}`)
+      .find(`[data-label="${tableCellLabel}"][role="cell"]`);
 
   const findPagination = () => extendedWrapper(wrapper.find(GlPagination));
 
@@ -101,7 +98,6 @@ describe('MembersTable', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    wrapper = null;
   });
 
   describe('fields', () => {
@@ -125,7 +121,7 @@ describe('MembersTable', () => {
         tableFields: [field],
       });
 
-      expect(getByText(label, { selector: '[role="columnheader"]' }).exists()).toBe(true);
+      expect(wrapper.findByText(label, { selector: '[role="columnheader"]' }).exists()).toBe(true);
 
       if (expectedComponent) {
         expect(
@@ -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', () => {
       it('renders "Actions" field for screen readers', () => {
         createComponent({ members: [memberCanUpdate], tableFields: ['actions'] });
 
-        const actionField = getByTestId('col-actions');
+        const actionField = wrapper.findByTestId('col-actions');
 
         expect(actionField.exists()).toBe(true);
         expect(actionField.classes('gl-sr-only')).toBe(true);
@@ -151,7 +186,7 @@ describe('MembersTable', () => {
         it('does not render the "Actions" field', () => {
           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', () => {
         it('renders the "Actions" field', () => {
           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([
             'col-actions',
@@ -196,7 +231,7 @@ describe('MembersTable', () => {
         it('does not render the "Actions" field', () => {
           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', () => {
     it('displays a "No members found" message', () => {
       createComponent();
 
-      expect(getByText('No members found').exists()).toBe(true);
+      expect(wrapper.findByText('No members found').exists()).toBe(true);
     });
   });
 

spec/frontend/members/mock_data.js

-import { MEMBER_TYPES } from '~/members/constants';
+import { MEMBER_TYPES, MEMBER_STATE_CREATED } from '~/members/constants';
 
 export const member = {
   requestedAt: null,
@@ -14,6 +14,7 @@ export const member = {
     webUrl: 'https://gitlab.com/groups/foo-bar',
   },
   type: 'GroupMember',
+  state: MEMBER_STATE_CREATED,
   user: {
     id: 123,
     name: 'Administrator',
@@ -70,6 +71,7 @@ export const modalData = {
 const { user, ...memberNoUser } = member;
 export const invite = {
   ...memberNoUser,
+  state: MEMBER_STATE_CREATED,
   invite: {
     email: 'jewel@hudsonwalter.biz',
     avatarUrl: 'https://www.gravatar.com/avatar/cbab7510da7eec2f60f638261b05436d?s=80&d=identicon',

spec/models/member_spec.rb

@@ -169,6 +169,8 @@ RSpec.describe Member do
   describe 'Scopes & finders' do
     let_it_be(:project) { create(:project, :public) }
     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
       @owner_user = create(:user).tap { |u| group.add_owner(u) }
@@ -538,6 +540,25 @@ RSpec.describe Member do
         it { is_expected.to eq [example_member] }
       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
 
   describe 'Delegate methods' do

spec/serializers/member_entity_spec.rb

@@ -39,6 +39,10 @@ RSpec.describe MemberEntity do
 
       expect(entity_hash[:invite][:can_resend]).to be(true)
     end
+
+    it 'exposes `invite.user_state` as empty string' do
+      expect(entity_hash[:invite][:user_state]).to eq('')
+    end
   end
 
   shared_examples 'is_direct_member' do
@@ -59,6 +63,12 @@ RSpec.describe MemberEntity do
     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
     let(:group) { create(:group) }
     let(:source) { group }
@@ -79,6 +89,14 @@ RSpec.describe MemberEntity do
 
       it_behaves_like 'is_direct_member'
     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
 
   context 'project member' do
@@ -102,5 +120,13 @@ RSpec.describe MemberEntity do
 
       it_behaves_like 'is_direct_member'
     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