From addefabcc044c4154e85b5d9c1abdd3c95b00cf4 Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Tue, 26 Aug 2025 12:21:53 +1000 Subject: [PATCH 1/2] Revert "Email task performance improvements and time limit" This reverts commit 6837829dab72592e62a4968f2e45beb43433e0bc. --- classes/task/email_certificate_task.php | 11 ++--------- tests/email_certificate_task_test.php | 21 --------------------- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/classes/task/email_certificate_task.php b/classes/task/email_certificate_task.php index 9ba754efd..4a50e1638 100644 --- a/classes/task/email_certificate_task.php +++ b/classes/task/email_certificate_task.php @@ -49,7 +49,6 @@ public function get_name() { public function execute() { global $DB; - $lastruntime = $DB->get_field('task_scheduled', 'lastruntime', ['classname' => "\\" . self::class]); // Get all the certificates that have requested someone get emailed. $emailotherslengthsql = $DB->sql_length('c.emailothers'); $sql = "SELECT c.*, ct.id as templateid, ct.name as templatename, ct.contextid, co.id as courseid, @@ -61,14 +60,8 @@ public function execute() { ON c.course = co.id WHERE (c.emailstudents = :emailstudents OR c.emailteachers = :emailteachers - OR $emailotherslengthsql >= 3) - AND :lastruntime <= ( - SELECT MAX(ula.timeaccess) - FROM {user_lastaccess} ula - WHERE ula.courseid = co.id - )"; - - if (!$customcerts = $DB->get_records_sql($sql, ['emailstudents' => 1, 'emailteachers' => 1, "lastruntime" => $lastruntime])) { + OR $emailotherslengthsql >= 3)"; + if (!$customcerts = $DB->get_records_sql($sql, array('emailstudents' => 1, 'emailteachers' => 1))) { return; } diff --git a/tests/email_certificate_task_test.php b/tests/email_certificate_task_test.php index 99dba232e..14bd8cb79 100644 --- a/tests/email_certificate_task_test.php +++ b/tests/email_certificate_task_test.php @@ -54,7 +54,6 @@ public function setUp(): void { * @covers \mod_customcert\task\email_certificate_task */ public function test_email_certificates_no_elements() { - global $DB; // Create a course. $course = $this->getDataGenerator()->create_course(); @@ -67,9 +66,6 @@ public function test_email_certificates_no_elements() { // Enrol the user as a student. $this->getDataGenerator()->enrol_user($user1->id, $course->id); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Run the task. $sink = $this->redirectEmails(); $task = new email_certificate_task(); @@ -105,9 +101,6 @@ public function test_email_certificates_no_cap() { // Create a custom certificate. $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, 'emailstudents' => 1]); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Create template object. $template = new stdClass(); $template->id = $customcert->templateid; @@ -162,9 +155,6 @@ public function test_email_certificates_students() { $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, 'emailstudents' => 1]); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Create template object. $template = new stdClass(); $template->id = $customcert->templateid; @@ -251,8 +241,6 @@ public function test_email_certificates_teachers() { // Create a custom certificate. $customcert = $this->getDataGenerator()->create_module('customcert', array('course' => $course->id, 'emailteachers' => 1)); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); // Create template object. $template = new stdClass(); @@ -309,9 +297,6 @@ public function test_email_certificates_others() { $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, 'emailothers' => 'testcustomcert@example.com, doo@dah']); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Create template object. $template = new stdClass(); $template->id = $customcert->templateid; @@ -365,9 +350,6 @@ public function test_email_certificates_students_not_visible() { // Create a custom certificate. $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, 'emailstudents' => 1]); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Create template object. $template = new stdClass(); $template->id = $customcert->templateid; @@ -425,9 +407,6 @@ public function test_email_certificates_students_havent_met_required_time() { $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, 'emailstudents' => 1, 'requiredtime' => '60']); - // Register activity in the course to make it valid to be sent. - $DB->insert_record('user_lastaccess', ['userid' => $user1->id, 'courseid' => $course->id, 'lastaccess' => time()]); - // Create template object. $template = new stdClass(); $template->id = $customcert->templateid; From 226227c4d9e1d3b79dac58de72187d6fb784f19e Mon Sep 17 00:00:00 2001 From: Matthew Hilton Date: Tue, 26 Aug 2025 14:12:36 +1000 Subject: [PATCH 2/2] perf: cherry-pick email performance fixes from upstream --- classes/task/email_certificate_adhoc_task.php | 187 +++++++ classes/task/email_certificate_task.php | 264 ++++----- lang/en/customcert.php | 13 + settings.php | 21 + tests/email_certificate_task_test.php | 523 +++++++++++++++++- version.php | 2 +- 6 files changed, 838 insertions(+), 172 deletions(-) create mode 100644 classes/task/email_certificate_adhoc_task.php diff --git a/classes/task/email_certificate_adhoc_task.php b/classes/task/email_certificate_adhoc_task.php new file mode 100644 index 000000000..d47ccd1ec --- /dev/null +++ b/classes/task/email_certificate_adhoc_task.php @@ -0,0 +1,187 @@ +. + +/** + * An adhoc task for emailing certificates. + * + * @package mod_customcert + * @copyright 2017 Mark Nelson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_customcert\task; + +use mod_customcert\helper; + +/** + * An adhoc task for emailing certificates per issue. + * + * @package mod_customcert + * @copyright 2017 Mark Nelson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class email_certificate_adhoc_task extends \core\task\adhoc_task { + + /** + * Get a descriptive name for this task (shown to admins). + * + * @return string + */ + public function get_name() { + return get_string('taskemailcertificate', 'customcert'); + } + + /** + * Execute. + */ + public function execute() { + global $DB; + + $customdata = $this->get_custom_data(); + if (empty($customdata) || empty($customdata->issueid) || empty($customdata->customcertid)) { + return; + } + + $issueid = (int)$customdata->issueid; + $customcertid = (int)$customdata->customcertid; + $sql = "SELECT c.*, ct.id as templateid, ct.name as templatename, ct.contextid, co.id as courseid, + co.fullname as coursefullname, co.shortname as courseshortname + FROM {customcert} c + JOIN {customcert_templates} ct ON c.templateid = ct.id + JOIN {course} co ON c.course = co.id + WHERE c.id = :id"; + + $customcert = $DB->get_record_sql($sql, ['id' => $customcertid]); + if (!$customcert) { + return; + } + + // The renderers used for sending emails. + $page = new \moodle_page(); + $htmlrenderer = $page->get_renderer('mod_customcert', 'email', 'htmlemail'); + $textrenderer = $page->get_renderer('mod_customcert', 'email', 'textemail'); + + // Get the context. + $context = \context::instance_by_id($customcert->contextid); + + // Get the person we are going to send this email on behalf of. + $userfrom = \core_user::get_noreply_user(); + + $courseshortname = format_string($customcert->courseshortname, true, ['context' => $context]); + $coursefullname = format_string($customcert->coursefullname, true, ['context' => $context]); + $certificatename = format_string($customcert->name, true, ['context' => $context]); + + // Used to create the email subject. + $info = new \stdClass(); + $info->coursename = $courseshortname; // Added for BC, so users who have edited the string don't lose this value. + $info->courseshortname = $courseshortname; + $info->coursefullname = $coursefullname; + $info->certificatename = $certificatename; + + // Get the information about the user and the certificate issue. + $userfields = helper::get_all_user_name_fields('u'); + $sql = "SELECT u.id, u.username, $userfields, u.email, u.mailformat, ci.id as issueid, ci.emailed + FROM {customcert_issues} ci + JOIN {user} u + ON ci.userid = u.id + WHERE ci.customcertid = :customcertid + AND ci.id = :issueid"; + $user = $DB->get_record_sql($sql, ['customcertid' => $customcertid, 'issueid' => $issueid]); + if (!$user) { + return; + } + + // Create a directory to store the PDF we will be sending. + $tempdir = make_temp_directory('certificate/attachment'); + if (!$tempdir) { + return; + } + + // Setup the user for the cron. + $this->set_userid($user->id); + + $userfullname = fullname($user); + $info->userfullname = $userfullname; + + // Now, get the PDF. + $template = new \stdClass(); + $template->id = $customcert->templateid; + $template->name = $customcert->templatename; + $template->contextid = $customcert->contextid; + $template = new \mod_customcert\template($template); + $filecontents = $template->generate_pdf(false, $user->id, true); + + // Set the name of the file we are going to send. + $filename = $courseshortname . '_' . $certificatename; + $filename = \core_text::entities_to_utf8($filename); + $filename = strip_tags($filename); + $filename = rtrim($filename, '.'); + $filename = str_replace('&', '_', $filename) . '.pdf'; + + // Create the file we will be sending. + $tempfile = $tempdir . '/' . md5(microtime() . $user->id) . '.pdf'; + file_put_contents($tempfile, $filecontents); + + if ($customcert->emailstudents) { + $renderable = new \mod_customcert\output\email_certificate(true, $userfullname, $courseshortname, + $coursefullname, $certificatename, $context->instanceid); + + $subject = get_string('emailstudentsubject', 'customcert', $info); + $message = $textrenderer->render($renderable); + $messagehtml = $htmlrenderer->render($renderable); + email_to_user($user, $userfrom, html_entity_decode($subject, ENT_COMPAT), $message, + $messagehtml, $tempfile, $filename); + } + + if ($customcert->emailteachers) { + $teachers = get_enrolled_users($context, 'moodle/course:update'); + + $renderable = new \mod_customcert\output\email_certificate(false, $userfullname, $courseshortname, + $coursefullname, $certificatename, $context->instanceid); + + $subject = get_string('emailnonstudentsubject', 'customcert', $info); + $message = $textrenderer->render($renderable); + $messagehtml = $htmlrenderer->render($renderable); + foreach ($teachers as $teacher) { + email_to_user($teacher, $userfrom, html_entity_decode($subject, ENT_COMPAT), + $message, $messagehtml, $tempfile, $filename); + } + } + + if (!empty($customcert->emailothers)) { + $others = explode(',', $customcert->emailothers); + foreach ($others as $email) { + $email = trim($email); + if (validate_email($email)) { + $renderable = new \mod_customcert\output\email_certificate(false, $userfullname, + $courseshortname, $coursefullname, $certificatename, $context->instanceid); + + $subject = get_string('emailnonstudentsubject', 'customcert', $info); + $message = $textrenderer->render($renderable); + $messagehtml = $htmlrenderer->render($renderable); + + $emailuser = new \stdClass(); + $emailuser->id = -1; + $emailuser->email = $email; + email_to_user($emailuser, $userfrom, html_entity_decode($subject, ENT_COMPAT), $message, + $messagehtml, $tempfile, $filename); + } + } + } + + // Set the field so that it is emailed. + $DB->set_field('customcert_issues', 'emailed', 1, ['id' => $issueid]); + } +} diff --git a/classes/task/email_certificate_task.php b/classes/task/email_certificate_task.php index 4a50e1638..7fdd2babd 100644 --- a/classes/task/email_certificate_task.php +++ b/classes/task/email_certificate_task.php @@ -15,21 +15,19 @@ // along with Moodle. If not, see . /** - * A scheduled task for emailing certificates. + * A scheduled task for issuing certificates that have requested someone get emailed. * * @package mod_customcert - * @copyright 2017 Mark Nelson + * @copyright 2024 Oscar Nadjar * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ namespace mod_customcert\task; -use mod_customcert\helper; - /** - * A scheduled task for emailing certificates. + * A scheduled task for issuing certificates that have requested someone get emailed. * * @package mod_customcert - * @copyright 2017 Mark Nelson + * @copyright 2024 Oscar Nadjar * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class email_certificate_task extends \core\task\scheduled_task { @@ -39,8 +37,8 @@ class email_certificate_task extends \core\task\scheduled_task { * * @return string */ - public function get_name() { - return get_string('taskemailcertificate', 'customcert'); + public function get_name(): string { + return get_string('taskissuecertificate', 'customcert'); } /** @@ -49,27 +47,63 @@ public function get_name() { public function execute() { global $DB; - // Get all the certificates that have requested someone get emailed. + // Get the certificatesperrun, includeinnotvisiblecourses, and certificateexecutionperiod configurations. + $certificatesperrun = (int)get_config('customcert', 'certificatesperrun'); + $includeinnotvisiblecourses = (bool)get_config('customcert', 'includeinnotvisiblecourses'); + $certificateexecutionperiod = (int)get_config('customcert', 'certificateexecutionperiod'); + $offset = (int)get_config('customcert', 'certificate_offset'); + $emailothersselect = "c.emailothers"; $emailotherslengthsql = $DB->sql_length('c.emailothers'); - $sql = "SELECT c.*, ct.id as templateid, ct.name as templatename, ct.contextid, co.id as courseid, - co.fullname as coursefullname, co.shortname as courseshortname + + $sql = "SELECT DISTINCT c.id, c.templateid, c.course, c.requiredtime, c.emailstudents, c.emailteachers, $emailothersselect, + ct.id AS templateid, ct.name AS templatename, ct.contextid, co.id AS courseid, + co.fullname AS coursefullname, co.shortname AS courseshortname FROM {customcert} c JOIN {customcert_templates} ct ON c.templateid = ct.id JOIN {course} co ON c.course = co.id + LEFT JOIN {course_categories} cat + ON co.category = cat.id + LEFT JOIN {customcert_issues} ci + ON c.id = ci.customcertid WHERE (c.emailstudents = :emailstudents - OR c.emailteachers = :emailteachers - OR $emailotherslengthsql >= 3)"; - if (!$customcerts = $DB->get_records_sql($sql, array('emailstudents' => 1, 'emailteachers' => 1))) { + OR c.emailteachers = :emailteachers + OR $emailotherslengthsql >= 3)"; + + $params = ['emailstudents' => 1, 'emailteachers' => 1]; + + // Check the includeinnotvisiblecourses configuration. + if (!$includeinnotvisiblecourses) { + // Exclude certificates from hidden courses. + $sql .= " AND co.visible = 1 AND (cat.visible = 1 OR cat.id IS NULL)"; + } + + // Add condition based on certificate execution period. + if ($certificateexecutionperiod > 0) { + // Include courses with no end date or end date greater than the specified period. + $sql .= " AND (co.enddate > :enddate OR (co.enddate = 0 AND (ci.timecreated > :enddate2 OR ci.timecreated IS NULL)))"; + $params['enddate'] = time() - $certificateexecutionperiod; + $params['enddate2'] = $params['enddate']; + } + + // Execute the SQL query. + $customcerts = $DB->get_records_sql($sql, $params, $offset, $certificatesperrun); + + // When we get to the end of the list, reset the offset. + set_config('certificate_offset', !empty($customcerts) ? $offset + $certificatesperrun : 0, 'customcert'); + + if (empty($customcerts)) { return; } - // The renderers used for sending emails. - $page = new \moodle_page(); - $htmlrenderer = $page->get_renderer('mod_customcert', 'email', 'htmlemail'); - $textrenderer = $page->get_renderer('mod_customcert', 'email', 'textemail'); foreach ($customcerts as $customcert) { + // Check if the certificate is hidden, quit early. + $cm = get_course_and_cm_from_instance($customcert->id, 'customcert', $customcert->course)[1]; + if (!$cm->visible) { + continue; + } + // Do not process an empty certificate. $sql = "SELECT ce.* FROM {customcert_elements} ce @@ -85,173 +119,83 @@ public function execute() { // Get the context. $context = \context::instance_by_id($customcert->contextid); - // Set the $page context - this ensures settings, such as language, are kept and don't default to the site settings. - $page->set_context($context); - - // Get the person we are going to send this email on behalf of. - $userfrom = \core_user::get_noreply_user(); - - // Store teachers for later. - $teachers = get_enrolled_users($context, 'moodle/course:update'); - - $courseshortname = format_string($customcert->courseshortname, true, ['context' => $context]); - $coursefullname = format_string($customcert->coursefullname, true, ['context' => $context]); - $certificatename = format_string($customcert->name, true, ['context' => $context]); - - // Used to create the email subject. - $info = new \stdClass; - $info->coursename = $courseshortname; // Added for BC, so users who have edited the string don't lose this value. - $info->courseshortname = $courseshortname; - $info->coursefullname = $coursefullname; - $info->certificatename = $certificatename; - - // Get a list of all the issues. - $userfields = helper::get_all_user_name_fields('u'); - $sql = "SELECT u.id, u.username, $userfields, u.email, ci.id as issueid, ci.emailed + // Get a list of all the issues that are already emailed (skip these users). + $sql = "SELECT u.id FROM {customcert_issues} ci JOIN {user} u ON ci.userid = u.id - WHERE ci.customcertid = :customcertid"; + WHERE ci.customcertid = :customcertid + AND ci.emailed = 1"; $issuedusers = $DB->get_records_sql($sql, ['customcertid' => $customcert->id]); - // Now, get a list of users who can access the certificate but have not yet. - $enrolledusers = get_enrolled_users(\context_course::instance($customcert->courseid), 'mod/customcert:view'); - foreach ($enrolledusers as $enroluser) { - // Check if the user has already been issued. - if (in_array($enroluser->id, array_keys((array) $issuedusers))) { - continue; - } + // Now, get a list of users who can Manage the certificate. + $userswithmanage = get_users_by_capability($context, 'mod/customcert:manage', 'u.id'); + + // Get the context of the Custom Certificate module. + $cmcontext = \context_module::instance($cm->id); - // Now check if the certificate is not visible to the current user. - $cm = get_fast_modinfo($customcert->courseid, $enroluser->id)->instances['customcert'][$customcert->id]; - if (!$cm->uservisible) { + // Get users with the mod/customcert:receiveissue capability in the Custom Certificate module context. + $userswithissue = get_users_by_capability($cmcontext, 'mod/customcert:receiveissue'); + // Get users with mod/customcert:view capability. + $userswithview = get_users_by_capability($cmcontext, 'mod/customcert:view'); + // Users with both mod/customcert:view and mod/customcert:receiveissue capabilities. + $userswithissueview = array_intersect_key($userswithissue, $userswithview); + + // Filter remaining users by availability conditions. + $infomodule = new \core_availability\info_module($cm); + $filteredusers = $infomodule->filter_user_list($userswithissueview); + + foreach ($filteredusers as $filtereduser) { + // Skip if the user has already been issued and emailed. + if (in_array($filtereduser->id, array_keys((array)$issuedusers))) { continue; } - // Don't want to email those with the capability to manage the certificate. - if (has_capability('mod/customcert:manage', $context, $enroluser->id)) { + // Don't want to issue to teachers/managers. + if (in_array($filtereduser->id, array_keys((array)$userswithmanage))) { continue; } - // Only email those with the capability to receive the certificate. - if (!has_capability('mod/customcert:receiveissue', $context, $enroluser->id)) { + // Check whether the CM is visible to this user. + $usercm = get_fast_modinfo($customcert->courseid, $filtereduser->id)->instances['customcert'][$customcert->id]; + if (!$usercm->uservisible) { continue; } - // Check that they have passed the required time. + // Check required time (if any). if (!empty($customcert->requiredtime)) { if (\mod_customcert\certificate::get_course_time($customcert->courseid, - $enroluser->id) < ($customcert->requiredtime * 60)) { + $filtereduser->id) < ($customcert->requiredtime * 60)) { continue; } } - // Ensure the cert hasn't already been issued, e.g via the UI (view.php) - a race condition. - $issueid = $DB->get_field('customcert_issues', 'id', - ['userid' => $enroluser->id, 'customcertid' => $customcert->id], IGNORE_MULTIPLE); - if (empty($issueid)) { - // Ok, issue them the certificate. - $issueid = \mod_customcert\certificate::issue_certificate($customcert->id, $enroluser->id); - } - - // Add them to the array so we email them. - $enroluser->issueid = $issueid; - $enroluser->emailed = 0; - $issuedusers[] = $enroluser; - } - - // Remove all the users who have already been emailed. - foreach ($issuedusers as $key => $issueduser) { - if ($issueduser->emailed) { - unset($issuedusers[$key]); - } - } - - // If there are no users to email we can return early. - if (!$issuedusers) { - continue; - } - - // Create a directory to store the PDF we will be sending. - $tempdir = make_temp_directory('certificate/attachment'); - if (!$tempdir) { - return; - } - - // Now, email the people we need to. - foreach ($issuedusers as $user) { - // Set up the user. - cron_setup_user($user); - - $userfullname = fullname($user); - $info->userfullname = $userfullname; - - // Now, get the PDF. - $template = new \stdClass(); - $template->id = $customcert->templateid; - $template->name = $customcert->templatename; - $template->contextid = $customcert->contextid; - $template = new \mod_customcert\template($template); - $filecontents = $template->generate_pdf(false, $user->id, true); - - // Set the name of the file we are going to send. - $filename = $courseshortname . '_' . $certificatename; - $filename = \core_text::entities_to_utf8($filename); - $filename = strip_tags($filename); - $filename = rtrim($filename, '.'); - $filename = str_replace('&', '_', $filename) . '.pdf'; - - // Create the file we will be sending. - $tempfile = $tempdir . '/' . md5(microtime() . $user->id) . '.pdf'; - file_put_contents($tempfile, $filecontents); - - if ($customcert->emailstudents) { - $renderable = new \mod_customcert\output\email_certificate(true, $userfullname, $courseshortname, - $coursefullname, $certificatename, $context->instanceid); - - $subject = get_string('emailstudentsubject', 'customcert', $info); - $message = $textrenderer->render($renderable); - $messagehtml = $htmlrenderer->render($renderable); - email_to_user($user, fullname($userfrom), html_entity_decode($subject), $message, $messagehtml, - $tempfile, $filename); - } - - if ($customcert->emailteachers) { - $renderable = new \mod_customcert\output\email_certificate(false, $userfullname, $courseshortname, - $coursefullname, $certificatename, $context->instanceid); - - $subject = get_string('emailnonstudentsubject', 'customcert', $info); - $message = $textrenderer->render($renderable); - $messagehtml = $htmlrenderer->render($renderable); - foreach ($teachers as $teacher) { - email_to_user($teacher, fullname($userfrom), html_entity_decode($subject), $message, $messagehtml, - $tempfile, $filename); - } + // Ensure the cert hasn't already been issued; if not, issue it now. + $issue = $DB->get_record('customcert_issues', + ['userid' => $filtereduser->id, 'customcertid' => $customcert->id], + 'id, emailed'); + + $issueid = null; + $emailed = 0; + if (!empty($issue)) { + $issueid = (int)$issue->id; + $emailed = (int)$issue->emailed; + } else { + $issueid = \mod_customcert\certificate::issue_certificate($customcert->id, $filtereduser->id); + $emailed = 0; } - if (!empty($customcert->emailothers)) { - $others = explode(',', $customcert->emailothers); - foreach ($others as $email) { - $email = trim($email); - if (validate_email($email)) { - $renderable = new \mod_customcert\output\email_certificate(false, $userfullname, - $courseshortname, $coursefullname, $certificatename, $context->instanceid); - - $subject = get_string('emailnonstudentsubject', 'customcert', $info); - $message = $textrenderer->render($renderable); - $messagehtml = $htmlrenderer->render($renderable); - - $emailuser = new \stdClass(); - $emailuser->id = -1; - $emailuser->email = $email; - email_to_user($emailuser, fullname($userfrom), html_entity_decode($subject), $message, - $messagehtml, $tempfile, $filename); - } + // If we have an issue and it has not been emailed yet, send it now. + if (!empty($issueid) && $emailed === 0) { + $task = new \mod_customcert\task\email_certificate_adhoc_task(); + $task->set_custom_data(['issueid' => $issueid, 'customcertid' => $customcert->id]); + $useadhoc = get_config('customcert', 'useadhoc'); + if ($useadhoc) { + \core\task\manager::queue_adhoc_task($task, true); + } else { + $task->execute(); } } - - // Set the field so that it is emailed. - $DB->set_field('customcert_issues', 'emailed', 1, ['id' => $user->issueid]); } } } diff --git a/lang/en/customcert.php b/lang/en/customcert.php index 28910b8fc..009d677f2 100644 --- a/lang/en/customcert.php +++ b/lang/en/customcert.php @@ -224,3 +224,16 @@ // Acess API. $string['customcert:managelanguages'] = 'Manage language on edit form'; + +$string['certificateexecutionperiod'] = 'Certificate execution period'; +$string['certificateexecutionperiod_desc'] = 'If not 0, the task will not process certificates whose course has been inactive or the last issue is older than the configured time. This may help to improve the performance of the scheduled task.'; +$string['certificatesperrun'] = 'Certificates per run'; +$string['certificatesperrun_desc'] = 'Enter the number of certificates to process per scheduled task run where 0 means it will process all certificates.'; +$string['scheduledtaskconfigdesc'] = 'Configure the settings for the scheduled task that processes certificates.'; +$string['scheduledtaskconfigheading'] = 'Scheduled task configuration'; +$string['includeinnotvisiblecourses'] = 'Include certificates in hidden courses'; +$string['includeinnotvisiblecourses_desc'] = 'Certificates from hidden courses are not proccesed by default. If you want to include them, enable this setting.'; +$string['useadhoc'] = 'Use Email Certificate Ad-hoc Task'; +$string['useadhoc_desc'] = 'When enabled, emails related to certificates will be handled immediately through an ad-hoc task created for each issue. When disabled, emails will be managed by the regular scheduled task.'; +$string['taskemailcertificate'] = 'Handles emailing certificates.'; +$string['taskissuecertificate'] = 'Issue certificates task'; diff --git a/settings.php b/settings.php index 6a648edc3..28130d035 100644 --- a/settings.php +++ b/settings.php @@ -83,6 +83,27 @@ get_string('preventcopy_desc', 'customcert'), 0)); +$settings->add(new admin_setting_heading('scheduledtaskconfig', + get_string('scheduledtaskconfigheading', 'customcert'), + get_string('scheduledtaskconfigdesc', 'customcert'))); + +$settings->add(new admin_setting_configtext('customcert/certificatesperrun', + get_string('certificatesperrun', 'customcert'), + get_string('certificatesperrun_desc', 'customcert'), 0, PARAM_INT)); + +$settings->add(new admin_setting_configcheckbox('customcert/includeinnotvisiblecourses', + get_string('includeinnotvisiblecourses', 'customcert'), + get_string('includeinnotvisiblecourses_desc', 'customcert'), 0)); + +$settings->add(new admin_setting_configcheckbox('customcert/useadhoc', + get_string('useadhoc', 'customcert'), + get_string('useadhoc_desc', 'customcert'), 0)); + +$settings->add(new admin_setting_configduration('customcert/certificateexecutionperiod', + get_string('certificateexecutionperiod', 'customcert'), + get_string('certificateexecutionperiod_desc', 'customcert'), 365 * DAYSECS)); + + $ADMIN->add('customcert', $settings); // Element plugin settings. diff --git a/tests/email_certificate_task_test.php b/tests/email_certificate_task_test.php index 14bd8cb79..1f600cf1d 100644 --- a/tests/email_certificate_task_test.php +++ b/tests/email_certificate_task_test.php @@ -25,9 +25,12 @@ namespace mod_customcert; +use completion_info; +use context_module; use stdClass; use context_course; use advanced_testcase; +use mod_customcert\task\email_certificate_adhoc_task; use mod_customcert\task\email_certificate_task; /** @@ -37,23 +40,28 @@ * @category test * @copyright 2017 Mark Nelson * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - * @coversDefaultClass \mod_customcert\task\email_certificate_task + * @coversDefaultClass \mod_customcert\task\email_certificate_adhoc_task */ -class email_certificate_task_test extends advanced_testcase { +final class email_certificate_task_test extends advanced_testcase { /** * Test set up. */ public function setUp(): void { $this->resetAfterTest(); + + set_config('certificateexecutionperiod', 0, 'customcert'); + + parent::setUp(); } /** * Tests the email certificate task when there are no elements. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_no_elements() { + public function test_email_certificates_no_elements(): void { // Create a course. $course = $this->getDataGenerator()->create_course(); @@ -80,8 +88,9 @@ public function test_email_certificates_no_elements() { * Tests the email certificate task for users without a capability to receive a certificate. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_no_cap() { + public function test_email_certificates_no_cap(): void { global $DB; // Create a course. @@ -131,8 +140,9 @@ public function test_email_certificates_no_cap() { * Tests the email certificate task for students. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_students() { + public function test_email_certificates_students(): void { global $CFG, $DB; // Create a course. @@ -214,12 +224,92 @@ public function test_email_certificates_students() { $this->assertCount(0, $emails); } + /** + * Tests the email certificate task for instance on the site home course. + * + * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_certificates_sitehome(): void { + global $CFG, $DB, $SITE; + + // Create some users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + + // Create a custom certificate. + $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $SITE->id, + 'emailstudents' => 1]); + + $role = $DB->get_record('role', ['archetype' => 'user']); + role_change_permission($role->id, context_module::instance($customcert->cmid), 'mod/customcert:view', CAP_ALLOW); + role_change_permission($role->id, context_module::instance($customcert->cmid), 'mod/customcert:receiveissue', CAP_ALLOW); + + // Create template object. + $template = new stdClass(); + $template->id = $customcert->templateid; + $template->name = 'A template'; + $template->contextid = context_course::instance($SITE->id)->id; + $template = new template($template); + + // Add a page to this template. + $pageid = $template->add_page(); + + // Add an element to the page. + $element = new stdClass(); + $element->pageid = $pageid; + $element->name = 'Image'; + $DB->insert_record('customcert_elements', $element); + + // Ok, now issue this to one user. + \mod_customcert\certificate::issue_certificate($customcert->id, $user1->id); + + // Confirm there is only one entry in this table. + $this->assertEquals(1, $DB->count_records('customcert_issues')); + + // Run the task. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + // Get the issues from the issues table now. + $issues = $DB->get_records('customcert_issues'); + $this->assertCount(3, $issues); + + // Confirm that it was marked as emailed and was not issued to the teacher. + foreach ($issues as $issue) { + $this->assertEquals(1, $issue->emailed); + } + + // Confirm that we sent out emails to the two users. + $this->assertCount(3, $emails); + + $this->assertEquals($CFG->noreplyaddress, $emails[1]->from); + $this->assertEquals($user1->email, $emails[1]->to); + + $this->assertEquals($CFG->noreplyaddress, $emails[2]->from); + $this->assertEquals($user2->email, $emails[2]->to); + + // Now, run the task again and ensure we did not issue any more certificates. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + $issues = $DB->get_records('customcert_issues'); + + $this->assertCount(3, $issues); + $this->assertCount(0, $emails); + } + /** * Tests the email certificate task for teachers. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_teachers() { + public function test_email_certificates_teachers(): void { global $CFG, $DB; // Create a course. @@ -239,8 +329,8 @@ public function test_email_certificates_teachers() { $this->getDataGenerator()->enrol_user($user3->id, $course->id, $roleids['editingteacher']); // Create a custom certificate. - $customcert = $this->getDataGenerator()->create_module('customcert', array('course' => $course->id, - 'emailteachers' => 1)); + $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, + 'emailteachers' => 1]); // Create template object. $template = new stdClass(); @@ -278,8 +368,9 @@ public function test_email_certificates_teachers() { * Tests the email certificate task for others. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_others() { + public function test_email_certificates_others(): void { global $CFG, $DB; // Create a course. @@ -333,8 +424,9 @@ public function test_email_certificates_others() { * Tests the email certificate task when the certificate is not visible. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_students_not_visible() { + public function test_email_certificates_students_not_visible(): void { global $DB; // Create a course. @@ -387,8 +479,9 @@ public function test_email_certificates_students_not_visible() { * Tests the email certificate task when the student has not met the required time for the course. * * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task */ - public function test_email_certificates_students_havent_met_required_time() { + public function test_email_certificates_students_havent_met_required_time(): void { global $DB; // Set the standard log to on. @@ -436,4 +529,412 @@ public function test_email_certificates_students_havent_met_required_time() { // Confirm no emails were sent. $this->assertCount(0, $emails); } + + /** + * Tests the email certificate task when the student has not met the completion criteria. + * + * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_certificates_students_havent_met_required_criteria(): void { + global $CFG, $DB; + + $CFG->enablecompletion = true; + + // Create a course. + $course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]); + + // Create a user. + $user1 = $this->getDataGenerator()->create_user(); + + // Enrol them in the course. + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + + // Create a quiz. + $quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]); + + $quizmodule = $DB->get_record('course_modules', ['id' => $quiz->cmid]); + + // Set completion criteria for the quiz. + $quizmodule->completion = COMPLETION_TRACKING_AUTOMATIC; + $quizmodule->completionview = 1; // Require view to complete. + $quizmodule->completionexpected = 0; + $DB->update_record('course_modules', $quizmodule); + + // Set restrict access to the customcert activity based on the completion of the quiz. + $customcert = $this->getDataGenerator()->create_module('customcert', [ + 'course' => $course->id, + 'emailstudents' => 1, + 'availability' => json_encode( + [ + 'op' => '&', + 'c' => [ + [ + 'type' => 'completion', + 'cm' => $quiz->cmid, + 'e' => COMPLETION_COMPLETE, // Ensure the quiz is marked as complete. + ], + ], + 'showc' => [ + false, + ], + ], + ), + ]); + + // Create template object. + $template = new stdClass(); + $template->id = $customcert->templateid; + $template->name = 'A template'; + $template->contextid = context_course::instance($course->id)->id; + $template = new template($template); + + // Add a page to this template. + $pageid = $template->add_page(); + + // Add an element to the page. + $element = new stdClass(); + $element->pageid = $pageid; + $element->name = 'Image'; + $DB->insert_record('customcert_elements', $element); + + // Run the task. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + // Confirm there are no issues as the user can not view the certificate. + $issues = $DB->get_records('customcert_issues'); + $this->assertCount(0, $issues); + + // Confirm no emails were sent. + $this->assertCount(0, $emails); + } + + /** + * Tests the email certificate task when the student has met the completion criteria. + * + * @covers \mod_customcert\task\email_certificate_task + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_certificates_students_have_met_required_criteria(): void { + global $CFG, $DB; + + $CFG->enablecompletion = true; + + // Create a course. + $course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]); + + // Create a user. + $user1 = $this->getDataGenerator()->create_user(); + + // Enrol them in the course. + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + + // Create a quiz. + $quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]); + + $quizmodule = $DB->get_record('course_modules', ['id' => $quiz->cmid]); + + // Set completion criteria for the quiz. + $quizmodule->completion = COMPLETION_TRACKING_AUTOMATIC; + $quizmodule->completionview = 1; // Require view to complete. + $quizmodule->completionexpected = 0; + $DB->update_record('course_modules', $quizmodule); + + // Mark the quiz as complete for the user. + $completion = new completion_info($course); + $completion->update_state($quizmodule, COMPLETION_COMPLETE, $user1->id); + + // Set restrict access to the customcert activity based on the completion of the quiz. + $customcert = $this->getDataGenerator()->create_module('customcert', [ + 'course' => $course->id, + 'emailstudents' => 1, + 'availability' => json_encode( + [ + 'op' => '&', + 'c' => [ + [ + 'type' => 'completion', + 'cm' => $quiz->cmid, + 'e' => COMPLETION_COMPLETE, // Ensure the quiz is marked as complete. + ], + ], + 'showc' => [ + false, + ], + ], + ), + ]); + + // Create template object. + $template = new stdClass(); + $template->id = $customcert->templateid; + $template->name = 'A template'; + $template->contextid = context_course::instance($course->id)->id; + $template = new template($template); + + // Add a page to this template. + $pageid = $template->add_page(); + + // Add an element to the page. + $element = new stdClass(); + $element->pageid = $pageid; + $element->name = 'Image'; + $DB->insert_record('customcert_elements', $element); + + // Run the task. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + // Confirm there is an issue as the user can view the certificate. + $issues = $DB->get_records('customcert_issues'); + $this->assertCount(1, $issues); + + // Confirm an email was sent. + $this->assertCount(1, $emails); + } + + /** + * Tests the email certificate task running adhoc. + * + * @covers \mod_customcert\task\email_certificate_adhoc_task + * @covers \mod_customcert\task\email_certificate_task + */ + public function test_email_certificates_adhoc(): void { + global $CFG, $DB; + + set_config('useadhoc', 1, 'customcert'); + + // Create a course. + $course = $this->getDataGenerator()->create_course(); + + // Create some users. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(['firstname' => 'Teacher', 'lastname' => 'One']); + + // Enrol two of them in the course as students. + $roleids = $DB->get_records_menu('role', null, '', 'shortname, id'); + $this->getDataGenerator()->enrol_user($user1->id, $course->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id); + + // Enrol one of the users as a teacher. + $this->getDataGenerator()->enrol_user($user3->id, $course->id, $roleids['editingteacher']); + + // Create a custom certificate. + $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, + 'emailstudents' => 1]); + + // Create template object. + $template = new stdClass(); + $template->id = $customcert->templateid; + $template->name = 'A template'; + $template->contextid = context_course::instance($course->id)->id; + $template = new template($template); + + // Add a page to this template. + $pageid = $template->add_page(); + + // Add an element to the page. + $element = new stdClass(); + $element->pageid = $pageid; + $element->name = 'Image'; + $DB->insert_record('customcert_elements', $element); + + // Ok, now issue this to one user. + \mod_customcert\certificate::issue_certificate($customcert->id, $user1->id); + + // Confirm there is only one entry in this table. + $this->assertEquals(1, $DB->count_records('customcert_issues')); + + // Run the task. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + // Get the issues from the issues table now. + $issues = $DB->get_records('customcert_issues'); + $this->assertCount(2, $issues); + + // Confirm that it wasn't marked as emailed and was not issued to the teacher. + foreach ($issues as $issue) { + $this->assertEquals(0, $issue->emailed); + $this->assertNotEquals($user3->id, $issue->userid); + } + + // Now we send emails to the two users using the adhoc method. + $this->assertCount(0, $emails); + $issues = array_values($issues); + $task = new email_certificate_adhoc_task(); + $task->set_custom_data((object)['issueid' => $issues[0]->id, 'customcertid' => $customcert->id]); + $task->execute(); + $task->set_custom_data((object)['issueid' => $issues[1]->id, 'customcertid' => $customcert->id]); + $task->execute(); + $emails = $sink->get_messages(); + + // Get the issues from the issues table now. + $issues = $DB->get_records('customcert_issues'); + // Confirm that it wasn't marked as emailed and was not issued to the teacher. + foreach ($issues as $issue) { + $this->assertEquals(1, $issue->emailed); + $this->assertNotEquals($user3->id, $issue->userid); + } + + // Confirm that we sent out emails to the two users. + $this->assertCount(2, $emails); + + $this->assertEquals($CFG->noreplyaddress, $emails[0]->from); + $this->assertEquals($user1->email, $emails[0]->to); + + $this->assertEquals($CFG->noreplyaddress, $emails[1]->from); + $this->assertEquals($user2->email, $emails[1]->to); + + // Now, run the task again and ensure we did not issue any more certificates. + $sink = $this->redirectEmails(); + $task = new email_certificate_task(); + $task->execute(); + $emails = $sink->get_messages(); + + $issues = $DB->get_records('customcert_issues'); + + $this->assertCount(2, $issues); + $this->assertCount(0, $emails); + } + + /** + * Tests that we still issue a certificate if there are none when 'certificateexecutionperiod' is set. + * + * @covers \mod_customcert\task\email_certificate_task + */ + public function test_email_certificate_task_creates_issue_when_none_exist(): void { + global $CFG, $DB; + + // Create a course. + $course = $this->getDataGenerator()->create_course(); + + // Create a student user. + $student = $this->getDataGenerator()->create_user(); + + // Enrol the student in the course. + $this->getDataGenerator()->enrol_user($student->id, $course->id); + + // Create a custom certificate module with emailing enabled for students. + $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id, + 'emailstudents' => 1]); + + // Set up a basic certificate template. + $template = new \stdClass(); + $template->id = $customcert->templateid; + $template->name = 'Test Template'; + $template->contextid = \context_course::instance($course->id)->id; + $template = new template($template); + + // Add a page and an element to put the certificate in a valid state. + $pageid = $template->add_page(); + $element = new \stdClass(); + $element->pageid = $pageid; + $element->name = 'Test Element'; + $DB->insert_record('customcert_elements', $element); + + // Verify that no certificate issues exist before task execution. + $this->assertEmpty($DB->get_records('customcert_issues'), + 'No certificate issues should exist before executing the task.'); + + // Redirect emails to a sink so we can capture any outgoing messages. + $sink = $this->redirectEmails(); + + set_config('certificateexecutionperiod', 1, 'customcert'); + + // Execute the issue certificates task. + $task = new \mod_customcert\task\email_certificate_task(); + $task->execute(); + + // After executing the task, verify that a certificate issue record was created. + $issues = $DB->get_records('customcert_issues'); + $this->assertCount(1, $issues, + 'A certificate issue record should have been created by the task.'); + $issue = reset($issues); + $this->assertEquals(1, $issue->emailed, + 'The certificate issue should be marked as emailed.'); + + // Verify that an email was sent to the student. + $emails = $sink->get_messages(); + $this->assertCount(1, $emails, 'An email should have been sent to the student.'); + $this->assertEquals($CFG->noreplyaddress, $emails[0]->from, 'Email sender is incorrect.'); + $this->assertEquals($student->email, $emails[0]->to, 'Email recipient is incorrect.'); + } + + /** + * The email_certificate_adhoc_task should no-op (not fatal) when custom data is missing. + * + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_adhoc_task_ignores_missing_customdata(): void { + // No setup; call the adhoc task with no custom data. + $sink = $this->redirectEmails(); + + $task = new \mod_customcert\task\email_certificate_adhoc_task(); + // Intentionally DO NOT call set_custom_data(). + $task->execute(); + + // Should not throw; should not send any email. + $emails = $sink->get_messages(); + $this->assertCount(0, $emails); + } + + /** + * The email_certificate_adhoc_task should no-op when customcert id is invalid. + * + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_adhoc_task_invalid_customcertid(): void { + $sink = $this->redirectEmails(); + + $task = new \mod_customcert\task\email_certificate_adhoc_task(); + + // Point to bogus ids; both should be integers but not exist. + $task->set_custom_data((object)['issueid' => 999999, 'customcertid' => 999998]); + $task->execute(); + + $emails = $sink->get_messages(); + $this->assertCount(0, $emails); + } + + /** + * The email_certificate_adhoc_task should no-op when issue id is invalid (even if customcert exists). + * + * @covers \mod_customcert\task\email_certificate_adhoc_task + */ + public function test_email_adhoc_task_invalid_issueid_with_valid_customcert(): void { + global $DB; + + // Minimal valid customcert (with one element) so the customcert lookup succeeds. + $course = $this->getDataGenerator()->create_course(); + $customcert = $this->getDataGenerator()->create_module('customcert', ['course' => $course->id]); + + // Make the template valid. + $template = new \stdClass(); + $template->id = $customcert->templateid; + $template->name = 'T'; + $template->contextid = \context_course::instance($course->id)->id; + $template = new \mod_customcert\template($template); + $pageid = $template->add_page(); + $DB->insert_record('customcert_elements', (object)['pageid' => $pageid, 'name' => 'E']); + + $sink = $this->redirectEmails(); + + $task = new \mod_customcert\task\email_certificate_adhoc_task(); + + // Valid customcertid, but bogus issueid. + $task->set_custom_data((object)['issueid' => 123456789, 'customcertid' => $customcert->id]); + $task->execute(); + + $emails = $sink->get_messages(); + $this->assertCount(0, $emails); + } } diff --git a/version.php b/version.php index 70524b0b9..ab98d1d17 100644 --- a/version.php +++ b/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die('Direct access to this script is forbidden.'); -$plugin->version = 2022112806; // The current module version (Date: YYYYMMDDXX). +$plugin->version = 2022112806.02; // The current module version (Date: YYYYMMDDXX). $plugin->requires = 2022112800; // Requires this Moodle version (4.1). $plugin->cron = 0; // Period for cron to check this module (secs). $plugin->component = 'mod_customcert';