[4.1.0.beta1] Enum: build and first_or_create/populate_with_current_scope_attributes broken #13530

Closed
zealot128 opened this Issue Dec 29, 2013 · 12 comments

6 participants

@zealot128

Using enum in combination with first_or_create et.al. doesnt work:

class Issue < ActiveRecord::Base
  enum :status, [ :open, :finished]
end

Issue.open.first_or_initialize # Exception, full below
Issue.where(status: 0).first_or_initialize # same Exception
# even overriding not work:
Issue.where(status: 0).first_or_initialize(status: :open) # same Exception

# Exception: ArgumentError: '0' is not a valid level
# --
# 0: activerecord-4.1.0.beta1/lib/active_record/enum.rb:66:in `block (3 levels) in enum'
# 1: activerecord-4.1.0.beta1/lib/active_record/scoping.rb:26:in `block in populate_with_current_scope_attributes'
# 2: activerecord-4.1.0.beta1/lib/active_record/scoping.rb:25:in `each'
# 3: activerecord-4.1.0.beta1/lib/active_record/scoping.rb:25:in `populate_with_current_scope_attributes'
# 4: activerecord-4.1.0.beta1/lib/active_record/core.rb:177:in `initialize'
# 5: activerecord-4.1.0.beta1/lib/active_record/inheritance.rb:27:in `new'
# 6: activerecord-4.1.0.beta1/lib/active_record/inheritance.rb:27:in `new'
# 7: activerecord-4.1.0.beta1/lib/active_record/relation.rb:108:in `block in new'
# 8: activerecord-4.1.0.beta1/lib/active_record/relation.rb:285:in `scoping'
# 9: activerecord-4.1.0.beta1/lib/active_record/relation.rb:108:in `new'

Same happens when implicit creating a new object via association:

class User < ActiveRecord::Base
   has_many :open_issues, -> { open }, class_name: :Issue
end

User.first.open_issues.build # exception

# Exception: ArgumentError: '0' is not a valid level
# --
# 0: activerecord-4.1.0.beta1/lib/active_record/enum.rb:66:in `block (3 levels) in enum'
# 1: activerecord-4.1.0.beta1/lib/active_record/attribute_assignment.rb:45:in `public_send'
# 2: activerecord-4.1.0.beta1/lib/active_record/attribute_assignment.rb:45:in `_assign_attribute'
# 3: activerecord-4.1.0.beta1/lib/active_record/attribute_assignment.rb:32:in `block in assign_attributes'
# 4: activerecord-4.1.0.beta1/lib/active_record/attribute_assignment.rb:26:in `each'
# 5: activerecord-4.1.0.beta1/lib/active_record/attribute_assignment.rb:26:in `assign_attributes'
# 6: activerecord-4.1.0.beta1/lib/active_record/associations/association.rb:169:in `initialize_attributes'
# 7: activerecord-4.1.0.beta1/lib/active_record/associations/association.rb:248:in `block in build_record'
# 8: activerecord-4.1.0.beta1/lib/active_record/core.rb:183:in `initialize'
# 9: activerecord-4.1.0.beta1/lib/active_record/inheritance.rb:27:in `new'
@robin850
Ruby on Rails member

Hello @zealot128,

I think that your code has design problems ; Rails is not the culprit. You should either call :

Item.where("status <> 0")

or

Item.where(status: :open)

Also Active Record will not create a scope for each enum as far as I can see. You will have to define the open scope yourself :

scope :open, -> { where("status <> 0") }

However, I'm sometimes wrong on Active Record issues so I will let you or a core member close this issue in case I'm right.

Thanks for reporting anyway!

@zealot128

@robin850 I just used the scope in the block for demonstration purpose. The most important thing: How can I, e.g. ask for the first open issue or create one?

Issue.where(status: 0).first_or_initialize        # enum type exception
Issue.where(status: :open).first_or_initialize # database type exception
Issue.open.first_or_initialize                          # same enum type exception

both doesn't work.

Yes, enums create activerecord scopes (defined here: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/enum.rb#L79)

@robin850 robin850 added a commit to robin850/rails that referenced this issue Dec 30, 2013
@robin850 robin850 Fix the enums writer methods
Previously, the writer methods would simply check whether the passed
argument was the symbol representing the integer value of an enum field.
Therefore, it was not possible to specify the numeric value itself but
the dynamically defined scopes generate where clauses relying on this
kind of values so a chained call to a method like `find_or_initialize_by`
would trigger an `ArgumentError`.

Reference #13530
eeb28e1
@robin850
Ruby on Rails member

@zealot128 : Sorry you are totally right! I've investigated this a bit and there is several issues actually. I've a tiny patch to fix the issue with Issue.a_query_method.first_or_initialize (it would be very nice if you could try it on your application and give some feedback! ❤️ ).

However there is still another issue here with where:

class Book < ActiveRecord::Base
  enum status: [:proposed, :written]
end

Book.where(status: :proposed).to_sql
# => "SELECT  "books".* FROM "books"  WHERE "books"."status" = 'proposed'  ORDER BY "books"."id" ASC LIMIT 1
Book.where(status: 0).to_sql
# => "SELECT  "books".* FROM "books"  WHERE "books"."status" = 0  ORDER BY "books"."id" ASC LIMIT 1"

I don't know if this is intended but at least this is not consistent and this make first_or_initialize more or less broken because when no records are found, the instantiated object's status attribute is either correctly filled in or not.

@robin850
Ruby on Rails member

I'm not sure to be able (and have the time) to look at the problem with where so if anyone is interested, here is the failing gist.

@robin850 robin850 added a commit to robin850/rails that referenced this issue Dec 31, 2013
@robin850 robin850 Fix the enums writer methods
Previously, the writer methods would simply check whether the passed
argument was the symbol representing the integer value of an enum field.
Therefore, it was not possible to specify the numeric value itself but
the dynamically defined scopes generate where clauses relying on this
kind of values so a chained call to a method like `find_or_initialize_by`
would trigger an `ArgumentError`.

Reference #13530
7bd30d8
@robin850 robin850 added a commit to robin850/rails that referenced this issue Jan 1, 2014
@robin850 robin850 Fix the enums writer methods
Previously, the writer methods would simply check whether the passed
argument was the symbol representing the integer value of an enum field.
Therefore, it was not possible to specify the numeric value itself but
the dynamically defined scopes generate where clauses relying on this
kind of values so a chained call to a method like `find_or_initialize_by`
would trigger an `ArgumentError`.

Reference #13530
7aebcb6
@rafaelfranca
Ruby on Rails member

Closed by #13542

@rafaelfranca
Ruby on Rails member
Book.where(status: :proposed)

Is not supported yet and I'm not sure if it will. @dhh what do you think?

Right now, in our documentation we are explaining that we should use the constant to do queries using enum values:

  # Use that constant when you need to know the ordinal value of an enum:
  #
  #   Conversation.where("status <> ?", Conversation::STATUS[:archived])
@rafaelfranca rafaelfranca reopened this Jan 1, 2014
@dhh
Ruby on Rails member
@robin850
Ruby on Rails member

@dhh : The problem is that the current scopes don't work correctly with first_or_initialize but where(status: :proposed) generates wrong queries:

>> Book.proposed.first_or_initialize.proposed?
  Book Load (0.2ms)  SELECT  "books".* FROM "books"  WHERE "books"."status" = 0  ORDER BY "books"."id" ASC LIMIT 1
=> false
>> Book.where(status: :proposed).first_or_initialize.proposed?
  Book Load (0.3ms)  SELECT  "books".* FROM "books"  WHERE "books"."status" = 'proposed'  ORDER BY "books"."id" ASC LIMIT 1
=> true
@rafaelfranca
Ruby on Rails member

@robin850 this happen because Book.where(status: :proposed) is not supported and will be not. So users should not do that. I'll document this behavior.

@robin850
Ruby on Rails member

@rafaelfranca : But it's strange that it correctly sets the status value while it doesn't with the supported version ; at the very least we have a bug at this level

>> Book.where(status: 0).first_or_initialize
=> #<Book id: nil, status: nil>
@chancancode chancancode added a commit that closed this issue Jan 3, 2014
@chancancode chancancode Building new records with enum scopes now works as expected
Previously, this would give an `ArgumentError`:

   class Issue < ActiveRecord::Base
     enum :status, [:open, :finished]
   end

   Issue.open.build # => ArgumentError: '0' is not a valid status
   Issue.open.create # => ArgumentError: '0' is not a valid status

PR #13542 muted the error, but the issue remains. This commit fixes
the issue by allowing the enum value to be written directly via the
setter:

   Issue.new.status = 0 # This now sets status to :open

Assigning a value directly via the setter like this is not part of the
documented public API, so users should not rely on this behavior.

Closes #13530.
788bb40
@chancancode
Ruby on Rails member

Yuupp. Book.proposed.first_or_initialize.proposed? (and hence Book.where(status: 0).first_or_initialize) not working as expected is a boogg.

Fixed via 788bb40.

For this to work, we have to either make where(status: :proposed) work and use that in the generated scope, or we'll have to accept enum values in the enum writers. Since @robin850 already went down the path of the latter in his PR, I went with that for now, but we can revisit this.

FYI @dhh @senny

@robin850
Ruby on Rails member

Whoah, thanks @chancancode!

@senny senny added the enum label Sep 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment