-
Notifications
You must be signed in to change notification settings - Fork 10
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
#167749954 Implement Password Reset Feature #24
Conversation
3f84680
to
872150e
Compare
e7c6e61
to
a19f684
Compare
8b16e85
to
db4c3db
Compare
.travis.yml
Outdated
@@ -9,6 +9,11 @@ before_script: | |||
node_js: | |||
- 'stable' | |||
|
|||
env: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the environment variables, it has been added on Travis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes have been implemented.
src/config/swagger.json
Outdated
"firstname": { | ||
"type": "string" | ||
}, | ||
"lastname": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"lastname": { | |
"lastName": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes have been implemented
src/controllers/UserController.js
Outdated
Responses.setError(400, 'this address link has expired') | ||
return Responses.send(res); | ||
} | ||
if (req.method == 'GET') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (req.method == 'GET') { | |
if (req.method === 'GET') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes have been implemented
src/database/models/user.js
Outdated
@@ -19,4 +20,4 @@ export default (sequelize, DataTypes) => { | |||
}, | |||
}, {}); | |||
return User; | |||
}; | |||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can always add newlines at the end of a file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job David. I think you need to add new lines at the end of some of your files, please check through
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of files does not have newlines at the end. Consider adding them. You can also rebase off ft-JWT-auth-167749949
to implement some changes on your branch. This is good work, well done.
db4c3db
to
dec4880
Compare
src/routes/index.js
Outdated
@@ -0,0 +1,11 @@ | |||
import express from 'express'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has been deleted. Consider rebasing
src/tests/index.test.js
Outdated
@@ -18,13 +18,13 @@ describe('Handle requests on other endpoints', () => { | |||
}); | |||
}); | |||
|
|||
it('should return an error when a wrong url is provided', done => { | |||
it('should return a welcome message for version 1 of the API', done => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test has been removed as well
dec4880
to
6116872
Compare
0245ded
to
cf5e0a8
Compare
src/utils/Helper.js
Outdated
* @memberof Helper | ||
*/ | ||
static async verifyExistingEmail(email) { | ||
const check = await models.User.findOne({ where: { email , isVerified: true} }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A space is required before '}' object-curly-spacing
There should be no space before ',' comma-spacing
841e522
to
c984a91
Compare
src/tests/user.test.js
Outdated
}) | ||
.end((err, res) => { | ||
console.log(res.body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected console statement no-console
src/controllers/RequestController.js
Outdated
@@ -2,6 +2,7 @@ import RequestService from '../services/RequestService'; | |||
import Responses from '../utils/Responses'; | |||
import Helper from '../utils/Helper'; | |||
|
|||
console.log(Helper.hashPassword('Frank1234')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected console statement no-console
a373c32
to
88bb648
Compare
src/config/swagger.json
Outdated
} | ||
], | ||
"responses": { | ||
"201": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can change the status code here
"201": { | |
"200": { |
src/controllers/UserController.js
Outdated
id, email, createdAt, updatedAt | ||
} = rowsAffected; | ||
Responses.setSuccess( | ||
201, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
201, | |
200, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback @hustlaviola , changes have been implemented.
a6dc289
to
4ca4635
Compare
4fede16
to
a94b80c
Compare
src/controllers/UserController.js
Outdated
return Responses.send(res); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline required at end of file but not found eol-last
bf512a0
to
c85c118
Compare
- add test for password reset endpoint - add password reset endpoint routes. - add update Password service. - add verify email and send email Helper. - add password validation userSchema. - update documentation. [Delivers #167749954]
c85c118
to
f99c8ed
Compare
you still dey work on the stuff? |
What does this PR do?
Add password reset feature
Description of Task to be completed?
How should this be manually tested?
git clone
the branchPOST
request to/api/v1/users/reset
PATCH
request containing the new password to the link in the emailAny background context you want to provide?
nodemailer
was used to send the email.What are the relevant pivotal tracker stories?
#167749954