diff --git a/question/engine/datalib.php b/question/engine/datalib.php index 4efafbe0c3f..b4fe3a63989 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -69,7 +69,7 @@ class question_engine_data_mapper { $quba->set_id_from_database($newid); foreach ($quba->get_attempt_iterator() as $qa) { - $this->insert_question_attempt($qa); + $this->insert_question_attempt($qa, $quba->get_owning_context()); } } @@ -77,8 +77,9 @@ class question_engine_data_mapper { * Store an entire {@link question_attempt} in the database, * including all the question_attempt_steps that comprise it. * @param question_attempt $qa the question attempt to store. + * @param object $context the context of the owning question_usage_by_activity. */ - public function insert_question_attempt(question_attempt $qa) { + public function insert_question_attempt(question_attempt $qa, $context) { $record = new stdClass(); $record->questionusageid = $qa->get_usage_id(); $record->slot = $qa->get_slot(); @@ -94,16 +95,19 @@ class question_engine_data_mapper { $record->id = $this->db->insert_record('question_attempts', $record); foreach ($qa->get_step_iterator() as $seq => $step) { - $this->insert_question_attempt_step($step, $record->id, $seq); + $this->insert_question_attempt_step($step, $record->id, $seq, $context); } } /** * Store a {@link question_attempt_step} in the database. * @param question_attempt_step $qa the step to store. + * @param int $questionattemptid the question attept id this step belongs to. + * @param int $seq the sequence number of this stop. + * @param object $context the context of the owning question_usage_by_activity. */ public function insert_question_attempt_step(question_attempt_step $step, - $questionattemptid, $seq) { + $questionattemptid, $seq, $context) { $record = new stdClass(); $record->questionattemptid = $questionattemptid; $record->sequencenumber = $seq; @@ -120,6 +124,10 @@ class question_engine_data_mapper { $data->name = $name; $data->value = $value; $this->db->insert_record('question_attempt_step_data', $data, false); + + if ($value instanceof question_file_saver) { + $value->save_files($record->id, $context); + } } } @@ -911,10 +919,11 @@ class question_engine_unit_of_work implements question_usage_observer { $dm->delete_steps_for_question_attempts(array_keys($this->attemptstodeletestepsfor)); foreach ($this->stepsadded as $stepinfo) { list($step, $questionattemptid, $seq) = $stepinfo; - $dm->insert_question_attempt_step($step, $questionattemptid, $seq); + $dm->insert_question_attempt_step($step, $questionattemptid, $seq, + $this->quba->get_owning_context()); } foreach ($this->attemptsadded as $qa) { - $dm->insert_question_attempt($qa); + $dm->insert_question_attempt($qa, $this->quba->get_owning_context()); } foreach ($this->attemptsmodified as $qa) { $dm->update_question_attempt($qa); @@ -926,6 +935,53 @@ class question_engine_unit_of_work implements question_usage_observer { } +/** + * This class represents the promise to save some files from a particular draft + * file area into a particular file area. It is used beause the necessary + * information about what to save is to hand in the + * {@link question_attempt::process_response_files()} method, but we don't know + * if this question attempt will actually be saved in the database until later, + * when the {@link question_engine_unit_of_work} is saved, if it is. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_file_saver { + /** @var int the id of the draft file area to save files from. */ + protected $draftitemid; + /** @var string the owning component name. */ + protected $component; + /** @var string the file area name. */ + protected $filearea; + + /** + * Constuctor. + * @param int $draftitemid the draft area to save the files from. + * @param string $component the component for the file area to save into. + * @param string $filearea the name of the file area to save into. + */ + public function __construct($draftitemid, $component, $filearea) { + $this->draftitemid = $draftitemid; + $this->component = $component; + $this->filearea = $filearea; + } + + public function __toString() { + // When stored in the database, we want this value to appear as 1. + return '1'; + } + + /** + * Actually save the files. + * @param integer $itemid the item id for the file area to save into. + */ + public function save_files($itemid, $context) { + file_save_draft_area_files($this->draftitemid, $context->id, + $this->component, $this->filearea, $itemid); + } +} + + /** * This class represents a restriction on the set of question_usage ids to include * in a larger database query. Depending of the how you are going to restrict the diff --git a/question/engine/lib.php b/question/engine/lib.php index 63d57065c86..c1c8048e29f 100644 --- a/question/engine/lib.php +++ b/question/engine/lib.php @@ -1094,7 +1094,7 @@ class question_usage_by_activity { */ public function validate_sequence_number($slot, $postdata = null) { $qa = $this->get_question_attempt($slot); - $sequencecheck = question_attempt::get_submitted_var( + $sequencecheck = $qa->get_submitted_var( $qa->get_control_field_name('sequencecheck'), PARAM_INT, $postdata); if (is_null($sequencecheck)) { return false; @@ -1113,7 +1113,7 @@ class question_usage_by_activity { */ public function update_question_flags($postdata = null) { foreach ($this->questionattempts as $qa) { - $flagged = question_attempt::get_submitted_var( + $flagged = $qa->get_submitted_var( $qa->get_flag_field_name(), PARAM_BOOL, $postdata); if (!is_null($flagged) && $flagged != $qa->is_flagged()) { $qa->set_flagged($flagged); @@ -1342,8 +1342,8 @@ class question_attempt { const PARAM_MARK = 'parammark'; /** - * @var string special value used by manual grading because {@link PARAM_NUMBER} - * converts '' to 0. + * @var string special value to indicate a response variable that is uploaded + * files. */ const PARAM_FILES = 'paramfiles'; @@ -1710,18 +1710,28 @@ class question_attempt { $this->usageid, $this->slot, $file->get_itemid())) . - $file->get_filepath() . $file->get_filename()); + $file->get_filepath() . $file->get_filename(), true); } /** - * Get the URL of a file that belongs to a response variable of this - * question_attempt. - * @param stored_file $file the file to link to. + * Prepare a draft file are for the files belonging the a response variable + * of this question attempt. The draft area is populated with the files from + * the most recent step having files. + * + * @param string $name the variable name the files belong to. + * @param int $contextid the id of the context the quba belongs to. * @return int the draft itemid. */ public function prepare_response_files_draft_itemid($name, $contextid) { - $draftid = file_get_submitted_draft_itemid($this->get_qt_field_name($name)); - file_prepare_draft_area($draftid, $contextid, 'question', 'response_' . $name, $this->id); + foreach ($this->get_reverse_step_iterator() as $step) { + if ($step->has_qt_var($name)) { + return $step->prepare_response_files_draft_itemid($name, $contextid); + } + } + + // No files yet. + $draftid = 0; // Will be filled in by file_prepare_draft_area. + file_prepare_draft_area($draftid, $contextid, 'question', 'response_' . $name, null); return $draftid; } @@ -1800,7 +1810,7 @@ class question_attempt { * {@link get_fraction()} * {@link get_max_mark()}. */ public function get_current_manual_mark() { - $mark = self::get_submitted_var($this->get_behaviour_field_name('mark'), question_attempt::PARAM_MARK); + $mark = $this->get_submitted_var($this->get_behaviour_field_name('mark'), question_attempt::PARAM_MARK); if (is_null($mark)) { return $this->get_mark(); } else { @@ -2038,17 +2048,21 @@ class question_attempt { * data from this array, instead of from $_POST. * @return mixed the requested value. */ - public static function get_submitted_var($name, $type, $postdata = null) { + public function get_submitted_var($name, $type, $postdata = null) { // Special case to work around PARAM_NUMBER converting '' to 0. if ($type == self::PARAM_MARK) { - $mark = self::get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata); + $mark = $this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata); if ($mark === '') { return $mark; } else { - return self::get_submitted_var($name, PARAM_NUMBER, $postdata); + return $this->get_submitted_var($name, PARAM_NUMBER, $postdata); } } + if ($type == self::PARAM_FILES) { + return $this->process_response_files($name, $postdata); + } + if (is_null($postdata)) { $var = optional_param($name, null, $type); } else if (array_key_exists($name, $postdata)) { @@ -2064,6 +2078,37 @@ class question_attempt { return $var; } + /** + * Handle a submitted variable representing uploaded files. + * @param string $name the field name. + * @param array $postdata (optional, only inteded for testing use) take the + * data from this array, instead of from $_POST. At the moment, this + * behaves as if there were no files. + */ + protected function process_response_files($name, $postdata = null) { + global $USER; + + if ($postdata) { + // There can be no files with test data (at the moment). + return null; + } + + $draftitemid = file_get_submitted_draft_itemid($name); + if (!$draftitemid) { + return null; + } + + $fs = get_file_storage(); + $usercontext = get_context_instance(CONTEXT_USER, $USER->id); + + if ($fs->is_area_empty($usercontext->id, 'user', 'draft', $draftitemid)) { + return null; + } + + return new question_file_saver($draftitemid, 'question', 'response_' . + str_replace($this->get_field_prefix(), '', $name)); + } + /** * Get any data from the request that matches the list of expected params. * @param array $expected variable name => PARAM_... constant. @@ -2073,7 +2118,7 @@ class question_attempt { protected function get_expected_data($expected, $postdata, $extraprefix) { $submitteddata = array(); foreach ($expected as $name => $type) { - $value = self::get_submitted_var( + $value = $this->get_submitted_var( $this->get_field_prefix() . $extraprefix . $name, $type, $postdata); if (!is_null($value)) { $submitteddata[$extraprefix . $name] = $value; @@ -2576,6 +2621,10 @@ class question_attempt_step { } } + public function get_id() { + return $this->id; // TODO get rid of this. + } + /** @return question_state The state after this step. */ public function get_state() { return $this->state; @@ -2670,6 +2719,21 @@ class question_attempt_step { return $this->files[$name]; } + /** + * Prepare a draft file are for the files belonging the a response variable + * of this step. + * + * @param string $name the variable name the files belong to. + * @param int $contextid the id of the context the quba belongs to. + * @return int the draft itemid. + */ + public function prepare_response_files_draft_itemid($name, $contextid) { + $draftid = 0; // Will be filled in by file_prepare_draft_area. + file_prepare_draft_area($draftid, $contextid, 'question', + 'response_' . $name, $this->id); + return $draftid; + } + /** * Get all the question type variables. * @param array name => value pairs. @@ -2748,7 +2812,7 @@ class question_attempt_step { } /** - * Get all the data. behaviour variables have the ! at the start of + * Get all the data. behaviour variables have the - at the start of * their name. This is only intended for internal use, for example by * {@link question_engine_data_mapper::insert_question_attempt_step()}, * however, it can ocasionally be useful in test code. It should not be diff --git a/question/engine/simpletest/testquestionattempt.php b/question/engine/simpletest/testquestionattempt.php index ea11aca2861..e3ea8d1760b 100644 --- a/question/engine/simpletest/testquestionattempt.php +++ b/question/engine/simpletest/testquestionattempt.php @@ -105,27 +105,27 @@ class question_attempt_test extends UnitTestCase { } public function test_get_submitted_var_not_present_var_returns_null() { - $this->assertNull(question_attempt::get_submitted_var( + $this->assertNull($this->qa->get_submitted_var( 'reallyunlikelyvariablename', PARAM_BOOL)); } public function test_get_submitted_var_param_mark_not_present() { - $this->assertNull(question_attempt::get_submitted_var( + $this->assertNull($this->qa->get_submitted_var( 'name', question_attempt::PARAM_MARK, array())); } public function test_get_submitted_var_param_mark_blank() { - $this->assertIdentical('', question_attempt::get_submitted_var( + $this->assertIdentical('', $this->qa->get_submitted_var( 'name', question_attempt::PARAM_MARK, array('name' => ''))); } public function test_get_submitted_var_param_mark_number() { - $this->assertIdentical(123.0, question_attempt::get_submitted_var( + $this->assertIdentical(123.0, $this->qa->get_submitted_var( 'name', question_attempt::PARAM_MARK, array('name' => '123'))); } public function test_get_submitted_var_param_mark_invalid() { - $this->assertIdentical(0.0, question_attempt::get_submitted_var( + $this->assertIdentical(0.0, $this->qa->get_submitted_var( 'name', question_attempt::PARAM_MARK, array('name' => 'frog'))); } } diff --git a/question/type/essay/renderer.php b/question/type/essay/renderer.php index ab8ae17c1c3..1d05442b6d8 100644 --- a/question/type/essay/renderer.php +++ b/question/type/essay/renderer.php @@ -49,7 +49,7 @@ class qtype_essay_renderer extends qtype_renderer { $response, $question->responsefieldlines); } else { - $answer =$responseoutput->response_area_read_only($inputname, + $answer = $responseoutput->response_area_read_only($inputname, $response, $question->responsefieldlines); } @@ -107,7 +107,9 @@ class qtype_essay_renderer extends qtype_renderer { 'attachments', $options->context->id); $pickeroptions->context = $options->context; - return form_filemanager_render($pickeroptions); + return form_filemanager_render($pickeroptions) . html_writer::empty_tag( + 'input', array('type' => 'hidden', 'name' => $qa->get_qt_field_name('attachments'), + 'value' => $pickeroptions->itemid)); } /**