ActiveRecord::Base#toggle! - bad implementation or documentation

ActiveRecord::Base#toggle! is used to toggle an attribute true or false, and immediatly save it. It is implemented to call the toggle method, and then call update_attribute to save to the database.

The problem I see with this is that there is no validation, when toggling a boolean. In my opinion this is wrong, and there should be validation. I've heard people say this behaviour was correct, and that's how they wanted it, in which case the documentation is incomplete and can lead to unexpected results.

def toggle!(attribute) toggle(attribute).update_attribute(attribute, self[attribute]) end
You tell me, a patch to make toggle! validate or a documentation patch to say toggle! doesn't validate when saving.

I have a ticket open at http://dev.rubyonrails.org/ticket/11098 about this.

Regards,

Jan

I'd personally love to see #toggle! removed from ActiveRecord
entirely. I can't think of a single use case for #toggle! that isn't
bug-prone, or that wouldn't be better implemented by setting flags to
explicit values.

Tammer

I’d personally love to see #toggle! removed from ActiveRecord

entirely. I can’t think of a single use case for #toggle! that isn’t

bug-prone, or that wouldn’t be better implemented by setting flags to

explicit values.

I heartily second this. I haven’t used it since I ran into a situation where I was mis-guessing the initial state of a boolean.

::Jack Danger

Sure, I can: boolean fields that can be toggled by administrators. I
use a fair number of these in a project I'm doing. #toggle! is
exactly what I would have implemented had Rails not had the feature to
begin with.

James

I'd call that a bad practice, and I think it's commonly used only
because of the existence of AR's #toggle! method.

The "toggling" an admin flag via a web form is really "setting the
flag to true or false, depending on what the form thinks it currently
is". The problem with using #toggle! here is that when two
administrators both load the form and then both click the "publish"
button (or whatnot), the record will end up being unpublished.

The correct solution is very easy in a restful application, where the
"publish" link should put params[:record][:published] = true to the
update action. But even in non-restful applications, having the link
post to #publish and #unpublish, depending on the state of the record
when the page is loaded is much more explicit, and less error prone.

This, and the fact that #toggle! ignores validates (the original point
of the thread), makes #toggle! stick out as a very non-rails way of
doing things.

The existence of #toggle! encourages its use. It should be removed,
if only to guide new rails programmers toward best practices.

Tammer

Of course, the correct solution would be not to use a link (or GET) to do such an action in the first place. Other than, agreed.

//jarkko

Also, if for whatever reason the request was retryed (by a proxy
perhaps) even a single user's actions could result in an unknown final
toggle state.

+1 for removal

Also, if for whatever reason the request was retryed (by a proxy
perhaps) even a single user's actions could result in an unknown final
toggle state.

+1 for removal

Proxies don't retry non-idempotent requests. If they did we'd have a
whole pile of other issues.

Base#toggle works just fine for its intended use case, toggling a
boolean value. There are dozens of cases where this is perfectly
valid and the concurrency issues raised aren't the end of the world.
tadalist's todo lists for example.

Some patches to improve the documentation would be greatly
appreciated, but the feature's definitely not 'useless' :slight_smile: