padrino / padrino-framework

Padrino is a full-stack ruby framework built upon Sinatra.
http://www.padrinorb.com
MIT License
3.37k stars 509 forks source link

[Bug] 0.15.2 undefined method `update_attributes' for all models on edit/update submission on clean admin app and older apps #2266

Closed wakatara closed 1 year ago

wakatara commented 1 year ago

Do you want to request a feature or report a bug?

Bug! šŸ› (weird one and one of the very last things to fix before AArk app is fully upgraded. =] ).

What is the current behavior?

Whenever an edit action is performed and submitted, both in my upgraded app and in a fresh padrino app using the padrino-gen admin generator in 0.15.2 (so pure clean one), submission gives a:

undefined methodupdate_attributes' ` error. For AR/AS/AM 7.0.4 and postgres (also, in the cleanly generated padrino app, with SQLIte).

To reproduce.

padrino-gen admin-test --orm active-record -e haml
cd admin-test
bundle install --binstubs
padrino-gen admin
rake db:migrate
rake db:seed

then log into application. Create a new account. Try to edit it. Submit will throw the error.

Backtrace:


NoMethodError at /admin/accounts/update/2
undefined method `update_attributes' for #<Account id: 2, name: "Some Dude", surname: "lastname", email: "yabba@dabbadoo.com", crypted_password: "$2a$12$fDGcoogxqHS3EEngUSJDDO6PEvGUcVnFI3dOBzE.tGT...", role: "admin", created_at: "2023-01-11 10:57:25.735345000 +0000", updated_at: "2023-01-11 10:57:25.735345000 +0000">

    file: attribute_methods.rb location: method_missing line: 458 

 /Users/daryl/Code/dev/admin-test/admin/controllers/accounts.rb in block (2 levels) in <top (required)>

          if @account.update_attributes(params[:account])

/Users/daryl/Code/dev/admin-test/lib/connection_pool_management_middleware.rb in block in call

        ActiveRecord::Base.connection_pool.with_connection { @app.call(env) }

/Users/daryl/Code/dev/admin-test/lib/connection_pool_management_middleware.rb in call

        ActiveRecord::Base.connection_pool.with_connection { @app.call(env) }

So, appears to have something to do with active record or connection pool management. I have the option use ConnectionPoolManagement on in admin/app.rb (it is set by default in the clean admin-test app though also in my older app I'm upgrading.

What is the expected behavior?

The expected behaviour is that a submitted update gets taken up by the DB

Which versions of Ruby, Padrino, Sinatra, Rack, OS are you using? Did this work in previous versions?

ruby 3.2 Padrino 0.15.2 OSX 13.1 Venture (M1 chipped Macbook)

Gemfile. lock

GEM remote: https://rubygems.org/ specs: activemodel (7.0.4) activesupport (= 7.0.4) activerecord (7.0.4) activemodel (= 7.0.4) activesupport (= 7.0.4) activesupport (7.0.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) bcrypt (3.1.18) concurrent-ruby (1.1.10) daemons (1.4.1) date (3.3.3) eventmachine (1.2.7) haml (5.2.2) temple (>= 0.8.0) tilt i18n (1.12.0) concurrent-ruby (~> 1.0) mail (2.8.0) mini_mime (>= 0.1.1) net-imap net-pop net-smtp mime-types (3.4.1) mime-types-data (~> 3.2015) mime-types-data (3.2022.0105) mini_mime (1.1.2) mini_portile2 (2.8.1) minitest (5.17.0) moneta (1.1.1) mustermann (3.0.0) ruby2_keywords (~> 0.0.1) net-imap (0.3.4) date net-protocol net-pop (0.1.2) net-protocol net-protocol (0.2.1) timeout net-smtp (0.3.3) net-protocol padrino (0.15.2) padrino-admin (= 0.15.2) padrino-cache (= 0.15.2) padrino-core (= 0.15.2) padrino-gen (= 0.15.2) padrino-helpers (= 0.15.2) padrino-mailer (= 0.15.2) padrino-support (= 0.15.2) padrino-admin (0.15.2) padrino-core (= 0.15.2) padrino-helpers (= 0.15.2) padrino-cache (0.15.2) moneta (~> 1.1.0) padrino-core (= 0.15.2) padrino-helpers (= 0.15.2) padrino-core (0.15.2) padrino-support (= 0.15.2) sinatra (>= 2.2.4) thor (~> 1.0) padrino-gen (0.15.2) bundler (>= 1.0, < 3) padrino-core (= 0.15.2) padrino-helpers (0.15.2) i18n (>= 0.6.7, < 2) padrino-support (= 0.15.2) tilt (>= 1.4.1, < 3) padrino-mailer (0.15.2) mail (~> 2.5) mime-types (< 4) padrino-core (= 0.15.2) padrino-support (0.15.2) rack (2.2.5) rack-protection (3.0.5) rack rake (13.0.6) ruby2_keywords (0.0.5) sinatra (3.0.5) mustermann (~> 3.0) rack (~> 2.2, >= 2.2.4) rack-protection (= 3.0.5) tilt (~> 2.0) sqlite3 (1.5.4) mini_portile2 (~> 2.8.0) temple (0.9.1) thin (1.8.1) daemons (~> 1.0, >= 1.0.9) eventmachine (~> 1.0, >= 1.0.4) rack (>= 1, < 3) thor (1.2.1) tilt (2.0.11) timeout (0.3.1) tzinfo (2.0.5) concurrent-ruby (~> 1.0)

PLATFORMS arm64-darwin-22

DEPENDENCIES activerecord (>= 3.1) activesupport (>= 3.1) bcrypt haml (= 5.2.2) padrino (= 0.15.2) rake sqlite3 thin

BUNDLED WITH 2.4.3

jkowens commented 1 year ago

@wakatara I believe ActiveRecord 7.0 replaced update_attributes with update.

nesquena commented 1 year ago

Wow that's a pretty major change, we may need to then lock AR to < 7 until the admin module is updated to support either. This wouldn't be a big lift to support though unless there's more breaking changes as we could check which method the object responds to in the admin controller.

jkowens commented 1 year ago

It looks like the update and update! methods have been around since ActiveRecord 4.0. It might be ok just to do a straight up replacement. I was wrong, update_attributes was removed in 6.1.

https://blog.saeloun.com/2019/04/15/rails-6-deprecates-update-attributes.html

nesquena commented 1 year ago

Ah in that case, maybe it's an even easier fix if anyone wants to post a PR re generated admin code admin/controllers/accounts.rb in padrino-admin https://github.com/padrino/padrino-framework/blob/master/padrino-admin/lib/padrino-admin/generators/templates/page/controller.rb.tt

jkowens commented 1 year ago

It looks like this might be the place that needs updated: https://github.com/padrino/padrino-framework/blob/dc493e3870cd343f5bfe7edb21248389a5370963/padrino-admin/lib/padrino-admin/generators/orm.rb#L127

I think :activerecord and :minirecord (is this really supported? šŸ˜… ) could be moved to the same line as :datamapper and :ohm.

wakatara commented 1 year ago

@jkowens Yup, you're totally right. AR 7 dropped update_attributes. A simple global search and replace on all the controllers using update_attributes in the app made that work again without any further fuss. Thank you!

I've put a TODO in my GTD app to circle back and fix the generators in padrino as soon as I get through this upgrade push. Maybe this weekend, if not, next week... =]

As you say though, it looks like update_attributes and update_attributes! was always an alias for update and update! though it was certainly news to me to find that out (I remember way back when I did Rails work being told to switch to update_attributes so find it weird it's now deprecated.).

wakatara commented 1 year ago

I also find the reasons they removed them after advocating them pretty arbitrary, considering the number of gems it probably broke across the multiverse... 8-/

https://github.com/rails/rails/pull/31998