Re: [Rails-core] Suggestion: `before_save on: :create` should either work or raise an exception

[Allam Marcos Campanini Matsubara]

+1 for alternative 1, "`before_save on: :create` and `before_save on: :update` could be supported (and the same for `after_save` and `after_commit`)".

On 10 November 2012 11:51, Nathan Long <natha...@gmail.com> wrote:

There's a small inconsistency in ActiveRecord's callback syntax that has tripped me up before. It wouldn't be a big deal, but it can lead to a silent failure. I'd like to suggest that it either be made consistent or be made to fail loudly.

The issue is that to do something before validating, but only when creating, you use `before_validation on: :create`, but to do something before saving, but only when creating, you use `before_create`. 

I have tried to use `before_save on: :create`, and it does not produce an error, but it does something unexpected: it saves before create **and** before update. This isn't really something an application's tests would catch, I don't think.

I propose one of the following changes:

1. `before_save on: :create` and `before_save on: :update` could be supported (and the same for `after_save` and `after_commit`)

2. Using `before_save on: :create` and the like could raise an exception saying either "this isn't supported" or "this isn't supported but will be eventually"

3. The `before_validation on: :create` syntax could be dropped in favor of `before_save_validation` and `before_create_validation`, so that all a user's hook declarations are either successful or get `NoMethodError`

Thoughts?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/CU3L9lEhvTkJ.
To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

--
Allam Marcos Campanini Matsubara

skype: allam.matsubara
twitter: @allam_matsubara
Phone: +55 (41) 8847-8677

[Rafael Mendonça França]

What is the difference between before_create :foo and before_save :foo, on: :create?

I think this is more a documentation issue and maybe (2.) can help to avoid confusion.

I don't like (1.) and (3.).

Rafael Mendonça França
http://twitter.com/rafaelfranca
https://github.com/rafaelfranca

[Nathan Long]

I think this is more a documentation issue and maybe (2.) can help to avoid confusion.

The downside of (2.) is committing to keep an inconsistent API. Rails is a large framework to learn already. It's great to have documentation, but it's even better when things are so consistent and obvious that you don't need the docs. That's optimizing for programmer happiness. :)

A possible advantage of (1.) is that is that it could be extended to hook into user-defined events, like `before_save on: :archive`, or `before_validation on: :publish`. But that's a separate discussion.

A possible advantage of (3.) is that it's more discoverable: `before_save_validation` and `before_create_validation` would be listed in `@model.methods` and in Rdoc.

[Rafael Mendonça França]

The problem with (1.) is that before_save :foo, on: :create and before_create :foo do the same thing. For me this cause more confusion.

About (3.) being listed in RDoc we can do the same right now and explicitly say that before_save doesn't accept :on option. Like a said this is more a documentation issue.

Rafael Mendonça França
http://twitter.com/rafaelfranca
https://github.com/rafaelfranca

To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/187nnOqlnhoJ.

[Adam Hunter]

validate :foo, presence: true

and

validate_presence_of :foo

Also do the same thing.  This is a slightly different use case but I'm +1 for making before_save consistent with before_validation as far as an :on option goes. This has tripped me up before as well. 

Thanks,

Adam Hunter  

Sent from my iPhone

[Nathan Long]

Rafael - Now I see what you mean; you don't want two names for the same thing. I agree.

What I was trying to say is let's pick a consistent callback syntax, support that, and deprecate the others. This doesn't have to be done quickly; I just think it's a good direction to head.