From: nicolasconnault Date: Fri, 18 May 2007 08:05:53 +0000 (+0000) Subject: MDL-9506 New grade_category::set_as_parent($children) method (non-static). Crucial... X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=03f01eddedfacbef4a28a6e72463d42341e5f8f3;p=moodle.git MDL-9506 New grade_category::set_as_parent($children) method (non-static). Crucial method for adding categories over children. 4 constraints have been set up, and successfully tested in unit tests. However, the method fails when it comes to updating the children's parent fields. The test data may be missing some required fields. --- diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index dc7bd858ef..d39ed115b9 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -669,5 +669,87 @@ class grade_category extends grade_object { } return $this->parent_category; } + + /** + * Sets this category as the parent for the given children. + * A number of constraints are necessary: + * - The children must all be of the same type and at the same level + * - The children must be consecutive (no gap between them), this assumes they have been correctly ordered previously + * - The children cannot already be top categories + * - The children cannot already have a top category + * @param array $children An array of fully instantiated grade_category OR grade_item objects + * @return boolean Success or Failure + */ + function set_as_parent($children) { + global $CFG; + + // Check type and sortorder of first child + $first_child = current($children); + $first_child_type = get_class($first_child); + $first_child_sortorder = $first_child->get_sortorder(); + + foreach ($children as $child) { + if (get_class($child) != $first_child_type) { + debugging("Violated constraint: Attempted to set a category as a parent over children of 2 different types."); + return false; + } + if ($child->get_sortorder() != $first_child_sortorder++) { + debugging("Violated constraint: Attempted to set a category as a parent over children which were not consecutively arranged (gaps exist)."); + return false; + } + if (grade_tree::get_element_type($child) == 'topcat') { + debugging("Violated constraint: Attempted to set a category over children which are already top categories."); + return false; + } + if ($first_child_type == 'grade_item') { + $child->load_category(); + if (!empty($child->category->parent)) { + debugging("Violated constraint: Attempted to set a category over children that already have a top category."); + return false; + } + } elseif ($first_child_type == 'grade_category') { + if (!empty($child->parent)) { + debugging("Violated constraint: Attempted to set a category over children that already have a top category."); + return false; + } + } else { + debugging("Attempted to set a category over children that are neither grade_items nor grade_categories."); + return false; + } + } + + // We passed all the checks, time to set the category as a parent. + foreach ($children as $child) { + if ($first_child_type == 'grade_item') { + $child->categoryid = $this->id; + if (!$child->update()) { + debugging("Could not set this category as a parent for one of its child grade_items, DB operation failed."); + return false; + } + } elseif ($first_child_type == 'grade_category') { + $child->parent = $this->id; + if (!$child->update()) { + debugging("Could not set this category as a parent for one of its child categories, DB operation failed."); + return false; + } + } + } + + // TODO Assign correct sortorders to the newly assigned children and parent. Simply add 1 to all of them! + $this->load_grade_item(); + $this->grade_item->sortorder = $first_child->get_sortorder(); + + if (!$this->update()) { + debugging("Could not update this category's sortorder in DB."); + return false; + } + + $query = "UPDATE {$CFG->prefix}grade_items SET sortorder = sortorder + 1 WHERE sortorder >= $this->grade_item->sortorder"; + if (!execute_sql($query)) { + debugging("Could not update the sortorder of grade_items listed after this category."); + } else { + return true; + } + } } ?> diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index e15fbe18fd..6e249b0cdf 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -333,6 +333,16 @@ class grade_item extends grade_object { return $category; } + + /** + * Calls upon the get_category method to retrieve the grade_category object + * from the DB and assigns it to $this->category. It also returns the object. + * @return object Grade_category + */ + function load_category() { + $this->category = $this->get_category(); + return $this->category; + } /** * In addition to update() as defined in grade_object, handle the grade_outcome and grade_scale objects. diff --git a/lib/grade/grade_tree.php b/lib/grade/grade_tree.php index 8cad32c30b..5a1f749dc6 100644 --- a/lib/grade/grade_tree.php +++ b/lib/grade/grade_tree.php @@ -54,6 +54,19 @@ class grade_tree { * @var array $need_update */ var $need_update = array(); + + /** + * An array of objects that need inserting in the DB. + * @var array $need_insert + */ + var $need_insert = array(); + + /** + * An array of objects that need deleting from the DB. + * @var array $need_delete + */ + var $need_delete = array(); + /** * Constructor, retrieves and stores a hierarchical array of all grade_category and grade_item @@ -71,6 +84,8 @@ class grade_tree { } else { $this->tree_array = $this->get_tree($fullobjects); } + + $this->first_sortorder = key($this->tree_array); } /** @@ -134,7 +149,9 @@ class grade_tree { } /** - * Given an element object, returns its type (topcat, subcat or item). + * Given an element object, returns its type (topcat, subcat or item). + * The $element can be a straight object (fully instantiated), an array of 'object' and 'children'/'final_grades', or a stdClass element + * as produced by grade_tree::locate_element(). This method supports all three types of inputs. * @param object $element * @return string Type */ @@ -179,8 +196,9 @@ class grade_tree { /** * Removes the given element (a stdClass object or a sortorder), remove_elements - * it from the tree. This does not renumber the tree. - * @var object $element An stdClass object typically returned by $this->locate(), or a sortorder + * it from the tree. This does not renumber the tree. If a sortorder (int) is given, this + * method will first retrieve the referenced element from the tree, then re-run the method with that object. + * @var object $element An stdClass object typically returned by $this->locate(), or a sortorder (int) * @return boolean */ function remove_element($element) { @@ -203,6 +221,13 @@ class grade_tree { eval("unset($element_to_unset);"); + if (empty($element->element['object'])) { + debugging("Could not delete this element from the DB due to missing information."); + return false; + } + + $this->need_delete[] = $element->element['object']; + return true; } else { $element = $this->locate_element($element); @@ -286,9 +311,16 @@ class grade_tree { eval("array_splice($element_to_splice, \$position + \$offset, 0, \$destination_array);"); + if (!is_object($new_element)) { + debugging("Could not insert this element into the DB due to missing information."); + return false; + } + + $this->need_insert[] = $new_element; + return true; } - + /** * Moves an existing element in the tree to another position OF EQUAL LEVEL. This * constraint is essential and very important. @@ -326,7 +358,11 @@ class grade_tree { $sortorder = $starting_sortorder; if (empty($starting_sortorder)) { - $sortorder = $this->first_sortorder - 1; + if (empty($this->first_sortorder)) { + debugging("The tree's first_order variable isn't set, you must provide a starting_sortorder to the renumber method."); + return false; + } + $sortorder = $this->first_sortorder - 1; } $newtree = array(); diff --git a/lib/simpletest/grade/simpletest/testgradecategory.php b/lib/simpletest/grade/simpletest/testgradecategory.php index c50fb268a9..22cf81d369 100755 --- a/lib/simpletest/grade/simpletest/testgradecategory.php +++ b/lib/simpletest/grade/simpletest/testgradecategory.php @@ -76,7 +76,7 @@ class grade_category_test extends gradelib_test { $grade_category->fullname = 'unittestcategory4'; $grade_category->courseid = $this->courseid; - $grade_category->aggregation = GRADE_AGGREGATE_MODE; + $grade_category->aggregation = GRADE_AGGREGATE_MEAN; $grade_category->keephigh = 100; $grade_category->droplow = 10; $grade_category->hidden = 0; @@ -217,5 +217,44 @@ class grade_category_test extends gradelib_test { $raw_grade->insert(); return $raw_grade->gradevalue; } + + function test_grade_category_set_as_parent() { + // There are 4 constraints which, if violated, should return false and trigger a debugging message. Test each of them + $grade_category = new grade_category(); + $grade_category->fullname = 'new topcategory'; + $grade_category->courseid = $this->courseid; + + // 1. mixed types of children + $child1 = new grade_item(); + $child1->sortorder = 1; + $child2 = new grade_category(); + $child2->grade_item = new grade_item(); + $child2->grade_item->sortorder = 2; + $this->assertFalse($grade_category->set_as_parent(array($child1, $child2))); + + // 2. Non-consecutive children + $child1 = new grade_item(); + $child2 = new grade_item(); + $child1->sortorder = 1; + $child2->sortorder = 3; + $this->assertFalse($grade_category->set_as_parent(array($child1, $child2))); + + // 3. Child is a top category + $child1 = new grade_category($this->grade_categories[0]); + $this->assertFalse($grade_category->set_as_parent(array($child1))); + + // 4. Child already has a top category + $child1 = new grade_item($this->grade_items[0]); + $this->assertFalse($grade_category->set_as_parent(array($child1))); + + // Now test setting parent correctly + $child1 = new grade_item(); + $child2 = new grade_item(); + $child1->itemname = 'new grade_item'; + $child2->itemname = 'new grade_item'; + $child1->sortorder = 1; + $child2->sortorder = 2; + $this->assertTrue($grade_category->set_as_parent(array($child1, $child2))); + } } ?> diff --git a/lib/simpletest/grade/simpletest/testgradetree.php b/lib/simpletest/grade/simpletest/testgradetree.php index bcbe23148c..44be8e6177 100644 --- a/lib/simpletest/grade/simpletest/testgradetree.php +++ b/lib/simpletest/grade/simpletest/testgradetree.php @@ -35,7 +35,6 @@ global $CFG; require_once($CFG->libdir . '/simpletest/testgradelib.php'); class grade_tree_test extends gradelib_test { - /* function test_grade_tree_locate_element() { $tree = new grade_tree($this->courseid); @@ -86,8 +85,11 @@ class grade_tree_test extends gradelib_test { $this->assertEqual($this->grade_items[2]->itemname, $tree->tree_array[1]['children'][2]['children'][1]['object']->itemname); $this->assertFalse(empty($tree->tree_array[1]['children'][2]['children'][1]['final_grades'][1])); $this->assertEqual($this->grade_grades_final[6]->gradevalue, $tree->tree_array[1]['children'][2]['children'][1]['final_grades'][1]->gradevalue); + + // Check the need_insert array + $this->assertEqual(1, count($tree->need_insert)); } -*/ + function test_grade_tree_move_element() { $tree = new grade_tree($this->courseid); @@ -157,17 +159,61 @@ class grade_tree_test extends gradelib_test { function test_grade_tree_renumber() { $tree = new grade_tree($this->courseid); + $tree1 = $tree; $tree->renumber(); - + $this->assertEqual($tree1->tree_array[1]['object'], $tree->tree_array[1]['object']); } function test_grade_tree_remove_element() { $tree = new grade_tree($this->courseid); + // Removing the orphan grade_item + $tree->remove_element(7); + $this->assertTrue(empty($tree->tree_array[7])); + $this->assertFalse(empty($tree->tree_array[1])); + $this->assertFalse(empty($tree->tree_array[8])); + $tree->renumber(); + $this->assertFalse(empty($tree->tree_array[7])); + $this->assertFalse(empty($tree->tree_array[1])); + $this->assertTrue(empty($tree->tree_array[8])); + + // Removing a grade_item with only 1 parent + $tree->remove_element(8); + $this->assertTrue(empty($tree->tree_array[7]['children'][8])); + $this->assertFalse(empty($tree->tree_array[7]['children'][9])); + $tree->renumber(); + $this->assertFalse(empty($tree->tree_array[7]['children'][8])); + $this->assertTrue(empty($tree->tree_array[7]['children'][9])); + + // Now remove this sub-category (the one without a topcat) + $tree->remove_element(7); + $this->assertTrue(empty($tree->tree_array[7])); + + // At this point we're left with a topcat, 2 subcats and 3 items, so try removing an item first + $tree->remove_element(4); + $this->assertTrue(empty($tree->tree_array[1]['children'][2]['children'][4])); + $this->assertFalse(empty($tree->tree_array[1]['children'][5])); + $tree->renumber(); + $this->assertFalse(empty($tree->tree_array[1]['children'][4])); + + // Now remove a subcat sandwiched between a topcat and its items + $tree->remove_element(4); + $this->assertTrue(empty($tree->tree_array[1]['children'][4])); + $tree->renumber(); + $this->assertTrue(empty($tree->tree_array[1]['children'][4])); + + $this->assertEqual(12, count($tree->tree_array, COUNT_RECURSIVE)); + + // Check the need_delete array + $this->assertEqual(5, count($tree->need_delete)); } function test_grade_tree_get_filler() { $tree = new grade_tree($this->courseid); } + + function test_grade_tree_build_tree_filled() { + + } }