dev/drupal#127 - require email for the Useradd task and errors not showing

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/drupal/-/issues/127

This is more noticeable on drupal 8 because in drupal 7 the CMS prevents it and outputs its own error message, but either way the "Create User Record" action when viewing a contact doesn't properly display or check for errors when creating the CMS user.

1. Find a contact without an email.
2. Choose Create User Record from the actions dropdown.
3. Fill out the form.
4. When you submit, what happens on drupal 8 is it goes back to contact view with no message a success message but the CMS user is not created.

• Additionally, if you look at the original code even in drupal 7 it's written to display a success message even when the CMS user creation fails. That doesn't happen because of a separate bug (dev/drupal#127 - CRM_Core_Session::setStatus() gets ignored sometimes #17914), but you can see there's no check on the return value from the create call.

Before

Missing or incorrect message. Also the actual cause of the error is not recorded or displayed anywhere.

After

Proper message and cause displayed.

Technical Details

Also to be in line with https://lab.civicrm.org/dev/drupal/-/issues/91 I made email required on the form to head off errors from that.

Comments

Has test.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@demeritcowboy do we need to do this on other CMSes?

[demeritcowboy]

For drupal 7 no, because of the way it uses drupal's form layer to create a user, so the form errors display.

Let's take a look at what wordpress/joomla do...

[demeritcowboy]

Ok it took a minute to find but it looks like wordpress doesn't override checkPermissionAddUser() so it returns false so you don't even see this feature.

Ok now joomla...

[demeritcowboy]

The Joomla UF class has pretty extensive checking as a form rule via checkUserNameEmailExists(), and it's beyond my joomla knowledge to know how to get past it to trigger a fail in joomla itself. The only thing I can see it doesn't check for is a blank email, which this PR would cover. But createUser itself doesn't have any kind of error checking, and in fact seems like it would never return FALSE either way, so technically createUser might need some sprucing up, but I'm not a joomla person.

[seamuslee001]

@kcristiano @mattwire Thoughts on this? I think it seems fine to me from a Drupal 7/8 POV

[eileenmcnaughton]

@kcristiano @christianwach @andrewpthompson thoughts?

[demeritcowboy]

If it helps here's an updated table about what happens when you use the Create User Record action from the view contact summary for a contact who has no email.

CMS	Before patch status	After patch status
Drupal 7	Bit of a mixed state after #17914. It will show form errors from drupal, but now civi will at the same time happily show a success message no matter what happened.	Correct messages from drupal and civi.
Drupal 8	Says success but it didn't succeed.	Correct messages.
Wordpress	CRM/Utils/System/Wordpress doesn't override checkPermissionAddUser() so it returns FALSE so the feature doesn't appear as an action.	Same as before.
Joomla	Don't have one to test on as an admin, but reading the code it would depend on what joomla does when email is blank. In any case civi would always show a success message unless joomla throws a fatal error.	Email now required on the form, so should head off any issues there. The form rule validation looks like it will catch most other problems. In terms of other unexpected errors the createUser code never returns FALSE so again it would depend what joomla does.
Backdrop	Same as drupal 7	Code seems identical to drupal 7.

[christianwach]

@eileenmcnaughton WordPress would appear to be unaffected by this PR since the the feature doesn't appear as an action.

This can already be done in WordPress via:

• CiviCRM WordPress Profile Sync which adds an item to the "Bulk Actions" menu allowing one or more Users to be created from Contacts. Requires the CIVICRM_WP_PROFILE_SYNC_BULK constant to be set for the feature to appear.
• CiviCRM WordPress Member Sync which creates WordPress Users when a Membership is created and there's an association rule for that Member Type.
• BP Groups CiviCRM Sync which creates a WordPress User for a Contact when a Group Membership is created and there's an associated BuddyPress Group.

Having said all that, IMO it would be nice to have this in core at some stage.

[eileenmcnaughton]

I've tried to ping a few Joomla! people & also pasted on chat. Unfortunately Joomla! world is a bit quiet - I think at this stage I'm going to merge it - but @agh1 can you try to give some visibility to the fact this may change Joomla! behaviour in the release notes. I think it would be easy to fix if the change is considered regressive

@vingle pinging you as someone else with some Joomla! knowledge

[eileenmcnaughton]

Thanks for that analysis @christianwach

[andrewpthompson]

I've only given this a quick test on Joomla, and only for the case of trying to create a CMS user with no email address. The patch works as intended.

For info, before the patch: Joomla user is not created. No error is displayed by Joomla. Civi show a success message.

I'm not particularly familiar with this part of Joomla/Civi interaction but it looks OK to me.

[eileenmcnaughton]

Thanks @andrewpthompson - appreciate your feedback

[demeritcowboy]

Thanks everybody.