diff --git a/question/engine/datalib.php b/question/engine/datalib.php index 26af89ade9f..7d5f1804202 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -100,10 +100,7 @@ class question_engine_data_mapper { $stepdata[] = $this->insert_question_attempt($qa, $quba->get_owning_context()); } - $stepdata = call_user_func_array('array_merge', $stepdata); - if ($stepdata) { - $this->insert_all_step_data($stepdata); - } + $this->insert_all_step_data($this->combine_step_data($stepdata)); $quba->set_observer(new question_engine_unit_of_work($quba)); } @@ -150,7 +147,7 @@ class question_engine_data_mapper { $stepdata[] = $this->insert_question_attempt_step($step, $record->id, $seq, $context); } - return call_user_func_array('array_merge', $stepdata); + return $this->combine_step_data($stepdata); } /** @@ -171,6 +168,22 @@ class question_engine_data_mapper { return $record; } + /** + * Take an array of arrays, and flatten it, even if the outer array is empty. + * + * Only public so it can be called from the unit of work. Not part of the + * public API of this class. + * + * @param array $stepdata array of zero or more arrays. + * @return array made by concatenating all the separate arrays. + */ + public function combine_step_data(array $stepdata): array { + if (empty($stepdata)) { + return []; + } + return call_user_func_array('array_merge', $stepdata); + } + /** * Helper method used by insert_question_attempt_step and update_question_attempt_step * @param question_attempt_step $step the step to store. @@ -1581,9 +1594,7 @@ class question_engine_unit_of_work implements question_usage_observer { $dm->update_questions_usage_by_activity($this->quba); } - if ($stepdata) { - $dm->insert_all_step_data(call_user_func_array('array_merge', $stepdata)); - } + $dm->insert_all_step_data($dm->combine_step_data($stepdata)); $this->stepsdeleted = array(); $this->stepsmodified = array(); diff --git a/question/engine/tests/datalib_test.php b/question/engine/tests/datalib_test.php index 42b938e03d8..d00d260759a 100644 --- a/question/engine/tests/datalib_test.php +++ b/question/engine/tests/datalib_test.php @@ -230,4 +230,25 @@ class question_engine_data_mapper_testcase extends qbehaviour_walkthrough_test_b $this->assertEquals(0, $DB->count_records('question_attempts') - $initialqarows); $this->assertEquals(2, $DB->count_records('question_attempt_steps') - $initialqasrows); } + + /** + * Test that database operations on an empty usage work without errors. + */ + public function test_save_and_load_an_empty_usage() { + $this->resetAfterTest(); + + // Create a new usage. + $quba = question_engine::make_questions_usage_by_activity('test', context_system::instance()); + $quba->set_preferred_behaviour('deferredfeedback'); + + // Save it. + question_engine::save_questions_usage_by_activity($quba); + + // Reload it. + $reloadedquba = question_engine::load_questions_usage_by_activity($quba->get_id()); + $this->assertCount(0, $quba->get_slots()); + + // Delete it. + question_engine::delete_questions_usage_by_activity($quba->get_id()); + } }