feat: store topic title and tags in diffs (#10900)

* feat: store topic title and tags in diffs

allow restoring post diff if tags didn't change

* test: fix tests, fast computer problems

public/src/client/topic/events.js

@@ -158,7 +158,7 @@ define('forum/topic/events', [
 			hooks.fire('action:posts.edited', data);
 		}
 
-		if (data.topic.tags && tagsUpdated(data.topic.tags)) {
+		if (data.topic.tags && data.topic.tagsupdated) {
 			Benchpress.render('partials/topic/tags', { tags: data.topic.tags }).then(function (html) {
 				const tags = $('.tags');
 
@@ -171,19 +171,6 @@ define('forum/topic/events', [
 		postTools.removeMenu(components.get('post', 'pid', data.post.pid));
 	}
 
-	function tagsUpdated(tags) {
-		if (tags.length !== $('.tags').first().children().length) {
-			return true;
-		}
-
-		for (let i = 0; i < tags.length; i += 1) {
-			if (!$('.tags .tag-item[data-tag="' + tags[i].value + '"]').length) {
-				return true;
-			}
-		}
-		return false;
-	}
-
 	function onPostPurged(postData) {
 		if (!postData || parseInt(postData.tid, 10) !== parseInt(ajaxify.data.tid, 10)) {
 			return;

src/posts/diffs.js

@@ -7,7 +7,7 @@ const db = require('../database');
 const meta = require('../meta');
 const plugins = require('../plugins');
 const translator = require('../translator');
-
+const topics = require('../topics');
 
 module.exports = function (Posts) {
 	const Diffs = {};
@@ -38,16 +38,24 @@ module.exports = function (Posts) {
 	};
 
 	Diffs.save = async function (data) {
-		const { pid, uid, oldContent, newContent, edited } = data;
+		const { pid, uid, oldContent, newContent, edited, topic } = data;
 		const editTimestamp = edited || Date.now();
-		const patch = diff.createPatch('', newContent, oldContent);
+		const diffData = {
+			uid: uid,
+			pid: pid,
+		};
+		if (oldContent !== newContent) {
+			diffData.patch = diff.createPatch('', newContent, oldContent);
+		}
+		if (topic.renamed) {
+			diffData.title = topic.oldTitle;
+		}
+		if (topic.tagsupdated && Array.isArray(topic.oldTags)) {
+			diffData.tags = topic.oldTags.map(tag => tag && tag.value).filter(Boolean).join(',');
+		}
 		await Promise.all([
 			db.listPrepend(`post:${pid}:diffs`, editTimestamp),
-			db.setObject(`diff:${pid}.${editTimestamp}`, {
-				uid: uid,
-				pid: pid,
-				patch: patch,
-			}),
+			db.setObject(`diff:${pid}.${editTimestamp}`, diffData),
 		]);
 	};
 
@@ -71,6 +79,8 @@ module.exports = function (Posts) {
 			content: post.content,
 			req: req,
 			timestamp: since,
+			title: post.topic.title,
+			tags: post.topic.tags.map(tag => tag.value),
 		});
 	};
 
@@ -130,6 +140,16 @@ module.exports = function (Posts) {
 		// Replace content with re-constructed content from that point in time
 		post[0].content = diffs.reduce(applyPatch, validator.unescape(post[0].content));
 
+		const titleDiffs = diffs.filter(d => d.hasOwnProperty('title') && d.title);
+		if (titleDiffs.length && post[0].topic) {
+			post[0].topic.title = validator.unescape(String(titleDiffs[titleDiffs.length - 1].title));
+		}
+		const tagDiffs = diffs.filter(d => d.hasOwnProperty('tags') && d.tags);
+		if (tagDiffs.length && post[0].topic) {
+			const tags = tagDiffs[tagDiffs.length - 1].tags.split(',').map(tag => ({ value: tag }));
+			post[0].topic.tags = await topics.getTagData(tags);
+		}
+
 		return post[0];
 	}
 
@@ -144,9 +164,12 @@ module.exports = function (Posts) {
 	}
 
 	function applyPatch(content, aDiff) {
-		const result = diff.applyPatch(content, aDiff.patch, {
-			fuzzFactor: 1,
-		});
-		return typeof result === 'string' ? result : content;
+		if (aDiff && aDiff.patch) {
+			const result = diff.applyPatch(content, aDiff.patch, {
+				fuzzFactor: 1,
+			});
+			return typeof result === 'string' ? result : content;
+		}
+		return content;
 	}
 };

src/posts/edit.js

@@ -29,7 +29,9 @@ module.exports = function (Posts) {
 			throw new Error('[[error:no-post]]');
 		}
 
-		const topicData = await topics.getTopicFields(postData.tid, ['cid', 'mainPid', 'title', 'timestamp', 'scheduled', 'slug']);
+		const topicData = await topics.getTopicFields(postData.tid, [
+			'cid', 'mainPid', 'title', 'timestamp', 'scheduled', 'slug', 'tags',
+		]);
 
 		await scheduledTopicCheck(data, topicData);
 
@@ -53,7 +55,10 @@ module.exports = function (Posts) {
 		]);
 
 		await Posts.setPostFields(data.pid, result.post);
-		const contentChanged = data.content !== oldContent;
+		const contentChanged = data.content !== oldContent ||
+			topic.renamed ||
+			topic.tagsupdated;
+
 		if (meta.config.enablePostHistory === 1 && contentChanged) {
 			await Posts.diffs.save({
 				pid: data.pid,
@@ -61,6 +66,7 @@ module.exports = function (Posts) {
 				oldContent: oldContent,
 				newContent: data.content,
 				edited: editPostData.edited,
+				topic,
 			});
 		}
 		await Posts.uploads.sync(data.pid);
@@ -109,6 +115,7 @@ module.exports = function (Posts) {
 				title: validator.escape(String(topicData.title)),
 				isMainPost: false,
 				renamed: false,
+				tagsupdated: false,
 			};
 		}
 
@@ -124,15 +131,16 @@ module.exports = function (Posts) {
 			newTopicData.slug = `${tid}/${slugify(title) || 'topic'}`;
 		}
 
-		data.tags = data.tags || [];
+		const tagsupdated = Array.isArray(data.tags) &&
+			!_.isEqual(data.tags, topicData.tags.map(tag => tag.value));
 
-		if (data.tags.length) {
+		if (tagsupdated) {
 			const canTag = await privileges.categories.can('topics:tag', topicData.cid, data.uid);
 			if (!canTag) {
 				throw new Error('[[error:no-privileges]]');
 			}
+			await topics.validateTags(data.tags, topicData.cid, data.uid, tid);
 		}
-		await topics.validateTags(data.tags, topicData.cid, data.uid, tid);
 
 		const results = await plugins.hooks.fire('filter:topic.edit', {
 			req: data.req,
@@ -140,7 +148,9 @@ module.exports = function (Posts) {
 			data: data,
 		});
 		await db.setObject(`topic:${tid}`, results.topic);
-		await topics.updateTopicTags(tid, data.tags);
+		if (tagsupdated) {
+			await topics.updateTopicTags(tid, data.tags);
+		}
 		const tags = await topics.getTopicTagsObjects(tid);
 
 		if (rescheduling(data, topicData)) {
@@ -149,7 +159,7 @@ module.exports = function (Posts) {
 
 		newTopicData.tags = data.tags;
 		newTopicData.oldTitle = topicData.title;
-		const renamed = translator.escape(validator.escape(String(title))) !== topicData.title;
+		const renamed = title && translator.escape(validator.escape(String(title))) !== topicData.title;
 		plugins.hooks.fire('action:topic.edit', { topic: newTopicData, uid: data.uid });
 		return {
 			tid: tid,
@@ -160,8 +170,10 @@ module.exports = function (Posts) {
 			slug: newTopicData.slug || topicData.slug,
 			isMainPost: true,
 			renamed: renamed,
-			rescheduled: rescheduling(data, topicData),
+			tagsupdated: tagsupdated,
 			tags: tags,
+			oldTags: topicData.tags,
+			rescheduled: rescheduling(data, topicData),
 		};
 	}
 

src/posts/summary.js

@@ -76,9 +76,15 @@ module.exports = function (Posts) {
 	}
 
 	async function getTopicAndCategories(tids) {
-		const topicsData = await topics.getTopicsFields(tids, ['uid', 'tid', 'title', 'cid', 'slug', 'deleted', 'scheduled', 'postcount', 'mainPid', 'teaserPid']);
+		const topicsData = await topics.getTopicsFields(tids, [
+			'uid', 'tid', 'title', 'cid', 'tags', 'slug',
+			'deleted', 'scheduled', 'postcount', 'mainPid', 'teaserPid',
+		]);
 		const cids = _.uniq(topicsData.map(topic => topic && topic.cid));
-		const categoriesData = await categories.getCategoriesFields(cids, ['cid', 'name', 'icon', 'slug', 'parentCid', 'bgColor', 'color', 'backgroundImage', 'imageClass']);
+		const categoriesData = await categories.getCategoriesFields(cids, [
+			'cid', 'name', 'icon', 'slug', 'parentCid',
+			'bgColor', 'color', 'backgroundImage', 'imageClass',
+		]);
 		return { topics: topicsData, categories: categoriesData };
 	}
 

test/api.js

@@ -191,8 +191,11 @@ describe('API', async () => {
 		const socketAdmin = require('../src/socket.io/admin');
 		// export data for admin user
 		await socketUser.exportProfile({ uid: adminUid }, { uid: adminUid });
+		await wait(2000);
 		await socketUser.exportPosts({ uid: adminUid }, { uid: adminUid });
+		await wait(2000);
 		await socketUser.exportUploads({ uid: adminUid }, { uid: adminUid });
+		await wait(2000);
 		await socketAdmin.user.exportUsersCSV({ uid: adminUid }, {});
 		// wait for export child process to complete
 		await wait(5000);

test/posts.js

@@ -425,6 +425,7 @@ describe('Post\'s', () => {
 				cid: cid,
 				title: 'topic to edit',
 				content: 'A post to edit',
+				tags: ['nodebb'],
 			}, (err, data) => {
 				assert.ifError(err);
 				pid = data.postData.pid;