From 0e44eb8629abe5e4958030a66c44fbae535519da Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 20 Mar 2014 16:26:00 -0400 Subject: [PATCH] final refactoring pass for groups -- #1252 --- public/src/forum/admin/categories.js | 6 +-- src/categoryTools.js | 31 ++++++++---- src/controllers/index.js | 1 + src/groups.js | 76 ++++++++++++++++------------ src/socket.io/admin.js | 6 +-- src/upgrade.js | 64 +++++++++++++++++++---- 6 files changed, 124 insertions(+), 60 deletions(-) diff --git a/public/src/forum/admin/categories.js b/public/src/forum/admin/categories.js index 015e4a12ca..f71526b9e5 100644 --- a/public/src/forum/admin/categories.js +++ b/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 = $('') - .attr('data-gid', resultObj.gid) + .attr('data-name', resultObj.name) .html('

' + resultObj.name + '

' + '' + '
' + @@ -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) { diff --git a/src/categoryTools.js b/src/categoryTools.js index be76b77372..f4f94041ae 100644 --- a/src/categoryTools.js +++ b/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); } diff --git a/src/controllers/index.js b/src/controllers/index.js index 0cf3fde55b..0ae2d674e0 100644 --- a/src/controllers/index.js +++ b/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); }); } diff --git a/src/groups.js b/src/groups.js index 0bc16f1df0..cd9e89c96d 100644 --- a/src/groups.js +++ b/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) { - Groups.get(name, {}, callback); - }); + async.parallel([ + 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) { - 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 { + if (!values.name) { 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 - Groups.get(groupName, function(err, group) { + // If this is a hidden group, and it is now empty, delete it + 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)); diff --git a/src/socket.io/admin.js b/src/socket.io/admin.js index 2be696c957..cb0d11e8c1 100644 --- a/src/socket.io/admin.js +++ b/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); } diff --git a/src/upgrade.js b/src/upgrade.js index f6db77b5e9..10cc6ccf99 100644 --- a/src/upgrade.js +++ b/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'); - db.setObjectField('group:registered-users', 'system', '1'); - next(); + async.parallel([ + function(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) { - // Delete the group + // If empty, delete 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'); - process.exit(); + if (err) { + 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); + } }); }); });