CSVAccountTransactionExtractor: Do not blindly ignore amount sign

[pfalcon]

CSV importer's existing mental model is that each transaction would be explicitly categorized (via a "type" column) as Buy or Sell. It then just ignores the sign of transaction amount (by calling Math.abs()).

This assumption is not realistic to cover all the cases. Another way is to have a generic "security transaction" type, and the sign of the amount signifies the actual operation, e.g. negative for Buy, positive for Sell.

This change allows to handle such CSV statements, by "inverting" the configured transaction type. For example, if given type column value is mapped to "BUY", but amount is negative, it is turned into "SELL".

[pfalcon]

This addresses p.2 from #3616

[buchen]

Hi @pfalcon,

thanks for the pull request. I like the change. It will give some more flexibility.

The build failure is due to code formatting of the new change (curly braces). Can you be so kind and format it with the default formatter in Eclipse (Ctrl-F)?

I think we should add a test case to CSVAccountTransactionExtractorTest that tests the new behavior. Otherwise it is too easy to accidentally break this new feature with a future commit. Can you be so kind and add one?

Finally, I believe we should have the same logic when importing "only" portfolio transactions: CSVPortfolioTransactionExtractor.java#L243-L249

Thanks for taking the time for a contribution to Portfolio Performance.

Andreas.

[Nirus2000]

I have my doubts here that this is a good idea....

Several problems will arise here.

1. If the sale value of the shares is less than the fees. (Sale with negative amount)
2. Hereby cancellations are made possible of purchase and sale and not removed. The transactions are posted or duplicated at the wrong time. (Purchase or sale cancellation from day X).
3. The same applies to reverse transactions of deliveries and removals.
4. Also, here is created the possibility to book via the CSV options... (Call / Put). The "Put" would then be negative, whereby these are then not recorded as fees.
5. It would not matter which transaction is identified. As soon as any negative zusatzpunkte are added incorrectly booked. (e.g. dividends, where the tax burden is greater than the dividend itself).

[buchen]

@Nirus2000 writes:

Several problems will arise here.

Let's recap what this change enables.

Let's say we have a CSV of this format:

date;type;amount
2023-10-31;Ein-/Auszahlung;2000
2023-11-01;Ein-/Auszahlung;-1000

In this case, the type cannot be used to determine the transaction - the sign of the value is needed as well.
The sign of the amount can be used to make it a DEPOSIT or REMOVAL (= withdrawal).
In this case, one can import the CSV as a 2000 EUR deposit and 1000 EUR removal.
The imported transactions does not carry the sign anymore, of course, as all internal values must be positive in the model of Portfolio Performance.

If I understand your concerns, they are about negative values in the internal transactions model of Portfolio Performance. At the moment, I do not see this change allowing the negative values to leak in.

[pfalcon]

date;type;amount
2023-10-31;Ein-/Auszahlung;2000
2023-11-01;Ein-/Auszahlung;-1000

Yes, this exemplifies a case we see here. I understand that it may change behavior for some users of CSV import, so it would depend on benefits it brings vs possible corner cases, and IMHO benefits outweigh. All in all, I posted it for feedback. I'll try to address review comments as time permits. (I otherwise have a whole bunch of changes in my fork, some adhoc, all without tests, but some certainly may be useful to other people, and to make PP a more generic tool (== cover more usecases), which should be a good aim for an open-source project.)