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

AO3-5576 Use Devise::Recoverable. #3489

Merged
merged 6 commits into from
Dec 22, 2018

Conversation

tickinginstant
Copy link
Contributor

@tickinginstant tickinginstant commented Dec 22, 2018

Issue

https://otwarchive.atlassian.net/browse/AO3-5576
https://otwarchive.atlassian.net/browse/AO3-5577

Purpose

Adds Recoverable to the list of Devise modules for Users, and tries to move over the existing messages/emails to the Devise system.

Testing Instructions

Reset password, follow instructions, make sure that the user experience is what you expect (error messages, good instructions and phrasing, and so on).

Email links in the tests used to use a different host, which resulted in
sessions being "lost" because the user was visiting a different path
(and therefore had their cookies stored elsewhere). Now it's all the
same host, so the test can be slightly simplified.
@redsummernight
Copy link
Member

For my own notes, we now have 2 forms to change passwords: one linked from preferences, requiring the old password, the other one from Devise, reachable by email, requiring an auto-generated token.

@redsummernight redsummernight added Has Migrations Contains migrations and therefore needs special attention when deploying Has Production Config Changes Modifies the config file and needs special attention when deploying labels Dec 22, 2018
Copy link
Member

@redsummernight redsummernight 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!

@sarken sarken merged commit e6a71ef into otwcode:master Dec 22, 2018
@tickinginstant tickinginstant deleted the AO3-5576-use-recoverable branch December 28, 2018 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Migrations Contains migrations and therefore needs special attention when deploying Has Production Config Changes Modifies the config file and needs special attention when deploying Priority: High - Broken on Test Merge immediately after approval Reviewed: Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants