From aca55095d4a34402d888d7408cb14862d0b0cf49 Mon Sep 17 00:00:00 2001
From: Brendon Muir <brendon@spike.net.nz>
Date: Mon, 8 Apr 2024 12:00:38 +1200
Subject: [PATCH] Use `quote_table_name_for_assignment`...

...in `update_all` calls to guard against reserved word column names.
---
 CHANGELOG.md                  |  1 +
 lib/positioning/mechanisms.rb | 12 ++++++++----
 test/models/post.rb           |  5 +++++
 test/support/active_record.rb |  7 +++++++
 test/test_positioning.rb      | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 test/models/post.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f9c9644..436375e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## [Unreleased]
 
 - Fetch the adapter_name from #connection_db_config (@tijn)
+- Use `quote_table_name_for_assignment` in `update_all` calls to guard against reserved word column names.
 
 ## [0.2.0] - 2024-03-12
 
diff --git a/lib/positioning/mechanisms.rb b/lib/positioning/mechanisms.rb
index 0845e48..7bfa4ba 100644
--- a/lib/positioning/mechanisms.rb
+++ b/lib/positioning/mechanisms.rb
@@ -56,6 +56,10 @@ def primary_key
       @positioned.send primary_key_column
     end
 
+    def quoted_column
+      base_class.connection.quote_table_name_for_assignment base_class.table_name, @column
+    end
+
     def record_scope
       base_class.where("#{primary_key_column}": primary_key)
     end
@@ -86,13 +90,13 @@ def move_out_of_the_way
     end
 
     def expand(scope, range)
-      scope.where("#{@column}": range).update_all "#{@column} = #{@column} * -1"
-      scope.where("#{@column}": ..-1).update_all "#{@column} = #{@column} * -1 + 1"
+      scope.where("#{@column}": range).update_all "#{quoted_column} = #{quoted_column} * -1"
+      scope.where("#{@column}": ..-1).update_all "#{quoted_column} = #{quoted_column} * -1 + 1"
     end
 
     def contract(scope, range)
-      scope.where("#{@column}": range).update_all "#{@column} = #{@column} * -1"
-      scope.where("#{@column}": ..-1).update_all "#{@column} = #{@column} * -1 - 1"
+      scope.where("#{@column}": range).update_all "#{quoted_column} = #{quoted_column} * -1"
+      scope.where("#{@column}": ..-1).update_all "#{quoted_column} = #{quoted_column} * -1 - 1"
     end
 
     def solidify_position
diff --git a/test/models/post.rb b/test/models/post.rb
new file mode 100644
index 0000000..d994fbd
--- /dev/null
+++ b/test/models/post.rb
@@ -0,0 +1,5 @@
+class Post < ActiveRecord::Base
+  positioned column: :order
+
+  default_scope { order(:order) }
+end
diff --git a/test/support/active_record.rb b/test/support/active_record.rb
index 5ed2f3a..41d09d6 100644
--- a/test/support/active_record.rb
+++ b/test/support/active_record.rb
@@ -48,6 +48,13 @@
     end
 
     add_index :authors, [:list_id, :enabled, :position], unique: true
+
+    create_table :posts, force: true do |t|
+      t.string :name
+      t.integer :order, null: false
+    end
+
+    add_index :posts, :order, unique: true
   end
 end
 
diff --git a/test/test_positioning.rb b/test/test_positioning.rb
index e66c4fe..3302a25 100644
--- a/test/test_positioning.rb
+++ b/test/test_positioning.rb
@@ -7,6 +7,7 @@
 require_relative "models/author"
 require_relative "models/author/student"
 require_relative "models/author/teacher"
+require_relative "models/post"
 
 class TestRelativePositionStruct < Minitest::Test
   def test_struct_takes_keyword_arguments
@@ -527,6 +528,37 @@ def test_that_not_destroyed_via_positioning_scope_closes_gap
   end
 end
 
+class TestPositioningColumns < Minitest::Test
+  include Minitest::Hooks
+
+  def around
+    ActiveRecord::Base.transaction do
+      super
+      raise ActiveRecord::Rollback
+    end
+  end
+
+  def test_that_a_column_named_order_works
+    first_post = Post.create name: "First Post"
+    second_post = Post.create name: "Second Post"
+    third_post = Post.create name: "Third Post"
+
+    assert_equal [1, 2, 3], [first_post.reload, second_post.reload, third_post.reload].map(&:order)
+
+    second_post.update order: {before: first_post}
+
+    assert_equal [1, 2, 3], [second_post.reload, first_post.reload, third_post.reload].map(&:order)
+
+    first_post.update order: {after: third_post}
+
+    assert_equal [1, 2, 3], [second_post.reload, third_post.reload, first_post.reload].map(&:order)
+
+    third_post.destroy
+
+    assert_equal [1, 2], [second_post.reload, first_post.reload].map(&:order)
+  end
+end
+
 class TestPositioning < Minitest::Test
   include Minitest::Hooks