From 20df462f0bb5d51cca998729805dac08e841f7b1 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 1 Feb 2021 09:34:19 +1300 Subject: [PATCH] Fix membership form to correctly calculate tax when a discount is applied --- CRM/Financial/BAO/Order.php | 62 ++++++++++++++++++- CRM/Member/Form.php | 19 +++++- CRM/Member/Form/Membership.php | 16 +---- .../CRM/Member/Form/MembershipTest.php | 5 +- 4 files changed, 86 insertions(+), 16 deletions(-) diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 2779f003a2..66cfdc2104 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -15,11 +15,14 @@ use Civi\Api4\PriceField; * * @package CRM * @copyright CiviCRM LLC https://civicrm.org/licensing + * * Order class. * * This class is intended to become the object to manage orders, including via Order.api. * * As of writing it is in the process of having appropriate functions built up. + * It should **NOT** be accessed directly outside of tested core methods as it + * may change. */ class CRM_Financial_BAO_Order { @@ -75,6 +78,60 @@ class CRM_Financial_BAO_Order { */ protected $priceFieldMetadata = []; + /** + * Get form object. + * + * @return \CRM_Core_Form|NULL + */ + public function getForm(): ?CRM_Core_Form { + return $this->form; + } + + /** + * Set form object. + * + * @param \CRM_Core_Form|NULL $form + */ + public function setForm(?CRM_Core_Form $form): void { + $this->form = $form; + } + + /** + * The serialize & unserialize functions are to prevent the form being serialized & stored. + * + * The form could be potentially large & circular. + * + * We simply serialize the values needed to re-serialize the form. + * + * @return array + */ + public function _serialize(): array { + return [ + 'OverrideTotalAmount' => $this->getOverrideTotalAmount(), + 'OverrideFinancialType' => $this->getOverrideFinancialTypeID(), + 'PriceSelection' => $this->getPriceSelection(), + ]; + } + + /** + * Re-instantiate the the class with non-calculated variables. + * + * @param array $data + */ + public function _unserialize(array $data): void { + foreach ($data as $key => $value) { + $fn = 'set' . $key; + $this->$fn($value); + } + } + + /** + * Form object - if present the buildAmount hook will be called. + * + * @var \CRM_Member_Form_Membership|\CRM_Member_Form_MembershipRenewal + */ + protected $form; + /** * Get Set override for total amount of the order. * @@ -92,7 +149,7 @@ class CRM_Financial_BAO_Order { * * @param float $overrideTotalAmount */ - public function setOverrideTotalAmount(float $overrideTotalAmount) { + public function setOverrideTotalAmount(float $overrideTotalAmount): void { $this->overrideTotalAmount = $overrideTotalAmount; } @@ -189,6 +246,9 @@ class CRM_Financial_BAO_Order { public function getPriceFieldsMetadata(): array { if (empty($this->priceFieldMetadata)) { $this->priceFieldMetadata = CRM_Price_BAO_PriceSet::getCachedPriceSetDetail($this->getPriceSetID())['fields']; + if ($this->getForm()) { + CRM_Utils_Hook::buildAmount($this->form->getFormContext(), $this->form, $this->priceFieldMetadata); + } } return $this->priceFieldMetadata; } diff --git a/CRM/Member/Form.php b/CRM/Member/Form.php index b476a249a7..76a732dc64 100644 --- a/CRM/Member/Form.php +++ b/CRM/Member/Form.php @@ -70,6 +70,20 @@ class CRM_Member_Form extends CRM_Contribute_Form_AbstractEditPayment { */ protected $order; + /** + * This string is the used for passing to the buildAmount hook. + * + * @var string + */ + protected $formContext = 'membership'; + + /** + * @return string + */ + public function getFormContext(): string { + return $this->formContext; + } + /** * Explicitly declare the entity api name. */ @@ -465,7 +479,10 @@ class CRM_Member_Form extends CRM_Contribute_Form_AbstractEditPayment { $this->order = new CRM_Financial_BAO_Order(); $this->order->setPriceSelectionFromUnfilteredInput($formValues); $this->order->setPriceSetID($this->getPriceSetID($formValues)); - if (isset($formValues['total_amount'])) { + $this->order->setForm($this); + if ($priceSetDetails[$this->order->getPriceSetID()]['is_quick_config'] && isset($formValues['total_amount'])) { + // Amount overrides only permitted on quick config. + // Possibly Order object should enforce this... $this->order->setOverrideTotalAmount($formValues['total_amount']); } $this->order->setOverrideFinancialTypeID((int) $formValues['financial_type_id']); diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php index 9a18e3db5c..685edfd1fc 100644 --- a/CRM/Member/Form/Membership.php +++ b/CRM/Member/Form/Membership.php @@ -1072,20 +1072,10 @@ DESC limit 1"); $termsByType = []; - $lineItem = [$this->_priceSetId => []]; + $lineItem = [$this->order->getPriceSetID() => $this->order->getLineItems()]; - // BEGIN Fix for dev/core/issues/860 - // Prepare fee block and call buildAmount hook - based on CRM_Price_BAO_PriceSet::buildPriceSet(). - CRM_Utils_Hook::buildAmount('membership', $this, $this->_priceSet['fields']); - // END Fix for dev/core/issues/860 - - CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'], - $formValues, $lineItem[$this->_priceSetId], $this->_priceSetId); - - if (!empty($formValues['tax_amount'])) { - $params['tax_amount'] = $formValues['tax_amount']; - } - $params['total_amount'] = $formValues['amount'] ?? NULL; + $params['tax_amount'] = $this->order->getTotalTaxAmount(); + $params['total_amount'] = $this->order->getTotalAmount(); if (!empty($lineItem[$this->_priceSetId])) { foreach ($lineItem[$this->_priceSetId] as &$li) { if (!empty($li['membership_type_id'])) { diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index 3a00ecbe26..d3591809b4 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -1153,12 +1153,14 @@ Expires: ', public function testTwoMembershipsViaPriceSetInBackendWithDiscount(): void { // Register buildAmount hook to apply discount. $this->hookClass->setHook('civicrm_buildAmount', [$this, 'buildAmountMembershipDiscount']); - + $this->enableTaxAndInvoicing(); + $this->addTaxAccountToFinancialType(2); // Create two memberships for individual $this->_individualId, via a price set in the back end. $this->createTwoMembershipsViaPriceSetInBackEnd($this->_individualId); $contribution = $this->callAPISuccessGetSingle('Contribution', [ 'contact_id' => $this->_individualId, ]); + // Note: we can't check for the contribution total being discounted, because the total is set // when the contribution is created via $form->testSubmit(), but buildAmount isn't called // until testSubmit() runs. Fixing that might involve making testSubmit() more sophisticated, @@ -1170,6 +1172,7 @@ Expires: ', $this->assertEquals(2, $lineItemResult['count']); $discountedItems = 0; foreach ($lineItemResult['values'] as $lineItem) { + $this->assertEquals($lineItem['line_total'] * .1, $lineItem['tax_amount']); if (CRM_Utils_String::startsWith($lineItem['label'], 'Long Haired Goat')) { $this->assertEquals(15.0, $lineItem['line_total']); $this->assertEquals('Long Haired Goat - one leg free!', $lineItem['label']); -- 2.25.1