Project60 / org.project60.sepa

SEPA direct debit integration with civicrm
19 stars 46 forks source link

civicrm_financial_trxn.from_financial_account_id is not properly filled #628

Open Detsieber opened 2 years ago

Detsieber commented 2 years ago

CiviSEPA creates a transaction record, when the contribution is marked as "completed".

However, this transaction record currently refers to the wrong financial account id: Instead of the "revenue" account, the "accounts receivable" account linked to the financial type is used.

As a workaround, I always created additional "accounts receivable" accounts for each financial type. However this is not the right way it should be done (and apart of that, this workaround might break the accounts receivable logic within CiviCRM).

Instead, the completed contributions better should point to the correct revenue account.

I would assume this should be solved together with #606, which is another issue of CiviSEPA regarding the table "civicrm_financial_trxn".

Funding might be available.

bjendres commented 2 years ago

Hm, as far as I remember, CiviSEPA only uses the Contribution layer (via API) above the financial transactions. Are you sure this problem isn't coming from a payment processor, another module, or a configuration issue?

Detsieber commented 2 years ago

As you are asking like that: No, I am not sure!

And, yes, this might be an issue regarding the way, API, BAO and/or DAO are handling the status change from "in progress" to "completed": As we know, the contribution_status_id "in progress" is not widely used within CiviCRM - afair only within CiviSEPA. Which might be the reason, that core's misbehaviour in this case wasn't yet recognized elsewhere...

Anyhow, I did not get deep enough into details to find the exact part of the core code, where this is happening. And therefore, whether this should be fixed by a change within CiviSEPA or through a patch of the core (the latter to be preferred!).

Still, it is very probable that these two issues #606 and #628 both are linked to the same origin.

bjendres commented 2 years ago

Hm, I'm not sure how to proceed here. One way could be, that you specify a test case (for the unit test suite) to reliably reproduce the issue.

jensschuppe commented 1 year ago

@Detsieber Is this still an issue or have found a workaround/solution that you would be able to share?

Detsieber commented 1 year ago

This is still an issue. At one client, I am using this sql statement to fill the missing field: update civicrm_contribution as c, civicrm_entity_financial_trxn as e, civicrm_financial_trxn as f set f.payment_instrument_id = c.payment_instrument_id where c.id = e.entity_id and e.entity_table = "civicrm_contribution" and e.financial_trxn_id = f.id and c.payment_instrument_id in (8,9,10) and f.payment_instrument_id is null;

bjendres commented 1 year ago

Can anyone else confirm this behaviour? Just to rule out, that this is something specific to your setup.

Detsieber commented 1 year ago

As nobody yet answered to this: This behaviour happens always, and I have encountered it with several different CiviCRM installs.

However, it only matters when the functionality of "CiviAccount" is used (e.g. with the report "Bookkeeping Transactions" being in use). Maybe quite often this might not be the case?

I am still very interested in getting a solution on that issue, and I am quite sure we could make available the necessary funding on that.