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

APIv4 - CiviCase API: Fix openening a case with current user as creator #20238

Merged
merged 1 commit into from
May 7, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 6, 2021

Overview

Fixes APIv4 to be able to open a Case with the current user as creator.

Before

Passing user_contact_id as the case creator or case contact causes an error.

After

Works.

Technical Details

The DAOActionTrait::writeObjects function was formatting values but not by reference, so the CiviCase writeObjects function was using unformatted values to open the case, which would contain the raw string user_contact_id instead of the processed value.

@civibot
Copy link

civibot bot commented May 6, 2021

(Standard links)

@civibot civibot bot added the master label May 6, 2021
@eileenmcnaughton
Copy link
Contributor

@colemanw

Stacktrace
api\v4\Action\BasicCustomFieldTest::testWithSingleField
Error: Cannot pass parameter 1 by reference

/home/jenkins/bknix-dfl/build/core-20238-1pay5/web/sites/all/modules/civicrm/Civi/Api4/Generic/DAOUpdateAction.php:63
/home/jenkins/bknix-dfl/build/core-20238-1pay5/web/sites/all/modules/civicrm/Civi/Api4/Provider/ActionObjectProvider.php:68
/home/jenkins/bknix-dfl/build/core-20238-1pay5/web/sites/all/modules/civicrm/Civi/API/Kernel.php:149
/home/jenkins/bknix-dfl/build/core-20238-1pay5/web/sites/all/modules/civicrm/Civi/Api4/Generic/AbstractAction.php:239
/home/jenkins/bknix-dfl/build/core-20238-1pay5/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php:75
/home/jenkins/bknix-dfl/extern/phpunit7/phpunit7.phar:615

Copy link
Contributor

@aydun aydun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r-run: it works!

The DAOActionTrait::writeObjects function was formatting values but not by reference,
so the CiviCase writeObjects function was using unformatted values to open the case,
which would contain the raw string `user_contact_id` instead of the processed value.
@mattwire
Copy link
Contributor

mattwire commented May 6, 2021

@colemanw it looks like this bug could have been affecting other entities too such as address? Would it only affect them when called via api4 or more genericly?

@colemanw
Copy link
Member Author

colemanw commented May 6, 2021

It would only affect APIv4 calls, which are still few & far-between in core. I also don't think the bug affects other entities; reading the code I don't see others that do extra processing where the formatting matters.

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label May 6, 2021
@eileenmcnaughton
Copy link
Contributor

This seems mergeable & tested to me - added merge-ready in case @mattwire has any last comments

@mattwire
Copy link
Contributor

mattwire commented May 7, 2021

Merging based on @eileenmcnaughton review. I just wanted to get clarification from @colemanw which he has done :-)

@mattwire mattwire merged commit dd43aec into civicrm:master May 7, 2021
@eileenmcnaughton eileenmcnaughton deleted the civiCaseSave branch May 7, 2021 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-test master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants