From 766df8f72b64a573d3dee3da688df0fc93f66476 Mon Sep 17 00:00:00 2001 From: tjhunt Date: Thu, 18 Sep 2008 07:39:10 +0000 Subject: [PATCH] MDL-16334 Convert mod/quiz/comment.php to use attemptlib.php - final part of the work on the quiz navigtion, I hope. MDL-16559 Remove question/comment.html template, and instead just have a function in questionlib.php MDL-16562 Regression from MDL-16263, notices when regrading. Sorry, the three got tangled together (coupled through questionlib) while I was fixing bugs in preparation for a demo. --- lang/en_utf8/question.php | 1 + lib/questionlib.php | 71 ++++++++++++++++----------- mod/quiz/attemptlib.php | 36 ++++++++++++++ mod/quiz/comment.php | 79 +++++++++++++----------------- mod/quiz/report/grading/report.php | 26 +++++----- mod/quiz/reviewquestion.php | 7 +-- question/comment.html | 25 ---------- 7 files changed, 131 insertions(+), 114 deletions(-) delete mode 100644 question/comment.html diff --git a/lang/en_utf8/question.php b/lang/en_utf8/question.php index f70aa22d8f..cd32237365 100644 --- a/lang/en_utf8/question.php +++ b/lang/en_utf8/question.php @@ -74,6 +74,7 @@ $string['errordeletingquestionsfromcategory'] = 'Error deleting questions from c $string['errorduringpre'] = 'Error occurred during pre-processing!'; $string['errorduringproc'] = 'Error occurred during processing!'; $string['errorduringpost'] = 'Error occurred during post-processing!'; +$string['errorduringregrade'] = 'Could not regrade question $a->qid, going to state $a->stateid.'; $string['errorfilecannotbecopied'] = 'Error cannot copy file $a.'; $string['errorfilecannotbemoved'] = 'Error cannot move file $a.'; $string['errorfileschanged'] = 'Error files linked to from questions have changed since form was displayed.'; diff --git a/lib/questionlib.php b/lib/questionlib.php index 269440826a..0d7a055fd1 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -895,6 +895,8 @@ function get_question_options(&$questions) { */ function question_preload_states($attemptid) { global $DB; + // Note, changes here probably also need to be reflected in + // regrade_question_in_attempt and question_load_specific_state. // The questionid field must be listed first so that it is used as the // array index in the array returned by $DB->get_records_sql @@ -1082,7 +1084,8 @@ function get_question_states(&$questions, $cmoptions, $attempt, $lastattemptid = function question_load_specific_state($question, $cmoptions, $attempt, $stateid) { global $DB, $QUESTION_EVENTS_GRADED; // Load specified states for the question. - $sql = 'SELECT st.*, sess.sumpenalty, sess.manualcomment + // sess.sumpenalty is probably wrong here shoul really be a sum of penalties from before the one we are asking for. + $sql = 'SELECT st.*, sess.sumpenalty, sess.manualcomment, sess.flagged, sess.id as questionsessionid FROM {question_states} st, {question_sessions} sess WHERE st.id = ? AND st.attempt = ? @@ -1097,7 +1100,7 @@ function question_load_specific_state($question, $cmoptions, $attempt, $stateid) // Load the most recent graded states for the questions before the specified one. list($eventinsql, $params) = $DB->get_in_or_equal($QUESTION_EVENTS_GRADED); - $sql = 'SELECT st.*, sess.sumpenalty, sess.manualcomment + $sql = 'SELECT st.*, sess.sumpenalty, sess.manualcomment, sess.flagged, sess.id as questionsessionid FROM {question_states} st, {question_sessions} sess WHERE st.seq_number <= ? AND st.attempt = ? @@ -1381,13 +1384,9 @@ function regrade_question_in_attempt($question, $attempt, $cmoptions, $verbose=f $attempt->sumgrades -= $states[count($states)-1]->grade; // Initialise the replaystate - $state = clone($states[0]); - $state->manualcomment = $DB->get_field('question_sessions', 'manualcomment', - array('attemptid'=> $attempt->uniqueid, 'questionid'=>$question->id)); - restore_question_state($question, $state); - $state->sumpenalty = 0.0; - $replaystate = clone($state); - $replaystate->last_graded = $state; + $replaystate = question_load_specific_state($question, $cmoptions, $attempt, $states[0]->id); + $replaystate->sumpenalty = 0; + $replaystate->last_graded->sumpenalty = 0; $changed = false; for($j = 1; $j < count($states); $j++) { @@ -1397,9 +1396,8 @@ function regrade_question_in_attempt($question, $attempt, $cmoptions, $verbose=f $action->timestamp = $states[$j]->timestamp; // Change event to submit so that it will be reprocessed - if (QUESTION_EVENTCLOSE == $states[$j]->event - or QUESTION_EVENTGRADE == $states[$j]->event - or QUESTION_EVENTCLOSEANDGRADE == $states[$j]->event) { + if (in_array($states[$j]->event, array(QUESTION_EVENTCLOSE, + QUESTION_EVENTGRADE, QUESTION_EVENTCLOSEANDGRADE))) { $action->event = QUESTION_EVENTSUBMIT; // By default take the event that was saved in the database @@ -1431,8 +1429,11 @@ function regrade_question_in_attempt($question, $attempt, $cmoptions, $verbose=f } else { // Reprocess (regrade) responses if (!question_process_responses($question, $replaystate, - $action, $cmoptions, $attempt)) { - $verbose && notify("Couldn't regrade state #{$state->id}!"); + $action, $cmoptions, $attempt) && $verbose) { + $a = new stdClass; + $a->qid = $question->id; + $a->stateid = $states[$j]->id; + notify(get_string('errorduringregrade', 'question', $a)); } // We need rounding here because grades in the DB get truncated // e.g. 0.33333 != 0.3333333, but we want them to be equal here @@ -1441,6 +1442,13 @@ function regrade_question_in_attempt($question, $attempt, $cmoptions, $verbose=f or (round((float)$replaystate->grade, 5) != round((float)$states[$j]->grade, 5))) { $changed = true; } + // If this was previously a closed state, and it has been knoced back to + // graded, then fix up the state again. + if ($replaystate->event == QUESTION_EVENTGRADE && + ($states[$j]->event == QUESTION_EVENTCLOSE || + $states[$j]->event == QUESTION_EVENTCLOSEANDGRADE)) { + $replaystate->event = $states[$j]->event; + } } $replaystate->id = $states[$j]->id; @@ -1731,19 +1739,26 @@ function get_question_image($question) { return $img; } -function question_print_comment_box($question, $state, $attempt, $url) { - global $CFG; - - $prefix = 'response'; - $usehtmleditor = can_use_html_editor(); - $grade = round($state->last_graded->grade, 3); - echo '
'; - include($CFG->dirroot.'/question/comment.html'); - echo ''; - echo ''; - echo ''; - echo ''; - echo '
'; +function question_print_comment_fields($question, $state, $prefix, $cmoptions) { + $idprefix = preg_replace('/[^-_a-zA-Z0-9]/', '', $prefix); + $grade = question_format_grade($cmoptions, $state->last_graded->grade); + $maxgrade = question_format_grade($cmoptions, $question->maxgrade); + $fieldsize = strlen($maxgrade) - 1; + echo '' . "\n"; + echo '' . "\n"; + echo '\n"; + echo '\n"; + echo "\n"; + echo '' . "\n"; + echo '\n"; + echo '\n"; + echo "\n"; + echo "
'; + print_textarea(can_use_html_editor(), 15, 60, 630, 300, $prefix . '[comment]', + $state->manualcomment, 0, false, $idprefix . '_comment_box'); + echo "
/' . $maxgrade . "
\n"; } /** @@ -1788,7 +1803,7 @@ function question_process_comment($question, &$state, &$attempt, $comment, $grad } // Update the state if either the score has changed, or this is the first - // manual grade event and there is actually a grade of comment to process. + // manual grade event and there is actually a grade or comment to process. // We don't need to store the modified state in the database, we just need // to set the $state->changed flag. if (abs($state->last_graded->grade - $grade) > 0.002 || diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 71aa8aed2b..8c553a0fdb 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -447,6 +447,11 @@ class quiz_attempt extends quiz { return $this->attempt->id; } + /** @return integer the attempt unique id. */ + public function get_uniqueid() { + return $this->attempt->uniqueid; + } + /** @return object the row from the quiz_attempts table. */ public function get_attempt() { return $this->attempt; @@ -691,6 +696,37 @@ class quiz_attempt extends quiz { return implode(', ', $attemptlist); } + // Methods for processing manual comments ============================================== + // I am not sure it is a good idea to have update methods here - this class is only + // about getting data out of the question engine, and helping to display it, apart from + // this. + public function process_comment($questionid, $comment, $grade) { + $this->ensure_question_loaded($questionid); + $this->ensure_state_loaded($questionid); + $state = $this->states[$questionid]; + + $error = question_process_comment($this->questions[$questionid], + $state, $this->attempt, $comment, $grade); + + // If the state was update (successfully), save the changes. + if (!is_string($error) && $state->changed) { + if (!save_question_session($this->questions[$questionid], $state)) { + $error = get_string('errorudpatingquestionsession', 'quiz'); + } + if (!quiz_save_best_grade($this->quiz, $this->attempt->userid)) { + $error = get_string('errorudpatingbestgrade', 'quiz'); + } + } + return $error; + } + + public function question_print_comment_fields($questionid, $prefix) { + $this->ensure_question_loaded($questionid); + $this->ensure_state_loaded($questionid); + question_print_comment_fields($this->questions[$questionid], + $this->states[$questionid], $prefix, $this->quiz); + } + // Private methods ===================================================================== // Check that the state of a particular question is loaded, and if not throw an exception. private function ensure_state_loaded($id) { diff --git a/mod/quiz/comment.php b/mod/quiz/comment.php index 0376123d7f..1a68477865 100644 --- a/mod/quiz/comment.php +++ b/mod/quiz/comment.php @@ -15,66 +15,55 @@ $attemptobj = new quiz_attempt($attemptid); - // Teachers are only allowed to comment and grade on closed attempts - if (!($attempt->timefinish > 0)) { +/// Can only grade finished attempts. + if (!$attemptobj->is_finished()) { print_error('attemptclosed', 'quiz'); } - $cm = get_coursemodule_from_instance('quiz', $quiz->id); - require_login($course, true, $cm); +/// Check login and permissions. + require_login($attemptobj->get_courseid(), false, $attemptobj->get_cm()); + $attemptobj->require_capability('mod/quiz:grade'); - $context = get_context_instance(CONTEXT_MODULE, $cm->id); +/// Load the questions and states. + $questionids = array($questionid); + $attemptobj->load_questions($questionids); + $attemptobj->load_question_states($questionids); - require_capability('mod/quiz:grade', $context); - - // Load question - if (! $question = $DB->get_record('question', array('id' => $questionid))) { - print_error('questionmissing', 'quiz'); - } - $question->maxgrade = $DB->get_field('quiz_question_instances', 'grade', array('quiz' => $quiz->id, 'question' => $question->id)); - // Some of the questions code is optimised to work with several questions - // at once so it wants the question to be in an array. - $key = $question->id; - $questions[$key] = &$question; - // Add additional questiontype specific information to the question objects. - if (!get_question_options($questions)) { - print_error('cannotloadtypeinfo', 'quiz'); - } - - // Load state - $states = get_question_states($questions, $quiz, $attempt); - // The $states array is indexed by question id but because we are dealing - // with only one question there is only one entry in this array - $state = &$states[$question->id]; +/// Log this action. + add_to_log($attemptobj->get_courseid(), 'quiz', 'manualgrade', 'comment.php?attempt=' . + $attemptobj->get_attemptid() . '&question=' . $questionid, + $attemptobj->get_quizid(), $attemptobj->get_cmid()); +/// Print the page header print_header(); - print_heading(format_string($question->name)); - - //add_to_log($course->id, 'quiz', 'review', "review.php?id=$cm->id&attempt=$attempt->id", "$quiz->id", "$cm->id"); + print_heading(format_string($attemptobj->get_question($questionid)->name)); +/// Process any data that was submitted. if ($data = data_submitted() and confirm_sesskey()) { - // the following will update the state and attempt - $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 '
"; + $error = $attemptobj->process_comment($questionid, + $data->response['comment'], $data->response['grade']); + /// If success, notify and print a close button. + if (!is_string($error)) { + notify(get_string('changessaved'), 'notifysuccess'); + close_window_button('closewindow', false, true); print_footer(); exit; } + + /// Otherwise, display the error and fall throug to re-display the form. + notify($error); } - question_print_comment_box($question, $state, $attempt, $CFG->wwwroot.'/mod/quiz/comment.php'); +/// Print the comment form. + echo '
'; + $attemptobj->question_print_comment_fields($questionid, 'response'); + echo ''; + echo ''; + echo ''; + echo ''; + echo '
'; +/// End of the page. print_footer(); - ?> diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index 2359de6eaf..1b98749895 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -389,29 +389,33 @@ class quiz_grading_report extends quiz_default_report { $options = quiz_get_reviewoptions($quiz, $attempt, $context); unset($options->questioncommentlink); - $copy = $state->manualcomment; - $state->manualcomment = ''; - $options->readonly = 1; - $gradedclass = question_state_is_graded($state)?' class="highlightgraded" ':''; - $gradedstring = question_state_is_graded($state)?(' '.get_string('graded','quiz_grading')):''; + if (question_state_is_graded($state)) { + $gradedclass = ' class="highlightgraded" '; + $gradedstring = ' ' . get_string('graded','quiz_grading'); + } else { + $gradedclass = ''; + $gradedstring = ''; + } $a = new object(); $a->fullname = fullname($attempt, true); $a->attempt = $attempt->attempt; // print the user name, attempt count, the question, and some more hidden fields echo '
'; - echo "".get_string('gradingattempt','quiz_grading', $a); - echo $gradedstring.""; + echo "" . get_string('gradingattempt', 'quiz_grading', $a); + echo $gradedstring . ''; + // Print the question, without showing any previous comment. + $copy = $state->manualcomment; + $state->manualcomment = ''; print_question($question, $state, '', $quiz, $options); - $prefix = "manualgrades[$attempt->uniqueid]"; - $grade = round($state->last_graded->grade, 3); + // The print the comment and grade fields, putting back the previous comment. $state->manualcomment = $copy; - - include($CFG->dirroot . '/question/comment.html'); + question_print_comment_fields($question, $state, + 'manualgrades[' . $attempt->uniqueid . ']', $quiz); echo '
'; } diff --git a/mod/quiz/reviewquestion.php b/mod/quiz/reviewquestion.php index c32fbc05ee..12b5c07daa 100644 --- a/mod/quiz/reviewquestion.php +++ b/mod/quiz/reviewquestion.php @@ -20,10 +20,6 @@ /// Check login. require_login($attemptobj->get_courseid(), false, $attemptobj->get_cm()); -/// Create an object to manage all the other (non-roles) access rules. - $accessmanager = $attemptobj->get_access_manager(time()); - $options = $attemptobj->get_review_options(); - /// Permissions checks for normal users who do not have quiz:viewreports capability. if (!$attemptobj->has_capability('mod/quiz:viewreports')) { /// Can't review during the attempt - send them back to the attempt page. @@ -38,7 +34,8 @@ } /// Can't review unless Students may review -> Responses option is turned on. if (!$options->responses) { - notify($accessmanager->cannot_review_message($options)); + $accessmanager = $attemptobj->get_access_manager(time()); + notify($accessmanager->cannot_review_message($attemptobj->get_review_options())); close_window_button(); } } diff --git a/question/comment.html b/question/comment.html deleted file mode 100644 index 904efe397b..0000000000 --- a/question/comment.html +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - -
- : - - manualcomment); - ?> -
- : - - /maxgrade; ?> -
\ No newline at end of file -- 2.39.5