notifications tests

src/messaging/rooms.js

@@ -34,7 +34,7 @@ module.exports = function (Messaging) {
 			function (roomData, next) {
 				modifyRoomData(roomData);
 				next(null, roomData);
-			}
+			},
 		], callback);
 	};
 

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) {
+		function (sets, next) {
-		if (err) {
+			sets = sets.map(function (set) {
-			return callback(err);
+				return set.mergeId;
-		}
+			});
 
-		sets = sets.map(function (set) {
+			next(null, _nids.filter(function (nid, idx) {
-			return set.mergeId;
+				return mergeIds.indexOf(sets[idx]) !== -1;
-		});
+			}));
-
+		},
-		callback(null, _nids.filter(function (nid, idx) {
+	], callback);
-			return mergeIds.indexOf(sets[idx]) !== -1;
-		}));
-	});
 };
 
 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) {
+	async.waterfall([
-		if (err) {
+		function (next) {
-			return callback(err);
+			db.getObject('notifications:' + data.nid, next);
-		}
+		},
-
+		function (oldNotification, next) {
-		if (oldNotification) {
+			if (oldNotification) {
-			if (parseInt(oldNotification.pid, 10) === parseInt(data.pid, 10) && parseInt(oldNotification.importance, 10) > parseInt(data.importance, 10)) {
+				if (parseInt(oldNotification.pid, 10) === parseInt(data.pid, 10) && parseInt(oldNotification.importance, 10) > parseInt(data.importance, 10)) {
-				return callback(null, null);
+					return callback(null, null);
+				}
 			}
-		}
+			var now = Date.now();
-
+			data.datetime = now;
-		var now = Date.now();
+			async.parallel([
-		data.datetime = now;
+				function (next) {
-		async.parallel([
+					db.sortedSetAdd('notifications', now, data.nid, next);
-			function (next) {
+				},
-				db.sortedSetAdd('notifications', now, data.nid, next);
+				function (next) {
-			},
+					db.setObject('notifications:' + data.nid, data, next);
-			function (next) {
+				},
-				db.setObject('notifications:' + data.nid, data, next);
+			], function (err) {
-			},
+				next(err, data);
-		], function (err) {
+			});
-			callback(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) {
+	async.waterfall([
-		if (err || !Array.isArray(members) || !members.length) {
+		function (next) {
-			return callback(err);
+			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) {
+	async.waterfall([
-		if (err) {
+		function (next) {
-			return callback(err);
+			groups.getMembersOfGroups(groupNames, next);
-		}
+		},
-
+		function (groupMembers, next) {
-		var members = _.unique(_.flatten(groupMembers));
+			var members = _.unique(_.flatten(groupMembers));
-		Notifications.push(notification, members, callback);
+			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) {
+		callback(err);
-			winston.error('Encountered error rescinding notification (' + nid + '): ' + err.message);
-		} else {
-			winston.verbose('[notifications/rescind] Rescinded notification "' + nid + '"');
-		}
-
-		callback(err, nid);
 	});
 };
 
@@ -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) {
+			async.parallel([
-		if (err || !notification) {
+				async.apply(db.sortedSetRemove, 'uid:' + uid + ':notifications:read', nid),
-			return callback(err || new Error('[[error:no-notification]]'));
+				async.apply(db.sortedSetAdd, 'uid:' + uid + ':notifications:unread', notification.datetime, nid),
-		}
+			], next);
-		notification.datetime = notification.datetime || Date.now();
+		},
-
+	], callback);
-		async.parallel([
-			async.apply(db.sortedSetRemove, 'uid:' + uid + ':notifications:read', nid),
-			async.apply(db.sortedSetAdd, 'uid:' + uid + ':notifications:unread', notification.datetime, nid),
-		], 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;
 			});
 

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;
+	var uid;
-	notifications.rescind(nid);
+	async.waterfall([
-
+		function (next) {
-	posts.getPostField(pid, 'uid', function (err, uid) {
+			notifications.rescind('upvote:post:' + pid + ':uid:' + fromuid, next);
-		if (err) {
+		},
-			return winston.error(err);
+		function (next) {
-		}
+			posts.getPostField(pid, 'uid', next);
-
+		},
-		user.notifications.getUnreadCount(uid, function (err, count) {
+		function (_uid, next) {
-			if (err) {
+			uid = _uid;
-				return winston.error(err);
+			user.notifications.getUnreadCount(uid, next);
-			}
+		},
-
+		function (count, next) {
 			websockets.in('uid_' + uid).emit('event:notifications.updateCount', count);
-		});
+			next();
+		},
+	], function (err) {
+		if (err) {
+			winston.error(err);
+		}
 	});
 };
 

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();
+						});
+					});
+				});
+			});
 		});
 	});