Commit b5446564 authored by Thomas Randolph's avatar Thomas Randolph Committed by Phil Hughes

Update Merge Request list approvals / approvers to be more clear

parent 89b3e1dc
- 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 }
---
title: Improve the clarity of the MR approvals tooltip UI
merge_request: 33329
author:
type: changed
......@@ -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) }
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"] == "Approvals"}).to be true
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
......@@ -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 ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment