Hi, I'm implementing hook_email_registration_name() in my own module, and noticed that if the username I'm passing in has already been generated, an error is thrown and the user creation fails. I think it would be better if the module checked for an existing username when using the hook, the same way it does when the module itself is generating the username.

I've moved the duplicate name check out into its own function to avoid code duplication. Patch to follow momentarily.

Please take a moment to review this if you can. It's my first contributed patch, hope I've done it right!

CommentFileSizeAuthor
#1 email_registration-hook-duplicate-1434774-0.patch2.28 KBkris_mcl

Comments

kris_mcl’s picture

kris_mcl commented
Status: Active » Needs review
StatusFileSize
new email_registration-hook-duplicate-1434774-0.patch2.28 KB

Here's my patch. I've tested it using the following steps:

1 - In a custom module, implement hook_email_registration_name()

/**
 * Implements hook_email_registration_name();
 */
function mymodule_email_registration_name($edit, $account) {
	// Set account's user name to first name (field_firstname was added to core account fields)
	return $account->field_firstname['und'][0]['value'];
}

2 - Create a new user with first name Joey
3 - Create a second user with the exact same first name
4 - Note that the second user's username will be set to "Joey_2".

Without this patch, you will receive an error after step 3 as the username Joey already exists in the system.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work 
+++ b/email_registration.moduleundefined
@@ -21,19 +21,15 @@ function email_registration_user_insert(&$edit, &$account, $category = NULL) {
+  ¶

@@ -55,6 +51,19 @@ function email_registration_user_insert(&$edit, &$account, $category = NULL) {
+  ¶

trailing whitespace

+++ b/email_registration.moduleundefined
@@ -55,6 +51,19 @@ function email_registration_user_insert(&$edit, &$account, $category = NULL) {
+  if ((bool) db_query("SELECT 1 FROM {users} WHERE uid <> :uid AND LOWER(name) = LOWER(:new_name)", array(':uid' => $account->uid, ':new_name' => $new_name))->fetchField()) {
+    $name_idx = db_query_range("SELECT SUBSTRING_INDEX(name,'_',-1) FROM {users} WHERE name REGEXP :search ORDER BY CAST(SUBSTRING_INDEX(name,'_',-1) AS UNSIGNED) DESC", 0, 1, array(':search' => '^' . $new_name . '_[0-9]+$'))->fetchField();

this query should be simplified and probably should be replaced according #1430058: email_registration_user_insert isn't compatible with SQL Server

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs work » Closed (duplicate)

I didn't see this issue before and made a change that should fix this in #421078: Abstract name unique check for re-use, make it is cross-database compatible, make it more robust, us it on external names. The two changes are in such close areas of code so marking this issue as a duplicate of that.

@kris_mcl - could you test out that other patch?

gclicon’s picture

gclicon commented
Issue summary: View changes

Another issue to account for is that with a presave, the $account doesn't have a UID yet so you email_registration_unique_username is passed a null value as the second parameter. This causes the email_registration_unique_username function to not match with any existing usernames.