From 0d1fccaf15c4d3b8b0f251bdeb57f943f2c657ce Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 4 Mar 2013 16:41:22 +0000 Subject: [PATCH] MDL-38311 questions: manual grading API should accept commentformat Comment format (FORMAT_...) was correctly being processed when the manual grading happened as the result of a form submission. It was only when done using the question_usage or question_attempt API method that there was no way to specify the format. (Although I think the only place this API as used was in the unit tests.) Note that question_attempt::manual_grade API had to change, but I don't think that is a real API change. Calling code should be using question_usage::question_attempt, which is backwards compatible. Note that now, if you don't pass format, then no error is generated, but a developer debugging message is generated. --- .../adaptive/tests/walkthrough_test.php | 2 +- .../adaptivenopenalty/tests/walkthrough_test.php | 2 +- .../deferredcbm/tests/walkthrough_test.php | 2 +- .../deferredfeedback/tests/walkthrough_test.php | 2 +- .../immediatecbm/tests/walkthrough_test.php | 4 ++-- .../immediatefeedback/tests/walkthrough_test.php | 4 ++-- .../informationitem/tests/walkthrough_test.php | 4 ++-- .../interactive/tests/walkthrough_test.php | 2 +- .../manualgraded/tests/walkthrough_test.php | 16 ++++++++-------- question/engine/questionattempt.php | 13 +++++++++---- question/engine/questionusage.php | 4 ++-- question/engine/tests/helpers.php | 4 ++-- question/engine/tests/unitofwork_test.php | 2 +- 13 files changed, 33 insertions(+), 28 deletions(-) diff --git a/question/behaviour/adaptive/tests/walkthrough_test.php b/question/behaviour/adaptive/tests/walkthrough_test.php index b78e4706106..09ad153e93e 100644 --- a/question/behaviour/adaptive/tests/walkthrough_test.php +++ b/question/behaviour/adaptive/tests/walkthrough_test.php @@ -158,7 +158,7 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_correct_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/adaptivenopenalty/tests/walkthrough_test.php b/question/behaviour/adaptivenopenalty/tests/walkthrough_test.php index 546a5b5caa4..b217a591c2f 100644 --- a/question/behaviour/adaptivenopenalty/tests/walkthrough_test.php +++ b/question/behaviour/adaptivenopenalty/tests/walkthrough_test.php @@ -124,7 +124,7 @@ class qbehaviour_adaptivenopenalty_walkthrough_test extends qbehaviour_walkthrou $this->get_contains_correct_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/deferredcbm/tests/walkthrough_test.php b/question/behaviour/deferredcbm/tests/walkthrough_test.php index 852b0680d51..4e4710eba26 100644 --- a/question/behaviour/deferredcbm/tests/walkthrough_test.php +++ b/question/behaviour/deferredcbm/tests/walkthrough_test.php @@ -95,7 +95,7 @@ class qbehaviour_deferredcbm_walkthrough_test extends qbehaviour_walkthrough_tes $this->get_contains_correct_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/deferredfeedback/tests/walkthrough_test.php b/question/behaviour/deferredfeedback/tests/walkthrough_test.php index 80faf0c82ba..74eda1756bc 100644 --- a/question/behaviour/deferredfeedback/tests/walkthrough_test.php +++ b/question/behaviour/deferredfeedback/tests/walkthrough_test.php @@ -92,7 +92,7 @@ class qbehaviour_deferredfeedback_walkthrough_test extends qbehaviour_walkthroug $this->quba->get_response_summary($this->slot)); // Process a manual comment. - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); $this->check_current_state(question_state::$mangrpartial); $this->check_current_mark(1); diff --git a/question/behaviour/immediatecbm/tests/walkthrough_test.php b/question/behaviour/immediatecbm/tests/walkthrough_test.php index b0f3d9acb1f..d93b8a4cc38 100644 --- a/question/behaviour/immediatecbm/tests/walkthrough_test.php +++ b/question/behaviour/immediatecbm/tests/walkthrough_test.php @@ -109,7 +109,7 @@ class qbehaviour_immediatecbm_walkthrough_test extends qbehaviour_walkthrough_te $this->get_contains_correct_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 0.5); + $this->manual_grade('Not good enough!', 0.5, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); @@ -176,7 +176,7 @@ class qbehaviour_immediatecbm_walkthrough_test extends qbehaviour_walkthrough_te $this->get_contains_mc_radio_expectation(2, false, false)); // Process a manual comment. - $this->manual_grade('Not good enough!', 0.5); + $this->manual_grade('Not good enough!', 0.5, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/immediatefeedback/tests/walkthrough_test.php b/question/behaviour/immediatefeedback/tests/walkthrough_test.php index ebea45ebe35..103650074a3 100644 --- a/question/behaviour/immediatefeedback/tests/walkthrough_test.php +++ b/question/behaviour/immediatefeedback/tests/walkthrough_test.php @@ -116,7 +116,7 @@ class qbehaviour_immediatefeedback_walkthrough_test extends qbehaviour_walkthrou $this->get_contains_correct_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 0.5); + $this->manual_grade('Not good enough!', 0.5, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); @@ -185,7 +185,7 @@ class qbehaviour_immediatefeedback_walkthrough_test extends qbehaviour_walkthrou $this->get_contains_mc_radio_expectation(2, false, false)); // Process a manual comment. - $this->manual_grade('Not good enough!', 0.5); + $this->manual_grade('Not good enough!', 0.5, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/informationitem/tests/walkthrough_test.php b/question/behaviour/informationitem/tests/walkthrough_test.php index d2b4744861a..94c6b65838e 100644 --- a/question/behaviour/informationitem/tests/walkthrough_test.php +++ b/question/behaviour/informationitem/tests/walkthrough_test.php @@ -74,7 +74,7 @@ class qbehaviour_informationitem_walkthrough_test extends qbehaviour_walkthrough $this->get_contains_general_feedback_expectation($description)); // Process a manual comment. - $this->manual_grade('Not good enough!', null); + $this->manual_grade('Not good enough!', null, FORMAT_HTML); $this->check_current_state(question_state::$manfinished); $this->check_current_mark(null); @@ -83,6 +83,6 @@ class qbehaviour_informationitem_walkthrough_test extends qbehaviour_walkthrough // Check that trying to process a manual comment with a grade causes an exception. $this->setExpectedException('moodle_exception'); - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); } } diff --git a/question/behaviour/interactive/tests/walkthrough_test.php b/question/behaviour/interactive/tests/walkthrough_test.php index 4af3490607b..7dbcc67cf02 100644 --- a/question/behaviour/interactive/tests/walkthrough_test.php +++ b/question/behaviour/interactive/tests/walkthrough_test.php @@ -157,7 +157,7 @@ class qbehaviour_interactive_walkthrough_test extends qbehaviour_walkthrough_tes $this->get_no_hint_visible_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 0.5); + $this->manual_grade('Not good enough!', 0.5, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrpartial); diff --git a/question/behaviour/manualgraded/tests/walkthrough_test.php b/question/behaviour/manualgraded/tests/walkthrough_test.php index 32af7296692..cb7c1ce099c 100644 --- a/question/behaviour/manualgraded/tests/walkthrough_test.php +++ b/question/behaviour/manualgraded/tests/walkthrough_test.php @@ -90,7 +90,7 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te $this->quba->get_response_summary($this->slot)); // Process a manual comment. - $this->manual_grade('Not good enough!', 10); + $this->manual_grade('Not good enough!', 10, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrright); @@ -140,7 +140,7 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te $this->get_does_not_contain_specific_feedback_expectation()); // Process a manual comment. - $this->manual_grade('Not good enough!', 1); + $this->manual_grade('Not good enough!', 1, FORMAT_HTML); $this->check_current_state(question_state::$mangrpartial); $this->check_current_mark(1); @@ -181,14 +181,14 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te // Process a blank manual comment. Ensure it does not change the state. $numsteps = $this->get_step_count(); - $this->manual_grade('', ''); + $this->manual_grade('', '', FORMAT_HTML); $this->check_step_count($numsteps); $this->check_current_state(question_state::$needsgrading); $this->check_current_mark(null); // Process a comment, but with the mark blank. Should be recorded, but // not change the mark. - $this->manual_grade('I am not sure what grade to award.', ''); + $this->manual_grade('I am not sure what grade to award.', '', FORMAT_HTML); $this->check_step_count($numsteps + 1); $this->check_current_state(question_state::$needsgrading); $this->check_current_mark(null); @@ -197,7 +197,7 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te preg_quote('I am not sure what grade to award.') . '/')); // Now grade it. - $this->manual_grade('Pretty good!', '9.00000'); + $this->manual_grade('Pretty good!', '9.00000', FORMAT_HTML); $this->check_step_count($numsteps + 2); $this->check_current_state(question_state::$mangrpartial); $this->check_current_mark(9); @@ -205,13 +205,13 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te new question_pattern_expectation('/' . preg_quote('Pretty good!') . '/')); // Process the same data again, and make sure it does not add a step. - $this->manual_grade('Pretty good!', '9.00000'); + $this->manual_grade('Pretty good!', '9.00000', FORMAT_HTML); $this->check_step_count($numsteps + 2); $this->check_current_state(question_state::$mangrpartial); $this->check_current_mark(9); // Now set the mark back to blank. - $this->manual_grade('Actually, I am not sure any more.', ''); + $this->manual_grade('Actually, I am not sure any more.', '', FORMAT_HTML); $this->check_step_count($numsteps + 3); $this->check_current_state(question_state::$needsgrading); $this->check_current_mark(null); @@ -261,7 +261,7 @@ class qbehaviour_manualgraded_walkthrough_test extends qbehaviour_walkthrough_te $this->quba->get_response_summary($this->slot)); // Process a blank comment and a grade of 0. - $this->manual_grade('', 0); + $this->manual_grade('', 0, FORMAT_HTML); // Verify. $this->check_current_state(question_state::$mangrwrong); diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 1aa334aedeb..1c19f17ea9f 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -1114,14 +1114,19 @@ class question_attempt { /** * Perform a manual grading action on this attempt. - * @param $comment the comment being added. - * @param $mark the new mark. (Optional, if not given, then only a comment is added.) + * @param string $comment the comment being added. + * @param float $mark the new mark. If null, then only a comment is added. + * @param int $commentformat the FORMAT_... for $comment. Must be given. * @param int $timestamp the time to record for the action. (If not given, use now.) * @param int $userid the user to attribute the aciton to. (If not given, use the current user.) - * @return unknown_type */ - public function manual_grade($comment, $mark, $timestamp = null, $userid = null) { + public function manual_grade($comment, $mark, $commentformat = null, $timestamp = null, $userid = null) { $submitteddata = array('-comment' => $comment); + if (is_null($commentformat)) { + debugging('You should pass $commentformat to manual_grade.', DEBUG_DEVELOPER); + $commentformat = FORMAT_HTML; + } + $submitteddata['-commentformat'] = $commentformat; if (!is_null($mark)) { $submitteddata['-mark'] = $mark; $submitteddata['-maxmark'] = $this->maxmark; diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index e4de1979502..48150862719 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -647,9 +647,9 @@ class question_usage_by_activity { * @param number $mark the mark that is being assigned. Can be null to just * add a comment. */ - public function manual_grade($slot, $comment, $mark) { + public function manual_grade($slot, $comment, $mark, $commentformat = null) { $qa = $this->get_question_attempt($slot); - $qa->manual_grade($comment, $mark); + $qa->manual_grade($comment, $mark, $commentformat); $this->observer->notify_attempt_modified($qa); } diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 703ee8d9065..1a9605b9269 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -653,8 +653,8 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { $this->quba->process_action($this->slot, $data); } - protected function manual_grade($comment, $mark) { - $this->quba->manual_grade($this->slot, $comment, $mark); + protected function manual_grade($comment, $mark, $commentformat = null) { + $this->quba->manual_grade($this->slot, $comment, $mark, $commentformat); } protected function check_current_state($state) { diff --git a/question/engine/tests/unitofwork_test.php b/question/engine/tests/unitofwork_test.php index aa68a570ffa..f83de5ca6e4 100644 --- a/question/engine/tests/unitofwork_test.php +++ b/question/engine/tests/unitofwork_test.php @@ -174,7 +174,7 @@ class question_engine_unit_of_work_test extends data_loading_method_test_base { public function test_process_action() { - $this->quba->manual_grade($this->slot, 'Acutally, that is not quite right', 0.5); + $this->quba->manual_grade($this->slot, 'Acutally, that is not quite right', 0.5, FORMAT_HTML); // Here, however, were we are adding a step to an existing qa, we do need to track that. $this->assertEquals(0, count($this->observer->get_attempts_added()));