MDL-14835 - Grade not checked against valid range when manua grading

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';

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';

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;
 }
 
 /**

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']);
+        $error = 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 (is_string($error)) {
-        if ($state->changed) {
+            notify($error);
-            save_question_session($question, $state);
+        } else {
-            quiz_save_best_grade($quiz, $attempt->userid);
+            // 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 '<div class="boxaligncenter"><input type="button" onclick="window.opener.location.reload(1); self.close();return false;" value="' .
+                    get_string('closewindow') . "\" /></div>";
+
+            print_footer();
+            exit;
         }
-
-        notify(get_string('changessaved'));
-        echo '<div class="boxaligncenter"><input type="button" onclick="window.opener.location.reload(1); self.close();return false;" value="' .
-                get_string('closewindow') . "\" /></div>";
-
-        print_footer();
-        exit;
     }
 
     question_print_comment_box($question, $state, $attempt, $CFG->wwwroot.'/mod/quiz/comment.php');

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']);
+                $error = question_process_comment($question, $state, $attempt, $response['comment'], $response['grade']);
-
+                if (is_string($error)) {
-                // If the state has changed save it and update the quiz grade
+                    notify($error);
-                if ($state->changed) {
+                    $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