- Patch #396284 by Berdir, chx, jcfiala, csevb10 et al: make sure to order by...

- Patch #396284 by Berdir, chx, jcfiala, csevb10 et al: make sure to order by table headers first, before ordering by other fields.

includes/tablesort.inc

@@ -23,8 +23,13 @@ class TableSort extends SelectQueryExtender {
   protected $header = array();
 
   public function execute() {
+    return $this->query->execute();
+  }
+
+  public function orderByHeader(Array $header) {
+    $this->header = $header;
     $ts = $this->init();
-    if ($ts['sql']) {
+    if (!empty($ts['sql'])) {
       // Based on code from db_escape_table(), but this can also contain a dot.
       $field = preg_replace('/[^A-Za-z0-9_.]+/', '', $ts['sql']);
 
@@ -33,11 +38,6 @@ public function execute() {
       $sort = in_array($sort, array('ASC', 'DESC')) ? $sort : '';
       $this->orderBy($field, $sort);
     }
-    return $this->query->execute();
-  }
-
-  public function setHeader(Array $header) {
-    $this->header = $header;
     return $this;
   }
 

modules/comment/comment.admin.inc

@@ -77,7 +77,7 @@ function comment_admin_overview($type = 'new', $arg) {
     ->condition('c.status', $status)
     ->extend('PagerDefault')->extend('TableSort')
     ->limit(50)
-    ->setHeader($header);
+    ->orderByHeader($header);
   $result = $query->execute();
 
 

modules/dblog/dblog.admin.inc

@@ -65,20 +65,18 @@ function dblog_overview() {
     ->fields('w', array('wid', 'uid', 'severity', 'type', 'timestamp', 'message', 'variables', 'link'))
     ->addField('u', 'name');
   if (!empty($filter['where'])) {
-    //setHeader may not be chainable see Line 138
     $query
       ->where($filter['where'], $filter['args'])
       ->extend('PagerDefault')->extend('TableSort')
       ->limit(50, 0)
-      ->setHeader($header);
+      ->orderByHeader($header);
     $result = $query->execute();
   }
   else {
-    //setHeader may not be chainable see Line 138
     $query
       ->extend('PagerDefault')->extend('TableSort')
       ->limit(50)
-      ->setHeader($header);
+      ->orderByHeader($header);
     $result = $query->execute();
   }
 
@@ -131,7 +129,7 @@ function dblog_top($type) {
     ->groupBy('variables')
     ->extend('PagerDefault')->extend('TableSort')
     ->limit(30);
-  $query = $query->setHeader($header);
+  $query = $query->orderByHeader($header);
   $query->setCountQuery($count_query);
   $result = $query->execute();
 

modules/path/path.admin.inc

@@ -34,7 +34,7 @@ function path_admin_overview($keys = NULL) {
   }
   $result = $query
     ->fields('url_alias')
-    ->setHeader($header)
+    ->orderByHeader($header)
     ->limit(50)
     ->execute();
 

modules/poll/poll.pages.inc

@@ -68,7 +68,7 @@ function poll_votes($node) {
     ->extend('PagerDefault')
     ->limit($votes_per_page)
     ->extend('TableSort')
-    ->setHeader($header)
+    ->orderByHeader($header)
     ->execute()
     ->fetchAllAssoc('hostname');
 

modules/simpletest/tests/database_test.module

@@ -60,6 +60,10 @@ function database_test_menu() {
     'access callback' => TRUE,
     'page callback' => 'database_test_tablesort',
   );
+  $items['database_test/tablesort_first'] = array(
+    'access callback' => TRUE,
+    'page callback' => 'database_test_tablesort_first',
+  );
 
   return $items;
 }
@@ -146,7 +150,36 @@ function database_test_tablesort() {
   $query
     ->fields('t', array('tid', 'pid', 'task', 'priority'));
   
-  $query = $query->extend('TableSort')->setHeader($header);
+  $query = $query->extend('TableSort')->orderByHeader($header);
+
+  // We need all the results at once to check the sort.
+  $tasks = $query->execute()->fetchAll();
+
+  drupal_json(array(
+    'tasks' => $tasks,
+  ));
+  exit;
+}
+
+/**
+ * Run a tablesort query with a second order_by after and return the results.
+ *
+ * This function does care about the page GET parameter, as set by the
+ * simpletest HTTP call.
+ */
+function database_test_tablesort_first() {
+  $header = array(
+    'tid' => array('data' => t('Task ID'), 'field' => 'tid', 'sort' => 'desc'),
+    'pid' => array('data' => t('Person ID'), 'field' => 'pid'),
+    'task' => array('data' => t('Task'), 'field' => 'task'),
+    'priority' => array('data' => t('Priority'), 'field' => 'priority', ),
+  );
+
+  $query = db_select('test_task', 't');
+  $query
+    ->fields('t', array('tid', 'pid', 'task', 'priority'));
+  
+  $query = $query->extend('TableSort')->orderByHeader($header)->orderBy('priority');
 
   // We need all the results at once to check the sort.
   $tasks = $query->execute()->fetchAll();

modules/simpletest/tests/database_test.test

@@ -1769,6 +1769,32 @@ class DatabaseSelectTableSortDefaultTestCase extends DatabaseTestCase {
       $this->assertEqual($last->task, $sort['last'], t('Items appear in the correct order.'));
     }
   }
+  
+  /**
+   * Confirm that if a tablesort's orderByHeader is called before another orderBy, that the header happens first.
+   *
+   */
+  function testTableSortQueryFirst() {
+    $sorts = array(
+      array('field' => t('Task ID'), 'sort' => 'desc', 'first' => 'perform at superbowl', 'last' => 'eat'),
+      array('field' => t('Task ID'), 'sort' => 'asc', 'first' => 'eat', 'last' => 'perform at superbowl'),
+      array('field' => t('Task'), 'sort' => 'asc', 'first' => 'code', 'last' => 'sleep'),
+      array('field' => t('Task'), 'sort' => 'desc', 'first' => 'sleep', 'last' => 'code'),
+      // more elements here
+
+    );
+
+    foreach ($sorts as $sort) {
+      $this->drupalGet('database_test/tablesort_first/', array('query' => array('order' => $sort['field'], 'sort' => $sort['sort'])));
+      $data = json_decode($this->drupalGetContent());
+
+      $first = array_shift($data->tasks);
+      $last = array_pop($data->tasks);
+
+      $this->assertEqual($first->task, $sort['first'], t('Items appear in the correct order sorting by @field @sort.', array('@field' => $sort['field'], '@sort' => $sort['sort'])));
+      $this->assertEqual($last->task, $sort['last'], t('Items appear in the correct order sorting by @field @sort.', array('@field' => $sort['field'], '@sort' => $sort['sort'])));
+    }
+  }
 }
 
 /**

modules/statistics/statistics.admin.inc

@@ -23,7 +23,7 @@ function statistics_recent_hits() {
     ->fields('a', array('aid', 'timestamp', 'path', 'title', 'uid'))
     ->fields('u', array('name'))
     ->limit(30)
-    ->setHeader($header);
+    ->orderByHeader($header);
 
   $result = $query->execute();
   $rows = array();
@@ -66,7 +66,7 @@ function statistics_top_pages() {
     ->fields('accesslog', array('path'))
     ->groupBy('path')
     ->limit(30)
-    ->setHeader($header);
+    ->orderByHeader($header);
 
   $count_query = db_select('accesslog');
   $count_query->addExpression('COUNT(DISTINCT path)');
@@ -114,7 +114,7 @@ function statistics_top_visitors() {
     ->groupBy('u.name')
     ->groupBy('bl.iid')
     ->limit(30)
-    ->setHeader($header);
+    ->orderByHeader($header);
 
   $count_query = db_select('accesslog');
   $count_query->addExpression('COUNT(DISTINCT CONCAT(CAST(uid AS char), hostname))');
@@ -159,7 +159,7 @@ function statistics_top_referrers() {
     ->condition('url', '', '<>')
     ->groupBy('url')
     ->limit(30)
-    ->setHeader($header);
+    ->orderByHeader($header);
 
   $count_query = db_select('accesslog');
   $count_query->addExpression('COUNT(DISTINCT url)');

modules/statistics/statistics.pages.inc

@@ -25,7 +25,7 @@ function statistics_node_tracker() {
         ->condition('a.path', 'node/' . $node->nid)
         ->condition('a.path', 'node/' . $node->nid . '/%', 'LIKE'))
       ->limit(30)
-      ->setHeader($header);
+      ->orderByHeader($header);
 
     $result = $query->execute();
     $rows = array();
@@ -64,7 +64,7 @@ function statistics_user_tracker() {
       ->fields('a', array('aid', 'timestamp', 'path', 'title'))
       ->condition('uid', $account->uid)
       ->limit(30)
-      ->setHeader($header);
+      ->orderByHeader($header);
 
     $result = $query->execute();
     $rows = array();

modules/system/system.module

@@ -1713,7 +1713,7 @@ function system_actions_manage() {
   $result = $query
     ->fields('actions')
     ->limit(50)
-    ->setHeader($header)
+    ->orderByHeader($header)
     ->execute();
 
   foreach ($result as $action) {

modules/tracker/tracker.pages.inc

@@ -50,7 +50,7 @@ function tracker_page($account = NULL, $set_title = FALSE) {
   $query->distinct();
   $query->extend('PagerDefault')->extend('TableSort')
     ->limit(25, 0)
-    ->setHeader($header);
+    ->orderByHeader($header);
 
   $result = $query->execute();
 

modules/user/user.admin.inc

@@ -155,7 +155,7 @@ function user_admin_account() {
   $query
     ->fields('u', array('uid', 'name', 'status', 'created', 'access'))
     ->limit(50)
-    ->setHeader($header)
+    ->orderByHeader($header)
     ->setCountQuery($count_query);
   $result = $query->execute();