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,22 +209,97 @@ 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] - expect(rootUser.username).to.equal('root') - - const user = users[1] + 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[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') + + 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() + }) + }) + it('Should update the user password', function (done) { usersUtils.updateUser(server.url, userId, accessTokenUser, 'new password', function (err, res) { if (err) throw err 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