[D7] User IP address

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxdrupaladmin2191469git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi dr.admin,
1. There should onle blank line after the php tag in both the .install and .module file.
2. There is no need to take the variables like $columns and $indexes in .install file you can directly assign the array of these to the "columns" and "indexes" key where you return the array.
3. t('IP address must be in format a.b.c.d : @ip', array('ip' => $item['user_ip_address'])) it should be like that
t('IP address must be in format a.b.c.d : @ip', array('@ip' => $item['user_ip_address']))
4. Remove the one blank line from the function user_ip_address_field_is_empty() and user_ip_address_field_formatter_info()

@dr.admin, thank you for your work and contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. None.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

(*) No: Doesn't follows the guidelines for master branch. Please delete your master branch, according to https://drupal.org/empty-git-master.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes. If "no", list security issues identified.

Coding style & Drupal API usage

1. (*) I tested your module functionality and it works as intended. But there is a possibility when user can face issue, if user follow either of following approach:
   Case 1: If user select Required field then following issue will come at node save.
   
   
   
   
   Error at node save:
   
   
   

   Case 2: If user select Unlimited option for Number of values then following issue will come at node save.
   
   
   
   
   Unable to use this field, but it is visible.
   
   
   

   So there is two way to solve this issue, either you explicitly mention this in your documentation (Project Page or Readme.txt etc) or fix these by making these option disabled for this field.

2. hook_help() is missing in your module.
3. user_ip_address_field_formatter_view(): the switch() statement does not make sense here since you only have one case? Use if() instead? Same apply for user_ip_address_field_widget_form().

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

As I am not a git administrator, so I would recommend you, please help to review other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Nice module. thanks again!

Hi Roman!
Thanks for your contribution!

I have a couple of questions about “User IP address” module:

1. Isn't it better to store such information in revisions?
2. Can this module be useful in any other cases aside from compliance with certain government regulations?
3. If we need only a field why not to use https://www.drupal.org/project/field_ipaddress and provide hidden widget for it?

Code is clean! Good job!

Best,
Kate

Hi, very useful module :)

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

No: Do not follows the guidelines for master branch. Please create 7.x-1.x branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes.

Coding style & Drupal API usage

1. (*) Stored IP-address is published to all users by default. It would be better to add a separate permission to view IP-addresses. Or add some notes about mandatory field display settings to documentation.
2. (*) I see no widget on node editing form, but the module implements a widget and validation. May be it is not needed? Or better to do not display the field at all but display recorded IP inside widget only.
3. (+) According to documentation the IP is only stored on creating content only, but user_ip_address_field_presave() does something on entity update too.
4. Function user_ip_address_field_formatter_view() contains some misleading comments about color.
5. No empty lines nedded after switch, if, else, function declaration lines.
6. Add module version and dependencies to .info file.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

This review uses the Project Application Review Template.

Looks like you addressed all the issues, changing to RTBC.

Non-blocking issues:

• In user_ip_address_help(), use single quotes where possible for Drupal code standards and a very slight performance benefit

Blocking issues:

Code too simple

This project does not demonstrate sufficient hook usage and/or API integration, therefore it is only eligible for a one-time promotion to a full project. You can find more info at Working with the Drupal API.

This is an important criterion so that code integrates well and can be improved over time. I encourage you to continue developing and gaining from the feedback available in the git approval process.

Thank you for you contributions and understanding. Please indicate if you would like this project to be promoted by itself, without granting any user "git vetted user" status, or if you disagree with this opinion please provide detail.

Thanks for your contribution, dr.admin, this module has been promoted to a full project! https://www.drupal.org/project/user_ip_address

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.