EXPERIMENTAL: Remove move_to and helper methods in favor of using position field directly.

lib/mongoid/orderable/generators/position.rb

@@ -10,12 +10,21 @@ def orderable_position(field = nil)
           field ||= default_orderable_field
           send("orderable_\#{field}_position")
         end
+
+        def orderable_position_was(field = nil)
+          field ||= default_orderable_field
+          send("orderable_\#{field}_position_was")
+        end
       KLASS
 
       generate_method("orderable_#{field_name}_position") do
         send(field_name)
       end
 
+      generate_method("orderable_#{field_name}_position_was") do
+        send("#{field_name}_was")
+      end
+
       generate_method("orderable_#{field_name}_position=") do |value|
         send("#{field_name}=", value)
       end

lib/mongoid/orderable/handlers/base.rb

@@ -16,6 +16,7 @@ def initialize(doc)
     delegate :orderable_keys,
              :orderable_field,
              :orderable_position,
+             :orderable_position_was,
              :orderable_if,
              :orderable_unless,
              :orderable_scope,
@@ -37,12 +38,10 @@ def use_transactions
     end
 
     def any_field_changed?
-      orderable_keys.any? {|field| changed?(field) }
+      orderable_keys.any? {|field| changed?(field) } || move_all.any?
     end
 
-    def set_new_record_positions
-      return unless new_record?
-
+    def set_target_positions
       orderable_keys.each do |field|
         next unless (position = doc.send(field))
 
@@ -74,6 +73,8 @@ def apply_one_position(field, target_position)
                   nil
                 elsif persisted? && !embedded?
                   scope.where(_id: _id).pluck(f).first
+                elsif persisted? && embedded?
+                  orderable_position_was(field)
                 else
                   orderable_position(field)
                 end
@@ -86,16 +87,23 @@ def apply_one_position(field, target_position)
 
       # Return if there is no instruction to change the position
       in_list = persisted? && current
+      puts 'uuu'
+      puts persisted?.inspect
+      puts current.inspect
+      puts target_position.inspect
       return if in_list && !target_position
 
       target = resolve_target_position(field, target_position, in_list)
 
       # Use $inc operator to shift the position of the other documents
       if !in_list
+        puts '111'
         scope.gte(f => target).inc(f => 1)
       elsif target < current
+        puts '222'
         scope.where(f => { '$gte' => target, '$lt' => current }).inc(f => 1)
       elsif target > current
+        puts '333'
         scope.where(f => { '$gt' => current, '$lte' => target }).inc(f => -1)
       end
 

lib/mongoid/orderable/handlers/document.rb

@@ -5,7 +5,7 @@ module Orderable
 module Handlers
   class Document < Base
     def before_create
-      set_new_record_positions
+      set_target_positions
       apply_all_positions
     end
 
@@ -15,6 +15,7 @@ def after_create
 
     def before_update
       return unless any_field_changed?
+      set_target_positions
       apply_all_positions
     end
 

lib/mongoid/orderable/handlers/document_transactional.rb

@@ -5,7 +5,7 @@ module Orderable
 module Handlers
   class DocumentTransactional < Document
     def before_create
-      set_new_record_positions
+      set_target_positions
     end
 
     def after_create

spec/integration/embedded_spec.rb

@@ -187,6 +187,196 @@ def positions
         expect(child2.reload.position).to eq(1)
       end
     end
+
+    context '#save! on parent' do
+      let!(:parent) { EmbedsOrderable.last }
+      let!(:child1) { parent.embedded_orderables.first }
+      let!(:child2) { parent.embedded_orderables.second }
+      let!(:child3) { parent.embedded_orderables.third }
+
+      it 'sets existing item to top' do
+        child3.position = 1
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 1]
+      end
+
+      it 'sets existing item above top' do
+        child3.position = 0
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 0]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 1]
+      end
+
+      it 'sets existing item to middle' do
+        child3.position = 2
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 2]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 2]
+      end
+
+      it 'sets existing item to bottom' do
+        child1.position = 3
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [3, 2, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [3, 1, 2]
+      end
+
+      it 'sets existing item below bottom' do
+        child1.position = 4
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [4, 2, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [3, 1, 2]
+      end
+
+      it 'sets existing item with nil position' do
+        child2.position = nil
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 4, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 2]
+      end
+
+      it 'saves new item with position set to top' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 1
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+      end
+
+      it 'saves new item with position set above top' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 0
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+      end
+
+      it 'saves new item with position set to middle' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 2
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 3, 4, 2]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 4, 2]
+      end
+
+      it 'saves new item with position set to bottom' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 4
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 3, 5]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+
+      it 'saves new item with position set below bottom' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 5
+        parent.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+
+      it 'saves new item with nil position' do
+        parent.embedded_orderables.build
+        parent.save!
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+    end
+
+    context '#save! on child' do
+      let!(:parent) { EmbedsOrderable.last }
+      let!(:child1) { parent.embedded_orderables.first }
+      let!(:child2) { parent.embedded_orderables.second }
+      let!(:child3) { parent.embedded_orderables.third }
+
+      it 'sets existing item to top' do
+        child3.position = 1
+        child3.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 1]
+      end
+
+      it 'sets existing item above top' do
+        child3.position = 0
+        child3.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 0]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 1]
+      end
+
+      it 'sets existing item to middle' do
+        child3.position = 2
+        child3.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 2]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 2]
+      end
+
+      it 'sets existing item to bottom' do
+        child1.position = 3
+        child1.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [3, 2, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [3, 1, 2]
+      end
+
+      it 'sets existing item below bottom' do
+        child1.position = 4
+        child1.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [4, 2, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [3, 1, 2]
+      end
+
+      it 'sets existing item with nil position' do
+        child2.position = nil
+        child2.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 4, 3]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 2]
+      end
+
+      it 'saves new item with position set to top' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 1
+        child4.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+      end
+
+      it 'saves new item with position set above top' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 0
+        child4.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [2, 3, 4, 1]
+      end
+
+      it 'saves new item with position set to middle' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 2
+        child4.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 3, 4, 2]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 3, 4, 2]
+      end
+
+      it 'saves new item with position set to bottom' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 4
+        child4.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 3, 5]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+
+      it 'saves new item with position set below bottom' do
+        child4 = parent.embedded_orderables.build
+        child4.position = 5
+        child4.save!
+        # expect(parent.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+
+      it 'saves new item with nil position' do
+        child4 = parent.embedded_orderables.build
+        child4.save!
+        expect(parent.reload.embedded_orderables.map(&:position)).to eq [1, 2, 3, 4]
+      end
+    end
   end
 
   context 'with transactions' do