christianwach / civicrm-wp-member-sync

CiviCRM WordPress Member Sync plugin keeps a WordPress user in sync with a CiviCRM membership by granting either a role or capabilities to a WordPress user who has that membership.
https://wordpress.org/plugins/civicrm-wp-member-sync/
GNU General Public License v2.0
17 stars 10 forks source link

WP Username exists #27

Closed marcusjwilson closed 5 years ago

marcusjwilson commented 5 years ago

Hi Christian -

Great plugin. But am I correct in thinking that, if the username that your plugin tries to create from the CiviCRM Display Name already exists, the WP user is not created? This seems to be the case in my testing.

Rather than failing, could the plugin add an integer at the end of the username in the case of the username matching an existing WP user?

Thanks Marcus

christianwach commented 5 years ago

@marcusjwilson Sorry for the delay in responding - holiday season etc.

if the username that your plugin tries to create from the CiviCRM Display Name already exists, the WP user is not created

I have addressed this in 57c2825eefd495f6cfbbd46e00d34c898d1019b6. The plugin should now do what you want it to do.

marcusjwilson commented 5 years ago

That's great, Christian - Thanks for that welcome addition to the plugin!

I ended up hooking in to the function that handles this, and also creates the username from First Name and Last Name (for CiviCRM Individuals) rather than the Display name (as I was getting a lot of usernames like "mrjohnjamessmith" usernames when using the Display name.

This is what I came up with:

/* 
 * Customisations to CiviCRM Member Contact WP Username creation
 * Extends CiviCRM Member Sync plugin
 */
function spl_custom_username( $user_name, $civi_contact ) {

  /* For Individuals, create username from firstname lastname instead of Display Name */
  if ( $civi_contact['contact_type'] == 'Individual' ) {
    $user_name = preg_replace("/[^a-z]+/", "", strtolower($civi_contact['first_name'] . $civi_contact['last_name']));
    $user_name = sanitize_title( sanitize_user( $user_name ));  
  }

  $user_name = str_replace('-', '', $user_name);

  /* Check if the username already existing in WP */
  $user_id = username_exists( $user_name );

  /* If not, return the username - it's good */
  if ( !$user_id ) {
    return $user_name;

  /* Else stick an integer at the end of the Username to prevent the conflict */
  } else {
    $baseName = $user_name;

    for( $i = 1 ; $i <= 9 ; $i++ ) {
      $user_name = $baseName . $i;
      $user_id = username_exists( $user_name );

      if ( !$user_id ) {
        return $user_name;
      }    
    }
  }
}
add_filter('civi_wp_member_sync_new_username', 'spl_custom_username', 10, 3);

If this all looks okay and it won't conflict with your recent changes, I will probably leave it as is.

Thanks so much for all your great work! Marcus

christianwach commented 5 years ago

The civi_wp_member_sync_new_username filter runs after the code I've just added, so your callback will override the default username format - which is the intended behaviour.

@marcusjwilson Your code looks fine - though FWIW your filter should declare 10, 2 as there are only two params passed to the callback. And you might want to consider do - while syntax so you don't run out of usernames at suffix 9 :) See 57c2825eefd495f6cfbbd46e00d34c898d1019b6 for reference.

marcusjwilson commented 5 years ago

Wise words @christianwach - thanks!

I kind of figured that if with hit 9 users with the same name, then there's probably a bigger issue or error (particular with the small databases that we're dealing with), so I'd rather it failed to create usernames after 10 duplicates.

Thanks again!