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

Update form_with calls to be compatible with Rails 8 #1610

Merged
merged 2 commits into from
Feb 21, 2025

Conversation

sallyhall
Copy link
Contributor

Rails 8 removed deprecated support to passing nil to the model: argument of form_with.

To make the good job dashboard views compatible with Rails 8, this adds model: false to forms in both app/views/good_job/jobs/_table.erb and app/views/good_job/shared/_filter.erb.

Without these changes, loading the dashboard in a Rails 8 application causes ArgumentError (Passed nil to the :model argument, expect an object or false).

@bensheldon
Copy link
Owner

@sallyhall thank you! No objection to this for explicitness, but I wonder if your application has monkeypatched form_with. I suggest this because Rail's param default is model: false already:

https://github.com/rails/rails/blob/cf6ff17e9a3c6c1139040b519a341f55f0be16cf/actionview/lib/action_view/helpers/form_helper.rb#L756

@sallyhall
Copy link
Contributor Author

@sallyhall thank you! No objection to this for explicitness, but I wonder if your application has monkeypatched form_with. I suggest this because Rail's param default is model: false already:

https://github.com/rails/rails/blob/cf6ff17e9a3c6c1139040b519a341f55f0be16cf/actionview/lib/action_view/helpers/form_helper.rb#L756

@bensheldon Thank you for pointing that out! It turns out that several gems dependencies down, we are using https://github.com/amatsuda/html5_validators, which explicitly passes model: nil to form_with. I'll close this PR and make a pr there instead.

@sallyhall sallyhall closed this Feb 21, 2025
@bensheldon
Copy link
Owner

No worries. I imagine others might run into this too, so I'll merge your change. Can never be too defensive 😇

@bensheldon bensheldon reopened this Feb 21, 2025
@bensheldon bensheldon added the bug Something isn't working label Feb 21, 2025
@bensheldon bensheldon merged commit 21e5e79 into bensheldon:main Feb 21, 2025
88 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

2 participants