Skip to content

Commit

Permalink
GH-791 Remove unused context key check
Browse files Browse the repository at this point in the history
Since we are calling the preselect tasks directly via run now and do not
call the `process()` method in the abstract preselect_task class, the
`get_required_context_keys()` method is not called anymore.

Generally, this was not the best approach because its easy to use a
context key that is not listed in the `get_required_context_keys()`
method, so the actual code and the things defined there might diverge.

A better approach would be to use a class instead of an array for the
$context variable (e.g. preselect_context) and make sure it contains all
required fields as properties.
  • Loading branch information
davidszkiba committed Jan 24, 2025
1 parent cce80dd commit 682bbf3
Show file tree
Hide file tree
Showing 27 changed files with 1 addition and 366 deletions.
39 changes: 0 additions & 39 deletions classes/teststrategy/preselect_task.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
namespace local_catquiz\teststrategy;

use local_catquiz\local\result;
use local_catquiz\local\status;

/**
* Base class for a pre-select task.
Expand All @@ -43,42 +42,12 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class preselect_task {

/**
*
* @var array|null $context
*/
protected ?array $context;

/**
* The next task
*
* @var callable
*/
protected $nexttask;

/**
* Process test strategy
*
* @param array $context
* @param callable $nexttask
*
* @return result
*
*/
public function process(array &$context, callable $nexttask): result {
foreach ($this->get_required_context_keys() as $key) {
if (!array_key_exists($key, $context)) {
return result::err(status::ERROR_FETCH_NEXT_QUESTION);
}
}

$context['lastmiddleware'] = str_replace(__NAMESPACE__ . '\\preselect_task\\', '', get_class($this));
$this->context = $context;
$this->nexttask = $nexttask;
return $this->run($context, $nexttask);
}

/**
* This is the function in which the $context can be modified.
*
Expand All @@ -90,12 +59,4 @@ public function process(array &$context, callable $nexttask): result {
* @return result
*/
abstract public function run(array &$context): result;

/**
* If a middleware requires a specific key to be available in the $context
* input array, it can be specified here.
*
* @return array
*/
abstract public function get_required_context_keys(): array;
}
16 changes: 0 additions & 16 deletions classes/teststrategy/preselect_task/addscalestandarderror.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'contextid',
'questions',
'initial_standarderror',
'person_ability',
'progress',
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
final class addscalestandarderror_testing extends addscalestandarderror {

/**
* Returns array of required context keys.
*
* @return array
*/
public function get_required_context_keys(): array {
return [
...parent::get_required_context_keys(),
'fake_ancestor_scales',
'fake_child_scales',
];
}

/**
* Returns array with ancestor scales.
*
Expand Down
9 changes: 0 additions & 9 deletions classes/teststrategy/preselect_task/checkpagereload.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,4 @@ public function run(array &$context): result {

return result::ok($this->progress->get_last_question());
}

/**
* Returns the context key.
*
* @return array
*/
public function get_required_context_keys(): array {
return ['progress'];
}
}
14 changes: 0 additions & 14 deletions classes/teststrategy/preselect_task/filterbyquestionsperscale.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,4 @@ public function run(array &$context): result {
}
return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'progress',
'questionsperscale',
'teststrategy',
];
}
}
16 changes: 0 additions & 16 deletions classes/teststrategy/preselect_task/filterbystandarderror.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,22 +141,6 @@ public function run(array &$context): result {
return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
'progress',
'se_max',
'progress',
'pp_min_inc',
];
}

/**
* Returns the list of scales that should inherit from the given scale.
*
Expand Down
16 changes: 0 additions & 16 deletions classes/teststrategy/preselect_task/filterbytestinfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
'progress',
'se_max',
'progress',
'pp_min_inc',
];
}
}
15 changes: 0 additions & 15 deletions classes/teststrategy/preselect_task/firstquestionselector.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,21 +142,6 @@ public function run(array &$context): result {
return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'selectfirstquestion',
'questions_ordered_by',
'testid',
'progress',
];
}

/**
* Returns the median ability of the given person parameters.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,4 @@ final class firstquestionselector_testing extends firstquestionselector {
protected function get_personparams_for_adaptivequiz_test(array $context) {
return $context['fake_personparams_for_test'];
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return parent::get_required_context_keys()[] = ['fake_personparams_for_test'];
}
}
15 changes: 0 additions & 15 deletions classes/teststrategy/preselect_task/fisherinformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,4 @@ public function get_fisherinformation(\stdClass $question, float $ability): ?flo
);
return $fisherinformation;
}

/**
* Get required context keys
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'installed_models',
'person_ability',
'questions',
'original_questions',
];
}
}
13 changes: 0 additions & 13 deletions classes/teststrategy/preselect_task/lasttimeplayedpenalty.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,6 @@ public function run(array &$context): result {
return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
'penalty_threshold',
];
}

/**
* Calculates the penalty factor for the given question according to the time it was last played.
*
Expand Down
14 changes: 0 additions & 14 deletions classes/teststrategy/preselect_task/maximumquestionscheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questionsattempted',
'maximumquestions',
'progress',
];
}
}
13 changes: 0 additions & 13 deletions classes/teststrategy/preselect_task/maybe_return_pilot.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,4 @@ protected function should_return_pilot(): bool {
$shouldreturnpilot = $rand <= $this->context['pilot_ratio'];
return $shouldreturnpilot;
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'pilot_ratio',
'questions',
];
}
}
14 changes: 0 additions & 14 deletions classes/teststrategy/preselect_task/mayberemovescale.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
'max_attempts_per_scale',
'progress',
];
}
}
12 changes: 0 additions & 12 deletions classes/teststrategy/preselect_task/noremainingquestions.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
];
}
}
12 changes: 0 additions & 12 deletions classes/teststrategy/preselect_task/numberofgeneralattempts.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,6 @@ public function run(array &$context): result {
return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
];
}

/**
* Can be overwritten in the _testing class to prevent access to the DB.
* @param mixed $context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
final class numberofgeneralattempts_testing extends numberofgeneralattempts {
/**
* Returns array of required context keys.
*
* @return array
*/
public function get_required_context_keys(): array {
return parent::get_required_context_keys()[] = ['fake_questionattemptcounts'];
}
/**
* Returns array of questions with attempt scount.
*
Expand Down
12 changes: 0 additions & 12 deletions classes/teststrategy/preselect_task/playedincurrentattempt.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,4 @@ public function run(array &$context): result {

return result::ok($context);
}

/**
* Get required context keys.
*
* @return array
*
*/
public function get_required_context_keys(): array {
return [
'questions',
];
}
}
Loading

0 comments on commit 682bbf3

Please sign in to comment.