From a39cac258db82194386eb3d54cf2dff1435033ea Mon Sep 17 00:00:00 2001 From: nicolasconnault Date: Thu, 17 May 2007 09:04:52 +0000 Subject: [PATCH] MDL-9506 Elements of the array returned by grade_category::get_children are now indexed by sortorder, to work more easily with grade_tree methods. New get_sortorder method for grade_category and grade_item. This is used when the type of an object is unknown, and could be either one. Because categories have a "virtual" sortorder (through their grade_item), they can transparently return one as well. Unit tests fail for grade_tree at present because of faulty implementation which is being fixed. The $depth attribute has been completely removed, as well as a number of switch statements, in favour of an $index variable which is exploded and used to build strings of array keys for eval statements (unset and array_splice). Can't wait to get all this working :-) --- lib/grade/grade_calculation.php | 1 + lib/grade/grade_category.php | 46 ++++-- lib/grade/grade_grades_final.php | 1 + lib/grade/grade_grades_raw.php | 2 + lib/grade/grade_grades_text.php | 1 + lib/grade/grade_history.php | 1 + lib/grade/grade_item.php | 26 +++- lib/grade/grade_object.php | 1 + lib/grade/grade_outcome.php | 1 + lib/grade/grade_scale.php | 1 + lib/grade/grade_tree.php | 146 +++++++++++------- .../grade/simpletest/testgradetree.php | 98 ++++++------ 12 files changed, 209 insertions(+), 116 deletions(-) diff --git a/lib/grade/grade_calculation.php b/lib/grade/grade_calculation.php index 1508dc89a21..d80d3f7b9b0 100644 --- a/lib/grade/grade_calculation.php +++ b/lib/grade/grade_calculation.php @@ -102,6 +102,7 @@ class grade_calculation extends grade_object { return $grade_calculation; } } else { + debugging("No grade_calculation matching your criteria in the database."); return false; } } diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 41911983bd2..5a8ee86db12 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -183,6 +183,7 @@ class grade_category extends grade_object { return $grade_category; } } else { + debugging("No grade_category matching your criteria in the database."); return false; } } @@ -246,6 +247,7 @@ class grade_category extends grade_object { $grade_item->itemtype = 'category'; if (!$grade_item->insert()) { + debugging("Could not insert this grade_item in the database: " . print_r($grade_item, true)); return false; } @@ -257,6 +259,7 @@ class grade_category extends grade_object { $this->load_parent_category(); if (!empty($this->parent_category)) { if (!$this->parent_category->flag_for_update()) { + debugging("Could not notify parent category of the need to update its final grades."); return false; } } @@ -343,6 +346,7 @@ class grade_category extends grade_object { $children = $this->get_children(1, 'flat'); if (empty($children)) { + debugging("Could not generate grades for this category, it has no children."); return false; } @@ -395,6 +399,7 @@ class grade_category extends grade_object { */ function aggregate_grades($final_grade_sets) { if (empty($final_grade_sets)) { + debugging("Could not aggregate grades: no array of grades given to aggregate."); return null; } @@ -445,6 +450,7 @@ class grade_category extends grade_object { // If the gradevalue is null, we have a problem if (empty($aggregated_value)) { + debugging("There was an error during the aggregation procedure, an empty value resulted."); return false; } @@ -480,7 +486,7 @@ class grade_category extends grade_object { /** * Fetches and returns all the children categories and/or grade_items belonging to this category. * By default only returns the immediate children (depth=1), but deeper levels can be requested, - * as well as all levels (0). + * as well as all levels (0). The elements are indexed by sort order. * @param int $depth 1 for immediate children, 0 for all children, and 2+ for specific levels deeper than 1. * @param string $arraytype Either 'nested' or 'flat'. A nested array represents the true hierarchy, but is more difficult to work with. * @return array Array of child objects (grade_category and grade_item). @@ -513,19 +519,19 @@ class grade_category extends grade_object { if ($cat->has_children()) { if ($arraytype == 'nested') { - $children_array[] = array('object' => $cat, 'children' => $cat->get_children($newdepth, $arraytype)); + $children_array[$cat->get_sortorder()] = array('object' => $cat, 'children' => $cat->get_children($newdepth, $arraytype)); } else { - $children_array[] = $cat; + $children_array[$cat->get_sortorder()] = $cat; $cat_children = $cat->get_children($newdepth, $arraytype); foreach ($cat_children as $id => $cat_child) { - $children_array[] = new grade_category($cat_child, false); + $children_array[$cat_child->get_sortorder()] = new grade_category($cat_child, false); } } } else { if ($arraytype == 'nested') { - $children_array[] = array('object' => $cat); + $children_array[$cat->get_sortorder()] = array('object' => $cat); } else { - $children_array[] = $cat; + $children_array[$cat->get_sortorder()] = $cat; } } } @@ -536,7 +542,24 @@ class grade_category extends grade_object { return $children_array; } - + + /** + * Returns the sortorder of the associated grade_item. This method is also available in + * grade_item, for cases where the object type is not know. It will act as a virtual + * variable for a grade_category. + * @return int Sort order + */ + function get_sortorder() { + if (empty($this->sortorder)) { + $this->load_grade_item(); + if (!empty($this->grade_item)) { + return $this->grade_item->sortorder; + } + } else { + return $this->sortorder; + } + } + /** * Given an array of stdClass children of a certain $object_type, returns a flat or nested * array of these children, ready for appending to a tree built by get_children. @@ -550,10 +573,11 @@ class grade_category extends grade_object { $children_array = array(); foreach ($children as $id => $child) { + $child = new $object_type($child, false); if ($arraytype == 'nested') { - $children_array[] = array('object' => new $object_type($child, false)); + $children_array[$child->get_sortorder()] = array('object' => $child); } else { - $children_array[] = new $object_type($child); + $children_array[$child->get_sortorder()] = $child; } } @@ -645,8 +669,6 @@ class grade_category extends grade_object { $this->parent_category = new grade_category(array('id' => $this->parent)); } return $this->parent_category; - } - - + } } ?> diff --git a/lib/grade/grade_grades_final.php b/lib/grade/grade_grades_final.php index 18221ce1e42..6fb1fb6c664 100644 --- a/lib/grade/grade_grades_final.php +++ b/lib/grade/grade_grades_final.php @@ -147,6 +147,7 @@ class grade_grades_final extends grade_object { return $object; } } else { + debugging("No grade_grades_final matching your criteria in the database."); return false; } } diff --git a/lib/grade/grade_grades_raw.php b/lib/grade/grade_grades_raw.php index 0e4e298b243..9a1da085452 100644 --- a/lib/grade/grade_grades_raw.php +++ b/lib/grade/grade_grades_raw.php @@ -173,6 +173,7 @@ class grade_grades_raw extends grade_object { return $object; } } else { + debugging("No grade_grades_raw matching your criteria in the database."); return false; } } @@ -251,6 +252,7 @@ class grade_grades_raw extends grade_object { return $result; } else { + debugging("Could not update a raw grade in the database."); return false; } } diff --git a/lib/grade/grade_grades_text.php b/lib/grade/grade_grades_text.php index 0f215264e19..a312a604a38 100644 --- a/lib/grade/grade_grades_text.php +++ b/lib/grade/grade_grades_text.php @@ -115,6 +115,7 @@ class grade_grades_text extends grade_object { return $grade_text; } } else { + debugging("No grade_grades_text matching your criteria in the database."); return false; } } diff --git a/lib/grade/grade_history.php b/lib/grade/grade_history.php index 56fb5f669be..08c65778cb2 100644 --- a/lib/grade/grade_history.php +++ b/lib/grade/grade_history.php @@ -102,6 +102,7 @@ class grade_history extends grade_object { return $grade_history; } } else { + debugging("No grade_history matching your criteria in the database."); return false; } } diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index f41f7e4dc3d..e15fbe18fd7 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -271,6 +271,7 @@ class grade_item extends grade_object { } if (empty($grade_final_array)) { + debugging("No final grades recorded for this grade_item"); return false; } @@ -414,7 +415,8 @@ class grade_item extends grade_object { $grade_item = new grade_item($grade_item); return $grade_item; } - } else { + } else { + debugging("No grade_item matching these criteria in the database."); return false; } } @@ -446,9 +448,12 @@ class grade_item extends grade_object { $category = $this->get_category(); if (!empty($category)) { if (!$category->flag_for_update()) { + debugging("Could not notify parent category of the need to update its final grades."); return false; } } + } else { + debugging("Could not insert this grade_item in the database!"); } return $result; @@ -498,6 +503,7 @@ class grade_item extends grade_object { $this->grade_grades_raw[$userid] = $raw_grade; } } else { + debugging("The data given to grade_item::save_raw($data) was not valid, it must be an arra of raw grades."); return false; } } @@ -582,6 +588,7 @@ class grade_item extends grade_object { if (empty($calculation)) { // We are setting this item object's calculation variable from the DB $grade_calculation = $this->get_calculation(true); if (empty($grade_calculation)) { + debugging("No calculation to set for this grade_item."); return false; } else { $this->calculation = $grade_calculation; @@ -598,6 +605,7 @@ class grade_item extends grade_object { $this->calculation = $grade_calculation; return true; } else { + debugging("Could not save the calculation in the database, for this grade_item."); return false; } } else { // Updating @@ -640,6 +648,7 @@ class grade_item extends grade_object { } if (!$this->update()) { + debugging("Could not update this grade_item's locked state in the database."); return false; } @@ -650,6 +659,7 @@ class grade_item extends grade_object { foreach ($this->grade_grades_final as $id => $final) { $final->locked = $this->locked; if (!$final->update()) { + debugging("Could not update this grade_item's final grade's locked state in the database."); return false; } $count++; @@ -674,6 +684,7 @@ class grade_item extends grade_object { } if (!$this->update()) { + debugging("Could not update this grade_item's hidden state in the database."); return false; } @@ -684,6 +695,7 @@ class grade_item extends grade_object { foreach ($this->grade_grades_final as $id => $final) { $final->hidden = $this->hidden; if (!$final->update()) { + debugging("Could not update this grade_item's final grade's hidden state in the database."); return false; } $count++; @@ -747,6 +759,7 @@ class grade_item extends grade_object { if ($final->update()) { $count++; } else { + debugging("Could not update a final grade in this grade_item."); return false; } } @@ -838,5 +851,16 @@ class grade_item extends grade_object { return $result; } + + /** + * Returns the sortorder of this grade_item. This method is also available in + * grade_category, for cases where the object type is not know. It will act as a virtual + * variable for a grade_category. + * @return int Sort order + */ + function get_sortorder() { + return $this->sortorder; + } + } ?> diff --git a/lib/grade/grade_object.php b/lib/grade/grade_object.php index f1a3e53c517..111d642db64 100644 --- a/lib/grade/grade_object.php +++ b/lib/grade/grade_object.php @@ -137,6 +137,7 @@ class grade_object { */ function update_from_db() { if (empty($this->id)) { + debugging("The object could not be used in its state to retrieve a matching record from the DB, because it's id field is not set."); return false; } else { $class = get_class($this); diff --git a/lib/grade/grade_outcome.php b/lib/grade/grade_outcome.php index 707351935f0..7d2a855151f 100644 --- a/lib/grade/grade_outcome.php +++ b/lib/grade/grade_outcome.php @@ -117,6 +117,7 @@ class grade_outcome extends grade_object { return $grade_outcome; } } else { + debugging("No matching grade_outcome in DB with the given criteria."); return false; } } diff --git a/lib/grade/grade_scale.php b/lib/grade/grade_scale.php index 38264bc492c..d65555fb250 100644 --- a/lib/grade/grade_scale.php +++ b/lib/grade/grade_scale.php @@ -96,6 +96,7 @@ class grade_scale extends grade_object { return $grade_scale; } } else { + debugging("No matching grade_scale in DB with the given criteria."); return false; } } diff --git a/lib/grade/grade_tree.php b/lib/grade/grade_tree.php index 774aef06bd0..3a9598203ba 100644 --- a/lib/grade/grade_tree.php +++ b/lib/grade/grade_tree.php @@ -83,44 +83,44 @@ class grade_tree { function locate_element($sortorder) { $topcatcount = 0; $retval = false; - - foreach ($this->tree_array as $topcatkey => $topcat) { - $topcatcount++; - $subcatcount = 0; + debugging($sortorder); + if (empty($this->tree_array)) { + debugging("grade_tree->tree_array was empty, I could not locate the element at sortorder $sortorder"); + return false; + } + + foreach ($this->tree_array as $key1 => $level1) { + $level1count++; + $level2count = 0; $retval = new stdClass(); - $retval->topcatindex = $topcatkey; - unset($retval->subcatindex); - unset($retval->itemindex); + $retval->index = $key1; - if ($topcatkey == $sortorder) { - $retval->depth = 1; - $retval->element = $topcat; - $retval->position = $topcatcount; + if ($key1 == $sortorder) { + $retval->element = $level1; + $retval->position = $level1count; return $retval; } - if (!empty($topcat['children'])) { - foreach ($topcat['children'] as $subcatkey => $subcat) { - $subcatcount++; - unset($retval->itemindex); - $itemcount = 0; + if (!empty($level1['children'])) { + foreach ($level1['children'] as $level2key => $level2) { + $level2count++; + $level3count = 0; - $retval->subcatindex = $subcatkey; - if ($subcatkey == $sortorder) { - $retval->depth = 2; - $retval->element = $subcat; - $retval->position = $subcatcount; + $retval->index .= "/$level2key"; + if ($level2key == $sortorder) { + $retval->element = $level2; + $retval->position = $level2count; return $retval; } - if (!empty($subcat['children'])) { - foreach ($subcat['children'] as $itemkey => $item) { - $itemcount++; - $retval->itemindex = $itemkey; - if ($itemkey == $sortorder) { - $retval->depth = 3; - $retval->element = $item; - $retval->position = $itemcount; + if (!empty($level2['children'])) { + foreach ($level2['children'] as $level3key => $level3) { + $level3count++; + $retval->index .= "/$level3key"; + + if ($level3key == $sortorder) { + $retval->element = $level3; + $retval->position = $level3count; return $retval; } } @@ -142,24 +142,28 @@ class grade_tree { $this->first_sortorder = key($this->tree_array); } - if (isset($element->depth)) { - switch ($element->depth) { - case 1: - unset($this->tree_array[$element->topcatindex]); - break; - case 2: - unset($this->tree_array[$element->topcatindex]['children'][$element->subcatindex]); - break; - case 3: - unset($this->tree_array[$element->topcatindex]['children'][$element->subcatindex]['children'][$element->itemindex]); - break; + if (isset($element->index)) { + // Decompose the element's index and build string for eval(unset) statement to follow + $indices = explode('/', $element->index); + $element_to_unset = '$this->tree_array[' . $indices[0] . ']'; + + if (isset($indices[1])) { + $element_to_unset .= "['children'][" . $indices[1] . ']'; } + + if (isset($indices[2])) { + $element_to_unset .= "['children'][" . $indices[2] . ']'; + } + + eval("unset($element_to_unset);"); + return true; } else { $element = $this->locate_element($element); if (!empty($element)) { return $this->remove_element($element); } else { + debugging("The element you provided grade_tree::remove_element() is not valid."); return false; } } @@ -183,7 +187,8 @@ class grade_tree { } elseif ($position == 'after') { $offset = 0; } else { - die ('move_element(..... $position) can only be "before" or "after", you gave ' . $position); + debugging('move_element(..... $position) can only be "before" or "after", you gave ' . $position); + return false; } // TODO Problem when moving topcategories: sortorder gets reindexed when splicing the array @@ -193,23 +198,19 @@ class grade_tree { $destination_element = $this->locate_element($destination_sortorder); $position = $destination_element->position; - switch($element->depth) { - case 1: - array_splice($this->tree_array, - $position + $offset, 0, - $destination_array); - break; - case 2: - array_splice($this->tree_array[$destination_element->topcatindex]['children'], - $position + $offset, 0, - $destination_array); - break; - case 3: - array_splice($this->tree_array[$destination_element->topcatindex]['children'][$destination_element->subcatindex]['children'], - $position + $offset, 0, - $destination_array); - break; + // Decompose the element's index and build string for eval(array_splice) statement to follow + $indices = explode('/', $element->index); + $element_to_splice = '$this->tree_array'; + + if (isset($indices[0])) { + $element_to_splice .= '[' . $indices[0] . "]['children']"; } + + if (isset($indices[1])) { + $element_to_splice .= '[' . $indices[1] . "]['children']"; + } + + eval("array_splice($element_to_splice, \$position + \$offset, 0, \$destination_array);"); return true; } @@ -235,8 +236,8 @@ class grade_tree { $destination = $this->locate_element($destination_sortorder); - if ($destination->depth != $source->depth) { - echo "Source and Destination were at different levels."; + if (substr_count($destination->index, '/') != substr_count($source->index, '/')) { + debugging("Source and Destination were at different levels."); return false; } @@ -272,16 +273,27 @@ class grade_tree { foreach ($subcat['children'] as $item) { $sortorder++; $newtree[$topcatsortorder]['children'][$subcatsortorder]['children'][$sortorder] = $item; + if ($sortorder != $item['object']->sortorder) { + $this->need_update[$item['object']->sortorder] = $sortorder; + } } $newtree[$topcatsortorder]['children'][$subcatsortorder]['object'] = $subcat['object']; } else { $newtree[$topcatsortorder]['children'][$sortorder] = $subcat; } + + if ($sortorder != $subcat['object']->sortorder) { + $this->need_update[$subcat['object']->sortorder] = $sortorder; + } } $newtree[$topcatsortorder]['object'] = $topcat['object']; } else { $newtree[$sortorder] = $topcat; } + + if ($sortorder != $topcat['object']->sortorder) { + $this->need_update[$topcat['object']->sortorder] = $sortorder; + } } $this->tree_array = $newtree; unset($this->first_sortorder); @@ -354,6 +366,11 @@ class grade_tree { unset($fillers[$sortorder]); $this->tree_filled[$sortorder] = $this->get_filler($object, $fullobjects); + if (get_class($object) == 'grade_category') { + $object->get_children(); + } + $object->sortorder = $sortorder; + $tree[$sortorder] = $object; } $query = "SELECT $category_table.*, sortorder FROM $category_table, $items_table @@ -415,6 +432,11 @@ class grade_tree { if (!empty($fillers)) { foreach ($fillers as $sortorder => $object) { $this->tree_filled[$sortorder] = $this->get_filler($object, $fullobjects); + if (get_class($object) == 'grade_category') { + $object->get_children(); + } + $object->sortorder = $sortorder; + $tree[$sortorder] = $object; } } @@ -479,6 +501,12 @@ class grade_tree { function display_grades() { // 1. Fetch all top-level categories for this course, with all children preloaded, sorted by sortorder $tree = $this->tree_filled; + + if (empty($this->tree_filled)) { + debugging("The tree_filled array wasn't initialised, grade_tree could not display the grades correctly."); + return false; + } + $topcathtml = ''; $cathtml = ''; $itemhtml = ''; diff --git a/lib/simpletest/grade/simpletest/testgradetree.php b/lib/simpletest/grade/simpletest/testgradetree.php index 78cc5208d5b..f657c14f301 100644 --- a/lib/simpletest/grade/simpletest/testgradetree.php +++ b/lib/simpletest/grade/simpletest/testgradetree.php @@ -36,48 +36,12 @@ require_once($CFG->libdir . '/simpletest/testgradelib.php'); class grade_tree_test extends gradelib_test { - function test_grade_tree_constructor() { - $tree = new grade_tree($this->courseid); - - } - - function test_grade_tree_display_grades() { - $tree = new grade_tree($this->courseid); - $result_html = $tree->display_grades(); - - $expected_html = '
unittestcategory1  
unittestcategory2unittestcategory3 level1category
unittestgradeitem1unittestgradeitem2unittestgradeitem3unittestorphangradeitem1singleparentitem1singleparentitem2
'; - $this->assertEqual($expected_html, $result_html); - } - - function test_grade_tree_get_tree() { - $tree = new grade_tree($this->courseid); - $this->assertEqual(58, count($tree->tree_filled, COUNT_RECURSIVE)); - $this->assertEqual(27, count($tree->tree_array, COUNT_RECURSIVE)); - } - - function test_grade_tree_locate_element() { - $tree = new grade_tree($this->courseid); - $element = $tree->locate_element(5); - $this->assertEqual(1, $element->topcatindex); - $this->assertEqual(5, $element->subcatindex); - $this->assertTrue(empty($element->itemindex)); - $this->assertEqual(2, $element->depth); - $this->assertNotNull($element->element); - $this->assertEqual('unittestcategory3', $element->element['object']->fullname); - $this->assertEqual('unittestgradeitem3', $element->element['children'][6]['object']->itemname); - } - - function test_grade_tree_renumber() { - $tree = new grade_tree($this->courseid); - $tree->renumber(); - - } - function test_grade_tree_move_element() { $tree = new grade_tree($this->courseid); - $tree->move_element(4, 6); - $this->assertFalse(empty($tree->tree_array[1]['children'][5]['children'][0])); - $this->assertEqual('unittestgradeitem2', $tree->tree_array[1]['children'][5]['children'][0]['object']->itemname); + + $tree->move_element(4, 9); + $this->assertFalse(empty($tree->tree_array[8]['children'][1])); + $this->assertEqual('unittestgradeitem2', $tree->tree_array[8]['children'][1]['object']->itemname); $tree->renumber(); $this->assertFalse(empty($tree->tree_array[1]['children'][4]['children'][5])); $this->assertEqual('unittestgradeitem2', $tree->tree_array[1]['children'][4]['children'][5]['object']->itemname); @@ -97,8 +61,55 @@ class grade_tree_test extends gradelib_test { $this->assertFalse(empty($tree->tree_array[1]['children'][4])); $this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][4]['object']->fullname); - // Try moving elements from different levels - $this->assertFalse($tree->move_element(1, 2)); + // Try moving a subcategory + $tree = new grade_tree($this->courseid); + $original_count = count($tree->tree_array, COUNT_RECURSIVE); + $tree->move_element(8, 5); + $new_count = count($tree->tree_array, COUNT_RECURSIVE); + $this->assertEqual($original_count, $new_count); + $this->assertFalse(empty($tree->tree_array[1]['children'][1])); + $this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][1]['object']->fullname); + $tree->renumber(); + $this->assertFalse(empty($tree->tree_array[1]['children'][5])); + $this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][5]['object']->fullname); + $this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][5]['children'][6]->itemname); + } + + + function test_grade_tree_constructor() { + $tree = new grade_tree($this->courseid); + + } + + function test_grade_tree_display_grades() { + $tree = new grade_tree($this->courseid); + $result_html = $tree->display_grades(); + + $expected_html = '
unittestcategory1  
unittestcategory2unittestcategory3 level1category
unittestgradeitem1unittestgradeitem2unittestgradeitem3unittestorphangradeitem1singleparentitem1singleparentitem2
'; + $this->assertEqual($expected_html, $result_html); + } + + function test_grade_tree_get_tree() { + $tree = new grade_tree($this->courseid); + $this->assertEqual(58, count($tree->tree_filled, COUNT_RECURSIVE)); + $this->assertEqual(29, count($tree->tree_array, COUNT_RECURSIVE)); + } + + function test_grade_tree_locate_element() { + $tree = new grade_tree($this->courseid); + $element = $tree->locate_element(5); + $this->assertEqual(1, $element->topcatindex); + $this->assertEqual(5, $element->subcatindex); + $this->assertTrue(empty($element->itemindex)); + $this->assertNotNull($element->element); + $this->assertEqual('unittestcategory3', $element->element['object']->fullname); + $this->assertEqual('unittestgradeitem3', $element->element['children'][6]['object']->itemname); + } + + function test_grade_tree_renumber() { + $tree = new grade_tree($this->courseid); + $tree->renumber(); + } function test_grade_tree_insert_element() { @@ -114,6 +125,5 @@ class grade_tree_test extends gradelib_test { function test_grade_tree_get_filler() { $tree = new grade_tree($this->courseid); - } - + } }