From 8aa860d7b73c06d7a13e0e7ff3f4abc960c3a363 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Wed, 19 Jun 2024 11:22:20 +0100 Subject: [PATCH 1/2] MDL-82220 backup: ensure correct group/ing custom fields are included. We should include only the custom field values that belong to those groups and/or groupings that are being backed up. --- backup/moodle2/backup_stepslib.php | 39 +++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index e5496e40874..c5923905dca 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -1409,14 +1409,19 @@ class backup_groups_structure_step extends backup_structure_step { FROM {groups} g JOIN {backup_ids_temp} bi ON g.id = bi.itemid WHERE bi.backupid = ? - AND bi.itemname = 'groupfinal'", array(backup::VAR_BACKUPID)); + AND bi.itemname = 'groupfinal'", + [backup_helper::is_sqlparam($this->get_backupid())] + ); $grouping->set_source_sql(" SELECT g.* FROM {groupings} g JOIN {backup_ids_temp} bi ON g.id = bi.itemid WHERE bi.backupid = ? - AND bi.itemname = 'groupingfinal'", array(backup::VAR_BACKUPID)); + AND bi.itemname = 'groupingfinal'", + [backup_helper::is_sqlparam($this->get_backupid())] + ); + $groupinggroup->set_source_table('groupings_groups', array('groupingid' => backup::VAR_PARENTID)); // This only happens if we are including users. @@ -1424,9 +1429,17 @@ class backup_groups_structure_step extends backup_structure_step { $member->set_source_table('groups_members', array('groupid' => backup::VAR_PARENTID)); } - $courseid = $this->task->get_courseid(); - $groupcustomfield->set_source_array($this->get_group_custom_fields_for_backup($courseid)); - $groupingcustomfield->set_source_array($this->get_grouping_custom_fields_for_backup($courseid)); + $groupcustomfieldarray = $this->get_group_custom_fields_for_backup( + $group->get_source_sql(), + [$this->get_backupid()] + ); + $groupcustomfield->set_source_array($groupcustomfieldarray); + + $groupingcustomfieldarray = $this->get_grouping_custom_fields_for_backup( + $grouping->get_source_sql(), + [$this->get_backupid()] + ); + $groupingcustomfield->set_source_array($groupingcustomfieldarray); } // Define id annotations (as final) @@ -1445,14 +1458,16 @@ class backup_groups_structure_step extends backup_structure_step { /** * Get custom fields array for group - * @param int $courseid + * + * @param string $groupsourcesql + * @param array $groupsourceparams * @return array */ - protected function get_group_custom_fields_for_backup(int $courseid): array { + protected function get_group_custom_fields_for_backup(string $groupsourcesql, array $groupsourceparams): array { global $DB; $handler = \core_group\customfield\group_handler::create(); $fieldsforbackup = []; - if ($groups = $DB->get_records('groups', ['courseid' => $courseid], '', 'id')) { + if ($groups = $DB->get_records_sql($groupsourcesql, $groupsourceparams)) { foreach ($groups as $group) { $fieldsforbackup = array_merge($fieldsforbackup, $handler->get_instance_data_for_backup($group->id)); } @@ -1462,14 +1477,16 @@ class backup_groups_structure_step extends backup_structure_step { /** * Get custom fields array for grouping - * @param int $courseid + * + * @param string $groupingsourcesql + * @param array $groupingsourceparams * @return array */ - protected function get_grouping_custom_fields_for_backup(int $courseid): array { + protected function get_grouping_custom_fields_for_backup(string $groupingsourcesql, array $groupingsourceparams): array { global $DB; $handler = \core_group\customfield\grouping_handler::create(); $fieldsforbackup = []; - if ($groupings = $DB->get_records('groupings', ['courseid' => $courseid], '', 'id')) { + if ($groupings = $DB->get_records_sql($groupingsourcesql, $groupingsourceparams)) { foreach ($groupings as $grouping) { $fieldsforbackup = array_merge($fieldsforbackup, $handler->get_instance_data_for_backup($grouping->id)); } From ddd99c03d225680653c19ac013f1242cb74663ec Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Wed, 19 Jun 2024 14:03:25 +0100 Subject: [PATCH 2/2] MDL-82220 group: check valid mapping when restoring custom fields. --- backup/moodle2/restore_stepslib.php | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 39bdb0030b2..7f29ae798ae 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -1235,9 +1235,11 @@ class restore_groups_structure_step extends restore_structure_step { */ public function process_groupcustomfield($data) { $newgroup = $this->get_mapping('group', $data['groupid']); - $data['groupid'] = $newgroup->newitemid ?? $data['groupid']; - $handler = \core_group\customfield\group_handler::create(); - $handler->restore_instance_data_from_backup($this->task, $data); + if ($newgroup && $newgroup->newitemid) { + $data['groupid'] = $newgroup->newitemid; + $handler = \core_group\customfield\group_handler::create(); + $handler->restore_instance_data_from_backup($this->task, $data); + } } public function process_grouping($data) { @@ -1291,10 +1293,12 @@ class restore_groups_structure_step extends restore_structure_step { * @return void */ public function process_groupingcustomfield($data) { - $newgroup = $this->get_mapping('grouping', $data['groupingid']); - $data['groupingid'] = $newgroup->newitemid ?? $data['groupingid']; - $handler = \core_group\customfield\grouping_handler::create(); - $handler->restore_instance_data_from_backup($this->task, $data); + $newgrouping = $this->get_mapping('grouping', $data['groupingid']); + if ($newgrouping && $newgrouping->newitemid) { + $data['groupingid'] = $newgrouping->newitemid; + $handler = \core_group\customfield\grouping_handler::create(); + $handler->restore_instance_data_from_backup($this->task, $data); + } } public function process_grouping_group($data) {