diff --git a/src/messaging/rooms.js b/src/messaging/rooms.js index 0d7e1d2b1f..41f09c5125 100644 --- a/src/messaging/rooms.js +++ b/src/messaging/rooms.js @@ -34,7 +34,7 @@ module.exports = function (Messaging) { function (roomData, next) { modifyRoomData(roomData); next(null, roomData); - } + }, ], callback); }; diff --git a/src/notifications.js b/src/notifications.js index 931e0ad293..d5505f88dd 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -29,7 +29,7 @@ Notifications.get = function (nid, callback) { }; Notifications.getMultiple = function (nids, callback) { - if (!nids.length) { + if (!Array.isArray(nids) || !nids.length) { return setImmediate(callback, null, []); } var keys = nids.map(function (nid) { @@ -106,50 +106,47 @@ Notifications.findRelated = function (mergeIds, set, callback) { db.getObjectsFields(keys, ['mergeId'], next); }, - ], function (err, sets) { - if (err) { - return callback(err); - } - - sets = sets.map(function (set) { - return set.mergeId; - }); + function (sets, next) { + sets = sets.map(function (set) { + return set.mergeId; + }); - callback(null, _nids.filter(function (nid, idx) { - return mergeIds.indexOf(sets[idx]) !== -1; - })); - }); + next(null, _nids.filter(function (nid, idx) { + return mergeIds.indexOf(sets[idx]) !== -1; + })); + }, + ], callback); }; Notifications.create = function (data, callback) { if (!data.nid) { - return callback(new Error('no-notification-id')); + return callback(new Error('[[error:no-notification-id]]')); } data.importance = data.importance || 5; - db.getObject('notifications:' + data.nid, function (err, oldNotification) { - if (err) { - return callback(err); - } - - if (oldNotification) { - if (parseInt(oldNotification.pid, 10) === parseInt(data.pid, 10) && parseInt(oldNotification.importance, 10) > parseInt(data.importance, 10)) { - return callback(null, null); + async.waterfall([ + function (next) { + db.getObject('notifications:' + data.nid, next); + }, + function (oldNotification, next) { + if (oldNotification) { + if (parseInt(oldNotification.pid, 10) === parseInt(data.pid, 10) && parseInt(oldNotification.importance, 10) > parseInt(data.importance, 10)) { + return callback(null, null); + } } - } - - var now = Date.now(); - data.datetime = now; - async.parallel([ - function (next) { - db.sortedSetAdd('notifications', now, data.nid, next); - }, - function (next) { - db.setObject('notifications:' + data.nid, data, next); - }, - ], function (err) { - callback(err, data); - }); - }); + var now = Date.now(); + data.datetime = now; + async.parallel([ + function (next) { + db.sortedSetAdd('notifications', now, data.nid, next); + }, + function (next) { + db.setObject('notifications:' + data.nid, data, next); + }, + ], function (err) { + next(err, data); + }); + }, + ], callback); }; Notifications.push = function (notification, uids, callback) { @@ -233,25 +230,31 @@ function pushToUids(uids, notification, callback) { Notifications.pushGroup = function (notification, groupName, callback) { callback = callback || function () {}; - groups.getMembers(groupName, 0, -1, function (err, members) { - if (err || !Array.isArray(members) || !members.length) { - return callback(err); - } + async.waterfall([ + function (next) { + groups.getMembers(groupName, 0, -1, next); + }, + function (members, next) { + if (!Array.isArray(members) || !members.length) { + return callback(); + } - Notifications.push(notification, members, callback); - }); + Notifications.push(notification, members, next); + }, + ], callback); }; Notifications.pushGroups = function (notification, groupNames, callback) { callback = callback || function () {}; - groups.getMembersOfGroups(groupNames, function (err, groupMembers) { - if (err) { - return callback(err); - } - - var members = _.unique(_.flatten(groupMembers)); - Notifications.push(notification, members, callback); - }); + async.waterfall([ + function (next) { + groups.getMembersOfGroups(groupNames, next); + }, + function (groupMembers, next) { + var members = _.unique(_.flatten(groupMembers)); + Notifications.push(notification, members, next); + }, + ], callback); }; Notifications.rescind = function (nid, callback) { @@ -261,13 +264,7 @@ Notifications.rescind = function (nid, callback) { async.apply(db.sortedSetRemove, 'notifications', nid), async.apply(db.delete, 'notifications:' + nid), ], function (err) { - if (err) { - winston.error('Encountered error rescinding notification (' + nid + '): ' + err.message); - } else { - winston.verbose('[notifications/rescind] Rescinded notification "' + nid + '"'); - } - - callback(err, nid); + callback(err); }); }; @@ -284,18 +281,22 @@ Notifications.markUnread = function (nid, uid, callback) { if (!parseInt(uid, 10) || !nid) { return callback(); } + async.waterfall([ + function (next) { + db.getObject('notifications:' + nid, next); + }, + function (notification, next) { + if (!notification) { + return callback(new Error('[[error:no-notification]]')); + } + notification.datetime = notification.datetime || Date.now(); - db.getObject('notifications:' + nid, function (err, notification) { - if (err || !notification) { - return callback(err || new Error('[[error:no-notification]]')); - } - notification.datetime = notification.datetime || Date.now(); - - async.parallel([ - async.apply(db.sortedSetRemove, 'uid:' + uid + ':notifications:read', nid), - async.apply(db.sortedSetAdd, 'uid:' + uid + ':notifications:unread', notification.datetime, nid), - ], callback); - }); + async.parallel([ + async.apply(db.sortedSetRemove, 'uid:' + uid + ':notifications:read', nid), + async.apply(db.sortedSetAdd, 'uid:' + uid + ':notifications:unread', notification.datetime, nid), + ], next); + }, + ], callback); }; Notifications.markReadMultiple = function (nids, uid, callback) { @@ -377,9 +378,9 @@ Notifications.markAllRead = function (uid, callback) { Notifications.prune = function (callback) { callback = callback || function () {}; - var week = 604800000; + var week = 604800000; - var cutoffTime = Date.now() - week; + var cutoffTime = Date.now() - week; async.waterfall([ function (next) { @@ -390,7 +391,7 @@ Notifications.prune = function (callback) { return callback(); } - var keys = nids.map(function (nid) { + var keys = nids.map(function (nid) { return 'notifications:' + nid; }); diff --git a/src/socket.io/helpers.js b/src/socket.io/helpers.js index 241937d5fb..9eedab2fb0 100644 --- a/src/socket.io/helpers.js +++ b/src/socket.io/helpers.js @@ -169,21 +169,26 @@ SocketHelpers.sendNotificationToTopicOwner = function (tid, fromuid, command, no }; SocketHelpers.rescindUpvoteNotification = function (pid, fromuid) { - var nid = 'upvote:post:' + pid + ':uid:' + fromuid; - notifications.rescind(nid); - - posts.getPostField(pid, 'uid', function (err, uid) { + var uid; + async.waterfall([ + function (next) { + notifications.rescind('upvote:post:' + pid + ':uid:' + fromuid, next); + }, + function (next) { + posts.getPostField(pid, 'uid', next); + }, + function (_uid, next) { + uid = _uid; + user.notifications.getUnreadCount(uid, next); + }, + function (count, next) { + websockets.in('uid_' + uid).emit('event:notifications.updateCount', count); + next(); + }, + ], function (err) { if (err) { - return winston.error(err); + winston.error(err); } - - user.notifications.getUnreadCount(uid, function (err, count) { - if (err) { - return winston.error(err); - } - - websockets.in('uid_' + uid).emit('event:notifications.updateCount', count); - }); }); }; diff --git a/test/notifications.js b/test/notifications.js index c5a22a6d2c..fbbf4fd49b 100644 --- a/test/notifications.js +++ b/test/notifications.js @@ -24,11 +24,19 @@ describe('Notifications', function () { }); }); + it('should fail to create notification without a nid', function (done) { + notifications.create({}, function (err) { + assert.equal(err.message, '[[error:no-notification-id]]'); + done(); + }); + }); + it('should create a notification', function (done) { notifications.create({ bodyShort: 'bodyShort', nid: 'notification_id', path: '/notification/path', + pid: 1, }, function (err, _notification) { notification = _notification; assert.ifError(err); @@ -45,6 +53,29 @@ describe('Notifications', function () { }); }); + it('should return null if pid is same and importance is lower', function (done) { + notifications.create({ + bodyShort: 'bodyShort', + nid: 'notification_id', + path: '/notification/path', + pid: 1, + importance: 1, + }, function (err, notification) { + assert.ifError(err); + assert.strictEqual(notification, null); + done(); + }); + }); + + it('should get empty array', function (done) { + notifications.getMultiple(null, function (err, data) { + assert.ifError(err); + assert(Array.isArray(data)); + assert.equal(data.length, 0); + done(); + }); + }); + it('should get notifications', function (done) { notifications.getMultiple([notification.nid], function (err, notificationsData) { assert.ifError(err); @@ -55,6 +86,19 @@ describe('Notifications', function () { }); }); + it('should do nothing', function (done) { + notifications.push(null, [], function (err) { + assert.ifError(err); + notifications.push({ nid: null }, [], function (err) { + assert.ifError(err); + notifications.push(notification, [], function (err) { + assert.ifError(err); + done(); + }); + }); + }); + }); + it('should push a notification to uid', function (done) { notifications.push(notification, [uid], function (err) { assert.ifError(err); @@ -94,6 +138,16 @@ describe('Notifications', function () { }); }); + it('should not mark anything with invalid uid or nid', function (done) { + socketNotifications.markRead({ uid: null }, null, function (err) { + assert.ifError(err); + socketNotifications.markRead({ uid: uid }, null, function (err) { + assert.ifError(err); + done(); + }); + }); + }); + it('should mark a notification read', function (done) { socketNotifications.markRead({ uid: uid }, notification.nid, function (err) { assert.ifError(err); @@ -109,6 +163,23 @@ describe('Notifications', function () { }); }); + it('should not mark anything with invalid uid or nid', function (done) { + socketNotifications.markUnread({ uid: null }, null, function (err) { + assert.ifError(err); + socketNotifications.markUnread({ uid: uid }, null, function (err) { + assert.ifError(err); + done(); + }); + }); + }); + + it('should error if notification does not exist', function (done) { + socketNotifications.markUnread({ uid: uid }, 123123, function (err) { + assert.equal(err.message, '[[error:no-notification]]'); + done(); + }); + }); + it('should mark a notification unread', function (done) { socketNotifications.markUnread({ uid: uid }, notification.nid, function (err) { assert.ifError(err); @@ -143,6 +214,13 @@ describe('Notifications', function () { }); }); + it('should not do anything', function (done) { + socketNotifications.markAllRead({ uid: 1000 }, null, function (err) { + assert.ifError(err); + done(); + }); + }); + it('should link to the first unread post in a watched topic', function (done) { var categories = require('../src/categories'); var topics = require('../src/topics'); @@ -256,9 +334,23 @@ describe('Notifications', function () { bodyShort: 'bodyShort', nid: 'tobedeleted', path: '/notification/path', - }, function (err) { + }, function (err, notification) { assert.ifError(err); - notifications.prune(done); + notifications.prune(function (err) { + assert.ifError(err); + var week = 604800000; + db.sortedSetAdd('notifications', Date.now() - (2 * week), notification.nid, function (err) { + assert.ifError(err); + notifications.prune(function (err) { + assert.ifError(err); + notifications.get(notification.nid, function (err, data) { + assert.ifError(err); + assert(!data); + done(); + }); + }); + }); + }); }); });