feat: more discrete commit-on-save instead of commit-on-change w/ confirm modals (#8541)

* feat: privileges save button, #8537, WIP

* fix: disable firefox autocomplete on privilege form fields

* feat: closes #8537 privilege changes commit on save

- new language strings for confirmation and success modals/toasts
- indeterminate privilege handling (/cc @psychobunny)
- added new discard button
- both discard and save buttons now have confirmation dialogs

* fix(tests): remove duplicate template helper test

* fix(tests): broken template helper test

* feat: confirm dialogs for all privilege copy actions

Also, ability to add user to a privilege table without needing
to refresh the privilege table.

* feat: group row addition w/o table refresh

breaking: helpers.getUserPrivileges and helpers.getGroupPrivileges
no longer make socket calls to the following hooks:

- filter:privileges.list, filter:privileges.admin.list,
  filter:privileges.global.list, filter:privileges.groups.list,
  filter:privileges.admin.groups.list,
  filter:privileges.gloval.groups.list

The filters are still called, but done before the helper method
is called, and the results are passed in instead. This change
should only affect you if you directly call the helper methods,
otherwise the change is transparent.

* fix: stale ajaxify data on privilege category switch

* fix: implicit privileges not showing for user privs

* fix: groups, not group, also fix tests

* fix(tests): again

* fix: wrong tpl rendered when adding group to global priv table
v1.18.x
Julian Lam 5 years ago committed by GitHub
parent 0f10e0836b
commit a716a5529c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -27,6 +27,7 @@
"object-shorthand": "off",
"prefer-arrow-callback": "off",
"prefer-spread": "off",
"prefer-object-spread": "off",
"prefer-reflect": "off",
"prefer-template": "off"
},

@ -66,7 +66,6 @@
"alert.create-success": "Category successfully created!",
"alert.none-active": "You have no active categories.",
"alert.create": "Create a Category",
"alert.confirm-moderate": "<strong>Are you sure you wish to grant the moderation privilege to this user group?</strong> This group is public, and any users can join at will.",
"alert.confirm-purge": "<p class=\"lead\">Do you really want to purge this category \"%1\"?</p><h5><strong class=\"text-danger\">Warning!</strong> All topics and posts in this category will be purged!</h5> <p class=\"help-block\">Purging a category will remove all topics and posts, and delete the category from the database. If you want to remove a category <em>temporarily</em>, you'll want to \"disable\" the category instead.</p>",
"alert.purge-success": "Category purged!",
"alert.copy-success": "Settings Copied!",

@ -1,6 +1,5 @@
{
"global": "Global",
"global.no-users": "No user-specific global privileges.",
"admin": "Admin",
"group-privileges": "Group Privileges",
"user-privileges": "User Privileges",
@ -38,5 +37,16 @@
"admin-categories": "Categories",
"admin-privileges": "Privileges",
"admin-users": "Users",
"admin-settings": "Settings"
"admin-settings": "Settings",
"alert.confirm-moderate": "<strong>Are you sure you wish to grant the moderation privilege to this user group?</strong> This group is public, and any users can join at will.",
"alert.confirm-save": "Please confirm your intention to save these privileges",
"alert.saved": "Privilege changes saved and applied",
"alert.confirm-discard": "Are you sure you wish to discard your privilege changes?",
"alert.discarded": "Privilege changes discarded",
"alert.confirm-copyToAll": "Are you sure you wish to apply this privilege set to <strong>all categories</strong>?",
"alert.confirm-copyToAllGroup": "Are you sure you wish to apply this group's privilege set to <strong>all categories</strong>?",
"alert.confirm-copyToChildren": "Are you sure you wish to apply this privilege set to <strong>all descendant (child) categories</strong>?",
"alert.confirm-copyToChildrenGroup": "Are you sure you wish to apply this group's privilege set to <strong>all descendant (child) categories</strong>?",
"alert.no-undo": "<em>This action cannot be undone.</em>"
}

@ -1,11 +1,3 @@
.page-manage-privileges {
.ui-autocomplete {
height: 500px;
overflow-y: auto;
overflow-x: hidden;
}
}
.page-admin-privileges {
@keyframes fadeOut {
0% {background-color: @brand-primary;}
@ -16,6 +8,25 @@
animation-name: fadeOut;
animation-duration: 5s;
animation-fill-mode: both;
animation-timing-function: ease-out;
animation-timing-function: ease-out;
}
.privilege-table {
td[data-delta="true"] > input {
&:after {
border-color: @state-success-text;
background-color: @state-success-text;
}
}
td[data-delta="false"] > input {
&:after {
border-color: @state-danger-bg;
}
&:indeterminate:after {
background-color: @state-danger-bg;
}
}
}
}

@ -847,6 +847,17 @@ paths:
properties:
name:
type: string
keys:
type: object
properties:
users:
type: array
items:
type: string
groups:
type: array
items:
type: string
users:
type: array
items:

@ -28,38 +28,72 @@ define('admin/manage/privileges', [
Privileges.setupPrivilegeTable = function () {
$('.privilege-table-container').on('change', 'input[type="checkbox"]', function () {
var checkboxEl = $(this);
var privilege = checkboxEl.parent().attr('data-privilege');
var wrapperEl = checkboxEl.parent();
var privilege = wrapperEl.attr('data-privilege');
var state = checkboxEl.prop('checked');
var rowEl = checkboxEl.parents('tr');
var member = rowEl.attr('data-group-name') || rowEl.attr('data-uid');
var isPrivate = parseInt(rowEl.attr('data-private') || 0, 10);
var isGroup = rowEl.attr('data-group-name') !== undefined;
var delta = checkboxEl.prop('checked') === (wrapperEl.attr('data-value') === 'true') ? null : state;
if (member) {
if (isGroup && privilege === 'groups:moderate' && !isPrivate && state) {
bootbox.confirm('[[admin/manage/categories:alert.confirm-moderate]]', function (confirm) {
bootbox.confirm('[[admin/manage/privileges:alert.confirm-moderate]]', function (confirm) {
if (confirm) {
Privileges.setPrivilege(member, privilege, state, checkboxEl);
wrapperEl.attr('data-delta', delta);
Privileges.exposeAssumedPrivileges();
} else {
checkboxEl.prop('checked', !checkboxEl.prop('checked'));
}
});
} else {
Privileges.setPrivilege(member, privilege, state, checkboxEl);
wrapperEl.attr('data-delta', delta);
Privileges.exposeAssumedPrivileges();
}
} else {
app.alertError('[[error:invalid-data]]');
}
});
document.getElementById('save').addEventListener('click', function () {
bootbox.confirm('[[admin/manage/privileges:alert.confirm-save]]', function (ok) {
if (ok) {
var tableEl = document.querySelector('.privilege-table-container');
var requests = tableEl.querySelectorAll('td[data-delta]').forEach(function (el) {
var privilege = el.getAttribute('data-privilege');
var rowEl = el.parentNode;
var member = rowEl.getAttribute('data-group-name') || rowEl.getAttribute('data-uid');
var state = el.getAttribute('data-delta') === 'true' ? 1 : 0;
Privileges.setPrivilege(member, privilege, state);
});
$.when(requests).done(function () {
Privileges.refreshPrivilegeTable();
app.alertSuccess('[[admin/manage/privileges:alert.saved]]');
});
}
});
});
document.getElementById('discard').addEventListener('click', function () {
bootbox.confirm('[[admin/manage/privileges:alert.confirm-discard]]', function (ok) {
if (ok) {
Privileges.refreshPrivilegeTable();
app.alertSuccess('[[admin/manage/privileges:alert.discarded]]');
}
});
});
$('.privilege-table-container').on('click', '[data-action="search.user"]', Privileges.addUserToPrivilegeTable);
$('.privilege-table-container').on('click', '[data-action="search.group"]', Privileges.addGroupToPrivilegeTable);
$('.privilege-table-container').on('click', '[data-action="copyToChildren"]', function () {
Privileges.copyPrivilegesToChildren(cid, '');
throwConfirmModal('copyToChildren', Privileges.copyPrivilegesToChildren.bind(null, cid, ''));
});
$('.privilege-table-container').on('click', '[data-action="copyToChildrenGroup"]', function () {
var groupName = $(this).parents('[data-group-name]').attr('data-group-name');
Privileges.copyPrivilegesToChildren(cid, groupName);
throwConfirmModal('copyToChildrenGroup', Privileges.copyPrivilegesToChildren.bind(null, cid, groupName));
});
$('.privilege-table-container').on('click', '[data-action="copyPrivilegesFrom"]', function () {
@ -71,13 +105,21 @@ define('admin/manage/privileges', [
});
$('.privilege-table-container').on('click', '[data-action="copyToAll"]', function () {
Privileges.copyPrivilegesToAllCategories(cid, '');
throwConfirmModal('copyToAll', Privileges.copyPrivilegesToAllCategories.bind(null, cid, ''));
});
$('.privilege-table-container').on('click', '[data-action="copyToAllGroup"]', function () {
var groupName = $(this).parents('[data-group-name]').attr('data-group-name');
Privileges.copyPrivilegesToAllCategories(cid, groupName);
throwConfirmModal('copyToAllGroup', Privileges.copyPrivilegesToAllCategories.bind(null, cid, groupName));
});
function throwConfirmModal(method, onConfirm) {
bootbox.confirm('[[admin/manage/privileges:alert.confirm-' + method + ']]<br /><br />[[admin/manage/privileges:alert.no-undo]]', function (ok) {
if (ok) {
onConfirm.call();
}
});
}
Privileges.exposeAssumedPrivileges();
};
@ -87,6 +129,7 @@ define('admin/manage/privileges', [
return app.alertError(err.message);
}
ajaxify.data.privileges = privileges;
var tpl = parseInt(cid, 10) ? 'admin/partials/privileges/category' : 'admin/partials/privileges/global';
Benchpress.parse(tpl, {
privileges: privileges,
@ -113,8 +156,18 @@ define('admin/manage/privileges', [
privs.push(el.getAttribute('data-privilege'));
}
});
// Also apply to non-group privileges
privs = privs.concat(privs.map(function (priv) {
if (priv.startsWith('groups:')) {
return priv.slice(7);
}
return false;
})).filter(Boolean);
for (var x = 0, numPrivs = privs.length; x < numPrivs; x += 1) {
var inputs = $('.privilege-table tr[data-group-name]:not([data-group-name="registered-users"],[data-group-name="guests"],[data-group-name="spiders"]) td[data-privilege="' + privs[x] + '"] input');
var inputs = $('.privilege-table tr[data-group-name]:not([data-group-name="registered-users"],[data-group-name="guests"],[data-group-name="spiders"]) td[data-privilege="' + privs[x] + '"] input, .privilege-table tr[data-uid] td[data-privilege="' + privs[x] + '"] input');
inputs.each(function (idx, el) {
if (!el.checked) {
el.indeterminate = true;
@ -123,7 +176,9 @@ define('admin/manage/privileges', [
}
};
Privileges.setPrivilege = function (member, privilege, state, checkboxEl) {
Privileges.setPrivilege = function (member, privilege, state) {
var deferred = $.Deferred();
socket.emit('admin.categories.setPrivilege', {
cid: isNaN(cid) ? 0 : cid,
privilege: privilege,
@ -131,12 +186,14 @@ define('admin/manage/privileges', [
member: member,
}, function (err) {
if (err) {
deferred.reject(err);
return app.alertError(err.message);
}
checkboxEl.replaceWith('<i class="fa fa-spin fa-spinner"></i>');
Privileges.refreshPrivilegeTable();
deferred.resolve();
});
return deferred.promise();
};
Privileges.addUserToPrivilegeTable = function () {
@ -151,23 +208,29 @@ define('admin/manage/privileges', [
inputEl.focus();
autocomplete.user(inputEl, function (ev, ui) {
var defaultPrivileges;
if (ajaxify.data.url === '/admin/manage/privileges/admin') {
defaultPrivileges = ['admin:dashboard'];
} else {
defaultPrivileges = cid ? ['find', 'read', 'topics:read'] : ['chat'];
}
socket.emit('admin.categories.setPrivilege', {
cid: isNaN(cid) ? 0 : cid,
privilege: defaultPrivileges,
set: true,
member: ui.item.user.uid,
}, function (err) {
if (err) {
return app.alertError(err.message);
}
Privileges.refreshPrivilegeTable();
// Generate data for new row
var privilegeSet = ajaxify.data.privileges.keys.users.reduce(function (memo, cur) {
memo[cur] = false;
return memo;
}, {});
app.parseAndTranslate('admin/partials/privileges/' + (isNaN(cid) ? 'global' : 'category'), 'privileges.users', {
privileges: {
users: [
{
picture: ui.item.user.picture,
username: ui.item.user.username,
uid: ui.item.user.uid,
'icon:text': ui.item.user['icon:text'],
'icon:bgColor': ui.item.user['icon:bgColor'],
privileges: privilegeSet,
},
],
},
}, function (html) {
var tableEl = document.querySelectorAll('.privilege-table');
var rows = tableEl[1].querySelectorAll('tbody tr');
html.insertBefore(rows[rows.length - 1]);
modal.modal('hide');
});
});
@ -245,24 +308,28 @@ define('admin/manage/privileges', [
}
function addGroupToCategory(group, cb) {
var defaultPrivileges;
if (ajaxify.data.url === '/admin/manage/privileges/admin') {
defaultPrivileges = ['groups:admin:dashboard'];
} else {
defaultPrivileges = cid ? ['groups:find', 'groups:read', 'groups:topics:read'] : ['groups:chat'];
}
socket.emit('admin.categories.setPrivilege', {
cid: isNaN(cid) ? 0 : cid,
privilege: defaultPrivileges,
set: true,
member: group,
}, function (err) {
if (err) {
return app.alertError(err.message);
}
// Generate data for new row
var privilegeSet = ajaxify.data.privileges.keys.groups.reduce(function (memo, cur) {
memo[cur] = false;
return memo;
}, {});
app.parseAndTranslate('admin/partials/privileges/' + ((isNaN(cid) || cid === 0) ? 'global' : 'category'), 'privileges.groups', {
privileges: {
groups: [
{
name: group,
nameEscaped: translator.escape(group),
privileges: privilegeSet,
},
],
},
}, function (html) {
var tableEl = document.querySelector('.privilege-table');
var rows = tableEl.querySelectorAll('tbody tr');
html.insertBefore(rows[rows.length - 1]);
Privileges.exposeAssumedPrivileges();
Privileges.refreshPrivilegeTable(group);
if (typeof cb === 'function') {
cb();
}

@ -193,7 +193,7 @@
(member === 'spiders' && !spidersEnabled.includes(priv.name)) ||
(member === 'Global Moderators' && globalModDisabled.includes(priv.name));
return '<td class="text-center" data-privilege="' + priv.name + '"><input type="checkbox"' + (priv.state ? ' checked' : '') + (disabled ? ' disabled="disabled"' : '') + ' /></td>';
return '<td class="text-center" data-privilege="' + priv.name + '" data-value="' + priv.state + '"><input autocomplete="off" type="checkbox"' + (priv.state ? ' checked' : '') + (disabled ? ' disabled="disabled"' : '') + ' /></td>';
}).join('');
}

@ -117,11 +117,19 @@ module.exports = function (privileges) {
groups: plugins.fireHook('filter:privileges.admin.groups.list_human', privileges.admin.privilegeLabels.slice()),
});
}
const keys = await utils.promiseParallel({
users: plugins.fireHook('filter:privileges.admin.list', privileges.admin.userPrivilegeList.slice()),
groups: plugins.fireHook('filter:privileges.admin.groups.list', privileges.admin.groupPrivilegeList.slice()),
});
const payload = await utils.promiseParallel({
labels: getLabels(),
users: helpers.getUserPrivileges(0, 'filter:privileges.admin.list', privileges.admin.userPrivilegeList),
groups: helpers.getGroupPrivileges(0, 'filter:privileges.admin.groups.list', privileges.admin.groupPrivilegeList),
users: helpers.getUserPrivileges(0, keys.users),
groups: helpers.getGroupPrivileges(0, keys.groups),
});
payload.keys = keys;
// This is a hack because I can't do {labels.users.length} to echo the count in templates.js
payload.columnCount = payload.labels.users.length + 2;
return payload;

@ -22,11 +22,17 @@ module.exports = function (privileges) {
});
}
const keys = await utils.promiseParallel({
users: plugins.fireHook('filter:privileges.list', privileges.userPrivilegeList.slice()),
groups: plugins.fireHook('filter:privileges.groups.list', privileges.groupPrivilegeList.slice()),
});
const payload = await utils.promiseParallel({
labels: getLabels(),
users: helpers.getUserPrivileges(cid, 'filter:privileges.list', privileges.userPrivilegeList),
groups: helpers.getGroupPrivileges(cid, 'filter:privileges.groups.list', privileges.groupPrivilegeList),
users: helpers.getUserPrivileges(cid, keys.users),
groups: helpers.getGroupPrivileges(cid, keys.groups),
});
payload.keys = keys;
// This is a hack because I can't do {labels.users.length} to echo the count in templates.js
payload.columnCountUser = payload.labels.users.length + 2;

@ -55,11 +55,19 @@ module.exports = function (privileges) {
groups: plugins.fireHook('filter:privileges.global.groups.list_human', privileges.global.privilegeLabels.slice()),
});
}
const keys = await utils.promiseParallel({
users: plugins.fireHook('filter:privileges.global.list', privileges.global.userPrivilegeList.slice()),
groups: plugins.fireHook('filter:privileges.global.groups.list', privileges.global.groupPrivilegeList.slice()),
});
const payload = await utils.promiseParallel({
labels: getLabels(),
users: helpers.getUserPrivileges(0, 'filter:privileges.global.list', privileges.global.userPrivilegeList),
groups: helpers.getGroupPrivileges(0, 'filter:privileges.global.groups.list', privileges.global.groupPrivilegeList),
users: helpers.getUserPrivileges(0, keys.users),
groups: helpers.getGroupPrivileges(0, keys.groups),
});
payload.keys = keys;
// This is a hack because I can't do {labels.users.length} to echo the count in templates.js
payload.columnCount = payload.labels.users.length + 2;
return payload;

@ -88,8 +88,7 @@ async function isSystemGroupAllowedToPrivileges(privileges, uid, cid) {
return await groups.isMemberOfGroups(uidToSystemGroup[uid], groupKeys);
}
helpers.getUserPrivileges = async function (cid, hookName, userPrivilegeList) {
const userPrivileges = await plugins.fireHook(hookName, userPrivilegeList.slice());
helpers.getUserPrivileges = async function (cid, userPrivileges) {
let memberSets = await groups.getMembersOfGroups(userPrivileges.map(privilege => 'cid:' + cid + ':privileges:' + privilege));
memberSets = memberSets.map(function (set) {
return set.map(uid => parseInt(uid, 10));
@ -108,8 +107,7 @@ helpers.getUserPrivileges = async function (cid, hookName, userPrivilegeList) {
return memberData;
};
helpers.getGroupPrivileges = async function (cid, hookName, groupPrivilegeList) {
const groupPrivileges = await plugins.fireHook(hookName, groupPrivilegeList.slice());
helpers.getGroupPrivileges = async function (cid, groupPrivileges) {
const [memberSets, allGroupNames] = await Promise.all([
groups.getMembersOfGroups(groupPrivileges.map(privilege => 'cid:' + cid + ':privileges:' + privilege)),
groups.getGroups('groups:createtime', 0, -1),

@ -20,3 +20,13 @@
</div>
</form>
</div>
<div class="floating-button">
<button id="discard" class="mdl-button mdl-js-button mdl-button--fab mdl-js-ripple-effect mdl-button--colored">
<i class="material-icons">delete</i>
</button>
<button id="save" class="mdl-button mdl-js-button mdl-button--fab mdl-js-ripple-effect mdl-button--colored primary">
<i class="material-icons">save</i>
</button>
</div>

@ -52,16 +52,20 @@
<tr>
<td colspan="{privileges.columnCountGroup}">
<div class="btn-toolbar">
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.group">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="search.group">
<i class="fa fa-users"></i>
[[admin/manage/categories:privileges.search-group]]
</button>
<button type="button" class="btn btn-info pull-right" data-ajaxify="false" data-action="copyPrivilegesFrom">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="copyPrivilegesFrom">
<i class="fa fa-copy"></i>
[[admin/manage/categories:privileges.copy-from-category]]
</button>
<button type="button" class="btn btn-info pull-right" data-ajaxify="false" data-action="copyToChildren">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="copyToChildren">
<i class="fa fa-copy"></i>
[[admin/manage/categories:privileges.copy-to-children]]
</button>
<button type="button" class="btn btn-info pull-right" data-ajaxify="false" data-action="copyToAll">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="copyToAll">
<i class="fa fa-copy"></i>
[[admin/manage/categories:privileges.copy-privileges-to-all-categories]]
</button>
</div>
@ -101,7 +105,6 @@
</tr>
</thead>
<tbody>
<!-- IF privileges.users.length -->
<!-- BEGIN privileges.users -->
<tr data-uid="{privileges.users.uid}">
<td>
@ -117,20 +120,11 @@
<!-- END privileges.users -->
<tr>
<td colspan="{privileges.columnCountUser}">
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.user">
[[admin/manage/categories:privileges.search-user]]
</button>
</td>
</tr>
<!-- ELSE -->
<tr>
<td colspan="{privileges.columnCountUser}">
[[admin/manage/categories:privileges.no-users]]
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.user">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="search.user">
<i class="fa fa-user"></i>
[[admin/manage/categories:privileges.search-user]]
</button>
</td>
</tr>
<!-- ENDIF privileges.users.length -->
</tbody>
</table>

@ -24,7 +24,8 @@
<tr>
<td colspan="{privileges.columnCount}">
<div class="btn-toolbar">
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.group">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="search.group">
<i class="fa fa-users"></i>
[[admin/manage/categories:privileges.search-group]]
</button>
</div>
@ -50,7 +51,6 @@
</tr>
</thead>
<tbody>
<!-- IF privileges.users.length -->
<!-- BEGIN privileges.users -->
<tr data-uid="{privileges.users.uid}">
<td>
@ -66,20 +66,11 @@
<!-- END privileges.users -->
<tr>
<td colspan="{privileges.columnCount}">
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.user">
<button type="button" class="btn btn-default pull-right" data-ajaxify="false" data-action="search.user">
<i class="fa fa-user"></i>
[[admin/manage/categories:privileges.search-user]]
</button>
</td>
</tr>
<!-- ELSE -->
<tr>
<td colspan="{privileges.columnCount}">
[[admin/manage/privileges:global.no-users]]
<button type="button" class="btn btn-primary pull-right" data-ajaxify="false" data-action="search.user">
[[admin/manage/categories:privileges.search-user]]
</button>
</td>
</tr>
<!-- ENDIF privileges.users.length -->
</tbody>
</table>

@ -148,17 +148,7 @@ describe('helpers', function () {
read: true,
};
var html = helpers.spawnPrivilegeStates('guests', privs);
assert.equal(html, '<td class="text-center" data-privilege="find"><input type="checkbox" checked /></td><td class="text-center" data-privilege="read"><input type="checkbox" checked /></td>');
done();
});
it('should spawn privilege states', function (done) {
var privs = {
find: true,
read: true,
};
var html = helpers.spawnPrivilegeStates('guests', privs);
assert.equal(html, '<td class="text-center" data-privilege="find"><input type="checkbox" checked /></td><td class="text-center" data-privilege="read"><input type="checkbox" checked /></td>');
assert.equal(html, '<td class="text-center" data-privilege="find" data-value="true"><input autocomplete="off" type="checkbox" checked /></td><td class="text-center" data-privilege="read" data-value="true"><input autocomplete="off" type="checkbox" checked /></td>');
done();
});

Loading…
Cancel
Save