diff options
author | Pierre de La Morinerie <kemenaran@gmail.com> | 2016-03-09 08:19:54 +0100 |
---|---|---|
committer | Pierre de La Morinerie <kemenaran@gmail.com> | 2016-03-16 14:02:04 +0100 |
commit | 18295585d9d2a7177f52fc451db7b0d542cc49b5 (patch) | |
tree | cd60e7b0b194c6f2c187bf30a1ff2db360427f22 | |
parent | 7cbf8fd0d089264f863dc01217514d7e2c40e056 (diff) | |
download | gitlab-ce-18295585d9d2a7177f52fc451db7b0d542cc49b5.tar.gz |
Fix MergeRequest#source_sha when there is no diff
`MergeRequest#source_sha` is expected to return the sha of the source
branch last commit.
But when a open Merge Request has no diff (e.g. all commits have already
been merged to the target branch), `merge_request.source_sha`
incorrectly returns `nil`.
This was un-noticed before – but now that !2217 has been merged,
it makes `Gitlab::Git::Commit.between` raise an
"Unexpected nil argument" exception.
This fixes the crash, by making sure that `source_sha` returns a
correct result even when there is no diff available.
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/merge_request.rb | 6 | ||||
-rw-r--r-- | spec/factories/merge_requests.rb | 5 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 7 |
4 files changed, 18 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG index fcf659c07f9..ccc356a9617 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,6 +33,7 @@ v 8.6.0 (unreleased) - Fix bug where Bitbucket `closed` issues were imported as `opened` (Iuri de Silvio) - Don't show Issues/MRs from archived projects in Groups view - Fix wrong "iid of max iid" in Issuable sidebar for some merged MRs + - Fix empty source_sha on Merge Request when there is no diff (Pierre de La Morinerie) - Increase the notes polling timeout over time (Roberto Dip) - Add shortcut to toggle markdown preview (Florent Baldino) - Show labels in dashboard and group milestone views diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 188325045e2..30a7bd47be7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -520,7 +520,11 @@ class MergeRequest < ActiveRecord::Base end def source_sha - last_commit.try(:sha) + last_commit.try(:sha) || source_tip.try(:sha) + end + + def source_tip + source_branch && source_project.repository.commit(source_branch) end def fetch_ref diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index a9df5fa1d3a..e281e2f227b 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -51,6 +51,11 @@ FactoryGirl.define do trait :with_diffs do end + trait :without_diffs do + source_branch "improve/awesome" + target_branch "master" + end + trait :conflict do source_branch "feature_conflict" target_branch "feature" diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index aceb2406eb3..619c01b5523 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -96,6 +96,13 @@ describe MergeRequest, models: true do end end + context 'without diffs' do + subject { create(:merge_request, :without_diffs) } + it 'returns the sha of the source branch last commit' do + expect(subject.source_sha).to eq(last_branch_commit.sha) + end + end + context 'when the merge request is being created' do subject { build(:merge_request, source_branch: nil, compare_commits: []) } it 'returns nil' do |