[D7] Image Importer

Dear tomasribes,
pareview.sh shows lots of errors.

Please check this link (http://pareview.sh/pareview/httpgitdrupalorgsandboxtomasribes2175167git) to find more details about the errors.

Cheers!!!

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,

I have installed the module and go to the page 'image_importer/execute', it show me some warnings

Warning: scandir(/var/www/sites/default/files/): failed to open dir: No such file or directory in image_importer_scan_folder() (line 328 of x).

Warning: scandir(): (errno 0): Success in image_importer_scan_folder() (line 328 of /var/www/drupal-7.23/sites/all/modules/contrib/image_importer/image_importer.module).

Warning: Invalid argument supplied for foreach() in image_importer_scan_folder() (line 330 of /var/www/drupal-7.23/sites/all/modules/contrib/image_importer/image_importer.module).

My site is running as http://localhost/drupal7

so in the first warning - you can try to scan wrong directory

I have checked the code and found one thing - you used $_SERVER['DOCUMENT_ROOT'], In place of server variable you can use DRUPAL_ROOT (Root directory of Drupal installation).

Hi Tomas,

Your module does exactly what I found in your README.txt.

I checked your code with pareview and found one small error:
324 | ERROR | Files must end in a single new line character
Maybe you can check this before the last review.

Some of my impressions and ideas using the module:

• You could add a validation of the path entered in the settings form.
• The serialized info in case the image import is successful may show lots of content.I would log this with watchdog instead, so that users are able to recognize the success message
• You could rename your module to Image Importer. Seems like most module's name parts are capitalized.

The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

  remotes/origin/7.x-1.x-dev

Manual review:

1. I can already mass import images with the migrate module, what are differences to that? Please add that to you project page. https://drupal.org/project/migrate
2. image_importer_install(): no need to set variables upon installation as you can make use of default values with variable_get() anyway.
3. image_importer_permission(): why are the permissions set to "'restrict access' => TRUE,"? Is it possible for a user with that permission to take over the site? If yes, how? Please add a comment.
4. <li>Nodes id: ' . $nid . '</li><li>field name: ' . $field_name . '</li>: all user facing text must run through t() for translation.
5. "t('No node found with id:') . ' ' . $nid": do not concatenate variables into translatable strings, use placeholders with t() instead.
6. image_importer_cron(): that operation looks a bit expensive. If I have cron configured to run every 5 minutes it will always check all files and all nodes for import?
7. _image_importer_add_node_images(): why do you hard-code LANGUAGE_NONE here? Shouldn't you use field_language() or similar functions?
8. _image_importer_scan_folder(): desperately needs @param documentation, see https://drupal.org/node/1354#param
9. How do you prevent unnecessary node_save() calls for images that have been imported already? If I have a large directory of images and cron runs every 5 minutes that will create an immense server load? Maybe I have overlooked that in the code.
10. image_importer_settings_form(): do not query that field_config table directly, use field_info_field() or similar API functions.

Although you should definitely fix those issues they are not critical application blockers, so ...

Thanks for your contribution, tomasribes!

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.