From 20134db2e4f271c0de44445193bedc13bcef079c Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Fri, 25 Sep 2026 13:24:17 +0300 Subject: [PATCH] fix(grid): purge deleted rows and sanitize kNullSentinel after Save in staging buffer (Closes #884) After a successful Save the new baseline was built from effectiveRows, which kept rows marked for deletion and leaked the raw NULL sentinel into cells. Add DataGridStagingBuffer.committedRows (deleted baseline rows dropped, staged NULLs mapped to 'NULL') and use it as the post-save baseline in the Postgres/MySQL/SQLite table views and SQL workspaces. effectiveRows keeps row indices aligned with the grid for struck-through rendering but now also maps the sentinel to 'NULL'. --- lib/features/mysql/mysql_sql_workspace.dart | 2 +- lib/features/mysql/mysql_table_view.dart | 2 +- .../postgresql/postgres_sql_workspace.dart | 2 +- .../postgresql/postgres_table_view.dart | 2 +- lib/features/sqlite/sqlite_sql_workspace.dart | 2 +- lib/features/sqlite/sqlite_table_view.dart | 2 +- .../workspace/data_grid_staging_buffer.dart | 49 ++++++++++++---- .../data_grid_staging_buffer_test.dart | 57 +++++++++++++++++-- .../workspace/table_view_staging_test.dart | 40 +++++++++++++ 9 files changed, 136 insertions(+), 22 deletions(-) diff --git a/lib/features/mysql/mysql_sql_workspace.dart b/lib/features/mysql/mysql_sql_workspace.dart index e8838d7..ca0edef 100644 --- a/lib/features/mysql/mysql_sql_workspace.dart +++ b/lib/features/mysql/mysql_sql_workspace.dart @@ -538,7 +538,7 @@ class _MysqlSqlWorkspaceState extends material.State { await _refreshTxStatus(); if (!mounted) return; - final newRows = session.stagingBuffer!.effectiveRows; + final newRows = session.stagingBuffer!.committedRows; session.stagingBuffer?.dispose(); setState(() { session.rows = newRows; diff --git a/lib/features/mysql/mysql_table_view.dart b/lib/features/mysql/mysql_table_view.dart index e25c14f..6a152f9 100644 --- a/lib/features/mysql/mysql_table_view.dart +++ b/lib/features/mysql/mysql_table_view.dart @@ -575,7 +575,7 @@ class _MysqlTableViewState extends material.State { ); if (!mounted) return; if (outcome.isApplied) { - final newRows = buffer.effectiveRows; + final newRows = buffer.committedRows; buffer.dispose(); setState(() { _rows = newRows; diff --git a/lib/features/postgresql/postgres_sql_workspace.dart b/lib/features/postgresql/postgres_sql_workspace.dart index 503dcfb..090e51c 100644 --- a/lib/features/postgresql/postgres_sql_workspace.dart +++ b/lib/features/postgresql/postgres_sql_workspace.dart @@ -666,7 +666,7 @@ class _PostgresSqlWorkspaceState extends material.State { ); if (!mounted) return; - final newRows = session.stagingBuffer!.effectiveRows; + final newRows = session.stagingBuffer!.committedRows; session.stagingBuffer?.dispose(); setState(() { session.rows = newRows; diff --git a/lib/features/postgresql/postgres_table_view.dart b/lib/features/postgresql/postgres_table_view.dart index 3d4c7c0..8c4633d 100644 --- a/lib/features/postgresql/postgres_table_view.dart +++ b/lib/features/postgresql/postgres_table_view.dart @@ -596,7 +596,7 @@ class _PostgresTableViewState extends material.State { ); if (!mounted) return; if (outcome.isApplied) { - final newRows = buffer.effectiveRows; + final newRows = buffer.committedRows; buffer.dispose(); setState(() { _rows = newRows; diff --git a/lib/features/sqlite/sqlite_sql_workspace.dart b/lib/features/sqlite/sqlite_sql_workspace.dart index 5871249..22bfc07 100644 --- a/lib/features/sqlite/sqlite_sql_workspace.dart +++ b/lib/features/sqlite/sqlite_sql_workspace.dart @@ -504,7 +504,7 @@ class _SqliteSqlWorkspaceState extends material.State { await _refreshTxStatus(); if (!mounted) return; - final newRows = session.stagingBuffer!.effectiveRows; + final newRows = session.stagingBuffer!.committedRows; session.stagingBuffer?.dispose(); setState(() { session.rows = newRows; diff --git a/lib/features/sqlite/sqlite_table_view.dart b/lib/features/sqlite/sqlite_table_view.dart index 32c82b3..0cd9d0f 100644 --- a/lib/features/sqlite/sqlite_table_view.dart +++ b/lib/features/sqlite/sqlite_table_view.dart @@ -398,7 +398,7 @@ class _SqliteTableViewState extends material.State { ); if (!mounted) return; if (outcome.isApplied) { - final newRows = buffer.effectiveRows; + final newRows = buffer.committedRows; buffer.dispose(); setState(() { _rows = newRows; diff --git a/lib/features/workspace/data_grid_staging_buffer.dart b/lib/features/workspace/data_grid_staging_buffer.dart index f9cccb8..acaaab7 100644 --- a/lib/features/workspace/data_grid_staging_buffer.dart +++ b/lib/features/workspace/data_grid_staging_buffer.dart @@ -98,7 +98,8 @@ class DataGridStagingBuffer extends ChangeNotifier { return ''; } final insertIdx = row - _originalRows.length; - if (insertIdx < _insertedRows.length && col < _insertedRows[insertIdx].length) { + if (insertIdx < _insertedRows.length && + col < _insertedRows[insertIdx].length) { final ins = _insertedRows[insertIdx][col]; return ins == TableMutationEngine.kNullSentinel ? 'NULL' : ins; } @@ -118,7 +119,8 @@ class DataGridStagingBuffer extends ChangeNotifier { return false; } final insertIdx = row - _originalRows.length; - if (insertIdx < _insertedRows.length && col < _insertedRows[insertIdx].length) { + if (insertIdx < _insertedRows.length && + col < _insertedRows[insertIdx].length) { final ins = _insertedRows[insertIdx][col]; return ins == 'NULL' || ins == TableMutationEngine.kNullSentinel; } @@ -127,7 +129,10 @@ class DataGridStagingBuffer extends ChangeNotifier { /// Returns original baseline cell value, or null if row is inserted. String? getOriginalCellValue(int row, int col) { - if (row >= 0 && row < _originalRows.length && col >= 0 && col < _originalRows[row].length) { + if (row >= 0 && + row < _originalRows.length && + col >= 0 && + col < _originalRows[row].length) { return _originalRows[row][col]; } return null; @@ -169,7 +174,8 @@ class DataGridStagingBuffer extends ChangeNotifier { if (row < 0 || col < 0) return; if (row < _originalRows.length) { - final orig = col < _originalRows[row].length ? _originalRows[row][col] : ''; + final orig = + col < _originalRows[row].length ? _originalRows[row][col] : ''; if (value == orig) { if (_modifiedCells.containsKey(row)) { _modifiedCells[row]!.remove(col); @@ -281,7 +287,8 @@ class DataGridStagingBuffer extends ChangeNotifier { /// Read-only snapshot of modified cells mapping. Map> get modifiedCells => Map>.unmodifiable( - _modifiedCells.map((k, v) => MapEntry(k, Map.unmodifiable(v))), + _modifiedCells + .map((k, v) => MapEntry(k, Map.unmodifiable(v))), ); /// Read-only list of newly inserted rows. @@ -317,6 +324,10 @@ class DataGridStagingBuffer extends ChangeNotifier { /// Returns the full list of effective rows (original with modifications applied + inserted rows). /// + /// Row indices stay aligned with the grid (rows marked for deletion are still + /// present so they can render struck through); use [committedRows] for the + /// baseline after a successful save. Staged NULLs are shown as `'NULL'`. + /// /// Optimized for zero allocations when [isDirty] is false, and caches /// the effective row list to prevent breaking widget memoization. List> get effectiveRows { @@ -326,14 +337,27 @@ class DataGridStagingBuffer extends ChangeNotifier { if (_cachedEffectiveRows != null) { return _cachedEffectiveRows!; } + _cachedEffectiveRows = List.unmodifiable(_buildRows(includeDeleted: true)); + return _cachedEffectiveRows!; + } + + /// Rows as they exist in the database once the staged changes are applied: + /// deleted baseline rows are dropped and staged NULLs become `'NULL'`. + /// + /// Use this as the new baseline after a successful save. + List> get committedRows => + List.unmodifiable(_buildRows(includeDeleted: false)); + + List> _buildRows({required bool includeDeleted}) { final result = >[]; for (var r = 0; r < _originalRows.length; r++) { + if (!includeDeleted && _deletedRowIndices.contains(r)) continue; final mods = _modifiedCells[r]; if (mods != null) { final row = List.from(_originalRows[r]); for (final entry in mods.entries) { if (entry.key < row.length) { - row[entry.key] = entry.value; + row[entry.key] = _sanitizeNull(entry.value); } } result.add(row); @@ -342,12 +366,18 @@ class DataGridStagingBuffer extends ChangeNotifier { } } for (final ins in _insertedRows) { - result.add(ins); + result.add( + ins.any((v) => v == TableMutationEngine.kNullSentinel) + ? ins.map(_sanitizeNull).toList() + : ins, + ); } - _cachedEffectiveRows = List.unmodifiable(result); - return _cachedEffectiveRows!; + return result; } + static String _sanitizeNull(String value) => + value == TableMutationEngine.kNullSentinel ? 'NULL' : value; + @override void dispose() { UnsavedWorkRegistry.instance.unregister(this); @@ -358,4 +388,3 @@ class DataGridStagingBuffer extends ChangeNotifier { super.dispose(); } } - diff --git a/test/features/workspace/data_grid_staging_buffer_test.dart b/test/features/workspace/data_grid_staging_buffer_test.dart index 27ca7ad..b0a14dd 100644 --- a/test/features/workspace/data_grid_staging_buffer_test.dart +++ b/test/features/workspace/data_grid_staging_buffer_test.dart @@ -121,7 +121,45 @@ void main() { expect(eff[3], ['4', 'David', 'david@test.com']); }); - test('effectiveRows returns identical reference when clean and caches unmodifiable list when dirty', () { + test( + 'effectiveRows keeps deleted rows aligned and maps staged NULL to NULL', + () { + buffer.toggleDeleteRow(1); + buffer.setCellNull(0, 2); + buffer.addRow(['4', TableMutationEngine.kNullSentinel, 'd@test.com']); + + final eff = buffer.effectiveRows; + expect(eff.length, 4); + expect(eff[0], ['1', 'Alice', 'NULL']); + expect(eff[1], ['2', 'Bob', 'bob@test.com']); + expect(eff[3], ['4', 'NULL', 'd@test.com']); + expect( + eff.expand((r) => r).any((v) => v.contains('\u0000')), + isFalse, + ); + }); + + test('committedRows drops deleted rows and sanitizes NULL sentinels', () { + buffer.toggleDeleteRow(1); + buffer.setCell(1, 1, 'ignored'); // edit on a row that is also deleted + buffer.setCellNull(0, 2); + buffer.addRow(['4', TableMutationEngine.kNullSentinel, 'd@test.com']); + + final committed = buffer.committedRows; + expect(committed, [ + ['1', 'Alice', 'NULL'], + ['3', 'Charlie', 'charlie@test.com'], + ['4', 'NULL', 'd@test.com'], + ]); + }); + + test('committedRows equals baseline when clean', () { + expect(buffer.committedRows, buffer.originalRows); + }); + + test( + 'effectiveRows returns identical reference when clean and caches unmodifiable list when dirty', + () { // When clean, effectiveRows returns the exact baseline instance (zero allocation) final cleanRows1 = buffer.effectiveRows; final cleanRows2 = buffer.effectiveRows; @@ -160,7 +198,9 @@ void main() { expect(buffer.getCellValue(0, 1), 'NULL'); }); - test('generateMutationPlan builds correct DML statements from staged modifications', () { + test( + 'generateMutationPlan builds correct DML statements from staged modifications', + () { buffer.setCell(0, 1, 'Alice Updated'); buffer.addRow(['4', 'Diana', 'diana@test.com']); buffer.toggleDeleteRow(2); @@ -174,14 +214,19 @@ void main() { expect(plan.statementCount, 3); expect(plan.statements[0].type, MutationType.update); - expect(plan.statements[0].sql, 'UPDATE "public"."users" SET "name" = \'Alice Updated\' WHERE "id" = 1'); + expect(plan.statements[0].sql, + 'UPDATE "public"."users" SET "name" = \'Alice Updated\' WHERE "id" = 1'); expect(plan.statements[1].type, MutationType.insert); - expect(plan.statements[1].sql, 'INSERT INTO "public"."users" ("id", "name", "email") VALUES (4, \'Diana\', \'diana@test.com\')'); + expect(plan.statements[1].sql, + 'INSERT INTO "public"."users" ("id", "name", "email") VALUES (4, \'Diana\', \'diana@test.com\')'); expect(plan.statements[2].type, MutationType.delete); - expect(plan.statements[2].sql, 'DELETE FROM "public"."users" WHERE "id" = 3'); + expect(plan.statements[2].sql, + 'DELETE FROM "public"."users" WHERE "id" = 3'); }); - test('dispose clears internal collections and prevents further listener notifications', () { + test( + 'dispose clears internal collections and prevents further listener notifications', + () { buffer.setCell(0, 1, 'Alice Modified'); buffer.addRow(['4', 'Diana', 'diana@test.com']); buffer.toggleDeleteRow(2); diff --git a/test/features/workspace/table_view_staging_test.dart b/test/features/workspace/table_view_staging_test.dart index 900ed81..20b6881 100644 --- a/test/features/workspace/table_view_staging_test.dart +++ b/test/features/workspace/table_view_staging_test.dart @@ -347,6 +347,46 @@ void main() { expect(executed, isFalse); }); + testWidgets( + 'applied save yields a baseline without deleted rows or NULL sentinels', + (tester) async { + await tester.pumpWidget( + ShadcnApp( + theme: AppTheme.dark, + home: const material.Scaffold(body: material.SizedBox()), + ), + ); + final ctx = tester.element(find.byType(material.Scaffold)); + final buffer = DataGridStagingBuffer( + columns: ['id', 'name'], + rows: [ + ['1', 'Ada'], + ['2', 'Grace'], + ], + ); + buffer.toggleDeleteRow(1); + buffer.setCellNull(0, 1); + addTearDown(buffer.dispose); + + final future = applyTableViewStagedChanges( + context: ctx, + buffer: buffer, + dialect: SqlDialect.postgres, + tableName: 'users', + schema: 'public', + primaryKeys: ['id'], + execute: (_) async {}, + ); + await tester.pumpAndSettle(); + await tester.tap(find.text('Apply Changes')); + final outcome = await future; + + expect(outcome.isApplied, isTrue); + expect(buffer.committedRows, [ + ['1', 'NULL'], + ]); + }); + testWidgets('0-row DML is failed and the staging buffer stays dirty', (tester) async { await tester.pumpWidget(