MDL-51401 enrol: ensures correct order of roles in UI components

Role sort order was not available in the UI (in JavaScript) due to the use of PHP assoc arrays and JSON encoding/decoding. This patch addresses this by sending the roles in a json array, rather than json object.
robjoyce · Oct 7, 2016 · 0bb7f79 · 0bb7f79
1 parent d9520bc
commit 0bb7f79
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 11 deletions.
diff --git a/enrol/ajax.php b/enrol/ajax.php
@@ -88,7 +88,7 @@
         break;
     case 'getassignable':
         $otheruserroles = optional_param('otherusers', false, PARAM_BOOL);
-        $outcome->response = array_reverse($manager->get_assignable_roles($otheruserroles), true);
+        $outcome->response = $manager->get_assignable_roles_for_json($otheruserroles);
         break;
     case 'searchotherusers':
         $search = optional_param('search', '', PARAM_RAW);

diff --git a/enrol/locallib.php b/enrol/locallib.php
@@ -621,6 +621,22 @@ public function get_assignable_roles($otherusers = false) {
         }
     }
 
+    /**
+     * Gets all of the assignable roles for this course, wrapped in an array to ensure
+     * role sort order is not lost during json deserialisation.
+     *
+     * @param boolean $otherusers whether to include the assignable roles for other users
+     * @return array
+     */
+    public function get_assignable_roles_for_json($otherusers = false) {
+        $rolesarray = array();
+        $assignable = $this->get_assignable_roles($otherusers);
+        foreach ($assignable as $id => $role) {
+            $rolesarray[] = array('id' => $id, 'name' => $role);
+        }
+        return $rolesarray;
+    }
+
     /**
      * Gets all of the groups for this course.
      *

diff --git a/enrol/manual/yui/quickenrolment/quickenrolment.js b/enrol/manual/yui/quickenrolment/quickenrolment.js
@@ -198,8 +198,8 @@ YUI.add('moodle-enrol_manual-quickenrolment', function(Y) {
                 var index = 0, count = 0;
                 for (var i in roles) {
                     count++;
-                    var option = create('<option value="'+i+'">'+roles[i]+'</option>');
-                    if (i == v) {
+                    var option = create('<option value="' + roles[i].id + '">' + roles[i].name + '</option>');
+                    if (roles[i].id == v) {
                         index = count;
                     }
                     s.append(option);

diff --git a/enrol/yui/otherusersmanager/otherusersmanager.js b/enrol/yui/otherusersmanager/otherusersmanager.js
@@ -314,10 +314,10 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) {
                     )
                     .append(Y.Node.create('<div class="'+CSS.OPTIONS+'"><span class="label">'+M.util.get_string('assignrole', 'role')+': </span></div>'))
                 );
-            var id = 0, roles = this._manager.get(ASSIGNABLEROLES);
-            for (id in roles) {
-                var role = Y.Node.create('<a href="#" class="'+CSS.ROLEOPTION+'">'+roles[id]+'</a>');
-                role.on('click', this.assignRoleToUser, this, id, role);
+            var roles = this._manager.get(ASSIGNABLEROLES);
+            for (var i in roles) {
+                var role = Y.Node.create('<a href="#" class="' + CSS.ROLEOPTION + '">' + roles[i].name + '</a>');
+                role.on('click', this.assignRoleToUser, this, roles[i].id, role);
                 this._node.one('.'+CSS.OPTIONS).append(role);
             }
             return this._node;

diff --git a/enrol/yui/rolemanager/rolemanager.js b/enrol/yui/rolemanager/rolemanager.js
@@ -94,7 +94,7 @@ YUI.add('moodle-enrol-rolemanager', function(Y) {
                             if (o.error) {
                                 new M.core.ajaxException(o);
                             } else {
-                                this.users[userid].addRoleToDisplay(args.roleid, this.get(ASSIGNABLEROLES)[args.roleid]);
+                                this.users[userid].addRoleToDisplay(args.roleid, this._getAssignableRole(args.roleid));
                             }
                         } catch (e) {
                             new M.core.exception(e);
@@ -152,6 +152,15 @@ YUI.add('moodle-enrol-rolemanager', function(Y) {
                 }
             });
         },
+        _getAssignableRole: function(roleid) {
+            var roles = this.get(ASSIGNABLEROLES);
+            for (var i in roles) {
+                if (roles[i].id == roleid) {
+                    return roles[i].name;
+                }
+            }
+            return null;
+        },
         _loadAssignableRoles : function() {
             var c = this.get(COURSEID), params = {
                 id : this.get(COURSEID),
@@ -277,7 +286,7 @@ YUI.add('moodle-enrol-rolemanager', function(Y) {
             var current = this.get(CURRENTROLES);
             var allroles = true, i = 0;
             for (i in roles) {
-                if (!current[i]) {
+                if (!current[roles[i].id]) {
                     allroles = false;
                     break;
                 }
@@ -353,8 +362,10 @@ YUI.add('moodle-enrol-rolemanager', function(Y) {
             var content = element.one('.content');
             var roles = m.get(ASSIGNABLEROLES);
             for (i in roles) {
-                var button = Y.Node.create('<input type="button" value="'+roles[i]+'" id="add_assignable_role_'+i+'" />');
-                button.on('click', this.submit, this, i);
+                var buttonid = 'add_assignable_role_' + roles[i].id;
+                var buttonhtml = '<input type="button" value="' + roles[i].name + '" id="' + buttonid + '" />';
+                var button = Y.Node.create(buttonhtml);
+                button.on('click', this.submit, this, roles[i].id);
                 content.append(button);
             }
             Y.one(document.body).append(element);