CRM-20905: Make (simple_mail_limit) a configuration instead of h…

[eileenmcnaughton]

Overview

When using the "Search Contacts" results to send individual emails to multiple recipients, the number of recipients is limited. This functionality prevents spamming and prevents overloading the mail-delivery system.

Before

If a user picks more than 50 contacts and tries to send, it displays an error.

After

Each user-organization may change the limit from 50 to something else by updating the hidden setting, simple_mail_limit. (Ex: To change it to 25, run cv api setting.create simple_mail_limit=25)**

[eileenmcnaughton]

Note that most discussion of this PR is on #10693

[eileenmcnaughton]

@omarabuhussein if you are happy with this based on the discussion on #10693 please confirm here that you think it is OK to merge

[omarabuhussein]

left a note ... also two things :

1- can you add this new setting to api/v3/examples/Setting/GetFields.php .

2- this is a new change and I would prefer seeing a clear description for it and explaining why the option is hidden,

[omarabuhussein]

I liked what Andrew suggested about using quick instead of simple

[eileenmcnaughton]

Hmm perhaps but it's called 'Send Email' here https://docs.civicrm.org/user/en/stable/email/what-you-need-to-know/ - it feels like scope creep to rebrand that feature & I'm a bit reluctant to entrench a user-term in the code if it's not accepted at the user level yet.

I could change all those code places to 'send_email' to match the docs - but not sure that is clearer than simple_mail

[xurizaemon]

just found this PR via SE question

+1 eileens' comment and also consider: if a site starts trying to send 1000 emails in a single request, this will not be "quick mail" at all (it'll probably start being "timeouty mail")

Agree that the branding is not consistent, terms that are descriptive of the difference ("direct email" vs "batched email"?) might be better, even if only for internal use (eg settings names).

[eileenmcnaughton]

I have added the example update & updated the setting comment - per comment above I have reservations about starting to refer to it as quickEmail in the code if we haven't started using that in the user documentation

[omarabuhussein]

from coding perspective, this look fine for me to merge ..

though this should be documented here in the PR description and in the ticket description (as well as user docs maybe ?)

[totten]

This is a good idea, and the implementation looks straight-forward, and it's been reviewed by Omar.

I agree the description needs work. This might be a good case to try reworking per https://github.com/civicrm/civicrm-core/blob/master/.github/PULL_REQUEST_TEMPLATE.md

Since this is new structure, it may be useful for us to publicly work through some examples. Here's a suggested reworking for this one.

(Note: I've flagged text as follows: removals use strike-through notation; new content uses bold; comments use {{...}})

Overview

Sending direct email to more than one user from tasks in contact search page is limited. When using the "Search Contacts" results to send individual emails to multiple recipients, the number of recipients is limited. {{When describing a context, start from the big-context and drill-down to the specific element. If you start from the specific element, many readers won't recognize it.}} This functionality prevents spamming and prevents overloading the mail-delivery system. {{Someone coming from a CHANGELOG or a debug-session may lack basic context.}}

Before

~~hardcoded to only 50 contact ~~ If a user picks more than 50 contacts and tries to send, it displays an error.

After

this could be diffrenet from organization to another so it would probably better to be a configuration Each organization may change the limit from 50 to something else by updating the hidden setting, simple_mail_limit. (Ex: To change it to 25, run cv api setting.create simple_mail_limit=25) {{Many users expect settings to be in the UI. That's not a mandate -- it's ok we have some hidden settings, but that should be communicated.}}

IMHO, it's ok to merge, but I'd like to pause a moment in case you or others have revisions to the description. Flagging merge ready so that you can merge without be accused of self-approving.

[eileenmcnaughton]

24 days have passed (actually I thought this was merged) so merging

[omarabuhussein]

thanks @eileenmcnaughton