diff --git a/mod/quiz/report/statistics/classes/event/observer/attempt_submitted.php b/mod/quiz/report/statistics/classes/event/observer/attempt_submitted.php index acafe598377..b83ff8a7ef9 100644 --- a/mod/quiz/report/statistics/classes/event/observer/attempt_submitted.php +++ b/mod/quiz/report/statistics/classes/event/observer/attempt_submitted.php @@ -38,10 +38,6 @@ class attempt_submitted { */ public static function process(\mod_quiz\event\attempt_submitted $event): void { $data = $event->get_data(); - $quizid = $data['other']['quizid']; - - $task = recalculate::instance($quizid); - $task->set_next_run_time(time() + HOURSECS); - \core\task\manager::queue_adhoc_task($task, true); + recalculate::queue_future_run($data['other']['quizid']); } } diff --git a/mod/quiz/report/statistics/classes/quiz_attempt_deleted.php b/mod/quiz/report/statistics/classes/quiz_attempt_deleted.php new file mode 100644 index 00000000000..8c6dccda6c7 --- /dev/null +++ b/mod/quiz/report/statistics/classes/quiz_attempt_deleted.php @@ -0,0 +1,39 @@ +. + +namespace quiz_statistics; + +use quiz_statistics\task\recalculate; + +/** + * Queue a statistics recalculation when an attempt is deleted. + * + * @package quiz_statistics + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class quiz_attempt_deleted { + /** + * Queue a recalculation. + * + * @param int $quizid The quiz the attempt belongs to. + * @return void + */ + public static function callback(int $quizid): void { + recalculate::queue_future_run($quizid); + } +} diff --git a/mod/quiz/report/statistics/classes/task/recalculate.php b/mod/quiz/report/statistics/classes/task/recalculate.php index dd2cb144677..f869ec4e739 100644 --- a/mod/quiz/report/statistics/classes/task/recalculate.php +++ b/mod/quiz/report/statistics/classes/task/recalculate.php @@ -36,6 +36,11 @@ require_once($CFG->dirroot . '/mod/quiz/report/statistics/report.php'); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class recalculate extends \core\task\adhoc_task { + /** + * The time to delay queued runs by, to prevent repeated recalculations. + */ + const DELAY = HOURSECS; + /** * Create a new instance of the task. * @@ -93,4 +98,20 @@ class recalculate extends \core\task\adhoc_task { $report->calculate_questions_stats_for_question_bank($quiz->id); mtrace(' Calculations completed at ' . userdate(time(), $dateformat) . '.'); } + + /** + * Queue an instance of this task to happen after a delay. + * + * Multiple events may happen over a short period that require a recalculation. Rather than + * run the recalculation each time, this will queue a single run of the task for a given quiz, + * within the delay period. + * + * @param int $quizid The quiz to run the recalculation for. + * @return bool true of the task was queued. + */ + public static function queue_future_run(int $quizid): bool { + $task = self::instance($quizid); + $task->set_next_run_time(time() + self::DELAY); + return \core\task\manager::queue_adhoc_task($task, true); + } } diff --git a/mod/quiz/report/statistics/classes/tests/statistics_helper.php b/mod/quiz/report/statistics/classes/tests/statistics_helper.php index 23b7873664b..471356651bf 100644 --- a/mod/quiz/report/statistics/classes/tests/statistics_helper.php +++ b/mod/quiz/report/statistics/classes/tests/statistics_helper.php @@ -30,15 +30,22 @@ class statistics_helper { * We need a special function to do this as the tasks are deferred by one hour, * so we need to pass a custom $timestart argument. * + * @param bool $discardoutput Capture and discard output from executed tasks? * @return void */ - public static function run_pending_recalculation_tasks(): void { + public static function run_pending_recalculation_tasks(bool $discardoutput = false): void { while ($task = \core\task\manager::get_next_adhoc_task( time() + HOURSECS + 1, false, '\quiz_statistics\task\recalculate' )) { + if ($discardoutput) { + ob_start(); + } $task->execute(); + if ($discardoutput) { + ob_end_clean(); + } \core\task\manager::adhoc_task_complete($task); } } diff --git a/mod/quiz/report/statistics/classes/tests/statistics_test_trait.php b/mod/quiz/report/statistics/classes/tests/statistics_test_trait.php new file mode 100644 index 00000000000..80021bf0fda --- /dev/null +++ b/mod/quiz/report/statistics/classes/tests/statistics_test_trait.php @@ -0,0 +1,59 @@ +. +namespace quiz_statistics\tests; + +use quiz_statistics\task\recalculate; + +/** + * Test methods for statistics recalculations + * + * @package quiz_statistics + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +trait statistics_test_trait { + /** + * Return a user, and a quiz with 2 questions. + * + * @return array [$user, $quiz, $course] + */ + protected function create_test_data(): array { + $this->resetAfterTest(true); + $generator = $this->getDataGenerator(); + $user = $generator->create_user(); + $course = $generator->create_course(); + $quiz = $this->create_test_quiz($course); + $this->add_two_regular_questions($generator->get_plugin_generator('core_question'), $quiz); + return [$user, $quiz, $course]; + } + + /** + * Assert that a task is queued for a quiz. + * + * Check that the quizid stored in the task's custom data matches the provided quiz, + * and that the run time is in one hour from when the test is being run (within a small margin of error). + * + * @param recalculate $task + * @param \stdClass $quiz + * @return void + */ + protected function assert_task_is_queued_for_quiz(recalculate $task, \stdClass $quiz): void { + $data = $task->get_custom_data(); + $this->assertEquals($quiz->id, $data->quizid); + $this->assertEqualsWithDelta(time() + HOURSECS, $task->get_next_run_time(), 1); + } +} diff --git a/mod/quiz/report/statistics/tests/event/observer/attempt_submitted_test.php b/mod/quiz/report/statistics/tests/event/observer/attempt_submitted_test.php index 3d119f8fea6..796f5f08edf 100644 --- a/mod/quiz/report/statistics/tests/event/observer/attempt_submitted_test.php +++ b/mod/quiz/report/statistics/tests/event/observer/attempt_submitted_test.php @@ -23,6 +23,7 @@ require_once($CFG->dirroot . '/mod/quiz/tests/quiz_question_helper_test_trait.ph use core\task\manager; use quiz_statistics\task\recalculate; use quiz_statistics\tests\statistics_helper; +use quiz_statistics\tests\statistics_test_trait; /** * Unit tests for attempt_submitted observer @@ -35,37 +36,8 @@ use quiz_statistics\tests\statistics_helper; */ class attempt_submitted_test extends \advanced_testcase { use \quiz_question_helper_test_trait; + use statistics_test_trait; - /** - * Return a user, and a quiz with 2 questions. - * - * @return array [$user, $quiz, $course] - */ - protected function create_test_data(): array { - $this->resetAfterTest(true); - $generator = $this->getDataGenerator(); - $user = $generator->create_user(); - $course = $generator->create_course(); - $quiz = $this->create_test_quiz($course); - $this->add_two_regular_questions($generator->get_plugin_generator('core_question'), $quiz); - return [$user, $quiz, $course]; - } - - /** - * Assert that a task is queued for a quiz. - * - * Check that the quizid stored in the task's custom data matches the provided quiz, - * and that the run time is in one hour from when the test is being run (within a small margin of error). - * - * @param recalculate $task - * @param \stdClass $quiz - * @return void - */ - protected function assert_task_is_queued_for_quiz(recalculate $task, \stdClass $quiz): void { - $data = $task->get_custom_data(); - $this->assertEquals($quiz->id, $data->quizid); - $this->assertEqualsWithDelta(time() + HOURSECS, $task->get_next_run_time(), 1); - } /** * Attempting a quiz should queue the recalculation task for that quiz in 1 hour's time. diff --git a/mod/quiz/report/statistics/tests/quiz_attempt_deleted_test.php b/mod/quiz/report/statistics/tests/quiz_attempt_deleted_test.php new file mode 100644 index 00000000000..6b1ca956e3a --- /dev/null +++ b/mod/quiz/report/statistics/tests/quiz_attempt_deleted_test.php @@ -0,0 +1,154 @@ +. +namespace quiz_statistics; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/quiz/tests/quiz_question_helper_test_trait.php'); + +use core\task\manager; +use quiz_statistics\task\recalculate; +use quiz_statistics\tests\statistics_helper; +use quiz_statistics\tests\statistics_test_trait; + +/** + * Unit tests for attempt_deleted observer + * + * @package quiz_statistics + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \quiz_statistics\quiz_attempt_deleted + */ +class quiz_attempt_deleted_test extends \advanced_testcase { + use \quiz_question_helper_test_trait; + use statistics_test_trait; + + /** + * Deleting an attempt should queue the recalculation task for that quiz in 1 hour's time. + * + * @return void + */ + public function test_queue_task_on_deletion(): void { + [$user, $quiz] = $this->create_test_data(); + $this->attempt_quiz($quiz, $user); + [, , $attempt] = $this->attempt_quiz($quiz, $user, 2); + statistics_helper::run_pending_recalculation_tasks(true); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertEmpty($tasks); + + quiz_delete_attempt($attempt->get_attemptid(), $quiz); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertCount(1, $tasks); + $task = reset($tasks); + $this->assert_task_is_queued_for_quiz($task, $quiz); + } + + /** + * Deleting multiple attempts of the same quiz should only queue one instance of the task. + * + * @return void + */ + public function test_queue_single_task_for_multiple_deletions(): void { + [$user1, $quiz] = $this->create_test_data(); + $user2 = $this->getDataGenerator()->create_user(); + $this->attempt_quiz($quiz, $user1); + [, , $attempt1] = $this->attempt_quiz($quiz, $user1, 2); + $this->attempt_quiz($quiz, $user2); + [, , $attempt2] = $this->attempt_quiz($quiz, $user2, 2); + statistics_helper::run_pending_recalculation_tasks(true); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertEmpty($tasks); + + quiz_delete_attempt($attempt1->get_attemptid(), $quiz); + quiz_delete_attempt($attempt2->get_attemptid(), $quiz); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertCount(1, $tasks); + $task = reset($tasks); + $this->assert_task_is_queued_for_quiz($task, $quiz); + } + + /** + * Deleting another attempt after processing the task should queue a new task. + * + * @return void + */ + public function test_queue_new_task_after_processing(): void { + [$user1, $quiz, $course] = $this->create_test_data(); + $user2 = $this->getDataGenerator()->create_user(); + $this->attempt_quiz($quiz, $user1); + [, , $attempt1] = $this->attempt_quiz($quiz, $user1, 2); + $this->attempt_quiz($quiz, $user2); + [, , $attempt2] = $this->attempt_quiz($quiz, $user2, 2); + statistics_helper::run_pending_recalculation_tasks(true); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertEmpty($tasks); + + quiz_delete_attempt($attempt1->get_attemptid(), $quiz); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertCount(1, $tasks); + + $this->expectOutputRegex("~Re-calculating statistics for quiz {$quiz->name} \({$quiz->id}\) " . + "from course {$course->shortname} \({$course->id}\) with 3 attempts~"); + statistics_helper::run_pending_recalculation_tasks(); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertEmpty($tasks); + + quiz_delete_attempt($attempt2->get_attemptid(), $quiz); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertCount(1, $tasks); + + $task = reset($tasks); + $this->assert_task_is_queued_for_quiz($task, $quiz); + } + + /** + * Deleting attempts from different quizzes will queue a task for each. + * + * @return void + */ + public function test_queue_separate_tasks_for_multiple_quizzes(): void { + [$user1, $quiz1] = $this->create_test_data(); + [$user2, $quiz2] = $this->create_test_data(); + $this->attempt_quiz($quiz1, $user1); + [, , $attempt1] = $this->attempt_quiz($quiz1, $user1, 2); + $this->attempt_quiz($quiz2, $user2); + [, , $attempt2] = $this->attempt_quiz($quiz2, $user2, 2); + statistics_helper::run_pending_recalculation_tasks(true); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertEmpty($tasks); + + quiz_delete_attempt($attempt1->get_attemptid(), $quiz1); + quiz_delete_attempt($attempt2->get_attemptid(), $quiz2); + + $tasks = manager::get_adhoc_tasks(recalculate::class); + $this->assertCount(2, $tasks); + $task1 = array_shift($tasks); + $this->assert_task_is_queued_for_quiz($task1, $quiz1); + $task2 = array_shift($tasks); + $this->assert_task_is_queued_for_quiz($task2, $quiz2); + } +}