feat: rescheduling (editing ST) (#9445)

install/package.json

@@ -155,6 +155,7 @@
         "lint-staged": "10.5.4",
         "mocha": "8.3.2",
         "mocha-lcov-reporter": "1.3.0",
+	  	"mockdate": "3.0.5",
         "nyc": "15.1.0",
         "smtp-server": "3.8.0"
     },

public/src/client/topic/events.js

@@ -114,6 +114,10 @@ define('forum/topic/events', [
 		var navbarTitle = components.get('navbar/title').find('span');
 		var breadCrumb = components.get('breadcrumb/current');
 
+		if (data.topic.rescheduled) {
+			return ajaxify.go('topic/' + data.topic.slug, null, true);
+		}
+
 		if (topicTitle.length && data.topic.title && data.topic.renamed) {
 			ajaxify.data.title = data.topic.title;
 			var newUrl = 'topic/' + data.topic.slug + (window.location.search ? window.location.search : '');

src/api/posts.js

@@ -61,6 +61,7 @@ postsAPI.edit = async function (caller, data) {
 
 	data.uid = caller.uid;
 	data.req = apiHelpers.buildReqObject(caller);
+	data.timestamp = parseInt(data.timestamp, 10) || Date.now();
 
 	const editResult = await posts.edit(data);
 	if (editResult.topic.isMainPost) {

src/posts/diffs.js

@@ -52,6 +52,7 @@ module.exports = function (Posts) {
 	};
 
 	Diffs.load = async function (pid, since, uid) {
+		since = getValidatedTimestamp(since);
 		const post = await postDiffLoad(pid, since, uid);
 		post.content = String(post.content || '');
 
@@ -61,6 +62,7 @@ module.exports = function (Posts) {
 	};
 
 	Diffs.restore = async function (pid, since, uid, req) {
+		since = getValidatedTimestamp(since);
 		const post = await postDiffLoad(pid, since, uid);
 
 		return await Posts.edit({
@@ -68,6 +70,7 @@ module.exports = function (Posts) {
 			pid: pid,
 			content: post.content,
 			req: req,
+			timestamp: since,
 		});
 	};
 
@@ -119,8 +122,6 @@ module.exports = function (Posts) {
 
 	async function postDiffLoad(pid, since, uid) {
 		// Retrieves all diffs made since `since` and replays them to reconstruct what the post looked like at `since`
-		since = getValidatedTimestamp(since);
-
 		const [post, diffs] = await Promise.all([
 			Posts.getPostSummaryByPids([pid], uid, { parse: false }),
 			Posts.diffs.get(pid, since),

src/posts/edit.js

@@ -29,15 +29,13 @@ module.exports = function (Posts) {
 			throw new Error('[[error:no-post]]');
 		}
 
-		const topicData = await topics.getTopicFields(postData.tid, ['cid', 'title', 'timestamp', 'scheduled']);
+		const topicData = await topics.getTopicFields(postData.tid, ['cid', 'mainPid', 'title', 'timestamp', 'scheduled', 'slug']);
+
+		await scheduledTopicCheck(data, topicData);
+
 		const oldContent = postData.content; // for diffing purposes
-		// For posts in scheduled topics, if edited before, use edit timestamp
+		const editPostData = getEditPostData(data, topicData, postData);
-		const postTimestamp = topicData.scheduled ? (postData.edited || postData.timestamp) + 1 : Date.now();
+
-		const editPostData = {
-			content: data.content,
-			edited: postTimestamp,
-			editor: data.uid,
-		};
 		if (data.handle) {
 			editPostData.handle = data.handle;
 		}
@@ -62,7 +60,7 @@ module.exports = function (Posts) {
 				uid: data.uid,
 				oldContent: oldContent,
 				newContent: data.content,
-				edited: postTimestamp,
+				edited: editPostData.edited,
 			});
 		}
 		await Posts.uploads.sync(data.pid);
@@ -73,7 +71,7 @@ module.exports = function (Posts) {
 		const returnPostData = { ...postData, ...result.post };
 		returnPostData.cid = topic.cid;
 		returnPostData.topic = topic;
-		returnPostData.editedISO = utils.toISOString(postTimestamp);
+		returnPostData.editedISO = utils.toISOString(editPostData.edited);
 		returnPostData.changed = oldContent !== data.content;
 
 		await topics.notifyFollowers(returnPostData, data.uid, {
@@ -100,7 +98,7 @@ module.exports = function (Posts) {
 		const { tid } = postData;
 		const title = data.title ? data.title.trim() : '';
 
-		const isMain = await Posts.isMain(data.pid);
+		const isMain = parseInt(data.pid, 10) === parseInt(topicData.mainPid, 10);
 		if (!isMain) {
 			return {
 				tid: tid,
@@ -116,6 +114,7 @@ module.exports = function (Posts) {
 			cid: topicData.cid,
 			uid: postData.uid,
 			mainPid: data.pid,
+			timestamp: rescheduling(data, topicData) ? data.timestamp : topicData.timestamp,
 		};
 		if (title) {
 			newTopicData.title = title;
@@ -141,9 +140,12 @@ module.exports = function (Posts) {
 		await topics.updateTopicTags(tid, data.tags);
 		const tags = await topics.getTopicTagsObjects(tid);
 
+		if (rescheduling(data, topicData)) {
+			await topics.scheduled.reschedule(newTopicData);
+		}
+
 		newTopicData.tags = data.tags;
 		newTopicData.oldTitle = topicData.title;
-		newTopicData.timestamp = topicData.timestamp;
 		const renamed = translator.escape(validator.escape(String(title))) !== topicData.title;
 		plugins.hooks.fire('action:topic.edit', { topic: newTopicData, uid: data.uid });
 		return {
@@ -152,10 +154,49 @@ module.exports = function (Posts) {
 			uid: postData.uid,
 			title: validator.escape(String(title)),
 			oldTitle: topicData.title,
-			slug: newTopicData.slug,
+			slug: newTopicData.slug || topicData.slug,
 			isMainPost: true,
 			renamed: renamed,
+			rescheduled: rescheduling(data, topicData),
 			tags: tags,
 		};
 	}
+
+	async function scheduledTopicCheck(data, topicData) {
+		if (!topicData.scheduled) {
+			return;
+		}
+		const canSchedule = await privileges.categories.can('topics:schedule', topicData.cid, data.uid);
+		if (!canSchedule) {
+			throw new Error('[[error:no-privileges]]');
+		}
+		const isMain = parseInt(data.pid, 10) === parseInt(topicData.mainPid, 10);
+		if (isMain && (isNaN(data.timestamp) || data.timestamp < Date.now())) {
+			throw new Error('[[error:invalid-data]]');
+		}
+	}
+
+	function getEditPostData(data, topicData, postData) {
+		const editPostData = {
+			content: data.content,
+			editor: data.uid,
+		};
+
+		// For posts in scheduled topics, if edited before, use edit timestamp
+		editPostData.edited = topicData.scheduled ? (postData.edited || postData.timestamp) + 1 : Date.now();
+
+		// if rescheduling the main post
+		if (rescheduling(data, topicData)) {
+			// For main posts, use timestamp coming from user (otherwise, it is ignored)
+			editPostData.edited = data.timestamp;
+			editPostData.timestamp = data.timestamp;
+		}
+
+		return editPostData;
+	}
+
+	function rescheduling(data, topicData) {
+		const isMain = parseInt(data.pid, 10) === parseInt(topicData.mainPid, 10);
+		return isMain && topicData.scheduled && topicData.timestamp !== data.timestamp;
+	}
 };

src/topics/scheduled.js

@@ -32,7 +32,10 @@ Scheduled.handleExpired = async function () {
 
 	// Restore first to be not filtered for being deleted
 	// Restoring handles "updateRecentTid"
-	await Promise.all(topicsData.map(topicData => topics.restore(topicData.tid)));
+	await Promise.all([].concat(
+		topicsData.map(topicData => topics.restore(topicData.tid)),
+		topicsData.map(topicData => topics.updateLastPostTimeFromLastPid(topicData.tid))
+	));
 
 	await Promise.all([].concat(
 		sendNotifications(uids, topicsData),
@@ -55,6 +58,20 @@ Scheduled.pin = async function (tid, topicData) {
 	]);
 };
 
+Scheduled.reschedule = async function ({ cid, tid, timestamp, uid }) {
+	await Promise.all([
+		db.sortedSetsAdd([
+			'topics:scheduled',
+			`uid:${uid}:topics`,
+			'topics:tid',
+			`cid:${cid}:tids`,
+			`cid:${cid}:uid:${uid}:tids`,
+		], timestamp, tid),
+		shiftPostTimes(tid, timestamp),
+	]);
+	return topics.updateLastPostTimeFromLastPid(tid);
+};
+
 function unpin(tid, topicData) {
 	return [
 		topics.setTopicField(tid, 'pinned', 0),
@@ -79,26 +96,32 @@ async function sendNotifications(uids, topicsData) {
 		postData.topic = topicsData[idx];
 	});
 
-	return topicsData.map(
+	return Promise.all(topicsData.map(
 		(t, idx) => user.notifications.sendTopicNotificationToFollowers(t.uid, t, postsData[idx])
 	).concat(
 		topicsData.map(
 			(t, idx) => socketHelpers.notifyNew(t.uid, 'newTopic', { posts: [postsData[idx]], topic: t })
 		)
-	);
+	));
 }
 
 async function updateUserLastposttimes(uids, topicsData) {
 	const lastposttimes = (await user.getUsersFields(uids, ['lastposttime'])).map(u => u.lastposttime);
 
-	let timestampByUid = {};
+	let tstampByUid = {};
 	topicsData.forEach((tD) => {
-		timestampByUid[tD.uid] = timestampByUid[tD.uid] ? timestampByUid[tD.uid].concat(tD.timestamp) : [tD.timestamp];
+		tstampByUid[tD.uid] = tstampByUid[tD.uid] ? tstampByUid[tD.uid].concat(tD.lastposttime) : [tD.lastposttime];
 	});
-	timestampByUid = Object.fromEntries(
+	tstampByUid = Object.fromEntries(
-		Object.entries(timestampByUid).filter(uidTimestamp => [uidTimestamp[0], Math.max(...uidTimestamp[1])])
+		Object.entries(tstampByUid).map(uidTimestamp => [uidTimestamp[0], Math.max(...uidTimestamp[1])])
 	);
 
-	const uidsToUpdate = uids.filter((uid, idx) => timestampByUid[uid] > lastposttimes[idx]);
+	const uidsToUpdate = uids.filter((uid, idx) => tstampByUid[uid] > lastposttimes[idx]);
-	return uidsToUpdate.map(uid => user.setUserField(uid, 'lastposttime', String(timestampByUid[uid])));
+	return Promise.all(uidsToUpdate.map(uid => user.setUserField(uid, 'lastposttime', tstampByUid[uid])));
+}
+
+async function shiftPostTimes(tid, timestamp) {
+	const pids = (await posts.getPidsFromSet(`tid:${tid}:posts`, 0, -1, false));
+	// Leaving other related score values intact, since they reflect post order correctly, and it seems that's good enough
+	return db.setObjectBulk(pids.map(pid => `post:${pid}`), pids.map((_, idx) => ({ timestamp: timestamp + idx + 1 })));
 }

test/topics.js

@@ -3,6 +3,7 @@
 const async = require('async');
 const assert = require('assert');
 const validator = require('validator');
+const mockdate = require('mockdate');
 const nconf = require('nconf');
 const request = require('request');
 const util = require('util');
@@ -2695,7 +2696,8 @@ describe('Topic\'s', () => {
 			assert.deepStrictEqual(isMember, [false, false, false]);
 		});
 
-		it('should not update poster\'s lastposttime', async () => {
+		it('should update poster\'s lastposttime with "action time"', async () => {
+			// src/user/posts.js:56
 			const data = await User.getUsersFields([adminUid], ['lastposttime']);
 			assert.notStrictEqual(data[0].lastposttime, topicData.lastposttime);
 		});
@@ -2782,21 +2784,30 @@ describe('Topic\'s', () => {
 			assert(revisions[0].timestamp > revisions[1].timestamp);
 		});
 
-		it('should allow to purge a scheduled topic', async () => {
+		it('should able to reschedule', async () => {
-			const response = await requestType('delete', `${nconf.get('url')}/api/v3/topics/${topicData.tid}`, adminApiOpts);
+			const newDate = new Date(Date.now() + (5 * 86400000)).getTime();
-			assert.strictEqual(response.res.statusCode, 200);
+			const editData = { ...adminApiOpts, form: { ...topic, pid: topicData.mainPid, timestamp: newDate } };
-		});
+			const response = await requestType('put', `${nconf.get('url')}/api/v3/posts/${topicData.mainPid}`, editData);
 
-		it('should remove from topics:scheduled on purge', async () => {
+			const editedTopic = await topics.getTopicFields(topicData.tid, ['lastposttime', 'timestamp']);
-			const score = await db.sortedSetScore('topics:scheduled', topicData.tid);
+			const editedPost = await posts.getPostFields(postData.pid, ['timestamp']);
-			assert(!score);
+			assert(editedTopic.timestamp === newDate);
+			assert(editedPost.timestamp > editedTopic.timestamp);
+
+			const scores = await db.sortedSetsScore([
+				'topics:scheduled',
+				`uid:${adminUid}:topics`,
+				'topics:tid',
+				`cid:${topicData.cid}:tids`,
+				`cid:${topicData.cid}:uid:${adminUid}:tids`,
+			], topicData.tid);
+			assert(scores.every(publishTime => publishTime === editedTopic.timestamp));
 		});
 
 		it('should able to publish a scheduled topic', async () => {
-			topicData = (await topics.post(topic)).topicData;
+			const topicTimestamp = await topics.getTopicField(topicData.tid, 'timestamp');
-			// Manually trigger publishing
+
-			await db.sortedSetRemove('topics:scheduled', topicData.tid);
+			mockdate.set(topicTimestamp);
-			await db.sortedSetAdd('topics:scheduled', Date.now() - 1000, topicData.tid);
 			await topics.scheduled.handleExpired();
 
 			topicData = await topics.getTopicData(topicData.tid);
@@ -2809,7 +2820,28 @@ describe('Topic\'s', () => {
 
 		it('should update poster\'s lastposttime after a ST published', async () => {
 			const data = await User.getUsersFields([adminUid], ['lastposttime']);
+			assert.strictEqual(adminUid, topicData.uid);
 			assert.strictEqual(data[0].lastposttime, topicData.lastposttime);
 		});
+
+		it('should not be able to schedule a "published" topic', async () => {
+			const newDate = new Date(Date.now() + 86400000).getTime();
+			const editData = { ...adminApiOpts, form: { ...topic, pid: topicData.mainPid, timestamp: newDate } };
+			const response = await requestType('put', `${nconf.get('url')}/api/v3/posts/${topicData.mainPid}`, editData);
+			assert.strictEqual(response.body.response.timestamp, Date.now());
+
+			mockdate.reset();
+		});
+
+		it('should allow to purge a scheduled topic', async () => {
+			topicData = (await topics.post(topic)).topicData;
+			const response = await requestType('delete', `${nconf.get('url')}/api/v3/topics/${topicData.tid}`, adminApiOpts);
+			assert.strictEqual(response.res.statusCode, 200);
+		});
+
+		it('should remove from topics:scheduled on purge', async () => {
+			const score = await db.sortedSetScore('topics:scheduled', topicData.tid);
+			assert(!score);
+		});
 	});
 });