From ebc09cc3ad7574bc633f1ea04056b6a3ceb6256f Mon Sep 17 00:00:00 2001 From: Rajesh Taneja Date: Wed, 15 Jan 2014 09:49:34 +0800 Subject: [PATCH] MDL-43306 restore: Fix grade_item sortorder after restoring course While restoring course/activity, restore will blindly insert grade_item sortorder. Which cause dulicate sortorder and lead to unpredicatble sorting results. Now we will call grade_item::fix_duplicate_sortorder() after restore is finished to fix duplicate sortorder and order grade items to the closest order possible --- backup/moodle2/restore_stepslib.php | 6 ++ lib/grade/grade_item.php | 36 ++++++++++ lib/grade/tests/grade_item_test.php | 103 ++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index a4e42ef0bb7..4ef4d08f52d 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -2832,6 +2832,12 @@ class restore_activity_grades_structure_step extends restore_structure_step { } // no need to save any grade_letter mapping } + + public function after_restore() { + // Fix grade item's sortorder after restore, as it might have duplicates. + $courseid = $this->get_task()->get_courseid(); + grade_item::fix_duplicate_sortorder($courseid); + } } diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index 18d1180f75e..4a0aa6ef932 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -1209,6 +1209,42 @@ class grade_item extends grade_object { $this->set_sortorder($sortorder + 1); } + /** + * Detect duplicate grade item's sortorder and re-sort them. + * Note: Duplicate sortorder will be introduced while duplicating activities or + * merging two courses. + * + * @param int $courseid id of the course for which grade_items sortorder need to be fixed. + */ + public static function fix_duplicate_sortorder($courseid) { + global $DB; + + $transaction = $DB->start_delegated_transaction(); + + $sql = "SELECT g1.id, g1.courseid, g1.sortorder + FROM {grade_items} g1 + JOIN {grade_items} g2 ON g1.courseid = g2.courseid + WHERE g1.sortorder = g2.sortorder AND g1.id != g2.id AND g1.courseid = :courseid + ORDER BY g1.sortorder DESC, g1.id DESC"; + + // Get all duplicates in course highest sort order, and higest id first so that we can make space at the + // bottom higher end of the sort orders and work down by id. + $rs = $DB->get_recordset_sql($sql, array('courseid' => $courseid)); + + foreach($rs as $duplicate) { + $DB->execute("UPDATE {grade_items} + SET sortorder = sortorder + 1 + WHERE courseid = :courseid AND + (sortorder > :sortorder OR (sortorder = :sortorder2 AND id > :id))", + array('courseid' => $duplicate->courseid, + 'sortorder' => $duplicate->sortorder, + 'sortorder2' => $duplicate->sortorder, + 'id' => $duplicate->id)); + } + $rs->close(); + $transaction->allow_commit(); + } + /** * Returns the most descriptive field for this object. * diff --git a/lib/grade/tests/grade_item_test.php b/lib/grade/tests/grade_item_test.php index 7b29077bf57..34ba416ef72 100644 --- a/lib/grade/tests/grade_item_test.php +++ b/lib/grade/tests/grade_item_test.php @@ -65,6 +65,7 @@ class core_grade_item_testcase extends grade_base_testcase { $this->sub_test_grade_item_compute(); $this->sub_test_update_final_grade(); $this->sub_test_grade_item_can_control_visibility(); + $this->sub_test_grade_item_fix_sortorder(); } protected function sub_test_grade_item_construct() { @@ -630,4 +631,106 @@ class core_grade_item_testcase extends grade_base_testcase { $grade_item = new grade_item($this->grade_items[11], false); $this->assertFalse($grade_item->can_control_visibility()); } + + /** + * Test the {@link grade_item::fix_duplicate_sortorder() function with + * faked duplicate sortorder data. + */ + public function sub_test_grade_item_fix_sortorder() { + global $DB; + + $this->resetAfterTest(true); + + // Each set is used for filling the db with fake data and will be representing the result of query: + // "SELECT sortorder from {grade_items} WHERE courseid=? ORDER BY id". + $testsets = array( + // Items that need no action. + array(1,2,3), + array(5,6,7), + array(7,6,1,3,2,5), + // Items with sortorder duplicates + array(1,2,2,3,3,4,5), + // Only one sortorder duplicate. + array(1,1), + array(3,3), + // Non-sequential sortorders with one or multiple duplicates. + array(3,3,7,5,6,6,9,10,8,3), + array(7,7,3), + array(3,4,5,3,5,4,7,1) + ); + $origsequence = array(); + + // Generate the data and remember the initial sequence or items. + foreach ($testsets as $testset) { + $course = $this->getDataGenerator()->create_course(); + foreach ($testset as $sortorder) { + $this->insert_fake_grade_item_sortorder($course->id, $sortorder); + } + $DB->get_records('grade_items'); + $origsequence[$course->id] = $DB->get_fieldset_sql("SELECT id FROM {grade_items} ". + "WHERE courseid = ? ORDER BY sortorder, id", array($course->id)); + } + + $duplicatedetectionsql = "SELECT courseid, sortorder + FROM {grade_items} + WHERE courseid = :courseid + GROUP BY courseid, sortorder + HAVING COUNT(id) > 1"; + + // Do the work. + foreach ($origsequence as $courseid => $ignore) { + grade_item::fix_duplicate_sortorder($courseid); + // Verify that no duplicates are left in the database. + $dupes = $DB->record_exists_sql($duplicatedetectionsql, array('courseid' => $courseid)); + $this->assertFalse($dupes); + } + + // Verify that sequences are exactly the same as they were before upgrade script. + $idx = 0; + foreach ($origsequence as $courseid => $sequence) { + if (count(($testsets[$idx])) == count(array_unique($testsets[$idx]))) { + // If there were no duplicates for this course verify that sortorders are not modified. + $newsortorders = $DB->get_fieldset_sql("SELECT sortorder from {grade_items} WHERE courseid=? ORDER BY id", array($courseid)); + $this->assertEquals($testsets[$idx], $newsortorders); + } + $newsequence = $DB->get_fieldset_sql("SELECT id FROM {grade_items} ". + "WHERE courseid = ? ORDER BY sortorder, id", array($courseid)); + $this->assertEquals($sequence, $newsequence, + "Sequences do not match for test set $idx : ".join(',', $testsets[$idx])); + $idx++; + } + } + + /** + * Populate some fake grade items into the database with specified + * sortorder and course id. + * + * NOTE: This function doesn't make much attempt to respect the + * gradebook internals, its simply used to fake some data for + * testing the upgradelib function. Please don't use it for other + * purposes. + * + * @param int $courseid id of course + * @param int $sortorder numeric sorting order of item + * @return stdClass grade item object from the database. + */ + private function insert_fake_grade_item_sortorder($courseid, $sortorder) { + global $DB, $CFG; + require_once($CFG->libdir.'/gradelib.php'); + + $item = new stdClass(); + $item->courseid = $courseid; + $item->sortorder = $sortorder; + $item->gradetype = GRADE_TYPE_VALUE; + $item->grademin = 30; + $item->grademax = 110; + $item->itemnumber = 1; + $item->iteminfo = ''; + $item->timecreated = time(); + $item->timemodified = time(); + + $item->id = $DB->insert_record('grade_items', $item); + + return $DB->get_record('grade_items', array('id' => $item->id)); + } }