only check batch currency match when adding a financial_trxn

[ufundo]

Overview

Fix for regression(?) on Gift Aid extension https://lab.civicrm.org/extensions/ukgiftaid/-/issues/30 resulting from #20884 .

Before

The check for currency match looks up a financial_trxn using the entity_id parameter. But this is only a financial_trxn_id in the case where entity_table is adding financial transactions to the batch.

This leads to errors when batching contributions (e.g. https://lab.civicrm.org/extensions/ukgiftaid/-/issues/30#note_75292 ) and I imagine memberships too.

After

Only checks the currency matches when adding a financial_trxn.

Comments

I'm not sure whether it makes sense to write a separate check that works for checking contribution currencies. For other entities that can be batched (memberships?) I don't think it would make any sense, but I'm not really sure how these are used.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[MegaphoneJon]

@ufundo this seems fine to me - but as the comment on the line above notes, the SyntaxConformanceTest doesn't like this, hence the failed test. You'll need to account for it, unfortunately.

[ufundo]

Ah, I see - think I have it now!

[totten]

@ufundo - @eileenmcnaughton suggested that this might be worth backporting to 5.51-rc and 5.50-stable. What do you think?

[ufundo]

I think that would be great and don't see any conflicts.

I'm not totally sure of the process sorry? Should I open a PR against the 5.50/5.51 branches?

[totten]

@ufundo Yep, PRs for 5.51 and 5.50 would be perfect.

[ufundo]

#23788 and #23787 👍

[totten]

Thanks @ufundo. It'll go out with in 5.50.2.