refactor(coordinator): extract paginateIfPossible helper, add selectedTabAndIndex (#939)

* refactor(coordinator): extract paginateIfPossible helper, add selectedTabAndIndex

Pagination had seven methods (goToNextPage, goToPreviousPage, goToFirstPage, goToLastPage, updatePageSize, updateOffset, applyPaginationSettings) where six followed the same shape:

  guard let tabIndex = tabManager.selectedTabIndex,
        tabIndex < tabManager.tabs.count,
        tabManager.tabs[tabIndex].pagination.<condition> else { return }
  paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
      pagination.<action>()
  }

Six guard prologues and six tail calls collapse into one-line bodies via:

- New QueryTabManager.selectedTabAndIndex helper that returns (tab, index) atomically with the bounds check, replacing the two-step (selectedTabIndex + bounds-check + tabs[index]) pattern.
- New private paginateIfPossible(where:mutate:) that captures the selected-tab + condition + paginateAfterConfirmation chain. Each public pagination method becomes a one-liner using a KeyPath or short closure for the precondition.

Behavior unchanged. -30 LOC in Pagination, +5 in QueryTabManager. Smoke-tested all six pagination actions plus offset Go.

selectedTabAndIndex is also a foothold for further coordinator dedup — there are ~14 other selectedTabIndex + bounds-check sites in MainContentCoordinator extensions that could migrate, but each has slightly different surrounding logic so they're left for separate focused PRs.

* test(coordinator): cover QueryTabManager.selectedTabAndIndex contract

Four cases:
- nilWhenNoSelection: empty manager returns nil
- returnsSelectedTabAfterAdd: addTableTab autoselects the new tab; helper returns it with index 0
- nilWhenSelectionIsStale: if selectedTabId points to a removed tab, the bounds check kicks in and returns nil rather than crashing or returning stale data
- returnsCorrectPairAfterSwitch: explicit selectedTabId assignment resolves to the matching (tab, index) pair

The staleness test is the load-bearing one — it locks the contract that future migrations of the other ~14 selectedTabIndex + bounds-check sites can rely on. Without this, a refactor that loosens the bounds check could silently regress.

Author: datlechin

TablePro/Models/Query/QueryTabManager.swift

@@ -45,6 +45,11 @@ final class QueryTabManager {
         return _tabIndexMap[id]
     }
 
+    var selectedTabAndIndex: (tab: QueryTab, index: Int)? {
+        guard let index = selectedTabIndex, index < tabs.count else { return nil }
+        return (tabs[index], index)
+    }
+
     init() {
         tabs = []
         selectedTabId = nil

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

@@ -10,82 +10,47 @@ import Foundation
 extension MainContentCoordinator {
     // MARK: - Pagination
 
-    /// Navigate to next page
     func goToNextPage() {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count,
-              tabManager.tabs[tabIndex].pagination.hasNextPage else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.goToNextPage()
-        }
+        paginateIfPossible(where: \.hasNextPage) { $0.goToNextPage() }
     }
 
-    /// Navigate to previous page
     func goToPreviousPage() {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count,
-              tabManager.tabs[tabIndex].pagination.hasPreviousPage else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.goToPreviousPage()
-        }
+        paginateIfPossible(where: \.hasPreviousPage) { $0.goToPreviousPage() }
     }
 
-    /// Navigate to first page
     func goToFirstPage() {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count,
-              tabManager.tabs[tabIndex].pagination.hasPreviousPage else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.goToFirstPage()
-        }
+        paginateIfPossible(where: \.hasPreviousPage) { $0.goToFirstPage() }
     }
 
-    /// Navigate to last page
     func goToLastPage() {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count else { return }
-
-        let tab = tabManager.tabs[tabIndex]
-        guard tab.pagination.currentPage != tab.pagination.totalPages else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.goToLastPage()
-        }
+        paginateIfPossible(where: { $0.currentPage != $0.totalPages }) { $0.goToLastPage() }
     }
 
-    /// Update page size (limit) and reload
     func updatePageSize(_ newSize: Int) {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count,
-              newSize > 0 else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.updatePageSize(newSize)
-        }
+        guard newSize > 0 else { return }
+        paginateIfPossible { $0.updatePageSize(newSize) }
     }
 
-    /// Update offset and reload
     func updateOffset(_ newOffset: Int) {
-        guard let tabIndex = tabManager.selectedTabIndex,
-              tabIndex < tabManager.tabs.count,
-              newOffset >= 0 else { return }
-
-        paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
-            pagination.updateOffset(newOffset)
-        }
+        guard newOffset >= 0 else { return }
+        paginateIfPossible { $0.updateOffset(newOffset) }
     }
 
-    /// Apply both limit and offset changes and reload
     func applyPaginationSettings() {
         reloadCurrentPage()
     }
 
     // MARK: - Private
 
-    /// Confirm discard if needed, then mutate pagination state and reload.
+    private func paginateIfPossible(
+        where condition: (PaginationState) -> Bool = { _ in true },
+        mutate: @escaping (inout PaginationState) -> Void
+    ) {
+        guard let (tab, tabIndex) = tabManager.selectedTabAndIndex,
+              condition(tab.pagination) else { return }
+        paginateAfterConfirmation(tabIndex: tabIndex, mutate: mutate)
+    }
+
     private func paginateAfterConfirmation(
         tabIndex: Int,
         mutate: @escaping (inout PaginationState) -> Void

TableProTests/Models/Query/QueryTabManagerTests.swift

@@ -0,0 +1,60 @@
+//
+//  QueryTabManagerTests.swift
+//  TableProTests
+//
+//  Locks the contract for selectedTabAndIndex — the helper that
+//  MainContentCoordinator+Pagination (and future coordinator extensions)
+//  use in place of the selectedTabIndex + bounds-check + tabs[index]
+//  pattern. The tests guard against silent staleness if selectedTabId
+//  ever points to a removed tab.
+//
+
+import Foundation
+import Testing
+@testable import TablePro
+
+@Suite("QueryTabManager.selectedTabAndIndex")
+@MainActor
+struct QueryTabManagerSelectedTabAndIndexTests {
+    @Test("returns nil when no tab is selected")
+    func nilWhenNoSelection() {
+        let manager = QueryTabManager()
+        #expect(manager.selectedTabAndIndex == nil)
+    }
+
+    @Test("returns the selected tab and its index after addTableTab")
+    func returnsSelectedTabAfterAdd() {
+        let manager = QueryTabManager()
+        manager.addTableTab(tableName: "users")
+
+        let result = manager.selectedTabAndIndex
+        #expect(result?.index == 0)
+        #expect(result?.tab.tableContext.tableName == "users")
+    }
+
+    @Test("returns nil when selectedTabId points to a removed tab")
+    func nilWhenSelectionIsStale() {
+        let manager = QueryTabManager()
+        manager.addTableTab(tableName: "users")
+        let staleId = manager.tabs[0].id
+
+        manager.tabs.removeAll()
+        manager.selectedTabId = staleId
+
+        #expect(manager.selectedTabAndIndex == nil)
+    }
+
+    @Test("returns the correct (tab, index) pair after switching tabs")
+    func returnsCorrectPairAfterSwitch() {
+        let manager = QueryTabManager()
+        manager.addTableTab(tableName: "users")
+        manager.addTableTab(tableName: "orders")
+        let firstId = manager.tabs[0].id
+
+        manager.selectedTabId = firstId
+
+        let result = manager.selectedTabAndIndex
+        #expect(result?.index == 0)
+        #expect(result?.tab.tableContext.tableName == "users")
+    }
+}