[READY FOR REVIEW] MONGOID-5571 Fix options arg in #destroy! being ignored + specs (#5552)

* Fix options args in #destroy! being ignored.

* Add specs for #delete, #destroy, and #destroy :persist and :suppress options

* Update code docs

* rspec expect

Author: johnnyshields

lib/mongoid/persistable/deletable.rb

@@ -12,7 +12,11 @@ module Deletable
       # @example Remove the document.
       #   document.remove
       #
-      # @param [ Hash ] options Options to pass to remove.
+      # @param [ Hash ] options The options.
+      # @option options [ true | false ] :persist Whether to persist
+      #   the delete action.
+      # @option options [ true | false ] :suppress Whether to update
+      #   the parent document in-memory when deleting an embedded document.
       #
       # @return [ TrueClass ] True.
       def delete(options = {})

lib/mongoid/persistable/destroyable.rb

@@ -12,7 +12,11 @@ module Destroyable
       # @example Destroy a document.
       #   document.destroy
       #
-      # @param [ Hash ] options Options to pass to destroy.
+      # @param [ Hash ] options The options.
+      # @option options [ true | false ] :persist Whether to persist
+      #   the delete action. Callbacks will still be run even if false.
+      # @option options [ true | false ] :suppress Whether to update
+      #   the parent document in-memory when deleting an embedded document.
       #
       # @return [ true | false ] True if successful, false if not.
       def destroy(options = nil)
@@ -35,8 +39,21 @@ def destroy(options = nil)
         result
       end
 
+      # Remove the document from the database with callbacks. Raises
+      # an error if the document is not destroyed.
+      #
+      # @example Destroy a document.
+      #   document.destroy!
+      #
+      # @param [ Hash ] options The options.
+      # @option options [ true | false ] :persist Whether to persist
+      #   the delete action. Callbacks will still be run even if false.
+      # @option options [ true | false ] :suppress Whether to update
+      #   the parent document in-memory when deleting an embedded document.
+      #
+      # @return [ true ] Always true.
       def destroy!(options = {})
-        destroy || raise(Errors::DocumentNotDestroyed.new(_id, self.class))
+        destroy(options) || raise(Errors::DocumentNotDestroyed.new(_id, self.class))
       end
 
       module ClassMethods

spec/mongoid/persistable/deletable_spec.rb

@@ -57,7 +57,7 @@
       end
     end
 
-    context "when removing a root document" do
+    context "when deleting a root document" do
 
       let!(:deleted) do
         person.delete
@@ -76,9 +76,28 @@
       it "resets the flagged for destroy flag" do
         expect(person).to_not be_flagged_for_destroy
       end
+
+      context 'when :persist option false' do
+
+        let!(:deleted) do
+          person.delete(persist: false)
+        end
+
+        it "does not delete the document from the collection" do
+          expect(Person.find(person.id)).to eq person
+        end
+
+        it "returns true" do
+          expect(deleted).to be true
+        end
+
+        it "does not set the flagged for destroy flag" do
+          expect(person).to_not be_flagged_for_destroy
+        end
+      end
     end
 
-    context "when removing an embedded document" do
+    context "when deleting an embedded document" do
 
       let(:address) do
         person.addresses.build(street: "Bond Street")
@@ -114,13 +133,60 @@
           Person.find(person.id)
         end
 
-        it "removes the object from the parent and database" do
+        it "removes the object from the parent" do
+          expect(person.addresses).to be_empty
+        end
+
+        it "removes the object from the database" do
+          expect(from_db.addresses).to be_empty
+        end
+      end
+
+      context 'when :persist option false' do
+
+        before do
+          address.save!
+          address.delete(persist: false)
+        end
+
+        let(:from_db) do
+          Person.find(person.id)
+        end
+
+        it "does not remove the object from the parent" do
+          expect(person.addresses).to eq [address]
+          expect(person.addresses.first).to_not be_flagged_for_destroy
+        end
+
+        it "does not remove the object from the database" do
+          expect(from_db.addresses).to eq [address]
+          expect(from_db.addresses.first).to_not be_flagged_for_destroy
+        end
+      end
+
+      context 'when :suppress option true' do
+
+        before do
+          address.save!
+          address.delete(suppress: true)
+        end
+
+        let(:from_db) do
+          Person.find(person.id)
+        end
+
+        it "does not remove the object from the parent" do
+          expect(person.addresses).to eq [address]
+          expect(person.addresses.first).to_not be_flagged_for_destroy
+        end
+
+        it "removes the object from the database" do
           expect(from_db.addresses).to be_empty
         end
       end
     end
 
-    context "when removing deeply embedded documents" do
+    context "when deleting deeply embedded documents" do
 
       context "when the document has been saved" do