From 147b9760da5c5cbdd2ba68cd2656589edc555f29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Mon, 13 Aug 2012 22:31:36 +0200
Subject: [PATCH 1/5] MDL-34864 improve enrol_category sync performance

---
 enrol/category/locallib.php | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/enrol/category/locallib.php b/enrol/category/locallib.php
index b44f9d8323b..57675976197 100644
--- a/enrol/category/locallib.php
+++ b/enrol/category/locallib.php
@@ -265,6 +265,7 @@ function enrol_category_sync_full() {
 
     // first of all add necessary enrol instances to all courses
     $parentcat = $DB->sql_concat("cat.path", "'/%'");
+    $parentcctx = $DB->sql_concat("cctx.path", "'/%'");
     // need whole course records to be used by add_instance(), use inner view (ci) to
     // get distinct records only.
     // TODO: Moodle 2.1. Improve enrol API to accept courseid / courserec
@@ -293,12 +294,11 @@ function enrol_category_sync_full() {
     $sql = "SELECT e.*
               FROM {enrol} e
               JOIN {context} ctx ON (ctx.instanceid = e.courseid AND ctx.contextlevel = :courselevel)
-         LEFT JOIN (SELECT DISTINCT cctx.path
-                      FROM {course_categories} cc
+         LEFT JOIN ({course_categories} cc
                       JOIN {context} cctx ON (cctx.instanceid = cc.id AND cctx.contextlevel = :catlevel)
                       JOIN {role_assignments} ra ON (ra.contextid = cctx.id AND ra.roleid $roleids)
-                   ) cat ON (ctx.path LIKE $parentcat)
-             WHERE e.enrol = 'category' AND cat.path IS NULL";
+                   ) ON (ctx.path LIKE $parentcctx)
+             WHERE e.enrol = 'category' AND cc.id IS NULL";
 
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $instance) {
@@ -333,12 +333,11 @@ function enrol_category_sync_full() {
               FROM {enrol} e
               JOIN {context} ctx ON (ctx.instanceid = e.courseid AND ctx.contextlevel = :courselevel)
               JOIN {user_enrolments} ue ON (ue.enrolid = e.id)
-         LEFT JOIN (SELECT DISTINCT cctx.path, ra.userid
-                      FROM {course_categories} cc
+         LEFT JOIN ({course_categories} cc
                       JOIN {context} cctx ON (cctx.instanceid = cc.id AND cctx.contextlevel = :catlevel)
                       JOIN {role_assignments} ra ON (ra.contextid = cctx.id AND ra.roleid $roleids)
-                   ) cat ON (ctx.path LIKE $parentcat AND cat.userid = ue.userid)
-             WHERE e.enrol = 'category' AND cat.userid IS NULL";
+                   ) ON (ctx.path LIKE $parentcctx AND ra.userid = ue.userid)
+             WHERE e.enrol = 'category' AND cc.id IS NULL";
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $instance) {
         $userid = $instance->userid;

From 455748c882ed632d4bcfab0167f5c575ce9af9cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Tue, 14 Aug 2012 21:57:50 +0200
Subject: [PATCH 2/5] MDL-34864 add verbose option to CLI enrol_category sync

---
 enrol/category/cli/sync.php | 32 +++++++++++++++++++++++++++++---
 enrol/category/locallib.php | 28 +++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/enrol/category/cli/sync.php b/enrol/category/cli/sync.php
index 47b1e37477e..56a4b82d4dd 100644
--- a/enrol/category/cli/sync.php
+++ b/enrol/category/cli/sync.php
@@ -36,9 +36,35 @@ define('CLI_SCRIPT', true);
 
 require(dirname(dirname(dirname(dirname(__FILE__)))).'/config.php');
 require_once("$CFG->dirroot/enrol/category/locallib.php");
+require_once("$CFG->libdir/clilib.php");
 
-if (!enrol_is_enabled('category')) {
-     die('enrol_category plugin is disabled, sync is disabled');
+// 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));
 }
 
-enrol_category_sync_full();
+if ($options['help']) {
+    $help =
+        "Execute course category enrolment sync.
+
+Options:
+-v, --verbose         Print verbose progess information
+-h, --help            Print out this help
+
+Example:
+\$ sudo -u www-data /usr/bin/php enrol/category/cli/sync.php
+";
+    echo $help;
+    die;
+}
+
+
+if (!enrol_is_enabled('category')) {
+    cli_error('enrol_category plugin is disabled, synchronisation stopped');
+}
+
+$verbose = !empty($options['verbose']);
+return enrol_category_sync_full($verbose);
diff --git a/enrol/category/locallib.php b/enrol/category/locallib.php
index 57675976197..3ba62bb7035 100644
--- a/enrol/category/locallib.php
+++ b/enrol/category/locallib.php
@@ -233,12 +233,12 @@ function enrol_category_sync_course($course) {
     }
 }
 
-function enrol_category_sync_full() {
+function enrol_category_sync_full($verbose = false) {
     global $DB;
 
 
     if (!enrol_is_enabled('category')) {
-        return;
+        return 2;
     }
 
     // we may need a lot of time here
@@ -251,12 +251,22 @@ function enrol_category_sync_full() {
     // any interesting roles worth synchronising?
     if (!$roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext)) {
         // yay, nothing to do, so let's remove all leftovers
+        if ($verbose) {
+            mtrace("No roles with 'enrol/category:synchronised' capability found.");
+        }
         if ($instances = $DB->get_records('enrol', array('enrol'=>'category'))) {
             foreach ($instances as $instance) {
+                if ($verbose) {
+                    mtrace("  deleting category enrol instance from course {$instance->courseid}");
+                }
                 $plugin->delete_instance($instance);
             }
         }
-        return;
+        return 0;
+    }
+    $rolenames = role_fix_names($roles, null, ROLENAME_SHORT, true);
+    if ($verbose) {
+        mtrace('Synchronising category enrolments for roles: '.implode(', ', $rolenames).'...');
     }
 
     list($roleids, $params) = $DB->get_in_or_equal(array_keys($roles), SQL_PARAMS_NAMED, 'r');
@@ -325,6 +335,9 @@ function enrol_category_sync_full() {
         unset($instance->userid);
         unset($instance->estart);
         $plugin->enrol_user($instance, $userid, null, $estart);
+        if ($verbose) {
+            mtrace("  enrolling: user $userid ==> course $instance->courseid");
+        }
     }
     $rs->close();
 
@@ -343,6 +356,15 @@ function enrol_category_sync_full() {
         $userid = $instance->userid;
         unset($instance->userid);
         $plugin->unenrol_user($instance, $userid);
+        if ($verbose) {
+            mtrace("  unenrolling: user $userid ==> course $instance->courseid");
+        }
     }
     $rs->close();
+
+    if ($verbose) {
+        mtrace('...user enrolment synchronisation finished.');
+    }
+
+    return 0;
 }

From 6c326e2b19b52b9f265b48f42f26db0713008bf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Tue, 14 Aug 2012 22:16:36 +0200
Subject: [PATCH 3/5] MDL-34864 improve enrol_category phpdocs

---
 enrol/category/cli/sync.php               |  8 +--
 enrol/category/db/access.php              |  4 +-
 enrol/category/db/events.php              |  6 +-
 enrol/category/db/install.php             |  4 +-
 enrol/category/lang/en/enrol_category.php |  6 +-
 enrol/category/lib.php                    | 25 ++++---
 enrol/category/locallib.php               | 80 +++++++++++++++--------
 enrol/category/settings.php               |  7 +-
 enrol/category/version.php                |  5 +-
 9 files changed, 77 insertions(+), 68 deletions(-)

diff --git a/enrol/category/cli/sync.php b/enrol/category/cli/sync.php
index 56a4b82d4dd..7db80b80acc 100644
--- a/enrol/category/cli/sync.php
+++ b/enrol/category/cli/sync.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
@@ -19,15 +18,14 @@
  * CLI sync for full category enrol synchronisation.
  *
  * Sample execution:
- * $sudo -u www-data /usr/bin/php /var/www/moodle/enrol/category/cli/sync.php
+ * $ sudo -u www-data /usr/bin/php /var/www/moodle/enrol/category/cli/sync.php
  *
  * 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 category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -63,7 +61,7 @@ Example:
 
 
 if (!enrol_is_enabled('category')) {
-    cli_error('enrol_category plugin is disabled, synchronisation stopped');
+    cli_error('enrol_category plugin is disabled, synchronisation stopped', 2);
 }
 
 $verbose = !empty($options['verbose']);
diff --git a/enrol/category/db/access.php b/enrol/category/db/access.php
index 1a9ead040d0..7feea4dad1f 100644
--- a/enrol/category/db/access.php
+++ b/enrol/category/db/access.php
@@ -25,9 +25,9 @@
 defined('MOODLE_INTERNAL') || die();
 
 $capabilities = array(
-    // marks roles that have category role assignments synchronised to course enrolments
+    // Marks roles that have category role assignments synchronised to course enrolments
     // overrides below system context are ignored (for performance reasons).
-    // by default his is not allowed in new installs, admins have to explicitly allow category enrolments
+    // By default his is not allowed in new installs, admins have to explicitly allow category enrolments.
     'enrol/category:synchronised' => array(
         'captype' => 'write',
         'contextlevel' => CONTEXT_SYSTEM,
diff --git a/enrol/category/db/events.php b/enrol/category/db/events.php
index 66194df447d..3a678664b0a 100644
--- a/enrol/category/db/events.php
+++ b/enrol/category/db/events.php
@@ -17,10 +17,10 @@
 /**
  * Category enrolment plugin event handler definition.
  *
- * @package enrol_category
- * @category event
+ * @package   enrol_category
+ * @category  event
  * @copyright 2010 Petr Skoda {@link http://skodak.org}
- * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 defined('MOODLE_INTERNAL') || die();
diff --git a/enrol/category/db/install.php b/enrol/category/db/install.php
index 0de987f41e0..834aa6779c7 100644
--- a/enrol/category/db/install.php
+++ b/enrol/category/db/install.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
@@ -18,8 +17,7 @@
 /**
  * category enrolment plugin installation.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
diff --git a/enrol/category/lang/en/enrol_category.php b/enrol/category/lang/en/enrol_category.php
index 5a8bee30027..417b28d87f6 100644
--- a/enrol/category/lang/en/enrol_category.php
+++ b/enrol/category/lang/en/enrol_category.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
@@ -16,10 +15,9 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
- * Strings for component 'enrol_category', language 'en', branch 'MOODLE_20_STABLE'
+ * Strings for component 'enrol_category', language 'en'.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda  {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
diff --git a/enrol/category/lib.php b/enrol/category/lib.php
index ba60ba049f7..e6525e36b56 100644
--- a/enrol/category/lib.php
+++ b/enrol/category/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
@@ -18,25 +17,25 @@
 /**
  * Category enrolment plugin.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 defined('MOODLE_INTERNAL') || die();
 
+
 /**
  * category enrolment plugin implementation.
- * @author Petr Skoda
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @author  Petr Skoda
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class enrol_category_plugin extends enrol_plugin {
 
    /**
      * Is it possible to delete enrol instance via standard UI?
      *
-     * @param object $instance
+     * @param stdClass $instance
      * @return bool
      */
     public function instance_deleteable($instance) {
@@ -45,7 +44,7 @@ class enrol_category_plugin extends enrol_plugin {
         if (!enrol_is_enabled('category')) {
             return true;
         }
-        // allow delete only when no synced users here
+        // Allow delete only when no synced users here.
         return !$DB->record_exists('user_enrolments', array('enrolid'=>$instance->id));
     }
 
@@ -55,8 +54,8 @@ class enrol_category_plugin extends enrol_plugin {
      * @return moodle_url page url
      */
     public function get_newinstance_link($courseid) {
-        // instances are added automatically as necessary
-        return NULL;
+        // Instances are added automatically as necessary.
+        return null;
     }
 
     /**
@@ -78,8 +77,8 @@ class enrol_category_plugin extends enrol_plugin {
      * Called after updating/inserting course.
      *
      * @param bool $inserted true if course just inserted
-     * @param object $course
-     * @param object $data form data
+     * @param stdClass $course
+     * @param stdClass $data form data
      * @return void
      */
     public function course_updated($inserted, $course, $data) {
@@ -89,10 +88,8 @@ class enrol_category_plugin extends enrol_plugin {
             return;
         }
 
-        // sync category enrols
+        // Sync category enrols.
         require_once("$CFG->dirroot/enrol/category/locallib.php");
         enrol_category_sync_course($course);
     }
 }
-
-
diff --git a/enrol/category/locallib.php b/enrol/category/locallib.php
index 3ba62bb7035..2c16f766203 100644
--- a/enrol/category/locallib.php
+++ b/enrol/category/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
@@ -18,14 +17,14 @@
 /**
  * Local stuff for category enrolment plugin.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 defined('MOODLE_INTERNAL') || die();
 
+
 /**
  * Event handler for category enrolment plugin.
  *
@@ -33,6 +32,12 @@ defined('MOODLE_INTERNAL') || die();
  * it may fail sometimes, so we always do a full sync in cron too.
  */
 class enrol_category_handler {
+    /**
+     * Triggered when user is assigned a new role.
+     * @static
+     * @param stdClass $ra
+     * @return bool
+     */
     public static function role_assigned($ra) {
         global $DB;
 
@@ -40,20 +45,20 @@ class enrol_category_handler {
             return true;
         }
 
-        //only category level roles are interesting
+        // Only category level roles are interesting.
         $parentcontext = get_context_instance_by_id($ra->contextid);
         if ($parentcontext->contextlevel != CONTEXT_COURSECAT) {
             return true;
         }
 
-        // make sure the role is to be actually synchronised
-        // please note we are ignoring overrides of the synchronised capability (for performance reasons in full sync)
+        // Make sure the role is to be actually synchronised,
+        // please note we are ignoring overrides of the synchronised capability (for performance reasons in full sync).
         $syscontext = context_system::instance();
         if (!$DB->record_exists('role_capabilities', array('contextid'=>$syscontext->id, 'roleid'=>$ra->roleid, 'capability'=>'enrol/category:synchronised', 'permission'=>CAP_ALLOW))) {
             return true;
         }
 
-        // add necessary enrol instances
+        // Add necessary enrol instances.
         $plugin = enrol_get_plugin('category');
         $sql = "SELECT c.*
                   FROM {course} c
@@ -67,7 +72,7 @@ class enrol_category_handler {
         }
         $rs->close();
 
-        // now look for missing enrols
+        // Now look for missing enrolments.
         $sql = "SELECT e.*
                   FROM {course} c
                   JOIN {context} ctx ON (ctx.instanceid = c.id AND ctx.contextlevel = :courselevel AND ctx.path LIKE :match)
@@ -84,6 +89,12 @@ class enrol_category_handler {
         return true;
     }
 
+    /**
+     * Triggered when user role is unassigned.
+     * @static
+     * @param stdClass $ra
+     * @return bool
+     */
     public static function role_unassigned($ra) {
         global $DB;
 
@@ -91,13 +102,13 @@ class enrol_category_handler {
             return true;
         }
 
-        // only category level roles are interesting
+        // Only category level roles are interesting.
         $parentcontext = get_context_instance_by_id($ra->contextid);
         if ($parentcontext->contextlevel != CONTEXT_COURSECAT) {
             return true;
         }
 
-        // now this is going to be a bit slow, take all enrolments in child courses and verify each separately
+        // Now this is going to be a bit slow, take all enrolments in child courses and verify each separately.
         $syscontext = context_system::instance();
         if (!$roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext)) {
             return true;
@@ -119,7 +130,7 @@ class enrol_category_handler {
         foreach ($rs as $instance) {
             $coursecontext = context_course::instance($instance->courseid);
             $contextids = get_parent_contexts($coursecontext);
-            array_pop($contextids); // remove system context, we are interested in categories only
+            array_pop($contextids); // Remove system context, we are interested in categories only.
 
             list($contextids, $contextparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'c');
             $params = array_merge($params, $contextparams);
@@ -128,7 +139,7 @@ class enrol_category_handler {
                       FROM {role_assignments} ra
                      WHERE ra.userid = :userid AND ra.contextid $contextids AND ra.roleid $roleids";
             if (!$DB->record_exists_sql($sql, $params)) {
-                // user does not have any interesting role in any parent context, let's unenrol
+                // User does not have any interesting role in any parent context, let's unenrol.
                 $plugin->unenrol_user($instance, $ra->userid);
             }
         }
@@ -140,7 +151,7 @@ class enrol_category_handler {
 
 /**
  * Sync all category enrolments in one course
- * @param int $courseid course id
+ * @param stdClass $course
  * @return void
  */
 function enrol_category_sync_course($course) {
@@ -156,7 +167,7 @@ function enrol_category_sync_course($course) {
     $roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext);
 
     if (!$roles) {
-        //nothing to sync, so remove the instance completely if exists
+        // Nothing to sync, so remove the instance completely if exists.
         if ($instances = $DB->get_records('enrol', array('courseid'=>$course->id, 'enrol'=>'category'))) {
             foreach ($instances as $instance) {
                 $plugin->delete_instance($instance);
@@ -165,10 +176,10 @@ function enrol_category_sync_course($course) {
         return;
     }
 
-    // first find out if any parent category context contains interesting role assignments
+    // First find out if any parent category context contains interesting role assignments.
     $coursecontext = context_course::instance($course->id);
     $contextids = get_parent_contexts($coursecontext);
-    array_pop($contextids); // remove system context, we are interested in categories only
+    array_pop($contextids); // Remove system context, we are interested in categories only.
 
     list($roleids, $params) = $DB->get_in_or_equal(array_keys($roles), SQL_PARAMS_NAMED, 'r');
     list($contextids, $contextparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED, 'c');
@@ -180,7 +191,7 @@ function enrol_category_sync_course($course) {
              WHERE roleid $roleids AND contextid $contextids";
     if (!$DB->record_exists_sql($sql, $params)) {
         if ($instances = $DB->get_records('enrol', array('courseid'=>$course->id, 'enrol'=>'category'))) {
-            // should be max one instance, but anyway
+            // Should be max one instance, but anyway.
             foreach ($instances as $instance) {
                 $plugin->delete_instance($instance);
             }
@@ -188,7 +199,7 @@ function enrol_category_sync_course($course) {
         return;
     }
 
-    // make sure the enrol instance exists - there should be always only one instance
+    // Make sure the enrol instance exists - there should be always only one instance.
     $delinstances = array();
     if ($instances = $DB->get_records('enrol', array('courseid'=>$course->id, 'enrol'=>'category'))) {
         $instance = array_shift($instances);
@@ -198,7 +209,7 @@ function enrol_category_sync_course($course) {
         $instance = $DB->get_record('enrol', array('id'=>$i));
     }
 
-    // add new enrolments
+    // Add new enrolments.
     $sql = "SELECT ra.userid, ra.estart
               FROM (SELECT xra.userid, MIN(xra.timemodified) AS estart
                       FROM {role_assignments} xra
@@ -214,7 +225,7 @@ function enrol_category_sync_course($course) {
     }
     $rs->close();
 
-    // remove unwanted enrolments
+    // Remove unwanted enrolments.
     $sql = "SELECT DISTINCT ue.userid
               FROM {user_enrolments} ue
          LEFT JOIN {role_assignments} ra ON (ra.roleid $roleids AND ra.contextid $contextids AND ra.userid = ue.userid)
@@ -226,13 +237,24 @@ function enrol_category_sync_course($course) {
     $rs->close();
 
     if ($delinstances) {
-        // we have to do this as the last step in order to prevent temporary unenrolment
+        // We have to do this as the last step in order to prevent temporary unenrolment.
         foreach ($delinstances as $delinstance) {
             $plugin->delete_instance($delinstance);
         }
     }
 }
 
+/**
+ * Synchronise courses in all categories.
+ *
+ * It gets out-of-sync if:
+ * - you move course to different category
+ * - reorder categories
+ * - disable enrol_category and enable it again
+ *
+ * @param bool $verbose
+ * @return int exit code - 0 is ok, 1 means error, 2 if plugin disabled
+ */
 function enrol_category_sync_full($verbose = false) {
     global $DB;
 
@@ -241,14 +263,14 @@ function enrol_category_sync_full($verbose = false) {
         return 2;
     }
 
-    // we may need a lot of time here
+    // We may need a lot of time here.
     @set_time_limit(0);
 
     $plugin = enrol_get_plugin('category');
 
     $syscontext = context_system::instance();
 
-    // any interesting roles worth synchronising?
+    // Any interesting roles worth synchronising?
     if (!$roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext)) {
         // yay, nothing to do, so let's remove all leftovers
         if ($verbose) {
@@ -273,10 +295,10 @@ function enrol_category_sync_full($verbose = false) {
     $params['courselevel'] = CONTEXT_COURSE;
     $params['catlevel'] = CONTEXT_COURSECAT;
 
-    // first of all add necessary enrol instances to all courses
+    // First of all add necessary enrol instances to all courses.
     $parentcat = $DB->sql_concat("cat.path", "'/%'");
     $parentcctx = $DB->sql_concat("cctx.path", "'/%'");
-    // need whole course records to be used by add_instance(), use inner view (ci) to
+    // Need whole course records to be used by add_instance(), use inner view (ci) to
     // get distinct records only.
     // TODO: Moodle 2.1. Improve enrol API to accept courseid / courserec
     $sql = "SELECT c.*
@@ -299,8 +321,8 @@ function enrol_category_sync_full($verbose = false) {
     }
     $rs->close();
 
-    // now look for courses that do not have any interesting roles in parent contexts,
-    // but still have the instance and delete them
+    // Now look for courses that do not have any interesting roles in parent contexts,
+    // but still have the instance and delete them.
     $sql = "SELECT e.*
               FROM {enrol} e
               JOIN {context} ctx ON (ctx.instanceid = e.courseid AND ctx.contextlevel = :courselevel)
@@ -316,7 +338,7 @@ function enrol_category_sync_full($verbose = false) {
     }
     $rs->close();
 
-    // add missing enrolments
+    // Add missing enrolments.
     $sql = "SELECT e.*, cat.userid, cat.estart
               FROM {enrol} e
               JOIN {context} ctx ON (ctx.instanceid = e.courseid AND ctx.contextlevel = :courselevel)
@@ -341,7 +363,7 @@ function enrol_category_sync_full($verbose = false) {
     }
     $rs->close();
 
-    // remove stale enrolments
+    // Remove stale enrolments.
     $sql = "SELECT e.*, ue.userid
               FROM {enrol} e
               JOIN {context} ctx ON (ctx.instanceid = e.courseid AND ctx.contextlevel = :courselevel)
diff --git a/enrol/category/settings.php b/enrol/category/settings.php
index a8542672406..484c23b1d87 100644
--- a/enrol/category/settings.php
+++ b/enrol/category/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
@@ -16,10 +15,9 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
- * category enrolment plugin settings and presets.
+ * Category enrolment plugin settings and presets.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -35,4 +33,3 @@ if ($ADMIN->fulltree) {
     //--- enrol instance defaults ----------------------------------------------------------------------------
 
 }
-
diff --git a/enrol/category/version.php b/enrol/category/version.php
index 371a7f06dcb..73840f9ca28 100644
--- a/enrol/category/version.php
+++ b/enrol/category/version.php
@@ -17,8 +17,7 @@
 /**
  * Category enrolment plugin version specification.
  *
- * @package    enrol
- * @subpackage category
+ * @package    enrol_category
  * @copyright  2010 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -28,4 +27,4 @@ defined('MOODLE_INTERNAL') || die();
 $plugin->version   = 2012061700;        // The current plugin version (Date: YYYYMMDDXX)
 $plugin->requires  = 2012061700;        // Requires this Moodle version
 $plugin->component = 'enrol_category';  // Full name of the plugin (used for diagnostics)
-$plugin->cron      = 60;
\ No newline at end of file
+$plugin->cron      = 60;

From 4b0b9f1d9df2985a0d2d0800e0fa5acb2d08c753 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Wed, 15 Aug 2012 11:43:27 +0200
Subject: [PATCH 4/5] MDL-34864 add enrol_category unit tests for sync

---
 enrol/category/tests/sync_test.php | 365 +++++++++++++++++++++++++++++
 1 file changed, 365 insertions(+)
 create mode 100644 enrol/category/tests/sync_test.php

diff --git a/enrol/category/tests/sync_test.php b/enrol/category/tests/sync_test.php
new file mode 100644
index 00000000000..36497647ce3
--- /dev/null
+++ b/enrol/category/tests/sync_test.php
@@ -0,0 +1,365 @@
+<?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/>.
+
+/**
+ * Category enrolment sync functional test.
+ *
+ * @package    enrol_category
+ * @category   phpunit
+ * @copyright  2012 Petr Skoda {@link http://skodak.org}
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot.'/enrol/category/locallib.php');
+
+class enrol_category_testcase extends advanced_testcase {
+
+    protected function enable_plugin() {
+        $enabled = enrol_get_plugins(true);
+        $enabled['category'] = true;
+        $enabled = array_keys($enabled);
+        set_config('enrol_plugins_enabled', implode(',', $enabled));
+    }
+
+    protected function disable_plugin() {
+        $enabled = enrol_get_plugins(true);
+        unset($enabled['category']);
+        $enabled = array_keys($enabled);
+        set_config('enrol_plugins_enabled', implode(',', $enabled));
+    }
+
+    protected function enable_role_sync($roleid) {
+        global $DB;
+
+        $syscontext = context_system::instance();
+
+        if ($rc = $DB->record_exists('role_capabilities', array('capability'=>'enrol/category:synchronised', 'roleid'=>$roleid, 'contextid'=>$syscontext->id))) {
+            if ($rc->permission != CAP_ALLOW) {
+                $rc->permission = CAP_ALLOW;
+                $DB->update_record('role_capabilities', $rc);
+            }
+        } else {
+            $rc = new stdClass();
+            $rc->capability = 'enrol/category:synchronised';
+            $rc->roleid = $roleid;
+            $rc->contextid = $syscontext->id;
+            $rc->permission = CAP_ALLOW;
+            $rc->timemodified = time();
+            $rc->modifierid = 0;
+            $DB->insert_record('role_capabilities', $rc);
+        }
+    }
+
+    protected function disable_role_sync($roleid) {
+        global $DB;
+
+        $syscontext = context_system::instance();
+
+        $DB->delete_records('role_capabilities', array('capability'=>'enrol/category:synchronised', 'roleid'=>$roleid, 'contextid'=>$syscontext->id));
+    }
+
+    /**
+     * Test utility methods used in syn test, fail here means something
+     * in core accesslib was changed, but it is possible that only this test
+     * is affected, nto the plugin itself...
+     */
+    public function test_utils() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $syscontext = context_system::instance();
+
+        $this->assertFalse(enrol_is_enabled('category'));
+        $this->enable_plugin();
+        $this->assertTrue(enrol_is_enabled('category'));
+
+        $roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext);
+        $this->assertEmpty($roles);
+
+        $studentrole = $DB->get_record('role', array('shortname'=>'student'));
+        $this->assertNotEmpty($studentrole);
+
+        $this->enable_role_sync($studentrole->id);
+        $roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext);
+        $this->assertEquals(1, count($roles));
+        $this->assertEquals($studentrole, reset($roles));
+
+        $this->disable_role_sync($studentrole->id);
+        $roles = get_roles_with_capability('enrol/category:synchronised', CAP_ALLOW, $syscontext);
+        $this->assertEmpty($roles);
+    }
+
+    public function test_handler_sync() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup a few courses and categories.
+
+        $studentrole = $DB->get_record('role', array('shortname'=>'student'));
+        $this->assertNotEmpty($studentrole);
+        $teacherrole = $DB->get_record('role', array('shortname'=>'teacher'));
+        $this->assertNotEmpty($teacherrole);
+        $managerrole = $DB->get_record('role', array('shortname'=>'manager'));
+        $this->assertNotEmpty($managerrole);
+
+        $cat1 = $this->getDataGenerator()->create_category();
+        $cat2 = $this->getDataGenerator()->create_category();
+        $cat3 = $this->getDataGenerator()->create_category(array('parent'=>$cat2->id));
+
+        $course1 = $this->getDataGenerator()->create_course(array('category'=>$cat1->id));
+        $course2 = $this->getDataGenerator()->create_course(array('category'=>$cat2->id));
+        $course3 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+        $course4 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        $user4 = $this->getDataGenerator()->create_user();
+
+        $this->enable_role_sync($studentrole->id);
+        $this->enable_role_sync($teacherrole->id);
+        $this->enable_plugin();
+
+        $this->assertEquals(0, $DB->count_records('role_assignments', array()));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+
+        // Test assign event.
+
+        role_assign($managerrole->id, $user1->id, context_coursecat::instance($cat1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course2->id));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+
+        role_assign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course4->id), $user1->id));
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+        role_assign($managerrole->id, $user2->id, context_coursecat::instance($cat3->id));
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+        role_assign($teacherrole->id, $user4->id, context_coursecat::instance($cat1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4->id));
+        $this->assertEquals(4, $DB->count_records('user_enrolments', array()));
+
+        // Test role unassigned event.
+
+        role_unassign($teacherrole->id, $user4->id, context_coursecat::instance($cat1->id)->id);
+        $this->assertFalse(is_enrolled(context_course::instance($course1->id), $user4->id));
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+        // Make sure handlers are disabled when plugin disabled.
+
+        $this->disable_plugin();
+        role_unassign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id)->id);
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+        role_assign($studentrole->id, $user3->id, context_coursecat::instance($cat1->id));
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+    }
+
+    public function test_sync_course() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup a few courses and categories.
+
+        $studentrole = $DB->get_record('role', array('shortname'=>'student'));
+        $this->assertNotEmpty($studentrole);
+        $teacherrole = $DB->get_record('role', array('shortname'=>'teacher'));
+        $this->assertNotEmpty($teacherrole);
+        $managerrole = $DB->get_record('role', array('shortname'=>'manager'));
+        $this->assertNotEmpty($managerrole);
+
+        $cat1 = $this->getDataGenerator()->create_category();
+        $cat2 = $this->getDataGenerator()->create_category();
+        $cat3 = $this->getDataGenerator()->create_category(array('parent'=>$cat2->id));
+
+        $course1 = $this->getDataGenerator()->create_course(array('category'=>$cat1->id));
+        $course2 = $this->getDataGenerator()->create_course(array('category'=>$cat2->id));
+        $course3 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+        $course4 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        $user4 = $this->getDataGenerator()->create_user();
+
+        $this->enable_role_sync($studentrole->id);
+        $this->enable_role_sync($teacherrole->id);
+        $this->enable_plugin();
+
+        $this->assertEquals(0, $DB->count_records('role_assignments', array()));
+        role_assign($managerrole->id, $user1->id, context_coursecat::instance($cat1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course2->id));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+
+
+        $this->disable_plugin(); // Stops the event handlers.
+        role_assign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+        $this->enable_plugin();
+        enrol_category_sync_course($course2);
+        $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user1->id));
+        $this->assertFalse(is_enrolled(context_course::instance($course3->id), $user1->id));
+        $this->assertFalse(is_enrolled(context_course::instance($course4->id), $user1->id));
+        $this->assertEquals(1, $DB->count_records('user_enrolments', array()));
+
+        enrol_category_sync_course($course2);
+        enrol_category_sync_course($course3);
+        enrol_category_sync_course($course4);
+        $this->assertFalse(is_enrolled(context_course::instance($course1->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course4->id), $user1->id));
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+
+        $this->disable_plugin(); // Stops the event handlers.
+        role_assign($studentrole->id, $user2->id, context_coursecat::instance($cat1->id));
+        role_assign($teacherrole->id, $user4->id, context_coursecat::instance($cat1->id));
+        role_unassign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id)->id);
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+        $this->enable_plugin();
+        enrol_category_sync_course($course2);
+        $this->assertFalse(is_enrolled(context_course::instance($course2->id), $user1->id));
+        $this->assertFalse(is_enrolled(context_course::instance($course2->id), $user2->id));
+        $this->assertFalse(is_enrolled(context_course::instance($course2->id), $user4->id));
+        enrol_category_sync_course($course1);
+        enrol_category_sync_course($course3);
+        enrol_category_sync_course($course4);
+        $this->assertEquals(2, $DB->count_records('user_enrolments', array()));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user2->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4->id));
+
+        $this->disable_role_sync($studentrole->id);
+        enrol_category_sync_course($course1);
+        enrol_category_sync_course($course2);
+        enrol_category_sync_course($course3);
+        enrol_category_sync_course($course4);
+        $this->assertEquals(1, $DB->count_records('user_enrolments', array()));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4->id));
+
+        $this->assertEquals(1, $DB->count_records('enrol', array('enrol'=>'category')));
+        $this->disable_role_sync($teacherrole->id);
+        enrol_category_sync_course($course1);
+        enrol_category_sync_course($course2);
+        enrol_category_sync_course($course3);
+        enrol_category_sync_course($course4);
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+        $this->assertEquals(0, $DB->count_records('enrol', array('enrol'=>'category')));
+    }
+
+    public function test_sync_full() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup a few courses and categories.
+
+        $studentrole = $DB->get_record('role', array('shortname'=>'student'));
+        $this->assertNotEmpty($studentrole);
+        $teacherrole = $DB->get_record('role', array('shortname'=>'teacher'));
+        $this->assertNotEmpty($teacherrole);
+        $managerrole = $DB->get_record('role', array('shortname'=>'manager'));
+        $this->assertNotEmpty($managerrole);
+
+        $cat1 = $this->getDataGenerator()->create_category();
+        $cat2 = $this->getDataGenerator()->create_category();
+        $cat3 = $this->getDataGenerator()->create_category(array('parent'=>$cat2->id));
+
+        $course1 = $this->getDataGenerator()->create_course(array('category'=>$cat1->id));
+        $course2 = $this->getDataGenerator()->create_course(array('category'=>$cat2->id));
+        $course3 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+        $course4 = $this->getDataGenerator()->create_course(array('category'=>$cat3->id));
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $user3 = $this->getDataGenerator()->create_user();
+        $user4 = $this->getDataGenerator()->create_user();
+
+        $this->enable_role_sync($studentrole->id);
+        $this->enable_role_sync($teacherrole->id);
+        $this->enable_plugin();
+
+        $this->assertEquals(0, $DB->count_records('role_assignments', array()));
+        role_assign($managerrole->id, $user1->id, context_coursecat::instance($cat1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course1->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course2->id));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+
+        $result = enrol_category_sync_full();
+        $this->assertSame(0, $result);
+
+        $this->disable_plugin();
+        role_assign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id));
+        $this->enable_plugin();
+        $result = enrol_category_sync_full();
+        $this->assertSame(0, $result);
+        $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
+        $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user1->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course4->id), $user1->id));
+
+        $this->disable_plugin();
+        role_unassign($studentrole->id, $user1->id, context_coursecat::instance($cat2->id)->id);
+        role_assign($studentrole->id, $user2->id, context_coursecat::instance($cat1->id));
+        role_assign($teacherrole->id, $user4->id, context_coursecat::instance($cat1->id));
+        role_assign($teacherrole->id, $user3->id, context_coursecat::instance($cat2->id));
+        role_assign($managerrole->id, $user3->id, context_course::instance($course3->id));
+        $this->enable_plugin();
+        $result = enrol_category_sync_full();
+        $this->assertSame(0, $result);
+        $this->assertEquals(5, $DB->count_records('user_enrolments', array()));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user2->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user3->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course3->id), $user3->id));
+        $this->assertTrue(is_enrolled(context_course::instance($course4->id), $user3->id));
+
+        // Cleanup everything.
+
+        $this->assertNotEmpty($DB->count_records('role_assignments', array()));
+        $this->assertNotEmpty($DB->count_records('user_enrolments', array()));
+
+        $this->disable_plugin();
+        role_unassign_all(array('roleid'=>$studentrole->id));
+        role_unassign_all(array('roleid'=>$managerrole->id));
+        role_unassign_all(array('roleid'=>$teacherrole->id));
+
+        $result = enrol_category_sync_full();
+        $this->assertSame(2, $result);
+        $this->assertEquals(0, $DB->count_records('role_assignments', array()));
+        $this->assertNotEmpty($DB->count_records('user_enrolments', array()));
+        $this->disable_role_sync($studentrole->id);
+        $this->disable_role_sync($teacherrole->id);
+
+        $this->enable_plugin();
+        $result = enrol_category_sync_full();
+        $this->assertSame(0, $result);
+        $this->assertEquals(0, $DB->count_records('role_assignments', array()));
+        $this->assertEquals(0, $DB->count_records('user_enrolments', array()));
+        $this->assertEquals(0, $DB->count_records('enrol', array('enrol'=>'category')));
+    }
+}

From 8d9b8a489471a33c26d81a556aafca4f554d82c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= <commits@skodak.org>
Date: Sat, 18 Aug 2012 11:39:03 +0200
Subject: [PATCH 5/5] MDL-34864 bump up enrol category sync version

---
 enrol/category/version.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/enrol/category/version.php b/enrol/category/version.php
index 73840f9ca28..c44f22de54d 100644
--- a/enrol/category/version.php
+++ b/enrol/category/version.php
@@ -24,7 +24,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2012061700;        // The current plugin version (Date: YYYYMMDDXX)
-$plugin->requires  = 2012061700;        // Requires this Moodle version
+$plugin->version   = 2012081800;        // The current plugin version (Date: YYYYMMDDXX)
+$plugin->requires  = 2012062501;        // Requires this Moodle version
 $plugin->component = 'enrol_category';  // Full name of the plugin (used for diagnostics)
 $plugin->cron      = 60;