better parameter validation and general code cleanup for edit.php

diff --git a/mod/quiz/edit.php b/mod/quiz/edit.php
index 37e1231c95daf221dc061ceedc6cc92593cb6eba..4a90136e54526e89b292f64a436d0c734b0444a5 100644 (file)
--- a/mod/quiz/edit.php
+++ b/mod/quiz/edit.php
@@ -5,28 +5,12 @@
 
     require_login();
 
-    optional_variable($courseid);
-    optional_variable($quizid);
-    optional_variable($page, 0);
-    optional_variable($perpage, "20"); 
+    $courseid = optional_param('courseid');
+    $quizid   = optional_param('quizid');
+    $page     = optional_param('page', 0);
+    $perpage  = optional_param('perpage', 20);
 
-    if (empty($destination)) {
-        $destination = "";
-    }
-
-    $modform = data_submitted($destination);
-
-    if ($modform and !empty($modform->course)) { // data submitted
-
-        $modform->name = trim($modform->name);
-
-        if (empty($modform->name)) {
-            if (empty($modform->intro)) {
-                $modform->name = get_string('modulename', 'quiz');
-            } else {
-                $modform->name = strip_tags($modform->intro);
-            }
-        }
+    if ($modform = data_submitted() and !empty($modform->course)) { // data submitted
 
         $SESSION->modform = $modform;    // Save the form in the current session
 
@@ -79,17 +63,9 @@
     }
 
 
-    // Now, check for commands on this page and modify variables as necessary
-    
-    if (isset($cancel)) {
-        redirect('view.php?q='.$modform->instance);
-    }
-    
-    if (isset($recurse)) {
-        $modform->recurse = $recurse;
-    }
+/// Now, check for commands on this page and modify variables as necessary
 
-    if (!empty($up)) { /// Move the given question up a slot
+    if (isset($_REQUEST['up']) and confirm_sesskey()) { /// Move the given question up a slot
         $questions = explode(",", $modform->questions);
         if ($questions[0] <> $up) {
             foreach ($questions as $key => $question) {
@@ -107,7 +83,7 @@
         }
     }
 
-    if (!empty($down)) { /// Move the given question down a slot
+    if (isset($_REQUEST['down']) and confirm_sesskey()) { /// Move the given question down a slot
         $questions = explode(",", $modform->questions);
         if ($questions[count($questions)-1] <> $down) {
             foreach ($questions as $key => $question) {
@@ -125,7 +101,7 @@
         }
     }
 
-    if (!empty($add)) { /// Add a question to the current quiz
+    if (isset($_REQUEST['add']) and confirm_sesskey()) { /// Add a question to the current quiz
         $rawquestions = $_POST;
         if (!empty($modform->questions)) {
             $questions = explode(",", $modform->questions);
@@ -164,7 +140,7 @@
         quiz_questiongrades_update($modform->grades, $modform->instance);
     }
 
-    if (!empty($delete)) { /// Delete a question from the list 
+    if (isset($_REQUEST['delete']) and confirm_sesskey()) { /// Delete a question from the list 
         $questions = explode(",", $modform->questions);
         foreach ($questions as $key => $question) {
             if ($question == $delete) {
@@ -181,7 +157,7 @@
         }
     }
 
-    if (!empty($setgrades)) { /// The grades have been updated, so update our internal list
+    if (isset($_REQUEST['setgrades']) and confirm_sesskey()) { /// The grades have been updated, so update our internal list
         $rawgrades = $_POST;
         unset($modform->grades);
         foreach ($rawgrades as $key => $value) {    // Parse input for question -> grades
@@ -195,11 +171,16 @@
         }
         quiz_questiongrades_update($modform->grades, $modform->instance);
     }
- 
-
-    if (!empty($cat)) { //-----------------------------------------------------------
+    
+    if (isset($_REQUEST['cat'])) { /// coming from category selection drop-down menu
         $modform->category = $cat;
     }
+        
+    if (isset($_REQUEST['recurse'])) { /// coming from checkbox below category selection form
+        $modform->recurse = $recurse;
+    }
+    
+/// all commands have been dealt with, now print the page
 
     if (empty($modform->category)) {
         $category = quiz_get_default_category($course->id);
@@ -209,23 +190,15 @@
         $modform->recurse = 1;
     }
 
-    $modform->sumgrades = 0;
-    if (!empty($modform->grades)) {
-        foreach ($modform->grades as $grade) {
-            $modform->sumgrades += $grade;
-        }
-    }
-
     $SESSION->modform = $modform;
 
-    $strname    = get_string('name');
     $strquizzes = get_string('modulenameplural', 'quiz');
     $strediting = get_string('editquestions', "quiz");
-    $strheading = empty($modform->name) ? $strediting : $modform->name;
 
     // Print basic page layout.
 
     if (!isset($modform->instance)) {
+        // one column layout for non-quiz-specific editing page
         print_header_simple($strediting, '',
                  "<a href=\"index.php?id=$course->id\">$strquizzes</a>".
                  " -> $strediting");
@@ -233,6 +206,7 @@
         echo '<tr><td valign="top">';
 
     } else {
+        // two column layout with quiz info in left column
         print_header_simple($strediting, '',
                  "<a href=\"index.php?id=$course->id\">$strquizzes</a>".
                  " -> <a href=\"view.php?q=$modform->instance\">$modform->name</a>".
@@ -258,15 +232,16 @@
         }
 
         print_simple_box_end();
-        print_continue('view.php?q='.$modform->instance);
         echo '</td><td valign="top" width="50%">';
     }
+    // non-quiz-specific column
     print_simple_box_start("center", "100%", $THEME->cellcontent2);
+    // starts with category selection form
     quiz_print_category_form($course, $modform->category, $modform->recurse);
     print_simple_box_end();
     
     print_spacer(5,1);
-
+    // continues with list of questions
     print_simple_box_start("center", "100%", $THEME->cellcontent2);
     quiz_print_cat_question_list($modform->category,
                                  isset($modform->instance), $modform->recurse, $page, $perpage);
@@ -277,6 +252,8 @@
 
     if (!isset($modform->instance)) {
         print_continue("index.php?id=$modform->course");
+    } else {
+        print_continue('view.php?q='.$modform->instance);
     }
 
     print_footer($course);

diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php
index 290f10cdf1436e645bd09dc584c42f6d48db4dac..527abf0b79e9f46583b3bc98e04ece282694fba7 100644 (file)
--- a/mod/quiz/lib.php
+++ b/mod/quiz/lib.php
@@ -14,16 +14,21 @@ function quiz_add_instance($quiz) {
 /// will create a new instance and return the id number
 /// of the new instance.
 
-    global $SESSION;
-
-    unset($SESSION->modform);
-
     $quiz->created      = time();
     $quiz->timemodified = time();
     $quiz->timeopen = make_timestamp($quiz->openyear, $quiz->openmonth, $quiz->openday,
                                      $quiz->openhour, $quiz->openminute, 0);
     $quiz->timeclose = make_timestamp($quiz->closeyear, $quiz->closemonth, $quiz->closeday,
                                       $quiz->closehour, $quiz->closeminute, 0);
+    
+    if (empty($quiz->name)) {
+        if (empty($quiz->intro)) {
+            $quiz->name = get_string('modulename', 'quiz');
+        } else {
+            $quiz->name = strip_tags($quiz->intro);
+        }
+    }
+    $quiz->name = trim($quiz->name);
 
     if (!$quiz->id = insert_record("quiz", $quiz)) {
         return false;  // some error occurred
@@ -73,10 +78,6 @@ function quiz_update_instance($quiz) {
 /// (defined by the form in mod.html or edit.php) this function
 /// will update an existing instance with new data.
 
-    global $SESSION;
-
-    unset($SESSION->modform);
-
     $quiz->timemodified = time();
     if (isset($quiz->openyear)) { // this would not be set if we come from edit.php
         $quiz->timeopen = make_timestamp($quiz->openyear, $quiz->openmonth, $quiz->openday,

diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php
index af0bd671a116e6cc22cbbc4438d3d6961acd78ea..d3c458be2db3403902342bcebc6b34973220fa59 100644 (file)
--- a/mod/quiz/locallib.php
+++ b/mod/quiz/locallib.php
@@ -353,25 +353,25 @@ function quiz_get_question_grades($quizid, $questionlist) {
 }
 
 function quiz_questiongrades_update($grades, $quizid) {
+    // this is called from edit.php to store changes to the question grades 
+    // in the quiz_question_grades table. It does not update 'sumgrades' in the quiz table.
     $existing = get_records("quiz_question_grades", "quiz", $quizid, "", "question,grade,id");
     foreach ($grades as $question => $grade) {
-        if ($question) {
-            unset($questiongrade);
-            $questiongrade->quiz = $quizid;
-            $questiongrade->question = $question;
-            $questiongrade->grade = $grade;
-            if (isset($existing[$question])) {
-                if ($existing[$question]->grade != $grade) {
-                    $questiongrade->id = $existing[$question]->id;
-                    if (!update_record("quiz_question_grades", $questiongrade)) {
-                        return false;
-                    }
-                }
-            } else {
-                if (!insert_record("quiz_question_grades", $questiongrade)) {
+        unset($questiongrade);
+        $questiongrade->quiz = $quizid;
+        $questiongrade->question = $question;
+        $questiongrade->grade = $grade;
+        if (isset($existing[$question])) {
+            if ($existing[$question]->grade != $grade) {
+                $questiongrade->id = $existing[$question]->id;
+                if (!update_record("quiz_question_grades", $questiongrade)) {
                     return false;
                 }
             }
+        } else {
+            if (!insert_record("quiz_question_grades", $questiongrade)) {
+                return false;
+            }
         }
     }
 }
@@ -1109,7 +1109,7 @@ function quiz_print_question_list($questionlist, $grades) {
 // $questionlist is comma-separated list
 // $grades is an array of corresponding grades
 
-    global $THEME;
+    global $THEME, $USER;
 
     if (!$questionlist) {
         echo "<p align=\"center\">";
@@ -1143,6 +1143,7 @@ function quiz_print_question_list($questionlist, $grades) {
     $sumgrade = 0;
     $total = count($order);
     echo "<form method=\"post\" action=\"edit.php\">";
+    echo "<input type=\"hidden\" name=\"sesskey\" value=\"$USER->sesskey\">";
     echo "<table border=\"0\" cellpadding=\"5\" cellspacing=\"2\" width=\"100%\">\n";
     echo "<tr><th width=\"*\" colspan=\"3\" nowrap=\"nowrap\">$strorder</th><th align=\"left\" width=\"100%\" nowrap=\"nowrap\">$strquestionname</th><th width=\"*\" nowrap=\"nowrap\">$strtype</th><th width=\"*\" nowrap=\"nowrap\">$strgrade</th><th width=\"*\" nowrap=\"nowrap\">$stredit</th></tr>\n";
     foreach ($order as $qnum) {
@@ -1156,13 +1157,13 @@ function quiz_print_question_list($questionlist, $grades) {
         echo "<td>$count</td>";
         echo "<td>";
         if ($count != 1) {
-            echo "<a title=\"$strmoveup\" href=\"edit.php?up=$qnum\"><img
+            echo "<a title=\"$strmoveup\" href=\"edit.php?up=$qnum&amp;sesskey=$USER->sesskey\"><img
                  src=\"../../pix/t/up.gif\" border=\"0\" alt=\"$strmoveup\" /></a>";
         }
         echo "</td>";
         echo "<td>";
         if ($count != $total) {
-            echo "<a title=\"$strmovedown\" href=\"edit.php?down=$qnum\"><img
+            echo "<a title=\"$strmovedown\" href=\"edit.php?down=$qnum&amp;sesskey=$USER->sesskey\"><img
                  src=\"../../pix/t/down.gif\" border=\"0\" alt=\"$strmovedown\" /></a>";
         }
         echo "</td>";
@@ -1178,15 +1179,15 @@ function quiz_print_question_list($questionlist, $grades) {
                              "q$qnum", (string)$grades[$qnum], "");
         }
         echo "<td>";
-            echo "<a title=\"$strdelete\" href=\"edit.php?delete=$qnum\"><img
-                 src=\"../../pix/t/delete.gif\" border=\"0\" alt=\"$strdelete\" /></a>&nbsp;";
-            echo "<a title=\"$strpreview\" href=\"#\" onClick=\"openpopup('/mod/quiz/preview.php?id=$qnum','$strpreview','scrollbars=yes,resizable=yes,width=700,height=480', false)\"><img
-                  src=\"../../pix/t/preview.gif\" border=\"0\" alt=\"$strpreview\" /></a>&nbsp;";
-
-            if ($canedit) {
-                echo "<a title=\"$stredit\" href=\"question.php?id=$qnum\"><img
-                     src=\"../../pix/t/edit.gif\" border=\"0\" alt=\"$stredit\" /></a>\n";
-            }
+
+        if ($canedit) {
+            echo "<a title=\"$stredit\" href=\"question.php?id=$qnum\">
+                  <img src=\"../../pix/t/edit.gif\" border=\"0\" alt=\"$stredit\" /></a>&nbsp;";
+            echo "<a title=\"$strdelete\" href=\"edit.php?delete=$qnum&amp;sesskey=$USER->sesskey\">
+                  <img src=\"../../pix/t/delete.gif\" border=\"0\" alt=\"$strdelete\" /></a>&nbsp;";
+            echo "<a title=\"$strpreview\" href=\"#\" onClick=\"openpopup('/mod/quiz/preview.php?id=$qnum','$strpreview','scrollbars=yes,resizable=yes,width=700,height=480', false)\">
+                  <img src=\"../../pix/t/preview.gif\" border=\"0\" alt=\"$strpreview\" /></a>";
+        }
         echo "</td>";
 
         $sumgrade += $grades[$qnum];
@@ -1207,7 +1208,7 @@ function quiz_print_question_list($questionlist, $grades) {
 function quiz_print_cat_question_list($categoryid, $quizselected=true, $recurse=1, $page, $perpage) {
 // Prints the table of questions in a category with interactions
 
-    global $THEME, $QUIZ_QUESTION_TYPE;
+    global $THEME, $QUIZ_QUESTION_TYPE, $USER;
 
     $strcategory = get_string("category", "quiz");
     $strquestion = get_string("question", "quiz");
@@ -1290,6 +1291,7 @@ function quiz_print_cat_question_list($categoryid, $quizselected=true, $recurse=
     $canedit = isteacheredit($category->course);
 
     echo "<form method=\"post\" action=\"edit.php\">";
+    echo "<input type=\"hidden\" name=\"sesskey\" value=\"$USER->sesskey\">";
     echo "<table border=\"0\" cellpadding=\"5\" cellspacing=\"2\" width=\"100%\">";
     echo "<tr>";
     if ($quizselected) {