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

diff --git a/course/category.php b/course/category.php
index 70f93fa119a02d5511b2e93dcc3eecdd92194833..10c6eecb5d5d94d8d7fc3a693ce60176cc644b5f 100644 (file)
--- 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 '<a title="'.$strdelete.'" href="delete.php?id='.$acourse->id.'">'.
                          '<img src="'.$pixpath.'/t/delete.gif" height="11" width="11" border="0" alt="" /></a> ';
                     if (!empty($acourse->visible)) {
-                        echo '<a title="'.$strhide.'" href="category.php?id='.$category->id.
+                        echo '<a title="'.$strhide.'" href="category.php?id='.$category->id.'&amp;page='.$page.
                              '&amp;hide='.$acourse->id.'&amp;sesskey='.$USER->sesskey.'">'.
                              '<img src="'.$pixpath.'/t/hide.gif" height="11" width="11" border="0" alt="" /></a> ';
                     } else {
-                        echo '<a title="'.$strshow.'" href="category.php?id='.$category->id.
+                        echo '<a title="'.$strshow.'" href="category.php?id='.$category->id.'&amp;page='.$page.
                              '&amp;show='.$acourse->id.'&amp;sesskey='.$USER->sesskey.'">'.
                              '<img src="'.$pixpath.'/t/show.gif" height="11" width="11" border="0" alt="" /></a> ';
                     }
@@ -334,7 +327,7 @@
                              '<img src="'.$pixpath.'/t/restore.gif" height="11" width="11" border="0" alt="" /></a> ';
             
                     if ($up) {
-                        echo '<a title="'.$strmoveup.'" href="category.php?id='.$category->id.
+                        echo '<a title="'.$strmoveup.'" href="category.php?id='.$category->id.'&amp;page='.$page.
                              '&amp;moveup='.$acourse->id.'&amp;sesskey='.$USER->sesskey.'">'.
                              '<img src="'.$pixpath.'/t/up.gif" height="11" width="11" border="0" alt="" /></a> ';
                     } else {
@@ -342,7 +335,7 @@
                     }
         
                     if ($down) {
-                        echo '<a title="'.$strmovedown.'" href="category.php?id='.$category->id.
+                        echo '<a title="'.$strmovedown.'" href="category.php?id='.$category->id.'&amp;page='.$page.
                              '&amp;movedown='.$acourse->id.'&amp;sesskey='.$USER->sesskey.'">'.
                              '<img src="'.$pixpath.'/t/down.gif" height="11" width="11" border="0" alt="" /></a> ';
                     } else {

diff --git a/lib/datalib.php b/lib/datalib.php
index 10951a6c541e899610120146411e79209b5ea8b9..d0144d55bf6db9cc56e1a6775ee756c51dd24b20 100644 (file)
--- 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);