From 54a5d513e5f068c53fad3b2dac04998f5e9afd88 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Tue, 19 Feb 2019 13:27:53 -0700 Subject: [PATCH 1/9] Fixing CI icon mismatch MR list, and related MRs and branches were using a deprecated helper. Created a new icon haml file to help move these forward. --- app/views/ci/status/_icon.html.haml | 12 +++ .../projects/issues/_merge_requests.html.haml | 2 +- .../issues/_related_branches.html.haml | 2 +- .../merge_requests/_merge_request.html.haml | 2 +- ...-merge-requests-page-and-the-mr-itself.yml | 5 ++ spec/views/ci/status/_icon.html.haml_spec.rb | 88 +++++++++++++++++++ 6 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 app/views/ci/status/_icon.html.haml create mode 100644 changelogs/unreleased/45305-ci-status-icon-mismatch-on-merge-requests-page-and-the-mr-itself.yml create mode 100644 spec/views/ci/status/_icon.html.haml_spec.rb diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml new file mode 100644 index 00000000000..bfdf9c2ec9f --- /dev/null +++ b/app/views/ci/status/_icon.html.haml @@ -0,0 +1,12 @@ +- status = local_assigns.fetch(:status) +- size = local_assigns.fetch(:size, 16) +- link = local_assigns.fetch(:link, true) +- title = local_assigns.fetch(:title, nil) +- css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} #{'has-tooltip' if title.present?}" + +- if link && status.has_details? + = link_to status.details_path, class: css_classes, title: title, data: { html: title.present? } do + = sprite_icon(status.icon, size: size) +- else + %span{ class: css_classes, title: title, data: { html: title.present? } } + = sprite_icon(status.icon, size: size) \ No newline at end of file diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml index 310e339ac8d..ab3d7907ad8 100644 --- a/app/views/projects/issues/_merge_requests.html.haml +++ b/app/views/projects/issues/_merge_requests.html.haml @@ -27,7 +27,7 @@ = merge_request.to_reference %span.mr-ci-status.flex-md-grow-1.justify-content-end.d-flex.ml-md-2 - if merge_request.can_read_pipeline? - = render_pipeline_status(merge_request.head_pipeline, tooltip_placement: 'bottom') + = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true - elsif has_any_head_pipeline = icon('blank fw') diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml index ffdd96870ef..f8d1d64100a 100644 --- a/app/views/projects/issues/_related_branches.html.haml +++ b/app/views/projects/issues/_related_branches.html.haml @@ -8,7 +8,7 @@ - pipeline = @project.pipeline_for(branch, target.sha) if target - if can?(current_user, :read_pipeline, pipeline) %span.related-branch-ci-status - = render_pipeline_status(pipeline) + = render 'ci/status/icon', status: pipeline.detailed_status(current_user), link: true %span.related-branch-info %strong = link_to branch, project_compare_path(@project, from: @project.default_branch, to: branch), class: "ref-name" diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index ac29cd8f679..4111e823701 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -48,7 +48,7 @@ CLOSED - if can?(current_user, :read_pipeline, merge_request.head_pipeline) %li.issuable-pipeline-status.d-none.d-sm-inline-block - = render_pipeline_status(merge_request.head_pipeline) + = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true - if merge_request.open? && merge_request.broken? %li.issuable-pipeline-broken.d-none.d-sm-inline-block = link_to merge_request_path(merge_request), class: "has-tooltip", title: _('Cannot be merged automatically') do diff --git a/changelogs/unreleased/45305-ci-status-icon-mismatch-on-merge-requests-page-and-the-mr-itself.yml b/changelogs/unreleased/45305-ci-status-icon-mismatch-on-merge-requests-page-and-the-mr-itself.yml new file mode 100644 index 00000000000..64ab76a2b05 --- /dev/null +++ b/changelogs/unreleased/45305-ci-status-icon-mismatch-on-merge-requests-page-and-the-mr-itself.yml @@ -0,0 +1,5 @@ +--- +title: Fix pipeline status icon mismatch +merge_request: 25407 +author: +type: fixed diff --git a/spec/views/ci/status/_icon.html.haml_spec.rb b/spec/views/ci/status/_icon.html.haml_spec.rb new file mode 100644 index 00000000000..43806446164 --- /dev/null +++ b/spec/views/ci/status/_icon.html.haml_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe 'ci/status/_icon' do + let(:user) { create(:user) } + let(:project) { create(:project, :private) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + context 'when rendering status for build' do + let(:build) do + create(:ci_build, :success, pipeline: pipeline) + end + + context 'when user has ability to see details' do + before do + project.add_developer(user) + end + + it 'has link to build details page' do + details_path = project_job_path(project, build) + + render_status(build) + + expect(rendered).to have_link(href: details_path) + end + end + + context 'when user do not have ability to see build details' do + before do + render_status(build) + end + + it 'contains build status text' do + expect(rendered).to have_css('.ci-status-icon.ci-status-icon-success') + end + + it 'does not contain links' do + expect(rendered).not_to have_link + end + end + end + + context 'when rendering status for external job' do + context 'when user has ability to see commit status details' do + before do + project.add_developer(user) + end + + context 'status has external target url' do + before do + external_job = create(:generic_commit_status, + status: :running, + pipeline: pipeline, + target_url: 'http://gitlab.com') + + render_status(external_job) + end + + it 'contains valid commit status text' do + expect(rendered).to have_css('.ci-status-icon.ci-status-icon-running') + end + + it 'has link to external status page' do + expect(rendered).to have_link(href: 'http://gitlab.com') + end + end + + context 'status do not have external target url' do + before do + external_job = create(:generic_commit_status, status: :canceled) + + render_status(external_job) + end + + it 'contains valid commit status text' do + expect(rendered).to have_css('.ci-status-icon.ci-status-icon-canceled') + end + + it 'has link to external status page' do + expect(rendered).not_to have_link + end + end + end + end + + def render_status(resource) + render 'ci/status/icon', status: resource.detailed_status(user) + end +end From 52c910eeca47f140246d003fbb6b4748d1be8bb8 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Wed, 20 Feb 2019 12:20:09 -0700 Subject: [PATCH 2/9] Remove deprecated ci status helper function After changing all places that used the function, we can now remove it. --- app/helpers/ci_status_helper.rb | 6 ------ app/views/ci/status/_icon.html.haml | 2 +- spec/views/ci/status/_icon.html.haml_spec.rb | 1 + 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 923a06a0512..dfeeecf1228 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -123,12 +123,6 @@ module CiStatusHelper icon_size: 24) end - def render_pipeline_status(pipeline, tooltip_placement: 'left') - project = pipeline.project - path = project_pipeline_path(project, pipeline) - render_status_with_link('pipeline', pipeline.status, path, tooltip_placement: tooltip_placement) - end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index bfdf9c2ec9f..29ce9cd1afd 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -9,4 +9,4 @@ = sprite_icon(status.icon, size: size) - else %span{ class: css_classes, title: title, data: { html: title.present? } } - = sprite_icon(status.icon, size: size) \ No newline at end of file + = sprite_icon(status.icon, size: size) diff --git a/spec/views/ci/status/_icon.html.haml_spec.rb b/spec/views/ci/status/_icon.html.haml_spec.rb index 43806446164..626159fc512 100644 --- a/spec/views/ci/status/_icon.html.haml_spec.rb +++ b/spec/views/ci/status/_icon.html.haml_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'spec_helper' describe 'ci/status/_icon' do From 725dfc77c2ed241121666c8a708efb09122c287f Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Mon, 25 Feb 2019 11:12:01 -0700 Subject: [PATCH 3/9] Adding tooltip to the icon partial I apparently forgot to add tooltips to the partial before. --- app/views/ci/status/_icon.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 29ce9cd1afd..09ac086fa22 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,7 +1,7 @@ - status = local_assigns.fetch(:status) - size = local_assigns.fetch(:size, 16) - link = local_assigns.fetch(:link, true) -- title = local_assigns.fetch(:title, nil) +- title = local_assigns.fetch(:title, "Pipeline: #{status.label}") - css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} #{'has-tooltip' if title.present?}" - if link && status.has_details? From 506ac78d48b6a96081b02818c432650b8508913a Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Mon, 25 Feb 2019 11:25:00 -0700 Subject: [PATCH 4/9] Adding tooltip placement Adding the ability to position the tooltip of the status icon. --- app/views/ci/status/_icon.html.haml | 15 ++++++++------- .../projects/issues/_merge_requests.html.haml | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 09ac086fa22..1fd34eacdf7 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,12 +1,13 @@ -- status = local_assigns.fetch(:status) -- size = local_assigns.fetch(:size, 16) -- link = local_assigns.fetch(:link, true) -- title = local_assigns.fetch(:title, "Pipeline: #{status.label}") -- css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} #{'has-tooltip' if title.present?}" +- status = local_assigns.fetch(:status) +- size = local_assigns.fetch(:size, 16) +- link = local_assigns.fetch(:link, true) +- title = local_assigns.fetch(:title, "Pipeline: #{status.label}") +- tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") +- css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} #{'has-tooltip' if title.present?}" - if link && status.has_details? - = link_to status.details_path, class: css_classes, title: title, data: { html: title.present? } do + = link_to status.details_path, class: css_classes, title: title, data: { html: title.present?, placement: tooltip_placement } do = sprite_icon(status.icon, size: size) - else - %span{ class: css_classes, title: title, data: { html: title.present? } } + %span{ class: css_classes, title: title, data: { html: title.present?, placement: tooltip_placement } } = sprite_icon(status.icon, size: size) diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml index ab3d7907ad8..7468217418d 100644 --- a/app/views/projects/issues/_merge_requests.html.haml +++ b/app/views/projects/issues/_merge_requests.html.haml @@ -27,7 +27,7 @@ = merge_request.to_reference %span.mr-ci-status.flex-md-grow-1.justify-content-end.d-flex.ml-md-2 - if merge_request.can_read_pipeline? - = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true + = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true, tooltip_placement: 'bottom' - elsif has_any_head_pipeline = icon('blank fw') From 2b51745394e8568cf91ce6ee95574f84fc38722e Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Tue, 26 Feb 2019 10:59:41 -0700 Subject: [PATCH 5/9] Removed other deprecated status helpers Updating a couple other places that were still using deprecated status. --- app/helpers/ci_status_helper.rb | 37 ------------------- app/views/ci/status/_icon.html.haml | 13 +++---- app/views/projects/commits/_commit.html.haml | 4 +- .../projects/issues/_merge_requests.html.haml | 2 +- .../issues/_related_branches.html.haml | 2 +- .../merge_requests/_merge_request.html.haml | 2 +- app/views/shared/projects/_project.html.haml | 2 +- 7 files changed, 11 insertions(+), 51 deletions(-) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index dfeeecf1228..c02f3707b9e 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -100,43 +100,6 @@ module CiStatusHelper "pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}" end - def render_project_pipeline_status(pipeline_status, tooltip_placement: 'left') - project = pipeline_status.project - path = pipelines_project_commit_path(project, pipeline_status.sha, ref: pipeline_status.ref) - - render_status_with_link( - 'commit', - pipeline_status.status, - path, - tooltip_placement: tooltip_placement) - end - - def render_commit_status(commit, ref: nil, tooltip_placement: 'left') - project = commit.project - path = pipelines_project_commit_path(project, commit, ref: ref) - - render_status_with_link( - 'commit', - commit.status(ref), - path, - tooltip_placement: tooltip_placement, - icon_size: 24) - end - - def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) - klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" - title = "#{type.titleize}: #{ci_label_for_status(status)}" - data = { toggle: 'tooltip', placement: tooltip_placement, container: container } - - if path - link_to ci_icon_for_status(status, size: icon_size), path, - class: klass, title: title, data: data - else - content_tag :span, ci_icon_for_status(status, size: icon_size), - class: klass, title: title, data: data - end - end - def detailed_status?(status) status.respond_to?(:text) && status.respond_to?(:label) && diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 1fd34eacdf7..1fcbd9e7545 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,13 +1,10 @@ - status = local_assigns.fetch(:status) - size = local_assigns.fetch(:size, 16) -- link = local_assigns.fetch(:link, true) -- title = local_assigns.fetch(:title, "Pipeline: #{status.label}") +- type = local_assigns.fetch(:type, 'pipeline') +- title = local_assigns.fetch(:title, "#{type.titleize}: #{status.label}") - tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") -- css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} #{'has-tooltip' if title.present?}" +- css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip" -- if link && status.has_details? - = link_to status.details_path, class: css_classes, title: title, data: { html: title.present?, placement: tooltip_placement } do - = sprite_icon(status.icon, size: size) -- else - %span{ class: css_classes, title: title, data: { html: title.present?, placement: tooltip_placement } } +- if status.has_details? + = link_to status.details_path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do = sprite_icon(status.icon, size: size) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 0d3c6e7027c..1d7890f1c47 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -25,7 +25,7 @@ = commit.short_id - if commit_status .d-block.d-sm-none - = render_commit_status(commit, ref: ref) + = render 'ci/status/icon', status: commit.last_pipeline.detailed_status(current_user), type: 'commit', size: 24 - if commit.description? %button.text-expander.js-toggle-button = sprite_icon('ellipsis_h', size: 12) @@ -47,7 +47,7 @@ = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } - if commit_status - = render_commit_status(commit, ref: ref) + = render 'ci/status/icon', status: commit.last_pipeline.detailed_status(current_user), type: 'commit', size: 24 .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml index 7468217418d..6a66c2e57cc 100644 --- a/app/views/projects/issues/_merge_requests.html.haml +++ b/app/views/projects/issues/_merge_requests.html.haml @@ -27,7 +27,7 @@ = merge_request.to_reference %span.mr-ci-status.flex-md-grow-1.justify-content-end.d-flex.ml-md-2 - if merge_request.can_read_pipeline? - = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true, tooltip_placement: 'bottom' + = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), tooltip_placement: 'bottom' - elsif has_any_head_pipeline = icon('blank fw') diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml index f8d1d64100a..6da4956a036 100644 --- a/app/views/projects/issues/_related_branches.html.haml +++ b/app/views/projects/issues/_related_branches.html.haml @@ -8,7 +8,7 @@ - pipeline = @project.pipeline_for(branch, target.sha) if target - if can?(current_user, :read_pipeline, pipeline) %span.related-branch-ci-status - = render 'ci/status/icon', status: pipeline.detailed_status(current_user), link: true + = render 'ci/status/icon', status: pipeline.detailed_status(current_user) %span.related-branch-info %strong = link_to branch, project_compare_path(@project, from: @project.default_branch, to: branch), class: "ref-name" diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 4111e823701..bfa5a471886 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -48,7 +48,7 @@ CLOSED - if can?(current_user, :read_pipeline, merge_request.head_pipeline) %li.issuable-pipeline-status.d-none.d-sm-inline-block - = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), link: true + = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user) - if merge_request.open? && merge_request.broken? %li.issuable-pipeline-broken.d-none.d-sm-inline-block = link_to merge_request_path(merge_request), class: "has-tooltip", title: _('Cannot be merged automatically') do diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index df17ae95e2a..2a38dce4896 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -85,6 +85,6 @@ = number_with_delimiter(project.open_issues_count) - if pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project) %span.icon-wrapper.pipeline-status - = render_project_pipeline_status(project.pipeline_status, tooltip_placement: 'top') + = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top' .updated-note %span Updated #{updated_tooltip} From 40d15136352958206685197d4176f15781089849 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Tue, 26 Feb 2019 14:10:24 -0700 Subject: [PATCH 6/9] Adding ability to pass in path to status icon Project passed a very specific details path. Also reverted a change. --- app/helpers/ci_status_helper.rb | 26 ++++++++++++++++++++ app/views/ci/status/_icon.html.haml | 6 ++++- app/views/projects/commits/_commit.html.haml | 4 +-- app/views/shared/projects/_project.html.haml | 3 ++- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index c02f3707b9e..355b91a8661 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -100,6 +100,32 @@ module CiStatusHelper "pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}" end + def render_commit_status(commit, ref: nil, tooltip_placement: 'left') + project = commit.project + path = pipelines_project_commit_path(project, commit, ref: ref) + + render_status_with_link( + 'commit', + commit.status(ref), + path, + tooltip_placement: tooltip_placement, + icon_size: 24) + end + + def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) + klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" + title = "#{type.titleize}: #{ci_label_for_status(status)}" + data = { toggle: 'tooltip', placement: tooltip_placement, container: container } + + if path + link_to ci_icon_for_status(status, size: icon_size), path, + class: klass, title: title, data: data + else + content_tag :span, ci_icon_for_status(status, size: icon_size), + class: klass, title: title, data: data + end + end + def detailed_status?(status) status.respond_to?(:text) && status.respond_to?(:label) && diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 1fcbd9e7545..f406eb4b5a3 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -3,8 +3,12 @@ - type = local_assigns.fetch(:type, 'pipeline') - title = local_assigns.fetch(:title, "#{type.titleize}: #{status.label}") - tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") +- path = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil) - css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip" - if status.has_details? - = link_to status.details_path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do + = link_to path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do + = sprite_icon(status.icon, size: size) +- else + %span{ class: css_classes, title: title, data: { html: true, placement: tooltip_placement } } = sprite_icon(status.icon, size: size) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 1d7890f1c47..0d3c6e7027c 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -25,7 +25,7 @@ = commit.short_id - if commit_status .d-block.d-sm-none - = render 'ci/status/icon', status: commit.last_pipeline.detailed_status(current_user), type: 'commit', size: 24 + = render_commit_status(commit, ref: ref) - if commit.description? %button.text-expander.js-toggle-button = sprite_icon('ellipsis_h', size: 12) @@ -47,7 +47,7 @@ = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } - if commit_status - = render 'ci/status/icon', status: commit.last_pipeline.detailed_status(current_user), type: 'commit', size: 24 + = render_commit_status(commit, ref: ref) .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 2a38dce4896..8ff676c3cf8 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -84,7 +84,8 @@ = sprite_icon('issues', size: 14, css_class: 'append-right-4') = number_with_delimiter(project.open_issues_count) - if pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project) + - pipeline_path = pipelines_project_commit_path(project.pipeline_status.project, project.pipeline_status.sha, ref: project.pipeline_status.ref) %span.icon-wrapper.pipeline-status - = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top' + = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path .updated-note %span Updated #{updated_tooltip} From 12459e507dc853239c70a1ef302de01c5a682542 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Tue, 26 Feb 2019 15:26:01 -0700 Subject: [PATCH 7/9] Refactoring and i18n fixes Making these changes based on MR suggestions. --- app/views/ci/status/_icon.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index f406eb4b5a3..64119d73e03 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,12 +1,12 @@ - status = local_assigns.fetch(:status) - size = local_assigns.fetch(:size, 16) - type = local_assigns.fetch(:type, 'pipeline') -- title = local_assigns.fetch(:title, "#{type.titleize}: #{status.label}") +- title = local_assigns.fetch(:title, _("%{type}: %{label}" % {type: type.titleize, label: status.label})) - tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") - path = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil) - css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip" -- if status.has_details? +- if path = link_to path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do = sprite_icon(status.icon, size: size) - else From 871ca1e51f791c2d8b36830d521e90577608adf0 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Wed, 27 Feb 2019 09:21:42 -0700 Subject: [PATCH 8/9] Adjusting internationalization of tooltip title Based on MR comments, changing how the i18n works for the tooltip title. --- app/views/ci/status/_icon.html.haml | 4 +++- locale/gitlab.pot | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 64119d73e03..1ca0a55b156 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -1,10 +1,12 @@ - status = local_assigns.fetch(:status) - size = local_assigns.fetch(:size, 16) - type = local_assigns.fetch(:type, 'pipeline') -- title = local_assigns.fetch(:title, _("%{type}: %{label}" % {type: type.titleize, label: status.label})) - tooltip_placement = local_assigns.fetch(:tooltip_placement, "left") - path = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil) - css_classes = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip" +- title = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label} +- if type == 'commit' + - title = s_("PipelineStatusTooltip|Commit: %{ci_status}") % {ci_status: status.label} - if path = link_to path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 973950d27aa..ef063451c4f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5362,6 +5362,12 @@ msgstr "" msgid "PipelineSheduleIntervalPattern|Custom" msgstr "" +msgid "PipelineStatusTooltip|Commit: %{ci_status}" +msgstr "" + +msgid "PipelineStatusTooltip|Pipeline: %{ci_status}" +msgstr "" + msgid "Pipelines" msgstr "" From 99b2a5a2ebcf878b09e0591a7e4c39fe533e8e74 Mon Sep 17 00:00:00 2001 From: Scott Hampton Date: Wed, 27 Feb 2019 12:59:53 -0700 Subject: [PATCH 9/9] Remove extraneous data attribute I had `html: true` as a data attribute, but the tooltip was just text. --- app/views/ci/status/_icon.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/ci/status/_icon.html.haml b/app/views/ci/status/_icon.html.haml index 1ca0a55b156..f38bdb2e5ed 100644 --- a/app/views/ci/status/_icon.html.haml +++ b/app/views/ci/status/_icon.html.haml @@ -9,8 +9,8 @@ - title = s_("PipelineStatusTooltip|Commit: %{ci_status}") % {ci_status: status.label} - if path - = link_to path, class: css_classes, title: title, data: { html: true, placement: tooltip_placement } do + = link_to path, class: css_classes, title: title, data: { placement: tooltip_placement } do = sprite_icon(status.icon, size: size) - else - %span{ class: css_classes, title: title, data: { html: true, placement: tooltip_placement } } + %span{ class: css_classes, title: title, data: { placement: tooltip_placement } } = sprite_icon(status.icon, size: size)