proposal ActiveRecord::Base#saved?

Hi Core List,

Sometimes I'm required to keep track of whether an activerecord object was succesfully saved.

A case in point would be doing something in an after_filter of a controller. For example, I might want to keep track of how many times users post invalid active records.

One way of doing this would be to use #valid?, but this will hit the database again in some cases (e.g. if there's a validates_uniqueness_of), which I want to avoid.

I'm proposing a very simple API, as described here http://www.pastie.org/268045. To make this work is also very simple (here's a plugin verision http://pastie.org/268046).

Before I bundle this up into a patch with tests, I'm wondering if people think this is a good or bad idea?

Cheers, Ian

On a past project where we wanted something like an after_filter to check whether the user input was valid, it just looked at @whatever.errors.empty? to avoid rerunning the validations. Obviously that makes assumptions about what the action has done, but I think they're the same assumptions you'd have to make to use your API.

That seems like a good idea for an application that needs to track that in a few places, but I'd rather see it stay in a plugin than become part of core. It seems to crowd the API a bit, and adds something not many people need.

I could see people getting confused by the following, thinking #saved? should be the opposite of #new_record?.

  t = Thing.find(:first)   t.new_record? # => false   t.saved? # => nil   puts "But it's in the database! It is sooo saved!"

If people like this feature, I'd like to see #saved? renamed to reduce the likelihood of the above. Maybe "instance_saved?" is better, but I can still imagine people not grokking it until it's bitten them.

On a separate note, I think it's surprising to have a predicate method return nil.

-hume.

Ian White wrote:

Hi Core List,

Sometimes I'm required to keep track of whether an activerecord object
was succesfully saved.

A case in point would be doing something in an after_filter of a
controller. For example, I might want to keep track of how many times
users post invalid active records.

One way of doing this would be to use #valid?, but this will hit the
database again in some cases (e.g. if there's a
validates_uniqueness_of), which I want to avoid.

I'm proposing a very simple API, as described here http://www.pastie.org/268045 . To make this work is also very simple (here's a plugin verision http://pastie.org/268046) .

Before I bundle this up into a patch with tests, I'm wondering if
people think this is a good or bad idea?   

Could something be done with with the change tracking in 2.1?

How about: !(model.new_record? || model.changed?)

This would only be true if the record was saved or it was an update with no changes.

Jack

The point here is to be able to use that in a filter, since otherwise you do

   if model.save      # saved    end

In my view the definition of the predicates is going to depend on your particular needs. If you save and change an attribute in the next line what should the flags say? I don't really see it in the core API.

Hey guys,

Xavier Noria wrote:

If you save and change an attribute in the next line what should the flags say?

.saved? should be true - I'm just proposing something pretty simple (what was the result of the last #save ?)

Jack Christensen wrote:

!(model.new_record? || model.changed?)

This is great idea - it doesn't work for the reload case, but I don't think it needs to for the controller case

John D. Hume wrote:

it just looked at @whatever.errors.empty?

This also works great - and you can use the existence of the @errors var to find if a save (or validation) has been attempted. (Also doesn't work for the reload case)

However, making #saved? depend on the existing internals would be a bad idea in my view, because it'd so much clearer, and less prone to break if these other things change in implementation, to just have an instance variable tracking this

But I take your point on the semantics of saved? - it should be called #instance_saved?, in which case it's fugly and seems to show that it probably shouldn't be in the API

Summarizing: You can already do (most of) this, using ARs state. And it's a bit undermotivated when fleshed out. So keep as a plugin, if it's required at all.

Thanks very much for the input guys! - Saved me cluttering up lighthouse.

Cheers, Ian