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

Don't delegate `connection` to `self` #436

Closed davidcelis closed 3 months ago

davidcelis commented 3 months ago

We ran into an issue with using acts_as_list on models that define a connection method or have a connection association (we have several). Any model with a connection association, for example, was raising an error on save as soon as acts_as_list was added:

     NoMethodError:
       undefined method `marked_for_destruction?' for #<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x0000ffff67b71210 @transaction_manager=#<ActiveRecord::ConnectionAdapters::TransactionManager:0x0000ffff67b71198 @stack=[], @connection=#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x0000ffff67b71210 ...>...>

This turned out to be because our connection association was being re-defined by acts_as_list with a delegate :connection, to: :self. Rails has no underlying problems with models having a connection instance method, and I believe typically you should be relying on the class to grab a connection rather than the instance/record.

Because it seems like this delegator is used only for convenience purposes, I think it should be dropped for compatibility reasons and replaced with more explicit calls to self.class.connection

brendon commented 3 months ago

Agreed! Sorry about that 😳 This gem is terrible for stomping all over the model namespace in various ways! Most of it was from before my time thankfully :D

brendon commented 3 months ago

Released as 1.2.1.

davidcelis commented 3 months ago

Oh, awesome! Thanks so much for releasing the fix so quickly 🎉

brendon commented 3 months ago

No worries at all :)