Simplify handling for case checking.

[eileenmcnaughton]

Overview

Minor code simplification

Before

Slightly more expensive method used for civicase checking

After

The hasPermissionForActivityType check is run before the civicase check and an early return is done if the contact does not have permission to 'access my cases and activities', or 'access all cases and activities'

Technical Details

We already check if the contact has generic case permissions in the component checking section, we can do that first & remove it from the case check.

Comments

Test coverage in testGetActivityCheckPermissionsByCaseComponent

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

[eileenmcnaughton]

@seamuslee001 @colemanw this is a minor follow on for things you have reviewed

[seamuslee001]

@eileenmcnaughton is this taken care of in the hasPermissionForActivityType?

[eileenmcnaughton]

@seamuslee001 yep - it retrieves a list of permissable components & then checks types are associated with them

[eileenmcnaughton]

You can see it looks for components

$components = self::activityComponents(FALSE);

which filters with

        if ($compObj->info['name'] == 'CiviCase') {
          if (CRM_Case_BAO_Case::accessCiviCase()) {
            $components[$compObj->componentID] = $compObj->info['name'];
          }
        }

which calls

  /**
   * Since we drop 'access CiviCase', allow access
   * if user has 'access my cases and activities'
   * or 'access all cases and activities'
   */
  public static function accessCiviCase() {
    if (!self::enabled()) {
      return FALSE;
    }

    if (CRM_Core_Permission::check('access my cases and activities') ||
      CRM_Core_Permission::check('access all cases and activities')
    ) {
      return TRUE;
    }

    return FALSE;
  }

[eileenmcnaughton]

@seamuslee001 somewhat ironically I have a follow up that will make this kinda redundant for api calls but this allow() function is still called from a bunch of places. I added testGetActivityCheckPermissionsByCaseComponent as part of this refactoring effort - but it got added in advance of some of these changes so the link isn't obvious

[seamuslee001]

Looks fine to me and with the clarification i'm happy to merge this