Merge branch '330027-add-approve-button-to-pending-members' into 'master'

Add "Approve" button to pending members table

See merge request gitlab-org/gitlab!75756

ee/app/assets/javascripts/api/groups_api.js

@@ -56,3 +56,13 @@ export const fetchPendingGroupMembersList = (namespaceId, options = {}) => {
     },
   });
 };
+
+const APPROVE_PENDING_GROUP_MEMBER_PATH = '/api/:version/groups/:id/members/:user_id/approve';
+
+export const approvePendingGroupMember = (namespaceId, userId) => {
+  const url = buildApiUrl(APPROVE_PENDING_GROUP_MEMBER_PATH)
+    .replace(':id', namespaceId)
+    .replace(':user_id', userId);
+
+  return axios.put(url);
+};

ee/app/assets/javascripts/pending_members/components/app.vue

 <script>
 import { mapActions, mapState, mapGetters } from 'vuex';
-import { GlAvatarLabeled, GlAvatarLink, GlBadge, GlPagination, GlLoadingIcon } from '@gitlab/ui';
+import {
+  GlAlert,
+  GlAvatarLabeled,
+  GlAvatarLink,
+  GlBadge,
+  GlButton,
+  GlPagination,
+  GlModal,
+  GlModalDirective,
+  GlLoadingIcon,
+} from '@gitlab/ui';
+import { sprintf } from '~/locale';
 import { AVATAR_SIZE } from 'ee/seat_usage/constants';
-import { AWAITING_MEMBER_SIGNUP_TEXT } from 'ee/pending_members/constants';
+import {
+  AWAITING_MEMBER_SIGNUP_TEXT,
+  LABEL_APPROVE,
+  LABEL_CONFIRM,
+  LABEL_CONFIRM_APPROVE,
+} from 'ee/pending_members/constants';
 
 export default {
   name: 'PendingMembersApp',
-  components: { GlAvatarLabeled, GlAvatarLink, GlBadge, GlPagination, GlLoadingIcon },
+  components: {
+    GlAlert,
+    GlAvatarLabeled,
+    GlAvatarLink,
+    GlBadge,
+    GlButton,
+    GlPagination,
+    GlModal,
+    GlLoadingIcon,
+  },
+  directives: {
+    GlModalDirective,
+  },
   computed: {
     ...mapState([
       'isLoading',
+      'alertMessage',
+      'alertVariant',
       'page',
       'perPage',
       'total',
@@ -33,17 +63,24 @@ export default {
   created() {
     this.fetchPendingMembersList();
   },
-  AWAITING_MEMBER_SIGNUP_TEXT,
   methods: {
-    ...mapActions(['fetchPendingMembersList', 'setCurrentPage']),
+    ...mapActions(['fetchPendingMembersList', 'setCurrentPage', 'approveMember', 'dismissAlert']),
     avatarLabel(member) {
       if (member.invited) {
         return member.email;
       }
       return member.name ?? '';
     },
+    approveUserQuestion(member) {
+      return sprintf(LABEL_CONFIRM_APPROVE, {
+        user: member.name || member.email,
+      });
+    },
   },
   avatarSize: AVATAR_SIZE,
+  AWAITING_MEMBER_SIGNUP_TEXT,
+  LABEL_APPROVE,
+  LABEL_CONFIRM,
 };
 </script>
 <template>
@@ -52,25 +89,45 @@ export default {
       <gl-loading-icon class="mt-5" size="lg" />
     </div>
     <template v-else>
-      <div
-        v-for="item in tableItems"
-        :key="item.id"
-        class="gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid"
-        data-testid="pending-members-row"
-      >
-        <gl-avatar-link target="blank" :href="item.web_url" :alt="item.name">
-          <gl-avatar-labeled
-            :src="item.avatar_url"
-            :size="$options.avatarSize"
-            :label="avatarLabel(item)"
+      <div>
+        <gl-alert v-if="alertMessage" :variant="alertVariant" @dismiss="dismissAlert">
+          {{ alertMessage }}
+        </gl-alert>
+        <div
+          v-for="item in tableItems"
+          :key="item.id"
+          class="gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid gl-display-flex gl-justify-content-space-between"
+          data-testid="pending-members-row"
+        >
+          <gl-avatar-link target="blank" :href="item.web_url" :alt="item.name">
+            <gl-avatar-labeled
+              :src="item.avatar_url"
+              :size="$options.avatarSize"
+              :label="avatarLabel(item)"
+            >
+              <template #meta>
+                <gl-badge v-if="item.invited && item.approved" size="sm" variant="muted">
+                  {{ $options.AWAITING_MEMBER_SIGNUP_TEXT }}
+                </gl-badge>
+              </template>
+            </gl-avatar-labeled>
+          </gl-avatar-link>
+          <gl-button
+            v-gl-modal-directive="`approve-confirmation-modal-${item.id}`"
+            :loading="item.loading"
+            :disabled="item.approved"
+          >
+            {{ $options.LABEL_APPROVE }}
+          </gl-button>
+          <gl-modal
+            :modal-id="`approve-confirmation-modal-${item.id}`"
+            :title="$options.LABEL_CONFIRM"
+            no-fade
+            @primary="approveMember(item.id)"
           >
-            <template #meta>
-              <gl-badge v-if="item.invited && item.approved" size="sm" variant="muted">
-                {{ $options.AWAITING_MEMBER_SIGNUP_TEXT }}
-              </gl-badge>
-            </template>
-          </gl-avatar-labeled>
-        </gl-avatar-link>
+            <p>{{ approveUserQuestion(item) }}</p>
+          </gl-modal>
+        </div>
       </div>
     </template>
 

ee/app/assets/javascripts/pending_members/constants.js

-import { s__ } from '~/locale';
+import { s__, __ } from '~/locale';
 
 // Pending members HTTP headers
 export const HEADER_TOTAL_ENTRIES = 'x-total';
@@ -9,3 +9,9 @@ export const AWAITING_MEMBER_SIGNUP_TEXT = s__('Billing|Awaiting member signup')
 export const PENDING_MEMBERS_LIST_ERROR = s__(
   'Billing|An error occurred while loading pending members list',
 );
+
+export const LABEL_APPROVE = __('Approve');
+export const LABEL_CONFIRM = __('Confirm approval');
+export const LABEL_CONFIRM_APPROVE = __('Are you sure you want to approve %{user}?');
+export const APPROVAL_SUCCESSFUL_MESSAGE = s__('Billing|%{user} was successfully approved');
+export const APPROVAL_ERROR_MESSAGE = s__('Billing|An error occurred while approving %{user}');

ee/app/assets/javascripts/pending_members/store/actions.js

 import * as GroupsApi from 'ee/api/groups_api';
-import createFlash from '~/flash';
-import { PENDING_MEMBERS_LIST_ERROR } from 'ee/pending_members/constants';
+import {
+  APPROVAL_ERROR_MESSAGE,
+  APPROVAL_SUCCESSFUL_MESSAGE,
+  PENDING_MEMBERS_LIST_ERROR,
+} from '../constants';
 import * as types from './mutation_types';
 
 export const fetchPendingMembersList = ({ commit, state }) => {
@@ -12,8 +15,9 @@ export const fetchPendingMembersList = ({ commit, state }) => {
     .then(({ data, headers }) => commit(types.RECEIVE_PENDING_MEMBERS_SUCCESS, { data, headers }))
     .catch(() => {
       commit(types.RECEIVE_PENDING_MEMBERS_ERROR);
-      createFlash({
-        message: PENDING_MEMBERS_LIST_ERROR,
+      commit(types.SHOW_ALERT, {
+        alertMessage: PENDING_MEMBERS_LIST_ERROR,
+        alertVariant: 'danger',
       });
     });
 };
@@ -23,3 +27,28 @@ export const setCurrentPage = ({ commit, dispatch }, page) => {
 
   dispatch('fetchPendingMembersList');
 };
+
+export const approveMember = ({ commit, state }, id) => {
+  commit(types.SET_MEMBER_AS_LOADING, id);
+
+  return GroupsApi.approvePendingGroupMember(state.namespaceId, id)
+    .then(() => {
+      commit(types.SET_MEMBER_AS_APPROVED, id);
+      commit(types.SHOW_ALERT, {
+        memberId: id,
+        alertMessage: APPROVAL_SUCCESSFUL_MESSAGE,
+        alertVariant: 'info',
+      });
+    })
+    .catch(() => {
+      commit(types.SET_MEMBER_ERROR, id);
+      commit(types.SHOW_ALERT, {
+        memberId: id,
+        alertMessage: APPROVAL_ERROR_MESSAGE,
+        alertVariant: 'danger',
+      });
+    });
+};
+export const dismissAlert = ({ commit }) => {
+  commit(types.DISMISS_ALERT);
+};

ee/app/assets/javascripts/pending_members/store/mutation_types.js

@@ -3,3 +3,9 @@ export const RECEIVE_PENDING_MEMBERS_SUCCESS = 'RECEIVE_PENDING_MEMBERS_SUCCESS'
 export const RECEIVE_PENDING_MEMBERS_ERROR = 'RECEIVE_PENDING_MEMBERS_ERROR';
 
 export const SET_CURRENT_PAGE = 'SET_CURRENT_PAGE';
+
+export const SET_MEMBER_AS_LOADING = 'SET_MEMBER_AS_LOADING';
+export const SET_MEMBER_AS_APPROVED = 'SET_MEMBER_AS_APPROVED';
+export const SET_MEMBER_ERROR = 'SET_MEMBER_ERROR';
+export const DISMISS_ALERT = 'DISMISS_ALERT';
+export const SHOW_ALERT = 'SHOW_ALERT';

ee/app/assets/javascripts/pending_members/store/mutations.js

+import { sprintf } from '~/locale';
 import { HEADER_TOTAL_ENTRIES, HEADER_PAGE_NUMBER, HEADER_ITEMS_PER_PAGE } from '../constants';
 import * as types from './mutation_types';
 
@@ -26,4 +27,51 @@ export default {
   [types.SET_CURRENT_PAGE](state, pageNumber) {
     state.page = pageNumber;
   },
+
+  [types.SET_MEMBER_AS_LOADING](state, id) {
+    state.members = state.members.map((member) => {
+      if (member.id === id) {
+        return { ...member, loading: true };
+      }
+      return member;
+    });
+  },
+
+  [types.SET_MEMBER_AS_APPROVED](state, id) {
+    state.members = state.members.map((member) => {
+      if (member.id === id) {
+        return { ...member, approved: true, loading: false };
+      }
+      return member;
+    });
+  },
+
+  [types.SET_MEMBER_ERROR](state, id) {
+    state.members = state.members.map((member) => {
+      if (member.id === id) {
+        return { ...member, loading: false };
+      }
+      return member;
+    });
+  },
+
+  [types.DISMISS_ALERT](state) {
+    state.alertMessage = '';
+  },
+
+  [types.SHOW_ALERT](state, payload) {
+    const { memberId, alertMessage, alertVariant } = payload;
+
+    if (memberId) {
+      const member = state.members.find((m) => m.id === memberId);
+
+      state.alertMessage = sprintf(alertMessage, {
+        user: member.name || member.email,
+      });
+    } else {
+      state.alertMessage = alertMessage;
+    }
+
+    state.alertVariant = alertVariant;
+  },
 };

ee/app/assets/javascripts/pending_members/store/state.js

 export default ({ namespaceId = null, namespaceName = null } = {}) => ({
   isLoading: false,
   hasError: false,
+  alertMessage: '',
+  alertVariant: '',
   namespaceId,
   namespaceName,
   members: [],

ee/spec/frontend/api/groups_api_spec.js

@@ -12,6 +12,7 @@ describe('GroupsApi', () => {
     relative_url_root: dummyUrlRoot,
   };
   const namespaceId = 1000;
+  const memberId = 2;
 
   let originalGon;
   let mock;
@@ -45,7 +46,6 @@ describe('GroupsApi', () => {
     });
 
     describe('fetchBillableGroupMemberMemberships', () => {
-      const memberId = 2;
       const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${namespaceId}/billable_members/${memberId}/memberships`;
 
       it('fetches memberships for the member', async () => {
@@ -60,7 +60,6 @@ describe('GroupsApi', () => {
     });
 
     describe('removeBillableMemberFromGroup', () => {
-      const memberId = 2;
       const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${namespaceId}/billable_members/${memberId}`;
 
       it('removes a billable member from a group', async () => {
@@ -90,4 +89,18 @@ describe('GroupsApi', () => {
       });
     });
   });
+
+  describe('approvePendingGroupMember', () => {
+    const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${namespaceId}/members/${memberId}/approve`;
+
+    it('approves a pending member from a group', async () => {
+      jest.spyOn(axios, 'put');
+      mock.onPut(expectedUrl).replyOnce(httpStatus.OK, []);
+
+      const { data } = await GroupsApi.approvePendingGroupMember(namespaceId, memberId);
+
+      expect(data).toEqual([]);
+      expect(axios.put).toHaveBeenCalledWith(expectedUrl);
+    });
+  });
 });

ee/spec/frontend/pending_members/components/__snapshots__/app_spec.js.snap

@@ -2,15 +2,27 @@
 
 exports[`PendingMembersApp renders pending members 1`] = `
 Array [
-  "<div data-testid=\\"pending-members-row\\" class=\\"gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid\\">
+  "<div data-testid=\\"pending-members-row\\" class=\\"gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid gl-display-flex gl-justify-content-space-between\\">
   <gl-avatar-link-stub target=\\"blank\\" href=\\"http://127.0.0.1:3000/334050-1\\" alt=\\"334050-1 334050-1\\">
     <gl-avatar-labeled-stub label=\\"334050-1 334050-1\\" sublabel=\\"\\" labellink=\\"\\" sublabellink=\\"\\" src=\\"https://www.gravatar.com/avatar/9987bae8f71451bb2d422d0596367b25?s=80&amp;d=identicon\\" size=\\"32\\"></gl-avatar-labeled-stub>
   </gl-avatar-link-stub>
+  <gl-button-stub category=\\"primary\\" variant=\\"default\\" size=\\"medium\\" icon=\\"\\" buttontextclasses=\\"\\" role=\\"button\\" tabindex=\\"0\\">
+    Approve
+  </gl-button-stub>
+  <gl-modal-stub modalid=\\"approve-confirmation-modal-177\\" titletag=\\"h4\\" title=\\"Confirm approval\\" modalclass=\\"\\" size=\\"md\\" dismisslabel=\\"Close\\" no-fade=\\"\\">
+    <p>Are you sure you want to approve 334050-1 334050-1?</p>
+  </gl-modal-stub>
 </div>",
-  "<div data-testid=\\"pending-members-row\\" class=\\"gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid\\">
+  "<div data-testid=\\"pending-members-row\\" class=\\"gl-p-5 gl-border-0 gl-border-b-1! gl-border-gray-100 gl-border-solid gl-display-flex gl-justify-content-space-between\\">
   <gl-avatar-link-stub target=\\"blank\\">
     <gl-avatar-labeled-stub label=\\"first-invite@gitlab.com\\" sublabel=\\"\\" labellink=\\"\\" sublabellink=\\"\\" src=\\"https://www.gravatar.com/avatar/8bad6be3d5070e7f7865d91a50f44f1f?s=80&amp;d=identicon\\" size=\\"32\\"></gl-avatar-labeled-stub>
   </gl-avatar-link-stub>
+  <gl-button-stub category=\\"primary\\" variant=\\"default\\" size=\\"medium\\" icon=\\"\\" buttontextclasses=\\"\\" role=\\"button\\" tabindex=\\"0\\">
+    Approve
+  </gl-button-stub>
+  <gl-modal-stub modalid=\\"approve-confirmation-modal-178\\" titletag=\\"h4\\" title=\\"Confirm approval\\" modalclass=\\"\\" size=\\"md\\" dismisslabel=\\"Close\\" no-fade=\\"\\">
+    <p>Are you sure you want to approve first-invite@gitlab.com?</p>
+  </gl-modal-stub>
 </div>",
 ]
 `;

ee/spec/frontend/pending_members/store/actions_spec.js

@@ -3,14 +3,16 @@ import State from 'ee/pending_members/store/state';
 import * as GroupsApi from 'ee/api/groups_api';
 import * as actions from 'ee/pending_members/store/actions';
 import * as types from 'ee/pending_members/store/mutation_types';
+import {
+  PENDING_MEMBERS_LIST_ERROR,
+  APPROVAL_SUCCESSFUL_MESSAGE,
+  APPROVAL_ERROR_MESSAGE,
+} from 'ee/pending_members/constants';
 import { mockDataMembers } from 'ee_jest/pending_members/mock_data';
 import testAction from 'helpers/vuex_action_helper';
-import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
 import httpStatusCodes from '~/lib/utils/http_status';
 
-jest.mock('~/flash');
-
 describe('Pending members actions', () => {
   let state;
   let mock;
@@ -71,15 +73,97 @@ describe('Pending members actions', () => {
       });
 
       it('dispatches the request and error action', async () => {
+        const mockShowAlertPayload = {
+          alertMessage: PENDING_MEMBERS_LIST_ERROR,
+          alertVariant: 'danger',
+        };
+
         await testAction({
           action: actions.fetchPendingMembersList,
           state,
           expectedMutations: [
             { type: types.REQUEST_PENDING_MEMBERS },
             { type: types.RECEIVE_PENDING_MEMBERS_ERROR },
+            { type: types.SHOW_ALERT, payload: mockShowAlertPayload },
+          ],
+        });
+      });
+    });
+  });
+
+  describe('approveMember', () => {
+    const memberId = 2;
+
+    beforeEach(() => {
+      gon.api_version = 'v4';
+      state.namespaceId = 1;
+    });
+
+    it('passes correct arguments to API call', () => {
+      const spy = jest.spyOn(GroupsApi, 'approvePendingGroupMember');
+
+      testAction({
+        action: actions.approveMember,
+        payload: memberId,
+        state,
+        expectedMutations: expect.anything(),
+        expectedActions: expect.anything(),
+      });
+
+      expect(spy).toBeCalledWith(state.namespaceId, memberId);
+    });
+
+    describe('on success', () => {
+      beforeEach(() => {
+        mock
+          .onPut(`/api/v4/groups/1/members/${memberId}/approve`)
+          .replyOnce(httpStatusCodes.NO_CONTENT);
+      });
+
+      it('dispatches the request and success action', async () => {
+        const mockShowAlertPayload = {
+          memberId,
+          alertMessage: APPROVAL_SUCCESSFUL_MESSAGE,
+          alertVariant: 'info',
+        };
+
+        await testAction({
+          action: actions.approveMember,
+          payload: memberId,
+          state,
+          expectedMutations: [
+            { type: types.SET_MEMBER_AS_LOADING, payload: memberId },
+            { type: types.SET_MEMBER_AS_APPROVED, payload: memberId },
+            { type: types.SHOW_ALERT, payload: mockShowAlertPayload },
+          ],
+        });
+      });
+    });
+
+    describe('on error', () => {
+      beforeEach(() => {
+        mock
+          .onPut(`/api/v4/groups/1/members/${memberId}/approve`)
+          .replyOnce(httpStatusCodes.NOT_FOUND, {});
+      });
+
+      it('dispatches the request and error action', async () => {
+        const mockShowAlertPayload = {
+          memberId,
+          alertMessage: APPROVAL_ERROR_MESSAGE,
+          alertVariant: 'danger',
+        };
+
+        await testAction({
+          action: actions.approveMember,
+          payload: memberId,
+          state,
+          expectedMutations: [
+            { type: types.SET_MEMBER_AS_LOADING, payload: memberId },
+            { type: types.SET_MEMBER_ERROR, payload: memberId },
+            { type: types.SHOW_ALERT, payload: mockShowAlertPayload },
           ],
         });
-        expect(createFlash).toHaveBeenCalled();
       });
     });
   });

ee/spec/frontend/pending_members/store/mutations_spec.js

@@ -4,6 +4,8 @@ import mutations from 'ee/pending_members/store/mutations';
 import createState from 'ee/pending_members/store/state';
 
 describe('Pending members mutations', () => {
+  const alertMessage = 'This is an alert';
+  const alertVariant = 'info';
   let state;
 
   beforeEach(() => {
@@ -63,4 +65,74 @@ describe('Pending members mutations', () => {
       expect(state.page).toBe(1);
     });
   });
+
+  describe(types.DISMISS_ALERT, () => {
+    beforeEach(() => {
+      state.alertMessage = alertMessage;
+    });
+
+    it('cleans alertMessage state', () => {
+      mutations[types.DISMISS_ALERT](state);
+
+      expect(state.alertMessage).toBe('');
+    });
+  });
+
+  describe(types.SHOW_ALERT, () => {
+    beforeEach(() => {
+      state.alertMessage = '';
+      state.alertVariant = '';
+    });
+
+    it('sets alertMessage and alertVariant', () => {
+      mutations[types.SHOW_ALERT](state, { alertMessage, alertVariant });
+
+      expect(state.alertMessage).toBe(alertMessage);
+      expect(state.alertVariant).toBe(alertVariant);
+    });
+  });
+
+  describe('member specific mutations', () => {
+    const memberId = mockDataMembers.data[0].id;
+
+    beforeEach(() => {
+      state.members = mockDataMembers.data;
+    });
+
+    describe(types.SET_MEMBER_AS_LOADING, () => {
+      it('sets member loading state to true', () => {
+        mutations[types.SET_MEMBER_AS_LOADING](state, memberId);
+        const member = state.members.find((m) => m.id === memberId);
+        expect(member.loading).toBeTruthy();
+      });
+    });
+
+    describe(types.SET_MEMBER_AS_APPROVED, () => {
+      it('sets member loading state to false and approved state to true', () => {
+        mutations[types.SET_MEMBER_AS_APPROVED](state, memberId);
+        const member = state.members.find((m) => m.id === memberId);
+        expect(member.loading).toBeFalsy();
+        expect(member.approved).toBeTruthy();
+      });
+    });
+
+    describe(types.SHOW_ALERT, () => {
+      beforeEach(() => {
+        state.alertMessage = '';
+        state.alertVariant = '';
+      });
+
+      it('sets alertMessage and alertVariant', () => {
+        mutations[types.SHOW_ALERT](state, {
+          memberId,
+          alertMessage: `${alertMessage}%{user}`,
+          alertVariant,
+        });
+        const member = state.members.find((m) => m.id === memberId);
+
+        expect(state.alertMessage).toBe(`${alertMessage}${member.name}`);
+        expect(state.alertVariant).toBe(alertVariant);
+      });
+    });
+  });
 });

locale/gitlab.pot

@@ -4620,6 +4620,9 @@ msgstr ""
 msgid "Are you sure you want to %{action} %{name}?"
 msgstr ""
 
+msgid "Are you sure you want to approve %{user}?"
+msgstr ""
+
 msgid "Are you sure you want to attempt to merge?"
 msgstr ""
 
@@ -5535,9 +5538,15 @@ msgstr ""
 msgid "Billings|Your account has been validated"
 msgstr ""
 
+msgid "Billing|%{user} was successfully approved"
+msgstr ""
+
 msgid "Billing|An email address is only visible for users with public emails."
 msgstr ""
 
+msgid "Billing|An error occurred while approving %{user}"
+msgstr ""
+
 msgid "Billing|An error occurred while getting a billable member details"
 msgstr ""
 
@@ -9070,6 +9079,9 @@ msgstr ""
 msgid "Confirm"
 msgstr ""
 
+msgid "Confirm approval"
+msgstr ""
+
 msgid "Confirm new password"
 msgstr ""