CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export #10962

eileenmcnaughton · 2017-09-10T23:53:45Z

Overview

Steps to replicate:

Create pending pay later event registration with quantity selections
Pending pay later contribution no showing any financial transactions as expected.
Edit selections on event registration to a higher amount.

Before

Pending pay later contribution STILL no showing any financial transactions as expected.
There is no change in the financial_item to record this fee change. There should be entries reflecting this change.

After

Pending pay later contribution STILL no showing any financial transactions as expected.
There are three entries in civicrm_financial_item. Let me explain with a use-case:

Register an event using price-option A ($800) and pay-later
Then on change fee selection to price-option B ($1000)
After that three financial item entries are there in DB to record these changes

Here participant_id = 76

CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export

eileenmcnaughton · 2017-09-13T21:56:16Z

test this please

eileenmcnaughton · 2017-09-18T02:46:42Z

@pradpnayak @monishdeb @JoeMurray can I get your help on this one. This is the only remaining critical financial bug but I couldn't find my way through to resolving it

monishdeb · 2017-10-06T10:59:14Z

CRM/Price/BAO/LineItem.php

@@ -819,60 +825,107 @@ protected function _getFinancialItemsToRecord($entityID, $entityTable, $contribu

    // 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


There is no use of it as grouping is on fi.id basis

I't actually grouping on multiple fields not just id - is this relevant to this fix or just an additional tidy-up (if the latter I vote to remove from this PR)

AH I see further down - this IS part of the change - not relying on this value anymore

So to articulate -'differenceAmt' is a really unreliable way of determining what the status is and you are ditching it in favour of calculating the total recorded amount against each line item in the checkFinancialItemTotalAmountByLineItemID function. If the row is a match it then gets listed to be reverted.

I'm not sure yet how you avoid reverting multiple times as you clock up changes. Also, this seems to only revert positive lines - what if a negative contribution is altered - is that possible? We should try to find a way to test that

AH ok - you avoid reverting multiple times by keying on line item with this $financialItemsArray[$updateFinancialItemInfoValues['entity_id']]

It's worth noting that on ?ubuntu? you can't rely on natural sort to put the highest id last (which I think you are relying on - but perhaps the group by will cause it to sort like that - I wonder how we can check - I know Jaap was using the system without the natural sort from some test bugs that he hit)

@eileenmcnaughton the differenceAmt was an alias used in old code to calculate the sum of all financial amount that is related to a line-item, this difference amount is used to decide the surplus or due amount so to make an adjustment on changeFeeSelections. But later to handle adjustment at more detail esp. for mult-line-item contribution we started to fetch result on basis of financia_item.id. So then I realized that there's no use of the sum now as it represents each financial_item's amount. So yes it's a tidy up

a tidy up - but in the end I concluded that it was necessary to stop using it to make the fix work

monishdeb · 2017-10-06T12:03:04Z

@eileenmcnaughton I have fixed the test failures and completed the remaining change (new test failures are not related). However, I was unable to capture the use-case for UT.

eileenmcnaughton · 2017-10-10T04:19:14Z

test this please

eileenmcnaughton

@monishdeb I've gone through this & assessed the changes as are covered by the unit test. I'm satisfied that the unit test provided demonstrates the issue & that part of this patch solved the issue. I've pasted below the minimum required change to solve the issue. Note that I got to the patch below by removing the change & running the test & working through copying parts of the test in locally until I could see what was happening & the test was passing. I've pasted it in mostly to have a copy of what I actually managed to understand :-)

The rest of this patch addresses the issue when sales tax is enabled. I have not assessed that portion as yet & would hope that some others might look as well. This is the last of the data-related critical issues after the recent big push. Hopefully @KarinG @agileware can take a look at the patch as it relates to sales. What can we do to extend the test to cover sales? I'm OK with it testing the changeFeeAmount function rather than form submit based on my digging so far.

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index 2b782e4..1014f0d 100644
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -826,18 +826,21 @@ WHERE li.contribution_id = %1";
     ";
     $updateFinancialItemInfoDAO = CRM_Core_DAO::executeQuery($updateFinancialItem);
 
-    $invoiceSettings = Civi::settings()->get('contribution_invoice_settings');
-    $taxTerm = CRM_Utils_Array::value('tax_term', $invoiceSettings);
-    $updateFinancialItemInfoValues = array();
-    while ($updateFinancialItemInfoDAO->fetch()) {
-      $updateFinancialItemInfoValues = (array) $updateFinancialItemInfoDAO;
+    $taxTerm = $this->getTaxTerm();
+
+    $financialItemResult = $updateFinancialItemInfoDAO->fetchAll();
+    foreach ($financialItemResult as $updateFinancialItemInfoValues) {
       $updateFinancialItemInfoValues['transaction_date'] = date('YmdHis');
       // the below params are not needed
       $previousFinancialItemID = $updateFinancialItemInfoValues['id'];
       unset($updateFinancialItemInfoValues['id']);
       unset($updateFinancialItemInfoValues['created_date']);
+      $totalFinancialAmount = $this->_checkFinancialItemTotalAmountByLineItemID($updateFinancialItemInfoValues['entity_id']);
       // if not submitted and difference is not 0 make it negative
-      if (!in_array($updateFinancialItemInfoValues['price_field_value_id'], $submittedPriceFieldValueIDs) && $updateFinancialItemInfoValues['differenceAmt'] != 0) {
+      if (!in_array($updateFinancialItemInfoValues['price_field_value_id'], $submittedPriceFieldValueIDs) &&
+        $updateFinancialItemInfoValues['amount'] > 0 &&
+        $totalFinancialAmount == $updateFinancialItemInfoValues['amount']
+      ) {
         // INSERT negative financial_items
         $updateFinancialItemInfoValues['amount'] = -$updateFinancialItemInfoValues['amount'];
         // reverse the related financial trxn too
@@ -850,7 +853,7 @@ WHERE li.contribution_id = %1";
           }
         }
         // INSERT negative financial_items for tax amount
-        $financialItemsArray[] = $updateFinancialItemInfoValues;
+        $financialItemsArray[$updateFinancialItemInfoValues['entity_id']] = $updateFinancialItemInfoValues;
       }
       // if submitted and difference is 0 add a positive entry again
       elseif (in_array($updateFinancialItemInfoValues['price_field_value_id'], $submittedPriceFieldValueIDs) && $updateFinancialItemInfoValues['differenceAmt'] == 0) {
@@ -865,7 +868,7 @@ WHERE li.contribution_id = %1";
             $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;
       }
     }
 
@@ -873,6 +876,20 @@ WHERE li.contribution_id = %1";
   }
 
   /**
+   * 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
    *
    * @param array $inputParams
@@ -1158,4 +1175,14 @@ WHERE li.contribution_id = %1";
     return $adjustedTrxn;
   }
 
+  /**
+   * Helper to get tax term.
+   *
+   * @return mixed
+   */
+  protected function getTaxTerm() {
+    $invoiceSettings = Civi::settings()->get('contribution_invoice_settings');
+    return CRM_Utils_Array::value('tax_term', $invoiceSettings);
+  }
+
 }

```

eileenmcnaughton · 2017-10-10T04:23:55Z

CRM/Price/BAO/LineItem.php

@@ -720,7 +721,7 @@ public static function changeFeeSelections(

    $contributionCompletedStatusID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_DAO_Contribution', 'contribution_status_id', 'Completed');
    if (!empty($financialItemsArray)) {
-      foreach ($financialItemsArray as $updateFinancialItemInfoValues) {
+      foreach ($financialItemsArray as $key => $updateFinancialItemInfoValues) {


This change is not required (declares a var we don't use)

eileenmcnaughton · 2017-10-10T04:25:44Z

CRM/Price/BAO/LineItem.php

@@ -819,60 +825,107 @@ protected function _getFinancialItemsToRecord($entityID, $entityTable, $contribu

    // 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


I't actually grouping on multiple fields not just id - is this relevant to this fix or just an additional tidy-up (if the latter I vote to remove from this PR)

eileenmcnaughton · 2017-10-10T04:27:19Z

CRM/Price/BAO/LineItem.php

+   * @return array $financialItem
+   */
+  protected function _getSalesTaxFinancialItem($lineItemInfo) {
+    static $taxTerm;


We should really use \Civi:statics for static vars because, you know, unit tests.

( I feel like just putting this in a function would be tidy too

\civi::statics blah blah $taxTerm; $financialItem = array(); if (!$taxTerm) { $taxTerm = CRM_Utils_Array::value('tax_term', Civi::settings()->get('contribution_invoice_settings')); }

further thought on this

there is enough caching in Civi::settings()->get IMHO

I think a function getInvoiceSetting() would be handy - these lines are repeated a few times - so it would look lik

function getInvoiceSetting($name) { return CRM_Utils_Array::value($name, Civi::settings()->get('contribution_invoice_settings')); }

Doesn't really save any rows but might be more readable that the multiple places with 2 row retrievals at the moment

eileenmcnaughton · 2017-10-10T04:32:30Z

CRM/Price/BAO/LineItem.php

@@ -819,60 +825,107 @@ protected function _getFinancialItemsToRecord($entityID, $entityTable, $contribu

    // 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


AH I see further down - this IS part of the change - not relying on this value anymore

eileenmcnaughton · 2017-10-10T04:35:25Z

CRM/Price/BAO/LineItem.php

+   *
+   * @return array $financialItem
+   */
+  protected function _getSalesTaxFinancialItem($lineItemInfo) {


Actually I think putting underscores in front of class functions is a dated practice
https://stackoverflow.com/questions/663350/whats-the-deal-with-a-leading-underscore-in-php-class-methods

In this case, I used it to denote protected and helper function which is native to the class

yep - that was the php 4 pattern I think?

per php 4 style :-)

eileenmcnaughton · 2017-10-10T04:43:33Z

CRM/Price/BAO/LineItem.php

        }
+        // ensure that we are not creating duplicate entries
+        $params = array(


This array is not used

eileenmcnaughton · 2017-10-10T05:10:29Z

CRM/Price/BAO/LineItem.php

   *
   * @return array
   *      List of formatted Financial Items to be recorded
   */
-  protected function _getFinancialItemsToRecord($entityID, $entityTable, $contributionID, $submittedPriceFieldValueIDs) {
+  protected function _getFinancialItemsToRecord($entityID, $entityTable, $contributionID, $submittedPriceFieldValueIDs, $lineItemsToUpdate) {


Do you think it makes sense to rename this function
getRequiredFinancialItemReversalEntries()

it seems to be only returning the reversals, not the new items

Reversal and updated financial item, the first IF condition block handle a new reversal and second updates a existing financial item

eileenmcnaughton · 2017-10-10T05:38:38Z

CRM/Price/BAO/LineItem.php

@@ -819,60 +825,107 @@ protected function _getFinancialItemsToRecord($entityID, $entityTable, $contribu

    // 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


So to articulate -'differenceAmt' is a really unreliable way of determining what the status is and you are ditching it in favour of calculating the total recorded amount against each line item in the checkFinancialItemTotalAmountByLineItemID function. If the row is a match it then gets listed to be reverted.

I'm not sure yet how you avoid reverting multiple times as you clock up changes. Also, this seems to only revert positive lines - what if a negative contribution is altered - is that possible? We should try to find a way to test that

eileenmcnaughton · 2017-10-10T05:40:36Z

CRM/Price/BAO/LineItem.php

-   * Function to retrieve formatted financial items that need to be recorded as result of changed fee
+   * When a fee is changed or updated, then in order to adjust financial items we either:
+   *   a) Create new financial item to record the surplus or deducted amount
+   *   b) Update existing financial item if related price option is updated (as qty of text field is changed)


it seems to me that rather than updating we are creating a reversal & a new row?

This is a special case when the contribution is at pay-later i.e. no payment has been made but then user decided to do fee change by changing the qty of the selected text field. In this case, we are updating the existing financial item, see https://github.com/civicrm/civicrm-core/pull/10962/files#diff-e57ed384488b5985e5a0a5c166a6b0f0R874

eileenmcnaughton · 2017-10-10T07:18:03Z

CRM/Price/BAO/LineItem.php

@@ -819,60 +825,107 @@ protected function _getFinancialItemsToRecord($entityID, $entityTable, $contribu

    // 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


AH ok - you avoid reverting multiple times by keying on line item with this $financialItemsArray[$updateFinancialItemInfoValues['entity_id']]

It's worth noting that on ?ubuntu? you can't rely on natural sort to put the highest id last (which I think you are relying on - but perhaps the group by will cause it to sort like that - I wonder how we can check - I know Jaap was using the system without the natural sort from some test bugs that he hit)

eileenmcnaughton · 2017-10-10T07:28:40Z

CRM/Price/BAO/LineItem.php

-            $updateFinancialItemInfoValues['tax']['financial_account_id'] = CRM_Contribute_BAO_Contribution::getFinancialAccountId($lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['financial_type_id']);
-          }
+      // if fee is changed to higher amount then update the existing financial item, this condition will hit ONLY when no payment has been made yet.
+      elseif (!empty($lineItemsToUpdate) &&


this portion relates to tax - not assessed by me as yet

Now only tax, this actually fixes the bug mentioned in "CRM-19273 : Change FeeSelection on pay-later" where earlier financial items are not updated to reflect the change. Here we are updating the existing financial item since no payment has been made or financial type is not changed.

Just clarifying: no financial item should ever be changed, whether a payment has been made or not. If a price on a pay later is changed, that still requires the original financial item to remain there, and the change to be posted separately, in order to make the system auditable and compliant with accounting requirements.

If we can get the problem resolved for everything except Sales Taxes, and there are still issues with Sales Taxes, I suggest we create a separate issue for that, merge this PR, and close the current issue.

eileenmcnaughton · 2017-10-10T07:31:53Z

tests/phpunit/CRM/Event/BAO/CRM19273Test.php

@@ -81,7 +81,6 @@ protected function cleanup() {
   * @return int
   */
  protected function eventPriceSetCreate($amount = 0, $min_fee = 0) {
-


All these changes in the test were to get it working - turns out it didn't originally run so we didn't know it didn't work :-(

eileenmcnaughton · 2017-10-10T21:42:01Z

@kainuk I think I saw you are at a sprint - this is your unit test / issue brought back I think?

monishdeb · 2017-10-11T07:59:00Z

@eileenmcnaughton I have made changes as per your review 58a4c67 . Can you please take a look?

monishdeb · 2017-10-20T05:53:03Z

@eileenmcnaughton I have fixed the remaining issue for text price field and made some other changes. Can you please review?

agh1 · 2017-11-08T18:21:41Z

I tested this out with @alifrumin regarding the issues on CRM-21390 and it works properly in that context.

eileenmcnaughton · 2017-11-10T01:19:34Z

Turns out the CRM-19273 test was still not running - and I think it might be highlighting an issue when you change back to an option you previously changed away from

eileenmcnaughton · 2017-11-10T04:46:46Z

I spent (another) day trying to work through this. I thought I had replicated the remaining scenario in a unit test #11266 - but then found the test still passed with without the fix - so I put it up separately. I then realised the CRM-19273Test file was still not running - to I fixed that & it started failing. I identified that was happening when the option aligned with a previous option. I have pushed up a fix for that. If you agree with that fix I think we should ask @colemanw to merge this (we should squash it at that point because the commits are not each distinct).

The code still suffers from serious readability issues which makes this very hard to review & parse. However, there are code improvements and test coverage improvements and I have verified one scenario that was broken is fixed via UI testing & @alifrumin has verified another. Both the issues fixed are critical bugs. I've reached the point where I feel merging this is less risky than not merging it - although I still hope that people like @guanhuan / compucorp/ @KarinG / @agileware will test the rc - primarily around the financial entries when changing price set selections.

Last comment is that I wasn't aware of the practice of zero-ing out but keeping line items in the code. I'm not sure if it is newish (doesn't seem to be new to this PR) or just something I was unaware of.

eileenmcnaughton · 2017-11-10T04:49:02Z

argh - looks like only one of the test failures exposed by fixing the test is fixed - I'm sure I got through it locally

not ok 961 - Error: CRM_Event_BAO_CRM19273Test::testCRM19273

That relates (I think) to the scenario where we start with an option value & change to another before changing back to the original - on the change back it doesn't get it quite right. I didn't do anything to fix that but the change I made for the other test 'seemed' to fix it

monishdeb · 2017-11-10T05:07:34Z

@eileenmcnaughton so 1 test failure remaining.. so should I fix it?

…y cancelled. This is the first of the broken-out changes in civicrm#10962 that results in an actual functionality change. The code that retrieves the relevant financial items is separated into it's own function. But then a php loop iterates through and eliminates any items that have already been reversed. This is a scenario identified in the unit tests on 10962 and occurs when the the option value has is changed and then changed again - the items from the first change should not be re-changed.

monishdeb · 2017-11-24T10:56:16Z

CRM/Price/BAO/LineItem.php

-      $submittedPriceFieldValueIDs
-    );
+    $financialItemsArray = array();
+    if (!empty($requiredChanges['line_items_to_cancel']) || !empty($requiredChanges['line_items_to_update'])) {


@eileenmcnaughton I am getting a issue re text field qty change for adding the IF condition here. Here's the steps to replicate:

Do a pending Event registration with text price option 1x10$

Then change fee of text qty, say 2 and then save
Then on financial item table, we should expect 3 records:

First is the old financial item ($10, line_item, 111) as financial_item(amount, entity_table, entity_id)

Second is the reversal item (-$10, line_item, 111)

Third is the new item ($20, line_item, 111)

On top of this patch, I am not getting the reverse entry that is the 2nd record. This is the fix https://gist.github.com/monishdeb/c55891e6795f5f4da73f2898ab780baa

@monishdeb I'm not super keen about changing code to cover more cases without adding more tests. However, your change affects code within this PR and doesn't cause the existing unit tests to fail per se - so I've changed it.

Oddly the existing unit tests DID start to fail because this line of code started to be hit

CRM_Contribute_BAO_Contribution::getFinancialAccountId()

And that function seems not to exist at all! Either before or after this. I have simply removed the code that calls that function for now - since it's clearly not working. But I think it should be re-introduced with appropriate unit tests

eileenmcnaughton · 2017-11-28T01:55:18Z

@monishdeb looks like I've regressed a bit this time around #11331 should still be safe to merge - it moves the 'return' in the test down a few lines - so it gets further before it fails. Specifically if fixes the fatal.

I think this one passed in isolation locally so will retry on this.

eileenmcnaughton · 2017-11-29T05:08:06Z

@monishdeb I have added the IF you mentioned - it bothers me it can't be replicated in a test. I think we should merge this & start working on https://issues.civicrm.org/jira/browse/CRM-20676 straight away. I think tax is still pretty broken but that is covered in that issue and this fixes & locks in fixes for 2 critical issues.

…set values

monishdeb · 2017-11-29T07:21:33Z

I have tested the PR for all html kind of price-fields, on paylater-change-fee-selection, except for text price field where financial item is not updated, which is already broken and the patch is working for all other scenario I am going to merge this on test build pass. Will create a followup PR to fix that remaining issue with unit test.

eileenmcnaughton · 2017-11-29T07:24:01Z

Ok great - per discussions there are multiple bugs in this area and notably tax items were & remain broken as does the text price field scenario. However, those can be follow on fixes as they were pre-broken

The handling of line items & financial items has been munged together in a way that gains little, adds confusion, and blocks the financial items portion from being re-usable. This is a code cleanup preparatory to the actual changes in pr civicrm#10962

…y cancelled. This is the first of the broken-out changes in civicrm#10962 that results in an actual functionality change. The code that retrieves the relevant financial items is separated into it's own function. But then a php loop iterates through and eliminates any items that have already been reversed. This is a scenario identified in the unit tests on 10962 and occurs when the the option value has is changed and then changed again - the items from the first change should not be re-changed.

CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export

civicrm-builder added the master label Sep 10, 2017

eileenmcnaughton force-pushed the fitems branch 2 times, most recently from a3baf8e to c27e316 Compare September 11, 2017 00:06

monishdeb force-pushed the fitems branch from c27e316 to ea8a5bc Compare September 27, 2017 10:16

monishdeb force-pushed the fitems branch from ea8a5bc to 2e58fa2 Compare October 6, 2017 10:57

monishdeb reviewed Oct 6, 2017

View reviewed changes

monishdeb changed the title ~~{wip} CRM-19273 Fitems~~ CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export Oct 6, 2017

monishdeb mentioned this pull request Oct 6, 2017

CRM-21245: Incorrect Contribution status 'Pending Refund' #11077

Merged

eileenmcnaughton commented Oct 10, 2017

View reviewed changes

monishdeb force-pushed the fitems branch from 2e58fa2 to 58a4c67 Compare October 11, 2017 07:51

monishdeb mentioned this pull request Oct 17, 2017

Add test of CRM-17151 ported from kainuk PR #10919

Closed

monishdeb force-pushed the fitems branch from 2d28d7b to a8f6522 Compare October 20, 2017 05:50

eileenmcnaughton force-pushed the fitems branch from a8f6522 to d9636e1 Compare November 10, 2017 01:17

eileenmcnaughton force-pushed the fitems branch 2 times, most recently from 3fe21ed to 746a264 Compare November 10, 2017 04:17

eileenmcnaughton force-pushed the fitems branch from 746a264 to 3901be2 Compare November 10, 2017 05:16

eileenmcnaughton force-pushed the fitems branch 2 times, most recently from ab84c80 to 42fb17a Compare November 20, 2017 07:17

eileenmcnaughton force-pushed the fitems branch 2 times, most recently from 1be3402 to c687701 Compare November 21, 2017 05:27

monishdeb reviewed Nov 24, 2017

View reviewed changes

eileenmcnaughton force-pushed the fitems branch 5 times, most recently from 93c4815 to 2361e1b Compare November 27, 2017 03:24

eileenmcnaughton mentioned this pull request Nov 27, 2017

CRM-19273 / CRM-21390 fix fatal error when re-editing submitted price sets #11331

Merged

eileenmcnaughton force-pushed the fitems branch from 2361e1b to 41caaa4 Compare November 27, 2017 04:10

eileenmcnaughton force-pushed the fitems branch from 41caaa4 to 74b2f0c Compare November 28, 2017 06:48

eileenmcnaughton added 2 commits November 29, 2017 17:46

CRM-19273 fix fatal error when re-editing submitted price sets

e0f5889

CRM-19723 minor code simplification

217da64

eileenmcnaughton force-pushed the fitems branch 2 times, most recently from 6414560 to 5f0bbb5 Compare November 29, 2017 05:02

CRM-19273 fix recording of financial items when fixes changing price …

7b9065c

…set values

eileenmcnaughton force-pushed the fitems branch from 5f0bbb5 to 7b9065c Compare November 29, 2017 07:04

monishdeb added the merge on pass label Nov 29, 2017

monishdeb merged commit 7358239 into civicrm:master Nov 29, 2017

monishdeb deleted the fitems branch November 29, 2017 08:01

CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export #10962

CRM-19273: Changes to Event Selections on Pending (Pay Later) Contribution Not Creating Correct Financial Items Causing Imbalance in Accounting Batch Export #10962

Conversation

eileenmcnaughton commented Sep 10, 2017 • edited by monishdeb Loading

Overview

Before

After

eileenmcnaughton commented Sep 13, 2017

eileenmcnaughton commented Sep 18, 2017

monishdeb Oct 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monishdeb commented Oct 6, 2017

eileenmcnaughton commented Oct 10, 2017

eileenmcnaughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monishdeb Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monishdeb Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Oct 10, 2017

monishdeb commented Oct 11, 2017

monishdeb commented Oct 20, 2017 • edited Loading

agh1 commented Nov 8, 2017

eileenmcnaughton commented Nov 10, 2017

eileenmcnaughton commented Nov 10, 2017 • edited Loading

eileenmcnaughton commented Nov 10, 2017

monishdeb commented Nov 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Nov 28, 2017

eileenmcnaughton commented Nov 29, 2017

monishdeb commented Nov 29, 2017

eileenmcnaughton commented Nov 29, 2017

eileenmcnaughton commented Sep 10, 2017 •

edited by monishdeb

Loading

monishdeb Oct 6, 2017 •

edited

Loading

monishdeb Oct 10, 2017 •

edited

Loading

monishdeb Oct 11, 2017 •

edited

Loading

monishdeb commented Oct 20, 2017 •

edited

Loading

eileenmcnaughton commented Nov 10, 2017 •

edited

Loading