summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_gutter_content.vue6
-rw-r--r--app/assets/javascripts/diffs/components/diff_table_cell.vue10
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_table_row.vue6
-rw-r--r--app/assets/javascripts/diffs/components/parallel_diff_table_row.vue6
-rw-r--r--app/assets/javascripts/diffs/constants.js2
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js8
-rw-r--r--app/controllers/projects/blob_controller.rb2
-rw-r--r--app/controllers/projects/merge_requests/diffs_controller.rb6
-rw-r--r--app/models/diff_note.rb45
-rw-r--r--app/services/merge_requests/reload_diffs_service.rb4
-rw-r--r--app/services/notes/base_service.rb15
-rw-r--r--app/services/notes/create_service.rb3
-rw-r--r--app/services/notes/destroy_service.rb4
-rw-r--r--changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml5
-rw-r--r--lib/gitlab/diff/file.rb19
-rw-r--r--lib/gitlab/diff/line.rb13
-rw-r--r--lib/gitlab/diff/lines_unfolder.rb235
-rw-r--r--lib/gitlab/diff/position.rb12
-rw-r--r--spec/controllers/projects/blob_controller_spec.rb4
-rw-r--r--spec/features/merge_request/user_posts_diff_notes_spec.rb13
-rw-r--r--spec/javascripts/diffs/store/mutations_spec.js4
-rw-r--r--spec/lib/gitlab/diff/file_spec.rb46
-rw-r--r--spec/lib/gitlab/diff/lines_unfolder_spec.rb750
-rw-r--r--spec/services/merge_requests/reload_diffs_service_spec.rb21
-rw-r--r--spec/services/notes/create_service_spec.rb51
-rw-r--r--spec/services/notes/destroy_service_spec.rb33
26 files changed, 89 insertions, 1234 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
index e31a3546b69..f4a9be19496 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -55,6 +55,11 @@ export default {
required: false,
default: false,
},
+ isContextLine: {
+ type: Boolean,
+ required: false,
+ default: false,
+ },
isHover: {
type: Boolean,
required: false,
@@ -76,6 +81,7 @@ export default {
this.showCommentButton &&
this.isHover &&
!this.isMatchLine &&
+ !this.isContextLine &&
!this.isMetaLine &&
!this.hasDiscussions
);
diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue
index e26aa9c9b00..5d9a0b123fe 100644
--- a/app/assets/javascripts/diffs/components/diff_table_cell.vue
+++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue
@@ -3,6 +3,7 @@ import { mapGetters } from 'vuex';
import DiffLineGutterContent from './diff_line_gutter_content.vue';
import {
MATCH_LINE_TYPE,
+ CONTEXT_LINE_TYPE,
EMPTY_CELL_TYPE,
OLD_LINE_TYPE,
OLD_NO_NEW_LINE_TYPE,
@@ -70,6 +71,9 @@ export default {
isMatchLine() {
return this.line.type === MATCH_LINE_TYPE;
},
+ isContextLine() {
+ return this.line.type === CONTEXT_LINE_TYPE;
+ },
isMetaLine() {
const { type } = this.line;
@@ -84,7 +88,11 @@ export default {
[type]: type,
[LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
[LINE_HOVER_CLASS_NAME]:
- this.isLoggedIn && this.isHover && !this.isMatchLine && !this.isMetaLine,
+ this.isLoggedIn &&
+ this.isHover &&
+ !this.isMatchLine &&
+ !this.isContextLine &&
+ !this.isMetaLine,
};
},
lineNumber() {
diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
index 44c05e4b634..542acd3d930 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -4,6 +4,8 @@ import DiffTableCell from './diff_table_cell.vue';
import {
NEW_LINE_TYPE,
OLD_LINE_TYPE,
+ CONTEXT_LINE_TYPE,
+ CONTEXT_LINE_CLASS_NAME,
PARALLEL_DIFF_VIEW_TYPE,
LINE_POSITION_LEFT,
LINE_POSITION_RIGHT,
@@ -39,9 +41,13 @@ export default {
},
computed: {
...mapGetters('diffs', ['isInlineView']),
+ isContextLine() {
+ return this.line.type === CONTEXT_LINE_TYPE;
+ },
classNameMap() {
return {
[this.line.type]: this.line.type,
+ [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
[PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
};
},
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
index 39312cddfce..fcc3b3e9117 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
@@ -5,6 +5,8 @@ import DiffTableCell from './diff_table_cell.vue';
import {
NEW_LINE_TYPE,
OLD_LINE_TYPE,
+ CONTEXT_LINE_TYPE,
+ CONTEXT_LINE_CLASS_NAME,
OLD_NO_NEW_LINE_TYPE,
PARALLEL_DIFF_VIEW_TYPE,
NEW_NO_NEW_LINE_TYPE,
@@ -41,8 +43,12 @@ export default {
};
},
computed: {
+ isContextLine() {
+ return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE;
+ },
classNameMap() {
return {
+ [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
[PARALLEL_DIFF_VIEW_TYPE]: true,
};
},
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index f5f5c0ffc29..78a39baa4cb 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -3,6 +3,7 @@ export const PARALLEL_DIFF_VIEW_TYPE = 'parallel';
export const MATCH_LINE_TYPE = 'match';
export const OLD_NO_NEW_LINE_TYPE = 'old-nonewline';
export const NEW_NO_NEW_LINE_TYPE = 'new-nonewline';
+export const CONTEXT_LINE_TYPE = 'context';
export const EMPTY_CELL_TYPE = 'empty-cell';
export const COMMENT_FORM_TYPE = 'commentForm';
export const DIFF_NOTE_TYPE = 'DiffNote';
@@ -21,6 +22,7 @@ export const LINE_SIDE_RIGHT = 'right-side';
export const DIFF_VIEW_COOKIE_NAME = 'diff_view';
export const LINE_HOVER_CLASS_NAME = 'is-over';
export const LINE_UNFOLD_CLASS_NAME = 'unfold js-unfold';
+export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded';
export const UNFOLD_COUNT = 20;
export const COUNT_OF_AVATARS_IN_GUTTER = 3;
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index e651c197968..a7eea2c1449 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -65,13 +65,7 @@ export default {
const { highlightedDiffLines, parallelDiffLines } = diffFile;
removeMatchLine(diffFile, lineNumbers, bottom);
-
- const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({
- ...line,
- lineCode: line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`,
- discussions: line.discussions || [],
- }));
-
+ const lines = addLineReferences(contextLines, lineNumbers, bottom);
addContextLines({
inlineLines: highlightedDiffLines,
parallelLines: parallelDiffLines,
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 2a6fe3b9c97..c02ec407262 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController
@lines.map! do |line|
# These are marked as context lines but are loaded from blobs.
# We also have context lines loaded from diffs in other places.
- diff_line = Gitlab::Diff::Line.new(line, nil, nil, nil, nil)
+ diff_line = Gitlab::Diff::Line.new(line, 'context', nil, nil, nil)
diff_line.rich_text = line
diff_line
end
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index 740f41c0642..5307cd0720a 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -22,12 +22,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
def render_diffs
@environment = @merge_request.environments_for(current_user).last
- notes_grouped_by_path = @notes.group_by { |note| note.position.file_path }
-
- @diffs.diff_files.each do |diff_file|
- notes = notes_grouped_by_path.fetch(diff_file.file_path, [])
- notes.each { |note| diff_file.unfold_diff_lines(note.position) }
- end
@diffs.write_cache
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index c32008aa9c7..5f59e4832db 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -66,10 +66,6 @@ class DiffNote < Note
self.original_position.diff_refs == diff_refs
end
- def discussion_first_note?
- self == discussion.first_note
- end
-
private
def enqueue_diff_file_creation_job
@@ -82,33 +78,26 @@ class DiffNote < Note
end
def should_create_diff_file?
- on_text? && note_diff_file.nil? && discussion_first_note?
+ on_text? && note_diff_file.nil? && self == discussion.first_note
end
def fetch_diff_file
- file =
- if note_diff_file
- diff = Gitlab::Git::Diff.new(note_diff_file.to_hash)
- Gitlab::Diff::File.new(diff,
- repository: project.repository,
- diff_refs: original_position.diff_refs)
- elsif created_at_diff?(noteable.diff_refs)
- # We're able to use the already persisted diffs (Postgres) if we're
- # presenting a "current version" of the MR discussion diff.
- # So no need to make an extra Gitaly diff request for it.
- # As an extra benefit, the returned `diff_file` already
- # has `highlighted_diff_lines` data set from Redis on
- # `Diff::FileCollection::MergeRequestDiff`.
- noteable.diffs(original_position.diff_options).diff_files.first
- else
- original_position.diff_file(self.project.repository)
- end
-
- # Since persisted diff files already have its content "unfolded"
- # there's no need to make it pass through the unfolding process.
- file&.unfold_diff_lines(position) unless note_diff_file
-
- file
+ if note_diff_file
+ diff = Gitlab::Git::Diff.new(note_diff_file.to_hash)
+ Gitlab::Diff::File.new(diff,
+ repository: project.repository,
+ diff_refs: original_position.diff_refs)
+ elsif created_at_diff?(noteable.diff_refs)
+ # We're able to use the already persisted diffs (Postgres) if we're
+ # presenting a "current version" of the MR discussion diff.
+ # So no need to make an extra Gitaly diff request for it.
+ # As an extra benefit, the returned `diff_file` already
+ # has `highlighted_diff_lines` data set from Redis on
+ # `Diff::FileCollection::MergeRequestDiff`.
+ noteable.diffs(original_position.diff_options).diff_files.first
+ else
+ original_position.diff_file(self.project.repository)
+ end
end
def supported?
diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb
index c64b2e99b52..b47d8f3f63a 100644
--- a/app/services/merge_requests/reload_diffs_service.rb
+++ b/app/services/merge_requests/reload_diffs_service.rb
@@ -29,6 +29,10 @@ module MergeRequests
# rubocop: disable CodeReuse/ActiveRecord
def clear_cache(new_diff)
+ # Executing the iteration we cache highlighted diffs for each diff file of
+ # MergeRequestDiff.
+ cacheable_collection(new_diff).write_cache
+
# Remove cache for all diffs on this MR. Do not use the association on the
# model, as that will interfere with other actions happening when
# reloading the diff.
diff --git a/app/services/notes/base_service.rb b/app/services/notes/base_service.rb
deleted file mode 100644
index 431ff6c11c4..00000000000
--- a/app/services/notes/base_service.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-# frozen_string_literal: true
-
-module Notes
- class BaseService < ::BaseService
- def clear_noteable_diffs_cache(note)
- noteable = note.noteable
-
- if note.is_a?(DiffNote) &&
- note.discussion_first_note? &&
- note.position.unfolded_diff?(project.repository)
- noteable.diffs.clear_cache
- end
- end
- end
-end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index e03789e3ca9..049e6c5a871 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
module Notes
- class CreateService < ::Notes::BaseService
+ class CreateService < ::BaseService
def execute
merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha)
@@ -35,7 +35,6 @@ module Notes
if !only_commands && note.save
todo_service.new_note(note, current_user)
- clear_noteable_diffs_cache(note)
end
if command_params.present?
diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb
index fa0c2c5c86b..64e9accd97f 100644
--- a/app/services/notes/destroy_service.rb
+++ b/app/services/notes/destroy_service.rb
@@ -1,13 +1,11 @@
# frozen_string_literal: true
module Notes
- class DestroyService < ::Notes::BaseService
+ class DestroyService < BaseService
def execute(note)
TodoService.new.destroy_target(note) do |note|
note.destroy
end
-
- clear_noteable_diffs_cache(note)
end
end
end
diff --git a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml
deleted file mode 100644
index 7b48a94a993..00000000000
--- a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Allow commenting on any diff line in Merge Requests
-merge_request: 22398
-author:
-type: added
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 84595f8afd7..fb117baca9e 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -26,7 +26,6 @@ module Gitlab
@repository = repository
@diff_refs = diff_refs
@fallback_diff_refs = fallback_diff_refs
- @unfolded = false
# Ensure items are collected in the the batch
new_blob_lazy
@@ -136,24 +135,6 @@ module Gitlab
Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a
end
- # Changes diff_lines according to the given position. That is,
- # it checks whether the position requires blob lines into the diff
- # in order to be presented.
- def unfold_diff_lines(position)
- return unless position
-
- unfolder = Gitlab::Diff::LinesUnfolder.new(self, position)
-
- if unfolder.unfold_required?
- @diff_lines = unfolder.unfolded_diff_lines
- @unfolded = true
- end
- end
-
- def unfolded?
- @unfolded
- end
-
def highlighted_diff_lines
@highlighted_diff_lines ||=
Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb
index 70063071ee7..5b67cd46c48 100644
--- a/lib/gitlab/diff/line.rb
+++ b/lib/gitlab/diff/line.rb
@@ -3,9 +3,9 @@ module Gitlab
class Line
SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze
- attr_reader :line_code, :type, :old_pos, :new_pos
+ attr_reader :line_code, :type, :index, :old_pos, :new_pos
attr_writer :rich_text
- attr_accessor :text, :index
+ attr_accessor :text
def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil)
@text, @type, @index = text, type, index
@@ -19,14 +19,7 @@ module Gitlab
end
def self.init_from_hash(hash)
- new(hash[:text],
- hash[:type],
- hash[:index],
- hash[:old_pos],
- hash[:new_pos],
- parent_file: hash[:parent_file],
- line_code: hash[:line_code],
- rich_text: hash[:rich_text])
+ new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code], rich_text: hash[:rich_text])
end
def to_hash
diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb
deleted file mode 100644
index 9306b7e16a2..00000000000
--- a/lib/gitlab/diff/lines_unfolder.rb
+++ /dev/null
@@ -1,235 +0,0 @@
-# frozen_string_literal: true
-
-# Given a position, calculates which Blob lines should be extracted, treated and
-# injected in the current diff file lines in order to present a "unfolded" diff.
-module Gitlab
- module Diff
- class LinesUnfolder
- include Gitlab::Utils::StrongMemoize
-
- UNFOLD_CONTEXT_SIZE = 3
-
- def initialize(diff_file, position)
- @diff_file = diff_file
- @blob = diff_file.old_blob
- @position = position
- @generate_top_match_line = true
- @generate_bottom_match_line = true
-
- # These methods update `@generate_top_match_line` and
- # `@generate_bottom_match_line`.
- @from_blob_line = calculate_from_blob_line!
- @to_blob_line = calculate_to_blob_line!
- end
-
- # Returns merged diff lines with required blob lines with correct
- # positions.
- def unfolded_diff_lines
- strong_memoize(:unfolded_diff_lines) do
- next unless unfold_required?
-
- merged_diff_with_blob_lines
- end
- end
-
- # Returns the extracted lines from the old blob which should be merged
- # with the current diff lines.
- def blob_lines
- strong_memoize(:blob_lines) do
- # Blob lines, unlike diffs, doesn't start with an empty space for
- # unchanged line, so the parsing and highlighting step can get fuzzy
- # without the following change.
- line_prefix = ' '
- blob_as_diff_lines = @blob.data.each_line.map { |line| "#{line_prefix}#{line}" }
-
- lines = Gitlab::Diff::Parser.new.parse(blob_as_diff_lines, diff_file: @diff_file).to_a
-
- from = from_blob_line - 1
- to = to_blob_line - 1
-
- lines[from..to]
- end
- end
-
- def unfold_required?
- strong_memoize(:unfold_required) do
- next false unless @diff_file.text?
- next false unless @position.unchanged?
- next false if @diff_file.new_file? || @diff_file.deleted_file?
- next false unless @position.old_line
- # Invalid position (MR import scenario)
- next false if @position.old_line > @blob.lines.size
- next false if @diff_file.diff_lines.empty?
- next false if @diff_file.line_for_position(@position)
- next false unless unfold_line
-
- true
- end
- end
-
- private
-
- attr_reader :from_blob_line, :to_blob_line
-
- def merged_diff_with_blob_lines
- lines = @diff_file.diff_lines
- match_line = unfold_line
- insert_index = bottom? ? -1 : match_line.index
-
- lines -= [match_line] unless bottom?
-
- lines.insert(insert_index, *blob_lines_with_matches)
-
- # The inserted blob lines have invalid indexes, so we need
- # to reindex them.
- reindex(lines)
-
- lines
- end
-
- # Returns 'unchanged' blob lines with recalculated `old_pos` and
- # `new_pos` and the recalculated new match line (needed if we for instance
- # we unfolded once, but there are still folded lines).
- def blob_lines_with_matches
- old_pos = from_blob_line
- new_pos = from_blob_line + offset
-
- new_blob_lines = []
-
- new_blob_lines.push(top_blob_match_line) if top_blob_match_line
-
- blob_lines.each do |line|
- new_blob_lines << Gitlab::Diff::Line.new(line.text, line.type, nil, old_pos, new_pos,
- parent_file: @diff_file)
-
- old_pos += 1
- new_pos += 1
- end
-
- new_blob_lines.push(bottom_blob_match_line) if bottom_blob_match_line
-
- new_blob_lines
- end
-
- def reindex(lines)
- lines.each_with_index { |line, i| line.index = i }
- end
-
- def top_blob_match_line
- strong_memoize(:top_blob_match_line) do
- next unless @generate_top_match_line
-
- old_pos = from_blob_line
- new_pos = from_blob_line + offset
-
- build_match_line(old_pos, new_pos)
- end
- end
-
- def bottom_blob_match_line
- strong_memoize(:bottom_blob_match_line) do
- # The bottom line match addition is already handled on
- # Diff::File#diff_lines_for_serializer
- next if bottom?
- next unless @generate_bottom_match_line
-
- position = line_after_unfold_position.old_pos
-
- old_pos = position
- new_pos = position + offset
-
- build_match_line(old_pos, new_pos)
- end
- end
-
- def build_match_line(old_pos, new_pos)
- blob_lines_length = blob_lines.length
- old_line_ref = [old_pos, blob_lines_length].join(',')
- new_line_ref = [new_pos, blob_lines_length].join(',')
- new_match_line_str = "@@ -#{old_line_ref}+#{new_line_ref} @@"
-
- Gitlab::Diff::Line.new(new_match_line_str, 'match', nil, old_pos, new_pos)
- end
-
- # Returns the first line position that should be extracted
- # from `blob_lines`.
- def calculate_from_blob_line!
- return unless unfold_required?
-
- from = comment_position - UNFOLD_CONTEXT_SIZE
-
- # There's no line before the match if it's in the top-most
- # position.
- prev_line_number = line_before_unfold_position&.old_pos || 0
-
- if from <= prev_line_number + 1
- @generate_top_match_line = false
- from = prev_line_number + 1
- end
-
- from
- end
-
- # Returns the last line position that should be extracted
- # from `blob_lines`.
- def calculate_to_blob_line!
- return unless unfold_required?
-
- to = comment_position + UNFOLD_CONTEXT_SIZE
-
- return to if bottom?
-
- next_line_number = line_after_unfold_position.old_pos
-
- if to >= next_line_number - 1
- @generate_bottom_match_line = false
- to = next_line_number - 1
- end
-
- to
- end
-
- def offset
- unfold_line.new_pos - unfold_line.old_pos
- end
-
- def line_before_unfold_position
- return unless index = unfold_line&.index
-
- @diff_file.diff_lines[index - 1] if index > 0
- end
-
- def line_after_unfold_position
- return unless index = unfold_line&.index
-
- @diff_file.diff_lines[index + 1] if index >= 0
- end
-
- def bottom?
- strong_memoize(:bottom) do
- @position.old_line > last_line.old_pos
- end
- end
-
- # Returns the line which needed to be expanded in order to send a comment
- # in `@position`.
- def unfold_line
- strong_memoize(:unfold_line) do
- next last_line if bottom?
-
- @diff_file.diff_lines.find do |line|
- line.old_pos > comment_position && line.type == 'match'
- end
- end
- end
-
- def comment_position
- @position.old_line
- end
-
- def last_line
- @diff_file.diff_lines.last
- end
- end
- end
-end
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index 7bfab2d808f..f967494199e 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -101,10 +101,6 @@ module Gitlab
@diff_refs ||= DiffRefs.new(base_sha: base_sha, start_sha: start_sha, head_sha: head_sha)
end
- def unfolded_diff?(repository)
- diff_file(repository)&.unfolded?
- end
-
def diff_file(repository)
return @diff_file if defined?(@diff_file)
@@ -138,13 +134,7 @@ module Gitlab
return unless diff_refs.complete?
return unless comparison = diff_refs.compare_in(repository.project)
- file = comparison.diffs(diff_options).diff_files.first
-
- # We need to unfold diff lines according to the position in order
- # to correctly calculate the line code and trace position changes.
- file&.unfold_diff_lines(self)
-
- file
+ comparison.diffs(diff_options).diff_files.first
end
def get_formatter_class(type)
diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb
index 5fdf7f1229d..28f7e4634a5 100644
--- a/spec/controllers/projects/blob_controller_spec.rb
+++ b/spec/controllers/projects/blob_controller_spec.rb
@@ -157,7 +157,7 @@ describe Projects::BlobController do
match_line = JSON.parse(response.body).first
- expect(match_line['type']).to be_nil
+ expect(match_line['type']).to eq('context')
end
it 'adds bottom match line when "t"o is less than blob size' do
@@ -177,7 +177,7 @@ describe Projects::BlobController do
match_line = JSON.parse(response.body).last
- expect(match_line['type']).to be_nil
+ expect(match_line['type']).to eq('context')
end
end
end
diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb
index 51b78d3e7d1..fa148715855 100644
--- a/spec/features/merge_request/user_posts_diff_notes_spec.rb
+++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb
@@ -85,13 +85,12 @@ describe 'Merge request > User posts diff notes', :js do
# `.line_holder` will be an unfolded line.
let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') }
- it 'allows commenting on the left side' do
- should_allow_commenting(line_holder, 'left')
+ it 'does not allow commenting on the left side' do
+ should_not_allow_commenting(line_holder, 'left')
end
- it 'allows commenting on the right side' do
- # Automatically shifts comment box to left side.
- should_allow_commenting(line_holder, 'right')
+ it 'does not allow commenting on the right side' do
+ should_not_allow_commenting(line_holder, 'right')
end
end
end
@@ -148,8 +147,8 @@ describe 'Merge request > User posts diff notes', :js do
# `.line_holder` will be an unfolded line.
let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') }
- it 'allows commenting' do
- should_allow_commenting line_holder
+ it 'does not allow commenting' do
+ should_not_allow_commenting line_holder
end
end
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 8821cde76f4..fed04cbaed8 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => {
it('should call utils.addContextLines with proper params', () => {
const options = {
lineNumbers: { oldLineNumber: 1, newLineNumber: 2 },
- contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }],
+ contextLines: [{ oldLine: 1 }],
fileHash: 'ff9200',
params: {
bottom: true,
@@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => {
parallelDiffLines: [],
};
const state = { diffFiles: [diffFile] };
- const lines = [{ oldLine: 1, newLine: 1 }];
+ const lines = [{ oldLine: 1 }];
const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile);
const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine');
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index 3417896e259..2f51642b58e 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -41,52 +41,6 @@ describe Gitlab::Diff::File do
end
end
- describe '#unfold_diff_lines' do
- let(:unfolded_lines) { double('expanded-lines') }
- let(:unfolder) { instance_double(Gitlab::Diff::LinesUnfolder) }
- let(:position) { instance_double(Gitlab::Diff::Position, old_line: 10) }
-
- before do
- allow(Gitlab::Diff::LinesUnfolder).to receive(:new) { unfolder }
- end
-
- context 'when unfold required' do
- before do
- allow(unfolder).to receive(:unfold_required?) { true }
- allow(unfolder).to receive(:unfolded_diff_lines) { unfolded_lines }
- end
-
- it 'changes @unfolded to true' do
- diff_file.unfold_diff_lines(position)
-
- expect(diff_file).to be_unfolded
- end
-
- it 'updates @diff_lines' do
- diff_file.unfold_diff_lines(position)
-
- expect(diff_file.diff_lines).to eq(unfolded_lines)
- end
- end
-
- context 'when unfold not required' do
- before do
- allow(unfolder).to receive(:unfold_required?) { false }
- end
-
- it 'keeps @unfolded false' do
- diff_file.unfold_diff_lines(position)
-
- expect(diff_file).not_to be_unfolded
- end
-
- it 'does not update @diff_lines' do
- expect { diff_file.unfold_diff_lines(position) }
- .not_to change(diff_file, :diff_lines)
- end
- end
- end
-
describe '#mode_changed?' do
it { expect(diff_file.mode_changed?).to be_falsey }
end
diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb
deleted file mode 100644
index 8e00c8e0e30..00000000000
--- a/spec/lib/gitlab/diff/lines_unfolder_spec.rb
+++ /dev/null
@@ -1,750 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Diff::LinesUnfolder do
- let(:raw_diff) do
- <<-DIFF.strip_heredoc
- @@ -7,9 +7,6 @@
- "tags": ["devel", "development", "nightly"],
- "desktop-file-name-prefix": "(Development) ",
- "finish-args": [
- - "--share=ipc", "--socket=x11",
- - "--socket=wayland",
- - "--talk-name=org.gnome.OnlineAccounts",
- "--talk-name=org.freedesktop.Tracker1",
- "--filesystem=home",
- "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*",
- @@ -62,7 +59,7 @@
- },
- {
- "name": "gnome-desktop",
- - "config-opts": ["--disable-debug-tools", "--disable-udev"],
- + "config-opts": ["--disable-debug-tools", "--disable-"],
- "sources": [
- {
- "type": "git",
- @@ -83,11 +80,6 @@
- "buildsystem": "meson",
- "builddir": true,
- "name": "nautilus",
- - "config-opts": [
- - "-Denable-desktop=false",
- - "-Denable-selinux=false",
- - "--libdir=/app/lib"
- - ],
- "sources": [
- {
- "type": "git",
- DIFF
- end
-
- let(:raw_old_blob) do
- <<-BLOB.strip_heredoc
- {
- "app-id": "org.gnome.Nautilus",
- "runtime": "org.gnome.Platform",
- "runtime-version": "master",
- "sdk": "org.gnome.Sdk",
- "command": "nautilus",
- "tags": ["devel", "development", "nightly"],
- "desktop-file-name-prefix": "(Development) ",
- "finish-args": [
- "--share=ipc", "--socket=x11",
- "--socket=wayland",
- "--talk-name=org.gnome.OnlineAccounts",
- "--talk-name=org.freedesktop.Tracker1",
- "--filesystem=home",
- "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*",
- "--filesystem=xdg-run/dconf", "--filesystem=~/.config/dconf:ro",
- "--talk-name=ca.desrt.dconf", "--env=DCONF_USER_CONFIG_DIR=.config/dconf"
- ],
- "cleanup": [ "/include", "/share/bash-completion" ],
- "modules": [
- {
- "name": "exiv2",
- "sources": [
- {
- "type": "archive",
- "url": "http://exiv2.org/builds/exiv2-0.26-trunk.tar.gz",
- "sha256": "c75e3c4a0811bf700d92c82319373b7a825a2331c12b8b37d41eb58e4f18eafb"
- },
- {
- "type": "shell",
- "commands": [
- "cp -f /usr/share/gnu-config/config.sub ./config/",
- "cp -f /usr/share/gnu-config/config.guess ./config/"
- ]
- }
- ]
- },
- {
- "name": "gexiv2",
- "config-opts": [ "--disable-introspection" ],
- "sources": [
- {
- "type": "git",
- "url": "https://git.gnome.org/browse/gexiv2"
- }
- ]
- },
- {
- "name": "tracker",
- "cleanup": [ "/bin", "/etc", "/libexec" ],
- "config-opts": [ "--disable-miner-apps", "--disable-static",
- "--disable-tracker-extract", "--disable-tracker-needle",
- "--disable-tracker-preferences", "--disable-artwork",
- "--disable-tracker-writeback", "--disable-miner-user-guides",
- "--with-bash-completion-dir=no" ],
- "sources": [
- {
- "type": "git",
- "url": "https://git.gnome.org/browse/tracker"
- }
- ]
- },
- {
- "name": "gnome-desktop",
- "config-opts": ["--disable-debug-tools", "--disable-udev"],
- "sources": [
- {
- "type": "git",
- "url": "https://git.gnome.org/browse/gnome-desktop"
- }
- ]
- },
- {
- "name": "gnome-autoar",
- "sources": [
- {
- "type": "git",
- "url": "https://git.gnome.org/browse/gnome-autoar"
- }
- ]
- },
- {
- "buildsystem": "meson",
- "builddir": true,
- "name": "nautilus",
- "config-opts": [
- "-Denable-desktop=false",
- "-Denable-selinux=false",
- "--libdir=/app/lib"
- ],
- "sources": [
- {
- "type": "git",
- "url": "https://gitlab.gnome.org/GNOME/nautilus.git"
- }
- ]
- }
- ]
- },
- {
- "app-id": "foo",
- "runtime": "foo",
- "runtime-version": "foo",
- "sdk": "foo",
- "command": "foo",
- "tags": ["foo", "bar", "kux"],
- "desktop-file-name-prefix": "(Foo) ",
- {
- "buildsystem": "meson",
- "builddir": true,
- "name": "nautilus",
- "sources": [
- {
- "type": "git",
- "url": "https://gitlab.gnome.org/GNOME/nautilus.git"
- }
- ]
- }
- },
- {
- "app-id": "foo",
- "runtime": "foo",
- "runtime-version": "foo",
- "sdk": "foo",
- "command": "foo",
- "tags": ["foo", "bar", "kux"],
- "desktop-file-name-prefix": "(Foo) ",
- {
- "buildsystem": "meson",
- "builddir": true,
- "name": "nautilus",
- "sources": [
- {
- "type": "git",
- "url": "https://gitlab.gnome.org/GNOME/nautilus.git"
- }
- ]
- }
- }
- BLOB
- end
-
- let(:project) { create(:project) }
-
- let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) }
-
- let(:diff) do
- Gitlab::Git::Diff.new(diff: raw_diff,
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- a_mode: "100644",
- b_mode: "100644",
- new_file: false,
- renamed_file: false,
- deleted_file: false,
- too_large: false)
- end
-
- let(:diff_file) do
- Gitlab::Diff::File.new(diff, repository: project.repository)
- end
-
- before do
- allow(old_blob).to receive(:load_all_data!)
- allow(diff_file).to receive(:old_blob) { old_blob }
- end
-
- subject { described_class.new(diff_file, position) }
-
- context 'position requires a middle expansion and new match lines' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 43,
- new_line: 40)
- end
-
- context 'blob lines' do
- let(:expected_blob_lines) do
- [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"],
- [41, 41, " \"sources\": ["],
- [42, 42, " {"],
- [43, 43, " \"type\": \"git\","],
- [44, 44, " \"url\": \"https://git.gnome.org/browse/gexiv2\""],
- [45, 45, " }"],
- [46, 46, " ]"]]
- end
-
- it 'returns the extracted blob lines correctly' do
- extracted_lines = subject.blob_lines
-
- expect(extracted_lines.size).to eq(7)
-
- extracted_lines.each_with_index do |line, i|
- expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
- end
- end
- end
-
- context 'diff lines' do
- let(:expected_diff_lines) do
- [[7, 7, "@@ -7,9 +7,6 @@"],
- [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"],
- [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","],
- [9, 9, " \"finish-args\": ["],
- [10, 10, "- \"--share=ipc\", \"--socket=x11\","],
- [11, 10, "- \"--socket=wayland\","],
- [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","],
- [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","],
- [14, 11, " \"--filesystem=home\","],
- [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
-
- # New match line
- [40, 37, "@@ -40,7+37,7 @@"],
-
- # Injected blob lines
- [40, 37, " \"config-opts\": [ \"--disable-introspection\" ],"],
- [41, 38, " \"sources\": ["],
- [42, 39, " {"],
- [43, 40, " \"type\": \"git\","], # comment
- [44, 41, " \"url\": \"https://git.gnome.org/browse/gexiv2\""],
- [45, 42, " }"],
- [46, 43, " ]"],
- # end
-
- # Second match line
- [62, 59, "@@ -62,7+59,7 @@"],
-
- [62, 59, " },"],
- [63, 60, " {"],
- [64, 61, " \"name\": \"gnome-desktop\","],
- [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
- [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
- [66, 63, " \"sources\": ["],
- [67, 64, " {"],
- [68, 65, " \"type\": \"git\","],
- [83, 80, "@@ -83,11 +80,6 @@"],
- [83, 80, " \"buildsystem\": \"meson\","],
- [84, 81, " \"builddir\": true,"],
- [85, 82, " \"name\": \"nautilus\","],
- [86, 83, "- \"config-opts\": ["],
- [87, 83, "- \"-Denable-desktop=false\","],
- [88, 83, "- \"-Denable-selinux=false\","],
- [89, 83, "- \"--libdir=/app/lib\""],
- [90, 83, "- ],"],
- [91, 83, " \"sources\": ["],
- [92, 84, " {"],
- [93, 85, " \"type\": \"git\","]]
- end
-
- it 'return merge of blob lines with diff lines correctly' do
- new_diff_lines = subject.unfolded_diff_lines
-
- expected_diff_lines.each_with_index do |expected_line, i|
- line = new_diff_lines[i]
-
- expect([line.old_pos, line.new_pos, line.text])
- .to eq([expected_line[0], expected_line[1], expected_line[2]])
- end
- end
-
- it 'merged lines have correct line codes' do
- new_diff_lines = subject.unfolded_diff_lines
-
- new_diff_lines.each_with_index do |line, i|
- old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
-
- unless line.type == 'match'
- expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
- end
- end
- end
- end
- end
-
- context 'position requires a middle expansion and no top match line' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 16,
- new_line: 17)
- end
-
- context 'blob lines' do
- let(:expected_blob_lines) do
- [[16, 16, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","],
- [17, 17, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""],
- [18, 18, " ],"],
- [19, 19, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"]]
- end
-
- it 'returns the extracted blob lines correctly' do
- extracted_lines = subject.blob_lines
-
- expect(extracted_lines.size).to eq(4)
-
- extracted_lines.each_with_index do |line, i|
- expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
- end
- end
- end
-
- context 'diff lines' do
- let(:expected_diff_lines) do
- [[7, 7, "@@ -7,9 +7,6 @@"],
- [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"],
- [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","],
- [9, 9, " \"finish-args\": ["],
- [10, 10, "- \"--share=ipc\", \"--socket=x11\","],
- [11, 10, "- \"--socket=wayland\","],
- [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","],
- [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","],
- [14, 11, " \"--filesystem=home\","],
- [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
- # No new match needed
-
- # Injected blob lines
- [16, 13, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","],
- [17, 14, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""],
- [18, 15, " ],"],
- [19, 16, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"],
- # end
-
- # Second match line
- [62, 59, "@@ -62,4+59,4 @@"],
-
- [62, 59, " },"],
- [63, 60, " {"],
- [64, 61, " \"name\": \"gnome-desktop\","],
- [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
- [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
- [66, 63, " \"sources\": ["],
- [67, 64, " {"],
- [68, 65, " \"type\": \"git\","],
- [83, 80, "@@ -83,11 +80,6 @@"],
- [83, 80, " \"buildsystem\": \"meson\","],
- [84, 81, " \"builddir\": true,"],
- [85, 82, " \"name\": \"nautilus\","],
- [86, 83, "- \"config-opts\": ["],
- [87, 83, "- \"-Denable-desktop=false\","],
- [88, 83, "- \"-Denable-selinux=false\","],
- [89, 83, "- \"--libdir=/app/lib\""],
- [90, 83, "- ],"],
- [91, 83, " \"sources\": ["],
- [92, 84, " {"],
- [93, 85, " \"type\": \"git\","]]
- end
-
- it 'return merge of blob lines with diff lines correctly' do
- new_diff_lines = subject.unfolded_diff_lines
-
- expected_diff_lines.each_with_index do |expected_line, i|
- line = new_diff_lines[i]
-
- expect([line.old_pos, line.new_pos, line.text])
- .to eq([expected_line[0], expected_line[1], expected_line[2]])
- end
- end
-
- it 'merged lines have correct line codes' do
- new_diff_lines = subject.unfolded_diff_lines
-
- new_diff_lines.each_with_index do |line, i|
- old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
-
- unless line.type == 'match'
- expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
- end
- end
- end
- end
- end
-
- context 'position requires a middle expansion and no bottom match line' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 82,
- new_line: 79)
- end
-
- context 'blob lines' do
- let(:expected_blob_lines) do
- [[79, 79, " }"],
- [80, 80, " ]"],
- [81, 81, " },"],
- [82, 82, " {"]]
- end
-
- it 'returns the extracted blob lines correctly' do
- extracted_lines = subject.blob_lines
-
- expect(extracted_lines.size).to eq(4)
-
- extracted_lines.each_with_index do |line, i|
- expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
- end
- end
- end
-
- context 'diff lines' do
- let(:expected_diff_lines) do
- [[7, 7, "@@ -7,9 +7,6 @@"],
- [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"],
- [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","],
- [9, 9, " \"finish-args\": ["],
- [10, 10, "- \"--share=ipc\", \"--socket=x11\","],
- [11, 10, "- \"--socket=wayland\","],
- [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","],
- [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","],
- [14, 11, " \"--filesystem=home\","],
- [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
- [62, 59, "@@ -62,7 +59,7 @@"],
- [62, 59, " },"],
- [63, 60, " {"],
- [64, 61, " \"name\": \"gnome-desktop\","],
- [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
- [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
- [66, 63, " \"sources\": ["],
- [67, 64, " {"],
- [68, 65, " \"type\": \"git\","],
-
- # New top match line
- [79, 76, "@@ -79,4+76,4 @@"],
-
- # Injected blob lines
- [79, 76, " }"],
- [80, 77, " ]"],
- [81, 78, " },"],
- [82, 79, " {"],
- # end
-
- # No new second match line
- [83, 80, " \"buildsystem\": \"meson\","],
- [84, 81, " \"builddir\": true,"],
- [85, 82, " \"name\": \"nautilus\","],
- [86, 83, "- \"config-opts\": ["],
- [87, 83, "- \"-Denable-desktop=false\","],
- [88, 83, "- \"-Denable-selinux=false\","],
- [89, 83, "- \"--libdir=/app/lib\""],
- [90, 83, "- ],"],
- [91, 83, " \"sources\": ["],
- [92, 84, " {"],
- [93, 85, " \"type\": \"git\","]]
- end
-
- it 'return merge of blob lines with diff lines correctly' do
- new_diff_lines = subject.unfolded_diff_lines
-
- expected_diff_lines.each_with_index do |expected_line, i|
- line = new_diff_lines[i]
-
- expect([line.old_pos, line.new_pos, line.text])
- .to eq([expected_line[0], expected_line[1], expected_line[2]])
- end
- end
-
- it 'merged lines have correct line codes' do
- new_diff_lines = subject.unfolded_diff_lines
-
- new_diff_lines.each_with_index do |line, i|
- old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
-
- unless line.type == 'match'
- expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
- end
- end
- end
- end
- end
-
- context 'position requires a short top expansion' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 6,
- new_line: 6)
- end
-
- context 'blob lines' do
- let(:expected_blob_lines) do
- [[3, 3, " \"runtime\": \"org.gnome.Platform\","],
- [4, 4, " \"runtime-version\": \"master\","],
- [5, 5, " \"sdk\": \"org.gnome.Sdk\","],
- [6, 6, " \"command\": \"nautilus\","]]
- end
-
- it 'returns the extracted blob lines correctly' do
- extracted_lines = subject.blob_lines
-
- expect(extracted_lines.size).to eq(4)
-
- extracted_lines.each_with_index do |line, i|
- expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
- end
- end
- end
-
- context 'diff lines' do
- let(:expected_diff_lines) do
- # New match line
- [[3, 3, "@@ -3,4+3,4 @@"],
-
- # Injected blob lines
- [3, 3, " \"runtime\": \"org.gnome.Platform\","],
- [4, 4, " \"runtime-version\": \"master\","],
- [5, 5, " \"sdk\": \"org.gnome.Sdk\","],
- [6, 6, " \"command\": \"nautilus\","],
- # end
- [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"],
- [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","],
- [9, 9, " \"finish-args\": ["],
- [10, 10, "- \"--share=ipc\", \"--socket=x11\","],
- [11, 10, "- \"--socket=wayland\","],
- [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","],
- [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","],
- [14, 11, " \"--filesystem=home\","],
- [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
- [62, 59, "@@ -62,7 +59,7 @@"],
- [62, 59, " },"],
- [63, 60, " {"],
- [64, 61, " \"name\": \"gnome-desktop\","],
- [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
- [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
- [66, 63, " \"sources\": ["],
- [67, 64, " {"],
- [68, 65, " \"type\": \"git\","],
- [83, 80, "@@ -83,11 +80,6 @@"],
- [83, 80, " \"buildsystem\": \"meson\","],
- [84, 81, " \"builddir\": true,"],
- [85, 82, " \"name\": \"nautilus\","],
- [86, 83, "- \"config-opts\": ["],
- [87, 83, "- \"-Denable-desktop=false\","],
- [88, 83, "- \"-Denable-selinux=false\","],
- [89, 83, "- \"--libdir=/app/lib\""],
- [90, 83, "- ],"],
- [91, 83, " \"sources\": ["],
- [92, 84, " {"],
- [93, 85, " \"type\": \"git\","]]
- end
-
- it 'return merge of blob lines with diff lines correctly' do
- new_diff_lines = subject.unfolded_diff_lines
-
- expected_diff_lines.each_with_index do |expected_line, i|
- line = new_diff_lines[i]
-
- expect([line.old_pos, line.new_pos, line.text])
- .to eq([expected_line[0], expected_line[1], expected_line[2]])
- end
- end
-
- it 'merged lines have correct line codes' do
- new_diff_lines = subject.unfolded_diff_lines
-
- new_diff_lines.each_with_index do |line, i|
- old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
-
- unless line.type == 'match'
- expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
- end
- end
- end
- end
- end
-
- context 'position sits between two match lines (no expasion needed)' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 64,
- new_line: 61)
- end
-
- context 'diff lines' do
- it 'returns nil' do
- expect(subject.unfolded_diff_lines).to be_nil
- end
- end
- end
-
- context 'position requires bottom expansion and new match lines' do
- let(:position) do
- Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19",
- head_sha: "1487062132228de836236c522fe52fed4980a46c",
- old_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- new_path: "build-aux/flatpak/org.gnome.Nautilus.json",
- position_type: "text",
- old_line: 107,
- new_line: 99)
- end
-
- context 'blob lines' do
- let(:expected_blob_lines) do
- [[104, 104, " \"sdk\": \"foo\","],
- [105, 105, " \"command\": \"foo\","],
- [106, 106, " \"tags\": [\"foo\", \"bar\", \"kux\"],"],
- [107, 107, " \"desktop-file-name-prefix\": \"(Foo) \","],
- [108, 108, " {"],
- [109, 109, " \"buildsystem\": \"meson\","],
- [110, 110, " \"builddir\": true,"]]
- end
-
- it 'returns the extracted blob lines correctly' do
- extracted_lines = subject.blob_lines
-
- expect(extracted_lines.size).to eq(7)
-
- extracted_lines.each_with_index do |line, i|
- expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i])
- end
- end
- end
-
- context 'diff lines' do
- let(:expected_diff_lines) do
- [[7, 7, "@@ -7,9 +7,6 @@"],
- [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"],
- [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","],
- [9, 9, " \"finish-args\": ["],
- [10, 10, "- \"--share=ipc\", \"--socket=x11\","],
- [11, 10, "- \"--socket=wayland\","],
- [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","],
- [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","],
- [14, 11, " \"--filesystem=home\","],
- [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","],
- [62, 59, "@@ -62,7 +59,7 @@"],
- [62, 59, " },"],
- [63, 60, " {"],
- [64, 61, " \"name\": \"gnome-desktop\","],
- [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"],
- [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"],
- [66, 63, " \"sources\": ["],
- [67, 64, " {"],
- [68, 65, " \"type\": \"git\","],
- [83, 80, "@@ -83,11 +80,6 @@"],
- [83, 80, " \"buildsystem\": \"meson\","],
- [84, 81, " \"builddir\": true,"],
- [85, 82, " \"name\": \"nautilus\","],
- [86, 83, "- \"config-opts\": ["],
- [87, 83, "- \"-Denable-desktop=false\","],
- [88, 83, "- \"-Denable-selinux=false\","],
- [89, 83, "- \"--libdir=/app/lib\""],
- [90, 83, "- ],"],
- [91, 83, " \"sources\": ["],
- [92, 84, " {"],
- [93, 85, " \"type\": \"git\","],
- # New match line
- [104, 96, "@@ -104,7+96,7 @@"],
-
- # Injected blob lines
- [104, 96, " \"sdk\": \"foo\","],
- [105, 97, " \"command\": \"foo\","],
- [106, 98, " \"tags\": [\"foo\", \"bar\", \"kux\"],"],
- [107, 99, " \"desktop-file-name-prefix\": \"(Foo) \","],
- [108, 100, " {"],
- [109, 101, " \"buildsystem\": \"meson\","],
- [110, 102, " \"builddir\": true,"]]
- # end
- end
-
- it 'return merge of blob lines with diff lines correctly' do
- new_diff_lines = subject.unfolded_diff_lines
-
- expected_diff_lines.each_with_index do |expected_line, i|
- line = new_diff_lines[i]
-
- expect([line.old_pos, line.new_pos, line.text])
- .to eq([expected_line[0], expected_line[1], expected_line[2]])
- end
- end
-
- it 'merged lines have correct line codes' do
- new_diff_lines = subject.unfolded_diff_lines
-
- new_diff_lines.each_with_index do |line, i|
- old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1]
-
- unless line.type == 'match'
- expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos))
- end
- end
- end
- end
- end
-end
diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index 5acd01828cb..546c9f277c5 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/spec/services/merge_requests/reload_diffs_service_spec.rb
@@ -31,11 +31,32 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
end
context 'cache clearing' do
+ before do
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true)
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true)
+ end
+
+ it 'retrieves the diff files to cache the highlighted result' do
+ new_diff = merge_request.create_merge_request_diff
+ cache_key = new_diff.diffs_collection.cache_key
+
+ expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff)
+ expect(Rails.cache).to receive(:read).with(cache_key).and_call_original
+ expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original
+
+ subject.execute
+ end
+
it 'clears the cache for older diffs on the merge request' do
old_diff = merge_request.merge_request_diff
old_cache_key = old_diff.diffs_collection.cache_key
+ new_diff = merge_request.create_merge_request_diff
+ new_cache_key = new_diff.diffs_collection.cache_key
+ expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff)
expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original
+ expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original
+ expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original
subject.execute
end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 80b015d4cd0..b1290fd0d47 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -57,57 +57,6 @@ describe Notes::CreateService do
end
end
- context 'noteable highlight cache clearing' do
- let(:project_with_repo) { create(:project, :repository) }
- let(:merge_request) do
- create(:merge_request, source_project: project_with_repo,
- target_project: project_with_repo)
- end
-
- let(:position) do
- Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
- new_path: "files/ruby/popen.rb",
- old_line: nil,
- new_line: 14,
- diff_refs: merge_request.diff_refs)
- end
-
- let(:new_opts) do
- opts.merge(in_reply_to_discussion_id: nil,
- type: 'DiffNote',
- noteable_type: 'MergeRequest',
- noteable_id: merge_request.id,
- position: position.to_h)
- end
-
- before do
- allow_any_instance_of(Gitlab::Diff::Position)
- .to receive(:unfolded_diff?) { true }
- end
-
- it 'clears noteable diff cache when it was unfolded for the note position' do
- expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear)
-
- described_class.new(project_with_repo, user, new_opts).execute
- end
-
- it 'does not clear cache when note is not the first of the discussion' do
- prev_note =
- create(:diff_note_on_merge_request, noteable: merge_request,
- project: project_with_repo)
- reply_opts =
- opts.merge(in_reply_to_discussion_id: prev_note.discussion_id,
- type: 'DiffNote',
- noteable_type: 'MergeRequest',
- noteable_id: merge_request.id,
- position: position.to_h)
-
- expect(merge_request).not_to receive(:diffs)
-
- described_class.new(project_with_repo, user, reply_opts).execute
- end
- end
-
context 'note diff file' do
let(:project_with_repo) { create(:project, :repository) }
let(:merge_request) do
diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb
index b1f4e87e8ea..64445be560e 100644
--- a/spec/services/notes/destroy_service_spec.rb
+++ b/spec/services/notes/destroy_service_spec.rb
@@ -21,38 +21,5 @@ describe Notes::DestroyService do
expect { described_class.new(project, user).execute(note) }
.to change { user.todos_pending_count }.from(1).to(0)
end
-
- context 'noteable highlight cache clearing' do
- let(:repo_project) { create(:project, :repository) }
- let(:merge_request) do
- create(:merge_request, source_project: repo_project,
- target_project: repo_project)
- end
-
- let(:note) do
- create(:diff_note_on_merge_request, project: repo_project,
- noteable: merge_request)
- end
-
- before do
- allow(note.position).to receive(:unfolded_diff?) { true }
- end
-
- it 'clears noteable diff cache when it was unfolded for the note position' do
- expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
-
- described_class.new(repo_project, user).execute(note)
- end
-
- it 'does not clear cache when note is not the first of the discussion' do
- reply_note = create(:diff_note_on_merge_request, in_reply_to: note,
- project: repo_project,
- noteable: merge_request)
-
- expect(merge_request).not_to receive(:diffs)
-
- described_class.new(repo_project, user).execute(reply_note)
- end
- end
end
end