diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index 970047b1ba0e..adc6c57bd73b 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -633,58 +633,40 @@ public static function changeFeeSelections( // fetch submitted LineItems from input params and feeBlock information $submittedLineItems = $lineItemObj->getSubmittedLineItems($params, $feeBlock); - // retrieve the submitted price field value IDs from $submittedLineItems array keys - $submittedPriceFieldValueIDs = empty($submittedLineItems) ? array() : array_keys($submittedLineItems); - $requiredChanges = $lineItemObj->getLineItemsToAlter($submittedLineItems, $entityID, $entity); - // cancel previous line item - $additionalWhereClause = empty($submittedPriceFieldValueIDs) ? NULL : sprintf("price_field_value_id NOT IN (%s)", implode(', ', $submittedPriceFieldValueIDs)); - $lineItemObj->cancelLineItems($entityID, $entityTable, $additionalWhereClause); - // get financial information that need to be recorded on basis on submitted price field value IDs - $financialItemsArray = $lineItemObj->getReverseFinancialItemsToRecord( - $entityID, - $entityTable, - $contributionId, - $submittedPriceFieldValueIDs - ); + if (!empty($requiredChanges['line_items_to_cancel']) || !empty($requiredChanges['line_items_to_update'])) { + // @todo - this IF is to get this through PR merge but I suspect that it should not + // be necessary & is masking something else. + $financialItemsArray = $lineItemObj->getReverseFinancialItemsToRecord( + $entityID, + $entityTable, + $contributionId, + array_keys($requiredChanges['line_items_to_cancel']), + $requiredChanges['line_items_to_update'] + ); + } // update line item with changed line total and other information $totalParticipant = $participantCount = 0; $amountLevel = array(); if (!empty($requiredChanges['line_items_to_update'])) { foreach ($requiredChanges['line_items_to_update'] as $priceFieldValueID => $value) { - $taxAmount = "NULL"; - if (isset($value['tax_amount'])) { - $taxAmount = $value['tax_amount']; - } $amountLevel[] = $value['label'] . ' - ' . (float) $value['qty']; if ($entity == 'participant' && isset($value['participant_count'])) { - $participantCount = $value['participant_count']; $totalParticipant += $value['participant_count']; } - $updateLineItemSQL = " - UPDATE civicrm_line_item li - SET li.qty = {$value['qty']}, - li.line_total = {$value['line_total']}, - li.tax_amount = {$taxAmount}, - li.unit_price = {$value['unit_price']}, - li.participant_count = {$participantCount}, - li.label = %1 - WHERE (li.entity_table = '{$entityTable}' AND li.entity_id = {$entityID}) AND - (price_field_value_id = {$priceFieldValueID}) "; - - CRM_Core_DAO::executeQuery($updateLineItemSQL, array(1 => array($value['label'], 'String'))); } } - // insert new 'adjusted amount' transaction entry and update contribution entry. - // ensure entity_financial_trxn table has a linking of it. - // insert new line items + foreach (array_merge($requiredChanges['line_items_to_resurrect'], $requiredChanges['line_items_to_cancel'], $requiredChanges['line_items_to_update']) as $lineItemToAlter) { + // Must use BAO rather than api because a bad line it in the api which we want to avoid. + CRM_Price_BAO_LineItem::create($lineItemToAlter); + } + $lineItemObj->addLineItemOnChangeFeeSelection($requiredChanges['line_items_to_add'], $entityID, $entityTable, $contributionId); - // the recordAdjustedAmt code would execute over here $count = 0; if ($entity == 'participant') { $count = count(CRM_Event_BAO_Participant::getParticipantIds($contributionId)); @@ -714,14 +696,15 @@ public static function changeFeeSelections( } $trxn = $lineItemObj->recordAdjustedAmt($updatedAmount, $paidAmount, $contributionId, $taxAmount, $updateAmountLevel); - $contributionCompletedStatusID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_DAO_Contribution', 'contribution_status_id', 'Completed'); + $contributionStatus = CRM_Core_PseudoConstant::getName('CRM_Contribute_DAO_Contribution', 'contribution_status_id', CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionId, 'contribution_status_id')); + if (!empty($financialItemsArray)) { foreach ($financialItemsArray as $updateFinancialItemInfoValues) { $newFinancialItem = CRM_Financial_BAO_FinancialItem::create($updateFinancialItemInfoValues); // record reverse transaction only if Contribution is Completed because for pending refund or // partially paid we are already recording the surplus owed or refund amount - if (!empty($updateFinancialItemInfoValues['financialTrxn']) && ($contributionCompletedStatusID == - CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $contributionId, 'contribution_status_id')) + if (!empty($updateFinancialItemInfoValues['financialTrxn']) && ($contributionStatus == 'Completed' + ) ) { $updateFinancialItemInfoValues = array_merge($updateFinancialItemInfoValues['financialTrxn'], array( 'entity_id' => $newFinancialItem->id, @@ -737,89 +720,48 @@ public static function changeFeeSelections( )); unset($updateFinancialItemInfoValues['financialTrxn']); } - if (!empty($updateFinancialItemInfoValues['tax'])) { - $updateFinancialItemInfoValues['tax']['amount'] = $updateFinancialItemInfoValues['amount']; - $updateFinancialItemInfoValues['tax']['description'] = $updateFinancialItemInfoValues['description']; - if (!empty($updateFinancialItemInfoValues['financial_account_id'])) { - $updateFinancialItemInfoValues['financial_account_id'] = $updateFinancialItemInfoValues['tax']['financial_account_id']; - } - CRM_Financial_BAO_FinancialItem::create($updateFinancialItemInfoValues); - } } } + // @todo - it may be that trxn_id is always empty - flush out scenarios. Add tests. $trxnId = !empty($trxn->id) ? array('id' => $trxn->id) : array(); - $lineItemObj->addFinancialItemsOnLineItemsChange($requiredChanges['line_items_to_add'], $entityID, $entityTable, $contributionId, $trxnId); + $lineItemObj->addFinancialItemsOnLineItemsChange(array_merge($requiredChanges['line_items_to_add'], $requiredChanges['line_items_to_resurrect']), $entityID, $entityTable, $contributionId, $trxnId); // update participant fee_amount column $lineItemObj->updateEntityRecordOnChangeFeeSelection($params, $entityID, $entity); } /** - * Function to cancel Lineitem whose corrosponding price field option is - * unselected on membership or participant backoffice form - * - * @param int $entityID - * @param string $entityTable - * @param string $additionalWhereClause - * - */ - protected function cancelLineItems($entityID, $entityTable, $additionalWhereClause = NULL) { - $whereClauses = array( - "li.entity_id = %1", - "li.entity_table = %2", - ); - if ($additionalWhereClause) { - $whereClauses[] = $additionalWhereClause; - } - - $where = implode(' AND ', $whereClauses); - $sql = " - UPDATE civicrm_line_item li - INNER JOIN civicrm_financial_item fi ON (li.id = fi.entity_id AND fi.entity_table = 'civicrm_line_item') - SET li.qty = 0, - li.line_total = 0.00, - li.tax_amount = NULL, - li.participant_count = 0, - li.non_deductible_amount = 0.00 - WHERE {$where} "; - - CRM_Core_DAO::executeQuery($sql, array( - 1 => array($entityID, 'Integer'), - 2 => array($entityTable, 'String'), - )); - } - - /** - * Function to retrieve formatted financial items that need to be recorded as result of changed fee + * Function to retrieve financial items that need to be recorded as result of changed fee * * @param int $entityID * @param string $entityTable * @param int $contributionID - * @param array $submittedPriceFieldValueIDs + * @param array $priceFieldValueIDsToCancel + * @param array $lineItemsToUpdate * * @return array - * List of formatted Financial Items to be recorded + * List of formatted reverse Financial Items to be recorded */ - protected function getReverseFinancialItemsToRecord($entityID, $entityTable, $contributionID, $submittedPriceFieldValueIDs) { + protected function getReverseFinancialItemsToRecord($entityID, $entityTable, $contributionID, $priceFieldValueIDsToCancel, $lineItemsToUpdate) { $previousLineItems = CRM_Price_BAO_LineItem::getLineItems($entityID, str_replace('civicrm_', '', $entityTable)); $financialItemsArray = array(); - - if (empty($submittedPriceFieldValueIDs)) { - return $financialItemsArray; - } - $financialItemResult = $this->getNonCancelledFinancialItems($entityID, $entityTable); - foreach ($financialItemResult as $updateFinancialItemInfoValues) { $updateFinancialItemInfoValues['transaction_date'] = date('YmdHis'); - // the below params are not needed + + // the below params are not needed as we are creating new financial item $previousFinancialItemID = $updateFinancialItemInfoValues['id']; + $totalFinancialAmount = $this->checkFinancialItemTotalAmountByLineItemID($updateFinancialItemInfoValues['entity_id']); unset($updateFinancialItemInfoValues['id']); unset($updateFinancialItemInfoValues['created_date']); + // if not submitted and difference is not 0 make it negative - if (!in_array($updateFinancialItemInfoValues['price_field_value_id'], $submittedPriceFieldValueIDs) && $updateFinancialItemInfoValues['differenceAmt'] != 0) { + if ((empty($lineItemsToUpdate) || (in_array($updateFinancialItemInfoValues['price_field_value_id'], $priceFieldValueIDsToCancel) && + $totalFinancialAmount == $updateFinancialItemInfoValues['amount']) + ) && $updateFinancialItemInfoValues['amount'] > 0 + ) { // INSERT negative financial_items $updateFinancialItemInfoValues['amount'] = -$updateFinancialItemInfoValues['amount']; // reverse the related financial trxn too @@ -827,33 +769,29 @@ protected function getReverseFinancialItemsToRecord($entityID, $entityTable, $co if ($previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']) { $updateFinancialItemInfoValues['tax']['amount'] = -($previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']); $updateFinancialItemInfoValues['tax']['description'] = $this->getSalesTaxTerm(); - if ($updateFinancialItemInfoValues['financial_type_id']) { - $updateFinancialItemInfoValues['tax']['financial_account_id'] = CRM_Contribute_BAO_Contribution::getFinancialAccountId($updateFinancialItemInfoValues['financial_type_id']); - } } // INSERT negative financial_items for tax amount - $financialItemsArray[] = $updateFinancialItemInfoValues; - } - // if submitted and difference is 0 add a positive entry again - elseif (in_array($updateFinancialItemInfoValues['price_field_value_id'], $submittedPriceFieldValueIDs) && $updateFinancialItemInfoValues['differenceAmt'] == 0) { - $updateFinancialItemInfoValues['amount'] = $updateFinancialItemInfoValues['amount']; - // INSERT financial_items for tax amount - if ($updateFinancialItemInfoValues['entity_id'] == $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['id'] && - isset($lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['tax_amount']) - ) { - $updateFinancialItemInfoValues['tax']['amount'] = $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['tax_amount']; - $updateFinancialItemInfoValues['tax']['description'] = $this->getSalesTaxTerm(); - if ($lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['financial_type_id']) { - $updateFinancialItemInfoValues['tax']['financial_account_id'] = CRM_Contribute_BAO_Contribution::getFinancialAccountId($lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['financial_type_id']); - } - } - $financialItemsArray[] = $updateFinancialItemInfoValues; + $financialItemsArray[$updateFinancialItemInfoValues['entity_id']] = $updateFinancialItemInfoValues; } } return $financialItemsArray; } + /** + * Helper function to return sum of financial item's amount related to a line-item + * @param array $lineItemID + * + * @return float $financialItem + */ + protected function checkFinancialItemTotalAmountByLineItemID($lineItemID) { + return CRM_Core_DAO::singleValueQuery(" + SELECT SUM(amount) + FROM civicrm_financial_item + WHERE entity_table = 'civicrm_line_item' AND entity_id = {$lineItemID} + "); + } + /** * Helper function to retrieve submitted line items from form values $inputParams and used $feeBlock * @@ -873,14 +811,27 @@ protected function getSubmittedLineItems($inputParams, $feeBlock) { } /** - * Helper function to retrieve formatted line items that need to be altered. + * Helper function to retrieve line items that need to be altered. + * + * We iterate through the previous line items for the given entity to determine + * what alterations to line items need to be made to reflect the new line items. + * + * There are 4 possible changes required - per the keys in the return array. * * @param array $submittedLineItems * @param int $entityID * @param string $entity * * @return array - * Array of formatted line items + * Array of line items to alter with the following keys + * - line_items_to_add. If the line items required are new radio options that + * have not previously been set then we should add line items for them + * - line_items_to_update. If we have already been an active option and a change has + * happened then it should be in this array. + * - line_items_to_cancel. Line items currently selected but not selected in the new selection. + * These need to be zero'd out. + * - line_items_to_resurrect. Line items previously selected and then deselected. These need to be + * re-enabled rather than a new one added. */ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity) { $previousLineItems = CRM_Price_BAO_LineItem::getLineItems($entityID, $entity); @@ -888,32 +839,50 @@ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity) $lineItemsToAdd = $submittedLineItems; $lineItemsToUpdate = array(); $submittedPriceFieldValueIDs = array_keys($submittedLineItems); + $lineItemsToCancel = $lineItemsToResurrect = array(); foreach ($previousLineItems as $id => $previousLineItem) { - // check through the submitted items if the previousItem exists, - // if found in submitted items, do not use it for new item creations if (in_array($previousLineItem['price_field_value_id'], $submittedPriceFieldValueIDs)) { $submittedLineItem = $submittedLineItems[$previousLineItem['price_field_value_id']]; - // if submitted line items are existing don't fire INSERT query - if ($previousLineItem['line_total'] != 0) { + if (CRM_Utils_Array::value('html_type', $lineItemsToAdd[$previousLineItem['price_field_value_id']]) == 'Text') { + // If a 'Text' price field was updated by changing qty value, then we are not adding new line-item but updating the existing one, + // because unlike other kind of price-field, it's related price-field-value-id isn't changed and thats why we need to make an + // exception here by adding financial item for updated line-item and will reverse any previous financial item entries. + $lineItemsToUpdate[$previousLineItem['price_field_value_id']] = array_merge($submittedLineItem, array('id' => $id)); unset($lineItemsToAdd[$previousLineItem['price_field_value_id']]); } else { - $submittedLineItem['skip'] = TRUE; + $submittedLineItem = $submittedLineItems[$previousLineItem['price_field_value_id']]; + // for updating the line items i.e. use-case - once deselect-option selecting again + if (($previousLineItem['line_total'] != $submittedLineItem['line_total']) + || ( + // This would be a $0 line item - but why it should be catered to + // other than when the above condition is unclear. + $submittedLineItem['line_total'] == 0 && $submittedLineItem['qty'] == 1 + ) + || ( + $previousLineItem['qty'] != $submittedLineItem['qty'] + ) + ) { + $lineItemsToUpdate[$previousLineItem['price_field_value_id']] = $submittedLineItem; + $lineItemsToUpdate[$previousLineItem['price_field_value_id']]['id'] = $id; + // Format is actually '0.00' + if ($previousLineItem['line_total'] == 0) { + $lineItemsToAdd[$previousLineItem['price_field_value_id']]['id'] = $id; + $lineItemsToResurrect[] = $lineItemsToAdd[$previousLineItem['price_field_value_id']]; + } + } + // If there was previously a submitted line item for the same option value then there is + // either no change or a qty adjustment. In either case we are not doing an add + reversal. + unset($lineItemsToAdd[$previousLineItem['price_field_value_id']]); + unset($lineItemsToCancel[$previousLineItem['price_field_value_id']]); } - // for updating the line items i.e. use-case - once deselect-option selecting again - if (($previousLineItem['line_total'] != $submittedLineItem['line_total']) - || ( - // This would be a $0 line item - but why it should be catered to - // other than when the above condition is unclear. - $submittedLineItem['line_total'] == 0 && $submittedLineItem['qty'] == 1 - ) - || ( - $previousLineItem['qty'] != $submittedLineItem['qty'] - ) - ) { - $lineItemsToUpdate[$previousLineItem['price_field_value_id']] = $submittedLineItem; - $lineItemsToUpdate[$previousLineItem['price_field_value_id']]['id'] = $id; + } + else { + if (!$this->isCancelled($previousLineItem)) { + $cancelParams = array('qty' => 0, 'line_total' => 0, 'tax_amount' => 0, 'participant_count' => 0, 'non_deductible_amount' => 0, 'id' => $id); + $lineItemsToCancel[$previousLineItem['price_field_value_id']] = array_merge($previousLineItem, $cancelParams); + } } } @@ -921,11 +890,25 @@ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity) return array( 'line_items_to_add' => $lineItemsToAdd, 'line_items_to_update' => $lineItemsToUpdate, + 'line_items_to_cancel' => $lineItemsToCancel, + 'line_items_to_resurrect' => $lineItemsToResurrect, ); } /** - * Helper function to add lineitems or financial item related to it, to as result of fee change + * Check if a line item has already been cancelled. + * + * @param array $lineItem + * + * @return bool + */ + protected function isCancelled($lineItem) { + if ($lineItem['qty'] == 0 && $lineItem['line_total'] == 0) { + return TRUE; + } + } + /** + * Add line Items as result of fee change. * * @param array $lineItemsToAdd * @param int $entityID @@ -943,6 +926,10 @@ protected function addLineItemOnChangeFeeSelection( return; } + $changedFinancialTypeID = NULL; + $updatedContribution = new CRM_Contribute_BAO_Contribution(); + $updatedContribution->id = (int) $contributionID; + // insert financial items foreach ($lineItemsToAdd as $priceFieldValueID => $lineParams) { $lineParams = array_merge($lineParams, array( 'entity_table' => $entityTable, @@ -953,48 +940,35 @@ protected function addLineItemOnChangeFeeSelection( self::create($lineParams); } } + + if ($changedFinancialTypeID) { + $updatedContribution->financial_type_id = $changedFinancialTypeID; + $updatedContribution->save(); + } } /** - * Helper function to add lineitems or financial item related to it, to as result of fee change + * Add financial transactions when an array of line items is changed. * * @param array $lineItemsToAdd * @param int $entityID * @param string $entityTable * @param int $contributionID - * @param array $adjustedFinancialTrxnID - * - * @return void + * @param bool $isCreateAdditionalFinancialTrxn + * Is there a change to the total balance requiring additional transactions to be created. */ - protected function addFinancialItemsOnLineItemsChange( - $lineItemsToAdd, - $entityID, - $entityTable, - $contributionID, - $adjustedFinancialTrxnID = NULL - ) { - // if there is no line item to add, do not proceed - if (empty($lineItemsToAdd)) { - return; - } + protected function addFinancialItemsOnLineItemsChange($lineItemsToAdd, $entityID, $entityTable, $contributionID, $isCreateAdditionalFinancialTrxn) { + $updatedContribution = new CRM_Contribute_BAO_Contribution(); + $updatedContribution->id = $contributionID; + $updatedContribution->find(TRUE); - $changedFinancialTypeID = NULL; - $fetchCon = array('id' => $contributionID); - $updatedContribution = CRM_Contribute_BAO_Contribution::retrieve($fetchCon, CRM_Core_DAO::$_nullArray, CRM_Core_DAO::$_nullArray); - // insert financial items foreach ($lineItemsToAdd as $priceFieldValueID => $lineParams) { - $tempFinancialTrxnID = $adjustedFinancialTrxnID; $lineParams = array_merge($lineParams, array( 'entity_table' => $entityTable, 'entity_id' => $entityID, 'contribution_id' => $contributionID, )); - $changedFinancialTypeID = $this->addFinancialItemsOnLineItemChange(empty($adjustedFinancialTrxnID), $lineParams, $updatedContribution, $tempFinancialTrxnID, $changedFinancialTypeID); - } - - if ($changedFinancialTypeID) { - $updatedContribution->financial_type_id = $changedFinancialTypeID; - $updatedContribution->save(); + $this->addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution); } } @@ -1145,15 +1119,12 @@ protected function recordAdjustedAmt($updatedAmount, $paidAmount, $contributionI * @param bool $isCreateAdditionalFinancialTrxn * @param array $lineParams * @param \CRM_Contribute_BAO_Contribution $updatedContribution - * @param int $tempFinancialTrxnID - * @param int|NULL $changedFinancialTypeID - * - * @return int|NULL */ - protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution, $tempFinancialTrxnID, $changedFinancialTypeID) { + protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution) { + $tempFinancialTrxnID = NULL; // don't add financial item for cancelled line item if ($lineParams['qty'] == 0) { - return $changedFinancialTypeID; + return; } elseif ($isCreateAdditionalFinancialTrxn) { // This routine & the return below is super uncomfortable. @@ -1181,11 +1152,6 @@ protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancia $adjustedTrxn = CRM_Core_BAO_FinancialTrxn::create($adjustedTrxnValues); $tempFinancialTrxnID = array('id' => $adjustedTrxn->id); } - // don't add financial item if line_total and financial type aren't changed, - // which is identified by empty $adjustedFinancialTrxnID - else { - return $changedFinancialTypeID; - } } $lineObj = CRM_Price_BAO_LineItem::retrieve($lineParams, CRM_Core_DAO::$_nullArray); // insert financial items @@ -1194,7 +1160,6 @@ protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancia if (isset($lineObj->tax_amount)) { CRM_Financial_BAO_FinancialItem::add($lineObj, $updatedContribution, TRUE, $tempFinancialTrxnID); } - return $changedFinancialTypeID; } /** @@ -1207,8 +1172,9 @@ protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancia * Array of financial items that have not be reversed. */ protected function getNonCancelledFinancialItems($entityID, $entityTable) { + // gathering necessary info to record negative (deselected) financial_item $updateFinancialItem = " - SELECT fi.*, SUM(fi.amount) as differenceAmt, price_field_value_id, financial_type_id, tax_amount + SELECT fi.*, price_field_value_id, financial_type_id, tax_amount FROM civicrm_financial_item fi LEFT JOIN civicrm_line_item li ON (li.id = fi.entity_id AND fi.entity_table = 'civicrm_line_item') WHERE (li.entity_table = '{$entityTable}' AND li.entity_id = {$entityID}) GROUP BY li.entity_table, li.entity_id, price_field_value_id, fi.id diff --git a/tests/phpunit/CRM/Event/BAO/CRM19273Test.php b/tests/phpunit/CRM/Event/BAO/CRM19273Test.php index 16f066812222..88bb0839d90d 100644 --- a/tests/phpunit/CRM/Event/BAO/CRM19273Test.php +++ b/tests/phpunit/CRM/Event/BAO/CRM19273Test.php @@ -224,10 +224,8 @@ public function testCRM19273() { $priceSetParams['price_1'] = 1; $lineItem = CRM_Price_BAO_LineItem::getLineItems($this->_participantId, 'participant'); - // return here as the following lines will not work until the reset of PR 10962 has been merged. - return; - CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem, $this->_expensiveFee); + $this->balanceCheck($this->_expensiveFee); $priceSetParams['price_1'] = 3;