Fix specs

app/assets/javascripts/notes.js

@@ -769,6 +769,10 @@ require('./task_list');
       form.find('.js-note-new-discussion').remove();
       this.setupNoteForm(form);
 
+      form
+        .removeClass('js-main-target-form')
+        .addClass("discussion-form js-discussion-note-form");
+
       if (typeof gl.diffNotesCompileComponents !== 'undefined') {
         var $commentBtn = form.find('comment-and-resolve-btn');
         $commentBtn.attr(':discussion-id', "'" + discussionID + "'");
@@ -780,9 +784,6 @@ require('./task_list');
       form
         .find('.js-comment-resolve-button')
         .attr('data-discussion-id', discussionID);
-      form
-        .removeClass('js-main-target-form')
-        .addClass("discussion-form js-discussion-note-form");
     };
 
     /*

app/finders/notes_finder.rb

@@ -98,9 +98,9 @@ class NotesFinder
   #
   # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
   #
-  def search(query, notes)
+  def search(notes)
     query = @params[:search]
-    return unless query
+    return notes unless query
 
     pattern = "%#{query}%"
     notes.where(Note.arel_table[:note].matches(pattern))

app/views/notify/_note_email.html.haml

-- discussion = @note.discussion unless @discussion.individual_note?
+- discussion = @note.discussion if @note.part_of_discussion?
 - if discussion
   %p.details
     = succeed ':' do

app/views/notify/_note_email.text.erb

-<%  discussion = @note.discussion unless @discussion.individual_note? -%>
+<%  discussion = @note.discussion if @note.part_of_discussion? -%>
 <%  if discussion && !discussion.individual_note? -%>
 <%=   @note.author_name -%>
 <%    if discussion.new_discussion? -%>

lib/gitlab/diff/position.rb

@@ -42,7 +42,7 @@ module Gitlab
       def encode_with(coder)
         coder['attributes'] = self.to_h
       end
-      
+
       def key
         @key ||= [base_sha, start_sha, head_sha, Digest::SHA1.hexdigest(old_path || ""), Digest::SHA1.hexdigest(new_path || ""), old_line, new_line]
       end

spec/features/merge_requests/diff_notes_avatars_spec.rb

@@ -163,11 +163,9 @@ feature 'Diff note avatars', feature: true, js: true do
       end
 
       context 'multiple comments' do
-        before do
-          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
-          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
-          create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
+        let!(:notes) { create_list(:diff_note_on_merge_request, 3, project: project, noteable: merge_request, in_reply_to: note) }
 
+        before do
           visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: view)
 
           wait_for_ajax

spec/features/merge_requests/diff_notes_resolve_spec.rb

@@ -191,7 +191,7 @@ feature 'Diff notes resolve', feature: true, js: true do
 
     context 'multiple notes' do
       before do
-        create(:diff_note_on_merge_request, project: project, noteable: merge_request)
+        create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: note)
         visit_merge_request
       end
 

spec/helpers/notes_helper_spec.rb

@@ -36,21 +36,4 @@ describe NotesHelper do
       expect(helper.note_max_access_for_user(other_note)).to eq('Reporter')
     end
   end
-
-  describe '#preload_max_access_for_authors' do
-    before do
-      # This method reads cache from RequestStore, so make sure it's clean.
-      RequestStore.clear!
-    end
-
-    it 'loads multiple users' do
-      expected_access = {
-        owner.id => Gitlab::Access::OWNER,
-        master.id => Gitlab::Access::MASTER,
-        reporter.id => Gitlab::Access::REPORTER
-      }
-
-      expect(helper.preload_max_access_for_authors(notes, project)).to eq(expected_access)
-    end
-  end
 end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -55,6 +55,7 @@ Note:
 - resolved_at
 - resolved_by_id
 - discussion_id
+- original_discussion_id
 LabelLink:
 - id
 - label_id

spec/mailers/notify_spec.rb

@@ -755,7 +755,7 @@ describe Notify do
       end
 
       shared_examples 'an email for a note on a diff discussion' do  |model|
-        let(:note) { create(model, project: project, author: note_author) }
+        let(:note) { create(model, author: note_author) }
 
         it "includes diffs with character-level highlighting" do
           is_expected.to have_body_text '<span class="p">}</span></span>'

spec/models/concerns/noteable_spec.rb

@@ -4,14 +4,14 @@ describe MergeRequest, Noteable, model: true do
   let(:merge_request) { create(:merge_request) }
   let(:project) { merge_request.project }
   let!(:active_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
-  let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
+  let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: active_diff_note1) }
   let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) }
   let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
-  let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
+  let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position, in_reply_to: outdated_diff_note1) }
   let!(:discussion_note1) { create(:discussion_note_on_merge_request, project: project, noteable: merge_request) }
   let!(:discussion_note2) { create(:discussion_note_on_merge_request, in_reply_to: discussion_note1) }
   let!(:commit_diff_note1) { create(:diff_note_on_commit, project: project) }
-  let!(:commit_diff_note2) { create(:diff_note_on_commit, project: project) }
+  let!(:commit_diff_note2) { create(:diff_note_on_commit, project: project, in_reply_to: commit_diff_note1) }
   let!(:commit_note1) { create(:note_on_commit, project: project) }
   let!(:commit_note2) { create(:note_on_commit, project: project) }
   let!(:commit_discussion_note1) { create(:discussion_note_on_commit, project: project) }
@@ -63,7 +63,7 @@ describe MergeRequest, Noteable, model: true do
     subject { merge_request.grouped_diff_discussions }
 
     it "includes active discussions" do
-      discussions = subject.values
+      discussions = subject.values.flatten
 
       expect(discussions.count).to eq(2)
       expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id])
@@ -74,12 +74,12 @@ describe MergeRequest, Noteable, model: true do
     end
 
     it "doesn't include outdated discussions" do
-      expect(subject.values.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
+      expect(subject.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
     end
 
     it "groups the discussions by line code" do
-      expect(subject[active_diff_note1.line_code].id).to eq(active_diff_note1.discussion_id)
-      expect(subject[active_diff_note3.line_code].id).to eq(active_diff_note3.discussion_id)
+      expect(subject[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id)
+      expect(subject[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id)
     end
   end
 end

spec/models/concerns/resolvable_discussion_spec.rb

+require 'spec_helper'
+
+describe Discussion, ResolvableDiscussion, models: true do
+  subject { described_class.new([first_note, second_note, third_note]) }
+
+  let(:first_note) { create(:discussion_note_on_merge_request) }
+  let(:merge_request) { first_note.noteable }
+  let(:second_note) { create(:discussion_note_on_merge_request, in_reply_to: first_note) }
+  let(:third_note) { create(:discussion_note_on_merge_request) }
+
+  describe "#resolvable?" do
+    context "when potentially resolvable" do
+      before do
+        allow(subject).to receive(:potentially_resolvable?).and_return(true)
+      end
+
+      context "when all notes are unresolvable" do
+        before do
+          allow(first_note).to receive(:resolvable?).and_return(false)
+          allow(second_note).to receive(:resolvable?).and_return(false)
+          allow(third_note).to receive(:resolvable?).and_return(false)
+        end
+
+        it "returns false" do
+          expect(subject.resolvable?).to be false
+        end
+      end
+
+      context "when some notes are unresolvable and some notes are resolvable" do
+        before do
+          allow(first_note).to receive(:resolvable?).and_return(true)
+          allow(second_note).to receive(:resolvable?).and_return(false)
+          allow(third_note).to receive(:resolvable?).and_return(true)
+        end
+
+        it "returns true" do
+          expect(subject.resolvable?).to be true
+        end
+      end
+
+      context "when all notes are resolvable" do
+        before do
+          allow(first_note).to receive(:resolvable?).and_return(true)
+          allow(second_note).to receive(:resolvable?).and_return(true)
+          allow(third_note).to receive(:resolvable?).and_return(true)
+        end
+
+        it "returns true" do
+          expect(subject.resolvable?).to be true
+        end
+      end
+    end
+
+    context "when not potentially resolvable" do
+      before do
+        allow(subject).to receive(:potentially_resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.resolvable?).to be false
+      end
+    end
+  end
+
+  describe "#resolved?" do
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.resolved?).to be false
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+
+        allow(first_note).to receive(:resolvable?).and_return(true)
+        allow(second_note).to receive(:resolvable?).and_return(false)
+        allow(third_note).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when all resolvable notes are resolved" do
+        before do
+          allow(first_note).to receive(:resolved?).and_return(true)
+          allow(third_note).to receive(:resolved?).and_return(true)
+        end
+
+        it "returns true" do
+          expect(subject.resolved?).to be true
+        end
+      end
+
+      context "when some resolvable notes are not resolved" do
+        before do
+          allow(first_note).to receive(:resolved?).and_return(true)
+          allow(third_note).to receive(:resolved?).and_return(false)
+        end
+
+        it "returns false" do
+          expect(subject.resolved?).to be false
+        end
+      end
+    end
+  end
+
+  describe "#to_be_resolved?" do
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.to_be_resolved?).to be false
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+
+        allow(first_note).to receive(:resolvable?).and_return(true)
+        allow(second_note).to receive(:resolvable?).and_return(false)
+        allow(third_note).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when all resolvable notes are resolved" do
+        before do
+          allow(first_note).to receive(:resolved?).and_return(true)
+          allow(third_note).to receive(:resolved?).and_return(true)
+        end
+
+        it "returns false" do
+          expect(subject.to_be_resolved?).to be false
+        end
+      end
+
+      context "when some resolvable notes are not resolved" do
+        before do
+          allow(first_note).to receive(:resolved?).and_return(true)
+          allow(third_note).to receive(:resolved?).and_return(false)
+        end
+
+        it "returns true" do
+          expect(subject.to_be_resolved?).to be true
+        end
+      end
+    end
+  end
+
+  describe "#can_resolve?" do
+    let(:current_user) { create(:user) }
+
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.can_resolve?(current_user)).to be false
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when not signed in" do
+        let(:current_user) { nil }
+
+        it "returns false" do
+          expect(subject.can_resolve?(current_user)).to be false
+        end
+      end
+
+      context "when signed in" do
+        context "when the signed in user is the noteable author" do
+          before do
+            subject.noteable.author = current_user
+          end
+
+          it "returns true" do
+            expect(subject.can_resolve?(current_user)).to be true
+          end
+        end
+
+        context "when the signed in user can push to the project" do
+          before do
+            subject.project.team << [current_user, :master]
+          end
+
+          it "returns true" do
+            expect(subject.can_resolve?(current_user)).to be true
+          end
+        end
+
+        context "when the signed in user is a random user" do
+          it "returns false" do
+            expect(subject.can_resolve?(current_user)).to be false
+          end
+        end
+      end
+    end
+  end
+
+  describe "#resolve!" do
+    let(:current_user) { create(:user) }
+
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns nil" do
+        expect(subject.resolve!(current_user)).to be_nil
+      end
+
+      it "doesn't set resolved_at" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved_at).to be_nil
+      end
+
+      it "doesn't set resolved_by" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved_by).to be_nil
+      end
+
+      it "doesn't mark as resolved" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved?).to be false
+      end
+    end
+
+    context "when resolvable" do
+      let(:user) { create(:user) }
+      let(:second_note) { create(:diff_note_on_commit) } # unresolvable
+
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when all resolvable notes are resolved" do
+        before do
+          first_note.resolve!(user)
+          third_note.resolve!(user)
+
+          first_note.reload
+          third_note.reload
+        end
+
+        it "doesn't change resolved_at on the resolved notes" do
+          expect(first_note.resolved_at).not_to be_nil
+          expect(third_note.resolved_at).not_to be_nil
+
+          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_at }
+          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_at }
+        end
+
+        it "doesn't change resolved_by on the resolved notes" do
+          expect(first_note.resolved_by).to eq(user)
+          expect(third_note.resolved_by).to eq(user)
+
+          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_by }
+          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_by }
+        end
+
+        it "doesn't change the resolved state on the resolved notes" do
+          expect(first_note.resolved?).to be true
+          expect(third_note.resolved?).to be true
+
+          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved? }
+          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved? }
+        end
+
+        it "doesn't change resolved_at" do
+          expect(subject.resolved_at).not_to be_nil
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at }
+        end
+
+        it "doesn't change resolved_by" do
+          expect(subject.resolved_by).to eq(user)
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by }
+        end
+
+        it "doesn't change resolved state" do
+          expect(subject.resolved?).to be true
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved? }
+        end
+      end
+
+      context "when some resolvable notes are resolved" do
+        before do
+          first_note.resolve!(user)
+        end
+
+        it "doesn't change resolved_at on the resolved note" do
+          expect(first_note.resolved_at).not_to be_nil
+
+          expect { subject.resolve!(current_user) }.
+            not_to change { first_note.reload.resolved_at }
+        end
+
+        it "doesn't change resolved_by on the resolved note" do
+          expect(first_note.resolved_by).to eq(user)
+
+          expect { subject.resolve!(current_user) }.
+            not_to change { first_note.reload && first_note.resolved_by }
+        end
+
+        it "doesn't change the resolved state on the resolved note" do
+          expect(first_note.resolved?).to be true
+
+          expect { subject.resolve!(current_user) }.
+            not_to change { first_note.reload && first_note.resolved? }
+        end
+
+        it "sets resolved_at on the unresolved note" do
+          subject.resolve!(current_user)
+          third_note.reload
+
+          expect(third_note.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by on the unresolved note" do
+          subject.resolve!(current_user)
+          third_note.reload
+
+          expect(third_note.resolved_by).to eq(current_user)
+        end
+
+        it "marks the unresolved note as resolved" do
+          subject.resolve!(current_user)
+          third_note.reload
+
+          expect(third_note.resolved?).to be true
+        end
+
+        it "sets resolved_at" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved_by).to eq(current_user)
+        end
+
+        it "marks as resolved" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved?).to be true
+        end
+      end
+
+      context "when no resolvable notes are resolved" do
+        it "sets resolved_at on the unresolved notes" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved_at).not_to be_nil
+          expect(third_note.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by on the unresolved notes" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved_by).to eq(current_user)
+          expect(third_note.resolved_by).to eq(current_user)
+        end
+
+        it "marks the unresolved notes as resolved" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved?).to be true
+          expect(third_note.resolved?).to be true
+        end
+
+        it "sets resolved_at" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(subject.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(subject.resolved_by).to eq(current_user)
+        end
+
+        it "marks as resolved" do
+          subject.resolve!(current_user)
+          first_note.reload
+          third_note.reload
+
+          expect(subject.resolved?).to be true
+        end
+      end
+    end
+  end
+
+  describe "#unresolve!" do
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns nil" do
+        expect(subject.unresolve!).to be_nil
+      end
+    end
+
+    context "when resolvable" do
+      let(:user) { create(:user) }
+
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+
+        allow(first_note).to receive(:resolvable?).and_return(true)
+        allow(second_note).to receive(:resolvable?).and_return(false)
+        allow(third_note).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when all resolvable notes are resolved" do
+        before do
+          first_note.resolve!(user)
+          third_note.resolve!(user)
+        end
+
+        it "unsets resolved_at on the resolved notes" do
+          subject.unresolve!
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved_at).to be_nil
+          expect(third_note.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by on the resolved notes" do
+          subject.unresolve!
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved_by).to be_nil
+          expect(third_note.resolved_by).to be_nil
+        end
+
+        it "unmarks the resolved notes as resolved" do
+          subject.unresolve!
+          first_note.reload
+          third_note.reload
+
+          expect(first_note.resolved?).to be false
+          expect(third_note.resolved?).to be false
+        end
+
+        it "unsets resolved_at" do
+          subject.unresolve!
+          first_note.reload
+          third_note.reload
+
+          expect(subject.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by" do
+          subject.unresolve!
+          first_note.reload
+          third_note.reload
+
+          expect(subject.resolved_by).to be_nil
+        end
+
+        it "unmarks as resolved" do
+          subject.unresolve!
+
+          expect(subject.resolved?).to be false
+        end
+      end
+
+      context "when some resolvable notes are resolved" do
+        before do
+          first_note.resolve!(user)
+        end
+
+        it "unsets resolved_at on the resolved note" do
+          subject.unresolve!
+
+          expect(subject.first_note.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by on the resolved note" do
+          subject.unresolve!
+
+          expect(subject.first_note.resolved_by).to be_nil
+        end
+
+        it "unmarks the resolved note as resolved" do
+          subject.unresolve!
+
+          expect(subject.first_note.resolved?).to be false
+        end
+      end
+    end
+  end
+
+  describe "#first_note_to_resolve" do
+    it "returns the first note that still needs to be resolved" do
+      allow(first_note).to receive(:to_be_resolved?).and_return(false)
+      allow(second_note).to receive(:to_be_resolved?).and_return(true)
+
+      expect(subject.first_note_to_resolve).to eq(second_note)
+    end
+  end
+
+  describe "#last_resolved_note" do
+    let(:current_user) { create(:user) }
+
+    before do
+      first_note.resolve!(current_user)
+      third_note.resolve!(current_user)
+      second_note.resolve!(current_user)
+    end
+
+    it "returns the last note that was resolved" do
+      expect(subject.last_resolved_note).to eq(second_note)
+    end
+  end
+end

spec/models/discussion_spec.rb

@@ -27,541 +27,4 @@ describe Discussion, model: true do
       ])
     end
   end
-
-  describe "#resolvable?" do
-    context "when potentially resolvable" do
-      before do
-        allow(subject).to receive(:potentially_resolvable?).and_return(true)
-      end
-
-      context "when all notes are unresolvable" do
-        before do
-          allow(first_note).to receive(:resolvable?).and_return(false)
-          allow(second_note).to receive(:resolvable?).and_return(false)
-          allow(third_note).to receive(:resolvable?).and_return(false)
-        end
-
-        it "returns false" do
-          expect(subject.resolvable?).to be false
-        end
-      end
-
-      context "when some notes are unresolvable and some notes are resolvable" do
-        before do
-          allow(first_note).to receive(:resolvable?).and_return(true)
-          allow(second_note).to receive(:resolvable?).and_return(false)
-          allow(third_note).to receive(:resolvable?).and_return(true)
-        end
-
-        it "returns true" do
-          expect(subject.resolvable?).to be true
-        end
-      end
-
-      context "when all notes are resolvable" do
-        before do
-          allow(first_note).to receive(:resolvable?).and_return(true)
-          allow(second_note).to receive(:resolvable?).and_return(true)
-          allow(third_note).to receive(:resolvable?).and_return(true)
-        end
-
-        it "returns true" do
-          expect(subject.resolvable?).to be true
-        end
-      end
-    end
-
-    context "when not potentially resolvable" do
-      before do
-        allow(subject).to receive(:potentially_resolvable?).and_return(false)
-      end
-
-      it "returns false" do
-        expect(subject.resolvable?).to be false
-      end
-    end
-  end
-
-  describe "#resolved?" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns false" do
-        expect(subject.resolved?).to be false
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-
-        allow(first_note).to receive(:resolvable?).and_return(true)
-        allow(second_note).to receive(:resolvable?).and_return(false)
-        allow(third_note).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when all resolvable notes are resolved" do
-        before do
-          allow(first_note).to receive(:resolved?).and_return(true)
-          allow(third_note).to receive(:resolved?).and_return(true)
-        end
-
-        it "returns true" do
-          expect(subject.resolved?).to be true
-        end
-      end
-
-      context "when some resolvable notes are not resolved" do
-        before do
-          allow(first_note).to receive(:resolved?).and_return(true)
-          allow(third_note).to receive(:resolved?).and_return(false)
-        end
-
-        it "returns false" do
-          expect(subject.resolved?).to be false
-        end
-      end
-    end
-  end
-
-  describe "#to_be_resolved?" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns false" do
-        expect(subject.to_be_resolved?).to be false
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-
-        allow(first_note).to receive(:resolvable?).and_return(true)
-        allow(second_note).to receive(:resolvable?).and_return(false)
-        allow(third_note).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when all resolvable notes are resolved" do
-        before do
-          allow(first_note).to receive(:resolved?).and_return(true)
-          allow(third_note).to receive(:resolved?).and_return(true)
-        end
-
-        it "returns false" do
-          expect(subject.to_be_resolved?).to be false
-        end
-      end
-
-      context "when some resolvable notes are not resolved" do
-        before do
-          allow(first_note).to receive(:resolved?).and_return(true)
-          allow(third_note).to receive(:resolved?).and_return(false)
-        end
-
-        it "returns true" do
-          expect(subject.to_be_resolved?).to be true
-        end
-      end
-    end
-  end
-
-  describe "#can_resolve?" do
-    let(:current_user) { create(:user) }
-
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns false" do
-        expect(subject.can_resolve?(current_user)).to be false
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when not signed in" do
-        let(:current_user) { nil }
-
-        it "returns false" do
-          expect(subject.can_resolve?(current_user)).to be false
-        end
-      end
-
-      context "when signed in" do
-        context "when the signed in user is the noteable author" do
-          before do
-            subject.noteable.author = current_user
-          end
-
-          it "returns true" do
-            expect(subject.can_resolve?(current_user)).to be true
-          end
-        end
-
-        context "when the signed in user can push to the project" do
-          before do
-            subject.project.team << [current_user, :master]
-          end
-
-          it "returns true" do
-            expect(subject.can_resolve?(current_user)).to be true
-          end
-        end
-
-        context "when the signed in user is a random user" do
-          it "returns false" do
-            expect(subject.can_resolve?(current_user)).to be false
-          end
-        end
-      end
-    end
-  end
-
-  describe "#resolve!" do
-    let(:current_user) { create(:user) }
-
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns nil" do
-        expect(subject.resolve!(current_user)).to be_nil
-      end
-
-      it "doesn't set resolved_at" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved_at).to be_nil
-      end
-
-      it "doesn't set resolved_by" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved_by).to be_nil
-      end
-
-      it "doesn't mark as resolved" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved?).to be false
-      end
-    end
-
-    context "when resolvable" do
-      let(:user) { create(:user) }
-      let(:second_note) { create(:diff_note_on_commit) } # unresolvable
-
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when all resolvable notes are resolved" do
-        before do
-          first_note.resolve!(user)
-          third_note.resolve!(user)
-
-          first_note.reload
-          third_note.reload
-        end
-
-        it "doesn't change resolved_at on the resolved notes" do
-          expect(first_note.resolved_at).not_to be_nil
-          expect(third_note.resolved_at).not_to be_nil
-
-          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_at }
-          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_at }
-        end
-
-        it "doesn't change resolved_by on the resolved notes" do
-          expect(first_note.resolved_by).to eq(user)
-          expect(third_note.resolved_by).to eq(user)
-
-          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_by }
-          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_by }
-        end
-
-        it "doesn't change the resolved state on the resolved notes" do
-          expect(first_note.resolved?).to be true
-          expect(third_note.resolved?).to be true
-
-          expect { subject.resolve!(current_user) }.not_to change { first_note.resolved? }
-          expect { subject.resolve!(current_user) }.not_to change { third_note.resolved? }
-        end
-
-        it "doesn't change resolved_at" do
-          expect(subject.resolved_at).not_to be_nil
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at }
-        end
-
-        it "doesn't change resolved_by" do
-          expect(subject.resolved_by).to eq(user)
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by }
-        end
-
-        it "doesn't change resolved state" do
-          expect(subject.resolved?).to be true
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved? }
-        end
-      end
-
-      context "when some resolvable notes are resolved" do
-        before do
-          first_note.resolve!(user)
-        end
-
-        it "doesn't change resolved_at on the resolved note" do
-          expect(first_note.resolved_at).not_to be_nil
-
-          expect { subject.resolve!(current_user) }.
-            not_to change { first_note.reload.resolved_at }
-        end
-
-        it "doesn't change resolved_by on the resolved note" do
-          expect(first_note.resolved_by).to eq(user)
-
-          expect { subject.resolve!(current_user) }.
-            not_to change { first_note.reload && first_note.resolved_by }
-        end
-
-        it "doesn't change the resolved state on the resolved note" do
-          expect(first_note.resolved?).to be true
-
-          expect { subject.resolve!(current_user) }.
-            not_to change { first_note.reload && first_note.resolved? }
-        end
-
-        it "sets resolved_at on the unresolved note" do
-          subject.resolve!(current_user)
-          third_note.reload
-
-          expect(third_note.resolved_at).not_to be_nil
-        end
-
-        it "sets resolved_by on the unresolved note" do
-          subject.resolve!(current_user)
-          third_note.reload
-
-          expect(third_note.resolved_by).to eq(current_user)
-        end
-
-        it "marks the unresolved note as resolved" do
-          subject.resolve!(current_user)
-          third_note.reload
-
-          expect(third_note.resolved?).to be true
-        end
-
-        it "sets resolved_at" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved_at).not_to be_nil
-        end
-
-        it "sets resolved_by" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved_by).to eq(current_user)
-        end
-
-        it "marks as resolved" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved?).to be true
-        end
-      end
-
-      context "when no resolvable notes are resolved" do
-        it "sets resolved_at on the unresolved notes" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved_at).not_to be_nil
-          expect(third_note.resolved_at).not_to be_nil
-        end
-
-        it "sets resolved_by on the unresolved notes" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved_by).to eq(current_user)
-          expect(third_note.resolved_by).to eq(current_user)
-        end
-
-        it "marks the unresolved notes as resolved" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved?).to be true
-          expect(third_note.resolved?).to be true
-        end
-
-        it "sets resolved_at" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(subject.resolved_at).not_to be_nil
-        end
-
-        it "sets resolved_by" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(subject.resolved_by).to eq(current_user)
-        end
-
-        it "marks as resolved" do
-          subject.resolve!(current_user)
-          first_note.reload
-          third_note.reload
-
-          expect(subject.resolved?).to be true
-        end
-      end
-    end
-  end
-
-  describe "#unresolve!" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns nil" do
-        expect(subject.unresolve!).to be_nil
-      end
-    end
-
-    context "when resolvable" do
-      let(:user) { create(:user) }
-
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-
-        allow(first_note).to receive(:resolvable?).and_return(true)
-        allow(second_note).to receive(:resolvable?).and_return(false)
-        allow(third_note).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when all resolvable notes are resolved" do
-        before do
-          first_note.resolve!(user)
-          third_note.resolve!(user)
-        end
-
-        it "unsets resolved_at on the resolved notes" do
-          subject.unresolve!
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved_at).to be_nil
-          expect(third_note.resolved_at).to be_nil
-        end
-
-        it "unsets resolved_by on the resolved notes" do
-          subject.unresolve!
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved_by).to be_nil
-          expect(third_note.resolved_by).to be_nil
-        end
-
-        it "unmarks the resolved notes as resolved" do
-          subject.unresolve!
-          first_note.reload
-          third_note.reload
-
-          expect(first_note.resolved?).to be false
-          expect(third_note.resolved?).to be false
-        end
-
-        it "unsets resolved_at" do
-          subject.unresolve!
-          first_note.reload
-          third_note.reload
-
-          expect(subject.resolved_at).to be_nil
-        end
-
-        it "unsets resolved_by" do
-          subject.unresolve!
-          first_note.reload
-          third_note.reload
-
-          expect(subject.resolved_by).to be_nil
-        end
-
-        it "unmarks as resolved" do
-          subject.unresolve!
-
-          expect(subject.resolved?).to be false
-        end
-      end
-
-      context "when some resolvable notes are resolved" do
-        before do
-          first_note.resolve!(user)
-        end
-
-        it "unsets resolved_at on the resolved note" do
-          subject.unresolve!
-
-          expect(subject.first_note.resolved_at).to be_nil
-        end
-
-        it "unsets resolved_by on the resolved note" do
-          subject.unresolve!
-
-          expect(subject.first_note.resolved_by).to be_nil
-        end
-
-        it "unmarks the resolved note as resolved" do
-          subject.unresolve!
-
-          expect(subject.first_note.resolved?).to be false
-        end
-      end
-    end
-  end
-
-  describe "#first_note_to_resolve" do
-    it "returns the first note that still needs to be resolved" do
-      allow(first_note).to receive(:to_be_resolved?).and_return(false)
-      allow(second_note).to receive(:to_be_resolved?).and_return(true)
-
-      expect(subject.first_note_to_resolve).to eq(second_note)
-    end
-  end
-
-  describe "#last_resolved_note" do
-    let(:current_user) { create(:user) }
-
-    before do
-      first_note.resolve!(current_user)
-      third_note.resolve!(current_user)
-      second_note.resolve!(current_user)
-    end
-
-    it "returns the last note that was resolved" do
-      expect(subject.last_resolved_note).to eq(second_note)
-    end
-  end
 end

spec/models/note_spec.rb

@@ -263,10 +263,10 @@ describe Note, models: true do
     let!(:merge_request) { create(:merge_request) }
     let(:project) { merge_request.project }
     let!(:active_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
-    let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
+    let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: active_diff_note1) }
     let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) }
     let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
-    let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
+    let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: outdated_diff_note1) }
 
     let(:active_position2) do
       Gitlab::Diff::Position.new(
@@ -291,7 +291,7 @@ describe Note, models: true do
     subject { merge_request.notes.grouped_diff_discussions }
 
     it "includes active discussions" do
-      discussions = subject.values
+      discussions = subject.values.flatten
 
       expect(discussions.count).to eq(2)
       expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id])
@@ -302,12 +302,12 @@ describe Note, models: true do
     end
 
     it "doesn't include outdated discussions" do
-      expect(subject.values.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
+      expect(subject.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
     end
 
     it "groups the discussions by line code" do
-      expect(subject[active_diff_note1.line_code].id).to eq(active_diff_note1.discussion_id)
-      expect(subject[active_diff_note3.line_code].id).to eq(active_diff_note3.discussion_id)
+      expect(subject[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id)
+      expect(subject[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id)
     end
   end
 

spec/services/issues/build_service_spec.rb

@@ -91,9 +91,7 @@ describe Issues::BuildService, services: true do
       end
 
       describe 'with multiple discussions' do
-        before do
-          create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, line_number: 15)
-        end
+        let!(:diff_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, line_number: 15) }
 
         it 'mentions all the authors in the description' do
           authors = merge_request.resolvable_discussions.map(&:author)
@@ -109,7 +107,7 @@ describe Issues::BuildService, services: true do
         end
 
         it 'mentions additional notes' do
-          create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, line_number: 15)
+          create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, in_reply_to: diff_note)
 
           expect(issue.description).to include('(+2 comments)')
         end