Merge branch 'osw-comment-on-any-line-on-diffs-w-feature-flag-ee' into 'master'

[EE] Comment on any expanded diff line on MRs (with feature-flag)

See merge request gitlab-org/gitlab-ee!8362

app/assets/javascripts/diffs/store/mutations.js

@@ -65,7 +65,13 @@ export default {
     const { highlightedDiffLines, parallelDiffLines } = diffFile;
 
     removeMatchLine(diffFile, lineNumbers, bottom);
-    const lines = addLineReferences(contextLines, lineNumbers, bottom);
+
+    const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({
+      ...line,
+      lineCode: line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`,
+      discussions: line.discussions || [],
+    }));
+
     addContextLines({
       inlineLines: highlightedDiffLines,
       parallelLines: parallelDiffLines,

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, 'context', nil, nil, nil)
+      diff_line = Gitlab::Diff::Line.new(line, expanded_diff_line_type, nil, nil, nil)
       diff_line.rich_text = line
       diff_line
     end
@@ -132,6 +132,11 @@ class Projects::BlobController < Projects::ApplicationController
     render json: DiffLineSerializer.new.represent(@lines)
   end
 
+  def expanded_diff_line_type
+    # Context lines can't receive comments.
+    Feature.enabled?(:comment_in_any_diff_line, @project) ? nil : 'context'
+  end
+
   def add_match_line
     return unless @form.unfold?
 

app/controllers/projects/merge_requests/diffs_controller.rb

 # frozen_string_literal: true
 
 class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController
+  prepend ::EE::Projects::MergeRequests::DiffsController
+
   include DiffForPath
   include DiffHelper
   include RendersNotes
@@ -22,6 +24,12 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
 
   def render_diffs
     @environment = @merge_request.environments_for(current_user).last
+    notes_grouped_by_path = renderable_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
 
@@ -108,4 +116,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@compare.diff_refs)
     @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes), @merge_request)
   end
+
+  def renderable_notes
+    define_diff_comment_vars unless @notes
+
+    @notes
+  end
 end

app/models/diff_note.rb

@@ -66,6 +66,10 @@ 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
@@ -78,10 +82,11 @@ class DiffNote < Note
   end
 
   def should_create_diff_file?
-    on_text? && note_diff_file.nil? && self == discussion.first_note
+    on_text? && note_diff_file.nil? && 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,
@@ -98,6 +103,12 @@ class DiffNote < Note
       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
   end
 
   def supported?

app/services/merge_requests/reload_diffs_service.rb

@@ -29,10 +29,6 @@ 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.

app/services/notes/base_service.rb

+# frozen_string_literal: true
+
+module Notes
+  class BaseService < ::BaseService
+    def clear_noteable_diffs_cache(note)
+      if note.is_a?(DiffNote) &&
+          note.discussion_first_note? &&
+          note.position.unfolded_diff?(project.repository)
+        note.noteable.diffs.clear_cache
+      end
+    end
+  end
+end

app/services/notes/create_service.rb

 # frozen_string_literal: true
 
 module Notes
-  class CreateService < ::BaseService
+  class CreateService < ::Notes::BaseService
     def execute
       merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha)
 
@@ -35,6 +35,7 @@ module Notes
 
       if !only_commands && note.save
         todo_service.new_note(note, current_user)
+        clear_noteable_diffs_cache(note)
       end
 
       if command_params.present?

app/services/notes/destroy_service.rb

 # frozen_string_literal: true
 
 module Notes
-  class DestroyService < BaseService
+  class DestroyService < ::Notes::BaseService
     def execute(note)
       TodoService.new.destroy_target(note) do |note|
         note.destroy
       end
+
+      clear_noteable_diffs_cache(note)
     end
   end
 end

changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml

+---
+title: Allow commenting on any diff line in Merge Requests
+merge_request: 22914
+author:
+type: added

ee/app/controllers/ee/projects/merge_requests/diffs_controller.rb

+# frozen_string_literal: true
+
+module EE
+  module Projects
+    module MergeRequests
+      module DiffsController
+        extend ActiveSupport::Concern
+
+        def renderable_notes
+          draft_notes =
+            if current_user
+              merge_request.draft_notes.authored_by(current_user)
+            else
+              []
+            end
+
+          super.concat(draft_notes)
+        end
+      end
+    end
+  end
+end

ee/app/controllers/projects/merge_requests/drafts_controller.rb

@@ -34,7 +34,7 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli
   end
 
   def destroy
-    draft_note.destroy!
+    DraftNotes::DestroyService.new(merge_request, current_user).execute(draft_note)
 
     head :ok
   end
@@ -46,7 +46,7 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli
   end
 
   def discard
-    draft_notes.delete_all
+    DraftNotes::DestroyService.new(merge_request, current_user).execute
 
     head :ok
   end

ee/app/models/draft_note.rb

@@ -71,7 +71,7 @@ class DraftNote < ActiveRecord::Base
   end
 
   def line_code
-    @line_code ||= original_position&.line_code(project.repository)
+    @line_code ||= diff_file&.line_code_for_position(original_position)
   end
 
   def file_hash
@@ -99,11 +99,13 @@ class DraftNote < ActiveRecord::Base
     ActiveRecord::Associations::Preloader.new.preload(draft_notes, { author: :status })
   end
 
-  private
-
   def diff_file
     strong_memoize(:diff_file) do
-      original_position&.diff_file(project.repository)
+      file = original_position&.diff_file(project.repository)
+
+      file&.unfold_diff_lines(original_position)
+
+      file
     end
   end
 end

ee/app/services/draft_notes/base_service.rb

@@ -10,6 +10,10 @@ module DraftNotes
 
     private
 
+    def draft_notes
+      @draft_notes ||= merge_request.draft_notes.authored_by(current_user)
+    end
+
     def project
       merge_request.target_project
     end

ee/app/services/draft_notes/create_service.rb

@@ -27,6 +27,10 @@ module DraftNotes
       draft_note.author = current_user
       draft_note.save
 
+      if in_reply_to_discussion_id.blank? && draft_note.diff_file&.unfolded?
+        merge_request.diffs.clear_cache
+      end
+
       draft_note
     end
 

ee/app/services/draft_notes/destroy_service.rb

+# frozen_string_literal: true
+
+module DraftNotes
+  class DestroyService < DraftNotes::BaseService
+    # If no `draft` is given it fallsback to all
+    # draft notes of the given merge request and user.
+    def execute(draft = nil)
+      drafts = draft || draft_notes
+
+      clear_highlight_diffs_cache(Array.wrap(drafts))
+
+      drafts.is_a?(DraftNote) ? drafts.destroy! : drafts.delete_all
+    end
+
+    private
+
+    def clear_highlight_diffs_cache(drafts)
+      if drafts.any? { |draft| draft.diff_file&.unfolded? }
+        merge_request.diffs.clear_cache
+      end
+    end
+  end
+end

ee/app/services/draft_notes/publish_service.rb

@@ -29,6 +29,10 @@ module DraftNotes
     end
 
     def create_note_from_draft(draft)
+      # Make sure the diff file is unfolded in order to find the correct line
+      # codes.
+      draft.diff_file&.unfold_diff_lines(draft.original_position)
+
       note = Notes::CreateService.new(draft.project, draft.author, draft.publish_params).execute
       set_discussion_resolve_status(note, draft)
     end
@@ -44,9 +48,5 @@ module DraftNotes
         discussion.unresolve!
       end
     end
-
-    def draft_notes
-      @draft_notes ||= merge_request.draft_notes.authored_by(current_user)
-    end
   end
 end

ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb

@@ -294,7 +294,23 @@ describe Projects::MergeRequests::DraftsController do
       create(:draft_note, merge_request: merge_request, author: user)
     end
 
-    it 'destroys the draft note' do
+    context 'without permissions' do
+      before do
+        sign_in(user2)
+        project.add_developer(user2)
+      end
+
+      it 'does not allow destroying a draft note belonging to someone else' do
+        draft = create(:draft_note, merge_request: merge_request, author: user)
+
+        expect { post :destroy, params.merge(id: draft.id) }
+          .not_to change { DraftNote.count }
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    it 'destroys the draft note when ID is given' do
       draft = create_draft
 
       expect { delete :destroy, params.merge(id: draft.id) }.to change { DraftNote.count }.by(-1)
@@ -322,6 +338,22 @@ describe Projects::MergeRequests::DraftsController do
       expect { delete :discard, params }.to change { DraftNote.count }.by(-6)
       expect(response).to have_gitlab_http_status(200)
     end
+
+    context 'without permissions' do
+      before do
+        sign_in(user2)
+        project.add_developer(user2)
+      end
+
+      it 'does not destroys a draft note belonging to someone else' do
+        create(:draft_note, merge_request: merge_request, author: user)
+
+        expect { post :discard, params }
+          .not_to change { DraftNote.count }
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
   end
 
   shared_examples_for 'batch comments feature disabled' do

ee/spec/services/draft_notes/create_service_spec.rb

@@ -65,4 +65,30 @@ describe DraftNotes::CreateService do
     expect(draft.note).to eq('Comment on diff')
     expect(draft.original_position.to_json).to eq(position.to_json)
   end
+
+  context 'diff highlight cache clearing' do
+    context 'when diff file is unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { true }
+        end
+
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+
+    context 'when diff file is not unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { false }
+        end
+
+        expect(merge_request).not_to receive(:diffs)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+  end
 end

ee/spec/services/draft_notes/destroy_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::DestroyService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def destroy(draft_note = nil)
+    DraftNotes::DestroyService.new(merge_request, user).execute(draft_note)
+  end
+
+  it 'destroys a single draft note' do
+    drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy(drafts.first) }
+      .to change { DraftNote.count }.by(-1)
+
+    expect(DraftNote.count).to eq(1)
+  end
+
+  it 'destroys all draft notes for a user in a merge request' do
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy }.to change { DraftNote.count }.by(-2)
+    expect(DraftNote.count).to eq(0)
+  end
+
+  context 'diff highlight cache clearing' do
+    context 'when destroying all draft notes of a user' do
+      it 'clears highlighting cache if unfold required for any' do
+        drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy(drafts.first)
+      end
+    end
+
+    context 'when destroying one draft note' do
+      it 'clears highlighting cache if unfold required' do
+        create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy
+      end
+    end
+  end
+end

lib/gitlab/diff/file.rb

@@ -28,6 +28,7 @@ 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
@@ -137,6 +138,24 @@ 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

lib/gitlab/diff/line.rb

@@ -5,9 +5,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, :index, :old_pos, :new_pos
+      attr_reader :line_code, :type, :old_pos, :new_pos
       attr_writer :rich_text
-      attr_accessor :text
+      attr_accessor :text, :index
 
       def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil)
         @text, @type, @index = text, type, index
@@ -21,7 +21,14 @@ module Gitlab
       end
 
       def self.init_from_hash(hash)
-        new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code], rich_text: hash[:rich_text])
+        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])
       end
 
       def to_hash

lib/gitlab/diff/lines_unfolder.rb

+# 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

lib/gitlab/diff/position.rb

@@ -103,6 +103,10 @@ 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)
 
@@ -136,7 +140,13 @@ module Gitlab
         return unless diff_refs.complete?
         return unless comparison = diff_refs.compare_in(repository.project)
 
-        comparison.diffs(diff_options).diff_files.first
+        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
       end
 
       def get_formatter_class(type)

spec/controllers/projects/blob_controller_spec.rb

@@ -141,6 +141,28 @@ describe Projects::BlobController do
           expect(lines.first).to have_key('rich_text')
         end
 
+        context 'comment in any diff line feature flag' do
+          it 'renders context lines when feature disabled' do
+            stub_feature_flags(comment_in_any_diff_line: false)
+
+            do_get(since: 1, to: 5, offset: 10, from_merge_request: true)
+            lines = JSON.parse(response.body)
+            all_context = lines.all? { |line| line['type'] == 'context' }
+
+            expect(all_context).to be(true)
+          end
+
+          it 'renders unchanged lines when feature enabled' do
+            stub_feature_flags(comment_in_any_diff_line: true)
+
+            do_get(since: 1, to: 5, offset: 10, from_merge_request: true)
+            lines = JSON.parse(response.body)
+            all_unchanged = lines.all? { |line| line['type'].nil? }
+
+            expect(all_unchanged).to be(true)
+          end
+        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)
@@ -157,7 +179,7 @@ describe Projects::BlobController do
 
             match_line = JSON.parse(response.body).first
 
-            expect(match_line['type']).to eq('context')
+            expect(match_line['type']).to be_nil
           end
 
           it 'adds bottom match line when "t"o is less than blob size' do
@@ -177,7 +199,7 @@ describe Projects::BlobController do
 
             match_line = JSON.parse(response.body).last
 
-            expect(match_line['type']).to eq('context')
+            expect(match_line['type']).to be_nil
           end
         end
       end

spec/features/merge_request/user_posts_diff_notes_spec.rb

@@ -85,12 +85,13 @@ describe 'Merge request > User posts diff notes', :js do
       # `.line_holder` will be an unfolded line.
       let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') }
 
-      it 'does not allow commenting on the left side' do
-        should_not_allow_commenting(line_holder, 'left')
+      it 'allows commenting on the left side' do
+        should_allow_commenting(line_holder, 'left')
       end
 
-      it 'does not allow commenting on the right side' do
-        should_not_allow_commenting(line_holder, 'right')
+      it 'allows commenting on the right side' do
+        # Automatically shifts comment box to left side.
+        should_allow_commenting(line_holder, 'right')
       end
     end
   end
@@ -147,8 +148,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 'does not allow commenting' do
-        should_not_allow_commenting line_holder
+      it 'allows commenting' do
+        should_allow_commenting line_holder
       end
     end
 

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 }],
+        contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }],
         fileHash: 'ff9200',
         params: {
           bottom: true,
@@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => {
         parallelDiffLines: [],
       };
       const state = { diffFiles: [diffFile] };
-      const lines = [{ oldLine: 1 }];
+      const lines = [{ oldLine: 1, newLine: 1 }];
 
       const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile);
       const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine');

spec/lib/gitlab/diff/file_spec.rb

@@ -41,6 +41,52 @@ 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

spec/lib/gitlab/diff/lines_unfolder_spec.rb

+# 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

spec/services/merge_requests/reload_diffs_service_spec.rb

@@ -31,32 +31,11 @@ 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

spec/services/notes/create_service_spec.rb

@@ -57,6 +57,57 @@ 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

spec/services/notes/destroy_service_spec.rb

@@ -21,5 +21,38 @@ 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