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..122b8ee0314 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: @@ -407,6 +408,7 @@ Project: - only_allow_merge_if_all_discussions_are_resolved - auto_cancel_pending_pipelines - printing_merge_request_link_enabled +- resolve_outdated_diff_discussions - build_allow_git_fetch - last_repository_updated_at - ci_config_path 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