Skip to content

GitLab

Commit 9a7fa271 authored by David Monllaó's avatar David Monllaó
Browse files

Merge branch 'MDL-51090_28' of git://github.com/timhunt/moodle into MOODLE_28_STABLE

parents d772d11a 5c97a8da
Hide whitespace changes
Inline Side-by-side
lang/en/question.php
View file @ 9a7fa271
......@@ -195,6 +195,7 @@ $string['lasttry'] = 'Last try';
$string['linkedfiledoesntexist'] = 'Linked file {$a} doesn\'t exist';
$string['makechildof'] = 'Make child of \'{$a}\'';
$string['maketoplevelitem'] = 'Move to top level';
$string['manualgradeinvalidformat'] = 'That is not a valid number.';
$string['matchgrades'] = 'Match grades';
$string['matchgradeserror'] = 'Error if grade not listed';
$string['matchgradesnearest'] = 'Nearest grade if not listed';
......
mod/quiz/tests/behat/manually_mark_question.feature 0 → 100644
View file @ 9a7fa271
@mod @mod_quiz
Feature: Teachers can override the grade for any question
As a teacher
In order to correct errors
I must be able to override the grades that Moodle gives.
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| student1 | Student | 1 | student0@example.com |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| student1 | C1 | student |
And the following "question categories" exist:
| contextlevel | reference | name |
| Course | C1 | Test questions |
And the following "questions" exist:
| questioncategory | qtype | name | questiontext | defaultmark |
| Test questions | essay | TF1 | First question | 20 |
And the following "activities" exist:
| activity | name | intro | course | idnumber | grade |
| quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | 20 |
And quiz "Quiz 1" contains the following questions:
| question | page |
| TF1 | 1 |
And I log in as "student1"
And I follow "Course 1"
And I follow "Quiz 1"
And I press "Attempt quiz now"
And I follow "Finish attempt ..."
And I press "Submit all and finish"
And I click on "Submit all and finish" "button" in the "Confirmation" "dialogue"
And I log out
And I log in as "teacher1"
And I follow "Course 1"
And I follow "Quiz 1"
And I follow "Attempts: 1"
And I follow "Review attempt"
@javascript
Scenario: Validating the marking of an essay question attempt.
When I follow "Make comment or override mark"
And I switch to "commentquestion" window
And I set the field "Mark" to "25"
And I press "Save"
Then I should see "This grade is outside the valid range."
And I set the field "Mark" to "aa"
And I press "Save"
And I should see "That is not a valid number."
And I set the field "Mark" to "10.0"
And I press "Save"
And I should see "Changes saved"
question/behaviour/behaviourbase.php
View file @ 9a7fa271
......@@ -194,7 +194,7 @@ abstract class question_behaviour {
$vars = array('comment' => PARAM_RAW, 'commentformat' => PARAM_INT);
if ($this->qa->get_max_mark()) {
$vars['mark'] = question_attempt::PARAM_MARK;
$vars['mark'] = PARAM_RAW_TRIMMED;
$vars['maxmark'] = PARAM_FLOAT;
}
return $vars;
......@@ -467,15 +467,25 @@ abstract class question_behaviour {
}
if ($pendingstep->has_behaviour_var('mark')) {
$fraction = $pendingstep->get_behaviour_var('mark') /
$pendingstep->get_behaviour_var('maxmark');
if ($pendingstep->get_behaviour_var('mark') === '') {
$mark = question_utils::clean_param_mark($pendingstep->get_behaviour_var('mark'));
if ($mark === null) {
throw new coding_exception('Inalid number format ' . $pendingstep->get_behaviour_var('mark') .
' when processing a manual grading action.', 'Question ' . $this->question->id .
', slot ' . $this->qa->get_slot());
} else if ($mark === '') {
$fraction = null;
} else if ($fraction > $this->qa->get_max_fraction() || $fraction < $this->qa->get_min_fraction()) {
throw new coding_exception('Score out of range when processing ' .
'a manual grading action.', 'Question ' . $this->question->id .
', slot ' . $this->qa->get_slot() . ', fraction ' . $fraction);
} else {
$fraction = $pendingstep->get_behaviour_var('mark') /
$pendingstep->get_behaviour_var('maxmark');
if ($fraction > $this->qa->get_max_fraction() || $fraction < $this->qa->get_min_fraction()) {
throw new coding_exception('Score out of range when processing ' .
'a manual grading action.', 'Question ' . $this->question->id .
', slot ' . $this->qa->get_slot() . ', fraction ' . $fraction);
}
}
$pendingstep->set_fraction($fraction);
}
......
question/behaviour/manualgraded/tests/walkthrough_test.php
View file @ 9a7fa271
......@@ -384,4 +384,98 @@ class qbehaviour_manualgraded_walkthrough_testcase extends qbehaviour_walkthroug
$this->displayoptions->manualcomment = question_display_options::VISIBLE;
$this->check_output_contains('This should only appear if the displya options allow it');
}
public function test_manual_graded_invalid_value_throws_exception() {
global $PAGE;
// The current text editor depends on the users profile setting - so it needs a valid user.
$this->setAdminUser();
// Required to init a text editor.
$PAGE->set_url('/');
// Create an essay question.
$essay = test_question_maker::make_an_essay_question();
$this->start_attempt_at_question($essay, 'deferredfeedback', 10);
// Check the right model is being used.
$this->assertEquals('manualgraded', $this->quba->get_question_attempt(
$this->slot)->get_behaviour_name());
// Check the initial state.
$this->check_current_state(question_state::$todo);
$this->check_current_mark(null);
$this->check_current_output($this->get_contains_question_text_expectation($essay),
$this->get_does_not_contain_feedback_expectation());
// Simulate some data submitted by the student.
$this->process_submission(array('answer' => 'This is my wonderful essay!', 'answerformat' => FORMAT_HTML));
// Verify.
$this->check_current_state(question_state::$complete);
$this->check_current_mark(null);
$this->check_current_output(
new question_contains_tag_with_attribute('textarea', 'name',
$this->quba->get_question_attempt($this->slot)->get_qt_field_name('answer')),
$this->get_does_not_contain_feedback_expectation());
// Finish the attempt.
$this->quba->finish_all_questions();
// Verify.
$this->check_current_state(question_state::$needsgrading);
$this->check_current_mark(null);
$this->assertEquals('This is my wonderful essay!',
$this->quba->get_response_summary($this->slot));
// Try to process a an invalid grade.
$this->setExpectedException('coding_exception');
$this->manual_grade('Comment', 'frog', FORMAT_HTML);
}
public function test_manual_graded_out_of_range_throws_exception() {
global $PAGE;
// The current text editor depends on the users profile setting - so it needs a valid user.
$this->setAdminUser();
// Required to init a text editor.
$PAGE->set_url('/');
// Create an essay question.
$essay = test_question_maker::make_an_essay_question();
$this->start_attempt_at_question($essay, 'deferredfeedback', 10);
// Check the right model is being used.
$this->assertEquals('manualgraded', $this->quba->get_question_attempt(
$this->slot)->get_behaviour_name());
// Check the initial state.
$this->check_current_state(question_state::$todo);
$this->check_current_mark(null);
$this->check_current_output($this->get_contains_question_text_expectation($essay),
$this->get_does_not_contain_feedback_expectation());
// Simulate some data submitted by the student.
$this->process_submission(array('answer' => 'This is my wonderful essay!', 'answerformat' => FORMAT_HTML));
// Verify.
$this->check_current_state(question_state::$complete);
$this->check_current_mark(null);
$this->check_current_output(
new question_contains_tag_with_attribute('textarea', 'name',
$this->quba->get_question_attempt($this->slot)->get_qt_field_name('answer')),
$this->get_does_not_contain_feedback_expectation());
// Finish the attempt.
$this->quba->finish_all_questions();
// Verify.
$this->check_current_state(question_state::$needsgrading);
$this->check_current_mark(null);
$this->assertEquals('This is my wonderful essay!',
$this->quba->get_response_summary($this->slot));
// Try to process a an invalid grade.
$this->setExpectedException('coding_exception');
$this->manual_grade('Comment', '10.1', FORMAT_HTML);
}
}
question/behaviour/rendererbase.php
View file @ 9a7fa271
......@@ -125,8 +125,7 @@ abstract class qbehaviour_renderer extends plugin_renderer_base {
'id'=> $markfield
);
if (!is_null($currentmark)) {
$attributes['value'] = $qa->format_fraction_as_mark(
$currentmark / $maxmark, $options->markdp);
$attributes['value'] = $currentmark;
}
$a = new stdClass();
$a->max = $qa->format_max_mark($options->markdp);
......@@ -146,11 +145,11 @@ abstract class qbehaviour_renderer extends plugin_renderer_base {
'value' => $qa->get_max_fraction(),
));
$error = $qa->validate_manual_mark($currentmark);
$errorclass = '';
$error = '';
if ($currentmark > $maxmark * $qa->get_max_fraction() || $currentmark < $maxmark * $qa->get_min_fraction()) {
$errorclass = ' error';
$error = html_writer::tag('span', get_string('manualgradeoutofrange', 'question'),
if ($error !== '') {
$erroclass = ' error';
$error = html_writer::tag('span', $error,
array('class' => 'error')) . html_writer::empty_tag('br');
}
......
question/engine/lib.php
View file @ 9a7fa271
......@@ -143,7 +143,8 @@ abstract class question_engine {
$maxmark = optional_param($prefix . '-maxmark', null, PARAM_FLOAT);
$minfraction = optional_param($prefix . ':minfraction', null, PARAM_FLOAT);
$maxfraction = optional_param($prefix . ':maxfraction', null, PARAM_FLOAT);
return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxfraction * $maxmark);
return $mark === '' ||
($mark !== null && $mark >= $minfraction * $maxmark && $mark <= $maxfraction * $maxmark);
}
/**
......@@ -867,8 +868,9 @@ abstract class question_utils {
/**
* Typically, $mark will have come from optional_param($name, null, PARAM_RAW_TRIMMED).
* This method copes with:
* - keeping null or '' input unchanged.
* - nubmers that were typed as either 1.00 or 1,00 form.
* - keeping null or '' input unchanged - important to let teaches set a question back to requries grading.
* - numbers that were typed as either 1.00 or 1,00 form.
* - invalid things, which get turned into null.
*
* @param string|null $mark raw use input of a mark.
* @return float|string|null cleaned mark as a float if possible. Otherwise '' or null.
......@@ -878,7 +880,13 @@ abstract class question_utils {
return $mark;
}
return clean_param(str_replace(',', '.', $mark), PARAM_FLOAT);
$mark = str_replace(',', '.', $mark);
// This regexp should match the one in validate_param.
if (!preg_match('/^[\+-]?[0-9]*\.?[0-9]*(e[-+]?[0-9]+)?$/i', $mark)) {
return null;
}
return clean_param($mark, PARAM_FLOAT);
}
/**
......
question/engine/questionattempt.php
View file @ 9a7fa271
......@@ -49,10 +49,10 @@ class question_attempt {
const USE_RAW_DATA = 'use raw data';
/**
* @var string special value used by manual grading because {@link PARAM_FLOAT}
* converts '' to 0.
* @var string Should not longer be used.
* @deprecated since Moodle 3.0
*/
const PARAM_MARK = 'parammark';
const PARAM_MARK = PARAM_RAW_TRIMMED;
/**
* @var string special value to indicate a response variable that is uploaded
......@@ -651,13 +651,12 @@ class question_attempt {
* This is used by the manual grading code, particularly in association with
* validation. If there is a mark submitted in the request, then use that,
* otherwise use the latest mark for this question.
* @return number the current mark for this question.
* {@link get_fraction()} * {@link get_max_mark()}.
* @return number the current manual mark for this question, formatted for display.
*/
public function get_current_manual_mark() {
$mark = $this->get_submitted_var($this->get_behaviour_field_name('mark'), question_attempt::PARAM_MARK);
$mark = $this->get_submitted_var($this->get_behaviour_field_name('mark'), PARAM_RAW_TRIMMED);
if (is_null($mark)) {
return $this->get_mark();
return format_float($this->get_mark(), 7, true, true);
} else {
return $mark;
}
......@@ -1002,9 +1001,6 @@ class question_attempt {
*/
public function get_submitted_var($name, $type, $postdata = null) {
switch ($type) {
case self::PARAM_MARK:
// Special case to work around PARAM_FLOAT converting '' to 0.
return question_utils::clean_param_mark($this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata));
case self::PARAM_FILES:
return $this->process_response_files($name, $name, $postdata);
......@@ -1026,6 +1022,29 @@ class question_attempt {
}
}
/**
* Validate the manual mark for a question.
* @param unknown $currentmark the user input (e.g. '1,0', '1,0' or 'invalid'.
* @return string any errors with the value, or '' if it is OK.
*/
public function validate_manual_mark($currentmark) {
if ($currentmark === null || $currentmark === '') {
return '';
}
$mark = question_utils::clean_param_mark($currentmark);
if ($mark === null) {
return get_string('manualgradeinvalidformat', 'question');
}
$maxmark = $this->get_max_mark();
if ($mark > $maxmark * $this->get_max_fraction() || $mark < $maxmark * $this->get_min_fraction()) {
return get_string('manualgradeoutofrange', 'question');
}
return '';
}
/**
* Handle a submitted variable representing uploaded files.
* @param string $name the field name.
......
question/engine/tests/questionattempt_test.php
View file @ 9a7fa271
......@@ -109,36 +109,6 @@ class question_attempt_testcase extends advanced_testcase {
'reallyunlikelyvariablename', PARAM_BOOL));
}
public function test_get_submitted_var_param_mark_not_present() {
$this->assertNull($this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array()));
}
public function test_get_submitted_var_param_mark_blank() {
$this->assertSame('', $this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array('name' => '')));
}
public function test_get_submitted_var_param_mark_number() {
$this->assertSame(123.0, $this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array('name' => '123')));
}
public function test_get_submitted_var_param_mark_number_uk_decimal() {
$this->assertSame(123.45, $this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array('name' => '123.45')));
}
public function test_get_submitted_var_param_mark_number_eu_decimal() {
$this->assertSame(123.45, $this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array('name' => '123,45')));
}
public function test_get_submitted_var_param_mark_invalid() {
$this->assertSame(0.0, $this->qa->get_submitted_var(
'name', question_attempt::PARAM_MARK, array('name' => 'frog')));
}
public function test_get_all_submitted_qt_vars() {
$this->qa->set_usage_id('MDOgzdhS4W');
$this->qa->set_slot(1);
......
question/engine/tests/questionattempt_with_steps_test.php
View file @ 9a7fa271
......@@ -165,4 +165,20 @@ class question_attempt_with_steps_test extends advanced_testcase {
$this->setExpectedException('moodle_exception');
$qa->get_max_fraction();
}
public function test_validate_manual_mark() {
$this->qa->set_min_fraction(0);
$this->qa->set_max_fraction(1);
$this->assertSame('', $this->qa->validate_manual_mark(null));
$this->assertSame('', $this->qa->validate_manual_mark(''));
$this->assertSame('', $this->qa->validate_manual_mark('0'));
$this->assertSame('', $this->qa->validate_manual_mark('0.0'));
$this->assertSame('', $this->qa->validate_manual_mark('2,0'));
$this->assertSame(get_string('manualgradeinvalidformat', 'question'),
$this->qa->validate_manual_mark('frog'));
$this->assertSame(get_string('manualgradeoutofrange', 'question'),
$this->qa->validate_manual_mark('2.1'));
$this->assertSame(get_string('manualgradeoutofrange', 'question'),
$this->qa->validate_manual_mark('-0,01'));
}
}
question/engine/tests/questionutils_test.php
View file @ 9a7fa271
......@@ -194,6 +194,7 @@ class question_utils_test extends advanced_testcase {
public function test_clean_param_mark() {
$this->assertNull(question_utils::clean_param_mark(null));
$this->assertNull(question_utils::clean_param_mark('frog'));
$this->assertSame('', question_utils::clean_param_mark(''));
$this->assertSame(0.0, question_utils::clean_param_mark('0'));
$this->assertSame(1.5, question_utils::clean_param_mark('1.5'));
......
question/engine/upgrade.txt
View file @ 9a7fa271
This files describes API changes for the core question system.
=== 3.0, 2.9.2, 2.8.8 ===
1) The extra internal PARAM constant question_attempt::PARAM_MARK should no
longer be used. (It should not have been used outside the core of the
question system). See MDL-51090 if you want more explanation.
=== 2.6 ===
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment