Merge branch 'use-raw-diffs-for-merge-request-diffs' into 'master'
change the API on the merge_request_diff model from diffs -> raw_diffs ## What does this MR do? Unify diffs API ## Are there points in the code the reviewer needs to double check? ## Why was this MR needed? ## What are the relevant issue numbers? ## Screenshots (if relevant) ## Does this MR meet the acceptance criteria? - ~~[ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added~~ - ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - ~~[ ] API support added~~ - Tests - ~~[ ] Added for this feature/bug~~ - [ ] 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 !5650
This commit is contained in:
commit
8890376f0f
4 changed files with 14 additions and 14 deletions
|
@ -165,7 +165,7 @@ class MergeRequest < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def raw_diffs(*args)
|
||||
merge_request_diff ? merge_request_diff.diffs(*args) : compare.raw_diffs(*args)
|
||||
merge_request_diff ? merge_request_diff.raw_diffs(*args) : compare.raw_diffs(*args)
|
||||
end
|
||||
|
||||
def diffs(diff_options = nil)
|
||||
|
|
|
@ -33,12 +33,12 @@ class MergeRequestDiff < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def size
|
||||
real_size.presence || diffs.size
|
||||
real_size.presence || raw_diffs.size
|
||||
end
|
||||
|
||||
def diffs(options={})
|
||||
def raw_diffs(options={})
|
||||
if options[:ignore_whitespace_change]
|
||||
@diffs_no_whitespace ||= begin
|
||||
@raw_diffs_no_whitespace ||= begin
|
||||
compare = Gitlab::Git::Compare.new(
|
||||
repository.raw_repository,
|
||||
self.start_commit_sha || self.target_branch_sha,
|
||||
|
@ -47,8 +47,8 @@ class MergeRequestDiff < ActiveRecord::Base
|
|||
compare.diffs(options)
|
||||
end
|
||||
else
|
||||
@diffs ||= {}
|
||||
@diffs[options] ||= load_diffs(st_diffs, options)
|
||||
@raw_diffs ||= {}
|
||||
@raw_diffs[options] ||= load_diffs(st_diffs, options)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -10,7 +10,7 @@ describe MergeRequestDiff, models: true do
|
|||
expect(mr_diff).not_to receive(:load_diffs)
|
||||
expect(Gitlab::Git::Compare).to receive(:new).and_call_original
|
||||
|
||||
mr_diff.diffs(ignore_whitespace_change: true)
|
||||
mr_diff.raw_diffs(ignore_whitespace_change: true)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -18,19 +18,19 @@ describe MergeRequestDiff, models: true do
|
|||
before { mr_diff.update_attributes(st_diffs: '') }
|
||||
|
||||
it 'returns an empty DiffCollection' do
|
||||
expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection)
|
||||
expect(mr_diff.diffs).to be_empty
|
||||
expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
||||
expect(mr_diff.raw_diffs).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the raw diffs exist' do
|
||||
it 'returns the diffs' do
|
||||
expect(mr_diff.diffs).to be_a(Gitlab::Git::DiffCollection)
|
||||
expect(mr_diff.diffs).not_to be_empty
|
||||
expect(mr_diff.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
||||
expect(mr_diff.raw_diffs).not_to be_empty
|
||||
end
|
||||
|
||||
context 'when the :paths option is set' do
|
||||
let(:diffs) { mr_diff.diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) }
|
||||
let(:diffs) { mr_diff.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) }
|
||||
|
||||
it 'only returns diffs that match the (old path, new path) given' do
|
||||
expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb')
|
||||
|
|
|
@ -136,7 +136,7 @@ describe MergeRequest, models: true do
|
|||
it 'delegates to the MR diffs' do
|
||||
merge_request.merge_request_diff = MergeRequestDiff.new
|
||||
|
||||
expect(merge_request.merge_request_diff).to receive(:diffs).with(options)
|
||||
expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(options)
|
||||
|
||||
merge_request.raw_diffs(options)
|
||||
end
|
||||
|
@ -161,7 +161,7 @@ describe MergeRequest, models: true do
|
|||
it 'delegates to the MR diffs' do
|
||||
merge_request.merge_request_diff = MergeRequestDiff.new
|
||||
|
||||
expect(merge_request.merge_request_diff).to receive(:diffs).with(hash_including(options))
|
||||
expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options))
|
||||
|
||||
merge_request.diffs(options)
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue