WIP: Allow import of expired options

[starze]

As described in #3365 (comment) we need a possibility to import 0€ transactions to represent expired options (again).

Since PP 0.632023 I am building portfolio performance manually with these small fixes in the given pull request to get the option support working again.
I am aware that this code is not a full solution that can be merged directly but it seams that I am not the only one that would love to see this feature comming into pp again.

This pull request should be a starting point for an in-depth discussion about if you also want this feature getting into pp again and what is missing to get this done.
At least it can be a starting point for others to build their own versions of pp with support for expired options.

Here is a list of things I know we need to consider:

• allow import of IBFlexStatements with expired options (0€ transactions)
• fix unit test testIBFlexStatementFile02
• check and maybe fix CSV-Importer
• check and maybe fix PDF-Importers
• check and maybe fix the manual import
• check and maybe fix the key figure calculations and the evaluations
• Cleanup of commented out code

I hope this will help to get options support into pp again one day.

[buchen]

I haven't looked at the code in detail, but PP does not allow Sale transactions with an amount of 0. If at all, you have to do a delivery outbound with 0.

Options are not directly supported but I know that some are using PP to track them.

[starze]

In the case of an option expiration, it is actually a BUY transaction (not a SELL) with an amount of 0.
I have updated the code to represent this transaction as a TRANSFER_IN instead of a BUY. I hope this is the change you were looking for?
Importing testIBFlexStatementFile02.xml now reflects the expired option exactly as I would expect.

All existing tests are passing, but I would appreciate it if you could add some tests to ensure the missing parts you are looking for (e.g. correct calculation of key figures).

[starze]

I would appreciate it if you could add some tests to ensure the missing parts you are looking for

Hi @Nirus2000, @buchen,

I just wanted to kindly follow up and ask if you’ve had a chance to review the code and the open question in my pull request.

As far as I can tell, the performance calculations are correct. I've attached a few screenshots to illustrate the results. For clarity, I modified the test file testIBFlexStatementFile02.xml so that it now only includes a one-time deposit and the relevant option writer transaction.

The dashboard’s year-over-year performance is calculated correctly, and fees are properly included.
The per-trade performance values are NaN, which is expected in this case and should not prevent us from moving forward with adding support for options.

If there are any issues or concerns with the implementation, I’d really appreciate it if you could let me know what I should change to get the PR accepted. I’m happy to adjust it accordingly.

Thanks a lot for your time and feedback!

Best regards

[pfalcon]

Coming from #4825 (comment) :

@starze : I'd suggest there're 2 separate issues:

1. Support zero-amount BUY/SELL transactions in general.
2. Specifically unbreak IBKR XML importer in regard to zero-amount transactions.

It seems that you wanted to solve p.1 with this pull request. And I agree p.1 is absolutely a must, but based on the responses above, it's clear it's going to be a hard case. So, I wouldn't recommend to shoot for that right away.

Then, it should be just p.2. But then patches here do "too much", and questionable things.

The only change required to make zero amounts to be properly imported by IBKR XML imported is:

a/name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/ibflex/IBFlexStatementExtractor.java
+++ b/name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/ibflex/IBFlexStatementExtractor.java
@@ -543,11 +543,7 @@ public class IBFlexStatementExtractor implements Extractor
 
             BuySellEntryItem item = new BuySellEntryItem(portfolioTransaction);
 
-            if (portfolioTransaction.getPortfolioTransaction().getCurrencyCode() != null && portfolioTransaction.getPortfolioTransaction().getAmount() == 0)
-            {
-                item.setFailureMessage(Messages.MsgErrorTransactionTypeNotSupported);
-            }
-            else if (Messages.MsgErrorOrderCancellationUnsupported.equals(portfolioTransaction.getPortfolioTransaction().getNote()))
+            if (Messages.MsgErrorOrderCancellationUnsupported.equals(portfolioTransaction.getPortfolioTransaction().getNote()))
             {
                 item.setFailureMessage(Messages.MsgErrorOrderCancellationUnsupported);
             }

I have that in my tree for quite some time. I didn't try to submit it, because, again, I learned that it would be a hard case.

(To clarify, I don't say that no further changes can be done to IBFlexStatementExtractor.java. What I'm saying that for a specific simple issue, there's a simple and obvious patch (which addresses that specific issue in the most obvious way, and thus easy to review, test, etc.) )

Where to go from here? Well, for example, you could test the above patch and confirm that it indeed addresses simple specific issue in a simple and obvious way. Then there might have been 2 votes for that patch (votes which don't matter much, but still better than 1).

[pfalcon]

@starze :

In the case of an option expiration, it is actually a BUY transaction (not a SELL) with an amount of 0.

That's not the case. It's pretty obvious that short options are bought back to close the position (for 0 price in particular), while long options are sold (for 0 price in particular).

That again should emphasize importance of the simple and generic solutions, and doing everything in right order (starting with simple, and only later targeting more complex; starting with generic changes enabling large array of usecases and later targeting specific cases).