I definitely agree that there are some 'gotchas' here. I can't think of any disadvantages to switching to using `Integer()` instead of `to_i` - as you say, this would only raise an exception in instances where the user has a bug in their code, which is a good thing to discover. I don't think we could change this elsewhere though.
I also think having a new scope for the enum generated such as `Conversation .with_status(string_or_num_status)` would be a nice helper in cases where you have a dynamic value as it lets you avoid the alternative `Conversation.send(@some_conversation.status)` or `Conversation.where(status: Conversation.statuses(@some_conversation.status))`. On Thursday, October 23, 2014 7:02:27 AM UTC+11, Justin Gordon wrote: > > I wrote up this detailed description of the issues I'm having with Rails > Enums: > > Enums and Queries in Rails 4.1, and Understanding Ruby > <http://www.railsonmaui.com/blog/2014/10/22/enums-and-queries-in-rails-4-dot-1/> > > At the bottom of the article, I post my recommendation: > > > Recommendations to the Rails Core Team > > In response to this issue, I submitted this github issue: Rails where > query should see value is an enum and convert a string #17226 > <https://github.com/rails/rails/issues/17226> > > 1. @Bounga and @rafaelfranca on Github suggest that we can’t > automatically convert enum string values in queries. I think that is true > for converting cases of a ? or a named param, but I suspect that a > quick map lookup to see that the attribute is an enum, and a string is > passed, and then converting the string value to an integer is the right > thing to do for 2 reasons: > 1. This is the sort of “magic” that I expect from Rails. > 2. Existing methods find_or_initialize_by and find_or_create_by will > result in obscure bugs when string params are passed for enums. > > However, it’s worth considering if all default accessor methods > (setters) should be consistently be called for purposes of passing values > in a map to such methods. I would venture that Rails enums are some Rails > provided magic, and thus they should have a special case. If this > shouldn’t > go into Rails, then possibly a gem extension could provide a method like > Model.where_with_enum which would convert a String into the proper > numerical value for the enum. I’m not a huge fan of the generated Model > scopes for enums, as *I like to see what database field is being > queried against.* > 2. Aside from putting automatic conversion of the enum hash > attributes, I recommend we change the automatic conversion of Strings to > integers to use the stricter Integer(some_string) rather than > some_string.to_i. The difference is considerable, String#to_i is > extremely permissive. Try it in a console. With the to_i method, any > number characters at the beginning of the String are converted to an > Integer. If the first character is not a number, *0 is returned*, > which is almost certainly a default enum value. Thus, this simple change > would make it *extremely* clear when an enum string is improperly > used. I would guess that this would make some existing code crash, but in > all circumstances for a valid reason. As to whether this change should be > done for all integer attributes is a different discussion, as that could > have backwards compatibility ramifications. This change would require > changing the tests in ActiveRecord::ConnectionAdapters::TypesTest > > <https://github.com/rails/rails/blob/master/activerecord/test/cases/types_test.rb>. > > For example, this test: > > 1 > > assert_equal 0, type.type_cast_from_user('bad') > > would change to throw an exception, unless the cases are restricted to > using Integer.new() for enums. It is inconsistent that some type > conversions throw exceptions, such as converting a symbol to an integer. > Whether or not they should is much larger issue. In the case of enums, *I > definitely believe that proper enum string value should not silently > convert to zero every time.* > > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
