Merge branch 'show-status-from-branch' into 'master'

Show pipeline status from branch and commit than only commit

Closes #23615

See merge request !7034
Signed-off-by: Rémy Coutable <remy@rymai.me>

app/controllers/application_controller.rb

@@ -192,9 +192,10 @@ class ApplicationController < ActionController::Base
   end
 
   # JSON for infinite scroll via Pager object
-  def pager_json(partial, count)
+  def pager_json(partial, count, locals = {})
     html = render_to_string(
       partial,
+      locals: locals,
       layout: false,
       formats: [:html]
     )

app/controllers/projects/commits_controller.rb

@@ -26,8 +26,15 @@ class Projects::CommitsController < Projects::ApplicationController
 
     respond_to do |format|
       format.html
-      format.json { pager_json("projects/commits/_commits", @commits.size) }
       format.atom { render layout: false }
+
+      format.json do
+        pager_json(
+          'projects/commits/_commits',
+          @commits.size,
+          project: @project,
+          ref: @ref)
+      end
     end
   end
 end

app/controllers/projects/merge_requests_controller.rb

@@ -352,13 +352,23 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   def branch_from
     # This is always source
     @source_project = @merge_request.nil? ? @project : @merge_request.source_project
-    @commit = @repository.commit(params[:ref]) if params[:ref].present?
+
+    if params[:ref].present?
+      @ref = params[:ref]
+      @commit = @repository.commit(@ref)
+    end
+
     render layout: false
   end
 
   def branch_to
     @target_project = selected_target_project
-    @commit = @target_project.commit(params[:ref]) if params[:ref].present?
+
+    if params[:ref].present?
+      @ref = params[:ref]
+      @commit = @target_project.commit(@ref)
+    end
+
     render layout: false
   end
 

app/helpers/ci_status_helper.rb

@@ -54,10 +54,18 @@ module CiStatusHelper
     custom_icon(icon_name)
   end
 
-  def render_commit_status(commit, tooltip_placement: 'auto left')
+  def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left')
     project = commit.project
-    path = pipelines_namespace_project_commit_path(project.namespace, project, commit)
-    render_status_with_link('commit', commit.status, path, tooltip_placement: tooltip_placement)
+    path = pipelines_namespace_project_commit_path(
+      project.namespace,
+      project,
+      commit)
+
+    render_status_with_link(
+      'commit',
+      commit.status(ref),
+      path,
+      tooltip_placement: tooltip_placement)
   end
 
   def render_pipeline_status(pipeline, tooltip_placement: 'auto left')

app/helpers/commits_helper.rb

@@ -25,9 +25,11 @@ module CommitsHelper
     end
   end
 
-  def commit_to_html(commit, project, inline = true)
-    template = inline ? "inline_commit" : "commit"
-    render "projects/commits/#{template}", commit: commit, project: project unless commit.nil?
+  def commit_to_html(commit, ref, project)
+    render 'projects/commits/commit',
+      commit: commit,
+      ref: ref,
+      project: project
   end
 
   # Breadcrumb links for a Project and, if applicable, a tree path

app/models/commit.rb

@@ -226,12 +226,19 @@ class Commit
   end
 
   def pipelines
-    @pipeline ||= project.pipelines.where(sha: sha)
+    project.pipelines.where(sha: sha)
   end
 
-  def status
-    return @status if defined?(@status)
-    @status ||= pipelines.status
+  def status(ref = nil)
+    @statuses ||= {}
+
+    if @statuses.key?(ref)
+      @statuses[ref]
+    elsif ref
+      @statuses[ref] = pipelines.where(ref: ref).status
+    else
+      @statuses[ref] = pipelines.status
+    end
   end
 
   def revert_branch_name

app/views/projects/_last_commit.html.haml

-- if commit.status
-  = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{commit.status}" do
-    = ci_icon_for_status(commit.status)
-    = ci_label_for_status(commit.status)
+- ref = local_assigns.fetch(:ref)
+- status = commit.status(ref)
+- if status
+  = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do
+    = ci_icon_for_status(status)
+    = ci_label_for_status(status)
 
 = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id"
 = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message"

app/views/projects/blob/_blob.html.haml

@@ -20,7 +20,7 @@
 
 %ul.blob-commit-info.hidden-xs
   - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path)
-  = render blob_commit, project: @project
+  = render blob_commit, project: @project, ref: @ref
 
 %div#blob-content-holder.blob-content-holder
   %article.file-holder

app/views/projects/commits/_commit.html.haml

+- ref = local_assigns.fetch(:ref)
 - if @note_counts
   - note_count = @note_counts.fetch(commit.id, 0)
 - else
@@ -18,15 +19,15 @@
           %span.commit-row-message.visible-xs-inline
             ·
             = commit.short_id
-          - if commit.status
+          - if commit.status(ref)
             .visible-xs-inline
-              = render_commit_status(commit)
+              = render_commit_status(commit, ref: ref)
           - if commit.description?
             %a.text-expander.hidden-xs.js-toggle-button ...
 
         .commit-actions.hidden-xs
-          - if commit.status
-            = render_commit_status(commit)
+          - if commit.status(ref)
+            = render_commit_status(commit, ref: ref)
           = clipboard_button(clipboard_text: commit.id)
           = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent"
           = link_to_browse_code(project, commit)

app/views/projects/commits/_commit_list.html.haml

@@ -11,4 +11,4 @@
       %li.warning-row.unstyled
         #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues.
   - else
-    %ul.content-list= render commits, project: @project
+    %ul.content-list= render commits, project: @project, ref: @ref

app/views/projects/commits/_commits.html.haml

-- unless defined?(project)
-  - project = @project
-
+- ref = local_assigns.fetch(:ref)
 - commits, hidden = limited_commits(@commits)
 
 - commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits|
   %li.commit-header= "#{day.strftime('%d %b, %Y')} #{pluralize(commits.count, 'commit')}"
   %li.commits-row
     %ul.list-unstyled.commit-list
-      = render commits, project: project
+      = render commits, project: project, ref: ref
 
 - if hidden > 0
   %li.alert.alert-warning

app/views/projects/commits/show.html.haml

@@ -35,7 +35,7 @@
 
   %div{id: dom_id(@project)}
     %ol#commits-list.list-unstyled.content_list
-      = render "commits", project: @project
+      = render 'commits', project: @project, ref: @ref
   = spinner
 
 :javascript

app/views/projects/merge_requests/branch_from.html.haml

-= commit_to_html(@commit, @source_project, false)
+- if @commit
+  = commit_to_html(@commit, @ref, @source_project)

app/views/projects/merge_requests/branch_to.html.haml

-= commit_to_html(@commit, @target_project, false)
+- if @commit
+  = commit_to_html(@commit, @ref, @target_project)

app/views/projects/merge_requests/show/_commits.html.haml

@@ -3,4 +3,4 @@
   Most recent commits displayed first
 
 %ol#commits-list.list-unstyled
-  = render "projects/commits/commits", project: @merge_request.project
+  = render "projects/commits/commits", project: @merge_request.source_project, ref: @merge_request.source_branch

app/views/projects/show.html.haml

@@ -79,7 +79,7 @@
           = render 'shared/notifications/button', notification_setting: @notification_setting
   - if @repository.commit
     .project-last-commit{ class: container_class }
-      = render 'projects/last_commit', commit: @repository.commit, project: @project
+      = render 'projects/last_commit', commit: @repository.commit, ref: current_ref, project: @project
 
 %div{ class: container_class }
   - if @project.archived?

changelogs/unreleased/show-status-from-branch.yml

+---
+title: Fix showing pipeline status for a given commit from correct branch
+merge_request: 7034
+author:

spec/features/commits_spec.rb

@@ -12,11 +12,15 @@ describe 'Commits' do
     end
 
     let!(:pipeline) do
-      FactoryGirl.create :ci_pipeline, project: project, sha: project.commit.sha
+      create(:ci_pipeline,
+             project: project,
+             ref: project.default_branch,
+             sha: project.commit.sha,
+             status: :success)
     end
 
     context 'commit status is Generic Commit Status' do
-      let!(:status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline }
+      let!(:status) { create(:generic_commit_status, pipeline: pipeline) }
 
       before do
         project.team << [@user, :reporter]
@@ -39,7 +43,7 @@ describe 'Commits' do
     end
 
     context 'commit status is Ci Build' do
-      let!(:build) { FactoryGirl.create :ci_build, pipeline: pipeline }
+      let!(:build) { create(:ci_build, pipeline: pipeline) }
       let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
 
       context 'when logged as developer' do
@@ -48,13 +52,22 @@ describe 'Commits' do
         end
 
         describe 'Project commits' do
+          let!(:pipeline_from_other_branch) do
+            create(:ci_pipeline,
+                   project: project,
+                   ref: 'fix',
+                   sha: project.commit.sha,
+                   status: :failed)
+          end
+
           before do
             visit namespace_project_commits_path(project.namespace, project, :master)
           end
 
-          it 'shows build status' do
+          it 'shows correct build status from default branch' do
             page.within("//li[@id='commit-#{pipeline.short_sha}']") do
-              expect(page).to have_css(".ci-status-link")
+              expect(page).to have_css('.ci-status-link')
+              expect(page).to have_css('.ci-status-icon-success')
             end
           end
         end

spec/models/commit_spec.rb

@@ -205,12 +205,53 @@ eos
     end
   end
 
-  describe '#ci_commits' do
-    # TODO: kamil
-  end
-
   describe '#status' do
-    # TODO: kamil
+    context 'without arguments for compound status' do
+      shared_examples 'giving the status from pipeline' do
+        it do
+          expect(commit.status).to eq(Ci::Pipeline.status)
+        end
+      end
+
+      context 'with pipelines' do
+        let!(:pipeline) do
+          create(:ci_empty_pipeline, project: project, sha: commit.sha)
+        end
+
+        it_behaves_like 'giving the status from pipeline'
+      end
+
+      context 'without pipelines' do
+        it_behaves_like 'giving the status from pipeline'
+      end
+    end
+
+    context 'when a particular ref is specified' do
+      let!(:pipeline_from_master) do
+        create(:ci_empty_pipeline,
+               project: project,
+               sha: commit.sha,
+               ref: 'master',
+               status: 'failed')
+      end
+
+      let!(:pipeline_from_fix) do
+        create(:ci_empty_pipeline,
+               project: project,
+               sha: commit.sha,
+               ref: 'fix',
+               status: 'success')
+      end
+
+      it 'gives pipelines from a particular branch' do
+        expect(commit.status('master')).to eq(pipeline_from_master.status)
+        expect(commit.status('fix')).to eq(pipeline_from_fix.status)
+      end
+
+      it 'gives compound status if ref is nil' do
+        expect(commit.status(nil)).to eq(commit.status)
+      end
+    end
   end
 
   describe '#participants' do