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

Custom failure class for optional redirecting to logout url upon timeout #67

Merged
merged 1 commit into from
May 8, 2013
Merged

Conversation

geoffroh
Copy link
Contributor

Fixes #66 - Problem with single_sign_out and devise timeoutable module.

@kylejginavan
Copy link
Contributor

This patch would be really useful to us. Would you be able to merge and release a new version soon?

@nbudin
Copy link
Owner

nbudin commented Apr 29, 2013

Sorry, this is a large patch and I'm at RailsConf this week.  I don't know if I'll have time to review it.  In the meantime, I would recommend you use your Github branch in your bundle.
Nat

On Mon, Apr 29, 2013 at 6:24 AM, Kyle J. Ginavan [email protected]
wrote:

This patch would be really useful to us. Would you be able to merge and release a new version soon?

Reply to this email directly or view it on GitHub:
#67 (comment)

@kylejginavan
Copy link
Contributor

This gem is a dependency of another gem, therefore, it uses .gemspec which doesn't support git branches. No worries though. Have things called down enough after rails conf for you to look this over?

@nbudin
Copy link
Owner

nbudin commented May 8, 2013

Thanks for the reminder! I've just given it a quick look over, and it seems reasonable. My main concern is that it might break compatibility with older versions of Devise by accident. I'm going to merge it and see if it will do that, and if so, I might change it so that it only loads the custom failure app stuff if Devise::FailureApp is defined or something like that.

nbudin added a commit that referenced this pull request May 8, 2013
Custom failure class for optional redirecting to logout url upon timeout
@nbudin nbudin merged commit 23b644a into nbudin:master May 8, 2013
@nbudin
Copy link
Owner

nbudin commented May 8, 2013

OK, looking at it, it does seem to break things in older versions of Devise, but not for the reasons I expected. scope_path only exists from Devise 2.0 on up. However, I actually can't see why it is being overridden here, since no additional functionality is added (it just calls the aliased method). I'm thinking of removing the override and the alias, and in the redirect_url method, checking for respond_to?(:scope_path).

@nbudin
Copy link
Owner

nbudin commented May 13, 2013

Version 1.3.0 is now released with this change, and dropping support for Devise 1.0 and 1.1.

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.

Problem with single_sign_out and devise timeoutable module
3 participants