For Drupal7
This module is for checking whether user email address has been taken on user register form,by ajax.

Link to sandbox project:
http://drupal.org/sandbox/meadhu/1778580

Git repository:
git clone http://git.drupal.org/sandbox/meadhu/1778580.git email_ajax_check

Comments

cubeinspire’s picture

Great module! I've tested it and works as expected.

Only two issues I see.

One is that you still have some code standards problems:
http://ventral.org/pareview/httpgitdrupalorgsandboxmeadhu1778580git

The second one:
There I see a security issue as it takes the email address from $_GET without checking the source.
This could allow an external script exploit to fetch the registered users email addresses (including User1) from the database. I would add a pair key-hash to avoid that problem with a hash code on the configuration panel.

Otherwise great module I will add it to my Drupal Toolbox.

teknic’s picture

I like it overall, just a few things.

1. Your working out of master, you should have a version specific branch. 7.x-1.x etc.
2. Your module folder is called ajax_check_email but all your files are called email_ajax_check. You should pick one and rename files / functions accordingly.
3. You have a .info file called ajax_check_email.info that probably should just be deleted.
4. _email_ajax_check_is_user_exists uses one way of specifying the SQL query, then the next function (email_is_blocked) uses a different way of specifying the SQL query. How come? I think its better to standardize and pick which way you want to go.
5. You should add "Implementation of hook_form_alter()" comment to the form alter in the .module file

ANDiTKO’s picture

Status: Needs review » Needs work

Hello deapge and thanks for your work.

I tested your module on my local server and it works well. I also tried it with the module LoginToboggan and it seems to be working as expected.

However, when a user disables java script your module dose not work. But thats fine because it dose not break the registration functionality. Im just pointing that out so you can maybe figure something out about the users that have disabled java script or don't support it (like mobile devices).

I am changing the module status to "Needs work" so i can save the other reviewers some time.
Please work on the suggestion that the other reviewers reported and when you are done change the status back to "needs review".

Good luck!

deapge’s picture

Thanks for your reply. I will fix these issue on next version.

deapge’s picture

Assigned: Unassigned » deapge
Status: Needs work » Needs review

Hi,all
Please review this module,on previous version I fixed some issues.

fr3shw3b’s picture

Priority: Normal » Minor
Status: Needs review » Needs work

Hello,


Good work.


I tested it and it works perfectly,
I've seen no other issues apart from a few minor things in the info file.


The packaging information at the end isn't needed,
Do you need the dependency user stated?


User is a part of Drupal core and you need it to be able to use Drupal.


One other thing for future reference you don't have to add inverted commars
to the name and description in the info file, obviously it doesn't make a big difference
but I thought I'd let you know.


Thanks,

deapge’s picture

Assigned: deapge » Unassigned
Status: Needs work » Needs review

Hi,fr3shw3b
Thanks for your advice.
fixed.

klausi’s picture

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

deapge’s picture

OK. Let me look at review bonus.

deapge’s picture

Issue summary: View changes

Change git clone

fr3shw3b’s picture

Status: Needs review » Needs work

Hello,

My first thought is that you need a .install file to implement the hook_uninstall() function to use variable_del to remove the variable created in the settings form.

You need to use comments in the JavaScript file, firstly to explain what the file does and then to help explain the code activity within.

I would of thought it would be better to use Drupal's variables set and get methods as oppose to PHP $_GET or you could use the drupal static cache as it is called during the input to store the email address and get it for the email_ajax_check_callback.

the $mail variable in email_ajax_check.unique.inc doesn't need to be checkplained twice, once is enough as the variable has not been entered between both the checkplains.

mrharolda’s picture

Just checked out your module: looks promising!

First suggestion: use .change() (or .blur()) instead of .keyup(). Or at least provide an option to disable checking half-typed email addresses ... see next comment ...

mrharolda’s picture

I've added a couple patches for bug reports and feature request in the sandbox's issue queue: http://drupal.org/project/issues/1778580?categories=All

mrharolda’s picture

I'm also up for (co)maintainership, since I almost rewrote the entire module ;)

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

LordOfTheUniverse’s picture

Hi,

I am also using this module. It works fine. But its not working on edit user profile.
Although I am trying to add this on Edit profile page.

If anyone worked on same..let me know.

Thanks,

LordOfTheUniverse’s picture

Issue summary: View changes

add git repository