Flags API (#9666)

* feat: new routes for flags API

+ flag get
+ flag creation, migration from socket method
+ flag update, migration from socket method
* fixed bug where you could not unassign someone from a flag

* feat: tests for new flags API

added missing files for schema update

* fix: flag tests to use Write API instead of sockets

* feat: flag notes API + tests

* chore: remove debug line

* test: fix breaking test on mongo

src/api/flags.js

@@ -0,0 +1,80 @@
+'use strict';
+
+const user = require('../user');
+const flags = require('../flags');
+
+const flagsApi = module.exports;
+
+flagsApi.create = async (caller, data) => {
+	const required = ['type', 'id', 'reason'];
+	if (!required.every(prop => !!data[prop])) {
+		throw new Error('[[error:invalid-data]]');
+	}
+
+	const { type, id, reason } = data;
+
+	await flags.validate({
+		uid: caller.uid,
+		type: type,
+		id: id,
+	});
+
+	const flagObj = await flags.create(type, id, caller.uid, reason);
+	flags.notify(flagObj, caller.uid);
+
+	return flagObj;
+};
+
+flagsApi.update = async (caller, data) => {
+	const allowed = await user.isPrivileged(caller.uid);
+	if (!allowed) {
+		throw new Error('[[no-privileges]]');
+	}
+
+	const { flagId } = data;
+	delete data.flagId;
+
+	await flags.update(flagId, caller.uid, data);
+	return await flags.getHistory(flagId);
+};
+
+flagsApi.appendNote = async (caller, data) => {
+	const allowed = await user.isPrivileged(caller.uid);
+	if (!allowed) {
+		throw new Error('[[error:no-privileges]]');
+	}
+	if (data.datetime && data.flagId) {
+		try {
+			const note = await flags.getNote(data.flagId, data.datetime);
+			if (note.uid !== caller.uid) {
+				throw new Error('[[error:no-privileges]]');
+			}
+		} catch (e) {
+			// Okay if not does not exist in database
+			if (!e.message === '[[error:invalid-data]]') {
+				throw e;
+			}
+		}
+	}
+	await flags.appendNote(data.flagId, caller.uid, data.note, data.datetime);
+	const [notes, history] = await Promise.all([
+		flags.getNotes(data.flagId),
+		flags.getHistory(data.flagId),
+	]);
+	return { notes: notes, history: history };
+};
+
+flagsApi.deleteNote = async (caller, data) => {
+	const note = await flags.getNote(data.flagId, data.datetime);
+	if (note.uid !== caller.uid) {
+		throw new Error('[[error:no-privileges]]');
+	}
+
+	await flags.deleteNote(data.flagId, data.datetime);
+
+	const [notes, history] = await Promise.all([
+		flags.getNotes(data.flagId),
+		flags.getHistory(data.flagId),
+	]);
+	return { notes: notes, history: history };
+};

src/api/index.js

@@ -6,4 +6,5 @@ module.exports = {
 	topics: require('./topics'),
 	posts: require('./posts'),
 	categories: require('./categories'),
+	flags: require('./flags'),
 };

src/controllers/write/flags.js

@@ -0,0 +1,48 @@
+'use strict';
+
+const user = require('../../user');
+const flags = require('../../flags');
+const api = require('../../api');
+const helpers = require('../helpers');
+
+const Flags = module.exports;
+
+Flags.create = async (req, res) => {
+	const flagObj = await api.flags.create(req, { ...req.body });
+	helpers.formatApiResponse(200, res, await user.isPrivileged(req.uid) ? flagObj : undefined);
+};
+
+Flags.get = async (req, res) => {
+	const isPrivileged = await user.isPrivileged(req.uid);
+	if (!isPrivileged) {
+		return helpers.formatApiResponse(403, res);
+	}
+
+	helpers.formatApiResponse(200, res, await flags.get(req.params.flagId));
+};
+
+Flags.update = async (req, res) => {
+	const history = await api.flags.update(req, {
+		flagId: req.params.flagId,
+		...req.body,
+	});
+
+	helpers.formatApiResponse(200, res, { history });
+};
+
+Flags.appendNote = async (req, res) => {
+	const payload = await api.flags.appendNote(req, {
+		flagId: req.params.flagId,
+		...req.body,
+	});
+
+	helpers.formatApiResponse(200, res, payload);
+};
+
+Flags.deleteNote = async (req, res) => {
+	const payload = await api.flags.deleteNote(req, {
+		...req.params,
+	});
+
+	helpers.formatApiResponse(200, res, payload);
+};

src/controllers/write/index.js

@@ -7,6 +7,7 @@ Write.groups = require('./groups');
 Write.categories = require('./categories');
 Write.topics = require('./topics');
 Write.posts = require('./posts');
+Write.flags = require('./flags');
 Write.admin = require('./admin');
 Write.files = require('./files');
 Write.utilities = require('./utilities');

src/flags.js

@@ -102,7 +102,6 @@ Flags.get = async function (flagId) {
 	if (!base) {
 		return;
 	}
-
 	const flagObj = {
 		state: 'open',
 		assignee: null,
@@ -314,6 +313,11 @@ Flags.getNotes = async function (flagId) {
 };
 
 Flags.getNote = async function (flagId, datetime) {
+	datetime = parseInt(datetime, 10);
+	if (isNaN(datetime)) {
+		throw new Error('[[error:invalid-data]]');
+	}
+
 	let notes = await db.getSortedSetRangeByScoreWithScores(`flag:${flagId}:notes`, 0, 1, datetime, datetime);
 	if (!notes.length) {
 		throw new Error('[[error:invalid-data]]');
@@ -362,6 +366,11 @@ async function modifyNotes(notes) {
 }
 
 Flags.deleteNote = async function (flagId, datetime) {
+	datetime = parseInt(datetime, 10);
+	if (isNaN(datetime)) {
+		throw new Error('[[error:invalid-data]]');
+	}
+
 	const note = await db.getSortedSetRangeByScore(`flag:${flagId}:notes`, 0, 1, datetime, datetime);
 	if (!note.length) {
 		throw new Error('[[error:invalid-data]]');
@@ -610,8 +619,10 @@ Flags.update = async function (flagId, uid, changeset) {
 				}
 			}
 		} else if (prop === 'assignee') {
+			if (changeset[prop] === '') {
+				tasks.push(db.sortedSetRemove(`flags:byAssignee:${changeset[prop]}`, flagId));
 			/* eslint-disable-next-line */
-			if (!await isAssignable(parseInt(changeset[prop], 10))) {
+			} else if (!await isAssignable(parseInt(changeset[prop], 10))) {
 				delete changeset[prop];
 			} else {
 				tasks.push(db.sortedSetAdd(`flags:byAssignee:${changeset[prop]}`, now, flagId));
@@ -701,7 +712,14 @@ Flags.appendHistory = async function (flagId, uid, changeset) {
 
 Flags.appendNote = async function (flagId, uid, note, datetime) {
 	if (datetime) {
-		await Flags.deleteNote(flagId, datetime);
+		try {
+			await Flags.deleteNote(flagId, datetime);
+		} catch (e) {
+			// Do not throw if note doesn't exist
+			if (!e.message === '[[error:invalid-data]]') {
+				throw e;
+			}
+		}
 	}
 	datetime = datetime || Date.now();
 

src/middleware/assert.js

@@ -8,6 +8,7 @@
 const path = require('path');
 const nconf = require('nconf');
 
+const db = require('../database');
 const file = require('../file');
 const user = require('../user');
 const groups = require('../groups');
@@ -52,6 +53,14 @@ Assert.post = helpers.try(async (req, res, next) => {
 	next();
 });
 
+Assert.flag = helpers.try(async (req, res, next) => {
+	if (!await db.isSortedSetMember('flags:datetime', req.params.flagId)) {
+		return controllerHelpers.formatApiResponse(404, res, new Error('[[error:no-flag]]'));
+	}
+
+	next();
+});
+
 Assert.path = helpers.try(async (req, res, next) => {
 	// file: URL support
 	if (req.body.path.startsWith('file:///')) {

src/routes/write/flags.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const router = require('express').Router();
+const middleware = require('../../middleware');
+const controllers = require('../../controllers');
+const routeHelpers = require('../helpers');
+
+const { setupApiRoute } = routeHelpers;
+
+module.exports = function () {
+	const middlewares = [middleware.ensureLoggedIn];
+
+	setupApiRoute(router, 'post', '/', [...middlewares], controllers.write.flags.create);
+	// setupApiRoute(router, 'delete', ...);	// does not exist
+
+	setupApiRoute(router, 'get', '/:flagId', [...middlewares, middleware.assert.flag], controllers.write.flags.get);
+	setupApiRoute(router, 'put', '/:flagId', [...middlewares, middleware.assert.flag], controllers.write.flags.update);
+
+	setupApiRoute(router, 'post', '/:flagId/notes', [...middlewares, middleware.assert.flag], controllers.write.flags.appendNote);
+	setupApiRoute(router, 'delete', '/:flagId/notes/:datetime', [...middlewares, middleware.assert.flag], controllers.write.flags.deleteNote);
+
+	return router;
+};

src/routes/write/index.js

@@ -37,6 +37,7 @@ Write.reload = async (params) => {
 	router.use('/api/v3/categories', require('./categories')());
 	router.use('/api/v3/topics', require('./topics')());
 	router.use('/api/v3/posts', require('./posts')());
+	router.use('/api/v3/flags', require('./flags')());
 	router.use('/api/v3/admin', require('./admin')());
 	router.use('/api/v3/files', require('./files')());
 	router.use('/api/v3/utilities', require('./utilities')());

src/routes/write/users.js

@@ -40,11 +40,11 @@ function authenticatedRoutes() {
 
 	setupApiRoute(router, 'delete', '/:uid/sessions/:uuid', [...middlewares, middleware.assert.user], controllers.write.users.revokeSession);
 
-	// Shorthand route to access user routes by userslug
-	router.all('/+bySlug/:userslug*?', [], controllers.write.users.redirectBySlug);
-
 	setupApiRoute(router, 'post', '/:uid/invites', middlewares, controllers.write.users.invite);
 	setupApiRoute(router, 'get', '/:uid/invites/groups', [...middlewares, middleware.assert.user], controllers.write.users.getInviteGroups);
+
+	// Shorthand route to access user routes by userslug
+	router.all('/+bySlug/:userslug*?', [], controllers.write.users.redirectBySlug);
 }
 
 module.exports = function () {

src/socket.io/flags.js

@@ -1,69 +1,43 @@
 'use strict';
 
-const user = require('../user');
-const flags = require('../flags');
+const sockets = require('.');
+const api = require('../api');
 
 const SocketFlags = module.exports;
 
 SocketFlags.create = async function (socket, data) {
-	if (!socket.uid) {
-		throw new Error('[[error:not-logged-in]]');
+	sockets.warnDeprecated(socket, 'POST /api/v3/flags');
+	const response = await api.flags.create(socket, data);
+	if (response) {
+		return response.flagId;
 	}
-
-	if (!data || !data.type || !data.id || !data.reason) {
-		throw new Error('[[error:invalid-data]]');
-	}
-	await flags.validate({
-		uid: socket.uid,
-		type: data.type,
-		id: data.id,
-	});
-
-	const flagObj = await flags.create(data.type, data.id, socket.uid, data.reason);
-	await flags.notify(flagObj, socket.uid);
-	return flagObj.flagId;
 };
 
 SocketFlags.update = async function (socket, data) {
-	if (!data || !(data.flagId && data.data)) {
+	sockets.warnDeprecated(socket, 'PUT /api/v3/flags/:flagId');
+	if (!data || !(data.flagId && data.data)) {	// check only req'd in socket.io
 		throw new Error('[[error:invalid-data]]');
 	}
 
-	const allowed = await user.isPrivileged(socket.uid);
-	if (!allowed) {
-		throw new Error('[[no-privileges]]');
-	}
-	let payload = {};
-	// Translate form data into object
+	// Old socket method took input directly from .serializeArray(), v3 expects fully-formed obj.
+	let payload = {
+		flagId: data.flagId,
+	};
 	payload = data.data.reduce((memo, cur) => {
 		memo[cur.name] = cur.value;
 		return memo;
 	}, payload);
 
-	await flags.update(data.flagId, socket.uid, payload);
-	return await flags.getHistory(data.flagId);
+	return await api.flags.update(socket, payload);
 };
 
 SocketFlags.appendNote = async function (socket, data) {
+	sockets.warnDeprecated(socket, 'POST /api/v3/flags/:flagId/notes');
 	if (!data || !(data.flagId && data.note)) {
 		throw new Error('[[error:invalid-data]]');
 	}
-	const allowed = await user.isPrivileged(socket.uid);
-	if (!allowed) {
-		throw new Error('[[error:no-privileges]]');
-	}
-	if (data.datetime && data.flagId) {
-		const note = await flags.getNote(data.flagId, data.datetime);
-		if (note.uid !== socket.uid) {
-			throw new Error('[[error:no-privileges]]');
-		}
-	}
-	await flags.appendNote(data.flagId, socket.uid, data.note, data.datetime);
-	const [notes, history] = await Promise.all([
-		flags.getNotes(data.flagId),
-		flags.getHistory(data.flagId),
-	]);
-	return { notes: notes, history: history };
+
+	return await api.flags.appendNote(socket, data);
 };
 
 SocketFlags.deleteNote = async function (socket, data) {
@@ -71,18 +45,7 @@ SocketFlags.deleteNote = async function (socket, data) {
 		throw new Error('[[error:invalid-data]]');
 	}
 
-	const note = await flags.getNote(data.flagId, data.datetime);
-	if (note.uid !== socket.uid) {
-		throw new Error('[[error:no-privileges]]');
-	}
-
-	await flags.deleteNote(data.flagId, data.datetime);
-
-	const [notes, history] = await Promise.all([
-		flags.getNotes(data.flagId),
-		flags.getHistory(data.flagId),
-	]);
-	return { notes: notes, history: history };
+	return await api.flags.deleteNote(socket, data);
 };
 
 require('../promisify')(SocketFlags);