mirror of
https://github.com/moodle/moodle.git
synced 2025-04-22 17:02:03 +02:00
MDL-9506 Added ksort() of each level of categories and items for the gradebook. Unsorted arrays caused erratic behaviour when moving elements of the tree around.
This commit is contained in:
nicolasconnault
parent
bb15b33bf9
commit
c3096060de
@ -104,6 +104,7 @@ class grade_tree {
|
||||
|
||||
if (!empty($this->tree_array)) {
|
||||
$this->first_sortorder = key($this->tree_array);
|
||||
$this->renumber();
|
||||
}
|
||||
}
|
||||
|
||||
@ -389,7 +390,7 @@ class grade_tree {
|
||||
$sortorder = $starting_sortorder;
|
||||
|
||||
if (empty($elements) && empty($starting_sortorder)) {
|
||||
if (empty($this->first_sortorder)) {
|
||||
if (!isset($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;
|
||||
}
|
||||
@ -405,6 +406,10 @@ class grade_tree {
|
||||
foreach ($elements as $key => $element) {
|
||||
$this->first_sortorder++;
|
||||
$new_sortorder = $this->first_sortorder;
|
||||
$old_sortorder = $element['object']->sortorder;
|
||||
|
||||
// Assign new sortorder
|
||||
$element['object']->sortorder = $new_sortorder;
|
||||
|
||||
$element['object']->previous_sortorder = $this->get_neighbour_sortorder($element, 'previous');
|
||||
$element['object']->next_sortorder = $this->get_neighbour_sortorder($element, 'next');
|
||||
@ -416,10 +421,11 @@ class grade_tree {
|
||||
$newtree[$this->first_sortorder] = $element;
|
||||
}
|
||||
|
||||
if ($new_sortorder != $element['object']->sortorder) {
|
||||
if ($new_sortorder != $old_sortorder) {
|
||||
$this->need_update[$element['object']->get_item_id()] =
|
||||
array('old_sortorder' => $element['object']->sortorder,
|
||||
'new_sortorder' => $new_sortorder);
|
||||
array('old_sortorder' => $old_sortorder,
|
||||
'new_sortorder' => $new_sortorder,
|
||||
'name' => $element['object']->get_name());
|
||||
}
|
||||
}
|
||||
|
||||
@ -558,7 +564,8 @@ class grade_tree {
|
||||
|
||||
$itemtree[$element['object']->sortorder] = array('object' => $element['object'], 'finalgrades' => $finals);
|
||||
}
|
||||
|
||||
|
||||
ksort($itemtree);
|
||||
$element['children'] = $itemtree;
|
||||
} elseif (get_class($filler_object) == 'grade_item' && $this->include_grades) {
|
||||
$final_grades = $filler_object->get_final();
|
||||
@ -605,19 +612,19 @@ class grade_tree {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $objects;
|
||||
}
|
||||
|
||||
/**
|
||||
* Static method that returns a sorted, nested array of all grade_categories and grade_items for
|
||||
* a given course, or for the entire site if no courseid is given.
|
||||
* a given course, or for the entire site if no courseid is given. This method is not recursive
|
||||
* by design, because we want to limit the layers to 3, and because we want to avoid accessing
|
||||
* the DB with recursive methods.
|
||||
* @return array
|
||||
*/
|
||||
function get_tree() {
|
||||
global $CFG;
|
||||
$tree = array();
|
||||
$fillers = array();
|
||||
|
||||
$category_table = $CFG->prefix . 'grade_categories';
|
||||
$items_table = $CFG->prefix . 'grade_items';
|
||||
@ -653,7 +660,7 @@ class grade_tree {
|
||||
$topcats[0]->sortorder = 0;
|
||||
$topcats[0]->courseid = $this->courseid;
|
||||
}
|
||||
|
||||
|
||||
// If any of these categories has grade_items as children, create a topcategory filler with colspan=count(children)
|
||||
$topcats = $this->add_fillers($topcats);
|
||||
|
||||
@ -695,27 +702,30 @@ class grade_tree {
|
||||
$itemtree[$item->sortorder] = array('object' => $item, 'finalgrades' => $finals);
|
||||
}
|
||||
|
||||
ksort($itemtree);
|
||||
$sortorder = $subcat->sortorder;
|
||||
$subcat = new grade_category($subcat, false);
|
||||
$subcat->sortorder = $sortorder;
|
||||
$subcattree[$subcat->sortorder] = array('object' => $subcat, 'children' => $itemtree);
|
||||
}
|
||||
|
||||
|
||||
ksort($subcattree);
|
||||
$sortorder = $topcat->sortorder;
|
||||
$topcat = new grade_category($topcat, false);
|
||||
$topcat->sortorder = $sortorder;
|
||||
|
||||
$tree[$topcat->sortorder] = array('object' => $topcat, 'children' => $subcattree);
|
||||
}
|
||||
|
||||
// If there are still grade_items or grade_categories without a top category, add another filler
|
||||
if (!empty($this->fillers)) {
|
||||
ksort($this->fillers);
|
||||
foreach ($this->fillers as $sortorder => $object) {
|
||||
$tree = $this->include_fillers($tree);
|
||||
}
|
||||
}
|
||||
|
||||
$db->debug = false;
|
||||
ksort($tree);
|
||||
return $tree;
|
||||
}
|
||||
|
||||
@ -896,6 +906,8 @@ class grade_tree {
|
||||
}
|
||||
|
||||
$this->need_update = array();
|
||||
$this->reset_first_sortorder();
|
||||
$this->renumber();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -35,9 +35,151 @@ global $CFG;
|
||||
require_once($CFG->libdir . '/simpletest/testgradelib.php');
|
||||
|
||||
class grade_tree_test extends gradelib_test {
|
||||
|
||||
function test_grade_tree_move_element() {
|
||||
/* 0.
|
||||
* Starting layout:
|
||||
*__________________
|
||||
*|_________1_______| ____________
|
||||
*|_____2_____|__5__|_____|_____8____|
|
||||
*|__3__|__4__|__6__|__7__|__9__|_10_|
|
||||
*/
|
||||
$tree = new grade_tree($this->courseid);
|
||||
/* 1.
|
||||
* Desired result:
|
||||
*_____________
|
||||
*|_____1_____| _________________
|
||||
*|__2__|__4__|_____|________7_______|
|
||||
*|__3__|__5__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(4, 10);
|
||||
$tree->renumber();
|
||||
|
||||
// Check need_? fields
|
||||
$this->assertFalse(empty($tree->need_update));
|
||||
$this->assertFalse(empty($tree->need_insert));
|
||||
$this->assertFalse(empty($tree->need_delete));
|
||||
$this->assertEqual(6, count($tree->need_update));
|
||||
$this->assertEqual(1, count($tree->need_delete));
|
||||
$this->assertEqual(1, count($tree->need_insert));
|
||||
|
||||
// Check sortorders
|
||||
$this->assertEqual(1, $tree->tree_array[1]['object']->sortorder);
|
||||
$this->assertEqual(2, $tree->tree_array[1]['children'][2]['object']->sortorder);
|
||||
$this->assertEqual(3, $tree->tree_array[1]['children'][2]['children'][3]['object']->sortorder);
|
||||
$this->assertEqual(4, $tree->tree_array[1]['children'][4]['object']->sortorder);
|
||||
$this->assertEqual(5, $tree->tree_array[1]['children'][4]['children'][5]['object']->sortorder);
|
||||
$this->assertEqual(6, $tree->tree_array[6]['object']->sortorder);
|
||||
$this->assertEqual(7, $tree->tree_array[7]['object']->sortorder);
|
||||
$this->assertEqual(8, $tree->tree_array[7]['children'][8]['object']->sortorder);
|
||||
$this->assertEqual(9, $tree->tree_array[7]['children'][9]['object']->sortorder);
|
||||
$this->assertEqual(10, $tree->tree_array[7]['children'][10]['object']->sortorder);
|
||||
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][4]['children'][5]));
|
||||
$this->assertEqual('unittestgradeitem3', $tree->tree_array[1]['children'][4]['children'][5]['object']->itemname);
|
||||
$tree->need_update = array();
|
||||
|
||||
/* 2.
|
||||
* Desired result:
|
||||
*___________________
|
||||
*|________1________|_________________
|
||||
*|_____2_____|__5__|________7_______|
|
||||
*|__3__|__4__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(6, 3, 'after');
|
||||
$tree->renumber();
|
||||
$this->assertEqual(3, count($tree->need_update));
|
||||
$tree->need_update = array();
|
||||
|
||||
// Check sortorders
|
||||
$this->assertEqual(1, $tree->tree_array[1]['object']->sortorder);
|
||||
$this->assertEqual(2, $tree->tree_array[1]['children'][2]['object']->sortorder);
|
||||
$this->assertEqual(3, $tree->tree_array[1]['children'][2]['children'][3]['object']->sortorder);
|
||||
$this->assertEqual(4, $tree->tree_array[1]['children'][2]['children'][4]['object']->sortorder);
|
||||
$this->assertEqual(5, $tree->tree_array[1]['children'][5]['object']->sortorder);
|
||||
$this->assertEqual(6, $tree->tree_array[1]['children'][5]['children'][6]['object']->sortorder);
|
||||
$this->assertEqual(7, $tree->tree_array[7]['object']->sortorder);
|
||||
$this->assertEqual(8, $tree->tree_array[7]['children'][8]['object']->sortorder);
|
||||
$this->assertEqual(9, $tree->tree_array[7]['children'][9]['object']->sortorder);
|
||||
$this->assertEqual(10, $tree->tree_array[7]['children'][10]['object']->sortorder);
|
||||
|
||||
// Try moving a subcategory
|
||||
/* 3.
|
||||
* Desired result:
|
||||
*___________________
|
||||
*|________1________|_________________
|
||||
*|__2__|_____4_____|________7_______|
|
||||
*|__3__|__5__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(2, 5, 'after');
|
||||
$tree->renumber();
|
||||
$this->assertEqual(5, count($tree->need_update));
|
||||
$tree->need_update = array();
|
||||
|
||||
// Check sortorders
|
||||
$this->assertEqual(1, $tree->tree_array[1]['object']->sortorder);
|
||||
$this->assertEqual(2, $tree->tree_array[1]['children'][2]['object']->sortorder);
|
||||
$this->assertEqual(3, $tree->tree_array[1]['children'][2]['children'][3]['object']->sortorder);
|
||||
$this->assertEqual(4, $tree->tree_array[1]['children'][4]['object']->sortorder);
|
||||
$this->assertEqual(5, $tree->tree_array[1]['children'][4]['children'][5]['object']->sortorder);
|
||||
$this->assertEqual(6, $tree->tree_array[1]['children'][4]['children'][6]['object']->sortorder);
|
||||
$this->assertEqual(7, $tree->tree_array[7]['object']->sortorder);
|
||||
$this->assertEqual(8, $tree->tree_array[7]['children'][8]['object']->sortorder);
|
||||
$this->assertEqual(9, $tree->tree_array[7]['children'][9]['object']->sortorder);
|
||||
$this->assertEqual(10, $tree->tree_array[7]['children'][10]['object']->sortorder);
|
||||
|
||||
/* 4.
|
||||
* Desired result:
|
||||
*_________________________
|
||||
*|___________1___________|____________
|
||||
*|__2__|________4________|_____8_____|
|
||||
*|__3__|__5__|__6__|__7__|__9__|_10__|
|
||||
*/
|
||||
$tree->move_element(8, 6);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(3, count($tree->need_update));
|
||||
$tree->need_update = array();
|
||||
|
||||
// Check sortorders
|
||||
$this->assertEqual(1, $tree->tree_array[1]['object']->sortorder);
|
||||
$this->assertEqual(2, $tree->tree_array[1]['children'][2]['object']->sortorder);
|
||||
$this->assertEqual(3, $tree->tree_array[1]['children'][2]['children'][3]['object']->sortorder);
|
||||
$this->assertEqual(4, $tree->tree_array[1]['children'][4]['object']->sortorder);
|
||||
$this->assertEqual(5, $tree->tree_array[1]['children'][4]['children'][5]['object']->sortorder);
|
||||
$this->assertEqual(6, $tree->tree_array[1]['children'][4]['children'][6]['object']->sortorder);
|
||||
$this->assertEqual(7, $tree->tree_array[1]['children'][4]['children'][7]['object']->sortorder);
|
||||
$this->assertEqual(8, $tree->tree_array[8]['object']->sortorder);
|
||||
$this->assertEqual(9, $tree->tree_array[8]['children'][9]['object']->sortorder);
|
||||
$this->assertEqual(10, $tree->tree_array[8]['children'][10]['object']->sortorder);
|
||||
|
||||
// Try moving a top category
|
||||
/* 5.
|
||||
* Desired result:
|
||||
* ___________________
|
||||
* |_________2_______|___________
|
||||
*______|_____3_____|__6__|_____8____|
|
||||
*|__1__|__4__|__5__|__7__|__9__|_10_|
|
||||
*/
|
||||
$tree = new grade_tree($this->courseid);
|
||||
$tree->move_element(1, 8);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(7, count($tree->need_update));
|
||||
|
||||
// Check sortorders
|
||||
$this->assertEqual(1, $tree->tree_array[1]['object']->sortorder);
|
||||
$this->assertEqual(2, $tree->tree_array[2]['object']->sortorder);
|
||||
$this->assertEqual(3, $tree->tree_array[2]['children'][3]['object']->sortorder);
|
||||
$this->assertEqual(4, $tree->tree_array[2]['children'][3]['children'][4]['object']->sortorder);
|
||||
$this->assertEqual(5, $tree->tree_array[2]['children'][3]['children'][5]['object']->sortorder);
|
||||
$this->assertEqual(6, $tree->tree_array[2]['children'][6]['object']->sortorder);
|
||||
$this->assertEqual(7, $tree->tree_array[2]['children'][6]['children'][7]['object']->sortorder);
|
||||
$this->assertEqual(8, $tree->tree_array[8]['object']->sortorder);
|
||||
$this->assertEqual(9, $tree->tree_array[8]['children'][9]['object']->sortorder);
|
||||
$this->assertEqual(10, $tree->tree_array[8]['children'][10]['object']->sortorder);
|
||||
}
|
||||
|
||||
function test_grade_tree_get_neighbour_sortorder() {
|
||||
$tree = new grade_tree($this->courseid);
|
||||
$tree->renumber();
|
||||
|
||||
$element = $tree->locate_element(4);
|
||||
$this->assertEqual(3, $tree->get_neighbour_sortorder($element, 'previous'));
|
||||
@ -64,8 +206,6 @@ class grade_tree_test extends gradelib_test {
|
||||
// TODO write more thorough and useful tests here. The renumber method assigns previous_sortorder and next_sortorder variables
|
||||
function test_grade_tree_renumber() {
|
||||
$tree = new grade_tree($this->courseid);
|
||||
$this->assertTrue(empty($tree->tree_array[1]['object']->next_sortorder));
|
||||
$tree->renumber();
|
||||
$this->assertFalse(empty($tree->tree_array[1]['object']->next_sortorder));
|
||||
|
||||
$this->assertTrue(empty($tree->need_update));
|
||||
@ -125,113 +265,6 @@ class grade_tree_test extends gradelib_test {
|
||||
$this->assertEqual(1, count($tree->need_insert));
|
||||
}
|
||||
|
||||
function test_grade_tree_move_element() {
|
||||
/* 0.
|
||||
* Starting layout:
|
||||
*__________________
|
||||
*|_________1_______| ____________
|
||||
*|__2__|_____4_____|_____|_____8____|
|
||||
*|__3__|__5__|__6__|__7__|__9__|_10_|
|
||||
*/
|
||||
$tree = new grade_tree($this->courseid);
|
||||
/* 1.
|
||||
* Desired result:
|
||||
*_____________
|
||||
*|_____1_____| _________________
|
||||
*|__2__|__4__|_____|________7_______|
|
||||
*|__3__|__5__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(4, 10);
|
||||
$this->assertFalse(empty($tree->tree_array[8]['children'][1]));
|
||||
$this->assertEqual('unittestgradeitem2', $tree->tree_array[8]['children'][1]['object']->itemname);
|
||||
$tree->renumber();
|
||||
|
||||
// Check need_? fields
|
||||
$this->assertFalse(empty($tree->need_update));
|
||||
$this->assertFalse(empty($tree->need_insert));
|
||||
$this->assertFalse(empty($tree->need_delete));
|
||||
$this->assertEqual(6, count($tree->need_update));
|
||||
$this->assertEqual(1, count($tree->need_delete));
|
||||
$this->assertEqual(1, count($tree->need_insert));
|
||||
$this->assertEqual($this->grade_items[1]->itemname, $tree->need_delete[$this->grade_items[1]->id]->itemname);
|
||||
$this->assertEqual($this->grade_items[1]->itemname, $tree->need_insert[$this->grade_items[1]->id]->itemname);
|
||||
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][4]['children'][5]));
|
||||
$this->assertEqual('unittestgradeitem3', $tree->tree_array[1]['children'][4]['children'][5]['object']->itemname);
|
||||
$tree->need_update = array();
|
||||
|
||||
/* 2.
|
||||
* Desired result:
|
||||
*___________________
|
||||
*|________1________|_________________
|
||||
*|_____2_____|__5__|________7_______|
|
||||
*|__3__|__4__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(6, 3, 'after');
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][2]['children'][1]));
|
||||
$this->assertEqual('unittestorphangradeitem1', $tree->tree_array[1]['children'][2]['children'][1]['object']->itemname);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(4, count($tree->need_update));
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][2]['children'][4]));
|
||||
$this->assertEqual('unittestorphangradeitem1', $tree->tree_array[1]['children'][2]['children'][4]['object']->itemname);
|
||||
$tree->need_update = array();
|
||||
|
||||
// Try moving a subcategory
|
||||
/* 3.
|
||||
* Desired result:
|
||||
*___________________
|
||||
*|________1________|_________________
|
||||
*|__2__|_____4_____|________7_______|
|
||||
*|__3__|__5__|__6__|__8__|__9__|_10_|
|
||||
*/
|
||||
$tree->move_element(2, 5, 'after');
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][1]));
|
||||
$this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][1]['object']->fullname);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(5, count($tree->need_update));
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][4]));
|
||||
$this->assertEqual('unittestcategory2', $tree->tree_array[1]['children'][4]['object']->fullname);
|
||||
$tree->need_update = array();
|
||||
|
||||
/* 4.
|
||||
* Desired result:
|
||||
*_________________________
|
||||
*|___________1___________|____________
|
||||
*|__2__|________4________|_____8_____|
|
||||
*|__3__|__5__|__6__|__7__|__9__|_10__|
|
||||
*/
|
||||
$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('level1category', $tree->tree_array[1]['children'][1]['object']->fullname);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(4, count($tree->need_update));
|
||||
$this->assertFalse(empty($tree->tree_array[1]['children'][5]));
|
||||
$this->assertEqual('level1category', $tree->tree_array[1]['children'][5]['object']->fullname);
|
||||
$this->assertEqual('singleparentitem1', $tree->tree_array[1]['children'][5]['children'][6]['object']->itemname);
|
||||
$tree->need_update = array();
|
||||
|
||||
// Try moving a top category
|
||||
/* 5.
|
||||
* Desired result:
|
||||
* ___________________
|
||||
* |_________2_______|___________
|
||||
*______|__3__|_____5_____|_____8____|
|
||||
*|__1__|__4__|__6__|__7__|__9__|_10_|
|
||||
*/
|
||||
$tree = new grade_tree($this->courseid);
|
||||
$tree->move_element(1, 8);
|
||||
$this->assertFalse(empty($tree->tree_array[1]));
|
||||
$this->assertEqual('unittestcategory1', $tree->tree_array[1]['object']->fullname);
|
||||
$tree->renumber();
|
||||
$this->assertEqual(7, count($tree->need_update));
|
||||
$this->assertFalse(empty($tree->tree_array[2]));
|
||||
$this->assertEqual('unittestcategory1', $tree->tree_array[2]['object']->fullname);
|
||||
}
|
||||
|
||||
function test_grade_tree_constructor() {
|
||||
$tree = new grade_tree($this->courseid);
|
||||
|
||||
@ -343,5 +376,4 @@ class grade_tree_test extends gradelib_test {
|
||||
function test_grade_tree_display_edit_tree() {
|
||||
$tree = new grade_tree($this->courseid);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user