MDL-5270 - give students a feeback comment for their performance in the entire quiz.

Also, along the way I noticed and fixed MDL-6290 student grades not rescaled when quiz maximum grade changed.

lang/en_utf8/help/quiz/overallfeedback.html

@@ -0,0 +1,14 @@
+<p align="center"><b>Overall feedback</b></p>
+
+<p>The overall feedback is some text that is shown to a student after
+they have completed an attempt at the quiz. The text that is shown
+can depend on the grade the student got.</p>
+
+<p>For example, if you type "Well done" into the first feedback box, type 40% 
+in the first grade boundary box, and type "Please study this week's work again"
+in the second feedback box, then students who score 40% or better will see the 
+"Well done" message, and students who score less than 40% will see the other message.</p>
+
+<p>The grade boundaries can be specified either as a percentage, for example "31.41%", or 
+as a number, for example "7". If your quiz is out of 10 marks, a grade boundary of 7 means
+7/10 or better.</p>

lang/en_utf8/quiz.php

@@ -180,6 +180,11 @@ $string['exportnameformat'] = '%%Y%%m%%d-%%H%%M';
 $string['exportquestions'] = 'Export questions to file';
 $string['false'] = 'False';
 $string['feedback'] = 'Feedback';
+$string['feedbackerrorboundaryformat'] = 'Feedback grade boundaries must be either a percentage or a number. The value you entered in boundary $a is not recognised.';
+$string['feedbackerrorboundaryoutofrange'] = 'Feedback grade boundaries must be between 0%% and 100%%. The value you entered in boundary $a is out of range.';
+$string['feedbackerrorjunkinboundary'] = 'You must fill in the feedback boxes without leaving any gaps.';
+$string['feedbackerrorjunkinfeedback'] = 'You must fill in the feedback boxes without leaving any gaps.';
+$string['feedbackerrororder'] = 'Feedback grade boundaries must in order, highest first. The value you entered in boundary $a is out of sequence.';
 $string['file'] = 'File';
 $string['fileformat'] = 'File format';
 $string['fillcorrect'] = 'Fill with correct';
@@ -203,6 +208,7 @@ $string['geometric'] = 'Geometric';
 $string['gift'] = 'GIFT format';
 $string['grade'] = 'Grade';
 $string['gradeaverage'] = 'Average grade';
+$string['gradeboundary'] = 'Grade boundary';
 $string['gradehighest'] = 'Highest grade';
 $string['grademethod'] = 'Grading method';
 $string['gradingdetails'] = 'Marks for this submission: $a->raw/$a->max. ';
@@ -320,6 +326,7 @@ $string['onlyteachersimport'] = 'Only teachers with editing rights can import qu
 $string['onlyteachersexport'] = 'Only teachers can export questions';
 $string['optional'] = 'optional';
 $string['outof'] = '$a->grade out of a maximum of $a->maxgrade';
+$string['overallfeedback'] = 'Overall feedback';
 $string['overdue'] = 'Overdue';
 $string['pagesize'] = 'Attempts shown per page: ';
 $string['paragraphquestion'] = 'Paragraph Question not supported at line $a. The question will be ignored';
@@ -361,6 +368,7 @@ $string['quizcloses'] = 'Quiz closes';
 $string['quiznotavailable'] = 'The quiz will not be available until: $a';
 $string['quizopen'] = 'Open the quiz';
 $string['quizopens'] = 'Quiz opens';
+$string['quizsettings'] = 'Quiz settings';
 $string['quiztimelimit'] = 'Time limit: $a';
 $string['quiztimer'] = 'Quiz Timer';
 $string['pleaseclose'] = 'Your request has been processed. You can now close this window';

mod/quiz/backuplib.php

@@ -9,12 +9,12 @@
     //                        (CL,pk->id)
     //                            |
     //           -------------------------------------------------------------------
-    //           |                    |                        |                    |
+    //           |               |                |                |               |
-    //           |               quiz_grades                   |        quiz_question_versions
+    //           |          quiz_grades           |     quiz_question_versions     |
-    //           |           (UL,pk->id,fk->quiz)              |         (CL,pk->id,fk->quiz)
+    //           |      (UL,pk->id,fk->quiz)      |      (CL,pk->id,fk->quiz)      |
-    //           |                                             |
+    //           |                                |                                |
-    //      quiz_attempts                          quiz_question_instances
+    //      quiz_attempts             quiz_question_instances                quiz_feedback
-    //  (UL,pk->id,fk->quiz)                    (CL,pk->id,fk->quiz,question)
+    //  (UL,pk->id,fk->quiz)       (CL,pk->id,fk->quiz,question)         (CL,pk->id,fk->quiz)
     //
     // Meaning: pk->primary key field of the table
     //          fk->foreign key to link with parent
@@ -204,6 +204,8 @@
         fwrite ($bf,full_tag("DELAY2",4,false,$quiz->delay2));
         //Now we print to xml question_instances (Course Level)
         $status = backup_quiz_question_instances($bf,$preferences,$quiz->id);
+        //Now we print to xml quiz_feedback (Course Level)
+        $status = backup_quiz_feedback($bf,$preferences,$quiz->id);
         //Now we print to xml question_versions (Course Level)
         $status = backup_quiz_question_versions($bf,$preferences,$quiz->id);
         //if we've selected to backup users info, then execute:
@@ -242,9 +244,6 @@
 
     //Backup quiz_question_instances contents (executed from quiz_backup_mods)
     function backup_quiz_question_instances ($bf,$preferences,$quiz) {
-
-        global $CFG;
-
         $status = true;
 
         $quiz_question_instances = get_records("quiz_question_instances","quiz",$quiz,"id");
@@ -269,11 +268,41 @@
         return $status;
     }
 
+    //Backup quiz_question_instances contents (executed from quiz_backup_mods)
+    function backup_quiz_feedback ($bf,$preferences,$quiz) {
+        $status = true;
+
+        $quiz_feedback = get_records('quiz_feedback', 'quizid', $quiz, 'id');
+        // If there are question_instances ...
+        if ($quiz_feedback) {
+            // Write start tag.
+            $status = $status & fwrite($bf,start_tag('FEEDBACKS', 4, true));
+            
+            // Iterate over each question_instance.
+            foreach ($quiz_feedback as $feedback) {
+                
+                //Start feedback instance
+                $status = $status & fwrite($bf, start_tag('FEEDBACK',5,true));
+                
+                //Print question_instance contents.
+                $status = $status & fwrite($bf, full_tag('ID', 6, false, $feedback->id));
+                $status = $status & fwrite($bf, full_tag('QUIZID', 6, false, $feedback->quizid));
+                $status = $status & fwrite($bf, full_tag('FEEDBACKTEXT', 6, false, $feedback->feedbacktext));
+                $status = $status & fwrite($bf, full_tag('MINGRADE', 6, false, $feedback->mingrade));
+                $status = $status & fwrite($bf, full_tag('MAXGRADE', 6, false, $feedback->maxgrade));
+                
+                // End feedback instance.
+                $status = $status & fwrite($bf, end_tag('FEEDBACK', 5, true));
+            }
+            
+            // Write end tag.
+            $status = $status & fwrite($bf, end_tag('FEEDBACKS', 4, true));
+        }
+        return $status;
+    }
+    
     //Backup quiz_question_versions contents (executed from quiz_backup_mods)
     function backup_quiz_question_versions ($bf,$preferences,$quiz) {
-
-        global $CFG;
-
         $status = true;
 
         $quiz_question_versions = get_records("quiz_question_versions","quiz",$quiz,"id");
@@ -304,9 +333,6 @@
 
     //Backup quiz_grades contents (executed from quiz_backup_mods)
     function backup_quiz_grades ($bf,$preferences,$quiz) {
-
-        global $CFG;
-
         $status = true;
 
         $quiz_grades = get_records("quiz_grades","quiz",$quiz,"id");
@@ -334,9 +360,6 @@
 
     //Backup quiz_attempts contents (executed from quiz_backup_mods)
     function backup_quiz_attempts ($bf,$preferences,$quiz) {
-
-        global $CFG;
-
         $status = true;
 
         $quiz_attempts = get_records("quiz_attempts","quiz",$quiz,"id");

mod/quiz/db/mysql.php

@@ -1113,6 +1113,25 @@ function quiz_upgrade($oldversion) {
                 (($CFG->quiz_review & QUIZ_REVIEW_FEEDBACK) << 3));
     }
     
+    if ($success && $oldversion < 2006081400) {
+        $success = $success && modify_database('', "
+            CREATE TABLE prefix_quiz_feedback (
+                id int(10) unsigned NOT NULL auto_increment,
+                quizid int(10) unsigned NOT NULL default '0',
+                feedbacktext text NOT NULL default '',
+                mingrade double NOT NULL default '0',
+                maxgrade double NOT NULL default '0',
+                PRIMARY KEY (id),
+                KEY quizid (quizid)
+            ) TYPE=MyISAM COMMENT='Feedback given to students based on their overall score on the test';
+        ");
+    
+        $success = $success && execute_sql("
+            INSERT INTO {$CFG->prefix}quiz_feedback (quizid, feedbacktext, maxgrade, mingrade)
+            SELECT id, '', grade + 1, 0 FROM {$CFG->prefix}quiz;
+        ");
+    }
+
     return $success;
 }
 

mod/quiz/db/mysql.sql

@@ -62,6 +62,16 @@ CREATE TABLE prefix_quiz_question_versions (
   PRIMARY KEY  (id)
 ) TYPE=MyISAM COMMENT='The mapping between old and new versions of a question';
 
+CREATE TABLE prefix_quiz_feedback (
+  id int(10) unsigned NOT NULL auto_increment,
+  quizid int(10) unsigned NOT NULL default '0',
+  feedbacktext text NOT NULL default '',
+  mingrade double NOT NULL default '0',
+  maxgrade double NOT NULL default '0',
+  PRIMARY KEY (id),
+  KEY quizid (quizid),
+) TYPE=MyISAM COMMENT='Feedback given to students based on their overall score on the test';
+
 -- --------------------------------------------------------
 -- Quiz module, quiz runtime data.
 -- --------------------------------------------------------

mod/quiz/db/postgres7.php

@@ -1435,6 +1435,24 @@ function quiz_upgrade($oldversion) {
                 (($CFG->quiz_review & QUIZ_REVIEW_FEEDBACK) << 3));
     }
     
+    if ($success && $oldversion < 2006081400) {
+        $success = $success && modify_database('', "
+            CREATE TABLE prefix_quiz_feedback (
+                id SERIAL PRIMARY KEY,
+                quizid integer NOT NULL default '0',
+                feedbacktext text NOT NULL default '',
+                maxgrade real NOT NULL default '0',
+                mingrade real NOT NULL default '0'
+            );
+        ");
+        $success = $success && modify_database('',
+            "CREATE INDEX prefix_quiz_feedback_quizid_idx ON prefix_quiz_feedback (quizid);");
+            
+        $success = $success && execute_sql("
+            INSERT INTO {$CFG->prefix}quiz_feedback (quizid, feedbacktext, maxgrade, mingrade)
+            SELECT id, '', grade + 1, 0 FROM {$CFG->prefix}quiz;
+        ");
+    }
     
     return $success;
 }

mod/quiz/db/postgres7.sql

@@ -59,6 +59,15 @@ CREATE TABLE prefix_quiz_question_versions (
   timestamp integer NOT NULL default '0'
 );
 
+CREATE TABLE prefix_quiz_feedback (
+  id SERIAL PRIMARY KEY,
+  quizid integer NOT NULL default '0',
+  feedbacktext text NOT NULL default '',
+  mingrade real NOT NULL default '0',
+  maxgrade real NOT NULL default '0'
+);
+CREATE INDEX prefix_quiz_feedback_quizid_idx ON prefix_quiz_feedback (quizid);
+
 -- --------------------------------------------------------
 -- Quiz module, quiz runtime data.
 -- --------------------------------------------------------
@@ -207,7 +216,7 @@ CREATE TABLE prefix_question_states (
   penalty real NOT NULL default '0'
 );
 CREATE INDEX prefix_question_states_attempt_idx ON prefix_question_states (attempt);
-CREATE INDEX prefix_question_states_question_idx ON prefix_question_states (question);;
+CREATE INDEX prefix_question_states_question_idx ON prefix_question_states (question);
 
 -- --------------------------------------------------------
 -- Quiz log actions.

mod/quiz/edit.php

@@ -274,9 +274,8 @@ if (self.name == 'editquestion') {
 
         // If rescaling is required save the new maximum
         if (isset($_REQUEST['maxgrade'])) {
-            $modform->grade = optional_param('maxgrade', 0);
+            if (!quiz_set_grade(optional_param('maxgrade', 0), $modform)) {
-            if (!set_field('quiz', 'grade', $modform->grade, 'id', $modform->instance)) {
+                error('Could not set a new maximum grade for the quiz');
-                error('Could not set new maximal grade for quiz');
             }
         }
     }
@@ -308,7 +307,7 @@ if (self.name == 'editquestion') {
 
     // Print basic page layout.
 
-    if (isset($modform->instance) and record_exists_sql("SELECT * FROM {$CFG->prefix}quiz_attempts WHERE quiz = '$modform->instance' AND preview = '0' LIMIT 1")){
+    if (isset($modform->instance) and record_exists_select('quiz_attempts', "quiz = '$modform->instance' AND preview = '0'")){
         // one column layout with table of questions used in this quiz
         $strupdatemodule = has_capability('moodle/course:manageactivities', $coursecontext)
                     ? update_module_button($modform->cmid, $course->id, get_string('modulename', 'quiz'))

mod/quiz/index.php

@@ -121,20 +121,23 @@
             // or the quiz has no grade, display nothing in grade col
             if ($bestgrade === NULL || $quiz->grade == 0) {
                 $gradecol = "";
+                $feedbackcol = '';
             } else {
                 //If all quiz's attempts have visible results, show bestgrade
                 if(all_attempt_results_visible($quiz, $USER)) {
                     $gradecol = "$bestgrade / $quiz->grade";
+                    $feedbackcol = quiz_get_feedback($quiz, $bestgrade);
                 } else {
                     $gradecol = "";
+                    $feedbackcol = '';
                 }
             }
         }
 
         if ($course->format == "weeks" or $course->format == "topics") {
-            $table->data[] = array ($printsection, $link, $closequiz, $gradecol);
+            $table->data[] = array ($printsection, $link, $closequiz, $gradecol, $feedbackcol);
         } else {
-            $table->data[] = array ($link, $closequiz, $gradecol);
+            $table->data[] = array ($link, $closequiz, $gradecol, $feedbackcol);
         }
     }
 
@@ -146,4 +149,4 @@
 
     print_footer($course);
 
-?>
+?>

mod/quiz/lib.php

@@ -58,14 +58,18 @@ define("QUIZ_MAX_EVENT_LENGTH", "432000");   // 5 days maximum
  * of the new instance.
  * 
  * @param object $quiz the data that came from the form.
- * @return integer the id of the new instance.
+ * @return mixed the id of the new instance on success,
+ *          false or a string error message on failure.
  */
 function quiz_add_instance($quiz) {
 
     // Process the options from the form.
     $quiz->created = time();
-    quiz_process_options($quiz);
     $quiz->questions = '';
+    $result = quiz_process_options($quiz);
+    if ($result && is_string($result)) {
+        return $result;
+    }
 
     // Try to store it in the database.
     if (!$quiz->id = insert_record("quiz", $quiz)) {
@@ -84,12 +88,15 @@ function quiz_add_instance($quiz) {
  * will update an existing instance with new data.
  * 
  * @param object $quiz the data that came from the form.
- * @return boolean true on success, false on failure.
+ * @return mixed true on success, false or a string error message on failure.
  */
 function quiz_update_instance($quiz) {
 
     // Process the options from the form.
-    quiz_process_options($quiz);
+    $result = quiz_process_options($quiz);
+    if ($result && is_string($result)) {
+        return $result;
+    }
 
     // Update the database.
     $quiz->id = $quiz->instance;
@@ -120,7 +127,7 @@ function quiz_delete_instance($id) {
 
     if ($attempts = get_records("quiz_attempts", "quiz", "$quiz->id")) {
         foreach ($attempts as $attempt) {
-            // TODO: this should use function in questionlib.php
+            // TODO: this should use the delete_attempt($attempt->uniqueid) function in questionlib.php
             if (! delete_records("question_states", "attempt", "$attempt->uniqueid")) {
                 $result = false;
             }
@@ -130,20 +137,18 @@ function quiz_delete_instance($id) {
         }
     }
 
-    if (! delete_records("quiz_attempts", "quiz", "$quiz->id")) {
+    $tables_to_purge = array(
-        $result = false;
+        'quiz_attempts' => 'quiz',
-    }
+        'quiz_grades' => 'quiz',
-
+        'quiz_question_instances' => 'quiz',
-    if (! delete_records("quiz_grades", "quiz", "$quiz->id")) {
+        'quiz_grades' => 'quiz',
-        $result = false;
+        'quiz_feedback' => 'quizid',
-    }
+        'quiz' => 'id'
-
+    );
-    if (! delete_records("quiz_question_instances", "quiz", "$quiz->id")) {
+    foreach ($tables_to_purge as $table => $keyfield) {
-        $result = false;
+        if (!delete_records($table, $keyfield, $quiz->id)) {
-    }
+            $result = false;
-
+        }
-    if (! delete_records("quiz", "id", "$quiz->id")) {
-        $result = false;
     }
 
     $pagetypes = page_import_types('mod/quiz/');
@@ -495,7 +500,58 @@ function quiz_process_options(&$quiz) {
         $quiz->timelimit = 0;
     }
     $quiz->timelimit = round($quiz->timelimit);
-
+    
+    // Quiz feedback
+    
+    // Clean up the boundary text.
+    for ($i = 0; $i < count($quiz->feedbacktext); $i += 1) {
+        if (empty($quiz->feedbacktext[$i])) {
+            $quiz->feedbacktext[$i] = '';
+        } else {
+            $quiz->feedbacktext[$i] = trim($quiz->feedbacktext[$i]);
+        }
+    }
+    
+    // Check the boundary value is a number or a percentage, and in range.
+    $i = 0;
+    while (!empty($quiz->feedbackboundaries[$i])) {
+        $boundary = trim($quiz->feedbackboundaries[$i]);
+        if (!is_numeric($boundary)) {
+            if (strlen($boundary) > 0 && $boundary[strlen($boundary) - 1] == '%') {
+                $boundary = substr($boundary, 0, -1);
+                if (is_numeric($boundary)) {
+                    $boundary = $boundary * $quiz->grade / 100.0;
+                } else {
+                    return get_string('feedbackerrorboundaryformat', 'quiz', $i + 1);
+                }
+            }
+        }
+        if ($boundary <= 0 || $boundary >= $quiz->grade) {
+            return get_string('feedbackerrorboundaryoutofrange', 'quiz', $i + 1);
+        }
+        if ($i > 0 && $boundary >= $quiz->feedbackboundaries[$i - 1]) {
+            return get_string('feedbackerrororder', 'quiz', $i + 1);
+        }
+        $quiz->feedbackboundaries[$i] = $boundary;
+        $i += 1;
+    }
+    $numboundaries = $i;
+    
+    // Check there is nothing in the remaining unused fields.
+    for ($i = $numboundaries; $i < count($quiz->feedbackboundaries); $i += 1) {
+        if (!empty($quiz->feedbackboundaries[$i]) && trim($quiz->feedbackboundaries[$i]) != '') {
+            return get_string('feedbackerrorjunkinboundary', 'quiz', $i + 1);
+        }
+    }
+    for ($i = $numboundaries + 1; $i < count($quiz->feedbacktext); $i += 1) {
+        if (!empty($quiz->feedbacktext[$i]) && trim($quiz->feedbacktext[$i]) != '') {
+            return get_string('feedbackerrorjunkinfeedback', 'quiz', $i + 1);
+        }
+    }
+    $quiz->feedbackboundaries[-1] = $quiz->grade + 1; // Needs to be bigger than $quiz->grade because of '<' test in quiz_feedback_for_grade().
+    $quiz->feedbackboundaries[$numboundaries] = 0;
+    $quiz->feedbackboundarycount = $numboundaries;
+    
     // Settings that get combined to go into the optionflags column.
     $quiz->optionflags = 0;
     if (!empty($quiz->adaptive)) {
@@ -593,6 +649,20 @@ function quiz_process_options(&$quiz) {
  */
 function quiz_after_add_or_update($quiz) {
 
+    // Save the feedback
+    delete_records('quiz_feedback', 'quizid', $quiz->id);
+    
+    for ($i = 0; $i <= $quiz->feedbackboundarycount; $i += 1) {
+        $feedback = new stdClass;
+        $feedback->quizid = $quiz->id;
+        $feedback->feedbacktext = $quiz->feedbacktext[$i];
+        $feedback->mingrade = $quiz->feedbackboundaries[$i];
+        $feedback->maxgrade = $quiz->feedbackboundaries[$i - 1];
+        if (!insert_record('quiz_feedback', $feedback, false)) {
+            return "Could not save quiz feedback.";
+        }
+    }
+
     // Remember whether this user likes the advanced settings visible or hidden.
     if (isset($quiz->optionsettingspref)) {
         set_user_preference('quiz_optionsettingspref', $quiz->optionsettingspref);

mod/quiz/locallib.php

@@ -83,10 +83,18 @@ function quiz_get_user_attempt_unfinished($quizid, $userid) {
     return get_record("quiz_attempts", "quiz", $quizid, "userid", $userid, "timefinish", 0);
 }
 
+/**
+ * @param integer $quizid the quiz id.
+ * @param integer $userid the userid.
+ * @return an array of all the ueser's attempts at this quiz. Returns an empty array if there are none.
+ */
 function quiz_get_user_attempts($quizid, $userid) {
-// Returns a list of all attempts by a user
+    if ($attempts = get_records_select("quiz_attempts", "quiz = '$quizid' AND userid = '$userid' AND timefinish > 0",
-    return get_records_select("quiz_attempts", "quiz = '$quizid' AND userid = '$userid' AND timefinish > 0",
+            "attempt ASC")) {
-                              "attempt ASC");
+        return $attempts;                        
+    } else {
+        return array();
+    }
 }
 
 
@@ -249,30 +257,132 @@ function quiz_get_all_question_grades($quiz) {
     return $grades;
 }
 
-
+/**
+ * Get the best current grade for a particular user in a quiz.
+ * 
+ * @param object $quiz the quiz object.
+ * @param integer $userid the id of the user.
+ * @return float the user's current grade for this quiz.
+ */
 function quiz_get_best_grade($quiz, $userid) {
-/// Get the best current grade for a particular user in a quiz
+    $grade = get_field('quiz_grades', 'grade', 'quiz', $quiz->id, 'userid', $userid);
-if (!$grade = get_record('quiz_grades', 'quiz', $quiz->id, 'userid', $userid)) {
+
+    // Need to detect errors/no result, without catching 0 scores.
+    if (is_numeric($grade)) {
+        return round($grade,$quiz->decimalpoints);
+    } else {
         return NULL;
     }
-
-    return (round($grade->grade,$quiz->decimalpoints));
 }
 
 /**
-* Save the overall grade for a user at a quiz in the quiz_grades table
+ * Convert the raw grade stored in $attempt into a grade out of the maximum
-*
+ * grade for this quiz.
-* @return boolean        Indicates success or failure.
+ * 
-* @param object $quiz    The quiz for which the best grade is to be calculated
+ * @param float $rawgrade the unadjusted grade, fof example $attempt->sumgrades
-*                        and then saved.
+ * @param object $quiz the quiz object. Only the fields grade, sumgrades and decimalpoints are used.
-* @param integer $userid The id of the user to save the best grade for. Can be
+ * @return float the rescaled grade.
-*                        null in which case the current user is assumed.
+ */
-*/
+function quiz_rescale_grade($rawgrade, $quiz) {
-function quiz_save_best_grade($quiz, $userid=null) {
+    if ($quiz->sumgrades) {
+        return round($rawgrade*$quiz->grade/$quiz->sumgrades, $quiz->decimalpoints);
+    } else {
+        return 0;
+    }
+}
+
+/**
+ * Get the feedback text that should be show to a student who
+ * got this grade on this quiz.
+ * 
+ * @param float $grade a grade on this quiz.
+ * @param integer $quizid the id of the quiz object.
+ * @return string the comment that corresponds to this grade (empty string if there is not one.
+ */
+function quiz_feedback_for_grade($grade, $quizid) {
+    $feedback = get_field_select('quiz_feedback', 'feedbacktext',
+            "quizid = $quizid AND mingrade <= $grade AND $grade < maxgrade");
+
+    if (empty($feedback)) {
+        $feedback = '';
+    }
+    
+    return $feedback;
+}
+
+/**
+ * @param integer $quizid the id of the quiz object.
+ * @return boolean Whether this quiz has any non-blank feedback text.
+ */
+function quiz_has_feedback($quizid) {
+    static $cache = array();
+    if (!array_key_exists($quizid, $cache)) {
+        $cache[$quizid] = record_exists_select('quiz_feedback',
+                "quizid = $quizid AND feedbacktext <> ''");
+    }
+    return $cache[$quizid];
+}
+
+/**
+ * The quiz grade is the score that student's results are marked out of. When it 
+ * changes, the corresponding data in quiz_grades and quiz_feedback needs to be
+ * rescaled.
+ * 
+ * @param float $newgrade the new maximum grade for the quiz.
+ * @param object $quiz the quiz we are updating. Passed by reference so its grade field can be updated too.
+ * @return boolean indicating success or failure.
+ */
+function quiz_set_grade($newgrade, &$quiz) {
+    // This is potentially expensive, so only do it if necessary.
+    if (abs($quiz->grade - $newgrade) < 1e-7) {
+        // Nothing to do.
+        return true;
+    }
+    
+    // Use a transaction, so that on those databases that support it, this is safer.
+    begin_sql();
+    
+    // Update the quiz table.
+    $success = set_field('quiz', 'grade', $newgrade, 'id', $quiz->instance);
+    
+    // Rescaling the other data is only possible if the old grade was non-zero.
+    if ($quiz->grade > 1e-7) {
+        global $CFG;
+    
+        $factor = $newgrade/$quiz->grade;
+        $quiz->grade = $newgrade;
+
+        // Update the quiz_grades table.
+        $timemodified = time();
+        $success = $success && set_field('quiz_grades', 'grade',
+                '$factor * grade, timemodified = $timemodified',
+                'quiz', $quiz->id);
+        
+        // Update the quiz_grades table.
+        $success = $success && execute_sql('quiz_feedback', 'mingrade', 
+                '$factor * mingrade, maxgrade = $factor * maxgrade',
+                'quizid', $quiz->id);
+    }
+    
+    if ($success) {
+        return commit_sql();
+    } else {
+        rollback_sql();
+        return false;
+    }
+}
+
+/**
+ * Save the overall grade for a user at a quiz in the quiz_grades table
+ *
+ * @param object $quiz The quiz for which the best grade is to be calculated and then saved.
+ * @param integer $userid The userid to calculate the grade for. Defaults to the current user.
+ * @return boolean Indicates success or failure.
+ */
+function quiz_save_best_grade($quiz, $userid = null) {
     global $USER;
 
-    // Assume the current user if $userid is null
+    if (empty($userid)) {
-    if (is_null($userid)) {
         $userid = $USER->id;
     }
 
@@ -284,12 +394,10 @@ function quiz_save_best_grade($quiz, $userid=null) {
 
     // Calculate the best grade
     $bestgrade = quiz_calculate_best_grade($quiz, $attempts);
-    $bestgrade = $quiz->sumgrades ? (($bestgrade / $quiz->sumgrades) * $quiz->grade) : 0;
+    $bestgrade = quiz_rescale_grade($bestgrade, $quiz);
-    $bestgrade = round($bestgrade, $quiz->decimalpoints);
+    
-
     // Save the best grade in the database
-    if ($grade = get_record('quiz_grades', 'quiz', $quiz->id, 'userid',
+    if ($grade = get_record('quiz_grades', 'quiz', $quiz->id, 'userid', $userid)) {
-     $userid)) {
         $grade->grade = $bestgrade;
         $grade->timemodified = time();
         if (!update_record('quiz_grades', $grade)) {

mod/quiz/mod.html

@@ -1,7 +1,7 @@
-<!-- This page defines the form to create or edit an instance of this module -->
-<!-- It is used from /course/mod.php.  The whole instance is available as $form. -->
-
 <?php
+// This page defines the form to create or edit an instance of this module -->
+// It is used from /course/mod.php. The whole instance is available as $form. -->
+
     require_once("$CFG->dirroot/mod/quiz/locallib.php");
 
     // Set any form variables that have not been initialized to their default value.
@@ -77,6 +77,37 @@
     if (!isset($form->delay2)) {
          $form->delay2 = $CFG->quiz_delay2;
     }
+    
+    // Get any existing feedback text out of the database.
+    if (!empty($form->id)) {
+        $feedbacks = get_records('quiz_feedback', 'quizid', $form->id, 'mingrade DESC');
+    } else {
+        $feedbacks = array();
+    }
+    $form->feedbacktext = array();
+    $form->feedbackboundaries = array();
+    foreach ($feedbacks as $feedback) {
+        $form->feedbacktext[] = $feedback->feedbacktext;
+        if ($feedback->mingrade > 0) {
+            $form->feedbackboundaries[] = (100.0 * $feedback->mingrade / $form->grade) . '%';
+        }
+    }
+
+    // Make sure there are at least 5 feedbacktexts, or a bit more than the current nubmer.
+    $numfeedbacks = max(
+            count($form->feedbacktext) * 1.5,
+            count($form->feedbackboundaries) * 1.5,
+            5
+    );
+    
+    for ($i = 0; $i < $numfeedbacks; $i += 1) {
+        if (!array_key_exists($i, $form->feedbacktext)) {
+            $form->feedbacktext[$i] = '';
+        }
+        if (!array_key_exists($i, $form->feedbackboundaries)) {
+            $form->feedbackboundaries[$i] = '';
+        }
+    }
 
     // The following are used for drop-down menus
     $yesnooptions = array(get_string("no"), get_string("yes"));
@@ -186,11 +217,47 @@
     // This time out put the ones that were not fixed.
     $fix = output_quiz_options_fields($form, 0);
 
+    // Output standard module settings.
     print_standard_coursemodule_settings($form);
 
+    // Output the boxes for typing feedback depending on overall quiz score.
+?>
+<tr><td colspan="2">
+    <?php print_heading_with_help(get_string('overallfeedback', 'quiz'), 'overallfeedback', 'quiz'); ?>
+</td></tr>
+
+<tr valign="top">
+    <td align="right"><b><?php print_string('gradeboundary', 'quiz') ?>:</b></td>
+    <td align="left">100%</td>
+</tr>
+
+<?php for ($i = 0; $i < count($form->feedbacktext); $i = $i + 1) { ?>
+    
+<tr valign="top">
+    <td align="right"><b><?php print_string('feedback', 'quiz') ?>:</b></td>
+    <td align="left">
+        <input type="text" name="feedbacktext[]" size="60" value="<?php p($form->feedbacktext[$i]) ?>" />
+    </td>
+</tr>
+        
+<?php     if ($i < count($form->feedbacktext) - 1) { ?>    
+<tr valign="top">
+    <td align="right"><b><?php print_string('gradeboundary', 'quiz') ?>:</b></td>
+    <td align="left">
+        <input type="text" name="feedbackboundaries[]" size="20" value="<?php p($form->feedbackboundaries[$i]) ?>" />
+    </td>
+</tr>
+<?php     } ?>
+
+<?php } ?>
+<tr valign="top">
+    <td align="right"><b><?php print_string('gradeboundary', 'quiz') ?>:</b></td>
+    <td align="left">0%</td>
+</tr>
+
+<?php
     if ($fix) {
         // Some options were fixed by the admin. Show them, but hidden behind an Advanced button.
-
 ?>
 <tr>
     <td align="right"><b><?php print_string('advancedsettings') ?>:</b>
@@ -499,4 +566,4 @@ function output_quiz_options_fields($form, $showfixed) {
 <?php
     return $hidden;
 }
-?>
+?>

mod/quiz/report/overview/report.php

@@ -88,6 +88,7 @@ class quiz_report extends quiz_default_report {
         $noattempts = optional_param('noattempts', 0, PARAM_INT);
         $detailedmarks = optional_param('detailedmarks', 0, PARAM_INT);
         $pagesize = optional_param('pagesize', 10, PARAM_INT);
+        $hasfeedback = quiz_has_feedback($quiz->id) && $quiz->grade > 1.e-7 && $quiz->sumgrades > 1.e-7;
 
         // Now check if asked download of data
         if ($download) {
@@ -134,6 +135,11 @@ class quiz_report extends quiz_default_report {
             }
         }
 
+        if ($hasfeedback) {
+            $tablecolumns[] = 'feedback';
+            $tableheaders[] = get_string('feedback', 'quiz');
+        }
+        
         if (!$download) {
             // Set up the table
 
@@ -200,6 +206,9 @@ class quiz_report extends quiz_default_report {
                     $headers[] = '#'.$questions[$id]->number;
                 }
             }
+            if ($hasfeedback) {
+                $headers[] = get_string('feedback', 'quiz');
+            }
             $colnum = 0;
             foreach ($headers as $item) {
                 $myxls->write(0,$colnum,$item,$formatbc);
@@ -225,6 +234,9 @@ class quiz_report extends quiz_default_report {
                     $headers .= "\t#".$question->number;
                 }
             }
+            if ($hasfeedback) {
+                $headers .= "\t" . get_string('feedback', 'quiz');
+            }
             echo $headers." \n";
         }
 
@@ -319,7 +331,7 @@ class quiz_report extends quiz_default_report {
             if (empty($sort)) {
                 $sort = ' ORDER BY uniqueid';
             }
-
+            
             // Now it is time to page the data
             if (!isset($pagesize)  || ((int)$pagesize < 1) ) {
                 $pagesize = 10;
@@ -333,6 +345,14 @@ class quiz_report extends quiz_default_report {
             }
         }
 
+        // If there is feedback, include it in the query.
+        if ($hasfeedback) {
+            $factor = $quiz->grade/$quiz->sumgrades;
+            $select .= ', qf.feedbacktext ';
+            $from .= " JOIN {$CFG->prefix}quiz_feedback AS qf ON " .
+                    "qf.quizid = $quiz->id AND qf.mingrade <= qa.sumgrades * $factor AND qa.sumgrades * $factor < qf.maxgrade";
+        }
+
         // Fetch the attempts
         if (!empty($from)) { // if we're in the site course and displaying no attempts, it makes no sense to do the query.
             $attempts = get_records_sql($select.$from.$where.$sort.$limit);
@@ -403,6 +423,13 @@ class quiz_report extends quiz_default_report {
                             }
                         }
                     }
+                    if ($hasfeedback) {
+                        if ($attempt->timefinish) {
+                            $row[] = $attempt->feedbacktext;
+                        } else {
+                            $row[] = '-';
+                        }
+                    }
                     if (!$download) {
                         $table->add_data($row);
                     } else if ($download == 'Excel') {

mod/quiz/restorelib.php

@@ -103,6 +103,8 @@
                              $mod->id, $newid);
                 //We have to restore the question_instances now (course level table)
                 $status = quiz_question_instances_restore_mods($newid,$info,$restore);
+                //We have to restore the feedback now (course level table)
+                $status = quiz_feedback_restore_mods($newid, $info, $restore, $quiz);
                 //We have to restore the question_versions now (course level table)
                 $status = quiz_question_versions_restore_mods($newid,$info,$restore);
                 //Now check if want to restore user data and do it.
@@ -132,7 +134,11 @@
         $status = true;
 
         //Get the quiz_question_instances array
-        $instances = $info['MOD']['#']['QUESTION_INSTANCES']['0']['#']['QUESTION_INSTANCE'];
+        if (array_key_exists('QUESTION_INSTANCES', $info['MOD']['#'])) {
+            $instances = $info['MOD']['#']['QUESTION_INSTANCES']['0']['#']['QUESTION_INSTANCE'];
+        } else {
+            $instances = array();
+        }
 
         //Iterate over question_instances
         for($i = 0; $i < sizeof($instances); $i++) {
@@ -181,6 +187,52 @@
         return $status;
     }
 
+    //This function restores the quiz_question_instances
+    function quiz_feedback_restore_mods($quiz_id, $info, $restore, $quiz) {
+        $status = true;
+
+        //Get the quiz_feedback array
+        if (array_key_exists('FEEDBACKS', $info['MOD']['#'])) {
+            $feedbacks = $info['MOD']['#']['FEEDBACKS']['0']['#']['FEEDBACK'];
+
+            //Iterate over the feedbacks
+            foreach ($feedbacks as $feedback_info) {
+                //traverse_xmlize($feedback_info);                                                            //Debug
+                //print_object ($GLOBALS['traverse_array']);                                                  //Debug
+                //$GLOBALS['traverse_array']="";                                                              //Debug
+    
+                //We'll need this later!!
+                $oldid = backup_todb($feedback_info['#']['ID']['0']['#']);
+    
+                //Now, build the quiz_feedback record structure
+                $feedback = new stdClass();
+                $feedback->quizid = $quiz_id;
+                $feedback->feedbacktext = backup_todb($feedback_info['#']['FEEDBACKTEXT']['0']['#']);
+                $feedback->mingrade = backup_todb($feedback_info['#']['MINGRADE']['0']['#']);
+                $feedback->maxgrade = backup_todb($feedback_info['#']['MAXGRADE']['0']['#']);
+    
+                //The structure is equal to the db, so insert the quiz_question_instances
+                $newid = insert_record('quiz_feedback', $feedback);
+    
+                if ($newid) {
+                    //We have the newid, update backup_ids
+                    backup_putid($restore->backup_unique_code, 'quiz_feedback', $oldid, $newid);
+                } else {
+                    $status = false;
+                }
+            }
+        } else {
+            $feedback = new stdClass();
+            $feedback->quizid = $quiz_id;
+            $feedback->feedbacktext = '';
+            $feedback->mingrade = 0;
+            $feedback->maxgrade = $quiz->grade + 1;
+            insert_record('quiz_feedback', $feedback);
+        }
+
+        return $status;
+    }
+
     //This function restores the quiz_question_versions
     function quiz_question_versions_restore_mods($quiz_id,$info,$restore) {
 

mod/quiz/review.php

@@ -30,6 +30,9 @@
     if (! $cm = get_coursemodule_from_instance("quiz", $quiz->id, $course->id)) {
         error("The course module for the quiz with id $quiz->id is missing");
     }
+    
+    $grade = quiz_rescale_grade($attempt->sumgrades, $quiz);
+    $feedback = quiz_feedback_for_grade($grade, $attempt->quiz);
 
     if (!count_records('question_sessions', 'attemptid', $attempt->uniqueid)) {
         // this question has not yet been upgraded to the new model
@@ -194,13 +197,16 @@
             
             $a = new stdClass;
             $percentage = round(($attempt->sumgrades/$quiz->sumgrades)*100, 0);
-            $a->grade = round(($attempt->sumgrades/$quiz->sumgrades)*$quiz->grade, $CFG->quiz_decimalpoints);
+            $a->grade = $grade;
             $a->maxgrade = $quiz->grade;
             $rawscore = round($attempt->sumgrades, $CFG->quiz_decimalpoints);
             $table->data[] = array("$strscore:", "$rawscore/$quiz->sumgrades ($percentage %)");
             $table->data[] = array("$strgrade:", get_string('outof', 'quiz', $a));
         }
     }
+    if ($options->feedback && $feedback) {
+        $table->data[] = array(get_string('feedback', 'quiz'), $feedback);
+    }
     if ($isteacher and $attempt->userid == $USER->id) {
         // the teacher is at the end of a preview. Print button to start new preview
         unset($buttonoptions);

mod/quiz/version.php

@@ -5,7 +5,7 @@
 //  This fragment is called by moodle_needs_upgrading() and /admin/index.php
 ////////////////////////////////////////////////////////////////////////////////
 
-$module->version  = 2006081000;   // The (date) version of this module
+$module->version  = 2006081400;   // The (date) version of this module
 $module->requires = 2006080900;   // Requires this Moodle version
 $module->cron     = 0;            // How often should cron check this module (seconds)?
 

mod/quiz/view.php

@@ -48,15 +48,13 @@
 
     $timenow = time();
 
-// Initialize $PAGE, compute blocks
+    // Initialize $PAGE, compute blocks
-
     $PAGE       = page_create_instance($quiz->id);
     $pageblocks = blocks_setup($PAGE);
     $blocks_preferred_width = bounded_number(180, blocks_preferred_width($pageblocks[BLOCK_POS_LEFT]), 210);
 
-// Print the page header
+    // Print the page header
-
+    if ($edit != -1 and $PAGE->user_allowed_editing()) {
-    if (($edit != -1) and $PAGE->user_allowed_editing()) {
         $USER->editing = $edit;
     }
 
@@ -77,13 +75,15 @@
 
     $available = ($quiz->timeopen < $timenow and ($timenow < $quiz->timeclose or !$quiz->timeclose)) || $isteacher;
 
-// Print the main part of the page
+    // Print the main part of the page
 
     // Print heading and tabs for teacher
     if ($isteacher) {
         $currenttab = 'info';
         include('tabs.php');
     }
+    
+    // Print quiz name and description.
     print_heading(format_string($quiz->name));
 
     if (trim(strip_tags($quiz->intro))) {
@@ -91,12 +91,15 @@
         print_simple_box(format_text($quiz->intro, FORMAT_MOODLE, $formatoptions), "center");
     }
 
+    // Print information about number of attempts and grading method.
     if ($quiz->attempts > 1) {
         echo "<p align=\"center\">".get_string("attemptsallowed", "quiz").": $quiz->attempts</p>";
+    } 
+    if ($quiz->attempts != 1) {
         echo "<p align=\"center\">".get_string("grademethod", "quiz").": ".$QUIZ_GRADE_METHOD[$quiz->grademethod]."</p>";
-    } else {
-        echo "<br />";
     }
+    
+    // Print information about timings.
     if ($available) {
         if ($quiz->timelimit) {
             echo "<p align=\"center\">".get_string("quiztimelimit","quiz", format_time($quiz->timelimit * 60))."</p>";
@@ -118,296 +121,282 @@
             notify("<a href=\"report.php?mode=overview&amp;id=$cm->id\">".get_string('numattempts', 'quiz', $a).'</a>');
         }
         
-        echo '</td></tr></table>';
+        end_page($course);
-        print_footer($course);
         exit;
     }
 
+    // Guests can't do a quiz, so offer them a choice of logging in going back.
     if (isguest()) {
-
+        $loginurl = $CFG->wwwroot.'/login/index.php';
-        $wwwroot = $CFG->wwwroot.'/login/index.php';
         if (!empty($CFG->loginhttps)) {
-            $wwwroot = str_replace('http:','https:', $wwwroot);
+            $loginurl = str_replace('http:','https:', $loginurl);
         }
 
-        notice_yesno(get_string('guestsno', 'quiz').'<br /><br />'.get_string('liketologin'),
+        notice_yesno('<p>' . get_string('guestsno', 'quiz') . "</p>\n\n</p>" . 
-                     $wwwroot, $_SERVER['HTTP_REFERER']);
+                get_string('liketologin') . '</p>', $loginurl, $_SERVER['HTTP_REFERER']);
-        print_footer($course);
+        
-        echo '</td></tr></table>';
+        end_page($course);
         exit;
     }
 
-    if ($attempts = quiz_get_user_attempts($quiz->id, $USER->id)) {
+    // Get this user's attempts.
-        $numattempts = count($attempts);
+    $attempts = quiz_get_user_attempts($quiz->id, $USER->id);
-    } else {
-        $numattempts = 0;
-    }
-
     $unfinished = false;
-    if ($unfinishedattempt =  quiz_get_user_attempt_unfinished($quiz->id, $USER->id)) {
+    if ($unfinishedattempt = quiz_get_user_attempt_unfinished($quiz->id, $USER->id)) {
         $attempts[] = $unfinishedattempt;
         $unfinished = true;
     }
+    $numattempts = count($attempts);
 
     $strattempt       = get_string("attempt", "quiz");
     $strtimetaken     = get_string("timetaken", "quiz");
     $strtimecompleted = get_string("timecompleted", "quiz");
     $strgrade         = get_string("grade");
     $strmarks         = get_string('marks', 'quiz');
-    $strbestgrade     = $QUIZ_GRADE_METHOD[$quiz->grademethod];
+    $strfeedback      = get_string('feedback', 'quiz');
-
-    $windowoptions = "left=0, top=0, channelmode=yes, fullscreen=yes, scrollbars=yes, resizeable=no, directories=no, toolbar=no, titlebar=no, location=no, status=no, menubar=no";
 
     $mygrade = quiz_get_best_grade($quiz, $USER->id);
 
-/// Now print table with existing attempts
-    $gradecolumn=0;
-    $overallstats=1;
-
     if ($attempts) {
-                    
+        // Print table with existing attempts
-        //step thru each attempt, checking there are any attempts
+
-        //for which the score can be displayed (need grade columns),
+        // Work out which columns we need, taking account what data is available in each attempt.
-        //and checking if overall grades can be displayed - no attempts for 
+        $gradecolumn = 0;
-        //which the score cannot be displayed
+        $overallstats = 1;
         foreach ($attempts as $attempt) {            
             $attemptoptions = quiz_get_reviewoptions($quiz, $attempt, $isteacher);
-            $attemptoptions->scores ? $gradecolumn=1 : $overallstats=0;                    
+            if ($attemptoptions->scores) {
+                $gradecolumn = 1;
+            } else {
+                $overallstats = 0;
+            }
         }
-    /// prepare table header
+        $gradecolumn = $gradecolumn && $quiz->grade && $quiz->sumgrades;
+        $markcolumn = $gradecolumn && ($quiz->grade <> $quiz->sumgrades);
+        $feedbackcolumn = quiz_has_feedback($quiz->id);
+
+        // prepare table header
         $table->head = array($strattempt, $strtimecompleted);
         $table->align = array("center", "left");
         $table->size = array("", "");
-        if ($gradecolumn && $quiz->grade and $quiz->sumgrades) { // Grades used so have more columns in table
+        if ($markcolumn) {
-            if ($quiz->grade <> $quiz->sumgrades) {
+            $table->head[] = "$strmarks / $quiz->sumgrades";
-                $table->head[] = "$strmarks / $quiz->sumgrades";
+            $table->align[] = 'right';
-                $table->align[] = 'right';
+            $table->size[] = '';
-                $table->size[] = '';
+        }
-            }
+        if ($gradecolumn) {
             $table->head[] = "$strgrade / $quiz->grade";
             $table->align[] = 'right';
             $table->size[] = '';
         }
+        if ($feedbackcolumn) {
+            $table->head[] = $strfeedback;
+            $table->align[] = 'left';
+            $table->size[] = '';
+        }
         if (isset($quiz->showtimetaken)) {
             $table->head[] = $strtimetaken;
-            $table->align[] = 'center';
+            $table->align[] = 'left';
             $table->size[] = '';
         }
 
-    /// One row for each attempt
+        // One row for each attempt
         foreach ($attempts as $attempt) {
-
+            $attemptoptions = quiz_get_reviewoptions($quiz, $attempt, $isteacher);
-        /// prepare strings for time taken and date completed
+            $row = array();
+            
+            // Add the attempt number, making it a link, if appropriate.
+            $row[] = make_review_link('#' . $attempt->attempt, $quiz, $attempt);
+            
+            // prepare strings for time taken and date completed
             $timetaken = '';
             $datecompleted = '';
-            if ($attempt->timefinish > 0) { // attempt has finished
+            if ($attempt->timefinish > 0) {
+                // attempt has finished
                 $timetaken = format_time($attempt->timefinish - $attempt->timestart);
                 $datecompleted = userdate($attempt->timefinish);
-            } else if ($available) { // The student can continue this attempt, so put appropriate link
+            } else if ($available) {
+                // The attempt is still in progress.
                 $timetaken = format_time(time() - $attempt->timestart);
-                $datecompleted  = "\n".'<script language="javascript" type="text/javascript">';
+                $datecompleted = '';
-                $datecompleted .= "\n<!--\n"; // -->
+            } else if ($quiz->timeclose) {
-                if (!empty($CFG->usesid) && !isset($_COOKIE[session_name()])) {
+                // The attempt was not completed but is also not available any more becuase the quiz is closed.
-                    $attempturl=sid_process_url("attempt.php?id=$cm->id");
-                } else {
-                    $attempturl="attempt.php?id=$cm->id";
-                };
-                if (!empty($quiz->popup)) {
-                    $datecompleted .= "var windowoptions = 'left=0, top=0, height='+window.screen.height+
-                            ', width='+window.screen.width+', channelmode=yes, fullscreen=yes, scrollbars=yes, '+
-                            'resizeable=no, directories=no, toolbar=no, titlebar=no, location=no, status=no, '+
-                            'menubar=no';\n";
-                    $jslink  = "javascript:var popup = window.open(\\'$attempturl\\', \\'quizpopup\\', windowoptions);";
-                } else {
-                    $jslink = $attempturl;
-                }
-
-                $linktext = get_string('continueattemptquiz', 'quiz');
-                $datecompleted .= "document.write('<a href=\"$jslink\" alt=\"$linktext\">$linktext</a>');";
-                $datecompleted .= "\n-->\n";
-                $datecompleted .= '</script>';
-                $datecompleted .= '<noscript>';
-                $datecompleted .= '<strong>'.get_string('noscript', 'quiz').'</strong>';
-                $datecompleted .= '</noscript>';
-            } else { // attempt was not completed but is also not available any more.
                 $timetaken = format_time($quiz->timeclose - $attempt->timestart);
-                $datecompleted = $quiz->timeclose ? userdate($quiz->timeclose) : '';
+                $datecompleted = userdate($quiz->timeclose);
+            } else {
+                // Something wheird happened.
+                $timetaken = '';
+                $datecompleted = '';
             }
-
+            $row[] = $datecompleted;
-            $attemptoptions = quiz_get_reviewoptions($quiz, $attempt, $isteacher);
+            
-        /// prepare strings for attempt number, mark and grade
+            if ($markcolumn) {
-            //if attempt's score is allowed to be viewed, & qz->sumgrades and qz->sumgrades defined:
+                if ($attemptoptions->scores) {
-            if ($attemptoptions->scores && $quiz->grade and $quiz->sumgrades) {
+                    $row[] = make_review_link(round($attempt->sumgrades, $quiz->decimalpoints), $quiz, $attempt);
-                $attemptmark  = round($attempt->sumgrades,$quiz->decimalpoints);
-                $attemptgrade = round(($attempt->sumgrades/$quiz->sumgrades)*$quiz->grade,$quiz->decimalpoints);
-
-                // highlight the highest grade if appropriate
-                if ($overallstats && $attemptgrade == $mygrade and ($quiz->grademethod == QUIZ_GRADEHIGHEST)) {
-                    $attemptgrade = "<span class=\"highlight\">$attemptgrade</span>";
-                }
-
-                // if attempt is closed and review is allowed then make attemptnumber and
-                // mark and grade into links to review page
-                if (quiz_review_allowed($quiz) and $attempt->timefinish > 0) {
-                    if ($quiz->popup) { // need to link to popup window
-                        $attemptmark = link_to_popup_window ("/mod/quiz/review.php?q=$quiz->id&amp;attempt=$attempt->id", 'quizpopup', round($attempt->sumgrades,$quiz->decimalpoints), '+window.screen.height+', '+window.screen.width+', '', $windowoptions, true);
-                        $attemptgrade = link_to_popup_window ("/mod/quiz/review.php?q=$quiz->id&amp;attempt=$attempt->id", 'quizpopup', $attemptgrade, '+window.screen.height+', '+window.screen.width+', '', $windowoptions, true);
-                        $attempt->attempt = link_to_popup_window ("/mod/quiz/review.php?q=$quiz->id&amp;attempt=$attempt->id", 'quizpopup', "#$attempt->attempt", '+window.screen.height+', '+window.screen.width+', '', $windowoptions, true);
-                    } else {
-                        $attemptmark = "<a href=\"review.php?q=$quiz->id&amp;attempt=$attempt->id\">".round($attempt->sumgrades,$quiz->decimalpoints).'</a>';
-                        $attemptgrade = "<a href=\"review.php?q=$quiz->id&amp;attempt=$attempt->id\">$attemptgrade</a>";
-                        $attempt->attempt = "<a href=\"review.php?q=$quiz->id&amp;attempt=$attempt->id\">#$attempt->attempt</a>";
-                    }
-                }
-
-                if ($quiz->grade <> $quiz->sumgrades) {
-                    $table->data[] = array( $attempt->attempt,
-                                            $datecompleted,
-                                            $attemptmark, $attemptgrade);
                 } else {
-                    $table->data[] = array( $attempt->attempt,
+                    $row[] = '';
-                                            $datecompleted,
-                                            $attemptgrade);
-                }
-            } else {  // No grades are being used
-                if (quiz_review_allowed($quiz)) {
-                    if($attempt->timefinish > 0) {
-                        $attempt->attempt = "<a href=\"review.php?q=$quiz->id&amp;attempt=$attempt->id\">#$attempt->attempt</a>";
-                    } else {
-                        $attempt->attempt = "<a href=\"attempt.php?id=$id\">#$attempt->attempt</a>";
-                    }
                 }
+            } 
 
-                $helpbutton=helpbutton('missing\ grade', get_string('wheregrade', 'quiz'), 'quiz', true, false, '',true);
+            // Ouside the if becuase we may be showing feedback but not grades.
-                if($gradecolumn) {
+            $attemptgrade = quiz_rescale_grade($attempt->sumgrades, $quiz);
-                    $table->data[] = array( $attempt->attempt,
+            if ($gradecolumn) {
-                                            $datecompleted,
+                if ($attemptoptions->scores) {
-                                            $helpbutton);
+                    // highlight the highest grade if appropriate
-                     
+                    if ($overallstats && !is_null($mygrade) && $attemptgrade == $mygrade && $quiz->grademethod == QUIZ_GRADEHIGHEST) {
+                        $formattedgrade = "<span class='highlight'>$attemptgrade</span>";
+                    } else {
+                        $formattedgrade = $attemptgrade;
+                    }
+                    
+                    $row[] = make_review_link($formattedgrade, $quiz, $attempt);
                 } else {
-                    $table->data[] = array( $attempt->attempt,
+                    $row[] = '';
-                                            $datecompleted);
                 }
             }
+            
+            if ($feedbackcolumn) {
+                if ($attemptoptions->feedback) {
+                    $row[] = quiz_feedback_for_grade($attemptgrade, $quiz->id);
+                } else {
+                    $row[] = '';
+                }
+            } 
+            
             if (isset($quiz->showtimetaken)) {
-                $table->data[] = $timetaken;
+                $row[] = $timetaken;
             }
+            
+            $table->data[] = $row;
         }
         print_table($table);
     }
 
-    if (!$quiz->questions) {
+    // Print information about the student's best score for this quiz if possible.
-        print_heading(get_string("noquestions", "quiz"));
+    $moreattempts = $numattempts < $quiz->attempts || $quiz->attempts == 0;
-    } else {
+    if (!$moreattempts) {
-        if ($numattempts < $quiz->attempts or !$quiz->attempts) {
+        print_heading(get_string("nomoreattempts", "quiz"));
-          
+    }
-            if ($available) {
+    
-                $options["id"] = $cm->id;
+    if ($numattempts && $quiz->sumgrades) {
-                //if overall stats are allowed (no attemps' grade not visible),
+        if (!is_null($mygrade)) {
-                //and there is at least one attempt, and quiz->grade:
+            if ($available && $moreattempts) {
-                if ($overallstats and $numattempts and $quiz->grade) {
+                $strbestgrade = $QUIZ_GRADE_METHOD[$quiz->grademethod];
-                    print_heading("$strbestgrade: $mygrade / $quiz->grade.");
+                $grademessage = "$strbestgrade: $mygrade / $quiz->grade.";
-                }
+            } else {
-                
+                $grademessage = get_string("yourfinalgradeis", "quiz", "$mygrade / $quiz->grade");
-                echo "<br />";
-                echo "</p>";
-                echo "<div align=\"center\">";
-                if ($quiz->delay1 or $quiz->delay2) {
-                     //quiz enforced time delay
-                     $lastattempt_obj = get_record_select('quiz_attempts', "quiz = $quiz->id AND attempt = $numattempts AND userid = $USER->id", 'timefinish');
-                     if ($lastattempt_obj) {
-                         $lastattempt = $lastattempt_obj->timefinish;
-                     }
-                     if($numattempts == 1 && $quiz->delay1) {
-                         if ($timenow - $quiz->delay1 > $lastattempt) {
-                              print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
-                         } else {
-                             $notify_msg = get_string('temporaryblocked', 'quiz') . '<b>'. userdate($lastattempt + $quiz->delay1). '<b>';
-                             print_simple_box($notify_msg, "center");
-                         }
-                     } else if($numattempts > 1 && $quiz->delay2) {
-                         if ($timenow - $quiz->delay2 > $lastattempt) {
-                              print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
-                         } else {
-                              $notify_msg = get_string('temporaryblocked', 'quiz') . '<b>'. userdate($lastattempt + $quiz->delay2). '<b>';
-                              print_simple_box($notify_msg, "center");
-                         }
-                     } else {
-                         print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
-                     }
-                } else {
-                     print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
-                }     
-                echo "</div>\n";
             }
-        } else {
+        
-            print_heading(get_string("nomoreattempts", "quiz"));
+            if ($overallstats) {
-            //if $quiz->grade and $quiz->sumgrades, and student is allowed to 
+                print_heading($grademessage);
-            //see summary statistics (no attempt's grade is concealed),
-            //show the student their final grade
-            if ($quiz->grade and $quiz->sumgrades and $overallstats) {
-                print_heading(get_string("yourfinalgradeis", "quiz", "$mygrade / $quiz->grade"));
             }
-            print_continue('../../course/view.php?id='.$course->id);
+            
+            if ($feedbackcolumn) {
+                echo '<p align="center">', quiz_feedback_for_grade($mygrade, $quiz->id), '</p>';
+            }
+        }
+
+        if (!($moreattempts && $available)) {
+            print_continue($CFG->webroot . '/course/view.php?id=' . $course->id);
         }
     }
-// Finish the page
+    
-    echo '</td></tr></table>';
+    if ($quiz->questions) {
+        // Print a button to start the quiz if appropriate.
+        if ($available && $moreattempts) {
+            echo "<br />";
+            echo "<div align=\"center\">";
+            if ($quiz->delay1 or $quiz->delay2) {
+                 //quiz enforced time delay
+                 $lastattempt_obj = get_record_select('quiz_attempts', "quiz = $quiz->id AND attempt = $numattempts AND userid = $USER->id", 'timefinish');
+                 if ($lastattempt_obj) {
+                     $lastattempt = $lastattempt_obj->timefinish;
+                 }
+                 if($numattempts == 1 && $quiz->delay1) {
+                     if ($timenow - $quiz->delay1 > $lastattempt) {
+                          print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
+                     } else {
+                         $notify_msg = get_string('temporaryblocked', 'quiz') . '<b>'. userdate($lastattempt + $quiz->delay1). '<b>';
+                         print_simple_box($notify_msg, "center");
+                     }
+                 } else if($numattempts > 1 && $quiz->delay2) {
+                     if ($timenow - $quiz->delay2 > $lastattempt) {
+                          print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
+                     } else {
+                          $notify_msg = get_string('temporaryblocked', 'quiz') . '<b>'. userdate($lastattempt + $quiz->delay2). '<b>';
+                          print_simple_box($notify_msg, "center");
+                     }
+                 } else {
+                     print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
+                 }
+            } else {
+                 print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm);
+            }     
+            echo "</div>\n";
+        }
+    } else {
+        // No questions in quiz.
+        print_heading(get_string("noquestions", "quiz"));
+    }
 
+    // Finish the page - this needs to be the same as in the if teacher block above.
+    echo '</td></tr></table>';
     print_footer($course);
 
-    function quiz_review_allowed($quiz) {
+// Utility functions =================================================================
-        // If not even responses are to be shown in review then we
+
-        // don't allow any review
+function quiz_review_allowed($quiz) {
-        if (!($quiz->review & QUIZ_REVIEW_RESPONSES)) {
+    // If not even responses are to be shown in review then we
-            return false;
+    // don't allow any review
-        }
+    if (!($quiz->review & QUIZ_REVIEW_RESPONSES)) {
-        if ((!$quiz->timeclose or time() < $quiz->timeclose) and !($quiz->review & QUIZ_REVIEW_OPEN)) {
+        return false;
-            return false;
-        }
-        if (($quiz->timeclose and time() > $quiz->timeclose) and !($quiz->review & QUIZ_REVIEW_CLOSED)) {
-            return false;
-        }
-        return true;
     }
-    
+    if ((!$quiz->timeclose or time() < $quiz->timeclose) and !($quiz->review & QUIZ_REVIEW_OPEN)) {
-    
+        return false;
-    function print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm) {
+    }
-        $strconfirmstartattempt =  '';
+    if (($quiz->timeclose and time() > $quiz->timeclose) and !($quiz->review & QUIZ_REVIEW_CLOSED)) {
-        
+        return false;
-        if ($unfinished) {
+    }
-            $buttontext = get_string('continueattemptquiz', 'quiz');
+    return true;
-        } else {
+}
-            if ($numattempts) {
-                $buttontext = get_string('reattemptquiz', 'quiz');
-            } else {
-                $buttontext = get_string('attemptquiznow', 'quiz');
-            }
-            if ($quiz->timelimit && $quiz->attempts) {
-                $strconfirmstartattempt = addslashes(get_string('confirmstartattempttimelimit','quiz', $quiz->attempts));
-            } else if ($quiz->timelimit) {
-                $strconfirmstartattempt = addslashes(get_string('confirmstarttimelimit','quiz'));
-            } else if ($quiz->attempts) {
-                $strconfirmstartattempt = addslashes(get_string('confirmstartattemptlimit','quiz', $quiz->attempts));
-            }
-        }
-        $buttontext = htmlspecialchars($buttontext, ENT_QUOTES);
 
-        if (!empty($quiz->popup)) {
-            $window = 'quizpopup';
-            $windowoptions = "left=0, top=0, height='+window.screen.height+', " .
-                    "width='+window.screen.width+', channelmode=yes, fullscreen=yes, " .
-                    "scrollbars=yes, resizeable=no, directories=no, toolbar=no, " .
-                    "titlebar=no, location=no, status=no, menubar=no";
-        } else {
-            $window = '_self';
-            $windowoptions = '';
-        }
 
-        $attempturl = "attempt.php?id=$cm->id";
+function print_start_quiz_button($quiz, $attempts, $numattempts, $unfinished, $cm) {
-        if (!empty($CFG->usesid) && !isset($_COOKIE[session_name()])) {
+    $strconfirmstartattempt =  '';
-            $attempturl = sid_process_url($attempturl);
+    
+    if ($unfinished) {
+        $buttontext = get_string('continueattemptquiz', 'quiz');
+    } else {
+        if ($numattempts) {
+            $buttontext = get_string('reattemptquiz', 'quiz');
+        } else {
+            $buttontext = get_string('attemptquiznow', 'quiz');
         }
+        if ($quiz->timelimit && $quiz->attempts) {
+            $strconfirmstartattempt = addslashes(get_string('confirmstartattempttimelimit','quiz', $quiz->attempts));
+        } else if ($quiz->timelimit) {
+            $strconfirmstartattempt = addslashes(get_string('confirmstarttimelimit','quiz'));
+        } else if ($quiz->attempts) {
+            $strconfirmstartattempt = addslashes(get_string('confirmstartattemptlimit','quiz', $quiz->attempts));
+        }
+    }
+    $buttontext = htmlspecialchars($buttontext, ENT_QUOTES);
+
+    if (!empty($quiz->popup)) {
+        $window = 'quizpopup';
+        $windowoptions = "left=0, top=0, height='+window.screen.height+', " .
+                "width='+window.screen.width+', channelmode=yes, fullscreen=yes, " .
+                "scrollbars=yes, resizeable=no, directories=no, toolbar=no, " .
+                "titlebar=no, location=no, status=no, menubar=no";
+    } else {
+        $window = '_self';
+        $windowoptions = '';
+    }
+
+    $attempturl = "attempt.php?id=$cm->id";
+    if (!empty($CFG->usesid) && !isset($_COOKIE[session_name()])) {
+        $attempturl = sid_process_url($attempturl);
+    }
 ?>
 <script language="javascript" type="text/javascript">
 <!--
@@ -422,6 +411,27 @@ document.write('<input type="button" value="<?php echo $buttontext ?>" onclick="
     <strong><?php print_string('noscript', 'quiz'); ?></strong>
 </noscript>
 <?php
+}
+
+function make_review_link($linktext, $quiz, $attempt) {
+    $windowoptions = "left=0, top=0, channelmode=yes, fullscreen=yes, scrollbars=yes, resizeable=no, directories=no, toolbar=no, titlebar=no, location=no, status=no, menubar=no";
+
+    $link = $linktext;
+
+    if ($attempt->timefinish && quiz_review_allowed($quiz)) {
+        $url = "review.php?q=$quiz->id&amp;attempt=$attempt->id";
+        if ($quiz->popup) {
+            $link = link_to_popup_window('/mod/quiz/' . $url, 'quizpopup', $linktext, '+window.screen.height+', '+window.screen.width+', '', $windowoptions, true);
+        } else {
+            $link = "<a href='$url'>$linktext</a>";
+        }
     }
 
+    return $link;
+}
+
+function end_page($course) {
+    echo '</td></tr></table>';
+    print_footer($course);
+}
 ?>