fix zsets and update them on post move, closes #6823 (#6827)

src/categories/data.js

@@ -28,9 +28,15 @@ module.exports = function (Categories) {
 			return;
 		}
 
-		category.name = validator.escape(String(category.name || ''));
+		if (category.hasOwnProperty('name')) {
-		category.disabled = category.hasOwnProperty('disabled') ? parseInt(category.disabled, 10) === 1 : undefined;
+			category.name = validator.escape(String(category.name || ''));
-		category.isSection = category.hasOwnProperty('isSection') ? parseInt(category.isSection, 10) === 1 : undefined;
+		}
+		if (category.hasOwnProperty('disabled')) {
+			category.disabled = parseInt(category.disabled, 10) === 1;
+		}
+		if (category.hasOwnProperty('isSection')) {
+			category.isSection = parseInt(category.isSection, 10) === 1;
+		}
 
 		if (category.hasOwnProperty('icon')) {
 			category.icon = category.icon || 'hidden';

src/posts/delete.js

@@ -31,7 +31,7 @@ module.exports = function (Posts) {
 				postData.cid = topicData.cid;
 				async.parallel([
 					function (next) {
-						updateTopicTimestamp(topicData, next);
+						topics.updateLastPostTimeFromLastPid(postData.tid, next);
 					},
 					function (next) {
 						db.sortedSetRemove('cid:' + topicData.cid + ':pids', pid, next);
@@ -68,7 +68,7 @@ module.exports = function (Posts) {
 				postData.cid = topicData.cid;
 				async.parallel([
 					function (next) {
-						updateTopicTimestamp(topicData, next);
+						topics.updateLastPostTimeFromLastPid(topicData.tid, next);
 					},
 					function (next) {
 						db.sortedSetAdd('cid:' + topicData.cid + ':pids', postData.timestamp, pid, next);
@@ -85,35 +85,6 @@ module.exports = function (Posts) {
 		], callback);
 	};
 
-	function updateTopicTimestamp(topicData, callback) {
-		var timestamp;
-		async.waterfall([
-			function (next) {
-				topics.getLatestUndeletedPid(topicData.tid, next);
-			},
-			function (pid, next) {
-				if (!parseInt(pid, 10)) {
-					return callback();
-				}
-				Posts.getPostField(pid, 'timestamp', next);
-			},
-			function (_timestamp, next) {
-				timestamp = _timestamp;
-				if (!parseInt(timestamp, 10)) {
-					return callback();
-				}
-				topics.updateTimestamp(topicData.tid, timestamp, next);
-			},
-			function (next) {
-				if (parseInt(topicData.pinned, 10) !== 1) {
-					db.sortedSetAdd('cid:' + topicData.cid + ':tids', timestamp, topicData.tid, next);
-				} else {
-					next();
-				}
-			},
-		], callback);
-	}
-
 	Posts.purge = function (pid, uid, callback) {
 		async.waterfall([
 			function (next) {
@@ -194,10 +165,14 @@ module.exports = function (Posts) {
 						topics.updateTeaser(postData.tid, next);
 					},
 					function (next) {
-						updateTopicTimestamp(topicData, next);
+						topics.updateLastPostTimeFromLastPid(postData.tid, next);
 					},
 					function (next) {
-						db.sortedSetIncrBy('cid:' + topicData.cid + ':tids:posts', -1, postData.tid, next);
+						if (parseInt(topicData.pinned, 10) !== 1) {
+							db.sortedSetIncrBy('cid:' + topicData.cid + ':tids:posts', -1, postData.tid, next);
+						} else {
+							next();
+						}
 					},
 					function (next) {
 						db.sortedSetIncrBy('tid:' + postData.tid + ':posters', -1, postData.uid, next);

src/topics/data.js

@@ -92,12 +92,19 @@ module.exports = function (Topics) {
 		if (!topic) {
 			return;
 		}
+		if (topic.hasOwnProperty('title')) {
+			topic.titleRaw = topic.title;
+			topic.title = String(topic.title);
+		}
 
-		topic.titleRaw = topic.title;
-		topic.title = String(topic.title);
 		escapeTitle(topic);
-		topic.timestampISO = utils.toISOString(topic.timestamp);
+		if (topic.hasOwnProperty('timestamp')) {
-		topic.lastposttimeISO = utils.toISOString(topic.lastposttime);
+			topic.timestampISO = utils.toISOString(topic.timestamp);
+		}
+		if (topic.hasOwnProperty('lastposttime')) {
+			topic.lastposttimeISO = utils.toISOString(topic.lastposttime);
+		}
+
 		if (topic.hasOwnProperty('upvotes')) {
 			topic.upvotes = parseInt(topic.upvotes, 10) || 0;
 		}

src/topics/fork.js

@@ -68,7 +68,7 @@ module.exports = function (Topics) {
 				}, next);
 			},
 			function (next) {
-				Topics.updateTimestamp(tid, Date.now(), next);
+				Topics.updateLastPostTime(tid, Date.now(), next);
 			},
 			function (next) {
 				plugins.fireHook('action:topic.fork', { tid: tid, fromTid: fromTid, uid: uid });
@@ -106,7 +106,7 @@ module.exports = function (Topics) {
 			function (next) {
 				async.parallel([
 					function (next) {
-						updateCategoryPostCount(postData.tid, tid, next);
+						updateCategory(postData, tid, next);
 					},
 					function (next) {
 						Topics.decreasePostCount(postData.tid, next);
@@ -124,8 +124,8 @@ module.exports = function (Topics) {
 			},
 			function (results, next) {
 				async.parallel([
-					async.apply(updateRecentTopic, tid),
+					async.apply(Topics.updateLastPostTimeFromLastPid, tid),
-					async.apply(updateRecentTopic, postData.tid),
+					async.apply(Topics.updateLastPostTimeFromLastPid, postData.tid),
 				], function (err) {
 					next(err);
 				});
@@ -137,40 +137,42 @@ module.exports = function (Topics) {
 		], callback);
 	};
 
-	function updateCategoryPostCount(oldTid, tid, callback) {
+	function updateCategory(postData, toTid, callback) {
+		var topicData;
 		async.waterfall([
 			function (next) {
-				Topics.getTopicsFields([oldTid, tid], ['cid'], next);
+				Topics.getTopicsFields([postData.tid, toTid], ['cid', 'pinned'], next);
 			},
-			function (topicData, next) {
+			function (_topicData, next) {
+				topicData = _topicData;
 				if (!topicData[0].cid || !topicData[1].cid) {
 					return callback();
 				}
+				var tasks = [];
+				if (parseInt(topicData[0].pinned, 10) !== 1) {
+					tasks.push(async.apply(db.sortedSetIncrBy, 'cid:' + topicData[0].cid + ':tids:posts', -1, postData.tid));
+				}
+				if (parseInt(topicData[1].pinned, 10) !== 1) {
+					tasks.push(async.apply(db.sortedSetIncrBy, 'cid:' + topicData[1].cid + ':tids:posts', 1, toTid));
+				} else {
+					next();
+				}
+				async.series(tasks, function (err) {
+					next(err);
+				});
+			},
+			function (next) {
 				if (parseInt(topicData[0].cid, 10) === parseInt(topicData[1].cid, 10)) {
 					return callback();
 				}
+
 				async.parallel([
 					async.apply(db.incrObjectFieldBy, 'category:' + topicData[0].cid, 'post_count', -1),
 					async.apply(db.incrObjectFieldBy, 'category:' + topicData[1].cid, 'post_count', 1),
+					async.apply(db.sortedSetRemove, 'cid:' + topicData[0].cid + ':pids', postData.pid),
+					async.apply(db.sortedSetAdd, 'cid:' + topicData[1].cid + ':pids', postData.timestamp, postData.pid),
 				], next);
 			},
 		], callback);
 	}
-
-	function updateRecentTopic(tid, callback) {
-		async.waterfall([
-			function (next) {
-				Topics.getLatestUndeletedPid(tid, next);
-			},
-			function (pid, next) {
-				if (!pid) {
-					return callback();
-				}
-				posts.getPostField(pid, 'timestamp', next);
-			},
-			function (timestamp, next) {
-				Topics.updateTimestamp(tid, timestamp, next);
-			},
-		], callback);
-	}
 };

src/topics/posts.js

@@ -19,7 +19,7 @@ module.exports = function (Topics) {
 				Topics.increasePostCount(postData.tid, next);
 			},
 			function (next) {
-				Topics.updateTimestamp(postData.tid, postData.timestamp, next);
+				Topics.updateLastPostTime(postData.tid, postData.timestamp, next);
 			},
 			function (next) {
 				Topics.addPostToTopic(postData.tid, postData, next);

src/topics/recent.js

@@ -3,12 +3,14 @@
 'use strict';
 
 var async = require('async');
+var winston = require('winston');
 
 var db = require('../database');
 var plugins = require('../plugins');
 var privileges = require('../privileges');
 var user = require('../user');
 var meta = require('../meta');
+var posts = require('../posts');
 
 module.exports = function (Topics) {
 	var terms = {
@@ -227,34 +229,62 @@ module.exports = function (Topics) {
 		db.getSortedSetRevRangeByScore(set, start, count, '+inf', Date.now() - since, callback);
 	};
 
-	Topics.updateTimestamp = function (tid, timestamp, callback) {
+	Topics.updateLastPostTimeFromLastPid = function (tid, callback) {
+		async.waterfall([
+			function (next) {
+				Topics.getLatestUndeletedPid(tid, next);
+			},
+			function (pid, next) {
+				if (!parseInt(pid, 10)) {
+					return callback();
+				}
+				posts.getPostField(pid, 'timestamp', next);
+			},
+			function (timestamp, next) {
+				if (!parseInt(timestamp, 10)) {
+					return callback();
+				}
+				Topics.updateLastPostTime(tid, timestamp, next);
+			},
+		], callback);
+	};
+
+	Topics.updateLastPostTime = function (tid, lastposttime, callback) {
 		async.parallel([
 			function (next) {
-				var topicData;
 				async.waterfall([
 					function (next) {
-						Topics.getTopicFields(tid, ['cid', 'deleted'], next);
+						Topics.getTopicFields(tid, ['cid', 'deleted', 'pinned'], next);
-					},
-					function (_topicData, next) {
-						topicData = _topicData;
-						db.sortedSetAdd('cid:' + topicData.cid + ':tids:lastposttime', timestamp, tid, next);
 					},
-					function (next) {
+					function (topicData, next) {
-						if (parseInt(topicData.deleted, 10) === 1) {
+						var tasks = [
-							return next();
+							async.apply(db.sortedSetAdd, 'cid:' + topicData.cid + ':tids:lastposttime', lastposttime, tid),
+						];
+
+						if (parseInt(topicData.deleted, 10) !== 1) {
+							tasks.push(async.apply(Topics.updateRecent, tid, lastposttime));
 						}
-						Topics.updateRecent(tid, timestamp, next);
+
+						if (parseInt(topicData.pinned, 10) !== 1) {
+							tasks.push(async.apply(db.sortedSetAdd, 'cid:' + topicData.cid + ':tids', lastposttime, tid));
+						}
+						async.series(tasks, next);
 					},
 				], next);
 			},
 			function (next) {
-				Topics.setTopicField(tid, 'lastposttime', timestamp, next);
+				Topics.setTopicField(tid, 'lastposttime', lastposttime, next);
 			},
 		], function (err) {
 			callback(err);
 		});
 	};
 
+	Topics.updateTimestamp = function (tid, lastposttime, callback) {
+		winston.warn('[deprecated] Topics.updateTimestamp is deprecated please use Topics.updateLastPostTime');
+		Topics.updateLastPostTime(tid, lastposttime, callback);
+	};
+
 	Topics.updateRecent = function (tid, timestamp, callback) {
 		callback = callback || function () {};
 

src/upgrades/1.10.2/fix_category_post_zsets.js

@@ -0,0 +1,60 @@
+'use strict';
+
+var db = require('../../database');
+
+var async = require('async');
+var batch = require('../../batch');
+
+module.exports = {
+	name: 'Fix category post zsets',
+	timestamp: Date.UTC(2018, 9, 10),
+	method: function (callback) {
+		const progress = this.progress;
+
+		db.getSortedSetRange('categories:cid', 0, -1, function (err, cids) {
+			if (err) {
+				return callback(err);
+			}
+			var keys = cids.map(function (cid) {
+				return 'cid:' + cid + ':pids';
+			});
+			var posts = require('../../posts');
+			batch.processSortedSet('posts:pid', function (postData, next) {
+				async.eachSeries(postData, function (postData, next) {
+					progress.incr();
+					var pid = postData.value;
+					var timestamp = postData.score;
+					var cid;
+					async.waterfall([
+						function (next) {
+							posts.getCidByPid(pid, next);
+						},
+						function (_cid, next) {
+							cid = _cid;
+							db.isMemberOfSortedSets(keys, pid, next);
+						},
+						function (isMembers, next) {
+							var memberCids = [];
+							isMembers.forEach(function (isMember, index) {
+								if (isMember) {
+									memberCids.push(cids[index]);
+								}
+							});
+							if (memberCids.length > 1) {
+								async.waterfall([
+									async.apply(db.sortedSetRemove, memberCids.map(cid => 'cid:' + cid + ':pids'), pid),
+									async.apply(db.sortedSetAdd, 'cid:' + cid + ':pids', timestamp, pid),
+								], next);
+							} else {
+								next();
+							}
+						},
+					], next);
+				}, next);
+			}, {
+				progress: progress,
+				withScores: true,
+			}, callback);
+		});
+	},
+};

src/upgrades/1.10.2/fix_category_topic_zsets.js

@@ -0,0 +1,39 @@
+'use strict';
+
+var db = require('../../database');
+
+var async = require('async');
+var batch = require('../../batch');
+
+module.exports = {
+	name: 'Fix category topic zsets',
+	timestamp: Date.UTC(2018, 9, 11),
+	method: function (callback) {
+		const progress = this.progress;
+
+		var topics = require('../../topics');
+		batch.processSortedSet('topics:tid', function (tids, next) {
+			async.eachSeries(tids, function (tid, next) {
+				progress.incr();
+
+				async.waterfall([
+					function (next) {
+						db.getObjectFields('topic:' + tid, ['cid', 'pinned', 'postcount'], next);
+					},
+					function (topicData, next) {
+						if (parseInt(topicData.pinned, 10) === 1) {
+							return setImmediate(next);
+						}
+
+						db.sortedSetAdd('cid:' + topicData.cid + ':tids:posts', topicData.postcount, tid, next);
+					},
+					function (next) {
+						topics.updateLastPostTimeFromLastPid(tid, next);
+					},
+				], next);
+			}, next);
+		}, {
+			progress: progress,
+		}, callback);
+	},
+};

test/topics.js

@@ -403,6 +403,112 @@ describe('Topic\'s', function () {
 			});
 		});
 
+		it('should properly update sets when post is moved', function (done) {
+			var movedPost;
+			var previousPost;
+			var topic2LastReply;
+			var tid1;
+			var tid2;
+			var cid1 = topic.categoryId;
+			var cid2;
+			function checkCidSets(post1, post2, callback) {
+				async.waterfall([
+					function (next) {
+						async.parallel({
+							topicData: function (next) {
+								topics.getTopicsFields([tid1, tid2], ['lastposttime', 'postcount'], next);
+							},
+							scores1: function (next) {
+								db.sortedSetsScore([
+									'cid:' + cid1 + ':tids',
+									'cid:' + cid1 + ':tids:lastposttime',
+									'cid:' + cid1 + ':tids:posts',
+								], tid1, next);
+							},
+							scores2: function (next) {
+								db.sortedSetsScore([
+									'cid:' + cid2 + ':tids',
+									'cid:' + cid2 + ':tids:lastposttime',
+									'cid:' + cid2 + ':tids:posts',
+								], tid2, next);
+							},
+						}, next);
+					},
+					function (results, next) {
+						assert.equal(results.topicData[0].lastposttime, results.scores1[0]);
+						assert.equal(results.topicData[1].lastposttime, results.scores2[0]);
+						assert.equal(results.topicData[0].lastposttime, results.scores1[1]);
+						assert.equal(results.topicData[1].lastposttime, results.scores2[1]);
+						assert.equal(results.topicData[0].postcount, results.scores1[2]);
+						assert.equal(results.topicData[1].postcount, results.scores2[2]);
+						assert.equal(results.topicData[0].lastposttime, post1.timestamp);
+						assert.equal(results.topicData[1].lastposttime, post2.timestamp);
+						next();
+					},
+				], callback);
+			}
+
+			async.waterfall([
+				function (next) {
+					categories.create({
+						name: 'move to this category',
+						description: 'Test category created by testing script',
+					}, next);
+				},
+				function (category, next) {
+					cid2 = category.cid;
+					topics.post({ uid: adminUid, title: 'topic1', content: 'topic 1 mainPost', cid: cid1 }, next);
+				},
+				function (result, next) {
+					tid1 = result.topicData.tid;
+					topics.reply({ uid: adminUid, content: 'topic 1 reply 1', tid: tid1 }, next);
+				},
+				function (postData, next) {
+					previousPost = postData;
+					topics.reply({ uid: adminUid, content: 'topic 1 reply 2', tid: tid1 }, next);
+				},
+				function (postData, next) {
+					movedPost = postData;
+					topics.post({ uid: adminUid, title: 'topic2', content: 'topic 2 mainpost', cid: cid2 }, next);
+				},
+				function (results, next) {
+					tid2 = results.topicData.tid;
+					topics.reply({ uid: adminUid, content: 'topic 2 reply 1', tid: tid2 }, next);
+				},
+				function (postData, next) {
+					topic2LastReply = postData;
+					checkCidSets(movedPost, postData, next);
+				},
+				function (next) {
+					db.isMemberOfSortedSets(['cid:' + cid1 + ':pids', 'cid:' + cid2 + ':pids'], movedPost.pid, next);
+				},
+				function (isMember, next) {
+					assert.deepEqual(isMember, [true, false]);
+					categories.getCategoriesFields([cid1, cid2], ['post_count'], next);
+				},
+				function (categoryData, next) {
+					assert.equal(categoryData[0].post_count, 4);
+					assert.equal(categoryData[1].post_count, 2);
+					topics.movePostToTopic(movedPost.pid, tid2, next);
+				},
+				function (next) {
+					checkCidSets(previousPost, topic2LastReply, next);
+				},
+				function (next) {
+					db.isMemberOfSortedSets(['cid:' + cid1 + ':pids', 'cid:' + cid2 + ':pids'], movedPost.pid, next);
+				},
+				function (isMember, next) {
+					assert.deepEqual(isMember, [false, true]);
+					categories.getCategoriesFields([cid1, cid2], ['post_count'], next);
+				},
+				function (categoryData, next) {
+					assert.equal(categoryData[0].post_count, 3);
+					assert.equal(categoryData[1].post_count, 3);
+					next();
+				},
+			], done);
+		});
+
 		it('should purge the topic', function (done) {
 			socketTopics.purge({ uid: 1 }, { tids: [newTopic.tid], cid: categoryObj.cid }, function (err) {
 				assert.ifError(err);
@@ -794,8 +900,8 @@ describe('Topic\'s', function () {
 			request(nconf.get('url') + '/api/topic/' + topicData.slug + '/-1', { json: true }, function (err, res, body) {
 				assert.ifError(err);
 				assert.equal(res.statusCode, 200);
-				assert.equal(res.headers['x-redirect'], '/topic/13/topic-for-controller-test');
+				assert.equal(res.headers['x-redirect'], '/topic/15/topic-for-controller-test');
-				assert.equal(body, '/topic/13/topic-for-controller-test');
+				assert.equal(body, '/topic/15/topic-for-controller-test');
 				done();
 			});
 		});