Merge branch 'MDL-75576_401' of https://github.com/timhunt/moodle int…

…o MOODLE_401_STABLE

lib/db/tasks.php

@@ -221,15 +221,6 @@
         'dayofweek' => '*',
         'month' => '*'
     ),
-    array(
-        'classname' => 'core\task\question_stats_cleanup_task',
-        'blocking' => 0,
-        'minute' => '*',
-        'hour' => '*',
-        'day' => '*',
-        'dayofweek' => '*',
-        'month' => '*'
-    ),
     array(
         'classname' => 'core\task\registration_cron_task',
         'blocking' => 0,

mod/quiz/lib.php

@@ -2483,12 +2483,12 @@ function quiz_delete_references($quizid): void {
  * This enables quiz statistics to be shown in statistics columns in the database.
  *
  * @param context $context return the statistics related to this context (which will be a quiz context).
- * @return all_calculated_for_qubaid_condition|null The statistics for this quiz, if any, else null.
+ * @return all_calculated_for_qubaid_condition|null The statistics for this quiz, if available, else null.
  */
 function mod_quiz_calculate_question_stats(context $context): ?all_calculated_for_qubaid_condition {
     global $CFG;
     require_once($CFG->dirroot . '/mod/quiz/report/statistics/report.php');
     $cm = get_coursemodule_from_id('quiz', $context->instanceid);
     $report = new quiz_statistics_report();
-    return $report->calculate_questions_stats_for_question_bank($cm->instance);
+    return $report->calculate_questions_stats_for_question_bank($cm->instance, false);
 }

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

@@ -225,9 +225,13 @@ public function cache($qubaids) {
             $toinsert->standarderror = null;
         }
 
+        // Delete older statistics before we save the new ones.
+        $transaction = $DB->start_delegated_transaction();
+        $DB->delete_records('quiz_statistics', ['hashcode' => $qubaids->get_hash_code()]);
+
         // Store the data.
         $DB->insert_record('quiz_statistics', $toinsert);
-
+        $transaction->allow_commit();
     }
 
     /**

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

@@ -121,21 +121,24 @@ public function calculate($quizid, $whichattempts, \core\dml\sql_join $groupstud
         return $quizstats;
     }
 
-    /** @var integer Time after which statistics are automatically recomputed. */
+    /** @var int No longer used. Previously the time after which statistics are automatically recomputed. */
     const TIME_TO_CACHE = 900; // 15 minutes.
 
     /**
      * Load cached statistics from the database.
      *
-     * @param $qubaids \qubaid_condition
-     * @return calculated The statistics for overall attempt scores or false if not cached.
+     * @param \qubaid_condition $qubaids
+     * @return calculated|false The statistics for overall attempt scores or false if not cached.
      */
     public function get_cached($qubaids) {
         global $DB;
 
-        $timemodified = time() - self::TIME_TO_CACHE;
-        $fromdb = $DB->get_record_select('quiz_statistics', 'hashcode = ? AND timemodified > ?',
-                                         array($qubaids->get_hash_code(), $timemodified));
+        $lastcalculatedtime = $this->get_last_calculated_time($qubaids);
+        if (!$lastcalculatedtime) {
+            return false;
+        }
+        $fromdb = $DB->get_record('quiz_statistics', ['hashcode' => $qubaids->get_hash_code(),
+                'timemodified' => $lastcalculatedtime]);
         $stats = new calculated();
         $stats->populate_from_record($fromdb);
         return $stats;
@@ -145,14 +148,17 @@ public function get_cached($qubaids) {
      * Find time of non-expired statistics in the database.
      *
      * @param $qubaids \qubaid_condition
-     * @return integer|boolean Time of cached record that matches this qubaid_condition or false is non found.
+     * @return int|bool Time of cached record that matches this qubaid_condition or false is non found.
      */
     public function get_last_calculated_time($qubaids) {
         global $DB;
-
-        $timemodified = time() - self::TIME_TO_CACHE;
-        return $DB->get_field_select('quiz_statistics', 'timemodified', 'hashcode = ? AND timemodified > ?',
-                                         array($qubaids->get_hash_code(), $timemodified));
+        $lastcalculatedtime = $DB->get_field('quiz_statistics', 'COALESCE(MAX(timemodified), 0)',
+                ['hashcode' => $qubaids->get_hash_code()]);
+        if ($lastcalculatedtime) {
+            return $lastcalculatedtime;
+        } else {
+            return false;
+        }
     }
 
     /**

mod/quiz/report/statistics/report.php

@@ -25,6 +25,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
+use core_question\statistics\responses\analyser;
 use core_question\statistics\questions\all_calculated_for_qubaid_condition;
 
 require_once($CFG->dirroot . '/mod/quiz/report/default.php');
@@ -420,7 +421,7 @@ protected function output_individual_question_response_analysis($question, $vari
             }
         }
 
-        $responesanalyser = new \core_question\statistics\responses\analyser($question, $whichtries);
+        $responesanalyser = new analyser($question, $whichtries);
         $responseanalysis = $responesanalyser->load_cached($qubaids, $whichtries);
 
         $qtable->question_setup($reporturl, $question, $s, $responseanalysis);
@@ -627,11 +628,15 @@ protected function output_statistics_graph($quizorid, $qubaids) {
      * @param \core\dml\sql_join $groupstudentsjoins Contains joins, wheres, params for students in this group.
      * @param array  $questions          full question data.
      * @param \core\progress\base|null   $progress
+     * @param bool $calculateifrequired  if true (the default) the stats will be calculated if not already stored.
+     *                                   If false, [null, null] will be returned if the stats are not already available.
      * @return array with 2 elements:    - $quizstats The statistics for overall attempt scores.
      *                                   - $questionstats \core_question\statistics\questions\all_calculated_for_qubaid_condition
+     *                                   Both may be null, if $calculateifrequired is false.
      */
     public function get_all_stats_and_analysis(
-            $quiz, $whichattempts, $whichtries, \core\dml\sql_join $groupstudentsjoins, $questions, $progress = null) {
+            $quiz, $whichattempts, $whichtries, \core\dml\sql_join $groupstudentsjoins,
+            $questions, $progress = null, bool $calculateifrequired = true) {
 
         if ($progress === null) {
             $progress = new \core\progress\none();
@@ -645,6 +650,11 @@ public function get_all_stats_and_analysis(
 
         $progress->start_progress('', 3);
         if ($quizcalc->get_last_calculated_time($qubaids) === false) {
+            if (!$calculateifrequired) {
+                $progress->progress(3);
+                $progress->end_progress();
+                return [null, null];
+            }
 
             // Recalculate now.
             $questionstats = $qcalc->calculate($qubaids);
@@ -739,10 +749,8 @@ protected function analyse_responses_for_questions($questions, $qubaids, $whicht
         foreach ($questions as $question) {
             $progress->increment_progress();
             if (question_bank::get_qtype($question->qtype, false)->can_analyse_responses()  && !isset($done[$question->id])) {
-                $responesstats = new \core_question\statistics\responses\analyser($question, $whichtries);
-                if ($responesstats->get_last_analysed_time($qubaids, $whichtries) === false) {
-                    $responesstats->calculate($qubaids, $whichtries);
-                }
+                $responesstats = new analyser($question, $whichtries);
+                $responesstats->calculate($qubaids, $whichtries);
             }
             $done[$question->id] = 1;
         }
@@ -927,15 +935,21 @@ protected function output_all_question_response_analysis($qubaids,
      * Load question stats for a quiz
      *
      * @param int $quizid question usage
-     * @return all_calculated_for_qubaid_condition question stats
+     * @param bool $calculateifrequired if true (the default) the stats will be calculated if not already stored.
+     *     If false, null will be returned if the stats are not already available.
+     * @return ?all_calculated_for_qubaid_condition question stats
      */
-    public function calculate_questions_stats_for_question_bank(int $quizid): all_calculated_for_qubaid_condition {
+    public function calculate_questions_stats_for_question_bank(
+            int $quizid,
+            bool $calculateifrequired = true
+        ): ?all_calculated_for_qubaid_condition {
         global $DB;
         $quiz = $DB->get_record('quiz', ['id' => $quizid], '*', MUST_EXIST);
         $questions = $this->load_and_initialise_questions_for_calculations($quiz);
 
         [, $questionstats] = $this->get_all_stats_and_analysis($quiz,
-            $quiz->grademethod, question_attempt::ALL_TRIES, new \core\dml\sql_join(), $questions);
+            $quiz->grademethod, question_attempt::ALL_TRIES, new \core\dml\sql_join(),
+            $questions, null, $calculateifrequired);
 
         return $questionstats;
     }

mod/quiz/report/statistics/upgrade.txt

@@ -1,6 +1,18 @@
 This files describes API changes in /mod/quiz/report/statistics/*,
 information provided here is intended especially for developers.
 
+=== 4.1.4 ===
+
+* The methods quiz_statistics_report::calculate_questions_stats_for_question_bank and get_all_stats_and_analysis
+  (which are really private to the quiz, and not part of any API you should be using) now have a new
+  optional argument $calculateifrequired.
+
+* In the past, the methods \quiz_statistics\calculator::get_last_calculated_time() and calculator::get_cached()
+  only returned the pre-computed statistics if they were computed less than 15 minutes ago. Now, they will
+  always return any computed statistics that exist. The constant calculator::TIME_TO_CACHE will be
+  deprecated in Moodle 4.2.
+
+
 === 3.2 ===
 
 * The function quiz_statistics_graph_get_new_colour() is deprecated in favour of the

question/bank/statistics/classes/columns/discrimination_index.php

@@ -17,7 +17,7 @@
 namespace qbank_statistics\columns;
 
 use core_question\local\bank\column_base;
-use qbank_statistics\helper;
+
 /**
  * This columns shows a message about whether this question is OK or needs revision.
  *
@@ -30,11 +30,6 @@
  */
 class discrimination_index extends column_base {
 
-    /**
-     * Title for this column.
-     *
-     * @return string column title
-     */
     public function get_title(): string {
         return get_string('discrimination_index', 'qbank_statistics');
     }
@@ -43,24 +38,18 @@ public function help_icon(): ?\help_icon {
         return new \help_icon('discrimination_index', 'qbank_statistics');
     }
 
-    /**
-     * Column name.
-     *
-     * @return string column name
-     */
     public function get_name(): string {
         return 'discrimination_index';
     }
 
-    /**
-     * Output the contents of this column.
-     * @param object $question the row from the $question table, augmented with extra information.
-     * @param string $rowclasses CSS class names that should be applied to this row of output.
-     */
+    public function get_required_statistics_fields(): array {
+        return ['discriminationindex'];
+    }
+
     protected function display_content($question, $rowclasses) {
         global $PAGE;
-        // Average discrimination index per quiz.
-        $discriminationindex = helper::calculate_average_question_discrimination_index($question->id);
+
+        $discriminationindex = $this->qbank->get_aggregate_statistic($question->id, 'discriminationindex');
         echo $PAGE->get_renderer('qbank_statistics')->render_discrimination_index($discriminationindex);
     }
 

question/bank/statistics/classes/columns/discriminative_efficiency.php

@@ -17,7 +17,7 @@
 namespace qbank_statistics\columns;
 
 use core_question\local\bank\column_base;
-use qbank_statistics\helper;
+
 /**
  * This column show the average discriminative efficiency for this question.
  *
@@ -28,11 +28,6 @@
  */
 class discriminative_efficiency extends column_base {
 
-    /**
-     * Title for this column.
-     *
-     * @return string column title
-     */
     public function get_title(): string {
         return get_string('discriminative_efficiency', 'qbank_statistics');
     }
@@ -41,24 +36,18 @@ public function help_icon(): ?\help_icon {
         return new \help_icon('discriminative_efficiency', 'qbank_statistics');
     }
 
-    /**
-     * Column name.
-     *
-     * @return string column name
-     */
     public function get_name(): string {
         return 'discriminative_efficiency';
     }
 
-    /**
-     * Output the contents of this column.
-     * @param object $question the row from the $question table, augmented with extra information.
-     * @param string $rowclasses CSS class names that should be applied to this row of output.
-     */
+    public function get_required_statistics_fields(): array {
+        return ['discriminativeefficiency'];
+    }
+
     protected function display_content($question, $rowclasses) {
         global $PAGE;
-        // Average discriminative efficiency per quiz.
-        $discriminativeefficiency = helper::calculate_average_question_discriminative_efficiency($question->id);
+
+        $discriminativeefficiency = $this->qbank->get_aggregate_statistic($question->id, 'discriminativeefficiency');
         echo $PAGE->get_renderer('qbank_statistics')->render_discriminative_efficiency($discriminativeefficiency);
     }