Add specs for Discussion and MergeRequest models

app/models/discussion.rb

@@ -83,9 +83,7 @@ class Discussion
   end
   def to_be_resolved?
-    return @to_be_resolved if defined?(@to_be_resolved)
-
-    @to_be_resolved = notes.any?(&:to_be_resolved?)
+    resolvable? && !resolved?
   end
   def can_resolve?(current_user)
@@ -97,12 +95,16 @@ class Discussion
   end
   def resolve!(current_user)
+    return unless resolvable?
+
     notes.each do |note|
       note.resolve!(current_user) if note.resolvable?
     end
   end
   def unresolve!
+    return unless resolvable?
+
     notes.each do |note|
       note.unresolve! if note.resolvable?
     end

spec/models/discussion_spec.rb

+require 'spec_helper'
+
+describe Discussion, model: true do
+  subject { described_class.new([first_note, second_note, third_note]) }
+
+  let(:first_note) { create(:diff_note_on_merge_request) }
+  let(:second_note) { create(:diff_note_on_merge_request) }
+  let(:third_note) { create(:diff_note_on_merge_request) }
+
+  describe "#resolvable?" do
+    context "when a diff discussion" do
+      before do
+        allow(subject).to receive(:diff_discussion?).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 a diff discussion" do
+      before do
+        allow(subject).to receive(:diff_discussion?).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) }
+
+      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 "calls resolve! on every resolvable note" do
+          expect(first_note).to receive(:resolve!).with(current_user)
+          expect(second_note).not_to receive(:resolve!)
+          expect(third_note).to receive(:resolve!).with(current_user)
+
+          subject.resolve!(current_user)
+        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 "calls resolve! on every resolvable note" do
+          expect(first_note).to receive(:resolve!).with(current_user)
+          expect(second_note).not_to receive(:resolve!)
+          expect(third_note).to receive(:resolve!).with(current_user)
+
+          subject.resolve!(current_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.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.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.resolved? }
+        end
+
+        it "sets resolved_at on the unresolved note" do
+          subject.resolve!(current_user)
+
+          expect(third_note.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by on the unresolved note" do
+          subject.resolve!(current_user)
+
+          expect(third_note.resolved_by).to eq(current_user)
+        end
+
+        it "marks the unresolved note as resolved" do
+          subject.resolve!(current_user)
+
+          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 "calls resolve! on every resolvable note" do
+          expect(first_note).to receive(:resolve!).with(current_user)
+          expect(second_note).not_to receive(:resolve!)
+          expect(third_note).to receive(:resolve!).with(current_user)
+
+          subject.resolve!(current_user)
+        end
+
+        it "sets resolved_at on the unresolved notes" do
+          subject.resolve!(current_user)
+
+          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)
+
+          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)
+
+          expect(first_note.resolved?).to be true
+          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
+    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 "calls unresolve! on every resolvable note" do
+          expect(first_note).to receive(:unresolve!)
+          expect(second_note).not_to receive(:unresolve!)
+          expect(third_note).to receive(:unresolve!)
+
+          subject.unresolve!
+        end
+
+        it "unsets resolved_at on the resolved notes" do
+          subject.unresolve!
+
+          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!
+
+          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!
+
+          expect(first_note.resolved?).to be false
+          expect(third_note.resolved?).to be false
+        end
+
+        it "unsets resolved_at" do
+          subject.unresolve!
+
+          expect(subject.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by" do
+          subject.unresolve!
+
+          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 "calls unresolve! on every resolvable note" do
+          expect(first_note).to receive(:unresolve!)
+          expect(second_note).not_to receive(:unresolve!)
+          expect(third_note).to receive(:unresolve!)
+
+          subject.unresolve!
+        end
+
+        it "unsets resolved_at on the resolved note" do
+          subject.unresolve!
+
+          expect(first_note.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by on the resolved note" do
+          subject.unresolve!
+
+          expect(first_note.resolved_by).to be_nil
+        end
+
+        it "unmarks the resolved note as resolved" do
+          subject.unresolve!
+
+          expect(first_note.resolved?).to be false
+        end
+      end
+
+      context "when no resolvable notes are resolved" do
+        it "calls unresolve! on every resolvable note" do
+          expect(first_note).to receive(:unresolve!)
+          expect(second_note).not_to receive(:unresolve!)
+          expect(third_note).to receive(:unresolve!)
+
+          subject.unresolve!
+        end
+      end
+    end
+  end
+
+  describe "#collapsed?" do
+    context "when a diff discussion" do
+      before do
+        allow(subject).to receive(:diff_discussion?).and_return(true)
+      end
+
+      context "when resolvable" do
+        before do
+          allow(subject).to receive(:resolvable?).and_return(true)
+        end
+
+        context "when resolved" do
+          before do
+            allow(subject).to receive(:resolved?).and_return(true)
+          end
+
+          it "returns true" do
+            expect(subject.collapsed?).to be true
+          end
+        end
+
+        context "when not resolved" do
+          before do
+            allow(subject).to receive(:resolved?).and_return(false)
+          end
+
+          it "returns false" do
+            expect(subject.collapsed?).to be false
+          end
+        end
+      end
+
+      context "when not resolvable" do
+        before do
+          allow(subject).to receive(:resolvable?).and_return(false)
+        end
+
+        context "when active" do
+          before do
+            allow(subject).to receive(:active?).and_return(true)
+          end
+
+          it "returns false" do
+            expect(subject.collapsed?).to be false
+          end
+        end
+
+        context "when outdated" do
+          before do
+            allow(subject).to receive(:active?).and_return(false)
+          end
+
+          it "returns true" do
+            expect(subject.collapsed?).to be true
+          end
+        end
+      end
+    end
+
+    context "when not a diff discussion" do
+      before do
+        allow(subject).to receive(:diff_discussion?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.collapsed?).to be false
+      end
+    end
+  end
+end

spec/models/merge_request_spec.rb

@@ -741,4 +741,96 @@ describe MergeRequest, models: true do
       end
     end
   end
+
+  context "discussion status" do
+    let(:first_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
+    let(:second_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
+    let(:third_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
+
+    before do
+      allow(subject).to receive(:discussions).and_return([first_discussion, second_discussion, third_discussion])
+    end
+
+    describe "#discussions_resolvable?" do
+      context "when all discussions are unresolvable" do
+        before do
+          allow(first_discussion).to receive(:resolvable?).and_return(false)
+          allow(second_discussion).to receive(:resolvable?).and_return(false)
+          allow(third_discussion).to receive(:resolvable?).and_return(false)
+        end
+
+        it "returns false" do
+          expect(subject.discussions_resolvable?).to be false
+        end
+      end
+
+      context "when some discussions are unresolvable and some discussions are resolvable" do
+        before do
+          allow(first_discussion).to receive(:resolvable?).and_return(true)
+          allow(second_discussion).to receive(:resolvable?).and_return(false)
+          allow(third_discussion).to receive(:resolvable?).and_return(true)
+        end
+
+        it "returns true" do
+          expect(subject.discussions_resolvable?).to be true
+        end
+      end
+
+      context "when all discussions are resolvable" do
+        before do
+          allow(first_discussion).to receive(:resolvable?).and_return(true)
+          allow(second_discussion).to receive(:resolvable?).and_return(true)
+          allow(third_discussion).to receive(:resolvable?).and_return(true)
+        end
+
+        it "returns true" do
+          expect(subject.discussions_resolvable?).to be true
+        end
+      end
+    end
+
+    describe "#discussions_resolved?" do
+      context "when discussions are not resolvable" do
+        before do
+          allow(subject).to receive(:discussions_resolvable?).and_return(false)
+        end
+
+        it "returns false" do
+          expect(subject.discussions_resolved?).to be false
+        end
+      end
+
+      context "when discussions are resolvable" do
+        before do
+          allow(subject).to receive(:discussions_resolvable?).and_return(true)
+
+          allow(first_discussion).to receive(:resolvable?).and_return(true)
+          allow(second_discussion).to receive(:resolvable?).and_return(false)
+          allow(third_discussion).to receive(:resolvable?).and_return(true)
+        end
+
+        context "when all resolvable discussions are resolved" do
+          before do
+            allow(first_discussion).to receive(:resolved?).and_return(true)
+            allow(third_discussion).to receive(:resolved?).and_return(true)
+          end
+
+          it "returns true" do
+            expect(subject.discussions_resolved?).to be true
+          end
+        end
+
+        context "when some resolvable discussions are not resolved" do
+          before do
+            allow(first_discussion).to receive(:resolved?).and_return(true)
+            allow(third_discussion).to receive(:resolved?).and_return(false)
+          end
+
+          it "returns false" do
+            expect(subject.discussions_resolved?).to be false
+          end
+        end
+      end
+    end
+  end
 end