Index: admin/index.php =================================================================== @@ -516,18 +516,10 @@ get_context_instance(CONTEXT_COURSE, $newid); // Site created, add blocks for it - $page = page_create_object(PAGE_COURSE_VIEW, $newid); - blocks_repopulate_page($page); // Return value not checked because you can always edit later - - $cat = new object(); - $cat->name = get_string('miscellaneous'); - $cat->depth = 1; - if (!$catid = insert_record('course_categories', $cat)) { - error("Serious Error! Could not set up a default course category!"); - } - // make sure category context exists - get_context_instance(CONTEXT_COURSECAT, $catid); - mark_context_dirty('/'.SYSCONTEXTID); +// start MDL-15265 + // create default course category + $cat = get_course_category(); +// end MDL-15265 redirect('index.php'); } Index: lib/datalib.php =================================================================== @@ -11,8 +11,12 @@ */ /// Some constants +// start MDL-15265 + define('MAX_COURSES_IN_CATEGORY', 10000); // MAX_COURSES_IN_CATEGORY * MAX_COURSE_CATEGORIES must not be more than max integer! + define('MAX_COURSE_CATEGORIES', 10000); define('LASTACCESS_UPDATE_SECS', 60); /// Number of seconds to wait before /// updating lastaccess information in DB. +// end MDL-15265 /** * Escape all dangerous characters in a data record @@ -1266,189 +1270,240 @@ return $subcats; } +// start MDL-15265 +/* + * Return specified category, default if given does not exist + * @param int $catid course category id + * @return object caregory + */ +function get_course_category($catid=0) { + $category = false; + if (!empty($catid)) { + $category = get_record('course_categories', 'id', $catid); + } -/** -* 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, $depth=0, $path='') { + if (!$category) { + // the first category is considered default for now + if ($category = get_records('course_categories', null, 'sortorder', '*', 0, 1)) { + $category = reset($category); + } else { + $cat = new object(); + $cat->name = get_string('miscellaneous'); + $cat->depth = 1; + $cat->sortorder = MAX_COURSES_IN_CATEGORY; + $cat->timemodified = time(); + if (!$catid = insert_record('course_categories', $cat)) { + print_error('cannotsetupcategory', 'error'); + } + // make sure category context exists + get_context_instance(CONTEXT_COURSECAT, $catid); + mark_context_dirty('/'.SYSCONTEXTID); + $category = get_record('course_categories', 'id', $catid); + } + } + return $category; +} +/** + Fixes course category and course sortorder, also verifies category and course parents and paths. + * (circular references are not fixed) + */ +function fix_course_sortorder() { global $CFG; - $count = 0; - - $catgap = 1000; // "standard" category gap - $tolerance = 200; // how "close" categories can get - - if ($categoryid > 0){ - // update depth and path - $cat = get_record('course_categories', 'id', $categoryid); - if ($cat->parent == 0) { - $depth = 0; - $path = ''; - } else if ($depth == 0 ) { // doesn't make sense; get from DB - // this is only called if the $depth parameter looks dodgy - $parent = get_record('course_categories', 'id', $cat->parent); - $path = $parent->path; - $depth = $parent->depth; - } - $path = $path . '/' . $categoryid; - $depth = $depth + 1; - - if ($cat->path !== $path) { - set_field('course_categories', 'path', addslashes($path), 'id', $categoryid); - } - if ($cat->depth != $depth) { - set_field('course_categories', 'depth', $depth, 'id', $categoryid); - } - } - - // get some basic info about courses in the category - $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); - } - - if ($categoryid > 0 && $n==0) { // only passed category so don't shift it - $n = $min; - } - - // $hasgap flag indicates whether there's a gap in the sequence - $hasgap = false; - if ($max-$min+1 != $count) { - $hasgap = true; - } - - // $mustshift indicates whether the sequence must be shifted to - // meet its range - $mustshift = false; - if ($min < $n+$tolerance || $min > $n+$tolerance+$catgap ) { - $mustshift = true; - } - - // 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 || $hasgap || $mustshift ) ) { - // special, optimized case where all we need is to shift - if ( $mustshift && !$safe && !$hasgap) { - $shift = $n + $catgap - $min; - if ($shift < $count) { - $shift = $count + $catgap; - } - // UPDATE course SET sortorder=sortorder+$shift - execute_sql("UPDATE {$CFG->prefix}course - SET sortorder=sortorder+$shift - WHERE category=$categoryid", 0); - $n = $n + $catgap + $count; - - } else { // do it slowly - $n = $n + $catgap; - // if the new sequence overlaps the current sequence, lack of transactions - // will stop us -- shift things aside for a moment... - if ($safe || ($n >= $min && $n+$count+1 < $min && $CFG->dbfamily==='mysql')) { - $shift = $max + $n + 1000; - execute_sql("UPDATE {$CFG->prefix}course - SET sortorder=sortorder+$shift - WHERE category=$categoryid", 0); - } - - $courses = get_courses($categoryid, 'c.sortorder ASC', 'c.id,c.sortorder'); - begin_sql(); - $tx = true; // transaction sanity - foreach ($courses as $course) { - if ($tx && $course->sortorder != $n ) { // save db traffic - $tx = $tx && set_field('course', 'sortorder', $n, - 'id', $course->id); - } - $n++; - } - if ($tx) { - commit_sql(); + if ($unsorted = get_records('course_categories', 'sortorder', 0)) { + //move all categories that are not sorted yet to the end + set_field('course_categories', 'sortorder', MAX_COURSES_IN_CATEGORY*MAX_COURSE_CATEGORIES, 'sortorder',0); + } + + $allcats = get_records('course_categories', null, '', 'sortorder', 'id, sortorder, parent, depth, path'); + $topcats = array(); + $brokencats = array(); + foreach ($allcats as $cat) { + $sortorder = (int)$cat->sortorder; + if (!$cat->parent) { + while(isset($topcats[$sortorder])) { + $sortorder++; + } + $topcats[$sortorder] = $cat; + continue; + } + if (!isset($allcats[$cat->parent])) { + $brokencats[] = $cat; + continue; + } + if (!isset($allcats[$cat->parent]->children)) { + $allcats[$cat->parent]->children = array(); + } + while(isset($allcats[$cat->parent]->children[$sortorder])) { + $sortorder++; + } + $allcats[$cat->parent]->children[$sortorder] = $cat; + } + unset($allcats); + + // add broken cats to category tree + if ($brokencats) { + $defaultcat = reset($topcats); + foreach ($brokencats as $cat) { + $topcats[] = $cat; + } + } + // now walk recursively the tree and fix any problems found + $sortorder = 0; + $fixcontexts = array(); + _fix_course_cats($topcats, $sortorder, 0, 0, '', $fixcontexts); + + // detect if there are "multiple" frontpage courses and fix them if needed + $frontcourses = get_records('course', 'category', 0, 'id'); + if (count($frontcourses) > 1) { + if (isset($frontcourses[SITEID])) { + $frontcourse = $frontcourses[SITEID]; + unset($frontcourses[SITEID]); + } else { + $frontcourse = array_shift($frontcourses); + } + $defaultcat = reset($topcats); + foreach ($frontcourses as $course) { + set_field('course', 'category', $defaultcat->id, 'id',$course->id); + $context = get_context_instance(CONTEXT_COURSE, $course->id); + $fixcontexts[$context->id] = $context; + } + unset($frontcourses); + } + else { + $frontcourse = reset($frontcourses); + } + + // now fix the paths and depths in context table if needed + if ($fixcontexts) { + rebuild_contexts($fixcontexts); + } + + // release memory + unset($topcats); + unset($brokencats); + unset($fixcontexts); + + // fix frontpage course sortorder + if ($frontcourse->sortorder != 1) { + set_field('course', 'sortorder', 1, 'id', $frontcourse->id); + } + + // now fix the course counts in category records if needed + $sql = "SELECT cc.id, cc.coursecount, COUNT(c.id) AS newcount + FROM {$CFG->prefix}course_categories cc + LEFT JOIN {$CFG->prefix}course c ON c.category = cc.id + GROUP BY cc.id, cc.coursecount + HAVING cc.coursecount <> COUNT(c.id)"; + + if ($updatecounts = get_records_sql($sql)) { + foreach ($updatecounts as $cat) { + $cat->coursecount = $cat->newcount; + unset($cat->newcount); + update_record('course_categories', $cat, true); + } + } + + // now make sure that sortorders in course table are withing the category sortorder ranges + $sql = "SELECT cc.id, cc.sortorder + FROM {$CFG->prefix}course_categories cc + JOIN {$CFG->prefix}course c ON c.category = cc.id + WHERE c.sortorder < cc.sortorder OR c.sortorder > cc.sortorder + ".MAX_COURSES_IN_CATEGORY; + + if ($fixcategories = get_records_sql($sql)) { + //fix the course sortorder ranges + foreach ($fixcategories as $cat) { + $sql = "UPDATE {$CFG->prefix}course + SET sortorder = (sortorder % ".MAX_COURSES_IN_CATEGORY.") + {$cat->sortorder} + WHERE category = {$cat->id}"; + execute_sql($sql); + } + } + unset($fixcategories); + + // categories having courses with sortorder duplicates or having gaps in sortorder + $sql = "SELECT DISTINCT c1.category AS id , cc.sortorder + FROM {$CFG->prefix}course c1 + JOIN {$CFG->prefix}course c2 ON c1.sortorder = c2.sortorder + JOIN {$CFG->prefix}course_categories cc ON (c1.category = cc.id) + WHERE c1.id <> c2.id"; + $fixcategories = get_records_sql($sql); + + $sql = "SELECT cc.id, cc.sortorder, cc.coursecount, MAX(c.sortorder) AS maxsort, MIN(c.sortorder) AS minsort + FROM {$CFG->prefix}course_categories cc + JOIN {$CFG->prefix}course c ON c.category = cc.id + GROUP BY cc.id, cc.sortorder, cc.coursecount + HAVING (MAX(c.sortorder) <> cc.sortorder + cc.coursecount) OR (MIN(c.sortorder) <> cc.sortorder + 1)"; + $gapcategories = get_records_sql($sql); + if(!empty($gapcategories)){ + foreach ($gapcategories as $cat) { + if (isset($fixcategories[$cat->id])) { + // duplicates detected already + + } else if ($cat->minsort == $cat->sortorder and $cat->maxsort == $cat->sortorder + $cat->coursecount - 1) { + // easy - new course inserted with sortorder 0, the rest is ok + $sql = "UPDATE {$CFG->prefix}course + SET sortorder = sortorder + 1 + WHERE category = {$cat->id}"; + execute_sql($sql); } else { - rollback_sql(); - if (!$safe) { - // if we failed when called with !safe, try - // to recover calling self with safe=true - return fix_course_sortorder($categoryid, $n, true, $depth, $path); - } + // it needs full resorting + $fixcategories[$cat->id] = $cat; } } } - set_field('course_categories', 'coursecount', $count, 'id', $categoryid); - - // $n could need updating - $max = get_field_sql("SELECT MAX(sortorder) from {$CFG->prefix}course WHERE category=$categoryid"); - if ($max > $n) { - $n = $max; - } - - if ($categories = get_categories($categoryid)) { - foreach ($categories as $category) { - $n = fix_course_sortorder($category->id, $n, $safe, $depth, $path); + unset($gapcategories); + + // fix course sortorders in problematic categories only + if(!empty($fixcategories)){ + foreach ($fixcategories as $cat) { + $i = 1; + $courses = get_records('course', 'category',$cat->id, 'sortorder ASC, id DESC', 'id, sortorder'); + foreach ($courses as $course) { + if ($course->sortorder != $cat->sortorder + $i) { + $course->sortorder = $cat->sortorder + $i; + update_record('course', $course, true); + } + $i++; + } } } - - return $n+1; } /** - * Ensure all courses have a valid course category - * useful if a category has been removed manually - **/ -function fix_coursecategory_orphans() { - - global $CFG; - - // Note: the handling of sortorder here is arguably - // open to race conditions. Hard to fix here, unlikely - // to hit anyone in production. - - $sql = "SELECT c.id, c.category, c.shortname - FROM {$CFG->prefix}course c - LEFT OUTER JOIN {$CFG->prefix}course_categories cc ON c.category=cc.id - WHERE cc.id IS NULL AND c.id != " . SITEID; - - $rs = get_recordset_sql($sql); - - if (!rs_EOF($rs)) { // we have some orphans - - // the "default" category is the lowest numbered... - $default = get_field_sql("SELECT MIN(id) - FROM {$CFG->prefix}course_categories"); - $sortorder = get_field_sql("SELECT MAX(sortorder) - FROM {$CFG->prefix}course - WHERE category=$default"); - - - begin_sql(); - $tx = true; - while ($tx && $course = rs_fetch_next_record($rs)) { - $tx = $tx && set_field('course', 'category', $default, 'id', $course->id); - $tx = $tx && set_field('course', 'sortorder', ++$sortorder, 'id', $course->id); - } - if ($tx) { - commit_sql(); - } else { - rollback_sql(); + * Internal recursive category verification function, do not use directly! + */ +function _fix_course_cats($children, &$sortorder, $parent, $depth, $path, &$fixcontexts) { + + $depth++; + foreach ($children as $cat) { + $sortorder = $sortorder + MAX_COURSES_IN_CATEGORY; + $update = false; + if ($parent != $cat->parent or $depth != $cat->depth or $path.'/'.$cat->id != $cat->path) { + $cat->parent = $parent; + $cat->depth = $depth; + $cat->path = $path.'/'.$cat->id; + $update = true; + + // make sure context caches are rebuild and dirty contexts marked + $context = get_context_instance(CONTEXT_COURSECAT, $cat->id); + $fixcontexts[$context->id] = $context; + } + if ($cat->sortorder != $sortorder) { + $cat->sortorder = $sortorder; + $update = true; + } + if ($update) { + update_record('course_categories', $cat, true); + + } + if (isset($cat->children)) { + _fix_course_cats($cat->children, $sortorder, $cat->id, $cat->depth, $cat->path, $fixcontexts); } } - rs_close($rs); } +// end MDL-15265 /** * List of remote courses that a user has access to via MNET. Index: lib/accesslib.php =================================================================== @@ -5386,6 +5386,24 @@ return ($cap->component != $comp || $cap->contextlevel != $contextlevel); } +// start MDL-15265 +/** + * Rebuild all related context depth and path caches + * @param array $fixcontexts array of contexts + */ +function rebuild_contexts(array $fixcontexts) { + + foreach ($fixcontexts as $context) { + if ($context->path) { + mark_context_dirty($context->path); + } + set_field_select('context', 'depth', 0, "path LIKE '%/$context->id/%'"); + set_field('context', 'depth', 0, 'id',$context->id); + } + build_context_path(false); +} +// end MDL-15265 + /** * Populate context.path and context.depth where missing. * @param bool $force force a complete rebuild of the path and depth fields. Index: enrol/ldap/enrol.php =================================================================== @@ -583,19 +583,15 @@ $course->summary = empty($CFG->enrol_ldap_course_summary) || empty($course_ext[$CFG->enrol_ldap_course_summary][0]) ? '' : $course_ext[$CFG->enrol_ldap_course_summary][0]; + +// start MDL-15265 - if(!empty($CFG->enrol_ldap_category)){ // optional ... but ensure it is set! - $course->category = $CFG->enrol_ldap_category; - } - if ($course->category == 0){ // must be avoided as it'll break moodle - $course->category = 1; // the misc 'catch-all' category - } + $category = get_course_category($CFG->enrol_db_category); + + // put at the end of category + $course->sortorder = $category->sortorder + MAX_COURSES_IN_CATEGORY - 1; - // define the sortorder (yuck) - $sort = get_record_sql('SELECT MAX(sortorder) AS max, 1 FROM ' . $CFG->prefix . 'course WHERE category=' . $course->category); - $sort = $sort->max; - $sort++; - $course->sortorder = $sort; +// end MDL-15265 // override with local data $course->startdate = time(); Index: course/pending.php =================================================================== @@ -56,19 +56,9 @@ $course->$key = addslashes($course->$key); } - /// Ensure all is well before proceeding. - fix_course_sortorder(); - if (empty($CFG->defaultrequestcategory) or !record_exists('course_categories', 'id', $CFG->defaultrequestcategory)) { - /// default to first top level directory, hacky but means things don't break - $CFG->defaultrequestcategory = get_field('course_categories', 'id', 'parent', '0'); - } - - /// Build up a course record based on the request. - $course->category = $CFG->defaultrequestcategory; - $course->sortorder = get_field_sql("SELECT min(sortorder)-1 FROM {$CFG->prefix}course WHERE category=$course->category"); - if (empty($course->sortorder)) { - $course->sortorder = 1000; - } +// start MDL-15265 + $category = get_course_category($CFG->defaultrequestcategory); + $course->sortorder = $category->sortorder; // place as the first in category $course->requested = 1; unset($course->reason); unset($course->id); @@ -95,6 +85,8 @@ } delete_records('course_request','id',$approve); $success = 1; + fix_course_sortorder(); +// end MDL-15265 } if (!empty($success)) { $user = get_record('user','id',$teacherid); Index: course/index.php =================================================================== @@ -185,64 +185,46 @@ } /// Move a category up or down +// start MDL-15265 if ((!empty($moveup) or !empty($movedown)) and confirm_sesskey()) { + // fix_course_sortorder(); $swapcategory = NULL; $movecategory = NULL; if (!empty($moveup)) { require_capability('moodle/category:manage', get_context_instance(CONTEXT_COURSECAT, $moveup)); if ($movecategory = get_record('course_categories', 'id', $moveup)) { - $categories = get_categories($movecategory->parent); - - foreach ($categories as $category) { - if ($category->id == $movecategory->id) { - break; - } - $swapcategory = $category; + $sql = 'SELECT sortorder as sortorder, id + FROM '.$CFG->prefix.'course_categories + WHERE sortorder in (SELECT max(sortorder) + FROM '.$CFG->prefix.'course_categories + WHERE sortorder<'.$movecategory->sortorder.' AND parent='.$movecategory->parent.') + AND parent='.$movecategory->parent; + $swapcategory = get_record_sql($sql); + if (empty($swapcategory->id)) { + $swapcategory->sortorder = $movecategory->sortorder; + $swapcategory->id = $movecategory->id; } - unset($category); } - } - if (!empty($movedown)) { + } else { require_capability('moodle/category:manage', get_context_instance(CONTEXT_COURSECAT, $movedown)); if ($movecategory = get_record('course_categories', 'id', $movedown)) { - $categories = get_categories($movecategory->parent); - - $choosenext = false; - foreach ($categories as $category) { - if ($choosenext) { - $swapcategory = $category; - break; - } - if ($category->id == $movecategory->id) { - $choosenext = true; - } + if ($swapcategory = get_records_select('course_categories', "sortorder>".$movecategory->sortorder." AND parent=".$movecategory->parent, 'sortorder ASC', '*', 0, 1)) { + $swapcategory = reset($swapcategory); } - unset($category); } } - if ($swapcategory and $movecategory) { // Renumber everything for robustness - $count=0; - foreach ($categories as $category) { - $count++; - if ($category->id == $swapcategory->id) { - $category = $movecategory; - } else if ($category->id == $movecategory->id) { - $category = $swapcategory; - } - if (! set_field('course_categories', 'sortorder', $count, 'id', $category->id)) { - notify('Could not update that category!'); - } - } - unset($category); + if ($swapcategory and $movecategory) { + set_field('course_categories', 'sortorder', $swapcategory->sortorder, 'id', $movecategory->id); + set_field('course_categories', 'sortorder', $movecategory->sortorder, 'id', $swapcategory->id); } + // finally reorder courses + fix_course_sortorder(); } - -/// Find any orphan courses that don't yet have a valid category and set to default - fix_coursecategory_orphans(); - -/// Should be a no-op 99% of the cases - fix_course_sortorder(); + +/// This should not be needed anymore + //fix_course_sortorder(); +// end MDL-15265 /// Print out the categories with all the knobs $strcategories = get_string('categories'); Index: course/lib.php =================================================================== @@ -3109,46 +3109,34 @@ **/ function move_courses ($courseids, $categoryid) { +// start MDL-15265 global $CFG; - if (!empty($courseids)) { - - $courseids = array_reverse($courseids); - - foreach ($courseids as $courseid) { - - if (! $course = get_record("course", "id", $courseid)) { - notify("Error finding course $courseid"); - } else { - // figure out a sortorder that we can use in the destination category - $sortorder = get_field_sql('SELECT MIN(sortorder)-1 AS min - FROM ' . $CFG->prefix . 'course WHERE category=' . $categoryid); - if (is_null($sortorder) || $sortorder === false) { - // the category is empty - // rather than let the db default to 0 - // set it to > 100 and avoid extra work in fix_coursesortorder() - $sortorder = 200; - } else if ($sortorder < 10) { - fix_course_sortorder($categoryid); - } - - $course->category = $categoryid; - $course->sortorder = $sortorder; - - if (!update_record('course', addslashes_recursive($course))) { - notify("An error occurred - course not moved!"); - } - - $context = get_context_instance(CONTEXT_COURSE, $course->id); - $newparent = get_context_instance(CONTEXT_COURSECAT, $course->category); - context_moved($context, $newparent); + if (!empty($courseids) and $category = get_record('course_categories', 'id', $categoryid)) { + $courseids = array_reverse($courseids); + $i = 1; + + foreach ($courseids as $courseid) { + if (!$course = get_record("course","id",$courseid)) { + notify("Error finding course $courseid"); + } else { + $course->category = $categoryid; + $course->sortorder = $category->sortorder + MAX_COURSES_IN_CATEGORY - $i++; + if (!update_record('course', $course)) { + notify("An error occurred - course not moved!"); } + + $context = get_context_instance(CONTEXT_COURSE, $course->id); + $newparent = get_context_instance(CONTEXT_COURSECAT, $course->category); + context_moved($context, $newparent); } - fix_course_sortorder(); } + fix_course_sortorder(); + } return true; } +// end MDL-15265 /*** *** Efficiently moves a category - NOTE that this can have @@ -3176,10 +3164,13 @@ context_moved($context, $newparent); - // The most effective thing would be to find the common parent, - // until then, do it sitewide... - fix_course_sortorder(); +// start MDL-15265 + // now make it last in new category + set_field('course_categories', 'sortorder', MAX_COURSES_IN_CATEGORY*MAX_COURSE_CATEGORIES,'id',$category->id); + // and fix the sortorders +// end MDL-15265 + fix_course_sortorder(); return true; } @@ -3283,13 +3274,10 @@ } $data->timecreated = time(); - - // place at beginning of category - fix_course_sortorder(); - $data->sortorder = get_field_sql("SELECT min(sortorder)-1 FROM {$CFG->prefix}course WHERE category=$data->category"); - if (empty($data->sortorder)) { - $data->sortorder = 100; - } +// start MDL-15265 + // place at beginning of any category + $data->sortorder = 0; +// end MDL-15265 if ($newcourseid = insert_record('course', $data)) { // Set up new course Index: course/editcategory.php =================================================================== @@ -71,7 +71,9 @@ } else { // Create a new category. - $newcategory->sortorder = 999; +// start MDL-15265 + $newcategory->sortorder = MAX_COURSES_IN_CATEGORY*MAX_COURSE_CATEGORIES; // put as last category in any parent cat +// end MDL-15265 if (!$newcategory->id = insert_record('course_categories', $newcategory)) { error("Could not insert the new category '$newcategory->name' "); } Index: course/category.php =================================================================== @@ -56,17 +56,14 @@ /// Resort the category if requested if ($resort and confirm_sesskey()) { 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(); +// start MDL-15265 + $i = 1; foreach ($courses as $course) { - set_field('course', 'sortorder', $count, 'id', $course->id); - $count++; + set_field('course', 'sortorder', $category->sortorder+$i, 'id', $course->id); + $i++; } - commit_sql(); - fix_course_sortorder($category->id); + fix_course_sortorder(); // should not be needed +// end MDL-15265 } } } @@ -163,7 +160,9 @@ /// Move a course up or down if ((!empty($moveup) or !empty($movedown)) and confirm_sesskey()) { require_capability('moodle/category:manage', $context); - $movecourse = NULL; +// start MDL-15265 + // ensure the course order has continuous ordering + fix_course_sortorder(); $swapcourse = NULL; // ensure the course order has no gaps and isn't at 0 @@ -175,29 +174,21 @@ $max = $max->max + 100; if (!empty($moveup)) { - $movecourse = get_record('course', 'id', $moveup); - $swapcourse = get_record('course', 'category', $category->id, - 'sortorder', $movecourse->sortorder - 1); + if ($movecourse = get_record('course', 'id',$moveup)) { + $swapcourse = get_record('course', 'sortorder',$movecourse->sortorder-1); + } } else { - $movecourse = get_record('course', 'id', $movedown); - $swapcourse = get_record('course', 'category', $category->id, - 'sortorder', $movecourse->sortorder + 1); + if ($movecourse = get_record('course', 'id', $movedown)) { + $swapcourse = get_record('course', 'sortorder', $movecourse->sortorder+1); + } } - if ($swapcourse and $movecourse) { - // Renumber everything for robustness - begin_sql(); - 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(); + 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); } - } } // End of editing stuff +// end MDL-15265 if ($editingon && has_capability('moodle/category:manage', $context)) { echo '