Improve conditional in api3 ChainSubscriber #16718
Conversation
|
(Standard links)
|
|
Retest this please |
| @@ -54,7 +54,7 @@ public function onApiRespond(\Civi\API\Event\RespondEvent $event) { | |||
| $apiRequest = $event->getApiRequest(); | |||
| if ($apiRequest['version'] < 4) { | |||
| $result = $event->getResponse(); | |||
| if (!is_array($result) || ($result['is_error'] ?? 0) == 0) { | |||
| if (is_array($result) && empty($result['is_error'])) { | |||
There was a problem hiding this comment.
@colemanw I'm pretty sure from my testing that strings & ints (eg. results of getvalue) are expected to pass onto the chaining code. It's a bit odd I guess since I'm not quite sure what gets passed on but we'd need to be sure that's never a good thing before changing
There was a problem hiding this comment.
@eileenmcnaughton I see no way for that to work, nor can I get it to work in the api explorer.
There was a problem hiding this comment.
OK - I guess we don't have test cover so it's not in the contract & you would not be able to pass in anything
Some part of me things I've seen something like this before though
civicrm_api3('DummyEntity', 'dummyAction', [
'api.Thing1.do => 1,
'api.Thing2.do => 2,
'api.Thing13do => 2,
];
As a way of calling multiple actions from one js api call
I can't find it but we'd be breaking it for nothing if my vague feeling is right
There was a problem hiding this comment.
Again, no I don't think we'd be breaking it because that scenario is already non-functional. Chaining works by looping through $result['values']. If $result is not an array, then there's nothing to loop through, just some PHP errors and no chain.
Overview
Improve conditional in api3 ChainSubscriber, follow up from #16712
Before
Chains triggered even when result is not an array!
After
More sensible conditions.