[D7] User Reassign

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

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.

Question: why is this better/different from VBO?

Here are the issues I found (in no particular order):

1. README and module contain grammar/spelling/other errors. README: allowes should be allows; thats should be that's; the module name is incorrect; "going te be removed and" should be "going to be removed, and"; you have a required module section with no required modules; You state "When enabled, the module will prevent the links from appearing. To get the links back, disable the module and clear caches." What links are you referring to? There are a few other issues, but that's about it. You should also consider using no more than 80 characters per line. Module description: "module for reassigning user to another user with the former users content" should be "Module for reassigning content from one user to another" or something similar. Also you have empty @params.
2. Remove "version" from the ./user_reassign.info file, it will be added by drupal.org packaging automatically.
3. Looks to me like your module interacts with the l10n_update module, so you might want to note that.
4. You might want to think about altering the user-multiple-cancel-confirm form. You can delete a single user with this form.
5. I wonder if completely removing the option to assign content to anonymous is warranted?
6. I did not receive an error when I entered an incorrect user name. Instead, I received the following message: "PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'uid' cannot be null: INSERT INTO {tracker_user} (nid, uid, changed, published) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 90 [:db_insert_placeholder_1] => [:db_insert_placeholder_2] => 1471515057 [:db_insert_placeholder_3] => 1 ) in _tracker_add() (line 276 of modules/tracker/tracker.module)."

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.

Automated Review

Several issues found by pareview.sh here: https://pareview.sh/node/938

1. Add parameter type in user_reassign.api.php for better non-hook documentation. Example: * @param int $uid
2. In README.txt, Limit lines to no more than 80 characters.
3. In user_reassign_ldap.info, remove version and file[] as you do not need to declare the module file, it will be discovered automatically by Drupal.
4. In user_reassign.info, you do not need to declare the module file, it will be discovered automatically by Drupal.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

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

While similar in functionality to Views Bulk Operations, that module does not tie directly into the cancellation workflow, making it hard for individuals to manually find and reassign content before cancelling the user account. This module reduces an arduous process (especially for large sites) into a one-step process.

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

Yes: Follows the guidelines for in-project documentation and/or the README Template.

See the issue identified by pareview.sh in my automated review.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

See automated review feedback.

Additional Feedback

You should not use the global variable $user in user_reassign_user_cancel(). It is reserved by Drupal. While I was not able to find any documentation on this fact, this could expose the module to potential security issues if a user with sufficient privileges is able to become any user. Update that function like so:

/**
 * Implements hook_user_cancel().
 */
function user_reassign_user_cancel($edit, $account, $method) {

  // Get user uid by username.
  $new_account = user_load_by_name($edit['user_cancel_assign_user']);
  $uid = $new_account->uid;

  // Hook for changing the uid.
  drupal_alter('user_reassign_uid', $edit['user_cancel_assign_user'], $uid);

  if ($method == 'user_cancel_reassign') {
    // Assign nodes to any user.
    module_load_include('inc', 'node', 'node.admin');
    $nodes = db_select('node', 'n')
      ->fields('n', array('nid'))
      ->condition('uid', $account->uid)
      ->execute()
      ->fetchCol();
    node_mass_update($nodes, array('uid' => $uid));
    // Update old revisions.
    db_update('node_revision')
      ->fields(array('uid' => $uid))
      ->condition('uid', $account->uid)
      ->execute();
    // Clean history.
    db_delete('history')
      ->condition('uid', $account->uid)
      ->execute();
  }
}

@scott.allison : Looks like you forgot to change the status. Is this now RTBC after your review or are there application blockers left and this should be "needs work"?

Automated Review

Found some issues from pareview.sh : https://pareview.sh/node/938

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

Yes: Follows the guidelines for in-project documentation and/or the README Template.

README.txt : Few lines excedding 80 chars per line limit.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

Just minor coding standard issues as notified in https://pareview.sh/node/938

Overall looks good to me.

This review uses the Project Application Review Template.

Thank you for your contribution!

I updated your account so you can opt into security advisory coverage now.

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 stay involved!

Thank you, 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 go the dedicated reviewer(s) as well.