From 14cd15dfab401acf60fa20667704ce0800b236a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20D=C3=A9niz=20Falc=C3=B3n?= Date: Mon, 19 Aug 2019 16:30:09 +0100 Subject: [PATCH] MDL-46886 core_enrol: unenrolment due to inactivity notification --- enrol/self/lang/en/enrol_self.php | 6 ++ enrol/self/lib.php | 116 +++++++++++++++++++++++- enrol/self/tests/self_test.php | 146 ++++++++++++++++++++++++++++++ lib/enrollib.php | 97 ++++++++++++++------ 4 files changed, 338 insertions(+), 27 deletions(-) diff --git a/enrol/self/lang/en/enrol_self.php b/enrol/self/lang/en/enrol_self.php index 0577ce6b806..39128095ec7 100644 --- a/enrol/self/lang/en/enrol_self.php +++ b/enrol/self/lang/en/enrol_self.php @@ -53,6 +53,12 @@ $string['enrolstartdate'] = 'Start date'; $string['enrolstartdate_help'] = 'If enabled, users can enrol themselves from this date onward only.'; $string['expiredaction'] = 'Enrolment expiry action'; $string['expiredaction_help'] = 'Select action to carry out when user enrolment expires. Please note that some user data and settings are purged from course during course unenrolment.'; +$string['expiryinactivemessageenrolledbody'] = 'Hi {$a->user}, + +Your enrolment in the course {$a->course} expires on {$a->timeend} as you have not visited it in the last {$a->inactivetime} days. This means that you will no longer have access to the course. + +To keep your enrolment active, simply log in and visit {$a->course} before {$a->timeend}.'; +$string['expiryinactivemessageenrolledsubject'] = 'Your enrolment is expiring: {$a->course}'; $string['expirymessageenrollersubject'] = 'Self enrolment expiry notification'; $string['expirymessageenrollerbody'] = 'Self enrolment in the course \'{$a->course}\' will expire within the next {$a->threshold} for the following users: diff --git a/enrol/self/lib.php b/enrol/self/lib.php index b3bb22c1c4d..9bd1930abc2 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -506,6 +506,100 @@ class enrol_self_plugin extends enrol_plugin { return 0; } + /** + * Notify users about enrolment expiration. + * + * Users may be notified by the expiration time of enrollment or unenrollment due to inactivity. The latter is checked in + * the last condition of the where clause: + * days of inactivity + number of days in advance to send the notification > days of inactivity allowed before unenrollment + * + * @param int $timenow Current time. + * @param string $name Name of this enrol plugin. + * @param progress_trace $trace (accepts bool for backwards compatibility only) + * @return void + */ + protected function fetch_users_and_notify_expiry(int $timenow, string $name, progress_trace $trace): void { + global $DB, $CFG; + + $sql = "SELECT ue.*, e.expirynotify, e.notifyall, e.expirythreshold, e.courseid, c.fullname, e.customint2, + COALESCE(ul.timeaccess, 0) AS timeaccess, ue.timestart + FROM {user_enrolments} ue + JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = :name AND e.expirynotify > 0 AND e.status = :enabled) + JOIN {course} c ON (c.id = e.courseid) + JOIN {user} u ON (u.id = ue.userid AND u.deleted = 0 AND u.suspended = 0) + LEFT JOIN {user_lastaccess} ul ON (ul.userid = u.id AND ul.courseid = c.id) + WHERE ue.status = :active + AND ((ue.timeend > 0 AND ue.timeend > :now1 AND ue.timeend < (e.expirythreshold + :now2)) + OR (e.customint2 > 0 AND (:now3 - COALESCE(ul.timeaccess, 0) + e.expirythreshold > e.customint2))) + ORDER BY ue.enrolid ASC, u.lastname ASC, u.firstname ASC, u.id ASC"; + $params = [ + 'name' => $name, + 'enabled' => ENROL_INSTANCE_ENABLED, + 'active' => ENROL_USER_ACTIVE, + 'now1' => $timenow, + 'now2' => $timenow, + 'now3' => $timenow, + ]; + + $rs = $DB->get_recordset_sql($sql, $params); + + $lastenrollid = 0; + $users = []; + + foreach ($rs as $ue) { + $expirycond = ($ue->timeend > 0) && ($ue->timeend > $timenow) && ($ue->timeend < ($ue->expirythreshold + $timenow)); + $inactivitycond = ($ue->customint2 > 0) && (($timenow - $ue->timeaccess + $ue->expirythreshold) > $ue->customint2); + + $user = $DB->get_record('user', ['id' => $ue->userid]); + + if ($expirycond) { + if ($lastenrollid && $lastenrollid != $ue->enrolid) { + $this->notify_expiry_enroller($lastenrollid, $users, $trace); + $users = []; + } + $lastenrollid = $ue->enrolid; + + $enroller = $this->get_enroller($ue->enrolid); + $context = context_course::instance($ue->courseid); + + $users[] = [ + 'fullname' => fullname($user, has_capability('moodle/site:viewfullnames', $context, $enroller)), + 'timeend' => $ue->timeend, + ]; + } + // Notifications to inactive users only if inactive time limit is set. + if ($inactivitycond && $ue->notifyall) { + $ue->message = 'expiryinactivemessageenrolledbody'; + $lastaccess = $ue->timeaccess; + if (!$lastaccess) { + $lastaccess = $ue->timestart; + } + $ue->inactivetime = floor(($timenow - $lastaccess) / DAYSECS); + $this->notify_expiry_enrolled($user, $ue, $trace); + } + + if ($expirycond) { + if (!$ue->notifyall) { + continue; + } + + if ($ue->timeend - $ue->expirythreshold + 86400 < $timenow) { + // Notify enrolled users only once at the start of the threshold. + $trace->output("user $ue->userid was already notified that enrolment in course $ue->courseid expires on " . + userdate($ue->timeend, '', $CFG->timezone), 1); + continue; + } + + $this->notify_expiry_enrolled($user, $ue, $trace); + } + } + $rs->close(); + + if ($lastenrollid && $users) { + $this->notify_expiry_enroller($lastenrollid, $users, $trace); + } + } + /** * Returns the user who is responsible for self enrolments in given instance. * @@ -539,6 +633,26 @@ class enrol_self_plugin extends enrol_plugin { return $this->lasternoller; } + protected function get_expiry_message_body(stdClass $user, stdClass $ue, string $name, + stdClass $enroller, context $context): string { + $a = new stdClass(); + $a->course = format_string($ue->fullname, true, ['context' => $context]); + $a->user = fullname($user, true); + // If the enrolment duration is disabled, replace timeend with other data. + if ($ue->timeend == 0) { + $timenow = time(); + $lastaccess = $ue->timeaccess > 0 ? $ue->timeaccess : $ue->timestart; + $ue->timeend = $timenow + ($ue->customint2 - ($timenow - $lastaccess)); + } + $a->timeend = userdate($ue->timeend, '', $user->timezone); + $a->enroller = fullname($enroller, has_capability('moodle/site:viewfullnames', $context, $user)); + if (isset($ue->inactivetime)) { + $a->inactivetime = $ue->inactivetime; + } + $a->url = new moodle_url('/course/view.php', ['id' => $ue->courseid]); + return get_string($ue->message ?? 'expirymessageenrolledbody', 'enrol_' . $name, $a); + } + /** * Restore instance and map settings. * @@ -925,7 +1039,7 @@ class enrol_self_plugin extends enrol_plugin { } } - if ($data['expirynotify'] > 0 and $data['expirythreshold'] < 86400) { + if (($data['expirynotify'] > 0 || $data['customint2']) && $data['expirythreshold'] < 86400) { $errors['expirythreshold'] = get_string('errorthresholdlow', 'core_enrol'); } diff --git a/enrol/self/tests/self_test.php b/enrol/self/tests/self_test.php index b0fa85c8cc9..66e37d5e7d4 100644 --- a/enrol/self/tests/self_test.php +++ b/enrol/self/tests/self_test.php @@ -170,6 +170,152 @@ class self_test extends \advanced_testcase { $this->assertEquals(2, $DB->count_records('role_assignments', array('roleid'=>$teacherrole->id))); } + /** + * Data provider for longtimenosee notifications tests. + * + * @return array + */ + public static function longtimenosee_notifications_provider(): array { + + return [ + 'No inactive period' => [ + 'expirynotify' => 1, + 'notifyall' => 1, + 'expirythreshold' => DAYSECS * 3, + 'customint2' => 0, + 'numnotifications' => 2, + 'progresstrace' => true, + ], + 'Notifications disabled' => [ + 'expirynotify' => 0, + 'notifyall' => 1, + 'expirythreshold' => DAYSECS * 3, + 'customint2' => WEEKSECS, + 'numnotifications' => 0, + 'progresstrace' => true, + ], + 'Notifications enabled' => [ + 'expirynotify' => 1, + 'notifyall' => 1, + 'expirythreshold' => DAYSECS * 3, + 'customint2' => WEEKSECS, + 'numnotifications' => 4, + 'progresstrace' => false, + ], + ]; + } + + /** + * Tests for the inactivity unerol notification. + * + * Having enrolment duration (timeend) set to 0, the notifications about enrol expiration are not sent + * + * @dataProvider longtimenosee_notifications_provider + * @covers ::send_expiry_notifications + * @param int $expirynotify Whether enrolment expiry notification messages are sent + * @param int $notifyall Whether teachers and students are notified or only teachers + * @param int $expirythreshold How long before expiry are users notified (seconds) + * @param int $customint2 Time of inactivity before unerolling a user (seconds) + * @param int $numnotifications Expected number of notifications sent + * @param bool $progresstrace Progress tracing object + * @return void + */ + public function test_longtimenosee_notifications( + int $expirynotify, + int $notifyall, + int $expirythreshold, + int $customint2, + int $numnotifications, + bool $progresstrace, + ): void { + global $DB; + $this->resetAfterTest(); + $this->preventResetByRollback(); // Messaging does not like transactions... + + $selfplugin = enrol_get_plugin('self'); + + $now = time(); + $coursestartdate = $now - WEEKSECS * 4; + + $trace = new \null_progress_trace(); + + // Note: hopefully nobody executes the unit tests the last second before midnight... + $selfplugin->set_config('expirynotifylast', $now - DAYSECS); + $selfplugin->set_config('expirynotifyhour', 0); + + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $this->assertNotEmpty($studentrole); + $editingteacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']); + $this->assertNotEmpty($editingteacherrole); + $managerrole = $DB->get_record('role', ['shortname' => 'manager']); + $this->assertNotEmpty($managerrole); + + $user1 = $this->getDataGenerator()->create_user(['lastname' => 'xuser1']); + $user2 = $this->getDataGenerator()->create_user(['lastname' => 'xuser2']); + $user3 = $this->getDataGenerator()->create_user(['lastname' => 'xuser3']); + $user4 = $this->getDataGenerator()->create_user(['lastname' => 'xuser4']); + + $course1 = $this->getDataGenerator()->create_course(['fullname' => 'xcourse1', 'startdate' => $coursestartdate]); + + $instance1 = $DB->get_record('enrol', ['courseid' => $course1->id, 'enrol' => 'self'], '*', MUST_EXIST); + $instance1->expirythreshold = $expirythreshold; + $instance1->expirynotify = $expirynotify; + $instance1->notifyall = $notifyall; + $instance1->status = ENROL_INSTANCE_ENABLED; + $instance1->customint2 = $customint2; + $DB->update_record('enrol', $instance1); + + // Suspended users are not notified. + $selfplugin->enrol_user($instance1, $user1->id, $studentrole->id, $coursestartdate, 0, ENROL_USER_SUSPENDED); + // User accessed recently - should not be notified. + $selfplugin->enrol_user($instance1, $user2->id, $studentrole->id, $coursestartdate, 0); + $DB->insert_record('user_lastaccess', ['userid' => $user2->id, 'courseid' => $course1->id, 'timeaccess' => $now - + DAYSECS * 3]); + // User accessed long time ago - should be notified. + $selfplugin->enrol_user($instance1, $user3->id, $studentrole->id, $coursestartdate, $now + DAYSECS * 2 + HOURSECS); + $DB->insert_record('user_lastaccess', ['userid' => $user3->id, 'courseid' => $course1->id, 'timeaccess' => $now + - DAYSECS * 20]); + // User has never accessed the course - should be notified. + $selfplugin->enrol_user($instance1, $user4->id, $studentrole->id, $coursestartdate, 0); + + $sink = $this->redirectMessages(); + if ($progresstrace) { + $selfplugin->send_expiry_notifications($trace); + } else { + // If $trace is not an instance of the progress_trace, then set it to false to test whether debugging is triggered. + $selfplugin->send_expiry_notifications(false); + $this->assertDebuggingCalled( + 'enrol_plugin::send_expiry_notifications() now expects progress_trace instance as parameter!' + ); + } + $messages = $sink->get_messages(); + + $this->assertCount($numnotifications, $messages); + if ($numnotifications && ($customint2 > 0)) { + $this->assertEquals($user3->id, $messages[0]->useridto); + $this->assertStringContainsString('you have not visited', $messages[0]->fullmessagehtml); + } + + // Make sure that notifications are not repeated. + $sink->clear(); + + // Test that no more messages are sent the same day. + $selfplugin->send_expiry_notifications($trace); + $messages = $sink->get_messages(); + + $this->assertCount(0, $messages); + + // Test if an enrolment instance is disabled. + $selfplugin->update_status($instance1, ENROL_INSTANCE_DISABLED); + $this->assertNull($selfplugin->send_expiry_notifications($trace)); + $selfplugin->update_status($instance1, ENROL_INSTANCE_ENABLED); + + // Test if an expiry notify hour is null. + $selfplugin->set_config('expirynotifyhour', null); + $selfplugin->send_expiry_notifications($trace); + $this->assertDebuggingCalled('send_expiry_notifications() in self enrolment plugin needs expirynotifyhour setting'); + } + public function test_expired() { global $DB; $this->resetAfterTest(); diff --git a/lib/enrollib.php b/lib/enrollib.php index d27174a7bbd..efabbe85697 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -3157,7 +3157,7 @@ abstract class enrol_plugin { * @param progress_trace $trace (accepts bool for backwards compatibility only) */ public function send_expiry_notifications($trace) { - global $DB, $CFG; + global $CFG; $name = $this->get_name(); if (!enrol_is_enabled($name)) { @@ -3202,6 +3202,28 @@ abstract class enrol_plugin { $trace->output('Processing '.$name.' enrolment expiration notifications...'); // Notify users responsible for enrolment once every day. + $this->fetch_users_and_notify_expiry($timenow, $name, $trace); + + $trace->output('...notification processing finished.'); + $trace->finished(); + + $this->set_config('expirynotifylast', $timenow); + } + + /** + * Notify users about enrolment expiration. + * + * Retrieves enrolment data from the database and notifies users about their + * upcoming course enrolment expiration based on expiry thresholds and notification settings. + * + * @param int $timenow Current time. + * @param string $name Name of this enrol plugin. + * @param progress_trace $trace (accepts bool for backwards compatibility only). + * @return void + */ + protected function fetch_users_and_notify_expiry(int $timenow, string $name, progress_trace $trace): void { + global $DB, $CFG; + $sql = "SELECT ue.*, e.expirynotify, e.notifyall, e.expirythreshold, e.courseid, c.fullname FROM {user_enrolments} ue JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = :name AND e.expirynotify > 0 AND e.status = :enabled) @@ -3209,26 +3231,35 @@ abstract class enrol_plugin { JOIN {user} u ON (u.id = ue.userid AND u.deleted = 0 AND u.suspended = 0) WHERE ue.status = :active AND ue.timeend > 0 AND ue.timeend > :now1 AND ue.timeend < (e.expirythreshold + :now2) ORDER BY ue.enrolid ASC, u.lastname ASC, u.firstname ASC, u.id ASC"; - $params = array('enabled'=>ENROL_INSTANCE_ENABLED, 'active'=>ENROL_USER_ACTIVE, 'now1'=>$timenow, 'now2'=>$timenow, 'name'=>$name); + $params = [ + 'enabled' => ENROL_INSTANCE_ENABLED, + 'active' => ENROL_USER_ACTIVE, + 'now1' => $timenow, + 'now2' => $timenow, + 'name' => $name, + ]; $rs = $DB->get_recordset_sql($sql, $params); $lastenrollid = 0; - $users = array(); + $users = []; - foreach($rs as $ue) { - if ($lastenrollid and $lastenrollid != $ue->enrolid) { + foreach ($rs as $ue) { + if ($lastenrollid && $lastenrollid != $ue->enrolid) { $this->notify_expiry_enroller($lastenrollid, $users, $trace); - $users = array(); + $users = []; } $lastenrollid = $ue->enrolid; $enroller = $this->get_enroller($ue->enrolid); $context = context_course::instance($ue->courseid); - $user = $DB->get_record('user', array('id'=>$ue->userid)); + $user = $DB->get_record('user', ['id' => $ue->userid]); - $users[] = array('fullname'=>fullname($user, has_capability('moodle/site:viewfullnames', $context, $enroller)), 'timeend'=>$ue->timeend); + $users[] = [ + 'fullname' => fullname($user, has_capability('moodle/site:viewfullnames', $context, $enroller)), + 'timeend' => $ue->timeend, + ]; if (!$ue->notifyall) { continue; @@ -3236,7 +3267,8 @@ abstract class enrol_plugin { if ($ue->timeend - $ue->expirythreshold + 86400 < $timenow) { // Notify enrolled users only once at the start of the threshold. - $trace->output("user $ue->userid was already notified that enrolment in course $ue->courseid expires on ".userdate($ue->timeend, '', $CFG->timezone), 1); + $trace->output("user $ue->userid was already notified that enrolment in course $ue->courseid expires on ". + userdate($ue->timeend, '', $CFG->timezone), 1); continue; } @@ -3244,14 +3276,9 @@ abstract class enrol_plugin { } $rs->close(); - if ($lastenrollid and $users) { + if ($lastenrollid && $users) { $this->notify_expiry_enroller($lastenrollid, $users, $trace); } - - $trace->output('...notification processing finished.'); - $trace->finished(); - - $this->set_config('expirynotifylast', $timenow); } /** @@ -3287,14 +3314,10 @@ abstract class enrol_plugin { $enroller = $this->get_enroller($ue->enrolid); $context = context_course::instance($ue->courseid); - $a = new stdClass(); - $a->course = format_string($ue->fullname, true, array('context'=>$context)); - $a->user = fullname($user, true); - $a->timeend = userdate($ue->timeend, '', $user->timezone); - $a->enroller = fullname($enroller, has_capability('moodle/site:viewfullnames', $context, $user)); + $subject = get_string('expirymessageenrolledsubject', 'enrol_'.$name); + $body = $this->get_expiry_message_body($user, $ue, $name, $enroller, $context); - $subject = get_string('expirymessageenrolledsubject', 'enrol_'.$name, $a); - $body = get_string('expirymessageenrolledbody', 'enrol_'.$name, $a); + $coursename = format_string($ue->fullname, true, ['context' => $context]); $message = new \core\message\message(); $message->courseid = $ue->courseid; @@ -3308,18 +3331,40 @@ abstract class enrol_plugin { $message->fullmessageformat = FORMAT_MARKDOWN; $message->fullmessagehtml = markdown_to_html($body); $message->smallmessage = $subject; - $message->contexturlname = $a->course; - $message->contexturl = (string)new moodle_url('/course/view.php', array('id'=>$ue->courseid)); + $message->contexturlname = $coursename; + $message->contexturl = (string)new moodle_url('/course/view.php', ['id' => $ue->courseid]); if (message_send($message)) { - $trace->output("notifying user $ue->userid that enrolment in course $ue->courseid expires on ".userdate($ue->timeend, '', $CFG->timezone), 1); + $stringmessage = 'notifying user %s that enrolment in course %s expires on %s'; } else { - $trace->output("error notifying user $ue->userid that enrolment in course $ue->courseid expires on ".userdate($ue->timeend, '', $CFG->timezone), 1); + $stringmessage = 'error notifying user %s that enrolment in course %s expires on %s'; } + $outputmessage = sprintf($stringmessage, $ue->userid, $ue->courseid, userdate($ue->timeend, '', $CFG->timezone)); + $trace->output($outputmessage, 1); force_current_language($oldforcelang); } + /** + * Generate subject and body messages for enrolment expiration notification. + * + * @param stdClass $user An object representing the user. + * @param stdClass $ue An object containing enrolment data. + * @param string $name Name of this enrol plugin. + * @param stdClass $enroller The user who is responsible for enrolments. + * @param context $context The context object. + * @return string Return the body message. + */ + protected function get_expiry_message_body(stdClass $user, stdClass $ue, string $name, + stdClass $enroller, context $context): string { + $a = new stdClass(); + $a->course = format_string($ue->fullname, true, ['context' => $context]); + $a->user = fullname($user, true); + $a->timeend = userdate($ue->timeend, '', $user->timezone); + $a->enroller = fullname($enroller, has_capability('moodle/site:viewfullnames', $context, $user)); + return get_string('expirymessageenrolledbody', 'enrol_'.$name, $a); + } + /** * Notify person responsible for enrolments that some user enrolments will be expired soon, * it is called only if notification of enrollers (aka teachers) is enabled in course.