#167190529 Validate User Signup Inputs #19

abdulfatai360 · 2019-08-01T08:40:01Z

What does this PR do?

Write schema for user signup inputs
Validate the user signup inputs against the schema

Description of Task proposed in this pull request?

Create the file schema/userSignup.js inside the /server directory
Install @hapi/joi library to write the schema and validate the signup inputs
Install sinon to help in mocking ExpressJs req, res, and next objects for unit testing the validation middleware
Check user signup inputs for validation errors
Return validation errors to users, if any

How should this be manually tested (Quality Assurance)?

Run git clone https://github.com/andela/ah-rambo-backend.git to clone this repo onto your local machine
Navigate to the repo directory by running cd ah-rambo-backend/
Ensure your working directory is clean by running git status command
Get a copy of the PR by typing git fetch origin pull/19/head:ft-validate-user-signup-inputs-167190529
Now that you have a copy of the branch, switch to it using git checkout ft-validate-user-signup-inputs-167190529
Your directory will now be an exact copy of the PR
Run npm test
All tests should pass

What are the relevant pivotal tracker stories?

#167190529

Any background context you want to add (Operations Impact)?

Run npm install to get all dependencies onto your local machine

What I have learned working on this feature:

As it was my first time unit testing a middleware, I have learned how to do that now.

Screenshots:

hound · 2019-08-01T08:40:12Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('password');
+      expect(res.body.errors.password).to.match(
+        /Password should not contain spaces/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T08:40:12Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('password');
+      expect(res.body.errors.password).to.match(


Unexpected newline after '(' function-paren-newline

hound · 2019-08-01T08:40:12Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('password');
+      expect(res.body.errors.password).to.match(
+        /Password should not be less than 8 characters/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T08:40:13Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('password');
+      expect(res.body.errors.password).to.match(


Unexpected newline after '(' function-paren-newline

hound · 2019-08-01T08:40:13Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('userName');
+      expect(res.body.errors.userName).to.match(
+        /username should contain only letters and numbers/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T08:40:14Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('lastName');
+      expect(res.body.errors.lastName).to.match(


Unexpected newline after '(' function-paren-newline

hound · 2019-08-01T08:40:14Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('lastName');
+      expect(res.body.errors.lastName).to.match(
+        /last name should not be less than 2 characters/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T08:40:14Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('lastName');
+      expect(res.body.errors.lastName).to.match(


Unexpected newline after '(' function-paren-newline

hound · 2019-08-01T08:40:15Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('firstName');
+      expect(res.body.errors.firstName).to.match(
+        /first name should not be more than 50 characters/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T08:40:15Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('firstName');
+      expect(res.body.errors.firstName).to.match(


Unexpected newline after '(' function-paren-newline

dinobi · 2019-08-01T09:07:01Z

server/helpers/serverResponse.js

@@ -0,0 +1,10 @@
+/**


please remove this file

@dinobi, I have updated my PR, and I noticed that the file has been updated on my branch as well. Basically, my local branch is in sync with the remote develop, so this file is no longer a repeated file.

You would notice that this file is no longer in the Files changed section of this PR.

hound · 2019-08-01T11:52:52Z

test/middlewares/userValidation.test.js

+      expect(res.body.errors).to.haveOwnProperty('firstName');
+      expect(res.body.errors.firstName).to.match(
+        /first name should not be less than 2 characters/i
+      );


Unexpected newline before ')' function-paren-newline

hound · 2019-08-01T11:52:52Z

test/middlewares/userValidation.test.js

+      expect(res.statusCode).to.equal(422);
+      expect(res.body).to.haveOwnProperty('errors');
+      expect(res.body.errors).to.haveOwnProperty('firstName');
+      expect(res.body.errors.firstName).to.match(


Unexpected newline after '(' function-paren-newline

chialuka

Great job @abdulfatai360. Please, could you confirm that your work will throw an error when an unallowed variable is passed into the request body?

Adekoreday

Good job, setCustomMessage would have been better placed outside the userSignup.js
so it can be reusable for other input validations such as User Login, etc

abdulfatai360 · 2019-08-01T13:40:27Z

Great job @abdulfatai360. Please, could you confirm that your work will throw an error when an unallowed variable is passed into the request body?

@LukasChiama Thank you for your feedback. If you could remember, the decision to prevent an unwanted field from getting pass our validation middleware was why we changed from using express-validator to Joi as validation library. So I took that into serious consideration when implementing the feature.

According to Joi documentation, the allowUnknown property is set to false by default.

Refer to the link and screenshot below for evidences:
https://github.com/hapijs/joi/blob/master/API.md#anyvalidatevalue-options

- install joi validation library - install sinon spies and stubs library - write user signup schemas in a schema/ folder at server/ root - check user signup inouts for validation errors - return validation errors to users [Delivers #167190529]

abdulfatai360 · 2019-08-01T14:03:24Z

Good job, setCustomMessage would have been better placed outside the userSignup.js
so it can be reusable for other input validations such as User Login, etc

@Adekoreday Thanks for your feedback. What you pointed out will sure improve the code quality of our codebase.

I have implemented your feedback. And push the changes to the repo. Thanks, once again.

chialuka · 2019-08-01T14:10:54Z

server/schemas/userSignup.js

+    .max(254)
+    .regex(/\s/, { invert: true })
+    .error(setCustomMessage('password'))
+};


Please could you include the confirmPassword field here as well? It is required for a check in the controllers for checking that passwords match.

@LukasChiama I don't think that's part of our User schema

hound bot reviewed Aug 1, 2019

View reviewed changes

abdulfatai360 requested review from Adekoreday, chialuka, JuwonAbiola, dinobi, ayodejiAA and henryade August 1, 2019 08:41

abdulfatai360 added pr:review This is PR is currently under review by team type:feature A new feature labels Aug 1, 2019

dinobi reviewed Aug 1, 2019

View reviewed changes

abdulfatai360 force-pushed the ft-validate-user-signup-inputs-167190529 branch from 86945a8 to 20bc37f Compare August 1, 2019 11:52

hound bot reviewed Aug 1, 2019

View reviewed changes

chialuka reviewed Aug 1, 2019

View reviewed changes

Adekoreday suggested changes Aug 1, 2019

View reviewed changes

abdulfatai360 force-pushed the ft-validate-user-signup-inputs-167190529 branch from 20bc37f to 4a1b8f0 Compare August 1, 2019 13:53

chialuka reviewed Aug 1, 2019

View reviewed changes

henryade approved these changes Aug 1, 2019

View reviewed changes

Adekoreday self-requested a review August 1, 2019 14:18

Adekoreday approved these changes Aug 1, 2019

View reviewed changes

dinobi approved these changes Aug 1, 2019

View reviewed changes

dinobi added pr:ready This PR is ready to be merged and removed pr:review This is PR is currently under review by team labels Aug 1, 2019

dinobi merged commit df36c57 into develop Aug 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#167190529 Validate User Signup Inputs #19

#167190529 Validate User Signup Inputs #19

abdulfatai360 commented Aug 1, 2019 •

edited

Loading

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

hound bot Aug 1, 2019

dinobi Aug 1, 2019

abdulfatai360 Aug 1, 2019 •

edited

Loading

hound bot Aug 1, 2019

hound bot Aug 1, 2019

chialuka left a comment

Adekoreday left a comment

abdulfatai360 commented Aug 1, 2019

abdulfatai360 commented Aug 1, 2019

chialuka Aug 1, 2019

dinobi Aug 1, 2019

#167190529 Validate User Signup Inputs #19

#167190529 Validate User Signup Inputs #19

Conversation

abdulfatai360 commented Aug 1, 2019 • edited Loading

What does this PR do?

Description of Task proposed in this pull request?

How should this be manually tested (Quality Assurance)?

What are the relevant pivotal tracker stories?

Any background context you want to add (Operations Impact)?

What I have learned working on this feature:

Screenshots:

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

dinobi Aug 1, 2019

Choose a reason for hiding this comment

abdulfatai360 Aug 1, 2019 • edited Loading

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

hound bot Aug 1, 2019

Choose a reason for hiding this comment

chialuka left a comment

Choose a reason for hiding this comment

Adekoreday left a comment

Choose a reason for hiding this comment

abdulfatai360 commented Aug 1, 2019

abdulfatai360 commented Aug 1, 2019

chialuka Aug 1, 2019

Choose a reason for hiding this comment

dinobi Aug 1, 2019

Choose a reason for hiding this comment

abdulfatai360 commented Aug 1, 2019 •

edited

Loading

abdulfatai360 Aug 1, 2019 •

edited

Loading