Improve Bigbank PDF importer

[MonkeySon]

• Added support for transaction type "Interne Belastung"

It is currently implemented as a REMOVAL AccountTransaction Type, although this might not be fully correct.

Bigbank offers overnight accounts and fixed-term accounts. This transaction was from the overnight to the fixed-term account, therefore "Interne Belastung". I replaced the Transaction Type with TRANSFER_OUT and expected a selection GUI which lets me choose accounts for the IN/OUT transaction. I intend to use two separate PP accounts for each Bigbank account.

Unfortunately, this seems to be an unsupported transaction for PDF importers, as it gives me an UnsupportedOperationException (thrown here).

What would be your suggestion in that case?
Keep it as a simple removal and let the user manually handle that case,
or is there a way to create an IN/OUT transaction for the PDF importer?

Cheers,
MonkeySon

[Nirus2000]

Hello @MonkeySon
Thank you for your pull request.
First of all, it is important that the test cases should not be changed permanently. This can cause irritation for other developers as the history is also checked here from time to time.
Please just create another test case. 👍🏻

Your problem describes that you are moving money from one account to another, so "removal" is correct.
(Account not equal to Portfolio)
(Check Transaction types)

Regards
Alex

[MonkeySon]

Hello Alex,

sorry for the testcase-confusion, I made it a separate one now.

I'm aware that accounts and portfolios are not the same and that they have different transaction types.
But accounts also support Transfers (a combination of TRANSFER_OUT from one account and a TRANSFER_IN in an offset account).

My question is if it would be beneficial for PP users to directly create this transaction, instead of having a removal in account A and then a manual (or from another PDF import) deposit in account B.

EDIT:
... or if that is even feasible, since it is a dedicated UnsupportedOperation (for some reason I am not aware of).

[Nirus2000]

Hello @MonkeySon @buchen
ah... I understand. Sorry for that... 💯

In principle this is of course possible, but we have to make several adjustments.

1. The Importer changes

        // @formatter:off
        // 28.03.2024 EE123456789101112132 Interne Belastung wMGSJi WajHthpvl -3 500,00
        // @formatter:on
        Block transferOutBlock = new Block("^[\\d]{2}\\.[\\d]{2}\\.[\\d]{4}.*Interne Belastung.* \\-[\\.,\\d\\s]+$");
        type.addBlock(transferOutBlock);
        transferOutBlock.set(new Transaction<AccountTransferEntry>()

                        .subject(AccountTransferEntry::new)

                        .section("date", "amount") //
                        .documentContext("currency") //
                        .match("^(?<date>[\\d]{2}\\.[\\d]{2}\\.[\\d]{4}).*Interne Belastung.* \\-(?<amount>[\\.,\\d\\s]+)$") //
                        .assign((t, v) -> {
                            t.setDate(asDate(v.get("date")));

                            t.getSourceTransaction().setAmount(asAmount(v.get("amount")));
                            t.getSourceTransaction().setCurrencyCode(v.get("currency"));

                            t.getTargetTransaction().setAmount(asAmount(v.get("amount")));
                            t.getTargetTransaction().setCurrencyCode(v.get("currency"));
                        })

                        .wrap(t -> new AccountTransferItem(t, true)));

}

2. UI for checking (apparently no checking is done here)

2.1. Problem 1

Missing check of the DropDown

2.2. Problem 2

2.3 Problem 3

3. Extension of the ExtractorMatcher

• countAccountTranferTransactions()
• AssertImportActions().check(results, "EUR", "EUR"); <--- Works correct !? I dont know...

4, Test case in old structure (better with new one, but we need to fix some problems)

    public void testTransferOut01()
    {
        Client client = new Client();

        BigbankPDFExtractor extractor = new BigbankPDFExtractor(client);

        List<Exception> errors = new ArrayList<>();

        List<Item> results = extractor.extract(PDFInputFile.loadTestCase(getClass(), "Kontoauszug02.txt"), errors);

        assertThat(errors, empty());
        assertThat(countSecurities(results), is(0L));
        assertThat(countBuySell(results), is(0L));
        assertThat(countAccountTransactions(results), is(3L));
        assertThat(results.size(), is(4));
        new AssertImportActions().check(results, "EUR", "EUR"); // <-- FIX HERE THIS ISSUE AND CREATE A TEST CASE

        // assert transaction
        assertThat(results, hasItem(deposit(hasDate("2024-03-20"), hasAmount("EUR", 10.00), //
                        hasSource("Kontoauszug02.txt"), hasNote(null))));

        // assert transaction
        assertThat(results, hasItem(deposit(hasDate("2024-03-21"), hasAmount("EUR", 10500.00), //
                        hasSource("Kontoauszug02.txt"), hasNote(null))));

        // assert transaction
        assertThat(results, hasItem(removal(hasDate("2024-03-25"), hasAmount("EUR", 10.00), //
                        hasSource("Kontoauszug02.txt"), hasNote(null))));

        // HERE CHANGE THE OLD TEST CASE STRUCTURE AND ADD A NEW TEST CASE STRUCTURE IN ExtractorMatcher8()
        AccountTransferItem transaction = (AccountTransferItem) results.stream()
                        .filter(AccountTransferItem.class::isInstance).findFirst()
                        .orElseThrow(IllegalArgumentException::new);

        AccountTransferEntry transferTransaction = (AccountTransferEntry) transaction.getSubject();

        // Source
        assertThat(transferTransaction.getSourceTransaction().getType(), is(AccountTransaction.Type.TRANSFER_OUT));
        assertThat(transferTransaction.getSourceTransaction().getCurrencyCode(), is("EUR"));
        assertThat(transferTransaction.getSourceTransaction().getDateTime(),
                        is(LocalDateTime.parse("2024-03-28T00:00")));
        assertThat(transferTransaction.getSourceTransaction().getMonetaryAmount(),
                        is(Money.of("EUR", Values.Amount.factorize(3500.00))));
        assertThat(transferTransaction.getSourceTransaction().getGrossValue(),
                        is(Money.of("EUR", Values.Amount.factorize(3500.0))));

        // Target
        assertThat(transferTransaction.getTargetTransaction().getType(), is(AccountTransaction.Type.TRANSFER_IN));
        assertThat(transferTransaction.getTargetTransaction().getCurrencyCode(), is("EUR"));
        assertThat(transferTransaction.getTargetTransaction().getDateTime(),
                        is(LocalDateTime.parse("2024-03-28T00:00")));
        assertThat(transferTransaction.getTargetTransaction().getMonetaryAmount(),
                        is(Money.of("EUR", Values.Amount.factorize(3500.0))));
        assertThat(transferTransaction.getTargetTransaction().getGrossValue(),
                        is(Money.of("EUR", Values.Amount.factorize(3500.0))));
    }

I think it works... but we have to fix some issues...

Regards
Alex

[buchen]

@MonkeySon writes:

Bigbank offers overnight accounts and fixed-term accounts. This transaction was from the overnight to the fixed-term account, therefore "Interne Belastung". I replaced the Transaction Type with TRANSFER_OUT and expected a selection GUI which lets me choose accounts for the IN/OUT transaction. I intend to use two separate PP accounts for each Bigbank account.

There UI has some (limited) support to import account transfers.
As @Nirus2000 pointed out, you would need to create a AccountTransferItem (not a TransactionItem).
And there are no check for matching currencies and other things.
And there is no option to convert it to a deposit/withdrawal if one does not want it as a transfer.

So far our strategy for the "cash transfers" was:

• Create a cash deposit or removal (withdrawal) transaction

Why not a transfer transactions? Because

• the user might not want to maintain the other account in PP (let's say your regular payroll account)
• the user might import the documents from the other account/bank as well (which will create the transaction again).

But @Nirus2000 and me already discussed that it makes sense to provide more options after the import:

• Right-click to convert any deposit/withdrawal to a transfer transaction
• Select two transactions (a pair of deposit and withdrawal) and "join" them to a transfer
• Run some kind of heuristic after the import to suggest a transfer (account at the same bank, same amount in a transaction).

For this particular change, I propose to create a deposit/withdrawal transaction instead.

[Nirus2000]

Hello @MonkeySon
Now implemented as removal (retraction). 👍🏻
If you convert this from the "draft" to a "PR", then we can transfer this to the "master branch".

[MonkeySon]

Hello @buchen and @Nirus2000 ,

thanks for your effort and inputs on that topic!

• the user might not want to maintain the other account in PP (let's say your regular payroll account)

This was also my concern thinking about this... It just makes it a little bit less customizable in a way.

• the user might import the documents from the other account/bank as well (which will create the transaction again).

Here I would assume this can be checked, similar to the other import checks, where it skips existing equal transactions.

• Right-click to convert any deposit/withdrawal to a transfer transaction
• Select two transactions (a pair of deposit and withdrawal) and "join" them to a transfer
• Run some kind of heuristic after the import to suggest a transfer (account at the same bank, same amount in a transaction).

These seem nice ideas.
But personally, in that specific case, I dont really care what type of transaction it is in my records. The main idea was to save one extra manual entry / one extra PDF import.

But I fully understand the points mentioned and am fine with keeping it implemented as a removal.

Thank you for the merge!