Fix activity.getcount function to filter out unpermitted activities.
authoreileen <emcnaughton@wikimedia.org>
Tue, 1 Jan 2019 23:14:32 +0000 (12:14 +1300)
committereileen <emcnaughton@wikimedia.org>
Tue, 1 Jan 2019 23:33:32 +0000 (12:33 +1300)
commitff2a3553040a24da409e73cd02786c8bd11d6fd0
tree24e3218af3d98768c9c636ccba2dfee7de9f0a29
parent777612b329652e485d1cd0d8d6e6228c35bb6aa5
Fix activity.getcount function to filter out unpermitted activities.

As part of an ongoing performance refactor of the activity.get api handling this extends the application of activity
type filtering to the getcount function.

The current methodology of the activity.get api is to load all activities that meet the passed in criteria and
then go through them one by one running a series of queries on each one to determine if the contact has
permission to see them and if not then filtering them from the array. At some point this did work with
getcount but it had a crippling performance impact. More recently getcount was simply giving a fatal error
when acls were in play and right before this patch the status was that no permissions
were applied when the action is getcount.

This alters that to partially apply permissions - notably a filter is added to the query that reflects
which activity types the contact may access - this is basically those with no component or a component
the contact has high level permissions to. It is a check that is applied in the 'allow' function
and it still will be after this but it's a fairly cheap check and it will always return TRUE
from here on as no activities will be fetched if their activity type is not available.

The mechanism is that the CRM_Utils_SQL_Select class calls the relevant addSelectWhereClause but ONLY when
check_permissions is TRUE. In this case the activity type filter is added if they don't have
permission to check all activities.

Tangental changes
1) test enabled for getcount with activity type checks
2) I added a type casting for activity_type_id in the function that retrieves
and caches them - it should be pretty impossible for them not to be all integers
but as I later concat them this seems a good security insurance
3) I changed a very recently added function used in this from public to protected. It was added in Dec 2018
and is only called from the Activity BAO and I want to discourage other classes from calling it
now that I believe this is the correct approach.
CRM/Activity/BAO/Activity.php
tests/phpunit/api/v3/ACLPermissionTest.php