And corresponding unit test. Work by Marko Vidberg.
$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);
}
' 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();
'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";
}
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;
+}
--- /dev/null
+<?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);
+ }
+}