From 550c034a8262adb40e71eb064b1a40a0b4948225 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Fri, 25 Sep 2026 16:53:19 +0300 Subject: [PATCH] fix(sqlite): give every session its own handle; clearer data editing (Closes #989) SqliteConnection opened files with sqflite's default singleInstance: true, so the pool's read-only browse, table-write and SQL-editor sessions for one file shared a single Database. A Save session reused the read-only browse handle ('attempt to write a readonly database'), and closing any session (idle close after Save, the SQL editor, Disconnect) closed the file for every other view, so the database stopped opening. Open with singleInstance: false. Table Browser now re-acquires its browse session when it was closed under it instead of showing 'Not connected'. Editing UX: - One set of edit controls: the header copy (badge / Revert / Save) is removed from the PostgreSQL, MySQL and SQLite table views, so DDL, Refresh and paging stay visible while editing. The grid toolbar keeps row actions on the left and pins the pending badge, Revert All and Save Changes on the right with standard buttons; narrow widths collapse to icons / 'Save'. - Save errors are explained in plain language (read-only, busy, unique, NOT NULL, foreign key, CHECK, type, stale row, lost connection) with the raw error under Details and a copy button; the SQL workspaces use the same dialog. - The saved toast says '1 change saved' / 'N changes saved'. --- lib/core/database/sqlite_connection.dart | 6 + lib/features/mysql/mysql_sql_workspace.dart | 33 +- lib/features/mysql/mysql_table_view.dart | 10 +- .../postgresql/postgres_sql_workspace.dart | 27 +- .../postgresql/postgres_table_view.dart | 11 +- lib/features/sqlite/sqlite_sql_workspace.dart | 33 +- lib/features/sqlite/sqlite_table_view.dart | 40 ++- .../workspace/data_grid_staging_toolbar.dart | 335 ++++++++++-------- .../workspace/save_error_description.dart | 173 +++++++++ .../workspace/table_view_staging.dart | 214 ++++++----- .../core/database/sqlite_connection_test.dart | 96 +++++ test/features/workspace/results_tab_test.dart | 47 ++- .../save_error_description_test.dart | 138 ++++++++ .../workspace/table_view_staging_test.dart | 40 --- 14 files changed, 799 insertions(+), 404 deletions(-) create mode 100644 lib/features/workspace/save_error_description.dart create mode 100644 test/features/workspace/save_error_description_test.dart diff --git a/lib/core/database/sqlite_connection.dart b/lib/core/database/sqlite_connection.dart index 474d511..edf22ec 100644 --- a/lib/core/database/sqlite_connection.dart +++ b/lib/core/database/sqlite_connection.dart @@ -94,6 +94,12 @@ class SqliteConnection { path, options: OpenDatabaseOptions( readOnly: readOnly, + // Every SqliteConnection needs its own handle. With the sqflite + // default (singleInstance: true) a second open of the same path + // returns the first Database: a Save session would reuse a + // read-only browse handle, and closing any pooled session would + // close the file for every other session too. + singleInstance: false, onOpen: (db) async { await db.execute('PRAGMA busy_timeout = 5000'); await db.rawQuery('SELECT 1'); diff --git a/lib/features/mysql/mysql_sql_workspace.dart b/lib/features/mysql/mysql_sql_workspace.dart index 6765332..204901c 100644 --- a/lib/features/mysql/mysql_sql_workspace.dart +++ b/lib/features/mysql/mysql_sql_workspace.dart @@ -146,7 +146,8 @@ class _MysqlSqlWorkspaceState extends material.State { final hasDirtyStaging = session.stagingBuffer != null && session.stagingBuffer!.isDirty; final hasUnsavedText = session.isModified || - (session.filePath == null && session.controller.text.trim().isNotEmpty); + (session.filePath == null && + session.controller.text.trim().isNotEmpty); final String message; if (hasDirtyStaging && hasUnsavedText) { message = @@ -551,8 +552,7 @@ class _MysqlSqlWorkspaceState extends material.State { session.stagingBuffer?.dispose(); setState(() { session.rows = newRows; - session.stagingBuffer = - DataGridStagingBuffer( + session.stagingBuffer = DataGridStagingBuffer( columns: session.columns, rows: session.rows, primaryKeys: session.resultGridPrimaryKeys, @@ -562,32 +562,7 @@ class _MysqlSqlWorkspaceState extends material.State { } catch (e) { if (mounted) { setState(() => session.savingChanges = false); - await showAppDialog( - context: context, - builder: (ctx) => QueryaDialogCard( - constraints: const material.BoxConstraints(maxWidth: 420), - child: material.Padding( - padding: const material.EdgeInsets.all(20), - child: material.Column( - mainAxisSize: material.MainAxisSize.min, - crossAxisAlignment: material.CrossAxisAlignment.start, - children: [ - const Text('Save Changes Failed').semiBold().large(), - const Gap(8), - Text(e.toString()).muted().small(), - const Gap(20), - material.Align( - alignment: material.Alignment.centerRight, - child: PrimaryButton( - onPressed: () => material.Navigator.of(ctx).pop(), - child: const Text('OK'), - ), - ), - ], - ), - ), - ), - ); + await showTableViewSaveFailedDialog(context: context, error: e); } } } diff --git a/lib/features/mysql/mysql_table_view.dart b/lib/features/mysql/mysql_table_view.dart index a86764d..9606e39 100644 --- a/lib/features/mysql/mysql_table_view.dart +++ b/lib/features/mysql/mysql_table_view.dart @@ -587,7 +587,7 @@ class _MysqlTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); await _fetch(refreshCount: true); @@ -609,7 +609,7 @@ class _MysqlTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); return; @@ -705,12 +705,6 @@ class _MysqlTableViewState extends material.State { ), ), const Gap(6), - if (_stagingBuffer != null) - TableBrowserPendingActions( - buffer: _stagingBuffer!, - onSave: () => unawaited(_applyStagedChanges()), - isSaving: _isSaving, - ), OutlineButton( size: ButtonSize.small, onPressed: _openSqlEditor, diff --git a/lib/features/postgresql/postgres_sql_workspace.dart b/lib/features/postgresql/postgres_sql_workspace.dart index 23558f4..2377c06 100644 --- a/lib/features/postgresql/postgres_sql_workspace.dart +++ b/lib/features/postgresql/postgres_sql_workspace.dart @@ -689,32 +689,7 @@ class _PostgresSqlWorkspaceState extends material.State { } catch (e) { if (mounted) { setState(() => session.savingChanges = false); - await showAppDialog( - context: context, - builder: (ctx) => QueryaDialogCard( - constraints: const material.BoxConstraints(maxWidth: 420), - child: material.Padding( - padding: const material.EdgeInsets.all(20), - child: material.Column( - mainAxisSize: material.MainAxisSize.min, - crossAxisAlignment: material.CrossAxisAlignment.start, - children: [ - const Text('Save Changes Failed').semiBold().large(), - const Gap(8), - Text(e.toString()).muted().small(), - const Gap(20), - material.Align( - alignment: material.Alignment.centerRight, - child: PrimaryButton( - onPressed: () => material.Navigator.of(ctx).pop(), - child: const Text('OK'), - ), - ), - ], - ), - ), - ), - ); + await showTableViewSaveFailedDialog(context: context, error: e); } } } diff --git a/lib/features/postgresql/postgres_table_view.dart b/lib/features/postgresql/postgres_table_view.dart index bd2b6b5..f09df63 100644 --- a/lib/features/postgresql/postgres_table_view.dart +++ b/lib/features/postgresql/postgres_table_view.dart @@ -605,7 +605,7 @@ class _PostgresTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); await _fetch(refreshCount: true); @@ -627,7 +627,7 @@ class _PostgresTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); return; @@ -667,13 +667,6 @@ class _PostgresTableViewState extends material.State { onGoPrevious: _goToPreviousPage, onGoNext: _goToNextPage, onRefresh: () => unawaited(_onRefresh()), - pendingActions: _stagingBuffer == null - ? null - : TableBrowserPendingActions( - buffer: _stagingBuffer!, - onSave: () => unawaited(_applyStagedChanges()), - isSaving: _isSaving, - ), ); final buffer = _stagingBuffer; if (buffer == null) return toolbar(); diff --git a/lib/features/sqlite/sqlite_sql_workspace.dart b/lib/features/sqlite/sqlite_sql_workspace.dart index a0367cb..6ea9aa8 100644 --- a/lib/features/sqlite/sqlite_sql_workspace.dart +++ b/lib/features/sqlite/sqlite_sql_workspace.dart @@ -150,7 +150,8 @@ class _SqliteSqlWorkspaceState extends material.State { final hasDirtyStaging = session.stagingBuffer != null && session.stagingBuffer!.isDirty; final hasUnsavedText = session.isModified || - (session.filePath == null && session.controller.text.trim().isNotEmpty); + (session.filePath == null && + session.controller.text.trim().isNotEmpty); final String message; if (hasDirtyStaging && hasUnsavedText) { message = @@ -530,8 +531,7 @@ class _SqliteSqlWorkspaceState extends material.State { session.stagingBuffer?.dispose(); setState(() { session.rows = newRows; - session.stagingBuffer = - DataGridStagingBuffer( + session.stagingBuffer = DataGridStagingBuffer( columns: session.columns, rows: session.rows, primaryKeys: session.resultGridPrimaryKeys, @@ -541,32 +541,7 @@ class _SqliteSqlWorkspaceState extends material.State { } catch (e) { if (mounted) { setState(() => session.savingChanges = false); - await showAppDialog( - context: context, - builder: (ctx) => QueryaDialogCard( - constraints: const material.BoxConstraints(maxWidth: 420), - child: material.Padding( - padding: const material.EdgeInsets.all(20), - child: material.Column( - mainAxisSize: material.MainAxisSize.min, - crossAxisAlignment: material.CrossAxisAlignment.start, - children: [ - const Text('Save Changes Failed').semiBold().large(), - const Gap(8), - Text(e.toString()).muted().small(), - const Gap(20), - material.Align( - alignment: material.Alignment.centerRight, - child: PrimaryButton( - onPressed: () => material.Navigator.of(ctx).pop(), - child: const Text('OK'), - ), - ), - ], - ), - ), - ), - ); + await showTableViewSaveFailedDialog(context: context, error: e); } } } diff --git a/lib/features/sqlite/sqlite_table_view.dart b/lib/features/sqlite/sqlite_table_view.dart index 5685556..e1a6683 100644 --- a/lib/features/sqlite/sqlite_table_view.dart +++ b/lib/features/sqlite/sqlite_table_view.dart @@ -267,8 +267,32 @@ class _SqliteTableViewState extends material.State { ); } + /// Browse connection, re-acquired when the pooled session was closed under + /// us (tree Disconnect, another view interrupting the shared slot). + Future _ensureBrowseConnection() async { + final current = _connection; + if (current != null && current.isConnected) return current; + _lease?.release(); + _lease = null; + try { + final lease = await SqliteService.instance.acquire( + widget.connectionRow, + mode: SqliteSessionMode.readOnly, + ); + if (!mounted) { + lease.release(); + return null; + } + _lease = lease; + return lease.connection; + } catch (_) { + return null; + } + } + Future _fetch() async { - final conn = _connection; + final conn = await _ensureBrowseConnection(); + if (!mounted) return; if (conn == null || !conn.isConnected) { if (mounted && _loading) { setState(() { @@ -410,7 +434,7 @@ class _SqliteTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); await _fetch(); @@ -432,7 +456,7 @@ class _SqliteTableViewState extends material.State { }); showAppToast( context: context, - message: '${outcome.statementCount} change(s) saved', + message: tableViewSavedMessage(outcome.statementCount), variant: AppToastVariant.success, ); return; @@ -447,8 +471,8 @@ class _SqliteTableViewState extends material.State { } Future _showDdlDialog() async { - final conn = _connection; - if (conn == null || !conn.isConnected) return; + final conn = await _ensureBrowseConnection(); + if (!mounted || conn == null || !conn.isConnected) return; final navigator = material.Navigator.of(context, rootNavigator: true); unawaited(showAppDialog( context: context, @@ -622,12 +646,6 @@ class _SqliteTableViewState extends material.State { ), ), const Gap(6), - if (_stagingBuffer != null) - TableBrowserPendingActions( - buffer: _stagingBuffer!, - onSave: () => unawaited(_applyStagedChanges()), - isSaving: _isSaving, - ), OutlineButton( size: ButtonSize.small, onPressed: _loading diff --git a/lib/features/workspace/data_grid_staging_toolbar.dart b/lib/features/workspace/data_grid_staging_toolbar.dart index 1bc5c93..ea1e651 100644 --- a/lib/features/workspace/data_grid_staging_toolbar.dart +++ b/lib/features/workspace/data_grid_staging_toolbar.dart @@ -17,6 +17,9 @@ class DataGridStagingToolbar extends StatelessWidget { final VoidCallback? onApplyChanges; final bool isSaving; + /// Below this width row actions collapse to icons (tooltips keep the names). + static const double compactWidth = 620; + @override Widget build(BuildContext context) { final cs = Theme.of(context).colorScheme; @@ -30,13 +33,12 @@ class DataGridStagingToolbar extends StatelessWidget { final hasSelectedRow = selectedRow != null && selectedRow >= 0 && selectedRow < stagingBuffer.totalRowCount; - final isSelectedDeleted = hasSelectedRow && stagingBuffer.getRowStatus(selectedRow) == StagedRowStatus.deleted; return material.Container( - height: 32, - padding: const material.EdgeInsets.symmetric(horizontal: 10), + height: 38, + padding: const material.EdgeInsets.only(left: 6, right: 8), decoration: material.BoxDecoration( color: cs.card, border: material.Border( @@ -46,162 +48,128 @@ class DataGridStagingToolbar extends StatelessWidget { ), ), ), - child: material.SingleChildScrollView( - scrollDirection: material.Axis.horizontal, - child: material.Row( - mainAxisSize: material.MainAxisSize.min, - children: [ - // Add Row - _ToolbarButton( - label: 'Add Row', - icon: material.Icons.add_rounded, - tooltip: 'Add new row (Ctrl+Insert / Cmd+N)', - onPressed: isSaving ? null : () => stagingBuffer.addRow(), - ), - const Gap(4), - - // Delete / Restore Row - _ToolbarButton( - label: isSelectedDeleted ? 'Restore Row' : 'Delete Row', - icon: isSelectedDeleted - ? material.Icons.restore_from_trash_rounded - : material.Icons.remove_circle_outline_rounded, - color: isSelectedDeleted - ? cs.primary - : (hasSelectedRow ? cs.destructive : null), - tooltip: isSelectedDeleted - ? 'Restore marked row' - : 'Mark row for deletion (Ctrl+Delete / Cmd+Backspace)', - onPressed: isSaving || !hasSelectedRow - ? null - : () => stagingBuffer.toggleDeleteRow(selectedRow), - ), - const Gap(4), - - // Revert - if (isDirty) ...[ - _ToolbarButton( - label: 'Revert All', - icon: material.Icons.undo_rounded, - color: cs.mutedForeground, - tooltip: 'Revert all unstaged edits (Ctrl+Z / Cmd+Z)', - onPressed: isSaving ? null : () => stagingBuffer.revertAll(), - ), - const Gap(6), - ], - - // Badge - if (isDirty) - material.Container( - padding: const material.EdgeInsets.symmetric( - horizontal: 8, - vertical: 2, - ), - decoration: material.BoxDecoration( - color: cs.primary.withValues(alpha: 0.15), - borderRadius: material.BorderRadius.circular(10), - border: material.Border.all( - color: cs.primary.withValues(alpha: 0.3), - width: 1, + child: material.LayoutBuilder( + builder: (context, constraints) { + final compact = constraints.maxWidth < compactWidth; + return material.Row( + children: [ + // Row actions: may scroll when space is tight. + material.Expanded( + child: material.SingleChildScrollView( + scrollDirection: material.Axis.horizontal, + child: material.Row( + mainAxisSize: material.MainAxisSize.min, + children: [ + _ToolbarButton( + label: 'Add Row', + icon: material.Icons.add_rounded, + compact: compact, + tooltip: 'Add new row (Ctrl+Insert / Cmd+N)', + onPressed: + isSaving ? null : () => stagingBuffer.addRow(), + ), + const Gap(2), + _ToolbarButton( + label: + isSelectedDeleted ? 'Restore Row' : 'Delete Row', + icon: isSelectedDeleted + ? material.Icons.restore_from_trash_rounded + : material.Icons.remove_circle_outline_rounded, + compact: compact, + color: isSelectedDeleted + ? cs.primary + : (hasSelectedRow ? cs.destructive : null), + tooltip: isSelectedDeleted + ? 'Restore marked row' + : hasSelectedRow + ? 'Mark row for deletion (Ctrl+Delete / Cmd+Backspace)' + : 'Select a row to delete it', + onPressed: isSaving || !hasSelectedRow + ? null + : () => + stagingBuffer.toggleDeleteRow(selectedRow), + ), + ], ), ), - child: material.Row( + ), + const Gap(8), + // Commit area: always visible on the right. + if (isDirty) ...[ + _PendingBadge(count: changeCount, compact: compact), + const Gap(6), + material.Tooltip( + message: 'Discard all pending edits (Ctrl+Z / Cmd+Z)', + waitDuration: const Duration(milliseconds: 400), + child: compact + ? OutlineButton( + size: ButtonSize.small, + density: ButtonDensity.icon, + onPressed: isSaving + ? null + : () => stagingBuffer.revertAll(), + child: const material.Icon( + material.Icons.undo_rounded, + size: 14, + ), + ) + : OutlineButton( + size: ButtonSize.small, + onPressed: isSaving + ? null + : () => stagingBuffer.revertAll(), + leading: const material.Icon( + material.Icons.undo_rounded, + size: 14, + ), + child: const Text('Revert All'), + ), + ), + ] else + material.Row( mainAxisSize: material.MainAxisSize.min, children: [ - material.Container( - width: 5, - height: 5, - decoration: material.BoxDecoration( - color: cs.primary, - shape: material.BoxShape.circle, - ), + material.Icon( + material.Icons.check_circle_outline_rounded, + size: 13, + color: cs.mutedForeground, ), - const Gap(5), - Text( - '$changeCount pending ${changeCount == 1 ? 'change' : 'changes'}', - ).xSmall().semiBold(), + const Gap(4), + const Text('No changes').xSmall().muted(), ], ), - ) - else - material.Row( - mainAxisSize: material.MainAxisSize.min, - children: [ - material.Icon( - material.Icons.check_circle_outline_rounded, - size: 13, - color: cs.mutedForeground, - ), - const Gap(4), - const Text('No changes').xSmall().muted(), - ], - ), - - const Gap(12), - - // Save Changes button - material.Tooltip( - message: isDirty - ? 'Commit staged changes to database (Ctrl+S / Cmd+S)' - : 'No pending changes to commit', - waitDuration: const Duration(milliseconds: 400), - child: material.MouseRegion( - cursor: (isDirty && !isSaving) - ? material.SystemMouseCursors.click - : material.SystemMouseCursors.basic, - child: material.GestureDetector( - behavior: material.HitTestBehavior.opaque, - onTap: isDirty && !isSaving ? onApplyChanges : null, - child: material.Container( - padding: const material.EdgeInsets.symmetric( - horizontal: 10, - vertical: 4, - ), - decoration: material.BoxDecoration( - color: isDirty - ? cs.primary - : cs.muted.withValues(alpha: 0.4), - borderRadius: material.BorderRadius.circular(4), - ), - child: material.Row( - mainAxisSize: material.MainAxisSize.min, - children: [ - if (isSaving) - material.SizedBox( - width: 12, - height: 12, - child: material.CircularProgressIndicator( - strokeWidth: 2, - color: cs.primaryForeground, - ), - ) - else - material.Icon( - material.Icons.save_rounded, - size: 14, - color: isDirty - ? cs.primaryForeground - : cs.mutedForeground, - ), - const Gap(5), - material.Text( - isSaving ? 'Saving…' : 'Save Changes', - style: material.TextStyle( - fontSize: 12, - fontWeight: material.FontWeight.w500, - color: isDirty - ? cs.primaryForeground - : cs.mutedForeground, + const Gap(6), + material.Tooltip( + message: isDirty + ? 'Review and save pending changes (Ctrl+S / Cmd+S)' + : 'No pending changes to save', + waitDuration: const Duration(milliseconds: 400), + child: PrimaryButton( + size: ButtonSize.small, + onPressed: isDirty && !isSaving ? onApplyChanges : null, + leading: isSaving + ? material.SizedBox( + width: 12, + height: 12, + child: material.CircularProgressIndicator( + strokeWidth: 2, + color: cs.primaryForeground, ), + ) + : const material.Icon( + material.Icons.save_rounded, + size: 14, ), - ], - ), + child: Text( + isSaving + ? 'Saving…' + : (compact ? 'Save' : 'Save Changes'), ), ), ), - ), - ], - ), + ], + ); + }, ), ); }, @@ -209,6 +177,51 @@ class DataGridStagingToolbar extends StatelessWidget { } } +class _PendingBadge extends StatelessWidget { + const _PendingBadge({required this.count, required this.compact}); + + final int count; + final bool compact; + + @override + Widget build(BuildContext context) { + final cs = Theme.of(context).colorScheme; + final label = compact + ? '$count' + : '$count pending ${count == 1 ? 'change' : 'changes'}'; + return material.Tooltip( + message: '$count pending ${count == 1 ? 'change' : 'changes'} not saved yet', + waitDuration: const Duration(milliseconds: 400), + child: material.Container( + padding: const material.EdgeInsets.symmetric(horizontal: 8, vertical: 2), + decoration: material.BoxDecoration( + color: cs.primary.withValues(alpha: 0.15), + borderRadius: material.BorderRadius.circular(10), + border: material.Border.all( + color: cs.primary.withValues(alpha: 0.3), + width: 1, + ), + ), + child: material.Row( + mainAxisSize: material.MainAxisSize.min, + children: [ + material.Container( + width: 5, + height: 5, + decoration: material.BoxDecoration( + color: cs.primary, + shape: material.BoxShape.circle, + ), + ), + const Gap(5), + Text(label).xSmall().semiBold(), + ], + ), + ), + ); + } +} + class _ToolbarButton extends material.StatefulWidget { const _ToolbarButton({ required this.label, @@ -216,8 +229,11 @@ class _ToolbarButton extends material.StatefulWidget { this.onPressed, this.color, this.tooltip, + this.compact = false, }); + /// Icon only; the label moves into the tooltip. + final bool compact; final String label; final material.IconData icon; final material.VoidCallback? onPressed; @@ -267,15 +283,17 @@ class _ToolbarButtonState extends material.State<_ToolbarButton> { mainAxisSize: material.MainAxisSize.min, children: [ material.Icon(widget.icon, size: 14, color: fg), - const material.SizedBox(width: 4), - material.Text( - widget.label, - style: material.TextStyle( - fontSize: 12, - fontWeight: material.FontWeight.w500, - color: fg, + if (!widget.compact) ...[ + const material.SizedBox(width: 4), + material.Text( + widget.label, + style: material.TextStyle( + fontSize: 12, + fontWeight: material.FontWeight.w500, + color: fg, + ), ), - ), + ], ], ), ), @@ -283,9 +301,12 @@ class _ToolbarButtonState extends material.State<_ToolbarButton> { ), ); - if (widget.tooltip != null) { + final tooltip = widget.compact + ? '${widget.label}${widget.tooltip != null ? ' — ${widget.tooltip}' : ''}' + : widget.tooltip; + if (tooltip != null) { button = material.Tooltip( - message: widget.tooltip!, + message: tooltip, waitDuration: const Duration(milliseconds: 400), child: button, ); diff --git a/lib/features/workspace/save_error_description.dart b/lib/features/workspace/save_error_description.dart new file mode 100644 index 0000000..9be7cf1 --- /dev/null +++ b/lib/features/workspace/save_error_description.dart @@ -0,0 +1,173 @@ +/// Human-readable explanation of a failed staged-changes Save. +/// +/// Save runs every statement in one transaction, so a failure always means +/// nothing was written; the raw driver error stays available as [details]. +class SaveErrorDescription { + const SaveErrorDescription({ + required this.title, + required this.message, + this.hint, + required this.details, + }); + + final String title; + final String message; + final String? hint; + + /// The original error text, for the collapsible details / copy. + final String details; +} + +/// Maps common SQLite / PostgreSQL / MySQL errors to plain language. +SaveErrorDescription describeSaveError(Object error) { + final raw = error.toString(); + final text = _innermostMessage(raw); + final lower = raw.toLowerCase(); + + SaveErrorDescription d(String title, String message, [String? hint]) => + SaveErrorDescription( + title: title, + message: message, + hint: hint, + details: raw, + ); + + if (lower.contains('readonly database') || + lower.contains('read-only') || + lower.contains('read only transaction')) { + return d( + 'The database is read-only', + 'This connection cannot write to the database.', + 'Turn off "Read only" in the connection settings or check the file / user permissions.', + ); + } + + if (lower.contains('sqlite is busy') || + lower.contains('database is locked') || + lower.contains('sqlite_busy') || + lower.contains('lock wait timeout')) { + return d( + 'The database is busy', + 'Another connection is writing to the database right now.', + 'If the SQL editor has an open transaction, commit or roll it back, then save again.', + ); + } + + final uniqueCol = _firstGroup(text, [ + RegExp(r'UNIQUE constraint failed: ([\w."]+)', caseSensitive: false), + RegExp(r'violates unique constraint "([^"]+)"', caseSensitive: false), + RegExp(r"Duplicate entry '[^']*' for key '([^']+)'", caseSensitive: false), + ]); + if (uniqueCol != null || lower.contains('unique constraint')) { + return d( + 'Duplicate value', + uniqueCol == null + ? 'A value must be unique, but it already exists.' + : 'The value for ${_pretty(uniqueCol)} must be unique, but it already exists.', + 'Change the value or remove the other row first.', + ); + } + + final notNullCol = _firstGroup(text, [ + RegExp(r'NOT NULL constraint failed: ([\w."]+)', caseSensitive: false), + RegExp(r'null value in column "([^"]+)"', caseSensitive: false), + RegExp(r"Column '([^']+)' cannot be null", caseSensitive: false), + ]); + if (notNullCol != null || lower.contains('not-null constraint')) { + return d( + 'Missing required value', + notNullCol == null + ? 'A required column was left empty.' + : '${_pretty(notNullCol)} cannot be empty.', + 'Enter a value for this column and save again.', + ); + } + + if (lower.contains('foreign key')) { + return d( + 'Related row problem', + 'The change breaks a foreign key: a referenced row is missing, or another row still points to this one.', + 'Check the related table, then save again.', + ); + } + + if (lower.contains('check constraint')) { + return d( + 'Value not allowed', + 'A value breaks a CHECK rule on this table.', + ); + } + + if (lower.contains('datatype mismatch') || + lower.contains('invalid input syntax') || + lower.contains('incorrect integer value') || + lower.contains('incorrect decimal value') || + lower.contains('out of range')) { + return d( + 'Wrong value type', + 'A value does not match the column type.', + text.isEmpty ? null : text, + ); + } + + if (lower.contains('matched 0 rows')) { + return d( + 'The row changed', + 'The row was changed or deleted since it was loaded.', + 'Refresh the table and apply your edit again.', + ); + } + + if (lower.contains('instead of 1')) { + return d( + 'Row is not unique', + 'The edit would change several identical rows at once.', + 'The table has duplicate rows or no unique key. Add a primary key to edit such rows.', + ); + } + + if (lower.contains('not connected') || + lower.contains('database_closed') || + lower.contains('connection closed') || + lower.contains('connection refused') || + lower.contains('socket')) { + return d( + 'Connection lost', + 'The connection to the database was closed.', + 'Refresh to reconnect, then save again.', + ); + } + + return d( + 'Changes were not saved', + text.isEmpty ? raw : text, + ); +} + +/// Strips wrapper noise such as `Bad state: ` and the sqflite exception prefix. +String _innermostMessage(String raw) { + var s = raw; + final sqlite = RegExp(r'SqliteException\(\d+\): (?:while \w+, )?([^,\n]+)') + .firstMatch(s); + if (sqlite != null) return sqlite.group(1)!.trim(); + for (final prefix in ['Bad state: ', 'Exception: ', 'StateError: ']) { + if (s.startsWith(prefix)) s = s.substring(prefix.length); + } + final nl = s.indexOf('\n'); + return (nl == -1 ? s : s.substring(0, nl)).trim(); +} + +String? _firstGroup(String text, List patterns) { + for (final p in patterns) { + final m = p.firstMatch(text); + if (m != null) return m.group(1); + } + return null; +} + +/// `users.email` → `"email"`; constraint names are shown as-is. +String _pretty(String name) { + final bare = name.replaceAll('"', ''); + final dot = bare.lastIndexOf('.'); + return '"${dot == -1 ? bare : bare.substring(dot + 1)}"'; +} diff --git a/lib/features/workspace/table_view_staging.dart b/lib/features/workspace/table_view_staging.dart index 770a276..ef879e0 100644 --- a/lib/features/workspace/table_view_staging.dart +++ b/lib/features/workspace/table_view_staging.dart @@ -1,8 +1,10 @@ import 'package:flutter/material.dart' as material; +import 'package:flutter/services.dart' show Clipboard, ClipboardData; import 'package:querya_desktop/core/database/table_mutation_engine.dart'; import 'package:querya_desktop/core/database/table_schema_meta.dart'; import 'package:querya_desktop/features/workspace/data_grid_staging_buffer.dart'; import 'package:querya_desktop/features/workspace/dml_preview_dialog.dart'; +import 'package:querya_desktop/features/workspace/save_error_description.dart'; import 'package:querya_desktop/shared/widgets/widgets.dart'; /// Outcome of [loadTableViewSchema] (success vs swallowed getTableSchema error). @@ -188,36 +190,143 @@ void expectDmlMatchedRows(int affectedRows) { ); } +/// Success toast after Save, e.g. `1 change saved`, `3 changes saved`. +String tableViewSavedMessage(int count) => + '$count ${count == 1 ? 'change' : 'changes'} saved'; + +/// Explains a failed Save in plain language; the raw error is one click away. Future showTableViewSaveFailedDialog({ required material.BuildContext context, required Object error, }) { + final info = describeSaveError(error); return showAppDialog( context: context, - builder: (ctx) => QueryaDialogCard( - constraints: const material.BoxConstraints(maxWidth: 420), + builder: (ctx) => _SaveFailedDialog(info: info), + ); +} + +class _SaveFailedDialog extends material.StatefulWidget { + const _SaveFailedDialog({required this.info}); + + final SaveErrorDescription info; + + @override + material.State<_SaveFailedDialog> createState() => _SaveFailedDialogState(); +} + +class _SaveFailedDialogState extends material.State<_SaveFailedDialog> { + bool _showDetails = false; + bool _copied = false; + + Future _copy() async { + await Clipboard.setData(ClipboardData(text: widget.info.details)); + if (!mounted) return; + setState(() => _copied = true); + } + + @override + material.Widget build(material.BuildContext context) { + final cs = Theme.of(context).colorScheme; + final info = widget.info; + return QueryaDialogCard( + constraints: const material.BoxConstraints(maxWidth: 480), child: material.Padding( padding: const material.EdgeInsets.all(20), child: material.Column( mainAxisSize: material.MainAxisSize.min, crossAxisAlignment: material.CrossAxisAlignment.start, children: [ - const Text('Save Changes Failed').semiBold().large(), - const Gap(8), - Text(error.toString()).muted().small(), - const Gap(20), - material.Align( - alignment: material.Alignment.centerRight, - child: PrimaryButton( - onPressed: () => material.Navigator.of(ctx).pop(), - child: const Text('OK'), + material.Row( + children: [ + material.Icon( + material.Icons.error_outline_rounded, + size: 20, + color: cs.destructive, + ), + const Gap(8), + material.Expanded(child: Text(info.title).semiBold().large()), + ], + ), + const Gap(10), + Text(info.message).small(), + if (info.hint != null) ...[ + const Gap(6), + Text(info.hint!).muted().small(), + ], + const Gap(10), + const Text('No changes were applied. Your edits are still pending.') + .muted() + .xSmall(), + const Gap(12), + material.InkWell( + onTap: () => setState(() => _showDetails = !_showDetails), + borderRadius: material.BorderRadius.circular(4), + child: material.Padding( + padding: const material.EdgeInsets.symmetric(vertical: 4), + child: material.Row( + mainAxisSize: material.MainAxisSize.min, + children: [ + material.Icon( + _showDetails + ? material.Icons.expand_less_rounded + : material.Icons.expand_more_rounded, + size: 16, + color: cs.mutedForeground, + ), + const Gap(4), + const Text('Details').muted().xSmall(), + ], + ), ), ), + if (_showDetails) + material.Container( + width: double.infinity, + constraints: const material.BoxConstraints(maxHeight: 180), + margin: const material.EdgeInsets.only(top: 6), + padding: const material.EdgeInsets.all(10), + decoration: material.BoxDecoration( + color: cs.muted.withValues(alpha: 0.4), + borderRadius: material.BorderRadius.circular(6), + ), + child: material.SingleChildScrollView( + child: material.SelectableText( + info.details, + style: material.TextStyle( + fontFamily: 'monospace', + fontSize: 11, + color: cs.mutedForeground, + ), + ), + ), + ), + const Gap(18), + material.Row( + mainAxisAlignment: material.MainAxisAlignment.end, + children: [ + OutlineButton( + onPressed: _copy, + leading: material.Icon( + _copied + ? material.Icons.check_rounded + : material.Icons.copy_rounded, + size: 14, + ), + child: Text(_copied ? 'Copied' : 'Copy details'), + ), + const Gap(8), + PrimaryButton( + onPressed: () => material.Navigator.of(context).pop(), + child: const Text('OK'), + ), + ], + ), ], ), ), - ), - ); + ); + } } /// Result of [applyTableViewStagedChanges]. @@ -290,82 +399,3 @@ Future applyTableViewStagedChanges({ return TableViewApplyOutcome.failed(e); } } - -/// Compact Save / Revert + pending badge for Table Browser chrome. -class TableBrowserPendingActions extends material.StatelessWidget { - const TableBrowserPendingActions({ - super.key, - required this.buffer, - required this.onSave, - required this.isSaving, - }); - - final DataGridStagingBuffer buffer; - final material.VoidCallback? onSave; - final bool isSaving; - - @override - material.Widget build(material.BuildContext context) { - return ListenableBuilder( - listenable: buffer, - builder: (context, _) { - if (!buffer.isDirty) return const material.SizedBox.shrink(); - final cs = Theme.of(context).colorScheme; - final n = buffer.changeCount; - return material.Row( - mainAxisSize: material.MainAxisSize.min, - children: [ - material.Container( - padding: const material.EdgeInsets.symmetric( - horizontal: 8, - vertical: 3, - ), - decoration: material.BoxDecoration( - color: cs.primary.withValues(alpha: 0.15), - borderRadius: material.BorderRadius.circular(4), - border: material.Border.all( - color: cs.primary.withValues(alpha: 0.3), - ), - ), - child: material.Text( - '$n pending ${n == 1 ? 'change' : 'changes'}', - style: material.TextStyle( - fontSize: 11, - fontWeight: material.FontWeight.w600, - color: cs.primary, - ), - ), - ), - const Gap(4), - OutlineButton( - size: ButtonSize.small, - onPressed: isSaving ? null : () => buffer.revertAll(), - leading: const material.Icon( - material.Icons.undo_rounded, - size: 14, - ), - child: const Text('Revert'), - ), - const Gap(4), - PrimaryButton( - size: ButtonSize.small, - onPressed: isSaving ? null : onSave, - leading: isSaving - ? material.SizedBox( - width: 12, - height: 12, - child: material.CircularProgressIndicator( - strokeWidth: 2, - color: cs.primaryForeground, - ), - ) - : const material.Icon(material.Icons.save_rounded, size: 14), - child: Text(isSaving ? 'Saving…' : 'Save'), - ), - const Gap(6), - ], - ); - }, - ); - } -} diff --git a/test/core/database/sqlite_connection_test.dart b/test/core/database/sqlite_connection_test.dart index a19e8ee..948d928 100644 --- a/test/core/database/sqlite_connection_test.dart +++ b/test/core/database/sqlite_connection_test.dart @@ -699,4 +699,100 @@ void main() { expect(colsComputed, ['flag', 'cnt']); }); }); + + group('SQLite sessions on the same file are independent', () { + late Directory dir; + late String path; + + setUp(() async { + dir = await Directory.systemTemp.createTemp('querya_sqlite_sessions_'); + path = '${dir.path}/shared.sqlite'; + final setup = SqliteConnection( + id: 1, + name: 'setup', + path: path, + createIfMissing: true, + ); + await setup.connect(); + await setup.execute('CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)'); + await setup.execute("INSERT INTO t (v) VALUES ('a'), ('b')"); + await setup.disconnect(); + }); + + tearDown(() async { + if (await dir.exists()) await dir.delete(recursive: true); + }); + + test('a write session opened after a read-only browse session can write', + () async { + final browse = + SqliteConnection(id: 1, name: 'x', path: path, readOnly: true); + final write = SqliteConnection(id: 1, name: 'x', path: path); + await browse.connect(); + await write.connect(); + addTearDown(browse.disconnect); + addTearDown(write.disconnect); + + expect( + await write.executeAffected("UPDATE t SET v = 'edited' WHERE id = 1"), + 1, + ); + }); + + test('closing one session does not close the file for the others', + () async { + final browse = + SqliteConnection(id: 1, name: 'x', path: path, readOnly: true); + final write = SqliteConnection(id: 1, name: 'x', path: path); + await browse.connect(); + await write.connect(); + addTearDown(browse.disconnect); + + await write.executeAffected("UPDATE t SET v = 'saved' WHERE id = 2"); + // The pool idle-closes the Save session a few seconds after Save. + await write.disconnect(); + + final rows = await browse.execute('SELECT v FROM t ORDER BY id'); + expect(rows.map((r) => r['v']), ['a', 'saved']); + }); + + test('pool sessions for one connection survive each other being released', + () async { + final pool = SqliteConnectionPool( + createAndConnect: (row, {required mode}) async { + final c = SqliteConnection( + id: row.id!, + name: row.name, + path: path, + readOnly: mode.isReadOnlySession, + ); + await c.connect(); + return c; + }, + idleDisposeDelay: Duration.zero, + ); + addTearDown(pool.disconnectAll); + final row = ConnectionRow( + id: 1, + type: 'sqlite', + name: 'x', + host: path, + createdAt: '', + ); + + final browse = await pool.acquire(row); + final save = await pool.acquire(row, mode: SqliteSessionMode.tableWrite); + expect( + await save.connection + .executeAffected("UPDATE t SET v = 'pooled' WHERE id = 1"), + 1, + ); + save.release(); + await Future.delayed(const Duration(milliseconds: 20)); + + final rows = await browse.connection.execute('SELECT v FROM t WHERE id = 1'); + expect(rows.single['v'], 'pooled'); + browse.release(); + }); + }); } diff --git a/test/features/workspace/results_tab_test.dart b/test/features/workspace/results_tab_test.dart index 2248393..23f6c1c 100644 --- a/test/features/workspace/results_tab_test.dart +++ b/test/features/workspace/results_tab_test.dart @@ -1193,7 +1193,7 @@ void main() { findsOneWidget, ); expect( - find.byTooltip('No pending changes to commit'), + find.byTooltip('No pending changes to save'), findsOneWidget, ); @@ -1202,16 +1202,57 @@ void main() { await tester.pumpAndSettle(); expect( - find.byTooltip('Revert all unstaged edits (Ctrl+Z / Cmd+Z)'), + find.byTooltip('Discard all pending edits (Ctrl+Z / Cmd+Z)'), findsOneWidget, ); expect( - find.byTooltip('Commit staged changes to database (Ctrl+S / Cmd+S)'), + find.byTooltip('Review and save pending changes (Ctrl+S / Cmd+S)'), findsOneWidget, ); buffer.dispose(); }); + + testWidgets('narrow toolbar keeps Save visible and shows row actions as icons', + (tester) async { + final buffer = DataGridStagingBuffer( + columns: ['id', 'name'], + rows: [ + ['1', 'Alice'], + ], + ); + addTearDown(buffer.dispose); + buffer.setCell(0, 1, 'Grace'); + + await tester.pumpWidget( + resultsShell( + child: material.Scaffold( + body: material.Align( + alignment: material.Alignment.topLeft, + child: material.SizedBox( + width: 420, + height: 60, + child: DataGridStagingToolbar( + stagingBuffer: buffer, + onApplyChanges: () {}, + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('Add Row'), findsNothing); + expect(find.byIcon(material.Icons.add_rounded), findsOneWidget); + final save = find.text('Save'); + expect(save, findsOneWidget); + expect(find.text('Revert All'), findsNothing); + expect(find.byIcon(material.Icons.undo_rounded), findsOneWidget); + final right = tester.getBottomRight(save).dx; + expect(right, lessThanOrEqualTo(420)); + expect(tester.takeException(), isNull); + }); }); group('ResultsTab selection updates (#885)', () { diff --git a/test/features/workspace/save_error_description_test.dart b/test/features/workspace/save_error_description_test.dart new file mode 100644 index 0000000..52898ce --- /dev/null +++ b/test/features/workspace/save_error_description_test.dart @@ -0,0 +1,138 @@ +import 'package:flutter/material.dart' as material; +import 'package:flutter_test/flutter_test.dart'; +import 'package:querya_desktop/features/workspace/save_error_description.dart'; +import 'package:querya_desktop/features/workspace/table_view_staging.dart'; + +import '../../support/querya_theme_test_shell.dart'; + +void main() { + group('describeSaveError', () { + test('SQLite read-only database', () { + final d = describeSaveError(Exception( + 'SqfliteFfiException(sqlite_error: 8, , SqliteException(8): while executing, ' + 'attempt to write a readonly database, attempt to write a readonly database (code 8)', + )); + expect(d.title, 'The database is read-only'); + expect(d.hint, contains('Read only')); + expect(d.details, contains('readonly database')); + }); + + test('busy / locked database', () { + expect( + describeSaveError(StateError('SQLite is busy (another connection is writing). Retry in a moment.')) + .title, + 'The database is busy', + ); + expect(describeSaveError(Exception('database is locked')).title, + 'The database is busy'); + expect(describeSaveError(Exception('Lock wait timeout exceeded')).title, + 'The database is busy'); + }); + + test('unique violations name the column or constraint', () { + expect( + describeSaveError(Exception('SqliteException(2067): while executing, UNIQUE constraint failed: users.email, constraint failed')) + .message, + contains('"email"'), + ); + expect( + describeSaveError(Exception('duplicate key value violates unique constraint "users_email_key"')) + .message, + contains('"users_email_key"'), + ); + expect( + describeSaveError(Exception("Duplicate entry 'a@b.c' for key 'users.email'")).title, + 'Duplicate value', + ); + }); + + test('not-null violations name the column', () { + expect( + describeSaveError(Exception('SqliteException(1299): while executing, NOT NULL constraint failed: users.full_name, x')) + .message, + '"full_name" cannot be empty.', + ); + expect( + describeSaveError(Exception('null value in column "name" of relation "t" violates not-null constraint')) + .message, + '"name" cannot be empty.', + ); + expect( + describeSaveError(Exception("Column 'name' cannot be null")).message, + '"name" cannot be empty.', + ); + }); + + test('foreign key, check and type errors', () { + expect(describeSaveError(Exception('FOREIGN KEY constraint failed')).title, + 'Related row problem'); + expect(describeSaveError(Exception('CHECK constraint failed: price')).title, + 'Value not allowed'); + expect( + describeSaveError(Exception('invalid input syntax for type integer: "abc"')).title, + 'Wrong value type', + ); + }); + + test('stale row and duplicate-row matches', () { + expect( + describeSaveError(StateError('Save failed: a statement matched 0 rows. The row may have been changed or deleted.')) + .title, + 'The row changed', + ); + expect( + describeSaveError(StateError('Save failed: a statement matched 2 rows instead of 1.')).title, + 'Row is not unique', + ); + }); + + test('lost connection', () { + expect(describeSaveError(StateError('Not connected to SQLite')).title, + 'Connection lost'); + }); + + test('unknown errors keep a readable first line and the full details', () { + final d = describeSaveError(StateError('something odd\nstack line')); + expect(d.title, 'Changes were not saved'); + expect(d.message, 'something odd'); + expect(d.details, contains('stack line')); + }); + }); + + testWidgets('save-failed dialog explains the error and reveals details', + (tester) async { + late material.BuildContext ctx; + await tester.pumpWidget(queryaThemeTestShell( + child: material.Scaffold( + body: material.Builder(builder: (c) { + ctx = c; + return const material.SizedBox.shrink(); + }), + ), + )); + + showTableViewSaveFailedDialog( + context: ctx, + error: Exception('NOT NULL constraint failed: users.full_name'), + ); + await tester.pumpAndSettle(); + + expect(find.text('Missing required value'), findsOneWidget); + expect(find.text('"full_name" cannot be empty.'), findsOneWidget); + expect(find.textContaining('No changes were applied'), findsOneWidget); + expect(find.byType(material.SelectableText), findsNothing); + + await tester.tap(find.text('Details')); + await tester.pumpAndSettle(); + expect(find.byType(material.SelectableText), findsOneWidget); + + await tester.tap(find.text('OK')); + await tester.pumpAndSettle(); + expect(find.text('Missing required value'), findsNothing); + }); + + test('saved toast uses singular and plural', () { + expect(tableViewSavedMessage(1), '1 change saved'); + expect(tableViewSavedMessage(3), '3 changes saved'); + }); +} diff --git a/test/features/workspace/table_view_staging_test.dart b/test/features/workspace/table_view_staging_test.dart index 20b6881..4fcf01e 100644 --- a/test/features/workspace/table_view_staging_test.dart +++ b/test/features/workspace/table_view_staging_test.dart @@ -456,46 +456,6 @@ void main() { }); }); - group('TableBrowserPendingActions', () { - testWidgets('shows pending badge and Save when dirty', (tester) async { - final buffer = DataGridStagingBuffer( - columns: ['id', 'name'], - rows: [ - ['1', 'Ada'], - ], - ); - addTearDown(buffer.dispose); - buffer.setCell(0, 1, 'Grace'); - - var saved = false; - await tester.pumpWidget( - ShadcnApp( - theme: AppTheme.dark, - home: material.Scaffold( - body: TableBrowserPendingActions( - buffer: buffer, - onSave: () => saved = true, - isSaving: false, - ), - ), - ), - ); - - expect(find.text('1 pending change'), findsOneWidget); - expect(find.text('Save'), findsOneWidget); - expect(find.text('Revert'), findsOneWidget); - - await tester.tap(find.text('Save')); - await tester.pump(); - expect(saved, isTrue); - - await tester.tap(find.text('Revert')); - await tester.pump(); - expect(buffer.isDirty, isFalse); - expect(find.text('1 pending change'), findsNothing); - }); - }); - group('confirmDiscardTableEditsIfDirty', () { testWidgets('returns true immediately when clean', (tester) async { await tester.pumpWidget(