Skip to content

Commit

Permalink
MDL-69573 core_course: Make MAX_COURSES_IN_CATEGORY configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
Nathan Nguyen committed Sep 30, 2020
1 parent e049d30 commit 10db3a0
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 11 deletions.
14 changes: 14 additions & 0 deletions config-dist.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,20 @@
//
// $CFG->forumpostcountchunksize = 5000;
//
// Course and category sorting
//
// If the number of courses in a category exceeds $CFG->maxcoursesincategory (10000 by default), it may lead to duplicate
// sort orders of courses in separated categories. For example:
// - Category A has the sort order of 10000, and has 10000 courses. The last course will have the sort order of 20000.
// - Category B has the sort order of 20000, and has a course with the sort order of 20001.
// - If we add another course in category A, it will have a sort order of 20001,
// which is the same as the course in category B
// The duplicate will cause sorting issue and hence we need to increase $CFG->maxcoursesincategory
// to fix the duplicate sort order
// Please also make sure $CFG->maxcoursesincategory * MAX_COURSE_CATEGORIES less than max integer.
//
// $CFG->maxcoursesincategory = 10000;
//
//=========================================================================
// 7. SETTINGS FOR DEVELOPMENT SERVERS - not intended for production use!!!
//=========================================================================
Expand Down
3 changes: 2 additions & 1 deletion course/classes/category.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2319,7 +2319,8 @@ protected function change_parent_raw(core_course_category $newparentcat) {
$context->update_moved($newparent);

// Now make it last in new category.
$DB->set_field('course_categories', 'sortorder', MAX_COURSES_IN_CATEGORY * MAX_COURSE_CATEGORIES, ['id' => $this->id]);
$DB->set_field('course_categories', 'sortorder',
get_max_courses_in_category() * MAX_COURSE_CATEGORIES, ['id' => $this->id]);

if ($hidecat) {
fix_course_sortorder();
Expand Down
2 changes: 1 addition & 1 deletion course/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2103,7 +2103,7 @@ function move_courses($courseids, $categoryid) {
$course->id = $dbcourse->id;
$course->timemodified = time();
$course->category = $category->id;
$course->sortorder = $category->sortorder + MAX_COURSES_IN_CATEGORY - $i++;
$course->sortorder = $category->sortorder + get_max_courses_in_category() - $i++;
if ($category->visible == 0) {
// Hide the course when moving into hidden category, do not update the visibleold flag - we want to get
// to previous state if somebody unhides the category.
Expand Down
35 changes: 27 additions & 8 deletions lib/datalib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,6 @@ function get_courses_search($searchterms, $sort, $page, $recordsperpage, &$total
*
* @global object
* @global object
* @uses MAX_COURSES_IN_CATEGORY
* @uses MAX_COURSE_CATEGORIES
* @uses SITEID
* @uses CONTEXT_COURSE
Expand All @@ -801,7 +800,8 @@ function fix_course_sortorder() {

if ($unsorted = $DB->get_records('course_categories', array('sortorder'=>0))) {
//move all categories that are not sorted yet to the end
$DB->set_field('course_categories', 'sortorder', MAX_COURSES_IN_CATEGORY*MAX_COURSE_CATEGORIES, array('sortorder'=>0));
$DB->set_field('course_categories', 'sortorder',
get_max_courses_in_category() * MAX_COURSE_CATEGORIES, array('sortorder' => 0));
$cacheevents['changesincoursecat'] = true;
}

Expand Down Expand Up @@ -901,15 +901,19 @@ function fix_course_sortorder() {
$categories = array();
foreach ($updatecounts as $cat) {
$cat->coursecount = $cat->newcount;
if ($cat->coursecount >= MAX_COURSES_IN_CATEGORY) {
if ($cat->coursecount >= get_max_courses_in_category()) {
$categories[] = $cat->id;
}
unset($cat->newcount);
$DB->update_record_raw('course_categories', $cat, true);
}
if (!empty($categories)) {
$str = implode(', ', $categories);
debugging("The number of courses (category id: $str) has reached MAX_COURSES_IN_CATEGORY (" . MAX_COURSES_IN_CATEGORY . "), it will cause a sorting performance issue, please increase the value of MAX_COURSES_IN_CATEGORY in lib/datalib.php file. See tracker issue: MDL-25669", DEBUG_DEVELOPER);
debugging("The number of courses (category id: $str) has reached max number of courses " .
"in a category (" . get_max_courses_in_category() . "). It will cause a sorting performance issue. " .
"Please set higher value for \$CFG->maxcoursesincategory in config.php. " .
"Please also make sure \$CFG->maxcoursesincategory * MAX_COURSE_CATEGORIES less than max integer. " .
"See tracker issues: MDL-25669 and MDL-69573", DEBUG_DEVELOPER);
}
$cacheevents['changesincoursecat'] = true;
}
Expand All @@ -918,13 +922,13 @@ function fix_course_sortorder() {
$sql = "SELECT DISTINCT cc.id, cc.sortorder
FROM {course_categories} cc
JOIN {course} c ON c.category = cc.id
WHERE c.sortorder < cc.sortorder OR c.sortorder > cc.sortorder + ".MAX_COURSES_IN_CATEGORY;
WHERE c.sortorder < cc.sortorder OR c.sortorder > cc.sortorder + " . get_max_courses_in_category();

if ($fixcategories = $DB->get_records_sql($sql)) {
//fix the course sortorder ranges
foreach ($fixcategories as $cat) {
$sql = "UPDATE {course}
SET sortorder = ".$DB->sql_modulo('sortorder', MAX_COURSES_IN_CATEGORY)." + ?
SET sortorder = ".$DB->sql_modulo('sortorder', get_max_courses_in_category())." + ?
WHERE category = ?";
$DB->execute($sql, array($cat->sortorder, $cat->id));
}
Expand Down Expand Up @@ -992,7 +996,6 @@ function fix_course_sortorder() {
* @todo Document the arguments of this function better
*
* @global object
* @uses MAX_COURSES_IN_CATEGORY
* @uses CONTEXT_COURSECAT
* @param array $children
* @param int $sortorder
Expand All @@ -1009,7 +1012,7 @@ function _fix_course_cats($children, &$sortorder, $parent, $depth, $path, &$fixc
$changesmade = false;

foreach ($children as $cat) {
$sortorder = $sortorder + MAX_COURSES_IN_CATEGORY;
$sortorder = $sortorder + get_max_courses_in_category();
$update = false;
if ($parent != $cat->parent or $depth != $cat->depth or $path.'/'.$cat->id != $cat->path) {
$cat->parent = $parent;
Expand Down Expand Up @@ -1804,3 +1807,19 @@ function decompose_update_into_safe_changes(array $newvalues, $unusedvalue) {

return $safechanges;
}

/**
* Return maximum number of courses in a category
*
* @uses MAX_COURSES_IN_CATEGORY
* @return int number of courses
*/
function get_max_courses_in_category() {
global $CFG;
// Use default MAX_COURSES_IN_CATEGORY if $CFG->maxcoursesincategory is not set or invalid.
if (!isset($CFG->maxcoursesincategory) || clean_param($CFG->maxcoursesincategory, PARAM_INT) == 0) {
return MAX_COURSES_IN_CATEGORY;
} else {
return $CFG->maxcoursesincategory;
}
}
2 changes: 1 addition & 1 deletion lib/db/install.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function xmldb_main_install() {
$cat = new stdClass();
$cat->name = get_string('miscellaneous');
$cat->depth = 1;
$cat->sortorder = MAX_COURSES_IN_CATEGORY;
$cat->sortorder = get_max_courses_in_category();
$cat->timemodified = time();
$catid = $DB->insert_record('course_categories', $cat);
$DB->set_field('course_categories', 'path', '/'.$catid, array('id'=>$catid));
Expand Down
93 changes: 93 additions & 0 deletions lib/tests/datalib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,4 +653,97 @@ public function test_get_all_instances_in_course() {
$this->assertInstanceOf('coding_exception', $e);
}
}

/**
* Test max courses in category
*/
public function test_max_courses_in_category() {
global $CFG;
$this->resetAfterTest();

// Default settings.
$this->assertEquals(MAX_COURSES_IN_CATEGORY, get_max_courses_in_category());

// Misc category.
$misc = core_course_category::get_default();
$this->assertEquals(MAX_COURSES_IN_CATEGORY, $misc->sortorder);

$category1 = $this->getDataGenerator()->create_category();
$category2 = $this->getDataGenerator()->create_category();

// Check category sort orders.
$this->assertEquals(MAX_COURSES_IN_CATEGORY, core_course_category::get($misc->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2, core_course_category::get($category1->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3, core_course_category::get($category2->id)->sortorder);

// Create courses.
$course1 = $this->getDataGenerator()->create_course(['category' => $category1->id]);
$course2 = $this->getDataGenerator()->create_course(['category' => $category2->id]);
$course3 = $this->getDataGenerator()->create_course(['category' => $category1->id]);
$course4 = $this->getDataGenerator()->create_course(['category' => $category2->id]);

// Check course sort orders.
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2 + 2, get_course($course1->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3 + 2, get_course($course2->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2 + 1, get_course($course3->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3 + 1, get_course($course4->id)->sortorder);

// Increase max course in category.
$CFG->maxcoursesincategory = 20000;
$this->assertEquals(20000, get_max_courses_in_category());

// The sort order has not yet fixed, these sort orders should be the same as before.
// Categories.
$this->assertEquals(MAX_COURSES_IN_CATEGORY, core_course_category::get($misc->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2, core_course_category::get($category1->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3, core_course_category::get($category2->id)->sortorder);
// Courses in category 1.
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2 + 2, get_course($course1->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 2 + 1, get_course($course3->id)->sortorder);
// Courses in category 2.
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3 + 2, get_course($course2->id)->sortorder);
$this->assertEquals(MAX_COURSES_IN_CATEGORY * 3 + 1, get_course($course4->id)->sortorder);

// Create new category so that the sort orders are applied.
$category3 = $this->getDataGenerator()->create_category();
// Categories.
$this->assertEquals(20000, core_course_category::get($misc->id)->sortorder);
$this->assertEquals(20000 * 2, core_course_category::get($category1->id)->sortorder);
$this->assertEquals(20000 * 3, core_course_category::get($category2->id)->sortorder);
$this->assertEquals(20000 * 4, core_course_category::get($category3->id)->sortorder);
// Courses in category 1.
$this->assertEquals(20000 * 2 + 2, get_course($course1->id)->sortorder);
$this->assertEquals(20000 * 2 + 1, get_course($course3->id)->sortorder);
// Courses in category 2.
$this->assertEquals(20000 * 3 + 2, get_course($course2->id)->sortorder);
$this->assertEquals(20000 * 3 + 1, get_course($course4->id)->sortorder);
}

/**
* Test debug message for max courses in category
*/
public function test_debug_max_courses_in_category() {
global $CFG;
$this->resetAfterTest();

// Set to small value so that we can check the debug message.
$CFG->maxcoursesincategory = 3;
$this->assertEquals(3, get_max_courses_in_category());

$category1 = $this->getDataGenerator()->create_category();

// There is only one course, no debug message.
$this->getDataGenerator()->create_course(['category' => $category1->id]);
$this->assertDebuggingNotCalled();
// There are two courses, no debug message.
$this->getDataGenerator()->create_course(['category' => $category1->id]);
$this->assertDebuggingNotCalled();
// There is debug message when number of courses reaches the maximum number.
$this->getDataGenerator()->create_course(['category' => $category1->id]);
$this->assertDebuggingCalled("The number of courses (category id: $category1->id) has reached max number of courses " .
"in a category (" . get_max_courses_in_category() . "). It will cause a sorting performance issue. " .
"Please set higher value for \$CFG->maxcoursesincategory in config.php. " .
"Please also make sure \$CFG->maxcoursesincategory * MAX_COURSE_CATEGORIES less than max integer. " .
"See tracker issues: MDL-25669 and MDL-69573");
}
}

0 comments on commit 10db3a0

Please sign in to comment.