The name of your module or theme -
CiviCRM contact distance search

A detailed description of what it does -
CiviCRM contact distance search module enable the facility to search CiviCRM contacts based on distance
from perticular UK post code

A link to its sandbox project on Drupal.org.
https://www.drupal.org/sandbox/vakeesan26/2498395

git repository:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/vakeesan26/2498395.git civicrm_contact_distance_search

The intended Drupal core version (eg. 6.x or 7.x)
7.x

Comments

PA robot’s picture

Status: Needs review » Needs work

Git clone failed for http://git.drupal.org/sandbox/vakeesan26/2498395.git while invoking http://pareview.sh/pareview/httpgitdrupalorgsandboxvakeesan262498395git

Git repository is empty. Aborting.

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.

vakeesan26’s picture

Issue summary: View changes
vakeesan26’s picture

git repository has been added

vakeesan26’s picture

Issue tags: +PAreview: review bonus
klausi’s picture

Status: Needs work » Needs review
Issue tags: -PAreview: review bonus

There are no reviews of other projects in the issue summary yet? Make sure to read https://www.drupal.org/node/1975228 again.

And I guess this needs review now?

Pravin Ajaaz’s picture

Issue summary: View changes
Pravin Ajaaz’s picture

Issue tags: +PAreview: security

Automated review:

Trying clearing the pareview errors (Most of which are intendations.):
http://pareview.sh/pareview/httpgitdrupalorgsandboxvakeesan262498395git

Manual Review:
1. You should not add js and css on hook_init() because they will be added in all the pages. so use it on the appropriate page callbacks
2. I cannot find the hook_permission() but you have used an access argument => administer civicrm_contact_distance_search settings . This might be a security issue.
3. Enhance your comments by adding @param and @return.

Pravin Ajaaz’s picture

Status: Needs review » Needs work
klausi’s picture

Issue tags: -PAreview: security

a missing permission is not a security issue, since access will just be denied in every situation except for user 1. Make sure to try to exploit security problems next time ;-)

Pravin Ajaaz’s picture

@klausi: Sure :) I thought everything related to permission will be considered as a security issue.

vakeesan26’s picture

Status: Needs work » Needs review

Thank you for the review
issues fixed now !

awasson’s picture

@vakeesan26: I'm quite interested in your project. Is there any reason why it is only focussed on UK postal codes; that seems a little bit limiting. Have you explored the possibility of checking against all countries that support postal or zip codes?

awasson’s picture

Ok, first things first. The module works and according to my testing, it works very well. I'm quite interested in this module in particular because I have several website projects that will benefit from the exact module so finding it in the approval queue is great as far as I'm concerned and I'll gladly help you get it pushed through approval if possible.

I know building modules (and going through the approval process) is labour intensive so please don't take my suggestions as criticism. I've spent the better part of today reviewing and commenting on your module. I think this module shows great promise and the things I'm suggesting aren't particularly difficult to incorporate but will improve it or make it simpler to maintain and extend in the future.

Below is my findings according to the reviewing template:

Individual user account
Yes: Follows the guidelines for individual user accounts but you could/should update your profile to be more informative. I believe the underlying theme for this part of the manual review process is to see if the individual has immersed themselves in the Drupal community; I'm not seing that according to your profile. Whether the powers the be take that into consideration when granting full project status, I don't know but it couldn't hurt to make your profile (account) more informative.

No duplication
Yes: Does not cause module duplication and/or fragmentation.

Master Branch
Yes: Follows the guidelines for master branch.

Licensing
Yes: Follows the licensing requirements.

3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.

Secure code
Yes: Meets the security requirements.

Coding style & Drupal API usage
Yes: Passes PAReview

NOTES ON MY REVIEW:

Below are notes I've made while reviewing the code and testing the module.

In the module code, for cleanliness, I have a few Drupal coding convention suggestions so that the module code and hooks therein are easier to follow and in the long run, maintain:

1) Custom functions should be moved into a mymodule.inc file

2) You custom code could use a bit more descriptive commenting according to the page here: https://www.drupal.org/node/1354#functions

3) you should break your admin page out into it's own file

4) Views should be moved into a mymodule/views directory. This is described on the API page: https://api.drupal.org/api/views/views.api.php/function/hook_views_defau...

* This is a good article to look at if you want to have several Views created by your module: http://websmiths.co/blog/drupal-views-tutorials-exporting-views-code

Regarding your project page, I have some suggestions:

1) On your project page, I suggest you remove the zip download attachment as people who are using the sandbox should be using GIT through "Version Control" to get the current version.

2) On the project page, the link to the Google API is broken. It looks like a simple typo.

3) On the project page I would suggest that you update the instructions to provide more detail about enabling the Google Geocoding API. I've been using the Google API for a variety of things and had several keys for messing about but I hadn't enabled the Geocoding API and spent a little while messing about. I realize you do mention that they should enable it but maybe for those of us who are little dense, it would be helpful to have a step by step instruction.

* Actually, it would be more useful to have it on the API configuration page if you put in point form that they need to log into or create a Google API account, enable the specific API's and get a Browser Key.

4) In the project page instructions and the README file, I would suggest or mention that users of the module clone the view so that they don't modify the one that comes with the module. Certainly not something that would stop your module from being approved but maybe a good idea.

FEATURE REQUEST:
I don't know if this will have any bearing on whether your module gets approved faster one way or another but I would like to see this extended to work in other parts of the world. I'll be happy to help with that. You've done the lion's share of the work so enabling other parts of the world should be possible. I have a need for this to work with Canada Postal codes and I'll be happy to get that working.

awasson’s picture

Status: Needs review » Needs work
vakeesan26’s picture

@awasson Thank you for your detail review
I will follow all the instructions and update asap !

vakeesan26’s picture

Status: Needs work » Needs review

hi @awasson
I have fixed your review comments
now it will support UK,CA,US postcodes as well
please have a look.
Thank you

awasson’s picture

Hi @vakeesan26,
I was away last week and have to head out again tonight but I will definitely have a look at those changes as soon as I can. Good work!

Andrew

awasson’s picture

Hi @vakeesan26,
My apologies for not being able to have a look at your updates earlier. I've been swamped with work and this is the first day I've been able to get back to your module.

I just installed the latest version and it works perfectly! I've only tested it for Canadian Postal Codes I know of in a CiviCRM site I know very well. I'll check it in another American site to check US zip codes too.

I'll dig into it and see if I can find anything to pester you about but for the moment I'm totally happy with it.

I think this is a welcome addition to the Drupal/CiviCRM toolkit so I'll get some other Drupal/Civi people to give it reviews as well and see if we can push this through the review process. The review process does take some time so I would encourage you to get the PAReview Bonus to push it along: https://www.drupal.org/node/1975228

Andrew

vakeesan26’s picture

Thank you very much @awasson

mlncn’s picture

Status: Needs review » Reviewed & tested by the community

Dear vakeesan26, i am terribly sorry this has gone nearly a year without formal approval after passing awasson's thorough review and you having fixed issues raised in those reviews and in Pravin Ajaaz's.

mlncn’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contribution! Congratulations, you are now a vetted Git user. You can promote this to a full project.

When you create new projects (typically as a sandbox to start) you can then promote them to 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 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.

vakeesan26’s picture

Thank you very much @mlncn

awasson’s picture

@vakeesan26: Congratulations.

I really like what you've created here. It will be very handy for those of us working with CiviCRM and Drupal.

vakeesan26’s picture

Thank you @awasson

Status: Fixed » Closed (fixed)

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