brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.05k stars 356 forks source link

Enum does not scope correctly #277

Closed scottmalone closed 7 years ago

scottmalone commented 7 years ago

Env: Rails 4.2.0, acts_as_list (0.9.5), postgresql.

Problem

Scope not working with enum resulting to enum being set to 0.

Solution

Change assignments within #check_scope to send to the setter.

def check_scope
  if internal_scope_changed?
    cached_changes = changes

    cached_changes.each { |attribute, values| send("#{attribute}=", values[0]) }
    send('decrement_positions_on_lower_items') if lower_item
    cached_changes.each { |attribute, values| send("#{attribute}=", values[1]) }

    send("add_to_list_#{add_new_at}") if add_new_at.present?
  end
end

The existing enum test in test_list.rb doesn't catch the issue because the assignment using self[attribute] = values[0] always assigns as 0 since the enum value of "active" or "archived" doesn't exist (active/archived are the enum keys). Assignment as 0 results in the position always being set to the first position.

I ran into this because I declared an enum as enum status: { inbox: 1, published: 2, archived: 3, deleted: 4 } with the values starting at 1, but the existing test can be improved by just adding a second EnumArrayScopeListMixin to one of those scopes.

brendon commented 7 years ago

Hi @scottmalone, that sounds interesting! Would you be interested in writing a Pull Request for us with a test? Doesn't sound like it'll be too hard :)

scottmalone commented 7 years ago

No prob @brendon, I'll do that.

I ran into a separate issue that's really confounding me w/ model specs not updating correctly but request specs doing fine (yes, using test_after_commit). Would you prefer I open another issue or go straight to a separate PR?

brendon commented 7 years ago

Thanks @scottmalone, yes open another issue then if you do a PR just mention that it closes that issue in the description and that should handle linking things together.