Remove most currency conversions from IBFlex importer

[lmb]

IBFlex: Refactor extractDate to work on all transaction types

Use a single method to extract a LocalDateTime from a transaction element.
This improves the result of some imports by adding LocalTime when there
previously wasn't one.

IBFlex: Improve handling of currency conversions

The existing exchange rate calculations are spread all over the place and
are not entirely consistent. For example, fxRateToBase can only safely be
used if the accountCurrency is known.

Improve this by using fxRateToBase correctly and by parsing ConversionRate
elements contained in the IB XML. This allows doing cross currency
conversions even if the accountCurrency is not known.

IBFlex: Remove most currency conversions

The IBFlex importer currently has a set of historically grown heuristics 
around currency conversions:

1. DIVIDEND and TAX transactions are converted to account base currency
  in buildAccountTransaction() 2. BUY and SELL transactions are converted to
account base currency
  in buildPortfolioTransaction()

As far as I can tell, Interactive Brokers doesn't actually work the way that
these heuristics assume.

- There are no automatic currency conversions, the platform creates
 forex balances on the fly.
- Performing a trade in a foreign currency requires a balance in
 that currency.
- Dividends are distributed in the "base" currency of a security.
- fxRateToBase attributes give the exchange rate to the base currency
 of the account.

It is possible to get into a situation where the internal currency of a
security doesn't match the listed currency. For example by buying a security
denominated in USD on a German exchange and then transferring this to
Interactive Brokers. The current heuristics make it impossible to import
transactions in this case.

Fix this by removing most currency conversions outright. Users will have to
maintain additional deposit accounts in the correct currency and then
manually track forex conversions.

We still create GROSS_VALUE units with forex amounts for securities where
the internal currency doesn't match the listed currency.

Closes #4978

[pfalcon]

Dividends are distributed in the listed currency of a security.

This is not correct assumption, taken literally. A security hosted on a particular exchange (and "listed" in currency of that exchange) can still pay dividends in another currency.

[pfalcon]

There're multiple unrelated whitespace changes. Please assume that your changes introduce regressions, so someone later will need to investigate which actual changed code hunk introduce a particular regressions. And having unrelated changes complicate that process grossly.

[pfalcon]

Another example of unrelated changes, and less benign than the whitespace ones: the commit description talks about currency conversion, but actually changes way transaction timestamp is parsed behind the scene. Again, please assume that leads to regressions, and someone who reads the description wouldn't even know this patch introduced it.

(Otherwise I certainly agree that getting transaction timestamp in IBFlexStatementExtractor leaves much to be desired, but these changes flip-flop one tangled way for another - that's clear, because the correct way to get transaction timestamp in modern IBKR XML is from "dateTime" attribute alone. And getting proper, second-resolution timestamp is imperative to properly group transactions into trades.)

[lmb]

Should I reverse the order then? (Broken out into a separate commit) start with date time and fall back to trade date etc.

[lmb]

I tried to improve this by centralising it all in a single function.

[lmb]

Thanks for taking a first pass! I squashed all of my changes into a single commit because I wasn’t sure what the policy re multiple commits in a single PR was. Happy to split things out again.

Dividends are distributed in the listed currency of a security.

This is not correct assumption, taken literally. A security hosted on a particular exchange (and "listed" in currency of that exchange) can still pay dividends in another currency.

I think we are trying to say the same thing, but I lack the correct vocabulary. I mean the currency that a fund is denominated in I guess? I also don’t think this has an effect on the code itself.

[buchen]

I believe getExchangeRate can return null.

As I understand it, getExchangeRate is also checking for fxRateToBase and it now attempts to calculate the cross rate using the account currency, to probably it is not a change. But maybe a check for null could make sense

[buchen]

Hi @lmb,

thanks for the contribution.

About cbf4f78: The changes to the date parsing make sense.
About 20f5503: Improving the currency conversion looks good to me. We could add two checks for null.

As I plan to a make a new release, I might cherry pick those two commits to be included in the release.
Let me comment on 4d4b293 later - need more time.

[buchen]

About removing currency conversions:

I believe there is value in cleaning up the code. In the past, it was only possible to import to one deposit account at a time (and a deposit account has exactly one currency). That is now much more flexible - one can pick a deposit account per transaction currency that is imported. So I guess some of the conversions can be removed.

However, PP makes the assumption that a dividend transactions contains a currency conversion between the transaction currency and the instrument currency. That can be different. Once can configure an instrument with EUR as currency (traded on a European exchange) but still receive dividends in a foreign currency on the deposit account. Then PP needs the exchange rate to properly import the entry.

I believe that was the reason for the code in the #buildAccountTransaction method to check for the instrument.

[buchen]

@lmb

Ok, thinking about it some more I see that the currency conversion to the instrument currency is included.

The change imports transactions always in the currency as denoted in the IBFlex XML file. It is an incompatible change. Users trading in multiple currencies have to create deposit accounts in the additional currencies.

In general, the change looks good to me. The change make sense - it is closer to what a user would see in IBFlex.

You write:

and then manually track forex conversions.

Can the forex conversions also be part of the IBFlex XML file? Then they would also be imported.

[lmb]

Can the forex conversions also be part of the IBFlex XML file? Then they would also be imported.

It does look like it:

<FxTransaction accountId="A" acctAlias="A" model="" assetCategory="CASH" reportDate="20241219" functionalCurrency="GBP" fxCurrency="EUR" activityDescription="CASH: XYZ.22 EUR.USD" dateTime="20241219;073909" quantity="XYZ.22" proceeds="XYZ.460865" cost="-XYZ.460865" realizedPL="0" code="O" levelOfDetail="TRANSACTION"/>
<FxTransaction accountId="A" acctAlias="A" model="" assetCategory="CASH" reportDate="20241219" functionalCurrency="GBP" fxCurrency="EUR" activityDescription="CASH: -1.8 EUR.USD" dateTime="20241219;073909" quantity="-1.8" proceeds="1.497573" cost="-1.497578" realizedPL="-0.000005" code="C" levelOfDetail="TRANSACTION"/>

As usual it's hard to know what the codes mean.

As I plan to a make a new release, I might cherry pick those two commits to be included in the release.

Please let me know how you want to proceed here. Should I make changes to this branch, or wait for you to cherry-pick and then rebase?