Skip to content

Commit a302be7

Browse files
christianbeeznestchristianbeeznest
committed
Exercise: Add validation to enforce correct answer and positive score - refs BT#22404
1 parent f67d2a3 commit a302be7

File tree

3 files changed

+144
-41
lines changed

3 files changed

+144
-41
lines changed

‎main/exercise/multiple_answer.class.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,58 @@ public function createAnswersForm($form)
184184
$form->setConstants(['nb_answers' => $nb_answers]);
185185
}
186186

187+
/**
188+
* Validate question answers before saving.
189+
*
190+
* @param object $form The form object.
191+
* @return array|bool True if valid, or an array with errors if invalid.
192+
*/
193+
public function validateAnswers($form)
194+
{
195+
$nb_answers = $form->getSubmitValue('nb_answers');
196+
$hasCorrectAnswer = false;
197+
$hasValidWeighting = false;
198+
$errors = [];
199+
$error_fields = [];
200+
201+
for ($i = 1; $i <= $nb_answers; $i++) {
202+
$isCorrect = $form->getSubmitValue("correct[$i]");
203+
$weighting = trim($form->getSubmitValue("weighting[$i]") ?? '');
204+
205+
if ($isCorrect) {
206+
$hasCorrectAnswer = true;
207+
208+
if (is_numeric($weighting) && floatval($weighting) > 0) {
209+
$hasValidWeighting = true;
210+
}
211+
}
212+
}
213+
214+
if (!$hasCorrectAnswer) {
215+
$errors[] = get_lang('AtLeastOneCorrectAnswerRequired');
216+
$error_fields[] = "correct";
217+
}
218+
219+
if ($hasCorrectAnswer && !$hasValidWeighting) {
220+
// Only show if at least one correct answer exists but its weighting is not valid
221+
$errors[] = get_lang('AtLeastOneCorrectAnswerMustHavePositiveWeighting');
222+
}
223+
224+
return empty($errors) ? true : ['errors' => $errors, 'error_fields' => $error_fields];
225+
}
226+
187227
/**
188228
* {@inheritdoc}
189229
*/
190230
public function processAnswersCreation($form, $exercise)
191231
{
232+
$validationResult = $this->validateAnswers($form);
233+
if ($validationResult !== true) {
234+
Display::addFlash(Display::return_message(implode("<br>", $validationResult['errors']), 'error'));
235+
236+
return;
237+
}
238+
192239
$questionWeighting = 0;
193240
$objAnswer = new Answer($this->iid);
194241
$nb_answers = $form->getSubmitValue('nb_answers');

‎main/exercise/question_admin.inc.php‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -51,50 +51,53 @@
5151
}
5252

5353
// FORM VALIDATION
54-
if (isset($_POST['submitQuestion']) && $form->validate()) {
55-
// Question
56-
$objQuestion->processCreation($form, $objExercise);
57-
$objQuestion->processAnswersCreation($form, $objExercise);
58-
// TODO: maybe here is the better place to index this tool, including answers text
59-
// redirect
60-
if (in_array($objQuestion->type, [HOT_SPOT, HOT_SPOT_COMBINATION, HOT_SPOT_DELINEATION])) {
61-
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&page='.$page.'&hotspotadmin='.$objQuestion->iid.'&'.api_get_cidreq(
62-
).'"</script>';
63-
} elseif (in_array($objQuestion->type, [MULTIPLE_ANSWER_DROPDOWN, MULTIPLE_ANSWER_DROPDOWN_COMBINATION])) {
64-
$url = 'admin.php?'
65-
.api_get_cidreq().'&'
66-
.http_build_query(['exerciseId' => $exerciseId, 'page' => $page, 'mad_admin' => $objQuestion->iid]);
67-
echo '<script type="text/javascript">window.location.href="'.$url.'"</script>';
68-
} else {
69-
if (isset($_GET['editQuestion'])) {
70-
if (empty($exerciseId)) {
71-
Display::addFlash(Display::return_message(get_lang('ItemUpdated')));
72-
$url = 'admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&editQuestion='.$objQuestion->iid;
73-
echo '<script type="text/javascript">window.location.href="'.$url.'"</script>';
74-
exit;
75-
}
76-
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&page='.$page.'&message=ItemUpdated"</script>';
54+
if (isset($_POST['submitQuestion'])) {
55+
$validationResult = true;
56+
if (method_exists($objQuestion, 'validateAnswers')) {
57+
$validationResult = $objQuestion->validateAnswers($form);
58+
}
59+
if (is_array($validationResult) && !empty($validationResult['errors'])) {
60+
echo Display::return_message(implode("<br>", $validationResult['errors']), 'error', false);
61+
} elseif ($form->validate()) {
62+
$objQuestion->processCreation($form, $objExercise);
63+
$objQuestion->processAnswersCreation($form, $objExercise);
64+
if (in_array($objQuestion->type, [HOT_SPOT, HOT_SPOT_COMBINATION, HOT_SPOT_DELINEATION])) {
65+
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&page='.$page.'&hotspotadmin='.$objQuestion->iid.'&'.api_get_cidreq().'";</script>';
66+
} elseif (in_array($objQuestion->type, [MULTIPLE_ANSWER_DROPDOWN, MULTIPLE_ANSWER_DROPDOWN_COMBINATION])) {
67+
$url = 'admin.php?'.api_get_cidreq().'&'.http_build_query(['exerciseId' => $exerciseId, 'page' => $page, 'mad_admin' => $objQuestion->iid]);
68+
echo '<script type="text/javascript">window.location.href="'.$url.'";</script>';
7769
} else {
78-
// New question
79-
$page = 1;
80-
$length = api_get_configuration_value('question_pagination_length');
81-
if (!empty($length)) {
82-
$page = round($objExercise->getQuestionCount() / $length);
70+
if (isset($_GET['editQuestion'])) {
71+
if (empty($exerciseId)) {
72+
Display::addFlash(Display::return_message(get_lang('ItemUpdated')));
73+
$url = 'admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&editQuestion='.$objQuestion->iid;
74+
echo '<script type="text/javascript">window.location.href="'.$url.'";</script>';
75+
exit;
76+
}
77+
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&page='.$page.'&message=ItemUpdated";</script>';
78+
} else {
79+
// New question
80+
$page = 1;
81+
$length = api_get_configuration_value('question_pagination_length');
82+
if (!empty($length)) {
83+
$page = round($objExercise->getQuestionCount() / $length);
84+
}
85+
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&page='.$page.'&message=ItemAdded";</script>';
8386
}
84-
echo '<script type="text/javascript">window.location.href="admin.php?exerciseId='.$exerciseId.'&'.api_get_cidreq().'&page='.$page.'&message=ItemAdded"</script>';
8587
}
88+
exit();
8689
}
87-
} else {
88-
if (isset($questionName)) {
89-
echo '<h3>'.$questionName.'</h3>';
90-
}
91-
if (!empty($pictureName)) {
92-
echo '<img src="../document/download.php?doc_url=%2Fimages%2F'.$pictureName.'" border="0">';
93-
}
94-
if (!empty($msgErr)) {
95-
echo Display::return_message($msgErr);
96-
}
97-
// display the form
98-
$form->display();
9990
}
91+
92+
if (isset($questionName)) {
93+
echo '<h3>'.$questionName.'</h3>';
94+
}
95+
if (!empty($pictureName)) {
96+
echo '<img src="../document/download.php?doc_url=%2Fimages%2F'.$pictureName.'" border="0">';
97+
}
98+
if (!empty($msgErr)) {
99+
echo Display::return_message($msgErr);
100+
}
101+
102+
$form->display();
100103
}

‎main/exercise/unique_answer.class.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,64 @@ public function setDirectOptions($i, FormValidator $form, $renderer, $select_lp_
335335
);
336336
}
337337

338+
/**
339+
* Validate question answers before saving.
340+
* @return bool|string True if valid, error message if invalid.
341+
*/
342+
public function validateAnswers($form)
343+
{
344+
$correct = $form->getSubmitValue('correct');
345+
$nb_answers = $form->getSubmitValue('nb_answers');
346+
$hasCorrectAnswer = false;
347+
$hasPositiveScore = false;
348+
$errors = [];
349+
$error_fields = [];
350+
351+
for ($i = 1; $i <= $nb_answers; $i++) {
352+
$answer = trim($form->getSubmitValue("answer[$i]") ?? '');
353+
$weighting = trim($form->getSubmitValue("weighting[$i]") ?? '');
354+
$isCorrect = ($correct == $i);
355+
if (empty($answer)) {
356+
$errors[] = sprintf(get_lang('QuestionCannotBeEmpty'), $i);
357+
$error_fields[] = "answer[$i]";
358+
}
359+
360+
if ($weighting === '' || !is_numeric($weighting)) {
361+
$errors[] = sprintf(get_lang('WeightingMustBeNumeric'), $i);
362+
$error_fields[] = "weighting[$i]";
363+
}
364+
365+
if ($isCorrect) {
366+
$hasCorrectAnswer = true;
367+
if (floatval($weighting) > 0) {
368+
$hasPositiveScore = true;
369+
}
370+
}
371+
}
372+
373+
if (!$hasCorrectAnswer) {
374+
$errors[] = get_lang('MustSelectCorrectAnswer');
375+
$error_fields[] = "correct";
376+
}
377+
378+
if (!$hasPositiveScore) {
379+
$errors[] = get_lang('CorrectAnswerMustHavePositiveScore');
380+
}
381+
382+
return empty($errors) ? true : ['errors' => $errors, 'error_fields' => $error_fields];
383+
}
384+
338385
/**
339386
* {@inheritdoc}
340387
*/
341388
public function processAnswersCreation($form, $exercise)
342389
{
390+
$validationResult = $this->validateAnswers($form);
391+
if ($validationResult !== true) {
392+
Display::addFlash(Display::return_message(implode("<br>", $validationResult['errors']), 'error'));
393+
return;
394+
}
395+
343396
$questionWeighting = $nbrGoodAnswers = 0;
344397
$correct = $form->getSubmitValue('correct');
345398
$objAnswer = new Answer($this->iid);

0 commit comments

Comments
 (0)