final refactoring pass for groups -- #1252

public/src/forum/admin/categories.js

@@ -327,7 +327,7 @@ define(['uploader'], function(uploader) {
 			for(var x = 0; x < numResults; x++) {
 				resultObj = results[x];
 				trEl = $('<tr />')
-					.attr('data-gid', resultObj.gid)
+					.attr('data-name', resultObj.name)
 					.html('<td><h4>' + resultObj.name + '</h4></td>' +
 						'<td>' +
 						'<div class="btn-group pull-right">' +
@@ -341,13 +341,13 @@ define(['uploader'], function(uploader) {
 
 		groupsResultsEl.off().on('click', '[data-gpriv]', function(e) {
 			var	btnEl = $(this),
-				gid = btnEl.parents('tr[data-gid]').attr('data-gid'),
+				name = btnEl.parents('tr[data-name]').attr('data-name'),
 				privilege = btnEl.attr('data-gpriv');
 			e.preventDefault();
 
 			socket.emit('admin.categories.setGroupPrivilege', {
 				cid: cid,
-				gid: gid,
+				name: name,
 				privilege: privilege,
 				set: !btnEl.hasClass('active')
 			}, function(err) {

src/categoryTools.js

@@ -17,10 +17,11 @@ CategoryTools.privileges = function(cid, uid, callback) {
 		"+r": function(next) {
 			var	key = 'cid:' + cid + ':privileges:+r';
 			Groups.exists(key, function(err, exists) {
+				console.log(key, exists);
 				if (exists) {
 					Groups.isMember(uid, key, next);
 				} else {
-					next(null, true);
+					next(null, null);
 				}
 			});
 		},
@@ -30,7 +31,7 @@ CategoryTools.privileges = function(cid, uid, callback) {
 				if (exists) {
 					Groups.isMember(uid, key, next);
 				} else {
-					next(null, true);
+					next(null, null);
 				}
 			});
 		},
@@ -40,7 +41,7 @@ CategoryTools.privileges = function(cid, uid, callback) {
 				if (exists) {
 					Groups.isMemberOfGroupList(uid, key, next);
 				} else {
-					next(null, true);
+					next(null, null);
 				}
 			});
 		},
@@ -50,7 +51,7 @@ CategoryTools.privileges = function(cid, uid, callback) {
 				if (exists) {
 					Groups.isMemberOfGroupList(uid, key, next);
 				} else {
-					next(null, true);
+					next(null, null);
 				}
 			});
 		},
@@ -67,12 +68,20 @@ CategoryTools.privileges = function(cid, uid, callback) {
 			"g+r": privileges['g+r'],
 			"g+w": privileges['g+w'],
 			read: (
-				privileges['+r'] || privileges['g+r'] ||
+				(
-				privileges.moderator ||	privileges.admin
+					(privileges['+r'] || privileges['+r'] === null) &&
+					(privileges['g+r'] || privileges['g+r'] === null)
+				) ||
+				privileges.moderator ||
+				privileges.admin
 			),
 			write: (
-				privileges['+w'] || privileges['g+w'] ||
+				(
-				privileges.moderator ||	privileges.admin
+					(privileges['+w'] || privileges['+w'] === null) &&
+					(privileges['g+w'] || privileges['g+w'] === null)
+				) ||
+				privileges.moderator ||
+				privileges.admin
 			),
 			editable: privileges.moderator || privileges.admin,
 			view_deleted: privileges.moderator || privileges.admin,
@@ -82,13 +91,13 @@ CategoryTools.privileges = function(cid, uid, callback) {
 	});
 };
 
-CategoryTools.groupPrivileges = function(cid, gid, callback) {
+CategoryTools.groupPrivileges = function(cid, groupName, callback) {
 	async.parallel({
 		"g+r": function(next) {
 			var	key = 'cid:' + cid + ':privileges:g+r';
 			Groups.exists(key, function(err, exists) {
 				if (exists) {
-					Groups.isMember(gid, key, next);
+					Groups.isMember(groupName, key, next);
 				} else {
 					next(null, false);
 				}
@@ -98,7 +107,7 @@ CategoryTools.groupPrivileges = function(cid, gid, callback) {
 			var	key = 'cid:' + cid + ':privileges:g+w';
 			Groups.exists(key, function(err, exists) {
 				if (exists) {
-					Groups.isMember(gid, key, next);
+					Groups.isMember(groupName, key, next);
 				} else {
 					next(null, false);
 				}

src/controllers/index.js

@@ -60,6 +60,7 @@ Controllers.home = function(req, res, next) {
 
 				function canSee(category, next) {
 					categoryTools.privileges(category.cid, ((req.user) ? req.user.uid || 0 : 0), function(err, privileges) {
+						console.log(category.cid, privileges);
 						next(!err && privileges.read);
 					});
 				}

src/groups.js

@@ -1,11 +1,6 @@
 'use strict';
 
 (function(Groups) {
-
-	/*  REMOVED
-		group lists need to be updated to contain... groups!
-	*/
-
 	var async = require('async'),
 		winston = require('winston'),
 		user = require('./user'),
@@ -42,7 +37,15 @@
 
 		async.parallel({
 			base: function (next) {
-				db.getObject('group:' + groupName, next);
+				db.getObject('group:' + groupName, function(err, groupObj) {
+					if (err) {
+						next(err);
+					} else if (!groupObj) {
+						next('group-not-found');
+					} else {
+						next(err, groupObj);
+					}
+				});
 			},
 			users: function (next) {
 				db.getSetMembers('group:' + groupName + ':members', function (err, uids) {
@@ -70,7 +73,7 @@
 				return callback(err);
 			}
 
-			results.base.count = results.users.length;
+			results.base.count = numUsers || results.users.length;
 			results.base.members = results.users;
 			results.base.memberCount = numUsers || results.users.length;
 
@@ -127,9 +130,16 @@
 					system: system ? '1' : '0'
 				};
 
-				db.setObject('group:' + name, groupData, function(err) {
+				async.parallel([
-					Groups.get(name, {}, callback);
+					function(next) {
-				});
+						db.setAdd('groups', name, next);
+					},
+					function(next) {
+						db.setObject('group:' + name, groupData, function(err) {
+							Groups.get(name, {}, next);
+						});
+					}
+				], callback);
 			} else {
 				callback(new Error('group-exists'));
 			}
@@ -146,24 +156,12 @@
 		db.exists('group:' + groupName, function (err, exists) {
 			if (!err && exists) {
 				// If the group was renamed, check for dupes
-				if (values.name) {
+				if (!values.name) {
-					Groups.exists(values.name, function(err, exists) {
-						if (!exists) {
-							db.rename('group:' + groupName, 'group:' + values.name, function(err) {
-								if (err) {
-									return callback(new Error('could-not-rename-group'));
-								}
-
-								db.setRemove('groups', groupName);
-								db.setAdd('groups', values.name);
-								db.setObject('group:' + values.name, values, callback);
-							});
-						} else {
-							callback(new Error('group-exists'));
-						}
-					});
-				} else {
 					db.setObject('group:' + groupName, values, callback);
+				} else {
+					if (callback) {
+						callback(new Error('name-change-not-allowed'));
+					}
 				}
 			} else {
 				if (callback) {
@@ -185,7 +183,21 @@
 	};
 
 	Groups.join = function(groupName, uid, callback) {
-		db.setAdd('group:' + groupName + ':members', uid, callback);
+		Groups.exists(groupName, function(err, exists) {
+			if (exists) {
+				db.setAdd('group:' + groupName + ':members', uid, callback);
+			} else {
+				Groups.create(groupName, '', function(err) {
+					if (err) {
+						winston.error('[groups.join] Could not create new hidden group: ' + err.message);
+						return callback(err);
+					}
+
+					Groups.hide(groupName);
+					db.setAdd('group:' + groupName + ':members', uid, callback);
+				});
+			}
+		});
 	};
 
 	Groups.leave = function(groupName, uid, callback) {
@@ -194,13 +206,13 @@
 				return callback(err);
 			}
 
-			// If this is a system group, and it is now empty, delete it
+			// If this is a hidden group, and it is now empty, delete it
-			Groups.get(groupName, function(err, group) {
+			Groups.get(groupName, {}, function(err, group) {
 				if (err) {
 					return callback(err);
 				}
 
-				if (group.system && group.memberCount === 0) {
+				if (group.hidden && group.memberCount === 0) {
 					Groups.destroy(groupName, callback);
 				} else {
 					return callback();
@@ -219,7 +231,7 @@
 						next();
 					}
 				});
-			});
+			}, callback);
 		});
 	};
 }(module.exports));

src/socket.io/admin.js

@@ -259,9 +259,9 @@ SocketAdmin.categories.setGroupPrivilege = function(socket, data, callback) {
 	}
 
 	if (data.set) {
-		groups.join('cid:' + data.cid + ':privileges:' + data.privilege, data.gid, callback);
+		groups.join('cid:' + data.cid + ':privileges:' + data.privilege, data.name, callback);
 	} else {
-		groups.leave('cid:' + data.cid + ':privileges:' + data.privilege, data.gid, callback);
+		groups.leave('cid:' + data.cid + ':privileges:' + data.privilege, data.name, callback);
 	}
 };
 
@@ -275,7 +275,7 @@ SocketAdmin.categories.groupsList = function(socket, cid, callback) {
 		}
 
 		async.map(data, function(groupObj, next) {
-			CategoryTools.groupPrivileges(cid, groupObj.gid, function(err, privileges) {
+			CategoryTools.groupPrivileges(cid, groupObj.name, function(err, privileges) {
 				if(err) {
 					return next(err);
 				}

src/upgrade.js

@@ -348,7 +348,16 @@ Upgrade.upgrade = function(callback) {
 						return next();
 					}
 
-					var	names = Object.keys(mapping);
+					var	names = Object.keys(mapping),
+						reverseMapping = {},
+						isGroupList = /^cid:[0-9]+:privileges:g\+[rw]$/;
+
+					for(var groupName in mapping) {
+						if (mapping.hasOwnProperty(groupName)) {
+							reverseMapping[parseInt(mapping[groupName], 10)] = groupName;
+						}
+					}
+
 					async.each(names, function(name, next) {
 						async.series([
 							function(next) {
@@ -356,9 +365,11 @@ Upgrade.upgrade = function(callback) {
 								db.deleteObjectField('gid:' + mapping[name], 'gid', next);
 							},
 							function(next) {
+								// Rename gid hash to groupName hash
 								db.rename('gid:' + mapping[name], 'group:' + name, next);
 							},
 							function(next) {
+								// Move member lists over
 								db.exists('gid:' + mapping[name] + ':members', function(err, exists) {
 									if (err) {
 										return next(err);
@@ -367,18 +378,32 @@ Upgrade.upgrade = function(callback) {
 									if (exists) {
 										db.rename('gid:' + mapping[name] + ':members', 'group:' + name + ':members', next);
 									} else {
-										// No members, do nothing
+										// No members, do nothing, they'll be removed later
 										next();
 									}
 								});
 							},
 							function(next) {
-								// Add groups to a directory (set)
+								// Add group to the directory (set)
 								db.setAdd('groups', name, next);
+							},
+							function(next) {
+								// If this group contained gids, map the gids to group names
+								if (isGroupList.test(name)) {
+									db.getSetMembers('group:' + name + ':members', function(err, gids) {
+										async.each(gids, function(gid, next) {
+											db.setRemove('group:' + name + ':members', gid);
+											db.setAdd('group:' + name + ':members', reverseMapping[gid], next);
+										}, next);
+									});
+								} else {
+									next();
+								}
 							}
 						], next);
 					}, function(err) {
 						// Clean-up
+						var	isValidHiddenGroup = /^cid:[0-9]+:privileges:(g)?\+[rw]$/;
 						async.series([
 							function(next) {
 								// Mapping
@@ -390,16 +415,28 @@ Upgrade.upgrade = function(callback) {
 							},
 							function(next) {
 								// Set 'administrators' and 'registered-users' as system groups
-								db.setObjectField('group:administrators', 'system', '1');
+								async.parallel([
-								db.setObjectField('group:registered-users', 'system', '1');
+									function(next) {
-								next();
+										db.setObject('group:administrators', {
+											system: '1',
+											hidden: '0'
+										}, next);
+									},
+									function(next) {
+										db.setObject('group:registered-users', {
+											system: '1',
+											hidden: '0'
+										}, next);
+									}
+								], next);
 							},
 							function(next) {
-								// Delete empty groups
 								Groups.list({ showAllGroups: true }, function(err, groups) {
 									async.each(groups, function(group, next) {
-										if (group.members.length === 0) {
+										// If empty, delete group
-											// Delete the group
+										if (group.memberCount === 0) {
+											Groups.destroy(group.name, next);
+										} else if (group.hidden && !isValidHiddenGroup.test(group.name)) {
 											Groups.destroy(group.name, next);
 										} else {
 											next();
@@ -408,8 +445,13 @@ Upgrade.upgrade = function(callback) {
 								});
 							}
 						], function(err) {
-							console.log('so far so good');
+							if (err) {
-							process.exit();
+								winston.error('[2014/3/19] Problem removing gids and pruning groups.');
+								next();
+							} else {
+								winston.info('[2014/3/19] Removing gids and pruning groups');
+								Upgrade.update(thisSchemaDate, next);
+							}
 						});
 					});
 				});