From 47465c929cf2116741065c278a538a36c7a02ce7 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 27 Mar 2019 19:23:28 +1300 Subject: [PATCH] Move code to assign tax information into shared parent --- CRM/Contribute/Form/Contribution/Confirm.php | 22 +++++++----- .../Form/FrontEndPaymentFormTrait.php | 34 +++++++++++++------ .../Contribute/Form/Contribution/Confirm.tpl | 6 ++-- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index f6841fc13675..c23c719b09d0 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -511,15 +511,20 @@ public function buildQuickForm() { // Make a copy of line items array to use for display only $tplLineItems = $this->_lineItem; if (CRM_Invoicing_Utils::isInvoicingEnabled()) { - list($getTaxDetails, $tplLineItems) = $this->alterLineItemsForTemplate($tplLineItems); - $this->assign('getTaxDetails', $getTaxDetails); - $this->assign('taxTerm', CRM_Invoicing_Utils::getTaxTerm()); + // @todo $params seems like exactly the wrong place to get totalTaxAmount from + // this is a calculated variable so we it should be transparent how we + // calculated it rather than coming from 'params' $this->assign('totalTaxAmount', $params['tax_amount']); } - if ($this->_priceSetId && !CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $this->_priceSetId, 'is_quick_config')) { - $this->assign('lineItem', $tplLineItems); - } - else { + $this->assignLineItemsToTemplate($tplLineItems); + + $isDisplayLineItems = $this->_priceSetId && !CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceSet', $this->_priceSetId, 'is_quick_config'); + + $this->assign('isDisplayLineItems', $isDisplayLineItems); + + if (!$isDisplayLineItems) { + // quickConfig is deprecated in favour of isDisplayLineItems. Lots of logic has been harnessed to quick config + // whereas isDisplayLineItems is specific & clear. $this->assign('is_quick_config', 1); $this->_params['is_quick_config'] = 1; } @@ -1811,8 +1816,7 @@ protected function getIsPending() { // The concept of contributeMode is deprecated. // the is_monetary concept probably should be too as it can be calculated from // the existence of 'amount' & seems fragile. - if (((isset($this->_contributeMode)) || !empty - ($this->_params['is_pay_later']) + if (((isset($this->_contributeMode)) || !empty($this->_params['is_pay_later']) ) && (($this->_values['is_monetary'] && $this->_amount > 0.0)) ) { diff --git a/CRM/Financial/Form/FrontEndPaymentFormTrait.php b/CRM/Financial/Form/FrontEndPaymentFormTrait.php index c65d8898ed44..638666a0bc0c 100644 --- a/CRM/Financial/Form/FrontEndPaymentFormTrait.php +++ b/CRM/Financial/Form/FrontEndPaymentFormTrait.php @@ -43,25 +43,39 @@ trait CRM_Financial_Form_FrontEndPaymentFormTrait { * CRM_Invoicing_Utils class with a potential end goal of moving this handling to an extension. * * @param $tplLineItems - * - * @return array */ - protected function alterLineItemsForTemplate($tplLineItems) { - $getTaxDetails = FALSE; + protected function alterLineItemsForTemplate(&$tplLineItems) { + if (!CRM_Invoicing_Utils::isInvoicingEnabled()) { + return; + } + // @todo this should really be the first time we are determining + // the tax rates - we can calculate them from the financial_type_id + // & amount here so we didn't need a deeper function to semi-get + // them but not be able to 'format them right' because they are + // potentially being used for 'something else'. + // @todo invoicing code - please feel the hate. Also move this 'hook-like-bit' + // to the CRM_Invoicing_Utils class. foreach ($tplLineItems as $key => $value) { foreach ($value as $k => $v) { if (isset($v['tax_rate']) && $v['tax_rate'] != '') { - $getTaxDetails = TRUE; + $this->assign('getTaxDetails', TRUE); + $this->assign('taxTerm', CRM_Invoicing_Utils::getTaxTerm()); // Cast to float to display without trailing zero decimals $tplLineItems[$key][$k]['tax_rate'] = (float) $v['tax_rate']; } } } - // @todo fix this to only return $tplLineItems. Calling function can check for tax rate and - // do all invoicing related assigns - // another discrete function (it's just one more iteration through a an array with only a handful of - // lines so the separation of concerns is more important than 'efficiency' - return [$getTaxDetails, $tplLineItems]; + } + + /** + * Assign line items to the template. + * + * @param $tplLineItems + */ + protected function assignLineItemsToTemplate($tplLineItems) { + // @todo this should be a hook that invoicing code hooks into rather than a call to it. + $this->alterLineItemsForTemplate($tplLineItems); + $this->assign('lineItem', $tplLineItems); } } diff --git a/templates/CRM/Contribute/Form/Contribution/Confirm.tpl b/templates/CRM/Contribute/Form/Contribution/Confirm.tpl index b9f3e1fbc4c7..a3553a4daaa9 100644 --- a/templates/CRM/Contribute/Form/Contribution/Confirm.tpl +++ b/templates/CRM/Contribute/Form/Contribution/Confirm.tpl @@ -44,16 +44,16 @@ {include file="CRM/Contribute/Form/Contribution/MembershipBlock.tpl" context="confirmContribution"} - {if $amount GTE 0 OR $minimum_fee GTE 0 OR ( $priceSetID and $lineItem ) } + {if $amount GTE 0 OR $minimum_fee GTE 0 OR ( $isDisplayLineItems and $lineItem ) }
{if !$useForMember}
- {if !$membershipBlock AND $amount OR ( $priceSetID and $lineItem ) }{ts}Contribution Amount{/ts}{else}{ts}Membership Fee{/ts} {/if} + {if !$membershipBlock AND $amount OR ( $isDisplayLineItems and $lineItem ) }{ts}Contribution Amount{/ts}{else}{ts}Membership Fee{/ts} {/if}
{/if}
{if !$useForMember} - {if $lineItem and $priceSetID} + {if $lineItem and $isDisplayLineItems} {if !$amount}{assign var="amount" value=0}{/if} {assign var="totalAmount" value=$amount} {include file="CRM/Price/Page/LineItem.tpl" context="Contribution"}