digest cleanup and tests

src/user/digest.js

@@ -12,121 +12,117 @@ var plugins = require('../plugins');
 var emailer = require('../emailer');
 var utils = require('../utils');
 
-(function (Digest) {
+var Digest = module.exports;
-	Digest.execute = function (interval, callback) {
-		callback = callback || function () {};
 
-		var digestsDisabled = parseInt(meta.config.disableEmailSubscriptions, 10) === 1;
+Digest.execute = function (interval, callback) {
-		if (digestsDisabled) {
+	callback = callback || function () {};
-			winston.info('[user/jobs] Did not send digests (' + interval + ') because subscription system is disabled.');
-			return callback();
-		}
 
-		if (!interval) {
+	var digestsDisabled = parseInt(meta.config.disableEmailSubscriptions, 10) === 1;
-			// interval is one of: day, week, month, or year
+	if (digestsDisabled) {
-			interval = 'day';
+		winston.info('[user/jobs] Did not send digests (' + interval + ') because subscription system is disabled.');
-		}
+		return callback();
-		var subscribers;
+	}
-		async.waterfall([
-			function (next) {
-				async.parallel({
-					topics: async.apply(topics.getLatestTopics, 0, 0, 9, interval),
-					subscribers: async.apply(Digest.getSubscribers, interval),
-				}, next);
-			},
-			function (data, next) {
-				subscribers = data.subscribers;
-				if (!data.subscribers.length) {
-					return callback();
-				}
 
-				// Fix relative paths in topic data
+	var subscribers;
-				data.topics.topics = data.topics.topics.map(function (topicObj) {
+	async.waterfall([
-					var user = topicObj.hasOwnProperty('teaser') && topicObj.teaser !== undefined ? topicObj.teaser.user : topicObj.user;
+		function (next) {
-					if (user && user.picture && utils.isRelativeUrl(user.picture)) {
+			async.parallel({
-						user.picture = nconf.get('base_url') + user.picture;
+				topics: async.apply(topics.getLatestTopics, 0, 0, 9, interval),
-					}
+				subscribers: async.apply(Digest.getSubscribers, interval),
-
+			}, next);
-					return topicObj;
+		},
-				});
+		function (data, next) {
-
+			subscribers = data.subscribers;
-				data.interval = interval;
+			if (!data.subscribers.length) {
-				Digest.send(data, next);
+				return callback();
-			},
-		], function (err) {
-			if (err) {
-				winston.error('[user/jobs] Could not send digests (' + interval + '): ' + err.message);
-			} else {
-				winston.info('[user/jobs] Digest (' + interval + ') scheduling completed. ' + subscribers.length + ' email(s) sent.');
 			}
 
-			callback(err);
+			// Fix relative paths in topic data
-		});
+			data.topics.topics = data.topics.topics.map(function (topicObj) {
-	};
+				var user = topicObj.hasOwnProperty('teaser') && topicObj.teaser !== undefined ? topicObj.teaser.user : topicObj.user;
+				if (user && user.picture && utils.isRelativeUrl(user.picture)) {
+					user.picture = nconf.get('base_url') + user.picture;
+				}
 
-	Digest.getSubscribers = function (interval, callback) {
+				return topicObj;
-		async.waterfall([
+			});
-			function (next) {
-				db.getSortedSetRange('digest:' + interval + ':uids', 0, -1, next);
-			},
-			function (subscribers, next) {
-				plugins.fireHook('filter:digest.subscribers', {
-					interval: interval,
-					subscribers: subscribers,
-				}, next);
-			},
-			function (results, next) {
-				next(null, results.subscribers);
-			},
-		], callback);
-	};
 
-	Digest.send = function (data, callback) {
+			data.interval = interval;
-		if (!data || !data.subscribers || !data.subscribers.length) {
+			Digest.send(data, next);
-			return callback();
+		},
+	], function (err) {
+		if (err) {
+			winston.error('[user/jobs] Could not send digests (' + interval + '): ' + err.message);
+		} else {
+			winston.info('[user/jobs] Digest (' + interval + ') scheduling completed. ' + subscribers.length + ' email(s) sent.');
 		}
-		var now = new Date();
 
-		async.waterfall([
+		callback(err);
-			function (next) {
+	});
-				user.getUsersFields(data.subscribers, ['uid', 'username', 'userslug', 'lastonline'], next);
+};
-			},
+
-			function (users, next) {
+Digest.getSubscribers = function (interval, callback) {
-				async.eachLimit(users, 100, function (userObj, next) {
+	async.waterfall([
-					async.waterfall([
+		function (next) {
-						function (next) {
+			db.getSortedSetRange('digest:' + interval + ':uids', 0, -1, next);
-							user.notifications.getDailyUnread(userObj.uid, next);
+		},
-						},
+		function (subscribers, next) {
-						function (notifications, next) {
+			plugins.fireHook('filter:digest.subscribers', {
-							notifications = notifications.filter(Boolean);
+				interval: interval,
-							// If there are no notifications and no new topics, don't bother sending a digest
+				subscribers: subscribers,
-							if (!notifications.length && !data.topics.topics.length) {
+			}, next);
-								return next();
+		},
+		function (results, next) {
+			next(null, results.subscribers);
+		},
+	], callback);
+};
+
+Digest.send = function (data, callback) {
+	if (!data || !data.subscribers || !data.subscribers.length) {
+		return callback();
+	}
+	var now = new Date();
+
+	async.waterfall([
+		function (next) {
+			user.getUsersFields(data.subscribers, ['uid', 'username', 'userslug', 'lastonline'], next);
+		},
+		function (users, next) {
+			async.eachLimit(users, 100, function (userObj, next) {
+				async.waterfall([
+					function (next) {
+						user.notifications.getDailyUnread(userObj.uid, next);
+					},
+					function (notifications, next) {
+						notifications = notifications.filter(Boolean);
+						// If there are no notifications and no new topics, don't bother sending a digest
+						if (!notifications.length && !data.topics.topics.length) {
+							return next();
+						}
+
+						notifications.forEach(function (notification) {
+							if (notification.image && !notification.image.startsWith('http')) {
+								notification.image = nconf.get('url') + notification.image;
 							}
+						});
 
-							notifications.forEach(function (notification) {
+						emailer.send('digest', userObj.uid, {
-								if (notification.image && !notification.image.startsWith('http')) {
+							subject: '[' + meta.config.title + '] [[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]',
-									notification.image = nconf.get('url') + notification.image;
+							username: userObj.username,
-								}
+							userslug: userObj.userslug,
-							});
+							url: nconf.get('url'),
-
+							site_title: meta.config.title || meta.config.browserTitle || 'NodeBB',
-							emailer.send('digest', userObj.uid, {
+							notifications: notifications,
-								subject: '[' + meta.config.title + '] [[email:digest.subject, ' + (now.getFullYear() + '/' + (now.getMonth() + 1) + '/' + now.getDate()) + ']]',
+							recent: data.topics.topics,
-								username: userObj.username,
+							interval: data.interval,
-								userslug: userObj.userslug,
+						});
-								url: nconf.get('url'),
+						next();
-								site_title: meta.config.title || meta.config.browserTitle || 'NodeBB',
+					},
-								notifications: notifications,
+				], next);
-								recent: data.topics.topics,
+			}, next);
-								interval: data.interval,
+		},
-							});
+	], function (err) {
-							next();
+		callback(err);
-						},
+	});
-					], next);
+};
-				}, next);
-			},
-		], function (err) {
-			callback(err);
-		});
-	};
-}(module.exports));

src/user/jobs.js

@@ -10,7 +10,7 @@ var jobs = {};
 module.exports = function (User) {
 	User.startJobs = function (callback) {
 		winston.verbose('[user/jobs] (Re-)starting user jobs...');
-		var terminated = 0;
+
 		var started = 0;
 		var digestHour = parseInt(meta.config.digestHour, 10);
 
@@ -21,37 +21,12 @@ module.exports = function (User) {
 			digestHour = 0;
 		}
 
-		// Terminate any active cron jobs
+		User.stopJobs();
-		for (var jobId in jobs) {
-			if (jobs.hasOwnProperty(jobId)) {
-				winston.verbose('[user/jobs] Terminating job (' + jobId + ')');
-				jobs[jobId].stop();
-				delete jobs[jobId];
-				terminated += 1;
-			}
-		}
-		winston.verbose('[user/jobs] ' + terminated + ' jobs terminated');
 
-		jobs['digest.daily'] = new cronJob('0 ' + digestHour + ' * * *', function () {
+		startDigestJob('digest.daily', '0 ' + digestHour + ' * * *', 'day');
-			winston.verbose('[user/jobs] Digest job (daily) started.');
+		startDigestJob('digest.weekly', '0 ' + digestHour + ' * * 0', 'week');
-			User.digest.execute('day');
+		startDigestJob('digest.monthly', '0 ' + digestHour + ' 1 * *', 'month');
-		}, null, true);
+		started += 3;
-		winston.verbose('[user/jobs] Starting job (digest.daily)');
-		started += 1;
-
-		jobs['digest.weekly'] = new cronJob('0 ' + digestHour + ' * * 0', function () {
-			winston.verbose('[user/jobs] Digest job (weekly) started.');
-			User.digest.execute('week');
-		}, null, true);
-		winston.verbose('[user/jobs] Starting job (digest.weekly)');
-		started += 1;
-
-		jobs['digest.monthly'] = new cronJob('0 ' + digestHour + ' 1 * *', function () {
-			winston.verbose('[user/jobs] Digest job (monthly) started.');
-			User.digest.execute('month');
-		}, null, true);
-		winston.verbose('[user/jobs] Starting job (digest.monthly)');
-		started += 1;
 
 		jobs['reset.clean'] = new cronJob('0 0 * * *', User.reset.clean, null, true);
 		winston.verbose('[user/jobs] Starting job (reset.clean)');
@@ -63,5 +38,29 @@ module.exports = function (User) {
 			callback();
 		}
 	};
+
+	function startDigestJob(name, cronString, term) {
+		jobs[name] = new cronJob(cronString, function () {
+			winston.verbose('[user/jobs] Digest job (' + name + ') started.');
+			User.digest.execute(term);
+		}, null, true);
+		winston.verbose('[user/jobs] Starting job (' + name + ')');
+	}
+
+	User.stopJobs = function () {
+		var terminated = 0;
+		// Terminate any active cron jobs
+		for (var jobId in jobs) {
+			if (jobs.hasOwnProperty(jobId)) {
+				winston.verbose('[user/jobs] Terminating job (' + jobId + ')');
+				jobs[jobId].stop();
+				delete jobs[jobId];
+				terminated += 1;
+			}
+		}
+		if (terminated > 0) {
+			winston.verbose('[user/jobs] ' + terminated + ' jobs terminated');
+		}
+	};
 };
 

test/user.js

@@ -1301,6 +1301,30 @@ describe('User', function () {
 		});
 	});
 
+	describe('user jobs', function () {
+		it('should start user jobs', function (done) {
+			User.startJobs(function (err) {
+				assert.ifError(err);
+				done();
+			});
+		});
+
+		it('should stop user jobs', function (done) {
+			User.stopJobs();
+			done();
+		});
+
+		it('should send digetst', function (done) {
+			db.sortedSetAdd('digest:day:uids', [Date.now(), Date.now()], [1, 2], function (err) {
+				assert.ifError(err);
+				User.digest.execute('day', function (err) {
+					assert.ifError(err);
+					done();
+				});
+			});
+		});
+	});
+
 
 	after(function (done) {
 		db.emptydb(done);