diff --git a/src/notificationdb.js b/src/notificationdb.js index b352cde3d..dbc5ef944 100644 --- a/src/notificationdb.js +++ b/src/notificationdb.js @@ -1,12 +1,12 @@ 'use strict'; exports = module.exports = { - get: get, - getByUserIdAndTitle: getByUserIdAndTitle, - add: add, - update: update, - del: del, - listByUserIdPaged: listByUserIdPaged, + get, + getByUserIdAndTitle, + add, + update, + del, + list, // exported for testing _clear: clear @@ -103,22 +103,27 @@ function del(id, callback) { }); } -function listByUserIdPaged(userId, options, page, perPage, callback) { - assert.strictEqual(typeof userId, 'string'); - assert.strictEqual(typeof options, 'object'); - assert(options.acknowledged === null || typeof options.acknowledged === 'boolean'); +function list(filters, page, perPage, callback) { + assert.strictEqual(typeof filters, 'object'); assert.strictEqual(typeof page, 'number'); assert.strictEqual(typeof perPage, 'number'); assert.strictEqual(typeof callback, 'function'); - let args = [ userId ]; - let query = `SELECT ${NOTIFICATION_FIELDS} FROM notifications WHERE userId=?`; + let args = []; - if ('acknowledged' in options) { - query += ' AND acknowledged=?'; - args.push(options.acknowledged); + let where = []; + if ('userId' in filters) { + where.push('userId=?'); + args.push(filters.userId); } + if ('acknowledged' in filters) { + where.push('acknowledged=?'); + args.push(filters.acknowledged); + } + + let query = `SELECT ${NOTIFICATION_FIELDS} FROM notifications`; + if (where.length) query += ' WHERE ' + where.join(' AND '); query += ' ORDER BY creationTime DESC LIMIT ?,?'; args.push((page-1)*perPage); diff --git a/src/notifications.js b/src/notifications.js index cd832348c..9069802a1 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -2,8 +2,8 @@ exports = module.exports = { get, - ack, - listByUserIdPaged, + update, + list, onEvent, @@ -68,27 +68,25 @@ function get(id, callback) { }); } -function ack(id, callback) { +function update(id, data, callback) { assert.strictEqual(typeof id, 'string'); + assert.strictEqual(typeof data, 'object'); assert.strictEqual(typeof callback, 'function'); - notificationdb.update(id, { acknowledged: true }, function (error) { + notificationdb.update(id, data, function (error) { if (error) return callback(error); callback(null); }); } -// if acknowledged === null we return all, otherwise yes or no based on acknowledged as a boolean -function listByUserIdPaged(userId, options, page, perPage, callback) { - assert.strictEqual(typeof userId, 'string'); +function list(options, page, perPage, callback) { assert.strictEqual(typeof options, 'object'); - assert(options.acknowledged === null || typeof options.acknowledged === 'boolean'); assert.strictEqual(typeof page, 'number'); assert.strictEqual(typeof perPage, 'number'); assert.strictEqual(typeof callback, 'function'); - notificationdb.listByUserIdPaged(userId, options, page, perPage, function (error, result) { + notificationdb.list({ userId: options.userId, acknowledge: options.acknowledged }, page, perPage, function (error, result) { if (error) return callback(error); callback(null, result); diff --git a/src/routes/notifications.js b/src/routes/notifications.js index 80028e367..4c83be849 100644 --- a/src/routes/notifications.js +++ b/src/routes/notifications.js @@ -4,7 +4,7 @@ exports = module.exports = { verifyOwnership, get, list, - ack + update }; let assert = require('assert'), @@ -44,17 +44,20 @@ function list(req, res, next) { const acknowledged = req.query.acknowledged ? req.query.acknowledged === 'true' : null; - notifications.listByUserIdPaged(req.user.id, { acknowledged }, page, perPage, function (error, result) { + notifications.list({ userId: req.user.id, acknowledged }, page, perPage, function (error, result) { if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(200, { notifications: result })); }); } -function ack(req, res, next) { +function update(req, res, next) { assert.strictEqual(typeof req.params.notificationId, 'string'); + assert.strictEqual(typeof req.body, 'object'); - notifications.ack(req.params.notificationId, function (error) { + if (typeof req.body.acknowledged !== 'boolean') return next(new HttpError(400, 'acknowledged must be a booliean')); + + notifications.update(req.params.notificationId, { acknowledged: req.body.acknowledged }, function (error) { if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(204, {})); diff --git a/src/server.js b/src/server.js index 2c391ba2f..98368b0f0 100644 --- a/src/server.js +++ b/src/server.js @@ -130,10 +130,10 @@ function initializeExpressSync() { router.get ('/api/v1/tasks/:taskId/logstream', token, authorizeAdmin, routes.tasks.getLogStream); router.post('/api/v1/tasks/:taskId/stop', json, token, authorizeAdmin, routes.tasks.stopTask); - // notification routes + // notification routes (these are server level) router.get ('/api/v1/notifications', token, routes.notifications.verifyOwnership, routes.notifications.list); router.get ('/api/v1/notifications/:notificationId', token, routes.notifications.verifyOwnership, routes.notifications.get); - router.post('/api/v1/notifications/:notificationId', json, token, routes.notifications.verifyOwnership, routes.notifications.ack); + router.post('/api/v1/notifications/:notificationId', json, token, routes.notifications.verifyOwnership, routes.notifications.update); // backup routes router.get ('/api/v1/backups', token, authorizeAdmin, routes.backups.list); diff --git a/src/test/database-test.js b/src/test/database-test.js index 6160c1615..abbdfd494 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -225,7 +225,7 @@ describe('database', function () { }); it('can list by user', function (done) { - notificationdb.listByUserIdPaged(USER_0.id, 1, 100, function (error, result) { + notificationdb.list({ userId: USER_0.id }, 1, 100, function (error, result) { expect(error).to.equal(null); expect(result).to.be.an('array'); expect(result.length).to.equal(2);