From 9b84c8709fe8b9aa00af440a423a4b84dda302be Mon Sep 17 00:00:00 2001 From: Fuh Austin Date: Fri, 18 Sep 2026 10:17:31 +0100 Subject: [PATCH 1/5] fix(sync): Reconcile duplicate entities and claim client ids consistently Two devices creating the same record independently produced two local rows fighting over one server id: the second silently folded into the first and read as data loss. Reconcile them instead. - Merge duplicates on server-id collision, repointing transactions, transfers and budget targets from the losing row to the survivor. - Adopt server ids for categories, wallets, parties and groups so a download can no longer strand a local row. - Guard local writes against duplicate names, comparing as the API does (utf8mb4_unicode_ci) rather than byte-for-byte. - Send the client id on creates and on the dedicated claim request only; an ordinary update must not repoint a record another device tracks. - Give configurations the claimClientId path every other entity already had. Without it the claim fell back to a plain update, which omits the field, so the server echoed a null client_generated_id and parsing threw "type 'Null' is not a subtype of type 'String'". - Parse transfer DTOs and paginated envelopes leniently so one malformed record cannot abort a whole page. --- assets/translations/de.json | 3 + assets/translations/en.json | 3 + assets/translations/es.json | 3 + assets/translations/fr.json | 3 + assets/translations/it.json | 3 + assets/translations/ru.json | 3 + lib/core/sync/sync_entity.dart | 29 ++ lib/data/database/app_database.dart | 281 +++++++++++ .../budget/budget_local_datasource.dart | 33 +- .../budget/budget_remote_datasource.dart | 9 +- .../budget/dtos/budget_complete_dto.dart | 7 +- .../category/category_local_datasource.dart | 33 +- .../category/category_remote_datasource.dart | 5 +- .../configuration_remote_datasource.dart | 28 +- .../datasources/core/pagination_response.dart | 41 ++ .../group/group_local_datasource.dart | 31 +- .../group/group_remote_datasource.dart | 5 +- .../holding/holding_remote_datasource.dart | 4 +- .../notification_remote_datasource.dart | 4 +- .../party/party_local_datasource.dart | 32 +- .../party/party_remote_datasource.dart | 5 +- .../reminder/reminder_remote_datasource.dart | 15 +- .../dto/transaction_complete_dto.dart | 12 +- .../transaction_remote_datasource.dart | 20 +- .../transfer/dto/transfer_dto.dart | 30 +- .../transfer/dto/transfer_dto.freezed.dart | 99 ++-- .../transfer/dto/transfer_dto.g.dart | 12 +- .../transfer/transfer_remote_datasource.dart | 13 +- .../wallet/wallet_local_datasource.dart | 51 +- .../wallet/wallet_remote_datasource.dart | 5 +- lib/data/sync/category_sync_handler.dart | 16 +- lib/data/sync/config_sync_handler.dart | 9 + lib/data/sync/group_sync_handler.dart | 16 +- lib/data/sync/party_sync_handler.dart | 16 +- lib/data/sync/transaction_sync_handler.dart | 14 + lib/data/sync/wallet_sync_handler.dart | 16 +- lib/gen/translations/codegen_loader.g.dart | 3 + lib/gen/translations/locale_keys.g.dart | 3 + .../category/add_category_screen.dart | 2 +- .../widgets/category_setup_widget.dart | 2 +- .../parties/widgets/add_party_form.dart | 5 +- lib/presentation/sync_history_screen.dart | 89 ++++ .../utils/dialogs/add_party_dialog.dart | 1 - .../utils/dialogs/custom_range_picker.dart | 1 - .../utils/forms/add_groups_form.dart | 5 +- .../utils/forms/add_wallet_form.dart | 27 +- pubspec.lock | 10 +- test/unit/category_duplicate_name_test.dart | 110 +++++ test/unit/category_merge_test.dart | 152 ++++++ test/unit/duplicate_name_guard_test.dart | 222 +++++++++ test/unit/duplicate_server_id_merge_test.dart | 448 ++++++++++++++++++ test/unit/pagination_lenient_test.dart | 74 +++ test/unit/sync_entity_test.dart | 50 ++ test/unit/transfer_dto_parsing_test.dart | 69 +++ test/unit/write_request_client_id_test.dart | 354 ++++++++++++++ 55 files changed, 2387 insertions(+), 149 deletions(-) create mode 100644 lib/core/sync/sync_entity.dart create mode 100644 test/unit/category_duplicate_name_test.dart create mode 100644 test/unit/category_merge_test.dart create mode 100644 test/unit/duplicate_name_guard_test.dart create mode 100644 test/unit/duplicate_server_id_merge_test.dart create mode 100644 test/unit/pagination_lenient_test.dart create mode 100644 test/unit/sync_entity_test.dart create mode 100644 test/unit/transfer_dto_parsing_test.dart create mode 100644 test/unit/write_request_client_id_test.dart diff --git a/assets/translations/de.json b/assets/translations/de.json index fdd7a371..a4933323 100644 --- a/assets/translations/de.json +++ b/assets/translations/de.json @@ -330,6 +330,9 @@ "deleteGroupConfirm": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", "duplicate": "Duplicate", "categoryNameAlreadyExists": "Kategoriename existiert bereits", + "walletNameAlreadyExists": "Eine Brieftasche mit diesem Namen und dieser Währung existiert bereits", + "groupNameAlreadyExists": "Gruppenname existiert bereits", + "partyNameAlreadyExists": "Name des Geschäftspartners existiert bereits", "deleteWallet": "Wallet löschen", "deleteWalletConfirm": "Are you sure you want to delete {name}?", "edit": "Edit", diff --git a/assets/translations/en.json b/assets/translations/en.json index c1f29fec..e8e649c6 100644 --- a/assets/translations/en.json +++ b/assets/translations/en.json @@ -332,6 +332,9 @@ "deleteGroupConfirm": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", "duplicate": "Duplicate", "categoryNameAlreadyExists": "Category name already exists", + "walletNameAlreadyExists": "A wallet with this name and currency already exists", + "groupNameAlreadyExists": "Group name already exists", + "partyNameAlreadyExists": "Party name already exists", "deleteWallet": "Delete wallet", "deleteWalletConfirm": "Are you sure you want to delete {name}?", "edit": "Edit", diff --git a/assets/translations/es.json b/assets/translations/es.json index c5ecd804..a49388ad 100644 --- a/assets/translations/es.json +++ b/assets/translations/es.json @@ -333,6 +333,9 @@ "deleteGroupConfirm": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", "duplicate": "Duplicate", "categoryNameAlreadyExists": "El nombre de la categoría ya existe", + "walletNameAlreadyExists": "Ya existe una billetera con este nombre y esta moneda", + "groupNameAlreadyExists": "El nombre del grupo ya existe", + "partyNameAlreadyExists": "El nombre de la parte ya existe", "deleteWallet": "Eliminar billetera", "deleteWalletConfirm": "Are you sure you want to delete {name}?", "defaultName": "Default", diff --git a/assets/translations/fr.json b/assets/translations/fr.json index 51868aa0..9f9fec33 100644 --- a/assets/translations/fr.json +++ b/assets/translations/fr.json @@ -331,6 +331,9 @@ "deleteGroupConfirm": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", "duplicate": "Duplicate", "categoryNameAlreadyExists": "Le nom de la catégorie existe déjà", + "walletNameAlreadyExists": "Un portefeuille avec ce nom et cette devise existe déjà", + "groupNameAlreadyExists": "Le nom du groupe existe déjà", + "partyNameAlreadyExists": "Le nom du tiers existe déjà", "deleteWallet": "Supprimer le portefeuille", "deleteWalletConfirm": "Are you sure you want to delete {name}?", "defaultName": "Default", diff --git a/assets/translations/it.json b/assets/translations/it.json index 0cfd63c3..ec32239f 100644 --- a/assets/translations/it.json +++ b/assets/translations/it.json @@ -332,6 +332,9 @@ "deleteGroupConfirm": "Are you sure you want to delete \"{name}\"? This action cannot be undone.", "duplicate": "Duplicate", "categoryNameAlreadyExists": "Il nome della categoria esiste già", + "walletNameAlreadyExists": "Esiste già un portafoglio con questo nome e questa valuta", + "groupNameAlreadyExists": "Il nome del gruppo esiste già", + "partyNameAlreadyExists": "Il nome della parte esiste già", "deleteWallet": "Elimina portafoglio", "deleteWalletConfirm": "Are you sure you want to delete {name}?", "edit": "Edit", diff --git a/assets/translations/ru.json b/assets/translations/ru.json index f6f2a104..86ed191d 100644 --- a/assets/translations/ru.json +++ b/assets/translations/ru.json @@ -332,6 +332,9 @@ "deleteGroupConfirm": "Вы уверены, что хотите удалить \"{name}\"? Это действие невозможно отменить.", "duplicate": "Дублировать", "categoryNameAlreadyExists": "Имя категории уже существует", + "walletNameAlreadyExists": "Кошелек с таким названием и валютой уже существует", + "groupNameAlreadyExists": "Название группы уже существует", + "partyNameAlreadyExists": "Имя стороны уже существует", "deleteWallet": "Удалить кошелек", "deleteWalletConfirm": "Вы уверены, что хотите удалить {name}?", "edit": "Edit", diff --git a/lib/core/sync/sync_entity.dart b/lib/core/sync/sync_entity.dart new file mode 100644 index 00000000..60bdeef4 --- /dev/null +++ b/lib/core/sync/sync_entity.dart @@ -0,0 +1,29 @@ +/// The entity kinds the sync layer names. +/// +/// The wire and storage boundaries speak plain strings — `drift_sync_core` +/// types `SyncTypeHandler.entityType` as `String`, and `local_changes.entityType` +/// is a text column holding values already written to users' databases — so +/// [key] is what crosses them, and it must keep matching the `entity` constant +/// on each handler. `sync_entity_test` asserts that it does. +enum SyncEntity { + wallet('wallet'), + category('category'), + group('group'), + party('party'), + transaction('transaction'), + transfer('transfer'), + budget('budget'), + budgetPeriodState('budget_period_state'), + reminder('reminder'), + notification('notification'), + config('config'), + media('media'), + + /// Read-through cache rather than a synced type: it has no handler, and + /// appears here only so its paged reads can name themselves. + holding('holding'); + + const SyncEntity(this.key); + + final String key; +} diff --git a/lib/data/database/app_database.dart b/lib/data/database/app_database.dart index 82044e7d..03fded1e 100644 --- a/lib/data/database/app_database.dart +++ b/lib/data/database/app_database.dart @@ -43,6 +43,7 @@ import 'package:trakli/presentation/utils/enums.dart'; import 'app_database.steps.dart'; import 'tables/sync_meta_data.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; part 'app_database.g.dart'; @@ -217,6 +218,286 @@ class AppDatabase extends _$AppDatabase with SynchronizerDb { return results.map((row) => row.readTable(categories)).toList(); } + /// Folds [loserClientId] into [winnerClientId]: re-tags everything the + /// duplicate categorised, moves budgets that targeted it, deletes it, and + /// clears its stuck outbox entry. Returns the number of re-tagged rows. + /// + /// Repairs a create the API *rejected* (400, sent without a client id): the + /// duplicate never received a server id, so it is the loser. A create that + /// did send one gets 200 and the existing record instead, which + /// [adoptCategoryServerId] reconciles the other way round. + Future mergeDuplicateCategory({ + required String loserClientId, + required String winnerClientId, + }) => + _mergeCategory(loserClientId, winnerClientId, requireWinner: true); + + /// Hands [serverId] to [clientId], folding away whichever local category + /// holds it. See [_adoptServerId]. + Future adoptCategoryServerId({ + required int? serverId, + required String clientId, + }) => + _adoptServerId('categories', serverId, clientId, _mergeCategory); + + /// Hands [serverId] to [clientId], folding away whichever local wallet + /// holds it. See [_adoptServerId]. + Future adoptWalletServerId({ + required int? serverId, + required String clientId, + }) => + _adoptServerId('wallets', serverId, clientId, _mergeWallet); + + /// Hands [serverId] to [clientId], folding away whichever local party + /// holds it. See [_adoptServerId]. + Future adoptPartyServerId({ + required int? serverId, + required String clientId, + }) => + _adoptServerId('parties', serverId, clientId, _mergeParty); + + /// Hands [serverId] to [clientId], folding away whichever local group + /// holds it. See [_adoptServerId]. + Future adoptGroupServerId({ + required int? serverId, + required String clientId, + }) => + _adoptServerId('groups', serverId, clientId, _mergeGroup); + + /// Makes [serverId] available to [clientId] by merging the row that owns it + /// into [clientId]. A no-op unless some *other* local row owns it. Call it + /// immediately before writing the row, inside the same transaction. + /// + /// The create endpoints answer a name the user already has with 200 and the + /// existing record, after moving the posted client id onto it — so the server + /// id that comes back is one this device filed under a different client id, + /// and `SyncTable.id` is unique locally. Downloads reach the same state from + /// the other side. Either way the server now points at the incoming copy, so + /// the older one gives up its references. + Future _adoptServerId( + String table, + int? serverId, + String clientId, + Future Function(String loser, String winner, + {required bool requireWinner}) + merge, + ) async { + if (serverId == null || clientId.isEmpty) return; + + final holder = await customSelect( + 'SELECT client_id FROM $table WHERE id = ? LIMIT 1', + variables: [Variable(serverId)], + ).getSingleOrNull(); + if (holder == null) return; + + final holderClientId = holder.read('client_id'); + if (holderClientId.isEmpty || holderClientId == clientId) return; + + // The caller writes the survivor next; on a download its row does not + // exist yet. + await merge(holderClientId, clientId, requireWinner: false); + } + + Future _mergeCategory( + String loser, + String winner, { + required bool requireWinner, + }) { + return _mergeDuplicate( + entityType: SyncEntity.category, + table: 'categories', + loserClientId: loser, + winnerClientId: winner, + targetType: BudgetTargetType.category, + requireWinner: requireWinner, + repoint: () async { + // Re-tag by insert-then-delete rather than by update: a transaction + // already tagged with both categories would collide on the + // (source, type, category) primary key. + await customStatement( + 'INSERT OR IGNORE INTO categorizables ' + '(categorizable_id, categorizable_type, category_client_id) ' + 'SELECT categorizable_id, categorizable_type, ? FROM categorizables ' + 'WHERE category_client_id = ?', + [winner, loser], + ); + return (delete(categorizables) + ..where((c) => c.categoryClientId.equals(loser))) + .go(); + }, + ); + } + + Future _mergeWallet( + String loser, + String winner, { + required bool requireWinner, + }) { + return _mergeDuplicate( + entityType: SyncEntity.wallet, + table: 'wallets', + loserClientId: loser, + winnerClientId: winner, + targetType: BudgetTargetType.wallet, + requireWinner: requireWinner, + repoint: () async { + var moved = await (update(transactions) + ..where((t) => t.walletClientId.equals(loser))) + .write(TransactionsCompanion(walletClientId: Value(winner))); + moved += await (update(transfers) + ..where((t) => t.fromWalletClientId.equals(loser))) + .write(TransfersCompanion(fromWalletClientId: Value(winner))); + moved += await (update(transfers) + ..where((t) => t.toWalletClientId.equals(loser))) + .write(TransfersCompanion(toWalletClientId: Value(winner))); + return moved; + }, + ); + } + + Future _mergeParty( + String loser, + String winner, { + required bool requireWinner, + }) { + return _mergeDuplicate( + entityType: SyncEntity.party, + table: 'parties', + loserClientId: loser, + winnerClientId: winner, + // Parties cannot be budget targets. + targetType: null, + requireWinner: requireWinner, + repoint: () => (update(transactions) + ..where((t) => t.partyClientId.equals(loser))) + .write(TransactionsCompanion(partyClientId: Value(winner))), + ); + } + + Future _mergeGroup( + String loser, + String winner, { + required bool requireWinner, + }) { + return _mergeDuplicate( + entityType: SyncEntity.group, + table: 'groups', + loserClientId: loser, + winnerClientId: winner, + targetType: BudgetTargetType.group, + requireWinner: requireWinner, + repoint: () => (update(transactions) + ..where((t) => t.groupClientId.equals(loser))) + .write(TransactionsCompanion(groupClientId: Value(winner))), + ); + } + + /// Shared body of the merges. [repoint] moves the rows that referenced the + /// duplicate and reports how many; everything around it is the same for + /// every entity. + Future _mergeDuplicate({ + required SyncEntity entityType, + required String table, + required String loserClientId, + required String winnerClientId, + required BudgetTargetType? targetType, + required bool requireWinner, + required Future Function() repoint, + }) { + if (loserClientId == winnerClientId) { + throw ArgumentError.value( + loserClientId, + 'loserClientId', + 'A ${entityType.key} cannot be merged into itself', + ); + } + + return transaction(() async { + final required = [ + loserClientId, + if (requireWinner) winnerClientId, + ]; + for (final clientId in required) { + final exists = await customSelect( + 'SELECT 1 FROM $table WHERE client_id = ? LIMIT 1', + variables: [Variable(clientId)], + ).getSingleOrNull(); + if (exists == null) { + throw StateError('No ${entityType.key} with client id $clientId'); + } + } + + final repointed = await repoint(); + if (targetType != null) { + await _repointBudgetTargets(targetType, loserClientId, winnerClientId); + } + + // Its queued write cannot stand alone, and re-pointing it would collide + // with the survivor's entry on (entity_id, entity_type). + await (delete(localChanges) + ..where((lc) => + lc.entityType.equals(entityType.key) & + lc.entityId.equals(loserClientId))) + .go(); + await (delete(deferredRemoteItems) + ..where((d) => + d.entityType.equals(entityType.key) & + d.clientId.equals(loserClientId))) + .go(); + + await _repointPayloads(loserClientId, winnerClientId); + + await customStatement( + 'DELETE FROM $table WHERE client_id = ?', + [loserClientId], + ); + + return repointed; + }); + } + + /// Budget targets are keyed by (budget, type, target), so a budget that + /// targeted both copies would collide on the primary key — add what is + /// missing, then drop the duplicate's rows. + Future _repointBudgetTargets( + BudgetTargetType targetType, + String loserClientId, + String winnerClientId, + ) async { + await customStatement( + 'INSERT OR IGNORE INTO budget_targets ' + '(budget_client_id, target_type, target_client_id) ' + 'SELECT budget_client_id, target_type, ? FROM budget_targets ' + 'WHERE target_type = ? AND target_client_id = ?', + [winnerClientId, targetType.name, loserClientId], + ); + await (delete(budgetTargets) + ..where((t) => + t.targetType.equalsValue(targetType) & + t.targetClientId.equals(loserClientId))) + .go(); + } + + /// Rewrites references to [loserClientId] inside the queued outbox payloads + /// and the parked download payloads. + /// + /// They are immutable JSON snapshots of whole DTO graphs — a queued + /// transaction embeds its wallet object, which is written back verbatim and + /// would resurrect the row this merge just deleted. References sit under many + /// keys, in both camelCase and snake_case, at any depth; client ids are + /// unique enough that swapping the text hits exactly them. + Future _repointPayloads( + String loserClientId, + String winnerClientId, + ) async { + for (final table in const ['local_changes', 'deferred_remote_items']) { + await customStatement( + 'UPDATE $table SET data = replace(data, ?, ?) WHERE data LIKE ?', + [loserClientId, winnerClientId, '%$loserClientId%'], + ); + } + } + @override Future concludeEntityLocalChanges( String entityType, diff --git a/lib/data/datasources/budget/budget_local_datasource.dart b/lib/data/datasources/budget/budget_local_datasource.dart index 5bce82d7..e6e34812 100644 --- a/lib/data/datasources/budget/budget_local_datasource.dart +++ b/lib/data/datasources/budget/budget_local_datasource.dart @@ -142,6 +142,20 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { .watch(); } + /// The same name is the same budget, case and surrounding spaces aside. + Future _findByName(String name, {String? excluding}) { + final normalized = name.trim().toLowerCase(); + return (database.select(database.budgets) + ..where((b) { + final matches = b.name.trim().lower().equals(normalized); + return excluding == null + ? matches + : matches & b.clientId.isNotValue(excluding); + }) + ..limit(1)) + .getSingleOrNull(); + } + @override Future insertBudget({ required String name, @@ -158,10 +172,7 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { bool isActive = true, List targets = const [], }) async { - final existing = await (database.select(database.budgets) - ..where((b) => b.name.equals(name))) - .getSingleOrNull(); - if (existing != null) { + if (await _findByName(name) != null) { throw DuplicateException('Budget with name "$name" already exists'); } @@ -172,7 +183,7 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { final inserted = await database.into(database.budgets).insertReturning( BudgetsCompanion.insert( clientId: Value(clientId), - name: name, + name: name.trim(), slug: slug, amount: amount, currency: currency, @@ -224,14 +235,8 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { bool? isActive, List? targets, }) async { - if (name != null) { - final dupe = await (database.select(database.budgets) - ..where((b) => - b.name.equals(name) & b.clientId.isNotValue(clientId))) - .getSingleOrNull(); - if (dupe != null) { - throw DuplicateException('Budget with name "$name" already exists'); - } + if (name != null && await _findByName(name, excluding: clientId) != null) { + throw DuplicateException('Budget with name "$name" already exists'); } final now = getNewFormattedUtcDateTime(); @@ -241,7 +246,7 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { ..where((b) => b.clientId.equals(clientId))) .writeReturning( BudgetsCompanion( - name: name != null ? Value(name) : const Value.absent(), + name: name != null ? Value(name.trim()) : const Value.absent(), slug: slug != null ? Value(slug) : const Value.absent(), amount: amount != null ? Value(amount) : const Value.absent(), currency: currency != null ? Value(currency) : const Value.absent(), diff --git a/lib/data/datasources/budget/budget_remote_datasource.dart b/lib/data/datasources/budget/budget_remote_datasource.dart index 9738e1f8..7baaf1d8 100644 --- a/lib/data/datasources/budget/budget_remote_datasource.dart +++ b/lib/data/datasources/budget/budget_remote_datasource.dart @@ -6,6 +6,7 @@ import 'package:trakli/data/datasources/budget/dtos/budget_complete_dto.dart'; import 'package:trakli/data/datasources/budget/dtos/budget_period_state_dto.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class BudgetRemoteDataSource { Future> getAllBudgets({ @@ -60,10 +61,11 @@ class BudgetRemoteDataSourceImpl implements BudgetRemoteDataSource { final response = await dio.get('budgets', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginated = PaginationResponse.fromJson( + final paginated = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => BudgetCompleteDto.fromServerJson(json! as Map), + entityType: SyncEntity.budget, ); allItems.addAll(paginated.data); @@ -99,7 +101,7 @@ class BudgetRemoteDataSourceImpl implements BudgetRemoteDataSource { Future updateBudget(BudgetCompleteDto dto) async { final response = await dio.put( 'budgets/${dto.budget.id}', - data: dto.toServerJson(), + data: dto.toServerJson(includeClientId: false), ); final apiResponse = ApiResponse.fromJson(response.data); return BudgetCompleteDto.fromServerJson( @@ -162,11 +164,12 @@ class BudgetRemoteDataSourceImpl implements BudgetRemoteDataSource { ); final apiResponse = ApiResponse.fromJson(response.data); - final paginated = PaginationResponse.fromJson( + final paginated = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => BudgetPeriodStateDto.fromJson( JsonDefaultsHelper.addDefaults(json! as Map), ), + entityType: SyncEntity.budgetPeriodState, ); allItems.addAll(paginated.data); diff --git a/lib/data/datasources/budget/dtos/budget_complete_dto.dart b/lib/data/datasources/budget/dtos/budget_complete_dto.dart index 6ef17eae..58b0d883 100644 --- a/lib/data/datasources/budget/dtos/budget_complete_dto.dart +++ b/lib/data/datasources/budget/dtos/budget_complete_dto.dart @@ -76,9 +76,12 @@ class BudgetCompleteDto { return BudgetCompleteDto.fromServerJson(patched); } - Map toServerJson() { + /// The client id is for creates only: repointing a record's client id is + /// the claim endpoint's job, not a side effect of an edit. + Map toServerJson({bool includeClientId = true}) { return { - if (budget.clientId.isNotEmpty) 'client_id': budget.clientId, + if (includeClientId && budget.clientId.isNotEmpty) + 'client_id': budget.clientId, 'name': budget.name, if (budget.description != null && budget.description!.trim().isNotEmpty) 'description': budget.description, diff --git a/lib/data/datasources/category/category_local_datasource.dart b/lib/data/datasources/category/category_local_datasource.dart index d17602af..30634bd9 100644 --- a/lib/data/datasources/category/category_local_datasource.dart +++ b/lib/data/datasources/category/category_local_datasource.dart @@ -40,14 +40,27 @@ class CategoryLocalDataSourceImpl implements CategoryLocalDataSource { .get(); } + /// The same name is the same category, whatever its type, case and + /// surrounding spaces aside. + Future _findByServerName(String name, {String? excluding}) { + final normalized = name.trim().toLowerCase(); + return (database.select(database.categories) + ..where((c) { + final matches = c.name.trim().lower().equals(normalized); + return excluding == null + ? matches + : matches & c.clientId.isNotValue(excluding); + }) + ..limit(1)) + .getSingleOrNull(); + } + @override Future insertCategory( String name, String slug, TransactionType type, {String? description, Media? media}) async { // Check if category name already exists - final existingCategory = await (database.select(database.categories) - ..where((c) => c.name.equals(name))) - .getSingleOrNull(); + final existingCategory = await _findByServerName(name); if (existingCategory != null) { throw DuplicateException('Category with name "$name" already exists'); @@ -58,8 +71,8 @@ class CategoryLocalDataSourceImpl implements CategoryLocalDataSource { final model = await database.into(database.categories).insertReturning( CategoriesCompanion.insert( clientId: Value(await generateDeviceScopedId()), - name: name, - slug: slug, + name: name.trim(), + slug: slug.trim(), type: type, description: Value(description), icon: Value(media), @@ -81,10 +94,8 @@ class CategoryLocalDataSourceImpl implements CategoryLocalDataSource { }) async { // Check if category name already exists (only if name is being updated) if (name != null) { - final existingCategory = await (database.select(database.categories) - ..where( - (c) => c.name.equals(name) & c.clientId.isNotValue(clientId))) - .getSingleOrNull(); + final existingCategory = + await _findByServerName(name, excluding: clientId); if (existingCategory != null) { throw DuplicateException('Category with name "$name" already exists'); @@ -97,8 +108,8 @@ class CategoryLocalDataSourceImpl implements CategoryLocalDataSource { ..where((c) => c.clientId.equals(clientId))) .writeReturning( CategoriesCompanion( - name: name != null ? Value(name) : const Value.absent(), - slug: slug != null ? Value(slug) : const Value.absent(), + name: name != null ? Value(name.trim()) : const Value.absent(), + slug: slug != null ? Value(slug.trim()) : const Value.absent(), type: type != null ? Value(type) : const Value.absent(), description: description != null ? Value(description) : const Value.absent(), diff --git a/lib/data/datasources/category/category_remote_datasource.dart b/lib/data/datasources/category/category_remote_datasource.dart index 9809ad2b..a71503c5 100644 --- a/lib/data/datasources/category/category_remote_datasource.dart +++ b/lib/data/datasources/category/category_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/core/utils/json_defaults.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class CategoryRemoteDataSource { Future> getAllCategories( @@ -50,10 +51,11 @@ class CategoryRemoteDataSourceImpl implements CategoryRemoteDataSource { await dio.get('categories', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Category.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.category, ); allItems.addAll(paginatedResponse.data); @@ -107,7 +109,6 @@ class CategoryRemoteDataSourceImpl implements CategoryRemoteDataSource { 'categories/${category.id}', data: { 'type': category.type.serverKey, - 'client_id': category.clientId, 'name': category.name, if (category.description != null && category.description!.trim().isNotEmpty) ...{ diff --git a/lib/data/datasources/configuration/configuration_remote_datasource.dart b/lib/data/datasources/configuration/configuration_remote_datasource.dart index b47d0283..56fc6d2d 100644 --- a/lib/data/datasources/configuration/configuration_remote_datasource.dart +++ b/lib/data/datasources/configuration/configuration_remote_datasource.dart @@ -7,6 +7,7 @@ import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/data/mappers/config_mapper.dart'; import 'package:trakli/domain/entities/config_entity.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class ConfigRemoteDataSource { Future> getAllConfigs({ @@ -20,6 +21,12 @@ abstract class ConfigRemoteDataSource { Future updateConfig(Config config); + Future claimClientId({ + required String key, + required String clientId, + required DateTime updatedAt, + }); + Future deleteConfig(String id); // Legacy methods for backward compatibility @@ -51,17 +58,18 @@ class ConfigRemoteDataSourceImpl implements ConfigRemoteDataSource { final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Config.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.config, ); return paginatedResponse.data; } @override - Future getConfig( String id) async { + Future getConfig(String id) async { final response = await dio.get('configurations/$id'); if (response.data == null) return null; @@ -92,7 +100,6 @@ class ConfigRemoteDataSourceImpl implements ConfigRemoteDataSource { data: { 'type': config.type.serverKey, 'value': config.value, - 'client_id': config.clientId, }, ); @@ -100,6 +107,20 @@ class ConfigRemoteDataSourceImpl implements ConfigRemoteDataSource { return Config.fromJson(apiResponse.data); } + @override + Future claimClientId({ + required String key, + required String clientId, + required DateTime updatedAt, + }) async { + final response = await dio.put('configurations/$key', data: { + 'client_id': clientId, + 'updated_at': formatServerIsoDateTimeString(updatedAt), + }); + final apiResponse = ApiResponse.fromJson(response.data); + return Config.fromJson(apiResponse.data); + } + @override Future deleteConfig(String id) async { await dio.delete('configurations/$id'); @@ -111,5 +132,4 @@ class ConfigRemoteDataSourceImpl implements ConfigRemoteDataSource { final configs = await getAllConfigs(); return ConfigMapper.toDomainList(configs); } - } diff --git a/lib/data/datasources/core/pagination_response.dart b/lib/data/datasources/core/pagination_response.dart index 3d9f8425..12c68a47 100644 --- a/lib/data/datasources/core/pagination_response.dart +++ b/lib/data/datasources/core/pagination_response.dart @@ -1,4 +1,6 @@ import 'package:freezed_annotation/freezed_annotation.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; +import 'package:trakli/core/utils/services/logger.dart'; part 'pagination_response.freezed.dart'; part 'pagination_response.g.dart'; @@ -20,6 +22,45 @@ class PaginationResponse with _$PaginationResponse { PaginationResponse._(); + /// Like [fromJson], but drops rows [fromJsonT] cannot read instead of + /// failing the whole page. + /// + /// Down-sync catches parse errors per entity type, not per row, so one + /// unreadable row used to fail every row of that type on every sync cycle — + /// indefinitely, since the row never changes. (A null `datetime` on + /// pre-February transfers did exactly that.) Skipping costs the one row + /// until the data is fixed; failing the page costs all of them, forever. + factory PaginationResponse.lenient( + Map json, + T Function(Object? json) fromJsonT, { + required SyncEntity entityType, + }) { + final rows = json['data'] as List? ?? const []; + final parsed = []; + + for (final row in rows) { + try { + parsed.add(fromJsonT(row)); + } catch (e, stackTrace) { + final clientId = row is Map + ? row['client_generated_id'] ?? row['id'] + : null; + logger.w( + 'Skipped an unreadable ${entityType.key} row ($clientId) while syncing', + error: e, + stackTrace: stackTrace, + ); + } + } + + return PaginationResponse( + currentPage: (json['current_page'] as num).toInt(), + lastPage: (json['last_page'] as num).toInt(), + perPage: (json['per_page'] as num).toInt(), + data: parsed, + ); + } + factory PaginationResponse.empty() => PaginationResponse( currentPage: -1, lastPage: -1, diff --git a/lib/data/datasources/group/group_local_datasource.dart b/lib/data/datasources/group/group_local_datasource.dart index 174a25bd..8484630a 100644 --- a/lib/data/datasources/group/group_local_datasource.dart +++ b/lib/data/datasources/group/group_local_datasource.dart @@ -4,6 +4,7 @@ import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; +import 'package:trakli/core/error/exceptions.dart'; abstract class GroupLocalDataSource { Future> getAllGroups(); @@ -38,17 +39,36 @@ class GroupLocalDataSourceImpl implements GroupLocalDataSource { .get(); } + /// The same name is the same group, case and surrounding spaces aside. + Future _findByServerName(String name, {String? excluding}) { + final normalized = name.trim().toLowerCase(); + return (database.select(database.groups) + ..where((g) { + final matches = g.name.trim().lower().equals(normalized); + return excluding == null + ? matches + : matches & g.clientId.isNotValue(excluding); + }) + ..limit(1)) + .getSingleOrNull(); + } + @override Future insertGroup( String name, { String? description, Media? icon, }) async { + final existing = await _findByServerName(name); + if (existing != null) { + throw DuplicateException('Group with name "$name" already exists'); + } + final dateTime = getNewFormattedUtcDateTime(); final companion = GroupsCompanion.insert( clientId: Value(await generateDeviceScopedId()), - name: name, + name: name.trim(), description: Value(description), createdAt: Value(dateTime), updatedAt: Value(dateTime), @@ -67,10 +87,17 @@ class GroupLocalDataSourceImpl implements GroupLocalDataSource { String? description, Media? icon, }) async { + if (name != null) { + final existing = await _findByServerName(name, excluding: clientId); + if (existing != null) { + throw DuplicateException('Group with name "$name" already exists'); + } + } + DateTime dateTime = getNewFormattedUtcDateTime(); final companion = GroupsCompanion( - name: name != null ? Value(name) : const Value.absent(), + name: name != null ? Value(name.trim()) : const Value.absent(), description: description != null ? Value(description) : const Value.absent(), updatedAt: Value(dateTime), diff --git a/lib/data/datasources/group/group_remote_datasource.dart b/lib/data/datasources/group/group_remote_datasource.dart index 4ad3559a..a34b0e89 100644 --- a/lib/data/datasources/group/group_remote_datasource.dart +++ b/lib/data/datasources/group/group_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/core/utils/json_defaults.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class GroupRemoteDataSource { Future> getAllGroups({DateTime? syncedSince, bool? noClientId}); @@ -51,10 +52,11 @@ class GroupRemoteDataSourceImpl implements GroupRemoteDataSource { final response = await dio.get('groups', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Group.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.group, ); allItems.addAll(paginatedResponse.data); @@ -96,7 +98,6 @@ class GroupRemoteDataSourceImpl implements GroupRemoteDataSource { Future updateGroup(Group group) async { final response = await dio.put('groups/${group.id}', data: { 'name': group.name, - 'client_id': group.clientId, 'description': group.description, if (group.icon != null) ...{ 'icon': group.icon?.content, diff --git a/lib/data/datasources/holding/holding_remote_datasource.dart b/lib/data/datasources/holding/holding_remote_datasource.dart index d087171d..f9ea0c2f 100644 --- a/lib/data/datasources/holding/holding_remote_datasource.dart +++ b/lib/data/datasources/holding/holding_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/data/datasources/holding/dto/coin_search_result_dto.dart'; import 'package:trakli/data/datasources/holding/dto/holding_dto.dart'; import 'package:trakli/domain/entities/holding_entity.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; /// Polymorphic owner type expected by the backend holdings endpoints. const String _userOwnerType = 'App\\Models\\User'; @@ -53,9 +54,10 @@ class HoldingRemoteDataSourceImpl implements HoldingRemoteDataSource { queryParameters: {'per_page': 200}, ); final apiResponse = ApiResponse.fromJson(response.data); - final paged = PaginationResponse.fromJson( + final paged = PaginationResponse.lenient( apiResponse.data as Map, (json) => HoldingDto.fromJson(json! as Map), + entityType: SyncEntity.holding, ); return paged.data; } diff --git a/lib/data/datasources/notification/notification_remote_datasource.dart b/lib/data/datasources/notification/notification_remote_datasource.dart index 17a14788..10a6400e 100644 --- a/lib/data/datasources/notification/notification_remote_datasource.dart +++ b/lib/data/datasources/notification/notification_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/core/utils/json_defaults.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class NotificationRemoteDataSource { Future> getAllNotifications( @@ -43,10 +44,11 @@ class NotificationRemoteDataSourceImpl implements NotificationRemoteDataSource { await dio.get('notifications', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Notification.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.notification, ); allItems.addAll(paginatedResponse.data); diff --git a/lib/data/datasources/party/party_local_datasource.dart b/lib/data/datasources/party/party_local_datasource.dart index 6019b35b..35d218d1 100644 --- a/lib/data/datasources/party/party_local_datasource.dart +++ b/lib/data/datasources/party/party_local_datasource.dart @@ -4,6 +4,7 @@ import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; +import 'package:trakli/core/error/exceptions.dart'; import 'package:trakli/domain/entities/party_entity.dart'; abstract class PartyLocalDataSource { @@ -45,6 +46,21 @@ class PartyLocalDataSourceImpl implements PartyLocalDataSource { .getSingleOrNull(); } + /// The same name is the same party, whatever its type, case and + /// surrounding spaces aside. + Future _findByServerName(String name, {String? excluding}) { + final normalized = name.trim().toLowerCase(); + return (database.select(database.parties) + ..where((p) { + final matches = p.name.trim().lower().equals(normalized); + return excluding == null + ? matches + : matches & p.clientId.isNotValue(excluding); + }) + ..limit(1)) + .getSingleOrNull(); + } + @override Future insertParty( String name, { @@ -52,12 +68,17 @@ class PartyLocalDataSourceImpl implements PartyLocalDataSource { Media? media, PartyType? type, }) async { + final existing = await _findByServerName(name); + if (existing != null) { + throw DuplicateException('Party with name "$name" already exists'); + } + final now = getNewFormattedUtcDateTime(); final model = await database.into(database.parties).insertReturning( PartiesCompanion.insert( clientId: Value(await generateDeviceScopedId()), - name: name, + name: name.trim(), description: Value(description), createdAt: Value(now), updatedAt: Value(now), @@ -77,13 +98,20 @@ class PartyLocalDataSourceImpl implements PartyLocalDataSource { Media? media, PartyType? type, }) async { + if (name != null) { + final existing = await _findByServerName(name, excluding: clientId); + if (existing != null) { + throw DuplicateException('Party with name "$name" already exists'); + } + } + final now = getNewFormattedUtcDateTime(); final party = await (database.update(database.parties) ..where((p) => p.clientId.equals(clientId))) .writeReturning( PartiesCompanion( - name: name != null ? Value(name) : const Value.absent(), + name: name != null ? Value(name.trim()) : const Value.absent(), description: description != null ? Value(description) : const Value.absent(), updatedAt: Value(now), diff --git a/lib/data/datasources/party/party_remote_datasource.dart b/lib/data/datasources/party/party_remote_datasource.dart index 3fc4e5ab..51b17ab0 100644 --- a/lib/data/datasources/party/party_remote_datasource.dart +++ b/lib/data/datasources/party/party_remote_datasource.dart @@ -6,6 +6,7 @@ import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/domain/entities/party_entity.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class PartyRemoteDataSource { Future> getAllParties({DateTime? syncedSince, bool? noClientId}); @@ -49,10 +50,11 @@ class PartyRemoteDataSourceImpl implements PartyRemoteDataSource { final response = await dio.get('parties', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Party.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.party, ); allItems.addAll(paginatedResponse.data); @@ -102,7 +104,6 @@ class PartyRemoteDataSourceImpl implements PartyRemoteDataSource { Future updateParty(Party party) async { final data = { 'name': party.name, - 'client_id': party.clientId, 'description': party.description, if (party.icon != null) ...{ 'icon': party.icon?.content, diff --git a/lib/data/datasources/reminder/reminder_remote_datasource.dart b/lib/data/datasources/reminder/reminder_remote_datasource.dart index 58498cfb..c08f0a03 100644 --- a/lib/data/datasources/reminder/reminder_remote_datasource.dart +++ b/lib/data/datasources/reminder/reminder_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/core/utils/json_defaults.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class ReminderRemoteDataSource { Future> getAllReminders({ @@ -52,10 +53,11 @@ class ReminderRemoteDataSourceImpl implements ReminderRemoteDataSource { final response = await dio.get('reminders', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => Reminder.fromJson( JsonDefaultsHelper.addDefaults(json! as Map)), + entityType: SyncEntity.reminder, ); allItems.addAll(paginatedResponse.data); @@ -74,10 +76,12 @@ class ReminderRemoteDataSourceImpl implements ReminderRemoteDataSource { return Reminder.fromJson(apiResponse.data); } - Map _writeData(Reminder r, {bool includeCreatedAt = false}) { + /// [forCreate] gates the two fields only `POST /reminders` accepts: + /// created_at, and the client id — see [claimClientId] for repointing it. + Map _writeData(Reminder r, {bool forCreate = false}) { return { 'title': r.title, - 'client_id': r.clientId, + if (forCreate) 'client_id': r.clientId, if (r.description != null) 'description': r.description, 'type': r.type, if (r.triggerAt != null) @@ -86,8 +90,7 @@ class ReminderRemoteDataSourceImpl implements ReminderRemoteDataSource { 'repeat_rule': r.repeatRule, if (r.timezone != null) 'timezone': r.timezone, 'priority': r.priority, - if (includeCreatedAt) - 'created_at': formatServerIsoDateTimeString(r.createdAt), + if (forCreate) 'created_at': formatServerIsoDateTimeString(r.createdAt), }; } @@ -95,7 +98,7 @@ class ReminderRemoteDataSourceImpl implements ReminderRemoteDataSource { Future insertReminder(Reminder reminder) async { final response = await dio.post( 'reminders', - data: _writeData(reminder, includeCreatedAt: true), + data: _writeData(reminder, forCreate: true), ); final apiResponse = ApiResponse.fromJson(response.data); return Reminder.fromJson(apiResponse.data); diff --git a/lib/data/datasources/transaction/dto/transaction_complete_dto.dart b/lib/data/datasources/transaction/dto/transaction_complete_dto.dart index 422ea167..1b595dc5 100644 --- a/lib/data/datasources/transaction/dto/transaction_complete_dto.dart +++ b/lib/data/datasources/transaction/dto/transaction_complete_dto.dart @@ -161,10 +161,12 @@ class TransactionCompleteDto with _$TransactionCompleteDto { return _$TransactionCompleteDtoToJson(this); } - Map toServerJson() { + /// The client id is for creates only: repointing a record's client id is + /// the claim endpoint's job, not a side effect of an edit. + Map toServerJson({bool includeClientId = true}) { final data = { ...transaction.toJson(), - 'client_id': transaction.clientId, + if (includeClientId) 'client_id': transaction.clientId, 'type': transaction.type.serverKey, 'datetime': transaction.datetime != null ? formatServerIsoDateTimeString(transaction.datetime!) @@ -180,6 +182,12 @@ class TransactionCompleteDto with _$TransactionCompleteDto { 'group_id': group?.id, }; + // `transaction.toJson()` spreads the drift row, which names the client id + // client_generated_id; drop that too so an update carries neither spelling. + if (!includeClientId) { + data.remove('client_generated_id'); + } + // Refund state is set via the dedicated endpoint, not a normal write request. data.remove('is_refund'); data.remove('refund_of_transaction_id'); diff --git a/lib/data/datasources/transaction/transaction_remote_datasource.dart b/lib/data/datasources/transaction/transaction_remote_datasource.dart index 818a4888..6374545f 100644 --- a/lib/data/datasources/transaction/transaction_remote_datasource.dart +++ b/lib/data/datasources/transaction/transaction_remote_datasource.dart @@ -8,6 +8,7 @@ import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/data/datasources/transaction/dto/transaction_complete_dto.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; /// Booleans as '1'/'0': Laravel's boolean rule rejects 'true'/'false'. String formDataFieldValue(dynamic value) { @@ -37,12 +38,12 @@ abstract class TransactionRemoteDataSource { Future deleteTransaction(int id); - Future addMediaToTransaction( + Future addMediaToTransaction( int transactionId, MediaFile media, ); - Future deleteMediaFromTransaction( + Future deleteMediaFromTransaction( int transactionId, int fileId, ); @@ -71,8 +72,8 @@ class TransactionRemoteDataSourceImpl implements TransactionRemoteDataSource { Future> getAllTransactions( {DateTime? syncedSince, bool? noClientId}) async { final allItems = []; - await for (final page - in getAllTransactionsStream(syncedSince: syncedSince, noClientId: noClientId)) { + await for (final page in getAllTransactionsStream( + syncedSince: syncedSince, noClientId: noClientId)) { allItems.addAll(page); } return allItems; @@ -88,22 +89,22 @@ class TransactionRemoteDataSourceImpl implements TransactionRemoteDataSource { 'page': currentPage, }; if (syncedSince != null) { - queryParams['synced_since'] = formatServerIsoDateTimeString(syncedSince); + queryParams['synced_since'] = + formatServerIsoDateTimeString(syncedSince); } if (noClientId != null) { queryParams['no_client_id'] = noClientId; } - - final response = await dio.get('transactions', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => TransactionCompleteDto.fromServerJson( json! as Map), + entityType: SyncEntity.transaction, ); if (paginatedResponse.data.isNotEmpty) { @@ -115,7 +116,6 @@ class TransactionRemoteDataSourceImpl implements TransactionRemoteDataSource { } currentPage++; } - } @override @@ -171,7 +171,7 @@ class TransactionRemoteDataSourceImpl implements TransactionRemoteDataSource { TransactionCompleteDto transaction) async { var response = await dio.put( 'transactions/${transaction.transaction.id}', - data: transaction.toServerJson(), + data: transaction.toServerJson(includeClientId: false), ); final data = response.data; diff --git a/lib/data/datasources/transfer/dto/transfer_dto.dart b/lib/data/datasources/transfer/dto/transfer_dto.dart index a4e9d451..8a7c919a 100644 --- a/lib/data/datasources/transfer/dto/transfer_dto.dart +++ b/lib/data/datasources/transfer/dto/transfer_dto.dart @@ -2,6 +2,7 @@ import 'package:freezed_annotation/freezed_annotation.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/database/tables/sync_table.dart'; import 'package:trakli/data/datasources/core/amount_parser.dart'; +import 'package:trakli/data/datasources/core/util.dart'; import 'package:trakli/data/datasources/wallet/dtos/wallet_dto.dart'; part 'transfer_dto.freezed.dart'; @@ -16,8 +17,10 @@ class TransferDto with _$TransferDto { @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) required String clientId, String? rev, - @JsonKey(name: 'created_at') required DateTime createdAt, - @JsonKey(name: 'updated_at') required DateTime updatedAt, + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + DateTime? createdAt, + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + DateTime? updatedAt, @JsonKey(name: 'deleted_at') DateTime? deletedAt, @JsonKey(name: 'last_synced_at') DateTime? lastSyncedAt, @JsonKey(fromJson: parseAmount) required double amount, @@ -29,7 +32,7 @@ class TransferDto with _$TransferDto { @JsonKey(name: 'to_wallet_client_id') String? toWalletClientId, @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) double? exchangeRate, - required DateTime datetime, + @JsonKey(fromJson: safeParseDateTime) DateTime? datetime, @JsonKey(name: 'expense_transaction_client_id') String? expenseTransactionClientId, @JsonKey(name: 'income_transaction_client_id') @@ -61,13 +64,28 @@ class TransferDto with _$TransferDto { incomeTransactionClientId: transfer.incomeTransactionClientId, ); + /// The server's `transfers.datetime` column is nullable and was only + /// populated from the 2026-02-23 backend change onwards, so transfers + /// created before then arrive as `null`. The date columns are all + /// non-nullable locally, and a single unparseable row used to throw out of + /// the page parse and fail the whole transfer down-sync, so resolve them + /// here instead of casting in the generated parser. + DateTime get _createdAt => createdAt ?? updatedAt ?? datetime ?? _epoch; + + DateTime get _updatedAt => updatedAt ?? _createdAt; + + DateTime get _datetime => datetime ?? _createdAt; + + static final DateTime _epoch = + DateTime.fromMillisecondsSinceEpoch(0, isUtc: true); + Transfer toTransfer() => Transfer( id: id, userId: userId, clientId: clientId, rev: rev, - createdAt: createdAt, - updatedAt: updatedAt, + createdAt: _createdAt, + updatedAt: _updatedAt, deletedAt: deletedAt, lastSyncedAt: lastSyncedAt, amount: amount, @@ -76,7 +94,7 @@ class TransferDto with _$TransferDto { fromWalletClientId: sourceWallet?.clientId ?? fromWalletClientId, toWalletClientId: destinationWallet?.clientId ?? toWalletClientId, exchangeRate: exchangeRate, - datetime: datetime, + datetime: _datetime, expenseTransactionClientId: expenseTransactionClientId, incomeTransactionClientId: incomeTransactionClientId, ); diff --git a/lib/data/datasources/transfer/dto/transfer_dto.freezed.dart b/lib/data/datasources/transfer/dto/transfer_dto.freezed.dart index 4b157281..9d2b197b 100644 --- a/lib/data/datasources/transfer/dto/transfer_dto.freezed.dart +++ b/lib/data/datasources/transfer/dto/transfer_dto.freezed.dart @@ -26,10 +26,10 @@ mixin _$TransferDto { @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) String get clientId => throw _privateConstructorUsedError; String? get rev => throw _privateConstructorUsedError; - @JsonKey(name: 'created_at') - DateTime get createdAt => throw _privateConstructorUsedError; - @JsonKey(name: 'updated_at') - DateTime get updatedAt => throw _privateConstructorUsedError; + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + DateTime? get createdAt => throw _privateConstructorUsedError; + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + DateTime? get updatedAt => throw _privateConstructorUsedError; @JsonKey(name: 'deleted_at') DateTime? get deletedAt => throw _privateConstructorUsedError; @JsonKey(name: 'last_synced_at') @@ -50,7 +50,8 @@ mixin _$TransferDto { String? get toWalletClientId => throw _privateConstructorUsedError; @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) double? get exchangeRate => throw _privateConstructorUsedError; - DateTime get datetime => throw _privateConstructorUsedError; + @JsonKey(fromJson: safeParseDateTime) + DateTime? get datetime => throw _privateConstructorUsedError; @JsonKey(name: 'expense_transaction_client_id') String? get expenseTransactionClientId => throw _privateConstructorUsedError; @JsonKey(name: 'income_transaction_client_id') @@ -78,8 +79,10 @@ abstract class $TransferDtoCopyWith<$Res> { @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) String clientId, String? rev, - @JsonKey(name: 'created_at') DateTime createdAt, - @JsonKey(name: 'updated_at') DateTime updatedAt, + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + DateTime? createdAt, + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + DateTime? updatedAt, @JsonKey(name: 'deleted_at') DateTime? deletedAt, @JsonKey(name: 'last_synced_at') DateTime? lastSyncedAt, @JsonKey(fromJson: parseAmount) double amount, @@ -91,7 +94,7 @@ abstract class $TransferDtoCopyWith<$Res> { @JsonKey(name: 'to_wallet_client_id') String? toWalletClientId, @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) double? exchangeRate, - DateTime datetime, + @JsonKey(fromJson: safeParseDateTime) DateTime? datetime, @JsonKey(name: 'expense_transaction_client_id') String? expenseTransactionClientId, @JsonKey(name: 'income_transaction_client_id') @@ -120,8 +123,8 @@ class _$TransferDtoCopyWithImpl<$Res, $Val extends TransferDto> Object? userId = freezed, Object? clientId = null, Object? rev = freezed, - Object? createdAt = null, - Object? updatedAt = null, + Object? createdAt = freezed, + Object? updatedAt = freezed, Object? deletedAt = freezed, Object? lastSyncedAt = freezed, Object? amount = null, @@ -132,7 +135,7 @@ class _$TransferDtoCopyWithImpl<$Res, $Val extends TransferDto> Object? fromWalletClientId = freezed, Object? toWalletClientId = freezed, Object? exchangeRate = freezed, - Object? datetime = null, + Object? datetime = freezed, Object? expenseTransactionClientId = freezed, Object? incomeTransactionClientId = freezed, }) { @@ -153,14 +156,14 @@ class _$TransferDtoCopyWithImpl<$Res, $Val extends TransferDto> ? _value.rev : rev // ignore: cast_nullable_to_non_nullable as String?, - createdAt: null == createdAt + createdAt: freezed == createdAt ? _value.createdAt : createdAt // ignore: cast_nullable_to_non_nullable - as DateTime, - updatedAt: null == updatedAt + as DateTime?, + updatedAt: freezed == updatedAt ? _value.updatedAt : updatedAt // ignore: cast_nullable_to_non_nullable - as DateTime, + as DateTime?, deletedAt: freezed == deletedAt ? _value.deletedAt : deletedAt // ignore: cast_nullable_to_non_nullable @@ -201,10 +204,10 @@ class _$TransferDtoCopyWithImpl<$Res, $Val extends TransferDto> ? _value.exchangeRate : exchangeRate // ignore: cast_nullable_to_non_nullable as double?, - datetime: null == datetime + datetime: freezed == datetime ? _value.datetime : datetime // ignore: cast_nullable_to_non_nullable - as DateTime, + as DateTime?, expenseTransactionClientId: freezed == expenseTransactionClientId ? _value.expenseTransactionClientId : expenseTransactionClientId // ignore: cast_nullable_to_non_nullable @@ -259,8 +262,10 @@ abstract class _$$TransferDtoImplCopyWith<$Res> @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) String clientId, String? rev, - @JsonKey(name: 'created_at') DateTime createdAt, - @JsonKey(name: 'updated_at') DateTime updatedAt, + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + DateTime? createdAt, + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + DateTime? updatedAt, @JsonKey(name: 'deleted_at') DateTime? deletedAt, @JsonKey(name: 'last_synced_at') DateTime? lastSyncedAt, @JsonKey(fromJson: parseAmount) double amount, @@ -272,7 +277,7 @@ abstract class _$$TransferDtoImplCopyWith<$Res> @JsonKey(name: 'to_wallet_client_id') String? toWalletClientId, @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) double? exchangeRate, - DateTime datetime, + @JsonKey(fromJson: safeParseDateTime) DateTime? datetime, @JsonKey(name: 'expense_transaction_client_id') String? expenseTransactionClientId, @JsonKey(name: 'income_transaction_client_id') @@ -301,8 +306,8 @@ class __$$TransferDtoImplCopyWithImpl<$Res> Object? userId = freezed, Object? clientId = null, Object? rev = freezed, - Object? createdAt = null, - Object? updatedAt = null, + Object? createdAt = freezed, + Object? updatedAt = freezed, Object? deletedAt = freezed, Object? lastSyncedAt = freezed, Object? amount = null, @@ -313,7 +318,7 @@ class __$$TransferDtoImplCopyWithImpl<$Res> Object? fromWalletClientId = freezed, Object? toWalletClientId = freezed, Object? exchangeRate = freezed, - Object? datetime = null, + Object? datetime = freezed, Object? expenseTransactionClientId = freezed, Object? incomeTransactionClientId = freezed, }) { @@ -334,14 +339,14 @@ class __$$TransferDtoImplCopyWithImpl<$Res> ? _value.rev : rev // ignore: cast_nullable_to_non_nullable as String?, - createdAt: null == createdAt + createdAt: freezed == createdAt ? _value.createdAt : createdAt // ignore: cast_nullable_to_non_nullable - as DateTime, - updatedAt: null == updatedAt + as DateTime?, + updatedAt: freezed == updatedAt ? _value.updatedAt : updatedAt // ignore: cast_nullable_to_non_nullable - as DateTime, + as DateTime?, deletedAt: freezed == deletedAt ? _value.deletedAt : deletedAt // ignore: cast_nullable_to_non_nullable @@ -382,10 +387,10 @@ class __$$TransferDtoImplCopyWithImpl<$Res> ? _value.exchangeRate : exchangeRate // ignore: cast_nullable_to_non_nullable as double?, - datetime: null == datetime + datetime: freezed == datetime ? _value.datetime : datetime // ignore: cast_nullable_to_non_nullable - as DateTime, + as DateTime?, expenseTransactionClientId: freezed == expenseTransactionClientId ? _value.expenseTransactionClientId : expenseTransactionClientId // ignore: cast_nullable_to_non_nullable @@ -408,8 +413,8 @@ class _$TransferDtoImpl extends _TransferDto { @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) required this.clientId, this.rev, - @JsonKey(name: 'created_at') required this.createdAt, - @JsonKey(name: 'updated_at') required this.updatedAt, + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) this.createdAt, + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) this.updatedAt, @JsonKey(name: 'deleted_at') this.deletedAt, @JsonKey(name: 'last_synced_at') this.lastSyncedAt, @JsonKey(fromJson: parseAmount) required this.amount, @@ -421,7 +426,7 @@ class _$TransferDtoImpl extends _TransferDto { @JsonKey(name: 'to_wallet_client_id') this.toWalletClientId, @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) this.exchangeRate, - required this.datetime, + @JsonKey(fromJson: safeParseDateTime) this.datetime, @JsonKey(name: 'expense_transaction_client_id') this.expenseTransactionClientId, @JsonKey(name: 'income_transaction_client_id') @@ -442,11 +447,11 @@ class _$TransferDtoImpl extends _TransferDto { @override final String? rev; @override - @JsonKey(name: 'created_at') - final DateTime createdAt; + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + final DateTime? createdAt; @override - @JsonKey(name: 'updated_at') - final DateTime updatedAt; + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + final DateTime? updatedAt; @override @JsonKey(name: 'deleted_at') final DateTime? deletedAt; @@ -478,7 +483,8 @@ class _$TransferDtoImpl extends _TransferDto { @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) final double? exchangeRate; @override - final DateTime datetime; + @JsonKey(fromJson: safeParseDateTime) + final DateTime? datetime; @override @JsonKey(name: 'expense_transaction_client_id') final String? expenseTransactionClientId; @@ -583,8 +589,10 @@ abstract class _TransferDto extends TransferDto { @JsonKey(name: 'client_generated_id', defaultValue: defaultClientId) required final String clientId, final String? rev, - @JsonKey(name: 'created_at') required final DateTime createdAt, - @JsonKey(name: 'updated_at') required final DateTime updatedAt, + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + final DateTime? createdAt, + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + final DateTime? updatedAt, @JsonKey(name: 'deleted_at') final DateTime? deletedAt, @JsonKey(name: 'last_synced_at') final DateTime? lastSyncedAt, @JsonKey(fromJson: parseAmount) required final double amount, @@ -596,7 +604,7 @@ abstract class _TransferDto extends TransferDto { @JsonKey(name: 'to_wallet_client_id') final String? toWalletClientId, @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) final double? exchangeRate, - required final DateTime datetime, + @JsonKey(fromJson: safeParseDateTime) final DateTime? datetime, @JsonKey(name: 'expense_transaction_client_id') final String? expenseTransactionClientId, @JsonKey(name: 'income_transaction_client_id') @@ -617,11 +625,11 @@ abstract class _TransferDto extends TransferDto { @override String? get rev; @override - @JsonKey(name: 'created_at') - DateTime get createdAt; + @JsonKey(name: 'created_at', fromJson: safeParseDateTime) + DateTime? get createdAt; @override - @JsonKey(name: 'updated_at') - DateTime get updatedAt; + @JsonKey(name: 'updated_at', fromJson: safeParseDateTime) + DateTime? get updatedAt; @override @JsonKey(name: 'deleted_at') DateTime? get deletedAt; @@ -653,7 +661,8 @@ abstract class _TransferDto extends TransferDto { @JsonKey(name: 'exchange_rate', fromJson: parseAmountNullable) double? get exchangeRate; @override - DateTime get datetime; + @JsonKey(fromJson: safeParseDateTime) + DateTime? get datetime; @override @JsonKey(name: 'expense_transaction_client_id') String? get expenseTransactionClientId; diff --git a/lib/data/datasources/transfer/dto/transfer_dto.g.dart b/lib/data/datasources/transfer/dto/transfer_dto.g.dart index 25480711..794b4924 100644 --- a/lib/data/datasources/transfer/dto/transfer_dto.g.dart +++ b/lib/data/datasources/transfer/dto/transfer_dto.g.dart @@ -12,8 +12,8 @@ _$TransferDtoImpl _$$TransferDtoImplFromJson(Map json) => userId: (json['user_id'] as num?)?.toInt(), clientId: json['client_generated_id'] as String? ?? '', rev: json['rev'] as String?, - createdAt: DateTime.parse(json['created_at'] as String), - updatedAt: DateTime.parse(json['updated_at'] as String), + createdAt: safeParseDateTime(json['created_at']), + updatedAt: safeParseDateTime(json['updated_at']), deletedAt: json['deleted_at'] == null ? null : DateTime.parse(json['deleted_at'] as String), @@ -33,7 +33,7 @@ _$TransferDtoImpl _$$TransferDtoImplFromJson(Map json) => fromWalletClientId: json['from_wallet_client_id'] as String?, toWalletClientId: json['to_wallet_client_id'] as String?, exchangeRate: parseAmountNullable(json['exchange_rate']), - datetime: DateTime.parse(json['datetime'] as String), + datetime: safeParseDateTime(json['datetime']), expenseTransactionClientId: json['expense_transaction_client_id'] as String?, incomeTransactionClientId: @@ -46,8 +46,8 @@ Map _$$TransferDtoImplToJson(_$TransferDtoImpl instance) => 'user_id': instance.userId, 'client_generated_id': instance.clientId, 'rev': instance.rev, - 'created_at': instance.createdAt.toIso8601String(), - 'updated_at': instance.updatedAt.toIso8601String(), + 'created_at': instance.createdAt?.toIso8601String(), + 'updated_at': instance.updatedAt?.toIso8601String(), 'deleted_at': instance.deletedAt?.toIso8601String(), 'last_synced_at': instance.lastSyncedAt?.toIso8601String(), 'amount': instance.amount, @@ -58,7 +58,7 @@ Map _$$TransferDtoImplToJson(_$TransferDtoImpl instance) => 'from_wallet_client_id': instance.fromWalletClientId, 'to_wallet_client_id': instance.toWalletClientId, 'exchange_rate': instance.exchangeRate, - 'datetime': instance.datetime.toIso8601String(), + 'datetime': instance.datetime?.toIso8601String(), 'expense_transaction_client_id': instance.expenseTransactionClientId, 'income_transaction_client_id': instance.incomeTransactionClientId, }; diff --git a/lib/data/datasources/transfer/transfer_remote_datasource.dart b/lib/data/datasources/transfer/transfer_remote_datasource.dart index 29115bba..2012326c 100644 --- a/lib/data/datasources/transfer/transfer_remote_datasource.dart +++ b/lib/data/datasources/transfer/transfer_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/data/datasources/transfer/dto/transfer_dto.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class TransferRemoteDataSource { Future> getAllTransfers({ @@ -73,10 +74,11 @@ class TransferRemoteDataSourceImpl implements TransferRemoteDataSource { final response = await dio.get('transfers', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => TransferDto.fromJson(json! as Map).toTransfer(), + entityType: SyncEntity.transfer, ); if (paginatedResponse.data.isNotEmpty) { @@ -110,7 +112,7 @@ class TransferRemoteDataSourceImpl implements TransferRemoteDataSource { Future updateTransfer(Transfer transfer) async { final response = await dio.put( 'transfers/${transfer.id}', - data: toServerJson(transfer), + data: toServerJson(transfer, includeClientId: false), ); final apiResponse = ApiResponse.fromJson(response.data); return TransferDto.fromJson(apiResponse.data as Map) @@ -138,9 +140,12 @@ class TransferRemoteDataSourceImpl implements TransferRemoteDataSource { } } -Map toServerJson(Transfer transfer) { +/// The client id is for creates only: repointing a record's client id is the +/// claim endpoint's job, not a side effect of an edit. +Map toServerJson(Transfer transfer, + {bool includeClientId = true}) { return { - 'client_id': transfer.clientId, + if (includeClientId) 'client_id': transfer.clientId, 'amount': transfer.amount, 'from_wallet_id': transfer.fromWalletId, 'to_wallet_id': transfer.toWalletId, diff --git a/lib/data/datasources/wallet/wallet_local_datasource.dart b/lib/data/datasources/wallet/wallet_local_datasource.dart index aef67275..2c539257 100644 --- a/lib/data/datasources/wallet/wallet_local_datasource.dart +++ b/lib/data/datasources/wallet/wallet_local_datasource.dart @@ -6,6 +6,7 @@ import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/presentation/utils/enums.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; +import 'package:trakli/core/error/exceptions.dart'; abstract class WalletLocalDataSource { Future> getAllWallets(); @@ -52,6 +53,27 @@ class WalletLocalDataSourceImpl implements WalletLocalDataSource { return await query.getSingleOrNull(); } + /// The same name and currency is the same wallet, case and surrounding + /// spaces aside. + Future _findByServerIdentity( + String name, + String currency, { + String? excluding, + }) { + final normalizedName = name.trim().toLowerCase(); + final normalizedCurrency = currency.trim().toLowerCase(); + return (database.select(database.wallets) + ..where((w) { + final matches = w.name.trim().lower().equals(normalizedName) & + w.currency.trim().lower().equals(normalizedCurrency); + return excluding == null + ? matches + : matches & w.clientId.isNotValue(excluding); + }) + ..limit(1)) + .getSingleOrNull(); + } + @override Future insertWallet( String name, @@ -61,13 +83,18 @@ class WalletLocalDataSourceImpl implements WalletLocalDataSource { String? description, Media? icon, }) async { + final existing = await _findByServerIdentity(name, currency); + if (existing != null) { + throw DuplicateException('Wallet "$name" in $currency already exists'); + } + DateTime dateTime = getNewFormattedUtcDateTime(); final companion = WalletsCompanion.insert( clientId: Value( await generateDeviceScopedId(), ), - name: name, + name: name.trim(), type: type, balance: Value(balance), currency: currency, @@ -92,10 +119,30 @@ class WalletLocalDataSourceImpl implements WalletLocalDataSource { String? description, Media? icon, }) async { + // Name and currency identify a wallet together, so check the pair the + // row will hold afterwards, not just the field being changed. + if (name != null || currency != null) { + final current = await getWallet(clientId); + if (current != null) { + final effectiveName = name ?? current.name; + final effectiveCurrency = currency ?? current.currency; + final existing = await _findByServerIdentity( + effectiveName, + effectiveCurrency, + excluding: clientId, + ); + if (existing != null) { + throw DuplicateException( + 'Wallet "$effectiveName" in $effectiveCurrency already exists', + ); + } + } + } + DateTime dateTime = getNewFormattedUtcDateTime(); final companion = WalletsCompanion( - name: name != null ? Value(name) : const Value.absent(), + name: name != null ? Value(name.trim()) : const Value.absent(), type: type != null ? Value(type) : const Value.absent(), balance: balance != null ? Value(balance) : const Value.absent(), currency: currency != null ? Value(currency) : const Value.absent(), diff --git a/lib/data/datasources/wallet/wallet_remote_datasource.dart b/lib/data/datasources/wallet/wallet_remote_datasource.dart index 49c7aa4d..0a99065f 100644 --- a/lib/data/datasources/wallet/wallet_remote_datasource.dart +++ b/lib/data/datasources/wallet/wallet_remote_datasource.dart @@ -5,6 +5,7 @@ import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/core/api_response.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/data/datasources/wallet/dtos/wallet_dto.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; abstract class WalletRemoteDataSource { Future> getAllWallets({DateTime? syncedSince, bool? noClientId}); @@ -48,10 +49,11 @@ class WalletRemoteDataSourceImpl implements WalletRemoteDataSource { final response = await dio.get('wallets', queryParameters: queryParams); final apiResponse = ApiResponse.fromJson(response.data); - final paginatedResponse = PaginationResponse.fromJson( + final paginatedResponse = PaginationResponse.lenient( apiResponse.data as Map, (Object? json) => WalletDto.fromJson(json! as Map).toModel(), + entityType: SyncEntity.wallet, ); allItems.addAll(paginatedResponse.data); @@ -109,7 +111,6 @@ class WalletRemoteDataSourceImpl implements WalletRemoteDataSource { if (wallet.description != null) 'description': wallet.description, 'icon': wallet.icon?.content, 'icon_type': wallet.icon?.type.name, - 'client_id': wallet.clientId, }; final response = await dio.put( diff --git a/lib/data/sync/category_sync_handler.dart b/lib/data/sync/category_sync_handler.dart index e6c85567..56e0aa95 100644 --- a/lib/data/sync/category_sync_handler.dart +++ b/lib/data/sync/category_sync_handler.dart @@ -121,7 +121,13 @@ class CategorySyncHandler extends SyncTypeHandler icon: Value(entity.icon), ); - await table.insertOne(category, mode: InsertMode.insertOrReplace); + await db.transaction(() async { + await db.adoptCategoryServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOne(category, mode: InsertMode.insertOrReplace); + }); } @override @@ -146,7 +152,13 @@ class CategorySyncHandler extends SyncTypeHandler lastSyncedAt: Value(entity.lastSyncedAt), icon: Value(entity.icon), ); - await table.insertOnConflictUpdate(category); + await db.transaction(() async { + await db.adoptCategoryServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOnConflictUpdate(category); + }); } } } diff --git a/lib/data/sync/config_sync_handler.dart b/lib/data/sync/config_sync_handler.dart index c0697ace..fa768c1e 100644 --- a/lib/data/sync/config_sync_handler.dart +++ b/lib/data/sync/config_sync_handler.dart @@ -58,6 +58,15 @@ class ConfigSyncHandler extends SyncTypeHandler return await remoteDataSource.getConfig(id.toString()); } + @override + Future claimClientId(Config entity) { + return remoteDataSource.claimClientId( + key: entity.key, + clientId: entity.clientId, + updatedAt: entity.updatedAt, + ); + } + @override Future restPutRemote(Config entity) async { if (entity.id == null) { diff --git a/lib/data/sync/group_sync_handler.dart b/lib/data/sync/group_sync_handler.dart index 17fc983e..12b323e9 100644 --- a/lib/data/sync/group_sync_handler.dart +++ b/lib/data/sync/group_sync_handler.dart @@ -101,7 +101,13 @@ class GroupSyncHandler extends SyncTypeHandler lastSyncedAt: Value(entity.lastSyncedAt), ); - await table.insertOne(group, mode: InsertMode.insertOrReplace); + await db.transaction(() async { + await db.adoptGroupServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOne(group, mode: InsertMode.insertOrReplace); + }); } @override @@ -126,7 +132,13 @@ class GroupSyncHandler extends SyncTypeHandler icon: Value(entity.icon), lastSyncedAt: Value(entity.lastSyncedAt), ); - await table.insertOnConflictUpdate(group); + await db.transaction(() async { + await db.adoptGroupServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOnConflictUpdate(group); + }); } } } diff --git a/lib/data/sync/party_sync_handler.dart b/lib/data/sync/party_sync_handler.dart index 20f928fe..4548d279 100644 --- a/lib/data/sync/party_sync_handler.dart +++ b/lib/data/sync/party_sync_handler.dart @@ -87,7 +87,13 @@ class PartySyncHandler extends SyncTypeHandler @override Future upsertLocal(Party entity) async { - await table.insertOne(entity, mode: InsertMode.insertOrReplace); + await db.transaction(() async { + await db.adoptPartyServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOne(entity, mode: InsertMode.insertOrReplace); + }); } @override @@ -101,7 +107,13 @@ class PartySyncHandler extends SyncTypeHandler // If the entity is marked as deleted, remove it locally await table.deleteWhere((p) => p.clientId.equals(entity.clientId)); } else { - await table.insertOnConflictUpdate(entity); + await db.transaction(() async { + await db.adoptPartyServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOnConflictUpdate(entity); + }); } } } diff --git a/lib/data/sync/transaction_sync_handler.dart b/lib/data/sync/transaction_sync_handler.dart index 8c3a7d3f..9a0b1902 100644 --- a/lib/data/sync/transaction_sync_handler.dart +++ b/lib/data/sync/transaction_sync_handler.dart @@ -242,15 +242,29 @@ class TransactionSyncHandler mode: InsertMode.insertOrReplace); } + // Written verbatim, so each must take its server id from whichever + // local copy holds it, or a stale snapshot would resurrect it. + await db.adoptWalletServerId( + serverId: entity.wallet.id, + clientId: entity.wallet.clientId, + ); await db.wallets .insertOne(entity.wallet, mode: InsertMode.insertOrReplace); if (entity.party != null) { + await db.adoptPartyServerId( + serverId: entity.party!.id, + clientId: entity.party!.clientId, + ); await db.parties .insertOne(entity.party!, mode: InsertMode.insertOrReplace); } if (entity.group != null) { + await db.adoptGroupServerId( + serverId: entity.group!.id, + clientId: entity.group!.clientId, + ); await db.groups .insertOne(entity.group!, mode: InsertMode.insertOrReplace); } diff --git a/lib/data/sync/wallet_sync_handler.dart b/lib/data/sync/wallet_sync_handler.dart index 139ebb47..59328223 100644 --- a/lib/data/sync/wallet_sync_handler.dart +++ b/lib/data/sync/wallet_sync_handler.dart @@ -125,7 +125,13 @@ class WalletSyncHandler extends SyncTypeHandler stats: Value(entity.stats), icon: Value(entity.icon), ); - await table.insertOne(companion, mode: InsertMode.insertOrReplace); + await db.transaction(() async { + await db.adoptWalletServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOne(companion, mode: InsertMode.insertOrReplace); + }); } @override @@ -155,7 +161,13 @@ class WalletSyncHandler extends SyncTypeHandler stats: Value(entity.stats), icon: Value(entity.icon), ); - await table.insertOnConflictUpdate(companion); + await db.transaction(() async { + await db.adoptWalletServerId( + serverId: entity.id, + clientId: entity.clientId, + ); + await table.insertOnConflictUpdate(companion); + }); } } } diff --git a/lib/gen/translations/codegen_loader.g.dart b/lib/gen/translations/codegen_loader.g.dart index 15ddf2d4..1b14eade 100644 --- a/lib/gen/translations/codegen_loader.g.dart +++ b/lib/gen/translations/codegen_loader.g.dart @@ -335,6 +335,9 @@ abstract class LocaleKeys { static const deleteGroupConfirm = 'deleteGroupConfirm'; static const duplicate = 'duplicate'; static const categoryNameAlreadyExists = 'categoryNameAlreadyExists'; + static const walletNameAlreadyExists = 'walletNameAlreadyExists'; + static const groupNameAlreadyExists = 'groupNameAlreadyExists'; + static const partyNameAlreadyExists = 'partyNameAlreadyExists'; static const deleteWallet = 'deleteWallet'; static const deleteWalletConfirm = 'deleteWalletConfirm'; static const defaultName = 'defaultName'; diff --git a/lib/gen/translations/locale_keys.g.dart b/lib/gen/translations/locale_keys.g.dart index 15ddf2d4..1b14eade 100644 --- a/lib/gen/translations/locale_keys.g.dart +++ b/lib/gen/translations/locale_keys.g.dart @@ -335,6 +335,9 @@ abstract class LocaleKeys { static const deleteGroupConfirm = 'deleteGroupConfirm'; static const duplicate = 'duplicate'; static const categoryNameAlreadyExists = 'categoryNameAlreadyExists'; + static const walletNameAlreadyExists = 'walletNameAlreadyExists'; + static const groupNameAlreadyExists = 'groupNameAlreadyExists'; + static const partyNameAlreadyExists = 'partyNameAlreadyExists'; static const deleteWallet = 'deleteWallet'; static const deleteWalletConfirm = 'deleteWalletConfirm'; static const defaultName = 'defaultName'; diff --git a/lib/presentation/category/add_category_screen.dart b/lib/presentation/category/add_category_screen.dart index f133a1b5..37a1ee55 100644 --- a/lib/presentation/category/add_category_screen.dart +++ b/lib/presentation/category/add_category_screen.dart @@ -28,7 +28,7 @@ class AddCategoryScreen extends StatefulWidget { class _AddCategoryScreenState extends State { String _generateSlug(String name) { - return name.toLowerCase().replaceAll(' ', '-'); + return name.trim().toLowerCase().replaceAll(RegExp(r'\s+'), '-'); } @override diff --git a/lib/presentation/onboarding/widgets/category_setup_widget.dart b/lib/presentation/onboarding/widgets/category_setup_widget.dart index a5da6e6a..c244f9f6 100644 --- a/lib/presentation/onboarding/widgets/category_setup_widget.dart +++ b/lib/presentation/onboarding/widgets/category_setup_widget.dart @@ -28,7 +28,7 @@ class _CategorySetupWidgetState extends State { bool _isCreating = false; String _generateSlug(String name) { - return name.toLowerCase().replaceAll(' ', '-'); + return name.trim().toLowerCase().replaceAll(RegExp(r'\s+'), '-'); } Future _createDefaultCategoriesIfNeeded() async { diff --git a/lib/presentation/parties/widgets/add_party_form.dart b/lib/presentation/parties/widgets/add_party_form.dart index 63b8201c..5b884d03 100644 --- a/lib/presentation/parties/widgets/add_party_form.dart +++ b/lib/presentation/parties/widgets/add_party_form.dart @@ -92,7 +92,10 @@ class _AddPartyFormState extends State { if (state.failure.hasError) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( - content: Text(state.failure.customMessage), + content: Text(state.failure.maybeMap( + orElse: () => state.failure.customMessage, + duplicate: (_) => LocaleKeys.partyNameAlreadyExists.tr(), + )), backgroundColor: Colors.red, ), ); diff --git a/lib/presentation/sync_history_screen.dart b/lib/presentation/sync_history_screen.dart index 7f2581bb..14aa7cc9 100644 --- a/lib/presentation/sync_history_screen.dart +++ b/lib/presentation/sync_history_screen.dart @@ -128,6 +128,80 @@ class _SyncHistoryScreenState extends State { _triggerSync(); } + /// A category the API rejected as a duplicate can never sync, and it holds + /// up every transaction tagged with it. Fold it into the copy the server did + /// accept — matched on name the way the API compares them, ignoring case and + /// surrounding spaces. + Future _mergeDuplicateCategory(LocalChange change) async { + final duplicate = await (_db.select(_db.categories) + ..where((c) => c.clientId.equals(change.entityId))) + .getSingleOrNull(); + if (duplicate == null) { + _showMessage('That category is no longer on this device.'); + return; + } + + final winner = await (_db.select(_db.categories) + ..where((c) => + c.clientId.isNotValue(duplicate.clientId) & + c.id.isNotNull() & + c.name.trim().lower().equals(duplicate.name.trim().toLowerCase())) + ..limit(1)) + .getSingleOrNull(); + if (winner == null) { + _showMessage( + 'No synced category named "${duplicate.name.trim()}" to merge into. ' + 'Sync once more, then try again.', + ); + return; + } + + if (!mounted) return; + final confirmed = await showDialog( + context: context, + builder: (context) => AlertDialog( + title: const Text('Merge duplicate category'), + content: Text( + '"${duplicate.name}" was rejected by the server as a duplicate of ' + '"${winner.name}".\n\n' + 'Everything tagged with it will be re-tagged to "${winner.name}", ' + 'and the duplicate will be removed from this device.', + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context, false), + child: Text(LocaleKeys.cancel.tr()), + ), + TextButton( + onPressed: () => Navigator.pop(context, true), + child: const Text('Merge'), + ), + ], + ), + ); + if (confirmed != true) return; + + try { + final retagged = await _db.mergeDuplicateCategory( + loserClientId: duplicate.clientId, + winnerClientId: winner.clientId, + ); + _showMessage(retagged == 0 + ? 'Merged into "${winner.name}".' + : 'Merged into "${winner.name}" — $retagged re-tagged.'); + await _loadData(); + await _triggerSync(); + } catch (e) { + _showMessage('Could not merge: $e'); + } + } + + void _showMessage(String message) { + if (!mounted) return; + ScaffoldMessenger.of(context) + .showSnackBar(SnackBar(content: Text(message))); + } + String _formatDateTime(DateTime? dateTime) { if (dateTime == null) return LocaleKeys.notSet.tr(); return DateFormat('MMM d, yyyy HH:mm').format(dateTime.toLocal()); @@ -560,6 +634,8 @@ class _SyncHistoryScreenState extends State { onSelected: (value) { if (value == 'retry') { _retryQuarantinedChange(change); + } else if (value == 'merge') { + _mergeDuplicateCategory(change); } else if (value == 'details') { _showChangeDetails(change); } @@ -575,6 +651,19 @@ class _SyncHistoryScreenState extends State { ], ), ), + // Retrying a name the server already holds just fails + // again; merging is the only way out of this one. + if (change.entityType == 'category') + const PopupMenuItem( + value: 'merge', + child: Row( + children: [ + Icon(Icons.merge_type, size: 18), + SizedBox(width: 8), + Text('Merge duplicate'), + ], + ), + ), PopupMenuItem( value: 'retry', child: Row( diff --git a/lib/presentation/utils/dialogs/add_party_dialog.dart b/lib/presentation/utils/dialogs/add_party_dialog.dart index ac3e3ae4..a1479844 100644 --- a/lib/presentation/utils/dialogs/add_party_dialog.dart +++ b/lib/presentation/utils/dialogs/add_party_dialog.dart @@ -10,7 +10,6 @@ class AddPartyDialog extends StatelessWidget { @override Widget build(BuildContext context) { return Dialog( - backgroundColor: Colors.white, shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(8.r)), child: Padding( padding: const EdgeInsets.all(16.0), diff --git a/lib/presentation/utils/dialogs/custom_range_picker.dart b/lib/presentation/utils/dialogs/custom_range_picker.dart index cdbf8fd4..001c6733 100644 --- a/lib/presentation/utils/dialogs/custom_range_picker.dart +++ b/lib/presentation/utils/dialogs/custom_range_picker.dart @@ -39,7 +39,6 @@ class _CustomRangePickerState extends State { horizontal: 16.w, vertical: 20.h, ), - backgroundColor: Colors.white, shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(16.r), ), diff --git a/lib/presentation/utils/forms/add_groups_form.dart b/lib/presentation/utils/forms/add_groups_form.dart index 22f133bb..3d3c562f 100644 --- a/lib/presentation/utils/forms/add_groups_form.dart +++ b/lib/presentation/utils/forms/add_groups_form.dart @@ -57,7 +57,10 @@ class _AddGroupsFormState extends State { if (state.failure.hasError) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( - content: Text(state.failure.customMessage), + content: Text(state.failure.maybeMap( + orElse: () => state.failure.customMessage, + duplicate: (_) => LocaleKeys.groupNameAlreadyExists.tr(), + )), backgroundColor: Colors.red, ), ); diff --git a/lib/presentation/utils/forms/add_wallet_form.dart b/lib/presentation/utils/forms/add_wallet_form.dart index 1177c945..35476f74 100644 --- a/lib/presentation/utils/forms/add_wallet_form.dart +++ b/lib/presentation/utils/forms/add_wallet_form.dart @@ -101,12 +101,37 @@ class _AddWalletFormState extends State { icon: mediaEntity, ); } - AppNavigator.pop(context); } } @override Widget build(BuildContext context) { + // Close only once the write landed: a rejected duplicate name has to + // stay on screen with its message. + return BlocListener( + listenWhen: (previous, current) => previous.isSaving != current.isSaving, + listener: (context, state) { + if (state.failure.hasError) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text(state.failure.maybeMap( + orElse: () => state.failure.customMessage, + duplicate: (_) => LocaleKeys.walletNameAlreadyExists.tr(), + )), + backgroundColor: Colors.red, + ), + ); + } + + if (!state.isSaving && !state.failure.hasError) { + AppNavigator.pop(context); + } + }, + child: _form(context), + ); + } + + Widget _form(BuildContext context) { return SingleChildScrollView( padding: EdgeInsets.symmetric( horizontal: 16.w, diff --git a/pubspec.lock b/pubspec.lock index c99845b6..3fa15707 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -181,10 +181,10 @@ packages: dependency: transitive description: name: characters - sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 + sha256: faf38497bda5ead2a8c7615f4f7939df04333478bf32e4173fcb06d428b5716b url: "https://pub.dev" source: hosted - version: "1.4.0" + version: "1.4.1" charcode: dependency: transitive description: @@ -1292,10 +1292,10 @@ packages: dependency: transitive description: name: material_color_utilities - sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec + sha256: "9c337007e82b1889149c82ed242ed1cb24a66044e30979c44912381e9be4c48b" url: "https://pub.dev" source: hosted - version: "0.11.1" + version: "0.13.0" meta: dependency: transitive description: @@ -2174,5 +2174,5 @@ packages: source: hosted version: "3.1.3" sdks: - dart: ">=3.8.0 <4.0.0" + dart: ">=3.9.0-0 <4.0.0" flutter: ">=3.32.0" diff --git a/test/unit/category_duplicate_name_test.dart b/test/unit/category_duplicate_name_test.dart new file mode 100644 index 00000000..405f18e2 --- /dev/null +++ b/test/unit/category_duplicate_name_test.dart @@ -0,0 +1,110 @@ +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/core/error/exceptions.dart'; +import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/category/category_local_datasource.dart'; +import 'package:trakli/presentation/utils/enums.dart'; + +void main() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + + late AppDatabase db; + late CategoryLocalDataSourceImpl dataSource; + + setUp(() async { + db = AppDatabase(NativeDatabase.memory()); + dataSource = CategoryLocalDataSourceImpl(db); + + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'WhileSmart', + slug: 'whilesmart', + type: TransactionType.income, + clientId: const Value('device:existing'), + )); + }); + + tearDown(() async { + await db.close(); + }); + + // The API's collation is case-insensitive and ignores trailing spaces, so + // these all come back as "Category already exists" — HTTP 200 carrying the + // category the user already had, whose server id then folds this device's + // older copy away and drops whatever was typed here. Reject them instead. + for (final name in ['Whilesmart ', 'whilesmart', ' WhileSmart ']) { + test('insert rejects "$name" as a duplicate of "WhileSmart"', () { + expect( + () => dataSource.insertCategory( + name, + 'whilesmart', + TransactionType.income, + ), + throwsA(isA()), + ); + }); + } + + // The server checks the name alone, without filtering on type. + test('insert rejects a same-name category of the other type', () { + expect( + () => dataSource.insertCategory( + 'whilesmart', + 'whilesmart', + TransactionType.expense, + ), + throwsA(isA()), + ); + }); + + test('insert stores a genuinely different name, trimmed', () async { + final created = await dataSource.insertCategory( + 'Rent ', + 'rent-', + TransactionType.expense, + ); + + expect(created.name, 'Rent'); + expect(created.slug, 'rent-'); + }); + + test('rename rejects a name that collides with another category', () async { + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Rent', + slug: 'rent', + type: TransactionType.expense, + clientId: const Value('device:rent'), + )); + + expect( + () => dataSource.updateCategory('device:rent', name: 'whilesmart '), + throwsA(isA()), + ); + }); + + test('rename trims the stored name and slug', () async { + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Rent', + slug: 'rent', + type: TransactionType.expense, + clientId: const Value('device:rent'), + )); + + final updated = await dataSource.updateCategory( + 'device:rent', + name: 'Housing ', + slug: 'housing-', + ); + + expect(updated.name, 'Housing'); + expect(updated.slug, 'housing-'); + }); + + test('a category can still be renamed to a variant of its own name', + () async { + final updated = + await dataSource.updateCategory('device:existing', name: 'Whilesmart'); + + expect(updated.name, 'Whilesmart'); + }); +} diff --git a/test/unit/category_merge_test.dart b/test/unit/category_merge_test.dart new file mode 100644 index 00000000..a15388a3 --- /dev/null +++ b/test/unit/category_merge_test.dart @@ -0,0 +1,152 @@ +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/presentation/utils/enums.dart'; + +void main() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + + late AppDatabase db; + + /// The category the API accepted, and the local duplicate it keeps + /// rejecting with "Category already exists". + const winner = 'device:winner'; + const loser = 'device:loser'; + + Future tag(String transactionClientId, String categoryClientId) { + return db.categorizables.insertOne(CategorizablesCompanion.insert( + categorizableId: transactionClientId, + categorizableType: CategorizableType.transaction, + categoryClientId: categoryClientId, + )); + } + + setUp(() async { + db = AppDatabase(NativeDatabase.memory()); + + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'WhileSmart', + slug: 'whilesmart', + type: TransactionType.income, + id: const Value(41), + clientId: const Value(winner), + )); + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Whilesmart ', + slug: 'whilesmart-', + type: TransactionType.income, + clientId: const Value(loser), + )); + await db.localChanges.insertOne(LocalChangesCompanion.insert( + entityType: 'category', + entityId: loser, + entityRev: '1', + deleted: false, + data: const {'name': 'Whilesmart '}, + createAt: DateTime(2026, 7, 9), + concluded: false, + dismissed: false, + quarantinedAt: Value(DateTime(2026, 8, 17)), + error: const Value('HTTP 400 — Category already exists'), + )); + }); + + tearDown(() async { + await db.close(); + }); + + test('re-tags the duplicate\'s transactions onto the surviving category', + () async { + await tag('txn-1', loser); + await tag('txn-2', loser); + + final retagged = await db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: winner, + ); + + expect(retagged, 2); + final rows = await db.categorizables.all().get(); + expect(rows.map((r) => r.categoryClientId), everyElement(winner)); + expect(rows.map((r) => r.categorizableId), containsAll(['txn-1', 'txn-2'])); + }); + + test('drops the duplicate and its quarantined change', () async { + await tag('txn-1', loser); + + await db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: winner, + ); + + final remaining = await db.categories.all().get(); + expect(remaining.map((c) => c.clientId), [winner]); + expect(await db.localChanges.all().get(), isEmpty); + }); + + // A transaction carrying both categories would otherwise collide on the + // (source, type, category) primary key. + test('collapses a transaction already tagged with both', () async { + await tag('txn-1', loser); + await tag('txn-1', winner); + + final retagged = await db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: winner, + ); + + expect(retagged, 1); + final rows = await db.categorizables.all().get(); + expect(rows, hasLength(1)); + expect(rows.single.categoryClientId, winner); + }); + + test('leaves other categories and their tags alone', () async { + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Groceries', + slug: 'groceries', + type: TransactionType.expense, + id: const Value(42), + clientId: const Value('device:groceries'), + )); + await tag('txn-9', 'device:groceries'); + await tag('txn-1', loser); + + await db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: winner, + ); + + final groceryTags = await (db.select(db.categorizables) + ..where((c) => c.categoryClientId.equals('device:groceries'))) + .get(); + expect(groceryTags, hasLength(1)); + expect(await db.categories.all().get(), hasLength(2)); + }); + + test('refuses a merge into itself', () { + expect( + () => db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: loser, + ), + throwsA(isA()), + ); + }); + + test('refuses an unknown target and changes nothing', () async { + await tag('txn-1', loser); + + await expectLater( + db.mergeDuplicateCategory( + loserClientId: loser, + winnerClientId: 'device:nope', + ), + throwsA(isA()), + ); + + expect(await db.categories.all().get(), hasLength(2)); + expect(await db.categorizables.all().get(), hasLength(1)); + }); +} diff --git a/test/unit/duplicate_name_guard_test.dart b/test/unit/duplicate_name_guard_test.dart new file mode 100644 index 00000000..569e57b2 --- /dev/null +++ b/test/unit/duplicate_name_guard_test.dart @@ -0,0 +1,222 @@ +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/core/error/exceptions.dart'; +import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/group/group_local_datasource.dart'; +import 'package:trakli/data/datasources/party/party_local_datasource.dart'; +import 'package:trakli/data/datasources/wallet/wallet_local_datasource.dart'; +import 'package:trakli/domain/entities/party_entity.dart'; +import 'package:trakli/presentation/utils/enums.dart'; + +/// `adopt*ServerId` is recovery, not prevention: a second wallet named like +/// one the user already has still syncs and still folds into the first, which +/// reads as the new one vanishing. These guards refuse the write up front, +/// comparing names as the API does (`utf8mb4_unicode_ci`). +void main() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + + late AppDatabase db; + + setUp(() { + db = AppDatabase(NativeDatabase.memory()); + }); + + tearDown(() async { + await db.close(); + }); + + group('wallets', () { + late WalletLocalDataSourceImpl source; + + setUp(() async { + source = WalletLocalDataSourceImpl(database: db); + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'USD', + clientId: const Value('device:cash'), + )); + }); + + for (final name in ['Cash', 'cash', 'CASH ', ' cash ']) { + test('insert rejects "$name" in the same currency', () { + expect( + () => source.insertWallet(name, WalletType.bank, 0, 'USD'), + throwsA(isA()), + ); + }); + } + + test('insert rejects a currency differing only in case', () { + expect( + () => source.insertWallet('Cash', WalletType.cash, 0, 'usd'), + throwsA(isA()), + ); + }); + + // The API keys on name *and* currency, so this is a genuinely new wallet. + test('insert allows the same name in another currency', () async { + final created = + await source.insertWallet('Cash', WalletType.cash, 0, 'XAF'); + + expect(created.currency, 'XAF'); + expect(await db.wallets.all().get(), hasLength(2)); + }); + + test('insert stores a genuinely different name, trimmed', () async { + final created = + await source.insertWallet('Savings ', WalletType.bank, 0, 'USD'); + + expect(created.name, 'Savings'); + }); + + test('rename rejects a collision with another wallet', () async { + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Savings', + type: WalletType.bank, + currency: 'USD', + clientId: const Value('device:savings'), + )); + + expect( + () => source.updateWallet('device:savings', name: 'cash '), + throwsA(isA()), + ); + }); + + // Name and currency identify the wallet together, so a currency change + // can collide just as a rename can. + test('a currency change onto an existing pair is rejected', () async { + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'XAF', + clientId: const Value('device:cash-xaf'), + )); + + expect( + () => source.updateWallet('device:cash-xaf', currency: 'USD'), + throwsA(isA()), + ); + }); + + test('a wallet can still be renamed to a variant of its own name', + () async { + final updated = await source.updateWallet('device:cash', name: 'CASH'); + + expect(updated.name, 'CASH'); + }); + + test('an unrelated edit is untouched by the guard', () async { + final updated = await source.updateWallet('device:cash', balance: 250); + + expect(updated.balance, 250); + expect(updated.name, 'Cash'); + }); + }); + + group('groups', () { + late GroupLocalDataSourceImpl source; + + setUp(() async { + source = GroupLocalDataSourceImpl(database: db); + await db.groups.insertOne(GroupsCompanion.insert( + name: 'Household', + clientId: const Value('device:household'), + )); + }); + + for (final name in ['Household', 'household', 'HOUSEHOLD ']) { + test('insert rejects "$name"', () { + expect( + () => source.insertGroup(name), + throwsA(isA()), + ); + }); + } + + test('insert stores a genuinely different name, trimmed', () async { + final created = await source.insertGroup('Work '); + + expect(created.name, 'Work'); + }); + + test('rename rejects a collision with another group', () async { + await db.groups.insertOne(GroupsCompanion.insert( + name: 'Work', + clientId: const Value('device:work'), + )); + + expect( + () => source.updateGroup('device:work', name: 'household '), + throwsA(isA()), + ); + }); + + test('a group can still be renamed to a variant of its own name', + () async { + final updated = + await source.updateGroup('device:household', name: 'HouseHold'); + + expect(updated.name, 'HouseHold'); + }); + }); + + group('parties', () { + late PartyLocalDataSourceImpl source; + + setUp(() async { + source = PartyLocalDataSourceImpl(db); + await db.parties.insertOne(PartiesCompanion.insert( + name: 'Jane Doe', + type: const Value(PartyType.individual), + clientId: const Value('device:jane'), + )); + }); + + for (final name in ['Jane Doe', 'jane doe', 'JANE DOE ']) { + test('insert rejects "$name"', () { + expect( + () => source.insertParty(name), + throwsA(isA()), + ); + }); + } + + // The API compares the name alone, so a different type is not a different + // party to it. + test('insert rejects the same name under another type', () { + expect( + () => source.insertParty('jane doe', type: PartyType.organization), + throwsA(isA()), + ); + }); + + test('insert stores a genuinely different name, trimmed', () async { + final created = await source.insertParty('Acme Ltd '); + + expect(created.name, 'Acme Ltd'); + }); + + test('rename rejects a collision with another party', () async { + await db.parties.insertOne(PartiesCompanion.insert( + name: 'Acme Ltd', + clientId: const Value('device:acme'), + )); + + expect( + () => source.updateParty('device:acme', name: 'jane doe '), + throwsA(isA()), + ); + }); + + test('a party can still be renamed to a variant of its own name', + () async { + final updated = + await source.updateParty('device:jane', name: 'jane Doe'); + + expect(updated.name, 'jane Doe'); + }); + }); +} diff --git a/test/unit/duplicate_server_id_merge_test.dart b/test/unit/duplicate_server_id_merge_test.dart new file mode 100644 index 00000000..b1f0216a --- /dev/null +++ b/test/unit/duplicate_server_id_merge_test.dart @@ -0,0 +1,448 @@ +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/category/category_remote_datasource.dart'; +import 'package:trakli/data/datasources/wallet/wallet_remote_datasource.dart'; +import 'package:trakli/data/sync/category_sync_handler.dart'; +import 'package:trakli/data/sync/wallet_sync_handler.dart'; +import 'package:trakli/presentation/utils/enums.dart'; + +class _MockWalletRemote extends Mock implements WalletRemoteDataSource {} + +class _MockCategoryRemote extends Mock implements CategoryRemoteDataSource {} + +/// The create endpoints answer a name the user already has with 200 and the +/// existing record, after moving the posted client id onto it. The server id +/// that comes back is one this device filed under a different client id, and +/// `SyncTable.id` is unique locally, so the older copy has to hand over +/// everything that pointed at it and go away. +void main() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + + late AppDatabase db; + + /// Synced as server id 41, and the target of every reference below. + const older = 'device:older'; + + /// Just created locally; its POST came back with wallet 41. + const fresh = 'device:fresh'; + + setUp(() async { + db = AppDatabase(NativeDatabase.memory()); + + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'USD', + id: const Value(41), + clientId: const Value(older), + )); + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'USD', + clientId: const Value(fresh), + )); + }); + + tearDown(() async { + await db.close(); + }); + + Future addTransaction(String clientId, String walletClientId) { + return db.transactions.insertOne(TransactionsCompanion.insert( + amount: 10, + type: TransactionType.expense, + walletClientId: walletClientId, + clientId: Value(clientId), + )); + } + + Future addBudgetTargeting( + String budgetClientId, + BudgetTargetType type, + String targetClientId, + ) async { + await db.budgets.insertOne(BudgetsCompanion.insert( + name: budgetClientId, + slug: budgetClientId, + amount: 500, + currency: 'USD', + periodType: BudgetPeriodType.monthly, + startDate: DateTime(2026, 9), + clientId: Value(budgetClientId), + )); + await db.budgetTargets.insertOne(BudgetTargetsCompanion.insert( + budgetClientId: budgetClientId, + targetType: type, + targetClientId: targetClientId, + )); + } + + Future queueChange( + String entityType, + String entityId, + Map data, + ) { + return db.localChanges.insertOne(LocalChangesCompanion.insert( + entityType: entityType, + entityId: entityId, + entityRev: '1', + deleted: false, + data: data, + createAt: DateTime(2026, 9, 9), + concluded: false, + dismissed: false, + )); + } + + group('adoptWalletServerId', () { + test('moves the transactions of the copy that held the server id', + () async { + await addTransaction('t-1', older); + await addTransaction('t-2', older); + await addTransaction('t-3', fresh); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final rows = await db.transactions.all().get(); + expect(rows.map((t) => t.walletClientId), everyElement(fresh)); + expect(rows, hasLength(3)); + }); + + test('moves both legs of a transfer', () async { + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'Savings', + type: WalletType.bank, + currency: 'USD', + id: const Value(42), + clientId: const Value('device:savings'), + )); + await db.transfers.insertOne(TransfersCompanion.insert( + amount: 25, + datetime: DateTime(2026, 9, 9), + clientId: const Value('tr-out'), + fromWalletClientId: const Value(older), + toWalletClientId: const Value('device:savings'), + )); + await db.transfers.insertOne(TransfersCompanion.insert( + amount: 25, + datetime: DateTime(2026, 9, 9), + clientId: const Value('tr-in'), + fromWalletClientId: const Value('device:savings'), + toWalletClientId: const Value(older), + )); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final out = await (db.select(db.transfers) + ..where((t) => t.clientId.equals('tr-out'))) + .getSingle(); + final into = await (db.select(db.transfers) + ..where((t) => t.clientId.equals('tr-in'))) + .getSingle(); + expect(out.fromWalletClientId, fresh); + expect(out.toWalletClientId, 'device:savings'); + expect(into.toWalletClientId, fresh); + }); + + test('moves budgets that targeted the duplicate', () async { + await addBudgetTargeting('b-1', BudgetTargetType.wallet, older); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final targets = await db.budgetTargets.all().get(); + expect(targets.single.targetClientId, fresh); + }); + + // Keyed by (budget, type, target), so a budget targeting both copies + // would collide on its own primary key. + test('collapses a budget that targeted both copies', () async { + await addBudgetTargeting('b-1', BudgetTargetType.wallet, older); + await db.budgetTargets.insertOne(BudgetTargetsCompanion.insert( + budgetClientId: 'b-1', + targetType: BudgetTargetType.wallet, + targetClientId: fresh, + )); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final targets = await db.budgetTargets.all().get(); + expect(targets, hasLength(1)); + expect(targets.single.targetClientId, fresh); + }); + + test('leaves the duplicate holding the server id and nothing else', + () async { + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final wallets = await db.wallets.all().get(); + expect(wallets, hasLength(1)); + expect(wallets.single.clientId, fresh); + // The caller writes the row next; what matters is that nothing else + // is left claiming server id 41. + expect(wallets.single.id, isNull); + }); + + test('rewrites the client id inside queued payloads', () async { + await queueChange('transaction', 't-9', { + 'walletClientId': older, + 'wallet': {'client_generated_id': older, 'id': 41}, + }); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final queued = await (db.select(db.localChanges) + ..where((lc) => lc.entityId.equals('t-9'))) + .getSingle(); + expect(queued.data['walletClientId'], fresh); + expect((queued.data['wallet'] as Map)['client_generated_id'], fresh); + }); + + test('rewrites the client id inside parked downloads', () async { + await db.deferredRemoteItems.insertOne( + DeferredRemoteItemsCompanion.insert( + entityType: 'transaction', + clientId: 't-8', + data: '{"walletClientId":"$older"}', + ), + ); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + final parked = await db.deferredRemoteItems.all().get(); + expect(parked.single.data, contains(fresh)); + expect(parked.single.data, isNot(contains(older))); + }); + + test("drops the duplicate's own queued write and parked download", + () async { + await queueChange('wallet', older, {'name': 'Cash'}); + await db.deferredRemoteItems.insertOne( + DeferredRemoteItemsCompanion.insert( + entityType: 'wallet', + clientId: older, + data: '{}', + ), + ); + + await db.adoptWalletServerId(serverId: 41, clientId: fresh); + + expect(await db.localChanges.all().get(), isEmpty); + expect(await db.deferredRemoteItems.all().get(), isEmpty); + }); + + test('does nothing when the same row already holds the server id', + () async { + await addTransaction('t-1', older); + + await db.adoptWalletServerId(serverId: 41, clientId: older); + + expect(await db.wallets.all().get(), hasLength(2)); + final txn = await db.transactions.all().get(); + expect(txn.single.walletClientId, older); + }); + + test('does nothing when no local row holds the server id', () async { + await db.adoptWalletServerId(serverId: 999, clientId: fresh); + + expect(await db.wallets.all().get(), hasLength(2)); + }); + + test('does nothing without a server id', () async { + await db.adoptWalletServerId(serverId: null, clientId: fresh); + + expect(await db.wallets.all().get(), hasLength(2)); + }); + }); + + group('adoptPartyServerId', () { + test('moves the transactions of the copy that held the server id', + () async { + await db.parties.insertOne(PartiesCompanion.insert( + name: 'Jane Doe', + id: const Value(7), + clientId: const Value('party:older'), + )); + await db.parties.insertOne(PartiesCompanion.insert( + name: 'Jane Doe', + clientId: const Value('party:fresh'), + )); + await db.transactions.insertOne(TransactionsCompanion.insert( + amount: 10, + type: TransactionType.expense, + walletClientId: older, + clientId: const Value('t-1'), + partyClientId: const Value('party:older'), + )); + + await db.adoptPartyServerId(serverId: 7, clientId: 'party:fresh'); + + final parties = await db.parties.all().get(); + expect(parties.single.clientId, 'party:fresh'); + final txn = await db.transactions.all().get(); + expect(txn.single.partyClientId, 'party:fresh'); + }); + }); + + group('adoptGroupServerId', () { + test('moves transactions and budget targets', () async { + await db.groups.insertOne(GroupsCompanion.insert( + name: 'Household', + id: const Value(5), + clientId: const Value('group:older'), + )); + await db.groups.insertOne(GroupsCompanion.insert( + name: 'Household', + clientId: const Value('group:fresh'), + )); + await db.transactions.insertOne(TransactionsCompanion.insert( + amount: 10, + type: TransactionType.expense, + walletClientId: older, + clientId: const Value('t-1'), + groupClientId: const Value('group:older'), + )); + await addBudgetTargeting('b-1', BudgetTargetType.group, 'group:older'); + + await db.adoptGroupServerId(serverId: 5, clientId: 'group:fresh'); + + final groups = await db.groups.all().get(); + expect(groups.single.clientId, 'group:fresh'); + final txn = await db.transactions.all().get(); + expect(txn.single.groupClientId, 'group:fresh'); + final targets = await db.budgetTargets.all().get(); + expect(targets.single.targetClientId, 'group:fresh'); + }); + }); + + group('adoptCategoryServerId', () { + test('re-tags what the duplicate categorised', () async { + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Rent', + slug: 'rent', + type: TransactionType.expense, + id: const Value(9), + clientId: const Value('cat:older'), + )); + // Byte-different, so the local unique index on name allows both; the + // API's collation is what considers them the same category. + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'rent ', + slug: 'rent-', + type: TransactionType.expense, + clientId: const Value('cat:fresh'), + )); + await db.categorizables.insertOne(CategorizablesCompanion.insert( + categorizableId: 't-1', + categorizableType: CategorizableType.transaction, + categoryClientId: 'cat:older', + )); + await addBudgetTargeting('b-1', BudgetTargetType.category, 'cat:older'); + + await db.adoptCategoryServerId(serverId: 9, clientId: 'cat:fresh'); + + final categories = await db.categories.all().get(); + expect(categories.single.clientId, 'cat:fresh'); + final tags = await db.categorizables.all().get(); + expect(tags.single.categoryClientId, 'cat:fresh'); + final targets = await db.budgetTargets.all().get(); + expect(targets.single.targetClientId, 'cat:fresh'); + }); + }); + + // Persisting the server's answer used to fail with + // `UNIQUE constraint failed: wallets.id`, quarantining an accepted change. + group('WalletSyncHandler', () { + test('persists a create answered with the wallet the user already had', + () async { + final handler = WalletSyncHandler( + remoteDataSource: _MockWalletRemote(), + db: db, + ); + await addTransaction('t-1', older); + + final existing = await (db.select(db.wallets) + ..where((w) => w.clientId.equals(older))) + .getSingle(); + + // What the API returns: the existing wallet, client id moved to ours. + await handler.upsertAllLocal([existing.copyWith(clientId: fresh)]); + + final wallets = await db.wallets.all().get(); + expect(wallets, hasLength(1)); + expect(wallets.single.clientId, fresh); + expect(wallets.single.id, 41); + final txn = await db.transactions.all().get(); + expect(txn.single.walletClientId, fresh); + }); + }); + + // `POST /categories` used to answer a duplicate with 400; it now returns + // the existing category like the other endpoints. What has to come out the + // far side is the remote category: its name, its server id, and the client + // id the server now files it under. + group('CategorySyncHandler', () { + late CategorySyncHandler handler; + + setUp(() async { + handler = CategorySyncHandler(db, _MockCategoryRemote()); + + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'Rent', + slug: 'rent', + type: TransactionType.expense, + id: const Value(9), + clientId: const Value('cat:older'), + )); + // The duplicate just created here, with the name as the user typed it. + await db.categories.insertOne(CategoriesCompanion.insert( + name: 'rent ', + slug: 'rent-', + type: TransactionType.expense, + description: const Value('typed on this device'), + clientId: const Value('cat:fresh'), + )); + for (final tag in [('t-1', 'cat:older'), ('t-2', 'cat:fresh')]) { + await db.categorizables.insertOne(CategorizablesCompanion.insert( + categorizableId: tag.$1, + categorizableType: CategorizableType.transaction, + categoryClientId: tag.$2, + )); + } + }); + + test('the surviving category is the one the server already had', () async { + final remote = await (db.select(db.categories) + ..where((c) => c.clientId.equals('cat:older'))) + .getSingle(); + + // What the API returns: the existing category, client id moved to ours. + await handler.upsertAllLocal([remote.copyWith(clientId: 'cat:fresh')]); + + final categories = await db.categories.all().get(); + expect(categories, hasLength(1)); + final survivor = categories.single; + expect(survivor.id, 9, reason: 'keeps the remote server id'); + expect(survivor.name, 'Rent', reason: "the remote's name, not the typed one"); + expect(survivor.description, isNull, + reason: 'the locally typed description is not the remote category'); + expect(survivor.clientId, 'cat:fresh', + reason: 'the client id the server now files that category under'); + }); + + test('both copies\' transactions end up on it', () async { + final remote = await (db.select(db.categories) + ..where((c) => c.clientId.equals('cat:older'))) + .getSingle(); + + await handler.upsertAllLocal([remote.copyWith(clientId: 'cat:fresh')]); + + final tags = await db.categorizables.all().get(); + expect(tags.map((t) => t.categoryClientId), everyElement('cat:fresh')); + expect(tags.map((t) => t.categorizableId), containsAll(['t-1', 't-2'])); + }); + }); +} diff --git a/test/unit/pagination_lenient_test.dart b/test/unit/pagination_lenient_test.dart new file mode 100644 index 00000000..9b04a30d --- /dev/null +++ b/test/unit/pagination_lenient_test.dart @@ -0,0 +1,74 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/data/datasources/core/pagination_response.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; + +Map _page(List> rows) => { + 'current_page': 1, + 'last_page': 2, + 'per_page': 20, + 'data': rows, + }; + +String _parseName(Object? json) { + final row = json! as Map; + return row['name'] as String; +} + +void main() { + group('PaginationResponse.lenient', () { + test('parses every readable row and keeps the page metadata', () { + final page = PaginationResponse.lenient( + _page([ + {'name': 'a'}, + {'name': 'b'} + ]), + _parseName, + entityType: SyncEntity.transfer, + ); + + expect(page.data, ['a', 'b']); + expect(page.hasMore, isTrue); + }); + + // The whole point: one unreadable row cost us every transfer, every sync, + // for five weeks. + test('skips an unreadable row and keeps the rest', () { + final page = PaginationResponse.lenient( + _page([ + {'name': 'a'}, + {'name': null}, + {'name': 'c'} + ]), + _parseName, + entityType: SyncEntity.transfer, + ); + + expect(page.data, ['a', 'c']); + }); + + test('an entirely unreadable page still advances paging', () { + final page = PaginationResponse.lenient( + _page([ + {'name': null} + ]), + _parseName, + entityType: SyncEntity.transfer, + ); + + expect(page.data, isEmpty); + expect(page.currentPage, 1); + expect(page.hasMore, isTrue); + }); + + test('tolerates a page with no data key', () { + final page = PaginationResponse.lenient( + {'current_page': 2, 'last_page': 2, 'per_page': 20}, + _parseName, + entityType: SyncEntity.transfer, + ); + + expect(page.data, isEmpty); + expect(page.hasMore, isFalse); + }); + }); +} diff --git a/test/unit/sync_entity_test.dart b/test/unit/sync_entity_test.dart new file mode 100644 index 00000000..a4d39184 --- /dev/null +++ b/test/unit/sync_entity_test.dart @@ -0,0 +1,50 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/core/sync/sync_entity.dart'; +import 'package:trakli/data/sync/budget_period_state_sync_handler.dart'; +import 'package:trakli/data/sync/budget_sync_handler.dart'; +import 'package:trakli/data/sync/category_sync_handler.dart'; +import 'package:trakli/data/sync/config_sync_handler.dart'; +import 'package:trakli/data/sync/group_sync_handler.dart'; +import 'package:trakli/data/sync/media_sync_handler.dart'; +import 'package:trakli/data/sync/notification_sync_handler.dart'; +import 'package:trakli/data/sync/party_sync_handler.dart'; +import 'package:trakli/data/sync/reminder_sync_handler.dart'; +import 'package:trakli/data/sync/transaction_sync_handler.dart'; +import 'package:trakli/data/sync/transfer_sync_handler.dart'; +import 'package:trakli/data/sync/wallet_sync_handler.dart'; + +/// The handlers must keep their `entity` as a `const String`, because +/// `sync_database`'s `reconciledEntities` is a const map keyed by them. So the +/// spelling lives in two places, and these tests are what stops them drifting: +/// `local_changes.entityType` already holds these strings in users' databases, +/// and a key that stopped matching its handler would orphan every queued +/// change of that type. +void main() { + test('every key matches the handler that owns it', () { + expect(SyncEntity.wallet.key, WalletSyncHandler.entity); + expect(SyncEntity.category.key, CategorySyncHandler.entity); + expect(SyncEntity.group.key, GroupSyncHandler.entity); + expect(SyncEntity.party.key, PartySyncHandler.entity); + expect(SyncEntity.transaction.key, TransactionSyncHandler.entity); + expect(SyncEntity.transfer.key, TransferSyncHandler.entity); + expect(SyncEntity.budget.key, BudgetSyncHandler.entity); + expect( + SyncEntity.budgetPeriodState.key, + BudgetPeriodStateSyncHandler.entity, + ); + expect(SyncEntity.reminder.key, ReminderSyncHandler.entity); + expect(SyncEntity.notification.key, NotificationSyncHandler.entity); + expect(SyncEntity.config.key, ConfigSyncHandler.entity); + expect(SyncEntity.media.key, MediaSyncHandler.entity); + }); + + test('every handler has an entry', () { + // holding is the one value without a handler: it is a read-through cache. + expect(SyncEntity.values, hasLength(13)); + }); + + test('keys are unique', () { + final keys = SyncEntity.values.map((e) => e.key).toList(); + expect(keys.toSet(), hasLength(keys.length)); + }); +} diff --git a/test/unit/transfer_dto_parsing_test.dart b/test/unit/transfer_dto_parsing_test.dart new file mode 100644 index 00000000..426f86a7 --- /dev/null +++ b/test/unit/transfer_dto_parsing_test.dart @@ -0,0 +1,69 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:trakli/data/datasources/transfer/dto/transfer_dto.dart'; + +/// Server shape for `GET /transfers`, trimmed to the fields the DTO reads. +Map _payload({ + Object? datetime = '2026-07-09T22:56:21.000000Z', + Object? createdAt = '2026-07-09T22:56:20.000000Z', + Object? updatedAt = '2026-07-09T22:56:20.000000Z', +}) => + { + 'id': 12, + 'user_id': 2, + 'client_generated_id': 'device:transfer-1', + 'rev': '1', + 'amount': '7500.0000', + 'exchange_rate': '1.000000', + 'from_wallet_id': 6, + 'to_wallet_id': 7, + 'datetime': datetime, + 'created_at': createdAt, + 'updated_at': updatedAt, + 'deleted_at': null, + 'last_synced_at': null, + 'source_wallet': null, + 'destination_wallet': null, + 'expense_transaction_client_id': null, + 'income_transaction_client_id': null, + }; + +void main() { + group('TransferDto.fromJson', () { + test('parses a fully populated transfer', () { + final transfer = TransferDto.fromJson(_payload()).toTransfer(); + + expect(transfer.datetime, DateTime.parse('2026-07-09T22:56:21.000000Z')); + expect(transfer.createdAt, DateTime.parse('2026-07-09T22:56:20.000000Z')); + expect(transfer.amount, 7500.0); + }); + + // `transfers.datetime` is nullable server-side and was only populated from + // the 2026-02-23 backend change onwards. One such row used to throw + // "type 'Null' is not a subtype of type 'String'" out of the page parse, + // which failed the whole transfer down-sync on every cycle. + test('falls back to created_at when datetime is null', () { + final transfer = + TransferDto.fromJson(_payload(datetime: null)).toTransfer(); + + expect(transfer.datetime, DateTime.parse('2026-07-09T22:56:20.000000Z')); + }); + + test('survives null timestamps', () { + final transfer = TransferDto.fromJson( + _payload(datetime: null, createdAt: null, updatedAt: null), + ).toTransfer(); + + expect(transfer.datetime, transfer.createdAt); + expect(transfer.updatedAt, transfer.createdAt); + }); + + test('a missing datetime key is treated like a null one', () { + final json = _payload()..remove('datetime'); + + expect( + TransferDto.fromJson(json).toTransfer().datetime, + DateTime.parse('2026-07-09T22:56:20.000000Z'), + ); + }); + }); +} diff --git a/test/unit/write_request_client_id_test.dart b/test/unit/write_request_client_id_test.dart new file mode 100644 index 00000000..335ece09 --- /dev/null +++ b/test/unit/write_request_client_id_test.dart @@ -0,0 +1,354 @@ +import 'package:dio/dio.dart'; +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/budget/dtos/budget_complete_dto.dart'; +import 'package:trakli/data/datasources/category/category_remote_datasource.dart'; +import 'package:trakli/data/datasources/configuration/configuration_remote_datasource.dart'; +import 'package:trakli/data/datasources/group/group_remote_datasource.dart'; +import 'package:trakli/data/datasources/party/party_remote_datasource.dart'; +import 'package:trakli/data/datasources/reminder/reminder_remote_datasource.dart'; +import 'package:trakli/data/datasources/transaction/dto/transaction_complete_dto.dart'; +import 'package:trakli/data/datasources/transfer/transfer_remote_datasource.dart'; +import 'package:trakli/data/datasources/wallet/wallet_remote_datasource.dart'; +import 'package:trakli/domain/entities/config_entity.dart'; +import 'package:trakli/presentation/utils/enums.dart'; + +class _MockDio extends Mock implements Dio {} + +/// Thrown by the stubs once the request body has been captured: these tests +/// are about what goes out, not what comes back. +class _Captured implements Exception {} + +/// A record remembers exactly one client id, so it belongs to the create call +/// and the dedicated claim request, nowhere else. Sending it on an ordinary +/// update lets a routine edit repoint a record another device is tracking — +/// which is how two local rows end up fighting over one server id. +void main() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + + late AppDatabase db; + late _MockDio dio; + Map? sent; + + setUp(() { + db = AppDatabase(NativeDatabase.memory()); + dio = _MockDio(); + sent = null; + }); + + tearDown(() async { + await db.close(); + }); + + Never capture(Invocation invocation) { + sent = invocation.namedArguments[const Symbol('data')] + as Map?; + throw _Captured(); + } + + void stubPost() { + when(() => dio.post(any(), data: any(named: 'data'))) + .thenAnswer((i) async => capture(i)); + } + + void stubPut() { + when(() => dio.put(any(), data: any(named: 'data'))) + .thenAnswer((i) async => capture(i)); + } + + Future expectCaptured(Future Function() call) { + return expectLater(call(), throwsA(isA<_Captured>())); + } + + group('wallets', () { + late Wallet wallet; + + setUp(() async { + wallet = await db.wallets.insertReturning(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'USD', + id: const Value(41), + clientId: const Value('device:wallet'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = WalletRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertWallet(wallet)); + + expect(sent!['client_id'], 'device:wallet'); + }); + + test('update does not', () async { + stubPut(); + final source = WalletRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateWallet(wallet)); + + expect(sent, isNot(contains('client_id'))); + }); + + test('claiming still does', () async { + stubPut(); + final source = WalletRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.claimClientId( + id: 41, + clientId: 'device:wallet', + updatedAt: DateTime(2026, 9, 9), + )); + + expect(sent!['client_id'], 'device:wallet'); + }); + }); + + group('groups', () { + late Group group; + + setUp(() async { + group = await db.groups.insertReturning(GroupsCompanion.insert( + name: 'Household', + id: const Value(5), + clientId: const Value('device:group'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = GroupRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertGroup(group)); + + expect(sent!['client_id'], 'device:group'); + }); + + test('update does not', () async { + stubPut(); + final source = GroupRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateGroup(group)); + + expect(sent, isNot(contains('client_id'))); + }); + }); + + group('parties', () { + late Party party; + + setUp(() async { + party = await db.parties.insertReturning(PartiesCompanion.insert( + name: 'Jane Doe', + id: const Value(7), + clientId: const Value('device:party'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = PartyRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertParty(party)); + + expect(sent!['client_id'], 'device:party'); + }); + + test('update does not', () async { + stubPut(); + final source = PartyRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateParty(party)); + + expect(sent, isNot(contains('client_id'))); + }); + }); + + group('categories', () { + late Category category; + + setUp(() async { + category = await db.categories.insertReturning(CategoriesCompanion.insert( + name: 'Rent', + slug: 'rent', + type: TransactionType.expense, + id: const Value(9), + clientId: const Value('device:category'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = CategoryRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertCategory(category)); + + expect(sent!['client_id'], 'device:category'); + }); + + test('update does not', () async { + stubPut(); + final source = CategoryRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateCategory(category)); + + expect(sent, isNot(contains('client_id'))); + }); + }); + + group('configurations', () { + late Config config; + + setUp(() async { + config = await db.configs.insertReturning(ConfigsCompanion.insert( + key: 'wallets.allow_negative_balance', + type: ConfigType.bool, + value: true, + id: const Value(3), + clientId: const Value('device:config'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = ConfigRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertConfig(config)); + + expect(sent!['client_id'], 'device:config'); + }); + + test('update does not', () async { + stubPut(); + final source = ConfigRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateConfig(config)); + + expect(sent, isNot(contains('client_id'))); + }); + + test('claiming still does', () async { + stubPut(); + final source = ConfigRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.claimClientId( + key: config.key, + clientId: 'device:config', + updatedAt: DateTime(2026, 9, 9), + )); + + expect(sent!['client_id'], 'device:config'); + }); + }); + + group('reminders', () { + late Reminder reminder; + + setUp(() async { + reminder = await db.reminders.insertReturning(RemindersCompanion.insert( + title: 'Log today', + id: const Value(4), + clientId: const Value('device:reminder'), + )); + }); + + test('create sends the client id', () async { + stubPost(); + final source = ReminderRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.insertReminder(reminder)); + + expect(sent!['client_id'], 'device:reminder'); + }); + + test('update does not', () async { + stubPut(); + final source = ReminderRemoteDataSourceImpl(dio: dio); + + await expectCaptured(() => source.updateReminder(reminder)); + + expect(sent, isNot(contains('client_id'))); + }); + }); + + group('transfers', () { + test('the create body carries the client id, the update body does not', + () async { + final transfer = await db.transfers.insertReturning( + TransfersCompanion.insert( + amount: 25, + datetime: DateTime(2026, 9, 9), + id: const Value(12), + clientId: const Value('device:transfer'), + ), + ); + + expect(toServerJson(transfer)['client_id'], 'device:transfer'); + expect( + toServerJson(transfer, includeClientId: false), + isNot(contains('client_id')), + ); + }); + }); + + group('budgets', () { + test('the create body carries the client id, the update body does not', + () async { + final budget = await db.budgets.insertReturning(BudgetsCompanion.insert( + name: 'Groceries', + slug: 'groceries', + amount: 500, + currency: 'USD', + periodType: BudgetPeriodType.monthly, + startDate: DateTime(2026, 9), + id: const Value(21), + clientId: const Value('device:budget'), + )); + final dto = BudgetCompleteDto(budget: budget); + + expect(dto.toServerJson()['client_id'], 'device:budget'); + expect( + dto.toServerJson(includeClientId: false), + isNot(contains('client_id')), + ); + }); + }); + + group('transactions', () { + test('the create body carries the client id, the update body does not', + () async { + final wallet = await db.wallets.insertReturning(WalletsCompanion.insert( + name: 'Cash', + type: WalletType.cash, + currency: 'USD', + id: const Value(41), + clientId: const Value('device:wallet'), + )); + final transaction = await db.transactions.insertReturning( + TransactionsCompanion.insert( + amount: 10, + type: TransactionType.expense, + walletClientId: wallet.clientId, + id: const Value(31), + clientId: const Value('device:transaction'), + ), + ); + final dto = TransactionCompleteDto( + transaction: transaction, + wallet: wallet, + ); + + expect(dto.toServerJson()['client_id'], 'device:transaction'); + + final update = dto.toServerJson(includeClientId: false); + expect(update, isNot(contains('client_id'))); + // The drift row spread names it client_generated_id; neither spelling + // may ride along on an update. + expect(update, isNot(contains('client_generated_id'))); + }); + }); +} From 35f4d72b42bda08d716365690173e1addb9ed083 Mon Sep 17 00:00:00 2001 From: Fuh Austin Date: Fri, 18 Sep 2026 11:02:52 +0100 Subject: [PATCH 2/5] refactor(sync): Match payload ids with instr() instead of LIKE LIKE treats % and _ in the pattern as wildcards and compares ASCII case-insensitively. The pattern is built from a client id whose device half comes from the platform, so it can select rows that replace() then leaves untouched. instr() is an exact, case-sensitive literal search and says what the code means. --- lib/data/database/app_database.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/data/database/app_database.dart b/lib/data/database/app_database.dart index 03fded1e..23f784ec 100644 --- a/lib/data/database/app_database.dart +++ b/lib/data/database/app_database.dart @@ -492,8 +492,11 @@ class AppDatabase extends _$AppDatabase with SynchronizerDb { ) async { for (final table in const ['local_changes', 'deferred_remote_items']) { await customStatement( - 'UPDATE $table SET data = replace(data, ?, ?) WHERE data LIKE ?', - [loserClientId, winnerClientId, '%$loserClientId%'], + // instr() is an exact, case-sensitive substring test. LIKE would treat + // % and _ in the id as wildcards and match case-insensitively, so it + // can select rows replace() then leaves untouched. + 'UPDATE $table SET data = replace(data, ?, ?) WHERE instr(data, ?) > 0', + [loserClientId, winnerClientId, loserClientId], ); } } From 15450872cb28437acb82f855c04b57f2e13b2525 Mon Sep 17 00:00:00 2001 From: Fuh Austin Date: Fri, 18 Sep 2026 11:15:26 +0100 Subject: [PATCH 3/5] fix(sync): Carry the value on a configuration client-id claim PUT /configurations/{key} documents value as required. A claim body of only client_id and updated_at risks a 422 before the update hook records the id, replacing one failure with another. The claimed entity comes straight from getAllRemote, so this echoes the server's own value back rather than writing a stale local one. --- .../configuration/configuration_remote_datasource.dart | 7 +++++++ lib/data/sync/config_sync_handler.dart | 1 + test/unit/write_request_client_id_test.dart | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/lib/data/datasources/configuration/configuration_remote_datasource.dart b/lib/data/datasources/configuration/configuration_remote_datasource.dart index 56fc6d2d..58095c53 100644 --- a/lib/data/datasources/configuration/configuration_remote_datasource.dart +++ b/lib/data/datasources/configuration/configuration_remote_datasource.dart @@ -25,6 +25,7 @@ abstract class ConfigRemoteDataSource { required String key, required String clientId, required DateTime updatedAt, + required dynamic value, }); Future deleteConfig(String id); @@ -112,10 +113,16 @@ class ConfigRemoteDataSourceImpl implements ConfigRemoteDataSource { required String key, required String clientId, required DateTime updatedAt, + required dynamic value, }) async { final response = await dio.put('configurations/$key', data: { 'client_id': clientId, 'updated_at': formatServerIsoDateTimeString(updatedAt), + // A claim is still a PUT, and the endpoint documents `value` as + // required, so a leaner body risks a 422 before the client id is ever + // recorded. The entity comes straight from the server, so this echoes + // the value back rather than writing a stale local one. + 'value': value, }); final apiResponse = ApiResponse.fromJson(response.data); return Config.fromJson(apiResponse.data); diff --git a/lib/data/sync/config_sync_handler.dart b/lib/data/sync/config_sync_handler.dart index fa768c1e..3a720f0e 100644 --- a/lib/data/sync/config_sync_handler.dart +++ b/lib/data/sync/config_sync_handler.dart @@ -64,6 +64,7 @@ class ConfigSyncHandler extends SyncTypeHandler key: entity.key, clientId: entity.clientId, updatedAt: entity.updatedAt, + value: entity.value, ); } diff --git a/test/unit/write_request_client_id_test.dart b/test/unit/write_request_client_id_test.dart index 335ece09..d41af6f7 100644 --- a/test/unit/write_request_client_id_test.dart +++ b/test/unit/write_request_client_id_test.dart @@ -239,9 +239,13 @@ void main() { key: config.key, clientId: 'device:config', updatedAt: DateTime(2026, 9, 9), + value: config.value, )); expect(sent!['client_id'], 'device:config'); + // PUT /configurations/{key} documents `value` as required, so the claim + // must carry it or the validator rejects it before the id is recorded. + expect(sent!['value'], config.value); }); }); From 407ecae2188886cc47e625f4fe267619488a2e02 Mon Sep 17 00:00:00 2001 From: Fuh Austin Date: Fri, 18 Sep 2026 11:22:11 +0100 Subject: [PATCH 4/5] fix(sync): Fold names in Dart so the duplicate guards match the API The guards folded case twice with different rules: the column through SQLite's lower(), which only folds ASCII, and the literal through Dart's toLowerCase(), which is Unicode-aware. Stored 'CAFE' with an accent lowered to 'cafE' in SQL but 'cafe' in Dart, so the comparison never matched, the duplicate was written locally, and the next sync folded it into the existing row - the vanishing-record symptom the guards exist to prevent. Move the comparison into Dart behind one shared helper, replacing five copies of the rule in the budget, category, group, party and wallet data sources and the category merge lookup in the sync history screen. --- .../budget/budget_local_datasource.dart | 26 ++++++------- .../category/category_local_datasource.dart | 21 +++++----- lib/data/datasources/core/name_matching.dart | 38 +++++++++++++++++++ .../group/group_local_datasource.dart | 21 +++++----- .../party/party_local_datasource.dart | 21 +++++----- .../transfer/transfer_local_datasource.dart | 8 ++-- .../wallet/wallet_local_datasource.dart | 24 ++++++------ lib/presentation/sync_history_screen.dart | 16 +++++--- test/unit/duplicate_name_guard_test.dart | 19 ++++++++++ 9 files changed, 124 insertions(+), 70 deletions(-) create mode 100644 lib/data/datasources/core/name_matching.dart diff --git a/lib/data/datasources/budget/budget_local_datasource.dart b/lib/data/datasources/budget/budget_local_datasource.dart index e6e34812..8668ecfb 100644 --- a/lib/data/datasources/budget/budget_local_datasource.dart +++ b/lib/data/datasources/budget/budget_local_datasource.dart @@ -1,5 +1,6 @@ import 'package:drift/drift.dart'; import 'package:injectable/injectable.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/core/error/exceptions.dart'; import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/core/utils/id_helper.dart'; @@ -143,17 +144,15 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { } /// The same name is the same budget, case and surrounding spaces aside. - Future _findByName(String name, {String? excluding}) { - final normalized = name.trim().toLowerCase(); - return (database.select(database.budgets) - ..where((b) { - final matches = b.name.trim().lower().equals(normalized); - return excluding == null - ? matches - : matches & b.clientId.isNotValue(excluding); - }) - ..limit(1)) - .getSingleOrNull(); + Future _findByName(String name, {String? excluding}) async { + final rows = await database.select(database.budgets).get(); + return firstMatchingName( + rows, + name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + excluding: excluding, + ); } @override @@ -250,9 +249,8 @@ class BudgetLocalDataSourceImpl implements BudgetLocalDataSource { slug: slug != null ? Value(slug) : const Value.absent(), amount: amount != null ? Value(amount) : const Value.absent(), currency: currency != null ? Value(currency) : const Value.absent(), - periodType: periodType != null - ? Value(periodType) - : const Value.absent(), + periodType: + periodType != null ? Value(periodType) : const Value.absent(), startDate: startDate != null ? Value(startDate) : const Value.absent(), endDate: endDate != null ? Value(endDate) : const Value.absent(), diff --git a/lib/data/datasources/category/category_local_datasource.dart b/lib/data/datasources/category/category_local_datasource.dart index 30634bd9..6ce0d37b 100644 --- a/lib/data/datasources/category/category_local_datasource.dart +++ b/lib/data/datasources/category/category_local_datasource.dart @@ -2,6 +2,7 @@ import 'package:drift/drift.dart'; import 'package:injectable/injectable.dart'; import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/presentation/utils/enums.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; @@ -42,17 +43,15 @@ class CategoryLocalDataSourceImpl implements CategoryLocalDataSource { /// The same name is the same category, whatever its type, case and /// surrounding spaces aside. - Future _findByServerName(String name, {String? excluding}) { - final normalized = name.trim().toLowerCase(); - return (database.select(database.categories) - ..where((c) { - final matches = c.name.trim().lower().equals(normalized); - return excluding == null - ? matches - : matches & c.clientId.isNotValue(excluding); - }) - ..limit(1)) - .getSingleOrNull(); + Future _findByServerName(String name, {String? excluding}) async { + final rows = await database.select(database.categories).get(); + return firstMatchingName( + rows, + name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + excluding: excluding, + ); } @override diff --git a/lib/data/datasources/core/name_matching.dart b/lib/data/datasources/core/name_matching.dart new file mode 100644 index 00000000..ecf8d56f --- /dev/null +++ b/lib/data/datasources/core/name_matching.dart @@ -0,0 +1,38 @@ +/// Name comparison for the local duplicate guards. +/// +/// The API decides what counts as a duplicate with `utf8mb4_unicode_ci`, so +/// the client has to fold case the same way or the guard and the server +/// disagree. The fold therefore happens in Dart: SQLite's built-in `lower()` +/// only folds ASCII, so a stored `CAFÉ` lowers to `cafÉ` while Dart's +/// `toLowerCase()` gives `café`. Comparing the two in SQL never matches, the +/// duplicate is written locally, and the next sync folds it into the existing +/// row — which is the record-vanishing symptom the guards exist to prevent. +/// +/// The client's equivalence class must stay a subset of the server's: these +/// guards may refuse a write the server would accept, but must never accept +/// one the server would fold. +library; + +/// Trims and case-folds [value] the way the guards compare names. +String normalizeName(String value) => value.trim().toLowerCase(); + +/// The first row in [rows] whose name matches [query] once both are +/// normalized, skipping the row whose client id equals [excluding]. +/// +/// Callers pass every candidate row and the comparison runs in Dart. These +/// tables hold tens of rows per user, so the scan costs less than getting the +/// collation wrong. +T? firstMatchingName( + Iterable rows, + String query, { + required String Function(T row) nameOf, + required String Function(T row) clientIdOf, + String? excluding, +}) { + final normalized = normalizeName(query); + for (final row in rows) { + if (excluding != null && clientIdOf(row) == excluding) continue; + if (normalizeName(nameOf(row)) == normalized) return row; + } + return null; +} diff --git a/lib/data/datasources/group/group_local_datasource.dart b/lib/data/datasources/group/group_local_datasource.dart index 8484630a..2e128199 100644 --- a/lib/data/datasources/group/group_local_datasource.dart +++ b/lib/data/datasources/group/group_local_datasource.dart @@ -2,6 +2,7 @@ import 'package:drift/drift.dart'; import 'package:injectable/injectable.dart'; import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; import 'package:trakli/core/error/exceptions.dart'; @@ -40,17 +41,15 @@ class GroupLocalDataSourceImpl implements GroupLocalDataSource { } /// The same name is the same group, case and surrounding spaces aside. - Future _findByServerName(String name, {String? excluding}) { - final normalized = name.trim().toLowerCase(); - return (database.select(database.groups) - ..where((g) { - final matches = g.name.trim().lower().equals(normalized); - return excluding == null - ? matches - : matches & g.clientId.isNotValue(excluding); - }) - ..limit(1)) - .getSingleOrNull(); + Future _findByServerName(String name, {String? excluding}) async { + final rows = await database.select(database.groups).get(); + return firstMatchingName( + rows, + name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + excluding: excluding, + ); } @override diff --git a/lib/data/datasources/party/party_local_datasource.dart b/lib/data/datasources/party/party_local_datasource.dart index 35d218d1..0f051a01 100644 --- a/lib/data/datasources/party/party_local_datasource.dart +++ b/lib/data/datasources/party/party_local_datasource.dart @@ -2,6 +2,7 @@ import 'package:drift/drift.dart'; import 'package:injectable/injectable.dart'; import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/data/models/media.dart'; import 'package:trakli/core/utils/id_helper.dart'; import 'package:trakli/core/error/exceptions.dart'; @@ -48,17 +49,15 @@ class PartyLocalDataSourceImpl implements PartyLocalDataSource { /// The same name is the same party, whatever its type, case and /// surrounding spaces aside. - Future _findByServerName(String name, {String? excluding}) { - final normalized = name.trim().toLowerCase(); - return (database.select(database.parties) - ..where((p) { - final matches = p.name.trim().lower().equals(normalized); - return excluding == null - ? matches - : matches & p.clientId.isNotValue(excluding); - }) - ..limit(1)) - .getSingleOrNull(); + Future _findByServerName(String name, {String? excluding}) async { + final rows = await database.select(database.parties).get(); + return firstMatchingName( + rows, + name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + excluding: excluding, + ); } @override diff --git a/lib/data/datasources/transfer/transfer_local_datasource.dart b/lib/data/datasources/transfer/transfer_local_datasource.dart index 72503779..77a3e6a0 100644 --- a/lib/data/datasources/transfer/transfer_local_datasource.dart +++ b/lib/data/datasources/transfer/transfer_local_datasource.dart @@ -37,9 +37,10 @@ class TransferLocalDataSourceImpl implements TransferLocalDataSource { @override Future insertTransfer(TransfersCompanion companion) async { final now = getNewFormattedUtcDateTime(); - final clientId = companion.clientId.present && companion.clientId.value.isNotEmpty - ? companion.clientId.value - : await generateDeviceScopedId(); + final clientId = + companion.clientId.present && companion.clientId.value.isNotEmpty + ? companion.clientId.value + : await generateDeviceScopedId(); final toInsert = companion.copyWith( clientId: Value(clientId), createdAt: Value(now), @@ -70,7 +71,6 @@ class TransferLocalDataSourceImpl implements TransferLocalDataSource { return updated.first; } - @override Future deleteTransfer(String clientId) async { final transfer = await (database.select(database.transfers) diff --git a/lib/data/datasources/wallet/wallet_local_datasource.dart b/lib/data/datasources/wallet/wallet_local_datasource.dart index 2c539257..b31c8af9 100644 --- a/lib/data/datasources/wallet/wallet_local_datasource.dart +++ b/lib/data/datasources/wallet/wallet_local_datasource.dart @@ -1,6 +1,7 @@ import 'package:drift/drift.dart'; import 'package:drift_sync_core/drift_sync_core.dart'; import 'package:injectable/injectable.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/core/utils/date_util.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/presentation/utils/enums.dart'; @@ -59,19 +60,16 @@ class WalletLocalDataSourceImpl implements WalletLocalDataSource { String name, String currency, { String? excluding, - }) { - final normalizedName = name.trim().toLowerCase(); - final normalizedCurrency = currency.trim().toLowerCase(); - return (database.select(database.wallets) - ..where((w) { - final matches = w.name.trim().lower().equals(normalizedName) & - w.currency.trim().lower().equals(normalizedCurrency); - return excluding == null - ? matches - : matches & w.clientId.isNotValue(excluding); - }) - ..limit(1)) - .getSingleOrNull(); + }) async { + final normalizedCurrency = normalizeName(currency); + final rows = await database.select(database.wallets).get(); + return firstMatchingName( + rows.where((w) => normalizeName(w.currency) == normalizedCurrency), + name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + excluding: excluding, + ); } @override diff --git a/lib/presentation/sync_history_screen.dart b/lib/presentation/sync_history_screen.dart index 14aa7cc9..59b811fe 100644 --- a/lib/presentation/sync_history_screen.dart +++ b/lib/presentation/sync_history_screen.dart @@ -5,6 +5,7 @@ import 'package:drift_sync_core/drift_sync_core.dart'; import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:flutter_screenutil/flutter_screenutil.dart'; +import 'package:trakli/data/datasources/core/name_matching.dart'; import 'package:trakli/core/sync/sync_database.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/di/injection.dart'; @@ -141,13 +142,16 @@ class _SyncHistoryScreenState extends State { return; } - final winner = await (_db.select(_db.categories) + final synced = await (_db.select(_db.categories) ..where((c) => - c.clientId.isNotValue(duplicate.clientId) & - c.id.isNotNull() & - c.name.trim().lower().equals(duplicate.name.trim().toLowerCase())) - ..limit(1)) - .getSingleOrNull(); + c.clientId.isNotValue(duplicate.clientId) & c.id.isNotNull())) + .get(); + final winner = firstMatchingName( + synced, + duplicate.name, + nameOf: (row) => row.name, + clientIdOf: (row) => row.clientId, + ); if (winner == null) { _showMessage( 'No synced category named "${duplicate.name.trim()}" to merge into. ' diff --git a/test/unit/duplicate_name_guard_test.dart b/test/unit/duplicate_name_guard_test.dart index 569e57b2..edf58668 100644 --- a/test/unit/duplicate_name_guard_test.dart +++ b/test/unit/duplicate_name_guard_test.dart @@ -48,6 +48,25 @@ void main() { }); } + // SQLite's lower() folds ASCII only, so comparing a Dart-folded literal + // against a SQL-folded column never matched for non-ASCII names: 'CAFÉ' + // lowered to 'cafÉ' in SQL but 'café' in Dart. The duplicate was written + // locally and folded on sync, which is the symptom the guard prevents. + test('insert rejects a name differing only in an accented letter\'s case', + () async { + await db.wallets.insertOne(WalletsCompanion.insert( + name: 'CAFÉ', + type: WalletType.cash, + currency: 'USD', + clientId: const Value('device:cafe'), + )); + + expect( + () => source.insertWallet('café', WalletType.bank, 0, 'USD'), + throwsA(isA()), + ); + }); + test('insert rejects a currency differing only in case', () { expect( () => source.insertWallet('Cash', WalletType.cash, 0, 'usd'), From bcbecd97cf0b5f8fbdd14a1fb8d5fb31af1f582a Mon Sep 17 00:00:00 2001 From: Fuh Austin Date: Fri, 18 Sep 2026 11:27:04 +0100 Subject: [PATCH 5/5] fix(sync): Tolerate a malformed page envelope and test the real call sites The lenient factory recovered from an unreadable row but still cast the envelope, so a missing current_page threw past that recovery and lost the whole page it exists to save. Fall back to -1, matching empty(), which leaves hasMore false: keep what parsed, fetch nothing further. The transfers, budgets and transactions groups only called the body builder, so they could not fail if the update call site kept sending the client id - the defect they are meant to guard. Drive the real insert/update methods through the stubbed Dio, as the other groups do. --- .../datasources/core/pagination_response.dart | 10 ++-- test/unit/pagination_lenient_test.dart | 22 ++++++++ test/unit/write_request_client_id_test.dart | 51 ++++++++++++++----- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/lib/data/datasources/core/pagination_response.dart b/lib/data/datasources/core/pagination_response.dart index 12c68a47..3deacacb 100644 --- a/lib/data/datasources/core/pagination_response.dart +++ b/lib/data/datasources/core/pagination_response.dart @@ -53,10 +53,14 @@ class PaginationResponse with _$PaginationResponse { } } + // Casting the envelope would throw past the row-level recovery above and + // lose the whole page anyway. -1 matches PaginationResponse.empty() and + // leaves hasMore false, so a malformed envelope degrades to "keep what + // parsed, fetch nothing further" rather than looping or failing. return PaginationResponse( - currentPage: (json['current_page'] as num).toInt(), - lastPage: (json['last_page'] as num).toInt(), - perPage: (json['per_page'] as num).toInt(), + currentPage: (json['current_page'] as num?)?.toInt() ?? -1, + lastPage: (json['last_page'] as num?)?.toInt() ?? -1, + perPage: (json['per_page'] as num?)?.toInt() ?? -1, data: parsed, ); } diff --git a/test/unit/pagination_lenient_test.dart b/test/unit/pagination_lenient_test.dart index 9b04a30d..2704bdc8 100644 --- a/test/unit/pagination_lenient_test.dart +++ b/test/unit/pagination_lenient_test.dart @@ -2,10 +2,15 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:trakli/data/datasources/core/pagination_response.dart'; import 'package:trakli/core/sync/sync_entity.dart'; +/// The documented GET envelope, kept whole so the lenient factory is exercised +/// against the real wire shape rather than the subset it happens to read. Map _page(List> rows) => { + 'success': true, 'current_page': 1, 'last_page': 2, 'per_page': 20, + 'total': 40, + 'last_sync': '2025-12-26T10:30:45.000000Z', 'data': rows, }; @@ -70,5 +75,22 @@ void main() { expect(page.data, isEmpty); expect(page.hasMore, isFalse); }); + + test('a malformed envelope keeps the parsed rows and stops paging', () { + // Casting the envelope used to throw past the row-level recovery, losing + // the whole page the lenient factory exists to save. + final page = PaginationResponse.lenient( + { + 'data': [ + {'name': 'a'} + ] + }, + _parseName, + entityType: SyncEntity.transfer, + ); + + expect(page.data, ['a']); + expect(page.hasMore, isFalse); + }); }); } diff --git a/test/unit/write_request_client_id_test.dart b/test/unit/write_request_client_id_test.dart index d41af6f7..e17f720d 100644 --- a/test/unit/write_request_client_id_test.dart +++ b/test/unit/write_request_client_id_test.dart @@ -5,6 +5,8 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; import 'package:trakli/data/database/app_database.dart'; import 'package:trakli/data/datasources/budget/dtos/budget_complete_dto.dart'; +import 'package:trakli/data/datasources/budget/budget_remote_datasource.dart'; +import 'package:trakli/data/datasources/transaction/transaction_remote_datasource.dart'; import 'package:trakli/data/datasources/category/category_remote_datasource.dart'; import 'package:trakli/data/datasources/configuration/configuration_remote_datasource.dart'; import 'package:trakli/data/datasources/group/group_remote_datasource.dart'; @@ -291,11 +293,23 @@ void main() { ), ); - expect(toServerJson(transfer)['client_id'], 'device:transfer'); - expect( - toServerJson(transfer, includeClientId: false), - isNot(contains('client_id')), - ); + final source = TransferRemoteDataSourceImpl(dio: dio); + + stubPost(); + await expectCaptured(() => source.insertTransfer(transfer)); + expect(sent!['client_id'], 'device:transfer'); + + stubPut(); + await expectCaptured(() => source.updateTransfer(transfer)); + expect(sent, isNot(contains('client_id'))); + + stubPut(); + await expectCaptured(() => source.claimClientId( + id: transfer.id!, + clientId: transfer.clientId, + updatedAt: DateTime(2026, 9, 9), + )); + expect(sent!['client_id'], 'device:transfer'); }); }); @@ -314,11 +328,15 @@ void main() { )); final dto = BudgetCompleteDto(budget: budget); - expect(dto.toServerJson()['client_id'], 'device:budget'); - expect( - dto.toServerJson(includeClientId: false), - isNot(contains('client_id')), - ); + final source = BudgetRemoteDataSourceImpl(dio: dio); + + stubPost(); + await expectCaptured(() => source.insertBudget(dto)); + expect(sent!['client_id'], 'device:budget'); + + stubPut(); + await expectCaptured(() => source.updateBudget(dto)); + expect(sent, isNot(contains('client_id'))); }); }); @@ -346,13 +364,18 @@ void main() { wallet: wallet, ); - expect(dto.toServerJson()['client_id'], 'device:transaction'); + final source = TransactionRemoteDataSourceImpl(dio: dio); + + stubPost(); + await expectCaptured(() => source.insertTransaction(dto)); + expect(sent!['client_id'], 'device:transaction'); - final update = dto.toServerJson(includeClientId: false); - expect(update, isNot(contains('client_id'))); + stubPut(); + await expectCaptured(() => source.updateTransaction(dto)); + expect(sent, isNot(contains('client_id'))); // The drift row spread names it client_generated_id; neither spelling // may ride along on an update. - expect(update, isNot(contains('client_generated_id'))); + expect(sent, isNot(contains('client_generated_id'))); }); }); }