Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Degiro PDF import #3831

Closed
wants to merge 1 commit into from
Closed

Fix Degiro PDF import #3831

wants to merge 1 commit into from

Conversation

Nonnonnoo
Copy link

@Nonnonnoo Nonnonnoo commented Mar 9, 2024 •
edited by Nirus2000
Loading

Closes #3991

Ran into failure for a few months now that Degiro imports didnt work. Setup development environment to investigate and find the issue. Also looked at reports on the forum which had the same problem: https://forum.portfolio-performance.info/t/pdf-import-from-degiro/12145/135

04-10-2023 09:33 HANETF MANAGEMENT LTD. IE00BMYMHS24 XET XETA 73 6,8 EUR -496,40 EUR -496,40 EUR -3,00 EUR -499,40 EUR

The regex indicates that minimum is 2, so this PR changes only this certain situation. Need to investigate more if the other cases needs updating as well including a add a unit-test for it.
Therefor PR is still in progress.
Note: First time creating PR to fix bug issue on Github.

@OnkelDok
Copy link
Member

OnkelDok commented Mar 9, 2024

Therefor PR is still in progress.

Then please set this PR to draft.

@Nonnonnoo Nonnonnoo marked this pull request as draft March 9, 2024 12:15
@buchen
Copy link
Member

buchen commented Mar 9, 2024

Note: First time creating PR to fix bug issue on Github.

Welcome to github then.

Push the next changes to the same branch -and they will end up in this pull request.
Remove the "draft" label once you want us to have a look at it.

We strive to always have a test case for PDF changes - that makes it save to do future changes.

@Nonnonnoo
Copy link
Author

Fixing this importer seems a bit harder than I thought :). Especially lacking the RegEx skills to debug properly which RegEx is failing the testcase examples. Do you have any tips/tricks? or website that helps with parsing examples?

@buchen
Copy link
Member

buchen commented Apr 27, 2024

Fixing this importer seems a bit harder than I thought :). Especially lacking the RegEx skills to debug properly which RegEx is failing the testcase examples. Do you have any tips/tricks? or website that helps with parsing examples?

I think you definitely picked the hardest importer at all. Degiro is a tricky beast. Because the format in the file is cumbersome and because of the many languages.

This site https://regex101.com looks interesting in testing regular expression. Be sure to pick the Java flavor.

@buchen buchen added the pdf label Apr 27, 2024
@Nirus2000 Nirus2000 self-requested a review May 7, 2024 10:46
@Nonnonnoo
Copy link
Author

I think the PR in #3992 is in better condition than mine :) It has the similair change regarding the decimal value. So shall we close this one?

@Nirus2000
Copy link
Member

I think the PR in #3992 is in better condition than mine :) It has the similair change regarding the decimal value. So shall we close this one?

Okay 👍🏻

Closes

@Nirus2000 Nirus2000 closed this May 13, 2024
@Nonnonnoo Nonnonnoo deleted the fix-degiro-pdf-import branch May 14, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nirus2000 Nirus2000 Awaiting requested review from Nirus2000

Assignees
No one assigned
Labels
pdf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

French Degiro PDF import issues
4 participants
@Nonnonnoo @OnkelDok @buchen @Nirus2000