[D7] Node Update Fields

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

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.

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.

I guess this needs review? See the project applications workflow.

Master Branch
It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

At line 84 you are using 'LANGUAGE_NONE'.
If the field translation module is used (or maybe entity translation), this condition might not be true. Check out the field_language() function or the entity_metadata_wrapper here. Otherwise looks good to me, but will take a closer look at it tomorrow.

when module is disabled after use Hook_uninstall delete variable
http://drupalcode.org/sandbox/naiduhsh/2065955.git/blob/HEAD:/node_updat...
http://drupalcode.org/sandbox/naiduhsh/2065955.git/blob/HEAD:/node_updat...

Hey Naiduharish,

Please have a read of https://drupal.org/empty-git-master. Fairly quick one; just need to remove the current master and develop branch's and follow the naming conventions provided.

Please name all of your persistent variables with a prefix of your module name. So node_update_fields_VARIABLE_NAME. This also means updating your implementation of hook_uninstall.

node_update_fields.module, line 41.
variable_get allows the setting of a default value, you don't need to set $result as an empty string.

Having worked with modules which aim to offer similar functionality it can get complicated when dealing with multilingual fields etc. Simple data migration is generally an ad hoc experience but good work.

Cheers,

Dear naiduharish, here are my observations on your project:

node_update_fields.info file:
"description = Allows the settings for updating the common fields of similar nodes."
What do you mean by "similar"? This does not seem clear.

node_update_fields.install file:
There is a large section of the code commented out. Why? Is it intentional? If you don't need to define the schema, just remove the code. The git revision control system helps you to keep track of removed code, should you ever need it again.

node_update_fields.module file:
You read the 'node_update_fields_update_nodes' and 'node_update_fields_field_table_name' persistent variables. It's not clear to me how you intend to use these variables. Please make sure that the code is reentrant and can be used by several users in parallel.

The README.txt file is not very useful. 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

There are also typos:
"Go the" should read "Go to the"

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.

node_update_fields.install:
You can remove the commented code.

node_update_fields.admin.inc:
The variable variable_set('update_nodes', $variable_array); is put in database but not removed at uninstall.
The name of this variable is not prefixed.

Documentation is very poor.
The module seems to not work correctly.

The code basically looks ok but it would be more readable if you can put more spaces between lines. For example the following piece of code looks a bit crammed:

if (array_key_exists($node_type, $update_nodes)) {
      $common_field = $update_nodes[$node_type]['common_field'];
      $common_field_to_update = explode(',', $update_nodes[$node_type]['update_fields']);
      $node_field_value = '';
      $is_prop = FALSE;

      if (is_array($node->{$common_field})) {
        $common_field_table = array_keys($sql_field_table[$node_type][$common_field]);
        list($fetch_value) = array_keys($sql_field_table[$node_type][$common_field][$common_field_table[0]]);
        $field_items = field_get_items('node', $node, $common_field);
        $first_field_items = reset($field_items);
        $field_value = $first_field_items[$fetch_value];

        $node_field_value = $field_value;
      }

@naiduharish
1. On page admin/config/content/common_fields, after saving the settings for the content type,I get the notice for - Notice: Undefined index: sql in node_update_fields_admin_content() (line 25 of /var/www/drupal-7.28/sites/all/modules/node_update_fields/node_update_fields.admin.inc).

2. Git repository link is not correct. It includes your username, therefore asks for your password while cloning.
It should be git clone --branch master http://git.drupal.org/sandbox/naiduhsh/2065955.git node_update_fields

3. Also the project page should have READ DOCUMENTATION link.
4. Also the steps to Use the module should be clear and clear. A better README.txt is possible.
Please fix these.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

Hi @naiduharish,

It's working fine for me.

Thanks
Rahul Nahar

It looks good to me.

I downloaded this and tested it as best I could. Everything appears to be working and I can't identify any issues.

Removing review bonus tag, there is only one review listed in the issue summary? Make sure to read https://www.drupal.org/node/1975228 again and provide links to exact review comments. Thanks!

Hi naiduharish,

Module is not working fine for me. I am attaching configration screenshot.

1: I have created 10 nodes with same title and body content for Article and Page.
2: Then i edited one node and updated Body content. (Checked checkbox : Update common fields for other nodes)

Then i checked all created node content. No one was updated.

Please let me know , if i am missing anything.

Hi naiduharish,

Fix the https://pareview.sh/node/779
There is some error like https://www.screencast.com/t/r0Reem9WTQS

Regards
Satyam

Minor coding standard issues are surely not application blockers, anything else that you found or should this be RTBC instead?

Hi naiduharish,

Please find below listed manual reviews.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

1. Drupal's Confirm form should be implemented on node form as other nodes are also being updated apart from current node.
2. Also on updating the content of common fields, please check if current user has access to update the other nodes (Node-level permissions). Is the module compatible with other node operation modules? for ex. Nodeaccess, etc.

Coding style & Drupal API usage

1. (*) On admin/config/content/node-update-fields page, selected "Common field" value should not be provided as option in "Fileds to update" fields.
2. (*) There will be session timeout issue in case of large number of node updates. In this case, Drupal's batch operation should be implemented.
3. (+) Update the field label of "Fileds to update" to "Fields to update" on admin/config/content/node-update-fields page.
4. (+) Update function comments having in-correct URL paths provided by module.
5. (+) Reset button is not working for the already saved fields on admin configuration form. It can be remove if not required.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

@naidusharish : Please fix the git error.
"There is still a master branch, make sure to set the correct default branch"
https://pareview.sh/node/779

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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