fix: #13990, don't blindly set user field on notification objects

that don't have a "from" property

install/package.json

@@ -107,10 +107,10 @@
         "nodebb-plugin-spam-be-gone": "2.3.2",
         "nodebb-plugin-web-push": "0.7.6",
         "nodebb-rewards-essentials": "1.0.2",
-        "nodebb-theme-harmony": "2.2.10",
+        "nodebb-theme-harmony": "2.2.11",
         "nodebb-theme-lavender": "7.1.21",
         "nodebb-theme-peace": "2.2.51",
-        "nodebb-theme-persona": "14.2.4",
+        "nodebb-theme-persona": "14.2.5",
         "nodebb-widget-essentials": "7.0.42",
         "nodemailer": "8.0.1",
         "nprogress": "0.2.0",

src/notifications.js

@@ -86,7 +86,7 @@ Notifications.getMultiple = async function (nids) {
 	const keys = nids.map(nid => `notifications:${nid}`);
 	const notifications = await db.getObjects(keys);
 
-	const userKeys = notifications.map(n => n && n.from);
+	const userKeys = notifications.filter(n => n && n.from).map(n => n.from);
 	let [usersData, categoriesData] = await Promise.all([
 		User.getUsersFields(userKeys, ['username', 'userslug', 'picture']),
 		categories.getCategoriesFields(userKeys, ['cid', 'name', 'slug', 'picture']),
@@ -105,8 +105,10 @@ Notifications.getMultiple = async function (nids) {
 
 		return userData;
 	});
+	// from can be either uid or cid
+	const userMap = new Map(userKeys.map((from, index) => [from, usersData[index]]));
 
-	notifications.forEach((notification, index) => {
+	notifications.forEach((notification) => {
 		if (notification) {
 			intFields.forEach((field) => {
 				if (notification.hasOwnProperty(field)) {
@@ -123,8 +125,9 @@ Notifications.getMultiple = async function (nids) {
 			if (notification.bodyLong) {
 				notification.bodyLong = utils.stripHTMLTags(notification.bodyLong, ['img', 'p', 'a']);
 			}
-
-			notification.user = usersData[index];
+			if (userMap.has(notification.from)) {
+				notification.user = userMap.get(notification.from);
+			}
 			if (notification.user && notification.from) {
 				notification.image = notification.user.picture || null;
 				if (notification.user.username === '[[global:guest]]') {
@@ -166,7 +169,7 @@ Notifications.create = async function (data) {
 	const oldNotif = await db.getObject(`notifications:${data.nid}`);
 	if (
 		oldNotif &&
-		parseInt(oldNotif.pid, 10) === parseInt(data.pid, 10) &&
+		String(oldNotif.pid, 10) === String(data.pid, 10) &&
 		parseInt(oldNotif.importance, 10) > parseInt(data.importance, 10)
 	) {
 		return null;

test/notifications.js

@@ -19,15 +19,8 @@ describe('Notifications', () => {
 	let uid;
 	let notification;
 
-	before((done) => {
-		user.create({ username: 'poster' }, (err, _uid) => {
-			if (err) {
-				return done(err);
-			}
-
-			uid = _uid;
-			done();
-		});
+	before(async () => {
+		uid = await user.create({ username: 'poster' });
 	});
 
 	it('should fail to create notification without a nid', (done) => {
@@ -59,6 +52,32 @@ describe('Notifications', () => {
 		});
 	});
 
+	it('should create a notification with a custom icon', async () => {
+		const nid = 'custom-icon-notification';
+		await notifications.create({
+			nid: nid,
+			bodyShort: 'Notification with custom icon',
+			icon: 'fa-solid fa-bell',
+		});
+		const notifData = await notifications.get(nid);
+		assert.strictEqual(notifData.user, undefined);
+		assert.strictEqual(notifData.icon, 'fa-solid fa-bell');
+	});
+
+	it('should create a notification with a user icon/bgColor', async () => {
+		const uid = await user.create({ username: 'iconuser' });
+		const nid = 'user-icon-notification';
+		await notifications.create({
+			nid: nid,
+			bodyShort: 'Notification with user icon',
+			from: uid,
+		});
+		const notifData = await notifications.get(nid);
+		assert.strictEqual(notifData.icon, undefined);
+		assert.strictEqual(notifData.user['icon:text'], 'I');
+		assert.strictEqual(notifData.user['icon:bgColor'], '#3f51b5');
+	});
+
 	it('should return null if pid is same and importance is lower', (done) => {
 		notifications.create({
 			bodyShort: 'bodyShort',