fix: group membership methods for guests/spiders

add tests to cover different combinations

src/groups/membership.js

@@ -25,7 +25,7 @@ module.exports = function (Groups) {
 
 	Groups.isMember = async function (uid, groupName) {
 		if (!uid || parseInt(uid, 10) <= 0 || !groupName) {
-			return false;
+			return isMemberOfEphemeralGroup(uid, groupName);
 		}
 
 		const cacheKey = `${uid}:${groupName}`;
@@ -43,8 +43,8 @@ module.exports = function (Groups) {
 			return uids.map(() => false);
 		}
 
-		if (groupName === 'guests') {
+		if (groupName === 'guests' || groupName === 'spiders') {
-			return uids.map(uid => parseInt(uid, 10) === 0);
+			return uids.map(uid => isMemberOfEphemeralGroup(uid, groupName));
 		}
 
 		const cachedData = {};
@@ -64,7 +64,7 @@ module.exports = function (Groups) {
 
 	Groups.isMemberOfGroups = async function (uid, groups) {
 		if (!uid || parseInt(uid, 10) <= 0 || !groups.length) {
-			return groups.map(groupName => groupName === 'guests');
+			return groups.map(groupName => isMemberOfEphemeralGroup(uid, groupName));
 		}
 		const cachedData = {};
 		const nonCachedGroups = groups.filter(groupName => filterNonCached(cachedData, uid, groupName));
@@ -82,6 +82,11 @@ module.exports = function (Groups) {
 		return groups.map(groupName => cachedData[`${uid}:${groupName}`]);
 	};
 
+	function isMemberOfEphemeralGroup(uid, groupName) {
+		return (groupName === 'guests' && parseInt(uid, 10) === 0) ||
+			(groupName === 'spiders' && parseInt(uid, 10) === -1);
+	}
+
 	function filterNonCached(cachedData, uid, groupName) {
 		const isMember = Groups.cache.get(`${uid}:${groupName}`);
 		const isInCache = isMember !== undefined;

test/groups.js

@@ -209,36 +209,54 @@ describe('Groups', () => {
 	});
 
 	describe('.isMember()', () => {
-		it('should return boolean true when a user is in a group', (done) => {
+		it('should return boolean true when a user is in a group', async () => {
-			Groups.isMember(1, 'Test', (err, isMember) => {
+			const isMember = await Groups.isMember(1, 'Test');
-				assert.ifError(err);
+			assert.strictEqual(isMember, true);
-				assert.strictEqual(isMember, true);
-				done();
-			});
 		});
 
-		it('should return boolean false when a user is not in a group', (done) => {
+		it('should return boolean false when a user is not in a group', async () => {
-			Groups.isMember(2, 'Test', (err, isMember) => {
+			const isMember = await Groups.isMember(2, 'Test');
-				assert.ifError(err);
+			assert.strictEqual(isMember, false);
-				assert.strictEqual(isMember, false);
-				done();
-			});
 		});
 
-		it('should return true for uid 0 and guests group', (done) => {
+		it('should return true for uid 0 and guests group', async () => {
-			Groups.isMembers([1, 0], 'guests', (err, isMembers) => {
+			const isMember = await Groups.isMember(0, 'guests');
-				assert.ifError(err);
+			assert.strictEqual(isMember, true);
-				assert.deepStrictEqual(isMembers, [false, true]);
-				done();
-			});
 		});
 
-		it('should return true for uid 0 and guests group', (done) => {
+		it('should return false for uid 0 and spiders group', async () => {
-			Groups.isMemberOfGroups(0, ['guests', 'registered-users'], (err, isMembers) => {
+			const isMember = await Groups.isMember(0, 'spiders');
-				assert.ifError(err);
+			assert.strictEqual(isMember, false);
-				assert.deepStrictEqual(isMembers, [true, false]);
+		});
-				done();
+
-			});
+		it('should return true for uid -1 and spiders group', async () => {
+			const isMember = await Groups.isMember(-1, 'spiders');
+			assert.strictEqual(isMember, true);
+		});
+
+		it('should return false for uid -1 and guests group', async () => {
+			const isMember = await Groups.isMember(-1, 'guests');
+			assert.strictEqual(isMember, false);
+		});
+
+		it('should return true for uid 0, false for uid -1 with guests group', async () => {
+			const isMembers = await Groups.isMembers([1, 0, -1], 'guests');
+			assert.deepStrictEqual(isMembers, [false, true, false]);
+		});
+
+		it('should return false for uid 0, true for uid -1 with spiders group', async () => {
+			const isMembers = await Groups.isMembers([1, 0, -1], 'spiders');
+			assert.deepStrictEqual(isMembers, [false, false, true]);
+		});
+
+		it('should return true for uid 0 and guests group', async () => {
+			const isMembers = await Groups.isMemberOfGroups(0, ['guests', 'registered-users', 'spiders']);
+			assert.deepStrictEqual(isMembers, [true, false, false]);
+		});
+
+		it('should return true for uid -1 and spiders group', async () => {
+			const isMembers = await Groups.isMemberOfGroups(-1, ['guests', 'registered-users', 'spiders']);
+			assert.deepStrictEqual(isMembers, [false, false, true]);
 		});
 	});
 
@@ -406,16 +424,12 @@ describe('Groups', () => {
 	});
 
 	describe('.hide()', () => {
-		it('should mark the group as hidden', (done) => {
+		it('should mark the group as hidden', async () => {
-			Groups.hide('foo', (err) => {
+			await Groups.hide('foo');
-				assert.ifError(err);
+			const groupObj = await Groups.get('foo', {});
-
+			assert.strictEqual(1, groupObj.hidden);
-				Groups.get('foo', {}, (err, groupObj) => {
+			const isMember = await db.isSortedSetMember('groups:visible:createtime', 'foo');
-					assert.ifError(err);
+			assert.strictEqual(isMember, false);
-					assert.strictEqual(1, groupObj.hidden);
-					done();
-				});
-			});
 		});
 	});
 
@@ -798,20 +812,6 @@ describe('Groups', () => {
 		});
 	});
 
-	describe('.hide()', () => {
-		it('should make a group hidden', (done) => {
-			Groups.hide('Test', function (err) {
-				assert.ifError(err);
-				assert.equal(arguments.length, 1);
-				db.isSortedSetMember('groups:visible:createtime', 'Test', (err, isMember) => {
-					assert.ifError(err);
-					assert.strictEqual(isMember, false);
-					done();
-				});
-			});
-		});
-	});
-
 	describe('socket methods', () => {
 		it('should error if data is null', (done) => {
 			socketGroups.before({ uid: 0 }, 'groups.join', null, (err) => {