CRM-19773 - Call hook_civicrm_selectWhereClause when checking event permissions

[colemanw]

• CRM-19773: Call hook_civicrm_selectWhereClause from the BAOs

[nganivet]

So cool ...

[colemanw]

Yep, that's 3 levels of joins to get the state_province name :)

[nganivet]

Wouldn't keeping $allEvents = CRM_Event_PseudoConstant::event(NULL, TRUE); be more efficient than an API call?

[colemanw]

The api call invokes hook_civicrm_selectWhereClause and the pseudoconstant lookup does not (I did add it there, but because of the $all flag being set to true it skips the hook). The pseudoconstant function is deprecated anyway.

[nganivet]

So if I understand correctly we do not need to filter in the Manage Events page since the filtering takes place in CRM_Event_BAO_Event::checkPermissions() ?

[colemanw]

Right.

[colemanw]

@civicrm-builder retest this please

[eileenmcnaughton]

test this please

[eileenmcnaughton]

It would be good to get a test on this

[jitendrapurohit]

Tested, works fine. Also added a test #10125. Not sure why the build isn't passing here.

[colemanw]

@civicrm-builder test this please

[jitendrapurohit]

Build shows a break at this point - maybe all failing test needs $this->createLoggedInUser(); in them to validate 'created_id' => 'user_contact_id' param?

[eileenmcnaughton]

I don't know why the test is failing but once you get it to pass it's good to merge based on @jitendrapurohit's review & my reading of the code - adding merge-on-pass

[eileenmcnaughton]

@colemanw this has been reviewed AND @jitendrapurohit added a unit test! - but can't merge as tests are failing

I think the trick is to allow 'user_contact_id' to be converted to NULL & rely on whether it is mandatory or not to enforce the id - in the case of the failing tests it is not

  if (!empty($fieldValue) || $fieldValue === '0' || $fieldValue === 0) {
    // if value = 'user_contact_id' (or similar), replace value with contact id
    if (!is_numeric($fieldValue) && is_scalar($fieldValue)) {
      $realContactId = _civicrm_api3_resolve_contactID($fieldValue);
      if ('unknown-user' === $realContactId) {
        throw new API_Exception("\"$fieldName\" \"{$fieldValue}\" cannot be resolved to a contact ID", 2002, array('error_field' => $fieldName, "type" => "integer"));
      }
      elseif (is_numeric($realContactId)) {
        $fieldValue = $realContactId;
      }
    }
    if (!empty($fieldInfo['pseudoconstant']) || !empty($fieldInfo['options'])) {
      _civicrm_api3_api_match_pseudoconstant($fieldValue, $entity, $fieldName, $fieldInfo, $op);
    }

    // After swapping options, ensure we have an integer(s)
    foreach ((array) ($fieldValue) as $value) {
      if ($value && !is_numeric($value) && $value !== 'null' && !is_array($value)) {
        throw new API_Exception("$fieldName is not a valid integer", 2001, array('error_field' => $fieldName, "type" => "integer"));
      }
    }

[eileenmcnaughton]

from function _civicrm_api3_validate_integer(&$params, $fieldName, &$fieldInfo, $entity) {

[eileenmcnaughton]

@colemanw needs rebasing but will hopefully pass now

[eileenmcnaughton]

@colemanw this appears to have caused a regression due to default limit of 25 on the api calls

[mfb]

Yes, it's fixed if I add 'options' => array('limit' => 0) to these API calls. See also https://civicrm.stackexchange.com/questions/18516/events-missing-after-upgrade-to-4-7-19

[colemanw]

I've opened a PR - #10327