[D7] Quick registration

Hello,
there are some issues with coding style, that you can find here:
http://ventral.org/pareview/httpgitdrupalorgsandboxmlebedev2039475git
(or installing the coder module).

Manual review:
- missing an help link for the module list. You can implement it with:

function quick_registration_help($path, $arg) {
  switch ($path) {
    case 'admin/help#quick_registration':
      $output = file_get_contents(drupal_get_path('module', 'quick_registration') . '/README.txt');
      return nl2br($output);
  }
}

- the "I am" field should fetch the list of options from the chosen field rather than hard-coding them, as if values differs nothing is stored in the profile field.

Welcome!
I'm a little confused about how do you make changes to the form - for example, add or remove fields, rearrange them, etc.
Modules doesn't seem to create first and last name fields, the gender field, birthday field, etc. Could you explain the installation part ?
I could not test the module yet but looking at the code, here are some changes you might want to make before getting this the RTBC flag.

quick_registration.info

- Remove version = 7.x-1.x-dev. Drupal.org will add it automatically from the repo tags.

quick_registration.module

- Do not use t() function in hook_menu title parameter.
- .. ditto.., description parameter.
- Your module does not seem to honor the global user registration switch that admins can choose registrations to be "Public registrations", "admin only" or "public, administer must approve the account" setting in user settings page.

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.

Thanks for your hardwork!
I'm sorry I didn't mention these in my previous review, but here are some minor points:

quick_registration.module

- In line 82, 265: I think it would be better if we use $form['#attached'] to attach necessary CSS files. drupal_add_css will do the job, but #attached will ensure assets are included even from a form rebuild from cache.
- Line 260,261: You have used the !link placeholder but the more secure one is @link. Changing !link to @link in both places will do the job. (!link will offer no sanitizing. Even though a simple path is OK, lets follow the best practice)
- Line 220: 'und' is the value of LANGUAGE_NONE constant. Please use this constant instead of this 'und'.

Great job adding configurable fields and the ability to create new ones! Looks like a really nice module to me!
I'm not setting the status to "needs work" because these are not serious stuff. Hopefully you can fix them too, and a couple of people will review this as well, so we can move to RTBC status :)

Wish you all the best!

Thanks! I'm just a volunteer helping others to keep the project queue low. Usually 2 or more people will get involved in the thread, review the code and if everything is OK, the status will be set to RTBC. Klausi or some other administrator will give the final approval and your account will be upgraded to a role that you can uncheck "this is a sandbox project" checkbox which promotes your module to an actual project. For your future projects, you can freely promote your work as a real project instead of going through this approval process again.

Hi,

I went through your code and found an issue -

.install file
- all variables have not been deleted

All else seems fine. Good work

When the form is submitted, $form_state['values'] will contain 4 extra values security purposed, and the module should not save them in the database (storing much data in the variable is not good too).

Try this:

form_state_values_clean($form_state); // will be passed by reference. 
variable_set('quick_registration_set', $form_state['values']);

- form_state_values_clean()

Hi m.lebedev,

I installed this module and tried its functionality. I can confirm this working well, but I found 2 changes that can be made.

1. In 'quick_registration_settings_form', you can use l() to construct the link instead of href there.
2. Git clone creates a folder with the name '2039475', it would be better if you can change this to your module's name.

Nice work though!

The minor link issue is not an application blocker, anything else you found?

You cannot determine the folder name for git clone of a sandbox, it is imply a number until it gets promoted to a full project. You can specify a folder name yourself after the git clone link.

Hi m.lebedev,

it would be helpful to mention that the email address will be the username after quick registration. Could be important to know because normal users can't change their username by default and perhaps their wondering what to type in the field "username" at the login box.

You should add a hint to your README.txt and project page. So the administrator will be aware about this issue. Perhaps you describe how to grant users or user roles to change their username, too.

Kind regards

Hi,

I tested and reviewed your module and have some thought.

- The field "I am" is probably better to name "Gender" from a general use perspective.
- The way fields can be created from the Quick Registration settings form is, however fancy and convenient, a little not-the-drupal-way for those who is used to the normal field creation forms. My suggestion here is to remove that bit and instead give proper instructions on how to incorporate additional fields.
- Also, the field Birthday gets created but is not incorporated in the registration block form.
- If the "quick" fields creation should still be in the quick registration settings form, it might be user friendly to remove the checkboxes for already created fields?
- The dependency on the date module seems odd from a general use perspective.

This scenario gave an error:
1. created the fields in the Quick Registration settings page.
2. Set First name - First name, Last name - Last name, I am - I am, Birthday - Empty (couldn't assign)
3. One user with name Martin already existed.
4. Created another user with same name gave the following error:

"PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'martin' for key 'name': [..]"

Pareview gives two errors about unused global variable $user. (http://pareview.sh/pareview/httpgitdrupalorgsandboxmlebedev2039475git)

Possible Duplication

It seems that Email Registration provides very similar functionality. Could you describe how your module differs? I think Drupal core already provides a registration block, and core profile allows you to add profile fields. What other features does this module have besides allowing registration with just an email address?

We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org. If the differences between these modules are not too fundamental for patching the existing one, we would love to see you joining forces and concentrate all power on enhancing one module. (If the existing module is abandoned, please think about taking it over).

If that fails for whatever reason please get back to us and set this back to "needs review".

----
Top Shelf Modules - Crafted, Curated, Contributed.

Email Registration is actively maintained, has a single goal and does it pretty well already. Don't get me wrong, I'm all for collaboration just that I fail to see anything useful here. m.lebedev you emailed me although I really don't understand what it is you want. Cheers, Chris.

As co-maintainer of e-mail registration module I confirm that this application is not duplication.
Chris already pointed that email_registration exists for single and simple purpose that done good and there's no reason to extend it with application specific features.

So I approve this. Also I know Michael personally (we was co-workers) and I glad to support his efforts to contribute - he has great skills to find bugs and write clean code.

PS: @kscheirer suppose greggles have the same opinion see #583152-1: email confirmation option

Ok then, I'll leave it in RTBC for someone else to review.

Hi m.lebedev,

Really nice module you got here. Found 1 minor issue though. Gender doesn't seem to be recorded when I checked the new user registered using Quick registration block. I checked also the database table, gender value is also NULL. You can see what I'm talking about in the screenshots provided:

User edit page: http://imtp.me/68m901lbp.p
field_data_qr_gender: http://imtp.me/68md01lbp.p

Also, I think it would be better if the default gender choices is changed to "Male" and "Female" instead of "Man" and "Woman". Just my opinion.

I did not receive any private message. Anyway, here's what I did on my tests to reproduce the error:

1. Activate the module
2. Configure module settings using default.
3. Added QR block to left sidebar.
4. Using another browser, register using the QR block.
5. Go back to the browser that's logged in as admin and go to edit user page of new user. You can see that Gender is not set.
6. Open phpmyadmin and go to the gender field table. You can see the new user there with no Gender set.

Hope that helps.

It's been a month without any problems reported, so I'm promoting this myself as per https://drupal.org/node/1125818.

Thanks for your contribution, m.lebedev!

I 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.

----
Top Shelf Modules - Crafted, Curated, Contributed.