portfolio-performance / portfolio

Track and evaluate the performance of your investment portfolio across stocks, cryptocurrencies, and other assets.
http://www.portfolio-performance.info
Eclipse Public License 1.0
2.92k stars 600 forks source link

CSV Import of Buy/Sell Securities fails - "Exchange rate of gross value is missing" #3000

Closed matt-winfield closed 2 years ago

matt-winfield commented 2 years ago

Bug Description When trying to import security transactions from CSV where type is buy or sell, and the transaction currency is different from the security currency (in my case, GBP and GBX), then those rows fail to be imported. The error indicates "Exchange rate of gross value is missing (transaction currency GBP and security currency GBX)"

To Reproduce Steps to reproduce the behavior:

  1. File -> Import -> CSV Files
  2. Select CSV file containing transaction with type "Buy"
  3. Data type is "Account Transactions"
  4. Map the "Date", "Type", "Shares", "Value", "Exchange Rate", "Gross Amount" and "Currency Gross Amount" fields to the correct columns
  5. Click next
  6. The row is crossed out, right click to see the error

Adding the transaction manually through the UI works fine for this account

Example CSV that doesn't work:

Date,Type,Shares,Symbol,Description,Value,Contract Ref,ISIN,Sedol,Exchange Rate,Gross Value,Currency
17-Dec-2018,Buy,12.34,XYZ,EXAMPLE STOCK GBP,500.0,ZYX,GB00123,ABC,100,50000.0,GBX

Expected behavior The transactions should be created successfully. It seems there was an update made that allowed this to work for dividend payments: https://forum.portfolio-performance.info/t/dividend-csv-update/15977 But it doesn't seem to work when the type is buy/sell.

This issue is discussed on the forum here: https://forum.portfolio-performance.info/t/exchange-rate-of-gross-value-is-missing-when-importing-csv-transactions-with-currency-conversions/20633/5?u=mattw

Screenshots Screenshot of the error (Ignore “transaction does already exist”, that shouldn’t affect this): image

And the field mapping - I’ve tried mapping currency, gross amount, currency gross amount: image

Possible solution I had a look through the code and identified what may be causing the issue. I don't have local development set up to test the changes, but hopefully this points someone who's worked with this project more in the right direction:

It seems the bug might be caused by missing transaction.addUnit for buy/sell types. Here is the code that imports Dividends and most other transaction types: image

And here is the equivilant part for buy/sell: image

You can see there is not something like the t.addUnit(grossAmount.get() for buy/sell, I would imagine if somehting like buySellEntry.getPortfolioTransaction().addUnit(grossAmount.get()) was added here it might fix the issue.

Since that unit is not set, when transaction currency does not match security currency, it will try to get that unit and fail: image

Desktop:

Nirus2000 commented 2 years ago

Hello @matt-winfield The same problem exists with IBFlexquery and PDF importers. It is not an exchange rate error in that sense, but a currency error. (GBX <-> GBP | ZAC <-> ZAR | ILA <-> ILS | USD <->AED ... and so on). So one factor...

That itself results if you put a security in GBP for example and then the quotes are pulled in GPX.

I have not yet found a solution to the problem. Maybe @buchen has an idea how we could check and process fixed currency pairs with each other.

Look at -> here <-

Greetings Alex

matt-winfield commented 2 years ago

Hello @matt-winfield The same problem exists with IBFlexquery and PDF importers. It is not an exchange rate error in that sense, but a currency error. (GBX <-> GBP | ZAC <-> ZAR | ILA <-> ILS | USD <->AED ... and so on). So one factor...

That itself results if you put a security in GBP for example and then the quotes are pulled in GPX.

I have not yet found a solution to the problem. Maybe @buchen has an idea how we could check and process fixed currency pairs with each other.

Look at -> here <-

Greetings Alex

I'm not sure I understand - If my security is in GBX, and the transaction quote is in GBP (mapped to the "Currency Gross Amount" field), I have the "Exchange Rate" field that tells PP how to convert from GBX to GBP, doesn't it have all of the information it needs?

Nirus2000 commented 2 years ago

Hello @matt-winfield I'll try to explain it as far as I can interpret the source code.

We look at the gross currency in the CSV Importer. The security is created in this currency. In your case the currency is GPX. Your exchange rate, which I'm sure you added manually, is not 100 but 0.01. 500.00 GBP / 0.01 GPX = 50000.00 GPX

I have successfully run this test here.

    @Test
    public void testBuyTransactionWithForexWithSecurityInGBX()
    {
        Client client = new Client();
        Security security = new Security();
        security.setTickerSymbol("SAP.DE");
        security.setCurrencyCode("GBX");
        client.addSecurity(security);

        CSVExtractor extractor = new CSVPortfolioTransactionExtractor(client);

        List<Exception> errors = new ArrayList<>();
        List<Item> results = extractor.extract(0,
                        Arrays.<String[]>asList(new String[] { "17-12-2018", "", "DE0007164600", "SAP", "", "SAP SE",
                                        "500,00", "GBP", "0,00", "0,00", "50000,00", "GBX", "0,01", "12,34", "BUY", "Notiz" }),
                        buildField2Column(extractor), errors);

        assertThat(errors, empty());
        assertThat(results.size(), is(1));
        new AssertImportActions().check(results, "GBP");

        BuySellEntry entry = (BuySellEntry) results.stream().filter(BuySellEntryItem.class::isInstance).findAny()
                        .orElseThrow(IllegalArgumentException::new).getSubject();

        PortfolioTransaction t = entry.getPortfolioTransaction();
        assertThat(t.getType(), is(PortfolioTransaction.Type.BUY));
        assertThat(t.getMonetaryAmount(), is(Money.of("GBP", 500_00)));
        assertThat(t.getSecurity(), is(security));
        assertThat(t.getUnitSum(Unit.Type.TAX), is(Money.of("GBP", 0_00)));
        assertThat(t.getUnitSum(Unit.Type.FEE), is(Money.of("GBP", 0_00)));

        Unit grossAmount = t.getUnit(Unit.Type.GROSS_VALUE).orElseThrow(IllegalArgumentException::new);
        assertThat(grossAmount.getAmount(), is(Money.of("GBP", 500_00)));
        assertThat(grossAmount.getForex(), is(Money.of("GBX", 50000_00)));
        assertThat(grossAmount.getExchangeRate(), is(BigDecimal.valueOf(0.01)));
    }

Test successful. grafik

grafik grafik grafik Test.csv csv-config-for-test.txt <-- rename to *.json

I hope this helps you.

Greetings Alex

matt-winfield commented 2 years ago

Ah thanks @Nirus2000, I've got it working now. I see the issues I was having with my input data, the main one being my exchange rate being the wrong way around. Thank you for your help!

dakkusingh commented 1 year ago

Same issue and the solution doesnt seem to work... Here is my test file.. Test.csv

Screenshot 2023-01-09 at 15 08 58 Screenshot 2023-01-09 at 15 08 39 Screenshot 2023-01-09 at 15 08 46
dakkusingh commented 1 year ago

it seems you have to choose: Type of Data as "PortfolioTransactions" in order for the import to work correctly!

Nirus2000 commented 1 year ago

Thx, thats the solution... @dakkusingh

DrBroker007 commented 4 months ago

it seems you have to choose: Type of Data as "PortfolioTransactions" in order for the import to work correctly!

Perfect! This worked!! Thanks @dakkusingh !!