Server: add user list sort/pagination

11 files changed, 199 insertions, 16 deletions

diff --git a/server/controllers/api/v1/users.js b/server/controllers/api/v1/users.js
index 704df770c..975e25e68 100644
--- a/server/controllers/api/v1/users.js
+++ b/server/controllers/api/v1/users.js
@@ -11,6 +11,10 @@ const logger = require('../../../helpers/logger')
 const middlewares = require('../../../middlewares')
 const admin = middlewares.admin
 const oAuth = middlewares.oauth
+const pagination = middlewares.pagination
+const sort = middlewares.sort
+const validatorsPagination = middlewares.validators.pagination
+const validatorsSort = middlewares.validators.sort
 const validatorsUsers = middlewares.validators.users
 
 const User = mongoose.model('User')
@@ -18,9 +22,16 @@ const Video = mongoose.model('Video')
 
 const router = express.Router()
 
-router.get('/', listUsers)
 router.get('/me', oAuth.authenticate, getUserInformation)
 
+router.get('/',
+  validatorsPagination.pagination,
+  validatorsSort.usersSort,
+  sort.setUsersSort,
+  pagination.setPagination,
+  listUsers
+)
+
 router.post('/',
   oAuth.authenticate,
   admin.ensureIsAdmin,
@@ -73,10 +84,10 @@ function getUserInformation (req, res, next) {
 }
 
 function listUsers (req, res, next) {
-  User.list(function (err, usersList) {
+  User.listForApi(req.query.start, req.query.count, req.query.sort, function (err, usersList, usersTotal) {
     if (err) return next(err)
 
-    res.json(getFormatedUsers(usersList))
+    res.json(getFormatedUsers(usersList, usersTotal))
   })
 }
 
@@ -145,7 +156,7 @@ function success (req, res, next) {
 
 // ---------------------------------------------------------------------------
 
-function getFormatedUsers (users) {
+function getFormatedUsers (users, usersTotal) {
   const formatedUsers = []
 
   users.forEach(function (user) {
@@ -153,6 +164,7 @@ function getFormatedUsers (users) {
   })
 
   return {
+    total: usersTotal,
     data: formatedUsers
   }
 }
diff --git a/server/initializers/checker.js b/server/initializers/checker.js
index 871d3cac2..4b6997547 100644
--- a/server/initializers/checker.js
+++ b/server/initializers/checker.js
@@ -39,7 +39,7 @@ function clientsExist (callback) {
 }
 
 function usersExist (callback) {
-  User.count(function (err, totalUsers) {
+  User.countTotal(function (err, totalUsers) {
     if (err) return callback(err)
 
     return callback(null, totalUsers !== 0)
diff --git a/server/initializers/constants.js b/server/initializers/constants.js
index 416356400..cd2e0cfb9 100644
--- a/server/initializers/constants.js
+++ b/server/initializers/constants.js
@@ -63,6 +63,7 @@ const SEEDS_IN_PARALLEL = 3
 
 // Sortable columns per schema
 const SORTABLE_COLUMNS = {
+  USERS: [ 'username', '-username', 'createdDate', '-createdDate' ],
   VIDEOS: [ 'name', '-name', 'duration', '-duration', 'createdDate', '-createdDate' ]
 }
 
diff --git a/server/middlewares/sort.js b/server/middlewares/sort.js
index 9f52290a6..8ed157805 100644
--- a/server/middlewares/sort.js
+++ b/server/middlewares/sort.js
@@ -1,9 +1,16 @@
 'use strict'
 
 const sortMiddleware = {
+  setUsersSort: setUsersSort,
   setVideosSort: setVideosSort
 }
 
+function setUsersSort (req, res, next) {
+  if (!req.query.sort) req.query.sort = '-createdDate'
+
+  return next()
+}
+
 function setVideosSort (req, res, next) {
   if (!req.query.sort) req.query.sort = '-createdDate'
 
diff --git a/server/middlewares/validators/sort.js b/server/middlewares/validators/sort.js
index 56b63cc8b..37b34ef52 100644
--- a/server/middlewares/validators/sort.js
+++ b/server/middlewares/validators/sort.js
@@ -5,9 +5,20 @@ const constants = require('../../initializers/constants')
 const logger = require('../../helpers/logger')
 
 const validatorsSort = {
+  usersSort: usersSort,
   videosSort: videosSort
 }
 
+function usersSort (req, res, next) {
+  const sortableColumns = constants.SORTABLE_COLUMNS.USERS
+
+  req.checkQuery('sort', 'Should have correct sortable column').optional().isIn(sortableColumns)
+
+  logger.debug('Checking sort parameters', { parameters: req.query })
+
+  checkErrors(req, res, next)
+}
+
 function videosSort (req, res, next) {
   const sortableColumns = constants.SORTABLE_COLUMNS.VIDEOS
 
diff --git a/server/models/user.js b/server/models/user.js
index d289da19a..c9c35b3e2 100644
--- a/server/models/user.js
+++ b/server/models/user.js
@@ -1,10 +1,15 @@
 const mongoose = require('mongoose')
 
 const customUsersValidators = require('../helpers/custom-validators').users
+const modelUtils = require('./utils')
 
 // ---------------------------------------------------------------------------
 
 const UserSchema = mongoose.Schema({
+  createdDate: {
+    type: Date,
+    default: Date.now
+  },
   password: String,
   username: String,
   role: String
@@ -19,9 +24,9 @@ UserSchema.methods = {
 }
 
 UserSchema.statics = {
-  count: count,
+  countTotal: countTotal,
   getByUsernameAndPassword: getByUsernameAndPassword,
-  list: list,
+  listForApi: listForApi,
   loadById: loadById,
   loadByUsername: loadByUsername
 }
@@ -30,7 +35,7 @@ mongoose.model('User', UserSchema)
 
 // ---------------------------------------------------------------------------
 
-function count (callback) {
+function countTotal (callback) {
   return this.count(callback)
 }
 
@@ -38,8 +43,9 @@ function getByUsernameAndPassword (username, password) {
   return this.findOne({ username: username, password: password })
 }
 
-function list (callback) {
-  return this.find(callback)
+function listForApi (start, count, sort, callback) {
+  const query = {}
+  return modelUtils.listForApiWithCount.call(this, query, start, count, sort, callback)
 }
 
 function loadById (id, callback) {
diff --git a/server/models/utils.js b/server/models/utils.js
new file mode 100644
index 000000000..a961e8c5b
--- /dev/null
+++ b/server/models/utils.js
@@ -0,0 +1,30 @@
+'use strict'
+
+const parallel = require('async/parallel')
+
+const utils = {
+  listForApiWithCount: listForApiWithCount
+}
+
+function listForApiWithCount (query, start, count, sort, callback) {
+  const self = this
+
+  parallel([
+    function (asyncCallback) {
+      self.find(query).skip(start).limit(count).sort(sort).exec(asyncCallback)
+    },
+    function (asyncCallback) {
+      self.count(query, asyncCallback)
+    }
+  ], function (err, results) {
+    if (err) return callback(err)
+
+    const data = results[0]
+    const total = results[1]
+    return callback(null, data, total)
+  })
+}
+
+// ---------------------------------------------------------------------------
+
+module.exports = utils
diff --git a/server/models/video.js b/server/models/video.js
index a5540d127..63afc2efe 100644
--- a/server/models/video.js
+++ b/server/models/video.js
@@ -197,7 +197,7 @@ function getDurationFromFile (videoPath, callback) {
 
 function listForApi (start, count, sort, callback) {
   const query = {}
-  return modelUtils.findWithCount.call(this, query, start, count, sort, callback)
+  return modelUtils.listForApiWithCount.call(this, query, start, count, sort, callback)
 }
 
 function listByUrlAndMagnet (fromUrl, magnetUri, callback) {
@@ -234,7 +234,7 @@ function search (value, field, start, count, sort, callback) {
     query[field] = new RegExp(value)
   }
 
-  modelUtils.findWithCount.call(this, query, start, count, sort, callback)
+  modelUtils.listForApiWithCount.call(this, query, start, count, sort, callback)
 }
 
 function seedAllExisting (callback) {
diff --git a/server/tests/api/check-params.js b/server/tests/api/check-params.js
index 882948fac..fc8b0a42a 100644
--- a/server/tests/api/check-params.js
+++ b/server/tests/api/check-params.js
@@ -459,6 +459,32 @@ describe('Test parameters validator', function () {
     let userId = null
     let userAccessToken = null
 
+    describe('When listing users', function () {
+      it('Should fail with a bad start pagination', function (done) {
+        request(server.url)
+          .get(path)
+          .query({ start: 'hello' })
+          .set('Accept', 'application/json')
+          .expect(400, done)
+      })
+
+      it('Should fail with a bad count pagination', function (done) {
+        request(server.url)
+          .get(path)
+          .query({ count: 'hello' })
+          .set('Accept', 'application/json')
+          .expect(400, done)
+      })
+
+      it('Should fail with an incorrect sort', function (done) {
+        request(server.url)
+          .get(path)
+          .query({ sort: 'hello' })
+          .set('Accept', 'application/json')
+          .expect(400, done)
+      })
+    })
+
     describe('When adding a new user', function () {
       it('Should fail with a too small username', function (done) {
         const data = {
diff --git a/server/tests/api/users.js b/server/tests/api/users.js
index a2557d2ab..c6c892bf2 100644
--- a/server/tests/api/users.js
+++ b/server/tests/api/users.js
@@ -209,17 +209,92 @@ describe('Test users', function () {
     usersUtils.getUsersList(server.url, function (err, res) {
       if (err) throw err
 
-      const users = res.body.data
+      const result = res.body
+      const total = result.total
+      const users = result.data
 
+      expect(total).to.equal(2)
       expect(users).to.be.an('array')
       expect(users.length).to.equal(2)
 
-      const rootUser = users[0]
+      const user = users[0]
+      expect(user.username).to.equal('user_1')
+
+      const rootUser = users[1]
       expect(rootUser.username).to.equal('root')
+      userId = user.id
+
+      done()
+    })
+  })
+
+  it('Should list only the first user by username asc', function (done) {
+    usersUtils.getUsersListPaginationAndSort(server.url, 0, 1, 'username', function (err, res) {
+      if (err) throw err
+
+      const result = res.body
+      const total = result.total
+      const users = result.data
+
+      expect(total).to.equal(2)
+      expect(users.length).to.equal(1)
 
-      const user = users[1]
+      const user = users[0]
+      expect(user.username).to.equal('root')
+
+      done()
+    })
+  })
+
+  it('Should list only the first user by username desc', function (done) {
+    usersUtils.getUsersListPaginationAndSort(server.url, 0, 1, '-username', function (err, res) {
+      if (err) throw err
+
+      const result = res.body
+      const total = result.total
+      const users = result.data
+
+      expect(total).to.equal(2)
+      expect(users.length).to.equal(1)
+
+      const user = users[0]
       expect(user.username).to.equal('user_1')
-      userId = user.id
+
+      done()
+    })
+  })
+
+  it('Should list only the second user by createdDate desc', function (done) {
+    usersUtils.getUsersListPaginationAndSort(server.url, 0, 1, '-createdDate', function (err, res) {
+      if (err) throw err
+
+      const result = res.body
+      const total = result.total
+      const users = result.data
+
+      expect(total).to.equal(2)
+      expect(users.length).to.equal(1)
+
+      const user = users[0]
+      expect(user.username).to.equal('user_1')
+
+      done()
+    })
+  })
+
+  it('Should list all the users by createdDate asc', function (done) {
+    usersUtils.getUsersListPaginationAndSort(server.url, 0, 2, 'createdDate', function (err, res) {
+      if (err) throw err
+
+      const result = res.body
+      const total = result.total
+      const users = result.data
+
+      expect(total).to.equal(2)
+      expect(users.length).to.equal(2)
+
+      expect(users[0].username).to.equal('root')
+      expect(users[1].username).to.equal('user_1')
 
       done()
     })
diff --git a/server/tests/utils/users.js b/server/tests/utils/users.js
index 3b560e409..0cf4e4adb 100644
--- a/server/tests/utils/users.js
+++ b/server/tests/utils/users.js
@@ -6,6 +6,7 @@ const usersUtils = {
   createUser: createUser,
   getUserInformation: getUserInformation,
   getUsersList: getUsersList,
+  getUsersListPaginationAndSort: getUsersListPaginationAndSort,
   removeUser: removeUser,
   updateUser: updateUser
 }
@@ -52,6 +53,20 @@ function getUsersList (url, end) {
     .end(end)
 }
 
+function getUsersListPaginationAndSort (url, start, count, sort, end) {
+  const path = '/api/v1/users'
+
+  request(url)
+    .get(path)
+    .query({ start: start })
+    .query({ count: count })
+    .query({ sort: sort })
+    .set('Accept', 'application/json')
+    .expect(200)
+    .expect('Content-Type', /json/)
+    .end(end)
+}
+
 function removeUser (url, userId, accessToken, expectedStatus, end) {
   if (!end) {
     end = expectedStatus