Merge branch 'ph/fixReviewersLoadingStates' into 'master'

Fixes loading icons when re-requesting a review

See merge request gitlab-org/gitlab!53962

app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue

 <script>
-// NOTE! For the first iteration, we are simply copying the implementation of Assignees
-// It will soon be overhauled in Issue https://gitlab.com/gitlab-org/gitlab/-/issues/233736
 import { GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui';
-import { __, sprintf } from '~/locale';
 import ReviewerAvatarLink from './reviewer_avatar_link.vue';
 
-const DEFAULT_RENDER_COUNT = 5;
+const LOADING_STATE = 'loading';
+const SUCCESS_STATE = 'success';
 
 export default {
   components: {
@@ -34,35 +32,21 @@ export default {
   data() {
     return {
       showLess: true,
-      loading: false,
-      requestedReviewSuccess: false,
+      loadingStates: {},
     };
   },
-  computed: {
-    firstUser() {
-      return this.users[0];
-    },
-    hasOneUser() {
-      return this.users.length === 1;
-    },
-    hiddenReviewersLabel() {
-      const { numberOfHiddenReviewers } = this;
-      return sprintf(__('+ %{numberOfHiddenReviewers} more'), { numberOfHiddenReviewers });
-    },
-    renderShowMoreSection() {
-      return this.users.length > DEFAULT_RENDER_COUNT;
-    },
-    numberOfHiddenReviewers() {
-      return this.users.length - DEFAULT_RENDER_COUNT;
-    },
-    uncollapsedUsers() {
-      const uncollapsedLength = this.showLess
-        ? Math.min(this.users.length, DEFAULT_RENDER_COUNT)
-        : this.users.length;
-      return this.showLess ? this.users.slice(0, uncollapsedLength) : this.users;
-    },
-    username() {
-      return `@${this.firstUser.username}`;
+  watch: {
+    users: {
+      handler(users) {
+        this.loadingStates = users.reduce(
+          (acc, user) => ({
+            ...acc,
+            [user.id]: acc[user.id] || null,
+          }),
+          this.loadingStates,
+        );
+      },
+      immediate: true,
     },
   },
   methods: {
@@ -70,21 +54,23 @@ export default {
       this.showLess = !this.showLess;
     },
     reRequestReview(userId) {
-      this.loading = true;
+      this.loadingStates[userId] = LOADING_STATE;
       this.$emit('request-review', { userId, callback: this.requestReviewComplete });
     },
-    requestReviewComplete(success) {
+    requestReviewComplete(userId, success) {
       if (success) {
-        this.requestedReviewSuccess = true;
+        this.loadingStates[userId] = SUCCESS_STATE;
 
         setTimeout(() => {
-          this.requestedReviewSuccess = false;
+          this.loadingStates[userId] = null;
         }, 1500);
+      } else {
+        this.loadingStates[userId] = null;
       }
-
-      this.loading = false;
     },
   },
+  LOADING_STATE,
+  SUCCESS_STATE,
 };
 </script>
 
@@ -100,20 +86,22 @@ export default {
         <div class="gl-ml-3">@{{ user.username }}</div>
       </reviewer-avatar-link>
       <gl-icon
-        v-if="requestedReviewSuccess"
+        v-if="loadingStates[user.id] === $options.SUCCESS_STATE"
         :size="24"
         name="check"
         class="float-right gl-text-green-500"
+        data-testid="re-request-success"
       />
       <gl-button
         v-else-if="user.can_update_merge_request && user.reviewed"
         v-gl-tooltip.left
         :title="__('Re-request review')"
-        :loading="loading"
+        :loading="loadingStates[user.id] === $options.LOADING_STATE"
         class="float-right gl-text-gray-500!"
         size="small"
         icon="redo"
         variant="link"
+        data-testid="re-request-button"
         @click="reRequestReview(user.id)"
       />
     </div>

app/assets/javascripts/sidebar/sidebar_mediator.js

@@ -58,9 +58,9 @@ export default class SidebarMediator {
       .then(() => {
         this.store.updateReviewer(userId);
         toast(__('Requested review'));
-        callback(true);
+        callback(userId, true);
       })
-      .catch(() => callback(false));
+      .catch(() => callback(userId, false));
   }
 
   setMoveToProjectId(projectId) {

locale/gitlab.pot

@@ -1027,9 +1027,6 @@ msgstr ""
 msgid "+ %{numberOfHiddenAssignees} more"
 msgstr ""
 
-msgid "+ %{numberOfHiddenReviewers} more"
-msgstr ""
-
 msgid "+%d more"
 msgid_plural "+%d more"
 msgstr[0] ""

spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js

+import { shallowMount } from '@vue/test-utils';
+import { TEST_HOST } from 'helpers/test_constants';
+import UncollapsedReviewerList from '~/sidebar/components/reviewers/uncollapsed_reviewer_list.vue';
+import ReviewerAvatarLink from '~/sidebar/components/reviewers/reviewer_avatar_link.vue';
+import userDataMock from '../../user_data_mock';
+
+describe('UncollapsedReviewerList component', () => {
+  let wrapper;
+
+  function createComponent(props = {}) {
+    const propsData = {
+      users: [],
+      rootPath: TEST_HOST,
+      ...props,
+    };
+
+    wrapper = shallowMount(UncollapsedReviewerList, {
+      propsData,
+    });
+  }
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('single reviewer', () => {
+    beforeEach(() => {
+      const user = userDataMock();
+
+      createComponent({
+        users: [user],
+      });
+    });
+
+    it('only has one user', () => {
+      expect(wrapper.findAll(ReviewerAvatarLink).length).toBe(1);
+    });
+
+    it('shows one user with avatar, username and author name', () => {
+      expect(wrapper.text()).toContain(`@root`);
+    });
+
+    it('renders re-request loading icon', async () => {
+      await wrapper.setData({ loadingStates: { 1: 'loading' } });
+
+      expect(wrapper.find('[data-testid="re-request-button"]').props('loading')).toBe(true);
+    });
+
+    it('renders re-request success icon', async () => {
+      await wrapper.setData({ loadingStates: { 1: 'success' } });
+
+      expect(wrapper.find('[data-testid="re-request-success"]').exists()).toBe(true);
+    });
+  });
+
+  describe('multiple reviewers', () => {
+    beforeEach(() => {
+      const user = userDataMock();
+
+      createComponent({
+        users: [user, { ...user, id: 2, username: 'hello-world' }],
+      });
+    });
+
+    it('only has one user', () => {
+      expect(wrapper.findAll(ReviewerAvatarLink).length).toBe(2);
+    });
+
+    it('shows one user with avatar, username and author name', () => {
+      expect(wrapper.text()).toContain(`@root`);
+      expect(wrapper.text()).toContain(`@hello-world`);
+    });
+
+    it('renders re-request loading icon', async () => {
+      await wrapper.setData({ loadingStates: { 2: 'loading' } });
+
+      expect(wrapper.findAll('[data-testid="re-request-button"]').length).toBe(2);
+      expect(wrapper.findAll('[data-testid="re-request-button"]').at(1).props('loading')).toBe(
+        true,
+      );
+    });
+
+    it('renders re-request success icon', async () => {
+      await wrapper.setData({ loadingStates: { 2: 'success' } });
+
+      expect(wrapper.findAll('[data-testid="re-request-button"]').length).toBe(1);
+      expect(wrapper.findAll('[data-testid="re-request-success"]').length).toBe(1);
+      expect(wrapper.find('[data-testid="re-request-success"]').exists()).toBe(true);
+    });
+  });
+});

spec/frontend/sidebar/user_data_mock.js

@@ -8,4 +8,6 @@ export default () => ({
   username: 'root',
   web_url: `${TEST_HOST}/root`,
   can_merge: true,
+  can_update_merge_request: true,
+  reviewed: true,
 });