feat: consolidation of flags to reduce flagspam, #8510

Squashed commit of the following:

commit c6d0939620
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 24 13:41:32 2020 -0400

    fix: more tests

commit 32f9af2a87
Merge: e50907535 4eae927d1
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 24 10:53:04 2020 -0400

    Merge remote-tracking branch 'origin/master' into singleton-flags

commit e509075351
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 24 10:52:46 2020 -0400

    fix: controllers-admin test

commit fd5af99e30
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 17 17:26:55 2020 -0400

    fix(tests): dummy commit to trigger travisCI

commit c452a6ffcf
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 17 17:05:09 2020 -0400

    fix(openapi): openapi spec changes

commit 8089a74e89
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 17 15:48:00 2020 -0400

    fix: reversing the order of reports for display purposes

commit a099892b37
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 17 15:45:44 2020 -0400

    refactor: run all flag creation calls in a single batch

commit b24999682f
Author: Julian Lam <julian@nodebb.org>
Date:   Fri Jul 17 15:08:23 2020 -0400

    feat: handling multiple reporters per flag, #8510

commit 08c75c0200
Author: Julian Lam <julian@nodebb.org>
Date:   Thu Jul 16 20:53:18 2020 -0400

    feat: upgrade script for #8510

public/language/en-GB/flags.json

@@ -1,8 +1,7 @@
 {
 	"state": "State",
-	"reporter": "Reporter",
+	"reports": "Reports",
-	"reported-at": "Reported At",
+	"first-reported": "First Reported",
-	"description": "Description",
 	"no-flags": "Hooray! No flags found.",
 	"assignee": "Assignee",
 	"update": "Update",

public/openapi/read.yaml

@@ -3089,6 +3089,9 @@ paths:
                     type: boolean
                   downvoted:
                     type: boolean
+                  flagId:
+                    type: number
+                    description: The flag identifier, if this particular post has been flagged before
   "/api/topic/tid/{id}":
     get:
       tags:
@@ -3735,6 +3738,9 @@ paths:
                               type: boolean
                             display_post_menu:
                               type: boolean
+                            flagId:
+                              type: number
+                              description: The flag identifier, if this particular post has been flagged before
                       category:
                         $ref: components/schemas/CategoryObject.yaml#/CategoryObject
                       tagWhitelist:
@@ -4788,6 +4794,9 @@ paths:
                           properties:
                             state:
                               type: string
+                            heat:
+                              type: number
+                              description: The number of reports that make up this flag
                             flagId:
                               type: number
                             type:
@@ -4796,34 +4805,8 @@ paths:
                               oneOf:
                                 - type: string
                                 - type: number
-                            description:
-                              type: string
-                            uid:
-                              type: number
-                              description: A user identifier
                             datetime:
                               type: number
-                            reporter:
-                              type: object
-                              properties:
-                                username:
-                                  type: string
-                                  description: A friendly name for a given user account
-                                picture:
-                                  nullable: true
-                                  type: string
-                                icon:bgColor:
-                                  type: string
-                                  description: A six-character hexadecimal colour code assigned to the user. This
-                                    value is used in conjunction with
-                                    `icon:text` for the user's auto-generated
-                                    icon
-                                  example: "#f44336"
-                                icon:text:
-                                  type: string
-                                  description: A single-letter representation of a username. This is used in the
-                                    auto-generated icon given to users without
-                                    an avatar
                             labelClass:
                               type: string
                             target_readable:
@@ -4886,11 +4869,6 @@ paths:
                         type: string
                       targetId:
                         type: number
-                      description:
-                        type: string
-                      uid:
-                        type: number
-                        description: A user identifier
                       datetime:
                         type: number
                       datetimeISO:
@@ -5006,6 +4984,17 @@ paths:
                                     `icon:text` for the user's auto-generated
                                     icon
                                   example: "#f44336"
+                      reports:
+                        type: array
+                        items:
+                          type: object
+                          properties:
+                            value:
+                              type: string
+                            timestamp:
+                              type: number
+                            timestampISO:
+                              type: string
                             reporter:
                               type: object
                               properties:

public/src/client/flags/list.js

@@ -6,6 +6,11 @@ define('forum/flags/list', ['components', 'Chart'], function (components, Chart)
 	Flags.init = function () {
 		Flags.enableFilterForm();
 
+		components.get('flags/list').on('click', '[data-flag-id]', function () {
+			var flagId = this.getAttribute('data-flag-id');
+			ajaxify.go('flags/' + flagId);
+		});
+
 		var graphWrapper = $('#flags-daily-wrapper');
 		var graphFooter = graphWrapper.siblings('.panel-footer');
 		$('#flags-daily-wrapper').one('shown.bs.collapse', function () {

src/flags.js

@@ -84,32 +84,28 @@ Flags.init = async function () {
 };
 
 Flags.get = async function (flagId) {
-	const [base, history, notes] = await Promise.all([
+	const [base, history, notes, reports] = await Promise.all([
 		db.getObject('flag:' + flagId),
 		Flags.getHistory(flagId),
 		Flags.getNotes(flagId),
+		Flags.getReports(flagId),
 	]);
 	if (!base) {
 		return;
 	}
 
-	const [userObj, targetObj] = await Promise.all([
-		user.getUserFields(base.uid, ['username', 'userslug', 'picture', 'reputation']),
-		Flags.getTarget(base.type, base.targetId, 0),
-	]);
-
 	const flagObj = {
 		state: 'open',
 		assignee: null,
 		...base,
-		description: validator.escape(base.description),
 		datetimeISO: utils.toISOString(base.datetime),
 		target_readable: base.type.charAt(0).toUpperCase() + base.type.slice(1) + ' ' + base.targetId,
-		target: targetObj,
+		target: await Flags.getTarget(base.type, base.targetId, 0),
 		history: history,
 		notes: notes,
-		reporter: userObj,
+		reports: reports,
 	};
+
 	const data = await plugins.fireHook('filter:flags.get', {
 		flag: flagObj,
 	});
@@ -160,24 +156,19 @@ Flags.list = async function (filters, uid) {
 	const pageCount = Math.ceil(flagIds.length / flagsPerPage);
 	flagIds = flagIds.slice((filters.page - 1) * flagsPerPage, filters.page * flagsPerPage);
 
-	const flags = await Promise.all(flagIds.map(async (flagId) => {
+	const reportCounts = await db.sortedSetsCard(flagIds.map(flagId => `flag:${flagId}:reports`));
+
+	const flags = await Promise.all(flagIds.map(async (flagId, idx) => {
 		let flagObj = await db.getObject('flag:' + flagId);
-		const userObj = await user.getUserFields(flagObj.uid, ['username', 'picture']);
 		flagObj = {
 			state: 'open',
 			assignee: null,
+			heat: reportCounts[idx],
 			...flagObj,
-			reporter: {
-				username: userObj.username,
-				picture: userObj.picture,
-				'icon:bgColor': userObj['icon:bgColor'],
-				'icon:text': userObj['icon:text'],
-			},
 		};
 		flagObj.labelClass = Flags._constants.state_class[flagObj.state];
 
 		return Object.assign(flagObj, {
-			description: validator.escape(String(flagObj.description)),
 			target_readable: flagObj.type.charAt(0).toUpperCase() + flagObj.type.slice(1) + ' ' + flagObj.targetId,
 			datetimeISO: utils.toISOString(flagObj.datetime),
 		});
@@ -242,6 +233,24 @@ Flags.getNote = async function (flagId, datetime) {
 	return notes[0];
 };
 
+Flags.getFlagIdByTarget = async function (type, id) {
+	let method;
+	switch (type) {
+		case 'post':
+			method = posts.getPostField;
+			break;
+
+		case 'user':
+			method = user.getUserField;
+			break;
+
+		default:
+			throw new Error('[[error:invalid-data]]');
+	}
+
+	return await method(id, 'flagId');
+};
+
 async function modifyNotes(notes) {
 	const uids = [];
 	notes = notes.map(function (note) {
@@ -277,11 +286,13 @@ Flags.create = async function (type, id, uid, reason, timestamp) {
 		timestamp = Date.now();
 		doHistoryAppend = true;
 	}
-	const [flagExists, targetExists,, targetUid, targetCid] = await Promise.all([
+	const [flagExists, targetExists,, targetFlagged, targetUid, targetCid] = await Promise.all([
 		// Sanity checks
 		Flags.exists(type, id, uid),
 		Flags.targetExists(type, id),
 		Flags.canFlag(type, id, uid),
+		Flags.targetFlagged(type, id),
+
 		// Extra data for zset insertion
 		Flags.getTargetUid(type, id),
 		Flags.getTargetCid(type, id),
@@ -292,45 +303,91 @@ Flags.create = async function (type, id, uid, reason, timestamp) {
 		throw new Error('[[error:invalid-data]]');
 	}
 
-	const flagId = await db.incrObjectField('global', 'nextFlagId');
+	// If the flag already exists, just add the report
+	if (targetFlagged) {
+		const flagId = await Flags.getFlagIdByTarget(type, id);
+		await Promise.all([
+			Flags.addReport(flagId, uid, reason, timestamp),
+			Flags.update(flagId, uid, { state: 'open' }),
+		]);
 
-	await db.setObject('flag:' + flagId, {
+		return await Flags.get(flagId);
+	}
+
+	const flagId = await db.incrObjectField('global', 'nextFlagId');
+	const batched = [];
+
+	batched.push(
+		db.setObject.bind(db, 'flag:' + flagId, {
 			flagId: flagId,
 			type: type,
 			targetId: id,
-		description: reason,
-		uid: uid,
 			datetime: timestamp,
-	});
+		}),
-	await db.sortedSetAdd('flags:datetime', timestamp, flagId); // by time, the default
+		Flags.addReport.bind(Flags, flagId, uid, reason, timestamp),
-	await db.sortedSetAdd('flags:byReporter:' + uid, timestamp, flagId); // by reporter
+		db.sortedSetAdd.bind(db, 'flags:datetime', timestamp, flagId), // by time, the default
-	await db.sortedSetAdd('flags:byType:' + type, timestamp, flagId);	// by flag type
+		db.sortedSetAdd.bind(db, 'flags:byType:' + type, timestamp, flagId),	// by flag type
-	await db.sortedSetAdd('flags:hash', flagId, [type, id, uid].join(':')); // save zset for duplicate checking
+		db.sortedSetAdd.bind(db, 'flags:hash', flagId, [type, id, uid].join(':')), // save zset for duplicate checking
-	await db.sortedSetIncrBy('flags:byTarget', 1, [type, id].join(':'));	// by flag target (score is count)
+		db.sortedSetIncrBy.bind(db, 'flags:byTarget', 1, [type, id].join(':')),	// by flag target (score is count)
-	await analytics.increment('flags'); // some fancy analytics
+		analytics.increment.bind(analytics, 'flags') // some fancy analytics
+	);
 
 	if (targetUid) {
-		await db.sortedSetAdd('flags:byTargetUid:' + targetUid, timestamp, flagId); // by target uid
+		batched.push(db.sortedSetAdd.bind(db, 'flags:byTargetUid:' + targetUid, timestamp, flagId)); // by target uid
 	}
 
 	if (targetCid) {
-		await db.sortedSetAdd('flags:byCid:' + targetCid, timestamp, flagId); // by target cid
+		batched.push(db.sortedSetAdd.bind(db, 'flags:byCid:' + targetCid, timestamp, flagId)); // by target cid
 	}
 
 	if (type === 'post') {
-		await db.sortedSetAdd('flags:byPid:' + id, timestamp, flagId);	// by target pid
+		batched.push(
+			db.sortedSetAdd.bind(db, 'flags:byPid:' + id, timestamp, flagId),	// by target pid
+			posts.setPostField.bind(posts, id, 'flagId', flagId)
+		);
+
 		if (targetUid) {
-			await user.incrementUserFlagsBy(targetUid, 1);
+			batched.push(user.incrementUserFlagsBy.bind(user, targetUid, 1));
 		}
+	} else if (type === 'user') {
+		batched.push(user.setUserField.bind(user, id, 'flagId', flagId));
 	}
 
+	// Run all the database calls in one single batched call...
+	await Promise.all(batched.map(async method => await method()));
+
 	if (doHistoryAppend) {
-		await Flags.update(flagId, uid, { state: 'open' });
+		Flags.update(flagId, uid, { state: 'open' });
 	}
 
 	return await Flags.get(flagId);
 };
 
+Flags.getReports = async function (flagId) {
+	const [reports, reporterUids] = await Promise.all([
+		db.getSortedSetRevRangeWithScores(`flag:${flagId}:reports`, 0, -1),
+		db.getSortedSetRevRange(`flag:${flagId}:reporters`, 0, -1),
+	]);
+
+	await Promise.all(reports.map(async (report, idx) => {
+		report.timestamp = report.score;
+		report.timestampISO = new Date(report.score).toISOString();
+		delete report.score;
+		report.reporter = await user.getUserFields(reporterUids[idx], ['username', 'userslug', 'picture', 'reputation']);
+	}));
+
+	return reports;
+};
+
+Flags.addReport = async function (flagId, uid, reason, timestamp) {
+	// adds to reporters/report zsets
+	await Promise.all([
+		db.sortedSetAdd(`flags:byReporter:${uid}`, timestamp, flagId),
+		db.sortedSetAdd(`flag:${flagId}:reports`, timestamp, reason),
+		db.sortedSetAdd(`flag:${flagId}:reporters`, timestamp, uid),
+	]);
+};
+
 Flags.exists = async function (type, id, uid) {
 	return await db.isSortedSetMember('flags:hash', [type, id, uid].join(':'));
 };
@@ -386,6 +443,10 @@ Flags.targetExists = async function (type, id) {
 	throw new Error('[[error:invalid-data]]');
 };
 
+Flags.targetFlagged = async function (type, id) {
+	return await db.sortedSetScore('flags:byTarget', [type, id].join(':')) >= 1;
+};
+
 Flags.getTargetUid = async function (type, id) {
 	if (type === 'post') {
 		return await posts.getPostField(id, 'uid');
@@ -443,7 +504,7 @@ Flags.update = async function (flagId, uid, changeset) {
 					tasks.push(db.sortedSetAdd('flags:byState:' + changeset[prop], now, flagId));
 					tasks.push(db.sortedSetRemove('flags:byState:' + current[prop], flagId));
 					if (changeset[prop] === 'resolved' || changeset[prop] === 'rejected') {
-						tasks.push(notifications.rescind('flag:' + current.type + ':' + current.targetId + ':uid:' + current.uid));
+						tasks.push(notifications.rescind('flag:' + current.type + ':' + current.targetId));
 					}
 				}
 			} else if (prop === 'assignee') {
@@ -545,11 +606,11 @@ Flags.notify = async function (flagObj, uid) {
 
 		notifObj = await notifications.create({
 			type: 'new-post-flag',
-			bodyShort: '[[notifications:user_flagged_post_in, ' + flagObj.reporter.username + ', ' + titleEscaped + ']]',
+			bodyShort: '[[notifications:user_flagged_post_in, ' + flagObj.reports[flagObj.reports.length - 1].reporter.username + ', ' + titleEscaped + ']]',
 			bodyLong: flagObj.description,
 			pid: flagObj.targetId,
 			path: '/flags/' + flagObj.flagId,
-			nid: 'flag:post:' + flagObj.targetId + ':uid:' + uid,
+			nid: 'flag:post:' + flagObj.targetId,
 			from: uid,
 			mergeId: 'notifications:user_flagged_post_in|' + flagObj.targetId,
 			topicTitle: title,
@@ -558,10 +619,10 @@ Flags.notify = async function (flagObj, uid) {
 	} else if (flagObj.type === 'user') {
 		notifObj = await notifications.create({
 			type: 'new-user-flag',
-			bodyShort: '[[notifications:user_flagged_user, ' + flagObj.reporter.username + ', ' + flagObj.target.username + ']]',
+			bodyShort: '[[notifications:user_flagged_user, ' + flagObj.reports[flagObj.reports.length - 1].reporter.username + ', ' + flagObj.target.username + ']]',
 			bodyLong: flagObj.description,
 			path: '/flags/' + flagObj.flagId,
-			nid: 'flag:user:' + flagObj.targetId + ':uid:' + uid,
+			nid: 'flag:user:' + flagObj.targetId,
 			from: uid,
 			mergeId: 'notifications:user_flagged_user|' + flagObj.targetId,
 		});

src/upgrades/1.14.3/consolidate_flags.js

@@ -0,0 +1,46 @@
+'use strict';
+
+const db = require('../../database');
+const batch = require('../../batch');
+const posts = require('../../posts');
+const user = require('../../user');
+
+module.exports = {
+	name: 'Consolidate multiple flags reports, going forward',
+	timestamp: Date.UTC(2020, 6, 16),
+	method: async function () {
+		const progress = this.progress;
+
+		let flags = await db.getSortedSetRange('flags:datetime', 0, -1);
+		flags = flags.map(flagId => `flag:${flagId}`);
+		flags = await db.getObjectsFields(flags, ['flagId', 'type', 'targetId', 'uid', 'description', 'datetime']);
+		progress.total = flags.length;
+
+		await batch.processArray(flags, async function (subset) {
+			progress.incr(subset.length);
+
+			await Promise.all(subset.map(async (flagObj) => {
+				const methods = [];
+				switch (flagObj.type) {
+					case 'post':
+						methods.push(posts.setPostField.bind(posts, flagObj.targetId, 'flagId', flagObj.flagId));
+						break;
+
+					case 'user':
+						methods.push(user.setUserField.bind(user, flagObj.targetId, 'flagId', flagObj.flagId));
+						break;
+				}
+
+				methods.push(
+					db.sortedSetAdd.bind(db, `flag:${flagObj.flagId}:reports`, flagObj.datetime, flagObj.description),
+					db.sortedSetAdd.bind(db, `flag:${flagObj.flagId}:reporters`, flagObj.datetime, flagObj.uid)
+				);
+
+				await Promise.all(methods.map(async method => method()));
+			}));
+		}, {
+			progress: progress,
+			batch: 500,
+		});
+	},
+};

test/controllers-admin.js

@@ -712,7 +712,9 @@ describe('Admin Controllers', function () {
 				request(nconf.get('url') + '/api/flags/' + flagId, { jar: moderatorJar, json: true }, function (err, res, body) {
 					assert.ifError(err);
 					assert(body);
-					assert.equal(body.reporter.username, 'regular');
+					assert(body.reports);
+					assert(Array.isArray(body.reports));
+					assert.equal(body.reports[0].reporter.username, 'regular');
 					done();
 				});
 			});

test/flags.js

@@ -48,10 +48,10 @@ describe('Flags', function () {
 				assert.ifError(err);
 				var compare = {
 					flagId: 1,
-					uid: 1,
 					targetId: 1,
 					type: 'post',
-					description: 'Test flag',
+					state: 'open',
+					target_readable: 'Post 1',
 				};
 				assert(flagData);
 				for (var key in compare) {
@@ -124,11 +124,10 @@ describe('Flags', function () {
 				assert.ifError(err);
 				var compare = {
 					flagId: 1,
-					uid: 1,
 					targetId: 1,
 					type: 'post',
-					description: 'Test flag',
 					state: 'open',
+					target_readable: 'Post 1',
 				};
 				assert(flagData);
 				for (var key in compare) {
@@ -378,14 +377,14 @@ describe('Flags', function () {
 			await sleep(2000);
 
 			let userNotifs = await User.notifications.getAll(adminUid);
-			assert(userNotifs.includes('flag:post:' + result.postData.pid + ':uid:' + uid1));
+			assert(userNotifs.includes('flag:post:' + result.postData.pid));
 
 			await Flags.update(flagId, adminUid, {
 				state: 'resolved',
 			});
 
 			userNotifs = await User.notifications.getAll(adminUid);
-			assert(!userNotifs.includes('flag:post:' + result.postData.pid + ':uid:' + uid1));
+			assert(!userNotifs.includes('flag:post:' + result.postData.pid));
 		});
 	});