From 854e20ac0bc6c2065fe65509578b8d82d831cd8b Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 15:31:16 +0100 Subject: [PATCH 01/15] add update user service and tests --- src/routes/users.js | 32 ------------- src/services/errors.js | 4 ++ src/services/users/index.js | 4 +- src/services/users/update-user.js | 31 ++++++++++++ src/services/users/update-user.test.js | 65 ++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 33 deletions(-) delete mode 100644 src/routes/users.js create mode 100644 src/services/users/update-user.js create mode 100644 src/services/users/update-user.test.js diff --git a/src/routes/users.js b/src/routes/users.js deleted file mode 100644 index 1e3acbc2..00000000 --- a/src/routes/users.js +++ /dev/null @@ -1,32 +0,0 @@ -const express = require("express") -const db = require("../db") -const isAuthenticated = require("../middleware/isAuthenticated") - -const router = express.Router() - -const fetchUsers = (req, res) => { - res.send("get users") -} -const createUsers = () => {} -const getUser = async (req, res) => { - const { id } = req.params - const { rows } = await db.query("SELECT * FROM users WHERE id = $1", [id]) - res.send(rows[0]) -} -const updateUser = () => {} -const deleteUser = () => {} - -// isAuthenticated middle to protect all posts related requests -router.use(isAuthenticated()) - -router - .route("/") - .get(fetchUsers) - .post(createUsers) -router - .route("/:id") - .get(getUser) - .patch(updateUser) - .delete(deleteUser) - -module.exports = router diff --git a/src/services/errors.js b/src/services/errors.js index 87785581..bae3a2a1 100644 --- a/src/services/errors.js +++ b/src/services/errors.js @@ -62,6 +62,10 @@ module.exports= { ArticleHasAlreadyBeenFlaggedError : { name : "ArticleHasAlreadyBeenFlaggedError", message : "Article has already been flagged" + }, + UserNotFoundError : { + name : "UserNotFoundError", + message : "User not found" } } \ No newline at end of file diff --git a/src/services/users/index.js b/src/services/users/index.js index a8745967..2feffe49 100644 --- a/src/services/users/index.js +++ b/src/services/users/index.js @@ -12,6 +12,7 @@ const updateRefreshToken = require("./update-refresh-token") const sendPasswordResetLink = require('./send-password-reset-link') const resetPassword = require('./reset-password') const createNewUser = require('./create-new-user') +const updateUser = require('./update-user') const { RefreshTokenIsInvalidError, @@ -158,5 +159,6 @@ module.exports = { getNewTokens, getInvitedUserDetail, sendPasswordResetLink, - resetPassword + resetPassword, + updateUser } diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js new file mode 100644 index 00000000..454914b8 --- /dev/null +++ b/src/services/users/update-user.js @@ -0,0 +1,31 @@ +const db = require("../../db") +const customError = require("../../lib/custom-error") +const { UserNotFoundError } = require("../errors") + + +const updateUser = async ({ + id, + firstName, + lastName, + email, + gender, + jobRole, + department, + address +}) => { + const { rows } = await db.query( + `UPDATE users SET "firstName" = $1, "lastName" = $2, + "email" = $3, "gender" = $4, "jobRole" = $5, "department" = $6, + "address" = $7 WHERE id = $8 RETURNING id, "firstName", "lastName", + "email", "gender", "jobRole", "department", "address", "refreshToken"`, + [firstName, lastName, email, gender, jobRole, department, address, id] + ) + const user = rows[0] + if(!user) { + throw customError(UserNotFoundError) + } + + return user +} + +module.exports = updateUser \ No newline at end of file diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js new file mode 100644 index 00000000..9bf9d0eb --- /dev/null +++ b/src/services/users/update-user.test.js @@ -0,0 +1,65 @@ +const { expect } = require('chai') +const { faker } = require('@faker-js/faker') +const db = require('../../db') +const {fixtures} = require('../../../test/utils') +const updateUser = require('./update-user') + + +describe('Update User', () => { + let user + let updatedInfo = {} + + beforeEach(async () => { + user = await fixtures.insertUser() + updatedInfo = { + firstName: faker.name.firstName(), + lastName: faker.name.lastName(), + email: faker.internet.email(), + gender: faker.name.gender(), + jobRole: faker.name.jobTitle(), + department: faker.name.jobArea(), + address: faker.address.streetAddress() + } + }) + + it('should throw an error if user does not exist', async () => { + const { id } = user + const { firstName, lastName, email, gender } = updatedInfo + await db.query( + `DELETE FROM users + WHERE id = $1`, [id]) + + return expect(updateUser({ id, firstName, lastName, email, gender })) + .to.be.rejectedWith( + 'User not found') + }) + + it('should update user', async () => { + const { id } = user + const { firstName, lastName, email, + gender, jobRole, department, address } = updatedInfo + const newUpdatedUser = await updateUser( + { id, firstName, lastName, email, + gender, jobRole, department, address }) + + const { rows } = await db.query( + `SELECT * FROM users + WHERE id = $1`, [id]) + + const updatedUser = rows[0] + + return expect(newUpdatedUser).to.eql({ + id: updatedUser.id, + firstName: updatedUser.firstName, + lastName: updatedUser.lastName, + email: updatedUser.email, + gender: updatedUser.gender, + jobRole: updatedUser.jobRole, + department: updatedUser.department, + address: updatedUser.address, + refreshToken: updatedUser.refreshToken + }) + + }) + +}) \ No newline at end of file From efecc169d00660d415374558605fdca16329bb4a Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 15:32:13 +0100 Subject: [PATCH 02/15] add update user route and schema --- src/routes/users/index.js | 92 ++++++++++ src/routes/users/update-user.2e.test.js | 228 ++++++++++++++++++++++++ src/schema.js | 37 +++- 3 files changed, 356 insertions(+), 1 deletion(-) create mode 100644 src/routes/users/index.js create mode 100644 src/routes/users/update-user.2e.test.js diff --git a/src/routes/users/index.js b/src/routes/users/index.js new file mode 100644 index 00000000..0d062261 --- /dev/null +++ b/src/routes/users/index.js @@ -0,0 +1,92 @@ +const express = require("express") +const db = require("../../db") +const isAuthenticated = require("../../middleware/isAuthenticated") +const userSevice = require('../../services/users') +const validateSchema = require('../../middleware/validateSchema') +const { + updateUserSchema +} = require("../../schema") +const { catchAsync, AppError } = require("../../lib") +const { UserNotFoundError } = require("../../services/errors") + +const ERROR_MAP = { + [ UserNotFoundError.name ] : 404 +} + +const router = express.Router() + +const fetchUsers = (req, res) => { + res.send("get users") +} +const createUsers = () => {} +const getUser = async (req, res) => { + const { id } = req.params + const { rows } = await db.query("SELECT * FROM users WHERE id = $1", [id]) + res.send(rows[0]) +} +const updateUser = catchAsync(async (req, res ) => { + const { + firstName, + lastName, + email, + gender, + jobRole, + department, + address + } = req.body + + const { id } = req.params + + const userDetails = await userSevice.updateUser({ + id, + firstName, + lastName, + email, + gender, + jobRole, + department, + address + }) + + res.status(200).json({ + status: "success", + data: { + userId: userDetails.id, + firstName: userDetails.firstName, + lastName: userDetails.lastName, + email: userDetails.email, + gender: userDetails.gender, + jobRole: userDetails.jobRole, + department: userDetails.department, + address: userDetails.address, + refreshToken: userDetails.refreshToken + } + }) +}) +const deleteUser = () => {} + +// isAuthenticated middle to protect all posts related requests +router.use(isAuthenticated()) + +router + .route("/") + .get(fetchUsers) + .post(createUsers) +router + .route("/:id") + .get(getUser) + .patch(validateSchema(updateUserSchema), updateUser) + .delete(deleteUser) + +router + .use((err, req, res, next)=> { + const error = err + error.success = false + if(ERROR_MAP[error.name] ){ + next(new AppError( error.message ,ERROR_MAP[error.name] )) + + } + next(err) + }) + +module.exports = router diff --git a/src/routes/users/update-user.2e.test.js b/src/routes/users/update-user.2e.test.js new file mode 100644 index 00000000..c8e482d7 --- /dev/null +++ b/src/routes/users/update-user.2e.test.js @@ -0,0 +1,228 @@ +const { expect } = require('chai') +const { faker } = require('@faker-js/faker') +const { fixtures } = require('../../../test/utils') + +describe('PATCH /users/:id', () => { + + describe('Failure', () => { + let user + let accessToken + before(async ()=>{ + user = await fixtures.insertUser() + const body = { id: user.id, email: user.email } + accessToken = fixtures.generateAccessToken( + {user : body} + ) + }) + it('should return 401 if request is not authenticated', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .expect(401) + ) + it('should return 400 if id is not a number', async () => { + const expectedError = { + "error": { + "message": "id must be a number" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/sdsdsd`) + .set('Authorization', `Bearer ${accessToken}`) + .send( + {firstName : faker.name.firstName(), + lastName : faker.name.lastName(), + email : faker.internet.email() + }) + .expect(400, expectedError) + + }) + + it('should return 400 if email is not a valid email', async () => { + const expectedError = { + "error": { + "message": "email must be a valid email" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send( + { email: 'sdsdsd', + firstName: user.firstName, + lastName: user.lastName }) + .expect(400, expectedError) + }) + it('should return 400 if firstName is not a string', async () => { + const expectedError = { + "error": { + "message": "firstName must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ firstName: 123, + lastName: user.lastName, + email: user.email }) + .expect(400, expectedError) + }) + it('should return 400 if lastName is not a string', async () => { + const expectedError = { + "error": { + "message": "lastName must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ lastName: 123, + firstName: user.firstName, + email: user.email }) + .expect(400, expectedError) + }) + it('should return 400 if jobRole is not a string', async () => { + const expectedError = { + "error": { + "message": "jobRole must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ jobRole: 123, + firstName: user.firstName, + lastName: user.lastName, + email: user.email }) + .expect(400, expectedError) + }) + it('should return 400 if department is not a string', async () => { + const expectedError = { + "error": { + "message": "department must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ department: 123, + firstName: user.firstName, + lastName: user.lastName, + email: user.email }) + .expect(400, expectedError) + }) + it('should return 400 if address is not a string', async () => { + const expectedError = { + "error": { + "message": "address must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ address: 123, + firstName: user.firstName, + lastName: user.lastName, + email: user.email }) + .expect(400, expectedError) + }) + + it('should return 400 if gender is not a string', async () => { + const expectedError = { + "error": { + "message": "gender must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + gender: 123 + }) + .expect(400, expectedError) + }) + + it('should return 404 if user is not found', async () => + + fixtures.api() + .patch(`/api/v1/users/${user.id + 2}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + address: faker.address.streetAddress() }) + .expect(404) + .then((res) => { + expect(res.body.status).eql('fail') + expect(res.body.message).eql('User not found') + }) + ) + + }) + describe('Success', () => { + let user + let accessToken + before(async ()=>{ + user = await fixtures.insertUser() + const body = { id: user.id, email: user.email } + accessToken = fixtures.generateAccessToken( + {user : body} + ) + }) + it('should return 200 if user is updated', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + address: faker.address.streetAddress(), + department: faker.name.jobArea(), + gender: faker.name.gender() + }) + .expect(200) + + ) + it('should return the right response', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + address: faker.address.streetAddress(), + department: faker.name.jobArea() + }) + .expect(200) + .then((res) => { + expect(res.body.status).eql('success') + expect(res.body.data).to.have.property('userId') + expect(res.body.data).to.have.property('firstName') + expect(res.body.data).to.have.property('lastName') + expect(res.body.data).to.have.property('email') + expect(res.body.data).to.have.property('address') + expect(res.body.data).to.have.property('department') + expect(res.body.data.firstName).to.be.a('string') + expect(res.body.data.lastName).to.be.a('string') + expect(res.body.data.email).to.be.a('string') + expect(res.body.data.userId).to.be.a('number') + expect(res.body.data.address).to.be.a('string') + expect(res.body.data.department).to.be.a('string') + }) + ) + + }) +}) \ No newline at end of file diff --git a/src/schema.js b/src/schema.js index 45a77db2..68a1462e 100644 --- a/src/schema.js +++ b/src/schema.js @@ -256,6 +256,40 @@ const unflagPostSchema = Joi.object({ .required() } }) + +const updateUserSchema = Joi.object({ + params: { + id: Joi.number() + .required() + }, + body: { + firstName: Joi.string() + .alphanum() + .min(3) + .max(30) + .required(), + + lastName: Joi.string() + .alphanum() + .min(3) + .max(30) + .required(), + + email: Joi.string() + .email() + .required(), + gender: Joi.string(), + jobRole: Joi.string() + .alphanum() + .min(3) + .max(100), + department: Joi.string() + .alphanum() + .min(3) + .max(100), + address: Joi.string() + } +}) module.exports = { authSchema, signinSchema, @@ -277,5 +311,6 @@ module.exports = { likePostSchema, unlikePostSchema, flagPostSchema, - unflagPostSchema + unflagPostSchema, + updateUserSchema } From 700f211b2ded86718c32fb0a75352a40e7decb78 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 15:43:49 +0100 Subject: [PATCH 03/15] add transform function --- src/routes/users/index.js | 16 +++++----------- src/routes/users/update-user.2e.test.js | 4 ++-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/routes/users/index.js b/src/routes/users/index.js index 0d062261..6f880e12 100644 --- a/src/routes/users/index.js +++ b/src/routes/users/index.js @@ -13,6 +13,10 @@ const ERROR_MAP = { [ UserNotFoundError.name ] : 404 } +const transformUserResponse = (userDetails) => ({ + ...userDetails } +) + const router = express.Router() const fetchUsers = (req, res) => { @@ -50,17 +54,7 @@ const updateUser = catchAsync(async (req, res ) => { res.status(200).json({ status: "success", - data: { - userId: userDetails.id, - firstName: userDetails.firstName, - lastName: userDetails.lastName, - email: userDetails.email, - gender: userDetails.gender, - jobRole: userDetails.jobRole, - department: userDetails.department, - address: userDetails.address, - refreshToken: userDetails.refreshToken - } + data: transformUserResponse(userDetails) }) }) const deleteUser = () => {} diff --git a/src/routes/users/update-user.2e.test.js b/src/routes/users/update-user.2e.test.js index c8e482d7..bceec0e9 100644 --- a/src/routes/users/update-user.2e.test.js +++ b/src/routes/users/update-user.2e.test.js @@ -209,7 +209,7 @@ describe('PATCH /users/:id', () => { .expect(200) .then((res) => { expect(res.body.status).eql('success') - expect(res.body.data).to.have.property('userId') + expect(res.body.data).to.have.property('id') expect(res.body.data).to.have.property('firstName') expect(res.body.data).to.have.property('lastName') expect(res.body.data).to.have.property('email') @@ -218,7 +218,7 @@ describe('PATCH /users/:id', () => { expect(res.body.data.firstName).to.be.a('string') expect(res.body.data.lastName).to.be.a('string') expect(res.body.data.email).to.be.a('string') - expect(res.body.data.userId).to.be.a('number') + expect(res.body.data.id).to.be.a('number') expect(res.body.data.address).to.be.a('string') expect(res.body.data.department).to.be.a('string') }) From e039a1044aed0065d123f5d1251353d5004b69d4 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 15:53:06 +0100 Subject: [PATCH 04/15] add docs --- docs/swagger.yaml | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index de53db56..4a5e85fa 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1379,6 +1379,60 @@ paths: type: string security: - access_token: [] + /users/{id}: + patch: + security: + - access_token: [] + tags: + - user + operationId: Update a user + summary: Update a user + description: Update a user + parameters: + - in : path + name : id + schema: + type: integer + required: true + description : Numeric ID of the user to get + requestBody: + description: Update a user + content: + application/json: + schema: + type: object + properties: + firstName: + type: string + description: First name of user + example: John + required: true + lastName: + type: string + description: Last name of user + example: Doe + required: true + email: + type: string + description: Email of user + example: arikeade@gmail.com + required: true + gender: + type: string + description: gender of the user + example: non-binary + address: + type: string + description: address of the user + example: 10, Ogunlana Drive, Surulere, Lagos + jobRole: + type: string + description: job role of the user + example: Software Developer + department: + type: string + description: department of the user + example: Software Development components: schemas: User: From 114200c5dfb5c662aa5450e53740a7cee47fac3c Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 19:21:48 +0100 Subject: [PATCH 05/15] update to include profilePictureUrl --- docs/swagger.yaml | 4 ++++ src/routes/users/index.js | 6 ++++-- src/routes/users/update-user.2e.test.js | 20 ++++++++++++++++++++ src/schema.js | 4 +++- src/services/users/update-user.js | 12 ++++++++---- src/services/users/update-user.test.js | 12 ++++++++---- 6 files changed, 47 insertions(+), 11 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index dc5e2b27..567a4ca2 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1449,6 +1449,10 @@ paths: type: string description: department of the user example: Software Development + profilePictureUrl: + type: string + description: profile picture url of the user + example: https://imgbb.com/ components: schemas: User: diff --git a/src/routes/users/index.js b/src/routes/users/index.js index 6f880e12..e61c5ec9 100644 --- a/src/routes/users/index.js +++ b/src/routes/users/index.js @@ -36,7 +36,8 @@ const updateUser = catchAsync(async (req, res ) => { gender, jobRole, department, - address + address, + profilePictureUrl } = req.body const { id } = req.params @@ -49,7 +50,8 @@ const updateUser = catchAsync(async (req, res ) => { gender, jobRole, department, - address + address, + profilePictureUrl }) res.status(200).json({ diff --git a/src/routes/users/update-user.2e.test.js b/src/routes/users/update-user.2e.test.js index bceec0e9..7be361b2 100644 --- a/src/routes/users/update-user.2e.test.js +++ b/src/routes/users/update-user.2e.test.js @@ -152,6 +152,26 @@ describe('PATCH /users/:id', () => { .expect(400, expectedError) }) + it('should return 400 if profilePictureUrl is not a valid url', + async () => { + const expectedError = { + "error": { + "message": "profilePictureUrl must be a valid uri" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + profilePictureUrl: "inavlid-profile-picture-url" + }) + .expect(400, expectedError) + }) + it('should return 404 if user is not found', async () => fixtures.api() diff --git a/src/schema.js b/src/schema.js index 94477bd0..45c72b38 100644 --- a/src/schema.js +++ b/src/schema.js @@ -289,7 +289,9 @@ const updateUserSchema = Joi.object({ .alphanum() .min(3) .max(100), - address: Joi.string() + address: Joi.string(), + profilePictureUrl: Joi.string() + .uri() } }) module.exports = { diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index 454914b8..a210c603 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -11,14 +11,18 @@ const updateUser = async ({ gender, jobRole, department, - address + address, + profilePictureUrl }) => { const { rows } = await db.query( `UPDATE users SET "firstName" = $1, "lastName" = $2, "email" = $3, "gender" = $4, "jobRole" = $5, "department" = $6, - "address" = $7 WHERE id = $8 RETURNING id, "firstName", "lastName", - "email", "gender", "jobRole", "department", "address", "refreshToken"`, - [firstName, lastName, email, gender, jobRole, department, address, id] + "address" = $7, "profilePictureUrl" = $8 WHERE id = $9 + RETURNING id, "firstName", "lastName", + "email", "gender", "jobRole", "department", "address", "refreshToken", + "profilePictureUrl"`, + [firstName, lastName, email, gender, jobRole, + department, address, profilePictureUrl, id] ) const user = rows[0] if(!user) { diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index 9bf9d0eb..b08cbd60 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -18,7 +18,8 @@ describe('Update User', () => { gender: faker.name.gender(), jobRole: faker.name.jobTitle(), department: faker.name.jobArea(), - address: faker.address.streetAddress() + address: faker.address.streetAddress(), + profilePictureUrl: faker.image.imageUrl() } }) @@ -37,10 +38,12 @@ describe('Update User', () => { it('should update user', async () => { const { id } = user const { firstName, lastName, email, - gender, jobRole, department, address } = updatedInfo + gender, jobRole, department, address, + profilePictureUrl } = updatedInfo const newUpdatedUser = await updateUser( { id, firstName, lastName, email, - gender, jobRole, department, address }) + gender, jobRole, department, address, + profilePictureUrl }) const { rows } = await db.query( `SELECT * FROM users @@ -57,7 +60,8 @@ describe('Update User', () => { jobRole: updatedUser.jobRole, department: updatedUser.department, address: updatedUser.address, - refreshToken: updatedUser.refreshToken + refreshToken: updatedUser.refreshToken, + profilePictureUrl: updatedUser.profilePictureUrl }) }) From df38eb4a1946263dc3920b85e6b17f77f44b72b6 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Mon, 10 Oct 2022 20:50:41 +0100 Subject: [PATCH 06/15] fix doc errors --- docs/swagger.yaml | 55 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index ff48f7f9..a040a30e 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1460,17 +1460,14 @@ paths: type: string description: First name of user example: John - required: true lastName: type: string description: Last name of user example: Doe - required: true email: type: string description: Email of user example: arikeade@gmail.com - required: true gender: type: string description: gender of the user @@ -1491,6 +1488,58 @@ paths: type: string description: profile picture url of the user example: https://imgbb.com/ + responses: + '200': + description: Successfully updating a user + content: + application/json: + schema: + type: object + properties: + status: + type: string + description: successful updating a user + example: success + data: + type: object + description: Data for updating a user + properties: + id: + type: integer + description: user id + example: 10 + firstName: + type: string + description: First name of user + example: John + lastName: + type: string + description: Last name of user + example: Doe + email: + type: string + description: Email of user + example: atilade@gmail.com + gender: + type: string + description: gender of the user + example: non-binary + address: + type: string + description: address of the user + example: 10, Ogunlana Drive, Surulere, Lagos + jobRole: + type: string + description: job role of the user + example: Software Developer + department: + type: string + description: department of the user + example: Software Development + refreshToken: + type: string + description: Refresh token + example: 44yytruy76iiui945436t components: schemas: User: From 187e53b0bbd6f6c9291c50519b2a845298639377 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Tue, 11 Oct 2022 08:52:34 +0100 Subject: [PATCH 07/15] make all fields optional --- docs/swagger.yaml | 4 -- src/routes/users/index.js | 2 - src/routes/users/update-user.2e.test.js | 53 +++---------------------- src/schema.js | 11 +---- src/services/users/update-user.js | 38 +++++++++++++----- src/services/users/update-user.test.js | 5 +-- 6 files changed, 37 insertions(+), 76 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index a040a30e..ac3586ca 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1464,10 +1464,6 @@ paths: type: string description: Last name of user example: Doe - email: - type: string - description: Email of user - example: arikeade@gmail.com gender: type: string description: gender of the user diff --git a/src/routes/users/index.js b/src/routes/users/index.js index e61c5ec9..7f8df7d8 100644 --- a/src/routes/users/index.js +++ b/src/routes/users/index.js @@ -32,7 +32,6 @@ const updateUser = catchAsync(async (req, res ) => { const { firstName, lastName, - email, gender, jobRole, department, @@ -46,7 +45,6 @@ const updateUser = catchAsync(async (req, res ) => { id, firstName, lastName, - email, gender, jobRole, department, diff --git a/src/routes/users/update-user.2e.test.js b/src/routes/users/update-user.2e.test.js index 7be361b2..e018e9fa 100644 --- a/src/routes/users/update-user.2e.test.js +++ b/src/routes/users/update-user.2e.test.js @@ -31,29 +31,12 @@ describe('PATCH /users/:id', () => { .set('Authorization', `Bearer ${accessToken}`) .send( {firstName : faker.name.firstName(), - lastName : faker.name.lastName(), - email : faker.internet.email() + lastName : faker.name.lastName() }) .expect(400, expectedError) }) - it('should return 400 if email is not a valid email', async () => { - const expectedError = { - "error": { - "message": "email must be a valid email" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send( - { email: 'sdsdsd', - firstName: user.firstName, - lastName: user.lastName }) - .expect(400, expectedError) - }) it('should return 400 if firstName is not a string', async () => { const expectedError = { "error": { @@ -65,8 +48,7 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) .send({ firstName: 123, - lastName: user.lastName, - email: user.email }) + lastName: user.lastName}) .expect(400, expectedError) }) it('should return 400 if lastName is not a string', async () => { @@ -80,8 +62,7 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) .send({ lastName: 123, - firstName: user.firstName, - email: user.email }) + firstName: user.firstName }) .expect(400, expectedError) }) it('should return 400 if jobRole is not a string', async () => { @@ -94,10 +75,7 @@ describe('PATCH /users/:id', () => { return fixtures.api() .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) - .send({ jobRole: 123, - firstName: user.firstName, - lastName: user.lastName, - email: user.email }) + .send({ jobRole: 123 }) .expect(400, expectedError) }) it('should return 400 if department is not a string', async () => { @@ -110,10 +88,7 @@ describe('PATCH /users/:id', () => { return fixtures.api() .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) - .send({ department: 123, - firstName: user.firstName, - lastName: user.lastName, - email: user.email }) + .send({ department: 123 }) .expect(400, expectedError) }) it('should return 400 if address is not a string', async () => { @@ -126,10 +101,7 @@ describe('PATCH /users/:id', () => { return fixtures.api() .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) - .send({ address: 123, - firstName: user.firstName, - lastName: user.lastName, - email: user.email }) + .send({ address: 123 }) .expect(400, expectedError) }) @@ -144,9 +116,6 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) .send({ - firstName: user.firstName, - lastName: user.lastName, - email: user.email, gender: 123 }) .expect(400, expectedError) @@ -164,9 +133,6 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) .send({ - firstName: user.firstName, - lastName: user.lastName, - email: user.email, profilePictureUrl: "inavlid-profile-picture-url" }) .expect(400, expectedError) @@ -178,9 +144,6 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id + 2}`) .set('Authorization', `Bearer ${accessToken}`) .send({ - firstName: user.firstName, - lastName: user.lastName, - email: user.email, address: faker.address.streetAddress() }) .expect(404) .then((res) => { @@ -206,8 +169,6 @@ describe('PATCH /users/:id', () => { .set('Authorization', `Bearer ${accessToken}`) .send({ firstName: user.firstName, - lastName: user.lastName, - email: user.email, address: faker.address.streetAddress(), department: faker.name.jobArea(), gender: faker.name.gender() @@ -220,9 +181,7 @@ describe('PATCH /users/:id', () => { .patch(`/api/v1/users/${user.id}`) .set('Authorization', `Bearer ${accessToken}`) .send({ - firstName: user.firstName, lastName: user.lastName, - email: user.email, address: faker.address.streetAddress(), department: faker.name.jobArea() }) diff --git a/src/schema.js b/src/schema.js index 45c72b38..4d28c961 100644 --- a/src/schema.js +++ b/src/schema.js @@ -268,18 +268,11 @@ const updateUserSchema = Joi.object({ firstName: Joi.string() .alphanum() .min(3) - .max(30) - .required(), - + .max(30), lastName: Joi.string() .alphanum() .min(3) - .max(30) - .required(), - - email: Joi.string() - .email() - .required(), + .max(30), gender: Joi.string(), jobRole: Joi.string() .alphanum() diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index a210c603..c2fbaf56 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -7,29 +7,45 @@ const updateUser = async ({ id, firstName, lastName, - email, gender, jobRole, department, address, profilePictureUrl }) => { + + const { rows:user } = await db.query( + `SELECT * FROM users WHERE id = $1`, [id]) + const userDefault = user[0] + + if(!userDefault) { + throw customError(UserNotFoundError) + } + + + const newFirstName = firstName || userDefault.firstName + const newLastName = lastName || userDefault.lastName + const newGender = gender || userDefault.gender + const newJobRole = jobRole || userDefault.jobRole + const newDepartment = department || userDefault.department + const newAddress = address || userDefault.address + const newProfilePictureUrl = + profilePictureUrl || userDefault.profilePictureUrl + + const { rows } = await db.query( `UPDATE users SET "firstName" = $1, "lastName" = $2, - "email" = $3, "gender" = $4, "jobRole" = $5, "department" = $6, - "address" = $7, "profilePictureUrl" = $8 WHERE id = $9 + "gender" = $3, "jobRole" = $4, "department" = $5, + "address" = $6, "profilePictureUrl" = $7 WHERE id = $8 RETURNING id, "firstName", "lastName", "email", "gender", "jobRole", "department", "address", "refreshToken", "profilePictureUrl"`, - [firstName, lastName, email, gender, jobRole, - department, address, profilePictureUrl, id] + [newFirstName, newLastName, newGender, newJobRole, + newDepartment, newAddress, newProfilePictureUrl, id] ) - const user = rows[0] - if(!user) { - throw customError(UserNotFoundError) - } - - return user + const userUpdated = rows[0] + + return userUpdated } module.exports = updateUser \ No newline at end of file diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index b08cbd60..353c0c3e 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -14,7 +14,6 @@ describe('Update User', () => { updatedInfo = { firstName: faker.name.firstName(), lastName: faker.name.lastName(), - email: faker.internet.email(), gender: faker.name.gender(), jobRole: faker.name.jobTitle(), department: faker.name.jobArea(), @@ -25,12 +24,12 @@ describe('Update User', () => { it('should throw an error if user does not exist', async () => { const { id } = user - const { firstName, lastName, email, gender } = updatedInfo + const { firstName, lastName, gender } = updatedInfo await db.query( `DELETE FROM users WHERE id = $1`, [id]) - return expect(updateUser({ id, firstName, lastName, email, gender })) + return expect(updateUser({ id, firstName, lastName, gender })) .to.be.rejectedWith( 'User not found') }) From e30ef015852850e3b8fad20d4b70964d2ac78405 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Wed, 12 Oct 2022 16:26:49 +0100 Subject: [PATCH 08/15] update user updates to use true patch --- docs/swagger.yaml | 36 ----------- src/routes/users/index.js | 24 ++------ src/services/users/update-user.js | 85 +++++++++++++++----------- src/services/users/update-user.test.js | 4 +- 4 files changed, 55 insertions(+), 94 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 82075c2e..b98987ec 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1773,42 +1773,6 @@ paths: type: string security: - access_token: [] - /tags/{tagId}: - delete: - tags: - - tag - operationId: Delete a tag - summary: Delete a tag - description: Delete a tag - parameters: - - in : path - name : tagId - schema: - type: integer - required: true - description : Numeric ID of the tag to get - responses: - '200': - description: Successfully deleting a tag - content: - application/json: - schema: - type: object - properties: - status: - type: string - description: successful deleting a tag - example: success - data: - type: object - description: Data for deleting a tag - properties: - message: - type: string - description: Message for deleting a tag - example: Tag has been successfully deleted - security: - - access_token: [] /users/{id}: patch: security: diff --git a/src/routes/users/index.js b/src/routes/users/index.js index 7f8df7d8..6d76a4cd 100644 --- a/src/routes/users/index.js +++ b/src/routes/users/index.js @@ -29,28 +29,12 @@ const getUser = async (req, res) => { res.send(rows[0]) } const updateUser = catchAsync(async (req, res ) => { - const { - firstName, - lastName, - gender, - jobRole, - department, - address, - profilePictureUrl - } = req.body - + const { id } = req.params - const userDetails = await userSevice.updateUser({ - id, - firstName, - lastName, - gender, - jobRole, - department, - address, - profilePictureUrl - }) + const requestBody = { ...req.body } + + const userDetails = await userSevice.updateUser(id, requestBody) res.status(200).json({ status: "success", diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index c2fbaf56..04178d5b 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -2,48 +2,61 @@ const db = require("../../db") const customError = require("../../lib/custom-error") const { UserNotFoundError } = require("../errors") +/** + * + * @param {Number} id - id of the user to be updated + * @param {Object} cols - columns to be updated + * @returns {String} - query to be executed + */ +const constructQuery = (id, cols) => { -const updateUser = async ({ - id, - firstName, - lastName, - gender, - jobRole, - department, - address, - profilePictureUrl -}) => { - - const { rows:user } = await db.query( - `SELECT * FROM users WHERE id = $1`, [id]) - const userDefault = user[0] - - if(!userDefault) { - throw customError(UserNotFoundError) - } - + // Setup static beginning of query + const query = ['UPDATE users'] + query.push('SET') + + // Create another array storing each set command + // and assigning a number value for parameterized query + const set = [] + Object.keys(cols).forEach((key, i) => { + set.push(`"${key}" = ($${(i + 1)})`) + }) + query.push(set.join(', ')) + + // Add the WHERE statement to look up by id + query.push(`WHERE id = ${id} RETURNING "id", + "firstName", "lastName", "email", + "gender", "jobRole", "department", + "address", "refreshToken", "profilePictureUrl"` ) + + // Return a complete query string + return query.join(' ') +} - const newFirstName = firstName || userDefault.firstName - const newLastName = lastName || userDefault.lastName - const newGender = gender || userDefault.gender - const newJobRole = jobRole || userDefault.jobRole - const newDepartment = department || userDefault.department - const newAddress = address || userDefault.address - const newProfilePictureUrl = - profilePictureUrl || userDefault.profilePictureUrl +/** + * + * @param {Number} id - id of the user to be updated + * @param {*} requestBody - reqquest body + * @returns {Object} - updated user + */ +const updateUser = async (id, + requestBody +) => { + const query = constructQuery(id, requestBody) - const { rows } = await db.query( - `UPDATE users SET "firstName" = $1, "lastName" = $2, - "gender" = $3, "jobRole" = $4, "department" = $5, - "address" = $6, "profilePictureUrl" = $7 WHERE id = $8 - RETURNING id, "firstName", "lastName", - "email", "gender", "jobRole", "department", "address", "refreshToken", - "profilePictureUrl"`, - [newFirstName, newLastName, newGender, newJobRole, - newDepartment, newAddress, newProfilePictureUrl, id] + const colValues = Object.keys( + requestBody ) + + const { rows } = await db.query( + query, colValues) + const userUpdated = rows[0] + + + if(!userUpdated) { + throw customError(UserNotFoundError) + } return userUpdated } diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index 353c0c3e..358eb087 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -29,7 +29,7 @@ describe('Update User', () => { `DELETE FROM users WHERE id = $1`, [id]) - return expect(updateUser({ id, firstName, lastName, gender })) + return expect(updateUser(id, { firstName, lastName, gender })) .to.be.rejectedWith( 'User not found') }) @@ -40,7 +40,7 @@ describe('Update User', () => { gender, jobRole, department, address, profilePictureUrl } = updatedInfo const newUpdatedUser = await updateUser( - { id, firstName, lastName, email, + id, { firstName, lastName, email, gender, jobRole, department, address, profilePictureUrl }) From 7d40f43cfc028e9c03297557fc21e09d9eecf389 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Wed, 12 Oct 2022 16:43:11 +0100 Subject: [PATCH 09/15] fix query constructor function --- src/services/users/update-user.js | 2 +- src/services/users/update-user.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index 04178d5b..0153a563 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -46,7 +46,7 @@ const updateUser = async (id, const colValues = Object.keys( requestBody - ) + ).map(key => requestBody[key]) const { rows } = await db.query( query, colValues) diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index 358eb087..9baedd02 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -36,11 +36,11 @@ describe('Update User', () => { it('should update user', async () => { const { id } = user - const { firstName, lastName, email, + const { firstName, lastName, gender, jobRole, department, address, profilePictureUrl } = updatedInfo const newUpdatedUser = await updateUser( - id, { firstName, lastName, email, + id, { firstName, lastName, gender, jobRole, department, address, profilePictureUrl }) From c1ae212708679e9edfcba7ee9edae6efebfef2ea Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Wed, 12 Oct 2022 20:15:36 +0100 Subject: [PATCH 10/15] implement reviews --- src/routes/auth/index.js | 16 +- src/routes/common/transformers.js | 16 +- src/routes/users/index.js | 9 +- src/routes/users/update-user.e2e.test.js | 207 +++++++++++++++++++++++ src/services/users/update-user.js | 22 +-- src/services/users/update-user.test.js | 8 +- 6 files changed, 247 insertions(+), 31 deletions(-) create mode 100644 src/routes/users/update-user.e2e.test.js diff --git a/src/routes/auth/index.js b/src/routes/auth/index.js index 4210909c..960d08d3 100644 --- a/src/routes/auth/index.js +++ b/src/routes/auth/index.js @@ -1,5 +1,5 @@ const express = require('express') -const userSevice = require('../../services/users') +const userService = require('../../services/users') const validateSchema = require('../../middleware/validateSchema') const isAuthenticated = require('../../middleware/isAuthenticated') const isAdmin = require('../../middleware/isAdmin') @@ -49,7 +49,7 @@ router.post( catchAsync(async (req, res) => { const { email, password } = req.body - const userDetails = await userSevice + const userDetails = await userService .signInUserByEmail(email, password) return res.json({ status: 'success', @@ -81,7 +81,7 @@ router.post( catchAsync(async (req, res) => { const { email } = req.body - const { email:userEmail, status } = await userSevice.inviteUser(email) + const { email:userEmail, status } = await userService.inviteUser(email) res.status(200).json({ status: 'success', @@ -109,7 +109,7 @@ router.post( const userDetails = - await userSevice.createNewUser({ + await userService.createNewUser({ firstName, lastName, email, @@ -138,7 +138,7 @@ router.post( refreshToken : currentRefreshToken } = req.body - const userDetails = await userSevice.getNewTokens( + const userDetails = await userService.getNewTokens( email, currentRefreshToken ) @@ -158,7 +158,7 @@ router.get( token } = req.params const userDetails = - await userSevice.getInvitedUserDetail( + await userService.getInvitedUserDetail( token ) @@ -177,7 +177,7 @@ router.post('/password', catchAsync(async (req, res) => { const { email } = req.body - await userSevice.sendPasswordResetLink(email) + await userService.sendPasswordResetLink(email) return res.status(200).json({ status: 'success', @@ -195,7 +195,7 @@ router.patch( const { token } = req.params const { newPassword } = req.body - await userSevice.resetPassword({ token, newPassword }) + await userService.resetPassword({ token, newPassword }) return res.status(200).json({ status: "success", diff --git a/src/routes/common/transformers.js b/src/routes/common/transformers.js index 3c757952..49e1cad6 100644 --- a/src/routes/common/transformers.js +++ b/src/routes/common/transformers.js @@ -20,7 +20,21 @@ const transformGifResponse = (gif) => ({ }) +const transformUserResponse = (userDetails) => ({ + id: userDetails.id, + firstName: userDetails.firstName, + lastName: userDetails.lastName, + email: userDetails.email, + gender: userDetails.gender, + jobRole: userDetails.jobRole, + department: userDetails.department, + address: userDetails.address, + profilePictureUrl: userDetails.profilePictureUrl +}) + + module.exports = { transformArticleResponse, - transformGifResponse + transformGifResponse, + transformUserResponse } \ No newline at end of file diff --git a/src/routes/users/index.js b/src/routes/users/index.js index 6d76a4cd..418c5ecb 100644 --- a/src/routes/users/index.js +++ b/src/routes/users/index.js @@ -1,21 +1,19 @@ const express = require("express") const db = require("../../db") const isAuthenticated = require("../../middleware/isAuthenticated") -const userSevice = require('../../services/users') +const userService = require('../../services/users') const validateSchema = require('../../middleware/validateSchema') const { updateUserSchema } = require("../../schema") const { catchAsync, AppError } = require("../../lib") const { UserNotFoundError } = require("../../services/errors") +const { transformUserResponse }= require("../common/transformers") const ERROR_MAP = { [ UserNotFoundError.name ] : 404 } -const transformUserResponse = (userDetails) => ({ - ...userDetails } -) const router = express.Router() @@ -32,9 +30,8 @@ const updateUser = catchAsync(async (req, res ) => { const { id } = req.params - const requestBody = { ...req.body } - const userDetails = await userSevice.updateUser(id, requestBody) + const userDetails = await userService.updateUser(id, req.body) res.status(200).json({ status: "success", diff --git a/src/routes/users/update-user.e2e.test.js b/src/routes/users/update-user.e2e.test.js new file mode 100644 index 00000000..e018e9fa --- /dev/null +++ b/src/routes/users/update-user.e2e.test.js @@ -0,0 +1,207 @@ +const { expect } = require('chai') +const { faker } = require('@faker-js/faker') +const { fixtures } = require('../../../test/utils') + +describe('PATCH /users/:id', () => { + + describe('Failure', () => { + let user + let accessToken + before(async ()=>{ + user = await fixtures.insertUser() + const body = { id: user.id, email: user.email } + accessToken = fixtures.generateAccessToken( + {user : body} + ) + }) + it('should return 401 if request is not authenticated', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .expect(401) + ) + it('should return 400 if id is not a number', async () => { + const expectedError = { + "error": { + "message": "id must be a number" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/sdsdsd`) + .set('Authorization', `Bearer ${accessToken}`) + .send( + {firstName : faker.name.firstName(), + lastName : faker.name.lastName() + }) + .expect(400, expectedError) + + }) + + it('should return 400 if firstName is not a string', async () => { + const expectedError = { + "error": { + "message": "firstName must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ firstName: 123, + lastName: user.lastName}) + .expect(400, expectedError) + }) + it('should return 400 if lastName is not a string', async () => { + const expectedError = { + "error": { + "message": "lastName must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ lastName: 123, + firstName: user.firstName }) + .expect(400, expectedError) + }) + it('should return 400 if jobRole is not a string', async () => { + const expectedError = { + "error": { + "message": "jobRole must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ jobRole: 123 }) + .expect(400, expectedError) + }) + it('should return 400 if department is not a string', async () => { + const expectedError = { + "error": { + "message": "department must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ department: 123 }) + .expect(400, expectedError) + }) + it('should return 400 if address is not a string', async () => { + const expectedError = { + "error": { + "message": "address must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ address: 123 }) + .expect(400, expectedError) + }) + + it('should return 400 if gender is not a string', async () => { + const expectedError = { + "error": { + "message": "gender must be a string" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + gender: 123 + }) + .expect(400, expectedError) + }) + + it('should return 400 if profilePictureUrl is not a valid url', + async () => { + const expectedError = { + "error": { + "message": "profilePictureUrl must be a valid uri" + }, + "status": "failed" + } + return fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + profilePictureUrl: "inavlid-profile-picture-url" + }) + .expect(400, expectedError) + }) + + it('should return 404 if user is not found', async () => + + fixtures.api() + .patch(`/api/v1/users/${user.id + 2}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + address: faker.address.streetAddress() }) + .expect(404) + .then((res) => { + expect(res.body.status).eql('fail') + expect(res.body.message).eql('User not found') + }) + ) + + }) + describe('Success', () => { + let user + let accessToken + before(async ()=>{ + user = await fixtures.insertUser() + const body = { id: user.id, email: user.email } + accessToken = fixtures.generateAccessToken( + {user : body} + ) + }) + it('should return 200 if user is updated', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + firstName: user.firstName, + address: faker.address.streetAddress(), + department: faker.name.jobArea(), + gender: faker.name.gender() + }) + .expect(200) + + ) + it('should return the right response', async () => + fixtures.api() + .patch(`/api/v1/users/${user.id}`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + lastName: user.lastName, + address: faker.address.streetAddress(), + department: faker.name.jobArea() + }) + .expect(200) + .then((res) => { + expect(res.body.status).eql('success') + expect(res.body.data).to.have.property('id') + expect(res.body.data).to.have.property('firstName') + expect(res.body.data).to.have.property('lastName') + expect(res.body.data).to.have.property('email') + expect(res.body.data).to.have.property('address') + expect(res.body.data).to.have.property('department') + expect(res.body.data.firstName).to.be.a('string') + expect(res.body.data.lastName).to.be.a('string') + expect(res.body.data.email).to.be.a('string') + expect(res.body.data.id).to.be.a('number') + expect(res.body.data.address).to.be.a('string') + expect(res.body.data.department).to.be.a('string') + }) + ) + + }) +}) \ No newline at end of file diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index 0153a563..74ed1a43 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -8,25 +8,23 @@ const { UserNotFoundError } = require("../errors") * @param {Object} cols - columns to be updated * @returns {String} - query to be executed */ -const constructQuery = (id, cols) => { +const constructQuery = (cols) => { // Setup static beginning of query const query = ['UPDATE users'] query.push('SET') - // Create another array storing each set command // and assigning a number value for parameterized query const set = [] Object.keys(cols).forEach((key, i) => { set.push(`"${key}" = ($${(i + 1)})`) }) + query.push(set.join(', ')) // Add the WHERE statement to look up by id - query.push(`WHERE id = ${id} RETURNING "id", - "firstName", "lastName", "email", - "gender", "jobRole", "department", - "address", "refreshToken", "profilePictureUrl"` ) + + query.push(`WHERE "id" = ($${set.length + 1}) RETURNING *` ) // Return a complete query string return query.join(' ') @@ -35,30 +33,32 @@ const constructQuery = (id, cols) => { /** * * @param {Number} id - id of the user to be updated - * @param {*} requestBody - reqquest body + * @param {Object} requestBody - request body * @returns {Object} - updated user */ const updateUser = async (id, requestBody ) => { - const query = constructQuery(id, requestBody) + const query = constructQuery(requestBody) const colValues = Object.keys( requestBody ).map(key => requestBody[key]) + colValues.push(id) + const { rows } = await db.query( query, colValues) - const userUpdated = rows[0] + const updatedUser = rows[0] - if(!userUpdated) { + if(!updatedUser) { throw customError(UserNotFoundError) } - return userUpdated + return updatedUser } module.exports = updateUser \ No newline at end of file diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index 9baedd02..14e02ca3 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -39,7 +39,7 @@ describe('Update User', () => { const { firstName, lastName, gender, jobRole, department, address, profilePictureUrl } = updatedInfo - const newUpdatedUser = await updateUser( + await updateUser( id, { firstName, lastName, gender, jobRole, department, address, profilePictureUrl }) @@ -50,16 +50,14 @@ describe('Update User', () => { const updatedUser = rows[0] - return expect(newUpdatedUser).to.eql({ - id: updatedUser.id, + + return expect(updatedInfo).to.eql({ firstName: updatedUser.firstName, lastName: updatedUser.lastName, - email: updatedUser.email, gender: updatedUser.gender, jobRole: updatedUser.jobRole, department: updatedUser.department, address: updatedUser.address, - refreshToken: updatedUser.refreshToken, profilePictureUrl: updatedUser.profilePictureUrl }) From fff7308e4332e61b8f84a73b1ca4a21f8965160c Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Wed, 12 Oct 2022 20:16:21 +0100 Subject: [PATCH 11/15] remove old test file --- src/routes/users/update-user.2e.test.js | 207 ------------------------ 1 file changed, 207 deletions(-) delete mode 100644 src/routes/users/update-user.2e.test.js diff --git a/src/routes/users/update-user.2e.test.js b/src/routes/users/update-user.2e.test.js deleted file mode 100644 index e018e9fa..00000000 --- a/src/routes/users/update-user.2e.test.js +++ /dev/null @@ -1,207 +0,0 @@ -const { expect } = require('chai') -const { faker } = require('@faker-js/faker') -const { fixtures } = require('../../../test/utils') - -describe('PATCH /users/:id', () => { - - describe('Failure', () => { - let user - let accessToken - before(async ()=>{ - user = await fixtures.insertUser() - const body = { id: user.id, email: user.email } - accessToken = fixtures.generateAccessToken( - {user : body} - ) - }) - it('should return 401 if request is not authenticated', async () => - fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .expect(401) - ) - it('should return 400 if id is not a number', async () => { - const expectedError = { - "error": { - "message": "id must be a number" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/sdsdsd`) - .set('Authorization', `Bearer ${accessToken}`) - .send( - {firstName : faker.name.firstName(), - lastName : faker.name.lastName() - }) - .expect(400, expectedError) - - }) - - it('should return 400 if firstName is not a string', async () => { - const expectedError = { - "error": { - "message": "firstName must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ firstName: 123, - lastName: user.lastName}) - .expect(400, expectedError) - }) - it('should return 400 if lastName is not a string', async () => { - const expectedError = { - "error": { - "message": "lastName must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ lastName: 123, - firstName: user.firstName }) - .expect(400, expectedError) - }) - it('should return 400 if jobRole is not a string', async () => { - const expectedError = { - "error": { - "message": "jobRole must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ jobRole: 123 }) - .expect(400, expectedError) - }) - it('should return 400 if department is not a string', async () => { - const expectedError = { - "error": { - "message": "department must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ department: 123 }) - .expect(400, expectedError) - }) - it('should return 400 if address is not a string', async () => { - const expectedError = { - "error": { - "message": "address must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ address: 123 }) - .expect(400, expectedError) - }) - - it('should return 400 if gender is not a string', async () => { - const expectedError = { - "error": { - "message": "gender must be a string" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - gender: 123 - }) - .expect(400, expectedError) - }) - - it('should return 400 if profilePictureUrl is not a valid url', - async () => { - const expectedError = { - "error": { - "message": "profilePictureUrl must be a valid uri" - }, - "status": "failed" - } - return fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - profilePictureUrl: "inavlid-profile-picture-url" - }) - .expect(400, expectedError) - }) - - it('should return 404 if user is not found', async () => - - fixtures.api() - .patch(`/api/v1/users/${user.id + 2}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - address: faker.address.streetAddress() }) - .expect(404) - .then((res) => { - expect(res.body.status).eql('fail') - expect(res.body.message).eql('User not found') - }) - ) - - }) - describe('Success', () => { - let user - let accessToken - before(async ()=>{ - user = await fixtures.insertUser() - const body = { id: user.id, email: user.email } - accessToken = fixtures.generateAccessToken( - {user : body} - ) - }) - it('should return 200 if user is updated', async () => - fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - firstName: user.firstName, - address: faker.address.streetAddress(), - department: faker.name.jobArea(), - gender: faker.name.gender() - }) - .expect(200) - - ) - it('should return the right response', async () => - fixtures.api() - .patch(`/api/v1/users/${user.id}`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - lastName: user.lastName, - address: faker.address.streetAddress(), - department: faker.name.jobArea() - }) - .expect(200) - .then((res) => { - expect(res.body.status).eql('success') - expect(res.body.data).to.have.property('id') - expect(res.body.data).to.have.property('firstName') - expect(res.body.data).to.have.property('lastName') - expect(res.body.data).to.have.property('email') - expect(res.body.data).to.have.property('address') - expect(res.body.data).to.have.property('department') - expect(res.body.data.firstName).to.be.a('string') - expect(res.body.data.lastName).to.be.a('string') - expect(res.body.data.email).to.be.a('string') - expect(res.body.data.id).to.be.a('number') - expect(res.body.data.address).to.be.a('string') - expect(res.body.data.department).to.be.a('string') - }) - ) - - }) -}) \ No newline at end of file From 8c0af2f084b339b4ba6f01d5ee4033b7f98e72cd Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Thu, 13 Oct 2022 08:47:09 +0100 Subject: [PATCH 12/15] implement review changes --- src/routes/common/transformers.js | 2 +- src/routes/users/update-user.e2e.test.js | 8 +------- src/services/users/update-user.test.js | 26 +++++++++--------------- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/routes/common/transformers.js b/src/routes/common/transformers.js index 49e1cad6..6316a7c8 100644 --- a/src/routes/common/transformers.js +++ b/src/routes/common/transformers.js @@ -21,7 +21,7 @@ const transformGifResponse = (gif) => ({ }) const transformUserResponse = (userDetails) => ({ - id: userDetails.id, + userId: userDetails.id, firstName: userDetails.firstName, lastName: userDetails.lastName, email: userDetails.email, diff --git a/src/routes/users/update-user.e2e.test.js b/src/routes/users/update-user.e2e.test.js index e018e9fa..85172d0e 100644 --- a/src/routes/users/update-user.e2e.test.js +++ b/src/routes/users/update-user.e2e.test.js @@ -188,16 +188,10 @@ describe('PATCH /users/:id', () => { .expect(200) .then((res) => { expect(res.body.status).eql('success') - expect(res.body.data).to.have.property('id') - expect(res.body.data).to.have.property('firstName') - expect(res.body.data).to.have.property('lastName') - expect(res.body.data).to.have.property('email') - expect(res.body.data).to.have.property('address') - expect(res.body.data).to.have.property('department') expect(res.body.data.firstName).to.be.a('string') expect(res.body.data.lastName).to.be.a('string') expect(res.body.data.email).to.be.a('string') - expect(res.body.data.id).to.be.a('number') + expect(res.body.data.userId).to.be.a('number') expect(res.body.data.address).to.be.a('string') expect(res.body.data.department).to.be.a('string') }) diff --git a/src/services/users/update-user.test.js b/src/services/users/update-user.test.js index 14e02ca3..d7bf2f13 100644 --- a/src/services/users/update-user.test.js +++ b/src/services/users/update-user.test.js @@ -24,25 +24,19 @@ describe('Update User', () => { it('should throw an error if user does not exist', async () => { const { id } = user - const { firstName, lastName, gender } = updatedInfo await db.query( `DELETE FROM users WHERE id = $1`, [id]) - return expect(updateUser(id, { firstName, lastName, gender })) + return expect(updateUser(id, updatedInfo)) .to.be.rejectedWith( 'User not found') }) it('should update user', async () => { const { id } = user - const { firstName, lastName, - gender, jobRole, department, address, - profilePictureUrl } = updatedInfo await updateUser( - id, { firstName, lastName, - gender, jobRole, department, address, - profilePictureUrl }) + id, updatedInfo) const { rows } = await db.query( `SELECT * FROM users @@ -51,14 +45,14 @@ describe('Update User', () => { const updatedUser = rows[0] - return expect(updatedInfo).to.eql({ - firstName: updatedUser.firstName, - lastName: updatedUser.lastName, - gender: updatedUser.gender, - jobRole: updatedUser.jobRole, - department: updatedUser.department, - address: updatedUser.address, - profilePictureUrl: updatedUser.profilePictureUrl + return expect(updatedUser).to.include({ + firstName: updatedInfo.firstName, + lastName: updatedInfo.lastName, + gender: updatedInfo.gender, + jobRole: updatedInfo.jobRole, + department: updatedInfo.department, + address: updatedInfo.address, + profilePictureUrl: updatedInfo.profilePictureUrl }) }) From 5f417b1c4ea028c77b1e03d84050cf46760570c8 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Thu, 13 Oct 2022 09:05:15 +0100 Subject: [PATCH 13/15] change signin response --- src/routes/auth/index.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/routes/auth/index.js b/src/routes/auth/index.js index 960d08d3..3692728a 100644 --- a/src/routes/auth/index.js +++ b/src/routes/auth/index.js @@ -4,6 +4,7 @@ const validateSchema = require('../../middleware/validateSchema') const isAuthenticated = require('../../middleware/isAuthenticated') const isAdmin = require('../../middleware/isAdmin') const { catchAsync , AppError} = require('../../lib') +const { transformUserResponse } = require('../common/transformers') const { @@ -36,7 +37,7 @@ const ERROR_MAP = { [ InvalidResetTokenError.name ] : 401 } -const transformUserResponse = (userDetails) => ({ +const transformCreateUserResponse = (userDetails) => ({ accessToken : userDetails.accessToken, userId : userDetails.userId, refreshToken : userDetails.refreshToken @@ -56,17 +57,8 @@ router.post( data:{ ...transformUserResponse(userDetails), refreshToken: userDetails.user.refreshToken, - userId : userDetails.user.id, - firstName: userDetails.user.firstName, - lastName: userDetails.user.lastName, - email: userDetails.user.email, - gender: userDetails.user.gender, - role: userDetails.user.role, - department: userDetails.user.department, - address: userDetails.user.department, - jobRole: userDetails.user.jobRole, createdAt : userDetails.user.createdAt, - profilePictureUrl: userDetails.user.profilePictureUrl + accessToken : userDetails.accessToken } }) }) @@ -122,7 +114,7 @@ router.post( status: 'success', data: { message: 'User account successfully created', - ...transformUserResponse(userDetails) + ...transformCreateUserResponse(userDetails) } }) }) From ab25d5ba4e39a6175ac3510c57a693a779d78ddb Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Thu, 13 Oct 2022 09:07:54 +0100 Subject: [PATCH 14/15] change signin response --- src/routes/auth/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/auth/index.js b/src/routes/auth/index.js index 3692728a..d61cee21 100644 --- a/src/routes/auth/index.js +++ b/src/routes/auth/index.js @@ -55,7 +55,7 @@ router.post( return res.json({ status: 'success', data:{ - ...transformUserResponse(userDetails), + ...transformUserResponse(userDetails.user), refreshToken: userDetails.user.refreshToken, createdAt : userDetails.user.createdAt, accessToken : userDetails.accessToken From 426ab4d51be3e5a3f5486b427f01cf1eb7e1cb22 Mon Sep 17 00:00:00 2001 From: Gideon Balogun Date: Thu, 13 Oct 2022 09:12:47 +0100 Subject: [PATCH 15/15] improve update user service --- src/services/users/update-user.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/services/users/update-user.js b/src/services/users/update-user.js index 74ed1a43..7ffd2265 100644 --- a/src/services/users/update-user.js +++ b/src/services/users/update-user.js @@ -37,14 +37,14 @@ const constructQuery = (cols) => { * @returns {Object} - updated user */ const updateUser = async (id, - requestBody + data ) => { - const query = constructQuery(requestBody) + const query = constructQuery(data) - const colValues = Object.keys( - requestBody - ).map(key => requestBody[key]) + const colValues = Object.values( + data + ) colValues.push(id)