From a5f754198016da5a2c62f39e4429581d456df1b9 Mon Sep 17 00:00:00 2001 From: Nikola Petrov Date: Fri, 2 Oct 2026 21:48:29 +0200 Subject: [PATCH] Name NLB uploads, delete statements, and stop importing on upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four changes to statement handling in the web app, made together and touching the same upload and statements-list code. Name NLB uploads by statement date. parser.Namer is an optional interface, like Warner, through which a parser names its statements; nlb reads the "Datum izpiska" from the izpisek header and names it izpisek_YYYY_MM_DD, lowercase, extension included -- ported from the rename_izpiski.py it replaces. Uploads are staged as dotfiles, invisible to import, so the parser can read them; two downloads of one statement then meet under one name and the second is recognised as already there, while a different statement of the same date is numbered _2 as the script did. Only uploads are named: source_files records statements by path, so renaming a file already in a folder would orphan its rows. Delete a statement from the statements list. The file is removed from disk for good -- the page says so before it asks -- and store.ForgetSourceFile drops its transactions and their transfer rows. A row two overlapping statements share is stored once, under the file imported first, so it goes too; the account's other statements forget their checksums and show as changed until the next Import re-reads them and restores it. A file already gone from disk can be forgotten. Upload and delete no longer import. Importing stays the user's call, made with the Import button, so a batch can be put together and looked over first. Delete still re-pairs transfers, which reads no statement. Show rows and new rows per statement. The list read "0" for a file whose rows an earlier, overlapping statement already held, which looked like a file that failed to parse. source_files now records how many transactions each statement holds, and the list reads "3 rows · 0 new". This adds a column the code reads, so an index built by an earlier version fails with "no such column: s.rows": delete index.db and import again. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 63 +++++--- README.md | 43 +++-- internal/importer/importer.go | 2 +- internal/parser/nlb.go | 22 +++ internal/parser/nlb_test.go | 12 ++ internal/parser/parser.go | 11 ++ internal/rules/rules_test.go | 2 +- internal/store/store.go | 72 ++++++++- internal/web/server.go | 295 ++++++++++++++++++++++++++-------- internal/web/server_test.go | 2 +- internal/web/static/app.js | 55 ++++--- internal/web/upload_test.go | 219 +++++++++++++++++++++++-- 12 files changed, 652 insertions(+), 146 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3aa7281..825e2ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,14 +143,14 @@ Anything new that reports a rule must report its file position too. settles ties: a saved rule cannot displace an equally specific one written by hand, while a narrower one is meant to take precedence and does. -**The report's period narrows the report and nothing else.** The screen opens -on last month, and `←`/`→` step along `report.Periods` — the named windows, -then the months the index holds. The report and the transaction list share a -scope (account, search, untagged), which `web.filterFrom` reads and -`state.filter` holds in the page; the period is read by the report handler -alone and kept in `state.report`, so opening on last month cannot hide the -rest of the index from the list. Nothing may put the period into that shared -scope, which is exactly what would make it leak. Covered by `TestReportPeriodLeavesTransactionsAlone`. +**The report's period narrows the report and nothing else.** The screen opens on +last month, and `←`/`→` step along `report.Periods` — the named windows, then +the months the index holds. The report and the transaction list share a scope +(account, search, untagged), which `web.filterFrom` reads and `state.filter` +holds in the page; the period is read by the report handler alone and kept in +`state.report`, so opening on last month cannot hide the rest of the index from +the list. Nothing may put the period into that shared scope, which is exactly +what would make it leak. Covered by `TestReportPeriodLeavesTransactionsAlone`. The axis is built from `store.Months` over the *whole* index, not from the rows in view, or it would grow and shrink as the account or search filter changed and @@ -241,13 +241,14 @@ That is why `/api/upload` takes files base64 inside JSON rather than as multipart: multipart is precisely what a cross-site form can send. **An upload only puts a file where `money import` looks.** It writes into an -existing account folder and then runs the same import as `/api/import`, so the -statements stay the source of truth and nothing reaches the index any other -way. It never overwrites a statement (identical contents are a no-op, different -ones a 409), refuses any name import would not read back — not a plain file -name, a dotfile, `account.toml`, outside the account's `include` — and checks a -whole batch before writing any of it. Covered by -`TestUploadNeverReplacesAStatement` and `TestUploadRefusesBadNames`. +existing account folder and stops there: importing stays the user's call, made +with the Import button, so the statements stay the source of truth and nothing +reaches the index any other way. It never overwrites a statement (identical +contents are a no-op, different ones a 409), refuses any name import would not +read back — not a plain file name, a dotfile, `account.toml`, outside the +account's `include` — and checks a whole batch before writing any of it. Covered +by `TestUploadSavesWithoutImporting`, `TestUploadNeverReplacesAStatement` and +`TestUploadRefusesBadNames`. **The statements list is read from the folders, not the index.** `/api/files` walks `importer.StatementFiles` — the same list import reads — and only then @@ -255,11 +256,23 @@ looks each file up in `store.SourceFiles`, comparing `importer.Checksum`, so a file is listed exactly when import would read it and a file the index remembers but the disk lost shows as `missing` instead of vanishing. `/api/files/{account}/{name}` serves a file only by finding it in that list, -never by joining the name onto a path. A statement is served from the app's origin, where a script could drive -the API, so nothing is ever rendered as a page: text is `text/plain` under -`CSP: sandbox`, anything not text or PDF is a sandboxed download, and PDFs — -whose viewers refuse a sandbox — open in the browser's own isolated viewer. -Covered by `TestServeFileServesOnlyStatements`. +never by joining the name onto a path. A statement is served from the app's +origin, where a script could drive the API, so nothing is ever rendered as a +page: text is `text/plain` under `CSP: sandbox`, anything not text or PDF is a +sandboxed download, and PDFs — whose viewers refuse a sandbox — open in the +browser's own isolated viewer. Covered by `TestServeFileServesOnlyStatements`. + +**Deleting a statement marks its neighbours to be re-read.** `/api/files/delete` +finds the file the same way `serveFile` does and deletes it — permanently, by +the user's choice; the page says so before it asks. `store.ForgetSourceFile` +then drops its transactions and their transfer rows, and `Link` re-pairs, which +reads no statement. It does not import, any more than an upload does. A +transaction two overlapping statements share is stored once, under the file +imported first, so it goes too; `ForgetSourceFile` therefore blanks the +checksums of the account's other statements, which show as `changed` until the +next Import re-reads them and restores what they hold. Without that the checksum +skip would leave those rows gone for good. Covered by +`TestRemoveFileKeepsWhatAnotherStatementHolds`. ## Adding a bank parser @@ -300,6 +313,16 @@ description — at the end, and not in the position it held on the page, so an IBAN wrapped across continuation lines stays contiguous for a glob to match. A new parser must do the same rather than reintroduce a structured field. +**A parser may name its statements, and only uploads use it.** `parser.Namer` +is optional, like `parser.Warner`: `nlb` names an izpisek `izpisek_YYYY_MM_DD` +after its *Datum izpiska*, ported from the `rename_izpiski.py` it replaced. +`/api/upload` stages each file as a dotfile — invisible to import — so the +parser can read it, then numbers a chosen name that is taken by different +contents (`_2`, as the script did) where a name the user gave is refused with +409. Nothing renames a file already in a folder: `source_files` records +statements by path, so a rename would orphan its rows as `missing` until the +index is rebuilt. Covered by `TestUploadNamesStatementsTheParserCanName`. + ## Verifying ``` diff --git a/README.md b/README.md index c1aca71..8835548 100644 --- a/README.md +++ b/README.md @@ -102,8 +102,7 @@ money config # which data root is in use, and why An account folder appears in the app only once its statements have been imported — creating an `account.toml` is not enough on its own. Run -`money import` (or press Import in the web app) after adding one, or add its -first statements from the Accounts screen, which imports them as it saves them. +`money import` (or press Import in the web app) after adding one. `--uniq` turns `ls` into a list of patterns still to write rather than a list of rows to read: one line per distinct description, since fifty visits to the same @@ -178,21 +177,35 @@ folder, with what the index made of it: | `•` not imported | new since the last import — or an import tried and failed, in which case the error is in the import report | | `✗` gone from disk | imported once, then removed; its transactions stay in the index only until it is rebuilt | -**Added** is how many transactions the file brought in. Statements that overlap -share rows, and a shared row counts towards whichever file was imported first, -so a later statement covering the same days can add fewer rows than it holds. +**Transactions** reads like `3 rows · 0 new`: how many transactions the file +holds, then how many of them it was the first to bring in. Statements that +overlap share rows, and a shared row is stored once, under whichever file was +imported first — so a later statement covering the same days shows fewer new +rows than it holds, down to `0 new` when an earlier one already had them all. +That is deduplication working, not a file that failed to parse. Click a file name to open it. PDFs and CSVs open in the browser; anything else downloads. Only files import would read are listed or served — not `account.toml`, dotfiles, or anything outside an account's `include` patterns. +**Delete** removes a statement from disk for good — there is no copy kept +and no undo, so keep the bank's original if you might want it back. Its +transactions leave the index, and transfers that used them lose their pairing: +the other leg shows as unpaired, which is the truth once one side is gone. +Deleting does not import. A row that an overlapping statement also contains goes +too, for now: the account's other statements are marked *changed*, and the next +**Import** re-reads them and puts it back. For a file already gone from disk the +button says **Forget** and only clears the index. + ### Adding statements from the browser The Accounts screen (`1`) has an **Add statements** panel: pick the account, -drop files on it (or click to choose them) and press **Upload and import**. The -files are saved into that account's folder, exactly where you would have copied -them by hand, and then imported — the folder stays the source of truth, so -deleting `index.db` and re-importing still gets everything back. +drop files on it (or click to choose them) and press **Upload**. The files are +saved into that account's folder, exactly where you would have copied them by +hand — the folder stays the source of truth, so deleting `index.db` and +re-importing still gets everything back. Uploading does not import: the files +wait on the statements list as *not imported* until you press **Import**, so you +can put a batch together and look it over first. An upload never replaces a statement. A file whose name is already in the folder is skipped if its contents are identical and refused if they differ; rename it @@ -202,6 +215,16 @@ too, and a batch with one bad file writes none of them. The folder itself must already exist with an `account.toml`: an upload adds statements to an account, it does not create one. +Some banks name their downloads unhelpfully, so a parser may name the statement +instead. An NLB izpisek is saved as `izpisek_YYYY_MM_DD.pdf` after the *Datum +izpiska* in its header — lowercase throughout, extension included — so the +folder sorts by date and the status line says what each file became. Downloading +the same statement twice then lands on the same name and is recognised as +already there; a different statement with the same date (a reissue) is numbered +`_2`, `_3` rather than refused. A statement with no date it can find keeps its +own name. Only uploads are named — files already in a folder are never renamed, +since the index records them by path. + ### Keys | Key | Action | @@ -654,7 +677,7 @@ nothing else to configure — `parser` names one of these and that is all. | `parser` | Statement | Notes | | --- | --- | --- | -| `nlb` | NLB izpisek PDF | Wrapped descriptions are folded in from continuation lines, and the IBAN column is appended to the description. | +| `nlb` | NLB izpisek PDF | Wrapped descriptions are folded in from continuation lines, and the IBAN column is appended to the description. Uploads are saved as `izpisek_YYYY_MM_DD.pdf` after the statement date. | | `traderepublic` | Trade Republic PDF | Handles both the single-line and the stacked layout by measuring column positions. | | `revolut` | `account-statement*.csv` | Skips non-COMPLETED rows, folds the fee into the amount. | diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 30330b6..e1a5494 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -162,7 +162,7 @@ func importFile(root string, db *store.DB, acc *config.Account, accountID int64, } fr.Warnings = append(fr.Warnings, checkBalances(txns, acc.Digits())...) - sourceID, err := db.SourceFile(accountID, rel, sum, time.Now().UTC().Format(time.RFC3339)) + sourceID, err := db.SourceFile(accountID, rel, sum, time.Now().UTC().Format(time.RFC3339), len(txns)) if err != nil { fr.Err = err return fr diff --git a/internal/parser/nlb.go b/internal/parser/nlb.go index 047882d..b727b14 100644 --- a/internal/parser/nlb.go +++ b/internal/parser/nlb.go @@ -43,6 +43,10 @@ var ( nlbAccount = regexp.MustCompile(`^SI\d{2}(?:\s?\d{4}){3}\s?\d{3}$`) ) +// nlbStatementDate is the issue date in an izpisek's header. It names the +// statement: NLB's downloads are not named for what they hold. +var nlbStatementDate = regexp.MustCompile(`Datum izpiska\s+(\d{2})\.(\d{2})\.(\d{4})`) + // nlbMinContinuationIndent is the shallowest indent a wrapped description line // may have. The real threshold is the description column of the transaction // the line belongs to, measured per line rather than hardcoded: pdftotext @@ -61,6 +65,24 @@ func (p *nlbParser) Parse(path string, acc *config.Account) ([]RawTxn, error) { return parseNLBText(text, p.digits) } +// StatementName names an izpisek after its statement date, izpisek_YYYY_MM_DD, +// so the folder sorts by date and two downloads of one statement collide. +func (p *nlbParser) StatementName(path string) (string, error) { + text, err := pdfToText(path) + if err != nil { + return "", err + } + return nlbStatementName(text), nil +} + +func nlbStatementName(text string) string { + m := nlbStatementDate.FindStringSubmatch(text) + if m == nil { + return "" + } + return fmt.Sprintf("izpisek_%s_%s_%s", m[3], m[2], m[1]) +} + // parseNLBText holds the whole parser, separated from PDF extraction so it can // be tested against captured pdftotext output. func parseNLBText(text string, digits int) ([]RawTxn, error) { diff --git a/internal/parser/nlb_test.go b/internal/parser/nlb_test.go index aa86f7f..8a2aa56 100644 --- a/internal/parser/nlb_test.go +++ b/internal/parser/nlb_test.go @@ -112,3 +112,15 @@ func TestParseNLBRejectsMalformedLine(t *testing.T) { t.Error("expected an error for a transaction line with no amount") } } + +// An izpisek is named after its statement date, as rename_izpiski.py did. +func TestNLBStatementName(t *testing.T) { + header := " IZPISEK 001/2026\n" + + " Datum izpiska 31.01.2026 Stran 1\n" + if got := nlbStatementName(header + nlbPage); got != "izpisek_2026_01_31" { + t.Errorf("name = %q, want izpisek_2026_01_31", got) + } + if got := nlbStatementName(nlbPage); got != "" { + t.Errorf("name without a statement date = %q, want none", got) + } +} diff --git a/internal/parser/parser.go b/internal/parser/parser.go index 1b8d980..9084551 100644 --- a/internal/parser/parser.go +++ b/internal/parser/parser.go @@ -43,6 +43,17 @@ type Warner interface { Warnings() []string } +// Namer is an optional interface for parsers whose bank names its downloads +// unhelpfully. StatementName reads the statement at path and returns the name, +// without extension, it should be stored under — the upload's extension is +// kept, lowercased — or "" when the statement does not say, in which case it +// keeps the name it came with. Only an upload uses +// it; files already in a folder are never renamed, since the index records +// them by path. +type Namer interface { + StatementName(path string) (string, error) +} + // Factory builds a parser from an account's config, validating it up front so // a bad account.toml fails before any file is read. type Factory func(acc *config.Account) (Parser, error) diff --git a/internal/rules/rules_test.go b/internal/rules/rules_test.go index d0e3408..9433aaf 100644 --- a/internal/rules/rules_test.go +++ b/internal/rules/rules_test.go @@ -28,7 +28,7 @@ func seed(t *testing.T, db *store.DB, descriptions ...string) int64 { if err != nil { t.Fatal(err) } - sourceID, err := db.SourceFile(accountID, "checking/st.csv", "sha", "2026-01-01T00:00:00Z") + sourceID, err := db.SourceFile(accountID, "checking/st.csv", "sha", "2026-01-01T00:00:00Z", 0) if err != nil { t.Fatal(err) } diff --git a/internal/store/store.go b/internal/store/store.go index b7bb96d..7002ab5 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -37,6 +37,9 @@ CREATE TABLE IF NOT EXISTS source_files ( path TEXT NOT NULL, sha256 TEXT NOT NULL, imported_at TEXT NOT NULL, + -- How many transactions the statement holds, as parsed. Not how many it + -- brought in: rows an overlapping statement already had are deduplicated. + rows INTEGER NOT NULL DEFAULT 0, UNIQUE(account_id, path) ); @@ -130,15 +133,17 @@ func (d *DB) Accounts() ([]model.Account, error) { return out, rows.Err() } -// SourceFile records that a statement file was imported, returning its id. -func (d *DB) SourceFile(accountID int64, path, sha, importedAt string) (int64, error) { +// SourceFile records that a statement file was imported, holding rows +// transactions, and returns its id. +func (d *DB) SourceFile(accountID int64, path, sha, importedAt string, rows int) (int64, error) { _, err := d.sql.Exec(` - INSERT INTO source_files (account_id, path, sha256, imported_at) - VALUES (?, ?, ?, ?) + INSERT INTO source_files (account_id, path, sha256, imported_at, rows) + VALUES (?, ?, ?, ?, ?) ON CONFLICT(account_id, path) DO UPDATE SET sha256 = excluded.sha256, - imported_at = excluded.imported_at`, - accountID, path, sha, importedAt) + imported_at = excluded.imported_at, + rows = excluded.rows`, + accountID, path, sha, importedAt, rows) if err != nil { return 0, fmt.Errorf("record source file %s: %w", path, err) } @@ -399,6 +404,8 @@ type SourceFileInfo struct { Path string // relative to the data root, as the importer records it SHA256 string ImportedAt string + // Rows is how many transactions the statement holds, as parsed. + Rows int // Added counts the transactions this file introduced. A row seen again in // an overlapping statement is deduplicated and stays with the file that // brought it first, so this is not how many rows the file holds. @@ -408,7 +415,7 @@ type SourceFileInfo struct { // SourceFiles lists every statement the index has imported. func (d *DB) SourceFiles() ([]SourceFileInfo, error) { rows, err := d.sql.Query(` - SELECT a.slug, s.path, s.sha256, s.imported_at, COUNT(t.id) + SELECT a.slug, s.path, s.sha256, s.imported_at, s.rows, COUNT(t.id) FROM source_files s JOIN accounts a ON a.id = s.account_id LEFT JOIN transactions t ON t.source_file_id = s.id @@ -421,7 +428,7 @@ func (d *DB) SourceFiles() ([]SourceFileInfo, error) { var out []SourceFileInfo for rows.Next() { var f SourceFileInfo - if err := rows.Scan(&f.AccountSlug, &f.Path, &f.SHA256, &f.ImportedAt, &f.Added); err != nil { + if err := rows.Scan(&f.AccountSlug, &f.Path, &f.SHA256, &f.ImportedAt, &f.Rows, &f.Added); err != nil { return nil, err } out = append(out, f) @@ -429,6 +436,55 @@ func (d *DB) SourceFiles() ([]SourceFileInfo, error) { return out, rows.Err() } +// ForgetSourceFile removes a statement from the index: its transactions, any +// transfer pairing that used them, and the record of it, returning how many +// transactions went. Pairing is derived state that the next Link rewrites, so +// dropping a pair here loses nothing. +// +// A transaction two overlapping statements share is stored once, under the +// file that brought it first, so removing that file can take rows another +// statement still holds. The account's other statements therefore forget +// their checksums, and the next import re-reads them and restores those rows. +func (d *DB) ForgetSourceFile(accountSlug, path string) (int, error) { + tx, err := d.sql.Begin() + if err != nil { + return 0, err + } + defer tx.Rollback() + + var id, accountID int64 + err = tx.QueryRow(` + SELECT s.id, s.account_id FROM source_files s JOIN accounts a ON a.id = s.account_id + WHERE a.slug = ? AND s.path = ?`, accountSlug, path).Scan(&id, &accountID) + if err == sql.ErrNoRows { + return 0, nil + } + if err != nil { + return 0, fmt.Errorf("find source file %s: %w", path, err) + } + if _, err := tx.Exec(` + DELETE FROM transfers WHERE + out_txn_id IN (SELECT id FROM transactions WHERE source_file_id = ?) OR + in_txn_id IN (SELECT id FROM transactions WHERE source_file_id = ?)`, id, id); err != nil { + return 0, fmt.Errorf("forget transfers from %s: %w", path, err) + } + res, err := tx.Exec(`DELETE FROM transactions WHERE source_file_id = ?`, id) + if err != nil { + return 0, fmt.Errorf("forget transactions from %s: %w", path, err) + } + n, err := res.RowsAffected() + if err != nil { + return 0, err + } + if _, err := tx.Exec(`DELETE FROM source_files WHERE id = ?`, id); err != nil { + return 0, fmt.Errorf("forget %s: %w", path, err) + } + if _, err := tx.Exec(`UPDATE source_files SET sha256 = '' WHERE account_id = ?`, accountID); err != nil { + return 0, fmt.Errorf("mark %s's statements for re-reading: %w", accountSlug, err) + } + return int(n), tx.Commit() +} + // Tags lists every tag in use, for completion in the rule builder. func (d *DB) Tags() ([]string, error) { rows, err := d.sql.Query(` diff --git a/internal/web/server.go b/internal/web/server.go index aef35ac..c5e121f 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -27,6 +27,7 @@ import ( "git.petrovv.com/nikola/money/internal/glob" "git.petrovv.com/nikola/money/internal/importer" "git.petrovv.com/nikola/money/internal/model" + "git.petrovv.com/nikola/money/internal/parser" "git.petrovv.com/nikola/money/internal/report" "git.petrovv.com/nikola/money/internal/rules" "git.petrovv.com/nikola/money/internal/store" @@ -91,6 +92,7 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("POST /api/upload", s.write(s.upload)) mux.HandleFunc("GET /api/files", s.read(s.fileList)) mux.HandleFunc("GET /api/files/{account}/{name}", s.serveFile) + mux.HandleFunc("POST /api/files/delete", s.write(s.removeFile)) mux.HandleFunc("POST /api/retag", s.write(s.retag)) return mux } @@ -1211,26 +1213,22 @@ func (s *Server) runImport(r *http.Request) (any, error) { if err := decode(r, &req); err != nil { return nil, err } - return s.importAll(req.Force) -} - -func (s *Server) importAll(force bool) (importJSON, error) { accounts, err := config.LoadAccounts(s.root) if err != nil { - return importJSON{}, err + return nil, err } if len(accounts) == 0 { - return importJSON{}, badRequest("no accounts found in %s (an account is a folder containing %s)", + return nil, badRequest("no accounts found in %s (an account is a folder containing %s)", s.root, config.AccountFile) } if err := s.reloadRules(); err != nil { - return importJSON{}, err + return nil, err } s.accounts = accounts - res, err := importer.Run(s.root, s.db, s.accounts, s.engine, s.links, importer.Options{Force: force}) + res, err := importer.Run(s.root, s.db, s.accounts, s.engine, s.links, importer.Options{Force: req.Force}) if err != nil { - return importJSON{}, err + return nil, err } _, added, skipped := res.Total() @@ -1269,10 +1267,11 @@ type uploadReq struct { Files []uploadFile `json:"files"` } -// upload saves statements into an account folder and imports them. The files -// on disk are the source of truth and the index is derived from them, so an -// upload is nothing more than putting a file where `money import` looks; the -// import that follows is the one the Import button runs. +// upload saves statements into an account folder. The files on disk are the +// source of truth and the index is derived from them, so an upload is nothing +// more than putting a file where `money import` looks. It does not import: +// that stays the user's call, made with the Import button, so a batch can be +// put together and checked on the statements list before it is read. // // Files arrive base64 inside JSON rather than as multipart: a multipart body // is exactly what a cross-site form can send, and the JSON-only rule is the @@ -1300,60 +1299,133 @@ func (s *Server) upload(r *http.Request) (any, error) { req.Account, s.root, config.AccountFile) } - // Everything is checked before anything is written, so a bad file in a - // batch leaves the folder as it was rather than half uploaded. - var write []uploadFile - var same []string - seen := map[string]bool{} - for _, f := range req.Files { - if err := checkStatementName(acc, f.Name); err != nil { - return nil, err - } - if seen[f.Name] { - return nil, badRequest("%s is in the upload twice", f.Name) - } - seen[f.Name] = true - existing, err := os.ReadFile(filepath.Join(acc.Dir, f.Name)) - switch { - case err == nil && bytes.Equal(existing, f.Data): - same = append(same, f.Name) - case err == nil: - // A statement is the source of truth for what it already - // imported; replacing it under the same name is not an upload's - // decision to make. - return nil, &apiError{http.StatusConflict, fmt.Sprintf( - "%s/%s already exists with different contents; rename the file or remove the old one first", - acc.Slug, f.Name)} - case !errors.Is(err, fs.ErrNotExist): - return nil, err - default: - write = append(write, f) - } + p, err := parser.For(acc) + if err != nil { + return nil, badRequest("%v", err) } - for _, f := range write { - if err := writeStatement(acc.Dir, f); err != nil { + namer, _ := p.(parser.Namer) + + // Each file is staged as a dotfile — invisible to import — so a parser + // that names its statements can read it. Everything is then checked + // before anything is moved into place, so a bad file in a batch leaves + // the folder as it was rather than half uploaded. + type staged struct { + orig, name, tmp string + data []byte + named bool // the parser chose name, from the statement + } + var batch []*staged + defer func() { + for _, f := range batch { + if f.tmp != "" { + os.Remove(f.tmp) + } + } + }() + for _, f := range req.Files { + if err := checkPlainName(f.Name); err != nil { return nil, err } + tmp, err := stageStatement(acc.Dir, f.Data) + if err != nil { + return nil, err + } + st := &staged{orig: f.Name, name: f.Name, tmp: tmp, data: f.Data} + batch = append(batch, st) + if namer != nil { + stem, err := namer.StatementName(tmp) + if err != nil { + return nil, badRequest("%s: %v", f.Name, err) + } + if stem != "" { + st.name, st.named = stem+strings.ToLower(filepath.Ext(f.Name)), true + } + } } - out, err := s.importAll(false) - if err != nil { - return nil, err + var write []*staged + var same, renamed []string + claimed := map[string][]byte{} + for _, f := range batch { + base, n := f.name, 2 + resolve: + for { + if err := checkStatementName(acc, f.name); err != nil { + return nil, err + } + existing, inBatch := claimed[f.name] + var err error + if !inBatch { + existing, err = os.ReadFile(filepath.Join(acc.Dir, f.name)) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + } + switch { + case !inBatch && err != nil: // free + claimed[f.name] = f.data + write = append(write, f) + break resolve + case bytes.Equal(existing, f.data): + same = append(same, f.orig) + break resolve + case f.named: + // A name the parser chose can be taken by a different + // statement of the same date — a reissue — so it is + // numbered, as rename_izpiski.py did, rather than refused. + f.name = fmt.Sprintf("%s_%d%s", strings.TrimSuffix(base, filepath.Ext(base)), n, filepath.Ext(base)) + n++ + case inBatch: + return nil, badRequest("%s is in the upload twice", f.orig) + default: + // A statement is the source of truth for what it already + // imported; replacing it under the same name is not an + // upload's decision to make. + return nil, &apiError{http.StatusConflict, fmt.Sprintf( + "%s/%s already exists with different contents; rename the file or remove the old one first", + acc.Slug, f.name)} + } + } } + for _, f := range write { + path := filepath.Join(acc.Dir, f.name) + if err := os.Rename(f.tmp, path); err != nil { + return nil, fmt.Errorf("write %s: %w", path, err) + } + f.tmp = "" + if f.name != f.orig { + renamed = append(renamed, f.orig+" → "+f.name) + } + } + msg := fmt.Sprintf("uploaded %d file(s) to %s", len(write), acc.Slug) if len(same) > 0 { msg += fmt.Sprintf(" (%d already there)", len(same)) } - out.Status = msg + " · " + out.Status - return out, nil + if len(renamed) > 0 { + msg += ", named by statement date: " + strings.Join(renamed, ", ") + } + if len(write) > 0 { + msg += " · press Import to read them" + } + return status{msg}, nil +} + +// checkPlainName refuses anything that is not a plain file name, before the +// file is so much as staged. +func checkPlainName(name string) error { + if name == "" || name != filepath.Base(name) || strings.ContainsAny(name, "/\\\x00") || + name == "." || name == ".." { + return badRequest("%q is not a plain file name", name) + } + return nil } // checkStatementName refuses any name the importer would not read back as a // statement of this account, and anything that is not a plain file name. func checkStatementName(acc *config.Account, name string) error { - if name == "" || name != filepath.Base(name) || strings.ContainsAny(name, "/\\\x00") || - name == "." || name == ".." { - return badRequest("%q is not a plain file name", name) + if err := checkPlainName(name); err != nil { + return err } if strings.HasPrefix(name, ".") || name == config.AccountFile { return badRequest("%s would be ignored by import (dotfiles and %s are not statements)", @@ -1366,29 +1438,28 @@ func checkStatementName(acc *config.Account, name string) error { return nil } -// writeStatement writes through a dotfile and renames it into place, so a +// stageStatement writes an upload to a dotfile in the account folder. Import +// skips dotfiles, so it is invisible until renamed into place — and a // concurrent `money import` never reads half a statement. -func writeStatement(dir string, f uploadFile) error { - path := filepath.Join(dir, f.Name) +func stageStatement(dir string, data []byte) (string, error) { tmp, err := os.CreateTemp(dir, ".upload-*") if err != nil { - return fmt.Errorf("write %s: %w", path, err) + return "", fmt.Errorf("write to %s: %w", dir, err) } - defer os.Remove(tmp.Name()) // a no-op once renamed - if _, err := tmp.Write(f.Data); err != nil { + if _, err := tmp.Write(data); err != nil { tmp.Close() - return fmt.Errorf("write %s: %w", path, err) + os.Remove(tmp.Name()) + return "", fmt.Errorf("write %s: %w", tmp.Name(), err) } if err := tmp.Close(); err != nil { - return fmt.Errorf("write %s: %w", path, err) + os.Remove(tmp.Name()) + return "", fmt.Errorf("write %s: %w", tmp.Name(), err) } if err := os.Chmod(tmp.Name(), 0o644); err != nil { - return err + os.Remove(tmp.Name()) + return "", err } - if err := os.Rename(tmp.Name(), path); err != nil { - return fmt.Errorf("write %s: %w", path, err) - } - return nil + return tmp.Name(), nil } type fileRow struct { @@ -1404,7 +1475,11 @@ type fileRow struct { // is rebuilt). Status string `json:"status"` ImportedAt string `json:"importedAt"` - Added int `json:"added"` + // Rows is how many transactions the statement holds; Added how many of + // them it was the first to bring in, which overlap with another statement + // can make fewer. + Rows int `json:"rows"` + Added int `json:"added"` } // fileList is every statement on disk next to what the index recorded about @@ -1443,7 +1518,7 @@ func (s *Server) fileList(*http.Request) (any, error) { row.Size, row.Modified = info.Size(), info.ModTime().Format("2006-01-02 15:04") } if rec, ok := byPath[rel]; ok { - row.ImportedAt, row.Added, row.Status = rec.ImportedAt, rec.Added, "imported" + row.ImportedAt, row.Rows, row.Added, row.Status = rec.ImportedAt, rec.Rows, rec.Added, "imported" if sum, err := importer.Checksum(path); err != nil { return nil, err } else if sum != rec.SHA256 { @@ -1457,7 +1532,7 @@ func (s *Server) fileList(*http.Request) (any, error) { if !onDisk[rec.Path] { out = append(out, fileRow{ Account: rec.AccountSlug, Name: filepath.Base(rec.Path), Status: "missing", - ImportedAt: rec.ImportedAt, Added: rec.Added, + ImportedAt: rec.ImportedAt, Rows: rec.Rows, Added: rec.Added, }) } } @@ -1532,6 +1607,90 @@ func (s *Server) serveFile(w http.ResponseWriter, r *http.Request) { http.ServeFile(w, r, path) } +type removeFileReq struct { + Account string `json:"account"` + Name string `json:"name"` +} + +// removeFile takes a statement out of the data: the file is deleted and its +// transactions leave the index. A file already gone from disk is only +// forgotten by the index. The deletion is permanent — the page says so before +// asking — so nothing here keeps a copy. +// +// It does not import. Rows the file shared with an overlapping statement went +// with it, and ForgetSourceFile marks the account's other statements so the +// next Import re-reads them and restores those rows; until then they show as +// changed. Pairing is re-derived here, though: it reads no statement, and a +// leg whose partner just went should not stay counted as a transfer. +func (s *Server) removeFile(r *http.Request) (any, error) { + var req removeFileReq + if err := decode(r, &req); err != nil { + return nil, err + } + if err := checkPlainName(req.Name); err != nil { + return nil, err + } + rel := filepath.Join(req.Account, req.Name) + + // The file is found in the account's statement list, never by joining the + // request onto a path, exactly as serveFile does. + var path string + accounts, err := config.LoadAccounts(s.root) + if err != nil { + return nil, err + } + for _, acc := range accounts { + if acc.Slug != req.Account { + continue + } + paths, err := importer.StatementFiles(acc) + if err != nil { + return nil, err + } + for _, p := range paths { + if filepath.Base(p) == req.Name { + path = p + } + } + } + + if path != "" { + if err := os.Remove(path); err != nil { + return nil, fmt.Errorf("remove %s: %w", path, err) + } + } + dropped, err := s.db.ForgetSourceFile(req.Account, rel) + if err != nil { + return nil, err + } + if path == "" && dropped == 0 { + known, err := s.db.SourceFiles() + if err != nil { + return nil, err + } + if !slices.ContainsFunc(known, func(f store.SourceFileInfo) bool { return f.Path == rel }) { + return nil, &apiError{http.StatusNotFound, fmt.Sprintf("no statement %s", rel)} + } + } + + _, unpaired, err := s.links.Link(s.db) + if err != nil { + return nil, err + } + verb := "deleted" + if path == "" { + verb = "forgot" + } + msg := fmt.Sprintf("%s %s, %d transaction(s) dropped", verb, rel, dropped) + if unpaired > 0 { + msg += fmt.Sprintf(" · %d transfer leg(s) unpaired", unpaired) + } + if dropped > 0 { + msg += " · press Import to restore any of them another statement also holds" + } + return status{msg}, nil +} + // retag is `money retag`: both halves of what rules.toml decides, re-derived // from the file as it is on disk now. func (s *Server) retag(*http.Request) (any, error) { diff --git a/internal/web/server_test.go b/internal/web/server_test.go index 2d20d95..8532d65 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -44,7 +44,7 @@ func newTestServer(t *testing.T, rulesToml string, txns ...fixtureTxn) (*Server, if err != nil { t.Fatal(err) } - src, err := db.SourceFile(id, slug+"/st.csv", "sha", "2026-01-01T00:00:00Z") + src, err := db.SourceFile(id, slug+"/st.csv", "sha", "2026-01-01T00:00:00Z", 0) if err != nil { t.Fatal(err) } diff --git a/internal/web/static/app.js b/internal/web/static/app.js index 442dad0..2cadc70 100644 --- a/internal/web/static/app.js +++ b/internal/web/static/app.js @@ -205,31 +205,22 @@ async function refreshAll() { // --- import and retag ----------------------------------------------------- -function runImport(force) { - return importing(() => api('POST', '/api/import', { force: !!force })); -} - -// importing runs a request that ends in an import — the Import button, or an -// upload — and reports it the same way. It answers whether the request -// succeeded; per-file failures inside the import still count as success. -async function importing(request) { - if (state.busy) return false; +async function runImport(force) { + if (state.busy) return; state.busy = true; const btn = document.getElementById('import'); btn.disabled = true; btn.textContent = 'Importing…'; setStatus('importing… parsing PDF statements can take a while'); try { - const res = await request(); + const res = await api('POST', '/api/import', { force: !!force }); state.status = res.status; state.error = res.failed ? `${res.failed} file(s) failed to import` : ''; renderBanners(); showImportDetails(res); await refreshAll(); - return true; } catch (e) { setError(e); - return false; } finally { state.busy = false; btn.disabled = false; @@ -317,8 +308,8 @@ function fileList(list) { panel.append(h('table', {}, h('thead', {}, h('tr', {}, h('th', {}, 'Account'), h('th', {}, 'File'), h('th', { class: 'num' }, 'Size'), h('th', {}, 'Modified'), h('th', {}, ''), h('th', {}, 'Status'), - h('th', { class: 'num', title: 'Transactions this file brought into the index; rows already brought by an overlapping statement count there' }, 'Added'), - h('th', {}, 'Imported'))), + h('th', { class: 'num', title: 'Transactions the file holds, and how many of them it was the first to bring in — an overlapping statement imported earlier already holds the rest' }, 'Transactions'), + h('th', {}, 'Imported'), h('th', {}, ''))), h('tbody', {}, list.map((f) => { const [mark, cls, text] = FILE_STATUS[f.status] || ['', '', f.status]; return h('tr', {}, @@ -331,8 +322,11 @@ function fileList(list) { h('td', { class: 'muted' }, f.modified), h('td', { class: 'mark ' + cls }, mark), h('td', { class: cls || 'muted' }, text), - h('td', { class: 'num' }, f.status === 'new' ? '' : f.added), - h('td', { class: 'muted' }, localTime(f.importedAt))); + h('td', { class: 'num' }, f.status === 'new' ? '' : `${f.rows} ${f.rows === 1 ? 'row' : 'rows'} · ${f.added} new`), + h('td', { class: 'muted' }, localTime(f.importedAt)), + h('td', { class: 'actions' }, h('button', { + type: 'button', class: 'danger', onclick: () => removeFile(f), + }, f.status === 'missing' ? 'Forget' : 'Delete'))); })))); return h('div', {}, h('h3', { class: 'section' }, 'Statements ', h('span', { class: 'sub' }, `· ${list.length} file(s)` + (pending ? ` · ${pending} need attention` : ''))), panel); @@ -348,6 +342,22 @@ function localTime(iso) { return `${d.getFullYear()}-${p(d.getMonth() + 1)}-${p(d.getDate())} ${p(d.getHours())}:${p(d.getMinutes())}`; } +// removeFile deletes a statement for good. It does not import: rows another +// statement also holds come back at the next Import, so the question asked +// here is only about what this file alone brought in. +function removeFile(f) { + const where = `${f.account}/${f.name}`; + const rows = f.status === 'new' ? 'It has not been imported, so no transactions change.' + : `The ${f.added} transaction(s) it brought in leave the index; any another statement also holds come back at the next Import.`; + const question = f.status === 'missing' + ? `Forget ${where}? The file is already gone from disk. ${rows}` + : `Delete ${where} permanently? The file is removed from disk and cannot be restored from here. ${rows}`; + if (!confirm(question)) return; + api('POST', '/api/files/delete', { account: f.account, name: f.name }) + .then((res) => { setStatus(res.status); return refreshAll(); }) + .catch(setError); +} + function formatSize(bytes) { if (bytes < 1024) return `${bytes} B`; if (bytes < 1024 * 1024) return `${Math.round(bytes / 1024)} KB`; @@ -377,7 +387,7 @@ function uploader(o) { onchange: (e) => choose(e.target.files), }); const chosen = h('div', { class: 'chosen muted' }, 'No files chosen'); - const send = h('button', { type: 'button', class: 'primary', disabled: true, onclick: submit }, 'Upload and import'); + const send = h('button', { type: 'button', class: 'primary', disabled: true, onclick: submit }, 'Upload'); const zone = h('div', { class: 'dropzone', tabindex: 0, @@ -404,10 +414,11 @@ function uploader(o) { account: account.value, files: await Promise.all(files.map(async (f) => ({ name: f.name, data: await base64(f) }))), }; - if (await importing(() => api('POST', '/api/upload', payload))) { - picker.value = ''; - choose([]); - } + const res = await api('POST', '/api/upload', payload); + picker.value = ''; + choose([]); + setStatus(res.status); + await refreshAll(); } catch (e) { setError(e); } finally { @@ -419,7 +430,7 @@ function uploader(o) { h('div', { class: 'toolbar' }, h('label', {}, 'Account ', account), send), zone, picker, chosen, h('div', { class: 'hint muted' }, - 'Files are saved into the account folder, next to the statements already there, and imported. ' + + 'Files are saved into the account folder, next to the statements already there; press Import to read them. ' + 'A file with the same name and different contents is refused rather than replaced.')); return panel; } diff --git a/internal/web/upload_test.go b/internal/web/upload_test.go index d25a905..8642ad0 100644 --- a/internal/web/upload_test.go +++ b/internal/web/upload_test.go @@ -1,7 +1,9 @@ package web import ( + "errors" "fmt" + "io/fs" "net/http" "net/http/httptest" "os" @@ -23,10 +25,30 @@ func init() { parser.Register("webtest", func(acc *config.Account) (parser.Parser, error) { return csvParser{digits: acc.Digits()}, nil }) + parser.Register("webnamed", func(acc *config.Account) (parser.Parser, error) { + return namingParser{csvParser{digits: acc.Digits()}}, nil + }) } type csvParser struct{ digits int } +// namingParser is csvParser for a bank whose downloads are named unhelpfully: +// it names a statement after its first date, as nlb does after the statement +// date, and says nothing for a statement with no rows. +type namingParser struct{ csvParser } + +func (namingParser) StatementName(path string) (string, error) { + body, err := os.ReadFile(path) + if err != nil { + return "", err + } + lines := strings.Split(strings.TrimSpace(string(body)), "\n") + if len(lines) < 2 { + return "", nil + } + return "stmt_" + strings.Split(lines[1], ",")[0], nil +} + func (p csvParser) Parse(path string, _ *config.Account) ([]parser.RawTxn, error) { body, err := os.ReadFile(path) if err != nil { @@ -81,32 +103,38 @@ const checkingTOML = "currency = \"EUR\"\nparser = \"webtest\"\n" const statement = "date,description,amount\n2026-02-01,LIDL SOFIA,-12.50\n2026-02-03,SALARY,1000.00\n" -func TestUploadSavesAndImports(t *testing.T) { +// An upload only saves: the file waits on the statements list as new until +// the user presses Import. +func TestUploadSavesWithoutImporting(t *testing.T) { dir, h := newUploadServer(t, checkingTOML) req := uploadReq{Account: "checking", Files: []uploadFile{{Name: "2026-02.csv", Data: []byte(statement)}}} - var res importJSON + var res status call(t, h, "POST", "/api/upload", req, http.StatusOK, &res) - if !strings.Contains(res.Status, "uploaded 1 file(s) to checking") || !strings.Contains(res.Status, "2 new") { + if res.Status != "uploaded 1 file(s) to checking · press Import to read them" { t.Errorf("status = %q", res.Status) } if got, err := os.ReadFile(filepath.Join(dir, "2026-02.csv")); err != nil || string(got) != statement { t.Errorf("file on disk = %q, %v", got, err) } - var txns struct{ Rows []txnRow } - call(t, h, "GET", "/api/transactions", nil, http.StatusOK, &txns) - if len(txns.Rows) != 2 { - t.Errorf("index holds %d rows, want the 2 uploaded", len(txns.Rows)) + if got := descriptions(t, h); got != "" { + t.Errorf("upload imported %s; it should only save", got) + } + var files struct{ Files []fileRow } + call(t, h, "GET", "/api/files", nil, http.StatusOK, &files) + if len(files.Files) != 1 || files.Files[0].Status != "new" { + t.Errorf("files = %+v, want the upload waiting as new", files.Files) } - // The same file again is not an error, and adds nothing. - call(t, h, "POST", "/api/upload", req, http.StatusOK, &res) - if !strings.Contains(res.Status, "uploaded 0 file(s)") || !strings.Contains(res.Status, "1 already there") { - t.Errorf("re-upload status = %q", res.Status) + call(t, h, "POST", "/api/import", importReq{}, http.StatusOK, nil) + if got := descriptions(t, h); got != "SALARY,LIDL SOFIA" { + t.Errorf("after Import the index holds %s", got) } - call(t, h, "GET", "/api/transactions", nil, http.StatusOK, &txns) - if len(txns.Rows) != 2 { - t.Errorf("re-upload left %d rows, want 2", len(txns.Rows)) + + // The same file again is not an error, and saves nothing. + call(t, h, "POST", "/api/upload", req, http.StatusOK, &res) + if res.Status != "uploaded 0 file(s) to checking (1 already there)" { + t.Errorf("re-upload status = %q", res.Status) } } @@ -203,7 +231,8 @@ func TestFileListStatuses(t *testing.T) { t.Errorf("%s status = %q, want %q", name, got[name].Status, want) } } - if got["a.csv"].Added != 2 || got["c.csv"].Added != 1 || got["a.csv"].ImportedAt == "" { + if got["a.csv"].Rows != 2 || got["a.csv"].Added != 2 || got["c.csv"].Rows != 1 || got["c.csv"].Added != 1 || + got["a.csv"].ImportedAt == "" { t.Errorf("a.csv = %+v, c.csv = %+v", got["a.csv"], got["c.csv"]) } if got["a.csv"].Size != int64(len(statement)) { @@ -251,3 +280,163 @@ func TestServeFileServesOnlyStatements(t *testing.T) { } } } + +// A parser that names its statements decides the stored name, so the bank's +// "download (3).pdf" lands as something that sorts by date. A reissue of the +// same date is numbered rather than refused, and a statement that does not +// say keeps the name it came with. +func TestUploadNamesStatementsTheParserCanName(t *testing.T) { + dir, h := newUploadServer(t, "currency = \"EUR\"\nparser = \"webnamed\"\n") + upload := func(files ...uploadFile) string { + t.Helper() + var res status + call(t, h, "POST", "/api/upload", uploadReq{Account: "checking", Files: files}, http.StatusOK, &res) + return res.Status + } + reissue := "date,description,amount\n2026-02-01,LIDL SOFIA,-12.50\n2026-02-04,ZARA,-30.00\n" + other := "date,description,amount\n2026-03-01,KAUFLAND,-9.00\n" + + status := upload(uploadFile{Name: "download (3).csv", Data: []byte(statement)}) + if !strings.Contains(status, "download (3).csv → stmt_2026-02-01.csv") { + t.Errorf("status = %q, want the rename named", status) + } + // The same statement downloaded again is recognised under its new name. + if status := upload(uploadFile{Name: "download (4).csv", Data: []byte(statement)}); !strings.Contains(status, "1 already there") { + t.Errorf("second download: status = %q", status) + } + // A different statement of the same date, and two of one date in a batch. + upload(uploadFile{Name: "x.csv", Data: []byte(reissue)}, + uploadFile{Name: "y.csv", Data: []byte(reissue + "2026-02-05,BOLT,-4.00\n")}, + uploadFile{Name: "Z.CSV", Data: []byte(other)}, // a chosen name is lowercase + uploadFile{Name: "empty.csv", Data: []byte("date,description,amount\n")}) + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + var names []string + for _, e := range entries { + names = append(names, e.Name()) + } + want := config.AccountFile + " empty.csv stmt_2026-02-01.csv stmt_2026-02-01_2.csv stmt_2026-02-01_3.csv stmt_2026-03-01.csv" + if strings.Join(names, " ") != want { + t.Errorf("folder = %v, want %s", names, want) + } +} + +func writeFile(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +func descriptions(t *testing.T, h http.Handler) string { + t.Helper() + var txns struct{ Rows []txnRow } + call(t, h, "GET", "/api/transactions", nil, http.StatusOK, &txns) + var out []string + for _, r := range txns.Rows { + out = append(out, r.Description) + } + return strings.Join(out, ",") +} + +// Removing a statement deletes the file and takes out what it brought in. A +// row an overlapping statement also holds was stored under the file imported +// first; the next Import brings it back from the other one. +func TestRemoveFileKeepsWhatAnotherStatementHolds(t *testing.T) { + dir, h := newUploadServer(t, checkingTOML) + writeFile(t, filepath.Join(dir, "a.csv"), "date,description,amount\n2026-01-30,ONLY IN A,-1.00\n2026-02-01,SHARED,-2.00\n") + writeFile(t, filepath.Join(dir, "b.csv"), "date,description,amount\n2026-02-01,SHARED,-2.00\n2026-02-03,ONLY IN B,-3.00\n") + call(t, h, "POST", "/api/import", importReq{}, http.StatusOK, nil) + + // Each holds two rows; b.csv was imported second, so its shared row + // stayed with a.csv and it brought in only one. + var before struct{ Files []fileRow } + call(t, h, "GET", "/api/files", nil, http.StatusOK, &before) + if f := before.Files; len(f) != 2 || f[0].Rows != 2 || f[0].Added != 2 || f[1].Rows != 2 || f[1].Added != 1 { + t.Errorf("files = %+v, want a.csv 2 rows · 2 new and b.csv 2 rows · 1 new", f) + } + + var res status + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: "a.csv"}, http.StatusOK, &res) + if !strings.HasPrefix(res.Status, "deleted checking/a.csv, 2 transaction(s) dropped") { + t.Errorf("status = %q", res.Status) + } + // Deleting does not import, so the shared row is gone for now, and b.csv + // says it is due to be read again. + if got := descriptions(t, h); got != "ONLY IN B" { + t.Errorf("index holds %s right after the delete, want only b's own row", got) + } + var files struct{ Files []fileRow } + call(t, h, "GET", "/api/files", nil, http.StatusOK, &files) + if len(files.Files) != 1 || files.Files[0].Status != "changed" { + t.Errorf("files = %+v, want b.csv marked to be re-read", files.Files) + } + + call(t, h, "POST", "/api/import", importReq{}, http.StatusOK, nil) + if got := descriptions(t, h); got != "ONLY IN B,SHARED" { + t.Errorf("after Import the index holds %s, want the shared row restored from b", got) + } + if _, err := os.Stat(filepath.Join(dir, "a.csv")); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("a.csv is still on disk: %v", err) + } + if entries, _ := os.ReadDir(dir); len(entries) != 2 { + t.Errorf("folder holds %d entries, want account.toml and b.csv only", len(entries)) + } + + call(t, h, "GET", "/api/files", nil, http.StatusOK, &files) + if len(files.Files) != 1 || files.Files[0].Name != "b.csv" || files.Files[0].Status != "imported" { + t.Errorf("files = %+v, want only b.csv, imported", files.Files) + } +} + +// A removed leg takes its transfer pairing with it, and the other leg is left +// unpaired — which is the truth once one side is gone. +func TestRemoveFileUnpairsItsTransfers(t *testing.T) { + dir, h := newUploadServer(t, checkingTOML) + writeFile(t, filepath.Join(filepath.Dir(dir), config.RulesFile), ` +[[transfer]] +from_account = "checking" +from_desc = "*OUT*" +to_account = "checking" +to_desc = "*IN*" +`) + writeFile(t, filepath.Join(dir, "out.csv"), "date,description,amount\n2026-02-01,MOVE OUT,-5.00\n") + writeFile(t, filepath.Join(dir, "in.csv"), "date,description,amount\n2026-02-02,MOVE IN,5.00\n") + call(t, h, "POST", "/api/import", importReq{}, http.StatusOK, nil) + + var res status + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: "in.csv"}, http.StatusOK, &res) + if !strings.Contains(res.Status, "1 transfer leg(s) unpaired") { + t.Errorf("status = %q, want the remaining leg reported unpaired", res.Status) + } +} + +// A file already gone from disk is forgotten by the index; a name the index +// never had is not found. +func TestRemoveFileForgetsAMissingOne(t *testing.T) { + dir, h := newUploadServer(t, checkingTOML) + writeFile(t, filepath.Join(dir, "a.csv"), statement) + call(t, h, "POST", "/api/import", importReq{}, http.StatusOK, nil) + if err := os.Remove(filepath.Join(dir, "a.csv")); err != nil { + t.Fatal(err) + } + + var res status + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: "a.csv"}, http.StatusOK, &res) + if !strings.HasPrefix(res.Status, "forgot checking/a.csv, 2 transaction(s) dropped") { + t.Errorf("status = %q", res.Status) + } + if got := descriptions(t, h); got != "" { + t.Errorf("index still holds %s", got) + } + + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: "a.csv"}, http.StatusNotFound, nil) + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: "../rules.toml"}, http.StatusBadRequest, nil) + if _, err := os.Stat(filepath.Join(dir, config.AccountFile)); err != nil { + t.Fatal(err) + } + call(t, h, "POST", "/api/files/delete", removeFileReq{Account: "checking", Name: config.AccountFile}, http.StatusNotFound, nil) +}