Merge pull request #52 from felixbuenemann/customize-primary-key

* Allow for use of a different primary key
* Fix unreliable column information reset in tests

lib/acts_as_tree.rb

@@ -54,6 +54,8 @@ def self.included(base)
   module ClassMethods
     # Configuration options are:
     #
+    # * <tt>primary_key</tt> - specifies the column name for relations
+    #                          (default: +id+)
     # * <tt>foreign_key</tt> - specifies the column name to use for tracking
     #                          of the tree (default: +parent_id+)
     # * <tt>order</tt> - makes it possible to sort the children according to
@@ -63,6 +65,7 @@ module ClassMethods
     #                            a custom column by passing a symbol or string.
     def acts_as_tree(options = {})
       configuration = {
+        primary_key:   primary_key,
         foreign_key:   "parent_id",
         order:         nil,
         counter_cache: nil,
@@ -78,6 +81,7 @@ def acts_as_tree(options = {})
 
       belongs_to_opts = {
         class_name:    name,
+        primary_key:   configuration[:primary_key],
         foreign_key:   configuration[:foreign_key],
         counter_cache: configuration[:counter_cache],
         touch:         configuration[:touch],
@@ -91,11 +95,13 @@ def acts_as_tree(options = {})
       if ActiveRecord::VERSION::MAJOR >= 4
         has_many :children, lambda { order configuration[:order] },
           class_name:  name,
+          primary_key: configuration[:primary_key],
           foreign_key: configuration[:foreign_key],
           dependent:   configuration[:dependent],
           inverse_of:  :parent
       else
         has_many :children, class_name:  name,
+          primary_key: configuration[:primary_key],
           foreign_key: configuration[:foreign_key],
           order:       configuration[:order],
           dependent:   configuration[:dependent],
@@ -135,7 +141,7 @@ def leaves
         class_eval <<-EOV
           def self.leaves
             internal_ids = select(:#{configuration[:foreign_key]}).where(arel_table[:#{configuration[:foreign_key]}].not_eq(nil))
-            where("id NOT IN (\#{internal_ids.to_sql})").default_tree_order
+            where("\#{connection.quote_column_name('#{configuration[:primary_key]}')} NOT IN (\#{internal_ids.to_sql})").default_tree_order
           end
         EOV
       end

test/acts_as_tree_test.rb

@@ -47,19 +47,26 @@ def capture_stdout(&block)
 
 ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:"
 
-def setup_db(counter_cache = false)
+def setup_db(counter_cache = false, external_ids = false)
   # AR keeps printing annoying schema statements
   capture_stdout do
     ActiveRecord::Base.logger
     ActiveRecord::Schema.define(version: 1) do
       create_table :mixins do |t|
         t.column :type, :string
         t.column :parent_id, :integer
+        t.column :external_id, :integer if external_ids
+        t.column :external_parent_id, :integer if external_ids
         t.column(:children_count, :integer, default: 0) if counter_cache
         t.timestamps null: false
       end
     end
 
+    # Fix broken reset_column_information in some activerecord versions.
+    if ActiveRecord::VERSION::MAJOR == 3 && ActiveRecord::VERSION::MINOR == 2 ||
+       ActiveRecord::VERSION::MAJOR == 4 && ActiveRecord::VERSION::MINOR == 1
+      ActiveRecord::Base.connection.schema_cache.clear!
+    end
     Mixin.reset_column_information
   end
 end
@@ -99,21 +106,30 @@ class RecursivelyCascadedTreeMixin < Mixin
 end
 
 class TreeMixinWithTouch < Mixin
-   acts_as_tree foreign_key: "parent_id", order: "id", touch: true
+  acts_as_tree foreign_key: "parent_id", order: "id", touch: true
+end
+
+class ExternalTreeMixin < Mixin
+  acts_as_tree foreign_key: "external_parent_id", primary_key: "external_id"
+end
+
+class ExternalTreeMixinNullify < Mixin
+  acts_as_tree foreign_key: "external_parent_id", primary_key: "external_id", order: "id", dependent: :nullify
 end
 
 class TreeTest < ActsAsTreeTestCase
 
   def setup
     setup_db
+    @tree_mixin = TreeMixin
 
-    @root1              = TreeMixin.create!
-    @root_child1        = TreeMixin.create! parent_id: @root1.id
-    @child1_child       = TreeMixin.create! parent_id: @root_child1.id
-    @child1_child_child = TreeMixin.create! parent_id: @child1_child.id
-    @root_child2        = TreeMixin.create! parent_id: @root1.id
-    @root2              = TreeMixin.create!
-    @root3              = TreeMixin.create!
+    @root1              = @tree_mixin.create!
+    @root_child1        = @tree_mixin.create! parent_id: @root1.id
+    @child1_child       = @tree_mixin.create! parent_id: @root_child1.id
+    @child1_child_child = @tree_mixin.create! parent_id: @child1_child.id
+    @root_child2        = @tree_mixin.create! parent_id: @root1.id
+    @root2              = @tree_mixin.create!
+    @root3              = @tree_mixin.create!
   end
 
   def teardown
@@ -135,12 +151,12 @@ def test_parent
   end
 
   def test_delete
-    assert_equal 7, TreeMixin.count
+    assert_equal 7, @tree_mixin.count
     @root1.destroy
-    assert_equal 2, TreeMixin.count
+    assert_equal 2, @tree_mixin.count
     @root2.destroy
     @root3.destroy
-    assert_equal 0, TreeMixin.count
+    assert_equal 0, @tree_mixin.count
   end
 
   def test_insert
@@ -166,7 +182,7 @@ def test_ancestors
   end
 
   def test_root
-    assert_equal @root1, TreeMixin.root
+    assert_equal @root1, @tree_mixin.root
     assert_equal @root1, @root1.root
     assert_equal @root1, @root_child1.root
     assert_equal @root1, @child1_child.root
@@ -176,15 +192,15 @@ def test_root
   end
 
   def test_roots
-    assert_equal [@root1, @root2, @root3], TreeMixin.roots
+    assert_equal [@root1, @root2, @root3], @tree_mixin.roots
   end
 
   def test_leaves
-    assert_equal [@child1_child_child, @root_child2, @root2, @root3], TreeMixin.leaves
+    assert_equal [@child1_child_child, @root_child2, @root2, @root3], @tree_mixin.leaves
   end
 
   def test_default_tree_order
-    assert_equal [@root1, @root_child1, @child1_child, @child1_child_child, @root_child2, @root2, @root3], TreeMixin.default_tree_order
+    assert_equal [@root1, @root_child1, @child1_child, @child1_child_child, @root_child2, @root2, @root3], @tree_mixin.default_tree_order
   end
 
   def test_siblings
@@ -261,9 +277,9 @@ def test_is_leaf
   end
 
   def test_tree_view
-    assert_equal false, Mixin.respond_to?(:tree_view)
-    Mixin.extend ActsAsTree::TreeView
-    assert_equal true,  TreeMixin.respond_to?(:tree_view)
+    assert_equal false, @tree_mixin.respond_to?(:tree_view)
+    @tree_mixin.extend ActsAsTree::TreeView
+    assert_equal true,  @tree_mixin.respond_to?(:tree_view)
 
     tree_view_outputs = <<-END.gsub(/^ {6}/, '')
       root
@@ -275,15 +291,15 @@ def test_tree_view
        |_ 6
        |_ 7
     END
-    assert_equal tree_view_outputs, capture_stdout { TreeMixin.tree_view(:id) }
+    assert_equal tree_view_outputs, capture_stdout { @tree_mixin.tree_view(:id) }
   end
 
   def test_tree_walker
-    assert_equal false, TreeMixin.respond_to?(:walk_tree)
-    assert_equal false, TreeMixin.new.respond_to?(:walk_tree)
-    TreeMixin.extend ActsAsTree::TreeWalker
-    assert_equal true,  TreeMixin.respond_to?(:walk_tree)
-    assert_equal true,  TreeMixin.new.respond_to?(:walk_tree)
+    assert_equal false, @tree_mixin.respond_to?(:walk_tree)
+    assert_equal false, @tree_mixin.new.respond_to?(:walk_tree)
+    @tree_mixin.extend ActsAsTree::TreeWalker
+    assert_equal true,  @tree_mixin.respond_to?(:walk_tree)
+    assert_equal true,  @tree_mixin.new.respond_to?(:walk_tree)
 
     walk_tree_dfs_output = <<-END.gsub(/^\s+/, '')
       1
@@ -294,7 +310,7 @@ def test_tree_walker
       6
       7
       END
-    assert_equal walk_tree_dfs_output, capture_stdout { TreeMixin.walk_tree{|elem, level| puts "#{'-'*level}#{elem.id}"} }
+    assert_equal walk_tree_dfs_output, capture_stdout { @tree_mixin.walk_tree{|elem, level| puts "#{'-'*level}#{elem.id}"} }
 
     walk_tree_dfs_sub_output = <<-END.gsub(/^\s+/, '')
       2
@@ -313,7 +329,7 @@ def test_tree_walker
       --3
       ---4
       END
-    assert_equal walk_tree_bfs_output, capture_stdout { TreeMixin.walk_tree(:algorithm => :bfs){|elem, level| puts "#{'-'*level}#{elem.id}"} }
+    assert_equal walk_tree_bfs_output, capture_stdout { @tree_mixin.walk_tree(:algorithm => :bfs){|elem, level| puts "#{'-'*level}#{elem.id}"} }
 
     walk_tree_bfs_sub_output = <<-END.gsub(/^\s+/, '')
       2
@@ -538,3 +554,32 @@ def test_updated_at
     assert @root.updated_at != previous_root_updated_at
   end
 end
+
+class ExternalTreeTest < TreeTest
+  def setup
+    teardown_db
+    setup_db false, true
+    @tree_mixin = ExternalTreeMixin
+
+    @root1              = @tree_mixin.create! external_id: 1101
+    @root_child1        = @tree_mixin.create! external_id: 1102, external_parent_id: @root1.external_id
+    @child1_child       = @tree_mixin.create! external_id: 1103, external_parent_id: @root_child1.external_id
+    @child1_child_child = @tree_mixin.create! external_id: 1104, external_parent_id: @child1_child.external_id
+    @root_child2        = @tree_mixin.create! external_id: 1105, external_parent_id: @root1.external_id
+    @root2              = @tree_mixin.create! external_id: 1106
+    @root3              = @tree_mixin.create! external_id: 1107
+  end
+
+  def test_nullify
+    root4       = ExternalTreeMixinNullify.create! external_id: 1108
+    root4_child = ExternalTreeMixinNullify.create! external_id: 1109, external_parent_id: root4.external_id
+
+    assert_equal 2, ExternalTreeMixinNullify.count
+    assert_equal root4.external_id, root4_child.external_parent_id
+
+    root4.destroy
+
+    assert_equal 1, ExternalTreeMixinNullify.count
+    assert_nil root4_child.reload.external_parent_id
+  end
+end