#165412914 add reset password endpoint #29

luc-tuyishime · 2019-05-08T20:03:11Z

What does this PR do?

Have Create and Update on ressources endpoint for reset password in express REST API

Description of Task to be completed?

Have the following endpoints working

POST api/v1/auth/reset/
UPDATE api/v1/auth/:token

How should this be manually tested?

Using Postman for testing all the endpoints above

Any background context you want to provide?

None

What are the relevant pivotal tracker stories?

(#165412914)[https://www.pivotaltracker.com/story/show/165412914]

Screenshots (if appropriate)

None

e-liyai · 2019-05-09T13:32:37Z

src/middlewares/errorCatcher.js

@@ -0,0 +1,14 @@
+import status from '../config/status';
+
+const errorCatcher = controller => async (req, res, next) => {


@luc-tuyishime give the function a more appropriate name

Changes applied

e-liyai · 2019-05-09T13:35:44Z

src/controllers/AuthLocalController.js

+      });
+    }
+    sgMail.setApiKey(process.env.SENDGRID_API_KEY);
+    const isSent = process.env.NODE_ENV === 'test' ? null : await sgMail.send(helper.sendgridMailTemplate.resetPassword(email, 'noreply@authersheaven.com'));


I believe we agreed on a sendEmail helper that anyone can use

changes applied

e-liyai

requested changes

e-liyai · 2019-05-10T10:17:47Z

src/controllers/AuthLocalController.js

+      });
+    }
+
+    await helper.sendMail(email); // send mail


@luc-tuyishime this should be wrapped by an error handler.

we have an async handler middleware that catches errors.

how has it been connected to this sendMail function?

I call the middleware in the routes.
the path to the route: routes/auth/local.js

e-liyai · 2019-05-10T10:27:02Z

src/middlewares/errorHandler.js

@@ -0,0 +1,14 @@
+import status from '../config/status';
+
+const errorHandler = controller => async (req, res, next) => {


this function name has not been renamed

changes applied

e-liyai · 2019-05-10T10:27:32Z

src/queries/users/updateUser.js

+ * @param {object} condition where to update
+ * @returns {boolean} return true when successfuly updated otherwise false
+ */
+export default async (what = {}, condition = {}) => {


why is the variable name what

It's what we want to update like a password.

give it a more appropriate name. what is not properly descriptive.

I have renamed the what name to value.

rwajon · 2019-05-12T14:08:00Z

src/helpers/sendgridMailTemplate.js

+    subject: 'Password Reset',
+    html: `<p>You are receiving this because you (or someone else) have requested the reset of the password,<br>
+     Click on the reset link bellow to complete the process<br>
+     <a href='http://localhost:3000/api/v1/auth/reset/${tokenGenerator({ email: to }, { expiresIn: '2h' })}' target='_blank'>Reset Password</a></p>`


@luc-tuyishime, I propose the redirect URL be defined in an .env file so that we don't have to change this file in case we want to use another URL, so it maybe something like APP_URL and the you use it this file

makes sense. Let's do that

Changes applied

rwajon · 2019-05-12T14:11:38Z

src/tests/routes/auth.test.js

+  passwordTwo: 'newpassworda',
+};
+
+const newUser = {


I suggest you use the helper that I defined to generate fake user accounts

there shouldn't be static declaration objects here, create files to generate this data or reuse what is already there.

Changes applied

e-liyai · 2019-05-13T10:03:22Z

src/helpers/sendgridMailTemplate.js

+    subject: 'Password Reset',
+    html: `<p>You are receiving this because you (or someone else) have requested the reset of the password,<br>
+     Click on the reset link bellow to complete the process<br>
+     <a href='http://localhost:3000/api/v1/auth/reset/${tokenGenerator({ email: to }, { expiresIn: '2h' })}' target='_blank'>Reset Password</a></p>`


makes sense. Let's do that

e-liyai · 2019-05-13T10:06:33Z

src/tests/routes/auth.test.js

+  passwordTwo: 'newpassworda',
+};
+
+const newUser = {


there shouldn't be static declaration objects here, create files to generate this data or reuse what is already there.

sengayire

rebase your PR with a new commit from develop

luc-tuyishime · 2019-05-14T07:22:53Z

rebase your PR with a new commit from develop

ok thanks

kagaramag · 2019-05-15T13:00:54Z

src/middlewares/verify-admin.js

+ * @export
+ * @param {object} req
+ * @param {object} res
+ * @param {void} next


It's a good practice to put descriptive JSDOC comments

kagaramag · 2019-05-15T19:01:46Z

src/middlewares/verify-admin.js

@@ -0,0 +1,32 @@
+import db from '../models';


It's better to use the same file naming convention.

kagaramag · 2019-05-15T19:04:02Z

src/helpers/sendgridMailTemplate.js

+      { expiresIn: '2h' }
+    )}' target='_blank'>Reset Password</a></p>`
+  })
+};


It's better you create a reusable mail template.
So that other team member can use this helper to send emails

This template is already a reusable template, you can use it to send emails.

#165412912 validate user input on signup

the reviewer on leave

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 13 times, most recently from f1f2de6 to eb577a5 Compare May 9, 2019 10:31

e-liyai reviewed May 9, 2019

View reviewed changes

e-liyai suggested changes May 9, 2019

View reviewed changes

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 3 times, most recently from 055c1e1 to a44f74c Compare May 10, 2019 09:04

e-liyai suggested changes May 10, 2019

View reviewed changes

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 3 times, most recently from c139442 to a537c7a Compare May 11, 2019 16:23

luc-tuyishime requested review from rwajon and MBenedicte May 12, 2019 11:03

rwajon reviewed May 12, 2019

View reviewed changes

MBenedicte approved these changes May 13, 2019

View reviewed changes

e-liyai previously requested changes May 13, 2019

View reviewed changes

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 2 times, most recently from 41c185c to d8a6cd7 Compare May 13, 2019 18:42

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 2 times, most recently from aa7e1b5 to db0e3c4 Compare May 13, 2019 19:17

sengayire approved these changes May 14, 2019

View reviewed changes

sengayire suggested changes May 14, 2019

View reviewed changes

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch 5 times, most recently from 96ad798 to 0f62d24 Compare May 15, 2019 12:34

kagaramag reviewed May 15, 2019

View reviewed changes

sengayire approved these changes May 15, 2019

View reviewed changes

kagaramag approved these changes May 15, 2019

View reviewed changes

kagaramag requested changes May 15, 2019

View reviewed changes

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch from 0f62d24 to 75ce816 Compare May 16, 2019 11:41

Merge pull request #38 from andela/ft-validate-user-input-165412912

e008dcd

#165412912 validate user input on signup

luc-tuyishime force-pushed the feature/be-reset-password-165412914 branch from 75ce816 to e008dcd Compare May 16, 2019 11:48

kagaramag approved these changes May 17, 2019

View reviewed changes

denislohan merged commit 54c93d4 into develop May 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#165412914 add reset password endpoint #29

#165412914 add reset password endpoint #29

luc-tuyishime commented May 8, 2019

e-liyai May 9, 2019

luc-tuyishime May 10, 2019

e-liyai May 9, 2019

luc-tuyishime May 10, 2019

e-liyai left a comment

e-liyai May 10, 2019

luc-tuyishime May 11, 2019

e-liyai May 11, 2019

luc-tuyishime May 11, 2019

e-liyai May 11, 2019

e-liyai May 10, 2019

luc-tuyishime May 11, 2019

e-liyai May 10, 2019

luc-tuyishime May 11, 2019

e-liyai May 11, 2019

luc-tuyishime May 11, 2019

rwajon May 12, 2019

e-liyai May 13, 2019

luc-tuyishime May 13, 2019

rwajon May 12, 2019

e-liyai May 13, 2019

luc-tuyishime May 13, 2019

e-liyai May 13, 2019

e-liyai May 13, 2019

sengayire left a comment

luc-tuyishime commented May 14, 2019

kagaramag May 15, 2019

kagaramag May 15, 2019

kagaramag May 15, 2019

luc-tuyishime May 16, 2019

		@@ -0,0 +1,14 @@
		import status from '../config/status';

		const errorCatcher = controller => async (req, res, next) => {

		@@ -0,0 +1,14 @@
		import status from '../config/status';

		const errorHandler = controller => async (req, res, next) => {

#165412914 add reset password endpoint #29

#165412914 add reset password endpoint #29

Conversation

luc-tuyishime commented May 8, 2019

What does this PR do?

Description of Task to be completed?

How should this be manually tested?

Any background context you want to provide?

What are the relevant pivotal tracker stories?

Screenshots (if appropriate)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e-liyai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sengayire left a comment

Choose a reason for hiding this comment

luc-tuyishime commented May 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment