thoughtbot / clearance

Rails authentication with email & password.
https://thoughtbot.com
MIT License
3.71k stars 467 forks source link

ActiveRecord::AssociationTypeMismatch in development when assigning current_user to an association #568

Closed phil-monroe closed 9 years ago

phil-monroe commented 9 years ago

When using clearance, we keep getting an ActiveRecord::AssociationTypeMismatch whenever Rails auto-reloads the user class and we try to set current_user as an association for a new record. I think the issue is stemming from this line:

https://github.com/thoughtbot/clearance/blob/master/lib/clearance/configuration.rb#L103

# Clearance:: Configuration
def user_model
  @user_model ||= ::User
end

This is storing a reference to the class of the User model as an instance variable, which won't get updated when rails auto-reloads the class. I've been able to verify this by using pry to debug at the beginning of a request.

[4] pry(#<ApplicationController>)> current_user.class == User
=> false

When trying to set the current user as an association on a model, an ActiveRecord::AssociationTypeMismatch is thrown.

[1] pry(#<InviteUserToOrg>)> $!
=> #<ActiveRecord::AssociationTypeMismatch: User(#70261513327700) expected, got User(#70261542797340)>

I can think of a couple ways of solving this, but would like the maintainers feedback before proceeding. Some possible options are:

1) Convert the class name to string and constantize (@user_model.name.constantize) to get the latest version of the class whenever accessing the user model. I feel like this is kind of a hack and a bit wastful from an memory usage standpoint, but I think I've seen it done elsewhere.

2) Allow user_model to be set to something callable that returns a user model. Would look something like this:

# Clearance:: Configuration
def user_model
  @user_model ||= -> { ::User }
  if @user_model.respond_to? :call
    @user_model.call
  else
    @user_model
  end
end

3) Add a before_action to Clearance::Controller to check if Clearance.configuration.user_model is the latest version and update if need be. Would be kind of nice as you can skip the before_action if Rails.configuration.cache_classes is set to true.

4) Kind of like 3, but add a cleanup hook to ActionDispatch::Reloader to reset the user _model.

Let me know what you all think and I'd be glad to supply a pull request and/or an example Rails app.

phil-monroe commented 9 years ago

And just for anyone that is interested, I'm currently working around the problem by using this monkeypatch that should be removed when the problem is fixed. If you use it, be sure to hardcode the proper class for your "user" model.

# confing/initializers/clearance.rb

# TODO: remove this monkeypatch when clearance is fixed
# https://github.com/thoughtbot/clearance/issues/568
module Clearance
  class Configuration
    def user_model
      ::User
    end
  end
end

Clearance.configure do |config|
#...
end
derekprior commented 9 years ago

@phil-monroe Are you using Clearance 1.9.0 or newer? We added this change to try to address these issues.

phil-monroe commented 9 years ago

Yup. Was using 1.8.0. Thanks!