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

adding support for Ginmon VL Sparen #4379

Merged

Conversation

christen90
Copy link
Contributor

Account Statements
Buys
Sells
Fees
Non Importables

Copy link
Member

@Nirus2000 Nirus2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hallo @christen90
I would prefer that we process “Ginmon” as a (new) separate PDF importer, as sub-companies tend to change/convert their software more often. We would then have the problem of always looping “Ginmon” into a fixed importer.
It should be relatively easy to separate this.

Regards
Alex

Comment on lines 1355 to 1380
Block BuySellBlock = new Block(
"^([\\d]{2}\\.[\\d]{2}\\.) ([\\d]{2}\\.[\\d]{2}\\.) (Wertpapierverkauf|Wertpapierkauf) ([\\.,\\d]+)[\\+\\-]$");
type.addBlock(BuySellBlock);
BuySellBlock.set(new Transaction<BuySellEntry>()

.subject(() -> {
BuySellEntry portfolioTransaction = new BuySellEntry();
portfolioTransaction.setType(PortfolioTransaction.Type.BUY);
return portfolioTransaction;
})

.section("amount", "date", "isin", "shares", "type") //
.documentContext("currency", "year") //
.match("^([\\d]{2}\\.[\\d]{2}\\.) (?<date>[\\d]{2}\\.[\\d]{2}\\.) (?<type>Wertpapierverkauf|Wertpapierkauf) (?<amount>[\\.,\\d]+)[\\+\\-]$") //
.match("^.* (?<isin>[A-Z]{2}[A-Z0-9]{9}[0-9])\\* +(?<shares>[\\.,\\d]+)$") //
.assign((t, v) -> {
if ("Wertpapierverkauf".equals(v.get("type")))
t.setType(PortfolioTransaction.Type.SELL);
t.setSecurity(getOrCreateSecurity(v));
t.setDate(asDate(v.get("date") + v.get("year")));
t.setCurrencyCode(v.get("currency"));
t.setShares(asShares(v.get("shares")));
t.setAmount(asAmount(v.get("amount")));
v.getTransactionContext().put(FAILURE,
Messages.MsgErrorTransactionAlternativeDocumentRequired);
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you know 😉, we do not import securities bookings via account statements. In your example, you have already shown via Kauf14.txt that there are separate statements.

Copy link
Contributor Author

@christen90 christen90 Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow they generate account statements with buys only. To have a better error message I tried it this way. Tagging it as non importable.

@Nirus2000 Nirus2000 added the pdf label Dec 2, 2024
@christen90
Copy link
Contributor Author

Hallo @christen90 I would prefer that we process “Ginmon” as a (new) separate PDF importer, as sub-companies tend to change/convert their software more often. We would then have the problem of always looping “Ginmon” into a fixed importer. It should be relatively easy to separate this.

Regards Alex

Currently the only Ginmon specific document is the invoice. And this will be marked as non importable. You mean that this one shall be moved to a new importer?

Best regards Christian

@Nirus2000
Copy link
Member

Hello @christen90

Currently the only Ginmon specific document is the invoice. And this will be marked as non importable. You mean that this one shall be moved to a new importer?

Yes, all documents created by “Ginmon Vermögensverwaltung GmbH”.

  • FeeInvoice01.txt
  • Kontoumsaetze08.txt
  • Kontoumsaetze09.txt
  • Kontoumsaetze10.txt
  • Kontoumsaetze12.txt
  • Kontoumsaetze12.txt

The way I see it, they all are. Except Verkauf14.txt is apparently a regular member of the DAB Group.


#4379 (comment)

Somehow they generate account statements with buys only. To have a better error message I tried it this way. Tagging it as non importable.
Yes, we should keep it that way. Very good, I had overlooked “MsgErrorTransactionAlternativeDocumentRequired”. 👍🏻

As a hint... if there are problems with the DAB, simply lock the document: PDFParser.java-->"mustNotInclude"

        public DocumentType(String mustInclude, String mustNotInclude,
                        Consumer<Transaction<DocumentContext>> contextBuilder)
        {
            this.mustInclude.add(Pattern.compile(mustInclude));
            this.mustNotInclude.add(Pattern.compile(mustNotInclude));
            this.contextBuilder = contextBuilder;
        }

        public DocumentType(String mustInclude, String mustNotInclude)
        {
            this.mustInclude.add(Pattern.compile(mustInclude));
            this.mustNotInclude.add(Pattern.compile(mustNotInclude));
        }

Regards
Alex

@christen90 christen90 requested a review from Nirus2000 December 4, 2024 19:55
@christen90
Copy link
Contributor Author

christen90 commented Dec 4, 2024

I moved the invoice to a new importer.

Please see a picture of pdf document (reference Kontoumsaetze09.txt - example for all account statements I provided with this PR) and confirm if you really would treat them as Ginmon documents instead of DAB. For me Ginmon is just mentioned in the addressfield but I understand Ginmon not as the publisher of PDF.

Kontoumsaetze09 pdf

Add new Ginmon PDF-Importer
@Nirus2000
Copy link
Member

Hello @christen90
I have added the following changes:

For the Gimnon fee statement, we use the Gimnon statement document directly and not the account statement.
The Gimnon statement simply contains good information such as invoice number and period. If we have separate and detailed documents, then we will use these.

I have also tidied up the DAB a bit.

Regards
Alex

@Nirus2000 Nirus2000 merged commit 4b3e78d into portfolio-performance:master Dec 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

2 participants