From 994c8c35091150eba3324b0dda02411b0744ff5b Mon Sep 17 00:00:00 2001 From: tjhunt Date: Thu, 15 May 2008 16:02:12 +0000 Subject: [PATCH] MDL-14835 - Grade not checked against valid range when manua grading --- lang/en_utf8/question.php | 5 +++- lang/en_utf8/quiz.php | 3 ++- lib/questionlib.php | 39 +++++++++++++++++++++++++++--- mod/quiz/comment.php | 28 ++++++++++++--------- mod/quiz/report/grading/report.php | 18 ++++++++++---- 5 files changed, 71 insertions(+), 22 deletions(-) diff --git a/lang/en_utf8/question.php b/lang/en_utf8/question.php index b4a7fa02eb9..aec63858880 100644 --- a/lang/en_utf8/question.php +++ b/lang/en_utf8/question.php @@ -37,8 +37,11 @@ $string['erroraccessingcontext'] = 'Cannot access context'; $string['errordeletingquestionsfromcategory'] = 'Error deleting questions from category $a.'; $string['errorfilecannotbecopied'] = 'Error cannot copy file $a.'; $string['errorfilecannotbemoved'] = 'Error cannot move file $a.'; -$string['errormovingquestions'] = 'Error while moving questions with ids $a.'; $string['errorfileschanged'] = 'Error files linked to from questions have changed since form was displayed.'; +$string['errormanualgradeoutofrange'] = 'The grade $a->grade is not between 0 and $a->maxgrade for question $a->name. The score and comment have not been saved.'; +$string['errormovingquestions'] = 'Error while moving questions with ids $a.'; +$string['errorsavingcomment'] = 'Error saving the comment for question $a->name in the database.'; +$string['errorupdatingattempt'] = 'Error updating attempt $a->id in the database.'; $string['exportcategory'] = 'Export category'; $string['filesareasite']= 'the site files area'; $string['filesareacourse']= 'the course files area'; diff --git a/lang/en_utf8/quiz.php b/lang/en_utf8/quiz.php index ed58394af91..a6f05b6ad60 100644 --- a/lang/en_utf8/quiz.php +++ b/lang/en_utf8/quiz.php @@ -97,7 +97,8 @@ $string['categorymoveto'] = 'Move them to this category'; $string['categorynamecantbeblank'] = 'The category name cannot be blank.'; $string['categorynoedit'] = 'You do not have editing privileges in the category \'$a\'.'; $string['categoryupdated'] = 'The category was successfully updated'; -$string['changessaved'] = 'Grading Changes Saved'; +$string['changessaved'] = 'Grading changes saved'; +$string['changessavedwitherrors'] = 'Some errors occurred while saving the grading changes'; $string['checkanswer'] = 'Check'; $string['choice'] = 'Choice'; $string['choices'] = 'Available choices'; diff --git a/lib/questionlib.php b/lib/questionlib.php index d9ecc9683fb..b14bd2aa826 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1251,8 +1251,19 @@ function regrade_question_in_attempt($question, $attempt, $cmoptions, $verbose=f } if ($action->event == QUESTION_EVENTMANUALGRADE) { - question_process_comment($question, $replaystate, $attempt, + // Ensure that the grade is in range - in the past this was not checked, + // but now it is (MDL-14835) - so we need to ensure the data is valid before + // proceeding. + if ($states[$j]->grade < 0) { + $states[$j]->grade = 0; + } else if ($states[$j]->grade > $question->maxgrade) { + $states[$j]->grade = $question->maxgrade; + } + $error = question_process_comment($question, $replaystate, $attempt, $replaystate->manualcomment, $states[$j]->grade); + if (is_string($error)) { + notify($error); + } } else { // Reprocess (regrade) responses @@ -1549,13 +1560,34 @@ function question_print_comment_box($question, $state, $attempt, $url) { } } +/** + * Process a manual grading action. That is, use $comment and $grade to update + * $state and $attempt. The attempt and the comment text are stored in the + * database. $state is only updated in memory, it is up to the call to store + * that, if appropriate. + * + * @param object $question the question + * @param object $state the state to be updated. + * @param object $attempt the attempt the state belongs to, to be updated. + * @param string $comment the comment the teacher added + * @param float $grade the grade the teacher assigned. + * @return mixed true on success, a string error message if a problem is detected + * (for example score out of range). + */ function question_process_comment($question, &$state, &$attempt, $comment, $grade) { + if ($grade < 0 || $grade > $question->maxgrade) { + $a = new stdClass; + $a->grade = $grade; + $a->maxgrade = $question->maxgrade; + $a->name = $question->name; + return get_string('errormanualgradeoutofrange', 'question', $a); + } // Update the comment and save it in the database $comment = trim($comment); $state->manualcomment = $comment; if (!set_field('question_sessions', 'manualcomment', $comment, 'attemptid', $attempt->uniqueid, 'questionid', $question->id)) { - print_error('cannotsavecomment'); + return get_string('errorsavingcomment', 'question', $question); } // Update the attempt if the score has changed. @@ -1563,7 +1595,7 @@ function question_process_comment($question, &$state, &$attempt, $comment, $grad $attempt->sumgrades = $attempt->sumgrades - $state->last_graded->grade + $grade; $attempt->timemodified = time(); if (!update_record('quiz_attempts', $attempt)) { - print_error('cannotsavequiz'); + return get_string('errorupdatingattempt', 'question', $attempt); } } @@ -1594,6 +1626,7 @@ function question_process_comment($question, &$state, &$attempt, $comment, $grad $state->changed = 1; } + return true; } /** diff --git a/mod/quiz/comment.php b/mod/quiz/comment.php index c361577dab1..29b8ae4eda2 100644 --- a/mod/quiz/comment.php +++ b/mod/quiz/comment.php @@ -61,19 +61,23 @@ if ($data = data_submitted() and confirm_sesskey()) { // the following will update the state and attempt - question_process_comment($question, $state, $attempt, $data->response['comment'], $data->response['grade']); - // If the state has changed save it and update the quiz grade - if ($state->changed) { - save_question_session($question, $state); - quiz_save_best_grade($quiz, $attempt->userid); + $error = question_process_comment($question, $state, $attempt, $data->response['comment'], $data->response['grade']); + if (is_string($error)) { + notify($error); + } else { + // If the state has changed save it and update the quiz grade + if ($state->changed) { + save_question_session($question, $state); + quiz_save_best_grade($quiz, $attempt->userid); + } + + notify(get_string('changessaved')); + echo '
"; + + print_footer(); + exit; } - - notify(get_string('changessaved')); - echo '
"; - - print_footer(); - exit; } question_print_comment_box($question, $state, $attempt, $CFG->wwwroot.'/mod/quiz/comment.php'); diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index 93f58faaeb5..89a7c5de5a8 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -70,6 +70,7 @@ class quiz_report extends quiz_default_report { confirm_sesskey(); // now go through all of the responses and save them. + $allok = true; foreach($data->manualgrades as $uniqueid => $response) { // get our attempt if (! $attempt = get_record('quiz_attempts', 'uniqueid', $uniqueid)) { @@ -83,15 +84,22 @@ class quiz_report extends quiz_default_report { $state = &$states[$question->id]; // the following will update the state and attempt - question_process_comment($question, $state, $attempt, $response['comment'], $response['grade']); - - // If the state has changed save it and update the quiz grade - if ($state->changed) { + $error = question_process_comment($question, $state, $attempt, $response['comment'], $response['grade']); + if (is_string($error)) { + notify($error); + $allok = false; + } else if ($state->changed) { + // If the state has changed save it and update the quiz grade save_question_session($question, $state); quiz_save_best_grade($quiz, $attempt->userid); } } - notify(get_string('changessaved', 'quiz'), 'notifysuccess'); + + if ($allok) { + notify(get_string('changessaved', 'quiz'), 'notifysuccess'); + } else { + notify(get_string('changessavedwitherrors', 'quiz'), 'notifysuccess'); + } } // our 3 different views