Commit 866bef80 authored by Patrick Bajao's avatar Patrick Bajao

Fix suggestion on lines that are not part of an MR

Return the `text` as plain string in the response instead of
including HTML tags but keep `rich_text` as is.

The fix is to modify `Blob::UnfoldPresenter#diff_files` to map
each raw diff line (limited by the range specified) to a
corresponding line in an array of highlighted lines to use as
`rich_text`.
parent bf172b11
...@@ -4,7 +4,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated ...@@ -4,7 +4,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
presents :blob presents :blob
def highlight(plain: nil) def highlight(plain: nil)
blob.load_all_data! if blob.respond_to?(:load_all_data!) load_all_blob_data
Gitlab::Highlight.highlight( Gitlab::Highlight.highlight(
blob.path, blob.path,
...@@ -17,4 +17,10 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated ...@@ -17,4 +17,10 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
def web_url def web_url
Gitlab::Routing.url_helpers.project_blob_url(blob.repository.project, File.join(blob.commit_id, blob.path)) Gitlab::Routing.url_helpers.project_blob_url(blob.repository.project, File.join(blob.commit_id, blob.path))
end end
private
def load_all_blob_data
blob.load_all_data! if blob.respond_to?(:load_all_data!)
end
end end
...@@ -16,8 +16,12 @@ module Blobs ...@@ -16,8 +16,12 @@ module Blobs
attribute :indent, Integer, default: 0 attribute :indent, Integer, default: 0
def initialize(blob, params) def initialize(blob, params)
# Load all blob data first as we need to ensure they're all loaded first
# so we can accurately show the rest of the diff when unfolding.
load_all_blob_data
@subject = blob @subject = blob
@all_lines = highlight.lines @all_lines = blob.data.lines
super(params) super(params)
if full? if full?
...@@ -25,10 +29,12 @@ module Blobs ...@@ -25,10 +29,12 @@ module Blobs
end end
end end
# Converts a String array to Gitlab::Diff::Line array, with match line added # Returns an array of Gitlab::Diff::Line with match line added
def diff_lines def diff_lines
diff_lines = lines.map do |line| diff_lines = lines.map.with_index do |line, index|
Gitlab::Diff::Line.new(line, nil, nil, nil, nil, rich_text: line) full_line = limited_blob_lines[index].delete("\n")
Gitlab::Diff::Line.new(full_line, nil, nil, nil, nil, rich_text: line)
end end
add_match_line(diff_lines) add_match_line(diff_lines)
...@@ -37,11 +43,7 @@ module Blobs ...@@ -37,11 +43,7 @@ module Blobs
end end
def lines def lines
strong_memoize(:lines) do @lines ||= limit(highlight.lines).map(&:html_safe)
lines = @all_lines
lines = lines[since - 1..to - 1] unless full?
lines.map(&:html_safe)
end
end end
def match_line_text def match_line_text
...@@ -71,5 +73,15 @@ module Blobs ...@@ -71,5 +73,15 @@ module Blobs
bottom? ? diff_lines.push(match_line) : diff_lines.unshift(match_line) bottom? ? diff_lines.push(match_line) : diff_lines.unshift(match_line)
end end
def limited_blob_lines
@limited_blob_lines ||= limit(@all_lines)
end
def limit(lines)
return lines if full?
lines[since - 1..to - 1]
end
end end
end end
---
title: Fix suggestion on lines that are not part of an MR
merge_request: 30606
author:
type: fixed
...@@ -54,8 +54,10 @@ describe Blobs::UnfoldPresenter do ...@@ -54,8 +54,10 @@ describe Blobs::UnfoldPresenter do
expect(lines.size).to eq(total_lines) expect(lines.size).to eq(total_lines)
lines.each.with_index do |line, index| lines.each.with_index do |line, index|
expect(line.text).to include("LC#{index + 1}") line_number = index + 1
expect(line.text).to eq(line.rich_text)
expect(line.text).to eq(line_number.to_s)
expect(line.rich_text).to include("LC#{line_number}")
expect(line.type).to be_nil expect(line.type).to be_nil
end end
end end
......
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