Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't delegate connection to self #436

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

davidcelis
Copy link
Contributor

@davidcelis davidcelis commented Jun 5, 2024

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
Copy link
Owner

brendon commented Jun 5, 2024

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 brendon merged commit 52da91b into brendon:master Jun 5, 2024
27 checks passed
@brendon
Copy link
Owner

brendon commented Jun 5, 2024

Released as 1.2.1.

@davidcelis
Copy link
Contributor Author

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

@davidcelis davidcelis deleted the fix-connection branch June 5, 2024 20:15
@brendon
Copy link
Owner

brendon commented Jun 5, 2024

No worries at all :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants