Skip to content

Commit

Permalink
MDL-27649 support question variants as a first-class concept in the q…
Browse files Browse the repository at this point in the history
…uestion engine.
  • Loading branch information
timhunt committed May 26, 2011
1 parent f55192b commit 1da821b
Show file tree
Hide file tree
Showing 46 changed files with 283 additions and 98 deletions.
5 changes: 3 additions & 2 deletions lib/db/install.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1347,8 +1347,9 @@
<FIELD NAME="questionusageid" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="false" COMMENT="Foreign key, references question_usages.id" PREVIOUS="id" NEXT="slot"/>
<FIELD NAME="slot" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="false" COMMENT="Used to number the questions in one attempt sequentially." PREVIOUS="questionusageid" NEXT="behaviour"/>
<FIELD NAME="behaviour" TYPE="char" LENGTH="32" NOTNULL="true" SEQUENCE="false" COMMENT="The name of the question behaviour that is managing this question attempt." PREVIOUS="slot" NEXT="questionid"/>
<FIELD NAME="questionid" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="false" COMMENT="The id of the question being attempted. Foreign key references question.id." PREVIOUS="behaviour" NEXT="maxmark"/>
<FIELD NAME="maxmark" TYPE="number" LENGTH="12" NOTNULL="true" UNSIGNED="false" SEQUENCE="false" DECIMALS="7" COMMENT="The grade this question is marked out of in this attempt." PREVIOUS="questionid" NEXT="minfraction"/>
<FIELD NAME="questionid" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="false" COMMENT="The id of the question being attempted. Foreign key references question.id." PREVIOUS="behaviour" NEXT="variant"/>
<FIELD NAME="variant" TYPE="int" LENGTH="10" NOTNULL="true" UNSIGNED="true" SEQUENCE="false" COMMENT="The variant of the qusetion being used." PREVIOUS="questionid" NEXT="maxmark"/>
<FIELD NAME="maxmark" TYPE="number" LENGTH="12" NOTNULL="true" UNSIGNED="false" SEQUENCE="false" DECIMALS="7" COMMENT="The grade this question is marked out of in this attempt." PREVIOUS="variant" NEXT="minfraction"/>
<FIELD NAME="minfraction" TYPE="number" LENGTH="12" NOTNULL="true" UNSIGNED="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." PREVIOUS="maxmark" NEXT="flagged"/>
<FIELD NAME="flagged" TYPE="int" LENGTH="1" NOTNULL="true" UNSIGNED="true" DEFAULT="0" SEQUENCE="false" COMMENT="Whether this question has been flagged within the attempt." PREVIOUS="minfraction" NEXT="questionsummary"/>
<FIELD NAME="questionsummary" TYPE="text" LENGTH="small" 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." PREVIOUS="flagged" NEXT="rightanswer"/>
Expand Down
15 changes: 15 additions & 0 deletions lib/db/upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -6414,6 +6414,21 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2011051212);
}

if ($oldversion < 2011051213) {
// Define field variant to be added to question_attempts
$table = new xmldb_table('question_attempts');
$field = new xmldb_field('variant', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED,
XMLDB_NOTNULL, null, 1, 'questionid');

// Launch add field component
if (!$dbman->field_exists($table, $field)) {
$dbman->add_field($table, $field);
}

// Main savepoint reached
upgrade_main_savepoint(true, 2011051213);
}

return true;
}

Expand Down
1 change: 1 addition & 0 deletions local/qeupgradehelper/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ public function test_{$question->qtype}_{$quiz->preferredbehaviour}_{$namesuffix
echo "
'behaviour' => '{$quiz->preferredbehaviour}',
'questionid' => {$question->id},
'variant' => 1,
'maxmark' => {$question->maxmark},
'minfraction' => 0,
'flagged' => 0,
Expand Down
2 changes: 1 addition & 1 deletion mod/quiz/startattempt.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@
}

// Start all the quetsions.
$quba->start_all_questions(time(), null);
$quba->start_all_questions(null, time(), null);

// Update attempt layout.
$newlayout = array();
Expand Down
5 changes: 3 additions & 2 deletions question/behaviour/behaviourbase.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,10 @@ public abstract function summarise_action(question_attempt_step $step);
*
* @param question_attempt_step $step the first step of the
* question_attempt being started.
* @param int $variant which variant of the question to use.
*/
public function init_first_step(question_attempt_step $step) {
$this->question->start_attempt($step);
public function init_first_step(question_attempt_step $step, $variant) {
$this->question->start_attempt($step, $variant);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions question/behaviour/interactive/behaviour.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ public function get_state_string($showcorrectness) {
}
}

public function init_first_step(question_attempt_step $step) {
parent::init_first_step($step);
public function init_first_step(question_attempt_step $step, $variant) {
parent::init_first_step($step, $variant);
$step->set_behaviour_var('_triesleft', count($this->question->hints) + 1);
}

Expand Down
2 changes: 1 addition & 1 deletion question/behaviour/missing/behaviour.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function summarise_action(question_attempt_step $step) {
return '';
}

public function init_first_step(question_attempt_step $step) {
public function init_first_step(question_attempt_step $step, $variant) {
throw new coding_exception('The behaviour used for this question is not available. ' .
'No processing is possible.');
}
Expand Down
10 changes: 5 additions & 5 deletions question/behaviour/missing/simpletest/testmissingbehaviour.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function test_missing_cannot_start() {
$qa = new question_attempt(test_question_maker::make_a_truefalse_question(), 0);
$behaviour = new qbehaviour_missing($qa, 'deferredfeedback');
$this->expectException();
$behaviour->init_first_step(new question_attempt_step(array()));
$behaviour->init_first_step(new question_attempt_step(array()), 1);
}

public function test_missing_cannot_process() {
Expand All @@ -62,15 +62,15 @@ public function test_missing_cannot_get_min_grade() {
public function test_render_missing() {
$records = testing_db_record_builder::build_db_records(array(
array('id', 'questionattemptid', 'contextid', 'questionusageid', 'slot',
'behaviour', 'questionid', 'maxmark', 'minfraction', 'flagged',
'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
'questionsummary', 'rightanswer', 'responsesummary',
'timemodified', 'attemptstepid', 'sequencenumber', 'state', 'fraction',
'timecreated', 'userid', 'name', 'value'),
array(1, 1, 123, 1, 1, 'strangeunknown', -1, 2.0000000, 0.0000000, 0, '', '', '',
array(1, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 1, 0, 'todo', null, 1256233700, 1, '_order', '1,2,3'),
array(2, 1, 123, 1, 1, 'strangeunknown', -1, 2.0000000, 0.0000000, 0, '', '', '',
array(2, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 2, 1, 'complete', 0.50, 1256233705, 1, '-submit', '1'),
array(3, 1, 123, 1, 1, 'strangeunknown', -1, 2.0000000, 0.0000000, 0, '', '', '',
array(3, 1, 123, 1, 1, 'strangeunknown', -1, 1, 2.0000000, 0.0000000, 0, '', '', '',
1256233790, 2, 1, 'complete', 0.50, 1256233705, 1, 'choice0', '1'),
));

Expand Down
6 changes: 6 additions & 0 deletions question/engine/datalib.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public function insert_question_attempt(question_attempt $qa, $context) {
$record->slot = $qa->get_slot();
$record->behaviour = $qa->get_behaviour_name();
$record->questionid = $qa->get_question()->id;
$record->variant = $qa->get_variant();
$record->maxmark = $qa->get_max_mark();
$record->minfraction = $qa->get_min_fraction();
$record->flagged = $qa->is_flagged();
Expand Down Expand Up @@ -180,6 +181,7 @@ public function load_question_attempt($questionattemptid) {
qa.slot,
qa.behaviour,
qa.questionid,
qa.variant,
qa.maxmark,
qa.minfraction,
qa.flagged,
Expand Down Expand Up @@ -236,6 +238,7 @@ public function load_questions_usage_by_activity($qubaid) {
qa.slot,
qa.behaviour,
qa.questionid,
qa.variant,
qa.maxmark,
qa.minfraction,
qa.flagged,
Expand Down Expand Up @@ -291,6 +294,7 @@ public function load_questions_usages_latest_steps(qubaid_condition $qubaids, $s
qa.slot,
qa.behaviour,
qa.questionid,
qa.variant,
qa.maxmark,
qa.minfraction,
qa.flagged,
Expand Down Expand Up @@ -549,6 +553,7 @@ public function load_attempts_at_question($questionid, qubaid_condition $qubaids
qa.slot,
qa.behaviour,
qa.questionid,
qa.variant,
qa.maxmark,
qa.minfraction,
qa.flagged,
Expand Down Expand Up @@ -819,6 +824,7 @@ public function question_attempt_latest_state_view($alias) {
{$alias}qa.slot,
{$alias}qa.behaviour,
{$alias}qa.questionid,
{$alias}qa.variant,
{$alias}qa.maxmark,
{$alias}qa.minfraction,
{$alias}qa.flagged,
Expand Down
30 changes: 30 additions & 0 deletions question/engine/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -760,3 +760,33 @@ public static function int_to_roman($number) {
self::$tens[$number / 10 % 10] . self::$units[$number % 10];
}
}


/**
* The interface for strategies for controlling which variant of each question is used.
*
* @copyright 2010 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
interface question_variant_selection_strategy {
/**
* @param int $maxvariants the num
* @param string $seed data that can be used to controls how the variant is selected
* in a semi-random way.
* @return int the variant to use, a number betweeb 1 and $maxvariants inclusive.
*/
public function choose_variant($maxvariants, $seed);
}


/**
* A {@link question_variant_selection_strategy} that is completely random.
*
* @copyright 2010 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class question_variant_random_strategy implements question_variant_selection_strategy {
public function choose_variant($maxvariants, $seed) {
return rand(1, $maxvariants);
}
}
37 changes: 32 additions & 5 deletions question/engine/questionattempt.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class question_attempt {
/** @var question_definition the question this is an attempt at. */
protected $question;

/** @var int which variant of the question to use. */
protected $variant;

/** @var number the maximum mark that can be scored at this question. */
protected $maxmark;

Expand Down Expand Up @@ -162,6 +165,14 @@ public function get_question() {
return $this->question;
}

/**
* Get the variant of the question being used in a given slot.
* @return int the variant number.
*/
public function get_variant() {
return $this->variant;
}

/**
* Set the number used to identify this question_attempt within the usage.
* For internal use only.
Expand Down Expand Up @@ -735,6 +746,16 @@ protected function add_step(question_attempt_step $step) {
$this->observer->notify_step_added($step, $this, key($this->steps));
}

/**
* Use a strategy to pick a variant.
* @param question_variant_selection_strategy $variantstrategy a strategy.
* @return int the selected variant.
*/
public function select_variant(question_variant_selection_strategy $variantstrategy) {
return $variantstrategy->choose_variant($this->get_question()->get_num_variants(),
$this->get_question()->get_variants_selection_seed());
}

/**
* Start this question attempt.
*
Expand All @@ -743,12 +764,17 @@ protected function add_step(question_attempt_step $step) {
*
* @param string|question_behaviour $preferredbehaviour the name of the
* desired archetypal behaviour, or an actual model instance.
* @param int $variant the variant of the question to start. Between 1 and
* $this->get_question()->get_num_variants() inclusive.
* @param array $submitteddata optional, used when re-starting to keep the same initial state.
* @param int $timestamp optional, the timstamp to record for this action. Defaults to now.
* @param int $userid optional, the user to attribute this action to. Defaults to the current user.
*/
public function start($preferredbehaviour, $submitteddata = array(), $timestamp = null, $userid = null) {
public function start($preferredbehaviour, $variant, $submitteddata = array(),
$timestamp = null, $userid = null) {

// Initialise the behaviour.
$this->variant = $variant;
if (is_string($preferredbehaviour)) {
$this->behaviour =
$this->question->make_behaviour($this, $preferredbehaviour);
Expand All @@ -766,7 +792,7 @@ public function start($preferredbehaviour, $submitteddata = array(), $timestamp
if ($submitteddata) {
$this->question->apply_attempt_state($firststep);
} else {
$this->behaviour->init_first_step($firststep);
$this->behaviour->init_first_step($firststep, $variant);
}
$this->add_step($firststep);

Expand All @@ -786,7 +812,7 @@ public function start($preferredbehaviour, $submitteddata = array(), $timestamp
* defines the starting point.
*/
public function start_based_on(question_attempt $oldqa) {
$this->start($oldqa->behaviour, $oldqa->get_resume_data());
$this->start($oldqa->behaviour, $oldqa->get_variant(), $oldqa->get_resume_data());
}

/**
Expand Down Expand Up @@ -1013,7 +1039,7 @@ public function regrade(question_attempt $oldqa, $finished) {
foreach ($oldqa->get_step_iterator() as $step) {
if ($first) {
$first = false;
$this->start($oldqa->behaviour, $step->get_all_data(),
$this->start($oldqa->behaviour, $oldqa->get_variant(), $step->get_all_data(),
$step->get_timecreated(), $step->get_user_id());
} else {
$this->process_action($step->get_submitted_data(),
Expand Down Expand Up @@ -1109,6 +1135,7 @@ public static function load_from_records(&$records, $questionattemptid,
null, $record->maxmark + 0);
$qa->set_database_id($record->questionattemptid);
$qa->set_number_in_usage($record->slot);
$qa->variant = $record->variant + 0;
$qa->minfraction = $record->minfraction + 0;
$qa->set_flagged($record->flagged);
$qa->questionsummary = $record->questionsummary;
Expand Down Expand Up @@ -1194,7 +1221,7 @@ protected function add_step(question_attempt_step $step) {
public function process_action($submitteddata, $timestamp = null, $userid = null) {
coding_exception('Cannot modify a question_attempt_with_restricted_history.');
}
public function start($preferredbehaviour, $submitteddata = array(), $timestamp = null, $userid = null) {
public function start($preferredbehaviour, $variant, $submitteddata = array(), $timestamp = null, $userid = null) {
coding_exception('Cannot modify a question_attempt_with_restricted_history.');
}

Expand Down
4 changes: 4 additions & 0 deletions question/engine/questionattemptstep.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ class question_attempt_step {
*/
public function __construct($data = array(), $timecreated = null, $userid = null) {
global $USER;

if (!is_array($data)) {
echo format_backtrace(debug_backtrace());
}
$this->state = question_state::$unprocessed;
$this->data = $data;
if (is_null($timecreated)) {
Expand Down
39 changes: 35 additions & 4 deletions question/engine/questionusage.php
Original file line number Diff line number Diff line change
Expand Up @@ -407,24 +407,55 @@ public function get_field_prefix($slot) {
return $this->get_question_attempt($slot)->get_field_prefix();
}

/**
* Get the number of variants available for the question in this slot.
* @param int $slot the number used to identify this question within this usage.
* @return int the number of variants available.
*/
public function get_num_variants($slot) {
return $this->get_question_attempt($slot)->get_question()->get_num_variants();
}

/**
* Get the variant of the question being used in a given slot.
* @param int $slot the number used to identify this question within this usage.
* @return int the variant of this question that is being used.
*/
public function get_variant($slot) {
return $this->get_question_attempt($slot)->get_variant();
}

/**
* Start the attempt at a question that has been added to this usage.
* @param int $slot the number used to identify this question within this usage.
* @param int $variant which variant of the question to use. Must be between
* 1 and ->get_num_variants($slot) inclusive. If not give, a variant is
* chosen at random.
*/
public function start_question($slot) {
public function start_question($slot, $variant = null) {
if (is_null($variant)) {
$variant = rand(1, $this->get_num_variants($slot));
}

$qa = $this->get_question_attempt($slot);
$qa->start($this->preferredbehaviour);
$qa->start($this->preferredbehaviour, $variant);
$this->observer->notify_attempt_modified($qa);
}

/**
* Start the attempt at all questions that has been added to this usage.
* @param question_variant_selection_strategy how to pick which variant of each question to use.
* @param int $timestamp optional, the timstamp to record for this action. Defaults to now.
* @param int $userid optional, the user to attribute this action to. Defaults to the current user.
*/
public function start_all_questions($timestamp = null, $userid = null) {
public function start_all_questions(question_variant_selection_strategy $variantstrategy = null,
$timestamp = null, $userid = null) {
if (is_null($variantstrategy)) {
$variantstrategy = new question_variant_random_strategy();
}

foreach ($this->questionattempts as $qa) {
$qa->start($this->preferredbehaviour);
$qa->start($this->preferredbehaviour, $qa->select_variant($variantstrategy));
$this->observer->notify_attempt_modified($qa);
}
}
Expand Down
Loading

0 comments on commit 1da821b

Please sign in to comment.