search

actualbudget / actual

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

[Bug]: Deduplication not handling gocardless inconsistencies after #2770 #2877

Closed matt-fidd closed 3 months ago

matt-fidd commented 3 months ago

Verified issue does not already exist?

What happened?

Firstly, I believe that #2770 was a sensible change, if two imported transactions have completely distinct IDs provided then they should be treated as separate transactions.

Unfortunately it seems that GoCardless have very inconsistent implementations and it is very common (at least with my accounts) that the financial_id (or imported_id) changes when a transaction is booked. #2770 breaks deduplication when this happens.

I've reported these issues to gocardless too as I believe that it's not the expected behaviour, however this was handled quite gracefully by actual before this change.

Below is a screenshot of the same transaction, the bottom pending and the top booked. image

Where are you hosting Actual?

Locally via Yarn

MatissJanis commented 3 months ago

👋 This is a good catch! GoCardless has in fact been quite loose in the "imported_id" fields.

I'm now considering to revert #2770 until we figure out a better solution.

cc @strazto maybe you have some ideas.

strazto commented 3 months ago

That's ugly on GoCardless's end

We could possibly make the behaviour introduced in #2770 optional and control it with a setting (though I don't love the added complexity)

Or revert #2770 with a TODO re. The gocardless bug, however, I don't know how fast gocardless tends to be with addressing tickets, especially when there's not much business case

Also I assume gocardless ingests various institutions' APIs to a common data model, and the behaviour may vary institute to institution, meaning that the fix may be more niche (who knows)

I don't really mind if this gets reverted, as long as I have a build I can use for my instance that includes #2770

Kidglove57 commented 3 months ago

For corroboration only.

I have three UK accounts with GoCardless. Starling, Halifax and Amex.

All three continue to sync correctly in 24.6.0 EXCEPT when I use the “edge” version whereupon Amex imports duplicate transactions, as transactions move from pending to settled.

MatissJanis commented 3 months ago

Created a revert: https://github.com/actualbudget/actual/pull/2910

This is purely to unblock the upcoming release - so we wouldn't break for a bunch of users using GoCardless. My hope is that we can re-introduce some sort of version of this change in a future release.

matt-fidd commented 3 months ago

Thanks @MatissJanis, one option I was thinking is to scrub the imported_id on pending gocardless transactions and then trust and save the one that comes in when it's booked. Though, there are banks whose implementation works correctly and keeps a consistent ID between states.