From 9b44e05727b25d543d8b7ccc4b7eebe8b977e8c5 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Tue, 1 Oct 2024 10:53:03 -0700 Subject: [PATCH] corrects closing of users loop. see https://github.com/ilios/moodle-enrol-ilios/issues/50 this addresses the regression outline in --- lib.php | 111 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 56 insertions(+), 55 deletions(-) diff --git a/lib.php b/lib.php index 5f6593b..6cceb52 100644 --- a/lib.php +++ b/lib.php @@ -297,7 +297,7 @@ public function sync($trace, $courseid = null): int { if (!empty($user->campusId)) { $urec = $DB->get_record('user', ["idnumber" => $user->campusId]); if (!empty($urec)) { - $iliosusers[$user->id] = [ 'id' => $urec->id, 'syncfield' => $urec->idnumber ]; + $iliosusers[$user->id] = ['id' => $urec->id, 'syncfield' => $urec->idnumber]; } } } @@ -328,19 +328,19 @@ public function sync($trace, $courseid = null): int { } // Don't re-enroll suspended enrollments for disabled Ilios users. - if (!empty($ue) && ENROL_USER_SUSPENDED === (int) $ue->status && !$user->enabled) { + if (!empty($ue) && ENROL_USER_SUSPENDED === (int)$ue->status && !$user->enabled) { continue; } // Flag actively enrolled users that are disabled in Ilios // for enrollment suspension further downstream. - if (!empty($ue) && ENROL_USER_ACTIVE === (int) $ue->status && !$user->enabled) { + if (!empty($ue) && ENROL_USER_ACTIVE === (int)$ue->status && !$user->enabled) { $suspendenrolments[] = $ue; continue; } // Continue if already enrolled with active status. - if (!empty($ue) && ENROL_USER_ACTIVE === (int) $ue->status) { + if (!empty($ue) && ENROL_USER_ACTIVE === (int)$ue->status) { continue; } @@ -353,7 +353,7 @@ public function sync($trace, $courseid = null): int { 0, ENROL_USER_ACTIVE ); - if (!empty($ue) && ENROL_USER_ACTIVE !== (int) $ue->status) { + if (!empty($ue) && ENROL_USER_ACTIVE !== (int)$ue->status) { $trace->output( "changing enrollment status to '" . ENROL_USER_ACTIVE @@ -371,70 +371,71 @@ public function sync($trace, $courseid = null): int { ); } } + } - // Suspend active enrollments for users that are disabled in Ilios. - foreach ($suspendenrolments as $ue) { - $trace->output( - "Suspending enrollment for disabled Ilios user: userid " - . " {$ue->userid} ==> courseid {$instance->courseid}." - , 1 - ); - $this->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED); - } - - // Unenrol as necessary. + // Suspend active enrollments for users that are disabled in Ilios. + foreach ($suspendenrolments as $ue) { $trace->output( - "Unenrolling users from Course ID " - . $instance->courseid." with Role ID " - . $instance->roleid - . " that no longer associate with Ilios Sync ID " - . $instance->id - . "." + "Suspending enrollment for disabled Ilios user: userid " + . " {$ue->userid} ==> courseid {$instance->courseid}." + , 1 ); + $this->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED); + } - $sql = "SELECT ue.* - FROM {user_enrolments} ue - WHERE ue.enrolid = $instance->id"; + // Unenrol as necessary. + $trace->output( + "Unenrolling users from Course ID " + . $instance->courseid." with Role ID " + . $instance->roleid + . " that no longer associate with Ilios Sync ID " + . $instance->id + . "." + ); - if (!empty($enrolleduserids)) { - $sql .= " AND ue.userid NOT IN ( ".implode(",", $enrolleduserids)." )"; - } + $sql = "SELECT ue.* + FROM {user_enrolments} ue + WHERE ue.enrolid = $instance->id"; + + if (!empty($enrolleduserids)) { + $sql .= " AND ue.userid NOT IN ( ".implode(",", $enrolleduserids)." )"; + } - $rs = $DB->get_recordset_sql($sql); - foreach ($rs as $ue) { - if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) { - // Remove enrolment together with group membership, grades, preferences, etc. - $this->unenrol_user($instance, $ue->userid); + $rs = $DB->get_recordset_sql($sql); + foreach ($rs as $ue) { + if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) { + // Remove enrolment together with group membership, grades, preferences, etc. + $this->unenrol_user($instance, $ue->userid); + $trace->output( + "unenrolling: $ue->userid ==> " + . $instance->courseid + . " via Ilios $synctype $syncid" + , 1 + ); + } else { // Would be ENROL_EXT_REMOVED_SUSPENDNOROLES. + // Just disable and ignore any changes. + if ($ue->status != ENROL_USER_SUSPENDED) { + $this->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED); + $context = context_course::instance($instance->courseid); + role_unassign_all([ + 'userid' => $ue->userid, + 'contextid' => $context->id, + 'component' => 'enrol_ilios', + 'itemid' => $instance->id, + ]); $trace->output( - "unenrolling: $ue->userid ==> " + "suspending and unassigning all roles: userid " + . $ue->userid + . " ==> courseid " . $instance->courseid - . " via Ilios $synctype $syncid" , 1 ); - } else { // Would be ENROL_EXT_REMOVED_SUSPENDNOROLES. - // Just disable and ignore any changes. - if ($ue->status != ENROL_USER_SUSPENDED) { - $this->update_user_enrol($instance, $ue->userid, ENROL_USER_SUSPENDED); - $context = context_course::instance($instance->courseid); - role_unassign_all([ - 'userid' => $ue->userid, - 'contextid' => $context->id, - 'component' => 'enrol_ilios', - 'itemid' => $instance->id, - ]); - $trace->output( - "suspending and unassigning all roles: userid " - . $ue->userid - . " ==> courseid " - . $instance->courseid - , 1 - ); - } } } - $rs->close(); } + $rs->close(); } + $instances->close(); unset($iliosusers);