[D7] Google Places API autocomplete

Hi,

please, replace mariancalinro@git.drupal.org:sandbox/mariancalinro/2493761.git with http://git.drupal.org/sandbox/mariancalinro/2493761.git

Also fix some error reported by Pereview http://pareview.sh/pareview/httpgitdrupalorgsandboxmariancalinro2493761git

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 mariancalinro,

I have test the module on drupal 7, add new field to node type with one or more numbers setting, it was worked fine.

PAReview

1. FILE: google_places_api_autocomplete/README.txt
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
75 | WARNING | Line exceeds 80 characters; contains 88 characters
---------------------------------------------------------------------------

I think url so long may be no problem, but for my suggestion is remove the hashtag from the url, just "https://developers.google.com/places/webservice/autocomplete" also very clearly to get places api notes.

2. places_api_autocomplete.module: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming

For the function "places_autocomplete", I think you need to rename it, your module functions should be prefixed with your module name, my suggestion is rename function name to "_places_api_autocomplete", also update on hook_menu.

Suggestions

1. variable name
It was not reported by pareview, but rename variable name "places_api_options" to "places_api_autocomplete_options" is good option.

2. Two README file (README.md, README.txt)

I think one file enough, you can merge it.

3. Remove variables when module uninstall.

<?php
/**
 * Implements hook_uninstall().
 */
function places_api_autocomplete_uninstall() {
  // Remove unused variables.
  variable_del('places_api_autocomplete_key');
  variable_del('places_api_options');
}
?>

4. PHP Notice when variables not saved yet.

Notice: Undefined index: offset in places_api_autocomplete_admin_settings() (line 38 of places_api_autocomplete.admin.inc).
Notice: Undefined index: location in places_api_autocomplete_admin_settings() (line 43 of places_api_autocomplete.admin.inc).
Notice: Undefined index: radius in places_api_autocomplete_admin_settings() (line 48 of places_api_autocomplete.admin.inc).
Notice: Undefined index: language in places_api_autocomplete_admin_settings() (line 53 of places_api_autocomplete.admin.inc).
Notice: Undefined index: types in places_api_autocomplete_admin_settings() (line 58 of places_api_autocomplete.admin.inc).
Notice: Undefined index: components in places_api_autocomplete_admin_settings() (line 63 of places_api_autocomplete.admin.inc).

Suggestion: Add default value when module install or use isset to check key exist in options array.

<?php
/**
 * Implements hook_install().
 */
function places_api_autocomplete_install() {
  // Default places api options.
  $options = array(
    'offset' => '',
    'location' => '',
    'radius' => '',
    'language' => '',
    'types' => '',
    'components' => '',
  );
  variable_set('places_api_options', $options);
}
?>

Hello

Mannual review
1. Please fix pareview.sh issues http://pareview.sh/pareview/httpgitdrupalorgsandboxmariancalinro2493761git
2. All functions should be prefixed with your module name.
3. Line exceeds 80 characters; contains 88 characters in Readme.md

Hi mariancalinro!

Manual review:

1. You should remove the .gitignore file
2. I agree with you on the README.txt and README.md files, they have different content and format, and it looks like the md one has just the practical stuff to get the module going.
3. It's also recommended that you include hook_install() in your .install file, take a look at the recommended practices.
4. I see you are setting the $documentation_link = 'https://developers.google.com/places/documentation/autocomplete#place_autocomplete_requests'; two times. Maybe you want to move that to a more common place as a constant, or maybe a places_api_documentation_link Drupal variable, that way it would be easier if the link changes and then you'll be able to use a hook update to change the variable. In that case, you can set the initial value in the install hook :)
5. includes/ApiAutocompleteCacheServiceInterface.php on line 5 says * Contains ~~. , you should change that to * Contains PlacesApiAutocompleteCacheServiceInterface..

Other than that, looks good to me :)
Good and detailed info in the issue page and the project page.

Nice module, congrats :)

@pablitt: that are nice tips for improvements but surely not application blockers, anything else that you found or should this be RTBC instead?

I think 1, 3 and 5 should be addressed. 4 Is only a suggestion :)

Thanks!

Hello,
I have test the module on drupal 7, worked fine.
Mannual review
1. Please fix pareview.sh issues http://pareview.sh/pareview/httpgitdrupalorgsandboxmariancalinro2493761git
2. Found issues in coder module.
places_api_autocomplete.module
severity: criticalreview: security_fapi_titleLine 182: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized. (Drupal Docs) [security_fapi_title]
'#title' => $instance['label'],

Review of the 7.x-1.x branch (commit 2ce7bd5):

• There should be only one README file, either README.md or README.txt.
• DrupalPractice has found some issues with your code, but could be false positives.
  

  FILE: /home/klausi/pareview_temp/places_api_autocomplete.install
  ---------------------------------------------------------------------------
  FOUND 1 ERROR AND 1 WARNING AFFECTING 1 LINE
  ---------------------------------------------------------------------------
   22 | WARNING | Messages are user facing text and must run through t() for
      |         | translation
   22 | ERROR   | Do not use t() or st() in installation phase hooks, use $t
      |         | = get_t() to retrieve the appropriate localization
      |         | function name
  ---------------------------------------------------------------------------
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

• places_api_autocomplete_init(): I think it is not necessary to implement hook_init() just to check the key. You can do that in hook_requirements() instead.

But otherwise looks good to me.

no objections for more than a week, so ...

Thanks for your contribution, mariancalinro!

I updated your account so you can 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:

• 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.