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

Logout with DELETE verb by default #161

Merged

Conversation

kennyadsl
Copy link
Member

A while ago Devise changed the default HTTP verb used to sign out from GET to DELETE.

This PR re-aligns this preference default with what is shipped with Devise. It also improves the code to be more dynamic since we were not really supporting changing that preference.

🚨Can be breaking 🚨

Stores that rely on signing out with GET verb will need to change the Devise.sign_out_via configuration at config/initializers/devise.rb:

Devise.sign_out_via = :get

@kennyadsl kennyadsl self-assigned this Jul 2, 2019
Copy link

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good change to stay with defaults

@kennyadsl kennyadsl force-pushed the kennyadsl/fix-default-delete-method branch from 10c6fc8 to 7bf7d62 Compare July 3, 2019 08:22
Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kennyadsl thank you! 👍

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks @kennyadsl

@kennyadsl kennyadsl added the WIP label Jul 12, 2019
@kennyadsl
Copy link
Member Author

I'm not sure about backward compatibility here and I'm not super sure what to do, leaving WIP for now, please do not merge yet.

This changed in Devise and we did not reflect this change over here.

This commit will change how sign out works, so if users are manually
using the GET verb to sign out users they will have to change it
manually.
At the moment we are only supporting usign GET but the frontend
routes allow customizing it using Devise.sign_out_via preference.
following value of Devise.sign_out_via preference
@kennyadsl kennyadsl force-pushed the kennyadsl/fix-default-delete-method branch from 7bf7d62 to 1ecfbca Compare July 17, 2019 15:45
@kennyadsl kennyadsl removed the WIP label Jul 17, 2019
@kennyadsl
Copy link
Member Author

We discussed this in the core team meeting and we are fine merging this as is and releasing a new version.

@kennyadsl kennyadsl merged commit 5e0c46d into solidusio:master Aug 12, 2019
@kennyadsl kennyadsl deleted the kennyadsl/fix-default-delete-method branch August 12, 2019 16:06
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