towards unique role sortorder - fixes for create_role() and new delete_role()

diff --git a/admin/roles/manage.php b/admin/roles/manage.php
index 8b2733149ab8a2be08214282336d4d09646d9fbf..9e017cd003a6d6967968a268a614ba634a166452 100755 (executable)
--- a/admin/roles/manage.php
+++ b/admin/roles/manage.php
@@ -63,7 +63,7 @@
                 }
 
                 if (empty($errors)) {
-                    $newrole = create_role($name, $shortname, $description, '', $rolescount);
+                    $newrole = create_role($name, $shortname, $description);
                 } else {
                     $newrole = new object();
                     $newrole->name = $name;
@@ -181,14 +181,8 @@
 
         case 'delete':
             if ($confirm and data_submitted() and confirm_sesskey()) {
-
-                // first unssign all users
-                if (!role_unassign($roleid)) {
-                    error("Error while unassigning all users from role with ID $roleid!");
-                }
-
-                if (!delete_records('role', 'id', $roleid)) {
-                    error("Could not delete role with ID $roleid!");
+                if (!delete_role($roleid)) {
+                    error('Could not delete role with ID '.$roleid);
                 }
 
             } else if (confirm_sesskey()){

diff --git a/lib/accesslib.php b/lib/accesslib.php
index c1d90dcbc87275e79ac8458e0f7b9afd1a73cab3..0d5932a497c0066d97a685ca45bc79c3379dc684 100755 (executable)
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -1201,7 +1201,7 @@ function get_local_override($roleid, $contextid, $capability) {
  * @param legacy - optional legacy capability
  * @return id or false
  */
-function create_role($name, $shortname, $description, $legacy='', $sortorder = -1) {
+function create_role($name, $shortname, $description, $legacy='') {
 
     // check for duplicate role name
 
@@ -1218,10 +1218,10 @@ function create_role($name, $shortname, $description, $legacy='', $sortorder = -
     $role->shortname = $shortname;
     $role->description = $description;
 
-    if ($sortorder = -1) {
-        $role->sortorder = count_records('role');
-    } else {
-        $role->sortorder = $sortorder;
+    //find free sortorder number
+    $role->sortorder = count_records('role');
+    while (get_record('role','sortorder', $role->sortorder)) {
+        $role->sortorder += 1;
     }
 
     $context = get_context_instance(CONTEXT_SYSTEM, SITEID);
@@ -1251,6 +1251,39 @@ function create_role($name, $shortname, $description, $legacy='', $sortorder = -
 
 }
 
+/**
+ * function that deletes a role and cleanups up after it
+ * @param roleid - id of role to delete
+ * @return success
+ */
+function delete_role($roleid) {
+    $success = true;
+
+// first unssign all users
+    if (!role_unassign($roleid)) {
+        debugging("Error while unassigning all users from role with ID $roleid!");
+        $success = false;
+    }
+
+// cleanup all references to this role, ignore errors
+    if ($success) {
+        delete_records('role_capabilities', 'roleid', $roleid);
+        delete_records('role_allow_assign', 'roleid', $roleid);
+        delete_records('role_allow_assign', 'allowassign', $roleid);
+        delete_records('role_allow_override', 'roleid', $roleid);
+        delete_records('role_allow_override', 'allowoverride', $roleid);
+        delete_records('role_names', 'roleid', $roleid);
+    }
+
+// finally delete the role itself
+    if ($success and !delete_records('role', 'id', $roleid)) {
+        debugging("Could not delete role with ID $roleid!");
+        $success = false;
+    }
+
+    return $success;
+}
+
 /**
  * Function to write context specific overrides, or default capabilities.
  * @param module - string name