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

Move require once in api kernel to support edge cases #19513

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Move require once in api kernel to support edge cases

Before

 drush  cvapi  Contribution.get version=4

fails with a require_once error

After

Above fails (correctly) with a permissions error

Technical Details

The function 'createError' on the kernel class calls 3 functions that are in the api v3 utils file.
It is possible to reach this from apiv4 using drush (I realise we
don't really know much about how to support api v4 with drush but it DOES work with a simple
api with this patch in my testing). We could go with only including utils.php in
v4 api AND the createError function. I think it's more readable this way.

To test this try

drush cvapi Contribution.get version=4

it will legitimately fail without checkPermissions=0 but it will fail with a
require_once error without this patch

Comments

The function 'createError' on the kernel class calls 3 functions that are in the api v3 utils file.
It is possible to reach this from apiv4 using drush (I realise we
don't really know much about how to support api v4 with drush but it DOES work with a simple
api with this patch in my testing). We could go with only including utils.php in
v4 api AND the createError function. I think it's more readable this way.

To test this try

 drush  cvapi  Contribution.get version=4

it will legitimately fail without checkPermissions=0 but it will fail with a
require_once error without this patch
@civibot
Copy link

civibot bot commented Feb 3, 2021

(Standard links)

@civibot civibot bot added the master label Feb 3, 2021
@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

This looks correct to me

@eileenmcnaughton eileenmcnaughton merged commit 9389fdc into civicrm:master Feb 3, 2021
@eileenmcnaughton eileenmcnaughton deleted the kernel_error branch February 3, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants