Only include parent transactions when matching during reconciliation

[lwid]

When doing fuzzy matching for reconciliation the current version includes child transactions (ie splits) when matching which makes no sense and results in incorrect matches. For instance a 1320 transactions which has been split into 1200+120 should only match the total 1320 and not the subs 1200 or 120.

The fix is simple, a condition for is_child==0 in the initial quere, have also supplied a test case.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.62%. Comparing base (a4b0a20) to head (31b427e).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #200   +/-   ##
=======================================
  Coverage   97.62%   97.62%           
=======================================
  Files          21       21           
  Lines        3237     3237           
=======================================
  Hits         3160     3160           
  Misses         77       77           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bvanelli]

Thanks for the PR! I saw this one last week but had to dig a bit deeper to find the cause on the TS source code.

The code uses the view v_transactions at https://github.com/actualbudget/actual/blob/6ead7ea42c35af8f04e50852fab4b2ebb822a57f/packages/loot-core/src/server/accounts/sync.ts#L780-L782.

The v_transactions calls v_transactions_internal_alive, that then calls v_transactions_internal, that is created like this:

CREATE VIEW v_transactions_internal_alive AS SELECT _.* FROM v_transactions_internal _
        LEFT JOIN transactions t2 ON (_.is_child = 1 AND t2.id = _.parent_id)
        WHERE IFNULL(_.tombstone, 0) = 0 AND (_.is_child = 0 OR t2.tombstone = 0);

In other words, this is a partial fix, but the true fix will probably also have to handle the tombstones correctly.

I also have noticed that the fuzzy matching isn't exactly like the code in https://github.com/actualbudget/actual/blob/6ead7ea42c35af8f04e50852fab4b2ebb822a57f/packages/loot-core/src/server/accounts/sync.ts#L704-L718:

• They also call v_transactions when matching imported_id, but we don't
• They have an option to reimportDeleted, we don't.

I believe this fits in a follow up to align the behaviour (and possible use the view too).

Otherwise LGTM!

[lwid]

Cool, sounds like there is room for future improvement. Will you take care of merging when you have a few minutes over?

[bvanelli]

Cool, sounds like there is room for future improvement. Will you take care of merging when you have a few minutes over?

You can merge it 😉 You should have the relevant permissions.

Just squash and place a relevant information on the squash commit.