[NFC] Extract case activity permission check.

[eileenmcnaughton]

Overview

Function extraction for code legibility - no functional change

Before

long block of code

After

shorter block of code

Technical Details

This is part of refactoring to address duplicate queries for permission checks - no change
in this commit other than an extraction

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

[pradpnayak]

The changes look good to me. However do we need to perform checks one more time for CiviCase at 2755?

[eileenmcnaughton]

@pradpnayak I don't think I've removed any checks - I did move them higher up in the function as well as extracting - but that should be no-change

[pradpnayak]

@eileenmcnaughton I meant line 2755 is doing same checks for Civicase for defined permission at

civicrm-core/CRM/Activity/BAO/Activity.php

Line 2721 in ac4b9bc

'CiviCase' => array(

.

Since you are already checking for CiviCase than isn't it safe to remove those permission from $compPermissions array?

[eileenmcnaughton]

@pradpnayak so bigger picture what I think will work better is to generate a list of activity types that the contact can access before the main query & then setting an activity_type_id filter before the query runs - the current methodology is broken for getcount :-( and painfully slow under some circumstances.

I kinda need to resolve this https://lab.civicrm.org/dev/core/issues/454 to be able to do that part - but I figure in the meantime I can clean up the functions a bit to make the next change easier.

I agree the case permission checks are being doubled up - that isn't new I just repeated the array rather than passing it into that function as I thought that was more readable

[pradpnayak]

@eileenmcnaughton make sense now. This is good to merge but would be great if @colemanw have a look at this once since he has better sense on Check Permissions and CiviCase.

[eileenmcnaughton]

Thanks @pradpnayak

[eileenmcnaughton]

test this please