From ba87a4daf5f462691027ec3a901d0018b24d1248 Mon Sep 17 00:00:00 2001 From: mjollnir_ Date: Wed, 17 Nov 2004 06:57:28 +0000 Subject: [PATCH] Merged from MOODLE_14_STABLE: Fixes to fix_course_sortorder() and course/category.php pages. (martinlanghoff) arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-160 Minor fix: moodle would crash on high number of courses when doing course creation -- should be more scalable now arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-164 Fix a bug I have introduced in fix_course_sortorder() - v2 arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-210 Fixed nested transaction in fix_course_sortorder() arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-215 Performance and memory usage fixes for re-sort courses function arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-268 Major performance and correctness improvements in the functions that move courses up and down, reorder by name, and in fix_course_sortorder(). All now assume course-sortorder is unique (this is enforced at the DB) arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-332 fix_coursesortorder() bugfixes and logic simplification arch-eduforge@catalyst.net.nz--2004/moodle--eduforge--1.3.3--patch-333 courses listing enhancements: bugfix on re-ordering, keep the right page on actions --- course/category.php | 81 +++++++++++++++++++++------------------------ lib/datalib.php | 74 +++++++++++++++++++++++++++++++++-------- 2 files changed, 97 insertions(+), 58 deletions(-) diff --git a/course/category.php b/course/category.php index 70f93fa119..10c6eecb5d 100644 --- a/course/category.php +++ b/course/category.php @@ -57,13 +57,18 @@ /// Resort the category if requested if (!empty($_GET['resort']) and confirm_sesskey()) { - if ($courses = get_courses($category->id, "fullname ASC")) { - $count = 0; + if ($courses = get_courses($category->id, "fullname ASC", 'c.id,c.fullname,c.sortorder')) { + // move it off the range + $count = get_record_sql('SELECT MAX(sortorder) AS max, 1 + FROM ' . $CFG->prefix . 'course WHERE category=' . $category->id); + $count = $count->max + 100; + begin_sql(); foreach ($courses as $course) { set_field('course', 'sortorder', $count, 'id', $course->id); $count++; } - fix_course_sortorder(); + commit_sql(); + fix_course_sortorder($category->id); } } } @@ -168,51 +173,39 @@ $movecourse = NULL; $swapcourse = NULL; - $courses = get_courses($category->id,'c.sortorder ASC', 'c.id,c.sortorder'); - + // ensure the course order has no gaps + // and isn't at 0 + fix_course_sortorder($category->id); + + // we are going to need to know the range + $max = get_record_sql('SELECT MAX(sortorder) AS max, 1 + FROM ' . $CFG->prefix . 'course WHERE category=' . $category->id); + $max = $max->max + 100; + if (isset($moveup)) { - if ($movecourse = get_record("course", "id", $moveup)) { - foreach ($courses as $course) { - if ($course->id == $movecourse->id) { - break; - } - $swapcourse = $course; - } - } - } - if (isset($movedown)) { - if ($movecourse = get_record("course", "id", $movedown)) { - $choosenext = false; - foreach ($courses as $course) { - if ($choosenext) { - $swapcourse = $course; - break; - } - if ($course->id == $movecourse->id) { - $choosenext = true; - } - } - } + $movecourse = get_record('course', 'id', $moveup); + $swapcourse = get_record('course', + 'category', $category->id, + 'sortorder', $movecourse->sortorder - 1); + } else { + $movecourse = get_record('course', 'id', $movedown); + $swapcourse = get_record('course', + 'category', $category->id, + 'sortorder', $movecourse->sortorder + 1); } + if ($swapcourse and $movecourse) { // Renumber everything for robustness - $count=0; begin_sql(); - foreach ($courses as $course) { - $count++; - if ($course->id == $swapcourse->id) { - $course = $movecourse; - } else if ($course->id == $movecourse->id) { - $course = $swapcourse; - } - if (! set_field("course", "sortorder", $count, "id", $course->id)) { - notify("Could not update that course!"); - } + if (!( set_field("course", "sortorder", $max, "id", $swapcourse->id) + && set_field("course", "sortorder", $swapcourse->sortorder, "id", $movecourse->id) + && set_field("course", "sortorder", $movecourse->sortorder, "id", $swapcourse->id) + )) { + notify("Could not update that course!"); } commit_sql(); } - } - fix_course_sortorder(); + } } // End of editing stuff @@ -317,11 +310,11 @@ echo ''. ' '; if (!empty($acourse->visible)) { - echo ''. ' '; } else { - echo ''. ' '; } @@ -334,7 +327,7 @@ ' '; if ($up) { - echo ''. ' '; } else { @@ -342,7 +335,7 @@ } if ($down) { - echo ''. ' '; } else { diff --git a/lib/datalib.php b/lib/datalib.php index 10951a6c54..d0144d55bf 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -2251,25 +2251,71 @@ function get_categories($parent='none', $sort='sortorder ASC') { /** - * This recursive function makes sure that the courseorder is consecutive - * - * @param int $categoryid ? - * @param int $n ? - * @return ? - * @todo Finish documenting this function - */ -function fix_course_sortorder($categoryid=0, $n=0) { +* This recursive function makes sure that the courseorder is consecutive +* +* @param type description +* +* $n is the starting point, offered only for compatilibity -- will be ignored! +* $safe (bool) prevents it from assuming category-sortorder is unique, used to upgrade +* safely from 1.4 to 1.5 +*/ +function fix_course_sortorder($categoryid=0, $n=0, $safe=0) { + + global $CFG; $count = 0; - if ($courses = get_courses($categoryid)) { - foreach ($courses as $course) { - set_field('course', 'sortorder', $n, 'id', $course->id); - $n++; - $count++; - } + + $n=100; + + // get some basic info + $info = get_record_sql('SELECT MIN(sortorder) AS min, + MAX(sortorder) AS max, + COUNT(sortorder) AS count + FROM ' . $CFG->prefix . 'course + WHERE category=' . $categoryid); + if (is_object($info)) { // no courses? + $max = $info->max; + $count = $info->count; + $min = $info->min; + unset($info); + } + + // actually sort only if there are courses, + // and we meet one ofthe triggers: + // - safe flag + // - they are not in a continuos block + // - they are too close to the 'bottom' + if ($count && ( $safe + || ($max-$min+1!=$count) + || $min < 10 ) ) { + if ($courses = get_courses($categoryid, 'c.sortorder ASC', 'c.id,c.sortorder')) { + begin_sql(); + + // find the ideal starting point + if ( ($min<$n&&$n<$max) || ($n+$count>=$min) || ($min<10) ) { + + $n = $max+100; // this is usually the ideal solution + + // if we are aiming way too high, try to bring it back to earth + if ($n > 100+3*$count) { + if ($min > 100+$count){ + $n = 100; + } + } + } + + foreach ($courses as $course) { + if ($course->sortorder != $n ) { // save db traffic + set_field('course', 'sortorder', $n, 'id', $course->id); + } + $n++; + } + commit_sql(); + } } set_field('course_categories', 'coursecount', $count, 'id', $categoryid); + $n=0; if ($categories = get_categories($categoryid)) { foreach ($categories as $category) { $n = fix_course_sortorder($category->id, $n); -- 2.39.5