Merge pull request #4485 from dks17/add_shift

fix before_remove and after_remove relation callbacks and add shift r…

Author: estolfo

lib/mongoid/association/embedded/embeds_many/proxy.rb

@@ -271,6 +271,29 @@ def pop(count = nil)
             end
           end
 
+          # Shift documents off the relation. This can be a single document or
+          # multiples, and will automatically persist the changes.
+          #
+          # @example Shift a single document.
+          #   relation.shift
+          #
+          # @example Shift multiple documents.
+          #   relation.shift(3)
+          #
+          # @param [ Integer ] count The number of documents to shift, or 1 if not
+          #   provided.
+          #
+          # @return [ Document, Array<Document> ] The shifted document(s).
+          def shift(count = nil)
+            if count
+              if _target.size > 0 && docs = _target[0, count]
+                docs.each { |doc| delete(doc) }
+              end
+            else
+              delete(_target[0])
+            end
+          end
+
           # Substitutes the supplied target documents for the existing documents
           # in the relation.
           #
@@ -526,4 +549,4 @@ def foreign_key_suffix
       end
     end
   end
-end
+end

spec/mongoid/association/embedded/embeds_many/proxy_spec.rb

@@ -2544,6 +2544,25 @@ class TrackingIdValidationHistory
         person.addresses.create(street: "hermannstr")
       end
 
+      context "when the number is zero" do
+
+        let!(:popped) do
+          person.addresses.pop(0)
+        end
+
+        it "returns an empty array" do
+          expect(popped).to eq([])
+        end
+
+        it "does not remove the document from the relation" do
+          expect(person.addresses).to eq([ address_one, address_two ])
+        end
+
+        it "does not persist the pop" do
+          expect(person.reload.addresses).to eq([ address_one, address_two ])
+        end
+      end
+
       context "when the number is not larger than the relation" do
 
         let!(:popped) do
@@ -2601,6 +2620,125 @@ class TrackingIdValidationHistory
     end
   end
 
+  describe "#shift" do
+
+    let(:person) do
+      Person.create
+    end
+
+    context "when no argument is provided" do
+
+      let!(:address_one) do
+        person.addresses.create(street: "sonnenallee")
+      end
+
+      let!(:address_two) do
+        person.addresses.create(street: "hermannstr")
+      end
+
+      let!(:shifted) do
+        person.addresses.shift
+      end
+
+      it "returns the shifted document" do
+        expect(shifted).to eq(address_one)
+      end
+
+      it "removes the document from the relation" do
+        expect(person.addresses).to eq([ address_two ])
+      end
+
+      it "persists the shift" do
+        expect(person.reload.addresses).to eq([ address_two ])
+      end
+    end
+
+    context "when an integer is provided" do
+
+      let!(:address_one) do
+        person.addresses.create(street: "sonnenallee")
+      end
+
+      let!(:address_two) do
+        person.addresses.create(street: "hermannstr")
+      end
+
+      context "when the number is zero" do
+
+        let!(:shifted) do
+          person.addresses.shift(0)
+        end
+
+        it "returns an empty array" do
+          expect(shifted).to eq([])
+        end
+
+        it "does not remove the document from the relation" do
+          expect(person.addresses).to eq([ address_one, address_two ])
+        end
+
+        it "does not persist the shift" do
+          expect(person.reload.addresses).to eq([ address_one, address_two ])
+        end
+      end
+
+      context "when the number is not larger than the relation" do
+
+        let!(:shifted) do
+          person.addresses.shift(2)
+        end
+
+        it "returns the shifted documents" do
+          expect(shifted).to eq([ address_one, address_two ])
+        end
+
+        it "removes the document from the relation" do
+          expect(person.addresses).to be_empty
+        end
+
+        it "persists the shift" do
+          expect(person.reload.addresses).to be_empty
+        end
+      end
+
+      context "when the number is larger than the relation" do
+
+        let!(:shifted) do
+          person.addresses.shift(4)
+        end
+
+        it "returns the shifted documents" do
+          expect(shifted).to eq([ address_one, address_two ])
+        end
+
+        it "removes the document from the relation" do
+          expect(person.addresses).to be_empty
+        end
+
+        it "persists the shift" do
+          expect(person.reload.addresses).to be_empty
+        end
+      end
+    end
+
+    context "when the relation is empty" do
+
+      context "when providing no number" do
+
+        it "returns nil" do
+          expect(person.addresses.shift).to be_nil
+        end
+      end
+
+      context "when providing a number" do
+
+        it "returns nil" do
+          expect(person.addresses.shift(2)).to be_nil
+        end
+      end
+    end
+  end
+
   describe "#scoped" do
 
     let(:person) do
@@ -3788,7 +3926,7 @@ class TrackingIdValidationHistory
     end
   end
 
-  context "#delete or #clear with before_remove callback" do
+  context "#delete, or #clear, or #pop, or #shift with before_remove callback" do
 
     let(:artist) do
       Artist.new
@@ -3834,35 +3972,81 @@ class TrackingIdValidationHistory
         end
       end
 
-      context "when errors are raised" do
+      describe "#pop" do
 
         before do
-          expect(artist).to receive(:before_remove_song).and_raise
+          artist.songs.pop
         end
 
-        describe "#delete" do
+        it "executes the callback" do
+          artist.songs.pop
+          expect(artist.before_remove_embedded_called).to be true
+        end
+      end
 
-          it "does not remove the document from the relation" do
-            begin; artist.songs.delete(song); rescue; end
-            expect(artist.songs).to eq([ song ])
-          end
+      describe "#shift" do
+
+        before do
+          artist.songs.shift
         end
 
-        describe "#clear" do
+        it "executes the callback" do
+          artist.songs.shift
+          expect(artist.before_remove_embedded_called).to be true
+        end
+      end
+    end
 
-          before do
-            begin; artist.songs.clear; rescue; end
-          end
+    context "when errors are raised" do
 
-          it "removes the documents from the relation" do
-            expect(artist.songs).to eq([ song ])
-          end
+      before do
+        expect(artist).to receive(:before_remove_song).and_raise
+      end
+
+      describe "#delete" do
+
+        it "does not remove the document from the relation" do
+          begin; artist.songs.delete(song); rescue; end
+          expect(artist.songs).to eq([ song ])
+        end
+      end
+
+      describe "#clear" do
+
+        before do
+          begin; artist.songs.clear; rescue; end
+        end
+
+        it "removes the documents from the relation" do
+          expect(artist.songs).to eq([ song ])
+        end
+      end
+
+      describe "#pop" do
+
+        before do
+          begin; artist.songs.pop; rescue; end
+        end
+
+        it "should remove from collection" do
+          expect(artist.songs).to eq([ song ])
+        end
+      end
+
+      describe "#shift" do
+
+        before do
+          begin; artist.songs.shift; rescue; end
+        end
+
+        it "should remove from collection" do
+          expect(artist.songs).to eq([ song ])
         end
       end
     end
   end
 
-  context "#delete or #clear with after_remove callback" do
+  context "#delete, or #clear, or #pop, or #shift with after_remove callback" do
 
     let(:artist) do
       Artist.new
@@ -3899,6 +4083,30 @@ class TrackingIdValidationHistory
           expect(artist.after_remove_embedded_called).to be true
         end
       end
+
+      describe "#pop" do
+
+        before do
+          artist.labels.pop
+        end
+
+        it "executes the callback" do
+          artist.labels.pop
+          expect(artist.after_remove_embedded_called).to be true
+        end
+      end
+
+      describe "#shift" do
+
+        before do
+          artist.labels.shift
+        end
+
+        it "executes the callback" do
+          artist.labels.shift
+          expect(artist.after_remove_embedded_called).to be true
+        end
+      end
     end
 
     context "when errors are raised" do
@@ -3928,6 +4136,28 @@ class TrackingIdValidationHistory
           expect(artist.labels).to be_empty
         end
       end
+
+      describe "#pop" do
+
+        before do
+          begin; artist.labels.pop; rescue; end
+        end
+
+        it "should remove from collection" do
+          expect(artist.labels).to be_empty
+        end
+      end
+
+      describe "#shift" do
+
+        before do
+          begin; artist.labels.shift; rescue; end
+        end
+
+        it "should remove from collection" do
+          expect(artist.labels).to be_empty
+        end
+      end
     end
   end