WIP - add ability to set position for new record

CHANGELOG.md

@@ -1,6 +1,8 @@
 ### 6.0.5 (Next)
 
-* Your contribution here.
+* [#75](https://github.com/mongoid/mongoid_orderable/pull/75): Fix: Preserve position when creating new records.
+* [#75](https://github.com/mongoid/mongoid_orderable/pull/75): Fix: .
+
 
 ### 6.0.4 (2023/02/01)
 

lib/mongoid/orderable/generators/position.rb

@@ -8,16 +8,16 @@ def generate(field_name)
       klass.class_eval <<~KLASS, __FILE__, __LINE__ + 1
         def orderable_position(field = nil)
           field ||= default_orderable_field
-          send "orderable_\#{field}_position"
+          send("orderable_\#{field}_position")
         end
       KLASS
 
       generate_method("orderable_#{field_name}_position") do
-        send field_name
+        send(field_name)
       end
 
       generate_method("orderable_#{field_name}_position=") do |value|
-        send "#{field_name}=", value
+        send("#{field_name}=", value)
       end
     end
   end

lib/mongoid/orderable/handlers/base.rb

@@ -8,6 +8,10 @@ class Base
 
     def initialize(doc)
       @doc = doc
+
+      # Required for transactions, which perform some actions
+      # in the after_create callback.
+      @new_record = doc.new_record?
     end
 
     protected
@@ -22,12 +26,15 @@ def initialize(doc)
              :orderable_top,
              :orderable_bottom,
              :_id,
-             :new_record?,
              :persisted?,
              :embedded?,
              :collection_name,
              to: :doc
 
+    def new_record?
+      @new_record
+    end
+
     def use_transactions
       false
     end
@@ -36,8 +43,18 @@ def any_field_changed?
       orderable_keys.any? {|field| changed?(field) }
     end
 
+    def set_new_record_positions
+      return unless new_record?
+
+      orderable_keys.each do |field|
+        next unless (position = doc.send(field))
+
+        move_all[field] ||= position
+      end
+    end
+
     def apply_all_positions
-      orderable_keys.map {|field| apply_one_position(field, move_all[field]) }
+      orderable_keys.each {|field| apply_one_position(field, move_all[field]) }
     end
 
     def apply_one_position(field, target_position)
@@ -56,7 +73,7 @@ def apply_one_position(field, target_position)
       end
 
       # Get the current position as exists in the database
-      current = if !persisted? || scope_changed
+      current = if new_record? || scope_changed
                   nil
                 elsif persisted? && !embedded?
                   scope.where(_id: _id).pluck(f).first
@@ -74,8 +91,9 @@ def apply_one_position(field, target_position)
       in_list = persisted? && current
       return if in_list && !target_position
 
-      # Use $inc operator to shift the position of the other documents
       target = resolve_target_position(field, target_position, in_list)
+
+      # Use $inc operator to shift the position of the other documents
       if !in_list
         scope.gte(f => target).inc(f => 1)
       elsif target < current
@@ -85,7 +103,7 @@ def apply_one_position(field, target_position)
       end
 
       # If persisted, update the field in the database atomically
-      doc.set({ f => target }.merge(changed_scope_hash(field))) if use_transactions && persisted?
+      doc.set({ f => target }.merge(changed_scope_hash(field))) if use_transactions
       doc.send("orderable_#{field}_position=", target)
     end
 
@@ -107,10 +125,14 @@ def move_all
       doc.send(:move_all)
     end
 
+    def clear_move_all!
+      doc.send(:clear_move_all!)
+    end
+
     def resolve_target_position(field, target_position, in_list)
       target_position ||= 'bottom'
 
-      unless target_position.is_a? Numeric
+      unless target_position.is_a?(Numeric)
         target_position = case target_position.to_s
                           when 'top' then (min ||= orderable_top(field))
                           when 'bottom' then (max ||= orderable_bottom(field, in_list))

lib/mongoid/orderable/handlers/document.rb

@@ -5,18 +5,26 @@ module Orderable
 module Handlers
   class Document < Base
     def before_create
+      set_new_record_positions
       apply_all_positions
     end
 
-    def after_create; end
+    def after_create
+      clear_move_all!
+    end
 
     def before_update
       return unless any_field_changed?
       apply_all_positions
     end
 
+    def after_update
+      clear_move_all!
+    end
+
     def after_destroy
       remove_all_positions
+      clear_move_all!
     end
   end
 end

lib/mongoid/orderable/handlers/document_transactional.rb

@@ -5,11 +5,12 @@ module Orderable
 module Handlers
   class DocumentTransactional < Document
     def before_create
-      clear_all_positions
+      set_new_record_positions
     end
 
     def after_create
       apply_all_positions
+      super
     end
 
     protected
@@ -18,10 +19,6 @@ def apply_all_positions
       with_transaction { super }
     end
 
-    def clear_all_positions
-      orderable_keys.each {|field| doc.send("orderable_#{field}_position=", nil) }
-    end
-
     def use_transactions
       true
     end

lib/mongoid/orderable/handlers/transaction.rb

@@ -15,7 +15,7 @@ def initialize(doc)
     end
 
     def with_transaction(&block)
-      Mongoid::QueryCache.uncached do
+      query_cache.uncached do
         if Thread.current[THREAD_KEY]
           yield
         else
@@ -65,6 +65,10 @@ def transaction_opts
     def transaction_max_retries
       doc.orderable_keys.map {|k| doc.class.orderable_configs.dig(k, :transaction_max_retries) }.compact.max
     end
+
+    def query_cache
+      defined?(Mongo::QueryCache) ? Mongo::QueryCache : Mongoid::QueryCache
+    end
   end
 end
 end

lib/mongoid/orderable/mixins/callbacks.rb

@@ -17,6 +17,7 @@ module Callbacks
       delegate :before_create,
                :after_create,
                :before_update,
+               :after_update,
                :after_destroy,
                to: :orderable_handler,
                prefix: :orderable

lib/mongoid/orderable/mixins/movable.rb

@@ -47,12 +47,16 @@ def move_#{symbol}!(options = {})
     protected
 
     def move_all
-      @move_all || {}
+      @move_all ||= {}
     end
 
     def move_field_to(position, options)
       field = options[:field] || default_orderable_field
-      @move_all = move_all.merge(field => position)
+      move_all[field] = position
+    end
+
+    def clear_move_all!
+      @move_all = {}
     end
   end
 end

spec/integration/simple_spec.rb

@@ -85,8 +85,10 @@ def positions
         expect(another.position).to eq(6)
         newbie.save!
         expect(positions).to eq([1, 2, 3, 4, 5, 6, 7])
-        expect(newbie.position).to eq(7)
-        expect(another.position).to eq(6)
+        # expect(newbie.position).to eq(6)
+        # expect(another.position).to eq(6) # not reloaded
+        expect(newbie.reload.position).to eq(6)
+        expect(another.reload.position).to eq(7)
       end
 
       it 'parallel updates' do
@@ -95,8 +97,43 @@ def positions
         another = SimpleOrderable.create!
         newbie.save!
         expect(positions).to eq([1, 2, 3, 4, 5, 6, 7])
-        expect(newbie.position).to eq(7)
-        expect(another.position).to eq(6)
+        # expect(newbie.position).to eq(6)
+        # expect(another.position).to eq(6) # not reloaded
+        expect(newbie.reload.position).to eq(6)
+        expect(another.reload.position).to eq(7)
+      end
+
+      it 'with correct specific position as a number' do
+        record = SimpleOrderable.create!(position: 3)
+        # expect(record.position).to eq(3)
+        expect(record.reload.position).to eq(3)
+      end
+
+      it 'with incorrect specific position as a number' do
+        record = SimpleOrderable.create!(position: -4)
+        # expect(record.position).to eq(1) # not reloaded
+        expect(record.reload.position).to eq(1)
+      end
+
+      it 'with correct specific position as a string' do
+        record = SimpleOrderable.create!(position: '4')
+        # expect(record.position).to eq(4)
+        expect(record.reload.position).to eq(4)
+      end
+
+      it 'with incorrect specific position as a string' do
+        record = SimpleOrderable.create!(position: '-4')
+        # expect(record.position).to eq(1)
+        expect(record.reload.position).to eq(1)
+      end
+
+      it 'should offset the positions of all the next elements' do
+        records = SimpleOrderable.all
+        expect(records.pluck(:position)).to eq([1, 2, 3, 4, 5])
+        SimpleOrderable.create!(position: 3)
+        # expect(records.pluck(:position)).to eq([1, 2, 3, 4, 5, 6]) # not reloaded
+        records.each(&:reload)
+        expect(records.pluck(:position)).to eq([1, 2, 4, 5, 6, 3])
       end
     end