From 00ab05eefc26da197f71f5301a43d0975945f92e Mon Sep 17 00:00:00 2001 From: davmar Date: Thu, 10 Sep 2026 23:59:00 +0200 Subject: [PATCH] attempt to fix test txs --- lib/models/bank.dart | 17 ++++ lib/providers/bank_provider.dart | 128 ++++++++++++++++++++++++------ lib/providers/sync_provider.dart | 91 +++++++++++++++++---- lib/services/pb_sync_service.dart | 103 +++++++++++++++++++----- lib/services/storage_service.dart | 47 +++++++++++ lib/utils/constants.dart | 3 + 6 files changed, 327 insertions(+), 62 deletions(-) diff --git a/lib/models/bank.dart b/lib/models/bank.dart index f749515..68a3565 100644 --- a/lib/models/bank.dart +++ b/lib/models/bank.dart @@ -113,6 +113,23 @@ class BankTransaction { /// Exact match for copying notes (same description + amount + currency). String get exactKey => '$descriptionKey|$amount|$currency'; + static String stripManualStamp(String description) => + description.replaceFirst(RegExp(r' ·#\d+$'), ''); + + /// True when [other] is the same row for delete/tombstone matching. + /// + /// Uses id first, then date + cents + currency + description (stamp-insensitive), + /// so PocketBase float/amount rehydration cannot bypass a local delete. + bool matchesForDelete(BankTransaction other) { + if (id == other.id) return true; + if (date != other.date) return false; + if (currency.toUpperCase() != other.currency.toUpperCase()) return false; + if ((amount * 100).round() != (other.amount * 100).round()) return false; + if (descriptionKey == other.descriptionKey) return true; + return stripManualStamp(description).trim().toLowerCase() == + stripManualStamp(other.description).trim().toLowerCase(); + } + BankTransaction copyWith({ String? date, String? description, diff --git a/lib/providers/bank_provider.dart b/lib/providers/bank_provider.dart index 2639953..40ba339 100644 --- a/lib/providers/bank_provider.dart +++ b/lib/providers/bank_provider.dart @@ -1,6 +1,8 @@ /// [ChangeNotifier] for bank link status, balance, and transactions. library; +import 'dart:async'; + import 'package:flutter/foundation.dart'; import '../models/bank.dart'; @@ -36,6 +38,18 @@ class BankController extends ChangeNotifier { _remoteTransactions = [ for (final tx in _remoteTransactions) _upgradeLegacyManual(tx), ]; + _pendingRemoteDeletes = _storage?.loadPendingTxDeletes() ?? const []; + // Drop anything still queued for remote delete from the local caches. + if (_pendingRemoteDeletes.isNotEmpty) { + _remoteTransactions = [ + for (final tx in _remoteTransactions) + if (!_isTombstoned(tx)) tx, + ]; + _importedTransactions = [ + for (final tx in _importedTransactions) + if (!_isTombstoned(tx)) tx, + ]; + } _invalidateMerged(); } @@ -55,29 +69,66 @@ class BankController extends ChangeNotifier { /// cannot leave the device behind the cloud (or wipe cloud-only history). VoidCallback? onAfterSuccessfulLoad; + /// Fired when local admin txs are removed/replaced and need a remote delete. + VoidCallback? onPendingRemoteDeletesChanged; + ConnectStatus _status = ConnectStatus.disconnected; BankBalance? _balance; List _remoteTransactions = const []; List _importedTransactions = const []; - List _pendingRemoteDeletes = const []; + List _pendingRemoteDeletes = const []; - /// Local tx_ids removed/replaced that should be deleted on the next PB push. - List takePendingRemoteDeletes() { - final out = List.of(_pendingRemoteDeletes); - _pendingRemoteDeletes = const []; - return out; - } + /// Tombstoned txs waiting to be removed from PocketBase (also persisted). + List get pendingRemoteDeletes => + List.unmodifiable(_pendingRemoteDeletes); - void requeueRemoteDeletes(Iterable txIds) { - for (final id in txIds) { - _queueRemoteDelete(id); + bool get hasPendingRemoteDeletes => _pendingRemoteDeletes.isNotEmpty; + + bool _isTombstoned(BankTransaction tx) { + for (final tomb in _pendingRemoteDeletes) { + if (tx.matchesForDelete(tomb)) return true; } + return false; } - void _queueRemoteDelete(String txId) { - if (txId.isEmpty) return; - if (_pendingRemoteDeletes.contains(txId)) return; - _pendingRemoteDeletes = [..._pendingRemoteDeletes, txId]; + /// Drop tombstones that match any of [txs] (fingerprint-aware). + void clearRemoteDeletesMatching(Iterable txs) { + final list = txs.toList(growable: false); + if (list.isEmpty) return; + final next = [ + for (final tomb in _pendingRemoteDeletes) + if (!list.any(tomb.matchesForDelete)) tomb, + ]; + if (next.length == _pendingRemoteDeletes.length) return; + _pendingRemoteDeletes = next; + unawaited(_persistPendingDeletes()); + } + + void clearAllPendingRemoteDeletes() { + if (_pendingRemoteDeletes.isEmpty) return; + _pendingRemoteDeletes = const []; + unawaited(_persistPendingDeletes()); + } + + void requeueRemoteDeletes(Iterable txs) { + for (final tx in txs) { + _queueRemoteDelete(tx, notify: false); + } + unawaited(_persistPendingDeletes()); + } + + void _queueRemoteDelete(BankTransaction tx, {bool notify = true}) { + if (tx.id.isEmpty) return; + if (_isTombstoned(tx)) return; + _pendingRemoteDeletes = [..._pendingRemoteDeletes, tx]; + unawaited(_persistPendingDeletes()); + if (notify) onPendingRemoteDeletesChanged?.call(); + } + + Future _persistPendingDeletes() async { + final storage = _storage; + if (storage == null) return; + await storage.savePendingTxDeletes(_pendingRemoteDeletes); } /// Keep [isManual] when a synced/API row replaces a matching imported test tx. @@ -300,7 +351,7 @@ class BankController extends ChangeNotifier { if (tx.id != previous.id) tx, ]; if (previous.id != updated.id) { - _queueRemoteDelete(previous.id); + _queueRemoteDelete(previous); } _importedTransactions = mergeTransactions(_importedTransactions, [updated]); _invalidateMerged(); @@ -317,13 +368,13 @@ class BankController extends ChangeNotifier { } _importedTransactions = [ for (final row in _importedTransactions) - if (row.id != tx.id) row, + if (!row.matchesForDelete(tx)) row, ]; _remoteTransactions = [ for (final row in _remoteTransactions) - if (row.id != tx.id) row, + if (!row.matchesForDelete(tx)) row, ]; - _queueRemoteDelete(tx.id); + _queueRemoteDelete(tx); _invalidateMerged(); await _persistImported(); await _persistCache(); @@ -337,17 +388,36 @@ class BankController extends ChangeNotifier { List fromCloud, { bool notify = true, }) async { - if (fromCloud.isEmpty) return; + if (fromCloud.isEmpty && _pendingRemoteDeletes.isEmpty) return; - final manuals = _collectManualIds(fromCloud.map(_upgradeLegacyManual)); - _remoteTransactions = mergeTransactions( - _remoteTransactions, - [for (final tx in fromCloud) _upgradeLegacyManual(tx)], - ); + // Never resurrect rows the user already deleted locally. + final incoming = [ + for (final tx in fromCloud) + if (!_isTombstoned(tx)) _upgradeLegacyManual(tx), + ]; + if (incoming.isEmpty && _pendingRemoteDeletes.isEmpty) return; + + final manuals = _collectManualIds(incoming); + if (incoming.isNotEmpty) { + _remoteTransactions = mergeTransactions(_remoteTransactions, incoming); + } + // Strip any local copies that match a pending delete (defensive). + if (_pendingRemoteDeletes.isNotEmpty) { + _remoteTransactions = [ + for (final tx in _remoteTransactions) + if (!_isTombstoned(tx)) tx, + ]; + } _importedTransactions = filterNewTransactions( _remoteTransactions, _importedTransactions, ); + if (_pendingRemoteDeletes.isNotEmpty) { + _importedTransactions = [ + for (final tx in _importedTransactions) + if (!_isTombstoned(tx)) tx, + ]; + } _reapplyManualFlags(manuals); _invalidateMerged(); @@ -444,6 +514,16 @@ class BankController extends ChangeNotifier { _remoteTransactions, _importedTransactions, ); + if (_pendingRemoteDeletes.isNotEmpty) { + _remoteTransactions = [ + for (final tx in _remoteTransactions) + if (!_isTombstoned(tx)) tx, + ]; + _importedTransactions = [ + for (final tx in _importedTransactions) + if (!_isTombstoned(tx)) tx, + ]; + } _reapplyManualFlags(manuals); _invalidateMerged(); await _persistImported(); diff --git a/lib/providers/sync_provider.dart b/lib/providers/sync_provider.dart index 1afb6bd..7669f2c 100644 --- a/lib/providers/sync_provider.dart +++ b/lib/providers/sync_provider.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; import '../models/app_cloud_prefs.dart'; +import '../models/bank.dart'; import '../models/home_filter_widget.dart'; import '../models/overview_favourite.dart'; import '../services/pb_sync_service.dart'; @@ -46,6 +47,7 @@ class SyncController extends ChangeNotifier { _prefs.onAppPrefsPersisted = _scheduleAppPrefsPush; _auth.addListener(_onAuthChanged); _bank.onAfterSuccessfulLoad = _onBankDataLoaded; + _bank.onPendingRemoteDeletesChanged = _scheduleBankTxPush; if (_auth.isLoggedIn) { unawaited(syncNow()); } @@ -64,6 +66,7 @@ class SyncController extends ChangeNotifier { bool _pushFavouritesPending = false; bool _pushHomeWidgetsPending = false; bool _pushAppPrefsPending = false; + bool _pushBankPending = false; Timer? _pushDebounce; Timer? _progressHold; Timer? _uiNotifyThrottle; @@ -198,6 +201,7 @@ class SyncController extends ChangeNotifier { _pushFavouritesPending = false; _pushHomeWidgetsPending = false; _pushAppPrefsPending = false; + _pushBankPending = false; _lastError = null; _statusDetail = ''; _showProgressRing = false; @@ -265,6 +269,28 @@ class SyncController extends ChangeNotifier { _armPushDebounce(); } + void _scheduleBankTxPush() { + _pushBankPending = true; + _armPushDebounce(); + } + + /// Removes tombstoned bank txs from PocketBase before pull/push can resurrect them. + Future _flushPendingTxDeletes(PbSyncService service) async { + final pending = List.of(_bank.pendingRemoteDeletes); + if (pending.isEmpty) return; + _setStatus('Removing deleted bank txs…'); + try { + final matched = await service.deleteTransactionsMatching(pending); + if (matched.isNotEmpty) { + _bank.clearRemoteDeletesMatching(matched); + } + // Do NOT clear unmatched tombstones — they still block pull resurrection + // until pushTransactions GC removes the remote rows. + } catch (error) { + debugPrint('SyncController: tx deletes failed ($error)'); + } + } + void _armPushDebounce() { if (!_auth.isLoggedIn) return; if (_syncing) { @@ -297,6 +323,8 @@ class SyncController extends ChangeNotifier { try { final service = PbSyncService(_auth.client); + // Delete first — otherwise pull re-merges the row and push recreates it. + await _flushPendingTxDeletes(service); final remoteMeta = await service.pull( onProgress: (p) { _setProgress(p * 0.35); @@ -408,22 +436,20 @@ class SyncController extends ChangeNotifier { } catch (error) { debugPrint('SyncController: app_prefs push skipped ($error)'); } - final pendingDeletes = _bank.takePendingRemoteDeletes(); - if (pendingDeletes.isNotEmpty) { - try { - await service.deleteTransactionsByTxIds(pendingDeletes); - } catch (error) { - debugPrint('SyncController: tx deletes skipped ($error)'); - _bank.requeueRemoteDeletes(pendingDeletes); - } - } + // Retry deletes in case any were queued during the pull/merge window. + await _flushPendingTxDeletes(service); + _pushBankPending = false; + final tombs = List.of(_bank.pendingRemoteDeletes); final bankSummary = await service.pushTransactions( _bank.transactions, + deleteTombs: tombs, progressStart: 0.82, progressEnd: 1.0, onProgress: _setProgress, onStatus: _setStatus, ); + // Push GC removes orphan admin test rows; tombstones are done. + _bank.clearAllPendingRemoteDeletes(); _lastSyncedAt = DateTime.now(); _lastError = null; _lastSyncSummary = '$metaSummary\n$bankSummary'; @@ -463,11 +489,17 @@ class SyncController extends ChangeNotifier { final pushFavourites = _pushFavouritesPending; final pushHomeWidgets = _pushHomeWidgetsPending; final pushAppPrefs = _pushAppPrefsPending; + final pushBank = _pushBankPending || _bank.pendingRemoteDeletes.isNotEmpty; _pushMetaPending = false; _pushFavouritesPending = false; _pushHomeWidgetsPending = false; _pushAppPrefsPending = false; - if (!pushMeta && !pushFavourites && !pushHomeWidgets && !pushAppPrefs) { + _pushBankPending = false; + if (!pushMeta && + !pushFavourites && + !pushHomeWidgets && + !pushAppPrefs && + !pushBank) { return; } @@ -480,13 +512,15 @@ class SyncController extends ChangeNotifier { pushFavourites, pushHomeWidgets, pushAppPrefs, + pushBank, )) { - (false, false, false, true) => 'App prefs', - (false, false, true, false) => 'Home widgets', - (false, false, true, true) => 'Home layout', - (false, true, false, false) => 'Favourites', - (false, true, true, _) => 'Home layout', - (true, false, false, false) => 'Annotations', + (false, false, false, false, true) => 'Bank txs', + (false, false, false, true, false) => 'App prefs', + (false, false, true, false, false) => 'Home widgets', + (false, false, true, true, _) => 'Home layout', + (false, true, false, false, false) => 'Favourites', + (false, true, true, _, _) => 'Home layout', + (true, false, false, false, false) => 'Annotations', _ => 'Changes', }; _statusDetail = 'Pushing $label…'; @@ -500,7 +534,8 @@ class SyncController extends ChangeNotifier { final total = (pushMeta ? 1 : 0) + (pushFavourites ? 1 : 0) + (pushHomeWidgets ? 1 : 0) + - (pushAppPrefs ? 1 : 0); + (pushAppPrefs ? 1 : 0) + + (pushBank ? 1 : 0); void bump() { done++; _setProgress(done / total); @@ -564,6 +599,27 @@ class SyncController extends ChangeNotifier { } bump(); } + if (pushBank) { + _setStatus('Updating bank txs…'); + await _flushPendingTxDeletes(service); + try { + final tombs = List.of(_bank.pendingRemoteDeletes); + final summary = await service.pushTransactions( + _bank.transactions, + deleteTombs: tombs, + progressStart: done / total, + progressEnd: (done + 1) / total, + onProgress: _setProgress, + onStatus: _setStatus, + ); + _bank.clearAllPendingRemoteDeletes(); + parts.add(summary); + } catch (error) { + debugPrint('SyncController: bank tx push skipped ($error)'); + _pushBankPending = true; + } + bump(); + } _setProgress(1.0); _lastSyncedAt = DateTime.now(); @@ -597,6 +653,7 @@ class SyncController extends ChangeNotifier { _uiNotifyThrottle?.cancel(); _auth.removeListener(_onAuthChanged); _bank.onAfterSuccessfulLoad = null; + _bank.onPendingRemoteDeletesChanged = null; _meta.onPersisted = null; _prefs.onFavouritesPersisted = null; _prefs.onHomeWidgetsPersisted = null; diff --git a/lib/services/pb_sync_service.dart b/lib/services/pb_sync_service.dart index 1d8f574..64c14a5 100644 --- a/lib/services/pb_sync_service.dart +++ b/lib/services/pb_sync_service.dart @@ -598,10 +598,14 @@ class PbSyncService { await _syncAppPrefs(userId, filter, prefs); } - /// Upserts local bank txs to PocketBase. Does **not** delete remote rows - /// (partial local caches must not wipe another device). + /// Upserts local bank txs to PocketBase. + /// + /// Also garbage-collects remote rows that are: + /// - admin test rows (`is_manual` / `·#…` stamp) no longer local, or + /// - listed in [deleteTombs] (local delete queue). Future pushTransactions( List local, { + List deleteTombs = const [], void Function(double progress)? onProgress, void Function(String message)? onStatus, double progressStart = 0.0, @@ -630,10 +634,19 @@ class PbSyncService { final ops = <_BatchOp>[]; var skipped = 0; final seen = {}; + final tombIds = { + for (final t in deleteTombs) + if (_clip(t.id, 2000).isNotEmpty) _clip(t.id, 2000), + }; for (final tx in local) { final syncId = _clip(tx.id, 2000); if (syncId.isEmpty || !seen.add(syncId)) continue; + // Never re-upsert a tombstoned row in the same pass. + if (tombIds.contains(syncId) || + deleteTombs.any(tx.matchesForDelete)) { + continue; + } final body = { 'user': userId, 'tx_id': syncId, @@ -648,7 +661,9 @@ class PbSyncService { 8, ), 'pending': tx.pending, - 'is_manual': tx.isManual, + // Only send when true so servers without the migration still accept + // normal bank/CSV upserts. Test rows are also detected via ·# stamp. + if (tx.isManual) 'is_manual': true, }; final remote = existing[syncId]; if (remote == null) { @@ -660,11 +675,25 @@ class PbSyncService { } } + // Remove admin test / tombstoned rows that this device no longer has. + var deletes = 0; + for (final entry in existing.entries) { + if (seen.contains(entry.key)) continue; + final remoteTx = _bankTxFromRecord(entry.value); + final tombHit = tombIds.contains(entry.key) || + (remoteTx != null && deleteTombs.any(remoteTx.matchesForDelete)); + final orphanManual = _remoteLooksLikeManualTest(entry.value); + if (!tombHit && !orphanManual) continue; + ops.add(_BatchOp.delete(entry.value.id)); + deletes++; + } + final creates = ops.where((o) => o.kind == _BatchOpKind.create).length; final updates = ops.where((o) => o.kind == _BatchOpKind.update).length; debugPrint( 'PbSyncService transactions: local=${local.length} ' - 'remote=${existing.length} create=$creates update=$updates skip=$skipped', + 'remote=${existing.length} create=$creates update=$updates ' + 'delete=$deletes skip=$skipped', ); onStatus?.call( ops.isEmpty @@ -678,8 +707,13 @@ class PbSyncService { onStatus: onStatus, ); return 'Bank txs: local ${local.length} → remote ' - '${existing.length + creates} ' - '(+$creates ~$updates · $skipped unchanged)'; + '${existing.length + creates - deletes} ' + '(+$creates ~$updates -$deletes · $skipped unchanged)'; + } + + bool _remoteLooksLikeManualTest(RecordModel r) { + if (r.getBoolValue('is_manual')) return true; + return RegExp(r' ·#\d+$').hasMatch(r.getStringValue('description')); } BankTransaction? _bankTxFromRecord(RecordModel r) { @@ -700,7 +734,8 @@ class PbSyncService { ? 'EUR' : r.getStringValue('currency'), pending: r.getBoolValue('pending'), - isManual: r.getBoolValue('is_manual'), + isManual: r.getBoolValue('is_manual') || + RegExp(r' ·#\d+$').hasMatch(description), ); } @@ -711,6 +746,8 @@ class PbSyncService { String s => double.tryParse(s) ?? 0, _ => 0.0, }; + final remoteManual = remote.getBoolValue('is_manual') || + RegExp(r' ·#\d+$').hasMatch(remote.getStringValue('description')); return remote.getStringValue('date') == tx.date && remote.getStringValue('description') == _clip(tx.description.isEmpty ? 'Unknown' : tx.description, 4000) && @@ -718,29 +755,53 @@ class PbSyncService { remote.getStringValue('currency').toUpperCase() == tx.currency.toUpperCase() && remote.getBoolValue('pending') == tx.pending && - remote.getBoolValue('is_manual') == tx.isManual; + remoteManual == tx.isManual; } - /// Deletes remote bank rows by local [BankTransaction.id] (tx_id field). - Future deleteTransactionsByTxIds(Iterable txIds) async { - final ids = { - for (final id in txIds) - if (_clip(id, 2000).isNotEmpty) _clip(id, 2000), - }; - if (ids.isEmpty) return 'Bank txs: nothing to delete'; + /// Deletes remote bank rows matching [tombstones] by tx_id or field fingerprint. + /// + /// Returns the tombstones that matched at least one remote row (safe to clear). + Future> deleteTransactionsMatching( + List tombstones, + ) async { + if (tombstones.isEmpty) return const []; final userId = _userId; final filter = 'user="$userId"'; final existing = await _fullList('transactions', filter); - final ops = <_BatchOp>[ - for (final r in existing) - if (ids.contains(r.getStringValue('tx_id'))) _BatchOp.delete(r.id), - ]; + final matchedTombs = []; + final ops = <_BatchOp>[]; + final seenRecordIds = {}; + + for (final r in existing) { + final remoteTx = _bankTxFromRecord(r); + if (remoteTx == null) continue; + for (final tomb in tombstones) { + final txId = r.getStringValue('tx_id'); + final idHit = txId == _clip(tomb.id, 2000); + if (!idHit && !remoteTx.matchesForDelete(tomb)) continue; + if (!seenRecordIds.add(r.id)) break; + ops.add(_BatchOp.delete(r.id)); + if (!matchedTombs.any(tomb.matchesForDelete)) { + matchedTombs.add(tomb); + } + break; + } + } + if (ops.isEmpty) { - return 'Bank txs: 0 remote deletes (ids ${ids.length})'; + debugPrint( + 'PbSyncService deleteTransactionsMatching: 0 hits ' + 'for ${tombstones.length} tombstone(s)', + ); + return const []; } await _runWriteOps('transactions', ops); - return 'Bank txs: deleted ${ops.length} remote row(s)'; + debugPrint( + 'PbSyncService deleteTransactionsMatching: deleted ${ops.length} ' + 'for ${matchedTombs.length} tombstone(s)', + ); + return matchedTombs; } Future _runOne(String collection, _BatchOp op) { diff --git a/lib/services/storage_service.dart b/lib/services/storage_service.dart index 6204624..329d367 100644 --- a/lib/services/storage_service.dart +++ b/lib/services/storage_service.dart @@ -260,6 +260,53 @@ class StorageService { return _prefs.remove(StorageKeys.importedTransactions); } + /// Tombstones for locally deleted bank txs awaiting remote removal. + /// + /// Supports legacy string-id lists and full transaction JSON objects. + List loadPendingTxDeletes() { + final raw = _prefs.getString(StorageKeys.pendingTxDeletes); + if (raw == null || raw.isEmpty) return const []; + try { + final decoded = jsonDecode(raw); + if (decoded is! List) return const []; + final out = []; + for (final e in decoded) { + if (e is Map) { + out.add(BankTransaction.fromJson(Map.from(e))); + } else if (e is String && e.isNotEmpty) { + // Legacy id-only entry — match by id string only. + final parts = e.split('|'); + if (parts.length >= 4) { + final amount = double.tryParse(parts[2]) ?? 0; + out.add( + BankTransaction( + date: parts[0], + currency: parts[1], + amount: amount, + description: parts.sublist(3).join('|'), + pending: false, + isManual: true, + ), + ); + } + } + } + return out; + } catch (_) { + return const []; + } + } + + Future savePendingTxDeletes(List txs) { + if (txs.isEmpty) { + return _prefs.remove(StorageKeys.pendingTxDeletes); + } + return _prefs.setString( + StorageKeys.pendingTxDeletes, + jsonEncode([for (final tx in txs) tx.toJson()]), + ); + } + TxMetaStoreData loadTxMetaStore() { final raw = _prefs.getString(StorageKeys.txMeta); if (raw != null && raw.isNotEmpty) { diff --git a/lib/utils/constants.dart b/lib/utils/constants.dart index 66fc94e..ae7bc04 100644 --- a/lib/utils/constants.dart +++ b/lib/utils/constants.dart @@ -46,6 +46,9 @@ abstract final class StorageKeys { static const bankCache = 'bank_cache_v1'; static const importedTransactions = 'imported_txs_v1'; + /// Local bank tx_ids queued for PocketBase deletion (JSON string list). + static const pendingTxDeletes = 'pending_tx_deletes_v1'; + /// Serialized PocketBase auth token + user record (separate from tx data). static const pocketBaseAuth = 'pb_auth_v1';