Comment on any expanded diff line on MRs

3 files changed, 84 insertions, 21 deletions

diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index 21f369a3818..3cd579b57ce 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/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
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index b1290fd0d47..80b015d4cd0 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/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
diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb
index 64445be560e..b1f4e87e8ea 100644
--- a/spec/services/notes/destroy_service_spec.rb
+++ b/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