[D7] Workbench Allocation

Automated Review

Pareview shows some issues please fix those: http://pareview.sh/pareview/httpgitdrupalorgsandboxstephenb0012462031git

Manual Review

• There is a .gitignore file in your repo. consider removing it
• Also consider using "switch" instead of lots of "if statement" (If possible)

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.

Automated Review

All is fine.

Manual Review

README.txt/README.md

You say activate workbench for each content type so I went to the content type first and could not find anything. Consider a better project page and README.txt.

Coding style & Drupal API usage

Release blocker:
You should delete all the variables you created with the module using hook_uninstall() function and variable_del().

Recommendations:

workbench_allocation.install line 5 has a small typo with "assigment".

consider adding a configuration section on the module section by adding "configure = /admin/config/workbench/moderation/allocation" in your .info file.

workbench_allocaiton.module

line 623
you assign $new_moderation_state = $current_state but immediately override it in the next statement. You can just remove the line completely as you assign it in the if statement. See http://stackoverflow.com/questions/4110340/variable-scope-issue-with-if-...

line 773
typo "assignements"

function workbench_allocation_node_revision_delete() Is it not better to remove the function is it is unused?

function _workbench_allocation_get_assigned_fields() returns an array or false. Which mean in the function _workbench_allocation_get_user_tids() you could be looping a FALSE (line 736). This caused some errors for me.

I have not managed to get your module working and tested. This maybe down to me not knowing Workbench in detail but I feel that you could improve the documentation to explain in detail the setup needed. I say this because I am getting errors in the code due to (I think) an incorrect setup. What would be helpful it to prevent the errors from occurring like in _workbench_allocation_get_user_tids() and try to present the user with a suggested fix.

Nice work, I was looking for something like this, but this module will not work for my project because we group users together using OG. I am not sure if other projects will need to group users using the taxonomy module, but most use OG. It would be helpful if you can add a feature that will allow a user to assign nodes to users in OG too. Also please add a configure link in the info file.

Automated Review

This module pass the automatic review.

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. But I did get confused as I was expecting the content type settings to appear in the Content Type edit page after reading the Readme.

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

None found.

The feature request above (integration with Organic Groups) shouldn't stop this from hitting contrib. It would be a good idea to start thinking about OG, potentially raise a feature request ticket for this in the queue.

In you .install file you do not need to call drupal_uninstall_schema() it is already handled
https://www.drupal.org/update/modules/6/7#install-schema

Also you define 3 constants. WORKBENCH_ALLOCATION_TYPE is not used and the other two are only used to make an array seem a little more readable. Is it not better to reference the index directly? The contents of the array are implied since you assign them to department_id and uid. If I have missed out the reasoning to keep them please do.

Thanks for your contribution!

I updated your account so you can 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 stay 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.