From 5416d0e0837f69f35a3a9deaf947fd62e5293661 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 21 Oct 2016 16:22:43 +0800
Subject: [PATCH 01/18] Pass `@ref` along so we know which pipeline to show
Closes #23615
---
app/controllers/application_controller.rb | 3 ++-
app/controllers/projects/commits_controller.rb | 9 ++++++++-
.../projects/merge_requests_controller.rb | 14 ++++++++++++--
app/helpers/ci_status_helper.rb | 5 +++++
app/helpers/commits_helper.rb | 8 +++++---
app/models/commit.rb | 4 ++++
app/views/projects/commits/_commit.html.haml | 4 ++--
app/views/projects/commits/_commit_list.html.haml | 2 +-
app/views/projects/commits/_commits.html.haml | 5 +----
app/views/projects/commits/show.html.haml | 2 +-
.../projects/merge_requests/branch_from.html.haml | 2 +-
.../projects/merge_requests/branch_to.html.haml | 2 +-
.../merge_requests/show/_commits.html.haml | 2 +-
13 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 37600ed875c..517ad4f03f3 100644
--- a/app/controllers/application_controller.rb
+++ b/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]
)
diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb
index c2e7bf1ffec..aba87b6144b 100644
--- a/app/controllers/projects/commits_controller.rb
+++ b/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
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 2ee53f7ceda..eff79b63cee 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/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
diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index b7f48630bd4..7d1b41b8fbe 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -54,6 +54,11 @@ module CiStatusHelper
custom_icon(icon_name)
end
+ def render_commit_ref_status(commit, ref = nil, **args)
+ pipeline = commit.pipelines_for(ref).last
+ render_pipeline_status(pipeline, **args)
+ end
+
def render_commit_status(commit, tooltip_placement: 'auto left')
project = commit.project
path = pipelines_namespace_project_commit_path(project.namespace, project, commit)
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 33dcee49aee..ed402b698fb 100644
--- a/app/helpers/commits_helper.rb
+++ b/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
diff --git a/app/models/commit.rb b/app/models/commit.rb
index e64fd1e0c1b..02e06657306 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -225,6 +225,10 @@ class Commit
)
end
+ def pipelines_for(ref)
+ project.pipelines.where(sha: sha, ref: ref)
+ end
+
def pipelines
@pipeline ||= project.pipelines.where(sha: sha)
end
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index fb48aef0559..00bf812f33f 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -20,13 +20,13 @@
= commit.short_id
- if commit.status
.visible-xs-inline
- = render_commit_status(commit)
+ = render_commit_ref_status(commit, ref)
- if commit.description?
%a.text-expander.hidden-xs.js-toggle-button ...
.commit-actions.hidden-xs
- if commit.status
- = render_commit_status(commit)
+ = render_commit_ref_status(commit, 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)
diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml
index 46e4de40042..ce416caa494 100644
--- a/app/views/projects/commits/_commit_list.html.haml
+++ b/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
diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml
index dd12eae8f7e..943ec24cfb5 100644
--- a/app/views/projects/commits/_commits.html.haml
+++ b/app/views/projects/commits/_commits.html.haml
@@ -1,13 +1,10 @@
-- unless defined?(project)
- - project = @project
-
- 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
diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml
index 876c8002627..9628cbd1634 100644
--- a/app/views/projects/commits/show.html.haml
+++ b/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
diff --git a/app/views/projects/merge_requests/branch_from.html.haml b/app/views/projects/merge_requests/branch_from.html.haml
index 4f90dde6fa8..f99f1946a5e 100644
--- a/app/views/projects/merge_requests/branch_from.html.haml
+++ b/app/views/projects/merge_requests/branch_from.html.haml
@@ -1 +1 @@
-= commit_to_html(@commit, @source_project, false)
+= commit_to_html(@commit, @ref, @source_project) if @commit
diff --git a/app/views/projects/merge_requests/branch_to.html.haml b/app/views/projects/merge_requests/branch_to.html.haml
index 67a7a6bcec9..a8b3c5d950a 100644
--- a/app/views/projects/merge_requests/branch_to.html.haml
+++ b/app/views/projects/merge_requests/branch_to.html.haml
@@ -1 +1 @@
-= commit_to_html(@commit, @target_project, false)
+= commit_to_html(@commit, @ref, @target_project) if @commit
diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml
index 0b05785430b..b488f343214 100644
--- a/app/views/projects/merge_requests/show/_commits.html.haml
+++ b/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.project, ref: @merge_request.source_branch
--
GitLab
From 17dfcf6d547c0153a8650df6e330c66397e4b2c9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 24 Oct 2016 23:23:28 +0800
Subject: [PATCH 02/18] Add a test for showing correct pipeline
---
spec/features/commits_spec.rb | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb
index 5910803df51..cd53a485ef4 100644
--- a/spec/features/commits_spec.rb
+++ b/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
--
GitLab
From 7fac0e17e9ceed12da831d40db0e2480b12b48ff Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 24 Oct 2016 23:24:32 +0800
Subject: [PATCH 03/18] Add CHANGELOG entry
---
CHANGELOG.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bee10eb4101..815c5332a90 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Add hover to trash icon in notes !7008 (blackst0ne)
- Simpler arguments passed to named_route on toggle_award_url helper method
- Fix: Backup restore doesn't clear cache
+ - Fix showing pipeline status from correct branch !7034
- Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
- Fix documents and comments on Build API `scope`
- Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
--
GitLab
From 51a012b8a855874df71802fdaa807559ff9817ae Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 25 Oct 2016 22:59:19 +0800
Subject: [PATCH 04/18] Be more specific, feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17397109
---
CHANGELOG.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index def94165c97..102c3b883e4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,7 +11,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Escape ref and path for relative links !6050 (winniehell)
- Simpler arguments passed to named_route on toggle_award_url helper method
- Fix: Backup restore doesn't clear cache
- - Fix showing pipeline status from correct branch !7034
+ - Fix showing pipeline status for a given commit from correct branch !7034
- Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
- Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
- Fix documents and comments on Build API `scope`
--
GitLab
From b4ef158a63807c850c1a17c75d66cd166c309047 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 27 Oct 2016 00:48:32 +0800
Subject: [PATCH 05/18] Still show status from pipelines, see:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17397201
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17397461
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6801#note_17468470
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17482654
---
app/helpers/ci_status_helper.rb | 19 ++++++++++--------
app/models/commit.rb | 21 +++++++++++++-------
app/views/projects/commits/_commit.html.haml | 4 ++--
3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index 7d1b41b8fbe..7851949ccf0 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -54,15 +54,18 @@ module CiStatusHelper
custom_icon(icon_name)
end
- def render_commit_ref_status(commit, ref = nil, **args)
- pipeline = commit.pipelines_for(ref).last
- render_pipeline_status(pipeline, **args)
- 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_for(ref),
+ path,
+ tooltip_placement: tooltip_placement)
end
def render_pipeline_status(pipeline, tooltip_placement: 'auto left')
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 02e06657306..337b7445b46 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -51,12 +51,14 @@ class Commit
end
attr_accessor :raw
+ attr_reader :statuses
def initialize(raw_commit, project)
raise "Nil as raw commit passed" unless raw_commit
@raw = raw_commit
@project = project
+ @statuses = {}
end
def id
@@ -225,17 +227,22 @@ class Commit
)
end
- def pipelines_for(ref)
- project.pipelines.where(sha: sha, ref: ref)
- 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
+ status_for(nil)
+ end
+
+ def status_for(ref)
+ 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
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 00bf812f33f..9303243b119 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -20,13 +20,13 @@
= commit.short_id
- if commit.status
.visible-xs-inline
- = render_commit_ref_status(commit, ref)
+ = 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_ref_status(commit, 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)
--
GitLab
From 6f6a8d2a671cf3bc2df2832081f528d9b06e27be Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 27 Oct 2016 01:37:41 +0800
Subject: [PATCH 06/18] Also update for default project page, feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17397573
---
app/views/projects/_last_commit.html.haml | 9 +++++----
app/views/projects/commits/_commit.html.haml | 4 ++--
app/views/projects/show.html.haml | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml
index 630ae7d6140..593967d9310 100644
--- a/app/views/projects/_last_commit.html.haml
+++ b/app/views/projects/_last_commit.html.haml
@@ -1,7 +1,8 @@
-- 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)
+- status = commit.status_for(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"
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 9303243b119..c5830b5b7f7 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -18,14 +18,14 @@
%span.commit-row-message.visible-xs-inline
·
= commit.short_id
- - if commit.status
+ - if commit.status_for(ref)
.visible-xs-inline
= render_commit_status(commit, ref: ref)
- if commit.description?
%a.text-expander.hidden-xs.js-toggle-button ...
.commit-actions.hidden-xs
- - if commit.status
+ - if commit.status_for(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"
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index ba16c641462..69a3bc5f046 100644
--- a/app/views/projects/show.html.haml
+++ b/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?
--
GitLab
From 141cb35dc5e8154f336e08ec7a38e150128577d6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 27 Oct 2016 03:44:53 +0800
Subject: [PATCH 07/18] Also pass ref here
---
app/views/projects/blob/_blob.html.haml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index 3ffc3fcb7ac..149ee7c59d6 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/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
--
GitLab
From fb19d9f4e2c552ac81c9ff0e8aaff6f0e0686fc3 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 28 Oct 2016 03:05:41 +0800
Subject: [PATCH 08/18] Use multi-line conditions in view, feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17522557
---
app/views/projects/merge_requests/branch_from.html.haml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/views/projects/merge_requests/branch_from.html.haml b/app/views/projects/merge_requests/branch_from.html.haml
index f99f1946a5e..3837c4b388d 100644
--- a/app/views/projects/merge_requests/branch_from.html.haml
+++ b/app/views/projects/merge_requests/branch_from.html.haml
@@ -1 +1,2 @@
-= commit_to_html(@commit, @ref, @source_project) if @commit
+- if @commit
+ = commit_to_html(@commit, @ref, @source_project)
--
GitLab
From f4b9de38edb3152888cb8e9ea0f9e48a6f08dd26 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 28 Oct 2016 03:12:12 +0800
Subject: [PATCH 09/18] It's not used as a public API right now, feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17522443
---
app/models/commit.rb | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 337b7445b46..20bb93f4663 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -51,7 +51,6 @@ class Commit
end
attr_accessor :raw
- attr_reader :statuses
def initialize(raw_commit, project)
raise "Nil as raw commit passed" unless raw_commit
@@ -236,12 +235,12 @@ class Commit
end
def status_for(ref)
- if statuses.key?(ref)
- statuses[ref]
+ if @statuses.key?(ref)
+ @statuses[ref]
elsif ref
- statuses[ref] = pipelines.where(ref: ref).status
+ @statuses[ref] = pipelines.where(ref: ref).status
else
- statuses[ref] = pipelines.status
+ @statuses[ref] = pipelines.status
end
end
--
GitLab
From 3e6a527e686e27b3cd9e048a17f1b491023d07d1 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 28 Oct 2016 03:47:49 +0800
Subject: [PATCH 10/18] Add tests for Commit#status and Commit#status_for,
feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17543036
---
spec/models/commit_spec.rb | 45 +++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 51be3f36135..468e198e9ea 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -210,7 +210,50 @@ eos
end
describe '#status' do
- # TODO: kamil
+ 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
+
+ describe '#status_for' 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_for('master')).to eq(pipeline_from_master.status)
+ expect(commit.status_for('fix')).to eq(pipeline_from_fix.status)
+ end
+
+ it 'gives compound status if ref is nil' do
+ expect(commit.status_for(nil)).to eq(commit.status)
+ end
end
describe '#participants' do
--
GitLab
From e036abf3a2f969036a2c4d59441a4d7a068d7ca4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 28 Oct 2016 03:53:18 +0800
Subject: [PATCH 11/18] Always use multiline in view
---
app/views/projects/merge_requests/branch_to.html.haml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/views/projects/merge_requests/branch_to.html.haml b/app/views/projects/merge_requests/branch_to.html.haml
index a8b3c5d950a..d69b71790a0 100644
--- a/app/views/projects/merge_requests/branch_to.html.haml
+++ b/app/views/projects/merge_requests/branch_to.html.haml
@@ -1 +1,2 @@
-= commit_to_html(@commit, @ref, @target_project) if @commit
+- if @commit
+ = commit_to_html(@commit, @ref, @target_project)
--
GitLab
From ee4c8b75de6b04a8c2ed755b1b409dab9f0536c5 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 3 Nov 2016 23:26:15 +0800
Subject: [PATCH 12/18] Fix CHANGELOG
---
CHANGELOG.md | 23 +----------------------
1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 188e6ca0332..67dafd4e422 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,28 +1,6 @@
Please view this file on the master branch, on stable branches it's out of date.
## 8.14.0 (2016-11-22)
- - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
- - Trim leading and trailing whitespace on project_path (Linus Thiel)
- - Prevent award emoji via notes for issues/MRs authored by user (barthc)
- - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO)
- - Fix extra space on Build sidebar on Firefox !7060
- - Fix HipChat notifications rendering (airatshigapov, eisnerd)
- - Add hover to trash icon in notes !7008 (blackst0ne)
- - Fix sidekiq stats in admin area (blackst0ne)
- - Escape ref and path for relative links !6050 (winniehell)
- - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
- - Fix filtering of milestones with quotes in title (airatshigapov)
- - Simpler arguments passed to named_route on toggle_award_url helper method
- - Fix typo in framework css class. !7086 (Daniel Voogsgerd)
- - Fix: Backup restore doesn't clear cache
- - Fix showing pipeline status for a given commit from correct branch !7034
- - API: Fix project deploy keys 400 and 500 errors when adding an existing key. !6784 (Joshua Welsh)
- - Replace jquery.cookie plugin with js.cookie !7085
- - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method
- - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
- - Fix documents and comments on Build API `scope`
- - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
-
- Backups do not fail anymore when using tar on annex and custom_hooks only. !5814
- Adds user project membership expired event to clarify why user was removed (Callum Dryden)
- Trim leading and trailing whitespace on project_path (Linus Thiel)
@@ -47,6 +25,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
- Fix filtering of milestones with quotes in title (airatshigapov)
- Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
+- Fix showing pipeline status for a given commit from correct branch !7034
- Update mail_room and enable sentinel support to Reply By Email (!7101)
- Add task completion status in Issues and Merge Requests tabs: "X of Y tasks completed" (!6527, @gmesalazar)
- Simpler arguments passed to named_route on toggle_award_url helper method
--
GitLab
From 1ae557c106e94c20742d0788dc7eb604603faa08 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 3 Nov 2016 23:39:37 +0800
Subject: [PATCH 13/18] Merge status_for and status, feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17742297
---
app/helpers/ci_status_helper.rb | 2 +-
app/models/commit.rb | 6 +-
app/views/projects/_last_commit.html.haml | 2 +-
app/views/projects/commits/_commit.html.haml | 4 +-
spec/models/commit_spec.rb | 70 ++++++++++----------
5 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index 7851949ccf0..3decedace4f 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -63,7 +63,7 @@ module CiStatusHelper
render_status_with_link(
'commit',
- commit.status_for(ref),
+ commit.status(ref),
path,
tooltip_placement: tooltip_placement)
end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 20bb93f4663..1fbda6f6acc 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -230,11 +230,7 @@ class Commit
project.pipelines.where(sha: sha)
end
- def status
- status_for(nil)
- end
-
- def status_for(ref)
+ def status(ref = nil)
if @statuses.key?(ref)
@statuses[ref]
elsif ref
diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml
index 593967d9310..8aa503159f9 100644
--- a/app/views/projects/_last_commit.html.haml
+++ b/app/views/projects/_last_commit.html.haml
@@ -1,4 +1,4 @@
-- status = commit.status_for(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)
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index c5830b5b7f7..fd70de9c13d 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -18,14 +18,14 @@
%span.commit-row-message.visible-xs-inline
·
= commit.short_id
- - if commit.status_for(ref)
+ - if commit.status(ref)
.visible-xs-inline
= render_commit_status(commit, ref: ref)
- if commit.description?
%a.text-expander.hidden-xs.js-toggle-button ...
.commit-actions.hidden-xs
- - if commit.status_for(ref)
+ - 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"
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 468e198e9ea..203fb6596a6 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -210,49 +210,51 @@ eos
end
describe '#status' do
- shared_examples 'giving the status from pipeline' do
- it do
- expect(commit.status).to eq(Ci::Pipeline.status)
+ 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
- end
- context 'with pipelines' do
- let!(:pipeline) do
- create(:ci_empty_pipeline, project: project, sha: commit.sha)
- 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
+ it_behaves_like 'giving the status from pipeline'
+ end
- context 'without pipelines' do
- it_behaves_like 'giving the status from pipeline'
+ context 'without pipelines' do
+ it_behaves_like 'giving the status from pipeline'
+ end
end
- end
- describe '#status_for' do
- let!(:pipeline_from_master) do
- create(:ci_empty_pipeline,
- project: project,
- sha: commit.sha,
- ref: 'master',
- status: 'failed')
- 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
+ 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_for('master')).to eq(pipeline_from_master.status)
- expect(commit.status_for('fix')).to eq(pipeline_from_fix.status)
- 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_for(nil)).to eq(commit.status)
+ it 'gives compound status if ref is nil' do
+ expect(commit.status(nil)).to eq(commit.status)
+ end
end
end
--
GitLab
From 0902ba524f6bfa7f7f6d234378c8847b541d1586 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 3 Nov 2016 23:43:17 +0800
Subject: [PATCH 14/18] Initialize @statuses in status rather than constructor
Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034#note_17742312
---
app/models/commit.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 1fbda6f6acc..9e7fde9503d 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -57,7 +57,6 @@ class Commit
@raw = raw_commit
@project = project
- @statuses = {}
end
def id
@@ -231,6 +230,8 @@ class Commit
end
def status(ref = nil)
+ @statuses ||= {}
+
if @statuses.key?(ref)
@statuses[ref]
elsif ref
--
GitLab
From f3c3d8e63ba078e55c0ce516e19ec11cea429e43 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 4 Nov 2016 00:00:03 +0800
Subject: [PATCH 15/18] There's no such method
---
spec/models/commit_spec.rb | 4 ----
1 file changed, 4 deletions(-)
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 203fb6596a6..e3bb3482d67 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -205,10 +205,6 @@ eos
end
end
- describe '#ci_commits' do
- # TODO: kamil
- end
-
describe '#status' do
context 'without arguments for compound status' do
shared_examples 'giving the status from pipeline' do
--
GitLab
From ce1dc4c25d3464b7a9a1b21d93157c9fed98f705 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 4 Nov 2016 02:42:27 +0800
Subject: [PATCH 16/18] Update for CHANGELOG
---
CHANGELOG.md | 1 -
changelogs/unreleased/show-status-from-branch.yml | 4 ++++
2 files changed, 4 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/show-status-from-branch.yml
diff --git a/CHANGELOG.md b/CHANGELOG.md
index edcfedece5b..7efa9efa4f8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,7 +38,6 @@ entry.
- Fix filtering of milestones with quotes in title (airatshigapov)
- Fix issue boards dragging bug in Safari
- Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
-- Fix showing pipeline status for a given commit from correct branch !7034
- Update mail_room and enable sentinel support to Reply By Email (!7101)
- Add task completion status in Issues and Merge Requests tabs: "X of Y tasks completed" (!6527, @gmesalazar)
- Simpler arguments passed to named_route on toggle_award_url helper method
diff --git a/changelogs/unreleased/show-status-from-branch.yml b/changelogs/unreleased/show-status-from-branch.yml
new file mode 100644
index 00000000000..6e51bf6932f
--- /dev/null
+++ b/changelogs/unreleased/show-status-from-branch.yml
@@ -0,0 +1,4 @@
+---
+title: Fix showing pipeline status for a given commit from correct branch
+merge_request: 7034
+author: Lin Jen-Shin
--
GitLab
From b9b508ad055db7633e44178cb4a00ef934313b04 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 4 Nov 2016 22:43:02 +0800
Subject: [PATCH 17/18] Remove author according to the document
https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/changelog.md
---
changelogs/unreleased/show-status-from-branch.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/changelogs/unreleased/show-status-from-branch.yml b/changelogs/unreleased/show-status-from-branch.yml
index 6e51bf6932f..1afc230c05c 100644
--- a/changelogs/unreleased/show-status-from-branch.yml
+++ b/changelogs/unreleased/show-status-from-branch.yml
@@ -1,4 +1,4 @@
---
title: Fix showing pipeline status for a given commit from correct branch
merge_request: 7034
-author: Lin Jen-Shin
+author:
--
GitLab
From 99410a4750b514b8d58a6d44f687ef29ecebc7cc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 4 Nov 2016 22:43:43 +0800
Subject: [PATCH 18/18] Fetch locals to avoid undefined method/local error
Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7034/diffs#note_17868074
---
app/views/projects/_last_commit.html.haml | 1 +
app/views/projects/commits/_commit.html.haml | 1 +
app/views/projects/commits/_commits.html.haml | 1 +
3 files changed, 3 insertions(+)
diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml
index 8aa503159f9..8e23d51b224 100644
--- a/app/views/projects/_last_commit.html.haml
+++ b/app/views/projects/_last_commit.html.haml
@@ -1,3 +1,4 @@
+- 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
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index fd70de9c13d..9f80a974d64 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -1,3 +1,4 @@
+- ref = local_assigns.fetch(:ref)
- if @note_counts
- note_count = @note_counts.fetch(commit.id, 0)
- else
diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml
index 943ec24cfb5..48756c68941 100644
--- a/app/views/projects/commits/_commits.html.haml
+++ b/app/views/projects/commits/_commits.html.haml
@@ -1,3 +1,4 @@
+- 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|
--
GitLab