Show IP Address in Comment Admin

My apologies for getting this in the wrong project.

As for uses, well, I agree it's subjective, and probably only useful for anonymous comments. If you're being spammed, it's likely it'll be distributed, so the IP is fairly useless. However, recently on my site a person posted dozens of very similar (relatively meaningful) comments. He didn't fill in any of the contact info with anything useful, so it was difficult to see if it was some misguided attempt to astroturf (or spam), or just slightly over-zelous commenting. Seeing that all the comments came from the same IP was useful in so much as that strongly suggests it's not a spam attack, and actually just a human being vocal (and makes it easy to identify all comments from that human, should they need to be deleted).

There's scope to alter the feature into something less significant. Perhaps some sort of 'mouseover' on the comment author, or perhaps just putting the IP in brackets after the user's name or something. That looked to me to be more of a deviation from convention in Drupal, but it could easily be done if you prefer?

It'd be easy enough to add a config option that says "show IP address of comment authors in admin screens" or some such. I'd imagine that's overkill, but could then be used by spam.module and others, if needs be?

Really? A contrib module just to add one column to an admin screen?

I can't see a way to do this as a contrib module without replacing comment.module with an almost exact replica. If you know better, can you point me at an example?

I'm getting along the right lines with this, but can't see a way to make it a pure module without HEAD patches.

Firstly, the IP address of the comment poster is already recorded, and inserted in the {comments} in the 'hostname' column. However, in comment.module, in comment_admin_overview() the database fetch is:

$result = pager_query('SELECT c.subject, c.nid, c.cid, c.comment, c.timestamp, c.status, c.name, c.homepage, u.name AS registered_name, u.uid FROM {comments} c INNER JOIN {users} u ON u.uid = c.uid WHERE c.status = %d'. tablesort_sql($form['header']['#value']), 50, 0, NULL, $status);

...which of course won't get the hostname out. Correspondingly, the while() loop that builds the rows of the table doesn't include the hostname either, so the theme_comment_admin_overview() function won't get access to hostname either.

So at this point, I can see that patching the select above to say "SELECT c.* ...", and the while() loop to add hostname to the rows array would probably do most of what is required.

In the theme function, I can't see an elegant way to adjust the header of the table without completely re-writing it. That said, it's not too bad. My theme function would look something like:

function theme_comment_admin_overview($form) {
  $output = drupal_render($form['options']);
  if (isset($form['subject']) && is_array($form['subject'])) {
    foreach (element_children($form['subject']) as $key) {
      $row = array();
      $row[] = drupal_render($form['comments'][$key]);
      $row[] = drupal_render($form['subject'][$key]);
      $row[] = drupal_render($form['username'][$key]);
      $row[] = drupal_render($form['hostname'][$key]);
      $row[] = drupal_render($form['timestamp'][$key]);
      $row[] = drupal_render($form['operations'][$key]);
      $rows[] = $row;
    }
  }
  else {
    $rows[] = array(array('data' => t('No comments available.'), 'colspan' => '6'));
  }

  $form['header'] = array('#type' => 'value', '#value' => array(
    theme('table_select_header_cell'),
    array('data' => t('Subject'), 'field' => 'subject'),
    array('data' => t('Author'), 'field' => 'name'),
    array('data' => t('IP Address'), 'field' => 'hostname'),
    array('data' => t('Time'), 'field' => 'timestamp', 'sort' => 'desc'),
    array('data' => t('Operations'))
  ));

  $output .= theme('table', $form['header']['#value'], $rows);
  if ($form['pager']['#value']) {
    $output .= drupal_render($form['pager']);
  }

  $output .= drupal_render($form);

  return $output;
}

In short, I can make the theme function to do what I need, but still need a HEAD patch to make it work. That said, the HEAD patch will do nothing to the actual application unless my modified theme function is used. Is this right, or am I missing something?
(Incidentally, minimalist HEAD patch attached)

Ahh, okay, maybe we're slightly cross-purposes here...

I wasn't (originally) concerned with showing the IP address of authors on the actual comments themselves, although if we're making a contrib module, that would make sense to add as well.

It seems silly to add the IP address of the comment author to a module specific table, as it's already recorded by Drupal in the {comments} table. Surely, we must be able to just use that for the admin screens as well as the comments themselves?

For the admin screens, I still don't see how theming alone can show the ip address of comments without patching HEAD (unless you duplicate the IP in your own table, and then use the theme function to step through each row of the table to look up the IP in your own table). This seems like a really daft way to go when a trivial patch to HEAD solves the problem prety elegantly (and saves numerous DB lookups!).

So we're left with two things to solve:

1) How to show IP addresses in the colums of the admin screen(s)
2) How to show IPs alongside author names on the comments themselves

I (still) propose a patch to HEAD for (1). I accept that theming is the way to actually show the IP, and thus make it optional via a module. I think this really solves the problem for everyone, and leaves Drupal in a clean, extensible state, without introducing UI elements no one wants.

As for (2) I haven't really looked at this in any detail, but I'd imagine a solution similar to (1) would be the best way to go. I don't know if this requires a similar patch to HEAD or if it can be achieved with some hook_comment or nodeapi stuff. I strongly suspect that a HEAD patch along the lines of (1) would solve a good deal of problems, making it a trivial theming issue, rather than needing hook_comment or nodeapi at all.

Someone wiser than I needs to decide on (1), which will either set a precident for (2) or else leave us open to build the contrib module as suggested.

Sorry, now I've re-read my previous post, I can see it wasn't too clear...

The theme-only patch doesn't work with 5.1/HEAD. Core doesn't (yet) put "hostname" into the $form array, so the theme patch you propose only places an empty column in the admin screen.

So we're left with the same problem: the "minimal" patch I submitted above solves the core issue, without changing the theme function. In other words, if you apply this patch to core as it stands, you won't see any difference. Of course, people like myself are free to do whatever we want with themes (as you suggest).

Assuming this approach is okay, this sets a precident for the other problem raised by meba - it's basically the same problem, with the same solutions (one HEAD patch, then one theme change in a module).