This module provides the IP of submitted comments on the comment overview page and adds an option to 'Delete comment and block IP' from the drop-down on the comment overview page. The idea being if you have allowed anonymous commenting you can delete and block spam comments.

Project Page
http://drupal.org/node/1921790

Git Clone
git clone --recursive --branch 7.x-1.x davidsonjames@git.drupal.org:sandbox/davidsonjames/1921790.git comment_ip

Module is for Drupal 7

Other project application reviews:
http://drupal.org/node/1932384#comment-7130436
http://drupal.org/node/1930370#comment-7130520
http://drupal.org/node/1934104#comment-7138022

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxdavidsonjames1921790git

cackle’s picture

cackle commented
Status: Needs work » Needs review

Hello,
please correct your git url, branch from master to 7.x-1.x, and check http://ventral.org/pareview/httpgitdrupalorgsandboxdavidsonjames1921790git.
From my manual review:
1. Add readme.txt
2. Add .install file.
3. Add description, core, and comment module dependecy to .info.

When I try to install that module I have a message "This version is not compatible with Drupal 7.x and should be replaced." Did you test it locally before submition?

Nevertheless I install it and test.
It's really adds IP to block list, but from my point of view there is no need to block user for site access displaying the message:"Sorry, 127.0.0.1 has been banned.". The ban user should have the restrictions only for posting new comments but not accessing the whole site.

May be you can add "block by ip" button to each comment for moderation without going to admin panel?
Cackle.

cackle’s picture

cackle commented
davidsonjames’s picture

davidsonjames commented
Status: Needs review » Needs work

Apologies, I had already branched into 7.x-1.x and made some alterations in that branch (I already created a readme.txt file in there), but there will still need to be some of your suggestions implemented.

The idea was to be able to quickly determine a visitors IP address and identify through their comments if they were a spam bot - and as such block them from accessing the site totally. With anonymous commenting the commenter wouldn't have a user to restrict access to - but if they were receiving constant spam from a particular IP, they may like to block that IP.

I'll run the branch 7.x-1.x through ventral.org, make alterations and test again on a fresh install.

Thank you!

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

Ok I've cleaned up all the errors from ventral.org, changed my info file - I needed to add the core version - that all seems to be ok now.

Do I need to have a .install file if it's going to be empty?

The git repo is:

git clone --recursive --branch 7.x-1.x davidsonjames@git.drupal.org:sandbox/davidsonjames/1921790.git comment_ip

AolPlugin’s picture

AolPlugin commented
Status: Needs review » Needs work

Seems like a pretty useful module, however there are a few problems.

Manual review:
- Major/Crash: After enabling your module, going to the comments page, selecting one or more comments and selecting the "Unpublish the selected comments" option, I get a 500 Internal Server Error
- Minor: there are several messages that aren't grammatically correct (eg. line 95 IP's should be IPs -- same for a few other places)

You do not need an install file, if you're not going to use it.

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

Aha, yeah sorry about that - major oversight on my side. I'd kept a submit handler which I no longer required.

I've fixed that up now - and altered to some of my messages which had incorrect grammar (not my strong suit).

Thanks for pointing that out!

DebtConsolidationCare’s picture

DebtConsolidationCare commented
Status: Needs review » Needs work

Hi, here is my suggestion as per manual review

1. As drupal also supports ip address blocking from 'settings.php' you can use hook_enable to check if $conf['blocked_ips'] is set in 'settings.php' file and give a warning message.
2. When downloading, using the git link in project page it is the master branch getting downloaded by default.

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

I've changed the default branch in the project page, thank you!

In regards to $conf['blocked_ips'] - I'm not sure what the benefit would be. If an IP is blocked in the settings.php - then a user wouldn't be commenting on the site, also there'd be little harm if the IP was in both the settings.php file and the blocked IP's table.

Were you thinking a message like how at the moment it says "The IP address %ip was already in the blocked IP list." it could also say "This IP address has already been blocked in your settings.php configuration"? If you meant something else, apologies.

I think this is something I could perhaps add at a later date - but I don't think its fundamental to the way the module works.

Thanks for the review.

DebtConsolidationCare’s picture

DebtConsolidationCare commented
Status: Needs review » Needs work

Hi,

As per my current knowledge ip-blocking from settings.php overrides all blocked-ip-list from {blocked_ips} table instead of merging the two list of ip-addresses.

Here is the scenario what I want to explain to you.

Suppose I have installed a fresh copy of drupal-7 and in sites/default/settings.php I set the following IP addreses blocked.
$conf['blocked_ips'] = array('10.1.10.100', '10.1.10.101');
Now I installed the module 'comment_ip' and deleted a comment from ip address '10.1.10.200' with option 'Delete the selected comments and block their IPs'

It shows me a message 'The IP address 10.1.10.200 has been blocked'
But the problem is comments can still be made from '10.1.10.200'.

Please correct me if I wrong.

davidsonjames’s picture

davidsonjames commented

I've just checked that out and you're absolutely right, I didn't know it did that. Thank you!

Ok yeah I'll check if that config is enabled and set a warning message and maybe even disable the 'block ips' option from the select if that is enabled.

Good spot.

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

Ok I've added in some logic to check if the blocked_ips config is being used, removed the block ip functionality if blocked_ips has been set, and given instructions to add the ip manually if that is the case.

I've also deleted the master branch as I had forgotten to do that.

davidsonjames’s picture

davidsonjames commented
Issue summary: View changes

Changed git repo branch.

davidsonjames’s picture

davidsonjames commented
Issue summary: View changes

Added review of another module.

davidsonjames’s picture

davidsonjames commented
Issue summary: View changes

Another project review.

davidsonjames’s picture

davidsonjames commented

Is there anything else I need to do from here?

denisz’s picture

denisz commented
Status: Needs review » Needs work

Manual review:

comment_ip.module line 29: drupal_set_message('It looks like you have used the blocked_ips config setting. To block an IP displayed below you will need to manually add that IP to your blocked_ip config.'); - Message must be wrapped in t() function.

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

Thank you! I've wrapped that in a t() function now.

denisz’s picture

denisz commented
Status: Needs review » Needs work

Manual review:
line 26 $form['options']['operation']['#options']['block'] = 'Delete the selected comments and block their IPs'; - the same issue, static text must be wrapper in t() function.

davidsonjames’s picture

davidsonjames commented
Status: Needs work » Needs review

Thank you. I've changed that. I think I've got all the t()'s now!

mitchell’s picture

mitchell commented
Assigned: Unassigned » mitchell
Status: Needs review » Reviewed & tested by the community

I tested this, and it worked well. The code also looks good. So, I think this is ready.

Please continue reporting issues in the project's issue queue.

mitchell’s picture

mitchell commented
Assigned: mitchell » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, davidsonjames!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks to the dedicated reviewers as well.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Another application review