From c09238dc2437eadb8764d990a86a081494393340 Mon Sep 17 00:00:00 2001 From: David Szkiba Date: Mon, 23 Dec 2024 16:13:58 +0100 Subject: [PATCH] GH-791 Remove checkitemparams preselect-task This removes the checkitemparams.php preselect-task middleware and makes it a normal method of the strategy class. --- .../preselect_task/checkitemparams.php | 94 ------------------- classes/teststrategy/strategy.php | 54 ++++++++++- .../teststrategy/strategy/classicalcat.php | 2 - .../strategy/inferallsubscales.php | 2 - .../strategy/infergreateststrength.php | 2 - .../strategy/inferlowestskillgap.php | 2 - .../teststrategy/strategy/relevantscales.php | 2 - .../strategy/teststrategy_fastest.php | 2 - 8 files changed, 52 insertions(+), 108 deletions(-) delete mode 100644 classes/teststrategy/preselect_task/checkitemparams.php diff --git a/classes/teststrategy/preselect_task/checkitemparams.php b/classes/teststrategy/preselect_task/checkitemparams.php deleted file mode 100644 index 74fb55fa8..000000000 --- a/classes/teststrategy/preselect_task/checkitemparams.php +++ /dev/null @@ -1,94 +0,0 @@ -. - -/** - * Class checkitemparams. - * - * @package local_catquiz - * @copyright 2024 Wunderbyte GmbH - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -namespace local_catquiz\teststrategy\preselect_task; - -use local_catquiz\catscale; -use local_catquiz\local\model\model_item_param_list; -use local_catquiz\local\model\model_strategy; -use local_catquiz\local\result; -use local_catquiz\local\status; -use local_catquiz\teststrategy\preselect_task; -use local_catquiz\wb_middleware; - -/** - * Checks if there are item parameters for the given combination of scale and context. - * - * @package local_catquiz - * @copyright 2024 Wunderbyte GmbH - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -final class checkitemparams extends preselect_task implements wb_middleware { - - /** - * Run. - * - * @param array $context - * @param callable $next - * - * @return result - * - */ - public function run(array &$context, callable $next): result { - $selectedscales = [$context['catscaleid'], ...$context['progress']->get_selected_subscales()]; - foreach ($selectedscales as $catscaleid) { - $catscalecontext = catscale::get_context_id($catscaleid); - $catscaleids = [ - $catscaleid, - ...catscale::get_subscale_ids($catscaleid), - ]; - $itemparamlists = []; - foreach (array_keys(model_strategy::get_installed_models()) as $model) { - $itemparamlists[$model] = model_item_param_list::get( - $catscalecontext, - $model, - $catscaleids - )->count(); - } - if (array_sum($itemparamlists) === 0) { - $context['progress']->drop_scale($catscaleid); - unset($selectedscales[array_search($catscaleid, $selectedscales)]); - } - } - - // If there are no active scales left, show a message that the quiz can not be started. - if (!$selectedscales) { - return result::err(status::ERROR_NO_ITEMS); - } - - return $next($context); - } - - /** - * Get required context keys. - * - * @return array - * - */ - public function get_required_context_keys(): array { - return [ - 'progress', - ]; - } -} diff --git a/classes/teststrategy/strategy.php b/classes/teststrategy/strategy.php index b6b840afa..54af72982 100644 --- a/classes/teststrategy/strategy.php +++ b/classes/teststrategy/strategy.php @@ -28,8 +28,9 @@ use coding_exception; use Exception; use dml_exception; -use local_catquiz\catquiz; use local_catquiz\catscale; +use local_catquiz\local\model\model_item_param_list; +use local_catquiz\local\model\model_strategy; use local_catquiz\local\result; use local_catquiz\local\status; use local_catquiz\output\attemptfeedback; @@ -38,7 +39,6 @@ use local_catquiz\teststrategy\progress; use local_catquiz\wb_middleware_runner; use moodle_exception; -use stdClass; /** * Base class for test strategies. @@ -83,6 +83,18 @@ abstract class strategy { */ protected progress $progress; + /** + * These data provide the context for the selection of the next question. + * + * In a previous implementation, this object was passed between middlewares + * and allowed them to decide what to do. + * It would be good to refactor this in such a way that the different + * elements of this array become class properties of this (strategy) class. + * + * @var array + */ + protected array $context; + /** * Instantioate parameters. */ @@ -125,6 +137,7 @@ public function get_description(): string { * */ public function return_next_testitem(array $context) { + $this->context = $context; $cache = cache::make('local_catquiz', 'adaptivequizattempt'); $maxtime = $context['progress']->get_starttime() + $context['max_attempttime_in_sec']; if (time() > $maxtime) { @@ -137,6 +150,12 @@ public function return_next_testitem(array $context) { return result::err(status::EXCEEDED_MAX_ATTEMPT_TIME); } + try { + $this->check_item_params(); + } catch (Exception $e) { + return result::err($e->getMessage()); + } + foreach ($this->get_preselecttasks() as $modifier) { // When this is called for running tests, check if there is a // X_testing class and if so, use that one. @@ -288,4 +307,35 @@ abstract public function select_scales_for_report( int $catscaleid = 0, bool $feedbackonlyfordefinedscaleid = false ): array; + + /** + * Checks if there are item params for the given combination of scale and context + */ + protected function check_item_params() { + $selectedscales = [$this->context['catscaleid'], ...$this->context['progress']->get_selected_subscales()]; + foreach ($selectedscales as $catscaleid) { + $catscalecontext = catscale::get_context_id($catscaleid); + $catscaleids = [ + $catscaleid, + ...catscale::get_subscale_ids($catscaleid), + ]; + $itemparamlists = []; + foreach (array_keys(model_strategy::get_installed_models()) as $model) { + $itemparamlists[$model] = model_item_param_list::get( + $catscalecontext, + $model, + $catscaleids + )->count(); + } + if (array_sum($itemparamlists) === 0) { + $this->context['progress']->drop_scale($catscaleid); + unset($selectedscales[array_search($catscaleid, $selectedscales)]); + } + } + + // If there are no active scales left, show a message that the quiz can not be started. + if (!$selectedscales) { + throw new Exception(status::ERROR_NO_ITEMS); + } + } } diff --git a/classes/teststrategy/strategy/classicalcat.php b/classes/teststrategy/strategy/classicalcat.php index 2c9b6b60f..88d5fa5f6 100644 --- a/classes/teststrategy/strategy/classicalcat.php +++ b/classes/teststrategy/strategy/classicalcat.php @@ -35,7 +35,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\fisherinformation; use local_catquiz\teststrategy\preselect_task\maximumquestionscheck; @@ -77,7 +76,6 @@ class classicalcat extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, checkpagereload::class, updatepersonability::class, diff --git a/classes/teststrategy/strategy/inferallsubscales.php b/classes/teststrategy/strategy/inferallsubscales.php index 455c16ea4..7db4a75e6 100644 --- a/classes/teststrategy/strategy/inferallsubscales.php +++ b/classes/teststrategy/strategy/inferallsubscales.php @@ -35,7 +35,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\filterbyquestionsperscale; use local_catquiz\teststrategy\preselect_task\filterbystandarderror; @@ -87,7 +86,6 @@ class inferallsubscales extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, updatepersonability::class, firstquestionselector::class, // If this is the first question of this attempt, return it here. diff --git a/classes/teststrategy/strategy/infergreateststrength.php b/classes/teststrategy/strategy/infergreateststrength.php index e8e379b4e..f7f5426ae 100644 --- a/classes/teststrategy/strategy/infergreateststrength.php +++ b/classes/teststrategy/strategy/infergreateststrength.php @@ -36,7 +36,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\filterbystandarderror; use local_catquiz\teststrategy\preselect_task\filterbytestinfo; @@ -92,7 +91,6 @@ class infergreateststrength extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, updatepersonability::class, firstquestionselector::class, // If this is the first question of this attempt, return it here. diff --git a/classes/teststrategy/strategy/inferlowestskillgap.php b/classes/teststrategy/strategy/inferlowestskillgap.php index eabe21b8a..84383aeb4 100644 --- a/classes/teststrategy/strategy/inferlowestskillgap.php +++ b/classes/teststrategy/strategy/inferlowestskillgap.php @@ -35,7 +35,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\filterbystandarderror; use local_catquiz\teststrategy\preselect_task\filterbytestinfo; @@ -86,7 +85,6 @@ class inferlowestskillgap extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, updatepersonability::class, firstquestionselector::class, // If this is the first question of this attempt, return it here. diff --git a/classes/teststrategy/strategy/relevantscales.php b/classes/teststrategy/strategy/relevantscales.php index 7dbc6f14f..11e9aec5d 100644 --- a/classes/teststrategy/strategy/relevantscales.php +++ b/classes/teststrategy/strategy/relevantscales.php @@ -35,7 +35,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\filterbystandarderror; use local_catquiz\teststrategy\preselect_task\filterbytestinfo; @@ -86,7 +85,6 @@ class relevantscales extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, updatepersonability::class, firstquestionselector::class, // If this is the first question of this attempt, return it here. diff --git a/classes/teststrategy/strategy/teststrategy_fastest.php b/classes/teststrategy/strategy/teststrategy_fastest.php index 72b915b6d..c7d69bfab 100644 --- a/classes/teststrategy/strategy/teststrategy_fastest.php +++ b/classes/teststrategy/strategy/teststrategy_fastest.php @@ -35,7 +35,6 @@ use local_catquiz\teststrategy\feedbacksettings; use local_catquiz\teststrategy\preselect_task\addscalestandarderror; use local_catquiz\teststrategy\preselect_task\checkbreak; -use local_catquiz\teststrategy\preselect_task\checkitemparams; use local_catquiz\teststrategy\preselect_task\checkpagereload; use local_catquiz\teststrategy\preselect_task\filterbystandarderror; use local_catquiz\teststrategy\preselect_task\firstquestionselector; @@ -81,7 +80,6 @@ class teststrategy_fastest extends strategy { */ public function get_preselecttasks(): array { return [ - checkitemparams::class, checkbreak::class, checkpagereload::class, firstquestionselector::class,