From 4e822cccfd595e652a8d1159d40e4d448fd1d118 Mon Sep 17 00:00:00 2001 From: Daniel Ziegenberg Date: Fri, 28 Mar 2025 10:36:08 +0100 Subject: [PATCH] MDL-85038 assignment: fix exception on sending assignment notification When the assignment gets deleted between the creation of the ad-hoc task mod_assign\task\send_assignment_due_soon_notification_to_user or mod_assign\task\send_assignment_overdue_notification_to_user and its execution, the ad-hoc task fails with a dml_missing_record_exception and the task gets stuck. In this case we do not want the task to fail and just return with an appropriate mtrace. Signed-off-by: Daniel Ziegenberg --- mod/assign/classes/notification_helper.php | 20 ++- mod/assign/tests/notification_helper_test.php | 120 ++++++++++++++++++ 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/mod/assign/classes/notification_helper.php b/mod/assign/classes/notification_helper.php index 6cd56f59fbe..d63e594459a 100644 --- a/mod/assign/classes/notification_helper.php +++ b/mod/assign/classes/notification_helper.php @@ -317,8 +317,14 @@ class notification_helper { * @param int $userid The user id. */ public static function send_due_soon_notification_to_user(int $assignmentid, int $userid): void { - // Get assignment data. - $assignmentobj = self::get_assignment_data($assignmentid); + try { + // Get assignment data. + $assignmentobj = self::get_assignment_data($assignmentid); + } catch (\dml_missing_record_exception) { + // The assignment has vanished, nothing to do. + mtrace("No notification send as the assignment $assignmentid can no longer be found in the database."); + return; + } // Check if the due date still within range. $assignmentobj->update_effective_access($userid); @@ -390,8 +396,14 @@ class notification_helper { * @param int $userid The user id. */ public static function send_overdue_notification_to_user(int $assignmentid, int $userid): void { - // Get assignment data. - $assignmentobj = self::get_assignment_data($assignmentid); + try { + // Get assignment data. + $assignmentobj = self::get_assignment_data($assignmentid); + } catch (\dml_missing_record_exception) { + // The assignment has vanished, nothing to do. + mtrace("No notification send as the assignment $assignmentid can no longer be found in the database."); + return; + } // Get the user and check they are a still a valid participant. $user = $assignmentobj->get_participant($userid); diff --git a/mod/assign/tests/notification_helper_test.php b/mod/assign/tests/notification_helper_test.php index 61db764523a..ea9b573f904 100644 --- a/mod/assign/tests/notification_helper_test.php +++ b/mod/assign/tests/notification_helper_test.php @@ -298,6 +298,65 @@ final class notification_helper_test extends \advanced_testcase { $sink->clear(); } + /** + * Test that we do not fail on deleted assignments with due soon notifications to a user. + */ + public function test_not_to_fail_on_deleted_assigment_with_due_soon_notifications_to_user(): void { + global $DB; + $this->resetAfterTest(); + $generator = $this->getDataGenerator(); + $clock = $this->mock_clock_with_frozen(); + + // Create a course and enrol a user. + $course = $generator->create_course(); + $user1 = $generator->create_user(); + $generator->enrol_user($user1->id, $course->id, 'student'); + + /** @var \mod_assign_generator $assignmentgenerator */ + $assignmentgenerator = $generator->get_plugin_generator('mod_assign'); + + // Create an assignment with a due date < 48 hours. + $duedate = $clock->time() + DAYSECS; + $assignment = $assignmentgenerator->create_instance([ + 'course' => $course->id, + 'duedate' => $duedate, + 'submissiondrafts' => 0, + 'assignsubmission_onlinetext_enabled' => 1, + ]); + $clock->bump(5); + + // Run the scheduled and ad-hoc task to queue the notifications. + $task = \core\task\manager::get_scheduled_task(\mod_assign\task\queue_all_assignment_due_soon_notification_tasks::class); + $task->execute(); + + $clock->bump(5); + $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $this->assertInstanceOf(\mod_assign\task\queue_assignment_due_soon_notification_tasks_for_users::class, $adhoctask); + $adhoctask->execute(); + \core\task\manager::adhoc_task_complete($adhoctask); + + // Delete the assignment. + $DB->delete_records('assign', ['id' => $assignment->id]); + + // Try to run the ad-hoc task to send the notifications. + $clock->bump(5); + $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $this->assertInstanceOf(\mod_assign\task\send_assignment_due_soon_notification_to_user::class, $adhoctask); + + ob_start(); + $adhoctask->execute(); + $output = ob_get_clean(); + + \core\task\manager::adhoc_task_complete($adhoctask); + + // The ad-hoc task should be deleted. + $this->assertNull(\core\task\manager::get_next_adhoc_task($clock->time())); + $this->assertStringContainsString( + needle: "No notification send as the assignment $assignment->id can no longer be found in the database.", + haystack: $output + ); + } + /** * Run all the tasks related to the 'overdue' notifications. */ @@ -574,6 +633,67 @@ final class notification_helper_test extends \advanced_testcase { $this->assertEmpty($sink->get_messages_by_component('mod_assign')); } + /** + * Test that we do not fail on deleted assignments with overdue notifications to a user. + */ + public function test_not_to_fail_on_deleted_assigment_with_overdue_notifications_to_user(): void { + global $DB; + $this->resetAfterTest(); + $generator = $this->getDataGenerator(); + $clock = $this->mock_clock_with_frozen(); + + // Create a course and enrol a user. + $course = $generator->create_course(); + $user1 = $generator->create_user(); + $generator->enrol_user($user1->id, $course->id, 'student'); + + /** @var \mod_assign_generator $assignmentgenerator */ + $assignmentgenerator = $generator->get_plugin_generator('mod_assign'); + + // Create an assignment that is overdue. + $duedate = $clock->time() - HOURSECS; + $cutoffdate = $clock->time() + DAYSECS; + $assignment = $assignmentgenerator->create_instance([ + 'course' => $course->id, + 'duedate' => $duedate, + 'cutoffdate' => $cutoffdate, + 'submissiondrafts' => 0, + 'assignsubmission_onlinetext_enabled' => 1, + ]); + $clock->bump(5); + + // Run the scheduled and ad-hoc task to queue the notifications. + $task = \core\task\manager::get_scheduled_task(\mod_assign\task\queue_all_assignment_overdue_notification_tasks::class); + $task->execute(); + + $clock->bump(5); + $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $this->assertInstanceOf(\mod_assign\task\queue_assignment_overdue_notification_tasks_for_users::class, $adhoctask); + $adhoctask->execute(); + \core\task\manager::adhoc_task_complete($adhoctask); + + // Delete the assignment. + $DB->delete_records('assign', ['id' => $assignment->id]); + + // Try to run the ad-hoc task to send the notifications. + $clock->bump(5); + $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $this->assertInstanceOf(\mod_assign\task\send_assignment_overdue_notification_to_user::class, $adhoctask); + + ob_start(); + $adhoctask->execute(); + $output = ob_get_clean(); + + \core\task\manager::adhoc_task_complete($adhoctask); + + // The ad-hoc task should be deleted. + $this->assertNull(\core\task\manager::get_next_adhoc_task($clock->time())); + $this->assertStringContainsString( + needle: "No notification send as the assignment $assignment->id can no longer be found in the database.", + haystack: $output + ); + } + /** * Run all the tasks related to the due digest notifications. */