From aeee327d2b9b004b8beca10f8d190263352d87c6 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 3 Sep 2020 20:13:01 +1200 Subject: [PATCH] Move ACls on LineItem create to financialacls core extension --- CRM/Price/BAO/LineItem.php | 13 +----------- ext/financialacls/financialacls.php | 9 +++++---- .../tests/phpunit/LineItemTest.php | 20 ++++++++++++++++++- tests/phpunit/api/v3/FinancialTypeACLTest.php | 2 +- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index b25c02c37c..0067fef0a6 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -35,11 +35,9 @@ class CRM_Price_BAO_LineItem extends CRM_Price_DAO_LineItem { $id = $params['id'] ?? NULL; if ($id) { CRM_Utils_Hook::pre('edit', 'LineItem', $id, $params); - $op = CRM_Core_Action::UPDATE; } else { CRM_Utils_Hook::pre('create', 'LineItem', $params['entity_id'], $params); - $op = CRM_Core_Action::ADD; } // unset entity table and entity id in $params @@ -54,21 +52,12 @@ class CRM_Price_BAO_LineItem extends CRM_Price_DAO_LineItem { $params['unit_price'] = 0; } } - if (CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus() && !empty($params['check_permissions'])) { - if (empty($params['financial_type_id'])) { - throw new Exception('Mandatory key(s) missing from params array: financial_type_id'); - } - CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($types, $op); - if (!in_array($params['financial_type_id'], array_keys($types))) { - throw new Exception('You do not have permission to create this line item'); - } - } $lineItemBAO = new CRM_Price_BAO_LineItem(); $lineItemBAO->copyValues($params); $return = $lineItemBAO->save(); - if ($lineItemBAO->entity_table == 'civicrm_membership' && $lineItemBAO->contribution_id && $lineItemBAO->entity_id) { + if ($lineItemBAO->entity_table === 'civicrm_membership' && $lineItemBAO->contribution_id && $lineItemBAO->entity_id) { $membershipPaymentParams = [ 'membership_id' => $lineItemBAO->entity_id, 'contribution_id' => $lineItemBAO->contribution_id, diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php index b079114d9f..c0edae5e58 100644 --- a/ext/financialacls/financialacls.php +++ b/ext/financialacls/financialacls.php @@ -146,7 +146,7 @@ function financialacls_civicrm_themes(&$themes) { /** * Intervene to prevent deletion, where permissions block it. * - * @param \CRM_Core_DAO $op + * @param string $op * @param string $objectName * @param int|null $id * @param array $params @@ -155,14 +155,15 @@ function financialacls_civicrm_themes(&$themes) { * @throws \CRM_Core_Exception */ function financialacls_civicrm_pre($op, $objectName, $id, &$params) { - if ($objectName === 'LineItem' && $op === 'delete' && !empty($params['check_permissions'])) { + if ($objectName === 'LineItem' && !empty($params['check_permissions'])) { if (CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus()) { - CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($types, CRM_Core_Action::DELETE); + $operationMap = ['delete' => CRM_Core_Action::DELETE, 'edit' => CRM_Core_Action::UPDATE, 'create' => CRM_Core_Action::ADD]; + CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($types, $operationMap[$op]); if (empty($params['financial_type_id'])) { $params['financial_type_id'] = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_LineItem', $params['id'], 'financial_type_id'); } if (!in_array($params['financial_type_id'], array_keys($types))) { - throw new API_Exception('You do not have permission to delete this line item'); + throw new API_Exception('You do not have permission to ' . $op . ' this line item'); } } } diff --git a/ext/financialacls/tests/phpunit/LineItemTest.php b/ext/financialacls/tests/phpunit/LineItemTest.php index 71eb6380c4..5738472994 100644 --- a/ext/financialacls/tests/phpunit/LineItemTest.php +++ b/ext/financialacls/tests/phpunit/LineItemTest.php @@ -43,7 +43,7 @@ class LineItemTest extends \PHPUnit\Framework\TestCase implements HeadlessInterf public function testLineItemApiPermissions() { $contact1 = $this->individualCreate(); $defaultPriceFieldID = $this->getDefaultPriceFieldID(); - $this->callAPISuccess('Order', 'create', [ + $order = $this->callAPISuccess('Order', 'create', [ 'financial_type_id' => 'Donation', 'contact_id' => $contact1, 'line_items' => [ @@ -73,6 +73,8 @@ class LineItemTest extends \PHPUnit\Framework\TestCase implements HeadlessInterf 'delete in CiviContribute', 'view contributions of type Donation', 'delete contributions of type Donation', + 'add contributions of type Donation', + 'edit contributions of type Donation', ]); Civi::settings()->set('acl_financial_type', TRUE); $this->createLoggedInUser(); @@ -83,6 +85,22 @@ class LineItemTest extends \PHPUnit\Framework\TestCase implements HeadlessInterf $this->callAPISuccess('LineItem', 'Delete', ['check_permissions' => TRUE, 'id' => $lineItems[0]['id']]); $this->callAPIFailure('LineItem', 'Delete', ['check_permissions' => TRUE, 'id' => $lineItems[1]['id']]); + $lineParams = [ + 'entity_id' => $order['id'], + 'entity_table' => 'civicrm_contribution', + 'line_total' => 20, + 'unit_price' => 20, + 'price_field_id' => $defaultPriceFieldID, + 'qty' => 1, + 'financial_type_id' => 'Donation', + 'check_permissions' => TRUE, + ]; + $line = $this->callAPISuccess('LineItem', 'Create', $lineParams); + $lineParams['financial_type_id'] = 'Event Fee'; + $this->callAPIFailure('LineItem', 'Create', $lineParams); + + $this->callAPIFailure('LineItem', 'Create', ['id' => $line['id'], 'check_permissions' => TRUE, 'financial_type_id' => 'Event Fee']); + $this->callAPISuccess('LineItem', 'Create', ['id' => $line['id'], 'check_permissions' => TRUE, 'financial_type_id' => 'Donation']); } /** diff --git a/tests/phpunit/api/v3/FinancialTypeACLTest.php b/tests/phpunit/api/v3/FinancialTypeACLTest.php index 7aa516ebd4..036960b090 100644 --- a/tests/phpunit/api/v3/FinancialTypeACLTest.php +++ b/tests/phpunit/api/v3/FinancialTypeACLTest.php @@ -230,7 +230,7 @@ class api_v3_FinancialTypeACLTest extends CiviUnitTestCase { 'add contributions of type Donation', 'delete contributions of type Donation', ]); - $this->callAPIFailure('contribution', 'create', $params, 'Error in call to LineItem_create : You do not have permission to create this line item'); + $this->callAPIFailure('Contribution', 'create', $params, 'Error in call to LineItem_create : You do not have permission to create this line item'); // Check that the entire contribution has rolled back. $contribution = $this->callAPISuccess('contribution', 'get', []); -- 2.25.1