Fix note resolution specsAshleyDumaine/gitlab-ce-36994-toggle-for-automatically-collapsing-outdated-diff-comments

3 files changed, 15 insertions, 10 deletions

diff --git a/app/models/concerns/resolvable_note.rb b/app/models/concerns/resolvable_note.rb
index 6f39029ff5a..668c5a079e3 100644
--- a/app/models/concerns/resolvable_note.rb
+++ b/app/models/concerns/resolvable_note.rb
@@ -58,15 +58,19 @@ module ResolvableNote
     self.resolved_at = Time.now
     self.resolved_by = current_user
     self.resolved_by_push = resolved_by_push
+
+    true
   end
 
   # If you update this method remember to also update `.unresolve!`
-  def unresolve_without_save(current_user)
+  def unresolve_without_save
     return false unless resolvable?
     return false unless resolved?
 
     self.resolved_at = nil
     self.resolved_by = nil
+
+    true
   end
 
   def resolve!(current_user, resolved_by_push: false)
@@ -75,6 +79,6 @@ module ResolvableNote
   end
 
   def unresolve!
-    unresolve_without_save(current_user) && save
+    unresolve_without_save && save!
   end
 end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 3708d54b056..a423067147e 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -65,6 +65,7 @@ Note:
 - change_position
 - resolved_at
 - resolved_by_id
+- resolved_by_push
 - discussion_id
 - original_discussion_id
 LabelLink:
diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb
index d00faa4f8be..91591017587 100644
--- a/spec/models/concerns/resolvable_note_spec.rb
+++ b/spec/models/concerns/resolvable_note_spec.rb
@@ -189,8 +189,8 @@ describe Note, ResolvableNote do
         allow(subject).to receive(:resolvable?).and_return(false)
       end
 
-      it "returns nil" do
-        expect(subject.resolve!(current_user)).to be_nil
+      it "returns false" do
+        expect(subject.resolve!(current_user)).to be_falsey
       end
 
       it "doesn't set resolved_at" do
@@ -224,8 +224,8 @@ describe Note, ResolvableNote do
           subject.resolve!(user)
         end
 
-        it "returns nil" do
-          expect(subject.resolve!(current_user)).to be_nil
+        it "returns false" do
+          expect(subject.resolve!(current_user)).to be_falsey
         end
 
         it "doesn't change resolved_at" do
@@ -279,8 +279,8 @@ describe Note, ResolvableNote do
         allow(subject).to receive(:resolvable?).and_return(false)
       end
 
-      it "returns nil" do
-        expect(subject.unresolve!).to be_nil
+      it "returns false" do
+        expect(subject.unresolve!).to be_falsey
       end
     end
 
@@ -320,8 +320,8 @@ describe Note, ResolvableNote do
       end
 
       context "when not resolved" do
-        it "returns nil" do
-          expect(subject.unresolve!).to be_nil
+        it "returns false" do
+          expect(subject.unresolve!).to be_falsey
         end
       end
     end