Merge branch 'approvals-count/ui' into 'master'

Update Merge Request list approvals / approvers to be more clear

See merge request gitlab-org/gitlab!33329

ee/app/views/projects/merge_requests/_approvals_count.html.haml

 - if merge_request.approval_needed?
-  - approval_tooltip = merge_request.has_approved?(current_user) ? _("Approvals (you've approved)") : _("Approvals")
-  %li.d-none.d-sm-inline-block.has-tooltip{ title: approval_tooltip, class: ('text-success' if merge_request.approved?) }
-    = sprite_icon((merge_request.has_approved?(current_user) ? 'approval-solid' : 'approval'), size: 16, css_class: 'align-middle')
-    = merge_request.approvals_given
-    %span of
-    = merge_request.approvals_required
+
+  - approved = merge_request.approved?
+  - self_approved = merge_request.has_approved?(current_user)
+  - given = merge_request.approvals_given
+  - total = merge_request.total_approvals_count
+
+  - approved_text = _("Required approvals (%{approvals_given} given, you've approved)") % { approvals_given: given }
+  - unapproved_text = _("Required approvals (%{approvals_given} given)") % { approvals_given: given }
+
+  - final_text = n_("%d approver", "%d approvers", total) % total
+  - final_self_text = n_("%d approver (you've approved)", "%d approvers (you've approved)", total) % total
+
+  - if approved
+    - approval_tooltip = self_approved ? final_self_text : final_text
+  - else
+    - approval_tooltip = self_approved ? approved_text : unapproved_text
+
+  - approval_icon = sprite_icon((self_approved ? 'approval-solid' : 'approval'), size: 16, css_class: 'align-middle')
+
+  %li.d-none.d-sm-inline-block.has-tooltip{ title: approval_tooltip, class: ('text-success' if approved) }
+    = approval_icon
+
+    - if approved
+      = _("Approved")
+    - else
+      = _("%{remaining_approvals} left") % { remaining_approvals: merge_request.approvals_left }

ee/changelogs/unreleased/approvals-count-ui.yml

+---
+title: Improve the clarity of the MR approvals tooltip UI
+merge_request: 33329
+author:
+type: changed

ee/spec/features/merge_requests/user_views_all_merge_requests_spec.rb

@@ -6,17 +6,60 @@ RSpec.describe 'User views all merge requests' do
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let(:project) { create(:project, :public, approvals_before_merge: 1) }
   let(:user) { create(:user) }
+  let(:another_user) { create(:user) }
 
-  it 'shows generic approvals tooltip' do
-    visit(project_merge_requests_path(project, state: :all))
-    expect(page.all('li').any? { |item| item["title"] == "Approvals"}).to be true
+  before do
+    project.add_developer(user)
+  end
+
+  describe 'more approvals are required' do
+    let!(:approval_rule) { create( :approval_merge_request_rule, merge_request: merge_request, users: [user, another_user], approvals_required: 2, name: "test rule" ) }
+
+    it 'shows generic approvals tooltip' do
+      visit(project_merge_requests_path(project, state: :all))
+      expect(page.all('li').any? { |item| item["title"] == "Required approvals (0 given)"}).to be true
+    end
+
+    it 'shows custom tooltip after a different user has approved' do
+      merge_request.approvals.create(user: another_user)
+      visit(project_merge_requests_path(project, state: :all))
+      expect(page.all('li').any? { |item| item["title"] == "Required approvals (1 given)"}).to be true
+    end
+
+    it 'shows custom tooltip after self has approved' do
+      merge_request.approvals.create(user: user)
+      sign_in(user)
+      visit(project_merge_requests_path(project, state: :all))
+      expect(page.all('li').any? { |item| item["title"] == "Required approvals (1 given, you've approved)"}).to be true
+    end
   end
 
   it 'shows custom tooltip after user has approved' do
-    project.add_developer(user)
     sign_in(user)
     merge_request.approvals.create(user: user)
     visit(project_merge_requests_path(project, state: :all))
-    expect(page.all('li').any? { |item| item["title"] == "Approvals (you've approved)"}).to be true
+    expect(page.all('li').any? { |item| item["title"] == "1 approver (you've approved)"}).to be true
+  end
+
+  it 'shows custom tooltip after a different user has approved' do
+    merge_request.approvals.create(user: another_user)
+    sign_in(user)
+    visit(project_merge_requests_path(project, state: :all))
+    expect(page.all('li').any? { |item| item["title"] == "1 approver"}).to be true
+  end
+
+  it 'shows custom tooltip after multiple users have approved' do
+    merge_request.approvals.create(user: another_user)
+    merge_request.approvals.create(user: user)
+    visit(project_merge_requests_path(project, state: :all))
+    expect(page.all('li').any? { |item| item["title"] == "2 approvers"}).to be true
+  end
+
+  it 'shows custom tooltip after multiple users have approved, including self' do
+    merge_request.approvals.create(user: another_user)
+    merge_request.approvals.create(user: user)
+    sign_in(user)
+    visit(project_merge_requests_path(project, state: :all))
+    expect(page.all('li').any? { |item| item["title"] == "2 approvers (you've approved)"}).to be true
   end
 end

locale/gitlab.pot

@@ -76,6 +76,16 @@ msgid_plural "%d URLs scanned"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%d approver"
+msgid_plural "%d approvers"
+msgstr[0] ""
+msgstr[1] ""
+
+msgid "%d approver (you've approved)"
+msgid_plural "%d approvers (you've approved)"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%d changed file"
 msgid_plural "%d changed files"
 msgstr[0] ""
@@ -506,6 +516,9 @@ msgid_plural "%{releases} releases"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%{remaining_approvals} left"
+msgstr ""
+
 msgid "%{retryButtonStart}Try again%{retryButtonEnd} or %{newFileButtonStart}attach a new file%{newFileButtonEnd}"
 msgstr ""
 
@@ -2633,12 +2646,6 @@ msgstr ""
 msgid "ApprovalRule|e.g. QA, Security, etc."
 msgstr ""
 
-msgid "Approvals"
-msgstr ""
-
-msgid "Approvals (you've approved)"
-msgstr ""
-
 msgid "Approve"
 msgstr ""
 
@@ -2648,6 +2655,9 @@ msgstr ""
 msgid "Approve the current merge request."
 msgstr ""
 
+msgid "Approved"
+msgstr ""
+
 msgid "Approved by: "
 msgstr ""
 
@@ -18654,6 +18664,12 @@ msgstr ""
 msgid "Require users to prove ownership of custom domains"
 msgstr ""
 
+msgid "Required approvals (%{approvals_given} given)"
+msgstr ""
+
+msgid "Required approvals (%{approvals_given} given, you've approved)"
+msgstr ""
+
 msgid "Requirement"
 msgstr ""