diff options
| -rw-r--r-- | CHANGELOG.txt | 2 | ||||
| -rw-r--r-- | includes/tests/cache.test | 98 | ||||
| -rw-r--r-- | install.php | 4 | ||||
| -rw-r--r-- | modules/comment/comment.install | 30 | ||||
| -rw-r--r-- | modules/comment/comment.module | 101 | ||||
| -rw-r--r-- | modules/comment/comment.test | 4 |
6 files changed, 106 insertions, 133 deletions
diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0e95ceed8..ec3f541e9 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -13,6 +13,8 @@ Drupal 7.0, xxxx-xx-xx (development version) * Implemented drag-and-drop positioning for poll options. * Provided descriptions for user permissions. * Removed comment controls for users. + * Removed display order settings for comment module. Comment display + order can now be customised using the Views module. - Search: * Added support for language-aware searches. - Testing: diff --git a/includes/tests/cache.test b/includes/tests/cache.test index 09a10214b..a6ff1e0b7 100644 --- a/includes/tests/cache.test +++ b/includes/tests/cache.test @@ -3,32 +3,32 @@ class CacheTestCase extends DrupalWebTestCase { protected $default_table = 'cache'; protected $default_cid = 'test_temporary'; protected $default_value = 'CacheTest'; - + /** * Check whether or not a cache entry exists. - * + * * @param $cid * The cache id. * @param $var * The variable the cache should contain. * @param $table * The table the cache item was stored in. - * @return + * @return * TRUE on pass, FALSE on fail. */ protected function checkCacheExists($cid, $var, $table = null) { if ($table == null) { $table = $this->default_table; } - + $cache = cache_get($cid, $table); - + return isset($cache->data) && $cache->data == $var; } /** * Assert or a cache entry exists. - * + * * @param $message * Message to display. * @param $var @@ -54,7 +54,7 @@ class CacheTestCase extends DrupalWebTestCase { /** * Assert or a cache entry has been removed. - * + * * @param $message * Message to display. * @param $cid @@ -69,9 +69,9 @@ class CacheTestCase extends DrupalWebTestCase { if ($cid == NULL) { $cid = $this->default_cid; } - + $cache = cache_get($cid, $table); - $this->assertFalse($cache, $message); + $this->assertFalse($cache, $message); } /** @@ -83,10 +83,10 @@ class CacheTestCase extends DrupalWebTestCase { if ($table == NULL) { $table = $this->default_table; } - - cache_clear_all(NULL, $table); + + cache_clear_all(NULL, $table); } - + /** * Setup the lifetime settings for caching. * @@ -95,8 +95,8 @@ class CacheTestCase extends DrupalWebTestCase { */ protected function setupLifetime($time) { variable_set('cache_lifetime', $time); - variable_set('cache_flush', 0); - } + variable_set('cache_flush', 0); + } } class CacheSavingCase extends CacheTestCase { @@ -147,10 +147,10 @@ class CacheSavingCase extends CacheTestCase { $test_object->test1 = $this->randomName('100'); $test_object->test2 = 100; $test_object->test3 = array('drupal1', 'drupal2' => 'drupal3', 'drupal4' => array('drupal5', 'drupal6')); - + cache_set('test_object', $test_object, 'cache'); $cache = cache_get('test_object', 'cache'); - $this->assertTrue(isset($cache->data) && $cache->data == $test_object, t('Object is saved and restored properly.')); + $this->assertTrue(isset($cache->data) && $cache->data == $test_object, t('Object is saved and restored properly.')); } /* @@ -159,7 +159,7 @@ class CacheSavingCase extends CacheTestCase { function checkVariable($var) { cache_set('test_var', $var, 'cache'); $cache = cache_get('test_var', 'cache'); - $this->assertTrue(isset($cache->data) && $cache->data === $var, t('@type is saved and restored properly.', array('@type' => ucfirst(gettype($var))))); + $this->assertTrue(isset($cache->data) && $cache->data === $var, t('@type is saved and restored properly.', array('@type' => ucfirst(gettype($var))))); } } @@ -181,53 +181,53 @@ class CacheClearCase extends CacheTestCase { function setUp() { $this->default_table = 'cache_page'; $this->default_value = $this->randomName(10); - + parent::setUp(); } - + /** * Test clearing using a cid. */ function testClearCid() { cache_set('test_cid_clear', $this->default_value, $this->default_table); - + $this->assertCacheExists(t('Cache was set for clearing cid.'), $this->default_value, 'test_cid_clear'); cache_clear_all('test_cid_clear', $this->default_table); - + $this->assertCacheRemoved(t('Cache was removed after clearing cid.'), 'test_cid_clear'); - + cache_set('test_cid_clear1', $this->default_value, $this->default_table); cache_set('test_cid_clear2', $this->default_value, $this->default_table); - $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) && $this->checkCacheExists('test_cid_clear2', $this->default_value), - t('Two caches were created for checking cid "*" with wildcard false.')); + t('Two caches were created for checking cid "*" with wildcard false.')); cache_clear_all('*', $this->default_table); - $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) && $this->checkCacheExists('test_cid_clear2', $this->default_value), - t('Two caches still exists after clearing cid "*" with wildcard false.')); + t('Two caches still exists after clearing cid "*" with wildcard false.')); } - + /** * Test clearing using wildcard. */ function testClearWildcard() { cache_set('test_cid_clear1', $this->default_value, $this->default_table); cache_set('test_cid_clear2', $this->default_value, $this->default_table); - $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) && $this->checkCacheExists('test_cid_clear2', $this->default_value), - t('Two caches were created for checking cid "*" with wildcard true.')); + t('Two caches were created for checking cid "*" with wildcard true.')); cache_clear_all('*', $this->default_table, TRUE); - $this->assertFalse($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertFalse($this->checkCacheExists('test_cid_clear1', $this->default_value) || $this->checkCacheExists('test_cid_clear2', $this->default_value), t('Two caches removed after clearing cid "*" with wildcard true.')); - + cache_set('test_cid_clear1', $this->default_value, $this->default_table); cache_set('test_cid_clear2', $this->default_value, $this->default_table); - $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertTrue($this->checkCacheExists('test_cid_clear1', $this->default_value) && $this->checkCacheExists('test_cid_clear2', $this->default_value), - t('Two caches were created for checking cid substring with wildcard true.')); + t('Two caches were created for checking cid substring with wildcard true.')); cache_clear_all('test_', $this->default_table, TRUE); - $this->assertFalse($this->checkCacheExists('test_cid_clear1', $this->default_value) + $this->assertFalse($this->checkCacheExists('test_cid_clear1', $this->default_value) || $this->checkCacheExists('test_cid_clear2', $this->default_value), t('Two caches removed after clearing cid substring with wildcard true.')); } @@ -267,11 +267,11 @@ class CacheExpiryCase extends CacheTestCase { cache_set($this->default_cid, $this->default_value, $this->default_table, CACHE_TEMPORARY); $this->assertCacheExists(t('Temporary cache data without lifetime exists before wipe.')); - + $user->cache = isset($user->cache) ? $user->cache +2 : time() + 2; $this->assertCacheExists(t('Temporary cache without lifetime valid after user cache expiration.')); $user->cache = $user->cache - 2; - + $this->generalWipe(); $this->assertCacheRemoved(t('Temporary cache without lifetime does not exists after wipe.')); } @@ -285,8 +285,8 @@ class CacheExpiryCase extends CacheTestCase { function testTemporaryLifetime() { $this->setupLifetime(5); cache_set($this->default_cid, $this->default_value, $this->default_table, CACHE_TEMPORARY); - - $this->assertCacheExists(t('Temporary cache with lifetime data exists before wipe.')); + + $this->assertCacheExists(t('Temporary cache with lifetime data exists before wipe.')); $user->cache = isset($user->cache) ? $user->cache + 2 : time() + 2; $this->assertCacheExists(t('Temporary cache without lifetime valid after user cache expiration.')); @@ -305,13 +305,13 @@ class CacheExpiryCase extends CacheTestCase { function testPermanentNoLifetime() { $this->setupLifetime(0); cache_set($this->default_cid, $this->default_value, $this->default_table, CACHE_PERMANENT); - + $this->assertCacheExists(t('Permanent cache data without lifetime exists before wipe.')); - + $user->cache = isset($user->cache) ? $user->cache + 2 : time() + 2; $this->assertCacheExists(t('Permanent cache without lifetime valid after user cache expiration.')); $user->cache = $user->cache - 2; - + $this->generalWipe(); $this->assertCacheExists(t('Permanent cache without lifetime exists after wipe.')); } @@ -330,7 +330,7 @@ class CacheExpiryCase extends CacheTestCase { $user->cache = isset($user->cache) ? $user->cache + 2 : time() + 2; $this->assertCacheExists(t('Permanent cache with lifetime valid after user cache expiration.')); - $user->cache = $user->cache - 2; + $user->cache = $user->cache - 2; $this->generalWipe(); $this->assertCacheExists(t('Permanent cache with lifetime exists after wipe.')); @@ -345,18 +345,18 @@ class CacheExpiryCase extends CacheTestCase { function testUnixTimestampNoLifetime() { $this->setupLifetime(0); cache_set($this->default_cid, $this->default_value, $this->default_table, time() + 1); - + $this->assertCacheExists(t('Unit timestamp cache data without lifetime exists before wipe.')); $this->generalWipe(); $this->assertCacheExists(t('Unit timestamp cache without lifetime exists after wipe.')); sleep(2); // expire $this->assertCacheExists(t('Unit timestamp cache without lifetime exists after expiration.')); - + $user->cache = isset($user->cache) ? $user->cache + 2 : time() + 2; $this->assertCacheExists(t('Unit timestamp cache without lifetime valid after user cache expiration.')); $user->cache = $user->cache - 2; - - $this->generalWipe(); + + $this->generalWipe(); $this->assertCacheRemoved(t('Unit timestamp cache without lifetime deleted after expiration and wipe.')); } @@ -370,17 +370,17 @@ class CacheExpiryCase extends CacheTestCase { $this->setupLifetime(5); cache_set($this->default_cid, $this->default_value, $this->default_table, time() + 1); global $user; - + $this->assertCacheExists(t('Unit timestamp cache data without lifetime exists before wipe.')); $this->generalWipe(); $this->assertCacheExists(t('Unit timestamp cache with lifetime exists after wipe.')); sleep(2); // expire $this->assertCacheExists(t('Unit timestamp cache with lifetime exists after expiration.')); - + $user->cache = $user->cache + 2; $this->assertCacheRemoved(t('Unit timestamp cache with lifetime not valid after user cache expiration.')); $user->cache = $user->cache - 2; - + $this->generalWipe(); $this->assertCacheRemoved(t('Unit timestamp cache with lifetime deleted after expiration and wipe.')); } diff --git a/install.php b/install.php index e66ddff86..1ec0eccce 100644 --- a/install.php +++ b/install.php @@ -26,8 +26,8 @@ function install_main() { if (preg_match("/^simpletest\d+$/", $_SERVER['HTTP_USER_AGENT'])) { header('HTTP/1.1 403 Forbidden'); exit; - } - + } + require_once './includes/bootstrap.inc'; drupal_bootstrap(DRUPAL_BOOTSTRAP_CONFIGURATION); diff --git a/modules/comment/comment.install b/modules/comment/comment.install index 598b476bf..cc1ea7353 100644 --- a/modules/comment/comment.install +++ b/modules/comment/comment.install @@ -24,6 +24,11 @@ function comment_update_1() { return array(); } +/** + * @defgroup updates-5.x-to-6.x Comment updates from 5.x to 6.x + * @{ + */ + function comment_update_6001() { $ret[] = update_sql("ALTER TABLE {comments} DROP score"); $ret[] = update_sql("ALTER TABLE {comments} DROP users"); @@ -70,6 +75,31 @@ function comment_update_6003() { return $ret; } +/** + * @} End of "defgroup updates-5.x-to-6.x" + * The next series of updates should start at 7000. + */ + +/** + * @defgroup updates-6.x-to-7.x Comment updates from 6.x to 7.x + * @{ + */ + +/** + * Remove comment settings for page ordering. + */ +function comment_update_7000() { + $types = node_get_types(); + foreach ($types as $type => $object) { + variable_del('comment_default_order' . $type); + } + return array(array('success' => TRUE, 'query' => 'Comment order settings removed.')); +} + +/** + * @} End of "defgroup updates-6.x-to-7.x" + * The next series of updates should start at 8000. + */ /** * Implementation of hook_schema(). diff --git a/modules/comment/comment.module b/modules/comment/comment.module index 67861b078..fd949a2f6 100644 --- a/modules/comment/comment.module +++ b/modules/comment/comment.module @@ -41,16 +41,6 @@ define('COMMENT_MODE_THREADED_COLLAPSED', 3); define('COMMENT_MODE_THREADED_EXPANDED', 4); /** - * Comments are ordered by date - newest first. - */ -define('COMMENT_ORDER_NEWEST_FIRST', 1); - -/** - * Comments are ordered by date - oldest first. - */ -define('COMMENT_ORDER_OLDEST_FIRST', 2); - -/** * Anonymous posters cannot enter their contact information. */ define('COMMENT_ANONYMOUS_MAYNOT_CONTACT', 0); @@ -230,7 +220,6 @@ function comment_node_type($op, $info) { $settings = array( 'comment', 'comment_default_mode', - 'comment_default_order', 'comment_default_per_page', 'comment_anonymous', 'comment_subject_field', @@ -350,35 +339,24 @@ function comment_get_recent($number = 10) { function comment_new_page_count($num_comments, $new_replies, $node) { $comments_per_page = _comment_get_display_setting('comments_per_page', $node); $mode = _comment_get_display_setting('mode', $node); - $order = _comment_get_display_setting('sort', $node); $pagenum = NULL; $flat = in_array($mode, array(COMMENT_MODE_FLAT_COLLAPSED, COMMENT_MODE_FLAT_EXPANDED)); - if ($num_comments <= $comments_per_page || ($flat && $order == COMMENT_ORDER_NEWEST_FIRST)) { - // Only one page of comments or flat forum and newest first. - // First new comment will always be on first page. + if ($num_comments <= $comments_per_page) { + // Only one page of comments. $pageno = 0; } + elseif ($flat) { + // Flat comments. + $count = $num_comments - $new_replies; + $pageno = $count / $comments_per_page; + } else { - if ($flat) { - // Flat comments and oldest first. - $count = $num_comments - $new_replies; - } - else { - // Threaded comments. See the documentation for comment_render(). - if ($order == COMMENT_ORDER_NEWEST_FIRST) { - // Newest first: find the last thread with a new comment. - $result = db_query('(SELECT thread FROM {comments} WHERE nid = %d AND status = 0 ORDER BY timestamp DESC LIMIT %d) ORDER BY thread DESC LIMIT 1', $node->nid, $new_replies); - $thread = db_result($result); - $result_count = db_query("SELECT COUNT(*) FROM {comments} WHERE nid = %d AND status = 0 AND thread > '" . $thread . "'", $node->nid); - } - else { - // Oldest first: find the first thread with a new comment. - $result = db_query('(SELECT thread FROM {comments} WHERE nid = %d AND status = 0 ORDER BY timestamp DESC LIMIT %d) ORDER BY SUBSTRING(thread, 1, (LENGTH(thread) - 1)) LIMIT 1', $node->nid, $new_replies); - $thread = substr(db_result($result), 0, -1); - $result_count = db_query("SELECT COUNT(*) FROM {comments} WHERE nid = %d AND status = 0 AND SUBSTRING(thread, 1, (LENGTH(thread) - 1)) < '" . $thread . "'", $node->nid); - } - $count = db_result($result_count); - } + // Threaded comments. + // Find the first thread with a new comment. + $result = db_query('(SELECT thread FROM {comments} WHERE nid = %d AND status = 0 ORDER BY timestamp DESC LIMIT %d) ORDER BY SUBSTRING(thread, 1, (LENGTH(thread) - 1)) LIMIT 1', $node->nid, $new_replies); + $thread = substr(db_result($result), 0, -1); + $result_count = db_query("SELECT COUNT(*) FROM {comments} WHERE nid = %d AND status = 0 AND SUBSTRING(thread, 1, (LENGTH(thread) - 1)) < '" . $thread . "'", $node->nid); + $count = db_result($result_count); $pageno = $count / $comments_per_page; } @@ -511,13 +489,6 @@ function comment_form_alter(&$form, $form_state, $form_id) { '#options' => _comment_get_modes(), '#description' => t('Expanded views display the body of the comment. Threaded views keep replies together.'), ); - $form['comment']['comment_default_order'] = array( - '#type' => 'radios', - '#title' => t('Display order'), - '#default_value' => variable_get('comment_default_order_' . $form['#node_type']->type, COMMENT_ORDER_NEWEST_FIRST), - '#options' => _comment_get_orders(), - '#description' => t('Comments are displayed in ascending or descending order.'), - ); $form['comment']['comment_default_per_page'] = array( '#type' => 'select', '#title' => t('Comments per page'), @@ -921,7 +892,6 @@ function comment_render($node, $cid = 0) { } $mode = _comment_get_display_setting('mode', $node); - $order = _comment_get_display_setting('sort', $node); $comments_per_page = _comment_get_display_setting('comments_per_page', $node); if ($cid && is_numeric($cid)) { @@ -955,30 +925,19 @@ function comment_render($node, $cid = 0) { $query_count .= ' AND c.status = %d'; $query_args[] = COMMENT_PUBLISHED; } - - if ($order == COMMENT_ORDER_NEWEST_FIRST) { - if ($mode == COMMENT_MODE_FLAT_COLLAPSED || $mode == COMMENT_MODE_FLAT_EXPANDED) { - $query .= ' ORDER BY c.cid DESC'; - } - else { - $query .= ' ORDER BY c.thread DESC'; - } + if ($mode == COMMENT_MODE_FLAT_COLLAPSED || $mode == COMMENT_MODE_FLAT_EXPANDED) { + $query .= ' ORDER BY c.cid'; } - elseif ($order == COMMENT_ORDER_OLDEST_FIRST) { - if ($mode == COMMENT_MODE_FLAT_COLLAPSED || $mode == COMMENT_MODE_FLAT_EXPANDED) { - $query .= ' ORDER BY c.cid'; - } - else { - // See comment above. Analysis reveals that this doesn't cost too - // much. It scales much much better than having the whole comment - // structure. - $query .= ' ORDER BY SUBSTRING(c.thread, 1, (LENGTH(c.thread) - 1))'; - } + else { + // See comment above. Analysis reveals that this doesn't cost too + // much. It scales much much better than having the whole comment + // structure. + $query .= ' ORDER BY SUBSTRING(c.thread, 1, (LENGTH(c.thread) - 1))'; } + $query = db_rewrite_sql($query, 'c', 'cid'); $query_count = db_rewrite_sql($query_count, 'c', 'cid'); - // Start a form, for use with comment control. $result = pager_query($query, $comments_per_page, 0, $query_count, $query_args); $divs = 0; @@ -1770,7 +1729,6 @@ function theme_comment_post_forbidden($node) { function template_preprocess_comment_wrapper(&$variables) { // Provide contextual information. $variables['display_mode'] = _comment_get_display_setting('mode', $variables['node']); - $variables['display_order'] = _comment_get_display_setting('sort', $variables['node']); $variables['template_files'][] = 'comment-wrapper-' . $variables['node']->type; } @@ -1805,19 +1763,6 @@ function _comment_get_modes() { } /** - * Return an array of viewing orders for comment listings. - * - * We can't use a global variable array because the locale system - * is not initialized yet when the comment module is loaded. - */ -function _comment_get_orders() { - return array( - COMMENT_ORDER_NEWEST_FIRST => t('Date - newest first'), - COMMENT_ORDER_OLDEST_FIRST => t('Date - oldest first') - ); -} - -/** * Return an array of "comments per page" settings from which the user * can choose. */ @@ -1839,10 +1784,6 @@ function _comment_get_display_setting($setting, $node) { $value = variable_get('comment_default_mode_' . $node->type, COMMENT_MODE_THREADED_EXPANDED); break; - case 'sort': - $value = variable_get('comment_default_order_' . $node->type, COMMENT_ORDER_NEWEST_FIRST); - break; - case 'comments_per_page': $value = variable_get('comment_default_per_page_' . $node->type, 50); } diff --git a/modules/comment/comment.test b/modules/comment/comment.test index 96a285040..4f28ea4f5 100644 --- a/modules/comment/comment.test +++ b/modules/comment/comment.test @@ -293,7 +293,7 @@ class CommentTestCase extends DrupalWebTestCase { } /** - * Checks current pag for specified comment. + * Checks current page for specified comment. * * @param object $comment Comment object. * @param boolean $reply The comment is a reply to another comment. @@ -373,7 +373,7 @@ class CommentTestCase extends DrupalWebTestCase { * Comments per page value. */ function setCommentsPerPage($number) { - $this->setCommentSettings('comment_default_per_page', $number, 'Number of comments per page set to ' . $number .'.'); + $this->setCommentSettings('comment_default_per_page_article', $number, 'Number of comments per page set to ' . $number .'.'); } /** |