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
Allow User model to be reloaded in development #547
Conversation
# @private | ||
def reload_user_model | ||
if @user_model.present? | ||
@user_model = @user_model.to_s.constantize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> ::User.to_s.constantize
=> User (call 'User.connection' to establish a connection)
>::User
=> User (call 'User.connection' to establish a connection)
since these return the same value, do we need to to_s
and constantize
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we basically have to force Ruby to re-look-up the constant, which is what we accomplish with constantize
from a string. In an actual application running in development mode, if you make changes to the user class, the reference saved by clearance won't actually be the same class that you get with this constantize dance.
As an added benefit (perhaps) this also is more permissive in configuration as users could pass a string or symbol which would still be constantized properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation.
Nice! :) Nitpicking: Rails::Railtie::Configuration#to_prepare defines generic callbacks to run before #after_initialize (not necessarily on every request). The ActionDispatch::Reloader#to_prepare callbacks are run before each request. |
That seems to be the context we're in inside that |
dcb6f07
to
90dbeb9
Compare
You are calling the |
Was not sure how to test if reloading was working and looked over the engine. This spec tests if the callback has been configured, which might be enough if you can rely on Rails to properly call them: describe "#reload_user_model" do
it "expect reload_user_model to be called on framework reload" do
expect(Clearance::Engine.config.to_prepare_blocks).to include(&:reload_user_model)
end
end |
I thought about that too. At that point I'm just guarding against someone deleting the |
It seems my tests for |
When a `user_model` is configured in a Clearance initializer, a reference to that class is immediately saved off. If that class is changed, Clearance will not know to automatically reload the class as Rails does automatically for classes in development. This change introduces a `to_prepare` block to the Engine that is responsible for forcing the configured user class to be reloaded. `to_prepare` runs once per request in development and only at startup in other environments.
23c0c3b
to
0181691
Compare
When a
user_model
is configured in a Clearance initializer, a referenceto that class is immediately saved off. If that class is changed, Clearance
will not know to automatically reload the class as Rails does automatically
for classes in development.
This change introduces a
to_prepare
block to the Engine that isresponsible for forcing the configured user class to be reloaded.
to_prepare
runs once per request in development and only at startup inother environments.