-
Notifications
You must be signed in to change notification settings - Fork 563
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
Rails 5: belongs_to_required_by_default #588
Comments
Sorry for the delay. Finally got finished with Rails v5.1 work. Great info and context links here too! I also liked this one form your Rails issue heartcombo/devise@c2c74b0 I'll have work in 5.1 and 5.0 soon to fix this. Do you think fixing the 3 freedom patches on ActiveRecord using ActiveSupport.on_load(:active_record) do
require 'active_record/connection_adapters/sqlserver_adapter'
end |
My personal preference would be fixing the 3 freedom patches. It is more "explicit" . While on the other hand, requiring the library entrypoint in a lazy-load hook would mean that if ever in the future, other patches would be included, these will automatically be loaded by the Rails lazy-load hooks. |
OK... master is all green now with the 5.1 work done. If you want to work on a PR, that'll be great. Otherwise, I'll get to this next week. |
* Use `ActiveSupport.on_load` to hook into ActiveRecord #588
Since Rails 5.0, a new framework default has been introduced:
belongs_to_required_by_default
This option is being set in the file
config/initializers/new_framework_defaults.rb
.Since this is being configured through
Rails.application.config
, it does not get picked up when using activerecord-sqlserver-adapter.This is due to the initialisation order of gems / application code. Details can be found at: rails/rails#23589
Rails maintainer outline to use
ActiveSupport.on_load(:active_record)
when extending AR functionality and not useActiveRecord::Base.send :include
I have noticed that
ActiveRecord::Base.send :include
on a few places in the code, which produces the behaviour where belongs_to is no longer required, even though the option is set to true.The text was updated successfully, but these errors were encountered: