[D7] Smart Import

Hi,
Found some issues:
1) Need to fix all errors in Coder Sniffer
http://ventral.org/pareview/httpgitdrupalorgsandboxcybernetikz2001438git

2) .info file
Need to delete from 10 to 14 lines, this info is generated automatically by drupal.org when we load module.

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 found some issues:

1) smart_import_node_form()
First parameter is $form.

<?php
function smart_import_node_form($form, $form_state) {
  [...]
}
?>

2) smart_import_node_form_submit()
Splling error:

<?php
$attribs = new stdClass();
$attrib->node_type = $form_state['values']['types'];
?>

Should be

<?php
$attrib = new stdClass();
$attrib->node_type = $form_state['values']['types'];
?>

3) Return value of smart_import_save_row_as_node()
smart_import_save_row_as_node() has no return value, but in smart_import_node_form_submit() is:

<?php
$status = smart_import_save_row_as_node($attrib);
?>

4) smart_import_save_row_as_node()
global $theme is not used.

5) smart_import_get_field_config()
Why don't call field_info_field() directly?

6) smart_import_set_error()
Use spaces instead of tabs.

7) smart_import_install()
variable_set('smart_import_installed', '1'); is redundant to module_exists(). So you can delete this line and variable_del() in smart_import_uninstall().

That's all for now.
Happy coding.

manual review:

1. project page is too short, see https://drupal.org/node/997024 . Also this module looks like it should be based on the migrate module, so you want to clarify the differences. https://drupal.org/project/migrate
2. "Code update" is not a useful git commit message, please follow https://drupal.org/node/52287
3. Hooks that are provided by a module should be documented in MODULENAME.api.php, see http://drupal.org/node/161085#api_php
4. All hooks that you provide need to be prefixed with your module's name to avoid name collisions with others.
5. "drupal_set_message("Import completed with Accept:" . $attrib->success_history['Accept'] . ' & Reject: ' . $attrib->success_history['Reject']);": all user facing text must run through t() for translation. Also elsewhere like " '#title' => 'Content Type',", please check all your strings.
6. smart_import_node_form(): I don't see where you check node access for the current user in order to create nodes? Since your permission is not marked as 'restrict access' => TRUE this looks like an access bypass vulnerability. Either you need to mark your permission as administrative or you need to check for node creation access. This is a security blocker. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
7. "foreach (module_implements('preprocess_row') as $module) {": why can't you use module_invoke_all()?
8. smart_import_init_error_session(): why do you need to call drupal_session_start(), just writing to $_SESSION should be enough?

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Sorry for the delay. Make sure to review more project applications and get a new review bonus and this will get finished faster.

Review of the 7.x-1.x branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/klausi/pareview_temp/smart_import.api.php
  --------------------------------------------------------------------------------
  FOUND 7 ERROR(S) AFFECTING 4 LINE(S)
  --------------------------------------------------------------------------------
   21 | ERROR | Missing parameter type at position 1
   23 | ERROR | Last parameter comment requires a blank newline after it
   23 | ERROR | Doc comment for var $erapper does not match actual variable name
      |       | $ewrapper at position 2
   23 | ERROR | Missing parameter type at position 2
   25 | ERROR | Additional blank line found at the end of doc comment
   31 | ERROR | Files must end in a single new line character
   31 | ERROR | Spaces must be used to indent lines; tabs are not allowed
  --------------------------------------------------------------------------------
  

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:

1. "t("Import completed with Accept:") . $attrib->success_history['Accept'] . t(' & Reject: ') . $attrib->success_history['Reject']": Do not concatenate variables into translatable strings, use placeholders with t() instead. Also elsewhere.
2. "smart_import_set_error("Row #" . $attrib->key . ' ' . $field_name . " is required.");": all user facing text must run through t() for translation. Please check all your strings.
3. README.txt needs an update for the hook name change.

But otherwise looks RTBC to me.

Assigning to sreynen as he might have time to take a final look at this.

Thanks for your contribution, cybernetikz!

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:

• 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 get 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 reviewers as well.