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

Confirm users on successful password recovery #119

Merged
merged 1 commit into from Oct 11, 2017

Conversation

Projects
None yet
2 participants
@brycekahle
Member

brycekahle commented Oct 11, 2017

Fixes #113

@brycekahle brycekahle requested a review from rybit Oct 11, 2017

@rybit

rybit approved these changes Oct 11, 2017

minor gripe, totally ignorable b/c a lot of the other tests follow this style. Up to you.

u, err := ts.API.db.FindUserByEmailAndAudience("", "test@example.com", ts.Config.JWT.Aud)
require.NoError(ts.T(), err)
u.RecoverySentAt = &time.Time{}
require.NoError(ts.T(), ts.API.db.UpdateUser(u))

This comment has been minimized.

@rybit

rybit Oct 11, 2017

Member

ts.Require.NoError(ts.API.db.UpdateUser(u)) I think you can do that. For almost all of the assert and require. I think the assert ones even are directly available (ts.Equal(....)).

@rybit

rybit Oct 11, 2017

Member

ts.Require.NoError(ts.API.db.UpdateUser(u)) I think you can do that. For almost all of the assert and require. I think the assert ones even are directly available (ts.Equal(....)).

This comment has been minimized.

@brycekahle

brycekahle Oct 11, 2017

Member

You are correct. I mostly copied the other style. I'd rather do a PR that is just changing the asserts with no functional changes.

@brycekahle

brycekahle Oct 11, 2017

Member

You are correct. I mostly copied the other style. I'd rather do a PR that is just changing the asserts with no functional changes.

@brycekahle brycekahle merged commit 31891bd into master Oct 11, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@brycekahle brycekahle deleted the recovery-confirm branch Oct 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment