feat(api): group ownership API route, switch client-side to use API route

public/openapi/write.yaml

@@ -74,6 +74,8 @@ paths:
     $ref: 'write/groups/slug.yaml'
   /groups/{slug}/membership/{uid}:
     $ref: 'write/groups/slug/membership/uid.yaml'
+  /groups/{slug}/ownership/{uid}:
+    $ref: 'write/groups/slug/ownership/uid.yaml'
   /categories/:
     $ref: 'write/categories.yaml'
   /categories/{cid}:

public/src/admin/manage/group.js

@@ -39,7 +39,7 @@ define('admin/manage/group', [
 			groupLabelPreview.css('color', changeGroupTextColor.val() || '#ffffff');
 		});
 
-		setupGroupMembersMenu(groupName);
+		setupGroupMembersMenu();
 
 		$('#group-icon, #group-icon-label').on('click', function () {
 			var currentIcon = groupIcon.attr('value');
@@ -96,7 +96,7 @@ define('admin/manage/group', [
 		});
 	};
 
-	function setupGroupMembersMenu(groupName) {
+	function setupGroupMembersMenu() {
 		$('[component="groups/members"]').on('click', '[data-action]', function () {
 			var btnEl = $(this);
 			var userRow = btnEl.parents('[data-uid]');
@@ -107,15 +107,9 @@ define('admin/manage/group', [
 
 			switch (action) {
 				case 'toggleOwnership':
-					socket.emit('groups.' + (isOwner ? 'rescind' : 'grant'), {
-						toUid: uid,
-						groupName: groupName,
-					}, function (err) {
-						if (err) {
-							return app.alertError(err.message);
-						}
+					api[isOwner ? 'del' : 'put'](`/groups/${ajaxify.data.group.slug}/ownership/${uid}`, {}).then(() => {
 						ownerFlagEl.toggleClass('invisible');
-					});
+					}).catch(app.alertError);
 					break;
 
 				case 'kick':

public/src/client/groups/details.js

@@ -64,16 +64,9 @@ define('forum/groups/details', [
 
 			switch (action) {
 				case 'toggleOwnership':
-					socket.emit('groups.' + (isOwner ? 'rescind' : 'grant'), {
-						toUid: uid,
-						groupName: groupName,
-					}, function (err) {
-						if (!err) {
-							ownerFlagEl.toggleClass('invisible');
-						} else {
-							app.alertError(err.message);
-						}
-					});
+					api[isOwner ? 'del' : 'put'](`/groups/${ajaxify.data.group.slug}/ownership/${uid}`, {}).then(() => {
+						ownerFlagEl.toggleClass('invisible');
+					}).catch(app.alertError);
 					break;
 
 				case 'kick':
@@ -83,16 +76,7 @@ define('forum/groups/details', [
 								return;
 							}
 
-							socket.emit('groups.kick', {
-								uid: uid,
-								groupName: groupName,
-							}, function (err) {
-								if (!err) {
-									userRow.slideUp().remove();
-								} else {
-									app.alertError(err.message);
-								}
-							});
+							api.del(`/groups/${ajaxify.data.group.slug}/membership/${uid}`, undefined).then(() => userRow.slideUp().remove()).catch(app.alertError);
 						});
 					});
 					break;
@@ -203,7 +187,7 @@ define('forum/groups/details', [
 				}
 			});
 
-			api.put(`/groups/${slugify(groupName)}`, settings).then(() => {
+			api.put(`/groups/${ajaxify.data.group.slug}`, settings).then(() => {
 				if (settings.name) {
 					var pathname = window.location.pathname;
 					pathname = pathname.substr(1, pathname.lastIndexOf('/'));
@@ -222,7 +206,7 @@ define('forum/groups/details', [
 			if (confirm) {
 				bootbox.prompt('Please enter the name of this group in order to delete it:', function (response) {
 					if (response === groupName) {
-						api.del(`/groups/${slugify(groupName)}`, {}).then(() => {
+						api.del(`/groups/${ajaxify.data.group.slug}`, {}).then(() => {
 							app.alertSuccess('[[groups:event.deleted, ' + utils.escapeHTML(groupName) + ']]');
 							ajaxify.go('groups');
 						}).catch(app.alertError);

src/api/groups.js

@@ -177,6 +177,28 @@ groupsAPI.leave = async function (caller, data) {
 	});
 };
 
+groupsAPI.grant = async (caller, data) => {
+	const groupName = await groups.getGroupNameByGroupSlug(data.slug);
+	await isOwner(caller, groupName);
+
+	await groups.ownership.grant(data.uid, groupName);
+	logGroupEvent(caller, 'group-owner-grant', {
+		groupName: groupName,
+		targetUid: data.uid,
+	});
+};
+
+groupsAPI.rescind = async (caller, data) => {
+	const groupName = await groups.getGroupNameByGroupSlug(data.slug);
+	await isOwner(caller, groupName);
+
+	await groups.ownership.rescind(data.uid, groupName);
+	logGroupEvent(caller, 'group-owner-rescind', {
+		groupName: groupName,
+		targetUid: data.uid,
+	});
+};
+
 async function isOwner(caller, groupName) {
 	if (typeof groupName !== 'string') {
 		throw new Error('[[error:invalid-group-name]]');

src/controllers/write/groups.js

@@ -37,3 +37,13 @@ Groups.leave = async (req, res) => {
 	await api.groups.leave(req, req.params);
 	helpers.formatApiResponse(200, res);
 };
+
+Groups.grant = async (req, res) => {
+	await api.groups.grant(req, req.params);
+	helpers.formatApiResponse(200, res);
+};
+
+Groups.rescind = async (req, res) => {
+	await api.groups.rescind(req, req.params);
+	helpers.formatApiResponse(200, res);
+};

src/groups/ownership.js

@@ -22,16 +22,15 @@ module.exports = function (Groups) {
 	};
 
 	Groups.ownership.grant = async function (toUid, groupName) {
-		// Note: No ownership checking is done here on purpose!
 		await db.setAdd('group:' + groupName + ':owners', toUid);
 		plugins.hooks.fire('action:group.grantOwnership', { uid: toUid, groupName: groupName });
 	};
 
 	Groups.ownership.rescind = async function (toUid, groupName) {
-		// Note: No ownership checking is done here on purpose!
-		// If the owners set only contains one member, error out!
+		// If the owners set only contains one member (and toUid is that member), error out!
 		const numOwners = await db.setCount('group:' + groupName + ':owners');
-		if (numOwners <= 1) {
+		const isOwner = await db.isSortedSetMember(`group:${groupName}:owners`);
+		if (numOwners <= 1 && isOwner) {
 			throw new Error('[[error:group-needs-owner]]');
 		}
 		await db.setRemove('group:' + groupName + ':owners', toUid);

src/routes/write/groups.js

@@ -16,6 +16,8 @@ module.exports = function () {
 	setupApiRoute(router, 'delete', '/:slug', [...middlewares, middleware.assert.group], controllers.write.groups.delete);
 	setupApiRoute(router, 'put', '/:slug/membership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.join);
 	setupApiRoute(router, 'delete', '/:slug/membership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.leave);
+	setupApiRoute(router, 'put', '/:slug/ownership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.grant);
+	setupApiRoute(router, 'delete', '/:slug/ownership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.rescind);
 
 	return router;
 };

src/socket.io/groups.js

@@ -79,6 +79,8 @@ async function isInvited(socket, data) {
 }
 
 SocketGroups.grant = async (socket, data) => {
+	sockets.warnDeprecated(socket, 'PUT /api/v3/groups/:slug/ownership/:uid');
+
 	await isOwner(socket, data);
 	await groups.ownership.grant(data.toUid, data.groupName);
 	logGroupEvent(socket, 'group-owner-grant', {
@@ -88,6 +90,8 @@ SocketGroups.grant = async (socket, data) => {
 };
 
 SocketGroups.rescind = async (socket, data) => {
+	sockets.warnDeprecated(socket, 'DELETE /api/v3/groups/:slug/ownership/:uid');
+
 	await isOwner(socket, data);
 	await groups.ownership.rescind(data.toUid, data.groupName);
 	logGroupEvent(socket, 'group-owner-rescind', {