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

allow Devise::Models::Authenticatable to be loaded before Rails #3493

Merged
merged 1 commit into from Feb 25, 2015
Merged

Conversation

betesh
Copy link
Contributor

@betesh betesh commented Feb 24, 2015

I am using some Devise modules in a Sinatra app. Because Devise::Models::Authenticatable expects Rails to be defined, I need to add the following hack:

require 'active_model'
module Rails
  VERSION ||= ActiveModel::VERSION # Needed in order to load devise/authenticatable
end

However, while this solves the immediate problem of making it possible to include Devise::Models::Authenticatable, it creates a new set of problems--other gems (annotate and recent versions of paperclip, delayed_job and ActiveRecord) use defined?(Rails) to determine whether some component of Rails is loaded.

Since the Rails version is being checked to determine whether serializable_hash is defined, it would be more appropriate to check the version of ActiveModel anyway, since that is where serializable_hash lives.

@betesh
Copy link
Contributor Author

betesh commented Feb 24, 2015

By the way, there are a lot of other tweaks that would make it easier to use devise with Sinatra. Let me know if you'd consider a PR to make this a little easier. I'd include documentation and keep it fully backwards-compatible.

@georgeguimaraes
Copy link
Contributor

Your suggestion about the version check makes sense. I'm going to merge it.

However, we don't support using Devise on top of Sinatra since almost all devise features are Rails specific.

@georgeguimaraes
Copy link
Contributor

The build is failing. Can you look at it?
https://travis-ci.org/plataformatec/devise/jobs/52027539

@betesh
Copy link
Contributor Author

betesh commented Feb 24, 2015

Fixed. I had to explicitly require "active_model/version" for Rails 4. Thank you for agreeing to merge so quickly!

almost all devise features are Rails specific.

I disagree. Several Devise::Model modules have been very helpful in implementing secure authentication in Sinatra. This ticket is probably the wrong place for this discussion, so where would be an appropriate place to continue it?

@rafaelfranca
Copy link
Collaborator

I know you can use some Devise modules in general Ruby applications not
just Rails, but I think we as maintainers, don't want to support anything
besides Rails right now.

On Tue, Feb 24, 2015, 18:35 betesh notifications@github.com wrote:

Fixed. I had to explicitly require "active_model/version" for Rails 4.
Thank you for agreeing to merge so quickly!

almost all devise features are Rails specific.

I disagree. Several Devise::Model modules have been very helpful in
implementing secure authentication in Sinatra. This ticket is probably the
wrong place for this discussion, so where would be an appropriate place to
continue it?


Reply to this email directly or view it on GitHub
#3493 (comment).

@betesh
Copy link
Contributor Author

betesh commented Feb 24, 2015

OK Fair enough.

@betesh
Copy link
Contributor Author

betesh commented Feb 25, 2015

You're still planning to merge this, correct?

@rafaelfranca
Copy link
Collaborator

This one yes

@rafaelfranca rafaelfranca reopened this Feb 25, 2015
rafaelfranca added a commit that referenced this pull request Feb 25, 2015
allow Devise::Models::Authenticatable to be loaded before Rails
@rafaelfranca rafaelfranca merged commit 4bb457f into heartcombo:master Feb 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants