Skip to content
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

#167891575 Implements Descriptive Signup/Registration Errors #15

Merged
merged 1 commit into from
Aug 26, 2019

Conversation

meetKazuki
Copy link
Contributor

@meetKazuki meetKazuki commented Aug 21, 2019

What does this PR do?

  • Implements validations during signup
  • Implements descriptive error messages during signup

Description of Task to be completed?

* Display error message when a user tries to signup with an already existing email
* Write tests
feature using Swagger

How should this manually be Tested?

  • git clone this repo
  • Checkout to this branch ft-signup-validation-167891575
  • Run npm i
  • Run npm test

What are the relevant Pivotal Tracker stories?

#167891575

Screenshots (if appropriate)

bansheePR__01
bansheePR__00

@@ -21,7 +21,7 @@ if (config.use_env_variable) {
}

fs.readdirSync(__dirname)
.filter(file => (file
.filter((file) => (file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected parentheses around single function argument having a body with no curly braces arrow-parens

src/database/migrations/20190817131405-create-users.js Outdated Show resolved Hide resolved
@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch from d2ec6fe to 6bcbf8b Compare August 22, 2019 03:24
test/routes/auth.test.js Outdated Show resolved Hide resolved
@@ -0,0 +1,29 @@
import chai, { should } from 'chai';
import chaiHttp from 'chai-http';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'chaiHttp' is defined but never used no-unused-vars

test/routes/auth.test.js Outdated Show resolved Hide resolved
const formatWord = word
.toLowerCase()
.split(' ')
.map((f) => f.charAt(0).toUpperCase() + f.substring(1))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected parentheses around single function argument having a body with no curly braces arrow-parens

@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch from 6bcbf8b to febcfd3 Compare August 22, 2019 15:40
defaultValue: Sequelize.fn('now')
}
})),
down: (queryInterface) => queryInterface.dropTable('Users')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected parentheses around single function argument having a body with no curly braces arrow-parens

@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch 6 times, most recently from 05593a7 to aa25c73 Compare August 26, 2019 02:44
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.gender).to.eql(
'Male or Female is the accepted value'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

test/auth.test.js Show resolved Hide resolved
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.gender).to.eql(
'Male or Female is the accepted value'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.gender).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.dob).to.eql(
'Users must be 18 and above'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.password).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.email).to.eql(
'Email address is invalid'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.email).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.lastName).to.eql(
'Last name should only contain alphabets'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.lastName).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch from aa25c73 to 7223882 Compare August 26, 2019 02:49
accumulator[key] = value.msg;
return accumulator;
}, {},
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

req = { ...req, ...matchedData(req) };

if (!errors.isEmpty()) {
const mapErrors = Object.entries(errors.mapped()).reduce(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

.isAlphanumeric()
.withMessage(
'Password must be alphanumeric and not be less than 8 characters'
),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.isLength({ min: 8, })
.withMessage('Password must be alphanumeric and not be less than 8 characters')
.isAlphanumeric()
.withMessage(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.lastName).to.eql(
'Last name should be between 2 to 15 characters'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.firstName).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.firstName).to.eql(
'First name should be between 2 to 15 characters'
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.end((err, res) => {
expect(res).to.have.status(400);
expect(res.body.message).to.eql('Validation Error!');
expect(res.body.data.firstName).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

expect(res).to.have.status(409);
expect(res.body.message).to.eql(
'User with email address already exist',
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline before ')' function-paren-newline

.send(user)
.end((err, res) => {
expect(res).to.have.status(409);
expect(res.body.message).to.eql(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected newline after '(' function-paren-newline

Copy link
Collaborator

@Pomile Pomile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice Job!
I like your creativity. However, I think your middleware/validator.js could be simpler. Please check express-validator website.

@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch from 09e171c to 5e66a6a Compare August 26, 2019 09:20
@meetKazuki meetKazuki force-pushed the ft-signup-validation-167891575 branch from 5e66a6a to 8e9ea9f Compare August 26, 2019 09:22
@daniellamarr daniellamarr merged commit 36f9a58 into develop Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants