Skip to content

Commit cb1a62b

Browse files
matheuszychthojou
matheuszych
and
thojou
committed
TestQuestionPool: Fix Formula Question Scoring Tab Exception
See: https://mantis.ilias.de/view.php?id=47888 The scoring tab reads `sign`, `value`, and `unit` from `assFormulaQuestionResult::getResultInfo()`. With advanced rating those keys were only set when the matching check passed, so an answer outside the tolerance crashed the tab. Simple rating already returned `points`, which is the only key that branch reads. `getResultInfo()` now always returns the same array shape. Unit checks use `instanceof`. If the participant selected no unit, the stored id is `-1` or `null`; the unit lookup falls back to `null` instead of indexing `$units` with a missing key. Co-authored-by: Thomas Joußen <tjoussen@databay.de>
1 parent 45ea937 commit cb1a62b

2 files changed

Lines changed: 75 additions & 48 deletions

File tree

‎components/ILIAS/TestQuestionPool/classes/class.assFormulaQuestion.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -515,9 +515,12 @@ public function substituteVariables(array $userdata, bool $graphicalOutput = fal
515515
$resulttext .= $found['points'] . " " . (($found['points'] == 1) ? $this->lng->txt('point') : $this->lng->txt('points'));
516516
}
517517
} else {
518-
$resulttext .= $this->lng->txt("rated_sign") . " " . (($found['sign']) ? $found['sign'] : 0) . " " . (($found['sign'] == 1) ? $this->lng->txt('point') : $this->lng->txt('points')) . ", ";
519-
$resulttext .= $this->lng->txt("rated_value") . " " . (($found['value']) ? $found['value'] : 0) . " " . (($found['value'] == 1) ? $this->lng->txt('point') : $this->lng->txt('points')) . ", ";
520-
$resulttext .= $this->lng->txt("rated_unit") . " " . (($found['unit']) ? $found['unit'] : 0) . " " . (($found['unit'] == 1) ? $this->lng->txt('point') : $this->lng->txt('points'));
518+
$result_texts = [
519+
"{$this->lng->txt('rated_sign')} {$found['sign']} {$this->lng->txt($found['sign'] == 1 ? 'point' : 'points')}",
520+
"{$this->lng->txt('rated_value')} {$found['value']} {$this->lng->txt($found['value'] == 1 ? 'point' : 'points')}",
521+
"{$this->lng->txt('rated_unit')} {$found['unit']} {$this->lng->txt($found['unit'] == 1 ? 'point' : 'points')}"
522+
];
523+
$resulttext .= implode(', ', $result_texts);
521524
}
522525

523526
$resulttext .= ")";

‎components/ILIAS/TestQuestionPool/classes/class.assFormulaQuestionResult.php‎

Lines changed: 69 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -496,59 +496,83 @@ private function transformAnswerValueAccordingToType(
496496
])->transform(round($value, $this->precision));
497497
}
498498

499+
/**
500+
* @return array{sign: int|float, value: int|float, unit: int|float, points: int|float}
501+
*/
499502
public function getResultInfo($variables, $results, $value, $unit, $units): array
500503
{
501504
if ($this->getRatingSimple()) {
502-
if ($this->isCorrect($variables, $results, $value, $units[$unit] ?? null)) {
503-
return ["points" => $this->getPoints()];
504-
} else {
505-
return ["points" => 0];
506-
}
507-
} else {
508-
$totalpoints = 0;
509-
$formula = $this->substituteFormula($variables, $results);
510-
if (preg_match_all("/(\\\$v\\d+)/ims", $formula, $matches)) {
511-
foreach ($matches[1] as $variable) {
512-
$varObj = $variables[$variable];
513-
$formula = preg_replace("/\\\$" . substr($variable, 1) . "(?![0-9]+)/", "(" . $varObj->getBaseValue() . ")" . "\\1", $formula);
514-
}
515-
}
516-
$math = new EvalMath();
517-
$math->suppress_errors = true;
518-
$result = $math->evaluate($formula);
519-
if ($this->getUnit() !== null) {
520-
$result = ilMath::_mul($result, $this->getUnit()->getFactor(), 100);
521-
}
522-
if (is_object($unit)) {
523-
$value = ilMath::_mul($value, $unit->getFactor(), 100);
524-
} else {
505+
return [
506+
'sign' => 0,
507+
'value' => 0,
508+
'unit' => 0,
509+
'points' => $this->isCorrect($variables, $results, $value, $units[$unit] ?? null)
510+
? $this->getPoints()
511+
: 0,
512+
];
513+
}
514+
515+
$totalpoints = 0;
516+
$formula = $this->substituteFormula($variables, $results);
517+
if (preg_match_all("/(\\\$v\\d+)/ims", $formula, $matches)) {
518+
foreach ($matches[1] as $variable) {
519+
$varObj = $variables[$variable];
520+
$formula = preg_replace("/\\\$" . substr($variable, 1) . "(?![0-9]+)/", "(" . $varObj->getBaseValue() . ")" . "\\1", $formula);
525521
}
526-
$details = [];
527-
if ($this->checkSign($result, $value)) {
528-
$points = ilMath::_mul($this->getPoints(), $this->getRatingSign() / 100);
529-
$totalpoints += $points;
530-
$details['sign'] = $points;
522+
}
523+
524+
$math = new EvalMath();
525+
$math->suppress_errors = true;
526+
$result = $math->evaluate($formula);
527+
528+
if ($this->getUnit() instanceof assFormulaQuestionUnit) {
529+
$result = ilMath::_mul($result, $this->getUnit()->getFactor(), 100);
530+
}
531+
532+
if ($unit instanceof assFormulaQuestionUnit) {
533+
$value = ilMath::_mul($value, $unit->getFactor(), 100);
534+
}
535+
536+
$details = [
537+
'sign' => 0,
538+
'value' => 0,
539+
'unit' => 0,
540+
'points' => 0,
541+
];
542+
543+
if ($this->checkSign($result, $value)) {
544+
$points = ilMath::_mul($this->getPoints(), $this->getRatingSign() / 100);
545+
$totalpoints += $points;
546+
$details['sign'] = $points;
547+
}
548+
549+
if ($this->isInTolerance(abs($value), abs($result), $this->getTolerance())) {
550+
$points = ilMath::_mul($this->getPoints(), $this->getRatingValue() / 100);
551+
$totalpoints += $points;
552+
$details['value'] = $points;
553+
}
554+
555+
if ($this->getUnit() instanceof assFormulaQuestionUnit) {
556+
$base1 = $units[$unit] ?? null;
557+
if ($base1 instanceof assFormulaQuestionUnit) {
558+
$base1 = $units[$base1->getBaseUnit()];
531559
}
532-
if ($this->isInTolerance(abs($value), abs($result), $this->getTolerance())) {
533-
$points = ilMath::_mul($this->getPoints(), $this->getRatingValue() / 100);
560+
561+
$base2 = $units[$this->getUnit()->getBaseUnit()];
562+
if (
563+
$base1 instanceof assFormulaQuestionUnit
564+
&& $base2 instanceof assFormulaQuestionUnit
565+
&& $base1->getId() === $base2->getId()
566+
) {
567+
$points = ilMath::_mul($this->getPoints(), $this->getRatingUnit() / 100);
534568
$totalpoints += $points;
535-
$details['value'] = $points;
569+
$details['unit'] = $points;
536570
}
537-
if ($this->getUnit() !== null) {
538-
$base1 = $units[$unit];
539-
if (is_object($base1)) {
540-
$base1 = $units[$base1->getBaseUnit()];
541-
}
542-
$base2 = $units[$this->getUnit()->getBaseUnit()];
543-
if (is_object($base1) && is_object($base2) && $base1->getId() == $base2->getId()) {
544-
$points = ilMath::_mul($this->getPoints(), $this->getRatingUnit() / 100);
545-
$totalpoints += $points;
546-
$details['unit'] = $points;
547-
}
548-
}
549-
$details['points'] = $totalpoints;
550-
return $details;
551571
}
572+
573+
$details['points'] = $totalpoints;
574+
575+
return $details;
552576
}
553577

554578
/************************************

0 commit comments

Comments
 (0)