fix: #7949, delete old user notifs

src/notifications.js

@@ -1,21 +1,21 @@
 'use strict';
 
-var async = require('async');
+const async = require('async');
-var winston = require('winston');
+const winston = require('winston');
-var cron = require('cron').CronJob;
+const cron = require('cron').CronJob;
-var nconf = require('nconf');
+const nconf = require('nconf');
-var _ = require('lodash');
+const _ = require('lodash');
 
-var db = require('./database');
+const db = require('./database');
-var User = require('./user');
+const User = require('./user');
-var groups = require('./groups');
+const groups = require('./groups');
-var meta = require('./meta');
+const meta = require('./meta');
-var batch = require('./batch');
+const batch = require('./batch');
-var plugins = require('./plugins');
+const plugins = require('./plugins');
-var utils = require('./utils');
+const utils = require('./utils');
-var emailer = require('./emailer');
+const emailer = require('./emailer');
 
-var Notifications = module.exports;
+const Notifications = module.exports;
 
 Notifications.baseTypes = [
 	'notificationType_upvote',
@@ -97,7 +97,7 @@ Notifications.findRelated = async function (mergeIds, set) {
 	const keys = nids.map(nid => 'notifications:' + nid);
 	let sets = await db.getObjectsFields(keys, ['mergeId']);
 	sets = sets.map(set => String(set.mergeId));
-	var mergeSet = new Set(mergeIds.map(id => String(id)));
+	const mergeSet = new Set(mergeIds.map(id => String(id)));
 	return nids.filter((nid, idx) => mergeSet.has(sets[idx]));
 };
 
@@ -251,7 +251,7 @@ Notifications.markRead = async function (nid, uid) {
 };
 
 Notifications.markUnread = async function (nid, uid) {
-	if (parseInt(uid, 10) <= 0 || !nid) {
+	if (!(parseInt(uid, 10) > 0) || !nid) {
 		return;
 	}
 	const notification = await db.getObject('notifications:' + nid);
@@ -268,7 +268,7 @@ Notifications.markUnread = async function (nid, uid) {
 
 Notifications.markReadMultiple = async function (nids, uid) {
 	nids = nids.filter(Boolean);
-	if (!Array.isArray(nids) || !nids.length) {
+	if (!Array.isArray(nids) || !nids.length || !(parseInt(uid, 10) > 0)) {
 		return;
 	}
 
@@ -308,6 +308,13 @@ Notifications.prune = async function () {
 			db.sortedSetRemove('notifications', nids),
 			db.deleteAll(nids.map(nid => 'notifications:' + nid)),
 		]);
+
+		await batch.processSortedSet('users:joindate', async function (uids) {
+			await Promise.all([
+				db.sortedSetsRemoveRangeByScore(uids.map(uid => 'uid:' + uid + ':notifications:unread'), '-inf', cutoffTime),
+				db.sortedSetsRemoveRangeByScore(uids.map(uid => 'uid:' + uid + ':notifications:read'), '-inf', cutoffTime),
+			]);
+		}, { batch: 500, interval: 100 });
 	} catch (err) {
 		if (err) {
 			winston.error('Encountered error pruning notifications', err);
@@ -317,7 +324,7 @@ Notifications.prune = async function () {
 
 Notifications.merge = async function (notifications) {
 	// When passed a set of notification objects, merge any that can be merged
-	var mergeIds = [
+	const mergeIds = [
 		'notifications:upvoted_your_post_in',
 		'notifications:user_started_following_you',
 		'notifications:user_posted_to',
@@ -326,28 +333,16 @@ Notifications.merge = async function (notifications) {
 		'new_register',
 		'post-queue',
 	];
-	var isolated;
-	var differentiators;
-	var differentiator;
-	var modifyIndex;
-	var set;
 
 	notifications = mergeIds.reduce(function (notifications, mergeId) {
-		isolated = notifications.filter(function (notifObj) {
+		const isolated = notifications.filter(n => n && n.hasOwnProperty('mergeId') && n.mergeId.split('|')[0] === mergeId);
-			if (!notifObj || !notifObj.hasOwnProperty('mergeId')) {
-				return false;
-			}
-
-			return notifObj.mergeId.split('|')[0] === mergeId;
-		});
-
 		if (isolated.length <= 1) {
 			return notifications;	// Nothing to merge
 		}
 
 		// Each isolated mergeId may have multiple differentiators, so process each separately
-		differentiators = isolated.reduce(function (cur, next) {
+		const differentiators = isolated.reduce(function (cur, next) {
-			differentiator = next.mergeId.split('|')[1] || 0;
+			const differentiator = next.mergeId.split('|')[1] || 0;
 			if (!cur.includes(differentiator)) {
 				cur.push(differentiator);
 			}
@@ -356,29 +351,25 @@ Notifications.merge = async function (notifications) {
 		}, []);
 
 		differentiators.forEach(function (differentiator) {
+			let set;
 			if (differentiator === 0 && differentiators.length === 1) {
 				set = isolated;
 			} else {
-				set = isolated.filter(function (notifObj) {
+				set = isolated.filter(n => n.mergeId === (mergeId + '|' + differentiator));
-					return notifObj.mergeId === (mergeId + '|' + differentiator);
-				});
 			}
 
-			modifyIndex = notifications.indexOf(set[0]);
+			const modifyIndex = notifications.indexOf(set[0]);
 			if (modifyIndex === -1 || set.length === 1) {
 				return notifications;
 			}
 
 			switch (mergeId) {
-			// intentional fall-through
 			case 'notifications:upvoted_your_post_in':
 			case 'notifications:user_started_following_you':
 			case 'notifications:user_posted_to':
 			case 'notifications:user_flagged_post_in':
 			case 'notifications:user_flagged_user':
-				var usernames = _.uniq(set.map(function (notifObj) {
+				var usernames = _.uniq(set.map(notifObj => notifObj && notifObj.user && notifObj.user.username));
-					return notifObj && notifObj.user && notifObj.user.username;
-				}));
 				var numUsers = usernames.length;
 
 				var title = utils.decodeHTMLEntities(notifications[modifyIndex].topicTitle || '');

src/upgrade.js

@@ -73,24 +73,14 @@ Upgrade.appendPluginScripts = function (files, callback) {
 	], callback);
 };
 
-Upgrade.check = function (callback) {
+Upgrade.check = async function () {
 	// Throw 'schema-out-of-date' if not all upgrade scripts have run
-	async.waterfall([
+	const files = await Upgrade.getAll();
-		async.apply(Upgrade.getAll),
+	const executed = await db.getSortedSetRange('schemaLog', 0, -1);
-		function (files, next) {
+	const remainder = files.filter(name => !executed.includes(path.basename(name, '.js')));
-			db.getSortedSetRange('schemaLog', 0, -1, function (err, executed) {
+	if (remainder.length > 0) {
-				if (err) {
+		throw new Error('schema-out-of-date');
-					return callback(err);
 	}
-
-				var remainder = files.filter(function (name) {
-					return !executed.includes(path.basename(name, '.js'));
-				});
-
-				next(remainder.length > 0 ? new Error('schema-out-of-date') : null);
-			});
-		},
-	], callback);
 };
 
 Upgrade.run = function (callback) {
@@ -218,3 +208,5 @@ Upgrade.incrementProgress = function (value) {
 		process.stdout.write('    [' + (filled ? new Array(filled).join('#') : '') + new Array(unfilled).join(' ') + '] (' + this.current + '/' + (this.total || '??') + ') ' + percentage + ' ');
 	}
 };
+
+require('./promisify')(Upgrade);

src/upgrades/1.13.0/cleanup_old_notifications.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const db = require('../../database');
+const batch = require('../../batch');
+
+module.exports = {
+	name: 'Clean up old notifications',
+	timestamp: Date.UTC(2019, 9, 7),
+	method: async function (callback) {
+		const progress = this.progress;
+		const week = 604800000;
+		const cutoffTime = Date.now() - week;
+		await batch.processSortedSet('users:joindate', async function (uids) {
+			progress.incr(uids.length);
+			await Promise.all([
+				db.sortedSetsRemoveRangeByScore(uids.map(uid => 'uid:' + uid + ':notifications:unread'), '-inf', cutoffTime),
+				db.sortedSetsRemoveRangeByScore(uids.map(uid => 'uid:' + uid + ':notifications:read'), '-inf', cutoffTime),
+			]);
+		}, {
+			batch: 500,
+			progress: progress,
+		});
+		callback();
+	},
+};