CRM-20521 Convert returning groups and mailings for CiviMail new mail…

[seamuslee001]

…ing screen into an API request rather than loading in the DOM

[seamuslee001]

@totten @eileenmcnaughton Would appreciate your thoughts on this. AUG is currently running with this on our production and we have found this generates a good performance boost for us also going to ping @jaapjansma not sure if you have hit similar things

[jaapjansma]

@seamuslee001 I haven't come accross this particulair issue but it makes sense in light of what we have discovered so far.

[totten]

Sounds like this would be a performance improvement when you have a large number of groups or a large number of mailings? Might also help in edge-cases where you're juggling multiple mailings in different tabs?

In terms of testing/review, I can't do it right now, but the first thing I'd suggest looking out for is variations in the dataflow, e.g. (a) creating a new mailing vs (b) loading an existing mailing on a fresh page vs (c) editing the url bar to switch to a different mailing.

[seamuslee001]

@totten yep that would be the idea, We have had this code running for ~2 months now and haven't had any major issues with it as far as i know. Not sure about the edge cases you speak of but certainly the main thing for us was the performance improvement

[totten]

Yup, it seems to work consistently for me.

One oddity: it feels like it's doing duplicate AJAX calls (at least in Firefox), e.g.

• If you click the down arrow, it does two calls to Group.getlist (with input="")
• If you type "ad", it does two calls to Group.getlist and two calls to Mailing.getlist (both with input="").

Any chance you might reduce the #AJAX calls? e.g.

• Throttle/debounce the duplicate calls? crmThrottle (ex) might help
• Send the Group.getlist and Mailing.getlist in the same REST call. (CRM.api3() and crmApi() should both accept multiple calls in array notation.)

[seamuslee001]

@totten maybe haven't looked at that at all, i'm guessing its doing one per include and exclude for each of the 2 different entities

[seamuslee001]

@totten i have just had a try at debugging but i don't think there will be an easy way to reduce down the AJAX calls. The primary reason is that Include Groups and Exclude Groups are considered 2 separate arrays that are just joined together i think for example. Same with mailings. In my opinion just because its doing 2 AJAX calls i don't think should mean its a blocker to this being merged. Functionally it does the same job as it was doing previously

[eileenmcnaughton]

@totten looks like this has been discussed & had low-level testing by you & production testing by @seamuslee001 - do you think it is good to merge?

[JoeMurray]

@monishdeb to test and possibly merge given comments above. I think it is not necessary for this PR to refactor the include and exclude group widgets so they are combined to a single call, though that would also improve performance. FWIW, we have had complaints from users about this performance issue.

[seamuslee001]

@monishdeb did you get anywhere with your review of this?

[monishdeb]

@seamuslee001 I am not getting the groups in 'Recipients' field. Please check the screencast below:

[seamuslee001]

Hi @monishdeb i just tested this on a local buildkit install and it showed me groups fine. I would check to make sure that a) you have groups and b) the groups are ticked as mailing list

[seamuslee001]

@monishdeb ignore my previous comment, i found the issue please re-test now

[monishdeb]

Yup the latest patch has fixed that issue.

I agree with you that it will need major refactoring change to reduce AJAX calls and it's not a blocker, so we can skip this for now. Apart from that everything else is working fine. Tested as per
(a) creating a new mailing vs(b) loading an existing mailing on a fresh page vs (c) editing the url bar to switch to a different mailing... like Tim suggested. Merging it now.

[monishdeb]

@seamuslee001 in exchange can you please QA #10718?

[davejenx]

I found that this performs much better in Chrome than in Firefox. In both, it greatly speeded up the UI first appearing. But in Firefox, on a site with 2.7k mailing groups and 13k unarchived mailings, the Recipients select took 2 minutes to load when clicked. In Chrome it was 1-2 seconds.

In either browser, there's a bunch more initialization that happens before the UI is ready to use: in my testing on the above site, this takes a few minutes, even with the current PR. See CRM-21260 CiviMail compose UI very slow to initialize when many mailing groups and past mailings.