fix(datagrid): restore each row's own value when a row inspector field they disagree on is cleared

CHANGELOG.md

@@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Stutter when scrolling a very long line in the SQL editor.
 - Row inspector edits missing from the grid cell and gone from the inspector on reselect. (#2851)
+- NULL written into every selected row when a field they disagree on was cleared in the row inspector.
+- Row inspector showing the discarded values after Discard Changes.
 - Empty Procedures and Functions lists on every SQL Server connection.
 - SQL Server rows that could not be saved on a table with a filtered index or an index on a computed column.
 - SQL Server CLR and extended procedures and functions missing from the Procedures and Functions lists.

TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift

@@ -80,17 +80,21 @@ extension RowEditingCoordinator {
            let tableRows = parent.tabSessionRegistry.existingTableRows(for: tab.id) {
             let tabId = tab.id
             let insertedIDs = parent.changeManager.insertedRowIDs
+            var restoredCells: [(rowID: RowID, columnIndex: Int)] = []
             let edits = parent.changeManager.getOriginalValues().compactMap { original in
-                tableRows.index(of: original.rowID).map {
-                    (row: $0, column: original.columnIndex, value: original.value)
+                tableRows.index(of: original.rowID).map { storageRow -> (row: Int, column: Int, value: PluginCellValue) in
+                    restoredCells.append((rowID: original.rowID, columnIndex: original.columnIndex))
+                    return (row: storageRow, column: original.columnIndex, value: original.value)
                 }
             }
             if !edits.isEmpty {
                 let editDelta = parent.mutateActiveTableRows(for: tabId) { rows in
                     rows.editMany(edits)
                 }
+                /// `editMany` names the rows it changed by their position in storage, and the grid
+                /// reads a delta's rows as display positions.
                 if editDelta != .none {
-                    deltas.append(editDelta)
+                    deltas.append(restoredCellsDelta(restoredCells, in: tableRows))
                 }
             }
             if !insertedIDs.isEmpty {
@@ -106,5 +110,25 @@ extension RowEditingCoordinator {
         for delta in deltas {
             parent.dataTabDelegate?.tableViewCoordinator?.applyDelta(delta)
         }
+        /// The row inspector reads its fields from the buffer this just rewrote, and nothing else
+        /// in `InspectorTrigger` moves on a discard.
+        parent.gridDisplayRevision &+= 1
+    }
+
+    private func restoredCellsDelta(
+        _ cells: [(rowID: RowID, columnIndex: Int)],
+        in tableRows: TableRows
+    ) -> Delta {
+        let displayIDs = parent.activeGridDisplayIDs
+        var positions: Set<CellPosition> = []
+        for cell in cells {
+            guard let displayRow = DisplayRowMapping.displayIndex(
+                forRowID: cell.rowID,
+                displayIDs: displayIDs,
+                in: tableRows
+            ) else { continue }
+            positions.insert(CellPosition(row: displayRow, column: cell.columnIndex))
+        }
+        return positions.isEmpty ? .none : .cellsChanged(positions)
     }
 }

TablePro/Core/Coordinators/RowEditingCoordinator.swift

@@ -173,7 +173,22 @@ final class RowEditingCoordinator {
     /// and each contributes its own `oldValue`. A shared one is wrong the moment a multi-row
     /// selection disagrees on the field, where the inspector holds no value for it at all.
     func stageInspectorFieldEdit(columnIndex: Int, value: PluginCellValue, rowIDs: [RowID]) {
-        guard let (tab, tabIndex) = parent.tabManager.selectedTabAndIndex, !rowIDs.isEmpty else { return }
+        stageInspectorEdits(
+            valuesByRow: Dictionary(rowIDs.map { ($0, value) }, uniquingKeysWith: { first, _ in first }),
+            columnIndex: columnIndex
+        )
+    }
+
+    /// Puts a field back to the values the inspector was configured with.
+    ///
+    /// A field the selected rows disagree on has no value of its own, and clearing it asks for each
+    /// row's own value back rather than for one value across all of them.
+    func revertInspectorFieldEdit(columnIndex: Int, valuesByRow: [RowID: PluginCellValue]) {
+        stageInspectorEdits(valuesByRow: valuesByRow, columnIndex: columnIndex)
+    }
+
+    private func stageInspectorEdits(valuesByRow: [RowID: PluginCellValue], columnIndex: Int) {
+        guard let (tab, tabIndex) = parent.tabManager.selectedTabAndIndex, !valuesByRow.isEmpty else { return }
         let tabId = tab.id
         let tableRows = parent.tabSessionRegistry.tableRows(for: tabId)
         guard tableRows.columns.indices.contains(columnIndex) else { return }
@@ -185,7 +200,7 @@ final class RowEditingCoordinator {
 
         var edits: [(row: Int, column: Int, value: PluginCellValue)] = []
         var editedRowIDs: Set<RowID> = []
-        for rowID in rowIDs {
+        for (rowID, value) in valuesByRow {
             guard let storageRow = tableRows.index(of: rowID) else { continue }
             let values = Array(tableRows.rows[storageRow].values)
             guard values.indices.contains(columnIndex), values[columnIndex] != value else { continue }

TablePro/Models/UI/MultiRowEditState.swift

@@ -75,6 +75,10 @@ final class MultiRowEditState {
 
     var onFieldChanged: ((Int, PluginCellValue) -> Void)?
 
+    /// A field the selected rows disagree on, cleared back to nothing. It has no single value to
+    /// send, so it asks for each row's own configured value instead.
+    var onFieldReverted: ((Int, [RowID: PluginCellValue]) -> Void)?
+
     private(set) var selectedRowIndices: Set<Int> = []
 
     /// The rows an edit is staged against, captured when the selection was configured.
@@ -247,9 +251,29 @@ final class MultiRowEditState {
         fields[index].pendingValue = pending
         fields[index].isPendingNull = false
         fields[index].isPendingDefault = false
-        if pending != nil || hadPendingEdit {
-            onFieldChanged?(index, PluginCellValue.fromOptional(pending ?? original))
+        if pending != nil {
+            onFieldChanged?(index, PluginCellValue.fromOptional(pending))
+        } else if hadPendingEdit {
+            /// `originalValue` is nil for two different situations, and only one of them is a
+            /// value: a stored NULL, and a selection whose rows do not agree. Sending it as one
+            /// value wrote NULL into every selected row when the user cleared a field they all
+            /// disagreed on, which the field then reported as unedited.
+            if fields[index].hasMultipleValues {
+                onFieldReverted?(index, configuredValues(atColumn: index))
+            } else {
+                onFieldChanged?(index, PluginCellValue.fromOptional(original))
+            }
+        }
+    }
+
+    /// What each row held when the selection was configured, which is what a field with no value of
+    /// its own reverts to.
+    private func configuredValues(atColumn index: Int) -> [RowID: PluginCellValue] {
+        var values: [RowID: PluginCellValue] = [:]
+        for (rowID, row) in zip(rowIDs, allRows) where row.indices.contains(index) {
+            values[rowID] = PluginCellValue.fromOptional(row[index])
         }
+        return values
     }
 
     private static func resolvePendingValue(_ value: String?, original: String?, isJson: Bool) -> String? {
@@ -327,6 +351,7 @@ final class MultiRowEditState {
     func releaseData() {
         fields = []
         onFieldChanged = nil
+        onFieldReverted = nil
         selectedRowIndices = []
         rowIDs = []
         allRows = []

TablePro/Views/Main/Extensions/MainContentCoordinator+RowOperations.swift

@@ -50,6 +50,10 @@ extension MainContentCoordinator {
         )
     }
 
+    func revertInspectorFieldEdit(columnIndex: Int, valuesByRow: [RowID: PluginCellValue]) {
+        rowEditingCoordinator.revertInspectorFieldEdit(columnIndex: columnIndex, valuesByRow: valuesByRow)
+    }
+
     func copySelectedRowsToClipboard(indices: Set<Int>) {
         rowEditingCoordinator.copySelectedRowsToClipboard(indices: indices)
     }

TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift

@@ -221,7 +221,7 @@ extension MainContentView {
         )
 
         guard isSidebarEditable else {
-            trailingPaneState.inspector.editState.onFieldChanged = nil
+            clearSidebarEditHandlers()
             return
         }
 
@@ -234,6 +234,9 @@ extension MainContentView {
                 rowIDs: capturedEditState.rowIDs
             )
         }
+        trailingPaneState.inspector.editState.onFieldReverted = { columnIndex, valuesByRow in
+            capturedCoordinator.revertInspectorFieldEdit(columnIndex: columnIndex, valuesByRow: valuesByRow)
+        }
     }
 
     /// The per-column display formats the grid is applying, in column order.
@@ -276,7 +279,12 @@ extension MainContentView {
 
     private func clearSidebarEditState() {
         trailingPaneState.inspector.editState.fields = []
+        clearSidebarEditHandlers()
+    }
+
+    private func clearSidebarEditHandlers() {
         trailingPaneState.inspector.editState.onFieldChanged = nil
+        trailingPaneState.inspector.editState.onFieldReverted = nil
     }
 
     /// Populate the inspector from the grid that owns a schema selection, and send every
@@ -293,11 +301,12 @@ extension MainContentView {
         trailingPaneState.inspector.editState.configure(schemaFields: row.fields, displayRow: displayRow)
 
         guard row.isEditable else {
-            trailingPaneState.inspector.editState.onFieldChanged = nil
+            clearSidebarEditHandlers()
             return
         }
 
         let capturedCoordinator = coordinator
+        trailingPaneState.inspector.editState.onFieldReverted = nil
         trailingPaneState.inspector.editState.onFieldChanged = { fieldIndex, newValue in
             capturedCoordinator.inspectorRowSource?.commitInspectorField(
                 displayRow: displayRow,

TableProTests/Views/Main/InspectorFieldEditStagingTests.swift

@@ -28,6 +28,27 @@ struct InspectorFieldEditStagingTests {
         func stage(_ value: PluginCellValue, column: Int = 1, rows: [RowID] = [.existing(1)]) {
             coordinator.stageInspectorFieldEdit(columnIndex: column, value: value, rowIDs: rows)
         }
+
+        /// A real `MultiRowEditState` wired the way the inspector wires it, so a field edit takes
+        /// the same route from the editor's binding to the staged change.
+        func configuredEditState(rows: [RowID]) -> MultiRowEditState {
+            let state = MultiRowEditState()
+            let selected = rows.compactMap { tableRows.row(withID: $0) }
+            state.configure(
+                selectedRowIndices: Set(selected.indices),
+                rowIDs: selected.map(\.id),
+                allRows: selected.map { $0.values.map(\.asText) },
+                columns: tableRows.columns,
+                columnTypes: [.text(rawType: nil), .text(rawType: nil)]
+            )
+            state.onFieldChanged = { [coordinator] columnIndex, value in
+                coordinator.stageInspectorFieldEdit(columnIndex: columnIndex, value: value, rowIDs: rows)
+            }
+            state.onFieldReverted = { [coordinator] columnIndex, valuesByRow in
+                coordinator.revertInspectorFieldEdit(columnIndex: columnIndex, valuesByRow: valuesByRow)
+            }
+            return state
+        }
     }
 
     private func makeFixture(generatedColumns: Set<String> = []) -> Fixture {
@@ -143,6 +164,45 @@ struct InspectorFieldEditStagingTests {
         #expect(!fixture.coordinator.changeManager.hasChanges)
     }
 
+    /// `originalValue` is nil both for a stored NULL and for a selection whose rows disagree, and
+    /// sending that nil as one value wrote NULL into every selected row.
+    @Test("clearing a field the selected rows disagree on puts each row's own value back")
+    func clearingAMultiValueFieldRestoresEachRow() {
+        let fixture = makeFixture()
+        let editState = fixture.configuredEditState(rows: [.existing(0), .existing(1)])
+
+        editState.updateField(at: 1, value: "Same")
+        editState.updateField(at: 1, value: "")
+
+        #expect(fixture.value(row: 0, column: 1) == .text("Alice"))
+        #expect(fixture.value(row: 1, column: 1) == .text("Bob"))
+        #expect(!fixture.coordinator.changeManager.hasChanges)
+    }
+
+    @Test("clearing a field the rows agree on still sends the stored value")
+    func clearingASingleValueFieldSendsTheStoredValue() {
+        let fixture = makeFixture()
+        let editState = fixture.configuredEditState(rows: [.existing(1)])
+
+        editState.updateField(at: 1, value: "Zed")
+        editState.updateField(at: 1, value: "Bob")
+
+        #expect(fixture.value(row: 1, column: 1) == .text("Bob"))
+        #expect(!fixture.coordinator.changeManager.hasChanges)
+    }
+
+    @Test("an explicit NULL over rows that disagree still stages")
+    func anExplicitNullStillStages() {
+        let fixture = makeFixture()
+        let editState = fixture.configuredEditState(rows: [.existing(0), .existing(1)])
+
+        editState.setFieldToNull(at: 1)
+
+        #expect(fixture.value(row: 0, column: 1) == .null)
+        #expect(fixture.value(row: 1, column: 1) == .null)
+        #expect(fixture.coordinator.changeManager.hasChanges)
+    }
+
     @Test("a row the buffer does not hold is skipped rather than staged")
     func anUnknownRowIsSkipped() {
         let fixture = makeFixture()