diff options
author | Rémy Coutable <remy@rymai.me> | 2016-09-22 10:37:55 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-22 10:37:55 +0000 |
commit | a41df736a77ee8a0ae362d7ea3d63ffd84203d1c (patch) | |
tree | f1f01947982d453baac0834ac41bc285d79bef71 | |
parent | 9636b08190b85b2c29b3305dd96c86b3cf789d02 (diff) | |
parent | b0ce4ca11012388b1e4fbc1d928cbf4c0da66104 (diff) | |
download | gitlab-ce-a41df736a77ee8a0ae362d7ea3d63ffd84203d1c.tar.gz |
Merge branch 'fix-pipeline-for-empty-merge-request-diff' into 'master'
Fix pipeline error when trying to read empty merge request diff
When a user pushed something which resulted an empty merge request diff, `st_commits` would be `nil`. Therefore we also need to check if there exists `st_commits`.
We could tell this from:
``` ruby
def commits
@commits ||= load_commits(st_commits || [])
end
```
and
``` ruby
def save_commits
new_attributes = {}
commits = compare.commits
if commits.present?
commits = Commit.decorate(commits, merge_request.source_project).reverse
new_attributes[:st_commits] = dump_commits(commits)
end
update_columns_serialized(new_attributes)
end
```
Closes #22438
See merge request !6470
-rw-r--r-- | app/models/merge_request_diff.rb | 6 | ||||
-rw-r--r-- | spec/models/merge_request_diff_spec.rb | 22 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 27 |
3 files changed, 48 insertions, 7 deletions
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 7362886e9f5..36b8b70870b 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -30,6 +30,10 @@ class MergeRequestDiff < ActiveRecord::Base select(column_names - ['st_diffs']) end + def st_commits + super || [] + end + # Collect information about commits and diff from repository # and save it to the database as serialized data def save_git_content @@ -83,7 +87,7 @@ class MergeRequestDiff < ActiveRecord::Base end def commits - @commits ||= load_commits(st_commits || []) + @commits ||= load_commits(st_commits) end def reload_commits diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e5b185dc3f6..530a7def553 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -64,5 +64,27 @@ describe MergeRequestDiff, models: true do end end end + + describe '#commits_sha' do + shared_examples 'returning all commits SHA' do + it 'returns all commits SHA' do + commits_sha = subject.commits_sha + + expect(commits_sha).to eq(subject.commits.map(&:sha)) + end + end + + context 'when commits were loaded' do + before do + subject.commits + end + + it_behaves_like 'returning all commits SHA' + end + + context 'when commits were not loaded' do + it_behaves_like 'returning all commits SHA' + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 433aba7747b..12df6adde44 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -517,7 +517,7 @@ describe MergeRequest, models: true do context 'with multiple irrelevant merge_request_diffs' do before do - subject.update(target_branch: 'markdown') + subject.update(target_branch: 'v1.0.0') end it_behaves_like 'returning pipelines with proper ordering' @@ -544,13 +544,28 @@ describe MergeRequest, models: true do subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq end - before do - subject.update(target_branch: 'markdown') + shared_examples 'returning all SHA' do + it 'returns all SHA from all merge_request_diffs' do + expect(subject.merge_request_diffs.size).to eq(2) + expect(subject.all_commits_sha).to eq(all_commits_sha) + end + end + + context 'with a completely different branch' do + before do + subject.update(target_branch: 'v1.0.0') + end + + it_behaves_like 'returning all SHA' end - it 'returns all SHA from all merge_request_diffs' do - expect(subject.merge_request_diffs.size).to eq(2) - expect(subject.all_commits_sha).to eq(all_commits_sha) + context 'with a branch having no difference' do + before do + subject.update(target_branch: 'v1.1.0') + subject.reload # make sure commits were not cached + end + + it_behaves_like 'returning all SHA' end end |