diff --git a/includes/common.inc b/includes/common.inc index 1e287f8..824b66a 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -4901,7 +4901,6 @@ function _drupal_bootstrap_full() { require_once DRUPAL_ROOT . '/includes/theme.inc'; require_once DRUPAL_ROOT . '/includes/pager.inc'; require_once DRUPAL_ROOT . '/' . variable_get('menu_inc', 'includes/menu.inc'); - require_once DRUPAL_ROOT . '/includes/tablesort.inc'; require_once DRUPAL_ROOT . '/includes/file.inc'; require_once DRUPAL_ROOT . '/includes/unicode.inc'; require_once DRUPAL_ROOT . '/includes/image.inc'; diff --git a/includes/tablesort.inc b/includes/tablesort.inc index 121a1b9..59b3d51 100644 --- a/includes/tablesort.inc +++ b/includes/tablesort.inc @@ -15,7 +15,7 @@ class TableSort extends SelectQueryExtender { /** - * The array of fields that can be sorted by. + * An array of table header columns that can be sorted by, as described in theme_table(). * * @var array */ @@ -33,23 +33,22 @@ class TableSort extends SelectQueryExtender { /** * Order the query based on a header array. * - * @see theme_table() * @param $header - * Table header array. + * An array of table header columns as described in theme_table(). + * * @return SelectQueryInterface * The called object. + * + * @see theme_table() */ - public function orderByHeader(Array $header) { + public function orderByHeader(array $header) { $this->header = $header; $ts = $this->init(); - 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']); - + if (!empty($ts['field'])) { // Sort order can only be ASC or DESC. $sort = drupal_strtoupper($ts['sort']); $sort = in_array($sort, array('ASC', 'DESC')) ? $sort : ''; - $this->orderBy($field, $sort); + $this->orderBy($ts['field'], $sort); } return $this; } @@ -68,7 +67,7 @@ class TableSort extends SelectQueryExtender { * Determine the current sort direction. * * @param $headers - * An array of column headers in the format described in theme_table(). + * An array of table header columns as described in theme_table(). * @return * The current sort direction ("asc" or "desc"). */ @@ -92,12 +91,10 @@ class TableSort extends SelectQueryExtender { /** * Determine the current sort criterion. * - * @param $headers - * An array of column headers in the format described in theme_table(). * @return * An associative array describing the criterion, containing the keys: * - "name": The localized title of the table column. - * - "sql": The name of the database field to sort on. + * - "field": The name of the database field to sort on. */ protected function order() { return tablesort_get_order($this->header); @@ -123,7 +120,7 @@ function tablesort_init($header) { * @param $cell * The cell to format. * @param $header - * An array of column headers in the format described in theme_table(). + * An array of table header columns as described in theme_table(). * @param $ts * The current table sort context as returned from tablesort_init(). * @return @@ -133,7 +130,7 @@ function tablesort_header($cell, $header, $ts) { // Special formatting for the currently sorted column header. if (is_array($cell) && isset($cell['field'])) { $title = t('sort by @s', array('@s' => $cell['data'])); - if ($cell['data'] == $ts['name']) { + if ($cell['field'] == $ts['field']) { $ts['sort'] = (($ts['sort'] == 'asc') ? 'desc' : 'asc'); $cell['class'][] = 'active'; $image = theme('tablesort_indicator', array('style' => $ts['sort'])); @@ -143,7 +140,12 @@ function tablesort_header($cell, $header, $ts) { $ts['sort'] = 'asc'; $image = ''; } - $cell['data'] = l($cell['data'] . $image, $_GET['q'], array('attributes' => array('title' => $title), 'query' => array_merge($ts['query'], array('sort' => $ts['sort'], 'order' => $cell['data'])), 'html' => TRUE)); + $options = array( + 'attributes' => array('title' => $title), + 'query' => array_merge($ts['query'], array('sort' => $ts['sort'], 'order' => $cell['field'])), + 'html' => TRUE, + ); + $cell['data'] = l($cell['data'] . $image, $_GET['q'], $options); unset($cell['field'], $cell['sort']); } @@ -158,7 +160,7 @@ function tablesort_header($cell, $header, $ts) { * @param $cell * The cell to format. * @param $header - * An array of column headers in the format described in theme_table(). + * An array of table header columns as described in theme_table(). * @param $ts * The current table sort context as returned from tablesort_init(). * @param $i @@ -167,7 +169,7 @@ function tablesort_header($cell, $header, $ts) { * A properly formatted cell, ready for _theme_table_cell(). */ function tablesort_cell($cell, $header, $ts, $i) { - if (isset($header[$i]['data']) && $header[$i]['data'] == $ts['name'] && !empty($header[$i]['field'])) { + if (isset($header[$i]['field']) && $header[$i]['field'] == $ts['field']) { if (is_array($cell)) { $cell['class'][] = 'active'; } @@ -192,37 +194,47 @@ function tablesort_get_query_parameters() { /** * Determine the current sort criterion. * - * @param $headers - * An array of column headers in the format described in theme_table(). + * @param $header + * An array of table headers columns as described in theme_table(). * @return * An associative array describing the criterion, containing the keys: * - "name": The localized title of the table column. - * - "sql": The name of the database field to sort on. + * - "field": The name of the database field to sort on. */ -function tablesort_get_order($headers) { +function tablesort_get_order($header) { $order = isset($_GET['order']) ? $_GET['order'] : ''; - foreach ($headers as $header) { - if (is_array($header)) { - if (isset($header['data']) && $order == $header['data']) { - $default = $header; - break; - } - - if (empty($default) && isset($header['sort']) && ($header['sort'] == 'asc' || $header['sort'] == 'desc')) { - $default = $header; - } + foreach ($header as $column) { + if (!is_array($column) || !isset($column['field'])) { + continue; } - } - if (!isset($default)) { - $default = reset($headers); - if (!is_array($default)) { - $default = array('data' => $default); + // Use the header column matching the URL parameter. + if ($order == $column['field']) { + return array('name' => $column['data'], 'field' => $column['field']); + } + // In case no header column will match the URL parameter, and this column + // defines 'sort', it is supposed to be the default sorting column. + if (isset($column['sort']) && ($column['sort'] == 'asc' || $column['sort'] == 'desc')) { + $default_column = array('name' => $column['data'], 'field' => $column['field']); + } + // In case there is no default sorting header column, store the first that + // defines a field. + elseif (!isset($first_column)) { + $first_column = array('name' => $column['data'], 'field' => $column['field']); } } - $default += array('data' => NULL, 'field' => NULL); - return array('name' => $default['data'], 'sql' => $default['field']); + // If there was a default sorting column, return that. + if (isset($default_column)) { + return $default_column; + } + // Otherwise, use the first header column that defines a field. + if (isset($first_column)) { + return $first_column; + } + // If we end up here, then a table header did not define any valid tablesort + // data. + throw new Exception(t('Invalid TableSort data; header needs to define at least one sorting field.')); } /** diff --git a/includes/theme.inc b/includes/theme.inc index 6c2b640..590c4ef 100644 --- a/includes/theme.inc +++ b/includes/theme.inc @@ -1688,22 +1688,28 @@ function theme_table($variables) { $rows[] = array(array('data' => $empty, 'colspan' => $header_count, 'class' => array('empty', 'message'))); } - // Format the table header: + // Format the table header. if (count($header)) { - $ts = tablesort_init($header); + // Determine whether we need to load tablesort. + foreach ($header as $column) { + if (is_array($column) && isset($column['field'])) { + require_once DRUPAL_ROOT . '/includes/tablesort.inc'; + $ts = tablesort_init($header); + break; + } + } // HTML requires that the thead tag has tr tags in it followed by tbody // tags. Using ternary operator to check and see if we have any rows. $output .= (count($rows) ? ' ' : ' '); foreach ($header as $cell) { - $cell = tablesort_header($cell, $header, $ts); + if (isset($ts)) { + $cell = tablesort_header($cell, $header, $ts); + } $output .= _theme_table_cell($cell, TRUE); } // Using ternary operator to close the tags based on whether or not there are rows $output .= (count($rows) ? " \n" : "\n"); } - else { - $ts = array(); - } // Format the table rows: if (count($rows)) { @@ -1738,7 +1744,9 @@ function theme_table($variables) { $output .= ' '; $i = 0; foreach ($cells as $cell) { - $cell = tablesort_cell($cell, $header, $ts, $i++); + if (isset($ts)) { + $cell = tablesort_cell($cell, $header, $ts, $i++); + } $output .= _theme_table_cell($cell); } $output .= " \n"; diff --git a/modules/forum/forum.module b/modules/forum/forum.module index f2ac5ac..f4652d3 100644 --- a/modules/forum/forum.module +++ b/modules/forum/forum.module @@ -58,7 +58,7 @@ function forum_theme() { return array( 'forums' => array( 'template' => 'forums', - 'variables' => array('forums' => NULL, 'topics' => NULL, 'parents' => NULL, 'tid' => NULL, 'sortby' => NULL, 'forum_per_page' => NULL), + 'variables' => array('forums' => NULL, 'topics' => array(), 'parents' => NULL, 'tid' => NULL, 'header' => array(), 'forum_per_page' => NULL), ), 'forum_list' => array( 'template' => 'forum-list', @@ -66,7 +66,7 @@ function forum_theme() { ), 'forum_topic_list' => array( 'template' => 'forum-topic-list', - 'variables' => array('tid' => NULL, 'topics' => NULL, 'sortby' => NULL, 'forum_per_page' => NULL), + 'variables' => array('tid' => NULL, 'topics' => array(), 'header' => array(), 'forum_per_page' => NULL), ), 'forum_icon' => array( 'template' => 'forum-icon', @@ -852,22 +852,8 @@ function _forum_topics_unread($term, $uid) { ->fetchField(); } -function forum_get_topics($tid, $sortby, $forum_per_page) { - global $user, $forum_topic_list_header; - - $forum_topic_list_header = array( - NULL, - array('data' => t('Topic'), 'field' => 'f.title'), - array('data' => t('Replies'), 'field' => 'f.comment_count'), - array('data' => t('Last reply'), 'field' => 'f.last_comment_timestamp'), - ); - - $order = _forum_get_topic_order($sortby); - for ($i = 0; $i < count($forum_topic_list_header); $i++) { - if ($forum_topic_list_header[$i]['field'] == $order['field']) { - $forum_topic_list_header[$i]['sort'] = $order['sort']; - } - } +function forum_get_topics($tid, $header, $forum_per_page) { + global $user; $query = db_select('forum_index', 'f')->extend('PagerDefault')->extend('TableSort'); $query->fields('f'); @@ -875,7 +861,7 @@ function forum_get_topics($tid, $sortby, $forum_per_page) { ->condition('f.tid', $tid) ->addTag('node_access') ->orderBy('f.sticky', 'DESC') - ->orderByHeader($forum_topic_list_header) + ->orderByHeader($header) ->limit($forum_per_page); $count_query = db_select('forum_index', 'f'); @@ -966,7 +952,7 @@ function forum_get_topics($tid, $sortby, $forum_per_page) { * - $topics * - $parents * - $tid - * - $sortby + * - $header * - $forum_per_page * * @see forums.tpl.php @@ -1085,20 +1071,20 @@ function template_preprocess_forum_list(&$variables) { * $variables contains the following data: * - $tid * - $topics - * - $sortby + * - $header * - $forum_per_page * * @see forum-topic-list.tpl.php * @see theme_forum_topic_list() */ function template_preprocess_forum_topic_list(&$variables) { - global $forum_topic_list_header; - - // Create the tablesorting header. - $ts = tablesort_init($forum_topic_list_header); + // Render the table header; this is required, because the forum topic list + // table does not use theme_table(). + require_once DRUPAL_ROOT . '/includes/tablesort.inc'; + $ts = tablesort_init($variables['header']); $header = ''; - foreach ($forum_topic_list_header as $cell) { - $cell = tablesort_header($cell, $forum_topic_list_header, $ts); + foreach ($variables['header'] as $cell) { + $cell = tablesort_header($cell, $variables['header'], $ts); $header .= _theme_table_cell($cell, TRUE); } $variables['header'] = $header; @@ -1136,10 +1122,6 @@ function template_preprocess_forum_topic_list(&$variables) { } } - else { - // Make this safe for the template - $variables['topics'] = array(); - } // Give meaning to $tid for themers. $tid actually stands for term id. $variables['topic_id'] = $variables['tid']; unset($variables['tid']); @@ -1212,16 +1194,15 @@ function _forum_get_topic_order($sortby) { switch ($sortby) { case 1: return array('field' => 'f.last_comment_timestamp', 'sort' => 'desc'); - break; + case 2: return array('field' => 'f.last_comment_timestamp', 'sort' => 'asc'); - break; + case 3: return array('field' => 'f.comment_count', 'sort' => 'desc'); - break; + case 4: return array('field' => 'f.comment_count', 'sort' => 'asc'); - break; } } diff --git a/modules/forum/forum.pages.inc b/modules/forum/forum.pages.inc index 29307e7..50d086a 100644 --- a/modules/forum/forum.pages.inc +++ b/modules/forum/forum.pages.inc @@ -15,14 +15,26 @@ function forum_page($forum_term = NULL) { } $forum_per_page = variable_get('forum_per_page', 25); - $sortby = variable_get('forum_order', 1); + + $header = array( + NULL, + array('data' => t('Topic'), 'field' => 'f.title'), + array('data' => t('Replies'), 'field' => 'f.comment_count'), + array('data' => t('Last reply'), 'field' => 'f.last_comment_timestamp'), + ); + $order = _forum_get_topic_order(variable_get('forum_order', 1)); + for ($i = 0; $i < count($header); $i++) { + if ($header[$i]['field'] == $order['field']) { + $header[$i]['sort'] = $order['sort']; + } + } if (empty($forum_term->container)) { - $topics = forum_get_topics($forum_term->tid, $sortby, $forum_per_page); + $topics = forum_get_topics($forum_term->tid, $header, $forum_per_page); } else { - $topics = ''; + $topics = array(); } - return theme('forums', array('forums' => $forum_term->forums, 'topics' => $topics, 'parents' => $forum_term->parents, 'tid' => $forum_term->tid, 'sortby' => $sortby, 'forums_per_page' => $forum_per_page)); + return theme('forums', array('forums' => $forum_term->forums, 'topics' => $topics, 'parents' => $forum_term->parents, 'tid' => $forum_term->tid, 'header' => $header, 'forums_per_page' => $forum_per_page)); } diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test index 76ca103..7e7df7a 100644 --- a/modules/simpletest/tests/database_test.test +++ b/modules/simpletest/tests/database_test.test @@ -2420,10 +2420,10 @@ class DatabaseSelectTableSortDefaultTestCase extends DatabaseTestCase { */ function testTableSortQuery() { $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'), + array('data' => t('Task ID'), 'field' => 'tid', 'sort' => 'desc', 'first' => 'perform at superbowl', 'last' => 'eat'), + array('data' => t('Task ID'), 'field' => 'tid', 'sort' => 'asc', 'first' => 'eat', 'last' => 'perform at superbowl'), + array('data' => t('Task'), 'field' => 'task', 'sort' => 'asc', 'first' => 'code', 'last' => 'sleep'), + array('data' => t('Task'), 'field' => 'task', 'sort' => 'desc', 'first' => 'sleep', 'last' => 'code'), // more elements here ); @@ -2446,10 +2446,10 @@ class DatabaseSelectTableSortDefaultTestCase extends DatabaseTestCase { */ 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'), + array('data' => t('Task ID'), 'field' => 'tid', 'sort' => 'desc', 'first' => 'perform at superbowl', 'last' => 'eat'), + array('data' => t('Task ID'), 'field' => 'tid', 'sort' => 'asc', 'first' => 'eat', 'last' => 'perform at superbowl'), + array('data' => t('Task'), 'field' => 'task', 'sort' => 'asc', 'first' => 'code', 'last' => 'sleep'), + array('data' => t('Task'), 'field' => 'task', 'sort' => 'desc', 'first' => 'sleep', 'last' => 'code'), // more elements here ); diff --git a/modules/simpletest/tests/tablesort.test b/modules/simpletest/tests/tablesort.test index 9c068f8..6dcce8c 100644 --- a/modules/simpletest/tests/tablesort.test +++ b/modules/simpletest/tests/tablesort.test @@ -45,6 +45,7 @@ class TableSortTest extends DrupalUnitTestCase { function testTableSortInit() { // Test simple table headers. + require_once DRUPAL_ROOT . '/includes/tablesort.inc'; $headers = array('foo', 'bar', 'baz'); // Reset $_GET to prevent parameters from Simpletest and Batch API ending