Update ActiveRecord::AttributeMethods#attribute_present? to return false for empty strings #5316

Merged
merged 1 commit into from Mar 7, 2012

Projects

None yet

3 participants

Contributor
Jacobkg commented Mar 7, 2012

Right now, attribute_present? returns true on empty strings, when the documentation says that it should return false:

"Returns true if the specified +attribute+ has been set by the user or by a database load and is neither
nil nor empty? (the latter only applies to objects that respond to empty?, most notably Strings)."

This change fixes the function to correctly return false for empty strings. Test is included.

Fixes #5314

Member

Thanks for the report. We should just use value.present? in the implementation, no? In any case, why would someone use such method? We have better approaches today like user.name? or even user.attribute?(:name) which does basically the same thing.

Contributor
Jacobkg commented Mar 7, 2012

I agree with you that this method (which as far as I can tell is super old) is ripe for deprecation.

The problem with using value.present? is that false.present? is false, which leads to the odd behavior (and previously documented bug) where you set an attribute to false, and then attribute_present? returns false. See #1613.

Owner

Yeah. This fix is fine to me. Using value.present? will not work like expected.

Another thing, user.name? and user.attribute?(:name) do not work in the same way that user.attribute_present?(:name).

@josevalim josevalim merged commit 3da31b9 into rails:master Mar 7, 2012
Member

Should I backport this to 3-2-stable?

Owner

Yes. It is a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment