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 path_prefix default to match the old default #103

Closed
wants to merge 1 commit into from

Conversation

swrobel
Copy link
Contributor

@swrobel swrobel commented Aug 16, 2013

The old route was /users/auth/:provider and this has changed the default to /user/auth/:provider
I believe the default should not break existing apps that were relying on that route

This should be committed to 2-0-stable && 1-3-stable as well

The old route was /users/auth/:provider and this has changed the default to /user/auth/:provider
I believe the default should not break existing apps that were relying on that route
swrobel added a commit that referenced this pull request Aug 30, 2013
The old route was /users/auth/:provider and this has changed the default to /user/auth/:provider
I believe the default should not break existing apps that were relying on that route

Fixes #103
@radar
Copy link
Contributor

radar commented Aug 30, 2013

Added to master, 1-3-stable and 2-0-stable. Sorry for the delay, I was on vacation last week and I'm still catching up with emails.

swrobel added a commit that referenced this pull request Aug 30, 2013
The old route was /users/auth/:provider and this has changed the default to /user/auth/:provider
I believe the default should not break existing apps that were relying on that route

Fixes #103
@swrobel swrobel closed this in 7b39847 Aug 30, 2013
@swrobel
Copy link
Contributor Author

swrobel commented Aug 30, 2013

No worries, glad to have you back!

@radar
Copy link
Contributor

radar commented Jul 3, 2014

This seems to now be causing #130. I think that if we changed the option to spree_user by default it might fix that issue.

@j-mcnally
Copy link
Contributor

If we allow the user to change this we need to document that the omniauth failure block should be hardcoded to the spree_user mapping.

OmniAuth.config.on_failure = Proc.new do |env|
  env['devise.mapping'] = Devise.mappings[:spree_user]
  controller_name  = ActiveSupport::Inflector.camelize(env['devise.mapping'].controllers[:omniauth_callbacks])
  controller_klass = ActiveSupport::Inflector.constantize("#{controller_name}Controller")
  controller_klass.action(:failure).call(env)
end

@troelskn
Copy link

I spent quite some time on getting this to work, so I'll just leave this here for the next poor soul who arrives here. You need to load devise before setting up this custom failure block - otherwise it'll just get overridden by devise's block. Simple put require 'devise/omniauth' on a line before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants