From 1ea93894e3ebce3c0f47ee1c04d0d4cf95a0c56f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 9 Jul 2026 13:01:08 +0700 Subject: [PATCH] fix(datagrid): reload the structure grid on filter and sort instead of showing stale rows --- CHANGELOG.md | 4 + .../Results/DataGridUpdateSnapshot.swift | 1 + TablePro/Views/Results/DataGridView.swift | 3 + .../Views/Structure/TableStructureView.swift | 3 +- .../Results/DataGridUpdateSnapshotTests.swift | 58 ++++++++++ ...ableViewCoordinatorDisplayCacheTests.swift | 63 +++++++++++ .../Structure/StructureRowProviderTests.swift | 106 ++++++++++++++++++ 7 files changed, 237 insertions(+), 1 deletion(-) create mode 100644 TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift create mode 100644 TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift create mode 100644 TableProTests/Views/Structure/StructureRowProviderTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 8de480233..9f3b9faa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- The Structure tab filter and column sort now update the grid instead of leaving stale rows on screen. + ## [0.56.0] - 2026-07-09 ### Added diff --git a/TablePro/Views/Results/DataGridUpdateSnapshot.swift b/TablePro/Views/Results/DataGridUpdateSnapshot.swift index 133ffd02d..41e496328 100644 --- a/TablePro/Views/Results/DataGridUpdateSnapshot.swift +++ b/TablePro/Views/Results/DataGridUpdateSnapshot.swift @@ -20,5 +20,6 @@ struct DataGridUpdateSnapshot: Equatable { let rowHeight: CGFloat let alternatingRows: Bool let reloadVersion: Int + let contentRevision: Int let showObjectComments: Bool } diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 86838c65f..abe41984b 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -42,6 +42,7 @@ struct DataGridView: NSViewRepresentable { @Binding var selectedRowIndices: Set @Binding var sortState: SortState @Binding var columnLayout: ColumnLayoutState + var contentRevision: Int = 0 // MARK: - NSViewRepresentable @@ -169,11 +170,13 @@ struct DataGridView: NSViewRepresentable { rowHeight: rowHeight, alternatingRows: alternatingRows, reloadVersion: changeManager.reloadVersion, + contentRevision: contentRevision, showObjectComments: AppSettingsManager.shared.general.showObjectComments ) if snapshot != coordinator.lastUpdateSnapshot { let contentChanged = snapshot.reloadVersion != coordinator.lastUpdateSnapshot?.reloadVersion + || snapshot.contentRevision != coordinator.lastUpdateSnapshot?.contentRevision applyStructuralUpdate( tableView: tableView, coordinator: coordinator, diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index 39d31f913..6ede82bfc 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -424,7 +424,8 @@ struct TableStructureView: View { delegate: gridDelegate, selectedRowIndices: $selectedRows, sortState: $sortState, - columnLayout: columnLayoutBinding(for: selectedTab) + columnLayout: columnLayoutBinding(for: selectedTab), + contentRevision: displayVersion ) .safeAreaInset(edge: .top, spacing: 0) { VStack(spacing: 0) { diff --git a/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift b/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift new file mode 100644 index 000000000..0bc7a4678 --- /dev/null +++ b/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift @@ -0,0 +1,58 @@ +// +// DataGridUpdateSnapshotTests.swift +// TableProTests +// + +import AppKit +import Testing + +@testable import TablePro + +@Suite("DataGridUpdateSnapshot reload gate") +struct DataGridUpdateSnapshotTests { + private func makeSnapshot( + rowDisplayCount: Int = 3, + columns: [String] = ["name", "type"], + reloadVersion: Int = 0, + contentRevision: Int = 0 + ) -> DataGridUpdateSnapshot { + DataGridUpdateSnapshot( + rowDisplayCount: rowDisplayCount, + columnCount: columns.count, + columns: columns, + sortedIDsCount: nil, + valueFilteredIDsCount: nil, + displayFormats: [], + configuration: DataGridConfiguration(), + isEditable: true, + hasMoveDelegate: false, + rowHeight: 24, + alternatingRows: true, + reloadVersion: reloadVersion, + contentRevision: contentRevision, + showObjectComments: false + ) + } + + @Test("A filter/sort change at the same row count still changes the snapshot") + func contentRevisionBumpChangesSnapshot() { + let before = makeSnapshot(rowDisplayCount: 3, contentRevision: 0) + let after = makeSnapshot(rowDisplayCount: 3, contentRevision: 1) + #expect(before != after) + } + + @Test("Nothing changing leaves the snapshot equal so the grid does not reload") + func identicalSnapshotsAreEqual() { + let first = makeSnapshot() + let second = makeSnapshot() + #expect(first == second) + } + + @Test("contentRevision is independent of reloadVersion and row count") + func contentRevisionIsIndependentSignal() { + let base = makeSnapshot(rowDisplayCount: 5, reloadVersion: 2, contentRevision: 4) + #expect(base != makeSnapshot(rowDisplayCount: 5, reloadVersion: 2, contentRevision: 5)) + #expect(base != makeSnapshot(rowDisplayCount: 6, reloadVersion: 2, contentRevision: 4)) + #expect(base != makeSnapshot(rowDisplayCount: 5, reloadVersion: 3, contentRevision: 4)) + } +} diff --git a/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift b/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift new file mode 100644 index 000000000..9733f69a4 --- /dev/null +++ b/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift @@ -0,0 +1,63 @@ +// +// TableViewCoordinatorDisplayCacheTests.swift +// TableProTests +// + +import AppKit +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@Suite("TableViewCoordinator display cache invalidation") +@MainActor +struct TableViewCoordinatorDisplayCacheTests { + private func makeCoordinator() -> TableViewCoordinator { + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: FakeDisplayCachePersister() + ) + var captured = TableRows( + rows: [Row(id: .existing(0), values: [.text("A")])], + columns: ["name"], + columnTypes: [.text(rawType: nil)] + ) + coordinator.tableRowsProvider = { captured } + coordinator.tableRowsMutator = { mutation in mutation(&captured) } + coordinator.updateCache() + return coordinator + } + + private func value(_ text: String) -> PluginCellValue { .text(text) } + + @Test("Cache returns the stale value for a reused RowID until it is invalidated") + func invalidationClearsStaleContent() { + let coordinator = makeCoordinator() + let column = 0 + let type: ColumnType = .text(rawType: nil) + + let primed = coordinator.displayValue(forID: .existing(0), column: column, rawValue: value("A"), columnType: type) + #expect(primed == "A") + + let stale = coordinator.displayValue(forID: .existing(0), column: column, rawValue: value("B"), columnType: type) + #expect(stale == "A") + + coordinator.invalidateDisplayCache() + + let fresh = coordinator.displayValue(forID: .existing(0), column: column, rawValue: value("B"), columnType: type) + #expect(fresh == "B") + } +} + +@MainActor +private final class FakeDisplayCachePersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + + func clear(for key: ColumnLayoutTableKey) {} +} diff --git a/TableProTests/Views/Structure/StructureRowProviderTests.swift b/TableProTests/Views/Structure/StructureRowProviderTests.swift new file mode 100644 index 000000000..da33e8576 --- /dev/null +++ b/TableProTests/Views/Structure/StructureRowProviderTests.swift @@ -0,0 +1,106 @@ +// +// StructureRowProviderTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@MainActor @Suite("StructureRowProvider filter and sort") +struct StructureRowProviderTests { + private func makeColumn(_ name: String) -> EditableColumnDefinition { + EditableColumnDefinition( + id: UUID(), + name: name, + dataType: "text", + isNullable: true, + defaultValue: nil, + autoIncrement: false, + unsigned: false, + comment: nil, + collation: nil, + onUpdate: nil, + charset: nil, + extra: nil, + isPrimaryKey: false + ) + } + + private func makeManager(columnNames: [String]) -> StructureChangeManager { + let manager = StructureChangeManager() + manager.workingColumns = columnNames.map(makeColumn) + return manager + } + + private func makeProvider( + columnNames: [String], + filterText: String? = nil, + sortDescriptor: StructureSortDescriptor? = nil + ) -> StructureRowProvider { + StructureRowProvider( + changeManager: makeManager(columnNames: columnNames), + tab: .columns, + databaseType: .mysql, + additionalFields: [.name, .type], + filterText: filterText, + sortDescriptor: sortDescriptor + ) + } + + private func names(_ provider: StructureRowProvider) -> [String] { + provider.rows.compactMap { $0.first ?? nil } + } + + @Test("No filter shows every column with an identity source map") + func noFilterShowsAll() { + let provider = makeProvider(columnNames: ["id", "username", "email", "created"]) + #expect(provider.totalRowCount == 4) + #expect(names(provider) == ["id", "username", "email", "created"]) + #expect(provider.filteredToSourceMap == [0, 1, 2, 3]) + } + + @Test("Filter matches the column name as a substring and maps back to the source index") + func filterMatchesNameSubstring() { + let provider = makeProvider(columnNames: ["id", "username", "email", "created"], filterText: "user") + #expect(provider.totalRowCount == 1) + #expect(names(provider) == ["username"]) + #expect(provider.filteredToSourceMap == [1]) + } + + @Test("Filter is case-insensitive") + func filterIsCaseInsensitive() { + let provider = makeProvider(columnNames: ["id", "username", "email", "created"], filterText: "EMAIL") + #expect(names(provider) == ["email"]) + #expect(provider.filteredToSourceMap == [2]) + } + + @Test("A filter that matches nothing yields an empty display set") + func filterMatchesNothing() { + let provider = makeProvider(columnNames: ["id", "username", "email", "created"], filterText: "zzz") + #expect(provider.totalRowCount == 0) + #expect(provider.filteredToSourceMap.isEmpty) + } + + @Test("Sorting reorders rows and keeps the source map aligned to display order") + func sortReordersAndKeepsSourceMap() { + let provider = makeProvider( + columnNames: ["id", "username", "email", "created"], + sortDescriptor: StructureSortDescriptor(column: 0, ascending: true) + ) + #expect(names(provider) == ["created", "email", "id", "username"]) + #expect(provider.filteredToSourceMap == [3, 2, 0, 1]) + } + + @Test("Filter and sort compose: filter first, then order the survivors") + func filterThenSort() { + let provider = makeProvider( + columnNames: ["alpha", "beta", "gamma", "alpine"], + filterText: "al", + sortDescriptor: StructureSortDescriptor(column: 0, ascending: true) + ) + #expect(names(provider) == ["alpha", "alpine"]) + #expect(provider.filteredToSourceMap == [0, 3]) + } +}