From 28bae3b6bc35d760f51577b30a790c7d3d186588 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 11 Apr 2018 13:54:55 +0000 Subject: [PATCH] Resolve "tree/master shows incorrect CI build status" --- app/controllers/projects/commit_controller.rb | 1 + app/helpers/ci_status_helper.rb | 4 +- app/views/projects/commits/_commit.html.haml | 3 +- ...4541-fix-file-tree-commit-status-cache.yml | 5 ++ spec/features/dashboard/projects_spec.rb | 4 +- .../files/user_reads_pipeline_status_spec.rb | 46 +++++++++++++++++++ 6 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/44541-fix-file-tree-commit-status-cache.yml create mode 100644 spec/features/projects/files/user_reads_pipeline_status_spec.rb diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index effb484ef0f..b7f548e0e63 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -34,6 +34,7 @@ class Projects::CommitController < Projects::ApplicationController def pipelines @pipelines = @commit.pipelines.order(id: :desc) + @pipelines = @pipelines.where(ref: params[:ref]) if params[:ref] respond_to do |format| format.html diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 636316da80a..f0afcac5986 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -94,7 +94,7 @@ module CiStatusHelper def render_project_pipeline_status(pipeline_status, tooltip_placement: 'auto left') project = pipeline_status.project - path = pipelines_project_commit_path(project, pipeline_status.sha) + path = pipelines_project_commit_path(project, pipeline_status.sha, ref: pipeline_status.ref) render_status_with_link( 'commit', @@ -105,7 +105,7 @@ module CiStatusHelper def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left') project = commit.project - path = pipelines_project_commit_path(project, commit) + path = pipelines_project_commit_path(project, commit, ref: ref) render_status_with_link( 'commit', diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 163432c9263..289bfdd69bc 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -5,6 +5,7 @@ - link = commit_path(project, commit, merge_request: merge_request) - cache_key = [project.full_path, + ref, commit.id, Gitlab::CurrentSettings.current_application_settings, @path.presence, @@ -54,7 +55,7 @@ - if commit.status(ref) = render_commit_status(commit, ref: ref) - .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id) } } + .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } .commit-sha-group .label.label-monospace diff --git a/changelogs/unreleased/44541-fix-file-tree-commit-status-cache.yml b/changelogs/unreleased/44541-fix-file-tree-commit-status-cache.yml new file mode 100644 index 00000000000..ff734fe0c05 --- /dev/null +++ b/changelogs/unreleased/44541-fix-file-tree-commit-status-cache.yml @@ -0,0 +1,5 @@ +--- +title: Fix pipeline status in branch/tag tree page +merge_request: 17995 +author: +type: fixed diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 986f864f0b5..257a3822503 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -89,7 +89,7 @@ feature 'Dashboard Projects' do end describe 'with a pipeline', :clean_gitlab_redis_shared_state do - let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.sha) } + let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.sha, ref: project.default_branch) } before do # Since the cache isn't updated when a new pipeline is created @@ -102,7 +102,7 @@ feature 'Dashboard Projects' do visit dashboard_projects_path page.within('.controls') do - expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit)}']") + expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']") expect(page).to have_css('.ci-status-link') expect(page).to have_css('.ci-status-icon-success') expect(page).to have_link('Commit: passed') diff --git a/spec/features/projects/files/user_reads_pipeline_status_spec.rb b/spec/features/projects/files/user_reads_pipeline_status_spec.rb new file mode 100644 index 00000000000..2fb9da2f0a2 --- /dev/null +++ b/spec/features/projects/files/user_reads_pipeline_status_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe 'user reads pipeline status', :js do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:v110_pipeline) { create_pipeline('v1.1.0', 'success') } + let(:x110_pipeline) { create_pipeline('x1.1.0', 'failed') } + + before do + project.add_master(user) + + project.repository.add_tag(user, 'x1.1.0', 'v1.1.0') + v110_pipeline + x110_pipeline + + sign_in(user) + end + + shared_examples 'visiting project tree' do + scenario 'sees the correct pipeline status' do + visit project_tree_path(project, expected_pipeline.ref) + wait_for_requests + + page.within('.blob-commit-info') do + expect(page).to have_link('', href: project_pipeline_path(project, expected_pipeline)) + expect(page).to have_selector(".ci-status-icon-#{expected_pipeline.status}") + end + end + end + + it_behaves_like 'visiting project tree' do + let(:expected_pipeline) { v110_pipeline } + end + + it_behaves_like 'visiting project tree' do + let(:expected_pipeline) { x110_pipeline } + end + + def create_pipeline(ref, status) + create(:ci_pipeline, + project: project, + ref: ref, + sha: project.commit(ref).sha, + status: status) + end +end