- Patch #248436 by catch: fixed some bugs in the statistics module, wrote some...

- Patch #248436 by catch: fixed some bugs in the statistics module, wrote some tests, and made some minor usability improvments along the way.  That is _exactly_ how we like it.

modules/statistics/statistics.admin.inc

@@ -79,18 +79,18 @@ function statistics_top_visitors() {
     array('data' => t('Hits'), 'field' => 'hits', 'sort' => 'desc'),
     array('data' => t('Visitor'), 'field' => 'u.name'),
     array('data' => t('Total page generation time'), 'field' => 'total'),
-    array('data' => t('Operations'))
+    array('data' => user_access('block IP addresses') ? t('Operations') : '', 'colspan' => 2),
   );
 
-  $sql = "SELECT COUNT(a.uid) AS hits, a.uid, u.name, a.hostname, SUM(a.timer) AS total, ac.aid FROM {accesslog} a LEFT JOIN {access} ac ON ac.type = 'host' AND LOWER(a.hostname) LIKE (ac.mask) LEFT JOIN {users} u ON a.uid = u.uid GROUP BY a.hostname, a.uid, u.name, ac.aid" . tablesort_sql($header);
+  $sql = "SELECT COUNT(a.uid) AS hits, a.uid, u.name, a.hostname, SUM(a.timer) AS total, bl.iid FROM {accesslog} a LEFT JOIN {blocked_ips} bl ON a.hostname = bl.ip LEFT JOIN {users} u ON a.uid = u.uid GROUP BY a.hostname, a.uid, u.name, bl.iid" . tablesort_sql($header);
   $sql_cnt = "SELECT COUNT(DISTINCT(CONCAT(uid, hostname))) FROM {accesslog}";
   $result = pager_query($sql, 30, 0, $sql_cnt);
 
   $rows = array();
   while ($account = db_fetch_object($result)) {
     $qs = drupal_get_destination();
-    $ban_link = $account->aid ? l(t('unban'), "admin/user/rules/delete/$account->aid", array('query' => $qs)) : l(t('ban'), "admin/user/rules/add/$account->hostname/host", array('query' => $qs));
-    $rows[] = array($account->hits, ($account->uid ? theme('username', $account) : $account->hostname), format_interval(round($account->total / 1000)), $ban_link);
+    $ban_link =  $account->iid ? l(t('unblock IP address'), "admin/settings/ip-blocking/delete/$account->iid", array('query' => $qs)) : l(t('block IP address'), "admin/settings/ip-blocking/$account->hostname", array('query' => $qs));
+    $rows[] = array($account->hits, ($account->uid ? theme('username', $account) : $account->hostname), format_interval(round($account->total / 1000)), (user_access('block IP addresses') && !$account->uid) ? $ban_link : '');
   }
 
   if (empty($rows)) {

modules/statistics/statistics.test

+<?php
+// $Id$
+
+class StatisticsBlockVisitorsTestCase extends DrupalWebTestCase {
+  /**
+   * Implementation of getInfo().
+   */
+  function getInfo() {
+    return array(
+      'name' => t('Top visitor blocking'),
+      'description' => t('Tests blocking of IP addresses via the top visitors report.'),
+      'group' => t('Statistics')
+    );
+  }
+
+  /**
+   * Implementation of setUp().
+   */
+  function setUp() {
+    parent::setUp('statistics');
+
+    // Create user.
+    $this->blocking_user = $this->drupalCreateUser(array('block IP addresses', 'access statistics'));
+
+    // Insert dummy access by anonymous user into accessi log.
+    db_query("INSERT INTO {accesslog} (title, path, url, hostname, uid, sid, timer, timestamp) values('%s', '%s', '%s', '%s', %d, '%s', %d, %d)", 'test', 'node/1', 'http://example.com', '192.168.1.1', '0', '10', '10', time());
+  }
+
+  /**
+   * Blocks an IP address via the top visitors report then uses the same page to unblock it.
+   */
+  function testIPAddressBlocking() {
+    // IP address for testing.
+    $test_ip_address = '192.168.1.1';
+
+    // Enable access logging (redundant since we insert the data manually).
+    variable_set('statistics_enable_access_log', 1);
+
+    // Verify the IP address from accesslog appears on the top visitors page
+    // and that a 'block IP adddress' link is displayed.
+    $this->drupalLogin($this->blocking_user);
+    $this->drupalGet('admin/reports/visitors');
+    $this->assertText($test_ip_address, t('IP address found.'));
+    $this->assertText(t('block IP address'), t('Block IP link displayed'));
+
+    // Block the IP address.
+    $this->clickLink('block IP address');
+    $this->assertText(t('IP address blocking'), t('IP blocking page displayed.'));
+    $edit = array();
+    $edit['ip'] = $test_ip_address;
+    $this->drupalPost('admin/settings/ip-blocking', $edit, t('Save'));
+    $ip = db_result(db_query("SELECT iid from {blocked_ips} WHERE ip = '%s'", $edit['ip']));
+    $this->assertNotNull($ip, t('IP address found in database'));
+    $this->assertRaw(t('The IP address %ip has been blocked.', array('%ip' => $edit['ip'])), t('IP address was blocked.'));
+
+    // Verify that the block/unblock link on the top visitors page has been altered.
+    $this->drupalGet('admin/reports/visitors');
+    $this->assertText(t('unblock IP address'), t('Unblock IP address link displayed'));
+
+    // Unblock the IP address.
+    $this->clickLink('unblock IP address');
+    $this->assertRaw(t('Are you sure you want to delete %ip?', array('%ip' => $test_ip_address)), t('IP address deletion confirmation found.'));
+    $edit = array();
+    $this->drupalPost('admin/settings/ip-blocking/delete/1', NULL, t('Delete'));
+    $this->assertRaw(t('The IP address %ip was deleted.', array('%ip' => $test_ip_address)), t('IP address deleted.'));
+  }
+}

modules/system/system.admin.inc

@@ -1104,10 +1104,10 @@ function system_ip_blocking() {
     );
   }
 
-  $output .= theme('table', $header, $rows);
-
   $output .= drupal_get_form('system_ip_blocking_form');
 
+  $output .= theme('table', $header, $rows);
+
   return $output;
 }
 
@@ -1124,6 +1124,7 @@ function system_ip_blocking_form($form_state) {
     '#type' => 'textfield',
     '#size' => 64,
     '#maxlength' => 32,
+    '#default_value' => arg(3),
     '#description' => t('Enter a valid IP address.'),
   );
   $form['submit'] = array(

modules/system/system.module

@@ -495,6 +495,13 @@ function system_menu() {
     'page callback' => 'system_ip_blocking',
     'access arguments' => array('block IP addresses'),
   );
+  $items['admin/settings/ip-blocking/%'] = array(
+    'title' => 'IP address blocking',
+    'description' => 'Manage blocked IP addresses.',
+    'page callback' => 'system_ip_blocking',
+    'access arguments' => array('block IP addresses'),
+    'type' => MENU_CALLBACK,
+  );
   $items['admin/settings/ip-blocking/delete/%blocked_ip'] = array(
     'title' => 'Delete IP address',
     'page callback' => 'drupal_get_form',