[D7] Webform node insert

First thing to do would be to use the automated code review at ventral.org

http://ventral.org/pareview/httpgitdrupalorgsandboxmrasia1696484git

You can see that there's some minor whitespace/coding standard issues in your sandbox.

The other problem is that you're supposed to use a branch called 7.x-1.x instead of "master", even if your module is only for D7 at the moment.

http://drupal.org/node/1127732

Then work through the checklist here - http://ventral.org/pachecklist

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also create a README.txt that follows the guidelines for in-project documentation.

An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

enough major issues for needs work.

Here's my thoughts on the code (ignoring that the comments are all in a weird format, see the automated review in my last comment)

- Maybe you should add the webform module as a dependency. It seems harmless enough with webform disabled but why not make sure the module gets cleaned up when webform gets disabled?
- Currently you don't have an .install file, and so you aren't cleaning out your variables from the database on uninstall. You can just use variable_del() once for each of your variables in hook_uninstall().
- There's a spelling mistake in one of your variables 'webform_node_insert_contet_type', you mean "content_type" right?
- Use full stops at the end of your descriptions on the admin form, ie. "Content type to store Webform submissions."
- You really need a README.txt file. It can be the same as the description on your sandbox page.

Hi, this looks like an interesting module.

• in webform_node_insert_menu you set the access_callback for the page to be "node_access " and pass the arguments "update", 1,... so if I understand this right, the $node is missing here, because your path is "admin/config/content/webform-node-insert", so arg 1 here is "config". Also, do you really want to allow users with "update" privilege for a certain node to configure your module? I suggest you to implement a permission like "Administer webform node insert".
• It's somehow unclear what are you doing in webform_node_insert_webform_submission_insert in the first if block. At first sight I read the code like that: If we don't have a title, load the node to $node_title and then assign $node_title->title to $node->title

Doh... I keep forgetting to change the status. Sorry.

Hi,

• You aren't documenting your hook implementation functions correctly. See this section for proper Drupal documentation standards of hook implementations and theme preprocess functions.
• For your hook_menu() implementation, use user_access, not node_access for your access callback. I got PHP errors immediately upon enabling your module and wasn't even able to view the admin page for your module.
• For watchdog messages, you're not supposed to have dynamic variables in the message, you're supposed to keep them translatable. Re-read the documentation for watchdog().
• I'm getting PHP errors every time I try to submit a webform that I've set up to create a node with your module.. even with the simplest content type with one textfield and a webform with one textfield. Do you have a use case scenario you can include in your issue summary that doesn't result in fatal errors?
• The admin page for your module is not very usable. In my opinion, you should create a more elegant admin page where the user can specify any number of webforms linked with any number of content-types. Then, allow the user to link webform components with their respective content-type field. That would be very useful to some people.

hi... This module work fine with webform module and it successfully creating the nodes. Not major issues but you need to check it.
1.

  $nodes = node_load_multiple(array(), array('type' => 'webform'));
  // Get node id & title.
  foreach ($nodes as $key => $node) {
    $node_names[$key] = array(
      $node->nid => $node->title,
    );
  }

try to avoid error when no webform exist so define empty array for $node_names = array();

2. you can optimize webform_node_insert_field_data($fields) as the $instance variable is defined twice.

3. When i edit a webform submission it is not reflected in the node content and can you make it working, if i edit a webform submission also it will create nodes if not exist. I need to implement it in sites where i already had submissions.

Needs work based on #10.

thanks for the great module.

i have success implement for creating field into content type with your module.

but i had a problem when inserting ‘term reference’ field select option by implementing hook_webform_select_options_info.

if i choose taxonomy in select option webform field, then your module always create ‘text’ field into content type.

how can i get ‘term reference’ field in my content type?

In your webform_node_insert_form_submission_import() you use $node->language to select the language code of the fields.

This will not always work, since node language and field language might often be different (for example when using term references).

The proper way is to use the function field_language() to get the language for the field instance or use and entity_metadata_wrapper() to handle it.

Also, it still doesn't comply with all coding standards:

FILE: ...les/pareview_temp/test_candidate/includes/webform_node_insert.admin.inc
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
92 | ERROR | Whitespace found at end of line
92 | ERROR | Function comment short description must be on a single line
--------------------------------------------------------------------------------

FILE: ...tes/all/modules/pareview_temp/test_candidate/webform_node_insert.module
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
137 | ERROR | Parameter comment indentation must be 2 additional spaces at
| | position 1
138 | ERROR | Whitespace found at end of line
140 | ERROR | Parameter comment indentation must be 2 additional spaces at
| | position 2
--------------------------------------------------------------------------------

The problem at line 137 and 140 is that you have an extra space in the indentiation. The paramter descriptions should be indented two spaces more than the line above.

The extra language support does seem like a feature request rather than a definite release blocking bug... esbenvb should probs open a ticket in the issue queue on the project so it isn't forgotten and can be included later.

While you're right that minor issues on Ventral shouldn't block an application from being approved, the D.O. standards *are* important. In this case incorrectly formatted and poorly documented code has been cited as reasons to postpone promotion of this sandbox to a full module a few times already, so it would make sense to spend a few minutes running some automated tests yourself before each time you set this thread back to "needs review".

The fact that the whitespace errors are still there after multiple rounds of documentation and style revisions does kind of indicate to us that you either regularly forget, don't know how, are dismissive of or are too lazy to follow a basic, automated and well documented QA process before submitting your work publicly as "finished". As reviewers we want to get excited about new projects and new developers getting involved with the Drupal contrib community, so it's just a little disappointing when the test bots come back with fails on really simple, totally avoidable things.

Take things a little slower if you have to in order to get it right for this application, it's not a race :)

You don't have to use the hosted Ventral tool to review your code during active development as that can be a very slow and frustrating way to work. Just run it once at the end of each development cycle. If you haven't already, I highly recommend you configure something that works within your text editor so that automated reviews can be bound to a keyboard shortcut, like http://drupal.org/project/drupalcs.

Hi hapax legomenon,

For below code lines:

  node_save($node);
  if ($node) {
    watchdog('webform_node_insert', 'new node @node has been created.', array(
      '@node' => $node->title)
    );
  }
  else {
    watchdog('webform_node_insert', 'node @node could not be created.', array(
      '@node' => $node->title)
    );
  }

Is it correct using $node object to indicate a node save or not？ Or you may use $node -> nid to
decide a node save or not.

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

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

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. webform_node_insert_access(): why do you need to check for two permissions here? The check for the 'configure webform node insert' in hook_menu() should be enough? Please add a comment.
2. Project page is a bit short, see http://drupal.org/node/997024
3. "t('name') => $content_types->names,": do not translate array keys that are not visible to the user.
4. webform_node_insert_global_settings(): HTML tags should be outside of t() where possible. And the link in the help text will not work if Drupal is installed in a subdirectory. Please use url() for the href instead.
5. _webform_node_insert_join_webform_component(): instead of running multiple SELECT queries you could just use the IN operator for the nids? Same in _webform_node_insert_node_sid().
6. "'#default_value' => check_plain(variable_get('webform_node ...": do not use check_plain() for #default_value in forms, that will be automatically sanitized. Please read http://drupal.org/node/28984 again.

Setting this back to "needs work", you need to know where to use check_plain() and where not. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll 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" (found some problems with the project) or "reviewed & tested by the community" (found no major flaws).

I stumbled onto this while working on a simple node insert for a webform. It's actually pretty useful in my use case scenario, hope you keep working on it :)

For anyone else stumbling upon this, here is a better alternative: https://www.drupal.org/project/webform_rules