From cd80e0ae07a88bdf8c03e0afe79fd4e20e0ff7b4 Mon Sep 17 00:00:00 2001 From: Marty Date: Wed, 10 Sep 2025 15:19:56 -0400 Subject: [PATCH] MDL-86423 mod_assign: Reduce duesoon/overdue notifications. Students shouldn't receive overdue notifications if there is nothing to submit in Moodle. Students shouldn't receive due soon notifications if they a) already have a grade or b) meet activity completion status. --- mod/assign/classes/notification_helper.php | 32 +++++++- mod/assign/lang/en/assign.php | 5 ++ mod/assign/tests/notification_helper_test.php | 80 ++++++++++++++++++- 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/mod/assign/classes/notification_helper.php b/mod/assign/classes/notification_helper.php index 4f10fac14cd..a7dbd45f391 100644 --- a/mod/assign/classes/notification_helper.php +++ b/mod/assign/classes/notification_helper.php @@ -229,6 +229,19 @@ class notification_helper { // Get our assignment users. $users = $assignmentobj->list_participants(0, true, false, true); + // If it's an overdue type and there are no submission plugins, skip the assignment. + if ($type == self::TYPE_OVERDUE) { + if (!$assignmentobj->is_any_submission_plugin_enabled()) { + // No enabled submission plugins. Removing users. + return []; + } + } + + // Get completion info for this module. + $cm = $assignmentobj->get_course_module(); + $course = $assignmentobj->get_course(); + $completion = new \completion_info($course); + foreach ($users as $key => $user) { // Check if the user has submitted already. $submission = $assignmentobj->get_user_submission($user->id, false); @@ -247,6 +260,19 @@ class notification_helper { continue; } + // Don't send if the user has a grade for this assignment already. + if ($assignmentobj->get_grading_status($user->id) === ASSIGN_GRADING_STATUS_GRADED) { + unset($users[$key]); + continue; + } + + // Don't send if the user has met completion conditions. + $completiondata = $completion->get_data($cm, false, $user->id); + if ($completiondata && $completiondata->completionstate != COMPLETION_INCOMPLETE) { + unset($users[$key]); + continue; + } + // Perform some checks depending on the notification type. $match = []; $checksent = true; @@ -370,10 +396,14 @@ class notification_helper { 'url' => $url->out(false), 'subject' => get_string('assignmentduesoonsubject', 'mod_assign', $stringparams), 'assignmentname' => $stringparams['assignmentname'], - 'html' => get_string('assignmentduesoonhtml', 'mod_assign', $stringparams), 'sms' => get_string('assignmentduesoonsms', 'mod_assign', $stringparams), ]; + // Offline assignments will have a slightly different message. + $messagedata['html'] = !$assignmentobj->is_any_submission_plugin_enabled() + ? get_string('assignmentduesoonofflinehtml', 'mod_assign', $stringparams) + : get_string('assignmentduesoonhtml', 'mod_assign', $stringparams); + $message = new \core\message\message(); $message->component = 'mod_assign'; $message->name = self::TYPE_DUE_SOON; diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index d6591ca39db..cb4921743da 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -79,6 +79,11 @@ $string['assignmentduesoonhtml'] = '

Hi {$a->firstname},

The assignment {$a->assignmentname} in course {$a->coursename} is due soon.

Due: {$a->duedate}

Go to activity

'; +$string['assignmentduesoonofflinehtml'] = '

Hi {$a->firstname},

+

The assignment {$a->assignmentname} in course {$a->coursename} is due soon.

+

If you have already submitted your work, no further action is required.

+

Due: {$a->duedate}

+

Go to activity

'; $string['assignmentduesoonsms'] = 'Your assignment {$a->assignmentname} is due on {$a->duedate}: {$a->url}'; $string['assignmentoverduehtml'] = '

Hi {$a->firstname},

{$a->assignmentname} in course {$a->coursename} was due on {$a->duedate}.

diff --git a/mod/assign/tests/notification_helper_test.php b/mod/assign/tests/notification_helper_test.php index 1ab5287a40a..fceee9097ac 100644 --- a/mod/assign/tests/notification_helper_test.php +++ b/mod/assign/tests/notification_helper_test.php @@ -90,19 +90,23 @@ final class notification_helper_test extends \advanced_testcase { $clock = $this->mock_clock_with_frozen(); // Create a course and enrol some users. - $course = $generator->create_course(); + $course = $generator->create_course(['enablecompletion' => 1]); $user1 = $generator->create_user(); $user2 = $generator->create_user(); $user3 = $generator->create_user(); $user4 = $generator->create_user(); $user5 = $generator->create_user(); $user6 = $generator->create_user(); + $user7 = $generator->create_user(); + $user8 = $generator->create_user(); $generator->enrol_user($user1->id, $course->id, 'student'); $generator->enrol_user($user2->id, $course->id, 'student'); $generator->enrol_user($user3->id, $course->id, 'student'); $generator->enrol_user($user4->id, $course->id, 'student'); $generator->enrol_user($user5->id, $course->id, 'student'); - $generator->enrol_user($user6->id, $course->id, 'teacher'); + $generator->enrol_user($user6->id, $course->id, 'student'); + $generator->enrol_user($user7->id, $course->id, 'student'); + $generator->enrol_user($user8->id, $course->id, 'teacher'); /** @var \mod_assign_generator $assignmentgenerator */ $assignmentgenerator = $generator->get_plugin_generator('mod_assign'); @@ -114,6 +118,9 @@ final class notification_helper_test extends \advanced_testcase { 'duedate' => $duedate, 'submissiondrafts' => 0, 'assignsubmission_onlinetext_enabled' => 1, + ], [ + 'completion' => \COMPLETION_TRACKING_AUTOMATIC, + 'completionview' => 1, ]); // User1 will have a user override, giving them an extra 1 hour for 'duedate'. @@ -152,9 +159,28 @@ final class notification_helper_test extends \advanced_testcase { 'assignsubmission_onlinetext_enabled' => 1, ]); + // User6 will exclude themselves by meeting completion conditions. + $completion = new \completion_info($course); + $assigncm = get_coursemodule_from_instance('assign', $assignment->id); + $completion->set_module_viewed($assigncm, $user6->id); + + // User7 will exclude themselves b/c they already have a grade. + $context = \context_module::instance($assigncm->id); + $assign = new \assign($context, $assigncm, $course); + $this->setUser($user8); + $gradedata = new \stdClass(); + $gradedata->grade = '80.0'; + $gradedata->attemptnumber = 1; + $assign->save_grade($user7->id, $gradedata); + + $this->setUser(); + // There should be 3 users with the teacher excluded. $users = $helper::get_users_within_assignment($assignment->id, $helper::TYPE_DUE_SOON); $this->assertCount(3, $users); + $this->assertArrayHasKey($user1->id, $users); + $this->assertArrayHasKey($user2->id, $users); + $this->assertArrayHasKey($user3->id, $users); } /** @@ -416,7 +442,7 @@ final class notification_helper_test extends \advanced_testcase { $clock = $this->mock_clock_with_frozen(); // Create a course and enrol some users. - $course = $generator->create_course(); + $course = $generator->create_course(['enablecompletion' => 1]); $user1 = $generator->create_and_enrol($course, 'student'); $user2 = $generator->create_and_enrol($course, 'student'); $user3 = $generator->create_and_enrol($course, 'student'); @@ -488,6 +514,54 @@ final class notification_helper_test extends \advanced_testcase { $this->assertArrayHasKey($user1->id, $users); $this->assertArrayHasKey($user2->id, $users); $this->assertArrayHasKey($user3->id, $users); + + // Create an overdue assignment with no submission plugins enabled. + $duedate = $clock->time() - HOURSECS; + $nosubassignment = $assignmentgenerator->create_instance([ + 'course' => $course->id, + 'duedate' => $duedate, + 'submissiondrafts' => 0, + 'assignsubmission_onlinetext_enabled' => 0, + 'assignsubmission_file_enabled' => 0, + ]); + + // There should be 0 users b/c this assignment has no deliverables. + $users = $helper::get_users_within_assignment($nosubassignment->id, $helper::TYPE_OVERDUE); + $this->assertCount(0, $users); + + // Create an assignment that requires a 'view' to be completed. + $viewassignment = $assignmentgenerator->create_instance([ + 'course' => $course->id, + 'name' => 'View, Grade Assignment', + 'duedate' => $duedate, + 'assignsubmission_onlinetext_enabled' => true, + ], [ + 'completion' => \COMPLETION_TRACKING_AUTOMATIC, + 'completionview' => 1, + ]); + + // Mark the assignment as viewed by User1, excluding them from the notification. + $completion = new \completion_info($course); + $assigncm = get_coursemodule_from_instance('assign', $viewassignment->id); + $completion->set_module_viewed($assigncm, $user1->id); + + // Submit a grade for User2, excluding them from the notification, even though they don't have a submission. + $context = \context_module::instance($assigncm->id); + $assign = new \assign($context, $assigncm, $course); + $this->setUser($user7); + $gradedata = new \stdClass(); + $gradedata->grade = '80.0'; + $gradedata->attemptnumber = 1; + $assign->save_grade($user2->id, $gradedata); + $this->setUser(); + + // There should be 4 users, because user1 met activity completion and user2 has a grade. + $users = $helper::get_users_within_assignment($viewassignment->id, $helper::TYPE_OVERDUE); + $this->assertCount(4, $users); + $this->assertArrayHasKey($user3->id, $users); + $this->assertArrayHasKey($user4->id, $users); + $this->assertArrayHasKey($user5->id, $users); + $this->assertArrayHasKey($user6->id, $users); } /**