diff --git a/app/views/projects/merge_requests/_approvals_count.html.haml b/app/views/projects/merge_requests/_approvals_count.html.haml index 35dd30c302bab82866ca6c21de3d15c1f9c05ca4..e4c226dab509a6b58586403ad66927707179e316 100644 --- a/app/views/projects/merge_requests/_approvals_count.html.haml +++ b/app/views/projects/merge_requests/_approvals_count.html.haml @@ -5,9 +5,9 @@ - if total > 0 - 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 + - approval_tooltip = self_approved ? final_self_text : final_text + - approval_icon = 'check' + - badge_variant = :success + - badge_label = _("Approved") - - approval_icon = sprite_icon((self_approved ? 'approval-solid' : 'approval'), css_class: 'align-middle') - - %li.gl-display-none.gl-sm-display-inline-block.has-tooltip.text-success{ title: self_approved ? final_self_text : final_text } - = approval_icon - = _("Approved") + %li.gl-display-flex= render Pajamas::BadgeComponent.new(badge_label, size: 'sm', variant: badge_variant, icon: approval_icon, title: approval_tooltip, class: 'has-tooltip', data: { 'testid': 'mr-appovals' }) diff --git a/ee/app/views/projects/merge_requests/_approvals_count.html.haml b/ee/app/views/projects/merge_requests/_approvals_count.html.haml index c90f0d3a57271ed78b4249cd48f8bcd4f2bd2bbe..72d17d397236ec97c8e340b993d9f5a2de0fa53e 100644 --- a/ee/app/views/projects/merge_requests/_approvals_count.html.haml +++ b/ee/app/views/projects/merge_requests/_approvals_count.html.haml @@ -1,29 +1,17 @@ - if merge_request.approval_needed? - - approved = merge_request.approved? - self_approved = merge_request.approved_by?(current_user) - given = merge_request.approvals_given - total = merge_request.total_approvals_count + - required = merge_request.approvals_required - - 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'), css_class: 'align-middle') - - %li.gl-display-none.gl-sm-display-inline-block.has-tooltip{ title: approval_tooltip, class: ('text-success' if approved) } - = approval_icon + - approved_text = _("Required approvals (%{approvals_given} of %{required} given, you've approved)") % { approvals_given: given, required: required } + - unapproved_text = _("Required approvals (%{approvals_given} of %{required} given)") % { approvals_given: given, required: required } + - approval_tooltip = self_approved ? approved_text : unapproved_text + - approval_icon = approved ? 'check' : 'approval' + - badge_variant = approved ? :success : :muted + - badge_label = approved ? _("Approved") : _("%{approvals_given} of %{required} Approvals") % { approvals_given: total, required: required } - - if approved - = _("Approved") - - else - = _("%{remaining_approvals} left") % { remaining_approvals: merge_request.approvals_left } + %li.gl-display-flex= render Pajamas::BadgeComponent.new(badge_label, size: 'sm', variant: badge_variant, icon: approval_icon, title: approval_tooltip, class: 'has-tooltip', data: { 'testid': 'mr-appovals' }) - else = render_ce "projects/merge_requests/approvals_count", merge_request: merge_request diff --git a/ee/spec/features/merge_requests/user_views_all_merge_requests_spec.rb b/ee/spec/features/merge_requests/user_views_all_merge_requests_spec.rb index 62a26081637f812ded84cbc4dbd509763be11b2c..a9b3c94a8b2a4dd0334df1b141bb34ef9cfc0977 100644 --- a/ee/spec/features/merge_requests/user_views_all_merge_requests_spec.rb +++ b/ee/spec/features/merge_requests/user_views_all_merge_requests_spec.rb @@ -17,49 +17,20 @@ 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 + expect(page.all('[data-testid="mr-appovals"]').any? { |item| item["title"] == "Required approvals (0 of 2 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 + expect(page.all('[data-testid="mr-appovals"]').any? { |item| item["title"] == "Required approvals (1 of 2 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 + expect(page.all('[data-testid="mr-appovals"]').any? { |item| item["title"] == "Required approvals (1 of 2 given, you've approved)" }).to be true end end - - it 'shows custom tooltip after user has approved' do - 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"] == "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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6889995bf840f94fbb4c3a4a6c7832810a841a75..f07d2373b0b35fbaf417c00931b169ebe88729f6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -540,6 +540,9 @@ msgstr "" msgid "%{address} is an invalid IP address range" msgstr "" +msgid "%{approvals_given} of %{required} Approvals" +msgstr "" + msgid "%{argument_name} must be formatted correctly. For example: 1h 30m." msgstr "" @@ -1100,9 +1103,6 @@ msgid_plural "%{releases} releases" msgstr[0] "" msgstr[1] "" -msgid "%{remaining_approvals} left" -msgstr "" - msgid "%{requireStart}Require%{requireEnd} %{approvalsRequired} %{approvalStart}approval%{approvalEnd} from:" msgid_plural "%{requireStart}Require%{requireEnd} %{approvalsRequired} %{approvalStart}approvals%{approvalEnd} from:" msgstr[0] "" @@ -42856,10 +42856,10 @@ msgstr "" msgid "Require expiration date" msgstr "" -msgid "Required approvals (%{approvals_given} given)" +msgid "Required approvals (%{approvals_given} of %{required} given)" msgstr "" -msgid "Required approvals (%{approvals_given} given, you've approved)" +msgid "Required approvals (%{approvals_given} of %{required} given, you've approved)" msgstr "" msgid "Required in this project." diff --git a/spec/features/merge_request/user_approves_spec.rb b/spec/features/merge_request/user_approves_spec.rb index 5b5ad4468ec45432cd5046018f1a4d3b14697fa9..8b4d1a5adf78b7feede9c4fa5c8fa34b516f5bce 100644 --- a/spec/features/merge_request/user_approves_spec.rb +++ b/spec/features/merge_request/user_approves_spec.rb @@ -27,7 +27,11 @@ def verify_approvals_count_on_index! visit(project_merge_requests_path(project, state: :all)) - expect(page.all('li').any? { |item| item["title"] == "1 approver (you've approved)" }).to be true + expect( + page.all('[data-testid="mr-appovals"]').any? do |item| + item["title"] == "1 approver (you've approved)" + end + ).to be true visit project_merge_request_path(project, merge_request) end