refactor: move groups.leave, fix some tests

src/api/groups.js

@@ -1,10 +1,14 @@
 'use strict';
 
+const validator = require('validator');
+
 const privileges = require('../privileges');
 const events = require('../events');
 const groups = require('../groups');
 const user = require('../user');
 const meta = require('../meta');
+const notifications = require('../notifications');
+const slugify = require('../slugify');
 
 const groupsAPI = module.exports;
 
@@ -100,10 +104,64 @@ groupsAPI.join = async function (caller, data) {
 	}
 };
 
-// groupsAPI.leave = async function (caller, data) {
+groupsAPI.leave = async function (caller, data) {
-// 	// TODO:
+	if (caller.uid <= 0) {
-// };
+		throw new Error('[[error:invalid-uid]]');
+	}
+	const isSelf = parseInt(caller.uid, 10) === parseInt(data.uid, 10);
+	const groupName = await groups.getGroupNameByGroupSlug(data.slug);
+	if (!groupName) {
+		throw new Error('[[error:no-group]]');
+	}
 
+	if (typeof groupName !== 'string') {
+		throw new Error('[[error:invalid-group-name]]');
+	}
+
+	if (groupName === 'administrators' && isSelf) {
+		throw new Error('[[error:cant-remove-self-as-admin]]');
+	}
+
+	const [groupData, isCallerAdmin, isCallerOwner, userExists, isMember] = await Promise.all([
+		groups.getGroupData(groupName),
+		user.isAdministrator(caller.uid),
+		groups.ownership.isOwner(caller.uid, groupName),
+		user.exists(data.uid),
+		groups.isMember(data.uid, groupName),
+	]);
+
+	if (!userExists) {
+		throw new Error('[[error:invalid-uid]]');
+	}
+	if (!isMember) {
+		return;
+	}
+
+	if (groupData.disableLeave && isSelf) {
+		throw new Error('[[error:group-leave-disabled]]');
+	}
+
+	if (isSelf || isCallerAdmin || isCallerOwner) {
+		await groups.leave(groupName, data.uid);
+	} else {
+		throw new Error('[[error:no-privileges]]');
+	}
+
+	const username = await user.getUserField(data.uid, 'username');
+	const notification = await notifications.create({
+		type: 'group-leave',
+		bodyShort: '[[groups:membership.leave.notification_title, ' + username + ', ' + groupName + ']]',
+		nid: 'group:' + validator.escape(groupName) + ':uid:' + data.uid + ':group-leave',
+		path: '/groups/' + slugify(groupName),
+	});
+	const uids = await groups.getOwners(groupName);
+	await notifications.push(notification, uids);
+
+	logGroupEvent(caller, 'group-leave', {
+		groupName: groupName,
+		targetUid: data.uid,
+	});
+};
 
 async function isOwner(caller, groupName) {
 	if (typeof groupName !== 'string') {

src/api/users.js

@@ -12,6 +12,9 @@ const events = require('../events');
 const usersAPI = module.exports;
 
 usersAPI.create = async function (caller, data) {
+	if (!data) {
+		throw new Error('[[error:invalid-data]]');
+	}
 	const uid = await user.create(data);
 	return await user.getUserData(uid);
 };

src/controllers/write/groups.js

@@ -1,12 +1,5 @@
 'use strict';
 
-const validator = require('validator');
-
-const user = require('../../user');
-const groups = require('../../groups');
-const events = require('../../events');
-const slugify = require('../../slugify');
-const notifications = require('../../notifications');
 const api = require('../../api');
 
 const helpers = require('../helpers');
@@ -29,46 +22,6 @@ Groups.join = async (req, res) => {
 };
 
 Groups.leave = async (req, res) => {
-	const [group, userExists] = await Promise.all([
+	await api.groups.leave(req, req.params);
-		groups.getByGroupslug(req.params.slug, {
-			uid: req.params.uid,
-		}),
-		user.exists(req.params.uid),
-	]);
-
-	if (!userExists) {
-		throw new Error('[[error:invalid-uid]]');
-	} else if (group.disableLeave) {
-		throw new Error('[[error:group-leave-disabled]]');
-	} else if (!group.isMember) {
-		// No change
-		return helpers.formatApiResponse(200, res);
-	}
-
-	await groups.leave(group.name, req.params.uid);
-
-	// Notify owners of user having left
-	const username = await user.getUserField(req.params.uid, 'username');
-	const notification = await notifications.create({
-		type: 'group-leave',
-		bodyShort: '[[groups:membership.leave.notification_title, ' + username + ', ' + group.name + ']]',
-		nid: 'group:' + validator.escape(group.name) + ':uid:' + req.params.uid + ':group-leave',
-		path: '/groups/' + slugify(group.name),
-	});
-	const uids = await groups.getOwners(group.name);
-	await notifications.push(notification, uids);
-
 	helpers.formatApiResponse(200, res);
-	logGroupEvent(req, 'group-leave', {
-		groupName: group.name,
-	});
 };
-
-function logGroupEvent(req, event, additional) {
-	events.log({
-		type: event,
-		uid: req.user.uid,
-		ip: req.ip,
-		...additional,
-	});
-}

src/socket.io/admin/groups.js

@@ -2,6 +2,7 @@
 
 const groups = require('../../groups');
 const sockets = require('..');
+const api = require('../../api');
 
 const Groups = module.exports;
 
@@ -25,34 +26,20 @@ Groups.create = async function (socket, data) {
 
 Groups.join = async (socket, data) => {
 	sockets.warnDeprecated(socket, 'PUT /api/v3/groups/:slug/membership/:uid');
-
 	if (!data) {
 		throw new Error('[[error:invalid-data]]');
 	}
-
+	const slug = await groups.getGroupField(data.groupName, 'slug');
-	const isMember = await groups.isMember(data.uid, data.groupName);
+	return await api.groups.join(socket, { slug: slug, uid: data.uid });
-	if (isMember) {
-		throw new Error('[[error:group-already-member]]');
-	}
-
-	return await groups.join(data.groupName, data.uid);
 };
 
 Groups.leave = async function (socket, data) {
 	sockets.warnDeprecated(socket, 'DELETE /api/v3/groups/:slug/membership/:uid');
-
 	if (!data) {
 		throw new Error('[[error:invalid-data]]');
 	}
-
+	const slug = await groups.getGroupField(data.groupName, 'slug');
-	if (socket.uid === parseInt(data.uid, 10) && data.groupName === 'administrators') {
+	await api.groups.leave(socket, { slug: slug, uid: data.uid });
-		throw new Error('[[error:cant-remove-self-as-admin]]');
-	}
-	const isMember = await groups.isMember(data.uid, data.groupName);
-	if (!isMember) {
-		throw new Error('[[error:group-not-member]]');
-	}
-	await groups.leave(data.groupName, data.uid);
 };
 
 Groups.update = async function (socket, data) {

src/socket.io/groups.js

@@ -1,13 +1,10 @@
 'use strict';
 
-const validator = require('validator');
 const groups = require('../groups');
 const user = require('../user');
 const utils = require('../utils');
-const slugify = require('../slugify');
 const events = require('../events');
 const api = require('../api');
-const notifications = require('../notifications');
 const sockets = require('.');
 
 const SocketGroups = module.exports;
@@ -26,38 +23,8 @@ SocketGroups.join = async (socket, data) => {
 
 SocketGroups.leave = async (socket, data) => {
 	sockets.warnDeprecated(socket, 'DELETE /api/v3/groups/:slug/membership/:uid');
-
+	const slug = await groups.getGroupField(data.groupName, 'slug');
-	if (socket.uid <= 0) {
+	await api.groups.leave(socket, { slug: slug, uid: data.uid || socket.uid });
-		throw new Error('[[error:invalid-uid]]');
-	}
-
-	if (typeof data.groupName !== 'string') {
-		throw new Error('[[error:invalid-group-name]]');
-	}
-
-	if (data.groupName === 'administrators') {
-		throw new Error('[[error:cant-remove-self-as-admin]]');
-	}
-
-	const groupData = await groups.getGroupData(data.groupName);
-	if (groupData.disableLeave) {
-		throw new Error('[[error:group-leave-disabled]]');
-	}
-
-	await groups.leave(data.groupName, socket.uid);
-	const username = await user.getUserField(socket.uid, 'username');
-	const notification = await notifications.create({
-		type: 'group-leave',
-		bodyShort: '[[groups:membership.leave.notification_title, ' + username + ', ' + data.groupName + ']]',
-		nid: 'group:' + validator.escape(data.groupName) + ':uid:' + socket.uid + ':group-leave',
-		path: '/groups/' + slugify(data.groupName),
-	});
-	const uids = await groups.getOwners(data.groupName);
-	await notifications.push(notification, uids);
-
-	logGroupEvent(socket, 'group-leave', {
-		groupName: data.groupName,
-	});
 };
 
 SocketGroups.addMember = async (socket, data) => {

test/groups.js

@@ -1320,9 +1320,9 @@ describe('Groups', function () {
 			});
 		});
 
-		it('should fail to if user is already member', function (done) {
+		it('should not error if user is already member', function (done) {
 			socketGroups.join({ uid: adminUid }, { uid: testUid, groupName: 'newgroup' }, function (err) {
-				assert.equal(err.message, '[[error:group-already-member]]');
+				assert.ifError(err);
 				done();
 			});
 		});
@@ -1341,9 +1341,16 @@ describe('Groups', function () {
 			});
 		});
 
-		it('should fail if user is not member', function (done) {
+		it('should not error if user is not member', function (done) {
 			socketGroups.leave({ uid: adminUid }, { uid: 3, groupName: 'newgroup' }, function (err) {
-				assert.equal(err.message, '[[error:group-not-member]]');
+				assert.ifError(err);
+				done();
+			});
+		});
+
+		it('should fail if trying to remove someone else from group', function (done) {
+			socketGroups.leave({ uid: testUid }, { uid: adminUid, groupName: 'newgroup' }, function (err) {
+				assert.strictEqual(err.message, '[[error:no-privileges]]');
 				done();
 			});
 		});

test/socket.io.js

@@ -5,6 +5,8 @@
 
 process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
 
+const util = require('util');
+const sleep = util.promisify(setTimeout);
 var assert = require('assert');
 var async = require('async');
 var nconf = require('nconf');
@@ -191,38 +193,35 @@ describe('socket.io', function () {
 	});
 
 	describe('user create/delete', function () {
-		var uid;
+		let uid;
-		it('should create a user', function (done) {
+		it('should create a user', async function () {
-			socketAdmin.user.createUser({ uid: adminUid }, { username: 'foo1' }, function (err, _uid) {
+			const userData = await socketAdmin.user.createUser({ uid: adminUid }, { username: 'foo1' });
-				assert.ifError(err);
+			uid = userData.uid;
-				uid = _uid;
+			const isMember = await groups.isMember(userData.uid, 'registered-users');
-				groups.isMember(uid, 'registered-users', function (err, isMember) {
+			assert(isMember);
-					assert.ifError(err);
-					assert(isMember);
-					done();
-				});
-			});
 		});
 
-		it('should delete users', function (done) {
+		it('should delete users', async function () {
-			socketAdmin.user.deleteUsers({ uid: adminUid }, [uid], function (err) {
+			await socketAdmin.user.deleteUsers({ uid: adminUid }, [uid]);
-				assert.ifError(err);
+			await sleep(500);
-				setTimeout(function () {
+			const isMember = await groups.isMember(uid, 'registered-users');
-					groups.isMember(uid, 'registered-users', function (err, isMember) {
+			assert(!isMember);
-						assert.ifError(err);
-						assert(!isMember);
-						done();
-					});
-				}, 500);
-			});
 		});
 
-		it('should delete users and their content', function (done) {
+		it('should error if user does not exist', function (done) {
 			socketAdmin.user.deleteUsersAndContent({ uid: adminUid }, [uid], function (err) {
-				assert.ifError(err);
+				assert.strictEqual(err.message, '[[error:no-user]]');
 				done();
 			});
 		});
+
+		it('should delete users and their content', async function () {
+			const userData = await socketAdmin.user.createUser({ uid: adminUid }, { username: 'foo2' });
+			await socketAdmin.user.deleteUsersAndContent({ uid: adminUid }, [userData.uid]);
+			await sleep(500);
+			const isMember = await groups.isMember(userData.uid, 'registered-users');
+			assert(!isMember);
+		});
 	});
 
 	it('should error with invalid data', function (done) {