MDL-30944 improve enrol_cohort plugin

List of changes:
* configurable unenrol action
* cron period increased to 1 hour in order to lower server load, courses should not get out of sync often
* CLI sync script for debugging or manual sync after configuration change
* phpdocs fixes
* when plugin is disabled all roles are removed, enrollments are kept
* uninstall script
* other bugfixing and performance improvements

diff --git a/enrol/cohort/addinstance_form.php b/enrol/cohort/addinstance_form.php
index a6bcbbc..7e15755 100644 (file)
--- a/enrol/cohort/addinstance_form.php
+++ b/enrol/cohort/addinstance_form.php
@@ -55,6 +55,7 @@ class enrol_cohort_addinstance_form extends moodleform {
         $rs->close();
 
         $roles = get_assignable_roles($coursecontext);
+        $roles[0] = get_string('none');
         $roles = array_reverse($roles, true); // descending default sortorder
 
         $mform->addElement('header','general', get_string('pluginname', 'enrol_cohort'));

diff --git a/enrol/cohort/cli/sync.php b/enrol/cohort/cli/sync.php
new file mode 100644 (file)
index 0000000..068c18b
--- /dev/null
+++ b/enrol/cohort/cli/sync.php
@@ -0,0 +1,66 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * CLI sync for cohort enrolments, use for debugging or immediate sync
+ * of all courses.
+ *
+ * Notes:
+ *   - it is required to use the web server account when executing PHP CLI scripts
+ *   - you need to change the "www-data" to match the apache user account
+ *   - use "su" if "sudo" not available
+ *
+ * @package    enrol
+ * @subpackage cohort
+ * @copyright  2011 Petr Skoda {@link http://skodak.org}
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+define('CLI_SCRIPT', true);
+
+require(dirname(dirname(dirname(dirname(__FILE__)))).'/config.php');
+require_once($CFG->libdir.'/clilib.php');
+require_once("$CFG->dirroot/enrol/cohort/locallib.php");
+
+// now get cli options
+list($options, $unrecognized) = cli_get_params(array('verbose'=>false, 'help'=>false), array('v'=>'verbose', 'h'=>'help'));
+
+if ($unrecognized) {
+    $unrecognized = implode("\n  ", $unrecognized);
+    cli_error(get_string('cliunknowoption', 'admin', $unrecognized));
+}
+
+if ($options['help']) {
+    $help =
+        "Execute cohort course enrol sync.
+
+Options:
+-v, --verbose         Print verbose progess information
+-h, --help            Print out this help
+
+Example:
+\$sudo -u www-data /usr/bin/php enrol/cohort/cli/sync.php
+";
+
+    echo $help;
+    die;
+}
+
+$verbose = !empty($options['verbose']);
+
+$result = enrol_cohort_sync(null, $verbose);
+
+exit($result);
\ No newline at end of file

diff --git a/enrol/cohort/db/access.php b/enrol/cohort/db/access.php
index 4530353..dcd09e8 100644 (file)
--- a/enrol/cohort/db/access.php
+++ b/enrol/cohort/db/access.php
@@ -38,6 +38,16 @@ $capabilities = array(
         )
     ),
 
+    /* This is used only when sync suspends users instead of full unenrolment */
+    'enrol/cohort:unenrol' => array(
+
+        'captype' => 'write',
+        'contextlevel' => CONTEXT_COURSE,
+        'archetypes' => array(
+            'manager' => CAP_ALLOW,
+        )
+    ),
+
 );
 
 

diff --git a/enrol/cohort/db/uninstall.php b/enrol/cohort/db/uninstall.php
new file mode 100644 (file)
index 0000000..58b0c05
--- /dev/null
+++ b/enrol/cohort/db/uninstall.php
@@ -0,0 +1,41 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Meta link enrolment plugin uninstallation.
+ *
+ * @package    enrol
+ * @subpackage cohort
+ * @copyright  2011 Petr Skoda {@link http://skodak.org}
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+function xmldb_enrol_cohort_uninstall() {
+    global $CFG, $DB;
+
+    $cohort = enrol_get_plugin('cohort');
+    $rs = $DB->get_recordset('enrol', array('enrol'=>'cohort'));
+    foreach ($rs as $instance) {
+        $cohort->delete_instance($instance);
+    }
+    $rs->close();
+
+    role_unassign_all(array('component'=>'enrol_cohort'));
+
+    return true;
+}
\ No newline at end of file

diff --git a/enrol/cohort/lang/en/enrol_cohort.php b/enrol/cohort/lang/en/enrol_cohort.php
index a2dcbf4..8196bb0 100644 (file)
--- a/enrol/cohort/lang/en/enrol_cohort.php
+++ b/enrol/cohort/lang/en/enrol_cohort.php
@@ -27,5 +27,6 @@
 $string['ajaxmore'] = 'More...';
 $string['cohortsearch'] = 'Search';
 $string['cohort:config'] = 'Configure cohort instances';
+$string['cohort:unenrol'] = 'Unenrol suspended users';
 $string['pluginname'] = 'Cohort sync';
 $string['pluginname_desc'] = 'Cohort enrolment plugin synchronises cohort members with course participants.';

diff --git a/enrol/cohort/lib.php b/enrol/cohort/lib.php
index 11c22f7..2009867 100644 (file)
--- a/enrol/cohort/lib.php
+++ b/enrol/cohort/lib.php
@@ -1,5 +1,4 @@
 <?php
-
 // This file is part of Moodle - http://moodle.org/
 //
 // Moodle is free software: you can redistribute it and/or modify
@@ -44,15 +43,16 @@ class enrol_cohort_plugin extends enrol_plugin {
         if (empty($instance)) {
             $enrol = $this->get_name();
             return get_string('pluginname', 'enrol_'.$enrol);
+
         } else if (empty($instance->name)) {
             $enrol = $this->get_name();
             if ($role = $DB->get_record('role', array('id'=>$instance->roleid))) {
                 $role = role_get_name($role, get_context_instance(CONTEXT_COURSE, $instance->courseid));
+                return get_string('pluginname', 'enrol_'.$enrol) . ' (' . format_string($DB->get_field('cohort', 'name', array('id'=>$instance->customint1))) . ' - ' . $role .')';
             } else {
-                $role = get_string('error');
+                return get_string('pluginname', 'enrol_'.$enrol) . ' (' . format_string($DB->get_field('cohort', 'name', array('id'=>$instance->customint1))) . ')';
             }
 
-            return get_string('pluginname', 'enrol_'.$enrol) . ' (' . format_string($DB->get_field('cohort', 'name', array('id'=>$instance->customint1))) . ' - ' . $role .')';
         } else {
             return format_string($instance->name);
         }
@@ -100,6 +100,7 @@ class enrol_cohort_plugin extends enrol_plugin {
         return false;
     }
 
+
     /**
      * Called for all enabled enrol plugins that returned true from is_cron_required().
      * @return void
@@ -107,12 +108,6 @@ class enrol_cohort_plugin extends enrol_plugin {
     public function cron() {
         global $CFG;
 
-        // purge all roles if cohort sync disabled, those can be recreated later here in cron
-        if (!enrol_is_enabled('cohort')) {
-            role_unassign_all(array('component'=>'cohort_enrol'));
-            return;
-        }
-
         require_once("$CFG->dirroot/enrol/cohort/locallib.php");
         enrol_cohort_sync();
     }
@@ -138,6 +133,59 @@ class enrol_cohort_plugin extends enrol_plugin {
 
     }
 
+    /**
+     * Update instance status
+     *
+     * @param stdClass $instance
+     * @param int $newstatus ENROL_INSTANCE_ENABLED, ENROL_INSTANCE_DISABLED
+     * @return void
+     */
+    public function update_status($instance, $newstatus) {
+        global $CFG;
+
+        parent::update_status($instance, $newstatus);
+
+        require_once("$CFG->dirroot/enrol/cohort/locallib.php");
+        enrol_cohort_sync($instance->courseid);
+    }
+
+    /**
+     * Does this plugin allow manual unenrolment of a specific user?
+     * Yes, but only if user suspended...
+     *
+     * @param stdClass $instance course enrol instance
+     * @param stdClass $ue record from user_enrolments table
+     *
+     * @return bool - true means user with 'enrol/xxx:unenrol' may unenrol this user, false means nobody may touch this user enrolment
+     */
+    public function allow_unenrol_user(stdClass $instance, stdClass $ue) {
+        if ($ue->status == ENROL_USER_SUSPENDED) {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Gets an array of the user enrolment actions
+     *
+     * @param course_enrolment_manager $manager
+     * @param stdClass $ue A user enrolment object
+     * @return array An array of user_enrolment_actions
+     */
+    public function get_user_enrolment_actions(course_enrolment_manager $manager, $ue) {
+        $actions = array();
+        $context = $manager->get_context();
+        $instance = $ue->enrolmentinstance;
+        $params = $manager->get_moodlepage()->url->params();
+        $params['ue'] = $ue->id;
+        if ($this->allow_unenrol_user($instance, $ue) && has_capability('enrol/cohort:unenrol', $context)) {
+            $url = new moodle_url('/enrol/unenroluser.php', $params);
+            $actions[] = new user_enrolment_action(new pix_icon('t/delete', ''), get_string('unenrol', 'enrol'), $url, array('class'=>'unenrollink', 'rel'=>$ue->id));
+        }
+        return $actions;
+    }
+
     /**
      * Returns a button to enrol a cohort or its users through the manual enrolment plugin.
      *

diff --git a/enrol/cohort/locallib.php b/enrol/cohort/locallib.php
index 3462638..2a8057b 100644 (file)
--- a/enrol/cohort/locallib.php
+++ b/enrol/cohort/locallib.php
@@ -1,5 +1,4 @@
 <?php
-
 // This file is part of Moodle - http://moodle.org/
 //
 // Moodle is free software: you can redistribute it and/or modify
@@ -36,6 +35,11 @@ require_once($CFG->dirroot . '/enrol/locallib.php');
  * it may fail sometimes, so we always do a full sync in cron too.
  */
 class enrol_cohort_handler {
+    /**
+     * Event processor - cohort member added
+     * @param stdClass $ca
+     * @return bool
+     */
     public function member_added($ca) {
         global $DB;
 
@@ -43,153 +47,252 @@ class enrol_cohort_handler {
             return true;
         }
 
-        // does anything want to sync with this parent?
-        //TODO: add join to role table to make sure that roleid actually exists
-        if (!$enrols = $DB->get_records('enrol', array('customint1'=>$ca->cohortid, 'enrol'=>'cohort'), 'id ASC')) {
+        // does any enabled cohort instance want to sync with this cohort?
+        $sql = "SELECT e.*, r.id as roleexists
+                  FROM {enrol} e
+             LEFT JOIN {role} r ON (r.id = e.roleid)
+                 WHERE customint1 = :cohortid AND enrol = 'cohort'
+              ORDER BY id ASC";
+        if (!$instances = $DB->get_records_sql($sql, array('cohortid'=>$ca->cohortid))) {
             return true;
         }
 
         $plugin = enrol_get_plugin('cohort');
-        foreach ($enrols as $enrol) {
+        foreach ($instances as $instance) {
+            if ($instance->status != ENROL_INSTANCE_ENABLED ) {
+                // no roles for disabled instances
+                $instance->roleid = 0;
+            } else if ($instance->roleid and !$instance->roleexists) {
+                // invalid role - let's just enrol, they will have to create new sync and delete this one
+                $instance->roleid = 0;
+            }
+            unset($instance->roleexists);
             // no problem if already enrolled
-            $plugin->enrol_user($enrol, $ca->userid, $enrol->roleid);
+            $plugin->enrol_user($instance, $ca->userid, $instance->roleid, 0, 0, ENROL_USER_ACTIVE);
         }
 
         return true;
     }
 
+    /**
+     * Event processor - cohort member removed
+     * @param stdClass $ca
+     * @return bool
+     */
     public function member_removed($ca) {
         global $DB;
 
-        // does anything want to sync with this parent?
-        if (!$enrols = $DB->get_records('enrol', array('customint1'=>$ca->cohortid, 'enrol'=>'cohort'), 'id ASC')) {
+        // does anything want to sync with this cohort?
+        if (!$instances = $DB->get_records('enrol', array('customint1'=>$ca->cohortid, 'enrol'=>'cohort'), 'id ASC')) {
             return true;
         }
 
         $plugin = enrol_get_plugin('cohort');
-        foreach ($enrols as $enrol) {
-            // no problem if already enrolled
-            $plugin->unenrol_user($enrol, $ca->userid);
+        $unenrolaction = $plugin->get_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
+
+        foreach ($instances as $instance) {
+            if (!$ue = $DB->get_record('user_enrolments', array('enrolid'=>$instance->id, 'userid'=>$ca->userid))) {
+                continue;
+            }
+            if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) {
+                $plugin->unenrol_user($instance, $ca->userid);
+
+            } else {
+                if ($ue->status != ENROL_USER_SUSPENDED) {
+                    $plugin->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED);
+                    $context = context_course::instance($instance->courseid);
+                    role_unassign_all(array('userid'=>$ue->userid, 'contextid'=>$context->id, 'component'=>'enrol_cohort', 'itemid'=>$instance->id));
+                }
+            }
         }
 
         return true;
     }
 
+    /**
+     * Event processor - cohort deleted
+     * @param stdClass $cohort
+     * @return bool
+     */
     public function deleted($cohort) {
         global $DB;
 
-        // does anything want to sync with this parent?
-        if (!$enrols = $DB->get_records('enrol', array('customint1'=>$cohort->id, 'enrol'=>'cohort'), 'id ASC')) {
+        // does anything want to sync with this cohort?
+        if (!$instances = $DB->get_records('enrol', array('customint1'=>$cohort->id, 'enrol'=>'cohort'), 'id ASC')) {
             return true;
         }
 
         $plugin = enrol_get_plugin('cohort');
-        foreach ($enrols as $enrol) {
-            $plugin->delete_instance($enrol);
+        $unenrolaction = $plugin->get_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
+
+        foreach ($instances as $instance) {
+            if ($unenrolaction == ENROL_EXT_REMOVED_SUSPENDNOROLES) {
+                $context = context_course::instance($instance->courseid);
+                role_unassign_all(array('contextid'=>$context->id, 'component'=>'enrol_cohort', 'itemid'=>$instance->id));
+                $plugin->update_status($instance, ENROL_INSTANCE_DISABLED);
+            } else {
+                $plugin->delete_instance($instance);
+            }
         }
 
         return true;
     }
 }
 
+
 /**
  * Sync all cohort course links.
  * @param int $courseid one course, empty mean all
- * @return void
+ * @param bool $verbose verbose CLI output
+ * @return int 0 means ok, 1 means error, 2 means plugin disabled
  */
-function enrol_cohort_sync($courseid = NULL) {
+function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     global $CFG, $DB;
 
-    // unfortunately this may take a long time
-    @set_time_limit(0); //if this fails during upgrade we can continue from cron, no big deal
+    // purge all roles if cohort sync disabled, those can be recreated later here by cron or CLI
+    if (!enrol_is_enabled('cohort')) {
+        if ($verbose) {
+            mtrace('Cohort sync plugin is disabled, unassigning all plugin roles and stopping.');
+        }
+        role_unassign_all(array('component'=>'enrol_cohort'));
+        return 2;
+    }
 
-    $cohort = enrol_get_plugin('cohort');
+    // unfortunately this may take a long time, this script can be interrupted without problems
+    @set_time_limit(0);
+    raise_memory_limit(MEMORY_HUGE);
+
+    if ($verbose) {
+        mtrace('Starting user enrolment synchronisation...');
+    }
+
+    $allroles = get_all_roles();
+    $instances = array(); //cache
+
+    $plugin = enrol_get_plugin('cohort');
+    $unenrolaction = $plugin->get_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
 
-    $onecourse = $courseid ? "AND e.courseid = :courseid" : "";
 
     // iterate through all not enrolled yet users
-    if (enrol_is_enabled('cohort')) {
-        $params = array();
-        $onecourse = "";
-        if ($courseid) {
-            $params['courseid'] = $courseid;
-            $onecourse = "AND e.courseid = :courseid";
+    $onecourse = $courseid ? "AND e.courseid = :courseid" : "";
+    $sql = "SELECT cm.userid, e.id AS enrolid, ue.status
+              FROM {cohort_members} cm
+              JOIN {enrol} e ON (e.customint1 = cm.cohortid AND e.enrol = 'cohort' $onecourse)
+         LEFT JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = cm.userid)
+             WHERE ue.id IS NULL OR ue.status = :suspended";
+    $params = array();
+    $params['courseid'] = $courseid;
+    $params['suspended'] = ENROL_USER_SUSPENDED;
+    $rs = $DB->get_recordset_sql($sql, $params);
+    foreach($rs as $ue) {
+        if (!isset($instances[$ue->enrolid])) {
+            $instances[$ue->enrolid] = $DB->get_record('enrol', array('id'=>$ue->enrolid));
         }
-        $sql = "SELECT cm.userid, e.id AS enrolid
-                  FROM {cohort_members} cm
-                  JOIN {enrol} e ON (e.customint1 = cm.cohortid AND e.status = :statusenabled AND e.enrol = 'cohort' $onecourse)
-             LEFT JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = cm.userid)
-                 WHERE ue.id IS NULL";
-        $params['statusenabled'] = ENROL_INSTANCE_ENABLED;
-        $params['courseid'] = $courseid;
-        $rs = $DB->get_recordset_sql($sql, $params);
-        $instances = array(); //cache
-        foreach($rs as $ue) {
-            if (!isset($instances[$ue->enrolid])) {
-                $instances[$ue->enrolid] = $DB->get_record('enrol', array('id'=>$ue->enrolid));
+        $instance = $instances[$ue->enrolid];
+        if ($ue->status == ENROL_USER_SUSPENDED) {
+            $plugin->update_user_enrol($instance, $ue->userid, ENROL_USER_ACTIVE);
+            if ($verbose) {
+                mtrace("  unsuspending: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
+            }
+        } else {
+            $plugin->enrol_user($instance, $ue->userid);
+            if ($verbose) {
+                mtrace("  enrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
             }
-            $cohort->enrol_user($instances[$ue->enrolid], $ue->userid);
         }
-        $rs->close();
-        unset($instances);
     }
+    $rs->close();
+
 
-    // unenrol as necessary - ignore enabled flag, we want to get rid of all
-    $sql = "SELECT ue.userid, e.id AS enrolid
+    // unenrol as necessary
+    $sql = "SELECT ue.*, e.courseid
               FROM {user_enrolments} ue
               JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'cohort' $onecourse)
-         LEFT JOIN {cohort_members} cm ON (cm.cohortid  = e.customint1 AND cm.userid = ue.userid)
+         LEFT JOIN {cohort_members} cm ON (cm.cohortid = e.customint1 AND cm.userid = ue.userid)
              WHERE cm.id IS NULL";
-    //TODO: this may use a bit of SQL optimisation
     $rs = $DB->get_recordset_sql($sql, array('courseid'=>$courseid));
-    $instances = array(); //cache
     foreach($rs as $ue) {
         if (!isset($instances[$ue->enrolid])) {
             $instances[$ue->enrolid] = $DB->get_record('enrol', array('id'=>$ue->enrolid));
         }
-        $cohort->unenrol_user($instances[$ue->enrolid], $ue->userid);
+        $instance = $instances[$ue->enrolid];
+        if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) {
+            // remove enrolment together with group membership, grades, preferences, etc.
+            $plugin->unenrol_user($instance, $ue->userid);
+            if ($verbose) {
+                mtrace("  unenrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
+            }
+
+        } else { // ENROL_EXT_REMOVED_SUSPENDNOROLES
+            // just disable and ignore any changes
+            if ($ue->status != ENROL_USER_SUSPENDED) {
+                $plugin->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED);
+                $context = context_course::instance($instance->courseid);
+                role_unassign_all(array('userid'=>$ue->userid, 'contextid'=>$context->id, 'component'=>'enrol_cohort', 'itemid'=>$instance->id));
+                if ($verbose) {
+                    mtrace("  suspending and unsassigning all roles: $ue->userid ==> $instance->courseid");
+                }
+            }
+        }
     }
     $rs->close();
     unset($instances);
 
-    // now assign all necessary roles
-    if (enrol_is_enabled('cohort')) {
-        $sql = "SELECT e.roleid, ue.userid, c.id AS contextid, e.id AS itemid
-                  FROM {user_enrolments} ue
-                  JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'cohort' AND e.status = :statusenabled $onecourse)
-                  JOIN {context} c ON (c.instanceid = e.courseid AND c.contextlevel = :coursecontext)
-             LEFT JOIN {role_assignments} ra ON (ra.contextid = c.id AND ra.userid = ue.userid AND ra.itemid = e.id AND ra.component = 'enrol_cohort' AND e.roleid = ra.roleid)
-                 WHERE ra.id IS NULL";
-        $params = array();
-        $params['statusenabled'] = ENROL_INSTANCE_ENABLED;
-        $params['coursecontext'] = CONTEXT_COURSE;
-        $params['courseid'] = $courseid;
-
-        $rs = $DB->get_recordset_sql($sql, $params);
-        foreach($rs as $ra) {
-            role_assign($ra->roleid, $ra->userid, $ra->contextid, 'enrol_cohort', $ra->itemid);
+
+    // now assign all necessary roles to enrolled users - skip suspended instances and users
+    $onecourse = $courseid ? "AND e.courseid = :courseid" : "";
+    $sql = "SELECT e.roleid, ue.userid, c.id AS contextid, e.id AS itemid, e.courseid
+              FROM {user_enrolments} ue
+              JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'cohort' AND e.status = :statusenabled $onecourse)
+              JOIN {role} r ON (r.id = e.roleid)
+              JOIN {context} c ON (c.instanceid = e.courseid AND c.contextlevel = :coursecontext)
+         LEFT JOIN {role_assignments} ra ON (ra.contextid = c.id AND ra.userid = ue.userid AND ra.itemid = e.id AND ra.component = 'enrol_cohort' AND e.roleid = ra.roleid)
+             WHERE ue.status = :useractive AND ra.id IS NULL";
+    $params = array();
+    $params['statusenabled'] = ENROL_INSTANCE_ENABLED;
+    $params['useractive'] = ENROL_USER_ACTIVE;
+    $params['coursecontext'] = CONTEXT_COURSE;
+    $params['courseid'] = $courseid;
+
+    $rs = $DB->get_recordset_sql($sql, $params);
+    foreach($rs as $ra) {
+        role_assign($ra->roleid, $ra->userid, $ra->contextid, 'enrol_cohort', $ra->itemid);
+        if ($verbose) {
+            mtrace("  assigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname);
         }
-        $rs->close();
     }
+    $rs->close();
+
 
-    // remove unwanted roles - include ignored roles and disabled plugins too
-    $onecourse = $courseid ? "AND c.instanceid = :courseid" : "";
-    $sql = "SELECT ra.roleid, ra.userid, ra.contextid, ra.itemid
+    // remove unwanted roles - sync role can not be changed, we only remove role when unenrolled
+    $onecourse = $courseid ? "AND e.courseid = :courseid" : "";
+    $sql = "SELECT ra.roleid, ra.userid, ra.contextid, ra.itemid, e.courseid
               FROM {role_assignments} ra
-              JOIN {context} c ON (c.id = ra.contextid AND c.contextlevel = :coursecontext $onecourse)
-         LEFT JOIN (SELECT e.id AS enrolid, e.roleid, ue.userid
-                      FROM {user_enrolments} ue
-                      JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'cohort')
-                   ) x ON (x.enrolid = ra.itemid AND ra.component = 'enrol_cohort' AND x.roleid = ra.roleid AND x.userid = ra.userid)
-             WHERE x.userid IS NULL AND ra.component = 'enrol_cohort'";
-    $params = array('coursecontext' => CONTEXT_COURSE, 'courseid' => $courseid);
+              JOIN {context} c ON (c.id = ra.contextid AND c.contextlevel = :coursecontext)
+              JOIN {enrol} e ON (e.id = ra.itemid AND e.enrol = 'cohort' $onecourse)
+         LEFT JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = ra.userid AND ue.status = :useractive)
+             WHERE ra.component = 'enrol_cohort' AND (ue.id IS NULL OR e.status <> :statusenabled)";
+    $params = array();
+    $params['statusenabled'] = ENROL_INSTANCE_ENABLED;
+    $params['useractive'] = ENROL_USER_ACTIVE;
+    $params['coursecontext'] = CONTEXT_COURSE;
+    $params['courseid'] = $courseid;
 
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $ra) {
         role_unassign($ra->roleid, $ra->userid, $ra->contextid, 'enrol_cohort', $ra->itemid);
+        if ($verbose) {
+            mtrace("  unassigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname);
+        }
     }
     $rs->close();
 
+
+    if ($verbose) {
+        mtrace('...user enrolment synchronisation finished.');
+    }
+
+    return 0;
 }
 
 /**
@@ -326,7 +429,7 @@ function enrol_cohort_search_cohorts(course_enrolment_manager $manager, $offset
     // Add some additional sensible conditions
     $tests = array('contextid ' . $sqlparents);
 
-    // Modify the quesry to perform the search if requred
+    // Modify the query to perform the search if required
     if (!empty($search)) {
         $conditions = array(
             'name',

diff --git a/enrol/cohort/settings.php b/enrol/cohort/settings.php
index 6856d08..8bfc71c 100644 (file)
--- a/enrol/cohort/settings.php
+++ b/enrol/cohort/settings.php
@@ -1,5 +1,4 @@
 <?php
-
 // This file is part of Moodle - http://moodle.org/
 //
 // Moodle is free software: you can redistribute it and/or modify
@@ -39,6 +38,11 @@ if ($ADMIN->fulltree) {
         $student = reset($student);
         $settings->add(new admin_setting_configselect('enrol_cohort/roleid',
             get_string('defaultrole', 'role'), '', $student->id, $options));
+
+        $options = array(
+            ENROL_EXT_REMOVED_UNENROL        => get_string('extremovedunenrol', 'enrol'),
+            ENROL_EXT_REMOVED_SUSPENDNOROLES => get_string('extremovedsuspendnoroles', 'enrol'));
+        $settings->add(new admin_setting_configselect('enrol_cohort/unenrolaction', get_string('extremovedaction', 'enrol'), get_string('extremovedaction_help', 'enrol'), ENROL_EXT_REMOVED_UNENROL, $options));
     }
 }
 

diff --git a/enrol/cohort/version.php b/enrol/cohort/version.php
index 34200b6..ef4adcb 100644 (file)
--- a/enrol/cohort/version.php
+++ b/enrol/cohort/version.php
@@ -25,7 +25,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2011112900;        // The current plugin version (Date: YYYYMMDDXX)
+$plugin->version   = 2011112901;        // The current plugin version (Date: YYYYMMDDXX)
 $plugin->requires  = 2011112900;        // Requires this Moodle version
 $plugin->component = 'enrol_cohort';    // Full name of the plugin (used for diagnostics)
-$plugin->cron      = 60;
\ No newline at end of file
+$plugin->cron      = 60*60;             // run cron every hour by default, it is not out-of-sync often
\ No newline at end of file