Merge branch 'MDL-79806_402_STABLE' of https://github.com/marxjohnson…

…/moodle into MOODLE_402_STABLE

mod/quiz/locallib.php

@@ -446,6 +446,11 @@ function quiz_delete_attempt($attempt, $quiz) {
         $event = \mod_quiz\event\attempt_deleted::create($params);
         $event->add_record_snapshot('quiz_attempts', $attempt);
         $event->trigger();
+
+        $callbackclasses = \core_component::get_plugin_list_with_class('quiz', 'quiz_attempt_deleted');
+        foreach ($callbackclasses as $callbackclass) {
+            component_class_callback($callbackclass, 'callback', [$quiz->id]);
+        }
     }
 
     // Search quiz_attempts for other instances by this user.

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']);
     }
 }

mod/quiz/report/statistics/classes/quiz_attempt_deleted.php

@@ -0,0 +1,39 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+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 <mark.johnson@catalyst-eu.net>
+ * @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);
+    }
+}

mod/quiz/report/statistics/classes/task/recalculate.php

@@ -36,6 +36,11 @@
  * @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 @@ public function execute(): void {
         $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);
+    }
 }

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);
         }
     }

mod/quiz/report/statistics/classes/tests/statistics_test_trait.php

@@ -0,0 +1,59 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+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 <mark.johnson@catalyst-eu.net>
+ * @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);
+    }
+}

mod/quiz/report/statistics/tests/event/observer/attempt_submitted_test.php

@@ -23,6 +23,7 @@
 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 @@
  */
 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.

mod/quiz/report/statistics/tests/quiz_attempt_deleted_test.php

@@ -0,0 +1,154 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+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 <mark.johnson@catalyst-eu.net>
+ * @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);
+    }
+}