This project is archived and is in readonly mode.

#1903 ✓hold
Justin French

errors.on(:att) should not return a String

Reported by Justin French | February 7th, 2009 @ 05:11 AM | in 2.x

I find it very surprising and awkward that ActiveRecord's errors.on(:attribute) can return nil, a String (when there's only one error) or an Array (more than one error).

It's more common elsewhere in ActiveRecord to return an array even when there's only item... find(:all) is a perfect example.

I've attached a patch which changes the behavior so that an array is returned when 1 or more errors exist on the attribute, which will mean developers no longer have to write special code to handle the String special case.

This change is mostly backwards compatible:

Developers should have already been expecting either nil, a String or an Array of errors, so this change just negates the need for the String handling in their code.

However, there's probably a lot of validation unit tests out there expecting Strings rather than Arrays.

Comments and changes to this ticket

  • Xavier Shay

    Xavier Shay February 7th, 2009 @ 05:21 AM

    yes please, I hate the previous behavior. I want it to return an empty array rather than nil also, but that would probably break too much stuff

  • Justin French

    Justin French February 7th, 2009 @ 06:10 AM

    yeah, i'd love an empty array too, and i started on that too, but I see a lot of code checking 'errors.on(:foo)' rather than 'errors.on(:foo).blank?'

  • DHH

    DHH February 7th, 2009 @ 01:41 PM

    • State changed from “new” to “hold”

    This is not going to be backwards compatible and I'm sorta on the fence about whether it's even a good idea. If you're primarily working with 1 error per field, it'd be a hassle to deal with the array. And if you are thinking there might be multiple, couldn't you just wrap the call in Array()?

  • Justin French

    Justin French February 8th, 2009 @ 12:54 AM

    I agree this isn't just a quick patch we can squeeze into stable any time we like, but I just don't buy the idea that most people, most of the time will only have or expect one error on a field. Nor can I buy the idea that they'd expect a String instead of an array for a pluralized method name like errors.

    And (aside from unit tests) it's not THAT much of a compatibility problem, is it? For those that were expecting a string Array#to_s returns the string, and to_s is called by ERB.

    I've seen plenty of developers have a "WTF?" moment on this. They all expected an array of one object. Hey, they all expected an empty array for zero errors too, so let's fix that!

    Major releases like 3.0 are an opportunity to reduce those WTF moments.

  • Jonas Schneider

    Jonas Schneider February 9th, 2009 @ 09:14 PM

    +1 for me, for sure. stumbled across this stuff so many times now... But it should be included first in Rails3 or something (where we break bc anyway :D)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href=""></a>

People watching this ticket