From 8b46da3853636a313543b8d9154d93054ed1193f Mon Sep 17 00:00:00 2001 From: Udo Walter Date: Thu, 21 Dec 2017 19:34:28 +0100 Subject: [permissionmanager] removed openRoleEdior function; changed searchFieldFunction() to use jQuery; fixed checkboxes in roleeditor not selecting on a click on the label; added selected row highlighting; disabled buttons if there are no users/roles selected; made role badges in user/location tables clickable; added phpdoc comments to most php functions; --- .../permissionmanager/clientscript.js | 16 ++--- .../inc/getpermissiondata.inc.php | 40 +++++++---- .../inc/permissiondbupdate.inc.php | 37 +++++++--- .../permissionmanager/inc/permissionutil.inc.php | 34 ++++++++++ .../permissionmanager/lang/de/template-tags.json | 5 +- .../permissionmanager/lang/en/template-tags.json | 5 +- modules-available/permissionmanager/page.inc.php | 50 ++++++++++++-- .../templates/locationstable.html | 2 +- .../permissionmanager/templates/roleeditor.html | 17 ++--- .../permissionmanager/templates/rolestable.html | 34 +++------- .../permissionmanager/templates/treenode.html | 4 +- .../permissionmanager/templates/treepanel.html | 12 ++-- .../permissionmanager/templates/userstable.html | 79 ++++++++++++++++++---- 13 files changed, 233 insertions(+), 102 deletions(-) diff --git a/modules-available/permissionmanager/clientscript.js b/modules-available/permissionmanager/clientscript.js index 65065e6e..4770fa6a 100644 --- a/modules-available/permissionmanager/clientscript.js +++ b/modules-available/permissionmanager/clientscript.js @@ -1,5 +1,5 @@ document.addEventListener("DOMContentLoaded", function() { - var selectize = $('#select-role'); + var selectize = $("#select-role"); if (selectize.length) { selectize = selectize.selectize({ allowEmptyOption: false, @@ -13,10 +13,10 @@ document.addEventListener("DOMContentLoaded", function() { // If Site gets refreshed, all data-selectizeCounts will be reset to 0, so delete the filters from the selectize selectize.clear(); - selectize.on('item_add', function (value, $item) { + selectize.on("item_add", function (value, $item) { // When first item gets added the filter isn't empty anymore, so hide all rows if (selectize.items.length === 1) { - $('.dataTable tbody').find('tr').hide(); + $(".dataTable tbody").find("tr").hide(); } // Find all rows which shall be shown and increase their counter by 1 $(".roleid-" + value).closest("tr").each(function () { @@ -25,10 +25,10 @@ document.addEventListener("DOMContentLoaded", function() { }); }); - selectize.on('item_remove', function (value, $item) { + selectize.on("item_remove", function (value, $item) { // When no items in the filter, show all rows again if (selectize.items.length === 0) { - $('.dataTable tbody').find('tr').show(); + $(".dataTable tbody").find("tr").show(); } else { // Find all rows which have the delete role, decrease their counter by 1 $(".roleid-" + value).closest("tr").each(function () { @@ -42,9 +42,9 @@ document.addEventListener("DOMContentLoaded", function() { }); } - $("tr").on('click', function (e) { - if (e.target.type !== "checkbox") { - $(this).find("input:checkbox").trigger("click"); + $("tr").on("click", function(e) { + if (e.target.type !== "checkbox" && e.target.tagName !== "A") { + $(this).find("input[type=checkbox]").trigger("click"); } }); diff --git a/modules-available/permissionmanager/inc/getpermissiondata.inc.php b/modules-available/permissionmanager/inc/getpermissiondata.inc.php index 13c7ca89..982fa0b7 100644 --- a/modules-available/permissionmanager/inc/getpermissiondata.inc.php +++ b/modules-available/permissionmanager/inc/getpermissiondata.inc.php @@ -2,9 +2,17 @@ class GetPermissionData { - // get UserIDs, User Login Names, User Roles + /** + * Get data for all users. + * + * @return array array of users (each with userid, username and roles (each with roleid and rolename)) + */ public static function getUserData() { - $res = self::queryUserData(); + $res = Database::simpleQuery("SELECT user.userid AS userid, user.login AS login, role.rolename AS rolename, role.roleid AS roleid + FROM user + LEFT JOIN user_x_role ON user.userid = user_x_role.userid + LEFT JOIN role ON user_x_role.roleid = role.roleid + "); $userdata= array(); while ($row = $res->fetch(PDO::FETCH_ASSOC)) { $userdata[$row['userid'].' '.$row['login']][] = array( @@ -24,7 +32,11 @@ class GetPermissionData { return $data; } - // get LocationIDs, Location Names, Roles of each Location + /** + * Get data for all locations. + * + * @return array array of locations (each including the roles that have permissions for them) + */ public static function getLocationData() { $res = Database::simpleQuery("SELECT role.roleid as roleid, rolename, GROUP_CONCAT(COALESCE(locationid, 0)) AS locationids FROM role INNER JOIN role_x_location ON role.roleid = role_x_location.roleid GROUP BY roleid ORDER BY rolename ASC"); @@ -46,7 +58,11 @@ class GetPermissionData { return array_values($locations); } - // get all roles from database (id and name) + /** + * Get all roles. + * + * @return array array roles (each with roleid and rolename) + */ public static function getRoles() { $res = Database::simpleQuery("SELECT roleid, rolename FROM role ORDER BY rolename ASC"); $data = array(); @@ -59,6 +75,12 @@ class GetPermissionData { return $data; } + /** + * Get permissions and locations for a given role. + * + * @param string $roleid id of the role + * @return array array containing an array of permissions and an array of locations + */ public static function getRoleData($roleid) { $query = "SELECT roleid, rolename FROM role WHERE roleid = :roleid"; $data = Database::queryFirst($query, array("roleid" => $roleid)); @@ -77,14 +99,4 @@ class GetPermissionData { return $data; } - // UserID, User Login Name, Roles of each User - private static function queryUserData() { - $res = Database::simpleQuery("SELECT user.userid AS userid, user.login AS login, role.rolename AS rolename, role.roleid AS roleid - FROM user - LEFT JOIN user_x_role ON user.userid = user_x_role.userid - LEFT JOIN role ON user_x_role.roleid = role.roleid - "); - return $res; - } - } \ No newline at end of file diff --git a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php index f144b35e..ffe5fac0 100644 --- a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php +++ b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php @@ -2,7 +2,12 @@ class PermissionDbUpdate { - // insert new user_x_role to database. "ignore" to ignore duplicate entry try + /** + * Insert all user/role combinations into the user_x_role table. + * + * @param array $users userids + * @param array $roles roleids + */ public static function addRoleToUser($users, $roles) { $query = "INSERT IGNORE INTO user_x_role (userid, roleid) VALUES (:userid, :roleid)"; foreach($users AS $userid) { @@ -12,24 +17,34 @@ class PermissionDbUpdate { } } - // remove user_x_role entry from database + /** + * Remove all user/role combinations from the user_x_role table. + * + * @param array $users userids + * @param array $roles roleids + */ public static function removeRoleFromUser($users, $roles) { $query = "DELETE FROM user_x_role WHERE userid IN (:users) AND roleid IN (:roles)"; Database::exec($query, array("users" => $users, "roles" => $roles)); } - // delete role, delete user_x_role relationships, delete role_x_location relationships, delete role_x_permission relationships + /** + * Delete role from the role table. + * + * @param string $roleid roleid + */ public static function deleteRole($roleid) { - $query = "DELETE FROM role WHERE roleid = :roleid"; - Database::exec($query, array("roleid" => $roleid)); - $query = "DELETE FROM user_x_role WHERE roleid = :roleid"; - Database::exec($query, array("roleid" => $roleid)); - $query = "DELETE FROM role_x_location WHERE roleid = :roleid"; - Database::exec($query, array("roleid" => $roleid)); - $query = "DELETE FROM role_x_permission WHERE roleid = :roleid"; - Database::exec($query, array("roleid" => $roleid)); + Database::exec("DELETE FROM role WHERE roleid = :roleid", array("roleid" => $roleid)); } + /** + * Save changes to a role or create a new one. + * + * @param string $rolename rolename + * @param array $locations array of locations + * @param array $permissions array of permissions + * @param string|null $roleid roleid or null if the role does not exist yet + */ public static function saveRole($rolename, $locations, $permissions, $roleid = NULL) { if ($roleid) { Database::exec("UPDATE role SET rolename = :rolename WHERE roleid = :roleid", diff --git a/modules-available/permissionmanager/inc/permissionutil.inc.php b/modules-available/permissionmanager/inc/permissionutil.inc.php index cd7fb09a..cb70c8bb 100644 --- a/modules-available/permissionmanager/inc/permissionutil.inc.php +++ b/modules-available/permissionmanager/inc/permissionutil.inc.php @@ -2,6 +2,14 @@ class PermissionUtil { + /** + * Check if the user has the given permission (for the given location). + * + * @param string $userid userid to check + * @param string $permissionid permissionid to check + * @param int|null $locationid locationid to check or null if the location should be disregarded + * @return bool true if user has permission, false if not + */ public static function userHasPermission($userid, $permissionid, $locationid) { $locations = array(); if (!is_null($locationid)) { @@ -27,6 +35,13 @@ class PermissionUtil return false; } + /** + * Get all locations where the user has the given permission. + * + * @param string $userid userid to check + * @param string $permissionid permissionid to check + * @return array array of locationids where the user has the given permission + */ public static function getAllowedLocations($userid, $permissionid) { $res = Database::simpleQuery("SELECT permissionid, COALESCE(locationid, 0) AS locationid FROM user_x_role @@ -51,6 +66,13 @@ class PermissionUtil return $allowedLocations; } + /** + * Extend an array of locations by adding all sublocations. + * + * @param array $tree tree of all locations (structured like Location::getTree()) + * @param array $locations the array of locationids to extend + * @return array extended array of locationids + */ public static function getSublocations($tree, $locations) { $result = array_flip($locations); foreach ($tree as $location) { @@ -65,6 +87,11 @@ class PermissionUtil return array_keys($result); } + /** + * Get all permissions of all active modules that have permissions in their permissions/permissions.json file. + * + * @return array permission tree as a multidimensional array + */ public static function getPermissions() { $permissions = array(); @@ -89,6 +116,13 @@ class PermissionUtil return $permissions; } + /** + * Place a permission into the given permission tree. + * + * @param string $permission the permission to place in the tree + * @param string $description the description of the permission + * @param array $tree the permission tree to modify + */ private static function putInPermissionTree($permission, $description, &$tree) { $subPermissions = explode('.', $permission); diff --git a/modules-available/permissionmanager/lang/de/template-tags.json b/modules-available/permissionmanager/lang/de/template-tags.json index 23b2dc68..88652152 100644 --- a/modules-available/permissionmanager/lang/de/template-tags.json +++ b/modules-available/permissionmanager/lang/de/template-tags.json @@ -2,12 +2,11 @@ "lang_Roles": "Rollen", "lang_Users": "Nutzer", "lang_Locations": "Räume", - "lang_addRole": "Rolle zuweisen", - "lang_removeRole": "Rolle entfernen", + "lang_addRole": "Rollen erteilen", + "lang_removeRole": "Rollen entziehen", "lang_newRole": "Rolle anlegen", "lang_Selected": "Ausgewählt", "lang_Edit": "Bearbeiten", - "lang_Remove": "Entfernen", "lang_Delete": "Löschen", "lang_removeCheck": "Sind Sie sich sicher, dass Sie diese Rolle entfernen wollen?", "lang_deleteCheck": "Sind Sie sich sicher, dass Sie diese Rolle löschen wollen?", diff --git a/modules-available/permissionmanager/lang/en/template-tags.json b/modules-available/permissionmanager/lang/en/template-tags.json index 01056632..08769f27 100644 --- a/modules-available/permissionmanager/lang/en/template-tags.json +++ b/modules-available/permissionmanager/lang/en/template-tags.json @@ -2,12 +2,11 @@ "lang_Roles": "Roles", "lang_Users": "Users", "lang_Locations": "Locations", - "lang_addRole": "Add Role", - "lang_removeRole": "Remove Role", + "lang_addRole": "Grant Roles", + "lang_removeRole": "Revoke Roles", "lang_newRole": "New Role", "lang_Selected": "Selected", "lang_Edit": "Edit", - "lang_Remove": "Remove", "lang_Delete": "Delete", "lang_removeCheck": "Are you sure you want to remove this role?", "lang_deleteCheck": "Are you sure you want to delete this role?", diff --git a/modules-available/permissionmanager/page.inc.php b/modules-available/permissionmanager/page.inc.php index 9aba80b3..eeed9c02 100644 --- a/modules-available/permissionmanager/page.inc.php +++ b/modules-available/permissionmanager/page.inc.php @@ -64,14 +64,14 @@ class Page_PermissionManager extends Page Render::addTemplate('locationstable', $data); } } elseif ($show === "roleEditor") { - $data = array(); + $data = array("cancelShow" => Request::get("cancel", "roles")); $selectedPermissions = array(); $selectedLocations = array(); - $roleID = Request::get("roleid", false); - if ($roleID) { - $roleData = GetPermissionData::getRoleData($roleID); - $data["roleid"] = $roleID; + $roleid = Request::get("roleid", false); + if ($roleid) { + $roleData = GetPermissionData::getRoleData($roleid); + $data["roleid"] = $roleid; $data["rolename"] = $roleData["rolename"]; $selectedPermissions = $roleData["permissions"]; $selectedLocations = $roleData["locations"]; @@ -85,12 +85,21 @@ class Page_PermissionManager extends Page } } - private static function generatePermissionHTML($subPermissions, $selectedPermissions = array(), $selectAll = false, $permString = "") + /** + * Recursively generate HTML code for the permission selection tree. + * + * @param array $permissions the permission tree + * @param array $selectedPermissions permissions that should be preselected + * @param array $selectAll true if all pemrissions should be preselected, false if only those in $selectedPermissions + * @param array $permString the prefix permission string with which all permissions in the permission tree should start + * @return string generated html code + */ + private static function generatePermissionHTML($permissions, $selectedPermissions = array(), $selectAll = false, $permString = "") { $res = ""; $toplevel = $permString == ""; if ($toplevel && in_array("*", $selectedPermissions)) $selectAll = true; - foreach ($subPermissions as $k => $v) { + foreach ($permissions as $k => $v) { $leaf = !is_array($v); $nextPermString = $permString ? $permString.".".$k : $k; $id = $leaf ? $nextPermString : $nextPermString.".*"; @@ -115,6 +124,15 @@ class Page_PermissionManager extends Page return $res; } + /** + * Recursively generate HTML code for the location selection tree. + * + * @param array $locations the location tree + * @param array $selectedLocations locations that should be preselected + * @param array $selectAll true if all locations should be preselected, false if only those in $selectedLocations + * @param array $toplevel true if the location tree are the children of the root location, false if not + * @return string generated html code + */ private static function generateLocationHTML($locations, $selectedLocations = array(), $selectAll = false, $toplevel = true) { $res = ""; @@ -141,6 +159,12 @@ class Page_PermissionManager extends Page return $res; } + /** + * Remove locations that are already covered by parent locations from the array. + * + * @param array $locations the locationid array + * @return array the locationid array without redundant locationids + */ private static function processLocations($locations) { if (in_array(0, $locations)) return array(NULL); @@ -158,6 +182,12 @@ class Page_PermissionManager extends Page return $result; } + /** + * Remove permissions that are already covered by parent permissions from the array. + * + * @param array $permissions the permissionid array + * @return array the permissionid array without redundant permissionids + */ private static function processPermissions($permissions) { if (in_array("*", $permissions)) return array("*"); @@ -171,6 +201,12 @@ class Page_PermissionManager extends Page return self::extractPermissions($result); } + /** + * Convert a multidimensional array of permissions to a flat array of permissions. + * + * @param array $permissions multidimensional array of permissionids + * @return array flat array of permissionids + */ private static function extractPermissions($permissions) { $result = array(); diff --git a/modules-available/permissionmanager/templates/locationstable.html b/modules-available/permissionmanager/templates/locationstable.html index dcfefa94..b910e3f3 100644 --- a/modules-available/permissionmanager/templates/locationstable.html +++ b/modules-available/permissionmanager/templates/locationstable.html @@ -26,7 +26,7 @@ {{locationpad}} {{locationname}} {{#roles}} - {{rolename}} + {{rolename}} {{/roles}} diff --git a/modules-available/permissionmanager/templates/roleeditor.html b/modules-available/permissionmanager/templates/roleeditor.html index b07e2112..ddf6ace9 100644 --- a/modules-available/permissionmanager/templates/roleeditor.html +++ b/modules-available/permissionmanager/templates/roleeditor.html @@ -10,11 +10,11 @@
  • {{lang_Locations}}
  • - {{lang_Name}}: - + +
  • - + {{lang_Cancel}}
    • @@ -56,15 +56,12 @@ } } }); - - $("#cancelButton").click(function () { - window.location.replace("?do=permissionmanager&show=roles"); - }); $('form').submit(function () { - var name = $.trim($('#rolename').val()); - if (name === '') { - $("#rolename").addClass("missingInput"); + var input = $("#rolename"); + var name = $.trim(input.val()); + if (!name) { + input.addClass("missingInput"); return false; } }); diff --git a/modules-available/permissionmanager/templates/rolestable.html b/modules-available/permissionmanager/templates/rolestable.html index 992feca1..7152a1dd 100644 --- a/modules-available/permissionmanager/templates/rolestable.html +++ b/modules-available/permissionmanager/templates/rolestable.html @@ -8,13 +8,13 @@
    - + {{lang_newRole}}
    - +
    @@ -25,10 +25,10 @@ {{#roles}} - - + +
    {{lang_Roles}}
    {{rolename}}
    {{rolename}} - + @@ -64,30 +64,18 @@ \ No newline at end of file diff --git a/modules-available/permissionmanager/templates/treenode.html b/modules-available/permissionmanager/templates/treenode.html index 336ca13e..ced973ca 100644 --- a/modules-available/permissionmanager/templates/treenode.html +++ b/modules-available/permissionmanager/templates/treenode.html @@ -1,8 +1,8 @@ {{#toplevel}}
      {{/toplevel}}
    • - - + +
        {{{HTML}}} diff --git a/modules-available/permissionmanager/templates/treepanel.html b/modules-available/permissionmanager/templates/treepanel.html index 53e316c9..6f358825 100644 --- a/modules-available/permissionmanager/templates/treepanel.html +++ b/modules-available/permissionmanager/templates/treepanel.html @@ -1,11 +1,11 @@ -
        -
        -
        - - +
        +
        +
        + +
        -
        +
        {{{HTML}}}
        diff --git a/modules-available/permissionmanager/templates/userstable.html b/modules-available/permissionmanager/templates/userstable.html index 9f684e99..749ae03d 100644 --- a/modules-available/permissionmanager/templates/userstable.html +++ b/modules-available/permissionmanager/templates/userstable.html @@ -13,8 +13,8 @@
        - - + +
        @@ -35,7 +35,7 @@
    		{{username}} {{#roles}} - {{rolename}} + {{rolename}} {{/roles}} @@ -89,7 +89,7 @@
    - +
    @@ -100,7 +100,7 @@
    - +
    @@ -132,7 +132,7 @@
    - +
    @@ -140,34 +140,85 @@ \ No newline at end of file -- cgit v1.2.3-55-g7522