Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COMCL-167: Accept multiple types argument #838

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Conversation

erawat
Copy link
Member

@erawat erawat commented Jan 12, 2022

Overview

This PR fixes the issue when the site throws an error when accessing SearchKit menu.

Before

Peek 2022-01-12 13-07

After

Peek 2022-01-12 13-01

Technical Details

When using SearchKit function, the APIWrapper hook get called and API v4 object is passed to $apiRequest, and CiviCase implements apiWrappers hook, and the implementation only accept a parameter as an array, hence, PHP throws the error.

According to https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_apiWrappers/ $apiRequest parameter can be passed as array or Civi\Api4\Generic\AbstractAction therefore the function must be able to handle multiple types argument.

Note on the CiviCase's API Wrapper hook, the implementation mainly handle an array and this should not be any issue as the Civi\Api4\Generic\AbstractAction class has implemented ArrayAccess interface that allow to access object's properties as an array.

See https://github.com/civicrm/civicrm-core/blob/5.39/Civi/Api4/Generic/AbstractAction.php#L43

Comment

Note that there is an issue when installing NODE JS, so the linter is not running probably which should be dealt with separately as it is not relevant to this PR.

$apiRequest parameter can be passed as array or Civi\Api4\Generic\AbstractAction according to
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_apiWrappers/ therefore the function must be able to handle multiple types argument.
@erawat
Copy link
Member Author

erawat commented Jan 14, 2022

@lisandro-compucorp I have rebased this branch with the master branch after the linter action was fixed and merged from this PR #839

I have also added a new commit that fixed the linter, can you re-review this PR again?

Fix - The array content should be split up over multiple lines
@erawat erawat merged commit 732fb2b into master Jan 20, 2022
@erawat erawat deleted the COMCL-167-searchkit-error branch January 20, 2022 17:53
This was referenced Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants