From 966b264cf37aa8edd8857f74af712502c38a8fd3 Mon Sep 17 00:00:00 2001 From: Simon Adams Date: Mon, 14 Oct 2024 10:35:51 +0100 Subject: [PATCH] MDL-71378 mod_qbank: Add question bank backup/restore and refactor tests --- admin/settings/courses.php | 3 - backup/moodle2/backup_activity_task.class.php | 4 - backup/moodle2/backup_course_task.class.php | 4 - backup/moodle2/backup_root_task.class.php | 4 - backup/moodle2/restore_stepslib.php | 203 +++++----- backup/moodle2/tests/moodle2_test.php | 25 +- .../tests/quiz_restore_decode_links_test.php | 3 +- .../dbops/backup_controller_dbops.class.php | 4 - .../dbops/restore_controller_dbops.class.php | 2 - backup/util/dbops/restore_dbops.class.php | 254 ++++++------ .../behat/restore_moodle2_courses.feature | 1 - lang/en/backup.php | 10 +- lang/en/deprecated.txt | 4 + lang/en/question.php | 1 + lib/db/upgrade.php | 8 + lib/tests/questionlib_test.php | 103 +++-- .../backup_qbank_activity_task.class.php | 12 +- .../restore_qbank_activity_task.class.php | 6 +- .../seb/tests/test_helper_trait.php | 6 +- mod/quiz/tests/quiz_question_restore_test.php | 46 ++- mod/quiz/tests/restore_attempt_test.php | 10 +- mod/quiz/tests/tags_test.php | 17 +- .../bank/exporttoxml/tests/helper_test.php | 17 +- question/tests/backup_test.php | 367 ++++++++++++++++-- .../fixtures/question_category_45_format.mbz | Bin 0 -> 6377 bytes question/tests/generator/lib.php | 30 +- question/tests/privacy/provider_test.php | 32 +- question/type/essay/tests/restore_test.php | 15 +- tag/tests/event/events_test.php | 8 +- 29 files changed, 772 insertions(+), 427 deletions(-) create mode 100644 question/tests/fixtures/question_category_45_format.mbz diff --git a/admin/settings/courses.php b/admin/settings/courses.php index d1776633a5f..a94d51f60bc 100644 --- a/admin/settings/courses.php +++ b/admin/settings/courses.php @@ -415,7 +415,6 @@ if ($hassiteconfig or has_any_capability($capabilities, $systemcontext)) { $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_userscompletion', new lang_string('generaluserscompletion','backup'), new lang_string('configgeneraluserscompletion','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_logs', new lang_string('generallogs','backup'), new lang_string('configgenerallogs','backup'), array('value'=>0, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_histories', new lang_string('generalhistories','backup'), new lang_string('configgeneralhistories','backup'), array('value'=>0, 'locked'=>0))); - $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_questionbank', new lang_string('generalquestionbank','backup'), new lang_string('configgeneralquestionbank','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_groups', new lang_string('generalgroups', 'backup'), new lang_string('configgeneralgroups', 'backup'), array('value' => 1, 'locked' => 0))); @@ -455,7 +454,6 @@ if ($hassiteconfig or has_any_capability($capabilities, $systemcontext)) { $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_import_blocks', new lang_string('generalblocks','backup'), new lang_string('configgeneralblocks','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_import_filters', new lang_string('generalfilters','backup'), new lang_string('configgeneralfilters','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_import_calendarevents', new lang_string('generalcalendarevents','backup'), new lang_string('configgeneralcalendarevents','backup'), array('value'=>1, 'locked'=>0))); - $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_import_questionbank', new lang_string('generalquestionbank','backup'), new lang_string('configgeneralquestionbank','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_import_groups', new lang_string('generalgroups', 'backup'), new lang_string('configgeneralgroups', 'backup'), array('value' => 1, 'locked' => 0))); @@ -585,7 +583,6 @@ if ($hassiteconfig or has_any_capability($capabilities, $systemcontext)) { $temp->add(new admin_setting_configcheckbox('backup/backup_auto_userscompletion', new lang_string('generaluserscompletion','backup'), new lang_string('configgeneraluserscompletion','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_logs', new lang_string('generallogs', 'backup'), new lang_string('configgenerallogs', 'backup'), 0)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_histories', new lang_string('generalhistories','backup'), new lang_string('configgeneralhistories','backup'), 0)); - $temp->add(new admin_setting_configcheckbox('backup/backup_auto_questionbank', new lang_string('generalquestionbank','backup'), new lang_string('configgeneralquestionbank','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_groups', new lang_string('generalgroups', 'backup'), new lang_string('configgeneralgroups', 'backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_competencies', new lang_string('generalcompetencies','backup'), new lang_string('configgeneralcompetencies','backup'), 1)); diff --git a/backup/moodle2/backup_activity_task.class.php b/backup/moodle2/backup_activity_task.class.php index c701cfbd6ce..9f3c632deae 100644 --- a/backup/moodle2/backup_activity_task.class.php +++ b/backup/moodle2/backup_activity_task.class.php @@ -286,10 +286,6 @@ abstract class backup_activity_task extends backup_task { // All these are common settings to be shared by all activities. $activityincluded = $this->add_activity_included_setting($settingprefix); - if (question_module_uses_questions($this->modulename)) { - $questionbank = $this->plan->get_setting('questionbank'); - $questionbank->add_dependency($activityincluded); - } $this->add_activity_userinfo_setting($settingprefix, $activityincluded); diff --git a/backup/moodle2/backup_course_task.class.php b/backup/moodle2/backup_course_task.class.php index 4f27232382e..09f414f2bbf 100644 --- a/backup/moodle2/backup_course_task.class.php +++ b/backup/moodle2/backup_course_task.class.php @@ -96,10 +96,6 @@ class backup_course_task extends backup_task { $this->add_step(new backup_annotate_groups_from_groupings('annotate_groups_from_groupings')); } - // Annotate the question_categories belonging to the course context (conditionally). - if ($this->get_setting_value('questionbank')) { - $this->add_step(new backup_calculate_question_categories('course_question_categories')); - } // Generate the roles file (optionally role assignments and always role overrides) $this->add_step(new backup_roles_structure_step('course_roles', 'roles.xml')); diff --git a/backup/moodle2/backup_root_task.class.php b/backup/moodle2/backup_root_task.class.php index 221aae42a9f..aa30a9d3015 100644 --- a/backup/moodle2/backup_root_task.class.php +++ b/backup/moodle2/backup_root_task.class.php @@ -166,10 +166,6 @@ class backup_root_task extends backup_task { // So let's define a dependency to prevent false expectations from our users. $activities->add_dependency($gradehistories); - // Define question bank inclusion setting. - $questionbank = new backup_generic_setting('questionbank', base_setting::IS_BOOLEAN, true); - $questionbank->set_ui(new backup_setting_ui_checkbox($questionbank, get_string('rootsettingquestionbank', 'backup'))); - $this->add_setting($questionbank); $groups = new backup_groups_setting('groups', base_setting::IS_BOOLEAN, true); $groups->set_ui(new backup_setting_ui_checkbox($groups, get_string('rootsettinggroups', 'backup'))); diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 5ad4a07cc02..41ad4e4ed72 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -5107,23 +5107,23 @@ class restore_create_categories_and_questions extends restore_structure_step { } $data->contextid = $mapping->parentitemid; + $context = \context::instance_by_id($data->contextid); + // Before 3.5, question categories could be created at top level. // From 3.5 onwards, all question categories should be a child of a special category called the "top" category. $restoretask = $this->get_task(); $before35 = $restoretask->backup_release_compare('3.5', '<') || $restoretask->backup_version_compare(20180205, '<'); + + // We need a 'Top' question category for an activity module and activity modules are mapped to CONTEXT_COURSE and moved + // to the correct module context in restore_move_module_questions_categories. + // As we can't create a 'Top' category in CONTEXT_COURSE we'll make a default + // qbank module and map it to that until they are created later. if (empty($mapping->info->parent) && $before35) { $top = question_get_top_category($data->contextid, true); $data->parent = $top->id; } - if (empty($data->parent)) { - if (!$top = question_get_top_category($data->contextid)) { - $top = question_get_top_category($data->contextid, true); - $this->set_mapping('question_category_created', $oldid, $top->id, false, null, $data->contextid); - } - $this->set_mapping('question_category', $oldid, $top->id); - } else { - + if (!empty($data->parent)) { // Before 3.1, the 'stamp' field could be erroneously duplicated. // From 3.1 onwards, there's a unique index of (contextid, stamp). // If we encounter a duplicate in an old restore file, just generate a new stamp. @@ -5388,20 +5388,19 @@ class restore_create_categories_and_questions extends restore_structure_step { if (core_tag_tag::is_enabled('core_question', 'question')) { $tagname = $data->rawname; - if (!empty($data->contextid) && $newcontextid = $this->get_mappingid('context', $data->contextid)) { - $tagcontextid = $newcontextid; - } else { - // Get the category, so we can then later get the context. - $categoryid = $this->get_new_parentid('question_category'); - if (empty($this->cachedcategory) || $this->cachedcategory->id != $categoryid) { - $this->cachedcategory = $DB->get_record('question_categories', array('id' => $categoryid)); - } - $tagcontextid = $this->cachedcategory->contextid; + // Get the category, so we can then later get the context. + $categoryid = $this->get_new_parentid('question_category'); + if (empty($this->cachedcategory) || $this->cachedcategory->id != $categoryid) { + $this->cachedcategory = $DB->get_record('question_categories', ['id' => $categoryid]); } + $tagcontextid = $this->cachedcategory->contextid; // Add the tag to the question. - core_tag_tag::add_item_tag('core_question', 'question', $newquestion, - context::instance_by_id($tagcontextid), - $tagname); + core_tag_tag::add_item_tag('core_question', + 'question', + $newquestion, + context::instance_by_id($tagcontextid), + $tagname + ); } } @@ -5473,90 +5472,112 @@ class restore_move_module_questions_categories extends restore_execution_step { $contexts = restore_dbops::restore_get_question_banks($this->get_restoreid(), CONTEXT_MODULE); foreach ($contexts as $contextid => $contextlevel) { + if (!$newcontext = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'context', $contextid)) { + // The bank for the question categories required by this module was not included in the backup, + // but if that context still exists on the site then don't move them. + if (context::instance_by_id($contextid, IGNORE_MISSING)) { + continue; + } + // We have no target question bank so create a default bank for categories without a module to attach to. + // This can occur when a quiz backup contains references to a question bank module, + // that was not included in the backup and does not exist in the site being restored to. + $course = get_course($this->get_courseid()); + $defaultqbank = core_question\local\bank\question_bank_helper::get_default_open_instance_system_type($course, true); + $context = context_module::instance($defaultqbank->id); + $newcontext = new stdClass(); + $newcontext->newitemid = $context->id; + } // Only if context mapping exists (i.e. the module has been restored) - if ($newcontext = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'context', $contextid)) { - // Update all the qcats having their parentitemid set to the original contextid - $modulecats = $DB->get_records_sql("SELECT itemid, newitemid, info - FROM {backup_ids_temp} - WHERE backupid = ? - AND itemname = 'question_category' - AND parentitemid = ?", array($this->get_restoreid(), $contextid)); - $top = question_get_top_category($newcontext->newitemid, true); - $oldtopid = 0; - $categoryids = []; - foreach ($modulecats as $modulecat) { - // Before 3.5, question categories could be created at top level. - // From 3.5 onwards, all question categories should be a child of a special category called the "top" category. - $info = backup_controller_dbops::decode_backup_temp_info($modulecat->info); - if ($after35 && empty($info->parent)) { - $oldtopid = $modulecat->newitemid; - $modulecat->newitemid = $top->id; - } else { - $cat = new stdClass(); - $cat->id = $modulecat->newitemid; - $cat->contextid = $newcontext->newitemid; - if (empty($info->parent)) { - $cat->parent = $top->id; - } - $DB->update_record('question_categories', $cat); - $categoryids[] = (int)$cat->id; + // Update all the qcats having their parentitemid set to the original contextid. + $modulecats = $DB->get_records_sql("SELECT itemid, newitemid, info + FROM {backup_ids_temp} + WHERE backupid = ? + AND itemname = 'question_category' + AND parentitemid = ?", + [$this->get_restoreid(), $contextid] + ); + $top = question_get_top_category($newcontext->newitemid, true); + $oldtopid = 0; + $categoryids = []; + foreach ($modulecats as $modulecat) { + // Before 3.5, question categories could be created at top level. + // From 3.5 onwards, all question categories should be a child of a special category called the "top" category. + $info = backup_controller_dbops::decode_backup_temp_info($modulecat->info); + if ($after35 && empty($info->parent)) { + $oldtopid = $modulecat->newitemid; + $modulecat->newitemid = $top->id; + } else { + $cat = new stdClass(); + $cat->id = $modulecat->newitemid; + $cat->contextid = $newcontext->newitemid; + if (empty($info->parent)) { + $cat->parent = $top->id; } - - // And set new contextid (and maybe update newitemid) also in question_category mapping (will be - // used by {@link restore_create_question_files} later. - restore_dbops::set_backup_ids_record($this->get_restoreid(), 'question_category', $modulecat->itemid, - $modulecat->newitemid, $newcontext->newitemid); + $DB->update_record('question_categories', $cat); + $categoryids[] = (int) $cat->id; } - // Update the context id of any tags applied to any questions in these categories. - if ($categoryids) { - [$categorysql, $categoryidparams] = $DB->get_in_or_equal($categoryids, SQL_PARAMS_NAMED); - $sqlupdate = "UPDATE {tag_instance} - SET contextid = :newcontext - WHERE component = :component - AND itemtype = :itemtype - AND itemid IN (SELECT DISTINCT bi.newitemid as questionid - FROM {backup_ids_temp} bi - JOIN {question} q ON q.id = bi.newitemid - JOIN {question_versions} qv ON qv.questionid = q.id - JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid - WHERE bi.backupid = :backupid AND bi.itemname = 'question_created' - AND qbe.questioncategoryid {$categorysql}) "; - $params = [ + // And set new contextid (and maybe update newitemid) also in question_category mapping (will be + // used by {@see restore_create_question_files} later. + restore_dbops::set_backup_ids_record($this->get_restoreid(), + 'question_category', + $modulecat->itemid, + $modulecat->newitemid, + $newcontext->newitemid + ); + } + + // Update the context id of any tags applied to any questions in these categories. + if ($categoryids) { + [$categorysql, $categoryidparams] = $DB->get_in_or_equal($categoryids, SQL_PARAMS_NAMED); + $sqlupdate = "UPDATE {tag_instance} + SET contextid = :newcontext + WHERE component = :component + AND itemtype = :itemtype + AND itemid IN (SELECT DISTINCT bi.newitemid as questionid + FROM {backup_ids_temp} bi + JOIN {question} q ON q.id = bi.newitemid + JOIN {question_versions} qv ON qv.questionid = q.id + JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid + WHERE bi.backupid = :backupid AND bi.itemname = 'question_created' + AND qbe.questioncategoryid {$categorysql}) "; + $params = [ 'newcontext' => $newcontext->newitemid, 'component' => 'core_question', 'itemtype' => 'question', 'backupid' => $this->get_restoreid(), - ]; - $params += $categoryidparams; - $DB->execute($sqlupdate, $params); + ]; + $params += $categoryidparams; + $DB->execute($sqlupdate, $params); - // As explained in {@see restore_quiz_activity_structure_step::process_quiz_question_legacy_instance()} - // question_set_references relating to random questions restored from old backups, - // which pick from context_module question_categores, will have been restored with the wrong questioncontextid. - // So, now, we need to find those, and updated the questioncontextid. - // We can only find them by picking apart the filter conditions, and seeign which categories they refer to. + // As explained in {@see restore_quiz_activity_structure_step::process_quiz_question_legacy_instance()} + // question_set_references relating to random questions restored from old backups, + // which pick from context_module question_categores, will have been restored with the wrong questioncontextid. + // So, now, we need to find those, and updated the questioncontextid. + // We can only find them by picking apart the filter conditions, and seeign which categories they refer to. - // We need to check all the question_set_references belonging to this context_module. - $references = $DB->get_records('question_set_references', ['usingcontextid' => $newcontext->newitemid]); - foreach ($references as $reference) { - $filtercondition = json_decode($reference->filtercondition); - if (!empty($filtercondition->questioncategoryid) && - in_array($filtercondition->questioncategoryid, $categoryids)) { - // This is one of ours, update the questionscontextid. - $DB->set_field('question_set_references', - 'questionscontextid', $newcontext->newitemid, - ['id' => $reference->id]); - } + // We need to check all the question_set_references belonging to this context_module. + $references = $DB->get_records('question_set_references', ['usingcontextid' => $newcontext->newitemid]); + foreach ($references as $reference) { + $filtercondition = json_decode($reference->filtercondition); + if (!empty($filtercondition->questioncategoryid) && + in_array($filtercondition->questioncategoryid, $categoryids)) { + // This is one of ours, update the questionscontextid. + $DB->set_field('question_set_references', + 'questionscontextid', $newcontext->newitemid, + ['id' => $reference->id]); } } + } - // Now set the parent id for the question categories that were in the top category in the course context - // and have been moved now. - if ($oldtopid) { - $DB->set_field('question_categories', 'parent', $top->id, - array('contextid' => $newcontext->newitemid, 'parent' => $oldtopid)); - } + // Now set the parent id for the question categories that were in the top category in the course context + // and have been moved now. + if ($oldtopid) { + $DB->set_field('question_categories', + 'parent', + $top->id, + ['contextid' => $newcontext->newitemid, 'parent' => $oldtopid] + ); } } } diff --git a/backup/moodle2/tests/moodle2_test.php b/backup/moodle2/tests/moodle2_test.php index 940330ef471..6b37f926058 100644 --- a/backup/moodle2/tests/moodle2_test.php +++ b/backup/moodle2/tests/moodle2_test.php @@ -141,7 +141,7 @@ final class moodle2_test extends \advanced_testcase { backup::TARGET_NEW_COURSE); $thrown = null; try { - $this->assertTrue($rc->execute_precheck()); + $rc->execute_precheck(); $rc->execute_plan(); $rc->destroy(); } catch (Exception $e) { @@ -155,6 +155,11 @@ final class moodle2_test extends \advanced_testcase { $this->assertNull($thrown); + // Backup contained a question category in a deprecated context. + $results = $rc->get_precheck_results(); + $this->assertCount(1, $results['warnings']); + $this->assertStringStartsWith('The questions category', $results['warnings'][0]); + // Get information about the resulting course and check that it is set // up correctly. $modinfo = get_fast_modinfo($newcourseid); @@ -1040,13 +1045,24 @@ final class moodle2_test extends \advanced_testcase { backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id, backup::TARGET_NEW_COURSE); - $this->assertTrue($rc->execute_precheck()); + $rc->execute_precheck(); $rc->execute_plan(); $rc->destroy(); + // Backup contained question category(s) in a deprecated context. + $expectedwarnings = $backupid === 'question_category_34_format' ? 1 : 2; + $results = $rc->get_precheck_results(); + $this->assertCount($expectedwarnings, $results['warnings']); + for ($i = 0; $i < $expectedwarnings; $i++) { + $this->assertStringStartsWith('The questions category', $results['warnings'][$i]); + } + // Get information about the resulting course and check that it is set up correctly. $modinfo = get_fast_modinfo($newcourseid); $quizzes = array_values($modinfo->get_instances_of('quiz')); + $qbanks = $modinfo->get_instances_of('qbank'); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); $contexts = $quizzes[0]->context->get_parent_contexts(true); $topcategorycount = []; @@ -1055,6 +1071,11 @@ final class moodle2_test extends \advanced_testcase { // Make sure all question categories that were inside the backup file were restored correctly. if ($context->contextlevel == CONTEXT_COURSE) { + // Course context categories are deprecated and now get transferred to a qbank instance on the course + // at point of restore. + $cats = $DB->get_records('question_categories', + ['contextid' => $qbank->context->id], 'parent', 'id, name, parent' + ); $this->assertEquals(['top', 'Default for C101'], array_column($cats, 'name')); } else if ($context->contextlevel == CONTEXT_MODULE) { $this->assertEquals(['top', 'Default for Q1'], array_column($cats, 'name')); diff --git a/backup/tests/quiz_restore_decode_links_test.php b/backup/tests/quiz_restore_decode_links_test.php index 0cc42ef2f9e..b493c9478ba 100644 --- a/backup/tests/quiz_restore_decode_links_test.php +++ b/backup/tests/quiz_restore_decode_links_test.php @@ -49,11 +49,12 @@ class quiz_restore_decode_links_test extends \advanced_testcase { array('createsections' => true)); $quiz = $generator->create_module('quiz', array( 'course' => $course->id)); + $qbank = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); // Create questions. $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); - $context = \context_course::instance($course->id); + $context = \context_module::instance($qbank->cmid); $cat = $questiongenerator->create_question_category(array('contextid' => $context->id)); $question = $questiongenerator->create_question('multichoice', null, array('category' => $cat->id)); diff --git a/backup/util/dbops/backup_controller_dbops.class.php b/backup/util/dbops/backup_controller_dbops.class.php index 7950985cd12..5c14eb1a52e 100644 --- a/backup/util/dbops/backup_controller_dbops.class.php +++ b/backup/util/dbops/backup_controller_dbops.class.php @@ -569,7 +569,6 @@ abstract class backup_controller_dbops extends backup_dbops { 'backup_general_userscompletion' => 'userscompletion', 'backup_general_logs' => 'logs', 'backup_general_histories' => 'grade_histories', - 'backup_general_questionbank' => 'questionbank', 'backup_general_groups' => 'groups', 'backup_general_competencies' => 'competencies', 'backup_general_contentbankcontent' => 'contentbankcontent', @@ -586,7 +585,6 @@ abstract class backup_controller_dbops extends backup_dbops { 'backup_import_filters' => 'filters', 'backup_import_calendarevents' => 'calendarevents', 'backup_import_permissions' => 'permissions', - 'backup_import_questionbank' => 'questionbank', 'backup_import_groups' => 'groups', 'backup_import_competencies' => 'competencies', 'backup_import_contentbankcontent' => 'contentbankcontent', @@ -600,7 +598,6 @@ abstract class backup_controller_dbops extends backup_dbops { 'activities', 'blocks', 'filters', - 'questionbank' ); self::force_enable_settings($controller, $settings); } @@ -620,7 +617,6 @@ abstract class backup_controller_dbops extends backup_dbops { 'backup_auto_userscompletion' => 'userscompletion', 'backup_auto_logs' => 'logs', 'backup_auto_histories' => 'grade_histories', - 'backup_auto_questionbank' => 'questionbank', 'backup_auto_groups' => 'groups', 'backup_auto_competencies' => 'competencies', 'backup_auto_contentbankcontent' => 'contentbankcontent', diff --git a/backup/util/dbops/restore_controller_dbops.class.php b/backup/util/dbops/restore_controller_dbops.class.php index 91849434485..586fa13cd83 100644 --- a/backup/util/dbops/restore_controller_dbops.class.php +++ b/backup/util/dbops/restore_controller_dbops.class.php @@ -156,7 +156,6 @@ abstract class restore_controller_dbops extends restore_dbops { 'restore_general_userscompletion' => 'userscompletion', 'restore_general_logs' => 'logs', 'restore_general_histories' => 'grade_histories', - 'restore_general_questionbank' => 'questionbank', 'restore_general_groups' => 'groups', 'restore_general_competencies' => 'competencies', 'restore_general_contentbankcontent' => 'contentbankcontent', @@ -195,7 +194,6 @@ abstract class restore_controller_dbops extends restore_dbops { 'activities', 'blocks', 'filters', - 'questionbank' ); self::force_enable_settings($controller, $settings); }; diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 42f3d18511a..067f3822526 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -538,10 +538,11 @@ abstract class restore_dbops { * the target contexts where each bank will be restored and returning * warnings/errors as needed. * - * Some contextlevels (system, coursecat), will delegate process to - * course level if any problem is found (lack of permissions, non-matching - * target context...). Other contextlevels (course, module) will - * cause return error if some problem is found. + * Question categories at CONTEXT_SYSTEM, CONTEXT_COURSE, and CONTEXT_COURSECAT + * are now deprecated, but we still have to account for them in backup files + * made with pre-deprecated code. As such, any categories in backup files that used + * to target these contexts will now be attached to a 'fallback' qbank + * instance on the course being restored. * * At the end, if no errors were found, all the categories in backup_temp_ids * will be pointing (parentitemid) to the target context where they must be @@ -570,13 +571,8 @@ abstract class restore_dbops { global $DB; // To return any errors and warnings found - $errors = array(); - $warnings = array(); - - // Specify which fallbacks must be performed - $fallbacks = array( - CONTEXT_SYSTEM => CONTEXT_COURSE, - CONTEXT_COURSECAT => CONTEXT_COURSE); + $errors = []; + $warnings = []; /** @var restore_controller $rc */ $rc = restore_controller_dbops::load_controller($restoreid); @@ -584,24 +580,22 @@ abstract class restore_dbops { $after35 = $plan->backup_release_compare('3.5', '>=') && $plan->backup_version_compare(20180205, '>'); $rc->destroy(); // Always need to destroy. - // For any contextlevel, follow this process logic: - // - // 0) Iterate over each context (qbank) - // 1) Iterate over each qcat in the context, matching by stamp for the found target context - // 2a) No match, check if user can create qcat and q - // 3a) User can, mark the qcat and all dependent qs to be created in that target context - // 3b) User cannot, check if we are in some contextlevel with fallback - // 4a) There is fallback, move ALL the qcats to fallback, warn. End qcat loop - // 4b) No fallback, error. End qcat loop. - // 2b) Match, mark qcat to be mapped and iterate over each q, matching by stamp and version - // 5a) No match, check if user can add q - // 6a) User can, mark the q to be created - // 6b) User cannot, check if we are in some contextlevel with fallback - // 7a) There is fallback, move ALL the qcats to fallback, warn. End qcat loop - // 7b) No fallback, error. End qcat loop - // 5b) Random question, must always create new. - // 5c) Match, mark q to be mapped - // 8) Check if backup is from Moodle >= 3.5 and error if more than one top-level category in the context. + /* + For any contextlevel, follow this process logic: + + 0) Iterate over each context (qbank) + 1) Iterate over each qcat in the context, matching by stamp for the found target context + 2a) No match, check if user can create qcat and q + 3a) User can, mark the qcat and all dependent qs to be created in that target context + 3b) User cannot. Move ALL the qcats to a default qbank instance, warn. End qcat loop + 2b) Match, mark qcat to be mapped and iterate over each q, matching by stamp and version + 4a) No match, check if user can add q + 5a) User can, mark the q to be created + 5b) User cannot. Move ALL the qcats to a default qbank instance, warn. End qcat loop + 4b) Random question, must always create new. + 4c) Match, mark q to be mapped + 6) Check if backup is from Moodle >= 3.5 and error if more than one top-level category in the context. + */ // Get all the contexts (question banks) in restore for the given contextlevel $contexts = self::restore_get_question_banks($restoreid, $contextlevel); @@ -610,7 +604,7 @@ abstract class restore_dbops { foreach ($contexts as $contextid => $contextlevel) { // Init some perms $canmanagecategory = false; - $canadd = false; + $canadd = false; // Top-level category counter. $topcats = 0; // get categories in context (bank) @@ -619,7 +613,7 @@ abstract class restore_dbops { // cache permissions if $targetcontext is found if ($targetcontext = self::restore_find_best_target_context($categories, $courseid, $contextlevel)) { $canmanagecategory = has_capability('moodle/question:managecategory', $targetcontext, $userid); - $canadd = has_capability('moodle/question:add', $targetcontext, $userid); + $canadd = has_capability('moodle/question:add', $targetcontext, $userid); } // 1) Iterate over each qcat in the context, matching by stamp for the found target context foreach ($categories as $category) { @@ -629,9 +623,10 @@ abstract class restore_dbops { $matchcat = false; if ($targetcontext) { - $matchcat = $DB->get_record('question_categories', array( - 'contextid' => $targetcontext->id, - 'stamp' => $category->stamp)); + $matchcat = $DB->get_record('question_categories', [ + 'contextid' => $targetcontext->id, + 'stamp' => $category->stamp, + ]); } // 2a) No match, check if user can create qcat and q if (!$matchcat) { @@ -646,26 +641,29 @@ abstract class restore_dbops { } self::set_backup_ids_record($restoreid, 'question_category', $category->id, 0, $parentitemid); // Nothing else to mark, newitemid = 0 means create - - // 3b) User cannot, check if we are in some contextlevel with fallback } else { - // 4a) There is fallback, move ALL the qcats to fallback, warn. End qcat loop - if (array_key_exists($contextlevel, $fallbacks)) { - foreach ($categories as $movedcat) { - $movedcat->contextlevel = $fallbacks[$contextlevel]; - self::set_backup_ids_record($restoreid, 'question_category', $movedcat->id, 0, $contextid, $movedcat); - // Warn about the performed fallback - $warnings[] = get_string('qcategory2coursefallback', 'backup', $movedcat); - } - - // 4b) No fallback, error. End qcat loop. - } else { - $errors[] = get_string('qcategorycannotberestored', 'backup', $category); + // 3b) User cannot. Move ALL the qcats to the fallback i.e. a default qbank instance, warn. End qcat loop. + $course = get_course($courseid); + $course->fullname = get_string('courserestore', 'question'); + $module = + core_question\local\bank\question_bank_helper::get_default_open_instance_system_type($course, true); + $fallbackcontext = $module->context; + foreach ($categories as $movedcat) { + $movedcat->contextlevel = $contextlevel; + self::set_backup_ids_record($restoreid, + 'question_category', + $movedcat->id, + 0, + $fallbackcontext->id, + $movedcat + ); + // Warn about the performed fallback. + $warnings[] = get_string('qcategory2coursefallback', 'backup', $movedcat); } - break; // out from qcat loop (both 4a and 4b), we have decided about ALL categories in context (bank) + break; // Out from qcat loop (both 3a and 3b), we have decided about ALL categories in context (bank). } - // 2b) Match, mark qcat to be mapped and iterate over each q, matching by stamp and version + // 2b) Match, mark qcat to be mapped and iterate over each q, matching by stamp and version } else { self::set_backup_ids_record($restoreid, 'question_category', $category->id, $matchcat->id, $targetcontext->id); $questions = self::restore_get_questions($restoreid, $category->id); @@ -687,35 +685,42 @@ abstract class restore_dbops { } else { $matchqid = false; } - // 5a) No match, check if user can add q + // 4a) No match, check if user can add q if (!$matchqid) { - // 6a) User can, mark the q to be created + // 5a) User can, mark the q to be created if ($canadd) { // Nothing to mark, newitemid means create - - // 6b) User cannot, check if we are in some contextlevel with fallback } else { - // 7a) There is fallback, move ALL the qcats to fallback, warn. End qcat loo - if (array_key_exists($contextlevel, $fallbacks)) { - foreach ($categories as $movedcat) { - $movedcat->contextlevel = $fallbacks[$contextlevel]; - self::set_backup_ids_record($restoreid, 'question_category', $movedcat->id, 0, $contextid, $movedcat); - // Warn about the performed fallback - $warnings[] = get_string('question2coursefallback', 'backup', $movedcat); - } - - // 7b) No fallback, error. End qcat loop - } else { - $errors[] = get_string('questioncannotberestored', 'backup', $question); + // 5b) User cannot. + // Move ALL the qcats to the fallback i.e. a default qbank instance, warn. End qcat loop. + $course = get_course($courseid); + $course->fullname = get_string('courserestore', 'question'); + $module = core_question\local\bank\question_bank_helper::get_default_open_instance_system_type( + $course, + true + ); + $fallbackcontext = $module->context; + foreach ($categories as $movedcat) { + $movedcat->contextlevel = $contextlevel; + self::set_backup_ids_record($restoreid, + 'question_category', + $movedcat->id, + 0, + $fallbackcontext->id, + $movedcat + ); + // Warn about the performed fallback. + $warnings[] = get_string('question2coursefallback', 'backup', $movedcat); } - break 2; // out from qcat loop (both 7a and 7b), we have decided about ALL categories in context (bank) + // Out from qcat loop (both 5a and 5b), we have decided about ALL categories in context (bank). + break 2; } - // 5b) Random questions must always be newly created. + // 4b) Random questions must always be newly created. } else if ($question->qtype == 'random') { // Nothing to mark, newitemid means create - // 5c) Match, mark q to be mapped. + // 4c) Match, mark q to be mapped. } else { self::set_backup_ids_record($restoreid, 'question', $question->id, $matchqid); } @@ -723,14 +728,14 @@ abstract class restore_dbops { } } - // 8) Check if backup is made on Moodle >= 3.5 and there are more than one top-level category in the context. + // 6) Check if backup is made on Moodle >= 3.5 and there are more than one top-level category in the context. if ($after35 && $topcats > 1) { $errors[] = get_string('restoremultipletopcats', 'question', $contextid); } } - return array($errors, $warnings); + return [$errors, $warnings]; } /** @@ -798,80 +803,57 @@ abstract class restore_dbops { } /** - * Calculates the best context found to restore one collection of qcats, - * al them belonging to the same context (question bank), returning the - * target context found (object) or false + * Calculates the best existing context to restore one collection of qcats. + * Uses the backup category stamp to match the target category stamp + * and categories must all belong to the same context (question bank). + * + * @param array $categories categories to find target context for + * @param int $courseid course to restore to + * @param int $contextlevel contextlevel to search for the target context + * @return bool|\core\context target context or false if no target context found */ public static function restore_find_best_target_context($categories, $courseid, $contextlevel) { global $DB; $targetcontext = false; - // Depending of $contextlevel, we perform different actions - switch ($contextlevel) { - // For system is easy, the best context is the system context - case CONTEXT_SYSTEM: - $targetcontext = context_system::instance(); - break; + // If context module we need to find any existing module instances with categories matching the category stamps + // from the backup. If multiple matches are found, that means that there is some annoying + // qbank "fragmentation" in the categories, so we'll fall back + // to creating a qbank instance at course level and putting the categories there. + if ($contextlevel == CONTEXT_MODULE) { + $stamps = []; + foreach ($categories as $category) { + $stamps[] = $category->stamp; + } + $modinfo = get_fast_modinfo($courseid); - // For coursecat, we are going to look for stamps in all the - // course categories between CONTEXT_SYSTEM and CONTEXT_COURSE - // (i.e. in all the course categories in the path) - // - // And only will return one "best" target context if all the - // matches belong to ONE and ONLY ONE context. If multiple - // matches are found, that means that there is some annoying - // qbank "fragmentation" in the categories, so we'll fallback - // to create the qbank at course level - case CONTEXT_COURSECAT: - // Build the array of stamps we are going to match - $stamps = array(); - foreach ($categories as $category) { - $stamps[] = $category->stamp; - } - $contexts = array(); - // Build the array of contexts we are going to look - $systemctx = context_system::instance(); - $coursectx = context_course::instance($courseid); - $parentctxs = $coursectx->get_parent_context_ids(); - foreach ($parentctxs as $parentctx) { - // Exclude system context - if ($parentctx == $systemctx->id) { - continue; - } - $contexts[] = $parentctx; - } - if (!empty($stamps) && !empty($contexts)) { - // Prepare the query - list($stamp_sql, $stamp_params) = $DB->get_in_or_equal($stamps); - list($context_sql, $context_params) = $DB->get_in_or_equal($contexts); - $sql = "SELECT DISTINCT contextid - FROM {question_categories} - WHERE stamp $stamp_sql - AND contextid $context_sql"; - $params = array_merge($stamp_params, $context_params); - $matchingcontexts = $DB->get_records_sql($sql, $params); - // Only if ONE and ONLY ONE context is found, use it as valid target - if (count($matchingcontexts) == 1) { - $targetcontext = context::instance_by_id(reset($matchingcontexts)->contextid); - } - } - break; + // Get contextids of modules from the course that support publishing questions. + $supportedcontextids = []; + foreach ($modinfo->get_cms() as $cm) { + if (plugin_supports('mod', $cm->modname, FEATURE_PUBLISHES_QUESTIONS, false)) { + $supportedcontextids[] = $cm->context->id; + } + } - // For course is easy, the best context is the course context - case CONTEXT_COURSE: - $targetcontext = context_course::instance($courseid); - break; - - // For module is easy, there is not best context, as far as the - // activity hasn't been created yet. So we return context course - // for them, so permission checks and friends will work. Note this - // case is handled by {@link prechek_precheck_qbanks_by_level} - // in an special way - case CONTEXT_MODULE: - $targetcontext = context_course::instance($courseid); - break; + if (!empty($stamps) && !empty($supportedcontextids)) { + [$stampsql, $stampparams] = $DB->get_in_or_equal($stamps); + [$contextsql, $contextparams] = $DB->get_in_or_equal($supportedcontextids); + $sql = "SELECT DISTINCT contextid + FROM {question_categories} + WHERE stamp {$stampsql} + AND contextid {$contextsql}"; + $params = array_merge($stampparams, $contextparams); + $matchingcontexts = $DB->get_records_sql($sql, $params); + // Only if ONE and ONLY ONE context is found, use it as valid target. + if (count($matchingcontexts) === 1) { + $targetcontext = context::instance_by_id(reset($matchingcontexts)->contextid); + } + } + // We don't have a target so set as course context until the module is created and then assign to the module context. + $targetcontext = $targetcontext ?: context_course::instance($courseid); } + return $targetcontext; } diff --git a/backup/util/ui/tests/behat/restore_moodle2_courses.feature b/backup/util/ui/tests/behat/restore_moodle2_courses.feature index bca95e15c4b..5e80f9a4b74 100644 --- a/backup/util/ui/tests/behat/restore_moodle2_courses.feature +++ b/backup/util/ui/tests/behat/restore_moodle2_courses.feature @@ -275,7 +275,6 @@ Feature: Restore Moodle 2 course backups | Initial | Include files | 0 | | Initial | Include filters | 0 | | Initial | Include calendar events | 0 | - | Initial | Include question bank | 0 | | Initial | Include groups and groupings | 0 | | Initial | Include competencies | 0 | | Initial | Include custom fields | 0 | diff --git a/lang/en/backup.php b/lang/en/backup.php index 18a411919e8..0171f90b3d6 100644 --- a/lang/en/backup.php +++ b/lang/en/backup.php @@ -134,7 +134,6 @@ $string['configgeneralfiles'] = 'Sets the default for including files in a backu $string['configgeneralfilters'] = 'Sets the default for including filters in a backup.'; $string['configgeneralhistories'] = 'Sets the default for including user history within a backup.'; $string['configgenerallogs'] = 'If enabled logs will be included in backups by default.'; -$string['configgeneralquestionbank'] = 'If enabled the question bank will be included in backups by default. PLEASE NOTE: Disabling this setting will disable the backup of activities which use the question bank, such as the quiz.'; $string['configgeneralgroups'] = 'Sets the default for including groups and groupings in a backup.'; $string['configgeneralroleassignments'] = 'If enabled by default roles assignments will also be backed up.'; $string['configgeneralpermissions'] = 'If enabled the role permissions will be imported. This may override existing permissions for enrolled users.'; @@ -236,7 +235,6 @@ $string['generalhistories'] = 'Include histories'; $string['generalgradehistories'] = 'Include histories'; $string['generallegacyfiles'] = 'Include legacy course files'; $string['generallogs'] = 'Include logs'; -$string['generalquestionbank'] = 'Include question bank'; $string['generalgroups'] = 'Include groups and groupings'; $string['generalrestoredefaults'] = 'General restore defaults'; $string['mergerestoredefaults'] = 'Restore defaults when merging into another course'; @@ -314,9 +312,9 @@ $string['privacy:metadata:backup_controllers:operation'] = 'The operation that w $string['privacy:metadata:backup_controllers:timecreated'] = 'The time when the action was created'; $string['privacy:metadata:backup_controllers:timemodified'] = 'The time when the action was modified'; $string['privacy:metadata:backup_controllers:type'] = 'The type of the item being operated on, eg. activity.'; -$string['qcategory2coursefallback'] = 'The questions category "{$a->name}", originally at system/course category context in backup file, will be created at course context by restore'; +$string['qcategory2coursefallback'] = 'The questions category "{$a->name}", originally at system|course|course_category context in backup file, will be created at a question bank module context by restore'; $string['qcategorycannotberestored'] = 'The questions category "{$a->name}" cannot be created by restore'; -$string['question2coursefallback'] = 'The questions category "{$a->name}", originally at system/course category context in backup file, will be created at course context by restore'; +$string['question2coursefallback'] = 'The questions category "{$a->name}", originally at system|course|course_category in backup file, will be created at a question bank module context by restore'; $string['questioncannotberestored'] = 'The questions "{$a->name}" cannot be created by restore'; $string['restoreactivity'] = 'Restore activity'; $string['restorecourse'] = 'Restore course'; @@ -437,3 +435,7 @@ $string['recyclebin_desc'] = 'Note that these settings will also be used for the // Deprecated since Moodle 4.4. $string['copycourseheading'] = 'Copy a course'; $string['backupcourse'] = 'Backup course: {$a}'; + +// Deprecated since Moodle 5.0. +$string['configgeneralquestionbank'] = 'If enabled the question bank will be included in backups by default. PLEASE NOTE: Disabling this setting will disable the backup of activities which use the question bank, such as the quiz.'; +$string['generalquestionbank'] = 'Include question bank'; diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index dfaadb3a558..7161e88eb38 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -136,3 +136,7 @@ filterfirstactive,core_grades filterlastactive,core_grades noreplybouncemessage,core noreplybouncesubject,core +configgeneralquestionbank,core_backup +generalquestionbank,core_backup +cannotdeletecategoryquestions,core_error +errordeletingquestionsfromcategory,core_question diff --git a/lang/en/question.php b/lang/en/question.php index 7caed3ff0e0..6a3cd244477 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -368,6 +368,7 @@ $string['contexterror'] = 'You shouldn\'t have got here if you\'re not moving a $string['correct'] = 'Correct'; $string['correctfeedback'] = 'For any correct response'; $string['correctfeedbackdefault'] = 'Your answer is correct.'; +$string['courserestore'] = 'Course restore'; $string['decimalplacesingrades'] = 'Decimal places in grades'; $string['defaultmark'] = 'Default mark'; $string['errorsavingflags'] = 'Error saving the flag state.'; diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index a310c6ab9d6..f2ab0ec4bac 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1461,6 +1461,14 @@ function xmldb_main_upgrade($oldversion) { // Main savepoint reached. upgrade_main_savepoint(true, 2024110400.00); } + if ($oldversion < 2024110800.00) { + // Delete settings that were removed from code. + $settings = ['backup_general_questionbank', 'backup_import_questionbank', 'backup_auto_questionbank']; + array_walk($settings, static fn($setting) => unset_config($setting, 'backup')); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2024110800.00); + } if ($oldversion < 2024110800.02) { // Changing type of field value on table user_preferences to text. diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php index 3102f5c149e..601f745bee2 100644 --- a/lib/tests/questionlib_test.php +++ b/lib/tests/questionlib_test.php @@ -16,6 +16,7 @@ namespace core; +use core_question\local\bank\question_bank_helper; use question_bank; defined('MOODLE_INTERNAL') || die(); @@ -162,83 +163,65 @@ class questionlib_test extends \advanced_testcase { // Set to admin user. $this->setAdminUser(); - // Create two course categories - we are going to delete one of these later and will expect - // all the questions belonging to the course in the deleted category to be moved. + // Create 2 qbank instances - we are going to delete one of these later and will expect + // all the questions belonging to the deleted module to be moved. $coursecat1 = $this->getDataGenerator()->create_category(); + $course1 = $this->getDataGenerator()->create_course(['category' => $coursecat1->id]); + $modqbank1 = $this->getDataGenerator()->create_module('qbank', ['course' => $course1->id]); $coursecat2 = $this->getDataGenerator()->create_category(); + $course2 = $this->getDataGenerator()->create_course(['category' => $coursecat2->id]); + $modqbank2 = $this->getDataGenerator()->create_module('qbank', ['course' => $course2->id]); // Create a couple of categories and questions. - $context1 = \context_coursecat::instance($coursecat1->id); - $context2 = \context_coursecat::instance($coursecat2->id); + $context1 = \context_module::instance($modqbank1->cmid); + $context2 = \context_module::instance($modqbank2->cmid); /** @var \core_question_generator $questiongenerator */ $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); - $questioncat1 = $questiongenerator->create_question_category(array('contextid' => - $context1->id)); - $questioncat2 = $questiongenerator->create_question_category(array('contextid' => - $context2->id)); - $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id)); - $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id)); - $question3 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id)); - $question4 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id)); + $questioncat1 = $questiongenerator->create_question_category(['contextid' => + $context1->id]); + $questioncat2 = $questiongenerator->create_question_category(['contextid' => + $context2->id]); + $question1 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat1->id]); + $question2 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat1->id]); + $question3 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat2->id]); + $question4 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat2->id]); // Now lets tag these questions. - \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $context1, array('tag 1', 'tag 2')); - \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $context1, array('tag 3', 'tag 4')); - \core_tag_tag::set_item_tags('core_question', 'question', $question3->id, $context2, array('tag 5', 'tag 6')); - \core_tag_tag::set_item_tags('core_question', 'question', $question4->id, $context2, array('tag 7', 'tag 8')); + \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $context1, ['tag 1', 'tag 2']); + \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $context1, ['tag 3', 'tag 4']); + \core_tag_tag::set_item_tags('core_question', 'question', $question3->id, $context2, ['tag 5', 'tag 6']); + \core_tag_tag::set_item_tags('core_question', 'question', $question4->id, $context2, ['tag 7', 'tag 8']); - // Test moving the questions to another category. - question_move_questions_to_category(array($question1->id, $question2->id), $questioncat2->id); - - // Test that all tag_instances belong to one context. - $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat2->contextid))); - - // Test moving them back. - question_move_questions_to_category(array($question1->id, $question2->id), $questioncat1->id); - - // Test that all tag_instances are now reset to how they were initially. - $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat1->contextid))); - $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat2->contextid))); - - // Now test moving a whole question category to another context. + // Test moving a whole question category to another context. question_move_category_to_context($questioncat1->id, $questioncat1->contextid, $questioncat2->contextid); // Test that all tag_instances belong to one context. - $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat2->contextid))); + $this->assertEquals(8, $DB->count_records('tag_instance', ['component' => 'core_question', + 'contextid' => $questioncat2->contextid])); // Now test moving them back. question_move_category_to_context($questioncat1->id, $questioncat2->contextid, - \context_coursecat::instance($coursecat1->id)->id); + \context_module::instance($modqbank1->cmid)->id); // Test that all tag_instances are now reset to how they were initially. - $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat1->contextid))); - $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat2->contextid))); - - // Now we want to test deleting the course category and moving the questions to another category. - question_delete_course_category($coursecat1, $coursecat2); - - // Test that all tag_instances belong to one context. - $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', - 'contextid' => $questioncat2->contextid))); + $this->assertEquals(4, $DB->count_records('tag_instance', ['component' => 'core_question', + 'contextid' => $questioncat1->contextid])); + $this->assertEquals(4, $DB->count_records('tag_instance', ['component' => 'core_question', + 'contextid' => $questioncat2->contextid])); // Create a course. $course = $this->getDataGenerator()->create_course(); + $modqbank3 = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); // Create some question categories and questions in this course. - $coursecontext = \context_course::instance($course->id); - $questioncat = $questiongenerator->create_question_category(array('contextid' => $coursecontext->id)); - $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id)); - $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id)); + $modcontext = \context_module::instance($modqbank3->cmid); + $questioncat = $questiongenerator->create_question_category(['contextid' => $modcontext->id]); + $question1 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat->id]); + $question2 = $questiongenerator->create_question('shortanswer', null, ['category' => $questioncat->id]); // Add some tags to these questions. - \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, array('tag 1', 'tag 2')); - \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, array('tag 1', 'tag 2')); + \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $modcontext, ['tag 1', 'tag 2']); + \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $modcontext, ['tag 1', 'tag 2']); // Create a course that we are going to restore the other course to. $course2 = $this->getDataGenerator()->create_course(); @@ -260,9 +243,21 @@ class questionlib_test extends \advanced_testcase { $rc->execute_precheck(); $rc->execute_plan(); + $modinfo = get_fast_modinfo($course2); + $qbanks = $modinfo->get_instances_of('qbank'); + $qbankids = array_column($qbanks, 'instance'); + $qbankrecords = $DB->get_records_list('qbank', 'id', $qbankids, '', 'id, type'); + $qbanks = array_filter($qbanks, static function($bank) use ($qbankrecords) { + if (isset($qbankrecords[$bank->instance])) { + return $qbankrecords[$bank->instance]->type === question_bank_helper::TYPE_STANDARD; + } + return false; + }); + $qbank = reset($qbanks); + // Get the created question category. $restoredcategory = $DB->get_record_select('question_categories', 'contextid = ? AND parent <> 0', - array(\context_course::instance($course2->id)->id), '*', MUST_EXIST); + [$qbank->context->id, '*', MUST_EXIST]); // Check that there are two questions in the restored to course's context. $this->assertEquals(2, $DB->get_record_sql('SELECT COUNT(q.id) as questioncount diff --git a/mod/qbank/backup/moodle2/backup_qbank_activity_task.class.php b/mod/qbank/backup/moodle2/backup_qbank_activity_task.class.php index abbcbdd5994..70d858a11ac 100644 --- a/mod/qbank/backup/moodle2/backup_qbank_activity_task.class.php +++ b/mod/qbank/backup/moodle2/backup_qbank_activity_task.class.php @@ -40,8 +40,9 @@ class backup_qbank_activity_task extends backup_activity_task { * Define (add) particular steps this activity can have */ protected function define_my_steps() { - // Qbank only has one structure step $this->add_step(new backup_qbank_activity_structure_step('qbank_structure', 'qbank.xml')); + $this->add_step(new backup_calculate_question_categories('qbank_activity_question_categories')); + $this->add_step(new backup_delete_temp_questions('clean_temp_questions')); } /** @@ -52,6 +53,13 @@ class backup_qbank_activity_task extends backup_activity_task { * @return string encoded content */ public static function encode_content_links($content) { - return $content; + global $CFG; + + $base = preg_quote($CFG->wwwroot, '/'); + + // Link to qbank view by moduleid. + $search = "/(".$base."\/mod\/qbank\/view.php\?id\=)([0-9]+)/"; + + return preg_replace($search, '$@QBANKVIEWBYID*$2@$', $content); } } diff --git a/mod/qbank/backup/moodle2/restore_qbank_activity_task.class.php b/mod/qbank/backup/moodle2/restore_qbank_activity_task.class.php index 62b9a9d4e09..46644f9b54b 100644 --- a/mod/qbank/backup/moodle2/restore_qbank_activity_task.class.php +++ b/mod/qbank/backup/moodle2/restore_qbank_activity_task.class.php @@ -60,7 +60,11 @@ class restore_qbank_activity_task extends restore_activity_task { * @return restore_decode_rule[]. */ public static function define_decode_rules(): array { - return []; + $rules = []; + + $rules[] = new restore_decode_rule('QBANKVIEWBYID', '/mod/qbank/view.php?id=$1', 'course_module'); + + return $rules; } /** diff --git a/mod/quiz/accessrule/seb/tests/test_helper_trait.php b/mod/quiz/accessrule/seb/tests/test_helper_trait.php index 9f02428d73c..40829fcd176 100644 --- a/mod/quiz/accessrule/seb/tests/test_helper_trait.php +++ b/mod/quiz/accessrule/seb/tests/test_helper_trait.php @@ -168,9 +168,13 @@ trait quizaccess_seb_test_helper_trait { $quiz->seb_showsebdownloadlink = 1; $quiz->coursemodule = $quiz->cmid; + // Create a question bank. + $qbank = self::getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbankcontext = context_module::instance($qbank->cmid); + // Create a couple of questions. $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); - $cat = $questiongenerator->create_question_category(); + $cat = $questiongenerator->create_question_category(['contextid' => $qbankcontext->id]); $saq = $questiongenerator->create_question('shortanswer', null, ['category' => $cat->id]); quiz_add_quiz_question($saq->id, $quiz); diff --git a/mod/quiz/tests/quiz_question_restore_test.php b/mod/quiz/tests/quiz_question_restore_test.php index ba4f24933fa..ac115904704 100644 --- a/mod/quiz/tests/quiz_question_restore_test.php +++ b/mod/quiz/tests/quiz_question_restore_test.php @@ -56,21 +56,21 @@ class quiz_question_restore_test extends \advanced_testcase { } /** - * Test a quiz backup and restore in a different course without attempts for course question bank. * * @covers \mod_quiz\question\bank\qbank_helper::get_question_structure */ - public function test_quiz_restore_in_a_different_course_using_course_question_bank(): void { + public function test_quiz_restore_in_a_different_course_using_question_bank(): void { $this->resetAfterTest(); // Create the test quiz. $quiz = $this->create_test_quiz($this->course); $oldquizcontext = \context_module::instance($quiz->cmid); + $qbank = self::getDataGenerator()->create_module('qbank', ['course' => $this->course]); + $qbankcontext = \context_module::instance($qbank->cmid); // Test for questions from a different context. - $coursecontext = \context_course::instance($this->course->id); $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); - $this->add_two_regular_questions($questiongenerator, $quiz, ['contextid' => $coursecontext->id]); - $this->add_one_random_question($questiongenerator, $quiz, ['contextid' => $coursecontext->id]); + $this->add_two_regular_questions($questiongenerator, $quiz, ['contextid' => $qbankcontext->id]); + $this->add_one_random_question($questiongenerator, $quiz, ['contextid' => $qbankcontext->id]); // Make the backup. $backupid = $this->backup_quiz($quiz, $this->user); @@ -303,12 +303,24 @@ class quiz_question_restore_test extends \advanced_testcase { $rc = new \restore_controller($backupid, $newcourseid, \backup::INTERACTIVE_NO, \backup::MODE_GENERAL, $USER->id, \backup::TARGET_NEW_COURSE); - $this->assertTrue($rc->execute_precheck()); + $rc->execute_precheck(); + $results = $rc->get_precheck_results(); + // Backup contains categories attached to deprecated contexts so the results should only contain warnings for these. + $this->assertCount(2, $results['warnings']); + foreach ($results['warnings'] as $warning) { + $this->assertStringContainsString('will be created at a question bank module context by restore', $warning); + } + $this->assertArrayNotHasKey('errors', $results); + $rc->execute_plan(); $rc->destroy(); // Get the information about the resulting course and check that it is set up correctly. $modinfo = get_fast_modinfo($newcourseid); + $qbanks = $modinfo->get_instances_of('qbank'); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + $this->assertEquals(get_string('systembank', 'question'), $qbank->name); $quiz = array_values($modinfo->get_instances_of('quiz'))[0]; $quizobj = \mod_quiz\quiz_settings::create($quiz->instance); $structure = structure::create_for_quiz($quizobj); @@ -322,10 +334,9 @@ class quiz_question_restore_test extends \advanced_testcase { $questions = $quizobj->get_questions(); $this->assertCount(1, $questions); - // Count the questions for course question bank. - $this->assertEquals(6, $this->question_count(\context_course::instance($newcourseid)->id)); - $this->assertEquals(6, $this->question_count(\context_course::instance($newcourseid)->id, - "AND q.qtype <> 'random'")); + // Count the questions for new course mod_qbank question bank. + $this->assertEquals(6, $this->question_count(\context_module::instance($qbank->id)->id)); + $this->assertEquals(6, $this->question_count(\context_module::instance($qbank->id)->id, "AND q.qtype <> 'random'")); // Count the questions in quiz qbank. $this->assertEquals(0, $this->question_count($quizobj->get_context()->id)); @@ -408,13 +419,20 @@ class quiz_question_restore_test extends \advanced_testcase { $rc = new \restore_controller($backupid, $newcourseid, \backup::INTERACTIVE_NO, \backup::MODE_GENERAL, $USER->id, \backup::TARGET_NEW_COURSE); - $this->assertTrue($rc->execute_precheck()); + $rc->execute_precheck(); + $results = $rc->get_precheck_results(); + // Backup contains categories attached to deprecated contexts, so we should only have warnings for those. + $this->assertCount(1, $results['warnings']); + $this->assertStringContainsString('will be created at a question bank module context by restore', $results['warnings'][0]); + $this->assertArrayNotHasKey('errors', $results); + $rc->execute_plan(); $rc->destroy(); // Get the information about the resulting course and check that it is set up correctly. // Each quiz should contain an instance of the random question. $modinfo = get_fast_modinfo($newcourseid); + $qbank = array_values($modinfo->get_instances_of('qbank'))[0]; $quizzes = $modinfo->get_instances_of('quiz'); $this->assertCount(2, $quizzes); foreach ($quizzes as $quiz) { @@ -431,10 +449,10 @@ class quiz_question_restore_test extends \advanced_testcase { $this->assertCount(1, $questions); } - // Count the questions for course question bank. + // Count the questions for new course mod_qbank question bank. // We should have a single question, the random question should have been deleted after the restore. - $this->assertEquals(1, $this->question_count(\context_course::instance($newcourseid)->id)); - $this->assertEquals(1, $this->question_count(\context_course::instance($newcourseid)->id, + $this->assertEquals(1, $this->question_count(\context_module::instance($qbank->id)->id)); + $this->assertEquals(1, $this->question_count(\context_module::instance($qbank->id)->id, "AND q.qtype <> 'random'")); // Count the questions in quiz qbank. diff --git a/mod/quiz/tests/restore_attempt_test.php b/mod/quiz/tests/restore_attempt_test.php index ac47257aade..1ded63b7579 100644 --- a/mod/quiz/tests/restore_attempt_test.php +++ b/mod/quiz/tests/restore_attempt_test.php @@ -68,7 +68,15 @@ class restore_attempt_test extends \advanced_testcase { $controller = new restore_controller($backuptempdir, $courseid, backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id, backup::TARGET_NEW_COURSE); - $this->assertTrue($controller->execute_precheck()); + $controller->execute_precheck(); + $results = $controller->get_precheck_results(); + // Backup contains categories attached to deprecated contexts so the results should only contain warnings for these. + $this->assertCount(2, $results['warnings']); + foreach ($results['warnings'] as $warning) { + $this->assertStringContainsString('will be created at a question bank module context by restore', $warning); + } + $this->assertArrayNotHasKey('errors', $results); + $controller->execute_plan(); $controller->destroy(); diff --git a/mod/quiz/tests/tags_test.php b/mod/quiz/tests/tags_test.php index ef6ab833731..f81a749c454 100644 --- a/mod/quiz/tests/tags_test.php +++ b/mod/quiz/tests/tags_test.php @@ -49,7 +49,14 @@ class tags_test extends \advanced_testcase { $rc = new \restore_controller($backupid, $newcourseid, \backup::INTERACTIVE_NO, \backup::MODE_GENERAL, $USER->id, \backup::TARGET_NEW_COURSE); - $this->assertTrue($rc->execute_precheck()); + $rc->execute_precheck(); + $results = $rc->get_precheck_results(); + // Backup contains categories attached to deprecated contexts so the results should only contain warnings for these. + $this->assertCount(2, $results['warnings']); + foreach ($results['warnings'] as $warning) { + $this->assertStringContainsString('will be created at a question bank module context by restore', $warning); + } + $this->assertArrayNotHasKey('errors', $results); $rc->execute_plan(); $rc->destroy(); @@ -85,7 +92,13 @@ class tags_test extends \advanced_testcase { $slottags = explode(',', $slottags); $this->assertEquals("{$tag2->id},{$tag2->name}", "{$slottags[0]},{$slottags[1]}"); - $defaultcategory = question_get_default_category(\context_course::instance($newcourseid)->id); + // Course context question cats get restored to a default qbank module instance. + $modinfo = get_fast_modinfo($newcourseid); + $qbanks = $modinfo->get_instances_of('qbank'); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + + $defaultcategory = question_get_default_category(\context_module::instance($qbank->id)->id, true); $this->assertEquals($defaultcategory->id, $question->category); $randomincludingsubcategories = $DB->get_record('question_set_references', ['itemid' => reset($slots)->id, 'component' => 'mod_quiz', 'questionarea' => 'slot']); diff --git a/question/bank/exporttoxml/tests/helper_test.php b/question/bank/exporttoxml/tests/helper_test.php index 9cb308fa73f..756a87fcbb2 100644 --- a/question/bank/exporttoxml/tests/helper_test.php +++ b/question/bank/exporttoxml/tests/helper_test.php @@ -43,32 +43,27 @@ class helper_test extends \advanced_testcase { // Create a course and an activity. $course = $generator->create_course(); + $qbank = self::getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbankcontext = \context_module::instance($qbank->cmid); $quiz = $generator->create_module('quiz', ['course' => $course->id]); // Create a question in each place. $questiongenerator = $generator->get_plugin_generator('core_question'); - $courseqcat = $questiongenerator->create_question_category(['contextid' => context_course::instance($course->id)->id]); - $courseq = $questiongenerator->create_question('truefalse', null, ['category' => $courseqcat->id]); + $qbankqcat = $questiongenerator->create_question_category(['contextid' => $qbankcontext->id]); + $qbankq = $questiongenerator->create_question('truefalse', null, ['category' => $qbankqcat->id]); $quizqcat = $questiongenerator->create_question_category(['contextid' => context_module::instance($quiz->cmid)->id]); $quizq = $questiongenerator->create_question('truefalse', null, ['category' => $quizqcat->id]); - $systemqcat = $questiongenerator->create_question_category(); - $systemq = $questiongenerator->create_question('truefalse', null, ['category' => $systemqcat->id]); // Verify some URLs. $this->assertEquals(new moodle_url('/question/bank/exporttoxml/exportone.php', - ['id' => $courseq->id, 'courseid' => $course->id, 'sesskey' => sesskey()]), + ['id' => $qbankq->id, 'cmid' => $qbank->cmid, 'sesskey' => sesskey()]), helper::question_get_export_single_question_url( - question_bank::load_question_data($courseq->id))); + question_bank::load_question_data($qbankq->id))); $this->assertEquals(new moodle_url('/question/bank/exporttoxml/exportone.php', ['id' => $quizq->id, 'cmid' => $quiz->cmid, 'sesskey' => sesskey()]), helper::question_get_export_single_question_url( question_bank::load_question($quizq->id))); - - $this->assertEquals(new moodle_url('/question/bank/exporttoxml/exportone.php', - ['id' => $systemq->id, 'courseid' => SITEID, 'sesskey' => sesskey()]), - helper::question_get_export_single_question_url( - question_bank::load_question($systemq->id))); } } diff --git a/question/tests/backup_test.php b/question/tests/backup_test.php index f439617aa9d..f06775a308e 100644 --- a/question/tests/backup_test.php +++ b/question/tests/backup_test.php @@ -22,6 +22,11 @@ use question_bank; defined('MOODLE_INTERNAL') || die(); +use backup; +use core_question\local\bank\question_bank_helper; +use restore_controller; +use restore_dbops; + global $CFG; require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); @@ -63,24 +68,43 @@ class backup_test extends \advanced_testcase { } /** - * Restores a backup that has been made earlier. + * Makes a backup of a course module. * - * @param string $backupid The unique identifier of the backup. - * @param string $fullname Full name of the new course that is going to be created. - * @param string $shortname Short name of the new course that is going to be created. - * @param int $categoryid The course category the backup is going to be restored in. - * @param string[] $expectedprecheckwarning - * @return int The new course id. + * @param int $modid The course_module id. + * @return string Unique identifier for this backup. */ - protected function restore_course($backupid, $fullname, $shortname, $categoryid, $expectedprecheckwarning = []) { + protected function backup_course_module(int $modid) { global $CFG, $USER; // Turn off file logging, otherwise it can't delete the file (Windows). $CFG->backup_file_logger_level = \backup::LOG_NONE; - // Do restore to new course with default settings. - $newcourseid = \restore_dbops::create_new_course($fullname, $shortname, $categoryid); - $rc = new \restore_controller($backupid, $newcourseid, + // Do backup with default settings. MODE_IMPORT means it will just + // create the directory and not zip it. + $bc = new \backup_controller(\backup::TYPE_1ACTIVITY, $modid, + \backup::FORMAT_MOODLE, \backup::INTERACTIVE_NO, \backup::MODE_IMPORT, + $USER->id); + $backupid = $bc->get_backupid(); + $bc->execute_plan(); + $bc->destroy(); + + return $backupid; + } + + /** + * Restores a backup that has been made earlier. + * + * @param string $backupid The unique identifier of the backup. + * @param int $courseid Course id of where the restore is happening. + * @param string[] $expectedprecheckwarning + */ + protected function restore_to_course(string $backupid, int $courseid, array $expectedprecheckwarning = []): void { + global $CFG, $USER; + + // Turn off file logging, otherwise it can't delete the file (Windows). + $CFG->backup_file_logger_level = \backup::LOG_NONE; + + $rc = new \restore_controller($backupid, $courseid, \backup::INTERACTIVE_NO, \backup::MODE_GENERAL, $USER->id, \backup::TARGET_NEW_COURSE); @@ -94,12 +118,10 @@ class backup_test extends \advanced_testcase { } $rc->execute_plan(); $rc->destroy(); - - return $newcourseid; } /** - * This function tests backup and restore of question tags and course level question tags. + * This function tests backup and restore of question tags. */ public function test_backup_question_tags(): void { global $DB; @@ -107,7 +129,7 @@ class backup_test extends \advanced_testcase { $this->resetAfterTest(); $this->setAdminUser(); - // Create a new course category and and a new course in that. + // Create a new course category and a new course in that. $category1 = $this->getDataGenerator()->create_category(); $course = $this->getDataGenerator()->create_course(['category' => $category1->id]); $courseshortname = $course->shortname; @@ -115,18 +137,17 @@ class backup_test extends \advanced_testcase { // Create 2 questions. $qgen = $this->getDataGenerator()->get_plugin_generator('core_question'); - $context = \context_coursecat::instance($category1->id); + $qbank = $this->getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $context = \context_module::instance($qbank->cmid); $qcat = $qgen->create_question_category(['contextid' => $context->id]); $question1 = $qgen->create_question('shortanswer', null, ['category' => $qcat->id, 'idnumber' => 'q1']); $question2 = $qgen->create_question('shortanswer', null, ['category' => $qcat->id, 'idnumber' => 'q2']); - // Tag the questions with 2 question tags and 2 course level question tags. + // Tag the questions with 2 question tags. $qcontext = \context::instance_by_id($qcat->contextid); $coursecontext = context_course::instance($course->id); \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $qcontext, ['qtag1', 'qtag2']); \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $qcontext, ['qtag3', 'qtag4']); - \core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, ['ctag1', 'ctag2']); - \core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, ['ctag3', 'ctag4']); // Create a quiz and add one of the questions to that. $quiz = $this->getDataGenerator()->create_module('quiz', ['course' => $course->id]); @@ -142,10 +163,20 @@ class backup_test extends \advanced_testcase { question_delete_question($question2->id); // Restore the backup we had made earlier into a new course. - $courseid2 = $this->restore_course($backupid1, $coursefullname, $courseshortname . '_2', $category1->id); + // Do restore to new course with default settings. + $courseid2 = \restore_dbops::create_new_course($coursefullname, $courseshortname . '_2', $category1->id); + $this->restore_to_course($backupid1, $courseid2); + $modinfo = get_fast_modinfo($courseid2); + $qbanks = $modinfo->get_instances_of('qbank'); + $qbanks = array_filter($qbanks, static fn($qbank) => $qbank->get_name() === 'Question bank 1'); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + $qbankcontext = \context_module::instance($qbank->id); + $cats = $DB->get_records_select('question_categories' , 'parent <> 0', ['contextid' => $qbankcontext->id]); + $this->assertCount(1, $cats); + $cat = reset($cats); - // The questions should remain in the question category they were which is - // a question category belonging to a course category context. + // The questions should be restored to a mod_qbank context in the new course. $sql = 'SELECT q.*, qbe.idnumber FROM {question} q @@ -153,7 +184,7 @@ class backup_test extends \advanced_testcase { JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid WHERE qbe.questioncategoryid = ? ORDER BY qbe.idnumber'; - $questions = $DB->get_records_sql($sql, [$qcat->id]); + $questions = $DB->get_records_sql($sql, [$cat->id]); $this->assertCount(2, $questions); // Retrieve tags for each question and check if they are assigned at the right context. @@ -162,15 +193,10 @@ class backup_test extends \advanced_testcase { $tags = \core_tag_tag::get_item_tags('core_question', 'question', $question->id); // Each question is tagged with 4 tags (2 question tags + 2 course tags). - $this->assertCount(4, $tags); + $this->assertCount(2, $tags); foreach ($tags as $tag) { - if (in_array($tag->name, ['ctag1', 'ctag2', 'ctag3', 'ctag4'])) { - $expected = context_course::instance($courseid2)->id; - } else if (in_array($tag->name, ['qtag1', 'qtag2', 'qtag3', 'qtag4'])) { - $expected = $qcontext->id; - } - $this->assertEquals($expected, $tag->taginstancecontextid); + $this->assertEquals($qbankcontext->id, $tag->taginstancecontextid); } // Also check idnumbers have been backed up and restored. @@ -188,14 +214,15 @@ class backup_test extends \advanced_testcase { // Create a new course category to restore the backup file into it. $category2 = $this->getDataGenerator()->create_category(); - $expectedwarnings = [ - get_string('qcategory2coursefallback', 'backup', (object) ['name' => 'top']), - get_string('qcategory2coursefallback', 'backup', (object) ['name' => $qcat->name]) - ]; - // Restore to a new course in the new course category. - $courseid3 = $this->restore_course($backupid2, $coursefullname, $courseshortname . '_3', $category2->id, $expectedwarnings); - $coursecontext3 = context_course::instance($courseid3); + $courseid3 = \restore_dbops::create_new_course($coursefullname, $courseshortname . '_3', $category2->id); + $this->restore_to_course($backupid2, $courseid3); + $modinfo = get_fast_modinfo($courseid3); + $qbanks = $modinfo->get_instances_of('qbank'); + $qbanks = array_filter($qbanks, static fn($qbank) => $qbank->get_name() === 'Question bank 1'); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + $context = \context_module::instance($qbank->id); // The questions should have been moved to a question category that belongs to a course context. $questions = $DB->get_records_sql("SELECT q.* @@ -203,18 +230,18 @@ class backup_test extends \advanced_testcase { JOIN {question_versions} qv ON qv.questionid = q.id JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid - WHERE qc.contextid = ?", [$coursecontext3->id]); + WHERE qc.contextid = ?", [$context->id]); $this->assertCount(2, $questions); // Now, retrieve tags for each question and check if they are assigned at the right context. foreach ($questions as $question) { $tags = \core_tag_tag::get_item_tags('core_question', 'question', $question->id); - // Each question is tagged with 4 tags (all are course tags now). - $this->assertCount(4, $tags); + // Each question is tagged with 2 tags (all are question context tags now). + $this->assertCount(2, $tags); foreach ($tags as $tag) { - $this->assertEquals($coursecontext3->id, $tag->taginstancecontextid); + $this->assertEquals($context->id, $tag->taginstancecontextid); } } @@ -462,4 +489,262 @@ class backup_test extends \advanced_testcase { $this->assertEquals('The reason: ' . $recodedcontent, $restoredquestion->options->answers[$firstanswerid]->feedback); $this->assertEquals('Hint: ' . $recodedcontent, $restoredquestion->hints[$firsthintid]->hint); } + + /** + * Boilerplate setup for the tests. Creates a course, a quiz, and a qbank module. It adds a category to each module context + * and adds a question to each category. Finally, it adds the 2 questions to the quiz. + * + * @return \stdClass + */ + private function add_course_quiz_and_qbank() { + $qgen = self::getDataGenerator()->get_plugin_generator('core_question'); + + // Create a new course. + $course = self::getDataGenerator()->create_course(); + + // Create a question bank module instance, a category for that module, and a question for that category. + $qbank = self::getDataGenerator()->create_module( + 'qbank', + ['type' => question_bank_helper::TYPE_STANDARD, 'course' => $course->id] + ); + $qbankcontext = \context_module::instance($qbank->cmid); + $bankqcat = $qgen->create_question_category(['contextid' => $qbankcontext->id]); + $bankquestion = $qgen->create_question('shortanswer', + null, + ['name' => 'bank question', 'category' => $bankqcat->id, 'idnumber' => 'bankq1'] + ); + + // Create a quiz module instance, a category for that module, and a question for that category. + $quiz = self::getDataGenerator()->create_module('quiz', ['course' => $course->id]); + $quizcontext = \context_module::instance($quiz->cmid); + $quizqcat = $qgen->create_question_category(['contextid' => $quizcontext->id]); + $quizquestion = $qgen->create_question('shortanswer', + null, + ['name' => 'quiz question', 'category' => $quizqcat->id, 'idnumber' => 'quizq1'] + ); + + quiz_add_quiz_question($bankquestion->id, $quiz); + quiz_add_quiz_question($quizquestion->id, $quiz); + + $data = new \stdClass(); + $data->course = $course; + $data->qbank = $qbank; + $data->qbankcategory = $bankqcat; + $data->qbankquestion = $bankquestion; + $data->quiz = $quiz; + $data->quizcategory = $quizqcat; + $data->quizquestion = $quizquestion; + + return $data; + } + + /** + * If the backup contains ONLY a quiz but that quiz uses questions from a qbank module and itself, + * then the non-quiz context categories and questions should restore to a default qbank module on the new course + * if the old qbank no longer exists. + * + * @return void + * @covers \restore_controller::execute_plan() + */ + public function test_quiz_activity_restore_to_new_course(): void { + global $DB; + + $this->resetAfterTest(); + self::setAdminUser(); + + // Create a course to make a backup. + $data = $this->add_course_quiz_and_qbank(); + $oldquiz = $data->quiz; + + // Backup ONLY the quiz module. + $backupid = $this->backup_course_module($oldquiz->cmid); + + // Create a new course to restore to. + $newcourse = self::getDataGenerator()->create_course(); + delete_course($data->course->id, false); + + $this->restore_to_course($backupid, $newcourse->id); + $modinfo = get_fast_modinfo($newcourse); + + // Assert we have our quiz including the category and question. + $newquizzes = $modinfo->get_instances_of('quiz'); + $this->assertCount(1, $newquizzes); + $newquiz = reset($newquizzes); + $newquizcontext = \context_module::instance($newquiz->id); + + $quizcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => $newquizcontext->id] + ); + $this->assertCount(1, $quizcats); + $quizcat = reset($quizcats); + $quizcatqs = get_questions_category($quizcat, false); + $this->assertCount(1, $quizcatqs); + $quizq = reset($quizcatqs); + $this->assertEquals('quiz question', $quizq->name); + + // The backup did not contain the qbank that held the categories, but it is dependant. + // So make sure the categories and questions got restored to a 'system' type default qbank module on the course. + $defaultbanks = $modinfo->get_instances_of('qbank'); + $this->assertCount(1, $defaultbanks); + $defaultbank = reset($defaultbanks); + $defaultbankcontext = \context_module::instance($defaultbank->id); + $bankcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => $defaultbankcontext->id] + ); + $bankcat = reset($bankcats); + $bankqs = get_questions_category($bankcat, false); + $this->assertCount(1, $bankqs); + $bankq = reset($bankqs); + $this->assertEquals('bank question', $bankq->name); + } + + /** + * If the backup contains BOTH a quiz and a qbank module and the quiz uses questions from the qbank module and itself, + * then we need to restore the categories and questions to the qbank and quiz modules included in the backup on the new course. + * + * @return void + * @covers \restore_controller::execute_plan() + */ + public function test_bank_and_quiz_activity_restore_to_new_course(): void { + // Create a new course. + global $DB; + + $this->resetAfterTest(); + self::setAdminUser(); + + // Create a course to make a backup from. + $data = $this->add_course_quiz_and_qbank(); + $oldcourse = $data->course; + + // Backup the course. + $backupid = $this->backup_course($oldcourse); + + // Create a new course to restore to. + $newcourse = self::getDataGenerator()->create_course(); + + // Restore it. + $this->restore_to_course($backupid, $newcourse->id); + + // Assert the quiz got its question catregories restored. + $modinfo = get_fast_modinfo($newcourse); + $newquizzes = $modinfo->get_instances_of('quiz'); + $this->assertCount(1, $newquizzes); + $newquiz = reset($newquizzes); + $newquizcontext = \context_module::instance($newquiz->id); + $quizcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => $newquizcontext->id] + ); + $quizcat = reset($quizcats); + $quizcatqs = get_questions_category($quizcat, false); + $this->assertCount(1, $quizcatqs); + $quizcatq = reset($quizcatqs); + $this->assertEquals('quiz question', $quizcatq->name); + + // Assert the qbank got its questions restored to the module in the backup. + $qbanks = $modinfo->get_instances_of('qbank'); + $qbanks = array_filter($qbanks, static function($bank) { + global $DB; + $modrecord = $DB->get_record('qbank', ['id' => $bank->instance]); + return $modrecord->type === question_bank_helper::TYPE_STANDARD; + }); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + $bankcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => \context_module::instance($qbank->id)->id] + ); + $bankcat = reset($bankcats); + $bankqs = get_questions_category($bankcat, false); + $this->assertCount(1, $bankqs); + $bankq = reset($bankqs); + $this->assertEquals('bank question', $bankq->name); + } + + /** + * The course backup file contains question banks and a quiz module. + * There is 1 question bank category per deprecated context level i.e. CONTEXT_SYSTEM, CONTEXT_COURSECAT, and CONTEXT_COURSE. + * The quiz included in the backup uses a question in each category. + * + * @return void + * @covers \restore_controller::execute_plan() + */ + public function test_pre_46_course_restore_to_new_course(): void { + global $DB, $USER; + self::setAdminUser(); + $this->resetAfterTest(); + + $backupid = 'question_category_45_format'; + $backuppath = make_backup_temp_directory($backupid); + get_file_packer('application/vnd.moodle.backup')->extract_to_pathname( + __DIR__ . "/fixtures/{$backupid}.mbz", + $backuppath + ); + + // Do restore to new course with default settings. + $categoryid = $DB->get_field_sql("SELECT MIN(id) FROM {course_categories}"); + $newcourseid = restore_dbops::create_new_course('Test fullname', 'Test shortname', $categoryid); + $rc = new restore_controller($backupid, $newcourseid, + backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id, + backup::TARGET_NEW_COURSE + ); + + $rc->execute_precheck(); + $rc->execute_plan(); + $rc->destroy(); + + $modinfo = get_fast_modinfo($newcourseid); + + $qbanks = $modinfo->get_instances_of('qbank'); + $qbanks = array_filter($qbanks, static function($bank) { + global $DB; + $modrecord = $DB->get_record('qbank', ['id' => $bank->instance]); + return $modrecord->type === question_bank_helper::TYPE_SYSTEM; + }); + $this->assertCount(1, $qbanks); + $qbank = reset($qbanks); + $qbankcontext = \context_module::instance($qbank->id); + $bankcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => $qbankcontext->id], + 'name ASC' + ); + // The categories and questions in the 3 deprecated contexts + // all got moved to the new default qbank module instance on the new course. + $this->assertCount(3, $bankcats); + $expectedidentifiers = [ + 'Default for Category 1', + 'Default for System', + 'Default for Test Course 1', + 'Default for Quiz', + ]; + $i = 0; + + foreach ($bankcats as $bankcat) { + $identifer = $expectedidentifiers[$i]; + $this->assertEquals($identifer, $bankcat->name); + $bankcatqs = get_questions_category($bankcat, false); + $this->assertCount(1, $bankcatqs); + $bankcatq = reset($bankcatqs); + $this->assertEquals($identifer, $bankcatq->name); + $i++; + } + + // The question category and question attached to the quiz got restored to its own context correctly. + $newquizzes = $modinfo->get_instances_of('quiz'); + $this->assertCount(1, $newquizzes); + $newquiz = reset($newquizzes); + $newquizcontext = \context_module::instance($newquiz->id); + $quizcats = $DB->get_records_select('question_categories', + 'parent <> 0 AND contextid = :contextid', + ['contextid' => $newquizcontext->id] + ); + $quizcat = reset($quizcats); + $quizcatqs = get_questions_category($quizcat, false); + $this->assertCount(1, $quizcatqs); + $quizcatq = reset($quizcatqs); + $this->assertEquals($expectedidentifiers[$i], $quizcatq->name); + } } diff --git a/question/tests/fixtures/question_category_45_format.mbz b/question/tests/fixtures/question_category_45_format.mbz new file mode 100644 index 0000000000000000000000000000000000000000..8a2902cfa140bd4cbe488ad01dd624d6d02471a2 GIT binary patch literal 6377 zcmV!WRwqu2s=eQn%+Ysu1HnJ|09o6ltIx1mS`bg^ux}S?8$-pQo z<4t-CMynKh(mVn8uSee$`@H^QqWJVv|;NQ9iIxR&Y2_ zS!5y!H#|(dEs}i(DD}j0W$L)t?lMrPMb7GCs-pL01z0^&pIUjYWeOqUO~5MJXEn?1 zG&E3y92RH`a!%z=ory74^vBBWyRL!eK-Hj*#8AF3V~%J9OtLJBw+uZEToqsJcRb;V z5_XF_^kNysZ}&Tv0Z*;!Vqe2bE zw!iBK_AvkN!_B^e?L4{#Wd$b;U%s?oTQ5f-kBcPD*I&N;_09L6PhNgCznRfqF~1p& zW@HAJ)?iB^3XA|CC{50)Dtj>@_5XXM_cgaOVoW{j^ZM(!F`R1t(R1|r|)TK%GoHu z{BICE8{C{Hq29rI{k7u9|V31(v*$49{!>_C|o35rnm5`rC+OrR`5vV=iz z(_&xF6AD=Y2!%@dY-}JRq2nlr(1Nc}pfnmVLK@X@T;0{xBHwS9phT29NFglpEUIdW zi~^+5eGBRhEQn1Y(Eo8YVR0(cVa7S}B52lj| z94J+wOc)oi=HfaBRWeT&CYf_uHs^Td6T~s7i_;a->a~Y6pw3N@LcV3V8!5 zhg^dklu>33sN>{tWxC$fQC{7_CY8g;aG4(U+$b}rqvOsaG`XzxK4t97I7I%9X$u5oSeTk)-4ibdRtcc%MjE4HCFyYI_OfZ1^ zO$3KSfMwSqv>w4WihL9+ZAqy`mu*6!JnvEGqE($Fz9%z86xR7dJ z!GVnr)aXSH@_G*02G=_22m~fmveYBUiE~tCLYiQZiFO&~Z(%&;18adM zSPn+#Gg9R;h^T~dvle)F7&rPuM^AB2=_&R{-&5R=qo=q}u%~#Z^c44_?IB^C<|P^rVh@?ph+>k%BH^lGYY z*sn#eR;WZWth8G3@M1!l3jJlQ0~GCMtoXDE@mTf~fq)i1*%Hl3E1!pKYR7UH6NeH{dWRGKuOYb2>i1)D9wh7fsS5Jo{QrW6_hQ<6e!BU)M* z#E7>0tX{$HRFrceP6dpSL2bUGARd_nV`R~KU4k{Pw738(L8~KL0yrWQ4Gar%2hdU) zlh>$EXb@hHMG5nkz?Mdf0o=pTbc%&nbm%8Xk%iAno{%vRquHcMLRPrx7ek;`9kOUc zzhnej$YLn`&5$`9psM{YfqaPYvX_w@y^eO;N*b3E@*u|ty@g>QH0~N#k_5^C91_b) zCgC74GD^bh+H&`)xFd{YADu?W_;1<1ZvWwhp*xKKeYj)Cf2CJQj3<~?XWyCHlLn4^ zlJ0=L``Oay;(y1s_4&W!TLb_1;g02h+$%7fqpyXcyw1i37zV+4U%JlmX#(sLdsza*MK_EEVxS*Tl_yb3iCumg|>GtFc0dLbPU1nI%W}gxm zsf6gr6I)a_fo1Y6k*TtT&X|h*4ps;twn5CpQLxI>gyV?sW+ZR4tGnwO5>L0J)e324 zp_N9B0qN;od4>>HP4x! zKaDQ=?*+al|83tM^#5Mmhn)XH3cEO&g?!SkmwFhBA<5Tc&v^|;i_Y#2-XU?OG&<*h zUSs_)9M=E(aUVSYTd%$d{Z;Q!Efb^9O3@rLt%y|`ofUvv;W zL}D-7qsJcI+6M#Y+bG{h*^KlrVJA^XH8CZnU)&}EtrLnBAjSFzMDLA62ae3QVlIIRHl0^-n7mSK|%?%_r+ z>|Bwk1NEp8URtYaWo@sj1#Vv=Q&M>qje{5sDk+(!(Jv9Ws*7!!*R~Rz0~>UXiOy-# zxh6WdN#~j9ye6G*qVt<{fr&0?(uF3vut_&D(M>eEcoUV)3M9}bWwQziv`N{lL;_VQ z6A(8O$G`?%GieM|>5$QEGeQDwQZ}O`&?aTGGz8kDY<3C)J!GTghbP!5G2(`e5;{@0 zP~{$mkBtaL#z`X4*G1wQDHkmg(SC*!@eI=Na#GM5q~IV02Pt@QDH!NdumaiY6B2^Y zV_Il+NkXej1X^9P&+Za>R+qH12QfE@x&Fi)6DDXGOd`9sUVp4>-1d8cG&lm z{dOCb5BQ=L@+CpViBA=PQzf5uu}k9$CMGb*fQVYGlf2$Z7VSB>tH?MBFkA~N-<+I2Rn49p5!Q~CuIUlCQMM=S zfj{Ul-2l8&NXD9kN=Wn1Q(!N_vP;mg6)cpiB{n3kzW+!H3z4EKdS z;rqhcLOkleF#Flw7j}|ldJ^}AaiR)B;4r9)G77}fOb9b9jUbCaMyiW_1&&%=Rw9(M zA`+&M%UxL}o>IL?^w@Q-|AxB$XIaDjU;Vh_=YOc5h>gMKE{_?^HUR|_5x-8zV=&+Z zhH?bX%C&3mdAP=N$rniDF#p?*@9FnH*yu{&f&Y7O$KU@W^%G*Gk&|sju_sEe{n5_O zEQi4dQg%DKxJ(G$y`rEL`VwRys=(ZP?Y_20|MJ&;nk8S1aA?Q2|CI-3 zbR;mzdNVONm(frPy_6UyG)63pLGT;eoLJ0|X4@RpFz~jh0WvEiHXb^RkfwQ@?cr2_ z+k2>hCXu*DzFO490^GyNOde$mcIAy0MHe|q0BUyY6oK#V?#iO5=bO6TRbPy04SViu zI8BVQO@U4{G>El0koC+WMHeoixm~%~L>1Jo9d8nP@iMU@XX;oh%bq0mbn3?bGKoBN zTKjN#Ee(1QmAw67fSSp*9s3q^OV}2JrdB$0G_B@S?Sw5684Z)#sm!sMOqbed%rS^} z8th3ktoDk7)JYyHI8~QJ%RN9xGQ*WRN+rdI_(zogu>FfY4OUbpsn%PO<`1 zJNDeA6@{Vv)_}5(AZyRUw1f11N4M?390~adTMz;IP$BmUB5Y!XURq_gRum5ILA>YI zKip#=U`m<_;Cd0M05+{ss%J(ST-27h0Z>Ybza_vyBX+qPc`b#1%LMJWyJ>N>UJxf* zA+0lZ5j^>pu}O;F@S#m6nMbMIY zEfe}v8r&R73PS)y4)SlQljCe@qDITBlvBnIjofilB5sEy_pX_$K?{JCNCs{}Hb>(H zvS~!$0*8AA8m-WWs#0^Mx^uoEsobRMlpK6Wj?!%v$F|*)A?p%J;;BKO2}XI5KWx+Y zZPk8}M0M++UJJ~{K})LkB~m3XGWJ8x1yT2$GFp9oq(lckg4X3A{ZEKRQ)(|M9TPO+eCByIiS!x`?ZxkLgOt-qbU zE}5Ft%qCZ$n3~m85t-OR_{&w~`Lg)9TI623N1?2T@-;}hV0!Ik-d4w~OD?$s*lGXi zH0-}T%OBSN`*6pu|FeFAF7`|Fl=X5G_}WDp+zF09+j=$oPuI7?#{KWX@cu_X?pXfU zm)V%bUt{@Ax%7$*w{@{ow*AEVy-QaNrNw;eCu6&3aROV9+KJvZY(U8<))lzL&B{MIpVdm9s z|0k*#DGUqr;IuHQ9k(LeE;lweSfDi6x8wl#2v*Ce<*@J-+VIT=jOfww5zjozB9~#K zY6I&%8KpV9p6WH#n;+IYI&7C3|lvoVLvTo=ajb*h^y4frbrkl-5;b@G@ z2mJOn_cRGv9E4Kx85$tluhV>iYjVM>cEP$?GX&Dn^XiUUxwK$07lsBdRH$&*e*;s6 zdcFW04kP5f;%^JI!h81xH-$%CjKf+33C?L}8I4wOOc$%*U^MsLl{h)762B)Laq(KT zKbl%(IAcGZ@vh!FTyQd)1iG6>_{`)uMoKY(MUcGa0O;x&G*3#RGC?z@av#gjqM1;} zqVnPxSbTgkk8`Pj=OJ__?yyW?+jZvbwPQu@j!a9-n`CC(k->(sawC%mDfEmUm#~-` zZPJ$VcT%uhdGvrLxQI@hH;s)GCnS7&ud zpFjQI*S~H4`CQb`Nvrz#KiK5&+U@sYswc_&CDhJ;e_yQJ%-H(j(@;D84z+XgB-Kv; zifX5x_5qewNRD6`W1{UUCy8KCH?^jC!s@0oeU;`v{J4%!Q#U8m4^lT-u;-+1P6u`K z@#>~+4f^KA^-a?_!O3V6(-ZYgG*KE9PMU*3;T#ms6ZMzrkqW0{b!aT%u^Nlli^j<& z7@p_7`oVr%{O}*oMdRc{RdfDK4jo;&{XR_NL|H$VzA3l;Z(nD-_dgrke)=@@O}9he zL?hf2R5;x$Dx7-S2RK?G>0}sVlI`jy3O*>CS~EOdWz+YZSNq@I{$~)Jrfho72PvB@ z*mF`g+ote?vU!5C84SAS#dS^75W&f468?$0W-zFlJO_iSIjEYaDlYc1o12pkUBy3E zS8;pSHE%c3&&l)AHJw&nQ@erT$_@EpN~cnWOK6{O(&*d2eP`?q=oW^jpnf`C>ZkK0 z)lcV&>ZhLe0VY>Sj^`m`+Uh0gZ`}+RjG=&t&)elfeS+M7% zkhab92Zi(mg|y8ObWljor;wTk4NgXra8Fc7y+I-6IT#euK_NA3H%(wVyaV>}yNf>j zNnO|d51tq3@Bf9?aQ}NR?xgoWmCGMS zUj8Ej}1%9~Y%I`necH{mB%NgGP z>%|?*|D>O2x4$%j*DlWBPIUYZ;tQsMcs%s|_V<6`x9wP=11L=q*UTPd(FKYGqpftlDq4 zQTYJ1cJ?Jf#a@C6z$D<^9z9bucmK^pSFTbYLfxiSy3EL3vh)*yquVIWqGg(;^}|0a z<;m0r>ZWCv&+$Wjmrq>Fd+jzmr&Hg5%Zl{{(&*rS$oN*{{kMVt`*HuU-O=}8S?(d% zp(oV5QB;qdFPzC}xBY$e4>Z>QJ|fmabsFK9Pf1_F_CIBjWuP2&J+E}rkxBxV`}OZ5 z_Oz-LGchn3)y2rRtRthziYU>d*=13Fj!d{mXCm6|vIi6oN}3r}Rp-cqcXS?DH0av( z5x$Qb33(JpR|KTl;|+-4@-jf_w=bdGJVzP)?lL?k3)c$EiXPo+C5J^$mPdx>&w8s- z;2go>apZ)a7f+||bZJcjFmYTE0kCu&=!xvuoqRZ#tjc10L_UX>X=xeP^*}ON239n2 z;&|dOEo(CIyvUCd5V=c#Xql_=OcJ}x$;xqpVCe-bZ)r`J5aujb(JBCT(wfZlu`M%w zL=BxG>@mVPJRppxk7}gpNsTn|LXbqx3WUH8qp9U5PP_ukAelz~$_MSUh;ubk=dfv? zqq-+gq`IfJ?}ok|1c5yTZoFJNtI$u}$euv$d*!xEtFu=((JZUC>23#-(eh!$9}nR* rd3?{|5j7|NjF3x~*4-0R8{~ag~A% literal 0 HcmV?d00001 diff --git a/question/tests/generator/lib.php b/question/tests/generator/lib.php index 39e25663ffe..c9dd94b585c 100644 --- a/question/tests/generator/lib.php +++ b/question/tests/generator/lib.php @@ -71,7 +71,8 @@ class core_question_generator extends component_generator_base { if (isset($record['parent'])) { $record['contextid'] = $DB->get_field('question_categories', 'contextid', ['id' => $record['parent']]); } else { - $record['contextid'] = context_system::instance()->id; + $qbank = $this->datagenerator->create_module('qbank', ['course' => SITEID]); + $record['contextid'] = context_module::instance($qbank->cmid)->id; } } if (!isset($record['parent'])) { @@ -162,33 +163,20 @@ class core_question_generator extends component_generator_base { } /** - * Setup a course category, course, a question category, and 2 questions - * for testing. + * Set up a course category, a course, a mod_qbank instance, a question category for the mod_qbank instance, + * and 2 questions for testing. * - * @param string $type The type of question category to create. - * @return array The created data objects + * @return array of the data objects mentioned above */ - public function setup_course_and_questions($type = 'course') { + public function setup_course_and_questions() { $datagenerator = $this->datagenerator; $category = $datagenerator->create_category(); $course = $datagenerator->create_course([ 'numsections' => 5, 'category' => $category->id ]); - - switch ($type) { - case 'category': - $context = context_coursecat::instance($category->id); - break; - - case 'system': - $context = context_system::instance(); - break; - - default: - $context = context_course::instance($course->id); - break; - } + $qbank = $datagenerator->create_module('qbank', ['course' => $course->id]); + $context = context_module::instance($qbank->cmid); $qcat = $this->create_question_category(['contextid' => $context->id]); @@ -197,7 +185,7 @@ class core_question_generator extends component_generator_base { $this->create_question('shortanswer', null, ['category' => $qcat->id]), ]; - return [$category, $course, $qcat, $questions]; + return [$category, $course, $qcat, $questions, $qbank]; } /** diff --git a/question/tests/privacy/provider_test.php b/question/tests/privacy/provider_test.php index 059a8e29d17..a0b3519bcea 100644 --- a/question/tests/privacy/provider_test.php +++ b/question/tests/privacy/provider_test.php @@ -197,15 +197,15 @@ class provider_test extends \core_privacy\tests\provider_testcase { // Create one question as each user in diferent contexts. $this->setUser($user); $userdata = $questiongenerator->setup_course_and_questions(); - $expectedcontext = \context_course::instance($userdata[1]->id); + $expectedcontext = \context_module::instance($userdata[4]->cmid); $this->setUser($otheruser); $otheruserdata = $questiongenerator->setup_course_and_questions(); - $unexpectedcontext = \context_course::instance($otheruserdata[1]->id); + $unexpectedcontext = \context_module::instance($otheruserdata[4]->cmid); // And create another one where we'll update a question as the test user. $moreotheruserdata = $questiongenerator->setup_course_and_questions(); - $otherexpectedcontext = \context_course::instance($moreotheruserdata[1]->id); + $otherexpectedcontext = \context_module::instance($moreotheruserdata[4]->cmid); $morequestions = $moreotheruserdata[3]; // Update the third set of questions. @@ -451,17 +451,17 @@ class provider_test extends \core_privacy\tests\provider_testcase { $this->setUser($user2); $user2data = $questiongenerator->setup_course_and_questions(); - $course1context = \context_course::instance($user1data[1]->id); - $course1questions = $user1data[3]; + $qbankcontext = \context_module::instance($user1data[4]->cmid); + $questions = $user1data[3]; // Log in as user3 and update the questions in course1. $this->setUser($user3); - foreach ($course1questions as $question) { + foreach ($questions as $question) { $questiongenerator->update_question($question); } - $userlist = new \core_privacy\local\request\userlist($course1context, 'core_question'); + $userlist = new \core_privacy\local\request\userlist($qbankcontext, 'core_question'); provider::get_users_in_context($userlist); // User1 has created questions and user3 has edited them. @@ -486,16 +486,16 @@ class provider_test extends \core_privacy\tests\provider_testcase { // Create one question as each user in different contexts. $this->setUser($user1); - $course1data = $questiongenerator->setup_course_and_questions(); - $course1 = $course1data[1]; - $course1qcat = $course1data[2]; - $course1questions = $course1data[3]; - $course1context = \context_course::instance($course1->id); + $coursedata = $questiongenerator->setup_course_and_questions(); + $qbank = $coursedata[4]; + $course1qcat = $coursedata[2]; + $questions = $coursedata[3]; + $qbankcontext = \context_module::instance($qbank->cmid); // Log in as user2 and update the questions in course1. $this->setUser($user2); - foreach ($course1questions as $question) { + foreach ($questions as $question) { $questiongenerator->update_question($question); } @@ -508,7 +508,7 @@ class provider_test extends \core_privacy\tests\provider_testcase { $this->setUser($user1); $questiongenerator->setup_course_and_questions(); - $approveduserlist = new \core_privacy\local\request\approved_userlist($course1context, 'core_question', + $approveduserlist = new \core_privacy\local\request\approved_userlist($qbankcontext, 'core_question', [$user1->id, $user2->id]); provider::delete_data_for_users($approveduserlist); @@ -521,7 +521,7 @@ class provider_test extends \core_privacy\tests\provider_testcase { JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid WHERE qc.contextid = ? AND (q.createdby = ? OR q.modifiedby = ? OR q.createdby = ? OR q.modifiedby = ?)", - [$course1context->id, $user1->id, $user1->id, $user2->id, $user2->id]) + [$qbankcontext->id, $user1->id, $user1->id, $user2->id, $user2->id]) ); // User3 data in course1 should not change. @@ -532,7 +532,7 @@ class provider_test extends \core_privacy\tests\provider_testcase { JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid WHERE qc.contextid = ? AND (q.createdby = ? OR q.modifiedby = ?)", - [$course1context->id, $user3->id, $user3->id]) + [$qbankcontext->id, $user3->id, $user3->id]) ); // User1 has authored 2 questions in another course. diff --git a/question/type/essay/tests/restore_test.php b/question/type/essay/tests/restore_test.php index 9d3d2eda89e..afb7731e680 100644 --- a/question/type/essay/tests/restore_test.php +++ b/question/type/essay/tests/restore_test.php @@ -43,8 +43,9 @@ class restore_test extends \restore_date_testcase { // Create a course with one essay question in its question bank. $generator = $this->getDataGenerator(); $course = $generator->create_course(); - $contexts = new \core_question\local\bank\question_edit_contexts(\context_course::instance($course->id)); - $category = question_make_default_categories($contexts->all()); + $qbank = $generator->create_module('qbank', ['course' => $course->id]); + $context = \context_module::instance($qbank->cmid); + $category = question_make_default_categories([$context]); $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); $essay = $questiongenerator->create_question('essay', null, array('category' => $category->id)); @@ -54,9 +55,15 @@ class restore_test extends \restore_date_testcase { // Do backup and restore. $newcourseid = $this->backup_and_restore($course); + $modinfo = get_fast_modinfo($newcourseid); + $newqbanks = array_filter( + $modinfo->get_instances_of('qbank'), + static fn($qbank) => $qbank->get_name() === 'Question bank 1' + ); + $newqbank = reset($newqbanks); + // Verify that the restored question has options. - $contexts = new \core_question\local\bank\question_edit_contexts(\context_course::instance($newcourseid)); - $newcategory = question_make_default_categories($contexts->all()); + $newcategory = question_make_default_categories([\context_module::instance($newqbank->id)]); $newessay = $DB->get_record_sql('SELECT q.* FROM {question} q JOIN {question_versions} qv ON qv.questionid = q.id diff --git a/tag/tests/event/events_test.php b/tag/tests/event/events_test.php index a0df0d56895..5c459695dab 100644 --- a/tag/tests/event/events_test.php +++ b/tag/tests/event/events_test.php @@ -100,7 +100,9 @@ class events_test extends \advanced_testcase { global $DB; // Create a course to tag. - $course = $this->getDataGenerator()->create_course(); + $course = self::getDataGenerator()->create_course(); + $qbank = self::getDataGenerator()->create_module('qbank', ['course' => $course->id]); + $qbankcontext = \context_module::instance($qbank->cmid); // Trigger and capture the event for tagging a course. $sink = $this->redirectEvents(); @@ -115,7 +117,7 @@ class events_test extends \advanced_testcase { // Create a question to tag. $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); - $cat = $questiongenerator->create_question_category(); + $cat = $questiongenerator->create_question_category(['contextid' => $qbankcontext->id]); $question = $questiongenerator->create_question('shortanswer', null, array('category' => $cat->id)); // Trigger and capture the event for tagging a question. @@ -129,7 +131,7 @@ class events_test extends \advanced_testcase { // Check that the tag was added to the question and the event data is valid. $this->assertEquals(1, $DB->count_records('tag_instance', array('component' => 'core'))); $this->assertInstanceOf('\core\event\tag_added', $event); - $this->assertEquals(\context_system::instance(), $event->get_context()); + $this->assertEquals($qbankcontext, $event->get_context()); } /**