From c509c4d5436b0f18efe842adcc126bdb24ccfdc0 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 10 Oct 2026 11:21:50 +0300 Subject: [PATCH 1/2] fix(connections): removing a folder keeps its connections connections.folder_id cascades on delete, so removing a folder deleted the connections in it. They are detached (folder_id = NULL) in the same transaction before the folder row goes, so the cascade finds nothing. No schema migration: the foreign key stays as it is. The confirmation dialog and the secret cleanup of the previous change are gone: nothing is deleted any more. --- lib/core/storage/local_db.dart | 43 +++++---------- .../connections/connections_panel.dart | 18 +------ .../connections/remove_folder_dialog.dart | 54 ------------------- .../storage/local_db_folder_removal_test.dart | 53 ++++++++++-------- .../remove_folder_dialog_test.dart | 39 -------------- 5 files changed, 44 insertions(+), 163 deletions(-) delete mode 100644 lib/features/connections/remove_folder_dialog.dart delete mode 100644 test/features/connections/remove_folder_dialog_test.dart diff --git a/lib/core/storage/local_db.dart b/lib/core/storage/local_db.dart index 0ec2f964..a5d70d66 100644 --- a/lib/core/storage/local_db.dart +++ b/lib/core/storage/local_db.dart @@ -611,38 +611,21 @@ class LocalDb { await db.insert('folders', {'name': n}); } - /// Removes the folder **and the connections in it** (the foreign key - /// cascades), so their secrets go with them: the cascade does not reach the - /// OS keychain. + /// Removes the folder; its connections stay, without a folder. + /// + /// `connections.folder_id` is `ON DELETE CASCADE`, which would delete them + /// with it. They are detached first, so the cascade finds nothing: no schema + /// migration of the connections table is needed. Future removeFolder(String name) async { final db = await _open(); - final inside = await db.rawQuery( - 'SELECT c.id AS id FROM connections c ' - 'JOIN folders f ON f.id = c.folder_id WHERE f.name = ?', - [name], - ); - for (final row in inside) { - final id = _sqliteInt(row['id']); - if (id == null) continue; - _historyInsertCounts.removeWhere((k, _) => k.startsWith('$id::')); - try { - await ConnectionSecretsStore.deleteForConnection(id); - } catch (_) { - // Do not block removing the folder when the OS store fails. - } - } - await db.delete('folders', where: 'name = ?', whereArgs: [name]); - } - - /// How many connections [removeFolder] would delete. - Future countConnectionsInFolder(String name) async { - final db = await _open(); - final rows = await db.rawQuery( - 'SELECT COUNT(*) AS n FROM connections c ' - 'JOIN folders f ON f.id = c.folder_id WHERE f.name = ?', - [name], - ); - return _sqliteInt(rows.first['n']) ?? 0; + await db.transaction((txn) async { + await txn.rawUpdate( + 'UPDATE connections SET folder_id = NULL WHERE folder_id IN ' + '(SELECT id FROM folders WHERE name = ?)', + [name], + ); + await txn.delete('folders', where: 'name = ?', whereArgs: [name]); + }); } Future clearFolders() async { diff --git a/lib/features/connections/connections_panel.dart b/lib/features/connections/connections_panel.dart index 6df841ed..90226a63 100644 --- a/lib/features/connections/connections_panel.dart +++ b/lib/features/connections/connections_panel.dart @@ -91,7 +91,6 @@ import 'package:querya_desktop/features/mongodb/mongo_database_dialog.dart'; import 'package:querya_desktop/features/postgresql/postgres_object_kind.dart'; import 'package:querya_desktop/features/mysql/mysql_object_kind.dart'; import 'new_folder_dialog.dart'; -import 'remove_folder_dialog.dart'; part 'connections_panel_sidebar.dart'; part 'connections_panel_redis.dart'; @@ -734,23 +733,8 @@ class ConnectionsPanelState extends State { await _loadData(); } - /// Removing a folder deletes the connections in it, so a folder that holds - /// some is confirmed first and their open sessions are closed. + /// Removes the folder; its connections stay in the list without one. Future _removeFolder(String name) async { - final inside = _connections - .where((c) => c.folderId == _folderIdByName[name] && c.id != null) - .toList(); - if (inside.isNotEmpty) { - final ok = await confirmRemoveFolder( - context, - name: name, - connectionCount: inside.length, - ); - if (!ok || !mounted) return; - for (final c in inside) { - await _closeSessionsOf(c.id!); - } - } await FoldersStorage.instance.remove(name); if (mounted) await _loadData(); } diff --git a/lib/features/connections/remove_folder_dialog.dart b/lib/features/connections/remove_folder_dialog.dart deleted file mode 100644 index c7432836..00000000 --- a/lib/features/connections/remove_folder_dialog.dart +++ /dev/null @@ -1,54 +0,0 @@ -import 'package:flutter/material.dart' as material; -import 'package:querya_desktop/shared/widgets/widgets.dart'; - -/// Asks before a folder with connections is removed: removing it deletes them -/// and their saved passwords. Returns true to remove. -Future confirmRemoveFolder( - material.BuildContext context, { - required String name, - required int connectionCount, -}) async { - final noun = connectionCount == 1 ? 'connection' : 'connections'; - final result = await showAppDialog( - context: context, - builder: (ctx) => QueryaDialogCard( - constraints: const material.BoxConstraints(maxWidth: 440), - child: material.Padding( - padding: const material.EdgeInsets.all(20), - child: material.Column( - mainAxisSize: material.MainAxisSize.min, - crossAxisAlignment: material.CrossAxisAlignment.start, - children: [ - Text('Remove folder "$name"?').semiBold().large(), - const Gap(8), - Text( - 'The folder holds $connectionCount $noun. Removing it deletes ' - '${connectionCount == 1 ? 'it' : 'them'} and the saved ' - '${connectionCount == 1 ? 'password' : 'passwords'}. This cannot ' - 'be undone.', - ).muted().small(), - const Gap(20), - material.Align( - alignment: material.Alignment.centerRight, - child: material.Wrap( - spacing: 8, - runSpacing: 8, - children: [ - OutlineButton( - onPressed: () => material.Navigator.of(ctx).pop(false), - child: const Text('Cancel'), - ), - DestructiveButton( - onPressed: () => material.Navigator.of(ctx).pop(true), - child: const Text('Remove folder'), - ), - ], - ), - ), - ], - ), - ), - ), - ); - return result ?? false; -} diff --git a/test/core/storage/local_db_folder_removal_test.dart b/test/core/storage/local_db_folder_removal_test.dart index 6eccc1c7..7d64c61b 100644 --- a/test/core/storage/local_db_folder_removal_test.dart +++ b/test/core/storage/local_db_folder_removal_test.dart @@ -6,8 +6,8 @@ import 'package:querya_desktop/core/storage/local_db.dart'; import '../../support/local_db_test_support.dart'; -/// Removing a folder deletes the connections in it (foreign key cascade). Their -/// secrets are in the OS store, which the cascade does not reach. +/// `connections.folder_id` cascades on delete; removing a folder must not take +/// its connections, or their saved passwords, with it. void main() { late Directory dir; @@ -23,28 +23,35 @@ void main() { createdAt: DateTime.utc(2026).toIso8601String(), ); - test('removing a folder deletes its connections and their secrets', + test('removing a folder keeps its connections, now without a folder', () async { final db = LocalDb.instance; - await db.addFolder('Doomed'); - final folderId = await db.getFolderIdByName('Doomed'); - final inside = await db.addConnection(row('Inside', folderId: folderId)); - final outside = await db.addConnection(row('Outside')); - await ConnectionSecretsStore.writeForConnection(inside, password: 'in-pw'); - await ConnectionSecretsStore.writeForConnection(outside, - password: 'out-pw'); - - expect(await db.countConnectionsInFolder('Doomed'), 1); - expect(await db.countConnectionsInFolder('Nowhere'), 0); - - await db.removeFolder('Doomed'); - - final names = (await db.getConnections()).map((c) => c.name); - expect(names, contains('Outside')); - expect(names, isNot(contains('Inside'))); - expect((await ConnectionSecretsStore.readForConnection(inside)).password, - anyOf(isNull, isEmpty)); - expect((await ConnectionSecretsStore.readForConnection(outside)).password, - 'out-pw'); + await db.addFolder('Team A'); + await db.addFolder('Team B'); + final a = await db.getFolderIdByName('Team A'); + final b = await db.getFolderIdByName('Team B'); + final inA = await db.addConnection(row('In A', folderId: a)); + final inB = await db.addConnection(row('In B', folderId: b)); + final root = await db.addConnection(row('Root')); + await ConnectionSecretsStore.writeForConnection(inA, password: 'a-pw'); + + await db.removeFolder('Team A'); + + final byId = {for (final c in await db.getConnections()) c.id: c}; + expect(byId.keys, containsAll([inA, inB, root])); + expect(byId[inA]!.folderId, isNull); + expect(byId[inB]!.folderId, b, reason: 'another folder is untouched'); + expect(byId[root]!.folderId, isNull); + expect(await db.getFolderIdByName('Team A'), isNull); + expect((await ConnectionSecretsStore.readForConnection(inA)).password, + 'a-pw'); + }); + + test('removing an empty or unknown folder is harmless', () async { + final db = LocalDb.instance; + await db.addFolder('Empty'); + await db.removeFolder('Empty'); + await db.removeFolder('Never existed'); + expect(await db.getFolderIdByName('Empty'), isNull); }); } diff --git a/test/features/connections/remove_folder_dialog_test.dart b/test/features/connections/remove_folder_dialog_test.dart deleted file mode 100644 index efc13405..00000000 --- a/test/features/connections/remove_folder_dialog_test.dart +++ /dev/null @@ -1,39 +0,0 @@ -import 'package:flutter/material.dart' as material; -import 'package:flutter_test/flutter_test.dart'; -import 'package:querya_desktop/features/connections/remove_folder_dialog.dart'; - -import '../../support/querya_theme_test_shell.dart'; - -void main() { - Future open(WidgetTester t, int count, String tap) async { - bool? result; - await t.pumpWidget(queryaThemeTestShell( - child: material.Builder( - builder: (context) => material.ElevatedButton( - onPressed: () async => result = await confirmRemoveFolder(context, - name: 'Team A', connectionCount: count), - child: const material.Text('Open'), - ), - ), - )); - await t.tap(find.text('Open')); - await t.pumpAndSettle(); - expect(find.text('Remove folder "Team A"?'), findsOneWidget); - await t.tap(find.text(tap)); - await t.pumpAndSettle(); - return result; - } - - testWidgets('names the folder and how many connections go with it', - (t) async { - await open(t, 3, 'Cancel'); - }); - - testWidgets('Cancel keeps the folder', (t) async { - expect(await open(t, 2, 'Cancel'), isFalse); - }); - - testWidgets('Remove folder confirms', (t) async { - expect(await open(t, 1, 'Remove folder'), isTrue); - }); -} From 0f5095202148c2908eb07de96bb5bc20327c6395 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 10 Oct 2026 11:25:05 +0300 Subject: [PATCH 2/2] test(e2e): wait for the sidebar after saving a SQLite connection The sidebar reloads from the database after the save; checking once raced it. The URI scenarios already waited. --- test/e2e/01_connection_lifecycle/e2e_connection_form_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/01_connection_lifecycle/e2e_connection_form_test.dart b/test/e2e/01_connection_lifecycle/e2e_connection_form_test.dart index 142766f2..6f40d945 100644 --- a/test/e2e/01_connection_lifecycle/e2e_connection_form_test.dart +++ b/test/e2e/01_connection_lifecycle/e2e_connection_form_test.dart @@ -203,7 +203,8 @@ void main() { expect(row, isNotNull); expect(row!.name, 'E2E Form SQLite'); expect(row.host, path); - expect(inSidebar('E2E Form SQLite'), findsOneWidget); + // The sidebar reloads from the database after the save. + await waitFor(tester, inSidebar('E2E Form SQLite'), 'the new connection'); await app.close(tester); });