MDL-37298 use progress_trace in enrol_cohort sync

enrol/cohort/ajax.php

@@ -90,7 +90,9 @@ switch ($action) {
         }
         $enrol = enrol_get_plugin('cohort');
         $enrol->add_instance($manager->get_course(), array('customint1' => $cohortid, 'roleid' => $roleid));
-        enrol_cohort_sync($manager->get_course()->id);
+        $trace = new null_progress_trace();
+        enrol_cohort_sync($trace, $manager->get_course()->id);
+        $trace->finished();
         break;
     case 'enrolcohortusers':
         //TODO: this should be moved to enrol_manual, see MDL-35618.

enrol/cohort/cli/sync.php

@@ -30,8 +30,8 @@
 
 define('CLI_SCRIPT', true);
 
-require(dirname(dirname(dirname(dirname(__FILE__)))).'/config.php');
+require(__DIR__.'/../../../config.php');
-require_once($CFG->libdir.'/clilib.php');
+require_once("$CFG->libdir/clilib.php");
 require_once("$CFG->dirroot/enrol/cohort/locallib.php");
 
 // Now get cli options.
@@ -51,15 +51,20 @@ Options:
 -h, --help            Print out this help
 
 Example:
-\$sudo -u www-data /usr/bin/php enrol/cohort/cli/sync.php
+\$ sudo -u www-data /usr/bin/php enrol/cohort/cli/sync.php
 ";
 
     echo $help;
     die;
 }
 
-$verbose = !empty($options['verbose']);
+if (empty($options['verbose'])) {
+    $trace = new null_progress_trace();
+} else {
+    $trace = new text_progress_trace();
+}
 
-$result = enrol_cohort_sync(null, $verbose);
+$result = enrol_cohort_sync($trace, null);
+$trace->finished();
 
 exit($result);

enrol/cohort/edit.php

@@ -92,7 +92,9 @@ if ($mform->is_cancelled()) {
     }  else {
         $enrol->add_instance($course, array('name'=>$data->name, 'status'=>$data->status, 'customint1'=>$data->customint1, 'roleid'=>$data->roleid, 'customint2'=>$data->customint2));
     }
-    enrol_cohort_sync($course->id);
+    $trace = new null_progress_trace();
+    enrol_cohort_sync($trace, $course->id);
+    $trace->finished();
     redirect($returnurl);
 }
 

enrol/cohort/lib.php

@@ -132,7 +132,9 @@ class enrol_cohort_plugin extends enrol_plugin {
         global $CFG;
 
         require_once("$CFG->dirroot/enrol/cohort/locallib.php");
-        enrol_cohort_sync();
+        $trace = new null_progress_trace();
+        enrol_cohort_sync($trace);
+        $trace->finished();
     }
 
     /**
@@ -160,7 +162,9 @@ class enrol_cohort_plugin extends enrol_plugin {
         parent::update_status($instance, $newstatus);
 
         require_once("$CFG->dirroot/enrol/cohort/locallib.php");
-        enrol_cohort_sync($instance->courseid);
+        $trace = new null_progress_trace();
+        enrol_cohort_sync($trace, $instance->courseid);
+        $trace->finished();
     }
 
     /**
@@ -279,7 +283,9 @@ class enrol_cohort_plugin extends enrol_plugin {
             $step->set_mapping('enrol', $oldid, $instanceid);
 
             require_once("$CFG->dirroot/enrol/cohort/locallib.php");
-            enrol_cohort_sync($course->id, false);
+            $trace = new null_progress_trace();
+            enrol_cohort_sync($trace, $course->id);
+            $trace->finished();
 
         } else if ($this->get_config('unenrolaction') == ENROL_EXT_REMOVED_SUSPENDNOROLES) {
             $data->customint1 = 0;
@@ -294,7 +300,9 @@ class enrol_cohort_plugin extends enrol_plugin {
             $step->set_mapping('enrol', $oldid, $instanceid);
 
             require_once("$CFG->dirroot/enrol/cohort/locallib.php");
-            enrol_cohort_sync($course->id, false);
+            $trace = new null_progress_trace();
+            enrol_cohort_sync($trace, $course->id);
+            $trace->finished();
 
         } else {
             $step->set_mapping('enrol', $oldid, 0);

enrol/cohort/locallib.php

@@ -151,19 +151,17 @@ class enrol_cohort_handler {
 
 /**
  * Sync all cohort course links.
+ * @param progress_trace $trace
  * @param int $courseid one course, empty mean all
- * @param bool $verbose verbose CLI output
  * @return int 0 means ok, 1 means error, 2 means plugin disabled
  */
-function enrol_cohort_sync($courseid = NULL, $verbose = false) {
+function enrol_cohort_sync(progress_trace $trace, $courseid = NULL) {
     global $CFG, $DB;
     require_once("$CFG->dirroot/group/lib.php");
 
     // Purge all roles if cohort sync disabled, those can be recreated later here by cron or CLI.
     if (!enrol_is_enabled('cohort')) {
-        if ($verbose) {
+        $trace->output('Cohort sync plugin is disabled, unassigning all plugin roles and stopping.');
-            mtrace('Cohort sync plugin is disabled, unassigning all plugin roles and stopping.');
-        }
         role_unassign_all(array('component'=>'enrol_cohort'));
         return 2;
     }
@@ -172,9 +170,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     @set_time_limit(0);
     raise_memory_limit(MEMORY_HUGE);
 
-    if ($verbose) {
+    $trace->output('Starting user enrolment synchronisation...');
-        mtrace('Starting user enrolment synchronisation...');
-    }
 
     $allroles = get_all_roles();
     $instances = array(); //cache
@@ -201,14 +197,10 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
         $instance = $instances[$ue->enrolid];
         if ($ue->status == ENROL_USER_SUSPENDED) {
             $plugin->update_user_enrol($instance, $ue->userid, ENROL_USER_ACTIVE);
-            if ($verbose) {
+            $trace->output("unsuspending: $ue->userid ==> $instance->courseid via cohort $instance->customint1", 1);
-                mtrace("  unsuspending: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
-            }
         } else {
             $plugin->enrol_user($instance, $ue->userid);
-            if ($verbose) {
+            $trace->output("enrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1", 1);
-                mtrace("  enrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
-            }
         }
     }
     $rs->close();
@@ -229,9 +221,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
         if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) {
             // Remove enrolment together with group membership, grades, preferences, etc.
             $plugin->unenrol_user($instance, $ue->userid);
-            if ($verbose) {
+            $trace->output("unenrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1", 1);
-                mtrace("  unenrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1");
-            }
 
         } else { // ENROL_EXT_REMOVED_SUSPENDNOROLES
             // Just disable and ignore any changes.
@@ -239,9 +229,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
                 $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) {
+                $trace->output("suspending and unsassigning all roles: $ue->userid ==> $instance->courseid", 1);
-                    mtrace("  suspending and unsassigning all roles: $ue->userid ==> $instance->courseid");
-                }
             }
         }
     }
@@ -267,9 +255,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     $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) {
+        $trace->output("assigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname, 1);
-            mtrace("  assigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname);
-        }
     }
     $rs->close();
 
@@ -291,9 +277,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     $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) {
+        $trace->output("unassigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname, 1);
-            mtrace("  unassigning role: $ra->userid ==> $ra->courseid as ".$allroles[$ra->roleid]->shortname);
-        }
     }
     $rs->close();
 
@@ -314,9 +298,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $gm) {
         groups_remove_member($gm->groupid, $gm->userid);
-        if ($verbose) {
+        $trace->output("removing user from group: $gm->userid ==> $gm->courseid - $gm->groupname", 1);
-            mtrace("  removing user from group: $gm->userid ==> $gm->courseid - $gm->groupname");
-        }
     }
     $rs->close();
 
@@ -333,16 +315,12 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) {
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $ue) {
         groups_add_member($ue->groupid, $ue->userid, 'enrol_cohort', $ue->enrolid);
-        if ($verbose) {
+        $trace->output("adding user to group: $ue->userid ==> $ue->courseid - $ue->groupname", 1);
-            mtrace("  adding user to group: $ue->userid ==> $ue->courseid - $ue->groupname");
-        }
     }
     $rs->close();
 
 
-    if ($verbose) {
+    $trace->output('...user enrolment synchronisation finished.');
-        mtrace('...user enrolment synchronisation finished.');
-    }
 
     return 0;
 }

enrol/cohort/tests/sync_test.php

@@ -260,6 +260,8 @@ class enrol_cohort_testcase extends advanced_testcase {
         global $DB;
         $this->resetAfterTest();
 
+        $trace = new null_progress_trace();
+
         // Setup a few courses and categories.
 
         $cohortplugin = enrol_get_plugin('cohort');
@@ -319,19 +321,19 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         // Test sync of one course only.
 
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(2, $DB->count_records('role_assignments', array()));
         $this->assertEquals(2, $DB->count_records('user_enrolments', array()));
 
 
         $this->enable_plugin();
-        enrol_cohort_sync($course2->id, false);
+        enrol_cohort_sync($trace, $course2->id);
         $this->assertEquals(3, $DB->count_records('role_assignments', array()));
         $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort3->id)); // Use low level DB api to prevent events!
         $DB->delete_records('cohort', array('id'=>$cohort3->id)); // Use low level DB api to prevent events!
 
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user1->id)));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user2->id)));
@@ -345,14 +347,14 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort2->id, 'userid'=>$user3->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(6, $DB->count_records('role_assignments', array()));
         $this->assertFalse($DB->record_exists('role_assignments', array('contextid'=>context_course::instance($course1->id)->id, 'userid'=>$user3->id, 'roleid'=>$teacherrole->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance2->id)));
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort1->id, 'userid'=>$user1->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(5, $DB->count_records('user_enrolments', array()));
         $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance2->id, 'userid'=>$user3->id)));
         $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user1->id)));
@@ -363,12 +365,12 @@ class enrol_cohort_testcase extends advanced_testcase {
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort1->id)); // Use low level DB api to prevent events!
         $DB->delete_records('cohort', array('id'=>$cohort1->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(5, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(3, $DB->count_records('role_assignments', array()));
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(3, $DB->count_records('role_assignments', array()));
 
@@ -400,7 +402,7 @@ class enrol_cohort_testcase extends advanced_testcase {
         cohort_add_member($cohort1->id, $user4->id);
         cohort_add_member($cohort2->id, $user4->id);
 
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
 
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(7, $DB->count_records('role_assignments', array()));
@@ -414,7 +416,7 @@ class enrol_cohort_testcase extends advanced_testcase {
         $cohortinstance1->customint2 = $group2->id;
         $DB->update_record('enrol', $cohortinstance1);
 
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertFalse(groups_is_member($group1->id, $user1->id));
         $this->assertTrue(groups_is_member($group2->id, $user1->id));
         $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id)));
@@ -437,6 +439,8 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         $this->resetAfterTest();
 
+        $trace = new null_progress_trace();
+
         // Setup a few courses and categories.
 
         $cohortplugin = enrol_get_plugin('cohort');
@@ -496,13 +500,13 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         // Test sync of one course only.
 
-        enrol_cohort_sync(null, false);
+        enrol_cohort_sync($trace, null);
         $this->assertEquals(2, $DB->count_records('role_assignments', array()));
         $this->assertEquals(2, $DB->count_records('user_enrolments', array()));
 
 
         $this->enable_plugin();
-        enrol_cohort_sync(null, false);
+        enrol_cohort_sync($trace, null);
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user1->id)));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user2->id)));
@@ -516,14 +520,14 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort2->id, 'userid'=>$user3->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(6, $DB->count_records('role_assignments', array()));
         $this->assertFalse($DB->record_exists('role_assignments', array('contextid'=>context_course::instance($course1->id)->id, 'userid'=>$user3->id, 'roleid'=>$teacherrole->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance2->id)));
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort1->id, 'userid'=>$user1->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(5, $DB->count_records('user_enrolments', array()));
         $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance2->id, 'userid'=>$user3->id)));
         $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user1->id)));
@@ -534,12 +538,12 @@ class enrol_cohort_testcase extends advanced_testcase {
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES);
         $DB->delete_records('cohort_members', array('cohortid'=>$cohort1->id)); // Use low level DB api to prevent events!
         $DB->delete_records('cohort', array('id'=>$cohort1->id)); // Use low level DB api to prevent events!
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(5, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(3, $DB->count_records('role_assignments', array()));
 
         $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL);
-        enrol_cohort_sync($course1->id, false);
+        enrol_cohort_sync($trace, $course1->id);
         $this->assertEquals(3, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(3, $DB->count_records('role_assignments', array()));
 
@@ -573,7 +577,7 @@ class enrol_cohort_testcase extends advanced_testcase {
 
         $this->enable_plugin();
 
-        enrol_cohort_sync(null, false);
+        enrol_cohort_sync($trace, null);
 
         $this->assertEquals(8, $DB->count_records('user_enrolments', array()));
         $this->assertEquals(8, $DB->count_records('role_assignments', array()));
@@ -593,7 +597,7 @@ class enrol_cohort_testcase extends advanced_testcase {
         $cohortinstance3->customint2 = $group3->id;
         $DB->update_record('enrol', $cohortinstance3);
 
-        enrol_cohort_sync(null, false);
+        enrol_cohort_sync($trace, null);
         $this->assertFalse(groups_is_member($group1->id, $user1->id));
         $this->assertTrue(groups_is_member($group2->id, $user1->id));
         $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id)));