Do not send email notification to the user if notify is not set in the params

[jitendrapurohit]

Overview

Do not send email notification to the user if notify is not set in the params

Before

Create User sends email notification even if $params['notify'] is set to FALSE.

After

Email is sent only if $params['notify'] = true;

Technical Details

Spotted a ToDo comment in D8 file -

@todo: Should we only send off emails if $params['notify'] is set?

which i think is yes, since we have the same implementation in D7 and Backdrop - https://github.com/civicrm/civicrm-core/blob/master/CRM/Utils/System/Drupal.php#L40

[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).

[demeritcowboy]

This looks ok just I can't see any place that notify ever gets set? The useradd form doesn't have an option for notify, profiles send the email via CRM_Core_BAO_UFGroup::commonSendMail not passing on "notify", etc. So I think this would end up never sending an email.

Was there a page where you noticed this?

[jitendrapurohit]

Yes, and is the same process that we've used with d7 from a long time? Eg, I don't see any emails sent in d7 when a user is created from action dropdown on a contact summary page.

I checked a bit more and seems register_no_approval_required and register_pending_approval is not dependent on this param. It is only admin notification register_admin_created that is avoided when notify is set to false on register form - /admin/people/create.

I've updated the PR to ignore notify for register_no_approval_required and register_pending_approval.

[demeritcowboy]

Yeah I think maybe the drupal 7 code is weird. This code doesn't get called from /admin/people/create, and there's nowhere in civi I can find that sets the param corresponding to that checkbox that is on the (drupal) form. So $params['notify'] is always empty. It would make sense to me to ADD a field called notify to the civi user add form that mimics the one on the drupal form, but that's a bigger PR.

So I dunno. I guess this PR makes it closer to how drupal 7 + civi works, whether that's right or wrong.