[D7] Multiple Registration

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.

Hello alexsergivan,

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 : Add hook_help.

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

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (+) Add configure = admin/config/people/multiple_registration in info file.

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.

Regards

Hi alexsergivan,

In the create new registration path form when i add a url for a specific role then when I navigate to the path it says forbidden. Need to know the exact use of this path.

If you are using this path for field settings page then you can have checkbox / radios saying "yes" or "no" to show the role in field settings form for displaying of field in user edit form.

Thanks

Multiple Applications

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

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

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

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.

@alexsergivan,

Thank you for your contribution this a very nice module and very handy.

I run the coder module and here are all the complains:

• Line 214: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized. '#title' => t('!role Registration page path', array('!role' => $roles[$rid])),

• Line 403: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
              '#description' => $descr,

I took an screenshot of the coder complains in the attached file.

You should fix these errors.

A personal recommendation is to change the name from multiple_registration to Multiple Registration in the .info file (this is a personal recommendation, this is not required.).

@alexsergivan,

Everything seem to be working fine to me.

Thank you for your contribution.

manual review:

1. "define('MULTIPLE_REGISTRATION_ADMIN_RID', 3);": this can be problematic on many sites, since there is no guarantee that the admin role ID is in fact 3. I think you should rather use variable_get('user_admin_role') to retrieve the admin role ID on the site. For example every site using https://www.drupal.org/project/role_export will have a different admin role ID, so you can get into security trouble pretty fast if someone adds a registration page for that role.
2. multiple_registration_theme_registry_alter(): why do you need this? Please add a comment. I think you should rathe ruse hook_form_FORM_ID_alter() to change the user roles admin page.
3. multiple_registration_settings_form(): do not call theme() in here just return a nested render array in the end. See https://www.drupal.org/node/930760
4. multiple_registration_create_registration_form(): do not call arg() here. Use 'page arguments' in hook_menu() to pass in the number from the URL as function parameter. See https://api.drupal.org/api/drupal/modules!system!system.api.php/function...
5. multiple_registration_remove_registration_form_submit(): don't use drupal_goto() in form submit handlers, use $form_state['redirect'] instead. See the docs on https://api.drupal.org/api/drupal/includes!form.inc/function/drupal_redi...
6. multiple_registration_form_alter(): instead of mixing this different use cases into one function you should use hook_form_FORM_ID_alter() instead. See https://api.drupal.org/api/drupal/modules!system!system.api.php/function...
7. multiple_registration_form_alter(): the check_plain() on $descr is wrong here, since no user provided input is involved? You hard code the translatable string, so it is trusted and cannot contain malicious text. Make sure to read https://www.drupal.org/node/28984 again.
8. multiple_registration_module_implements_alter(): why do you need to move your hook to the bottom? Please add a comment.
9. There is a lot of arg() usage which can be very fragile whenever the form/page is displayed in another context, so you should always pass around the user ID where possible. arg() should never be used, really.

But that are not critical application blockers, so ...

Thanks for your contribution, alexsergivan!

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.