Moodle
  1. Moodle
  2. MDL-25988

Database Sync from CLI removes roles from coding bug in enrol/database/lib.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Enrolments
    • Labels:
      None
    • Environment:
      Any
    • Database:
      Any
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      15938

      Description

      The file /enrol/database/lib.php deletes roles inappropriately because of an inadvertent re-assignment of the $roles variable.

      This will roughly fix the problem I think. The problem is that the $roles are initialized (cached) before the loop and the foreach inadvertently clobbers the roles mapping cache.

      The effect of this is that when sync is run, many teacher roles get removed, and default roles assigned for them.

      I'm not entirely sure whether the last $roles access listed below should refer to $userRoles or $roles but I think it needs to be $userRoles.

      Category mappings are still not working but that needs a separate ticket and I haven't found that problem yet.

      // enrol all users and sync roles

      • foreach ($requested_roles as $userid=>$roles) {
      • foreach ($roles as $roleid) {
        + foreach ($requested_roles as $userid=>$userRoles) {
        + foreach ($userRoles as $roleid) {
        if (empty($current_roles[$userid])) {
        $this->enrol_user($instance, $userid, $roleid);
        $current_roles[$userid][$roleid] = $roleid;
        @@ -441,7 +445,8 @@

      // unassign removed roles
      foreach($current_roles[$userid] as $cr) {

      • if (empty($roles[$cr])) {
        + if (empty($userRoles[$cr])) { role_unassign($cr, $userid, $context->id, 'enrol_database', $instance->id); unset($current_roles[$userid][$cr]); }

        Activity

        Hide
        Petr Škoda added a comment -

        Yes, you are right, sorry for the sloppy testing (I should have used much larger test sample).

        Show
        Petr Škoda added a comment - Yes, you are right, sorry for the sloppy testing (I should have used much larger test sample).
        Hide
        Petr Škoda added a comment -

        The fix will be in the next weekly build, thanks a lot for the report and patch.

        Petr

        Show
        Petr Škoda added a comment - The fix will be in the next weekly build, thanks a lot for the report and patch. Petr

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: