From aa0137b1c4c2bd9fca1a987f9c1caf468a04978a Mon Sep 17 00:00:00 2001 From: gasoved Date: Wed, 31 Mar 2021 00:51:23 +0300 Subject: [PATCH] feat: rescheduling (editing ST) (#9445) --- install/package.json | 1 + public/src/client/topic/events.js | 4 ++ src/api/posts.js | 1 + src/posts/diffs.js | 5 ++- src/posts/edit.js | 67 +++++++++++++++++++++++++------ src/topics/scheduled.js | 41 ++++++++++++++----- test/topics.js | 56 ++++++++++++++++++++------ 7 files changed, 139 insertions(+), 36 deletions(-) diff --git a/install/package.json b/install/package.json index 4319dbb57c..c464653df5 100644 --- a/install/package.json +++ b/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" }, diff --git a/public/src/client/topic/events.js b/public/src/client/topic/events.js index d0e30a09da..5e9a406e87 100644 --- a/public/src/client/topic/events.js +++ b/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 : ''); diff --git a/src/api/posts.js b/src/api/posts.js index 1a01b87215..2e47ad02f3 100644 --- a/src/api/posts.js +++ b/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) { diff --git a/src/posts/diffs.js b/src/posts/diffs.js index 9e8515ef97..9d541fe0aa 100644 --- a/src/posts/diffs.js +++ b/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), diff --git a/src/posts/edit.js b/src/posts/edit.js index 6714648f22..eff4bdc88f 100644 --- a/src/posts/edit.js +++ b/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 postTimestamp = topicData.scheduled ? (postData.edited || postData.timestamp) + 1 : Date.now(); - const editPostData = { - content: data.content, - edited: postTimestamp, - editor: data.uid, - }; + const editPostData = getEditPostData(data, topicData, postData); + 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; + } }; diff --git a/src/topics/scheduled.js b/src/topics/scheduled.js index 2957634356..7998a826f3 100644 --- a/src/topics/scheduled.js +++ b/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( - Object.entries(timestampByUid).filter(uidTimestamp => [uidTimestamp[0], Math.max(...uidTimestamp[1])]) + tstampByUid = Object.fromEntries( + Object.entries(tstampByUid).map(uidTimestamp => [uidTimestamp[0], Math.max(...uidTimestamp[1])]) ); - const uidsToUpdate = uids.filter((uid, idx) => timestampByUid[uid] > lastposttimes[idx]); - return uidsToUpdate.map(uid => user.setUserField(uid, 'lastposttime', String(timestampByUid[uid]))); + const uidsToUpdate = uids.filter((uid, idx) => tstampByUid[uid] > lastposttimes[idx]); + 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 }))); } diff --git a/test/topics.js b/test/topics.js index a49d621cb2..3eadd1aadd 100644 --- a/test/topics.js +++ b/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 () => { - const response = await requestType('delete', `${nconf.get('url')}/api/v3/topics/${topicData.tid}`, adminApiOpts); - assert.strictEqual(response.res.statusCode, 200); - }); + it('should able to reschedule', async () => { + const newDate = new Date(Date.now() + (5 * 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); - it('should remove from topics:scheduled on purge', async () => { - const score = await db.sortedSetScore('topics:scheduled', topicData.tid); - assert(!score); + const editedTopic = await topics.getTopicFields(topicData.tid, ['lastposttime', 'timestamp']); + const editedPost = await posts.getPostFields(postData.pid, ['timestamp']); + 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; - // Manually trigger publishing - await db.sortedSetRemove('topics:scheduled', topicData.tid); - await db.sortedSetAdd('topics:scheduled', Date.now() - 1000, topicData.tid); + const topicTimestamp = await topics.getTopicField(topicData.tid, 'timestamp'); + + mockdate.set(topicTimestamp); 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); + }); }); });