dev/core#86 Notify admin when testing email if CIVICRM_MAIL_LOG_AND_SEND is set

[mattwire]

Overview

When CIVICRM_MAIL_LOG_AND_SEND is set the admin is not notified if it is set, but they are if CIVICRM_MAIL_LOG is set. With this PR they are notified if either is set.

https://lab.civicrm.org/dev/core/issues/86

Before

No notification if CIVICRM_MAIL_LOG_AND_SEND is set.

After

[eileenmcnaughton]

fail is unrelated

[eileenmcnaughton]

Should this be defined('CIVICRM_MAIL_LOG_AND_SEND') && defined('CIVICRM_MAIL_LOG') since per below
CIVICRM_MAIL_LOG_AND_SEND doesn't work in isolation - also defined is not enough of a test as they could be defined as 0

[mattwire]

@eileenmcnaughton Ok, yes. Updated the PR to reflect this. We don't check the value of other defines, only check if they are defined or not - I've changed the "value" to 1 in the settings template for consistency/clarity.

[eileenmcnaughton]

OK - I'd prefer we did check the value but given the triviality of this fix I'm prepared to let it go

[monishdeb]

Merging as per tag

[davialexandre]

@mattwire @eileenmcnaughton I think that the actual constant name is CIVICRM_MAIL_LOG_AND SEND (without an underscore between AND and SEND), as it can be seen here:

• https://github.com/civicrm/civicrm-packages/blob/master/Mail/smtp.php#L249
• https://github.com/civicrm/civicrm-packages/blob/master/Mail/mail.php#L119
• https://github.com/civicrm/civicrm-packages/blob/master/Mail/sendmail.php#L109

[mattwire]

@davialexandre see https://lab.civicrm.org/dev/core/issues/86 there are actually 3 PRs for this

[davialexandre]

@mattwire since there’s no link anywhere to this third PR, it’s a bit hard to find it.
I think this PR should have been merged only after the civicrm-package one is merged.
Also, I think this is not a simple typo fix. This constant has been there for a while and people might been using it (I use it). So, I think that if we really want to fix the typo we should first deprecate the mistyped one, add a new constant, suport both for a while and then finally remove the deprecated one

[mattwire]

@davialexandre Sorry, the gitlab issue was meant to reference it but I put the wrong link in for the civicrm-packages PR - now fixed. Do constants with spaces actually work? It wasn't documented nor in the default settings file and it's generally meant for debug/development so I would think it's ok to just mention it in the release notes rather than have an overlap. If it's a problem, you can always define both in your config files.

[eileenmcnaughton]

@davialexandre @mattwire I think we should document CIVICRM_MAIL_LOG_AND_SEND but we should make both work

[davialexandre]

@mattwire it works partially (even though the documentation here indicates it should not work at all). Check this example: https://3v4l.org/BvFLb

[eileenmcnaughton]

@davialexandre - did you see I merged the packages PR so we can start to kiss it goodbye