github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-07-15 22:54:03 -05:00
Code Issues 3.0k Packages Projects Releases 58 Wiki Activity

[PR #8086] [MERGED] [AI] Skip unimportable Enable Banking transactions instead of failing the whole sync #127028

New Issue
Closed
opened 2026-06-14 06:32:32 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-06-14 06:32:32 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/8086
Author: @mheiland
Created: 6/4/2026
Status: ✅ Merged
Merged: 6/6/2026
Merged by: @MatissJanis

Base: master ← Head: fix/enablebanking-skip-unimportable-transactions

📝 Commits (6)

  • ad7b02a [AI] Skip unimportable Enable Banking transactions instead of failing the whole sync
  • a04b348 [AI] Add release note for #8086
  • 8775f0a [AI] Reject empty Enable Banking transaction amounts
  • 128e9d6 Update packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts
  • 67756b9 Update packages/sync-server/src/app-enablebanking/app-enablebanking.ts
  • e7fed10 [AI] Remove now-unused skippedCount counter

📊 Changes

7 files changed (+218 additions, -0 deletions)

View changed files

📝 packages/sync-server/src/app-enablebanking/app-enablebanking.ts (+15 -0)
📝 packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts (+20 -0)
📝 packages/sync-server/src/app-enablebanking/services/tests/fixtures.ts (+10 -0)
📝 packages/sync-server/src/app-enablebanking/services/tests/normalization.spec.ts (+50 -0)
📝 packages/sync-server/src/app-enablebanking/tests/poll-auth.spec.ts (+2 -0)
➕ packages/sync-server/src/app-enablebanking/tests/transactions.spec.ts (+115 -0)
➕ upcoming-release-notes/8086.md (+6 -0)

📄 Description

Description

Linking a bank via Enable Banking can fail the entire account sync with a
client-side SQLITE_ERROR ("Internal error: (1)"). loot-core throws while
inserting a fetched transaction (processBankSyncDownload → insertTransaction → convertForInsert), and because one bad record aborts the batch, no transactions
import for that account.

The trigger is a transaction the client can't store. The web client inserts each
transaction into a local SQLite database whose date is a required integer
derived from an ISO (YYYY-MM-DD) date and whose amount must be numeric. Enable
Banking occasionally returns a record that doesn't meet this — most commonly a
pending transaction with no booking_date, value_date, or
transaction_date, which normalizeTransaction maps to date: ''.

This adds an isImportableTransaction helper and uses it in the /transactions
handler to skip records that can't be imported (empty/non-ISO date or non-numeric
amount), logging how many were dropped, so the rest of the account imports. Normal
pending transactions (which carry a value_date/transaction_date) are
unaffected — they still import as cleared: false and reconcile via imported_id
when they post.

Related issue(s)

None found at time of writing.

Testing

  • Unit tests for isImportableTransaction (services/tests/normalization.spec.ts):
    accepts a normal booked transaction and a dated pending transaction; rejects a
    pending transaction with no date, a non-ISO date, and a non-numeric amount.
  • Handler test (tests/transactions.spec.ts): POST /transactions over a batch of
    [valid booked, valid pending, date-less pending] returns only the two importable
    transactions, correctly bucketed; the date-less record is dropped; an
    all-importable batch is returned unchanged.
  • Updated the existing pagination test (tests/poll-auth.spec.ts) whose mock
    transactions had no dates, so they stay importable under the new filter.
  • yarn typecheck, yarn lint, and the sync-server Enable Banking tests
    (106 tests) all pass.

Checklist

  • Release notes added
  • No obvious regressions in affected areas
  • Self-review has been performed

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/8086 **Author:** [@mheiland](https://github.com/mheiland) **Created:** 6/4/2026 **Status:** ✅ Merged **Merged:** 6/6/2026 **Merged by:** [@MatissJanis](https://github.com/MatissJanis) **Base:** `master` ← **Head:** `fix/enablebanking-skip-unimportable-transactions` --- ### 📝 Commits (6) - [`ad7b02a`](https://github.com/actualbudget/actual/commit/ad7b02ad05a875bb5d70de3fb9539c42f6a449b6) [AI] Skip unimportable Enable Banking transactions instead of failing the whole sync - [`a04b348`](https://github.com/actualbudget/actual/commit/a04b348cd0591fb7f9dbedbc0593bb66a2cc397c) [AI] Add release note for #8086 - [`8775f0a`](https://github.com/actualbudget/actual/commit/8775f0abc6f27a70430802f967e51f38ea5c816d) [AI] Reject empty Enable Banking transaction amounts - [`128e9d6`](https://github.com/actualbudget/actual/commit/128e9d674b4816982d569e8fc978f3b02f3c3702) Update packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts - [`67756b9`](https://github.com/actualbudget/actual/commit/67756b971eb475850c3ad48b3a164e09be6a57f1) Update packages/sync-server/src/app-enablebanking/app-enablebanking.ts - [`e7fed10`](https://github.com/actualbudget/actual/commit/e7fed10adaa68dfd86a1c0e247df572e8b8c341c) [AI] Remove now-unused skippedCount counter ### 📊 Changes **7 files changed** (+218 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `packages/sync-server/src/app-enablebanking/app-enablebanking.ts` (+15 -0) 📝 `packages/sync-server/src/app-enablebanking/services/enablebanking-service.ts` (+20 -0) 📝 `packages/sync-server/src/app-enablebanking/services/tests/fixtures.ts` (+10 -0) 📝 `packages/sync-server/src/app-enablebanking/services/tests/normalization.spec.ts` (+50 -0) 📝 `packages/sync-server/src/app-enablebanking/tests/poll-auth.spec.ts` (+2 -0) ➕ `packages/sync-server/src/app-enablebanking/tests/transactions.spec.ts` (+115 -0) ➕ `upcoming-release-notes/8086.md` (+6 -0) </details> ### 📄 Description ## Description Linking a bank via Enable Banking can fail the **entire** account sync with a client-side `SQLITE_ERROR` ("Internal error: (1)"). loot-core throws while inserting a fetched transaction (`processBankSyncDownload → insertTransaction → convertForInsert`), and because one bad record aborts the batch, no transactions import for that account. The trigger is a transaction the client can't store. The web client inserts each transaction into a local SQLite database whose `date` is a required integer derived from an ISO (`YYYY-MM-DD`) date and whose amount must be numeric. Enable Banking occasionally returns a record that doesn't meet this — most commonly a **pending transaction with no `booking_date`, `value_date`, or `transaction_date`**, which `normalizeTransaction` maps to `date: ''`. This adds an `isImportableTransaction` helper and uses it in the `/transactions` handler to skip records that can't be imported (empty/non-ISO date or non-numeric amount), logging how many were dropped, so the rest of the account imports. Normal pending transactions (which carry a `value_date`/`transaction_date`) are unaffected — they still import as `cleared: false` and reconcile via `imported_id` when they post. ## Related issue(s) _None found at time of writing._ ## Testing - Unit tests for `isImportableTransaction` (`services/tests/normalization.spec.ts`): accepts a normal booked transaction and a dated pending transaction; rejects a pending transaction with no date, a non-ISO date, and a non-numeric amount. - Handler test (`tests/transactions.spec.ts`): `POST /transactions` over a batch of [valid booked, valid pending, date-less pending] returns only the two importable transactions, correctly bucketed; the date-less record is dropped; an all-importable batch is returned unchanged. - Updated the existing pagination test (`tests/poll-auth.spec.ts`) whose mock transactions had no dates, so they stay importable under the new filter. - `yarn typecheck`, `yarn lint`, and the sync-server Enable Banking tests (106 tests) all pass. ## Checklist - [x] Release notes added - [x] No obvious regressions in affected areas - [x] Self-review has been performed --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-06-14 06:32:32 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
tech-support
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#127028
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?