MDL-42105 questions: let attempts have a max fraction > 1.

This parallels question_attempt->minfraction, which allows the fractional mark to go below zere. This is needed to allow the certainty-base marking behaviours to work better.
NorthernMichU · Oct 4, 2013 · 4e3d829 · 4e3d829
1 parent 474aa12
commit 4e3d829
Show file tree

Hide file tree

Showing 38 changed files with 251 additions and 65 deletions.
diff --git a/admin/tool/qeupgradehelper/locallib.php b/admin/tool/qeupgradehelper/locallib.php
@@ -572,6 +572,7 @@ public function test_{$question->qtype}_{$quiz->preferredbehaviour}_{$namesuffix
             'variant' => 1,
             'maxmark' => {$question->maxmark},
             'minfraction' => 0,
+            'maxfraction' => 1,
             'flagged' => 0,
             'questionsummary' => '',
             'rightanswer' => '',

diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php
@@ -209,7 +209,7 @@ protected function add_question_usages($element, $usageidname, $nameprefix = '')
 
         $qas = new backup_nested_element($nameprefix . 'question_attempts');
         $qa = new backup_nested_element($nameprefix . 'question_attempt', array('id'), array(
-                'slot', 'behaviour', 'questionid', 'maxmark', 'minfraction',
+                'slot', 'behaviour', 'questionid', 'maxmark', 'minfraction', 'maxfraction',
                 'flagged', 'questionsummary', 'rightanswer', 'responsesummary',
                 'timemodified'));
 

diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php
@@ -4047,6 +4047,10 @@ protected function restore_question_attempt_worker($data, $nameprefix) {
         $data->questionid      = $question->newitemid;
         $data->timemodified    = $this->apply_date_offset($data->timemodified);
 
+        if (!property_exists($data, 'maxfraction')) {
+            $data->maxfraction = 1;
+        }
+
         $newitemid = $DB->insert_record('question_attempts', $data);
 
         $this->set_mapping($nameprefix . 'question_attempt', $oldid, $newitemid);

diff --git a/lang/en/question.php b/lang/en/question.php
@@ -421,6 +421,7 @@
 $string['technicalinfo'] = 'Technical information';
 $string['technicalinfo_help'] = 'This technical information is probably only useful for developers working on new question types. It may also be helpful when trying to diagnose problems with questions.';
 $string['technicalinfominfraction'] = 'Minimum fraction: {$a}';
+$string['technicalinfomaxfraction'] = 'Maximum fraction: {$a}';
 $string['technicalinfoquestionsummary'] = 'Question summary: {$a}';
 $string['technicalinforightsummary'] = 'Right answer summary: {$a}';
 $string['technicalinfostate'] = 'Question state: {$a}';

diff --git a/lib/db/install.xml b/lib/db/install.xml
@@ -1392,6 +1392,7 @@
         <FIELD NAME="variant" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="1" SEQUENCE="false" COMMENT="The variant of the qusetion being used."/>
         <FIELD NAME="maxmark" TYPE="number" LENGTH="12" NOTNULL="true" SEQUENCE="false" DECIMALS="7" COMMENT="The grade this question is marked out of in this attempt."/>
         <FIELD NAME="minfraction" TYPE="number" LENGTH="12" NOTNULL="true" SEQUENCE="false" DECIMALS="7" COMMENT="Some questions can award negative marks. This indicates the most negative mark that can be awarded, on the faction scale where the maximum positive mark is 1."/>
+        <FIELD NAME="maxfraction" TYPE="number" LENGTH="12" NOTNULL="true" DEFAULT="1" SEQUENCE="false" DECIMALS="7" COMMENT="Some questions can give fractions greater than 1. This indicates the greatest fraction that can be awarded."/>
         <FIELD NAME="flagged" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Whether this question has been flagged within the attempt."/>
         <FIELD NAME="questionsummary" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="If this question uses randomisation, it should set this field to summarise what random version the student actually saw. This is a human-readable textual summary of the student's response which might, for example, be used in a report."/>
         <FIELD NAME="rightanswer" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="This is a human-readable textual summary of the right answer to this question. Might be used, for example on the quiz preview, to help people who are testing the question. Or might be used in reports."/>

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -2576,5 +2576,20 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2013092700.01);
     }
 
+    if ($oldversion < 2013100400.01) {
+
+        // Define field maxfraction to be added to question_attempts.
+        $table = new xmldb_table('question_attempts');
+        $field = new xmldb_field('maxfraction', XMLDB_TYPE_NUMBER, '12, 7', null, XMLDB_NOTNULL, null, '1', 'minfraction');
+
+        // Conditionally launch add field maxfraction.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2013100400.01);
+    }
+
     return true;
 }
diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php
@@ -160,15 +160,27 @@ public function get_applicable_hint() {
     /**
      * What is the minimum fraction that can be scored for this question.
      * Normally this will be based on $this->question->get_min_fraction(),
-     * but may be modified in some way by the model.
+     * but may be modified in some way by the behaviour.
      *
      * @return number the minimum fraction when this question is attempted under
-     * this model.
+     * this behaviour.
      */
     public function get_min_fraction() {
         return 0;
     }
 
+    /**
+     * Return the maximum possible fraction that can be scored for this question.
+     * Normally this will be based on $this->question->get_max_fraction(),
+     * but may be modified in some way by the behaviour.
+     *
+     * @return number the maximum fraction when this question is attempted under
+     * this behaviour.
+     */
+    public function get_max_fraction() {
+        return $this->question->get_max_fraction();
+    }
+
     /**
      * Return an array of the behaviour variables that could be submitted
      * as part of a question of this type, with their types, so they can be
@@ -432,7 +444,7 @@ public function process_comment(question_attempt_pending_step $pendingstep) {
                             $pendingstep->get_behaviour_var('maxmark');
             if ($pendingstep->get_behaviour_var('mark') === '') {
                 $fraction = null;
-            } else if ($fraction > 1 || $fraction < $this->qa->get_min_fraction()) {
+            } 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);

diff --git a/question/behaviour/deferredcbm/behaviour.php b/question/behaviour/deferredcbm/behaviour.php
@@ -49,6 +49,10 @@ public function get_min_fraction() {
         return question_cbm::adjust_fraction(parent::get_min_fraction(), question_cbm::HIGH);
     }
 
+    public function get_max_fraction() {
+        return question_cbm::adjust_fraction(parent::get_max_fraction(), question_cbm::HIGH);
+    }
+
     public function get_expected_data() {
         if ($this->qa->get_state()->is_active()) {
             return array('certainty' => PARAM_INT);

diff --git a/question/behaviour/immediatecbm/behaviour.php b/question/behaviour/immediatecbm/behaviour.php
@@ -47,6 +47,10 @@ public function get_min_fraction() {
         return question_cbm::adjust_fraction(parent::get_min_fraction(), question_cbm::HIGH);
     }
 
+    public function get_max_fraction() {
+        return question_cbm::adjust_fraction(parent::get_max_fraction(), question_cbm::HIGH);
+    }
+
     public function get_expected_data() {
         if ($this->qa->get_state()->is_active()) {
             return array(

diff --git a/question/behaviour/missing/behaviour.php b/question/behaviour/missing/behaviour.php
@@ -68,4 +68,9 @@ public function get_min_fraction() {
         throw new coding_exception('The behaviour used for this question is not available. ' .
                 'No processing is possible.');
     }
+
+    public function get_max_fraction() {
+        throw new coding_exception('The behaviour used for this question is not available. ' .
+                'No processing is possible.');
+    }
 }
diff --git a/question/behaviour/missing/tests/missingbehaviour_test.php b/question/behaviour/missing/tests/missingbehaviour_test.php
@@ -53,25 +53,32 @@ public function test_missing_cannot_process() {
         $behaviour->process_action(new question_attempt_pending_step(array()));
     }
 
-    public function test_missing_cannot_get_min_grade() {
+    public function test_missing_cannot_get_min_fraction() {
         $qa = new question_attempt(test_question_maker::make_question('truefalse', 'true'), 0);
         $behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
         $this->setExpectedException('moodle_exception');
         $behaviour->get_min_fraction();
     }
 
+    public function test_missing_cannot_get_max_fraction() {
+        $qa = new question_attempt(test_question_maker::make_question('truefalse', 'true'), 0);
+        $behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
+        $this->setExpectedException('moodle_exception');
+        $behaviour->get_max_fraction();
+    }
+
     public function test_render_missing() {
         $records = new question_test_recordset(array(
             array('questionattemptid', 'contextid', 'questionusageid', 'slot',
-                                   'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
+                                   'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'maxfraction', 'flagged',
                                             'questionsummary', 'rightanswer', 'responsesummary',
                     'timemodified', 'attemptstepid', 'sequencenumber', 'state', 'fraction',
                                                        'timecreated', 'userid', 'name', 'value'),
-            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
+            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 1.0000000, 0, '', '', '',
                     1256233790, 1, 0, 'todo',     null, 1256233700, 1,   '_order', '1,2,3'),
-            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
+            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 1.0000000, 0, '', '', '',
                     1256233790, 2, 1, 'complete', 0.50, 1256233705, 1,  '-submit',  '1'),
-            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
+            array(1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 1.0000000, 0, '', '', '',
                     1256233790, 2, 1, 'complete', 0.50, 1256233705, 1,  'choice0',  '1'),
         ));
 

diff --git a/question/behaviour/rendererbase.php b/question/behaviour/rendererbase.php
@@ -140,11 +140,15 @@ public function manual_comment_fields(question_attempt $qa, question_display_opt
                 'type' => 'hidden',
                 'name' => $qa->get_control_field_name('minfraction'),
                 'value' => $qa->get_min_fraction(),
+            )) . html_writer::empty_tag('input', array(
+                'type' => 'hidden',
+                'name' => $qa->get_control_field_name('maxfraction'),
+                'value' => $qa->get_max_fraction(),
             ));
 
             $errorclass = '';
             $error = '';
-            if ($currentmark > $maxmark || $currentmark < $maxmark * $qa->get_min_fraction()) {
+            if ($currentmark > $maxmark * $qa->get_max_fraction() || $currentmark < $maxmark * $qa->get_min_fraction()) {
                 $errorclass = ' error';
                 $error = html_writer::tag('span', get_string('manualgradeoutofrange', 'question'),
                         array('class' => 'error')) . html_writer::empty_tag('br');

diff --git a/question/engine/datalib.php b/question/engine/datalib.php
@@ -106,6 +106,7 @@ public function insert_question_attempt(question_attempt $qa, $context) {
         $record->variant = $qa->get_variant();
         $record->maxmark = $qa->get_max_mark();
         $record->minfraction = $qa->get_min_fraction();
+        $record->maxfraction = $qa->get_max_fraction();
         $record->flagged = $qa->is_flagged();
         $record->questionsummary = $qa->get_question_summary();
         if (core_text::strlen($record->questionsummary) > question_bank::MAX_SUMMARY_LENGTH) {
@@ -259,6 +260,7 @@ public function load_question_attempt($questionattemptid) {
     qa.variant,
     qa.maxmark,
     qa.minfraction,
+    qa.maxfraction,
     qa.flagged,
     qa.questionsummary,
     qa.rightanswer,
@@ -318,6 +320,7 @@ public function load_questions_usage_by_activity($qubaid) {
     qa.variant,
     qa.maxmark,
     qa.minfraction,
+    qa.maxfraction,
     qa.flagged,
     qa.questionsummary,
     qa.rightanswer,
@@ -377,6 +380,7 @@ public function load_questions_usages_latest_steps(qubaid_condition $qubaids, $s
     qa.variant,
     qa.maxmark,
     qa.minfraction,
+    qa.maxfraction,
     qa.flagged,
     qa.questionsummary,
     qa.rightanswer,
@@ -641,6 +645,7 @@ public function load_attempts_at_question($questionid, qubaid_condition $qubaids
     qa.variant,
     qa.maxmark,
     qa.minfraction,
+    qa.maxfraction,
     qa.flagged,
     qa.questionsummary,
     qa.rightanswer,
@@ -708,6 +713,7 @@ public function update_question_attempt(question_attempt $qa) {
         $record->id = $qa->get_database_id();
         $record->maxmark = $qa->get_max_mark();
         $record->minfraction = $qa->get_min_fraction();
+        $record->maxfraction = $qa->get_max_fraction();
         $record->flagged = $qa->is_flagged();
         $record->questionsummary = $qa->get_question_summary();
         $record->rightanswer = $qa->get_right_answer_summary();
@@ -949,6 +955,7 @@ public function question_attempt_latest_state_view($alias, qubaid_condition $qub
                        {$alias}qa.variant,
                        {$alias}qa.maxmark,
                        {$alias}qa.minfraction,
+                       {$alias}qa.maxfraction,
                        {$alias}qa.flagged,
                        {$alias}qa.questionsummary,
                        {$alias}qa.rightanswer,

diff --git a/question/engine/lib.php b/question/engine/lib.php
@@ -142,7 +142,8 @@ public static function is_manual_grade_in_range($qubaid, $slot) {
         $mark = question_utils::optional_param_mark($prefix . '-mark');
         $maxmark = optional_param($prefix . '-maxmark', null, PARAM_FLOAT);
         $minfraction = optional_param($prefix . ':minfraction', null, PARAM_FLOAT);
-        return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxmark);
+        $maxfraction = optional_param($prefix . ':maxfraction', null, PARAM_FLOAT);
+        return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxfraction * $maxmark);
     }
 
     /**

diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php
@@ -87,15 +87,25 @@ class question_attempt {
     /** @var int which variant of the question to use. */
     protected $variant;
 
-    /** @var number the maximum mark that can be scored at this question. */
+    /**
+     * @var float the maximum mark that can be scored at this question.
+     * Actually, this is only really a nominal maximum. It might be better thought
+     * of as the question weight.
+     */
     protected $maxmark;
 
     /**
-     * @var number the minimum fraction that can be scored at this question, so
+     * @var float the minimum fraction that can be scored at this question, so
      * the minimum mark is $this->minfraction * $this->maxmark.
      */
     protected $minfraction = null;
 
+    /**
+     * @var float the maximum fraction that can be scored at this question, so
+     * the maximum mark is $this->maxfraction * $this->maxmark.
+     */
+    protected $maxfraction = null;
+
     /**
      * @var string plain text summary of the variant of the question the
      * student saw. Intended for reporting purposes.
@@ -649,19 +659,32 @@ public function fraction_to_mark($fraction) {
         return $fraction * $this->maxmark;
     }
 
-    /** @return number the maximum mark possible for this question attempt. */
+    /**
+     * @return float the maximum mark possible for this question attempt.
+     * In fact, this is not strictly the maximum, becuase get_max_fraction may
+     * return a number greater than 1. It might be better to think of this as a
+     * question weight.
+     */
     public function get_max_mark() {
         return $this->maxmark;
     }
 
-    /** @return number the maximum mark possible for this question attempt. */
+    /** @return float the maximum mark possible for this question attempt. */
     public function get_min_fraction() {
         if (is_null($this->minfraction)) {
-            throw new coding_exception('This question_attempt has not been started yet, the min fraction is not yet konwn.');
+            throw new coding_exception('This question_attempt has not been started yet, the min fraction is not yet known.');
         }
         return $this->minfraction;
     }
 
+    /** @return float the maximum mark possible for this question attempt. */
+    public function get_max_fraction() {
+        if (is_null($this->maxfraction)) {
+            throw new coding_exception('This question_attempt has not been started yet, the max fraction is not yet known.');
+        }
+        return $this->maxfraction;
+    }
+
     /**
      * The current mark, formatted to the stated number of decimal places. Uses
      * {@link format_float()} to format floats according to the current locale.
@@ -885,8 +908,9 @@ public function start($preferredbehaviour, $variant, $submitteddata = array(),
             $this->behaviour = new $class($this, $preferredbehaviour);
         }
 
-        // Record the minimum fraction.
+        // Record the minimum and maximum fractions.
         $this->minfraction = $this->behaviour->get_min_fraction();
+        $this->maxfraction = $this->behaviour->get_max_fraction();
 
         // Initialise the first step.
         $firststep = new question_attempt_step($submitteddata, $timestamp, $userid, $existingstepid);
@@ -1287,6 +1311,7 @@ public static function load_from_records($records, $questionattemptid,
         $qa->set_slot($record->slot);
         $qa->variant = $record->variant + 0;
         $qa->minfraction = $record->minfraction + 0;
+        $qa->maxfraction = $record->maxfraction + 0;
         $qa->set_flagged($record->flagged);
         $qa->questionsummary = $record->questionsummary;
         $qa->rightanswer = $record->rightanswer;
@@ -1389,6 +1414,7 @@ public function __construct(question_attempt $baseqa, $lastseq, $preferredbehavi
         $this->question = $this->baseqa->question;
         $this->maxmark = $this->baseqa->maxmark;
         $this->minfraction = $this->baseqa->minfraction;
+        $this->maxfraction = $this->baseqa->maxfraction;
         $this->questionsummary = $this->baseqa->questionsummary;
         $this->responsesummary = $this->baseqa->responsesummary;
         $this->rightanswer = $this->baseqa->rightanswer;

diff --git a/question/engine/questionattemptstep.php b/question/engine/questionattemptstep.php
@@ -32,7 +32,7 @@
  *
  * The most important attributes of a step are the state, which is one of the
  * {@link question_state} constants, the fraction, which may be null, or a
- * number bewteen the attempt's minfraction and 1.0, and the array of submitted
+ * number bewteen the attempt's minfraction and maxfraction, and the array of submitted
  * data, about which more later.
  *
  * A step also tracks the time it was created, and the user responsible for
@@ -76,7 +76,10 @@ class question_attempt_step {
      */
     private $state;
 
-    /** @var null|number the fraction (grade on a scale of minfraction .. 1.0) or null. */
+    /**
+     * @var null|number the fraction (grade on a scale of
+     * minfraction .. maxfraction, normally 0..1) or null.
+     */
     private $fraction = null;
 
     /** @var integer the timestamp when this step was created. */
@@ -148,7 +151,8 @@ public function set_state($state) {
     }
 
     /**
-     * @return null|number the fraction (grade on a scale of minfraction .. 1.0)
+     * @return null|number the fraction (grade on a scale of
+     * minfraction .. maxfraction, normally 0..1),
      * or null if this step has not been marked.
      */
     public function get_fraction() {

diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php
@@ -43,6 +43,9 @@ public function add_step(question_attempt_step $step) {
     public function set_min_fraction($fraction) {
         $this->minfraction = $fraction;
     }
+    public function set_max_fraction($fraction) {
+        $this->maxfraction = $fraction;
+    }
     public function set_behaviour(question_behaviour $behaviour) {
         $this->behaviour = $behaviour;
     }