From 11cf09b41a3d0d5dff0b5f2b6180c8ab2f85abfb Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Tue, 21 Jan 2020 17:25:27 +0100 Subject: [PATCH 1/3] [ticket/15766] Check for role on changing permission settings PHPBB3-15766 --- phpBB/adm/style/permissions.js | 56 +++++++++++++++++++++++++++++++--- phpBB/adm/style/tooltip.js | 17 ----------- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/phpBB/adm/style/permissions.js b/phpBB/adm/style/permissions.js index af8e21ad51..8bcfd311c6 100644 --- a/phpBB/adm/style/permissions.js +++ b/phpBB/adm/style/permissions.js @@ -278,11 +278,26 @@ function reset_role(id) { return; } - t.options[0].selected = true; + var parent = t.parentNode, + roleId = match_role_settings(id.replace('role', 'perm')), + text = no_role_assigned, + index = 0; - var parent = t.parentNode; - parent.querySelector('span.dropdown-trigger').innerText = no_role_assigned; - parent.querySelector('input[data-name^=role]').value = '0'; + if (roleId) { + for (var i = 0; i < t.options.length; i++) { + if (t.options[i].value == roleId) { + text = t.options[i].text; + index = i; + break; + } + } + } + + t.value = roleId; + t.options[index].selected = true; + + parent.querySelector('span.dropdown-trigger').innerText = text; + parent.querySelector('input[data-name^=role]').value = roleId; } /** @@ -302,3 +317,36 @@ function set_role_settings(role_id, target_id) { mark_one_option(target_id, r, (settings[r] === 1) ? 'y' : 'n'); } } + +function match_role_settings(id) +{ + var fs = document.getElementById(id), + cbs = fs.getElementsByTagName('input'), + xyz = {}; + + for (var i = 0; i < cbs.length; i++) { + var matches = cbs[i].id.match(/setting\[\d+]\[\d+]\[([a-z_]+)]/); + + if (matches !== null && cbs[i].checked && cbs[i].value !== '-1') { + xyz[matches[1]] = parseInt(cbs[i].value); + } + } + + xyz = sort_and_stringify(xyz); + + for (var r in role_options) + { + if (sort_and_stringify(role_options[r]) === xyz) { + return r; + } + } + + return 0; +} + +function sort_and_stringify(obj) { + return JSON.stringify(Object.keys(obj).sort().reduce(function (result, key) { + result[key] = obj[key]; + return result; + }, {})); +} diff --git a/phpBB/adm/style/tooltip.js b/phpBB/adm/style/tooltip.js index 7b7abb11e6..9afd398e00 100644 --- a/phpBB/adm/style/tooltip.js +++ b/phpBB/adm/style/tooltip.js @@ -218,23 +218,6 @@ $(function() { // Prepare dropdown phpbb.prepareRolesDropdown(); - - // Reset role drop-down on modifying permissions in advanced tab - $('div.permissions-switch > a').on('click', function () { - $.each($('input[type=radio][name^="setting["]'), function () { - var $this = $(this); - $this.on('click', function () { - var $rolesOptions = $this.closest('fieldset.permissions').find('.roles-options'), - rolesSelect = $rolesOptions.find('select > option')[0]; - - // Set selected setting - $rolesOptions.children('span') - .text(rolesSelect.text); - $rolesOptions.children('input[type=hidden]') - .val(rolesSelect.value); - }); - }); - }); }); })(jQuery); // Avoid conflicts with other libraries From 18e75202d7eef33e38a51fabeb512612bc5f7f1a Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Tue, 21 Jan 2020 17:27:03 +0100 Subject: [PATCH 2/3] [ticket/15766] Use proper variable name for permission set PHPBB3-15766 --- phpBB/adm/style/permissions.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/adm/style/permissions.js b/phpBB/adm/style/permissions.js index 8bcfd311c6..c958df46e6 100644 --- a/phpBB/adm/style/permissions.js +++ b/phpBB/adm/style/permissions.js @@ -322,21 +322,21 @@ function match_role_settings(id) { var fs = document.getElementById(id), cbs = fs.getElementsByTagName('input'), - xyz = {}; + set = {}; for (var i = 0; i < cbs.length; i++) { var matches = cbs[i].id.match(/setting\[\d+]\[\d+]\[([a-z_]+)]/); if (matches !== null && cbs[i].checked && cbs[i].value !== '-1') { - xyz[matches[1]] = parseInt(cbs[i].value); + set[matches[1]] = parseInt(cbs[i].value); } } - xyz = sort_and_stringify(xyz); + set = sort_and_stringify(set); for (var r in role_options) { - if (sort_and_stringify(role_options[r]) === xyz) { + if (sort_and_stringify(role_options[r]) === set) { return r; } } From 87fe2847f085c5efcac2703c45d975791c7d224b Mon Sep 17 00:00:00 2001 From: mrgoldy Date: Mon, 3 Feb 2020 22:42:42 +0100 Subject: [PATCH 3/3] [ticket/15766] Docblocks, comments and longhand variable names PHPBB3-15766 --- phpBB/adm/style/permissions.js | 51 +++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/phpBB/adm/style/permissions.js b/phpBB/adm/style/permissions.js index c958df46e6..dc9951a7a2 100644 --- a/phpBB/adm/style/permissions.js +++ b/phpBB/adm/style/permissions.js @@ -269,8 +269,14 @@ function mark_one_option(id, field_name, s) { } /** -* Reset role dropdown field to Select role... if an option gets changed -*/ + * (Re)set the permission role dropdown. + * + * Try and match the set permissions to an existing role. + * Otherwise reset the dropdown to "Select a role.." + * + * @param {string} id The fieldset identifier + * @returns {void} + */ function reset_role(id) { var t = document.getElementById(id); @@ -278,14 +284,16 @@ function reset_role(id) { return; } + // Before resetting the role dropdown, try and match any permission role var parent = t.parentNode, roleId = match_role_settings(id.replace('role', 'perm')), text = no_role_assigned, index = 0; + // If a role permissions was matched, grab that option's value and index if (roleId) { for (var i = 0; i < t.options.length; i++) { - if (t.options[i].value == roleId) { + if (parseInt(t.options[i].value, 10) === roleId) { text = t.options[i].text; index = i; break; @@ -293,9 +301,11 @@ function reset_role(id) { } } + // Update the select's value and selected index t.value = roleId; t.options[index].selected = true; + // Update the dropdown trigger to show the new value parent.querySelector('span.dropdown-trigger').innerText = text; parent.querySelector('input[data-name^=role]').value = roleId; } @@ -304,7 +314,7 @@ function reset_role(id) { * Load role and set options accordingly */ function set_role_settings(role_id, target_id) { - settings = role_options[role_id]; + var settings = role_options[role_id]; if (!settings) { return; @@ -318,32 +328,47 @@ function set_role_settings(role_id, target_id) { } } -function match_role_settings(id) -{ - var fs = document.getElementById(id), - cbs = fs.getElementsByTagName('input'), +/** + * Match the set permissions against the available roles. + * + * @param {string} id The parent fieldset identifier + * @return {number} The permission role identifier + */ +function match_role_settings(id) { + var fieldset = document.getElementById(id), + radios = fieldset.getElementsByTagName('input'), set = {}; - for (var i = 0; i < cbs.length; i++) { - var matches = cbs[i].id.match(/setting\[\d+]\[\d+]\[([a-z_]+)]/); + // Iterate over all the radio buttons + for (var i = 0; i < radios.length; i++) { + var matches = radios[i].id.match(/setting\[\d+]\[\d+]\[([a-z_]+)]/); - if (matches !== null && cbs[i].checked && cbs[i].value !== '-1') { - set[matches[1]] = parseInt(cbs[i].value); + // Make sure the name attribute matches, the radio is checked and it is not the "No" (-1) value. + if (matches !== null && radios[i].checked && radios[i].value !== '-1') { + set[matches[1]] = parseInt(radios[i].value, 10); } } + // Sort and stringify the 'set permissions' object set = sort_and_stringify(set); + // Iterate over the available role options and return the first match for (var r in role_options) { if (sort_and_stringify(role_options[r]) === set) { - return r; + return parseInt(r, 10); } } return 0; } +/** + * Sort and stringify an Object so it can be easily compared against another object. + * + * @param {object} obj The object to sort (by key) and stringify + * @return {string} The sorted object as a string + */ function sort_and_stringify(obj) { return JSON.stringify(Object.keys(obj).sort().reduce(function (result, key) { result[key] = obj[key];