Merge branch 'w17_MDL-45214_m27_idsortlog' of https://github.com/skod…

…ak/moodle
hello-josh · Apr 23, 2014 · ecb98f6 · ecb98f6
2 parents 91a73fd + a8d7d61
commit ecb98f6
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 2 deletions.
diff --git a/admin/tool/log/classes/helper/reader.php b/admin/tool/log/classes/helper/reader.php
@@ -61,4 +61,26 @@ public function get_description() {
         }
         return $this->store;
     }
+
+    /**
+     * Adds ID column to $sort to make sure events from one request
+     * within 1 second are returned in the same order.
+     *
+     * @param string $sort
+     * @return string sort string
+     */
+    protected static function tweak_sort_by_id($sort) {
+        if (empty($sort)) {
+            // Mysql does this - unlikely to be used in real life because $sort is always expected.
+            $sort = "id ASC";
+        } else if (stripos($sort, 'timecreated') === false) {
+            $sort .= ", id ASC";
+        } else if (stripos($sort, 'timecreated DESC') !== false) {
+            $sort .= ", id DESC";
+        } else {
+            $sort .= ", id ASC";
+        }
+
+        return $sort;
+    }
 }
diff --git a/admin/tool/log/store/database/classes/log/store.php b/admin/tool/log/store/database/classes/log/store.php
@@ -167,6 +167,8 @@ public function get_events_select($selectwhere, array $params, $sort, $limitfrom
             return array();
         }
 
+        $sort = self::tweak_sort_by_id($sort);
+
         $events = array();
         $records = $this->extdb->get_records_select($dbtable, $selectwhere, $params, $sort, '*', $limitfrom, $limitnum);
 

diff --git a/admin/tool/log/store/database/tests/store_test.php b/admin/tool/log/store/database/tests/store_test.php
@@ -152,7 +152,7 @@ public function test_log_writing() {
 
         // Test reading.
         $this->assertSame(3, $store->get_events_select_count('', array()));
-        $events = $store->get_events_select('', array(), 'id', 0, 0);
+        $events = $store->get_events_select('', array(), 'timecreated ASC', 0, 0); // Is actually sorted by "timecreated ASC, id ASC".
         $this->assertCount(3, $events);
         $resev1 = array_shift($events);
         array_shift($events);

diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php
@@ -86,6 +86,8 @@ protected static function replace_sql_legacy($selectwhere, array $params, $sort
     public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) {
         global $DB;
 
+        $sort = self::tweak_sort_by_id($sort);
+
         // Replace the query with hardcoded mappings required for core.
         list($selectwhere, $params, $sort) = self::replace_sql_legacy($selectwhere, $params, $sort);
 

diff --git a/admin/tool/log/store/standard/classes/log/store.php b/admin/tool/log/store/standard/classes/log/store.php
@@ -69,6 +69,8 @@ protected function insert_event_entries($evententries) {
     public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) {
         global $DB;
 
+        $sort = self::tweak_sort_by_id($sort);
+
         $events = array();
         $records = $DB->get_records_select('logstore_standard_log', $selectwhere, $params, $sort, '*', $limitfrom, $limitnum);
 

diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php
@@ -122,7 +122,7 @@ public function test_log_writing() {
 
         // Test reading.
         $this->assertSame(3, $store->get_events_select_count('', array()));
-        $events = $store->get_events_select('', array(), 'id', 0, 0);
+        $events = $store->get_events_select('', array(), 'timecreated ASC', 0, 0); // Is actually sorted by "timecreated ASC, id ASC".
         $this->assertCount(3, $events);
         $resev1 = array_shift($events);
         array_shift($events);