[D7]Field From Field Collection

Link to the project page and git clone command are missing in the issue summary, please add them.

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,

Manual review :

1. explode() already return an array no need to set $values = array(); .module Line 287
2. Your Readme.txt doesn't explain how to use your module. Maybe you could add more description
3. $langcode = 'und'; You shloud use LANGUAGE_NONE instead of 'und'

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxRajabNatshah2052323git

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

Just a quick code review.

• Your project page has typos, etc.
• Your git command here is your personal clone. Fix that.
• Use l() instead of writing "a href" manually.
• Do you really need to translate variables? t('@node_title', array('@node_title' => $node->title))
• Fix your git branches: https://drupal.org/empty-git-master

Rajab,

Here are my observations:

field_from_field_collection.module file:
Typo in the 'field_from_field_collection_defualt' array key on line 32 and line 84 (it should be 'field_from_field_collection_default')
Some of the capitalization of the human-readable strings is erratic and inconsistent. For example, in 'Node Title and Field Collection Title Linked to content', why are 'Title' and 'Linked' capitalized?
The variable $fied_value_markup has a typo. It should probably read $field_value_markup.
On lines 100 and 129 there are spaces before commas.
The expressions between lines 343 and 352 seem very complicated. Consider breaking them into smaller pieces. The code would also run faster because several of the subexpressions are repeated and don't need to be evaluated more than once:

    $field = $form_state['field'][$element['#field_name']][$element['#language']]['field'];
    $field_values = $form_state['values'][$field['field_name']][$element['#language']][$element['#delta']];
    if (isset($field_values['fc_value'])) {
      $values = explode('_', $field_values['fc_value']);
      $nid = (integer)$values[0];
      $fc_id = (integer)$values[1];
      $form_state['values'][$field['field_name']][$element['#language']][$element['#delta']]['nid'] = $nid;
      $form_state['values'][$field['field_name']][$element['#language']][$element['#delta']]['fc_id'] = $fc_id;
      form_set_value($element, $field_values['fc_value'], $form_state);
    }

Ditto for this code at lines 380-381:

            $form_state['values'][$field_collection_field_name][$element['#language']][$delta_key][$field_collection_nested_element_field_name][$element['#language']][$field_delta_key]['nid'] = $nid;
            $form_state['values'][$field_collection_field_name][$element['#language']][$delta_key][$field_collection_nested_element_field_name][$element['#language']][$field_delta_key]['fc_id'] = $fc_id;

There are typos in the field_from_field_collection.info file:
"a list of..." (capitalization)
"So that…" (capitalization)
Also, perhaps this description can be made more clear.

The README.txt file is not very clear. What are the main features and advantages of this module? There are also several typos. I recommend that you write an explanation that can be easily understood by a non-expert Drupal user.
See also:
https://drupal.org/node/447604
https://drupal.org/node/161085
http://drupalcode.org/project/bot.git/blob/HEAD:/README.txt
http://drupalcode.org/project/admin_menu.git/blob/refs/heads/7.x-3.x:/RE...
http://drupalcode.org/project/actions.git/blob/HEAD:/README.txt

Comments, both inline and function should limit to 80 characters: https://drupal.org/coding-standards/docs#drupal
Function parameters and return type should be noted, except for the listed exceptions: https://drupal.org/coding-standards/docs#functions

There are still a lot of errors and warning returned by the PARreview: http://pareview.sh/pareview/httpgitdrupalorgsandboxRajabNatshah2052323git

.module file:
The 'item' variable used as a value by a foreach loop is already used in the same way by the outer foreach loop. (at line 86)

1. Basic application checks

1.1 Ensure your application contains a repository and project page link.
Yes
1.2 Ensure your project is not a duplication.
Unclear
1.3 Ensure you don't have multiple applications.
This is this user's only application

2. Basic repository checks

2.1 Ensure the repository actually contains code.
Repository contains code
2.2 Ensure you are working in a version specific branch.
Code is in a version specific branch
3. Security Review

3.1 Ensure the project does not contain any security issues.

In _field_from_field_collection_get_list_of_field_collection_data, you query nodes, but you don't respect node access tags. That MAY be a security problem.
Other than that, I didn't notice any, except the potential security problem of using $form_state['input'] directly.

4. Licensing checks

4.1 Ensure the repository does not contain a ‘LICENSE.txt’ file.
There is no LICENSE.txt file.
4.2 Ensure the repository does not contain any 3rd party (non-GPL) code.
Project does not contain 3rd-party code.

5. Documentation checks

5.1 Ensure the project page contains detailed information.

The project page seems to be fairly detailed.
5.2 Ensure the repository contains a detailed README.txt.

The README.txt does exist, and it reflects about the same information as the project page. However, I would like to see more detailed information on use-cases in the documentation.

5.3 Ensure the code contains a well-balanced amount of inline-comments.

A lot of the code is not well commented, and it does not self-comment, so sometimes it is non-obvious what it is doing.

6. Coding standards and style

6.1 Run an automated review and ensure there are no major issues.
FILE: .../www/drupal-7-pareview/pareview_temp/field_from_field_collection.module
--------------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
--------------------------------------------------------------------------------
234 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
396 | ERROR | Doc comment long description must start with a capital letter
397 | ERROR | There must be exactly one blank line before the tags in a doc
| | comment
417 | ERROR | There must be exactly one blank line before the tags in a doc
| | comment
470 | ERROR | Type hint "opject" missing for $field_collection
--------------------------------------------------------------------------------

These should probably be fixed, especially using raw $form_state['input'].

7. API and best practices Review

7.1 Ensure you are using Drupals API correctly.
Aside from using $form_state['input'] and not respecting node access, the APIs appear to be used correctly.

Specific Issues I Have:

A fair amount of the code could be refactored to use function names, variables and indices that self-comment. For instance, several of the callback functions have 1drop as part of their name. Is this an allusion to Drupal, or does it mean something in context?

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2310391

Project 2: https://www.drupal.org/node/2065565

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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