feat: show unread categories based on unread topics (#12317)

* feat: show unread categories based on unread topics

if a category has unread topics in one of its children then mark category unread
deprecate cid:<cid>:read_by_uid sets
upgrade script to remove the old sets

* chore: up harmony

install/package.json

@@ -103,7 +103,7 @@
         "nodebb-plugin-ntfy": "1.7.3",
         "nodebb-plugin-spam-be-gone": "2.2.0",
         "nodebb-rewards-essentials": "1.0.0",
-        "nodebb-theme-harmony": "1.2.10",
+        "nodebb-theme-harmony": "1.2.11",
         "nodebb-theme-lavender": "7.1.7",
         "nodebb-theme-peace": "2.2.0",
         "nodebb-theme-persona": "13.3.3",

public/openapi/read/categories.yaml

@@ -38,6 +38,9 @@ get:
                               type: array
                               items:
                                 type: string
+                            unread:
+                              type: boolean
+                              description: True if category or it's children have unread topics
                             unread-class:
                               type: string
                             children:

public/openapi/read/category/category_id.yaml

@@ -38,6 +38,9 @@ get:
                     type: array
                     items:
                       type: string
+                  unread:
+                    type: boolean
+                    description: True if category or it's children have unread topics
                   unread-class:
                     type: string
                   children:

public/openapi/read/index.yaml

@@ -33,6 +33,9 @@ get:
                               type: array
                               items:
                                 type: string
+                            unread:
+                              type: boolean
+                              description: True if category or it's children have unread topics
                             unread-class:
                               type: string
                             children:

src/api/categories.js

@@ -49,7 +49,7 @@ categoriesAPI.create = async function (caller, data) {
 	await hasAdminPrivilege(caller.uid);
 
 	const response = await categories.create(data);
-	const categoryObjs = await categories.getCategories([response.cid], caller.uid);
+	const categoryObjs = await categories.getCategories([response.cid]);
 	return categoryObjs[0];
 };
 

src/categories/index.js

@@ -5,6 +5,7 @@ const _ = require('lodash');
 
 const db = require('../database');
 const user = require('../user');
+const topics = require('../topics');
 const plugins = require('../plugins');
 const privileges = require('../privileges');
 const cache = require('../cache');
@@ -30,7 +31,7 @@ Categories.exists = async function (cids) {
 };
 
 Categories.getCategoryById = async function (data) {
-	const categories = await Categories.getCategories([data.cid], data.uid);
+	const categories = await Categories.getCategories([data.cid]);
 	if (!categories[0]) {
 		return null;
 	}
@@ -78,9 +79,9 @@ Categories.getAllCidsFromSet = async function (key) {
 	return cids.slice();
 };
 
-Categories.getAllCategories = async function (uid) {
+Categories.getAllCategories = async function () {
 	const cids = await Categories.getAllCidsFromSet('categories:cid');
-	return await Categories.getCategories(cids, uid);
+	return await Categories.getCategories(cids);
 };
 
 Categories.getCidsByPrivilege = async function (set, uid, privilege) {
@@ -90,7 +91,7 @@ Categories.getCidsByPrivilege = async function (set, uid, privilege) {
 
 Categories.getCategoriesByPrivilege = async function (set, uid, privilege) {
 	const cids = await Categories.getCidsByPrivilege(set, uid, privilege);
-	return await Categories.getCategories(cids, uid);
+	return await Categories.getCategories(cids);
 };
 
 Categories.getModerators = async function (cid) {
@@ -102,7 +103,7 @@ Categories.getModeratorUids = async function (cids) {
 	return await privileges.categories.getUidsWithPrivilege(cids, 'moderate');
 };
 
-Categories.getCategories = async function (cids, uid) {
+Categories.getCategories = async function (cids) {
 	if (!Array.isArray(cids)) {
 		throw new Error('[[error:invalid-cid]]');
 	}
@@ -110,22 +111,46 @@ Categories.getCategories = async function (cids, uid) {
 	if (!cids.length) {
 		return [];
 	}
-	uid = parseInt(uid, 10);
 
-	const [categories, tagWhitelist, hasRead] = await Promise.all([
+	const [categories, tagWhitelist] = await Promise.all([
 		Categories.getCategoriesData(cids),
 		Categories.getTagWhitelist(cids),
-		Categories.hasReadCategories(cids, uid),
 	]);
 	categories.forEach((category, i) => {
 		if (category) {
 			category.tagWhitelist = tagWhitelist[i];
-			category['unread-class'] = (category.topic_count === 0 || (hasRead[i] && uid !== 0)) ? '' : 'unread';
 		}
 	});
 	return categories;
 };
 
+Categories.setUnread = async function (tree, cids, uid) {
+	if (uid <= 0) {
+		return;
+	}
+	const { unreadCids } = await topics.getUnreadData({
+		uid: uid,
+		cid: cids,
+	});
+	if (!unreadCids.length) {
+		return;
+	}
+
+	function setCategoryUnread(category) {
+		if (category) {
+			category.unread = false;
+			if (unreadCids.includes(category.cid)) {
+				category.unread = category.topic_count > 0 && true;
+			} else if (category.children.length) {
+				category.children.forEach(setCategoryUnread);
+				category.unread = category.children.some(c => c && c.unread);
+			}
+			category['unread-class'] = category.unread ? 'unread' : '';
+		}
+	}
+	tree.forEach(setCategoryUnread);
+};
+
 Categories.getTagWhitelist = async function (cids) {
 	const cachedData = {};
 
@@ -210,10 +235,6 @@ async function getChildrenTree(category, uid) {
 	let childrenData = await Categories.getCategoriesData(childrenCids);
 	childrenData = childrenData.filter(Boolean);
 	childrenCids = childrenData.map(child => child.cid);
-	const hasRead = await Categories.hasReadCategories(childrenCids, uid);
-	childrenData.forEach((child, i) => {
-		child['unread-class'] = (child.topic_count === 0 || (hasRead[i] && uid !== 0)) ? '' : 'unread';
-	});
 	Categories.getTree([category].concat(childrenData), category.parentCid);
 }
 

src/categories/search.js

@@ -38,7 +38,7 @@ module.exports = function (Categories) {
 
 		const childrenCids = await getChildrenCids(cids, uid);
 		const uniqCids = _.uniq(cids.concat(childrenCids));
-		const categoryData = await Categories.getCategories(uniqCids, uid);
+		const categoryData = await Categories.getCategories(uniqCids);
 
 		Categories.getTree(categoryData, 0);
 		await Categories.getRecentTopicReplies(categoryData, uid, data.qs);

src/categories/unread.js

@@ -4,6 +4,8 @@ const db = require('../database');
 
 module.exports = function (Categories) {
 	Categories.markAsRead = async function (cids, uid) {
+		// TODO: remove in 4.0
+		console.warn('[deprecated] Categories.markAsRead deprecated');
 		if (!Array.isArray(cids) || !cids.length || parseInt(uid, 10) <= 0) {
 			return;
 		}
@@ -14,6 +16,8 @@ module.exports = function (Categories) {
 	};
 
 	Categories.markAsUnreadForAll = async function (cid) {
+		// TODO: remove in 4.0
+		console.warn('[deprecated] Categories.markAsUnreadForAll deprecated');
 		if (!parseInt(cid, 10)) {
 			return;
 		}
@@ -21,6 +25,8 @@ module.exports = function (Categories) {
 	};
 
 	Categories.hasReadCategories = async function (cids, uid) {
+		// TODO: remove in 4.0
+		console.warn('[deprecated] Categories.hasReadCategories deprecated, see Categories.setUnread');
 		if (parseInt(uid, 10) <= 0) {
 			return cids.map(() => false);
 		}
@@ -30,6 +36,8 @@ module.exports = function (Categories) {
 	};
 
 	Categories.hasReadCategory = async function (cid, uid) {
+		// TODO: remove in 4.0
+		console.warn('[deprecated] Categories.hasReadCategory deprecated, see Categories.setUnread');
 		if (parseInt(uid, 10) <= 0) {
 			return false;
 		}

src/controllers/admin/categories.js

@@ -14,7 +14,7 @@ const categoriesController = module.exports;
 
 categoriesController.get = async function (req, res, next) {
 	const [categoryData, parent, selectedData] = await Promise.all([
-		categories.getCategories([req.params.category_id], req.uid),
+		categories.getCategories([req.params.category_id]),
 		categories.getParents([req.params.category_id]),
 		helpers.getSelectedCategory(req.params.category_id),
 	]);

src/controllers/categories.js

@@ -30,9 +30,12 @@ categoriesController.list = async function (req, res) {
 
 	const allChildCids = _.flatten(await Promise.all(pageCids.map(categories.getChildrenCids)));
 	const childCids = await privileges.categories.filterCids('find', allChildCids, req.uid);
-	const categoryData = await categories.getCategories(pageCids.concat(childCids), req.uid);
+	const categoryData = await categories.getCategories(pageCids.concat(childCids));
 	const tree = categories.getTree(categoryData, 0);
-	await categories.getRecentTopicReplies(categoryData, req.uid, req.query);
+	await Promise.all([
+		categories.getRecentTopicReplies(categoryData, req.uid, req.query),
+		categories.setUnread(tree, pageCids.concat(childCids), req.uid),
+	]);
 
 	const data = {
 		title: meta.config.homePageTitle || '[[pages:home]]',

src/controllers/category.js

@@ -98,10 +98,15 @@ categoryController.get = async function (req, res, next) {
 	categories.modifyTopicsByPrivilege(categoryData.topics, userPrivileges);
 	categoryData.tagWhitelist = categories.filterTagWhitelist(categoryData.tagWhitelist, userPrivileges.isAdminOrMod);
 
-	await buildBreadcrumbs(req, categoryData);
+	const allCategories = [];
+	categories.flattenCategories(allCategories, categoryData.children);
+
+	await Promise.all([
+		buildBreadcrumbs(req, categoryData),
+		categories.setUnread([categoryData], allCategories.map(c => c.cid).concat(cid), req.uid),
+	]);
+
 	if (categoryData.children.length) {
-		const allCategories = [];
-		categories.flattenCategories(allCategories, categoryData.children);
 		await categories.getRecentTopicReplies(allCategories, req.uid, req.query);
 		categoryData.subCategoriesLeft = Math.max(0, categoryData.children.length - categoryData.subCategoriesPerPage);
 		categoryData.hasMoreSubCategories = categoryData.children.length > categoryData.subCategoriesPerPage;
@@ -124,9 +129,6 @@ categoryController.get = async function (req, res, next) {
 	categoryData.topicIndex = topicIndex;
 	categoryData.selectedTag = tagData.selectedTag;
 	categoryData.selectedTags = tagData.selectedTags;
-	if (req.loggedIn) {
-		categories.markAsRead([cid], req.uid);
-	}
 
 	if (!meta.config['feeds:disableRSS']) {
 		categoryData.rssFeedUrl = `${url}/category/${categoryData.cid}.rss`;

src/topics/create.js

@@ -224,7 +224,6 @@ module.exports = function (Topics) {
 
 	async function onNewPost(postData, data) {
 		const { tid, uid } = postData;
-		await Topics.markCategoryUnreadForAll(tid);
 		await Topics.markAsRead([tid], uid);
 		const [
 			userInfo,

src/topics/unread.js

@@ -80,6 +80,7 @@ module.exports = function (Topics) {
 			tids: data.tids,
 			counts: data.counts,
 			tidsByFilter: data.tidsByFilter,
+			unreadCids: data.unreadCids,
 			cid: params.cid,
 			filter: params.filter,
 			query: params.query || {},
@@ -90,9 +91,9 @@ module.exports = function (Topics) {
 	async function getTids(params) {
 		const counts = { '': 0, new: 0, watched: 0, unreplied: 0 };
 		const tidsByFilter = { '': [], new: [], watched: [], unreplied: [] };
-
+		const unreadCids = [];
 		if (params.uid <= 0) {
-			return { counts: counts, tids: [], tidsByFilter: tidsByFilter };
+			return { counts, tids: [], tidsByFilter, unreadCids };
 		}
 
 		params.cutoff = await Topics.unreadCutoff(params.uid);
@@ -126,7 +127,7 @@ module.exports = function (Topics) {
 		let tids = _.uniq(unreadTopics.map(topic => topic.value)).slice(0, 200);
 
 		if (!tids.length) {
-			return { counts: counts, tids: tids, tidsByFilter: tidsByFilter };
+			return { counts, tids, tidsByFilter, unreadCids };
 		}
 
 		const blockedUids = await user.blocks.list(params.uid);
@@ -157,6 +158,7 @@ module.exports = function (Topics) {
 				if (isTopicsFollowed[topic.tid] ||
 					[categories.watchStates.watching, categories.watchStates.tracking].includes(userCidState[topic.cid])) {
 					tidsByFilter[''].push(topic.tid);
+					unreadCids.push(topic.cid);
 				}
 
 				if (isTopicsFollowed[topic.tid]) {
@@ -182,6 +184,7 @@ module.exports = function (Topics) {
 			counts: counts,
 			tids: tidsByFilter[params.filter],
 			tidsByFilter: tidsByFilter,
+			unreadCids: _.uniq(unreadCids),
 		};
 	}
 
@@ -280,7 +283,6 @@ module.exports = function (Topics) {
 	Topics.markAsUnreadForAll = async function (tid) {
 		const now = Date.now();
 		const cid = await Topics.getTopicField(tid, 'cid');
-		await Topics.markCategoryUnreadForAll(tid);
 		await Topics.updateRecent(tid, now);
 		await db.sortedSetAdd(`cid:${cid}:tids:lastposttime`, now, tid);
 		await Topics.setTopicField(tid, 'lastposttime', now);
@@ -312,15 +314,11 @@ module.exports = function (Topics) {
 		}
 
 		const scores = topics.map(topic => (topic.scheduled ? topic.lastposttime : now));
-		const [topicData] = await Promise.all([
-			Topics.getTopicsFields(tids, ['cid']),
+		await Promise.all([
 			db.sortedSetAdd(`uid:${uid}:tids_read`, scores, tids),
 			db.sortedSetRemove(`uid:${uid}:tids_unread`, tids),
 		]);
 
-		const cids = _.uniq(topicData.map(t => t && t.cid).filter(Boolean));
-		await categories.markAsRead(cids, uid);
-
 		plugins.hooks.fire('action:topics.markAsRead', { uid: uid, tids: tids });
 		return true;
 	};
@@ -343,9 +341,11 @@ module.exports = function (Topics) {
 		user.notifications.pushCount(uid);
 	};
 
-	Topics.markCategoryUnreadForAll = async function (tid) {
-		const cid = await Topics.getTopicField(tid, 'cid');
-		await categories.markAsUnreadForAll(cid);
+	Topics.markCategoryUnreadForAll = async function (/* tid */) {
+		// TODO: remove in 4.x
+		console.warn('[deprecated] Topics.markCategoryUnreadForAll deprecated');
+		// const cid = await Topics.getTopicField(tid, 'cid');
+		// await categories.markAsUnreadForAll(cid);
 	};
 
 	Topics.hasReadTopics = async function (tids, uid) {

src/upgrades/3.7.0/category-read-by-uid.js

@@ -0,0 +1,26 @@
+/* eslint-disable no-await-in-loop */
+
+'use strict';
+
+const db = require('../../database');
+const batch = require('../../batch');
+
+module.exports = {
+	name: 'Remove cid:<cid>:read_by_uid sets',
+	timestamp: Date.UTC(2024, 0, 29),
+	method: async function () {
+		const { progress } = this;
+		const nextCid = await db.getObjectField('global', 'nextCid');
+		const allCids = [];
+		for (let i = 1; i <= nextCid; i++) {
+			allCids.push(i);
+		}
+		await batch.processArray(allCids, async (cids) => {
+			await db.deleteAll(cids.map(cid => `cid:${cid}:read_by_uid`));
+			progress.incr(cids.length);
+		}, {
+			batch: 500,
+			progress,
+		});
+	},
+};

test/categories.js

@@ -68,7 +68,7 @@ describe('Categories', () => {
 	});
 
 	it('should get all categories', (done) => {
-		Categories.getAllCategories(1, (err, data) => {
+		Categories.getAllCategories((err, data) => {
 			assert.ifError(err);
 			assert(Array.isArray(data));
 			assert.equal(data[0].cid, categoryObj.cid);