actualbudget / actual

A local-first personal finance app
https://actualbudget.org
MIT License
13.62k stars 1.09k forks source link

[Feedback] Nordigen bank-sync #724

Closed MatissJanis closed 1 year ago

MatissJanis commented 1 year ago

Nordigen bank sync is now available in the v23.3.1+ release as an experimental feature. This issue is to collect bug reports about the feature.

If you have bug reports - feel free to post them as comments on this thread and I will aggregate them on this root-level message.

How to test

  1. Install the latest master branch for actual and actual-server or v23.5.0
  2. Launch actual-server: yarn start
  3. Launch actual: yarn start:browser
  4. Open the frontend (http://localhost:3001) and set the server to the running local backend (http://localhost:5006).
  5. Once logged in, go to settings → advanced → experimental features. Read the message and click “I understand.”
  6. Enable the Bank sync experimental feature
  7. Create a new Actual account and link your bank
  8. when prompted to set-up API keys - register to Nordigen and get them from there

Priority issues

(we will launch Nordigen as an experimental feature; this list contains issues blocking the full first-party feature launch of the feature)

Other issues

MatissJanis commented 1 year ago

[RESOLVED]

Some design feedback

  1. "Link your account" modal - done 1.1. The dropdown UI could be improved 1.2. The "back" button can be removed as we already have a "x" at the top Screenshot 2023-03-04 at 09 00 24
  2. "Link accounts"

2.1. the modal is lacking in visual hierarchy; possibly use a table there? 2.2. account for long account names

Screenshot 2023-03-04 at 09 04 49
MatissJanis commented 1 year ago

[RESOLVED]

One more big item (and IMO super important):

Currently we support only 2-3 banks. However, we should be able to support a much larger set of banks if we provide meaningful defaults. Some work on this: https://github.com/actualbudget/actual-server/pull/144

Basically, if we define a "default bank" integration, we should be able to support the majority of Nordigen supported banks out of the box.

Some might not fully work.. but that's fine - people can always report them as bugs. But at least it will give us like 90% coverage without having to build each bank integration separately.

MatissJanis commented 1 year ago

Pending transactions are not added to the list.

The bank-sync API returns 2x transaction types: pending and complete. Handling of "complete" transactions works as expected. They are added to the transaction table as "cleared" transactions.

However, "pending" transactions do not show up. Ideally they should show up as "not-cleared".

MatissJanis commented 1 year ago

[RESOLVED]

Bug: creating linked accounts should only be available if actual-server is being used. For local-only installs we should ~not show the "link accounts" option as it will not work~ disable the account linking button, but link to the docs.

j-f1 commented 1 year ago

[RESOLVED]

Bug: creating linked accounts should only be available if actual-server is being used. For local-only installs we should not show the "link accounts" option as it will not work.

IMO we should show the button as disabled and link to docs (just like we do with e2ee)

fstybel commented 1 year ago

Bug: When we finish going through the wizard to add a new account and it disappeared we have a few seconds when nothing happens (in the background the app fetches transactions and processes them). But the user can be confused for a moment -- he should see some spinner or some loading screen with the information that synchronization happening.

fstybel commented 1 year ago

Feature: Display information when the requisition for the account will expire.

fstybel commented 1 year ago

Bug: Error handling is not sufficient. For example:

MatissJanis commented 1 year ago

Feature: Display information when the requisition for the account will expire.

Could you please expand on this one?

fstybel commented 1 year ago

Feature: Display information when the requisition for the account will expire.

Could you please expand on this one?

When we add an account from your bank to Actual we create the Requisition/Agreements on the backend side in Nordigen. This access is valid for 30 days (default). It would be nice to see that our requisition will expire soon and we can expect we will need to re-assign the accounts. Additionally, if we exceeded this time we can just suggest to the user that he has to re-link his accounts linked with the requisition instead of the current behavior -> User clicks the sync button then he receives errors about sync failing, and finally, he can see to re-link button which gives him the possibility to link accounts again.

It's easy to reproduce it by changing passing 1 as a value for accessValidForDays in create-web-token request. And try to sync account next day (after 24h)

fstybel commented 1 year ago

[RESOLVED]

Bug: The NordigenAPI access token expires 24h after creation - we should regenerate the token if it's expired (I'm working on the fix). The current workaround is to reset the server.

rich-howell commented 1 year ago

[RESOLVED]

If you are not using a server, it seems that when you are asked for your country, the app has an internal error, a message is presented to the user saying we have been notified, which isn't true.

image

Maybe the link bank button should be disabled if there is no server configured?

MatissJanis commented 1 year ago

[RESOLVED]

Bug:

Using it in fly.io doesn't seem to work. After confirming the Nordigen integration in my bank I was redirected to this page:

Screenshot 2023-03-09 at 21 31 53

Update: bank-link worked in the end. One of the tabs was in my budget page. The other was in /nordigen/link?ref=... and was showing the above error for some reason.

Still need to figure out why.. and what can be done to fix this. But at least we're not fully blocked on setting bank-sync up.

MatissJanis commented 1 year ago

[RESOLVED]

If you are not using a server, it seems that when you are asked for your country, the app has an internal error, a message is presented to the user saying we have been notified, which isn't true.

image

Maybe the link bank button should be disabled if there is no server configured?

@rich-howell thanks!

We have this bullet for the issue:

MatissJanis commented 1 year ago

----------- v23.3.0 --------------

zr40 commented 1 year ago

Linking with Nordigen succeeds, but retrieving transactions fails with Nordigen returning a 400 Bad Request status. According to the logs, Actual requests the following: /api/v2/accounts/.../transactions/?date_from=2024-01-28&date_to=2023-03-10. Note that date_from is far in the future.

Presumably this is caused by me having pre-entered transactions with a future date. 2024-01-28 is the date of the second most distant future transaction I've pre-entered.

DistroByte commented 1 year ago

[RESOLVED]

If you are not using a server, it seems that when you are asked for your country, the app has an internal error, a message is presented to the user saying we have been notified, which isn't true. image Maybe the link bank button should be disabled if there is no server configured?

@rich-howell thanks!

We have this bullet for the issue:

* actual (bug): creating linked accounts for non-server installs should not work

also occurred for me, running my own sync instance. Account was linked, but threw error

DistroByte commented 1 year ago

UI related issue, linked status indicator overlaps with currently active account image

DistroByte commented 1 year ago

[RESOLVED]

Certain accounts seem to work better than others, possibly a difference in how nordigen handles accounts.

One such error is this: image

I looked for logs, no joy

image

j-f1 commented 1 year ago

Is there any information in your browser’s console?

DistroByte commented 1 year ago

[RESOLVED]

There is!

image

I forgot about checking the browser window 😅

When I initially add the accout this is the error

DistroByte commented 1 year ago

After refreshing the page, the sync account button appears, and pressing it causes the same error to appear. I can try and paste it here for you but a cursory glance and my knowledge of js makes me thing it will be useless.

Jackenmen commented 1 year ago

[RESOLVED] Bug: It's impossible to tell the difference between multiple accounts with different currencies when they share the same IBAN: image

I'm not sure if the solution here should be to create a custom integration for Revolut or if this is not specific to Revolut and it may make sense to include the currency in the drop-down list. Just to note, I checked the server logs to see the difference between the accounts and the only difference that is meaningful to a user is the currency (the other differences are resourceId, id, and created fields which wouldn't help the user to select the proper account).

Jackenmen commented 1 year ago

[RESOLVED] Bug: Transactions for -0.00 (common when the merchant tries to verify the card) do not properly import the Payee - Actual tries to use debtorName rather than creditorName: https://github.com/actualbudget/actual/blob/0bae379c17cdfe997ab30a33f78ea2a42315e859/packages/loot-core/src/server/accounts/sync.js#L315-L349

Example transaction (I changed dates and IDs for privacy reasons):

{
    "transactionId": "8ae8eec6-565b-e854-cf6b-c6759028107d",
    "bookingDate": "2023-03-11",
    "valueDate": "2023-03-11",
    "bookingDateTime": "2023-03-11T13:54:07.123456Z",
    "valueDateTime": "2023-03-11T13:54:07.300108Z",
    "transactionAmount":
    {
        "amount": "-0.00",
        "currency": "PLN"
    },
    "creditorName": "Paypal",
    "remittanceInformationUnstructuredArray":
    [
        "Paypal"
    ],
    "proprietaryBankTransactionCode": "CARD_PAYMENT",
    "internalTransactionId": "a64d3c736cb6aef7c879da860181c419"
}

Additionally, I think that the Payee and Notes logic should probably also try to use remittanceInformationUnstructuredArray when the remittanceInformationUnstructured field isn't present.

Jackenmen commented 1 year ago

[RESOLVED] Bug: PayPal transactions cannot be imported due to an internal error in devtools console:

Notifications.js:76 Internal error: Error: `date` is required when adding a transaction
    at https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:106995
    at f (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:92890)
    at Generator.<anonymous> (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:94227)
    at Generator.next (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:93253)
    at E (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:98880)
    at a (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:99083)
    at https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:99142
    at new Promise (<anonymous>)
    at https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:99023
    at Q (https://my-url.fly.dev/kcab/kcab.worker.88611b7e16db2b62f64b.js:10:108151)

This appears to be caused by Actual only trying to use valueDate (which, in general, does actually make more sense) while PayPal only provides bookingDate. I'm not sure if it generally makes sense to fall back to bookingDate when valueDate is present or if this should be a PayPal-specific fix.

Example transactions (I changed dates, IDs, and amounts for privacy reasons):

[
    {
        "transactionId": "CJ7SSK007TVCA16XI",
        "bookingDate": "2023-03-11",
        "bookingDateTime": "2023-03-11T14:33:05.920799",
        "transactionAmount":
        {
            "amount": "-1.23",
            "currency": "PLN"
        },
        "creditorName": "[CREDITOR NAME HERE]",
        "debtorName": "Jakub Kuczys",
        "remittanceInformationUnstructured": "billing_agreement_id: B-QN38FJ8VSJ99TV4F6, invoice_number: 9682055026172410815",
        "additionalInformation": "PAYMENT",
        "internalTransactionId": "d28e4dc9c5ce980d1827c85f2b4c6e29"
    },
    {
        "transactionId": "S8RYLWLBA4QLIXUD9",
        "bookingDate": "2023-03-10",
        "bookingDateTime": "2023-03-10T14:39:59.127797",
        "transactionAmount":
        {
            "amount": "3.21",
            "currency": "USD"
        },
        "creditorName": "Jakub Kuczys",
        "debtorName": "[DEBTOR NAME HERE]",
        "additionalInformation": "PAYMENT",
        "internalTransactionId": "8675e80448ba62cfcb32379b43054c41"
    }
]
Jackenmen commented 1 year ago

Bug: When syncing all accounts (from the all accounts page), some accounts become bolded even though no new transactions or other changes to the balance were made (the image shows an example for sync that hasn't changed any of the 4 accounts that I linked but bolded Revolut and Nest Bank while it didn't bold either of ING Bank accounts): image

(Balances were removed from the image for privacy reasons)

I'm not sure how the bold status is determined after sync so I can't really provide more useful information.

Potjoe-97 commented 1 year ago

I'm not sure if it generally makes sense to fall back to bookingDatewhen valueDate is present or if this should be a PayPal-specific fix.

Some banks only show bookingDate, and never provide valueDate. It could explain why @DistroByte and myself are having the same error "date is required when adding transactions". From docker log, I can see valueDate is never provided.

j-f1 commented 1 year ago

In Safari, the attempt to open the page for linking the bank trips the popup blocker. This might be due to the create-web-token request taking too long maybe? Either way we should add a prompt to “Waiting on Nordigen…” with options to reopen the link (synchronously) and/or check the browser UI for blocked popups.

Jackenmen commented 1 year ago

Other issues

  • [ ] actual: improve error handling: here

I think that 429 part of this issue should be moved to launch-blocking issues as not following rate limits is rather serious and is generally considered being a bad API user. It's probably unlikely that an individual user would run into these enough to be suspended for it (unless they spam the sync button for long time) but it still seems important to fix this. I think that ideally, rate limit being depleted would be tracked per account to avoid repeatedly running into 429s during sync request within the same day when it's already clear that the request can't succeed until the next day.

This documentation is quite informative: https://nordigen.zendesk.com/hc/en-gb/articles/6761006738717-Bank-API-Rate-Limits

MatissJanis commented 1 year ago

----------- v23.3.2 --------------

MatissJanis commented 1 year ago

Feature (kinda):

We need to add a very visible warning that by connecting your Actual account to Nordigen - Nordigen (a 3rd party) will get access to READ all your bank transactions.

MatissJanis commented 1 year ago

[WONTFIX]

Bug:

remittanceInformationUnstructured is used in the notes field. However, by default it's not an empty string. It's - value. So all notes fields get the "-" value which is a bit annoying.

Fix: we should remove the default - value from remittanceInformationUnstructured (and the array equivalent)

remittanceInformationUnstructured":"-","remittanceInformationUnstructuredArray":["-"]
Jackenmen commented 1 year ago

remittanceInformationUnstructured is used in the notes field. However, by default, it's not an empty string. It's - value. So all notes fields get the "-" value which is a bit annoying.

Are you sure this isn't bank-specific? I can't really test for this since all of the transactions returned by my banks seem to always have some value in there and I have yet to encounter a transaction with "-"/["-"] or ""/[]

MatissJanis commented 1 year ago

remittanceInformationUnstructured is used in the notes field. However, by default, it's not an empty string. It's - value. So all notes fields get the "-" value which is a bit annoying.

Are you sure this isn't bank-specific? I can't really test for this since all of the transactions returned by my banks seem to always have some value in there and I have yet to encounter a transaction with "-"/["-"] or ""/[]

Interesting.. if this is bank-specific, then I'll just close this as a wont-fix as it can also be solved by creating a rule that removes these notes.

MatissJanis commented 1 year ago

FYI: This is how a transfer transaction looks like in Nordigen. Given this - it might not be possible to create a feature that automatically recognises transfers.

However: a workaround is possible. You can create a rule that matches the transaction name and then changes the payee to a transfer. That seems to be working for me.

{
    "transactionId": "xxxxxxxxxxx",
    "bookingDate": "2023-03-13",
    "valueDate": "2023-03-13",
    "transactionAmount": {
        "amount": "100.0",
        "currency": "EUR"
    },
    "debtorName": "From Other account to Main Account",
    "remittanceInformationUnstructured": "From Other account to Main Account",
    "remittanceInformationUnstructuredArray": [
        "From Other account to Main Account"
    ],
    "bankTransactionCode": "PMNT-RCDT-ESCT",
    "internalTransactionId": "xxxxxxxxxxx"
}
Jackenmen commented 1 year ago

This is how a transfer transaction looks like in Nordigen

It actually depends on the bank - some banks provide creditor/debtorAccount field while others don't. Sadly it doesn't appear to be consistent with Nordigen's spreadsheet though as out of 3 banks I can test from this list (ING, Nest Bank Private, Revolut) that supposedly provide it, only one actually does (though with Revolut its hard to tell since I don't make regular transfer to IBANs with it) - ING. It seems to provide these two fields for all transactions and these fields are in one of two formats:

ppmt commented 1 year ago

I have a Starling account and for the second time I noticed that the imported transaction gets the wrong payee field...

The Payee in Actual is filled in with the remittanceInformationUnstructured field rather than the creditorName field.

In the log from my browser I can see that both field are correctly populated by nordringen with the correct payee and information.

Let me know if you need more info as I am not sure what to provide.

Edit: The amount being paid is a positive amount (a deposit) and here is a copy of the modified message received in the browser.

1 Object { transactionId: "yyyyyyy", bookingDate: "2023-03-13", valueDate: "2023-03-13", … } transactionId "yyyyyy" bookingDate "2023-03-13" valueDate "2023-03-13" bookingDateTime "2023-03-13T08:10:42Z" valueDateTime "2023-03-13T08:10:42Z" transactionAmount Object { amount: "17.00", currency: "GBP" } creditorName "MR........" remittanceInformationUnstructured "5 ......" proprietaryBankTransactionCode "FASTER_PAYMENTS_IN" internalTransactionId "zzzzzzz"

TheiLLeniumStudios commented 1 year ago
image

Happens with using Wise sync

Jackenmen commented 1 year ago

@TheiLLeniumStudios are you using an up-to-date version of Actual? There was a problem such as this one reported previously and fixed in release v23.3.2: https://github.com/actualbudget/actual/issues/724#issuecomment-1464914546

If you are sure that you are using an up-to-date version of Actual, you should check server logs for the raw transaction data and post it here, after potentially stripping it from any private information you don't want to share.

TheiLLeniumStudios commented 1 year ago

@Jackenmen I am using v23.3.2. Here's what it looks like:

[{"transactionId":"redacted","bookingDate":"2023-03-13","bookingDateTime":"2023-03-13T09:06:35.071Z","transactionAmount":{"amount":"485.00","currency":"EUR"},"debtorName":"STRIPE","debtorAccount":{"bban":"redacted"},"remittanceInformationUnstructured":"redacted","additionalInformation":"redacted","proprietaryBankTransactionCode":"TRANSFER","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-03-08","bookingDateTime":"2023-03-08T16:38:02Z","transactionAmount":{"amount":"-297.42","currency":"EUR"},"currencyExchange":{"sourceCurrency":"EUR","exchangeRate":"1.0553","targetCurrency":"USD"},"creditorName":"redacted","remittanceInformationUnstructured":"Spent redacted","proprietaryBankTransactionCode":"CARD_TRANSACTION","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-03-06","bookingDateTime":"2023-03-06T11:12:19.391Z","transactionAmount":{"amount":"202.26","currency":"EUR"},"debtorName":"redacted","debtorAccount":{"bban":"redacted"},"remittanceInformationUnstructured":"redacted","additionalInformation":"redacted","proprietaryBankTransactionCode":"TRANSFER","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-03-02","bookingDateTime":"2023-03-02T13:43:46.551Z","transactionAmount":{"amount":"0.56","currency":"EUR"},"debtorName":"TransferWise","debtorAccount":{},"remittanceInformationUnstructured":"Received Balance cashback","additionalInformation":"BALANCE CASHBACK","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-27","bookingDateTime":"2023-02-27T11:32:08.674Z","transactionAmount":{"amount":"83.05","currency":"EUR"},"debtorName":"STRIPE","debtorAccount":{"bban":"redacted"},"remittanceInformationUnstructured":"Received STRIPE","additionalInformation":"redacted","proprietaryBankTransactionCode":"TRANSFER","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-24","bookingDateTime":"2023-02-24T08:02:48Z","transactionAmount":{"amount":"-3.35","currency":"EUR"},"creditorName":"Steam","remittanceInformationUnstructured":"Spent Steam","proprietaryBankTransactionCode":"CARD_TRANSACTION","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-23","bookingDateTime":"2023-02-23T10:19:17Z","transactionAmount":{"amount":"-40.00","currency":"EUR"},"creditorName":"redacted","remittanceInformationUnstructured":"redacted","proprietaryBankTransactionCode":"CARD_TRANSACTION","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-20","bookingDateTime":"2023-02-20T08:59:32Z","transactionAmount":{"amount":"-5.00","currency":"EUR"},"creditorName":"redacted","remittanceInformationUnstructured":"redacted","proprietaryBankTransactionCode":"CARD_TRANSACTION","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-20","bookingDateTime":"2023-02-20T08:49:42.461Z","transactionAmount":{"amount":"303.56","currency":"EUR"},"debtorName":"STRIPE","debtorAccount":{"bban":"redacted"},"remittanceInformationUnstructured":"Received STRIPE","additionalInformation":"redacted","proprietaryBankTransactionCode":"TRANSFER","internalTransactionId":"redacted"},{"transactionId":"redacted","bookingDate":"2023-02-19","bookingDateTime":"2023-02-19T06:33:01Z","transactionAmount":{"amount":"-81.09","currency":"EUR"},"creditorName":"redacted","remittanceInformationUnstructured":"redacted","proprietaryBankTransactionCode":"CARD_TRANSACTION","internalTransactionId":"redacted"}]
waseem-h commented 1 year ago

@Jackenmen this fixes the above issue https://github.com/actualbudget/actual/pull/754

Jackenmen commented 1 year ago

I have a Starling account and for the second time I noticed that the imported transaction gets the wrong payee field...

@ppmt I asked Nordigen support about this and based on the latest response, it seems that you should expect this to get fixed upstream, hopefully in near future: Screenshot_20230315_131726_Gmail

ppmt commented 1 year ago

Great news!

Thanks a lot for your help @Jackenmen

Will report when I see that it has been fixed!

Kidglove57 commented 1 year ago

When choosing an "Actual Bank Account" in the Link Accounts screen, it is not currently possible to scroll the list. On a small screen, my list of accounts will not fit and I had to zoom out to select the correct account match. A small thing but scrolling would be a nice UI improvement.

leikoilja commented 1 year ago

Can't seem to configure actual to use nordigen integration. My first local actualbudget spin-up:

regardless, i keep getting hit by the same image

Do i miss smth obvious in my setup? 😅

Solved: Not sure why config.json is not loaded and picked up by docker container, but realised that since I am using docker i need to explore the env to the container. So on the server, docker-compose adding

    ...
    restart: unless-stopped
    environment:
      ACTUAL_NORDIGEN_SECRET_ID: ...
      ACTUAL_NORDIGEN_SECRET_KEY: ...

fixed it for me

DistroByte commented 1 year ago

Enable the flag in Settings, it's an experimental feature. See step 6 in original message in this issue

leikoilja commented 1 year ago

Thanks @DistroByte. Sorry, forgot to mention - "sync accounts" flag is enabled

Jackenmen commented 1 year ago

@leikoilja did you actually restart the server after changing the configuration and/or env vars so it can start using it? In case of env vars you need to also make sure that the server is started in a context where those env vars set.

BTW, this is more of an issue for sending feedback about the feature than for support - I think it would be better to continue this conversation on Actual's Discord.

leikoilja commented 1 year ago

@Jackenmen, thanks, figured out that i needed to expose env variable to the docker container (updated my message above). Initially, thought the issue was related to the integration itself, but you re right, it's not the best place for it. (@MatissJanis, feel free to clean it up if you think it's unrelated)

leikoilja commented 1 year ago

Awesome, now that it works, i have been able to add 3 different Swedish banks (7 accounts) with nearly no problem, thank you, gents! Some minor UI feedback:

  1. When connecting multiple accounts, the UI get's a bit messy with overflows and alignment image
  2. 2/3 banks failed to correctly parse the transactions (payee and notes are missing). Supposedly after adding defaults? If so, any reference how I can help to implement those specific bank renderers/integrations? Screenshot 2023-03-16 at 13 32 16
  3. One of the banks pulled data quite sporadically... There was at least 1 transaction per day during the past month in the bank statement. However, syncing via nordigen on 16.03.22, i get 1 transaction dated as 02/22/2023, 1 transaction on 02/24/2023 and about 20 other transactions all dated on 02/28/2023... What's weirder is that some actual transaction that happen in January is important as happening on 02/28/2023, which is incorrect. I guess that bank also calls for unique bank integration/rendeder, as point 2?

Real: https://user-images.githubusercontent.com/10655107/225623487-a6562bb9-13f1-441f-a180-5fe04d19f3a8.jpeg

Actual:

image

Edit: new separate issue ticket for point 2: https://github.com/actualbudget/actual/issues/761