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.

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

Improve some PDF-Importer to standardized tax lost adjustment transaction #4366

Conversation

Nirus2000
Copy link
Member

The tax lost adjustment transaction has been standardized for various PDF importers.

@Nirus2000 Nirus2000 added the pdf label Nov 24, 2024
@Nirus2000
Copy link
Member Author

Nirus2000 commented Nov 24, 2024

I have no issues...
grafik

Hmm... AbstractPDFExtractor ?! The language?!
private final NumberFormat numberFormat = NumberFormat.getInstance(Locale.GERMANY);

@buchen
Copy link
Member

buchen commented Nov 29, 2024

The problem is the following.

  • The tests run with the "en" locale.
  • You write the shares to the context using String.format and hence the "en" locale (see line 177 in Direkt1822BankPDFExtractor), say "11.00000000"
  • You then read the shares always with the German locale (that is hard coded in the AbstractPDFExtractor) which uses the comma as decimal separator

And, whoops, you have a mismatch because parsing does not see the dot as decimal separator.

It does not happen for you locally, because I assume you run with the German locale.

If you want to reproduce the problem locally, add this to your launch configuration

-Duser.language=en -Duser.country=
Bildschirmfoto 2024-11-29 um 10 37 07

I think the important thing is to use the same formatter for reading and writing (and hence the same locale setup),
for example,

context.put("shares", getNumberFormat().format(item.getShares() / Values.Share.divider()));
[...]
asShares(context.get("shares"))

or

context.put("shares", Long.toString(item.getShares()));
[...]
t.setShares(Long.parseLong(context.get("shares")));

@buchen buchen marked this pull request as draft November 29, 2024 09:47
@pfalcon pfalcon mentioned this pull request Nov 29, 2024
The tax lost adjustment has been standardized for various PDF importers.
@Nirus2000 Nirus2000 force-pushed the Improve-some-PDF-Importer-to-standardized-tax-lost-adjustment branch from 7af3f90 to 23d7655 Compare November 29, 2024 20:17
@Nirus2000 Nirus2000 marked this pull request as ready for review November 29, 2024 20:20
@Nirus2000
Copy link
Member Author

Hello @buchen
Thanks for the hint.
I have changed it and moved it to the master.

Regards
Alex 👍🏻

@Nirus2000 Nirus2000 merged commit 07f16c5 into portfolio-performance:master Nov 29, 2024
2 checks passed
@Nirus2000 Nirus2000 deleted the Improve-some-PDF-Importer-to-standardized-tax-lost-adjustment branch November 29, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants