#160690389 remove username from required signup details and make it updateable #36

madeofhuman · 2018-09-22T16:14:47Z

What does this PR do?

Removes the username property from required data for user signup, automatically create a username for the user from their first name, and make the username property updateable

Description of Task to be completed?

remove username property from required data for user signup
automatically create a username for the user from their first name
make the username property updateable

How should this be manually tested?

Clone the repository
Use the .env.example to fill in the .env file
Startup the command line
Run npm install
Run npm start
Make a POST request to /api/v1/auth/signup and create an account with the properties firstName, lastName, email, password, and confirmPassword. The username property will be automatically generated from your first name and added to your user details on creation.
Make a PUT request to /api/v1/users/:username and update your profile with the properties firstName, lastName, email, username, password, and confirmPassword. Note that your email address will cannot be changed.

Any background context you want to provide?

The username field was removed from the signup process so we can reduce the amount of information the user had to supply.

What are the relevant pivotal tracker stories?

#160690389

Screenshots (if appropriate)

N/A

Questions:

N/A

[Delivers #160690389]

- remove username requirement in req.body for user signup - automatically create username from firstname - remove tests that depend on presence of username in req.body [Delivers #160690389]

- make username updateable - add tests for this functionality - modify error handler to return custom error message param in next() [Delivers #160690389]

server/v1/controllers/AuthController.js

server/index.js

coveralls · 2018-09-22T16:17:29Z

Pull Request Test Coverage Report for Build 436

10 of 11 (90.91%) changed or added relevant lines in 3 files are covered.
2 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.3%) to 94.99%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
server/index.js	1	2	50.0%

Files with Coverage Reduction	New Missed Lines	%
server/v1/controllers/ProfileController.js	1	95.92%
server/v1/controllers/AuthController.js	1	91.25%

Totals
Change from base Build 416:	0.3%
Covered Lines:	719
Relevant Lines:	743

💛 - Coveralls

- add jsdoc for createUsername function - properly handle errors with the next() function [Fixes #160690389]

- properly test the createusername function [Delivers #160690389]

server/v1/controllers/AuthController.js

- remove unused argument when calling the createUsername method [Fixes #160690389]

- exclude user password from returned search data [Fixes #159204729]

chukwuemekachm

Well done, @madeofhuman.
I like how you created a separate method to help create a username for a new user in lieu of adding the logic to the signup user method. This makes the code neat.
However, I feel you should also create a method or mechanism to ensure that the username is always unique and belongs to a single user. This is because the username field on our User model has a unique constraint and if a user tries to update their profile with a username which already belongs to another user the operation/database will crash.
Please endeavour to resolve this.
Good job.

- return an error when user tries to update their username to an already exisiting one - increase test timeout to accommodate slow network calls [Fixes #160690389]

madeofhuman · 2018-09-26T16:19:00Z

Thanks for pointing that out, @chukwuemekachm . I have fixed it.

- test more edge cases for username update [Delivers #160690389]

chukwuemekachm

Welldone @madeofhuman.