From eceecb72f3046e159c75a8d02832de66a28b2d31 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 10 Sep 2013 14:31:59 +0800 Subject: [PATCH] Revert "MDL-37028 Integrity check for course modules and sections" This is failing unit tests. This reverts commit 0bac49dc1983824e4861086ed1ab9e13af99d76d. This reverts commit 1f0a9ce48b3e6f8c5dc6628aa692323909925032. --- admin/cli/fix_course_sequence.php | 125 -------------------------- course/lib.php | 141 +----------------------------- course/tests/courselib_test.php | 120 ------------------------- 3 files changed, 1 insertion(+), 385 deletions(-) delete mode 100644 admin/cli/fix_course_sequence.php diff --git a/admin/cli/fix_course_sequence.php b/admin/cli/fix_course_sequence.php deleted file mode 100644 index 8715b1318e8..00000000000 --- a/admin/cli/fix_course_sequence.php +++ /dev/null @@ -1,125 +0,0 @@ -. - -/** - * This script fixed incorrectly deleted users. - * - * @package core - * @subpackage cli - * @copyright 2013 Marina Glancy - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -define('CLI_SCRIPT', true); - -require(__DIR__.'/../../config.php'); -require_once($CFG->libdir.'/clilib.php'); - -// Get cli options. -list($options, $unrecognized) = cli_get_params( - array( - 'courses' => false, - 'fix' => false, - 'help' => false - ), - array( - 'h' => 'help', - 'c' => 'courses', - 'f' => 'fix' - ) -); - -if ($options['help'] || empty($options['courses'])) { - $help = -"Checks and fixes that course modules and sections reference each other correctly. - -Compares DB fields course_sections.sequence and course_modules.section -checking that: -- course_sections.sequence contains each module id not more than once in the course -- for each moduleid from course_sections.sequence the field course_modules.section - refers to the same section id (this means course_sections.sequence is more - important if they are different) -- each module in the course is present in one of course_sections.sequence -- section sequences do not contain non-existing course modules - -If there are any mismatches, the message is displayed. If --fix is specified, -the records in DB are corrected. - -This script may run for a long time on big systems if called for all courses. - -Avoid executing the script when another user may simultaneously edit any of the -courses being checked (recommended to run in mainenance mode). - -Options: --c, --courses List courses that need to be checked (comma-separated - values or * for all). Required --f, --fix Fix the mismatches in DB. If not specified check only and - report problems to STDERR --h, --help Print out this help - -Example: -\$sudo -u www-data /usr/bin/php admin/cli/fix_course_sequence.php --courses=* -\$sudo -u www-data /usr/bin/php admin/cli/fix_course_sequence.php --courses=2,3,4 --fix -"; - - echo $help; - die; -} - -$courseslist = preg_split('/\s*,\s*/', $options['courses'], -1, PREG_SPLIT_NO_EMPTY); -if (in_array('*', $courseslist)) { - $where = ''; - $params = array(); -} else { - list($sql, $params) = $DB->get_in_or_equal($courseslist, SQL_PARAMS_NAMED, 'id'); - $where = 'WHERE id '. $sql; -} -$coursescount = $DB->get_field_sql('SELECT count(id) FROM {course} '. $where, $params); - -if (!$coursescount) { - cli_error('No courses found'); -} -echo "Checking $coursescount courses...\n\n"; - -require_once($CFG->dirroot. '/course/lib.php'); - -$problems = array(); -$courses = $DB->get_fieldset_sql('SELECT id FROM {course} '. $where, $params); -foreach ($courses as $courseid) { - $errors = course_integrity_check($courseid, null, null, true, empty($options['fix'])); - if ($errors) { - if (!empty($options['fix'])) { - // Reset the course cache to make sure cache is recalculated next time the course is viewed. - $DB->upgrade_record('course', array('modinfo' => null, 'id' => $courseid)); - } - foreach ($errors as $error) { - cli_problem($error); - } - $problems[] = $courseid; - } else { - echo "Course [$courseid] is OK\n"; - } -} -if (!count($problems)) { - echo "\n...All courses are OK\n"; -} else { - if (!empty($options['fix'])) { - echo "\n...Found and fixed ".count($problems)." courses with problems". "\n"; - } else { - echo "\n...Found ".count($problems)." courses with problems. To fix run:\n"; - echo "\$sudo -u www-data /usr/bin/php admin/cli/fix_course_sequence.php --courses=".join(',', $problems)." --fix". "\n"; - } -} \ No newline at end of file diff --git a/course/lib.php b/course/lib.php index 2703848fcd9..1ba8ff98853 100644 --- a/course/lib.php +++ b/course/lib.php @@ -853,138 +853,6 @@ function print_log_ods($course, $user, $date, $order='l.time DESC', $modname, return true; } -/** - * Checks the integrity of the course data. - * - * In summary - compares course_sections.sequence and course_modules.section. - * - * More detailed, checks that: - * - course_sections.sequence contains each module id not more than once in the course - * - for each moduleid from course_sections.sequence the field course_modules.section - * refers to the same section id (this means course_sections.sequence is more - * important if they are different) - * - ($fullcheck only) each module in the course is present in one of - * course_sections.sequence - * - ($fullcheck only) removes non-existing course modules from section sequences - * - * If there are any mismatches, the changes are made and records are updated in DB. - * - * Course cache is NOT rebuilt if there are any errors! - * - * This function is used each time when course cache is being rebuilt with $fullcheck = false - * and in CLI script admin/cli/fix_course_sequence.php with $fullcheck = true - * - * @param int $courseid id of the course - * @param array $rawmods result of funciton {@link get_course_mods()} - containst - * the list of enabled course modules in the course. Retrieved from DB if not specified. - * Argument ignored in cashe of $fullcheck, the list is retrieved form DB anyway. - * @param array $sections records from course_sections table for this course. - * Retrieved from DB if not specified - * @param bool $fullcheck Will add orphaned modules to their sections and remove non-existing - * course modules from sequences. Only to be used in site maintenance mode when we are - * sure that another user is not in the middle of the process of moving/removing a module. - * @param bool $checkonly Only performs the check without updating DB, outputs all errors as debug messages. - * @return array array of messages with found problems. Empty output means everything is ok - */ -function course_integrity_check($courseid, $rawmods = null, $sections = null, $fullcheck = false, $checkonly = false) { - global $DB; - $messages = array(); - if ($sections === null) { - $sections = $DB->get_records('course_sections', array('course' => $courseid), 'section', 'id,section,sequence'); - } - if ($fullcheck) { - // Retrieve all records from course_modules regardless of module type visibility. - $rawmods = $DB->get_records('course_modules', array('course' => $courseid), 'id', 'id,section'); - } - if ($rawmods === null) { - $rawmods = get_course_mods($courseid); - } - if (!$fullcheck && (empty($sections) || empty($rawmods))) { - // If either of the arrays is empty, no modules are displayed anyway. - return true; - } - $debuggingprefix = 'Failed integrity check for course ['.$courseid.']. '; - - // First make sure that each module id appears in section sequences only once. - // If it appears in several section sequences the last section wins. - // If it appears twice in one section sequence, the first occurence wins. - $modsection = array(); - foreach ($sections as $sectionid => $section) { - $sections[$sectionid]->newsequence = $section->sequence; - if (!empty($section->sequence)) { - $sequence = explode(",", $section->sequence); - $sequenceunique = array_unique($sequence); - if (count($sequenceunique) != count($sequence)) { - // Some course module id appears in this section sequence more than once. - ksort($sequenceunique); // Preserve initial order of modules. - $sequence = array_values($sequenceunique); - $sections[$sectionid]->newsequence = join(',', $sequence); - $messages[] = $debuggingprefix.'Sequence for course section ['. - $sectionid.'] is "'.$sections[$sectionid]->sequence.'", must be "'.$sections[$sectionid]->newsequence.'"'; - } - foreach ($sequence as $cmid) { - if (array_key_exists($cmid, $modsection) && isset($rawmods[$cmid])) { - // Some course module id appears to be in more than one section's sequences. - $wrongsectionid = $modsection[$cmid]; - $sections[$wrongsectionid]->newsequence = trim(preg_replace("/,$cmid,/", ',', ','.$sections[$wrongsectionid]->newsequence. ','), ','); - $messages[] = $debuggingprefix.'Course module ['.$cmid.'] must be removed from sequence of section ['. - $wrongsectionid.'] because it is also present in sequence of section ['.$sectionid.']'; - } - $modsection[$cmid] = $sectionid; - } - } - } - - // Add orphaned modules to their sections if they exist or to section 0 otherwise. - if ($fullcheck) { - foreach ($rawmods as $cmid => $mod) { - if (!isset($modsection[$cmid])) { - // This is a module that is not mentioned in course_section.sequence at all. - // Add it to the section $mod->section or to the last available section. - if ($mod->section && isset($sections[$mod->section])) { - $modsection[$cmid] = $mod->section; - } else { - $firstsection = reset($sections); - $modsection[$cmid] = $firstsection->id; - } - $sections[$modsection[$cmid]]->newsequence = trim($sections[$modsection[$cmid]]->newsequence.','.$cmid, ','); - $messages[] = $debuggingprefix.'Course module ['.$cmid.'] is missing from sequence of section ['. - $sectionid.']'; - } - } - foreach ($modsection as $cmid => $sectionid) { - if (!isset($rawmods[$cmid])) { - // Section $sectionid refers to module id that does not exist. - $sections[$sectionid]->newsequence = trim(preg_replace("/,$cmid,/", ',', ','.$sections[$sectionid]->newsequence.','), ','); - $messages[] = $debuggingprefix.'Course module ['.$cmid. - '] does not exist but is present in the sequence of section ['.$sectionid.']'; - } - } - } - - // Update changed sections. - if (!$checkonly && !empty($messages)) { - foreach ($sections as $sectionid => $section) { - if ($section->newsequence !== $section->sequence) { - $DB->update_record('course_sections', array('id' => $sectionid, 'sequence' => $section->newsequence)); - } - } - } - - // Now make sure that all modules point to the correct sections. - foreach ($rawmods as $cmid => $mod) { - if (isset($modsection[$cmid]) && $modsection[$cmid] != $mod->section) { - if (!$checkonly) { - $DB->update_record('course_modules', array('id' => $cmid, 'section' => $modsection[$cmid])); - } - $messages[] = $debuggingprefix.'Course module ['.$cmid. - '] points to section ['.$mod->section.'] instead of ['.$modsection[$cmid].']'; - } - } - - return $messages; -} - /** * For a given course, returns an array of course activity objects * Each item in the array contains he following properties: @@ -1016,14 +884,7 @@ function get_array_of_activities($courseid) { return $mod; // always return array } - if ($sections = $DB->get_records('course_sections', array('course' => $courseid), 'section ASC', 'id,section,sequence')) { - // First check and correct obvious mismatches between course_sections.sequence and course_modules.section. - if ($errormessages = course_integrity_check($courseid, $rawmods, $sections)) { - debugging(join('
', $errormessages)); - $rawmods = get_course_mods($courseid); - $sections = $DB->get_records('course_sections', array('course' => $courseid), 'section ASC', 'id,section,sequence'); - } - // Build array of activities. + if ($sections = $DB->get_records("course_sections", array("course"=>$courseid), "section ASC")) { foreach ($sections as $section) { if (!empty($section->sequence)) { $sequence = explode(",", $section->sequence); diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 6ef0cb2718b..ce029697804 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1751,124 +1751,4 @@ class core_course_courselib_testcase extends advanced_testcase { $expectedlegacydata = array($course->id, "course", "editsection", 'editsection.php?id=' . $id, $sectionnum); $this->assertEventLegacyLogData($expectedlegacydata, $event); } - - public function test_course_integrity_check() { - global $DB; - - $this->resetAfterTest(true); - $course = $this->getDataGenerator()->create_course(array('numsections' => 1), - array('createsections'=>true)); - - $forum = $this->getDataGenerator()->create_module('forum', array('course' => $course->id), - array('section' => 0)); - $page = $this->getDataGenerator()->create_module('page', array('course' => $course->id), - array('section' => 0)); - $quiz = $this->getDataGenerator()->create_module('quiz', array('course' => $course->id), - array('section' => 0)); - $correctseq = join(',', array($forum->cmid, $page->cmid, $quiz->cmid)); - - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($correctseq, $section0->sequence); - $this->assertEmpty($section1->sequence); - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section0->id, $cms[$page->cmid]->section); - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - $this->assertEmpty(course_integrity_check($course->id)); - - // Now let's make manual change in DB and let course_integrity_check() fix it: - - // 1. Module appears twice in one section. - $DB->update_record('course_sections', array('id' => $section0->id, 'sequence' => $section0->sequence. ','. $page->cmid)); - $this->assertEquals( - array('Failed integrity check for course ['. $course->id. - ']. Sequence for course section ['. $section0->id. '] is "'. - $section0->sequence. ','. $page->cmid. '", must be "'. - $section0->sequence. '"'), - course_integrity_check($course->id)); - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($correctseq, $section0->sequence); - $this->assertEmpty($section1->sequence); - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section0->id, $cms[$page->cmid]->section); - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - - // 2. Module appears in two sections (last section wins). - $DB->update_record('course_sections', array('id' => $section1->id, 'sequence' => ''. $page->cmid)); - // First message about double mentioning in sequence, second message about wrong section field for $page. - $this->assertEquals(array( - 'Failed integrity check for course ['. $course->id. ']. Course module ['. $page->cmid. - '] must be removed from sequence of section ['. $section0->id. - '] because it is also present in sequence of section ['. $section1->id. ']', - 'Failed integrity check for course ['. $course->id. ']. Course module ['. $page->cmid. - '] points to section ['. $section0->id. '] instead of ['. $section1->id. ']'), - course_integrity_check($course->id)); - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($forum->cmid. ','. $quiz->cmid, $section0->sequence); - $this->assertEquals(''. $page->cmid, $section1->sequence); - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section1->id, $cms[$page->cmid]->section); - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - - // 3. Module id is not present in course_section.sequence (integrity check with $fullcheck = false). - $DB->update_record('course_sections', array('id' => $section1->id, 'sequence' => '')); - $this->assertEmpty(course_integrity_check($course->id)); // Not an error! - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($forum->cmid. ','. $quiz->cmid, $section0->sequence); - $this->assertEmpty($section1->sequence); - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section1->id, $cms[$page->cmid]->section); // Not changed. - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - - // 4. Module id is not present in course_section.sequence (integrity check with $fullcheck = true). - $this->assertEquals(array('Failed integrity check for course ['. $course->id. ']. Course module ['. - $page->cmid. '] is missing from sequence of section ['. $section1->id. ']'), - course_integrity_check($course->id, null, null, true)); // Error! - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($forum->cmid. ','. $quiz->cmid, $section0->sequence); - $this->assertEquals(''. $page->cmid, $section1->sequence); // Yay, module added to section. - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section1->id, $cms[$page->cmid]->section); // Not changed. - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - - // 5. Module id is not present in course_section.sequence and it's section is invalid (integrity check with $fullcheck = true). - $DB->update_record('course_modules', array('id' => $page->cmid, 'section' => 8765)); - $DB->update_record('course_sections', array('id' => $section1->id, 'sequence' => '')); - $this->assertEquals(array( - 'Failed integrity check for course ['. $course->id. ']. Course module ['. $page->cmid. - '] is missing from sequence of section ['. $section1->id. ']', - 'Failed integrity check for course ['. $course->id. ']. Course module ['. $page->cmid. - '] points to section [8765] instead of ['. $section0->id. ']'), - course_integrity_check($course->id, null, null, true)); - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($forum->cmid. ','. $quiz->cmid. ','. $page->cmid, $section0->sequence); // Module added to section. - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section0->id, $cms[$page->cmid]->section); // Section changed to section0. - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - - // 6. Module is deleted from course_modules but not deleted in sequence (integrity check with $fullcheck = true). - $DB->delete_records('course_modules', array('id' => $page->cmid)); - $this->assertEquals(array('Failed integrity check for course ['. $course->id. ']. Course module ['. - $page->cmid. '] does not exist but is present in the sequence of section ['. $section0->id. ']'), - course_integrity_check($course->id, null, null, true)); - $section0 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 0)); - $section1 = $DB->get_record('course_sections', array('course' => $course->id, 'section' => 1)); - $cms = $DB->get_records('course_modules', array('course' => $course->id), 'id', 'id,section'); - $this->assertEquals($forum->cmid. ','. $quiz->cmid, $section0->sequence); - $this->assertEmpty($section1->sequence); - $this->assertEquals($section0->id, $cms[$forum->cmid]->section); - $this->assertEquals($section0->id, $cms[$quiz->cmid]->section); - $this->assertEquals(2, count($cms)); - } }