Default value for belongs_to? about nobrainer HOT 16 CLOSED

Sure

from nobrainer.

I'm almost a bit disappointed there was no debate about leaky abstractions or surprises :)

from nobrainer.

haha :) To be fair, I am as well. It was a weird feeling and but pleasant at the same time.

I've put some more thoughts around it. To generalize, you are saying that associations should behave like fields whenever as possible.

Another example of this is something like:

post = Post.first

# we should be able to do this:
  Comment.where(:post => post)
# instead of this:
  Comment.where(:post_id => post.id)

from nobrainer.

Yeah, exactly.

Another sort of similar (although I guess converse) example I just ran into is this:

class Foo
  belongs_to :bar, required: true
end

f = Foo.new bar_id: '531fd406caa9d02db5000001'
f.save # fails with "bar must be present" validation

from nobrainer.

This on the other hand is a bug that needs to be fixed.

from nobrainer.

Yeah, I kind of thought that as well. On the other hand, I could see an argument that just accepting any old string id for a required association isn't very rigorous. We're not verifying that the associated document actually exists and is persisted.

I suppose that NoBrainer could perform that check if the object hasn't been set directly, but that mostly negates the reason I'm setting the association like this in the first place: avoiding the extra query for a known valid id.

from nobrainer.

Basically, the presence validator should do the following (field is the belongs_to association in question):

• if field is not set and the field_id is not set, we should get a validation error.
• if field is set, and field_id != field.id, we should get a validation error.
• if field is set, and field.persisted? is not true (does not trigger a db query, it's okay), we should get a validation error.
• otherwise we are good.

from nobrainer.

Sorry. The presence validator should just do the first. The rest is normal behavior, user defined validators or not.

from nobrainer.

Sounds good to me.

from nobrainer.

I've added some test around the bug you described: 7ee860a

They are all green. There is actually no bug. The current behavior is better than what I proposed:
self.field is cleared whenever self.field_id is manually set, and self.field_id is set to self.field.id when self.field is set.

The validation failed in your example because Bar.find("531fd406caa9d02db5000001") failed. The target of the belong_to association has to be in the database when using a presence validator on the belongs_to association.

There are some optimization to be done though: run field validations only when the field changes. This will avoid a database fetch for presence validators on belongs_to associations.

from nobrainer.

By the way, how do you plan to use the default feature? (show the code you would have)

from nobrainer.

I've added support for where() clauses with belongs_to associations: 623a3c4

I'm not sure about the default option... What would happen if you specify both a default to field_id and the association. Which one would win in case of conflicts?

What happen when fetching a model that has a default on the association? We would have to fetch the association to see if it indeed exists to see if we need to apply the default. That's a pretty big performance issue because the defaults are assigned after initializing the model. We'd have to make the default assignments on demand. This is quite a lot of work. The other solution is to change our definition of existence to piggy back on the foreign key, and we would have to make the presence validator weaker which is not great.

Implementing the default option require a fair amount of work, and provide only little benefits over assigning a default value to the foreign key:

class Comment
  belongs_to :post
  field :post_id, :default => { Post.create(:comment => self).id }
end

Something a little more intense could be done in a after_initialize callback.

Another workaround would be to override the post getter and auto create if needed.

Also, since NoBrainer never autosaves a model, the default value feature would be fairly limited. Having a create() in default value block is not really something I would encourage without database transactions or some atomic mechanism. The default feature for belongs_to seems hard to do well.

from nobrainer.

Also, instead of running a validator only when its corresponding field changed, it might be preferable to specify :required => true not on the belongs_to association, but on its foreign key. This way, the user makes the choice of having weaker semantics to avoid an extra db query, or do the extra query and run the presence validation on the association.

from nobrainer.

If you can specify the foreign key field explicitly and it will work fine with the belongs_to definition, then that's totally fine. For some reason I figured this couldn't be done.

They are all green. There is actually no bug. The current behavior is better than what I proposed:
self.field is cleared whenever self.field_id is manually set, and self.field_id is set to self.field.id when self.field is set.

Yeah, this is my bad. My case was actually a bit more specialized than I was reporting (and I confused myself into thinking it was the general case). I was using a self-referencing association:

class Foo
  belongs_to :parent, class_name: 'Foo', required: true
end
f = Foo.new
f.parent_id = f.id
f.save # fails with "parent must be present" validation

I guess this is expected based on what you've described for the presence validation. Sounds like the simple workaround is to specify it like this:

class Foo
  belongs_to :parent, class_name: 'Foo'
  field :parent_id, required: true
end

from nobrainer.

Yes, I'm going to add some clarifications in the documentation.

from nobrainer.

http://nobrainer.io/docs/validations/#presence_validations_on_belongs_to_associations

from nobrainer.