Skip to content

Commit

Permalink
feat: more discrete commit-on-save instead of commit-on-change w/ con…
Browse files Browse the repository at this point in the history 
…firm 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
  • Loading branch information
@julianlam
julianlam committed Aug 4, 2020
1 parent 0f10e08 commit a716a55
Show file tree
Hide file tree
Showing 15 changed files with 213 additions and 109 deletions.
1 change: 1 addition & 0 deletions public/.eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"object-shorthand": "off",
"prefer-arrow-callback": "off",
"prefer-spread": "off",
"prefer-object-spread": "off",
"prefer-reflect": "off",
"prefer-template": "off"
},
Expand Down
1 change: 0 additions & 1 deletion public/language/en-GB/admin/manage/categories.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!",
Expand Down
14 changes: 12 additions & 2 deletions public/language/en-GB/admin/manage/privileges.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"global": "Global",
"global.no-users": "No user-specific global privileges.",
"admin": "Admin",
"group-privileges": "Group Privileges",
"user-privileges": "User Privileges",
Expand Down Expand Up @@ -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>"
}
29 changes: 20 additions & 9 deletions public/less/admin/manage/privileges.less
View file
Original file line number Diff line number Diff line change
@@ -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;}
Expand All @@ -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;
}
}
}
}
11 changes: 11 additions & 0 deletions public/openapi/read.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
159 changes: 113 additions & 46 deletions public/src/admin/manage/privileges.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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();
};

Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -123,20 +176,24 @@ 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,
set: state,
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 () {
Expand All @@ -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');
});
});
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion public/src/modules/helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('');
}

Expand Down
12 changes: 10 additions & 2 deletions src/privileges/admin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit a716a55

Please sign in to comment.