diff options
| -rw-r--r-- | app/controllers/projects/blob_controller.rb | 39 | ||||
| -rw-r--r-- | app/presenters/blobs/unfold_presenter.rb | 31 | ||||
| -rw-r--r-- | spec/controllers/projects/blob_controller_spec.rb | 70 | ||||
| -rw-r--r-- | spec/presenters/blobs/unfold_presenter_spec.rb | 75 |
4 files changed, 129 insertions, 86 deletions
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 99fda0bf137..0a33856a8d3 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -91,51 +91,20 @@ class Projects::BlobController < Projects::ApplicationController apply_diff_view_cookie! @form = Blobs::UnfoldPresenter.new(blob, params.to_unsafe_h) - @lines = @form.lines - @match_line = @form.match_line_text - # We can keep only 'render_diff_lines' from this conditional when + # keep only json rendering when # https://gitlab.com/gitlab-org/gitlab-ce/issues/44988 is done if rendered_for_merge_request? - render_diff_lines + render json: DiffLineSerializer.new.represent(@form.diff_lines) else + @lines = @form.lines + @match_line = @form.match_line_text render layout: false end end private - # Converts a String array to Gitlab::Diff::Line array - def render_diff_lines - @lines.map! do |line| - diff_line = Gitlab::Diff::Line.new(line, nil, nil, nil, nil) - diff_line.rich_text = line - diff_line - end - - add_match_line - - render json: DiffLineSerializer.new.represent(@lines) - end - - def add_match_line - return unless @form.unfold? - - if @form.bottom? && @form.to < @blob.lines.size - old_pos = @form.to - @form.offset - new_pos = @form.to - elsif @form.since != 1 - old_pos = new_pos = @form.since - end - - # Match line is not needed when it reaches the top limit or bottom limit of the file. - return unless new_pos - - @match_line = Gitlab::Diff::Line.new(@match_line, 'match', nil, old_pos, new_pos) - - @form.bottom? ? @lines.push(@match_line) : @lines.unshift(@match_line) - end - def blob @blob ||= @repository.blob_at(@commit.id, @path) diff --git a/app/presenters/blobs/unfold_presenter.rb b/app/presenters/blobs/unfold_presenter.rb index 908b2f97f89..7b13db3bb74 100644 --- a/app/presenters/blobs/unfold_presenter.rb +++ b/app/presenters/blobs/unfold_presenter.rb @@ -25,6 +25,17 @@ module Blobs end end + # Converts a String array to Gitlab::Diff::Line array, with match line added + def diff_lines + diff_lines = lines.map do |line| + Gitlab::Diff::Line.new(line, nil, nil, nil, nil, rich_text: line) + end + + add_match_line(diff_lines) + + diff_lines + end + def lines strong_memoize(:lines) do lines = @all_lines @@ -40,5 +51,25 @@ module Blobs line = [since, lines_length].join(',') "@@ -#{line}+#{line} @@" end + + private + + def add_match_line(diff_lines) + return unless unfold? + + if bottom? && to < @all_lines.size + old_pos = to - offset + new_pos = to + elsif since != 1 + old_pos = new_pos = since + end + + # Match line is not needed when it reaches the top limit or bottom limit of the file. + return unless new_pos + + match_line = Gitlab::Diff::Line.new(match_line_text, 'match', nil, old_pos, new_pos) + + bottom? ? diff_lines.push(match_line) : diff_lines.unshift(match_line) + end end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 27093141ed9..3801fca09dc 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -144,66 +144,34 @@ describe Projects::BlobController do end context 'when rendering for merge request' do - it 'renders diff context lines Gitlab::Diff::Line array' do - do_get(since: 1, to: 5, offset: 10, from_merge_request: true) - - lines = JSON.parse(response.body) - - expect(lines.first).to have_key('type') - expect(lines.first).to have_key('rich_text') - expect(lines.first).to have_key('rich_text') + let(:presenter) { double(:presenter, diff_lines: diff_lines) } + let(:diff_lines) do + Array.new(3, Gitlab::Diff::Line.new('plain', nil, nil, nil, nil, rich_text: 'rich')) end - context 'when rendering match lines' do - it 'adds top match line when "since" is less than 1' do - do_get(since: 5, to: 10, offset: 10, from_merge_request: true) - - match_line = JSON.parse(response.body).first - - expect(match_line['type']).to eq('match') - expect(match_line['meta_data']).to have_key('old_pos') - expect(match_line['meta_data']).to have_key('new_pos') - end - - it 'does not add top match line when "since" is equal 1' do - do_get(since: 1, to: 10, offset: 10, from_merge_request: true) - - match_line = JSON.parse(response.body).first - - expect(match_line['type']).to be_nil - end - - it 'adds bottom match line when "to" is less than blob size' do - do_get(since: 1, to: 5, offset: 10, from_merge_request: true, bottom: true) - - match_line = JSON.parse(response.body).last - - expect(match_line['type']).to eq('match') - expect(match_line['meta_data']).to have_key('old_pos') - expect(match_line['meta_data']).to have_key('new_pos') - end + before do + allow(Blobs::UnfoldPresenter).to receive(:new).and_return(presenter) + end - it 'does not add bottom match line when "to" is equal to blob size' do - commit_id = project.repository.commit('master').id - blob = project.repository.blob_at(commit_id, 'CHANGELOG') - do_get(since: 1, to: blob.lines.count, offset: 10, from_merge_request: true, bottom: true) + it 'renders diff context lines Gitlab::Diff::Line array' do + do_get(since: 1, to: 2, offset: 0, from_merge_request: true) - match_line = JSON.parse(response.body).last + lines = JSON.parse(response.body) - expect(match_line['type']).to be_nil + expect(lines.size).to eq(diff_lines.size) + lines.each do |line| + expect(line).to have_key('type') + expect(line['text']).to eq('plain') + expect(line['rich_text']).to eq('rich') end + end - it 'returns all lines if "full" is true' do - commit_id = project.repository.commit('master').id - blob = project.repository.blob_at(commit_id, 'CHANGELOG') - do_get(full: true, from_merge_request: true, bottom: true) + it 'handles full being true' do + do_get(full: true, from_merge_request: true) - match_lines = JSON.parse(response.body) + lines = JSON.parse(response.body) - expect(match_lines.size).to eq(blob.lines.count - 1) - expect(match_lines.last['type']).to be_nil - expect(match_lines.last['text']).to include(blob.lines.last) - end + expect(lines.size).to eq(diff_lines.size) end end end diff --git a/spec/presenters/blobs/unfold_presenter_spec.rb b/spec/presenters/blobs/unfold_presenter_spec.rb index c4d7d457705..7ece5f623ce 100644 --- a/spec/presenters/blobs/unfold_presenter_spec.rb +++ b/spec/presenters/blobs/unfold_presenter_spec.rb @@ -41,6 +41,81 @@ describe Blobs::UnfoldPresenter do end end + describe '#diff_lines' do + let(:total_lines) { 50 } + let(:blob) { fake_blob(path: 'foo', data: (1..total_lines).to_a.join("\n")) } + + context 'when "full" is true' do + let(:params) { { full: true } } + + it 'returns all lines' do + lines = subject.diff_lines + + expect(lines.size).to eq(total_lines) + + lines.each.with_index do |line, index| + expect(line.text).to include("LC#{index + 1}") + expect(line.text).to eq(line.rich_text) + expect(line.type).to be_nil + end + end + + context 'when last line is empty' do + let(:blob) { fake_blob(path: 'foo', data: "1\n2\n") } + + it 'disregards last line' do + lines = subject.diff_lines + + expect(lines.size).to eq(2) + end + end + end + + context 'when "since" is equal to 1' do + let(:params) { { since: 1, to: 10, offset: 10 } } + + it 'does not add top match line' do + line = subject.diff_lines.first + + expect(line.type).to be_nil + end + end + + context 'when since is greater than 1' do + let(:params) { { since: 5, to: 10, offset: 10 } } + + it 'adds top match line' do + line = subject.diff_lines.first + + expect(line.type).to eq('match') + expect(line.old_pos).to eq(5) + expect(line.new_pos).to eq(5) + end + end + + context 'when "to" is less than blob size' do + let(:params) { { since: 1, to: 5, offset: 10, bottom: true } } + + it 'adds bottom match line' do + line = subject.diff_lines.last + + expect(line.type).to eq('match') + expect(line.old_pos).to eq(-5) + expect(line.new_pos).to eq(5) + end + end + + context 'when "to" is equal to blob size' do + let(:params) { { since: 1, to: total_lines, offset: 10, bottom: true } } + + it 'does not add bottom match line' do + line = subject.diff_lines.last + + expect(line.type).to be_nil + end + end + end + describe '#lines' do context 'when scope is specified' do let(:params) { { since: 2, to: 3 } } |