brendon / acts_as_list

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

Undefined instance variable @scope_changed since 0.7.3 #199

Closed gamecreature closed 8 years ago

gamecreature commented 8 years ago

I'm using spree which combines acts_as_list and awesome_nested_set set.

With 0.7.2 everything works perfectly With version 0.7.3 the error below happens. After some painful debugging I noticed @scope_changed was coming from acts_as_list. Reverting acts_as_list to the 0.7.2 versions fixes it.

I assume the reason is there isn't an instance_variable @scope_changed when the record is committed.

So I guess the line of commit (5cba2f292feca6f7f54209b48ff57077ad1dbbcf)

after_save '@scope_changed = false'

was pretty important, for defining this variable?

NameError: instance variable @scope_changed not defined
(eval):1:in `remove_instance_variable'
(eval):1:in `block in make_lambda'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:435:in `instance_exec'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:435:in `block in make_lambda'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:263:in `call'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:263:in `block in simple'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `call'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `block in call'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `each'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:506:in `call'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:92:in `__run_callbacks__'
/var/lib/gems/2.2.0/gems/activesupport-4.2.6/lib/active_support/callbacks.rb:778:in `_run_commit_callbacks'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:314:in `committed!'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/transaction.rb:89:in `commit_records'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/transaction.rb:153:in `commit'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/transaction.rb:175:in `commit_transaction'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `within_new_transaction'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:220:in `transaction'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/transactions.rb:291:in `save!'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:41:in `set_left_and_rights'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:38:in `block in set_left_and_rights'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:38:in `set_left_and_rights'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:38:in `block in set_left_and_rights'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:38:in `set_left_and_rights'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:25:in `block in rebuild!'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation/delegation.rb:46:in `each'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/tree.rb:22:in `rebuild!'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/model/rebuildable.rb:15:in `block in rebuild!'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/scoping/default.rb:33:in `block in unscoped'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/relation.rb:302:in `scoping'
/var/lib/gems/2.2.0/gems/activerecord-4.2.6/lib/active_record/scoping/default.rb:33:in `unscoped'
/var/lib/gems/2.2.0/gems/awesome_nested_set-3.0.3/lib/awesome_nested_set/model/rebuildable.rb:14:in `rebuild!'
Tasks: TOP => bp:cron
brendon commented 8 years ago

Hi there, so sorry to see that caused a bug. I don't think it's that line, rather, we're not checking for the existence of the instance variable before trying to remove it.

How about this as a solution?

after_commit 'remove_instance_variable(:@scope_changed) if defined?(@scope_changed)'

Try bundling this branch and see if that fixes the problem. The tests probably didn't pick this up because the tests all trigger the creation of the variable.

gamecreature commented 8 years ago

Thank you very much for your quick response This indeed fixes my problem! 👍

brendon commented 8 years ago

I'm glad to hear it :) I think the actual reason the tests didn't pick up the problem is that tests are run in a transaction, so this piece of code actually never runs. I'm looking to add something that tests that callback and I'll push a new gem version soon.

brendon commented 8 years ago

0.7.4 should work for you now :) Thanks again for raising this.

gamecreature commented 8 years ago

Thank you for fixing this...!! 👍

damianlegawiec commented 8 years ago

Thanks @brendon for the quick fix!

brendon commented 8 years ago

You're welcome. I've yanked the 0.7.3 gem to be sure.

swanandp commented 8 years ago

Came here to say please yank the 0.7.3 version.