From 60e5cf3432f2aed4768645c9ab9eb1399e3345c9 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 28 Jul 2021 11:37:00 +1200 Subject: [PATCH] Fix order api to set price field value for default price set This makes it such that when using the default price set for contributions it not only fills the default price field id. I originally had this in the code but I pulled it out because the line item test failed. However, I'm seeing cases where not having a price_field_value_id fails to add tax and I think the issue with the line item test is that it is, incorrecly, trying to add 2 line items on the price field in the default price set - which is not actually the expectation on the default price set I do, separately, wonder if we need the unique id for price_field_id + contribution_id + price_field_value_id - but out of scope on this --- CRM/Financial/BAO/Order.php | 26 +++++++++++ .../Civi/Financialacls/BaseTestClass.php | 45 +++++++++++++++++++ .../Civi/Financialacls/LineItemTest.php | 26 +++-------- tests/phpunit/api/v3/OrderTest.php | 4 +- 4 files changed, 80 insertions(+), 21 deletions(-) diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 542eb85136..ad32b31c45 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -11,6 +11,7 @@ use Civi\Api4\LineItem; use Civi\Api4\PriceField; +use Civi\Api4\PriceFieldValue; use Civi\Api4\PriceSet; /** @@ -230,6 +231,13 @@ class CRM_Financial_BAO_Order { */ protected $defaultPriceField; + /** + * Cache of the default price field value ID. + * + * @var array + */ + protected $defaultPriceFieldValueID; + /** * Get parameters for the entities bought as part of this order. * @@ -663,6 +671,23 @@ class CRM_Financial_BAO_Order { return $this->defaultPriceField['id']; } + /** + * Get the id of the price field to use when just an amount is provided. + * + * @throws \API_Exception + * + * @return int + */ + public function getDefaultPriceFieldValueID():int { + if (!$this->defaultPriceFieldValueID) { + $this->defaultPriceFieldValueID = PriceFieldValue::get(FALSE) + ->addWhere('name', '=', 'contribution_amount') + ->addWhere('price_field_id.name', '=', 'contribution_amount') + ->execute()->first()['id']; + } + return $this->defaultPriceFieldValueID; + } + /** * Get line items. * @@ -1089,6 +1114,7 @@ class CRM_Financial_BAO_Order { $defaults = [ 'qty' => 1, 'price_field_id' => $this->getDefaultPriceFieldID(), + 'price_field_value_id' => $this->getDefaultPriceFieldValueID(), 'entity_table' => 'civicrm_contribution', 'unit_price' => $lineItem['line_total'], 'label' => ts('Contribution Amount'), diff --git a/ext/financialacls/tests/phpunit/Civi/Financialacls/BaseTestClass.php b/ext/financialacls/tests/phpunit/Civi/Financialacls/BaseTestClass.php index c1434d1661..7732d722ad 100644 --- a/ext/financialacls/tests/phpunit/Civi/Financialacls/BaseTestClass.php +++ b/ext/financialacls/tests/phpunit/Civi/Financialacls/BaseTestClass.php @@ -2,6 +2,9 @@ namespace Civi\Financialacls; +use Civi\Api4\PriceField; +use Civi\Api4\PriceFieldValue; +use Civi\Api4\PriceSet; use Civi\Test\HeadlessInterface; use Civi\Test\HookInterface; use Civi\Test\TransactionalInterface; @@ -16,6 +19,13 @@ class BaseTestClass extends \PHPUnit\Framework\TestCase implements HeadlessInter use ContactTestTrait; use Api3TestTrait; + /** + * IDs set up for test. + * + * @var array + */ + protected $ids = []; + /** * @return \Civi\Test\CiviEnvBuilder * @throws \CRM_Extension_Exception_ParseException @@ -57,4 +67,39 @@ class BaseTestClass extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->createLoggedInUser(); } + /** + * Create price set. + * + * @throws \API_Exception + */ + protected function createPriceSet(): void { + $priceSet = PriceSet::create(FALSE)->setValues([ + 'title' => 'Price Set', + 'name' => 'price_set', + 'financial_type_id.name' => 'Event Fee', + 'extends' => 1, + ])->execute()->first(); + $this->ids['PriceSet'][0] = $priceSet['id']; + $this->ids['PriceField'][0] = PriceField::create(FALSE)->setValues([ + 'label' => 'Price Field', + 'name' => 'price_field', + 'html_type' => 'CheckBox', + 'option_label' => ['1' => 'Price Field 1', '2' => 'Price Field 2'], + 'option_value' => ['1' => 100, '2' => 200], + 'option_name' => ['1' => 'Price Field 1', '2' => 'Price Field 2'], + 'option_weight' => ['1' => 1, '2' => 2], + 'option_amount' => ['1' => 100, '2' => 200], + 'is_display_amounts' => 1, + 'weight' => 1, + 'options_per_line' => 1, + 'is_active' => ['1' => 1, '2' => 1], + 'price_set_id' => $priceSet['id'], + 'is_enter_qty' => 1, + 'financial_type_id.name' => 'Event Fee', + ])->execute()->first()['id']; + $this->ids['PriceFieldValue'] = array_keys((array) PriceFieldValue::get() + ->addWhere('price_field_id', '=', $this->ids['PriceField'][0]) + ->execute()->indexBy('id')); + } + } diff --git a/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php b/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php index ac7dd5becd..60692e2a29 100644 --- a/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php +++ b/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php @@ -2,8 +2,6 @@ namespace Civi\Financialacls; -use Civi\Api4\PriceField; - // I fought the Autoloader and the autoloader won. require_once 'BaseTestClass.php'; @@ -27,10 +25,11 @@ class LineItemTest extends BaseTestClass { * Test api applies permissions on line item actions (delete & get). * * @dataProvider versionThreeAndFour + * @throws \API_Exception */ public function testLineItemApiPermissions($version): void { $contact1 = $this->individualCreate(); - $defaultPriceFieldID = $this->getDefaultPriceFieldID(); + $this->createPriceSet(); $order = $this->callAPISuccess('Order', 'create', [ 'financial_type_id' => 'Donation', 'contact_id' => $contact1, @@ -40,13 +39,15 @@ class LineItemTest extends BaseTestClass { [ 'financial_type_id' => \CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), 'line_total' => 40, - 'price_field_id' => $defaultPriceFieldID, + 'price_field_id' => $this->ids['PriceField'][0], + 'price_field_value_id' => $this->ids['PriceFieldValue'][0], 'qty' => 1, ], [ 'financial_type_id' => \CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Member Dues'), 'line_total' => 50, - 'price_field_id' => $defaultPriceFieldID, + 'price_field_id' => $this->ids['PriceField'][0], + 'price_field_value_id' => $this->ids['PriceFieldValue'][1], 'qty' => 1, ], ], @@ -68,7 +69,7 @@ class LineItemTest extends BaseTestClass { 'entity_table' => 'civicrm_contribution', 'line_total' => 20, 'unit_price' => 20, - 'price_field_id' => $defaultPriceFieldID, + 'price_field_id' => $this->ids['PriceField'][0], 'qty' => 1, 'financial_type_id' => 'Donation', 'check_permissions' => ($version == 3), @@ -82,17 +83,4 @@ class LineItemTest extends BaseTestClass { $this->callAPISuccess('LineItem', 'Create', ['id' => $line['id'], 'check_permissions' => ($version == 3), 'financial_type_id' => 'Donation']); } - /** - * @return mixed - * @throws \API_Exception - * @throws \Civi\API\Exception\UnauthorizedException - */ - protected function getDefaultPriceFieldID(): int { - return PriceField::get() - ->addWhere('price_set_id:name', '=', 'default_contribution_amount') - ->addWhere('name', '=', 'contribution_amount') - ->addWhere('html_type', '=', 'Text') - ->addSelect('id')->execute()->first()['id']; - } - } diff --git a/tests/phpunit/api/v3/OrderTest.php b/tests/phpunit/api/v3/OrderTest.php index 9e88aac047..4f83b9915d 100644 --- a/tests/phpunit/api/v3/OrderTest.php +++ b/tests/phpunit/api/v3/OrderTest.php @@ -241,7 +241,7 @@ class api_v3_OrderTest extends CiviUnitTestCase { $params = [ 'contribution_id' => $order['id'], ]; - $order = $this->callAPISuccess('order', 'get', $params); + $order = $this->callAPISuccess('Order', 'get', $params); $expectedResult = [ $order['id'] => [ 'total_amount' => 200, @@ -269,7 +269,7 @@ class api_v3_OrderTest extends CiviUnitTestCase { ], ]; $p['total_amount'] = 300; - $order = $this->callAPISuccess('order', 'create', $p); + $order = $this->callAPISuccess('Order', 'create', $p); $expectedResult = [ $order['id'] => [ 'total_amount' => 300, -- 2.25.1