Improve degiro v3 parsing #148

ikruglov · 2025-01-04T22:13:35Z

Added

This commit re-works degiroConverterV3 logic.

The main change is that the code can now consider a set of records as one instead action+tx record.

The following set of records is one sale with two transaction fee records. They can be reported as one in ghostfolio. The previous code didn't handle this.

23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.79,EUR,,1d2e7a85-cf67-4325-82c7-d5994a0afb98
23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-4.00,EUR,,1d2e7a85-cf67-4325-82c7-d5994a0afb98
23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,Verkoop 120 @ 1.202 GBX,,GBP,1442.40,GBP,,1d2e7a85-cf67-4325-82c7-d5994a0afb98

Another example is below. It has two sells and one tx record.

11-07-2024,16:46,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 16 @ 124,28 EUR",,EUR,1988.48,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c
11-07-2024,16:43,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-1.00,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c
11-07-2024,16:43,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 8 @ 124,28 EUR",,EUR,994.24,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c

Or this

24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.26,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818
24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-2.00,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818
24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.02,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818
24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 14 @ 61,7 EUR",,EUR,863.80,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818
24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 1 @ 61,7 EUR",,EUR,61.70,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818

To make it even more complicated. There are cases of taxes without any associated buy/sell:

16-12-2021,18:02,22-10-2021,MOODY'S CORP.,US6153691059,Dividendbelasting,,USD,-0.01,USD,0.00,
16-12-2021,18:02,22-10-2021,MORGAN STANLEY,US6174464486,Dividendbelasting,,USD,0.01,USD,0.01,
16-12-2021,18:02,22-10-2021,MOODY'S CORP.,US6153691059,Dividendbelasting,,USD,-0.01,USD,0.00,
16-12-2021,18:02,22-10-2021,MORGAN STANLEY,US6174464486,Dividendbelasting,,USD,0.01,USD,0.01,

There are also a bunch of cosmetic changes.

This is quite a significant change, so I'm looking into your feedback. I've collected a corpus of tests while debugging. I'm ready to write tests for them, but I would like your feedback to ensure you're comfortable accepting this PR.

Fixes

Checklist

Added relevant changes to README (if applicable)
Added relevant test(s)
Updated GitVersion file and corresponding version in package.json

Related issue (if applicable)

Possibly resolves #140

ikruglov · 2025-01-04T22:18:53Z

@dickwolff I just spotted #146. It's still a half-measure, I believe. There are complex cases when one sale or purchase can include two or more buy/sell records and multiple transaction fee records. Please take a look at the description for examples.

dickwolff

Thanks for the suggestions on how to improve the mapping for DEGIRO. I have done an initial review of the changes. Overall I think you’re on the right path, but I have some remarks, questions or suggestions.

Please note that I have reviewed it on my iPad, so I might not have the full picture of all the changes. I will look into it some more later when I can get on a pc.

src/converters/degiroConverterV3.ts

src/models/degiroRecord.ts

src/converters/degiroConverterV3.ts

dickwolff · 2025-01-05T09:41:00Z

src/converters/degiroConverterV3.ts

+         * since buy/sell txs are in EUR.
+         *
+         * This hack allows to buypass all internal heuristic and hardcode correct results.
+         * Format: IE00B02KXK85=FXAC.AS,foo=bar


I have seen a similar thing with the isin of VWRL resulting in VWRL.L instead of VWRL.AS (see comment on line 92 on main).

I think you’re on the right path to a solution, but I think it can be improved upon. I’d then rather use a simple text file with mappings which you as a user can provide.

I have created #149 for this to make this separate of this change. Could you revert any change regarding this mapping override, so the this PR is moreso a better fix for the transaction matching?

Do you have ready code? If not, I can write it since I need it. Do you have any particular preferences implementation wise?

No I have no code yet, but your suggestion gave me an idea on how to do it.

You can give it a go, but preferably separate from, and after this change has been merged

No I have no code yet, but your suggestion gave me an idea on how to do it.

You can give it a go, but preferably separate from, and after this change has been merged

so, just a plain file with foo=bar pairs on each line?

That was basically what I was going for. Then read it when present, and also provide a file volume mount in Docker.

But as stated, I’d prefer it separate from this PR

package.json

src/converters/degiroConverterV3.ts

src/models/degiroRecord.ts

ikruglov · 2025-01-05T11:36:03Z

btw, do you prefer rebase + force-push or extra commits?

dickwolff · 2025-01-05T11:37:41Z

I recently tried to switch to rebasing, so please

ikruglov · 2025-01-05T13:26:19Z

@dickwolff I fixed all your suggestions and added tests. But I didn't figure out how to update Yahoo Finance mock file to include info about new ISINs. How do I do it?

ikruglov · 2025-01-05T15:20:34Z

@dickwolff I just realized that the import logic seriously messes up with currencies. It just replaces record currency with looked-up-from-yahoo currency without any proper conversation.

I understand why, but it affects results. In my case, with INRG.L, quite seriously.

Asked question: ghostfolio/ghostfolio#4139 (comment)

What was your thinking when you wrote it?

dickwolff · 2025-01-05T15:39:56Z

@dickwolff I fixed all your suggestions and added tests. But I didn't figure out how to update Yahoo Finance mock file to include info about new ISINs. How do I do it?

See the wiki for instructions how to do this.

@dickwolff I just realized that the import logic seriously messes up with currencies. It just replaces record currency with looked-up-from-yahoo currency without any proper conversation.

I understand why, but it affects results. In my case, with INRG.L, quite seriously.

Asked question: ghostfolio/ghostfolio#4139 (comment)

What was your thinking when you wrote it?

This was a fix for GBP to GBp matching. I have since made some changes in a branch to revert that change, in response to #129. I have not merged that yet due to something I wanted to verify with the reporter first. But I think I will do that merge tomorrow, so you can continue working on this.

jonathanR79 · 2025-01-05T16:35:43Z

Some other samples of multi line transaction record that are currentlly not really well processed:

10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,Operation de change - Crédit,1.2105,USD,311.00,USD,0.00,a08ddbcc-73da-4250-ac23-yyyyyyyyy
10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,Opération de change - Débit,,EUR,-256.92,EUR,754.35,a08ddbcc-73da-4250-ac23-yyyyyyyyy
10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,Taxe sur les Opérations Boursières belge (TOB),,EUR,-0.90,EUR,1011.27,a08ddbcc-73da-4250-ac23-yyyyyyyyy
10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,Frais DEGIRO de courtage et/ou de parties tierces,,EUR,-0.33,EUR,1012.17,a08ddbcc-73da-4250-ac23-yyyyyyyyy
10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,Frais DEGIRO de courtage et/ou de parties tierces,,EUR,-0.50,EUR,1012.50,a08ddbcc-73da-4250-ac23-yyyyyyyyy
10-12-2020,17:54,10-12-2020,ADR ON AMBEV,US02319V1035,"Achat 100 ADR on Ambev@3,11 USD (US02319V1035)",,USD,-311.00,USD,-311.00,a08ddbcc-73da-4250-ac23-yyyyyyyyy

04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,Taxe sur les Opérations Boursières belge (TOB),,EUR,-0.30,EUR,506.28,b5c849a1-baef-473e-bdd7-yyyyyyyyy
04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,Frais DEGIRO de courtage et/ou de parties tierces,,EUR,-0.10,EUR,506.58,b5c849a1-baef-473e-bdd7-yyyyyyyyy
04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,Taxe sur les Opérations Boursières belge (TOB),,EUR,-0.30,EUR,506.68,b5c849a1-baef-473e-bdd7-yyyyyyyyy
04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,Frais DEGIRO de courtage et/ou de parties tierces,,EUR,-0.10,EUR,506.98,b5c849a1-baef-473e-bdd7-yyyyyyyyy
04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,"Achat 50 Quest For Growth@4,98 EUR (BE0003730448)",,EUR,-249.00,EUR,507.08,b5c849a1-baef-473e-bdd7-yyyyyyyyy
04-06-2020,12:21,04-06-2020,QUEST FOR GROWTH,BE0003730448,"Achat 50 Quest For Growth@4,97 EUR (BE0003730448)",,EUR,-248.50,EUR,756.08,b5c849a1-baef-473e-bdd7-yyyyyyyyy

dickwolff · 2025-01-05T16:39:03Z

@jonathanR79 Thanks for providing these. Of the first set, the first records should be ignored. I think that currently is already the case. Of the second set, would you want there to be 2 buy records, or is it fine to combine all these 6 lines into one activity for Ghostfolio?

jonathanR79 · 2025-01-05T16:42:58Z

@jonathanR79 Thanks for providing these. Of the first set, the first records should be ignored. I think that currently is already the case. Of the second set, would you want there to be 2 buy records, or is it fine to combine all these 6 lines into one activity for Ghostfolio?

On the first, it's detected as a divident for now.

On the second, the purchase price is different. So , ideally it should be two different records. A single entry with the average of the 2 should work as well but I'm not sure what is the easiest.

ikruglov · 2025-01-09T23:07:28Z

@dickwolff I've pushed an improved version. Unfortunately, I'm unable to test it locally. For some reason, ghostfolio tells me, "Don't have a currency rate from EUR to USD." I'm unable to work around it for now.

ikruglov · 2025-01-09T23:09:22Z

On the second, the purchase price is different. So , ideally it should be two different records. A single entry with the average of the 2 should work as well but I'm not sure what is the easiest.

@jonathanR79 this code will do two records. One with combination of TX costs, second one without one tx cost.

dickwolff · 2025-01-10T14:17:57Z

@dickwolff I've pushed an improved version. Unfortunately, I'm unable to test it locally. For some reason, ghostfolio tells me, "Don't have a currency rate from EUR to USD." I'm unable to work around it for now.

You should be able to this via Admin Control. Click the + Add Currency-button and enter EUR. Then you should have the exchange rate like 1 USD = 0,9745 EUR (on time of writing).

I will hopefully have some time this weekend to go through your changes and leave review.

ikruglov · 2025-01-10T19:45:49Z

@dickwolff I've pushed an improved version. Unfortunately, I'm unable to test it locally. For some reason, ghostfolio tells me, "Don't have a currency rate from EUR to USD." I'm unable to work around it for now.

You should be able to this via Admin Control. Click the + Add Currency-button and enter EUR. Then you should have the exchange rate like 1 USD = 0,9745 EUR (on time of writing).

I will hopefully have some time this weekend to go through your changes and leave review.

I tried that. It doesn't help. I'm kinda stuck. I see that ghostfolio extracts and adds historical data for USDEUR, but it fails with the opposite conversation: EURUSD. I don't know/see a way to import it.

ikruglov · 2025-01-10T21:38:27Z

oh, I figured it out. It's a bug in ghostfolio. I'm preparing a PR.

ikruglov · 2025-01-10T22:26:05Z

submitted ghostfolio/ghostfolio#4187

dickwolff

I just looked at the changes and they seem to make sense overall. I might do some testing and code cleanup myself after merging, but the idea is sound. Great work!

Finally, can you bump the versions to 0.23.0?

dickwolff · 2025-01-15T06:26:39Z

src/converters/degiroConverterV3.test.ts

@@ -35,7 +35,7 @@ describe("degiroConverterV3", () => {
      // Assert
      expect(actualExport).toBeTruthy();
      expect(actualExport.activities.length).toBeGreaterThan(0);
-      expect(actualExport.activities.length).toBe(26);
+      expect(actualExport.activities.length).toBe(43);


I see the tests are currently failing. V2 converter is missing 1 (as V2 and V3 tests are sharing the same input file for testing), and V3 only got 31 instead of expecting the 43 you put in the test. Can you take a look at this and make changes?

@dickwolff I need help with tests:

to update Yahoo Finance information. I think the instructions from the wiki are not complete. I got a missing import of a Mock object, which I didn't figure out how to import. Note that I'm a first-time user of Typescript

How do I print debug statements (to stdout/stderr) in tests? I'm getting unexpected behavior, which I need to debug.

Sure! Hope this helps:

If you use an IDE like VSCode, you can to CTRL + . (dot) and then import all usings.

In the tests, console statements are supressed. You can comment the whole beforeEach(() => { function, then you'll get the console log statements in your console. For debugging purposes I usually add console.log(...) statements on places that could help me debug.

…ord()

This commit re-works degiroConverterV3 logic. The main change is that the code now can consider a set of records as one, instread action+tx record. The following set of records are one sell with two transaction fees records. They can be reported as one in ghostfolio. The previous code didn't handle this. 23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.79,EUR,,1d2e7a85-cf67-4325-82c7-d5994a0afb98 23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-4.00,EUR,,1d2e7a85-cf67-4325-82c7-d5994a0afb98 23-12-2020,10:03,23-12-2020,ISHARES GLOBAL CLEAN ENERGY UCITS ETF,IE00B1XNHC34,Verkoop 120 @ 1.202 GBX,,GBP,1442.40,GBP,,1d2e7a85-cf67-4325-82c7-d5994a0afb98 Another example is below. It has two sells and one tx record. 11-07-2024,16:46,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 16 @ 124,28 EUR",,EUR,1988.48,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c 11-07-2024,16:43,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-1.00,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c 11-07-2024,16:43,11-07-2024,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 8 @ 124,28 EUR",,EUR,994.24,EUR,,56e1f16b-4142-4373-851d-45d0505fe12c Or this 24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.26,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818 24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-2.00,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818 24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,DEGIRO Transactiekosten en/of kosten van derden,,EUR,-0.02,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818 24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 14 @ 61,7 EUR",,EUR,863.80,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818 24-03-2020,13:10,24-03-2020,VANGUARD FTSE ALL-WORLD UCITS ETF,IE00B3RBWM25,"Verkoop 1 @ 61,7 EUR",,EUR,61.70,EUR,,810d6e1d-af8e-43a4-830e-0b85bf90c818

ikruglov requested a review from dickwolff as a code owner January 4, 2025 22:13

dickwolff requested changes Jan 5, 2025

View reviewed changes

src/models/degiroRecord.ts Show resolved Hide resolved

ikruglov force-pushed the improve-degiro-v3 branch 2 times, most recently from 8f96285 to 98b28af Compare January 5, 2025 11:35

ikruglov force-pushed the improve-degiro-v3 branch from 98b28af to 10612a2 Compare January 5, 2025 13:25

dickwolff mentioned this pull request Jan 5, 2025

DEGIRO - Buy record in French (Belgium) #140

Open

dickwolff mentioned this pull request Jan 5, 2025

Combine 2 transaction fee records if they are about the same product #146

Closed

3 tasks

ikruglov force-pushed the improve-degiro-v3 branch from 10612a2 to 821fb3e Compare January 9, 2025 23:05

ikruglov requested a review from dickwolff January 9, 2025 23:09

dickwolff requested changes Jan 15, 2025

View reviewed changes

dickwolff mentioned this pull request Jan 17, 2025

Allow manual symbol overrides from the SecurityService #149

Open

ikruglov added 3 commits January 19, 2025 20:39

degiroConverterV3: introduce mapInterestRecord() && mapPlatformFeeRec…

26d394d

…ord()

degiroConverterV3: dedup records base on md5 hash

bdeccb8

degiroConverterV3: extend ignored records

1f59298

ikruglov added 11 commits January 19, 2025 20:39

degiroConverterV3: cast records to DeGiroRecord objects

7f06480

DeGiroRecord: getAmount() & getAbsoluteAmount()

1942b6e

degiroConverterV3: formatFloat()

ea9bbe6

degiroConverterV3: recover simple broken CSV records

4441ccc

degiroConverterV3: don't combine records with different currencies

1e66d27

degiroConverterV3: inverse processing order

7621b02

degiroConverterV3: generate unique transaction records

ce1eb9a

degiroConverterV3: GBP <-> GBp/GBX converstion

71e35b0

degiroConverterV3: ignore record with zero change and zero fee

8c3ca17

degiroConverterV3.test: add tests

9a92844

ikruglov force-pushed the improve-degiro-v3 branch from a01e3db to 9a92844 Compare January 19, 2025 20:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve degiro v3 parsing #148

Improve degiro v3 parsing #148

ikruglov commented Jan 4, 2025 •

edited by dickwolff

Loading

ikruglov commented Jan 4, 2025

dickwolff left a comment

dickwolff Jan 5, 2025

ikruglov Jan 5, 2025

dickwolff Jan 5, 2025

ikruglov Jan 5, 2025

dickwolff Jan 5, 2025

ikruglov commented Jan 5, 2025

dickwolff commented Jan 5, 2025

ikruglov commented Jan 5, 2025

ikruglov commented Jan 5, 2025

dickwolff commented Jan 5, 2025 •

edited

Loading

jonathanR79 commented Jan 5, 2025

dickwolff commented Jan 5, 2025

jonathanR79 commented Jan 5, 2025 •

edited

Loading

ikruglov commented Jan 9, 2025

ikruglov commented Jan 9, 2025

dickwolff commented Jan 10, 2025 •

edited

Loading

ikruglov commented Jan 10, 2025

ikruglov commented Jan 10, 2025

ikruglov commented Jan 10, 2025

dickwolff left a comment

dickwolff Jan 15, 2025

ikruglov Jan 19, 2025

dickwolff Jan 19, 2025

Improve degiro v3 parsing #148

Are you sure you want to change the base?

Improve degiro v3 parsing #148

Conversation

ikruglov commented Jan 4, 2025 • edited by dickwolff Loading

Added

Fixes

Checklist

Related issue (if applicable)

ikruglov commented Jan 4, 2025

dickwolff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikruglov commented Jan 5, 2025

dickwolff commented Jan 5, 2025

ikruglov commented Jan 5, 2025

ikruglov commented Jan 5, 2025

dickwolff commented Jan 5, 2025 • edited Loading

jonathanR79 commented Jan 5, 2025

dickwolff commented Jan 5, 2025

jonathanR79 commented Jan 5, 2025 • edited Loading

ikruglov commented Jan 9, 2025

ikruglov commented Jan 9, 2025

dickwolff commented Jan 10, 2025 • edited Loading

ikruglov commented Jan 10, 2025

ikruglov commented Jan 10, 2025

ikruglov commented Jan 10, 2025

dickwolff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikruglov commented Jan 4, 2025 •

edited by dickwolff

Loading

dickwolff commented Jan 5, 2025 •

edited

Loading

jonathanR79 commented Jan 5, 2025 •

edited

Loading

dickwolff commented Jan 10, 2025 •

edited

Loading