Permalink
Browse files

MDL-34640 question repsponse files: remaining tidy up of this code.

  • Loading branch information...
1 parent d078258 commit dc1ee5cb2965705d3b518927bb050e0ba2be2dd5 @timhunt timhunt committed Mar 30, 2013
@@ -206,6 +206,8 @@ public function update_question_attempt_step(question_attempt_step $step,
public function load_question_attempt_step($stepid) {
$records = $this->db->get_recordset_sql("
SELECT
+ quba.contextid,
+ COALLESCE(q.qtype, 'missingtype') AS qtype,
qas.id AS attemptstepid,
qas.questionattemptid,
qas.sequencenumber,
@@ -216,7 +218,10 @@ public function load_question_attempt_step($stepid) {
qasd.name,
qasd.value
-FROM {question_attempt_steps} qas
+FROM {question_attempt_steps} qas
+JOIN {question_attempts} qa ON qa.id = qas.questionattemptid
+JOIN {question_usages} quba ON quba.id = qa.questionusageid
+LEFT JOIN {question} q ON q.id = qa.questionid
LEFT JOIN {question_attempt_step_data} qasd ON qasd.attemptstepid = qas.id
WHERE
@@ -227,7 +232,6 @@ public function load_question_attempt_step($stepid) {
throw new coding_exception('Failed to load question_attempt_step ' . $stepid);
}
- // TODO: pass a question_type and a contextid to load_from_records to get response files
$step = question_attempt_step::load_from_records($records, $stepid);
$records->close();
@@ -1285,7 +1285,7 @@ public static function load_from_records($records, $questionattemptid,
$autosavedsequencenumber = null;
while ($record && $record->questionattemptid == $questionattemptid && !is_null($record->attemptstepid)) {
$sequencenumber = $record->sequencenumber;
- $nextstep = question_attempt_step::load_from_records($records, $record->attemptstepid, $qa->get_question(), $record->contextid);
+ $nextstep = question_attempt_step::load_from_records($records, $record->attemptstepid, $qa->get_question()->get_type_name());
if ($sequencenumber < 0) {
if (!$autosavedstep) {
@@ -370,11 +370,11 @@ public function get_all_data() {
* Create a question_attempt_step from records loaded from the database.
* @param Iterator $records Raw records loaded from the database.
* @param int $stepid The id of the records to extract.
- * @param string $qtype The question type of which this is an attempt
- * @param int $contextid The contextid of this question attempt
+ * @param string $qtype The question type of which this is an attempt.
+ * If not given, each record must include a qtype field.
* @return question_attempt_step The newly constructed question_attempt_step.
*/
- public static function load_from_records($records, $attemptstepid, $qtype = null, $contextid = null) {
+ public static function load_from_records($records, $attemptstepid, $qtype = null) {
$currentrec = $records->current();
while ($currentrec->attemptstepid != $attemptstepid) {
$records->next();
@@ -386,6 +386,7 @@ public static function load_from_records($records, $attemptstepid, $qtype = null
}
$record = $currentrec;
+ $contextid = null;
$data = array();
while ($currentrec && $currentrec->attemptstepid == $attemptstepid) {
if (!is_null($currentrec->name)) {
@@ -410,14 +411,15 @@ public static function load_from_records($records, $attemptstepid, $qtype = null
// Somehow, we need to get that information to this point by modifying
// all the paths by which this method can be called.
// Can we only return files when it's possible? Should there be some kind of warning?
- if ($qtype && $contextid) {
- foreach (question_bank::get_qtype($qtype)->response_file_areas() as $area) {
- if (empty($step->data[$area])) {
- continue;
- }
-
- $step->data[$area] = new question_file_loader($step, $area, $step->get_qt_var($area), $contextid);
+ if (is_null($qtype)) {
+ $qtype = $record->qtype;
+ }
+ foreach (question_bank::get_qtype($qtype)->response_file_areas() as $area) {
+ if (empty($step->data[$area])) {
+ continue;
}
+
+ $step->data[$area] = new question_file_loader($step, $area, $step->data[$area], $record->contextid);
}
return $step;
@@ -0,0 +1,84 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * This file contains tests for the question_attempt_step class.
+ *
+ * @package moodlecore
+ * @subpackage questionengine
+ * @copyright 2009 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once(dirname(__FILE__) . '/../lib.php');
+require_once(dirname(__FILE__) . '/helpers.php');
+
+
+/**
+ * Unit tests for the loading data into the {@link question_attempt_step} class.
+ *
+ * @copyright 2009 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class question_attempt_step_db_test extends data_loading_method_test_base {
+ public function test_load_with_data() {
+ $records = new question_test_recordset(array(
+ array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value', 'qtype', 'contextid'),
+ array( 1, 1, 0, 'todo', null, 1256228502, 13, null, null, 'description', 1),
+ array( 2, 1, 1, 'complete', null, 1256228505, 13, 'x', 'a', 'description', 1),
+ array( 2, 1, 1, 'complete', null, 1256228505, 13, '_y', '_b', 'description', 1),
+ array( 2, 1, 1, 'complete', null, 1256228505, 13, '-z', '!c', 'description', 1),
+ array( 2, 1, 1, 'complete', null, 1256228505, 13, '-_t', '!_d', 'description', 1),
+ array( 3, 1, 2, 'gradedright', 1.0, 1256228515, 13, '-finish', '1', 'description', 1),
+ ));
+
+ $step = question_attempt_step::load_from_records($records, 2);
+ $this->assertEquals(question_state::$complete, $step->get_state());
+ $this->assertNull($step->get_fraction());
+ $this->assertEquals(1256228505, $step->get_timecreated());
+ $this->assertEquals(13, $step->get_user_id());
+ $this->assertEquals(array('x' => 'a', '_y' => '_b', '-z' => '!c', '-_t' => '!_d'), $step->get_all_data());
+ }
+
+ public function test_load_without_data() {
+ $records = new question_test_recordset(array(
+ array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value', 'contextid'),
+ array( 2, 1, 1, 'complete', null, 1256228505, 13, null, null, 1),
+ ));
+
+ $step = question_attempt_step::load_from_records($records, 2, 'description');
+ $this->assertEquals(question_state::$complete, $step->get_state());
+ $this->assertNull($step->get_fraction());
+ $this->assertEquals(1256228505, $step->get_timecreated());
+ $this->assertEquals(13, $step->get_user_id());
+ $this->assertEquals(array(), $step->get_all_data());
+ }
+
+ public function test_load_dont_be_too_greedy() {
+ $records = new question_test_recordset(array(
+ array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value', 'contextid'),
+ array( 1, 1, 0, 'todo', null, 1256228502, 13, 'x', 'right', 1),
+ array( 2, 2, 0, 'complete', null, 1256228505, 13, 'x', 'wrong', 1),
+ ));
+
+ $step = question_attempt_step::load_from_records($records, 1, 'description');
+ $this->assertEquals(array('x' => 'right'), $step->get_all_data());
+ }
+}
@@ -129,56 +129,3 @@ public function test_constructor_given_params() {
}
}
-
-
-/**
- * Unit tests for the loading data into the {@link question_attempt_step} class.
- *
- * @copyright 2009 The Open University
- * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-class question_attempt_step_db_test extends data_loading_method_test_base {
- public function test_load_with_data() {
- $records = new question_test_recordset(array(
- array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
- array( 1, 1, 0, 'todo', null, 1256228502, 13, null, null),
- array( 2, 1, 1, 'complete', null, 1256228505, 13, 'x', 'a'),
- array( 2, 1, 1, 'complete', null, 1256228505, 13, '_y', '_b'),
- array( 2, 1, 1, 'complete', null, 1256228505, 13, '-z', '!c'),
- array( 2, 1, 1, 'complete', null, 1256228505, 13, '-_t', '!_d'),
- array( 3, 1, 2, 'gradedright', 1.0, 1256228515, 13, '-finish', '1'),
- ));
-
- $step = question_attempt_step::load_from_records($records, 2);
- $this->assertEquals(question_state::$complete, $step->get_state());
- $this->assertNull($step->get_fraction());
- $this->assertEquals(1256228505, $step->get_timecreated());
- $this->assertEquals(13, $step->get_user_id());
- $this->assertEquals(array('x' => 'a', '_y' => '_b', '-z' => '!c', '-_t' => '!_d'), $step->get_all_data());
- }
-
- public function test_load_without_data() {
- $records = new question_test_recordset(array(
- array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
- array( 2, 1, 1, 'complete', null, 1256228505, 13, null, null),
- ));
-
- $step = question_attempt_step::load_from_records($records, 2);
- $this->assertEquals(question_state::$complete, $step->get_state());
- $this->assertNull($step->get_fraction());
- $this->assertEquals(1256228505, $step->get_timecreated());
- $this->assertEquals(13, $step->get_user_id());
- $this->assertEquals(array(), $step->get_all_data());
- }
-
- public function test_load_dont_be_too_greedy() {
- $records = new question_test_recordset(array(
- array('attemptstepid', 'questionattemptid', 'sequencenumber', 'state', 'fraction', 'timecreated', 'userid', 'name', 'value'),
- array( 1, 1, 0, 'todo', null, 1256228502, 13, 'x', 'right'),
- array( 2, 2, 0, 'complete', null, 1256228505, 13, 'x', 'wrong'),
- ));
-
- $step = question_attempt_step::load_from_records($records, 1);
- $this->assertEquals(array('x' => 'right'), $step->get_all_data());
- }
-}

0 comments on commit dc1ee5c

Please sign in to comment.