refactor(user): Back-ported changes in user controller

src/modules/users/controller.js

@@ -1,16 +1,23 @@
 const User = require('../../models/users')
 
-/**
+let _this
+class UserController {
+  constructor () {
+    _this = this
+    this.User = User
+  }
+
+  /**
  * @api {post} /users Create a new user
- * @apiPermission
+ * @apiPermission user
  * @apiName CreateUser
  * @apiGroup Users
  *
  * @apiExample Example usage:
- * curl -H "Content-Type: application/json" -X POST -d '{ "user": { "username": "johndoe", "password": "secretpasas" } }' localhost:5000/users
+ * curl -H "Content-Type: application/json" -X POST -d '{ "user": { "email": "email@format.com", "password": "secretpasas" } }' localhost:5001/users
  *
  * @apiParam {Object} user          User object (required)
- * @apiParam {String} user.username Username.
+ * @apiParam {String} user.email Email.
  * @apiParam {String} user.password Password.
  *
  * @apiSuccess {Object}   users           User object
@@ -18,6 +25,7 @@ const User = require('../../models/users')
  * @apiSuccess {String}   user.type       User type (admin or user)
  * @apiSuccess {String}   users.name      User name
  * @apiSuccess {String}   users.username  User username
+ * @apiSuccess {String}   users.email     User email
  *
  * @apiSuccessExample {json} Success-Response:
  *     HTTP/1.1 200 OK
@@ -25,7 +33,7 @@ const User = require('../../models/users')
  *       "user": {
  *          "_id": "56bd1da600a526986cf65c80"
  *          "name": "John Doe"
- *          "username": "johndoe"
+ *          "email": "email@format.com"
  *       }
  *     }
  *
@@ -38,17 +46,36 @@ const User = require('../../models/users')
  *       "error": "Unprocessable Entity"
  *     }
  */
-async function createUser (ctx) {
-  const user = new User(ctx.request.body.user)
+  async createUser (ctx) {
+    const user = new _this.User(ctx.request.body.user)
+
+    try {
+      /*
+       * ERROR HANDLERS
+       *
+       */
+      // Required property
+      if (!user.email || typeof user.email !== 'string') {
+        throw new Error(`Property 'email' must be a string!`)
+      }
+
+      const isEmail = await _this.validateEmail(user.email)
+      if (!isEmail) {
+        throw new Error(`Property 'email' must be email format!`)
+      }
+
+      if (!user.password || typeof user.password !== 'string') {
+        throw new Error(`Property 'password' must be a string!`)
+      }
+
+      if (user.name && typeof user.name !== 'string') {
+        throw new Error(`Property 'name' must be a string!`)
+      }
 
       // Enforce default value of 'user'
       user.type = 'user'
 
-  try {
       await user.save()
-  } catch (err) {
-    ctx.throw(422, err.message)
-  }
 
       const token = user.generateToken()
       const response = user.toJSON()
@@ -59,9 +86,12 @@ async function createUser (ctx) {
         user: response,
         token
       }
-}
+    } catch (err) {
+      ctx.throw(422, err.message)
+    }
+  }
 
-/**
+  /**
  * @api {get} /users Get all users
  * @apiPermission user
  * @apiName GetUsers
@@ -75,6 +105,7 @@ async function createUser (ctx) {
  * @apiSuccess {String}   user.type       User type (admin or user)
  * @apiSuccess {String}   users.name      User name
  * @apiSuccess {String}   users.username  User username
+ * @apiSuccess {String}   users.email     User email
  *
  * @apiSuccessExample {json} Success-Response:
  *     HTTP/1.1 200 OK
@@ -82,18 +113,22 @@ async function createUser (ctx) {
  *       "users": [{
  *          "_id": "56bd1da600a526986cf65c80"
  *          "name": "John Doe"
- *          "username": "johndoe"
+ *          "email": "email@format.com"
  *       }]
  *     }
  *
  * @apiUse TokenError
  */
-async function getUsers (ctx) {
-  const users = await User.find({}, '-password')
+  async getUsers (ctx) {
+    try {
+      const users = await _this.User.find({}, '-password')
       ctx.body = { users }
-}
+    } catch (error) {
+      ctx.throw(404)
+    }
+  }
 
-/**
+  /**
  * @api {get} /users/:id Get user by id
  * @apiPermission user
  * @apiName GetUser
@@ -107,6 +142,7 @@ async function getUsers (ctx) {
  * @apiSuccess {String}   user.type       User type (admin or user)
  * @apiSuccess {String}   users.name      User name
  * @apiSuccess {String}   users.username  User username
+ * @apiSuccess {String}   users.email     User email
  *
  * @apiSuccessExample {json} Success-Response:
  *     HTTP/1.1 200 OK
@@ -114,15 +150,15 @@ async function getUsers (ctx) {
  *       "user": {
  *          "_id": "56bd1da600a526986cf65c80"
  *          "name": "John Doe"
- *          "username": "johndoe"
+ *          "email": "email@format.com"
  *       }
  *     }
  *
  * @apiUse TokenError
  */
-async function getUser (ctx, next) {
+  async getUser (ctx, next) {
     try {
-    const user = await User.findById(ctx.params.id, '-password')
+      const user = await _this.User.findById(ctx.params.id, '-password')
       if (!user) {
         ctx.throw(404)
       }
@@ -137,15 +173,13 @@ async function getUser (ctx, next) {
 
       ctx.throw(500)
     }
-
     if (next) {
       return next()
     }
-}
-
-/**
+  }
+  /**
  * @api {put} /users/:id Update a user
- * @apiPermission
+ * @apiPermission user
  * @apiName UpdateUser
  * @apiGroup Users
  *
@@ -154,13 +188,14 @@ async function getUser (ctx, next) {
  *
  * @apiParam {Object} user          User object (required)
  * @apiParam {String} user.name     Name.
- * @apiParam {String} user.username Username.
+ * @apiParam {String} user.email Email.
  *
  * @apiSuccess {Object}   users           User object
  * @apiSuccess {ObjectId} users._id       User id
  * @apiSuccess {String}   user.type      User type (admin or user)
  * @apiSuccess {String}   users.name      Updated name
  * @apiSuccess {String}   users.username  Updated username
+ * @apiSuccess {String}   users.email     Updated email
  *
  * @apiSuccessExample {json} Success-Response:
  *     HTTP/1.1 200 OK
@@ -168,7 +203,7 @@ async function getUser (ctx, next) {
  *       "user": {
  *          "_id": "56bd1da600a526986cf65c80"
  *          "name": "Cool new name"
- *          "username": "johndoe"
+ *          "email": "email@format.com"
  *       }
  *     }
  *
@@ -183,29 +218,65 @@ async function getUser (ctx, next) {
  *
  * @apiUse TokenError
  */
-async function updateUser (ctx) {
-  const user = ctx.body.user
+  async updateUser (ctx) {
+    // Values obtain from user request.
+    // This variable is intended to validate the properties
+    // sent by the client
+    const userObj = ctx.request.body.user
 
+    const user = ctx.body.user
+    try {
+      /*
+       * ERROR HANDLERS
+       *
+       */
+      // Required property
+      if (userObj.email && typeof userObj.email !== 'string') {
+        throw new Error(`Property 'email' must be a string!`)
+      }
+      const isEmail = await _this.validateEmail(user.email)
+      if (userObj.email && !isEmail) {
+        throw new Error(`Property 'email' must be email format!`)
+      }
+      if (userObj.password && typeof userObj.password !== 'string') {
+        throw new Error(`Property 'password' must be a string!`)
+      }
+      if (userObj.name && typeof userObj.name !== 'string') {
+        throw new Error(`Property 'name' must be a string!`)
+      }
+      if (userObj.projects && !Array.isArray(userObj.projects)) {
+        throw new Error(`Property 'projects' must be a Array!`)
+      }
       // Save a copy of the original user type.
       const userType = user.type
 
-  Object.assign(user, ctx.request.body.user)
+      // If user type property is sent by the client
+      if (userObj.type) {
+        if (typeof userObj.type !== 'string') {
+          throw new Error(`Property 'type' must be a string!`)
+        }
+        // TODO: Here we can validate the user types allowed
 
         // Unless the calling user is an admin, they can not change the user type.
         if (userType !== 'admin') {
-    user.type = userType
+          throw new Error(`Property 'type' just can change for Admin user`)
         }
+      }
+
+      Object.assign(user, ctx.request.body.user)
 
       await user.save()
 
       ctx.body = {
         user
       }
-}
-
-/**
+    } catch (error) {
+      ctx.throw(422, error.message)
+    }
+  }
+  /**
  * @api {delete} /users/:id Delete a user
- * @apiPermission
+ * @apiPermission user
  * @apiName DeleteUser
  * @apiGroup Users
  *
@@ -222,22 +293,22 @@ async function updateUser (ctx) {
  *
  * @apiUse TokenError
  */
-
-async function deleteUser (ctx) {
+  async deleteUser (ctx) {
     const user = ctx.body.user
-
     await user.remove()
-
     ctx.status = 200
     ctx.body = {
       success: true
     }
+  }
+  // Validate Email Format
+  async validateEmail (email) {
+    // eslint-disable-next-line no-useless-escape
+    if (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(email)) {
+      return true
+    }
+    return false
+  }
 }
 
-module.exports = {
-  createUser,
-  getUsers,
-  getUser,
-  updateUser,
-  deleteUser
-}
+module.exports = UserController

src/modules/users/router.js

@@ -1,5 +1,6 @@
 const validator = require('../../middleware/validators')
-const user = require('./controller')
+const CONTROLLER = require('./controller')
+const controller = new CONTROLLER()
 
 // export const baseUrl = '/users'
 module.exports.baseUrl = '/users'
@@ -8,26 +9,40 @@ module.exports.routes = [
   {
     method: 'POST',
     route: '/',
-    handlers: [user.createUser]
+    handlers: [controller.createUser]
   },
   {
     method: 'GET',
     route: '/',
-    handlers: [validator.ensureUser, user.getUsers]
+    handlers: [
+      validator.ensureUser,
+      controller.getUsers
+    ]
   },
   {
     method: 'GET',
     route: '/:id',
-    handlers: [validator.ensureUser, user.getUser]
+    handlers: [
+      validator.ensureUser,
+      controller.getUser
+    ]
   },
   {
     method: 'PUT',
     route: '/:id',
-    handlers: [validator.ensureTargetUserOrAdmin, user.getUser, user.updateUser]
+    handlers: [
+      validator.ensureTargetUserOrAdmin,
+      controller.getUser,
+      controller.updateUser
+    ]
   },
   {
     method: 'DELETE',
     route: '/:id',
-    handlers: [validator.ensureTargetUserOrAdmin, user.getUser, user.deleteUser]
+    handlers: [
+      validator.ensureTargetUserOrAdmin,
+      controller.getUser,
+      controller.deleteUser
+    ]
   }
 ]

test/a02-users.spec.js

@@ -67,8 +67,77 @@ describe('Users', () => {
         }
       }
     })
+    it('should reject signup  if no email property is provided', async () => {
+      try {
+        const options = {
+          method: 'POST',
+          uri: `${LOCALHOST}/users`,
+          resolveWithFullResponse: true,
+          json: true,
+          body: {
+            user: {
+              password: 'supersecretpassword'
+            }
+          }
+        }
 
-    it('should sign up', async () => {
+        await rp(options)
+      } catch (err) {
+        assert.equal(err.statusCode, 422)
+        assert.include(
+          err.message,
+          `Property 'email' must be a string`
+        )
+      }
+    })
+    it('should reject signup  if  email property  provided is wrong format', async () => {
+      try {
+        const options = {
+          method: 'POST',
+          uri: `${LOCALHOST}/users`,
+          resolveWithFullResponse: true,
+          json: true,
+          body: {
+            user: {
+              email: 'badEmailFormat',
+              password: 'test'
+            }
+          }
+        }
+
+        await rp(options)
+      } catch (err) {
+        assert.equal(err.statusCode, 422)
+        assert.include(
+          err.message,
+          `Property 'email' must be email format`
+        )
+      }
+    })
+    it('should reject signup  if no password property is provided', async () => {
+      try {
+        const options = {
+          method: 'POST',
+          uri: `${LOCALHOST}/users`,
+          resolveWithFullResponse: true,
+          json: true,
+          body: {
+            user: {
+              email: 'test2@test.com'
+            }
+          }
+        }
+
+        await rp(options)
+      } catch (err) {
+        assert.equal(err.statusCode, 422)
+        assert.include(
+          err.message,
+          `Property 'password' must be a string`
+        )
+      }
+    })
+    it('should signup of type user by default', async () => {
       try {
         const options = {
           method: 'POST',
@@ -99,6 +168,7 @@ describe('Users', () => {
           'Password expected to be omited'
         )
         assert.property(result.body, 'token', 'Token property exists.')
+        assert.equal(result.body.user.type, 'user')
       } catch (err) {
         console.log(
           'Error authenticating test user: ' + JSON.stringify(err, null, 2)
@@ -330,7 +400,7 @@ describe('Users', () => {
       }
     })
 
-    it('should update user', async () => {
+    it('should update user with minimum inputs', async () => {
       const {
         user: { _id },
         token
@@ -363,6 +433,45 @@ describe('Users', () => {
       )
       assert.equal(user.email, 'testToUpdate@test.com')
     })
+    it('should update user with all inputs', async () => {
+      const {
+        user: { _id },
+        token
+      } = context
+
+      const options = {
+        method: 'PUT',
+        uri: `${LOCALHOST}/users/${_id}`,
+        resolveWithFullResponse: true,
+        json: true,
+        headers: {
+          Accept: 'application/json',
+          Authorization: `Bearer ${token}`
+        },
+        body: {
+          user: {
+            email: 'testToUpdate@test.com',
+            name: 'my name',
+            username: 'myUsername'
+          }
+        }
+      }
+
+      const result = await rp(options)
+      const user = result.body.user
+      // console.log(`user: ${util.inspect(user)}`)
+
+      assert.hasAnyKeys(user, ['type', '_id', 'email', 'name'])
+      assert.equal(user._id, _id)
+      assert.notProperty(
+        user,
+        'password',
+        'Password property should not be returned'
+      )
+      assert.equal(user.name, 'my name')
+      assert.equal(user.email, 'testToUpdate@test.com')
+      assert.equal(user.username, 'myUsername')
+    })
 
     it('should not be able to update user type', async () => {
       try {
@@ -382,16 +491,18 @@ describe('Users', () => {
           }
         }
 
-        let result = await rp(options)
+        const result = await rp(options)
 
         // console.log(`Users: ${JSON.stringify(result, null, 2)}`)
 
         assert(result.statusCode === 200, 'Status Code 200 expected.')
         assert(result.body.user.type === 'user', 'Type should be unchanged.')
       } catch (err) {
-        console.error('Error: ', err)
-        console.log('Error stringified: ' + JSON.stringify(err, null, 2))
-        throw err
+        assert.equal(err.statusCode, 422)
+        assert.include(
+          err.message,
+          "Property 'type' just can change for Admin user"
+        )
       }
     })
 
@@ -451,6 +562,69 @@ describe('Users', () => {
       const userName = result.body.user.name
       assert.equal(userName, 'This should work')
     })
+    it('should not be able to update if name property is wrong', async () => {
+      const {
+        user: { _id },
+        token
+      } = context
+
+      const options = {
+        method: 'PUT',
+        uri: `${LOCALHOST}/users/${_id}`,
+        resolveWithFullResponse: true,
+        json: true,
+        headers: {
+          Accept: 'application/json',
+          Authorization: `Bearer ${token}`
+        },
+        body: {
+          user: {
+            email: 'testToUpdate@test.com',
+            name: {}
+          }
+        }
+      }
+      try {
+        const result = await rp(options)
+        const user = result.body.user
+        // console.log(`user: ${util.inspect(user)}`)
+
+        assert.hasAnyKeys(user, ['type', '_id', 'username'])
+        assert.equal(user._id, _id)
+        assert.notProperty(
+          user,
+          'password',
+          'Password property should not be returned'
+        )
+        assert.notEqual(user.username, 'updatedcoolname31')
+      } catch (error) {
+        assert.equal(error.statusCode, 422)
+        assert.include(error.message, "Property 'name' must be a string!")
+      }
+    })
+    it('should not be able to update  if  email property  provided is wrong format', async () => {
+      try {
+        const options = {
+          method: 'POST',
+          uri: `${LOCALHOST}/users`,
+          resolveWithFullResponse: true,
+          json: true,
+          body: {
+            user: {
+              email: 'badEmailFormat'
+            }
+          }
+        }
+
+        await rp(options)
+      } catch (err) {
+        assert.equal(err.statusCode, 422)
+        assert.include(
+          err.message,
+          `Property 'email' must be email format`
+        )
+      }
+    })
   })
 
   describe('DELETE /users/:id', () => {