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

@@ -2832,6 +2832,12 @@ protected function process_grade_letter($data) {
         }
         // 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);
+    }
 }
 
 

lib/grade/grade_item.php

@@ -1209,6 +1209,42 @@ public function move_after_sortorder($sortorder) {
         $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.
      *

lib/grade/tests/grade_item_test.php

@@ -65,6 +65,7 @@ public function test_grade_item() {
         $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 @@ protected function sub_test_grade_item_can_control_visibility() {
         $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));
+    }
 }