Revert "MDL-37028 Integrity check for course modules and sections"
authorDamyon Wiese <damyon@moodle.com>
Tue, 10 Sep 2013 06:31:59 +0000 (14:31 +0800)
committerDamyon Wiese <damyon@moodle.com>
Tue, 10 Sep 2013 06:34:07 +0000 (14:34 +0800)
This is failing unit tests.

This reverts commit 0bac49dc1983824e4861086ed1ab9e13af99d76d.
This reverts commit 1f0a9ce48b3e6f8c5dc6628aa692323909925032.

admin/cli/fix_course_sequence.php [deleted file]
course/lib.php
course/tests/courselib_test.php

diff --git a/admin/cli/fix_course_sequence.php b/admin/cli/fix_course_sequence.php
deleted file mode 100644 (file)
index 8715b13..0000000
+++ /dev/null
@@ -1,125 +0,0 @@
-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * 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
index 2703848..1ba8ff9 100644 (file)
@@ -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('<br>', $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);
index 6ef0cb2..ce02969 100644 (file)
@@ -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));
-    }
 }