Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parent doesn't work when building associationg without saving to database #64

Closed
brodock opened this issue Jun 24, 2013 · 8 comments
Closed

Comments

@brodock
Copy link
Contributor

brodock commented Jun 24, 2013

I have a really specific model validation that must ensure parent is not defined to enable some validations.

The problem is that if I create both the parent and the children without saving to the database (via build), it fails. Here is a code snippet:

class Category < ActiveRecord::Base
  acts_as_tree

  validates :code, :presence => {:if => first_level?}

  def first_level?
    parent.nil?
  end
end

here is how to make it fail:

c = Category.new(:code => '123')
c1 = c.children.build
c1.valid?
=> false

I've searched for the problem and found this patch submmited to rails 2.3.6 (it's old but still there): https://rails.lighthouseapp.com/projects/8994/tickets/2815-nested-models-build-should-directly-assign-the-parent

The solution is to use "inverse_of" while defining the associations. In theory it will fix this error.

@brodock
Copy link
Contributor Author

brodock commented Jun 26, 2013

There is one more thing about it, that can help create the tests and solve the problem:

c1.root?
=> true
c1.child?
=> false

c1.root? should return false and c1.child? should return true...

I've double checked my example, and by using "inverse_of" on both sides of the relations definition, solved it.

@brodock
Copy link
Contributor Author

brodock commented Jun 26, 2013

I can write the tests if you point me which spec you would like it to be in...

here is a patch to solve the problem I reported:

diff --git a/lib/closure_tree/model.rb b/lib/closure_tree/model.rb
index 866d2e8..07526c9 100644
--- a/lib/closure_tree/model.rb
+++ b/lib/closure_tree/model.rb
@@ -7,7 +7,8 @@ module ClosureTree
     included do
       belongs_to :parent,
         :class_name => _ct.model_class.to_s,
-        :foreign_key => _ct.parent_column_name
+        :foreign_key => _ct.parent_column_name,
+        :inverse_of => :children

       attr_accessible :parent if _ct.use_attr_accessible?

@@ -16,7 +17,8 @@ module ClosureTree
       has_many :children, *_ct.has_many_with_order_option(
         :class_name => _ct.model_class.to_s,
         :foreign_key => _ct.parent_column_name,
-        :dependent => _ct.options[:dependent])
+        :dependent => _ct.options[:dependent],
+        :inverse_of => :parent)

       has_many :ancestor_hierarchies, *_ct.has_many_without_order_option(
         :class_name => _ct.hierarchy_class_name,

@mceachen
Copy link
Collaborator

Sweet, thanks, but it seems like that doesn't quite make it work. Am I missing something?

Check out ce25cf1 — and the travis build: https://travis-ci.org/mceachen/closure_tree/builds/8485398

@mceachen
Copy link
Collaborator

In running this in the debugger, I think inverse is a red-herring.

I don't believe Rails supports bi-directional references when the parent doesn't have an ID.

This, for example, works without your patch—but note that I'm doing a create! on the parent, not just new:

  context "Parent/child inverse relationships" do
    it "should associate both sides of the parent and child relationships" do
      parent = Label.create!(:name => 'parent')
      child = parent.children.build(:name => 'child')
      parent.should be_root
      parent.should_not be_leaf
      child.should_not be_root
      child.should be_leaf
    end
  end

@brodock
Copy link
Contributor Author

brodock commented Jun 27, 2013

Oh, sorry, I missed one thing... you can't check for "parent_id", the id will only be filled after the database persistence, but "parent" will refer to the correct object.

You will probably want to check if it's "persisted" (.persisted?) to prevent a "useless" database query.

The idea is close to the code below:

def root?
  if persisted?
    _ct_parent_id.nil?
  else
    parent.nil?
  end
end

@mceachen
Copy link
Collaborator

Nice, I'll change it to that.

On Wednesday, June 26, 2013, Gabriel Mazetto wrote:

Oh, sorry, I missed one thing... you can't check for "parent_id", the id
will only be filled after the database persistence, but "parent" will
refer to the correct object.

You will probably want to check if it's "persisted" (.persisted?) to
prevent a "useless" database query.

The idea is close to the code below:

def root?
if persisted?
_ct_parent_id.nil?
else
parent.nil?
endend


Reply to this email directly or view it on GitHubhttps://github.com//issues/64#issuecomment-20092166
.

@mceachen
Copy link
Collaborator

@mceachen
Copy link
Collaborator

Released with v4.2.4. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants