dev/core#2914 Fix for incorrect sent count in message

[eileenmcnaughton]

Overview

dev/core#2914 Fix for incorrect sent count in message

Before

count of sent emails doesn't reflect final send

After

Based on final send

Technical Details

@demeritcowboy does this cover it off
Comments

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

[totten]

r-test: The reported failure appears relevant. ⚠️

r-run: I tried two flows:

1. ✔️ "Contacts => New Email" (ie the flow from dev/core#2914.) Final confirmation message seems to be fixed (now reports a real tally instead of 0).
2. ✔️ "Find Contacts => ... => Email - send now". Worked before - and still works.

Tangentially, there is a bit of a UX quirk in how the tallies are reported before sending:

1. ("Contacts => New Email") The page opens and mentions "Number of selected contacts: 0" (bottom). That is accurate when the page is first opened - but it becomes inaccurate when edit the recipient list (as you must do). May be better to hide it in this case (since we're probably too lazy to make the tally auto-update via jquery)? Screenshot from 2021-10-14 23-35-26
2. ("Find Contacts => ...=> Email - send now") There are actually preselected contacts, and you usually don't need to add/remove recipients in this case... so it's plausible to show a tally. But this one doesn't. Of course, hiding the tally is OK - since the recipient list is still editable. Screenshot from 2021-10-14 23-36-10

[eileenmcnaughton]

@totten " (as you must do)." - I guess I may not understand properly how this form is used as I assumed that would not be the most common way to use the form.

[demeritcowboy]

Added a little note at https://lab.civicrm.org/dev/core/-/issues/2579#note_66389

[demeritcowboy]

The reason the test fails is because buildQuickForm is when it determines the suppressed emails, and doesn't rebuild it again in some way during submit, so the only way at the moment to get the correct count is via $this->_toContactDetails. I think it should rebuild the list based on the submitted values not the originally passed in values. It needs to do it twice - the first time because it notifies on the form, and then again during submit so that it does the right thing, not just for counting but for avoiding sending to on-hold people that have been added etc.

[totten]

" (as you must do)." - I guess I may not understand properly how this form is used as I assumed that would not be the most common way to use the form.

Yeah, there are several pageflows that load some variant of this form. One pageflow is to use the main menu to click "Contacts => New Email", which opens a blank email message. So there you need to type the recipient list. AFAIK, all the other pageflows come with some pre-selected recipients.

https://lab.civicrm.org/dev/core/-/issues/2579#note_66389

👍

[eileenmcnaughton]

@demeritcowboy I pushed up a fix for the test - I have some doubt as to the validity of dead people being added to the 'to' before submission but this gets us back to them being filtered out again

[demeritcowboy]

Thanks that seems to work.
Fair point about adding people using the form since aha it looks like the select doesn't present it as an option. (You could do it with the javascript console but nobody's going to do that.)