diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index dd4a95eca80..685d409632a 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -1491,6 +1491,10 @@ class mod_quiz_external extends external_api { $page = 'all'; } + // Make sure all users associated to the attempt steps are loaded. Otherwise, this will + // trigger a debugging message. + $attemptobj->preload_all_attempt_step_users(); + // Prepare the output. $result = []; $result['attempt'] = $attemptobj->get_attempt(); diff --git a/question/engine/questionattemptstep.php b/question/engine/questionattemptstep.php index f8bddf68507..da1750a090e 100644 --- a/question/engine/questionattemptstep.php +++ b/question/engine/questionattemptstep.php @@ -87,10 +87,10 @@ class question_attempt_step { */ private $fraction = null; - /** @var integer the timestamp when this step was created. */ + /** @var int the timestamp when this step was created. */ private $timecreated; - /** @var integer the id of the user resonsible for creating this step. */ + /** @var int the id of the user responsible for creating this step. */ private $userid; /** @var array name => value pairs. The submitted data. */ @@ -99,7 +99,7 @@ class question_attempt_step { /** @var array name => array of {@see stored_file}s. Caches the contents of file areas. */ private $files = array(); - /** @var stdClass User information. */ + /** @var stdClass|null User information. */ private $user = null; /** @@ -107,12 +107,12 @@ class question_attempt_step { * normally created by {@see question_attempt} methods like * {@see question_attempt::process_action()}. * @param array $data the submitted data that defines this step. - * @param int $timestamp the time to record for the action. (If not given, use now.) - * @param int $userid the user to attribute the aciton to. (If not given, use the current user.) - * @param int $existingstepid if this step is going to replace an existing step + * @param int|null $timecreated the time to record for the action. (If not given, use now.) + * @param int|null $userid the user to attribute the aciton to. (If not given, use the current user.) + * @param int|null $existingstepid if this step is going to replace an existing step * (for example, during a regrade) this is the id of the previous step we are replacing. */ - public function __construct($data = array(), $timecreated = null, $userid = null, + public function __construct($data = [], $timecreated = null, $userid = null, $existingstepid = null) { global $USER; @@ -196,9 +196,13 @@ class question_attempt_step { /** * Return the full user object. * - * @return stdClass Get full user object. + * @return null|stdClass Get full user object. */ - public function get_user(): stdClass { + public function get_user(): ?stdClass { + if ($this->user === null) { + debugging('Attempt to access the step user before it was initialised. ' . + 'Did you forget to call question_usage_by_activity::preload_all_step_users() or similar?', DEBUG_DEVELOPER); + } return $this->user; } @@ -208,7 +212,7 @@ class question_attempt_step { * @return string full name of user. */ public function get_user_fullname(): string { - return fullname($this->user); + return fullname($this->get_user()); } /** @return int the timestamp when this step was created. */ diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index c1aa1542014..798e369d738 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -1030,9 +1030,8 @@ class question_usage_by_activity { // Update user information for steps. foreach ($this->questionattempts as $qa) { foreach ($qa->get_full_step_iterator() as $step) { - $user = $users[$step->get_user_id()]; - if (isset($user)) { - $step->add_full_user_object($user); + if (isset($users[$step->get_user_id()])) { + $step->add_full_user_object($users[$step->get_user_id()]); } } } diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 402e2c10b5b..f83d9697a41 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -791,11 +791,13 @@ class question_no_pattern_expectation { abstract class qbehaviour_walkthrough_test_base extends question_testcase { /** @var question_display_options */ protected $displayoptions; + /** @var question_usage_by_activity */ protected $quba; - /** @var integer */ + /** @var int The slot number of the question_attempt we are using in $quba. */ protected $slot; + /** * @var string after {@link render()} has been called, this contains the * display of the question in its current state. @@ -804,7 +806,8 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { protected function setUp(): void { parent::setUp(); - $this->resetAfterTest(true); + $this->resetAfterTest(); + $this->setAdminUser(); $this->displayoptions = new question_display_options(); $this->quba = question_engine::make_questions_usage_by_activity('unit_test', @@ -914,6 +917,7 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { * $this->currentoutput so that it can be verified. */ protected function render() { + $this->quba->preload_all_step_users(); $this->currentoutput = $this->quba->render_question($this->slot, $this->displayoptions); } @@ -1005,9 +1009,9 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { * @param $condition one or more Expectations. (users varargs). */ protected function check_current_output() { - $html = $this->quba->render_question($this->slot, $this->displayoptions); + $this->render(); foreach (func_get_args() as $condition) { - $this->assert($condition, $html); + $this->assert($condition, $this->currentoutput); } } @@ -1019,9 +1023,9 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { * @param question_contains_select_expectation $expectations One or more expectations. */ protected function check_output_contains_selectoptions(...$expectations) { - $html = $this->quba->render_question($this->slot, $this->displayoptions); + $this->render(); foreach ($expectations as $expectation) { - $this->assert_select_options($expectation, $html); + $this->assert_select_options($expectation, $this->currentoutput); } } diff --git a/question/engine/tests/walkthrough_test.php b/question/engine/tests/walkthrough_test.php index a834b003aa4..678f7a57d33 100644 --- a/question/engine/tests/walkthrough_test.php +++ b/question/engine/tests/walkthrough_test.php @@ -107,6 +107,7 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { $displayoptions->userinfoinhistory = question_display_options::SHOW_ALL; $this->load_quba(); + $this->quba->preload_all_step_users(); $result = $this->quba->render_question($this->slot, $displayoptions); $numsteps = $this->quba->get_question_attempt($this->slot)->get_num_steps(); @@ -121,6 +122,13 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { $this->load_quba(); $result = $this->quba->render_question($this->slot, $displayoptions); + $message = 'Attempt to access the step user before it was initialised.'; + $message .= ' Did you forget to call question_usage_by_activity::preload_all_step_users() or similar?'; + $this->assertDebuggingCalled($message, DEBUG_DEVELOPER); + $this->resetDebugging(); + $this->quba->preload_all_step_users(); + $result = $this->quba->render_question($this->slot, $displayoptions); + $this->assertDebuggingNotCalled(); // The step just show the user profile link if the step's userid is different with student id. preg_match_all("/(.*)<\/a>/", $result, $matches);