Commit 7315401e authored by Rémy Coutable's avatar Rémy Coutable Committed by Robert Speicher

Merge branch 'fix/builds-api-nil-commit' into 'master'

Fix builds API response that did not include commit data

## What does this MR do?

This is fix for problem with builds API response not including information about commit this build is created for.

## What are the relevant issue numbers?

Closes #18476 

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4827
parent 421b5aaf
Please view this file on the master branch, on stable branches it's out of date.
v 8.9.0 (unreleased)
- Fix builds API response not including commit data
- Fix error when CI job variables key specified but not defined
- Fix pipeline status when there are no builds in pipeline
- Fix Error 500 when using closes_issues API with an external issue tracker
......
......@@ -54,6 +54,6 @@ class Projects::PipelinesController < Projects::ApplicationController
end
def commit
@commit ||= @pipeline.commit_data
@commit ||= @pipeline.commit
end
end
......@@ -37,22 +37,22 @@ module Ci
end
def git_author_name
commit_data.author_name if commit_data
commit.try(:author_name)
end
def git_author_email
commit_data.author_email if commit_data
commit.try(:author_email)
end
def git_commit_message
commit_data.message if commit_data
commit.try(:message)
end
def short_sha
Ci::Pipeline.truncate_sha(sha)
end
def commit_data
def commit
@commit ||= project.commit(sha)
rescue
nil
......
......@@ -8,6 +8,8 @@ class CommitStatus < ActiveRecord::Base
belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id, touch: true
belongs_to :user
delegate :commit, to: :pipeline
validates :pipeline, presence: true, unless: :importing?
validates_presence_of :name
......
......@@ -24,8 +24,8 @@
%span.label.label-warning stuck
%p.commit-title
- if commit_data = pipeline.commit_data
= link_to_gfm truncate(commit_data.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit_data.id), class: "commit-row-message"
- if commit = pipeline.commit
= link_to_gfm truncate(commit.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "commit-row-message"
- else
Cant find HEAD commit for this branch
......
......@@ -444,11 +444,7 @@ module API
expose :created_at, :started_at, :finished_at
expose :user, with: User
expose :artifacts_file, using: BuildArtifactFile, if: -> (build, opts) { build.artifacts? }
expose :commit, with: RepoCommit do |repo_obj, _options|
if repo_obj.respond_to?(:commit)
repo_obj.commit.commit_data
end
end
expose :commit, with: RepoCommit
expose :runner, with: Runner
end
......
......@@ -2,7 +2,12 @@ require 'spec_helper'
describe Ci::Build, models: true do
let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:pipeline) do
create(:ci_pipeline, project: project,
sha: project.commit.id)
end
let(:build) { create(:ci_build, pipeline: pipeline) }
it { is_expected.to validate_presence_of :ref }
......@@ -678,4 +683,10 @@ describe Ci::Build, models: true do
end
end
end
describe '#commit' do
it 'returns commit pipeline has been created for' do
expect(build.commit).to eq project.commit
end
end
end
require 'spec_helper'
describe CommitStatus, models: true do
let(:pipeline) { FactoryGirl.create :ci_pipeline }
let(:commit_status) { FactoryGirl.create :commit_status, pipeline: pipeline }
let(:project) { create(:project) }
let(:pipeline) do
create(:ci_pipeline, project: project, sha: project.commit.id)
end
let(:commit_status) { create(:commit_status, pipeline: pipeline) }
it { is_expected.to belong_to(:pipeline) }
it { is_expected.to belong_to(:user) }
......@@ -198,4 +203,10 @@ describe CommitStatus, models: true do
end
end
end
describe '#commit' do
it 'returns commit pipeline has been created for' do
expect(commit_status.commit).to eq project.commit
end
end
end
......@@ -9,8 +9,8 @@ describe API::API, api: true do
let!(:project) { create(:project, creator_id: user.id) }
let!(:developer) { create(:project_member, :developer, user: user, project: project) }
let!(:reporter) { create(:project_member, :reporter, user: user2, project: project) }
let(:pipeline) { create(:ci_pipeline, project: project)}
let(:build) { create(:ci_build, pipeline: pipeline) }
let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
let!(:build) { create(:ci_build, pipeline: pipeline) }
describe 'GET /projects/:id/builds ' do
let(:query) { '' }
......@@ -23,6 +23,11 @@ describe API::API, api: true do
expect(json_response).to be_an Array
end
it 'returns correct values' do
expect(json_response).not_to be_empty
expect(json_response.first['commit']['id']).to eq project.commit.id
end
context 'filter project with one scope element' do
let(:query) { 'scope=pending' }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment