diff --git a/.upgradenotes/MDL-85975-2025092314370040.yml b/.upgradenotes/MDL-85975-2025092314370040.yml new file mode 100644 index 00000000000..263b753aca2 --- /dev/null +++ b/.upgradenotes/MDL-85975-2025092314370040.yml @@ -0,0 +1,17 @@ +issueNumber: MDL-85975 +notes: + core: + - message: > + `restore_qtype_plugin::unset_excluded_fields` now returns the modified + questiondata structure, + + in order to support structures that contain arrays. + + If your qtype plugin overrides + `restore_qtype_plugin::remove_excluded_question_data` without + + calling the parent method, you may need to modify your overridden method + to use the returned + + value. + type: fixed diff --git a/public/backup/moodle2/restore_qtype_plugin.class.php b/public/backup/moodle2/restore_qtype_plugin.class.php index 52b10c6a548..4b80a13a022 100644 --- a/public/backup/moodle2/restore_qtype_plugin.class.php +++ b/public/backup/moodle2/restore_qtype_plugin.class.php @@ -568,8 +568,7 @@ abstract class restore_qtype_plugin extends restore_plugin { foreach ($excludefields as $excludefield) { $pathparts = explode('/', ltrim($excludefield, '/')); - $data = $questiondata; - self::unset_excluded_fields($data, $pathparts); + $questiondata = self::unset_excluded_fields($questiondata, $pathparts); } return $questiondata; @@ -581,25 +580,60 @@ abstract class restore_qtype_plugin extends restore_plugin { * If any of the elements in the path is an array, this is called recursively on each element in the array to unset fields * in each child of the array. * - * @param stdClass|array $data The questiondata object, or a subsection of it. + * @param stdClass|array $data The questiondata structure, or a subsection of it. * @param array $pathparts The remaining elements in the path to the excluded field. - * @return void + * @return stdClass|array The $data structure with excluded fields removed. */ - private static function unset_excluded_fields(stdClass|array $data, array $pathparts): void { + private static function unset_excluded_fields(stdClass|array $data, array $pathparts): stdClass|array { $element = array_shift($pathparts); - if (!isset($data->{$element})) { - // This element is not present in the data structure, nothing to unset. - return; + $unset = false; + // Get the current element from the data structure. + if (is_object($data)) { + if (!property_exists($data, $element)) { + // This element is not present in the data structure, nothing to unset. + return $data; + } + $dataelement = $data->{$element}; + } else { // It's an array. + if (!array_key_exists($element, $data)) { + return $data; + } + $dataelement = $data[$element]; } - if (is_object($data->{$element})) { - self::unset_excluded_fields($data->{$element}, $pathparts); - } else if (is_array($data->{$element})) { - foreach ($data->{$element} as $item) { - self::unset_excluded_fields($item, $pathparts); + // Check if we need to recur, or unset this element. + if (is_object($dataelement)) { + $dataelement = self::unset_excluded_fields($dataelement, $pathparts); + } else if (is_array($dataelement)) { + foreach ($dataelement as $key => $item) { + if (is_object($item) || is_array($item)) { + // This is an array of objects or arrays, recur. + $dataelement[$key] = self::unset_excluded_fields($item, $pathparts); + } else { + // This is an associative array of values, check if they should be removed. + $subelement = reset($pathparts); + if ($key == $subelement) { + unset($dataelement[$key]); + } + } } } else if (empty($pathparts)) { // This is the last element of the path and it's a scalar value, unset it. - unset($data->{$element}); + $unset = true; } + // Write the modified element back to the data structure, or unset it. + if (is_object($data)) { + if ($unset) { + unset($data->{$element}); + } else { + $data->{$element} = $dataelement; + } + } else { + if ($unset) { + unset($data[$element]); + } else { + $data[$element] = $dataelement; + } + } + return $data; } } diff --git a/public/backup/moodle2/tests/restore_qtype_plugin_test.php b/public/backup/moodle2/tests/restore_qtype_plugin_test.php new file mode 100644 index 00000000000..527763281fe --- /dev/null +++ b/public/backup/moodle2/tests/restore_qtype_plugin_test.php @@ -0,0 +1,147 @@ +. + +namespace core; + +/** + * Tests for question type restore methods + * + * @package core + * @copyright 2025 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \restore_qtype_plugin + */ +final class restore_qtype_plugin_test extends \basic_testcase { + /** + * All default and specified fields should be removed from the provided data structure. + */ + public function test_remove_excluded_question_data(): void { + global $CFG; + require_once($CFG->dirroot . '/backup/moodle2/restore_plugin.class.php'); + require_once($CFG->dirroot . '/backup/moodle2/restore_qtype_plugin.class.php'); + $data = (object) [ + // Default excluded fields should be removed. + 'id' => 1, + 'createdby' => 2, + 'modifiedby' => 3, + // This field is not specified for removal, it should remain. + 'questiontext' => 'Some question text', + // Excluded paths that address an array should operate on all items in the array. + 'hints' => [ + (object) [ + 'id' => 4, + 'questionid' => 1, + // This field is not specified for removal. + 'text' => 'Lorem ipsum', + ], + (object) [ + 'id' => 5, + 'questionid' => 1, + 'text' => 'Lorem ipsum', + ], + ], + 'options' => [ // This is an array of arrays, rather than an array of objects. It should be handled the same. + [ + 'id' => 6, + 'questionid' => 1, + // This field is not specified for removal. + 'option' => true, + ], + [ + 'id' => 7, + 'questionid' => 1, + 'option' => false, + ], + [ + 'id' => 8, + 'questionid' => 1, + 'option' => false, + ], + ], + 'custom1' => 'Some custom text', + // This field is not specified for removal. + 'custom2' => 'Some custom text2', + // Fields specified for removal should be removed even if they contain null values. + 'custom3' => null, + 'customarray' => [ + (object) [ + // Null values should also be removed. + 'id' => null, + // This field is not specified for removal. + 'text' => 'Custom item text', + ], + (object) [ + 'id' => null, + 'text' => 'Custom item text2', + ], + ], + 'customstructure' => [ // This array contains scalar values, not a list of objects/arrays. + 'id' => null, + 'text' => 'Custom structure text', + 'number' => 1, + 'bool' => true, + ], + ]; + + $expecteddata = (object) [ + 'questiontext' => 'Some question text', + 'hints' => [ + (object) [ + 'text' => 'Lorem ipsum', + ], + (object) [ + 'text' => 'Lorem ipsum', + ], + ], + 'options' => [ + [ + 'option' => true, + ], + [ + 'option' => false, + ], + [ + 'option' => false, + ], + ], + 'custom2' => 'Some custom text2', + 'customarray' => [ + (object) [ + 'text' => 'Custom item text', + ], + (object) [ + 'text' => 'Custom item text2', + ], + ], + 'customstructure' => [ + 'text' => 'Custom structure text', + 'number' => 1, + ], + ]; + + $excludedfields = [ + '/custom1', + '/custom3', + '/customarray/id', + '/customstructure/id', + '/customstructure/bool', + // A field that is not in the data structure will be ignored. + '/custom4', + ]; + $this->assertEquals($expecteddata, \restore_qtype_plugin::remove_excluded_question_data($data, $excludedfields)); + } +}