Merge branch 're-approve-icon' into 'master'

Add "reviewed but not approved" state in merge request sidebar

See merge request !116861



Merged-by: Annabel Dunstone Gray <annabel.m.gray@gmail.com>
Co-authored-by: Annabel Dunstone Gray <annabel.dunstone@gmail.com>

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

@@ -58,6 +58,9 @@ export default {
     approvedByTooltipTitle(user) {
       return sprintf(s__('MergeRequest|Approved by @%{username}'), user);
     },
+    reviewedButNotApprovedTooltip(user) {
+      return sprintf(s__('MergeRequest|Reviewed by @%{username} but not yet approved'), user);
+    },
     toggleShowLess() {
       this.showLess = !this.showLess;
     },
@@ -105,6 +108,19 @@ export default {
           {{ user.name }}
         </div>
       </reviewer-avatar-link>
+      <gl-button
+        v-if="user.mergeRequestInteraction.canUpdate && user.mergeRequestInteraction.reviewed"
+        v-gl-tooltip.left
+        :title="$options.i18n.reRequestReview"
+        :aria-label="$options.i18n.reRequestReview"
+        :loading="loadingStates[user.id] === $options.LOADING_STATE"
+        class="float-right gl-text-gray-500! gl-mr-2"
+        size="small"
+        icon="redo"
+        variant="link"
+        data-testid="re-request-button"
+        @click="reRequestReview(user.id)"
+      />
       <gl-icon
         v-if="user.mergeRequestInteraction.approved"
         v-gl-tooltip.left
@@ -115,24 +131,13 @@ export default {
         data-testid="re-approved"
       />
       <gl-icon
-        v-if="loadingStates[user.id] === $options.SUCCESS_STATE"
-        :size="24"
-        name="check"
-        class="float-right gl-py-2 gl-mr-2 gl-text-green-500"
-        data-testid="re-request-success"
-      />
-      <gl-button
-        v-else-if="user.mergeRequestInteraction.canUpdate && user.mergeRequestInteraction.reviewed"
+        v-if="!user.mergeRequestInteraction.approved && user.mergeRequestInteraction.reviewed"
         v-gl-tooltip.left
-        :title="$options.i18n.reRequestReview"
-        :aria-label="$options.i18n.reRequestReview"
-        :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)"
+        :size="16"
+        :title="reviewedButNotApprovedTooltip(user)"
+        name="dotted-circle"
+        class="float-right gl-my-2 gl-ml-auto gl-text-gray-300 gl-flex-shrink-0"
+        data-testid="re-approved"
       />
     </div>
   </div>

locale/gitlab.pot

@@ -27364,6 +27364,9 @@ msgstr ""
 msgid "MergeRequest|No files found"
 msgstr ""
+msgid "MergeRequest|Reviewed by @%{username} but not yet approved"
+msgstr ""
+
 msgid "MergeRequest|Search (e.g. *.vue) (%{MODIFIER_KEY}P)"
 msgstr ""

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

@@ -62,14 +62,6 @@ describe('UncollapsedReviewerList component', () => {
 
       expect(wrapper.find('[data-testid="re-request-button"]').props('loading')).toBe(true);
     });
-
-    it('renders re-request success icon', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      await wrapper.setData({ loadingStates: { 1: 'success' } });
-
-      expect(wrapper.find('[data-testid="re-request-success"]').exists()).toBe(true);
-    });
   });
 
   describe('multiple reviewers', () => {
@@ -120,15 +112,5 @@ describe('UncollapsedReviewerList component', () => {
         true,
       );
     });
-
-    it('renders re-request success icon', async () => {
-      // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
-      // eslint-disable-next-line no-restricted-syntax
-      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);
-    });
   });
 });