L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-75576 question bank: bulk-load statistics for all questions at once

Browse Source 
There is a new method on the column class, for the column to declare any
statistics it requires. The question then efficiently loads all those
statistics in one go, and makes them available as the columns are
rendererd, or do any subsequent processing.
This commit is contained in:
Tim Hunt
2023-05-15 10:03:26 +01:00
parent d1fcfd1de2
commit bc7dacecb6
9 changed files with 841 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/classes/local/bank/column_base.php
+15 -1
View File
@@ -325,6 +325,20 @@ abstract class column_base {
return [];
}
/**
* If this column requires any aggregated statistics, it should declare that here.
*
* This is those statistics can be efficiently loaded in bulk.
*
* The statistics are all loaded just before load_additional_data is called on each column.
* The values are then available from $this->qbank->get_aggregate_statistic(...);
*
* @return string[] the names of the required statistics fields. E.g. ['facility'].
*/
public function get_required_statistics_fields(): array {
return [];
}
/**
* If this column needs extra data (e.g. tags) then load that here.
*
@@ -332,7 +346,7 @@ abstract class column_base {
* Probably a good idea to check that another column has not already
* loaded the data you want.
*
* @param \stdClass[] $questions the questions that will be displayed.
* @param \stdClass[] $questions the questions that will be displayed, indexed by question id.
*/
public function load_additional_data(array $questions) {
}
question/classes/local/bank/view.php
+77 -8
View File
@@ -24,14 +24,15 @@
namespace core_question\local\bank;
use core_plugin_manager;
use core_question\bank\search\condition;
use core_question\local\statistics\statistics_bulk_loader;
use qbank_columnsortorder\column_manager;
use qbank_editquestion\editquestion_helper;
defined('MOODLE_INTERNAL') || die();
require_once($CFG->dirroot . '/question/editlib.php');
use core_plugin_manager;
use core_question\bank\search\condition;
use qbank_columnsortorder\column_manager;
use qbank_editquestion\editquestion_helper;
use qbank_managecategories\helper;
/**
* This class prints a view of the question bank.
@@ -90,19 +91,19 @@ class view {
public $course;
/**
* @var \question_bank_column_base[] these are all the 'columns' that are
* @var column_base[] these are all the 'columns' that are
* part of the display. Array keys are the class name.
*/
protected $requiredcolumns;
/**
* @var \question_bank_column_base[] these are the 'columns' that are
* @var column_base[] these are the 'columns' that are
* actually displayed as a column, in order. Array keys are the class name.
*/
protected $visiblecolumns;
/**
* @var \question_bank_column_base[] these are the 'columns' that are
* @var column_base[] these are the 'columns' that are
* actually displayed as an additional row (e.g. question text), in order.
* Array keys are the class name.
*/
@@ -139,6 +140,15 @@ class view {
*/
protected $sqlparams;
/**
* @var ?array Stores all the average statistics that this question bank view needs.
*
* This field gets initialised in {@see display_question_list()}. It is a two dimensional
* $this->loadedstatistics[$questionid][$fieldname] = $average value of that statistics for that question.
* Column classes in qbank plugins can access these values using {@see get_aggregate_statistic()}.
*/
protected $loadedstatistics = null;
/**
* @var condition[] search conditions.
*/
@@ -979,6 +989,11 @@ class view {
}
}
$questionsrs->close();
// Bulk load any required statistics.
$this->load_required_statistics($questions);
// Bulk load any extra data that any column requires.
foreach ($this->requiredcolumns as $name => $column) {
$column->load_additional_data($questions);
}
@@ -1005,6 +1020,60 @@ class view {
echo \html_writer::end_tag('form');
}
/**
* Work out the list of all the required statistics fields for this question bank view.
*
* This gathers all the required fields from all columns, so they can all be loaded at once.
*
* @return string[] the names of all the required fields for this question bank view.
*/
protected function determine_required_statistics(): array {
$requiredfields = [];
foreach ($this->requiredcolumns as $column) {
$requiredfields = array_merge($requiredfields, $column->get_required_statistics_fields());
}
return array_unique($requiredfields);
}
/**
* Load the aggregate statistics that all the columns require.
*
* @param \stdClass[] $questions the questions that will be displayed indexed by question id.
*/
protected function load_required_statistics(array $questions): void {
$requiredstatistics = $this->determine_required_statistics();
$this->loadedstatistics = statistics_bulk_loader::load_aggregate_statistics(
array_keys($questions), $requiredstatistics);
}
/**
* Get the aggregated value of a particular statistic for a particular question.
*
* You can only get values for the questions on the current page of the question bank view,
* and only if you declared the need for this statistic in the get_required_statistics_fields()
* method of your question bank column.
*
* @param int $questionid the id of a question
* @param string $fieldname the name of a statistics field, e.g. 'facility'.
* @return float|null the average (across all users) of this statistic for this question.
* Null if the value is not available right now.
*/
public function get_aggregate_statistic(int $questionid, string $fieldname): ?float {
if (!array_key_exists($questionid, $this->loadedstatistics)) {
throw new \coding_exception('Question ' . $questionid . ' is not on the current page of ' .
'this question bank view, so its statistics are not available.');
}
// Must be array_key_exists, not isset, because we care about null values.
if (!array_key_exists($fieldname, $this->loadedstatistics[$questionid])) {
throw new \coding_exception('Statistics field ' . $fieldname . ' was not requested by any ' .
'question bank column in this view, so it is not available.');
}
return $this->loadedstatistics[$questionid][$fieldname];
}
/**
* Display the top pagination bar.
*
question/classes/local/statistics/statistics_bulk_loader.php
+170
View File
@@ -0,0 +1,170 @@
<?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 core_question\local\statistics;
use core_question\local\bank\column_base;
use core_question\statistics\questions\all_calculated_for_qubaid_condition;
use core_component;
/**
* Helper to efficiently load all the statistics for a set of questions.
*
* If you are implementing a question bank column, do not use this method directly.
* Instead, override the {@see column_base::get_required_statistics_fields()} method
* in your column class, and the question bank view will take care of it for you.
*
* @package core_question
* @copyright 2023 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class statistics_bulk_loader {
/**
* Load and aggregate the requested statistics for all the places where the given questions are used.
*
* The returned array will contain a values for each questionid and field, which will be null if the value is not available.
*
* @param int[] $questionids array of question ids.
* @param string[] $requiredstatistics array of the fields required, e.g. ['facility', 'discriminationindex'].
* @return float[][] if a value is not available, it will be set to null.
*/
public static function load_aggregate_statistics(array $questionids, array $requiredstatistics): array {
$places = self::get_all_places_where_questions_were_attempted($questionids);
// Set up blank two-dimensional arrays to store the running totals. Indexed by questionid and field name.
$zerovaluesforonequestion = array_combine($requiredstatistics, array_fill(0, count($requiredstatistics), 0));
$counts = array_combine($questionids, array_fill(0, count($questionids), $zerovaluesforonequestion));
$sums = array_combine($questionids, array_fill(0, count($questionids), $zerovaluesforonequestion));
// Load the data for each place, and add to the running totals.
foreach ($places as $place) {
$statistics = self::load_statistics_for_place($place->component,
\context::instance_by_id($place->contextid));
if ($statistics === null) {
continue;
}
foreach ($questionids as $questionid) {
foreach ($requiredstatistics as $item) {
$value = self::extract_item_value($statistics, $questionid, $item);
if ($value === null) {
continue;
}
$counts[$questionid][$item] += 1;
$sums[$questionid][$item] += $value;
}
}
}
// Compute the averages from the final totals.
$aggregates = [];
foreach ($questionids as $questionid) {
$aggregates[$questionid] = [];
foreach ($requiredstatistics as $item) {
if ($counts[$questionid][$item] > 0) {
$aggregates[$questionid][$item] = $sums[$questionid][$item] / $counts[$questionid][$item];
} else {
$aggregates[$questionid][$item] = null;
}
}
}
return $aggregates;
}
/**
* For a list of questions find all the places, defined by (component, contextid), where there are attempts.
*
* @param int[] $questionids array of question ids that we are interested in.
* @return \stdClass[] list of objects with fields ->component and ->contextid.
*/
protected static function get_all_places_where_questions_were_attempted(array $questionids): array {
global $DB;
[$questionidcondition, $params] = $DB->get_in_or_equal($questionids);
// The MIN(qu.id) is just to ensure that the rows have a unique key.
$places = $DB->get_records_sql("
SELECT MIN(qu.id) AS somethingunique, qu.component, qu.contextid
FROM {question_usages} qu
JOIN {question_attempts} qatt ON qatt.questionusageid = qu.id
WHERE qatt.questionid $questionidcondition
GROUP BY qu.component, qu.contextid
ORDER BY qu.contextid ASC
", $params);
// Strip out the unwanted ids.
$places = array_values($places);
foreach ($places as $place) {
unset($place->somethingunique);
}
return $places;
}
/**
* Load the question statistics for all the attempts belonging to a particular component in a particular context.
*
* @param string $component frankenstyle component name, e.g. 'mod_quiz'.
* @param \context $context the context to load the statistics for.
* @return all_calculated_for_qubaid_condition|null question statistics.
*/
protected static function load_statistics_for_place(
string $component,
\context $context
): ?all_calculated_for_qubaid_condition {
// This check is basically if (component_exists).
if (empty(core_component::get_component_directory($component))) {
return null;
}
if (!component_callback_exists($component, 'calculate_question_stats')) {
return null;
}
return component_callback($component, 'calculate_question_stats', [$context]);
}
/**
* Extract the value for one question and one type of statistic from a set of statistics.
*
* @param all_calculated_for_qubaid_condition $statistics the batch of statistics.
* @param int $questionid a question id.
* @param string $item one of the field names in all_calculated_for_qubaid_condition, e.g. 'facility'.
* @return float|null the required value.
*/
protected static function extract_item_value(all_calculated_for_qubaid_condition $statistics,
int $questionid, string $item): ?float {
// Look in main questions.
foreach ($statistics->questionstats as $stats) {
if ($stats->questionid == $questionid && isset($stats->$item)) {
return $stats->$item;
}
}
// If not found, look in sub questions.
foreach ($statistics->subquestionstats as $stats) {
if ($stats->questionid == $questionid && isset($stats->$item)) {
return $stats->$item;
}
}
return null;
}
}