MDL-52098 enrol: cohort now looks at instance status

robjoyce · Dec 15, 2016 · a446095 · a446095
1 parent 5100c48
commit a446095
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 3 deletions.
diff --git a/enrol/cohort/locallib.php b/enrol/cohort/locallib.php
@@ -52,9 +52,11 @@ public static function member_added(\core\event\cohort_member_added $event) {
         $sql = "SELECT e.*, r.id as roleexists
                   FROM {enrol} e
              LEFT JOIN {role} r ON (r.id = e.roleid)
-                 WHERE e.customint1 = :cohortid AND e.enrol = 'cohort'
+                 WHERE e.customint1 = :cohortid AND e.enrol = 'cohort' AND e.status = :enrolstatus
               ORDER BY e.id ASC";
-        if (!$instances = $DB->get_records_sql($sql, array('cohortid'=>$event->objectid))) {
+        $params['cohortid'] = $event->objectid;
+        $params['enrolstatus'] = ENROL_INSTANCE_ENABLED;
+        if (!$instances = $DB->get_records_sql($sql, $params)) {
             return true;
         }
 
@@ -184,13 +186,14 @@ function enrol_cohort_sync(progress_trace $trace, $courseid = NULL) {
     $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)
+              JOIN {enrol} e ON (e.customint1 = cm.cohortid AND e.enrol = 'cohort' AND e.status = :enrolstatus $onecourse)
               JOIN {user} u ON (u.id = cm.userid AND u.deleted = 0)
          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;
+    $params['enrolstatus'] = ENROL_INSTANCE_ENABLED;
     $rs = $DB->get_recordset_sql($sql, $params);
     foreach($rs as $ue) {
         if (!isset($instances[$ue->enrolid])) {

diff --git a/enrol/cohort/tests/sync_test.php b/enrol/cohort/tests/sync_test.php
@@ -99,6 +99,8 @@ public function test_handler_sync() {
         $id = $cohortplugin->add_instance($course2, array('customint1'=>$cohort2->id, 'roleid'=>$studentrole->id));
         $cohortinstance3 = $DB->get_record('enrol', array('id'=>$id));
 
+        $id = $cohortplugin->add_instance($course2, array('customint1' => $cohort2->id, 'roleid' => $studentrole->id, 'status' => ENROL_INSTANCE_DISABLED));
+        $cohortinstance4 = $DB->get_record('enrol', array('id' => $id));
 
         // Test cohort member add event.
 
@@ -118,6 +120,7 @@ public function test_handler_sync() {
         $this->assertEquals(7, $DB->count_records('user_enrolments', array()));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance2->id, 'userid'=>$user3->id)));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance3->id, 'userid'=>$user3->id)));
+        $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid' => $cohortinstance4->id, 'userid' => $user3->id)));
         $this->assertEquals(7, $DB->count_records('role_assignments', array()));
         $this->assertTrue($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)));
         $this->assertTrue($DB->record_exists('role_assignments', array('contextid'=>context_course::instance($course2->id)->id, 'userid'=>$user3->id, 'roleid'=>$studentrole->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance3->id)));
@@ -313,6 +316,9 @@ public function test_sync_course() {
         $id = $cohortplugin->add_instance($course2, array('customint1'=>$cohort2->id, 'roleid'=>$studentrole->id));
         $cohortinstance3 = $DB->get_record('enrol', array('id'=>$id));
 
+        $id = $cohortplugin->add_instance($course2, array('customint1' => $cohort2->id, 'roleid' => $studentrole->id, 'status' => ENROL_INSTANCE_DISABLED));
+        $cohortinstance4 = $DB->get_record('enrol', array('id' => $id));
+
         cohort_add_member($cohort1->id, $user1->id);
         cohort_add_member($cohort1->id, $user2->id);
         cohort_add_member($cohort1->id, $user4->id);
@@ -343,6 +349,7 @@ public function test_sync_course() {
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user2->id)));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance1->id, 'userid'=>$user4->id)));
         $this->assertTrue($DB->record_exists('user_enrolments', array('enrolid'=>$cohortinstance2->id, 'userid'=>$user3->id)));
+        $this->assertFalse($DB->record_exists('user_enrolments', array('enrolid' => $cohortinstance4->id, 'userid' => $user3->id)));
         $this->assertEquals(7, $DB->count_records('role_assignments', array()));
         $this->assertTrue($DB->record_exists('role_assignments', array('contextid'=>context_course::instance($course1->id)->id, 'userid'=>$user1->id, 'roleid'=>$studentrole->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id)));
         $this->assertTrue($DB->record_exists('role_assignments', array('contextid'=>context_course::instance($course1->id)->id, 'userid'=>$user2->id, 'roleid'=>$studentrole->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id)));