fix(datagrid): keep a column order whose columns have not changed (#2461)

* fix(datagrid): keep a column order whose columns have not changed

* fix(datagrid): drop a saved column order when two columns share a name

---------

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>

Author: datlechin

CHANGELOG.md

@@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Connections strip not scrolling to the entry you switch to.
 - Grid cells left at the old column positions until the next click, after a resize, an auto-fit, a reorder, hiding a column, or a row-number width change. (#2449)
 - The row-number column draggable out of first place, which walked it to the far right on the next refresh.
+- A reordered column snapping back on the next refresh in the Structure tab and in query results.
 - Double-clicking a cell editing the wrong row, after deleting one of several rows added in the same session.
 - VoiceOver reading a cell's value under a different row's number after such a delete.
 

TablePro/Views/Results/DataGridCoordinator.swift

@@ -262,11 +262,24 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
         tabType = configuration.tabType
     }
 
+    /// A grid with no table behind it keeps a saved column order only while its columns are still the
+    /// ones that order was saved for.
+    ///
+    /// A query result's columns are authored by the SELECT list and their order is meaningful, so an
+    /// order saved for a different set must not be replayed over it: `computeTargetOrder` appends
+    /// every column the saved order does not name, which landed a newly written column at the far
+    /// right instead of where it was typed (#1565). Dropping the order outright answered that and
+    /// also threw away orders the columns had never moved under, so a reorder was captured, persisted
+    /// and then silently undone on the next update.
     func savedColumnLayout(binding: ColumnLayoutState) -> ColumnLayoutState? {
         guard tabType == .table else {
-            guard !binding.columnWidths.isEmpty || binding.columnContentWidths?.isEmpty == false else { return nil }
             var layout = binding
-            layout.columnOrder = nil
+            if let order = layout.columnOrder, !canRestoreColumnOrder(order) {
+                layout.columnOrder = nil
+            }
+            guard !layout.columnWidths.isEmpty
+                || layout.columnContentWidths?.isEmpty == false
+                || layout.columnOrder?.isEmpty == false else { return nil }
             return layout
         }
 
@@ -281,6 +294,15 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
         return binding
     }
 
+    /// A saved order is a list of names, and a name identifies a column only while the names are
+    /// unique. `SELECT a.id, b.id` gives two columns called `id`, and `ColumnIdentitySchema` resolves
+    /// a duplicate name to its last slot, so replaying such an order swaps the two columns.
+    private func canRestoreColumnOrder(_ order: [String]) -> Bool {
+        let columns = identitySchema.columnNames
+        let names = Set(columns)
+        return names.count == columns.count && Set(order) == names
+    }
+
     /// A saved width is the width the column had, accessory or not. Nothing here re-derives it from
     /// the accessory, because the accessory no longer contributes to column width: a layout saved
     /// while the arrow was showing and restored before it is known has to come back the same size.

TableProTests/Views/Results/TableViewCoordinatorLayoutTests.swift

@@ -128,6 +128,12 @@ struct TableViewCoordinatorLayoutTests {
             tableName: nil,
             persister: FakeColumnLayoutPersister()
         )
+        let rows = TableRows.from(
+            queryRows: [[.text("1"), .text("direct"), .text("EU")]],
+            columns: ["id", "business_model", "region"],
+            columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: 3)
+        )
+        coordinator.rebuildColumnMetadataCache(from: rows)
         var binding = ColumnLayoutState()
         binding.columnWidths = ["id": 60, "business_model": 120]
         binding.columnOrder = ["id", "business_model"]
@@ -138,6 +144,79 @@ struct TableViewCoordinatorLayoutTests {
         #expect(coordinator.savedColumnLayout(binding: binding) == expected)
     }
 
+    /// The order is dropped because the columns moved under it, not because the grid has no table.
+    /// A re-run of the same query, and every update of the Structure grid, arrives with the same
+    /// column set, and the reorder the user made has to survive it.
+    @Test("Query tab keeps a column order saved for the same columns")
+    func queryTabKeepsMatchingColumnOrder() {
+        let coordinator = makeCoordinator(
+            tabType: .query,
+            connectionId: nil,
+            tableName: nil,
+            persister: FakeColumnLayoutPersister()
+        )
+        let rows = TableRows.from(
+            queryRows: [[.text("1"), .text("direct")]],
+            columns: ["id", "business_model"],
+            columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: 2)
+        )
+        coordinator.rebuildColumnMetadataCache(from: rows)
+        var binding = ColumnLayoutState()
+        binding.columnWidths = ["id": 60, "business_model": 120]
+        binding.columnOrder = ["business_model", "id"]
+
+        #expect(coordinator.savedColumnLayout(binding: binding) == binding)
+    }
+
+    /// A saved order names its columns, and `SELECT a.id, b.id` gives two of them the same name.
+    /// `ColumnIdentitySchema` resolves a duplicate to its last slot, so replaying such an order
+    /// silently swaps the pair.
+    @Test("Query tab drops a column order when two columns share a name")
+    func queryTabDropsColumnOrderForDuplicateNames() {
+        let coordinator = makeCoordinator(
+            tabType: .query,
+            connectionId: nil,
+            tableName: nil,
+            persister: FakeColumnLayoutPersister()
+        )
+        let rows = TableRows.from(
+            queryRows: [[.text("1"), .text("2")]],
+            columns: ["id", "id"],
+            columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: 2)
+        )
+        coordinator.rebuildColumnMetadataCache(from: rows)
+        var binding = ColumnLayoutState()
+        binding.columnWidths = ["id": 60]
+        binding.columnOrder = ["id", "id"]
+
+        var expected = ColumnLayoutState()
+        expected.columnWidths = ["id": 60]
+
+        #expect(coordinator.savedColumnLayout(binding: binding) == expected)
+    }
+
+    /// A reorder with no width change is the whole layout, and it used to fall through the
+    /// emptiness guard and come back as nil.
+    @Test("Query tab keeps an order-only layout")
+    func queryTabKeepsAnOrderOnlyLayout() {
+        let coordinator = makeCoordinator(
+            tabType: .query,
+            connectionId: nil,
+            tableName: nil,
+            persister: FakeColumnLayoutPersister()
+        )
+        let rows = TableRows.from(
+            queryRows: [[.text("1"), .text("direct")]],
+            columns: ["id", "business_model"],
+            columnTypes: Array(repeating: ColumnType.text(rawType: "TEXT"), count: 2)
+        )
+        coordinator.rebuildColumnMetadataCache(from: rows)
+        var binding = ColumnLayoutState()
+        binding.columnOrder = ["business_model", "id"]
+
+        #expect(coordinator.savedColumnLayout(binding: binding) == binding)
+    }
+
     @Test("Query tab keeps remembered widths when there is no saved order")
     func queryTabKeepsWidths() {
         let coordinator = makeCoordinator(