Fix issue #3991 to be able to import French Transactions Degiro PDF #3992
Conversation
|
Hello @couclock We always check all transactions in a PDF debug as well as all securities that are determined. One more question... what is the "id" responsible for or how should it be processed further? Regards |
Do you mean in UT (name.abuchen.portfolio.tests/src/name/abuchen/portfolio/datatransfer/pdf/degiro/DegiroPDFExtractorTest.java) ?
This "id" attribute on Section is the only way I found to identify which section (and especially its regex) is applied on a line. Without it, it's almost impossible to find the right section by comparing only regex that are so close. Thanks a lot for your first feedback. PS: I noticed "Account statement" PDF is not yet supported in French, do you know if anyone is working on it ? |
|
Nice one, I think the "id" would be helpful in the future to reference to a regex. As you mentioned finding the exact regex is a needle in a haystack. |
|
Hello @couclock
Yes, please. 👍🏻 |
|
|
||
| /** | ||
| * Utility function to check one security | ||
| * | ||
| * @param security | ||
| * @param expName | ||
| * @param expIsin | ||
| * @param expCurrency | ||
| */ | ||
| private void checkOneSecurity( Security security, String expName, String expIsin, String expCurrency) | ||
| { | ||
| assertThat(security.getName(), is(expName)); | ||
| assertThat(security.getIsin(), is(expIsin)); | ||
| assertThat(security.getCurrencyCode(), is(expCurrency)); | ||
| } | ||
|
|
||
| /** | ||
| * Utility to check one Buy/Sell transaction | ||
| * | ||
| * @param entry | ||
| * @param portfolioTransactionType | ||
| * @param accountTransactionType | ||
| * @param dateTime | ||
| * @param shares | ||
| * @param currency | ||
| * @param monetaryAmount | ||
| * @param grossValue | ||
| * @param taxSum | ||
| * @param feeSum | ||
| * @param forexCurrency | ||
| * @param forexValue | ||
| */ | ||
| private void checkOneBuySellEntry( BuySellEntry entry, PortfolioTransaction.Type portfolioTransactionType, AccountTransaction.Type accountTransactionType, | ||
| String dateTime, double shares, String currency, double monetaryAmount, | ||
| double grossValue, double taxSum, double feeSum, String forexCurrency, double forexValue) | ||
| { | ||
|
|
||
| assertThat(entry.getPortfolioTransaction().getType(), is(portfolioTransactionType)); | ||
| assertThat(entry.getAccountTransaction().getType(), is(accountTransactionType)); | ||
|
|
||
| assertThat(entry.getPortfolioTransaction().getDateTime(), is(LocalDateTime.parse(dateTime))); | ||
| assertThat(entry.getPortfolioTransaction().getShares(), is(Values.Share.factorize(shares))); | ||
|
|
||
| assertThat(entry.getPortfolioTransaction().getMonetaryAmount(), | ||
| is(Money.of(currency, Values.Amount.factorize(monetaryAmount)))); | ||
| assertThat(entry.getPortfolioTransaction().getGrossValue(), | ||
| is(Money.of(currency, Values.Amount.factorize(grossValue)))); | ||
| assertThat(entry.getPortfolioTransaction().getUnitSum(Unit.Type.TAX), | ||
| is(Money.of(currency, Values.Amount.factorize(taxSum)))); | ||
| assertThat(entry.getPortfolioTransaction().getUnitSum(Unit.Type.FEE), | ||
| is(Money.of(currency, Values.Amount.factorize(feeSum)))); | ||
|
|
||
| if (forexCurrency != null) | ||
| { | ||
| Unit grossValueUnit = entry.getPortfolioTransaction().getUnit(Unit.Type.GROSS_VALUE) | ||
| .orElseThrow(IllegalArgumentException::new); | ||
| assertThat(grossValueUnit.getForex(), is(Money.of(forexCurrency, Values.Amount.factorize(forexValue)))); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Hello @couclock
please undo this. We already have a simple test variant.
As you can see in the other tests and also read in the contributing rules, we use these variants.
From April 2023 we will use the simplified notation of test cases (preferred variant)
ComDirect Bank
Baader Bank with testWertpapierKauf23()
Sbroker with testDividende11()
Sbroker with testGiroKontoauszug10()
|
@couclock writes:
I like the idea of the "id". If include in failure messages, it allows to quickly go to the code. However, at the moment
My proposal is to a) print the id as part of the exception and b) make sure the id is unique. I'll add another commit. |
|
merged |
Issue: #3991
Describe the bug
Using latest release of Portfolio Performance, I'm facing issue when I try to import Degiro Pdf of my latest transactions.
Few transactions are not valid. Consequently, I'm not able to import my transactions.
To Reproduce
Here is what I get when I use File > Import > Debug: Create text from PDF ... :
After analyzing errors using source code:
1st error with lines
The total amount (after share count) do not have expected decimal count {2,6} but only one and even none.
2st error with line
26-03-2024 10:27 ISHARES PROP US IE00B1FZSF77 EAM XAMS -4 25,25 EUR 101,00 EUR 101,00 EUR EUR 101,00 EURThe fee amount is not set, we can only see fee currency. This is because the transaction has been splitted and the fee has been charged only once.This PR is fixing all mentionned issues.
Expected behavior
I expect all Transactions PDF generated from Degiro website can be imported smoothly in PortfolioPerformance.
Screenshots
Find attached a screenshot showing the error I get.
Second screenshot show the PDF as exported by Degiro.
Desktop (please complete the following information):