#167484585 implemented email verification link approach #14

placideirandora · 2019-08-08T12:24:02Z

What does this PR do?

It implements the approach of sending an email verification link to the user upon a successful registration.

Description of Task to be done?

Adding a helper for sending emails.
Using the helper in the user controller to pass required information for sending an email.
Modifying the message property to include a message for notifying the user to check their email for the verification link.
Adding a route for verifying the email verification link.
Changing the status of a user from inactive to active after verifying their email address.

Screenshots

How should this be manually tested?

Create an account on sendgrid.com
Create an API key for sending emails on www.sendgrid.com.
Update your .env file with the following: SENDGRID_API_KEY=putthekeyhere, EMAIL_SENDER=no-reply@authorshaven.com, APP_URL=localhost.
Clone this branch.
Do npm install.
Register a new user and after successful registration, Check your email for a verification link. If you do not get it your inbox, check your spam folder. It should be there!
Copy the link and paste it in Postman and check for the verification confirmation link.

hound · 2019-08-08T12:24:11Z

src/helpers/mailer.js

@@ -0,0 +1,45 @@
+import dotenv from 'dotenv';


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

malaba6

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

placideirandora · 2019-08-08T14:48:21Z

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

malaba6 · 2019-08-08T15:11:44Z

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I have and there is nothing there. Actually this is the error it's throwing after creating the user

placideirandora · 2019-08-08T15:25:18Z

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I have and there is nothing there. Actually, this is the error it's throwing after creating the user

That is because you are not using a SendGrid API key. You should create an account on sendgrid.com and create an API key as well. Then, you will use the key in your .env file.

malaba6 · 2019-08-08T15:29:10Z

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I have and there is nothing there. Actually, this is the error it's throwing after creating the user

That is because you are not using a SendGrid API key. You should create an account on sendgrid.com and create an API key as well. Then, you will use the key in your .env file.

Could you please include that and the Environment variable in the PR as suggested by @mkiterian earlier?

placideirandora · 2019-08-08T15:30:00Z

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I don't know about you Placide, but I've tested this on my pc and it's not sending me a verification email. I used my two email addresses, and none has sent. Any reason why this is happening?

If you don't get the email in your inbox. You should check your Spam folder. It should be there!

I have and there is nothing there. Actually, this is the error it's throwing after creating the user

That is because you are not using a SendGrid API key. You should create an account on sendgrid.com and create an API key as well. Then, you will use the key in your .env file.

Could you please include that and the Environment variable in the PR as suggested by @mkiterian earlier?

Yes, I have done that!

raymond42

In my opinion, I think It's not necessary to put postman when sending a verification email. Because somebody maybe using another API testing app or browser extension like Restman and still works.

placideirandora · 2019-08-08T15:42:00Z

In my opinion, I think It's not necessary to put postman when sending a verification email. Because somebody maybe using another API testing app or browser extension like Restman and still works.

Thank you for mentioning that! I'm gonna remove Postman.

… user registration

mkiterian · 2019-08-08T18:51:47Z

src/helpers/mailer.js

+    mailer.send(message);
+  }
+};
+// eslint-disable-next-line import/prefer-default-export


why disable this eslint rule?

Before, When I used export default, the function didn't get detected. But, I have improved the way I use it in the user controller and it is working. So, I can re-enable the rule.

mkiterian · 2019-08-08T18:56:02Z

.env.sample

@@ -6,3 +6,6 @@ DB_NAME_TEST=
 DB_PORT=
 DATABASE_URL=
 SECRET_KEY=
+SENDGRID_API_KEY =


remove the spaces preceding the =

Thank you. I have removed the free spaces in .env.sample.

mkiterian · 2019-08-08T18:56:16Z

src/helpers/mailer.js

+      from: EMAIL_SENDER,
+      subject: subjectVerify,
+      text: 'Authors Haven',
+      html: `<div style="background:#ECF0F1;width:100%;padding:20px 0;">


I'd much rather this markup string is generated by a function e.g createEmailTemplate(contentVerify, linkVerify) and used here. Would make it much cleaner

Thank you. I'm going to implement that approach. It is really good.

hound · 2019-08-09T10:30:51Z

src/helpers/templates/mail.js

+  return message;
+};
+
+export default createEmailTemplate;


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

…ah-backend into ft-verify-email-167484585

hound · 2019-08-10T15:41:32Z

src/helpers/mailer.js

@@ -1,20 +1,31 @@
-import dotenv from 'dotenv';
+import { config } from 'dotenv';


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

hound · 2019-08-10T15:41:32Z

src/controllers/verifyEmail.js

@@ -0,0 +1,15 @@
+import { User } from '../sequelize/models/index';


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

src/config/config.js

mkiterian · 2019-08-11T07:10:44Z

src/sequelize/models/User.js

@@ -33,6 +33,11 @@ export default (sequelize, DataTypes) => {
      type: DataTypes.STRING,
      defaultValue: 'user',
    },
+    status: {


This should be a boolean field since we expect it to only have two states. You can name it either active/verified.

I have implemented that.

mkiterian · 2019-08-11T07:30:44Z

src/controllers/verifyEmail.js

@@ -0,0 +1,15 @@
+import { User } from '../sequelize/models/index';


This should be part of the userController

Excuse me... You mean the whole code inside the verifyEmail.js file, should be inside the userController.js file? Or, that single line of importing from sequelize should be inside the userController?

I mean the whole code inside the verifyEmail.js file

mkiterian · 2019-08-11T12:48:06Z

src/controllers/userController.js

@@ -11,6 +11,7 @@ config();
 */
 class Authentication {
  /**
+     * Eric MALABA & Placide IRANDORA


what's this for?

Oh! It should be @author Eric MALABA & Placide IRANDORA. I tried to indicate the developers who worked on that method/function. But, I'm gonna removed it if it is not necessary.

mkiterian · 2019-08-11T12:48:27Z

src/controllers/userController.js

@@ -84,7 +103,7 @@ class Authentication {
  }

  /**
-   *
+   * @author Carlos HARERIMANA


remove this

mkiterian · 2019-08-11T12:49:48Z

src/controllers/userController.js

@@ -60,6 +61,24 @@ class Authentication {
  }

  /**
+   * @author Placide IRANDORA


remove @author

mkiterian · 2019-08-11T12:51:39Z

src/sequelize/models/User.js

@@ -33,6 +33,11 @@ export default (sequelize, DataTypes) => {
      type: DataTypes.STRING,
      defaultValue: 'user',
    },
+    verified: {


the datatype should be DataTypes.BOOLEAN. We expect a user to either be verified or not. A True vs False value

I'm gonna implement that.

mkiterian · 2019-08-11T13:56:28Z

src/routes/routes.js

@@ -6,6 +6,7 @@ import verifyToken from '../middleware/verifyToken';
 const router = express.Router();

 router.post('/api/v1/users', Validation.signupValidation, UserAuth.signup);
+router.post('/api/v1/verify-email/:token', verifyToken, UserAuth.verifyEmail);


this should be a GET route

Okay. I'm gonna implement that.

mkiterian

Looks good

feat(verify-email): add a helper for sending email

bb80009

placideirandora requested review from mkiterian and harerakalex August 8, 2019 12:24

hound bot reviewed Aug 8, 2019

View reviewed changes

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 8, 2019 12:24 Inactive

placideirandora requested review from malaba6, Jkadhuwa and raymond42 August 8, 2019 12:24

placideirandora added the Need Reviews Feedback to PR raised label Aug 8, 2019

placideirandora force-pushed the ft-verify-email-167484585 branch from a74e835 to f0a4ac4 Compare August 8, 2019 12:33

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 8, 2019 12:33 Inactive

malaba6 reviewed Aug 8, 2019

View reviewed changes

raymond42 requested changes Aug 8, 2019

View reviewed changes

feat(verify-email): send an email verification link upon a successful…

9ad0f73

… user registration

raymond42 approved these changes Aug 8, 2019

View reviewed changes

mkiterian suggested changes Aug 8, 2019

View reviewed changes

placideirandora force-pushed the ft-verify-email-167484585 branch from f0a4ac4 to 8de0f54 Compare August 9, 2019 10:30

hound bot reviewed Aug 9, 2019

View reviewed changes

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 9, 2019 10:30 Inactive

placideirandora force-pushed the ft-verify-email-167484585 branch from 8de0f54 to 2805434 Compare August 9, 2019 10:49

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 9, 2019 10:49 Inactive

placideirandora added 2 commits August 10, 2019 16:26

feat(verify-email): implement the feedback

1458527

Merge branch 'development' of https://github.com/andela/coding-geeks-…

dca25b2

…ah-backend into ft-verify-email-167484585

placideirandora force-pushed the ft-verify-email-167484585 branch from 2805434 to 29113a1 Compare August 10, 2019 15:41

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 10, 2019 15:41 Inactive

hound bot reviewed Aug 10, 2019

View reviewed changes

placideirandora force-pushed the ft-verify-email-167484585 branch from 29113a1 to e0a716a Compare August 10, 2019 16:36

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 10, 2019 16:36 Inactive

mkiterian suggested changes Aug 11, 2019

View reviewed changes

mkiterian mentioned this pull request Aug 11, 2019

#167724280 user should be able to sign in with email and password #17

Merged

placideirandora force-pushed the ft-verify-email-167484585 branch from e0a716a to c72b287 Compare August 11, 2019 12:46

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 11, 2019 12:46 Inactive

mkiterian suggested changes Aug 11, 2019

View reviewed changes

placideirandora force-pushed the ft-verify-email-167484585 branch from c72b287 to fcf0286 Compare August 11, 2019 13:05

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 11, 2019 13:05 Inactive

placideirandora force-pushed the ft-verify-email-167484585 branch from fcf0286 to 2449697 Compare August 11, 2019 13:34

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 11, 2019 13:34 Inactive

mkiterian suggested changes Aug 11, 2019

View reviewed changes

placideirandora force-pushed the ft-verify-email-167484585 branch from 2449697 to 7b48112 Compare August 11, 2019 14:09

raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-14 August 11, 2019 14:09 Inactive

feat(verify-email): implement the feedback

218956a

placideirandora force-pushed the ft-verify-email-167484585 branch from 7b48112 to 218956a Compare August 11, 2019 14:26

raymond42 deployed to codinggeeks-ah-backnd-st-pr-14 August 11, 2019 14:26 View deployment

mkiterian approved these changes Aug 11, 2019

View reviewed changes

mkiterian merged commit 2393369 into development Aug 11, 2019

placideirandora added finished and removed Need Reviews Feedback to PR raised labels Aug 11, 2019

		@@ -1,20 +1,31 @@
		import dotenv from 'dotenv';
		import { config } from 'dotenv';

		@@ -0,0 +1,15 @@
		import { User } from '../sequelize/models/index';

#167484585 implemented email verification link approach #14

#167484585 implemented email verification link approach #14

Conversation

placideirandora commented Aug 8, 2019 • edited Loading

What does this PR do?

Description of Task to be done?

Screenshots

How should this be manually tested?

hound bot Aug 8, 2019

Choose a reason for hiding this comment

malaba6 left a comment

Choose a reason for hiding this comment

placideirandora commented Aug 8, 2019

malaba6 commented Aug 8, 2019

placideirandora commented Aug 8, 2019

malaba6 commented Aug 8, 2019

placideirandora commented Aug 8, 2019

raymond42 left a comment

Choose a reason for hiding this comment

placideirandora commented Aug 8, 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

mkiterian Aug 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hound bot Aug 9, 2019

Choose a reason for hiding this comment

hound bot Aug 10, 2019

Choose a reason for hiding this comment

hound bot Aug 10, 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

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

placideirandora Aug 11, 2019 • edited Loading

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

mkiterian left a comment

Choose a reason for hiding this comment

placideirandora commented Aug 8, 2019 •

edited

Loading

mkiterian Aug 8, 2019 •

edited

Loading

placideirandora Aug 11, 2019 •

edited

Loading