MDL-34684 tool_health: Added health check for course category structure
authorLogan Reynolds <logan.reynolds@remote-learner.net>
Fri, 7 Jun 2013 02:16:44 +0000 (21:16 -0500)
committerDavid Monllao <davidm@moodle.com>
Thu, 4 Dec 2014 02:42:58 +0000 (10:42 +0800)
And corresponding unit test. Work by Marko Vidberg.

admin/tool/health/index.php
lib/adminlib.php
lib/tests/adminlib_test.php [new file with mode: 0644]

index f0fe6c8..6a03291 100644 (file)
@@ -603,34 +603,10 @@ class problem_000017 extends problem_base {
             $categories = $DB->get_records('question_categories', array(), 'id');
 
             // Look for missing parents.
-            $missingparent = array();
-            foreach ($categories as $category) {
-                if ($category->parent != 0 && !array_key_exists($category->parent, $categories)) {
-                    $missingparent[$category->id] = $category;
-                }
-            }
+            $missingparent = health_category_find_missing_parents($categories);
 
             // Look for loops.
-            $loops = array();
-            while (!empty($categories)) {
-                $current = array_pop($categories);
-                $thisloop = array($current->id => $current);
-                while (true) {
-                    if (isset($thisloop[$current->parent])) {
-                        // Loop detected
-                        $loops[$current->id] = $thisloop;
-                        break;
-                    } else if (!isset($categories[$current->parent])) {
-                        // Got to the top level, or a category we already know is OK.
-                        break;
-                    } else {
-                        // Continue following the path.
-                        $current = $categories[$current->parent];
-                        $thisloop[$current->id] = $current;
-                        unset($categories[$current->id]);
-                    }
-                }
-            }
+            $loops = health_category_find_loops($categories);
 
             $answer = array($missingparent, $loops);
         }
@@ -651,28 +627,122 @@ class problem_000017 extends problem_base {
                 ' structures by the question_categories.parent field. Sometimes ' .
                 ' this tree structure gets messed up.</p>';
 
+        $description .= health_category_list_missing_parents($missingparent);
+        $description .= health_category_list_loops($loops);
+
+        return $description;
+    }
+
+    /**
+     * Outputs resolutions to problems outlined in MDL-34684 with items having themselves as parent
+     *
+     * @link https://tracker.moodle.org/browse/MDL-34684
+     * @return string Formatted html to be output to the browser with instructions and sql statements to run
+     */
+    function solution() {
+        global $CFG;
+        list($missingparent, $loops) = $this->find_problems();
+
+        $solution = '<p>Consider executing the following SQL queries. These fix ' .
+                'the problem by moving some categories to the top level.</p>';
+
         if (!empty($missingparent)) {
-            $description .= '<p>The following categories are missing their parents:</p><ul>';
-            foreach ($missingparent as $cat) {
-                $description .= "<li>Category $cat->id: " . s($cat->name) . "</li>\n";
-            }
-            $description .= "</ul>\n";
+            $solution .= "<pre>UPDATE " . $CFG->prefix . "question_categories\n" .
+                    "        SET parent = 0\n" .
+                    "        WHERE id IN (" . implode(',', array_keys($missingparent)) . ");</pre>\n";
         }
 
         if (!empty($loops)) {
-            $description .= '<p>The following categories form a loop of parents:</p><ul>';
-            foreach ($loops as $loop) {
-                $description .= "<li><ul>\n";
-                foreach ($loop as $cat) {
-                    $description .= "<li>Category $cat->id: " . s($cat->name) . " has parent $cat->parent</li>\n";
-                }
-                $description .= "</ul></li>\n";
-            }
-            $description .= "</ul>\n";
+            $solution .= "<pre>UPDATE " . $CFG->prefix . "question_categories\n" .
+                    "        SET parent = 0\n" .
+                    "        WHERE id IN (" . implode(',', array_keys($loops)) . ");</pre>\n";
         }
 
+        return $solution;
+    }
+}
+
+/**
+ * Check course categories tree structure for problems.
+ */
+class problem_000018 extends problem_base {
+    /**
+     * Generate title for this problem.
+     *
+     * @return string Title of problem.
+     */
+    function title() {
+        return 'Course categories tree structure';
+    }
+
+    /**
+     * Search for problems in the course categories.
+     *
+     * @uses $DB
+     * @return array List of categories that contain missing parents or loops.
+     */
+    function find_problems() {
+        global $DB;
+        static $answer = null;
+
+        if (is_null($answer)) {
+            $categories = $DB->get_records('course_categories', array(), 'id');
+
+            // Look for missing parents.
+            $missingparent = health_category_find_missing_parents($categories);
+
+            // Look for loops.
+            $loops = health_category_find_loops($categories);
+
+            $answer = array($missingparent, $loops);
+        }
+
+        return $answer;
+    }
+
+    /**
+     * Check if the problem exists.
+     *
+     * @return boolean True if either missing parents or loops found
+     */
+    function exists() {
+        list($missingparent, $loops) = $this->find_problems();
+        return !empty($missingparent) || !empty($loops);
+    }
+
+    /**
+     * Set problem severity.
+     *
+     * @return constant Problem severity.
+     */
+    function severity() {
+        return SEVERITY_ANNOYANCE;
+    }
+
+    /**
+     * Generate problem description.
+     *
+     * @return string HTML containing details of the problem.
+     */
+    function description() {
+        list($missingparent, $loops) = $this->find_problems();
+
+        $description = '<p>The course categories should be arranged into tree ' .
+                ' structures by the course_categories.parent field. Sometimes ' .
+                ' this tree structure gets messed up.</p>';
+
+        $description .= health_category_list_missing_parents($missingparent);
+        $description .= health_category_list_loops($loops);
+
         return $description;
     }
+
+    /**
+     * Generate solution text.
+     *
+     * @uses $CFG
+     * @return string HTML containing the suggested solution.
+     */
     function solution() {
         global $CFG;
         list($missingparent, $loops) = $this->find_problems();
@@ -681,14 +751,14 @@ class problem_000017 extends problem_base {
                 'the problem by moving some categories to the top level.</p>';
 
         if (!empty($missingparent)) {
-            $solution .= "<pre>UPDATE " . $CFG->prefix . "question_categories\n" .
-                    "        SET parent = 0\n" .
+            $solution .= "<pre>UPDATE " . $CFG->prefix . "course_categories\n" .
+                    "        SET parent = 0, depth = 1, path = CONCAT('/', id)\n" .
                     "        WHERE id IN (" . implode(',', array_keys($missingparent)) . ");</pre>\n";
         }
 
         if (!empty($loops)) {
-            $solution .= "<pre>UPDATE " . $CFG->prefix . "question_categories\n" .
-                    "        SET parent = 0\n" .
+            $solution .= "<pre>UPDATE " . $CFG->prefix . "course_categories\n" .
+                    "        SET parent = 0, depth = 1, path = CONCAT('/', id)\n" .
                     "        WHERE id IN (" . implode(',', array_keys($loops)) . ");</pre>\n";
         }
 
index 0c6aafe..28bce28 100644 (file)
@@ -8920,3 +8920,100 @@ class admin_setting_php_extension_enabled extends admin_setting {
         return $o;
     }
 }
+
+/**
+ * Given a list of categories, this function searches for ones
+ * that have a missing parent category.
+ *
+ * @param array $categories List of categories.
+ * @return array List of categories with missing parents.
+ */
+function health_category_find_missing_parents($categories) {
+    $missingparent = array();
+
+    foreach ($categories as $category) {
+        if ($category->parent != 0 && !array_key_exists($category->parent, $categories)) {
+            $missingparent[$category->id] = $category;
+        }
+    }
+
+    return $missingparent;
+}
+
+/**
+ * Generates a list of categories with missing parents.
+ *
+ * @param array $missingparent List of categories with missing parents.
+ * @return string Bullet point list of categories with missing parents.
+ */
+function health_category_list_missing_parents($missingparent) {
+    $description = '';
+
+    if (!empty($missingparent)) {
+        $description .= '<p>The following categories are missing their parents:</p><ul>';
+        foreach ($missingparent as $cat) {
+            $description .= "<li>Category $cat->id: " . s($cat->name) . "</li>\n";
+        }
+        $description .= "</ul>\n";
+    }
+
+    return $description;
+}
+
+/**
+ * Given a list of categories, this function searches for ones
+ * that have loops to previous parent categories.
+ *
+ * @param array $categories List of categories.
+ * @return array List of categories with loops.
+ */
+function health_category_find_loops($categories) {
+    $loops = array();
+
+    // TODO: Improve this code so that if the root node is included in a loop, only children in the actual loop are reported
+    while (!empty($categories)) {
+        $current = array_pop($categories);
+        $thisloop = array($current->id => $current);
+        while (true) {
+            if (isset($thisloop[$current->parent])) {
+                // Loop detected
+                $loops[$current->id] = $thisloop;
+                break;
+            } else if (!isset($categories[$current->parent])) {
+                // Got to the top level, or a category we already know is OK.
+                break;
+            } else {
+                // Continue following the path.
+                $current = $categories[$current->parent];
+                $thisloop[$current->id] = $current;
+                unset($categories[$current->id]);
+            }
+        }
+    }
+
+    return $loops;
+}
+
+/**
+ * Generates a list of categories with loops.
+ *
+ * @param array $loops List of categories with loops.
+ * @return string Bullet point list of categories with loops.
+ */
+function health_category_list_loops($loops) {
+    $description = '';
+
+    if (!empty($loops)) {
+        $description .= '<p>The following categories form a loop of parents:</p><ul>';
+        foreach ($loops as $loop) {
+            $description .= "<li><ul>\n";
+            foreach ($loop as $cat) {
+                $description .= "<li>Category $cat->id: " . s($cat->name) . " has parent $cat->parent</li>\n";
+            }
+            $description .= "</ul></li>\n";
+        }
+        $description .= "</ul>\n";
+    }
+
+    return $description;
+}
diff --git a/lib/tests/adminlib_test.php b/lib/tests/adminlib_test.php
new file mode 100644 (file)
index 0000000..e5f514e
--- /dev/null
@@ -0,0 +1,221 @@
+<?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/>.
+
+/**
+ * Unit tests for ../adminlib.php
+ *
+ * @package    core
+ * @category   phpunit
+ * @copyright  2012 Petr Skoda {@link http://skodak.org}
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->libdir.'/adminlib.php');
+
+class healthindex_testcase extends advanced_testcase {
+
+    /**
+     * Data provider for test_health_category_find_loops
+     */
+    public static function provider_loop_categories() {
+        return array(
+           // One item loop including root
+           0 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 1)
+                        ),
+                        array(
+                            '1' => array(
+                                '1' => (object) array('id' => 1, 'parent' => 1)
+                            ),
+                        ),
+                ),
+           // One item loop including root
+           0 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 1)
+                        ),
+                        array(
+                            '1' => array(
+                                '1' => (object) array('id' => 1, 'parent' => 1)
+                            ),
+                        ),
+                ),
+           // One item loop not including root
+           1 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 0),
+                            '2' => (object) array('id' => 2, 'parent' => 2)
+                        ),
+                        array(
+                            '2' => array(
+                                '2' => (object) array('id' => 2, 'parent' => 2)
+                            ),
+                        ),
+                ),
+           // Two item loop including root
+           2 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 2),
+                            '2' => (object) array('id' => 2, 'parent' => 1)
+                        ),
+                        array(
+                            '1' => array(
+                                 '2' => (object) array('id' => 2, 'parent' => 1),
+                                 '1' => (object) array('id' => 1, 'parent' => 2),
+                            ),
+                        )
+                ),
+           // Two item loop not including root
+           3 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 0),
+                            '2' => (object) array('id' => 2, 'parent' => 3),
+                            '3' => (object) array('id' => 3, 'parent' => 2),
+                        ),
+                        array(
+                            '2' => array(
+                                '3' => (object) array('id' => 3, 'parent' => 2),
+                                '2' => (object) array('id' => 2, 'parent' => 3),
+                            ),
+                        )
+                ),
+           // Three item loop including root
+           4 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 2),
+                            '2' => (object) array('id' => 2, 'parent' => 3),
+                            '3' => (object) array('id' => 3, 'parent' => 1),
+                        ),
+                        array(
+                            '2' => array(
+                                '3' => (object) array('id' => 3, 'parent' => 1),
+                                '1' => (object) array('id' => 1, 'parent' => 2),
+                                '2' => (object) array('id' => 2, 'parent' => 3),
+                            ),
+                        )
+                ),
+           // Three item loop not including root
+           5 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 0),
+                            '2' => (object) array('id' => 2, 'parent' => 3),
+                            '3' => (object) array('id' => 3, 'parent' => 4),
+                            '4' => (object) array('id' => 4, 'parent' => 2)
+                        ),
+                        array(
+                            '3' => array(
+                                '4' => (object) array('id' => 4, 'parent' => 2),
+                                '2' => (object) array('id' => 2, 'parent' => 3),
+                                '3' => (object) array('id' => 3, 'parent' => 4),
+                            ),
+                        )
+                ),
+           // Multi-loop
+           6 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 2),
+                            '2' => (object) array('id' => 2, 'parent' => 1),
+                            '3' => (object) array('id' => 3, 'parent' => 4),
+                            '4' => (object) array('id' => 4, 'parent' => 5),
+                            '5' => (object) array('id' => 5, 'parent' => 3),
+                            '6' => (object) array('id' => 6, 'parent' => 6),
+                            '7' => (object) array('id' => 7, 'parent' => 1),
+                            '8' => (object) array('id' => 8, 'parent' => 7),
+                        ),
+                        array(
+                            '2' => array(
+                                '1' => (object) array('id' => 1, 'parent' => 2),
+                                '2' => (object) array('id' => 2, 'parent' => 1),
+                                '8' => (object) array('id' => 8, 'parent' => 7),
+                                '7' => (object) array('id' => 7, 'parent' => 1),
+                            ),
+                            '6' => array(
+                                '6' => (object) array('id' => 6, 'parent' => 6),
+                            ),
+                            '4' => array(
+                                '5' => (object) array('id' => 5, 'parent' => 3),
+                                '3' => (object) array('id' => 3, 'parent' => 4),
+                                '4' => (object) array('id' => 4, 'parent' => 5),
+                            ),
+                        )
+                )
+        );
+    }
+
+    /**
+     * Test finding loops between two items referring to each other.
+     *
+     * @dataProvider provider_loop_categories
+     */
+    public function test_health_category_find_loops($categories, $expected) {
+        $loops = health_category_find_loops($categories);
+        $this->assertEquals($expected, $loops);
+    }
+
+    /**
+     * Data provider for test_health_category_find_missing_parents
+     */
+    public static function provider_missing_parent_categories() {
+        return array(
+           // Test for two items, both with direct ancestor (parent) missing
+           0 => array(
+                        array(
+                            '1' => (object) array('id' => 1, 'parent' => 0),
+                            '2' => (object) array('id' => 2, 'parent' => 3),
+                            '4' => (object) array('id' => 4, 'parent' => 5),
+                            '6' => (object) array('id' => 6, 'parent' => 2)
+                        ),
+                        array(
+                            '4' => (object) array('id' => 4, 'parent' => 5),
+                            '2' => (object) array('id' => 2, 'parent' => 3)
+                        ),
+                )
+        );
+    }
+
+    /**
+     * Test finding missing parent categories
+     * @dataProvider provider_missing_parent_categories
+     */
+    public function test_health_category_find_missing_parents($categories, $expected) {
+        $missingparent = health_category_find_missing_parents($categories);
+        $this->assertEquals($expected, $missingparent);
+    }
+
+    /**
+     * Test listing missing parent categories
+     */
+    public function test_health_category_list_missing_parents() {
+        $missingparent = array('1' => (object) array('id' => 2, 'parent' => 3, 'name' => 'test'),
+                               '2' => (object) array('id' => 4, 'parent' => 5, 'name' => 'test2'));
+        $result = health_category_list_missing_parents($missingparent);
+        $this->assertRegExp('/Category 2: test/', $result);
+        $this->assertRegExp('/Category 4: test2/', $result);
+    }
+
+    /**
+     * Test listing loop categories
+     */
+    public function test_health_category_list_loops() {
+        $loops = array('1' => array('1' => (object) array('id' => 2, 'parent' => 3, 'name' => 'test')));
+        $result = health_category_list_loops($loops);
+        $this->assertRegExp('/Category 2: test/', $result);
+    }
+}