Performance fix for alternate getActivity listing function

[eileenmcnaughton]

Overview

Performance fix for the function, not in general use as yet, to render the activity tab.

We have been developing an alternate function to render activity tab that performs better & calls the selectWhereClause. However, it's not 'live' yet. This PR improves performance on that function

This will not affect any sites that have not hacked their CiviCRM codebase to use the getActivities function. As far as I know only WMF has done this.

Before

Sites who are using the currently-experimental function will experience slow performance when rendering activity tab for a contact who has an activity with many contacts (e.g bulk maling)

After

Above performance issue fixed.

Technical Details

We have an alternate function to render the activity listing on the contact tab. It is
getActivities whereas the other is deprecatedGetActivities.

It was developed in order to replace the other and we have tests that compare the results of the 2. It is better in that it

1. performs better (on a WMF contact with many activities this is 'snappy' while the current deprecated one gives a white screen time out) and
2. calls the selectWhereClause hook, allowing hook alteration and respecting preferred architecture.

However, we didn't go live with it in core because it

1. has a remnant performance bugs (this PR fixes the last of these)
2. implements ACLs differently - it uses generic functions whereas the deprecated one
   applies more limited permissioning. This is something to clarify & resolve separately.

This PR fixes the last remaining performance issue - best described as

'When one of the activities to be displayed has many targets the activity listing is slow to load'

The reason for the slowness is that when 'target_contact_name' is passed to the api
the api does a call for each contact to fetch the contact's sort_name. For a bulk mailing that went to 50,000 people that equates to 50,000 extra queries.

However the actual display shows the first contact name and then gives a number for how many more should be retrieved. This PR hence does not ask the api for the display name, but rather does the check itself, but
only for 1 target contact rather than ALL

Note that a similar logic might be considered for assignee - I left that out of scope as I'm not
aware of situations where a large number of assignees would be assigned to a single activity.

The unit test ensures the output matches the deprecated function.

Comments

@pfigel @lucianov88 FYI

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

[colemanw]

@eileenmcnaughton in light of our recent discussions about fixing the api to respect acls, do you think this function will ever be used in core?

[eileenmcnaughton]

@colemanw yes - that discussion is delaying rather than preventing people from getting the performance & architectural benefits of switching across

[eileenmcnaughton]

test fail is unrelated

[eileenmcnaughton]

test this please

[pfigel]

This looks encouraging. We currently use the Fastactivity extension to get around performance issues in core's implementation, but this should allow us to switch back eventually.

[colemanw]

I'll go ahead and merge this based on tests passing and the fact that it's not currently used in core.

[eileenmcnaughton]

thanks @colemanw