Insert Field Collection

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxdevdude1887178git

Hi Devdude,

It's a pretty good module. Users of Insert module will definitely like it.

filter.class.inc
Probably you should rename this file to insert_fc.filter.inc as current name has a wide meaning.

Most people will probably use this module within the body field (like I just did), consider adding support for 'text_with_summary' as well.

 foreach ($this->node as $name => $field) {
      $field_info = field_info_field($name);

      // We only replace tags in long text input widgets (textareas).
      if ($field_info['type'] == 'text_long' || $field_info['type'] == 'text_with_summary') {
        $values = $field['und'];

        foreach ($values as $i => $value) {
          $replaced_value = preg_replace_callback("/\[\[.*?\]\]/s", array($this, 'posToEid'), $value['value']);
          $this->node->{$name}['und'][$i]['value'] = $replaced_value;
        }
      }
    }

insert_fc.module
There's a general thing here, make some checks to be sure that this actually IS your modules' replacement token:

function insert_fc_entity_to_markup($match) {
  $match = str_replace("[[", "", $match);
  $match = str_replace("]]", "", $match);
  $match = $match[0];

  $info = drupal_json_decode($match);
  $item = field_collection_item_load($info['eid']);

  $element = array();

  $element['content'] = field_attach_view('field_collection_item', $item, $info['view_mode']);

  return drupal_render($element);
}

In my case it didn't replace the position with field collection item id and $info['eid'] was undefined, which resulted in a crash.
Also consider the fact that other modules also use [[stuff]] and the regular expression you're using will actually grab all of the [[stuff]] patterns (even if they don't belong to your module). I'd recommend you to use some identifier for your replacement tokens and use more specific regular expressions.

Also consider using module_load_include instead of require_once, here:

function insert_fc_node_presave($node) {
  require_once drupal_get_path('module', 'insert_fc') . '/includes/filter.class.inc';

insert_fc.js
Make sure you get rid of the debugging code:

   console.log(entity);

Also don't forget to comment your code, check Comment Section here: http://drupal.org/node/172169
And also check JavaScript coding standards (in the same link above).

I would encourage you to create an object Drupal.insertFC and extend it like Drupal.insertFC.insertEntity just to make your functions re-usable and namespaced.

So far so good, all-in-all I think you did a good work and this might really be useful in some authoring experience cases.

Automated review

FILE: ...s/ain/NetBeansProjects/Drupal7/sites/all/modules/insert_fc/insert_fc.js
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 95 | ERROR | Whitespace found at end of line
--------------------------------------------------------------------------------


FILE: ...n/NetBeansProjects/Drupal7/sites/all/modules/insert_fc/insert_fc.module
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 136 | ERROR | Whitespace found at end of line
--------------------------------------------------------------------------------

Manual review

1. Fix the typos in the comments, I was able to locate quite a few.
2. Dependency on field_collection module is broken due to a typo in insert_fc.info. Thus, Drush fails to resolve this dependency on installation.
3. InsertFcFilter::posToEid() is not documented properly, @param and @return are missing.
4. insert_fc_field_widget_settings_form() parameters not documented.
5. README content doesn't match the project page, please sync and also provide the same help info on the project page.
6. $textareas in Javascript is only used once, no need to define it at the head of the code block, you could simply do $('textarea').each(… where it's needed. Same is true for $insert_buttons. Also, when you need to define a local variable, it's generally a good idea to define them on location, where they're actually needed so it improves readability. You're doing it in insertFC.insert().

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

I'm a robot and this is an automated message from Project Applications Scraper.