diff --git a/question/engine/datalib.php b/question/engine/datalib.php index 79506b12ef5..90c0da56b03 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -159,7 +159,7 @@ class question_engine_data_mapper { $record = new stdClass(); $record->questionattemptid = $questionattemptid; $record->sequencenumber = $seq; - $record->state = $step->get_state() ?? $step->get_state()->__toString() : null; + $record->state = $step->get_state() ? $step->get_state()->__toString() : null; $record->fraction = $step->get_fraction(); $record->timecreated = $step->get_timecreated(); $record->userid = $step->get_user_id(); diff --git a/question/engine/states.php b/question/engine/states.php index e48f0d553b0..f13d700175a 100644 --- a/question/engine/states.php +++ b/question/engine/states.php @@ -111,10 +111,23 @@ abstract class question_state { } /** + * Get the instance of this class for a given state name. + * * @param string $name a state name. - * @return question_state the state with that name. + * @return question_state|null the state with that name. (Null only in an exceptional case.) */ - public static function get($name) { + public static function get(string $name): ?question_state { + // In the past, there was a bug where null states got stored + // in the database as an empty string, which was wrong because + // the state column should be NOT NULL. + // That is no longer possible, but we need to avoid exceptions + // for people with old bad data in their database. + if ($name === '') { + debugging('Attempt to create a state from an empty string. ' . + 'This is probably a sign of bad data in your database. See MDL-80127.'); + return null; + } + return self::$$name; } diff --git a/question/engine/tests/questionstate_test.php b/question/engine/tests/questionstate_test.php index 85178ac8043..332fac84fa0 100644 --- a/question/engine/tests/questionstate_test.php +++ b/question/engine/tests/questionstate_test.php @@ -31,6 +31,7 @@ require_once($CFG->libdir . '/questionlib.php'); * @category test * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \question_state */ class questionstate_test extends \advanced_testcase { public function test_is_active() { @@ -154,4 +155,14 @@ class questionstate_test extends \advanced_testcase { $this->assertEquals(question_state::$mangrright, question_state::$gradedpartial->corresponding_commented_state(1)); } + + public function test_get(): void { + $this->assertEquals(question_state::$todo, question_state::get('todo')); + } + + public function test_get_bad_data(): void { + question_state::get(''); + $this->assertDebuggingCalled('Attempt to create a state from an empty string. ' . + 'This is probably a sign of bad data in your database. See MDL-80127.'); + } }