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 - merge ActionUtil with Request::create #16516

Merged
merged 2 commits into from
Feb 14, 2020

Conversation

colemanw
Copy link
Member

Overview

Consolidates some APIv4 code with v3.

Before

The Request::create function was broken for v4, and ActionUtil had been serving the same purpose but in a non-broken way.

After

Deleted ActionUtil and fixed Request::create to work correctly with v4.

The Request::create function was broken for v4, and ActionUtil had been serving the same purpose but in a non-broken way.
@civibot
Copy link

civibot bot commented Feb 12, 2020

(Standard links)

@civibot civibot bot added the master label Feb 12, 2020
@colemanw
Copy link
Member Author

Retest this please

@seamuslee001
Copy link
Contributor

@colemanw the test rig is getting tied up on the test suite.

CRM_Utils_API_MatchOptionTest

@colemanw
Copy link
Member Author

colemanw commented Feb 13, 2020

@seamuslee001 looks like that was an uncaught exception caused by civicrm_api3_create_success trying to do getfields with no entity.

@seamuslee001
Copy link
Contributor

@colemanw test failures look related

@seamuslee001
Copy link
Contributor

@colemanw latest test failures look related

@colemanw
Copy link
Member Author

@seamuslee001 I got the tests passing by cleaning up all the places where the api wrapper was being used incorrectly, e.g. with missing $params['version'] or null $entity.

@eileenmcnaughton
Copy link
Contributor

Looks like our test cover is working well here - merging now they are passing

@eileenmcnaughton eileenmcnaughton merged commit 7be4891 into civicrm:master Feb 14, 2020
@eileenmcnaughton eileenmcnaughton deleted the request branch February 14, 2020 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants