Skip to content

Commit

Permalink
MDL-75576 quiz statistics: don't compute when a user views the qbank
Browse files Browse the repository at this point in the history
Previously, when a user viewed the question bank, if the quiz statistics
had not already been calculated, we would try to compute them there an then.
This could be very, very slow, leading to session lock problems.

Now, we never try to compute the statistics on the fly. Instead, we rely
on the existing \quiz_statistics\task\recalculate scheduled task to do it.
  • Loading branch information
timhunt committed May 15, 2023
1 parent 056db33 commit 174e2ae
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 6 deletions.
4 changes: 2 additions & 2 deletions mod/quiz/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -2511,12 +2511,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);
}
23 changes: 19 additions & 4 deletions mod/quiz/report/statistics/report.php
Original file line number Diff line number Diff line change
Expand Up @@ -621,11 +621,15 @@ protected function output_statistics_graph($quiz, $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();
Expand All @@ -639,6 +643,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);
Expand Down Expand Up @@ -922,15 +931,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;
}
Expand Down
7 changes: 7 additions & 0 deletions mod/quiz/report/statistics/upgrade.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
This files describes API changes in /mod/quiz/report/statistics/*,
information provided here is intended especially for developers.

=== 4.3 ===

* 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.


=== 3.2 ===

* The function quiz_statistics_graph_get_new_colour() is deprecated in favour of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "50.00%" in the "TF1" "table_row"
And I should see "75.00%" in the "TF2" "table_row"
Expand All @@ -87,6 +88,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "50.00%" in the "TF1" "table_row"
And I should see "75.00%" in the "TF2" "table_row"
Expand All @@ -102,6 +104,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | True |
And I run the scheduled task "\quiz_statistics\task\recalculate"
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "Likely" in the "TF1" "table_row"
And I should see "Unlikely" in the "TF2" "table_row"
Expand All @@ -123,6 +126,7 @@ Feature: Show statistics in question bank
| slot | response |
| 1 | True |
| 2 | False |
And I run the scheduled task "\quiz_statistics\task\recalculate"
When I am on the "Course 1" "core_question > course question bank" page logged in as "admin"
Then I should see "Likely" in the "TF1" "table_row"
And I should see "Very likely" in the "TF2" "table_row"
Expand Down
6 changes: 6 additions & 0 deletions question/bank/statistics/tests/helper_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ private function prepare_and_submit_quizzes(array $quiz1attempts, array $quiz2at
foreach ($quiz2attempts as $attempt) {
$this->submit_quiz($quiz2, $attempt);
}

// Calculate the statistics.
$this->expectOutputRegex('~.*Calculations completed.*~');
$statisticstask = new \quiz_statistics\task\recalculate();
$statisticstask->execute();

return [$quiz1, $quiz2, $questions];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,12 @@ private function prepare_and_submit_quizzes(array $quiz1attempts, array $quiz2at
foreach ($quiz2attempts as $attempt) {
$this->submit_quiz($quiz2, $attempt);
}

// Calculate the statistics.
$this->expectOutputRegex('~.*Calculations completed.*~');
$statisticstask = new \quiz_statistics\task\recalculate();
$statisticstask->execute();

return [$quiz1, $quiz2, $questions];
}

Expand Down

0 comments on commit 174e2ae

Please sign in to comment.