Do not check financial permissions on contribution.create if check_perrmissions is FALSE.

[eileenmcnaughton]

Overview

Alters Contribution.create api to respect the check_permissions flag when Financial ACLs are enabled

Before

when calling
civicrm_api3('Contribution', 'create', ['check_permissions' => FALSE.....

The check_permissions is ignored when applying financial type acls

After

In above scenario the flag is respected

Technical Details

Our standard is to only check permissions at the api layer if check_permissions is TRUE
(which is is by default for javascript calls but not in php calls)

Comments

@monishdeb @JoeMurray

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@monishdeb can you check this? It came up when writing unit tests for the annual query

[monishdeb]

I agree with this one-liner change about respecting check_permissions on create action. Also Lineitem get and delete API does this similar check too. In addition I think you also need to add this check inside Contribution.delete API too:

diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php
index a52600f6d8..2e671b0876 100644
--- a/api/v3/Contribution.php
+++ b/api/v3/Contribution.php
@@ -221,7 +221,7 @@ function civicrm_api3_contribution_delete($params) {
   // First check contribution financial type
   $financialType = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionID, 'financial_type_id');
   // Now check permissioned lineitems & permissioned contribution
-  if (CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus()
+  if (!empty($params['check_permissions']) && CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus()
     && !CRM_Core_Permission::check('delete contributions of type ' . CRM_Contribute_PseudoConstant::financialType($financialType)) ||
       !CRM_Financial_BAO_FinancialType::checkPermissionedLineItems($contributionID, 'delete', FALSE)
   ) {

[eileenmcnaughton]

@monishdeb good spotting - I added to delete - took a bit of getting the braces right - double check you agree.

[monishdeb]

Yes, working fine. Marking with Merge-on-Pass

[monishdeb]

Jenkins test this please

[eileenmcnaughton]

merging per @monishdeb approval