Commerce Product Clone

Automatic review of the 7.x-1.x branch:

• Drupal Code 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.

PAReview details available here: http://ventral.org/pareview/httpgitdrupalorgsandboxagileadam1540050git

Manual review of the 7.x-1.x branch:

• commerce_product_clone.module
  • You have some UI strings not using t() in commerce_product_clone_menu()

Aside from this looking good to me, don't see anything else of concern other than that already noted in PAReview, works well. Unfortunate that the node_clone module is for nodes only, otherwise this would be a nice extension. A decent solution until entity_clone comes around.

There aren't any major issues in the automated report,
also the "manual review" is wrong: DO NOT use t() within hook_menu().

Therefore changing back to needs review.

Sorry agileadam, you're right about hook_menu and t().

Re line-endings looks like you have 2 newlines instead of 1 at the end of .module/.inc, but as patrickd mentioned, not a major issue.

+1 for RTBC.

commerce_product_clone.module:77
$access = user_access('clone product') || ($user->uid && ($product->uid == $user->uid) && user_access('clone own products'));

I would suggest using "===" instead of "==" to make sure you're not accidentally comparing against a boolean value. More of a suggestion though then an issue...

Updated git clone information; minor description changes.

Everything looks pretty solid to me. My testing of the functionality didn't show any errors and reviewing the code the only thing I see that looks out of place is that there are 2 blank lines at the end of the .module and .pages.inc files instead of just 1.

Other than that the only suggestion I can offer is that it'd be nice from a usability standpoint if hitting "save and add another" when on the clone tab it would prefill the information again rather than taking you to a blank product screen.

I've installed this onto my test site based on Commerce Kickstart 2. However, although I can see and change the Clone configuration and the clone permissions, I can't find any "clone" tabs, buttons or drop down lists on any of the product pages or lists. I'm not sure what I should be seeing, other something like the images on the sandbox page, but should there be an additional "Clone" tab for each product page along with "View" and "Edit"?

With the Kickstart 2 configuration the product list does not appear to be as the images on the sandbox page. The options by each product are in a drop down list which has the options: "Quick Edit", "Edit" and "Delete" only. This has not changed after the module was installed. Any ideas how to get this working?

Final review.

Congratulations: your are now a vetted git user and from now on you are allowed to create full projects.

Some minor notes:

• "'admin/help#commerce_product_clone'" in function commerce_product_clone_help() should be "'admin/help/commerce_product_clone'"
• After saving a (new) cloned product you are redirected to admin/commerce/products/%product_id/clone again and not to the new product or the product listing.

But these notes do not block your application, they are just simple hints to make your great module a little better :)

manual review:

• "drupal_set_title(check_plain($product->title));": no need for check_plain() here, D7 drupal_set_title() will run that automatically for you. And double escaping is bad.

But that is surely no blocker and the approval is fine.

@stBorchert: please also use the template from http://groups.drupal.org/node/184389 when approving applications:

Thanks for your contribution, agileadam!

stBorchert updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

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

Thanks, 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 to the dedicated reviewer(s) as well.

After reading through http://api.drupal.org/api/drupal/modules!help!help.api.php/function/hook... again, I think I used the right path for the help text. Maybe I'm misunderstanding the docs?

No, sorry. I was wrong.
Help module is using # to identify the module via path and creates the list of modules without using the path specified in the implementations of hook_help(). This path is simply used to identify the module.