From 847bc26634cc38eacc5287dd9b4aefbe555ae0e8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 28 Mar 2026 17:57:48 +0700 Subject: [PATCH 1/3] feat: add nested hierarchical groups for connection list (#478) --- CHANGELOG.md | 1 + TablePro/Core/Storage/GroupStorage.swift | 51 +- TablePro/Core/Sync/SyncRecordMapper.swift | 10 +- .../Models/Connection/ConnectionGroup.swift | 6 +- .../Connection/ConnectionGroupTree.swift | 166 ++++++ TablePro/ViewModels/WelcomeViewModel.swift | 120 +++-- .../Connection/ConnectionGroupPicker.swift | 126 ++++- .../Connection/WelcomeContextMenus.swift | 245 +++++---- .../Views/Connection/WelcomeWindowView.swift | 225 ++++++--- .../Models/ConnectionGroupTreeTests.swift | 477 ++++++++++++++++++ 10 files changed, 1150 insertions(+), 277 deletions(-) create mode 100644 TablePro/Models/Connection/ConnectionGroupTree.swift create mode 100644 TableProTests/Models/ConnectionGroupTreeTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index e4277ea2..3966469e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Nested hierarchical groups for connection list (up to 3 levels deep) with subgroup creation, group reparenting, and recursive delete - Confirmation dialogs for deep link queries, connection imports, and pre-connect scripts - JSON fields in Row Details sidebar now display in a scrollable monospaced text area diff --git a/TablePro/Core/Storage/GroupStorage.swift b/TablePro/Core/Storage/GroupStorage.swift index 782aef86..2790c1ee 100644 --- a/TablePro/Core/Storage/GroupStorage.swift +++ b/TablePro/Core/Storage/GroupStorage.swift @@ -53,35 +53,68 @@ final class GroupStorage { } } - /// Add a new group + /// Add a new group (duplicate check scoped to siblings, enforces depth cap and cycle prevention) func addGroup(_ group: ConnectionGroup) { var groups = loadGroups() - guard !groups.contains(where: { $0.name.lowercased() == group.name.lowercased() }) else { + guard !wouldCreateCircle(movingGroupId: group.id, toParentId: group.parentId, groups: groups) else { return } + guard validateDepth(parentId: group.parentId) else { return } + let siblings = groups.filter { $0.parentId == group.parentId } + guard !siblings.contains(where: { $0.name.lowercased() == group.name.lowercased() }) else { return } groups.append(group) saveGroups(groups) } - /// Update an existing group + /// Update an existing group (enforces cycle prevention and depth cap on parentId changes) func updateGroup(_ group: ConnectionGroup) { var groups = loadGroups() - if let index = groups.firstIndex(where: { $0.id == group.id }) { - groups[index] = group - saveGroups(groups) + guard let index = groups.firstIndex(where: { $0.id == group.id }) else { return } + if group.parentId != groups[index].parentId { + guard !wouldCreateCircle(movingGroupId: group.id, toParentId: group.parentId, groups: groups) else { return } + guard validateDepth(parentId: group.parentId) else { return } } + groups[index] = group + saveGroups(groups) } - /// Delete a group + /// Delete a group and all descendant groups, nil-out groupId on affected connections func deleteGroup(_ group: ConnectionGroup) { - SyncChangeTracker.shared.markDeleted(.group, id: group.id.uuidString) var groups = loadGroups() - groups.removeAll { $0.id == group.id } + let descendantIds = collectAllDescendantGroupIds(groupId: group.id, groups: groups) + let allIdsToDelete = descendantIds.union([group.id]) + + for deletedId in allIdsToDelete { + SyncChangeTracker.shared.markDeleted(.group, id: deletedId.uuidString) + } + + groups.removeAll { allIdsToDelete.contains($0.id) } saveGroups(groups) + + let storage = ConnectionStorage.shared + var connections = storage.loadConnections() + var changed = false + for i in connections.indices { + if let gid = connections[i].groupId, allIdsToDelete.contains(gid) { + connections[i].groupId = nil + changed = true + } + } + if changed { + storage.saveConnections(connections) + } } /// Get group by ID func group(for id: UUID) -> ConnectionGroup? { loadGroups().first { $0.id == id } } + + /// Validate that adding a child under parentId would not exceed max depth + func validateDepth(parentId: UUID?, maxDepth: Int = 3) -> Bool { + guard let pid = parentId else { return true } + let groups = loadGroups() + let parentDepth = depthOf(groupId: pid, groups: groups) + return parentDepth < maxDepth + } } diff --git a/TablePro/Core/Sync/SyncRecordMapper.swift b/TablePro/Core/Sync/SyncRecordMapper.swift index f3e554d1..15dfe67e 100644 --- a/TablePro/Core/Sync/SyncRecordMapper.swift +++ b/TablePro/Core/Sync/SyncRecordMapper.swift @@ -176,6 +176,10 @@ struct SyncRecordMapper { record["groupId"] = group.id.uuidString as CKRecordValue record["name"] = group.name as CKRecordValue record["color"] = group.color.rawValue as CKRecordValue + if let parentId = group.parentId { + record["parentId"] = parentId.uuidString as CKRecordValue + } + record["sortOrder"] = Int64(group.sortOrder) as CKRecordValue record["modifiedAtLocal"] = Date() as CKRecordValue record["schemaVersion"] = schemaVersion as CKRecordValue @@ -192,11 +196,15 @@ struct SyncRecordMapper { } let colorRaw = record["color"] as? String ?? ConnectionColor.none.rawValue + let parentId = (record["parentId"] as? String).flatMap { UUID(uuidString: $0) } + let sortOrder = (record["sortOrder"] as? Int64).map { Int($0) } ?? 0 return ConnectionGroup( id: groupId, name: name, - color: ConnectionColor(rawValue: colorRaw) ?? .none + color: ConnectionColor(rawValue: colorRaw) ?? .none, + parentId: parentId, + sortOrder: sortOrder ) } diff --git a/TablePro/Models/Connection/ConnectionGroup.swift b/TablePro/Models/Connection/ConnectionGroup.swift index 99164afb..36265af2 100644 --- a/TablePro/Models/Connection/ConnectionGroup.swift +++ b/TablePro/Models/Connection/ConnectionGroup.swift @@ -10,10 +10,14 @@ struct ConnectionGroup: Identifiable, Hashable, Codable { let id: UUID var name: String var color: ConnectionColor + var parentId: UUID? + var sortOrder: Int - init(id: UUID = UUID(), name: String, color: ConnectionColor = .none) { + init(id: UUID = UUID(), name: String, color: ConnectionColor = .none, parentId: UUID? = nil, sortOrder: Int = 0) { self.id = id self.name = name self.color = color + self.parentId = parentId + self.sortOrder = sortOrder } } diff --git a/TablePro/Models/Connection/ConnectionGroupTree.swift b/TablePro/Models/Connection/ConnectionGroupTree.swift new file mode 100644 index 00000000..bf2dee4c --- /dev/null +++ b/TablePro/Models/Connection/ConnectionGroupTree.swift @@ -0,0 +1,166 @@ +// +// ConnectionGroupTree.swift +// TablePro +// + +import Foundation + +enum ConnectionGroupTreeNode: Identifiable, Hashable { + case group(ConnectionGroup, children: [ConnectionGroupTreeNode]) + case connection(DatabaseConnection) + + var id: String { + switch self { + case .group(let g, _): "group-\(g.id)" + case .connection(let c): "conn-\(c.id)" + } + } + + static func == (lhs: Self, rhs: Self) -> Bool { lhs.id == rhs.id } + func hash(into hasher: inout Hasher) { hasher.combine(id) } +} + +// MARK: - Tree Building + +func buildGroupTree( + groups: [ConnectionGroup], + connections: [DatabaseConnection], + parentId: UUID?, + maxDepth: Int = 3, + currentDepth: Int = 0 +) -> [ConnectionGroupTreeNode] { + var items: [ConnectionGroupTreeNode] = [] + + let validGroupIds = Set(groups.map(\.id)) + + let levelGroups: [ConnectionGroup] + if parentId == nil { + levelGroups = groups + .filter { $0.parentId == nil || !validGroupIds.contains($0.parentId!) } + .sorted { $0.sortOrder != $1.sortOrder ? $0.sortOrder < $1.sortOrder : $0.name.localizedStandardCompare($1.name) == .orderedAscending } + } else { + levelGroups = groups + .filter { $0.parentId == parentId } + .sorted { $0.sortOrder != $1.sortOrder ? $0.sortOrder < $1.sortOrder : $0.name.localizedStandardCompare($1.name) == .orderedAscending } + } + + for group in levelGroups { + var children: [ConnectionGroupTreeNode] = [] + if currentDepth < maxDepth { + children = buildGroupTree( + groups: groups, + connections: connections, + parentId: group.id, + maxDepth: maxDepth, + currentDepth: currentDepth + 1 + ) + } + + let groupConnections = connections + .filter { $0.groupId == group.id } + for conn in groupConnections { + children.append(.connection(conn)) + } + + items.append(.group(group, children: children)) + } + + if parentId == nil { + let ungrouped = connections.filter { conn in + guard let groupId = conn.groupId else { return true } + return !validGroupIds.contains(groupId) + } + for conn in ungrouped { + items.append(.connection(conn)) + } + } + + return items +} + +// MARK: - Tree Filtering + +func filterGroupTree(_ items: [ConnectionGroupTreeNode], searchText: String) -> [ConnectionGroupTreeNode] { + guard !searchText.isEmpty else { return items } + + return items.compactMap { item in + switch item { + case .connection(let conn): + if conn.name.localizedCaseInsensitiveContains(searchText) + || conn.host.localizedCaseInsensitiveContains(searchText) + || conn.database.localizedCaseInsensitiveContains(searchText) { + return item + } + return nil + case .group(let group, let children): + if group.name.localizedCaseInsensitiveContains(searchText) { + return item + } + let filteredChildren = filterGroupTree(children, searchText: searchText) + if !filteredChildren.isEmpty { + return .group(group, children: filteredChildren) + } + return nil + } + } +} + +// MARK: - Tree Traversal + +func flattenVisibleConnections( + tree: [ConnectionGroupTreeNode], + expandedGroupIds: Set +) -> [DatabaseConnection] { + var result: [DatabaseConnection] = [] + for item in tree { + switch item { + case .connection(let conn): + result.append(conn) + case .group(let group, let children): + if expandedGroupIds.contains(group.id) { + result.append(contentsOf: flattenVisibleConnections(tree: children, expandedGroupIds: expandedGroupIds)) + } + } + } + return result +} + +func collectAllDescendantGroupIds(groupId: UUID, groups: [ConnectionGroup], visited: Set = []) -> Set { + var result = Set() + let directChildren = groups.filter { $0.parentId == groupId } + for child in directChildren where !visited.contains(child.id) { + result.insert(child.id) + result.formUnion(collectAllDescendantGroupIds(groupId: child.id, groups: groups, visited: visited.union(result).union([groupId]))) + } + return result +} + +func wouldCreateCircle(movingGroupId: UUID, toParentId: UUID?, groups: [ConnectionGroup]) -> Bool { + guard let targetId = toParentId else { return false } + if targetId == movingGroupId { return true } + let descendants = collectAllDescendantGroupIds(groupId: movingGroupId, groups: groups) + return descendants.contains(targetId) +} + +func depthOf(groupId: UUID?, groups: [ConnectionGroup], visited: Set = []) -> Int { + guard let gid = groupId else { return 0 } + guard !visited.contains(gid) else { return 0 } + guard let group = groups.first(where: { $0.id == gid }) else { return 0 } + return 1 + depthOf(groupId: group.parentId, groups: groups, visited: visited.union([gid])) +} + +func maxDescendantDepth(groupId: UUID, groups: [ConnectionGroup]) -> Int { + let children = groups.filter { $0.parentId == groupId } + if children.isEmpty { return 0 } + return 1 + children.map { maxDescendantDepth(groupId: $0.id, groups: groups) }.max()! +} + +func connectionCount(in groupId: UUID, connections: [DatabaseConnection], groups: [ConnectionGroup]) -> Int { + let directCount = connections.filter { $0.groupId == groupId }.count + let descendants = collectAllDescendantGroupIds(groupId: groupId, groups: groups) + let descendantCount = connections.filter { conn in + guard let gid = conn.groupId else { return false } + return descendants.contains(gid) + }.count + return directCount + descendantCount +} diff --git a/TablePro/ViewModels/WelcomeViewModel.swift b/TablePro/ViewModels/WelcomeViewModel.swift index 297892c0..61a3ec14 100644 --- a/TablePro/ViewModels/WelcomeViewModel.swift +++ b/TablePro/ViewModels/WelcomeViewModel.swift @@ -8,14 +8,14 @@ import os import SwiftUI enum WelcomeActiveSheet: Identifiable { - case newGroup + case newGroup(parentId: UUID?) case activation case importFile(URL) case exportConnections([DatabaseConnection]) var id: String { switch self { - case .newGroup: "newGroup" + case .newGroup(let parentId): "newGroup-\(parentId?.uuidString ?? "root")" case .activation: "activation" case .importFile(let u): "importFile-\(u.absoluteString)" case .exportConnections: "exportConnections" @@ -34,7 +34,7 @@ final class WelcomeViewModel { // MARK: - State var connections: [DatabaseConnection] = [] - var searchText = "" + var searchText = "" { didSet { rebuildTree() } } var selectedConnectionIds: Set = [] var groups: [ConnectionGroup] = [] var linkedConnections: [LinkedConnection] = [] @@ -49,14 +49,17 @@ final class WelcomeViewModel { var renameGroupName = "" var showRenameGroupAlert = false - var collapsedGroupIds: Set = { - let strings = UserDefaults.standard.stringArray(forKey: "com.TablePro.collapsedGroupIds") ?? [] + var expandedGroupIds: Set = { + let strings = UserDefaults.standard.stringArray(forKey: "com.TablePro.expandedGroupIds") ?? [] + if strings.isEmpty { + UserDefaults.standard.removeObject(forKey: "com.TablePro.collapsedGroupIds") + } return Set(strings.compactMap { UUID(uuidString: $0) }) }() { didSet { UserDefaults.standard.set( - Array(collapsedGroupIds.map(\.uuidString)), - forKey: "com.TablePro.collapsedGroupIds" + Array(expandedGroupIds.map(\.uuidString)), + forKey: "com.TablePro.expandedGroupIds" ) } } @@ -73,6 +76,17 @@ final class WelcomeViewModel { // MARK: - Computed Properties + private(set) var treeItems: [ConnectionGroupTreeNode] = [] + + func rebuildTree() { + let tree = buildGroupTree(groups: groups, connections: connections, parentId: nil) + if searchText.isEmpty { + treeItems = tree + } else { + treeItems = filterGroupTree(tree, searchText: searchText) + } + } + var filteredConnections: [DatabaseConnection] { if searchText.isEmpty { return connections @@ -85,25 +99,8 @@ final class WelcomeViewModel { } } - var ungroupedConnections: [DatabaseConnection] { - let validGroupIds = Set(groups.map(\.id)) - return filteredConnections.filter { conn in - guard let groupId = conn.groupId else { return true } - return !validGroupIds.contains(groupId) - } - } - - var activeGroups: [ConnectionGroup] { - let groupIds = Set(filteredConnections.compactMap(\.groupId)) - return groups.filter { groupIds.contains($0.id) } - } - var flatVisibleConnections: [DatabaseConnection] { - var result = ungroupedConnections - for group in activeGroups where !collapsedGroupIds.contains(group.id) { - result.append(contentsOf: connections(in: group)) - } - return result + flattenVisibleConnections(tree: treeItems, expandedGroupIds: expandedGroupIds) } var selectedConnections: [DatabaseConnection] { @@ -119,16 +116,19 @@ final class WelcomeViewModel { return groups.first { $0.id == groupId }?.name } - func connections(in group: ConnectionGroup) -> [DatabaseConnection] { - filteredConnections.filter { $0.groupId == group.id } - } - // MARK: - Setup & Teardown func setUp(openWindow: OpenWindowAction) { self.openWindow = openWindow guard connectionUpdatedObserver == nil else { return } + if expandedGroupIds.isEmpty { + let allGroupIds = Set(groupStorage.loadGroups().map(\.id)) + if !allGroupIds.isEmpty { + expandedGroupIds = allGroupIds + } + } + newConnectionObserver = NotificationCenter.default.addObserver( forName: .newConnection, object: nil, queue: .main ) { [weak self] _ in @@ -201,6 +201,7 @@ final class WelcomeViewModel { func loadGroups() { groups = groupStorage.loadGroups() + rebuildTree() } // MARK: - Connection Actions @@ -283,12 +284,8 @@ final class WelcomeViewModel { // MARK: - Groups func deleteGroup(_ group: ConnectionGroup) { - for i in connections.indices where connections[i].groupId == group.id { - connections[i].groupId = nil - } - storage.saveConnections(connections) groupStorage.deleteGroup(group) - groups = groupStorage.loadGroups() + loadConnections() } func beginRenameGroup(_ group: ConnectionGroup) { @@ -301,7 +298,8 @@ final class WelcomeViewModel { guard let target = renameGroupTarget else { return } let newName = renameGroupName.trimmingCharacters(in: .whitespaces) guard !newName.isEmpty else { return } - let isDuplicate = groups.contains { + let siblings = groups.filter { $0.parentId == target.parentId } + let isDuplicate = siblings.contains { $0.id != target.id && $0.name.lowercased() == newName.lowercased() } guard !isDuplicate else { return } @@ -325,6 +323,7 @@ final class WelcomeViewModel { connections[i].groupId = groupId } storage.saveConnections(connections) + rebuildTree() } func removeFromGroup(_ targets: [DatabaseConnection]) { @@ -333,6 +332,24 @@ final class WelcomeViewModel { connections[i].groupId = nil } storage.saveConnections(connections) + rebuildTree() + } + + func createSubgroup(under parentId: UUID) { + activeSheet = .newGroup(parentId: parentId) + } + + func moveGroup(_ group: ConnectionGroup, toParent newParentId: UUID?) { + guard !wouldCreateCircle(movingGroupId: group.id, toParentId: newParentId, groups: groups) else { return } + + let newParentDepth = depthOf(groupId: newParentId, groups: groups) + let subtreeDepth = maxDescendantDepth(groupId: group.id, groups: groups) + guard newParentDepth + 1 + subtreeDepth <= 3 else { return } + + var updated = group + updated.parentId = newParentId + groupStorage.updateGroup(updated) + groups = groupStorage.loadGroups() } // MARK: - Import / Export @@ -405,9 +422,9 @@ final class WelcomeViewModel { guard let id = selectedConnectionIds.first, let connection = connections.first(where: { $0.id == id }), let groupId = connection.groupId, - !collapsedGroupIds.contains(groupId) else { return } + expandedGroupIds.contains(groupId) else { return } withAnimation(.easeInOut(duration: 0.2)) { - collapsedGroupIds.insert(groupId) + expandedGroupIds.remove(groupId) } } @@ -415,9 +432,9 @@ final class WelcomeViewModel { guard let id = selectedConnectionIds.first, let connection = connections.first(where: { $0.id == id }), let groupId = connection.groupId, - collapsedGroupIds.contains(groupId) else { return } + !expandedGroupIds.contains(groupId) else { return } withAnimation(.easeInOut(duration: 0.2)) { - collapsedGroupIds.remove(groupId) + expandedGroupIds.insert(groupId) } } @@ -445,6 +462,7 @@ final class WelcomeViewModel { connections.move(fromOffsets: globalSource, toOffset: globalDestination) storage.saveConnections(connections) + rebuildTree() } func moveGroupedConnections(in group: ConnectionGroup, from source: IndexSet, to destination: Int) { @@ -465,29 +483,7 @@ final class WelcomeViewModel { connections.move(fromOffsets: globalSource, toOffset: globalDestination) storage.saveConnections(connections) - } - - func moveGroups(from source: IndexSet, to destination: Int) { - let active = activeGroups - let activeGroupIndices = active.compactMap { activeGroup in - groups.firstIndex(where: { $0.id == activeGroup.id }) - } - - guard source.allSatisfy({ $0 < activeGroupIndices.count }), - destination <= activeGroupIndices.count else { return } - - let globalSource = IndexSet(source.map { activeGroupIndices[$0] }) - let globalDestination: Int - if destination < activeGroupIndices.count { - globalDestination = activeGroupIndices[destination] - } else if let last = activeGroupIndices.last { - globalDestination = last + 1 - } else { - globalDestination = 0 - } - - groups.move(fromOffsets: globalSource, toOffset: globalDestination) - groupStorage.saveGroups(groups) + rebuildTree() } func focusConnectionFormWindow() { diff --git a/TablePro/Views/Connection/ConnectionGroupPicker.swift b/TablePro/Views/Connection/ConnectionGroupPicker.swift index 1e648e95..1e578f12 100644 --- a/TablePro/Views/Connection/ConnectionGroupPicker.swift +++ b/TablePro/Views/Connection/ConnectionGroupPicker.swift @@ -22,7 +22,6 @@ struct ConnectionGroupPicker: View { var body: some View { Menu { - // None option Button { selectedGroupId = nil } label: { @@ -37,27 +36,10 @@ struct ConnectionGroupPicker: View { Divider() - // Available groups - ForEach(allGroups) { group in - Button { - selectedGroupId = group.id - } label: { - HStack { - if !group.color.isDefault { - Image(nsImage: colorDot(group.color.color)) - } - Text(group.name) - if selectedGroupId == group.id { - Spacer() - Image(systemName: "checkmark") - } - } - } - } + hierarchicalGroupItems() Divider() - // Create new group Button { showingCreateSheet = true } label: { @@ -83,8 +65,8 @@ struct ConnectionGroupPicker: View { .fixedSize() .task { allGroups = groupStorage.loadGroups() } .sheet(isPresented: $showingCreateSheet) { - CreateGroupSheet { groupName, groupColor in - let group = ConnectionGroup(name: groupName, color: groupColor) + CreateGroupSheet { groupName, groupColor, parentId in + let group = ConnectionGroup(name: groupName, color: groupColor, parentId: parentId) groupStorage.addGroup(group) selectedGroupId = group.id allGroups = groupStorage.loadGroups() @@ -92,7 +74,27 @@ struct ConnectionGroupPicker: View { } } - /// Create a colored circle NSImage for use in menu items + @ViewBuilder + private func hierarchicalGroupItems() -> some View { + let flatGroups = flattenGroupsForMenu(groups: allGroups) + ForEach(flatGroups, id: \.group.id) { entry in + Button { + selectedGroupId = entry.group.id + } label: { + HStack { + if !entry.group.color.isDefault { + Image(nsImage: colorDot(entry.group.color.color)) + } + Text(String(repeating: " ", count: entry.depth) + entry.group.name) + if selectedGroupId == entry.group.id { + Spacer() + Image(systemName: "checkmark") + } + } + } + } + } + private func colorDot(_ color: Color) -> NSImage { let size = NSSize(width: 10, height: 10) let image = NSImage(size: size, flipped: false) { rect in @@ -111,7 +113,16 @@ struct CreateGroupSheet: View { @Environment(\.dismiss) private var dismiss @State private var groupName: String = "" @State private var groupColor: ConnectionColor = .none - let onSave: (String, ConnectionColor) -> Void + @State private var selectedParentId: UUID? + @State private var allGroups: [ConnectionGroup] = [] + + private let initialParentId: UUID? + let onSave: (String, ConnectionColor, UUID?) -> Void + + init(parentId: UUID? = nil, onSave: @escaping (String, ConnectionColor, UUID?) -> Void) { + self.initialParentId = parentId + self.onSave = onSave + } var body: some View { VStack(spacing: 16) { @@ -129,13 +140,22 @@ struct CreateGroupSheet: View { GroupColorPicker(selectedColor: $groupColor) } + if !allGroups.isEmpty { + VStack(alignment: .leading, spacing: 6) { + Text("Parent Group") + .font(.caption) + .foregroundStyle(.secondary) + ParentGroupPicker(selectedParentId: $selectedParentId, allGroups: allGroups) + } + } + HStack { Button("Cancel") { dismiss() } Button("Create") { - onSave(groupName, groupColor) + onSave(groupName, groupColor, selectedParentId) dismiss() } .keyboardShortcut(.return) @@ -145,12 +165,70 @@ struct CreateGroupSheet: View { } .padding(20) .frame(width: 300) + .onAppear { + allGroups = GroupStorage.shared.loadGroups() + selectedParentId = initialParentId + } .onExitCommand { dismiss() } } } +// MARK: - Parent Group Picker + +private struct ParentGroupPicker: View { + @Binding var selectedParentId: UUID? + let allGroups: [ConnectionGroup] + + var body: some View { + Menu { + Button { + selectedParentId = nil + } label: { + HStack { + Text("None (Top Level)") + if selectedParentId == nil { + Spacer() + Image(systemName: "checkmark") + } + } + } + + Divider() + + ForEach(allGroups.sorted(by: { $0.name.localizedStandardCompare($1.name) == .orderedAscending })) { group in + let depth = depthOf(groupId: group.id, groups: allGroups) + Button { + selectedParentId = group.id + } label: { + HStack { + Text(String(repeating: " ", count: max(0, depth - 1)) + group.name) + if selectedParentId == group.id { + Spacer() + Image(systemName: "checkmark") + } + } + } + .disabled(depth >= 3) + } + } label: { + Text(parentLabel) + .foregroundStyle(selectedParentId == nil ? .secondary : .primary) + } + .menuStyle(.borderlessButton) + .fixedSize() + } + + private var parentLabel: String { + guard let pid = selectedParentId, + let group = allGroups.first(where: { $0.id == pid }) else { + return String(localized: "None (Top Level)") + } + return group.name + } +} + // MARK: - Group Color Picker private struct GroupColorPicker: View { diff --git a/TablePro/Views/Connection/WelcomeContextMenus.swift b/TablePro/Views/Connection/WelcomeContextMenus.swift index fccea2e2..543ac72f 100644 --- a/TablePro/Views/Connection/WelcomeContextMenus.swift +++ b/TablePro/Views/Connection/WelcomeContextMenus.swift @@ -9,119 +9,129 @@ extension WelcomeWindowView { @ViewBuilder func contextMenuContent(for connection: DatabaseConnection) -> some View { if vm.isMultipleSelection, vm.selectedConnectionIds.contains(connection.id) { - Button { vm.connectSelectedConnections() } label: { - Label( - String(localized: "Connect \(vm.selectedConnectionIds.count) Connections"), - systemImage: "play.fill" - ) - } + multiSelectionContextMenu(for: connection) + } else { + singleConnectionContextMenu(for: connection) + } + } - Divider() + @ViewBuilder + private func multiSelectionContextMenu(for connection: DatabaseConnection) -> some View { + Button { vm.connectSelectedConnections() } label: { + Label( + String(localized: "Connect \(vm.selectedConnectionIds.count) Connections"), + systemImage: "play.fill" + ) + } - Button { - vm.exportConnections(Array(vm.selectedConnections)) - } label: { - Label( - String(localized: "Export \(vm.selectedConnectionIds.count) Connections..."), - systemImage: "square.and.arrow.up" - ) - } + Divider() - Divider() + Button { + vm.exportConnections(Array(vm.selectedConnections)) + } label: { + Label( + String(localized: "Export \(vm.selectedConnectionIds.count) Connections..."), + systemImage: "square.and.arrow.up" + ) + } - moveToGroupMenu(for: vm.selectedConnections) + Divider() - let validGroupIds = Set(vm.groups.map(\.id)) - if vm.selectedConnections.contains(where: { $0.groupId.map { validGroupIds.contains($0) } ?? false }) { - Button { vm.removeFromGroup(vm.selectedConnections) } label: { - Label(String(localized: "Remove from Group"), systemImage: "folder.badge.minus") - } + moveToGroupMenu(for: vm.selectedConnections) + + let validGroupIds = Set(vm.groups.map(\.id)) + if vm.selectedConnections.contains(where: { $0.groupId.map { validGroupIds.contains($0) } ?? false }) { + Button { vm.removeFromGroup(vm.selectedConnections) } label: { + Label(String(localized: "Remove from Group"), systemImage: "folder.badge.minus") } + } - Divider() + Divider() - Button(role: .destructive) { - vm.connectionsToDelete = vm.selectedConnections - vm.showDeleteConfirmation = true - } label: { - Label( - String(localized: "Delete \(vm.selectedConnectionIds.count) Connections"), - systemImage: "trash" - ) - } - } else { - Button { vm.connectToDatabase(connection) } label: { - Label(String(localized: "Connect"), systemImage: "play.fill") - } + Button(role: .destructive) { + vm.connectionsToDelete = vm.selectedConnections + vm.showDeleteConfirmation = true + } label: { + Label( + String(localized: "Delete \(vm.selectedConnectionIds.count) Connections"), + systemImage: "trash" + ) + } + } - Divider() + @ViewBuilder + private func singleConnectionContextMenu(for connection: DatabaseConnection) -> some View { + Button { vm.connectToDatabase(connection) } label: { + Label(String(localized: "Connect"), systemImage: "play.fill") + } - Button { - openWindow(id: "connection-form", value: connection.id as UUID?) - vm.focusConnectionFormWindow() - } label: { - Label(String(localized: "Edit"), systemImage: "pencil") - } + Divider() - Button { vm.duplicateConnection(connection) } label: { - Label(String(localized: "Duplicate"), systemImage: "doc.on.doc") - } + Button { + openWindow(id: "connection-form", value: connection.id as UUID?) + vm.focusConnectionFormWindow() + } label: { + Label(String(localized: "Edit"), systemImage: "pencil") + } - Divider() + Button { vm.duplicateConnection(connection) } label: { + Label(String(localized: "Duplicate"), systemImage: "doc.on.doc") + } - Button { - let pw = ConnectionStorage.shared.loadPassword(for: connection.id) - let sshPw: String? - let sshProfile: SSHProfile? - if let profileId = connection.sshProfileId { - sshPw = SSHProfileStorage.shared.loadSSHPassword(for: profileId) - sshProfile = SSHProfileStorage.shared.profile(for: profileId) - } else { - sshPw = ConnectionStorage.shared.loadSSHPassword(for: connection.id) - sshProfile = nil - } - let url = ConnectionURLFormatter.format( - connection, - password: pw, - sshPassword: sshPw, - sshProfile: sshProfile - ) - ClipboardService.shared.writeText(url) - } label: { - Label(String(localized: "Copy as URL"), systemImage: "link") - } + Divider() - Button { - let link = ConnectionExportService.buildImportDeeplink(for: connection) - ClipboardService.shared.writeText(link) - } label: { - Label(String(localized: "Copy as Import Link"), systemImage: "link.badge.plus") + Button { + let pw = ConnectionStorage.shared.loadPassword(for: connection.id) + let sshPw: String? + let sshProfile: SSHProfile? + if let profileId = connection.sshProfileId { + sshPw = SSHProfileStorage.shared.loadSSHPassword(for: profileId) + sshProfile = SSHProfileStorage.shared.profile(for: profileId) + } else { + sshPw = ConnectionStorage.shared.loadSSHPassword(for: connection.id) + sshProfile = nil } + let url = ConnectionURLFormatter.format( + connection, + password: pw, + sshPassword: sshPw, + sshProfile: sshProfile + ) + ClipboardService.shared.writeText(url) + } label: { + Label(String(localized: "Copy as URL"), systemImage: "link") + } - Button { - vm.exportConnections([connection]) - } label: { - Label(String(localized: "Export..."), systemImage: "square.and.arrow.up") - } + Button { + let link = ConnectionExportService.buildImportDeeplink(for: connection) + ClipboardService.shared.writeText(link) + } label: { + Label(String(localized: "Copy as Import Link"), systemImage: "link.badge.plus") + } + + Button { + vm.exportConnections([connection]) + } label: { + Label(String(localized: "Export..."), systemImage: "square.and.arrow.up") + } - Divider() + Divider() - moveToGroupMenu(for: [connection]) + moveToGroupMenu(for: [connection]) - if let groupId = connection.groupId, vm.groups.contains(where: { $0.id == groupId }) { - Button { vm.removeFromGroup([connection]) } label: { - Label(String(localized: "Remove from Group"), systemImage: "folder.badge.minus") - } + if let groupId = connection.groupId, vm.groups.contains(where: { $0.id == groupId }) { + Button { vm.removeFromGroup([connection]) } label: { + Label(String(localized: "Remove from Group"), systemImage: "folder.badge.minus") } + } - Divider() + Divider() - Button(role: .destructive) { - vm.connectionsToDelete = [connection] - vm.showDeleteConfirmation = true - } label: { - Label(String(localized: "Delete"), systemImage: "trash") - } + Button(role: .destructive) { + vm.connectionsToDelete = [connection] + vm.showDeleteConfirmation = true + } label: { + Label(String(localized: "Delete"), systemImage: "trash") } } @@ -129,24 +139,25 @@ extension WelcomeWindowView { func moveToGroupMenu(for targets: [DatabaseConnection]) -> some View { let isSingle = targets.count == 1 let currentGroupId = isSingle ? targets.first?.groupId : nil + let flatGroups = flattenGroupsForMenu(groups: vm.groups) Menu(String(localized: "Move to Group")) { - ForEach(vm.groups) { group in + ForEach(flatGroups, id: \.group.id) { entry in Button { - vm.moveConnections(targets, toGroup: group.id) + vm.moveConnections(targets, toGroup: entry.group.id) } label: { HStack { - if !group.color.isDefault { + if !entry.group.color.isDefault { Image(systemName: "circle.fill") - .foregroundStyle(group.color.color) + .foregroundStyle(entry.group.color.color) } - Text(group.name) - if currentGroupId == group.id { + Text(String(repeating: " ", count: entry.depth) + entry.group.name) + if currentGroupId == entry.group.id { Spacer() Image(systemName: "checkmark") } } } - .disabled(currentGroupId == group.id) + .disabled(currentGroupId == entry.group.id) } if !vm.groups.isEmpty { @@ -155,7 +166,7 @@ extension WelcomeWindowView { Button { vm.pendingMoveToNewGroup = targets - vm.activeSheet = .newGroup + vm.activeSheet = .newGroup(parentId: nil) } label: { Label(String(localized: "New Group..."), systemImage: "folder.badge.plus") } @@ -177,3 +188,39 @@ extension WelcomeWindowView { } } } + +// MARK: - Flat Group Entry + +struct FlatGroupEntry { + let group: ConnectionGroup + let depth: Int +} + +func flattenGroupsForMenu(groups: [ConnectionGroup], parentId: UUID? = nil, depth: Int = 0) -> [FlatGroupEntry] { + let validGroupIds = Set(groups.map(\.id)) + let levelGroups: [ConnectionGroup] + if parentId == nil { + levelGroups = groups + .filter { $0.parentId == nil || !validGroupIds.contains($0.parentId!) } + .sorted { + $0.sortOrder != $1.sortOrder + ? $0.sortOrder < $1.sortOrder + : $0.name.localizedStandardCompare($1.name) == .orderedAscending + } + } else { + levelGroups = groups + .filter { $0.parentId == parentId } + .sorted { + $0.sortOrder != $1.sortOrder + ? $0.sortOrder < $1.sortOrder + : $0.name.localizedStandardCompare($1.name) == .orderedAscending + } + } + + var result: [FlatGroupEntry] = [] + for group in levelGroups { + result.append(FlatGroupEntry(group: group, depth: depth)) + result.append(contentsOf: flattenGroupsForMenu(groups: groups, parentId: group.id, depth: depth + 1)) + } + return result +} diff --git a/TablePro/Views/Connection/WelcomeWindowView.swift b/TablePro/Views/Connection/WelcomeWindowView.swift index b181f80e..22dba8d3 100644 --- a/TablePro/Views/Connection/WelcomeWindowView.swift +++ b/TablePro/Views/Connection/WelcomeWindowView.swift @@ -60,11 +60,15 @@ struct WelcomeWindowView: View { } .sheet(item: $vm.activeSheet) { sheet in switch sheet { - case .newGroup: - CreateGroupSheet { name, color in - let group = ConnectionGroup(name: name, color: color) + case .newGroup(let parentId): + CreateGroupSheet(parentId: parentId) { name, color, pid in + let group = ConnectionGroup(name: name, color: color, parentId: pid) GroupStorage.shared.addGroup(group) vm.groups = GroupStorage.shared.loadGroups() + vm.expandedGroupIds.insert(group.id) + if let pid { + vm.expandedGroupIds.insert(pid) + } if !vm.pendingMoveToNewGroup.isEmpty { vm.moveConnections(vm.pendingMoveToNewGroup, toGroup: group.id) vm.pendingMoveToNewGroup = [] @@ -129,7 +133,7 @@ struct WelcomeWindowView: View { .buttonStyle(.plain) .help("New Connection (⌘N)") - Button(action: { vm.pendingMoveToNewGroup = []; vm.activeSheet = .newGroup }) { + Button(action: { vm.pendingMoveToNewGroup = []; vm.activeSheet = .newGroup(parentId: nil) }) { Image(systemName: "folder.badge.plus") .font(.system(size: ThemeEngine.shared.activeTheme.typography.medium, weight: .medium)) .foregroundStyle(.secondary) @@ -208,7 +212,7 @@ struct WelcomeWindowView: View { Divider() - if vm.filteredConnections.isEmpty { + if vm.treeItems.isEmpty && vm.filteredConnections.isEmpty { emptyState } else { connectionList @@ -224,33 +228,7 @@ struct WelcomeWindowView: View { private var connectionList: some View { ScrollViewReader { proxy in List(selection: $vm.selectedConnectionIds) { - ForEach(vm.ungroupedConnections) { connection in - connectionRow(for: connection) - } - .onMove { from, to in - guard vm.searchText.isEmpty else { return } - vm.moveUngroupedConnections(from: from, to: to) - } - - ForEach(vm.activeGroups) { group in - Section { - if !vm.collapsedGroupIds.contains(group.id) { - ForEach(vm.connections(in: group)) { connection in - connectionRow(for: connection) - } - .onMove { from, to in - guard vm.searchText.isEmpty else { return } - vm.moveGroupedConnections(in: group, from: from, to: to) - } - } - } header: { - groupHeader(for: group) - } - } - .onMove { from, to in - guard vm.searchText.isEmpty else { return } - vm.moveGroups(from: from, to: to) - } + treeRows(vm.treeItems) if !vm.linkedConnections.isEmpty, LicenseManager.shared.isFeatureAvailable(.linkedFolders) { Section { @@ -320,6 +298,46 @@ struct WelcomeWindowView: View { } } + // MARK: - Tree Rendering + + private func treeRows(_ items: [ConnectionGroupTreeNode], parentGroupId: UUID? = nil) -> AnyView { + AnyView( + ForEach(items) { item in + switch item { + case .connection(let conn): + connectionRow(for: conn) + case .group(let group, let children): + DisclosureGroup(isExpanded: expandedBinding(for: group.id)) { + treeRows(children, parentGroupId: group.id) + } label: { + groupLabel(for: group) + } + } + } + .onMove { from, to in + guard vm.searchText.isEmpty else { return } + if let parentGroupId, let group = vm.groups.first(where: { $0.id == parentGroupId }) { + vm.moveGroupedConnections(in: group, from: from, to: to) + } else { + vm.moveUngroupedConnections(from: from, to: to) + } + } + ) + } + + private func expandedBinding(for groupId: UUID) -> Binding { + Binding( + get: { vm.expandedGroupIds.contains(groupId) }, + set: { expanded in + if expanded { + vm.expandedGroupIds.insert(groupId) + } else { + vm.expandedGroupIds.remove(groupId) + } + } + ) + } + // MARK: - Rows private func connectionRow(for connection: DatabaseConnection) -> some View { @@ -370,78 +388,123 @@ struct WelcomeWindowView: View { } } - // MARK: - Group Header + // MARK: - Group Label - private func groupHeader(for group: ConnectionGroup) -> some View { - Button(action: { - withAnimation(.easeInOut(duration: 0.2)) { - if vm.collapsedGroupIds.contains(group.id) { - vm.collapsedGroupIds.remove(group.id) - } else { - vm.collapsedGroupIds.insert(group.id) - } + private func groupLabel(for group: ConnectionGroup) -> some View { + HStack(spacing: 6) { + if !group.color.isDefault { + Circle() + .fill(group.color.color) + .frame(width: 8, height: 8) } - }) { - HStack(spacing: 6) { - Image(systemName: vm.collapsedGroupIds.contains(group.id) ? "chevron.right" : "chevron.down") - .font(.system(size: ThemeEngine.shared.activeTheme.typography.small, weight: .medium)) - .foregroundStyle(.tertiary) - .frame(width: 12) - if !group.color.isDefault { - Circle() - .fill(group.color.color) - .frame(width: 8, height: 8) - } - - Text(group.name) - .font(.system(size: ThemeEngine.shared.activeTheme.typography.small, weight: .semibold)) - .foregroundStyle(.secondary) + Text(group.name) + .font(.system(size: ThemeEngine.shared.activeTheme.typography.small, weight: .semibold)) + .foregroundStyle(.secondary) - Text("\(vm.connections(in: group).count)") - .font(.system(size: ThemeEngine.shared.activeTheme.typography.tiny)) - .foregroundStyle(.tertiary) + Text("\(connectionCount(in: group.id, connections: vm.connections, groups: vm.groups))") + .font(.system(size: ThemeEngine.shared.activeTheme.typography.tiny)) + .foregroundStyle(.tertiary) - Spacer() - } - .contentShape(Rectangle()) + Spacer() } - .buttonStyle(.plain) - .accessibilityLabel(String(localized: "\(group.name), \(vm.collapsedGroupIds.contains(group.id) ? "expand" : "collapse")")) + .contentShape(Rectangle()) .contextMenu { - Button { - vm.beginRenameGroup(group) - } label: { - Label(String(localized: "Rename"), systemImage: "pencil") + groupContextMenu(for: group) + } + } + + // MARK: - Group Context Menu + + @ViewBuilder + private func groupContextMenu(for group: ConnectionGroup) -> some View { + Button { + vm.beginRenameGroup(group) + } label: { + Label(String(localized: "Rename"), systemImage: "pencil") + } + + let currentGroupDepth = depthOf(groupId: group.id, groups: vm.groups) + Button { + vm.createSubgroup(under: group.id) + } label: { + Label(String(localized: "New Subgroup"), systemImage: "folder.badge.plus") + } + .disabled(currentGroupDepth >= 3) + + Menu(String(localized: "Change Color")) { + ForEach(ConnectionColor.allCases) { color in + Button { + vm.updateGroupColor(group, color: color) + } label: { + HStack { + if color != .none { + Image(systemName: "circle.fill") + .foregroundStyle(color.color) + } + Text(color.displayName) + if group.color == color { + Spacer() + Image(systemName: "checkmark") + } + } + } } + } + + if vm.groups.count > 1 { + Menu(String(localized: "Move Group to...")) { + Button { + vm.moveGroup(group, toParent: nil) + } label: { + HStack { + Text("Top Level") + if group.parentId == nil { + Spacer() + Image(systemName: "checkmark") + } + } + } + .disabled(group.parentId == nil) + + Divider() + + ForEach(vm.groups.filter({ $0.id != group.id })) { targetGroup in + let wouldCircle = wouldCreateCircle( + movingGroupId: group.id, + toParentId: targetGroup.id, + groups: vm.groups + ) + let targetDepth = depthOf(groupId: targetGroup.id, groups: vm.groups) + let subtreeDepth = maxDescendantDepth(groupId: group.id, groups: vm.groups) + let wouldExceedDepth = targetDepth + 1 + subtreeDepth > 3 - Menu(String(localized: "Change Color")) { - ForEach(ConnectionColor.allCases) { color in Button { - vm.updateGroupColor(group, color: color) + vm.moveGroup(group, toParent: targetGroup.id) } label: { HStack { - if color != .none { + if !targetGroup.color.isDefault { Image(systemName: "circle.fill") - .foregroundStyle(color.color) + .foregroundStyle(targetGroup.color.color) } - Text(color.displayName) - if group.color == color { + Text(targetGroup.name) + if group.parentId == targetGroup.id { Spacer() Image(systemName: "checkmark") } } } + .disabled(wouldCircle || wouldExceedDepth || group.parentId == targetGroup.id) } } + } - Divider() + Divider() - Button(role: .destructive) { - vm.deleteGroup(group) - } label: { - Label(String(localized: "Delete Group"), systemImage: "trash") - } + Button(role: .destructive) { + vm.deleteGroup(group) + } label: { + Label(String(localized: "Delete Group"), systemImage: "trash") } } diff --git a/TableProTests/Models/ConnectionGroupTreeTests.swift b/TableProTests/Models/ConnectionGroupTreeTests.swift new file mode 100644 index 00000000..ac5e20cd --- /dev/null +++ b/TableProTests/Models/ConnectionGroupTreeTests.swift @@ -0,0 +1,477 @@ +// +// ConnectionGroupTreeTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("ConnectionGroupTree") +struct ConnectionGroupTreeTests { + + // MARK: - Helpers + + private func makeGroup( + id: UUID = UUID(), + name: String, + parentId: UUID? = nil, + sortOrder: Int = 0 + ) -> ConnectionGroup { + ConnectionGroup(id: id, name: name, parentId: parentId, sortOrder: sortOrder) + } + + private func makeConnection( + id: UUID = UUID(), + name: String, + groupId: UUID? = nil, + host: String = "localhost", + database: String = "" + ) -> DatabaseConnection { + DatabaseConnection(id: id, name: name, host: host, database: database, groupId: groupId) + } + + private func groupIds(from nodes: [ConnectionGroupTreeNode]) -> [UUID] { + nodes.compactMap { node in + if case .group(let g, _) = node { return g.id } + return nil + } + } + + private func connectionIds(from nodes: [ConnectionGroupTreeNode]) -> [UUID] { + nodes.compactMap { node in + if case .connection(let c) = node { return c.id } + return nil + } + } + + private func children(of node: ConnectionGroupTreeNode) -> [ConnectionGroupTreeNode] { + if case .group(_, let children) = node { return children } + return [] + } + + // MARK: - buildGroupTree + + @Test("Empty inputs produce empty tree") + func buildGroupTree_emptyInputs() { + let result = buildGroupTree(groups: [], connections: [], parentId: nil) + #expect(result.isEmpty) + } + + @Test("Ungrouped connections appear as top-level connection nodes") + func buildGroupTree_ungroupedConnections() { + let c1 = makeConnection(name: "DB1") + let c2 = makeConnection(name: "DB2") + + let result = buildGroupTree(groups: [], connections: [c1, c2], parentId: nil) + + #expect(result.count == 2) + let ids = connectionIds(from: result) + #expect(ids.contains(c1.id)) + #expect(ids.contains(c2.id)) + } + + @Test("Single-level groups contain their connections") + func buildGroupTree_singleLevelGroups() { + let gId = UUID() + let group = makeGroup(id: gId, name: "Production") + let c1 = makeConnection(name: "Prod DB", groupId: gId) + let c2 = makeConnection(name: "Local DB") + + let result = buildGroupTree(groups: [group], connections: [c1, c2], parentId: nil) + + #expect(result.count == 2) + let gIds = groupIds(from: result) + #expect(gIds.contains(gId)) + + let groupNode = result.first { if case .group(let g, _) = $0 { return g.id == gId } else { return false } }! + let groupChildren = children(of: groupNode) + let childConnIds = connectionIds(from: groupChildren) + #expect(childConnIds == [c1.id]) + + let topConnIds = connectionIds(from: result) + #expect(topConnIds == [c2.id]) + } + + @Test("Three-level nested groups produce correct hierarchy") + func buildGroupTree_threeLevelNesting() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let g1 = makeGroup(id: id1, name: "Level 1") + let g2 = makeGroup(id: id2, name: "Level 2", parentId: id1) + let g3 = makeGroup(id: id3, name: "Level 3", parentId: id2) + let conn = makeConnection(name: "Deep DB", groupId: id3) + + let result = buildGroupTree(groups: [g1, g2, g3], connections: [conn], parentId: nil) + + #expect(result.count == 1) + let level1Children = children(of: result[0]) + #expect(groupIds(from: level1Children) == [id2]) + + let level2Children = children(of: level1Children[0]) + #expect(groupIds(from: level2Children) == [id3]) + + let level3Children = children(of: level2Children[0]) + #expect(connectionIds(from: level3Children) == [conn.id]) + } + + @Test("Max depth cap stops recursion at depth 3") + func buildGroupTree_maxDepthCap() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let id4 = UUID() + let g1 = makeGroup(id: id1, name: "L1") + let g2 = makeGroup(id: id2, name: "L2", parentId: id1) + let g3 = makeGroup(id: id3, name: "L3", parentId: id2) + let g4 = makeGroup(id: id4, name: "L4", parentId: id3) + let conn = makeConnection(name: "Deep", groupId: id4) + + let result = buildGroupTree( + groups: [g1, g2, g3, g4], + connections: [conn], + parentId: nil, + maxDepth: 3 + ) + + // L1 -> L2 -> L3 -> (L4 should still appear since depth 0,1,2 are within maxDepth=3) + let l1Children = children(of: result[0]) + let l2Children = children(of: l1Children[0]) + let l3Children = children(of: l2Children[0]) + + // At depth 3, recursion builds children for L3, which finds L4 at currentDepth=3 + // Since currentDepth (3) is NOT < maxDepth (3), L4's children won't recurse further + // but L4 itself still appears as a group with only its direct connections + let l3GroupIds = groupIds(from: l3Children) + #expect(l3GroupIds == [id4]) + + // L4 should have the connection but no further nested groups + let l4Children = children(of: l3Children[0]) + #expect(connectionIds(from: l4Children) == [conn.id]) + } + + @Test("Orphan groups with non-existent parentId are treated as top-level") + func buildGroupTree_orphanGroups() { + let orphanGroup = makeGroup(name: "Orphan", parentId: UUID()) + let conn = makeConnection(name: "In orphan", groupId: orphanGroup.id) + + let result = buildGroupTree(groups: [orphanGroup], connections: [conn], parentId: nil) + + #expect(groupIds(from: result) == [orphanGroup.id]) + let groupChildren = children(of: result[0]) + #expect(connectionIds(from: groupChildren) == [conn.id]) + } + + @Test("Orphan connections with non-existent groupId are treated as top-level") + func buildGroupTree_orphanConnections() { + let conn = makeConnection(name: "Orphan Conn", groupId: UUID()) + + let result = buildGroupTree(groups: [], connections: [conn], parentId: nil) + + #expect(connectionIds(from: result) == [conn.id]) + } + + @Test("Groups are sorted by sortOrder first, then name") + func buildGroupTree_sorting() { + let g1 = makeGroup(name: "Beta", sortOrder: 2) + let g2 = makeGroup(name: "Alpha", sortOrder: 1) + let g3 = makeGroup(name: "Charlie", sortOrder: 1) + + // g2 and g3 each need a connection to appear in tree + let c1 = makeConnection(name: "c1", groupId: g1.id) + let c2 = makeConnection(name: "c2", groupId: g2.id) + let c3 = makeConnection(name: "c3", groupId: g3.id) + + let result = buildGroupTree( + groups: [g1, g2, g3], + connections: [c1, c2, c3], + parentId: nil + ) + + let names = result.compactMap { node -> String? in + if case .group(let g, _) = node { return g.name } + return nil + } + // sortOrder 1 first (Alpha, Charlie alphabetically), then sortOrder 2 (Beta) + #expect(names == ["Alpha", "Charlie", "Beta"]) + } + + // MARK: - filterGroupTree + + @Test("Empty search text returns input unchanged") + func filterGroupTree_emptySearch() { + let conn = makeConnection(name: "Test") + let tree: [ConnectionGroupTreeNode] = [.connection(conn)] + + let result = filterGroupTree(tree, searchText: "") + #expect(result.count == 1) + } + + @Test("Search matches connection name returns only matching connections") + func filterGroupTree_matchesConnectionName() { + let c1 = makeConnection(name: "Production DB") + let c2 = makeConnection(name: "Staging DB") + let tree: [ConnectionGroupTreeNode] = [.connection(c1), .connection(c2)] + + let result = filterGroupTree(tree, searchText: "Production") + #expect(result.count == 1) + #expect(connectionIds(from: result) == [c1.id]) + } + + @Test("Search matches group name preserves entire subtree") + func filterGroupTree_matchesGroupName() { + let group = makeGroup(name: "Production") + let conn = makeConnection(name: "mydb") + let tree: [ConnectionGroupTreeNode] = [ + .group(group, children: [.connection(conn)]) + ] + + let result = filterGroupTree(tree, searchText: "Production") + + #expect(result.count == 1) + if case .group(let g, let kids) = result[0] { + #expect(g.id == group.id) + // Entire subtree preserved when group name matches + #expect(kids.count == 1) + } else { + Issue.record("Expected group node") + } + } + + @Test("Search matches nested connection preserves parent chain") + func filterGroupTree_matchesNestedConnection() { + let group = makeGroup(name: "Servers") + let c1 = makeConnection(name: "Production DB") + let c2 = makeConnection(name: "Staging DB") + let tree: [ConnectionGroupTreeNode] = [ + .group(group, children: [.connection(c1), .connection(c2)]) + ] + + let result = filterGroupTree(tree, searchText: "Production") + + #expect(result.count == 1) + if case .group(_, let kids) = result[0] { + #expect(kids.count == 1) + #expect(connectionIds(from: kids) == [c1.id]) + } else { + Issue.record("Expected group node") + } + } + + @Test("No matches returns empty result") + func filterGroupTree_noMatches() { + let conn = makeConnection(name: "Test DB") + let tree: [ConnectionGroupTreeNode] = [.connection(conn)] + + let result = filterGroupTree(tree, searchText: "nonexistent") + #expect(result.isEmpty) + } + + // MARK: - flattenVisibleConnections + + @Test("All groups expanded returns all connections in depth-first order") + func flattenVisibleConnections_allExpanded() { + let gId = UUID() + let group = makeGroup(id: gId, name: "G") + let c1 = makeConnection(name: "Inside") + let c2 = makeConnection(name: "Outside") + let tree: [ConnectionGroupTreeNode] = [ + .group(group, children: [.connection(c1)]), + .connection(c2) + ] + + let result = flattenVisibleConnections(tree: tree, expandedGroupIds: [gId]) + + #expect(result.map(\.id) == [c1.id, c2.id]) + } + + @Test("Collapsed group skips its children") + func flattenVisibleConnections_collapsedGroup() { + let gId = UUID() + let group = makeGroup(id: gId, name: "G") + let c1 = makeConnection(name: "Inside") + let c2 = makeConnection(name: "Outside") + let tree: [ConnectionGroupTreeNode] = [ + .group(group, children: [.connection(c1)]), + .connection(c2) + ] + + let result = flattenVisibleConnections(tree: tree, expandedGroupIds: []) + + #expect(result.map(\.id) == [c2.id]) + } + + @Test("Nested collapse hides all descendants") + func flattenVisibleConnections_nestedCollapse() { + let gOuter = UUID() + let gInner = UUID() + let outerGroup = makeGroup(id: gOuter, name: "Outer") + let innerGroup = makeGroup(id: gInner, name: "Inner") + let c1 = makeConnection(name: "Deep") + let c2 = makeConnection(name: "Top") + let tree: [ConnectionGroupTreeNode] = [ + .group(outerGroup, children: [ + .group(innerGroup, children: [.connection(c1)]) + ]), + .connection(c2) + ] + + // Outer collapsed, inner expanded -> still no c1 visible + let result = flattenVisibleConnections(tree: tree, expandedGroupIds: [gInner]) + #expect(result.map(\.id) == [c2.id]) + } + + // MARK: - collectAllDescendantGroupIds + + @Test("Leaf group with no children returns empty set") + func collectAllDescendantGroupIds_leaf() { + let gId = UUID() + let group = makeGroup(id: gId, name: "Leaf") + + let result = collectAllDescendantGroupIds(groupId: gId, groups: [group]) + #expect(result.isEmpty) + } + + @Test("Single child returns set containing child") + func collectAllDescendantGroupIds_singleChild() { + let parentId = UUID() + let childId = UUID() + let parent = makeGroup(id: parentId, name: "Parent") + let child = makeGroup(id: childId, name: "Child", parentId: parentId) + + let result = collectAllDescendantGroupIds(groupId: parentId, groups: [parent, child]) + #expect(result == [childId]) + } + + @Test("Deep nesting returns all descendants") + func collectAllDescendantGroupIds_deepNesting() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let g1 = makeGroup(id: id1, name: "G1") + let g2 = makeGroup(id: id2, name: "G2", parentId: id1) + let g3 = makeGroup(id: id3, name: "G3", parentId: id2) + + let result = collectAllDescendantGroupIds(groupId: id1, groups: [g1, g2, g3]) + #expect(result == [id2, id3]) + } + + // MARK: - wouldCreateCircle + + @Test("Move to nil parent never creates circle") + func wouldCreateCircle_nilParent() { + let gId = UUID() + let group = makeGroup(id: gId, name: "G") + + #expect(!wouldCreateCircle(movingGroupId: gId, toParentId: nil, groups: [group])) + } + + @Test("Move to self creates circle") + func wouldCreateCircle_self() { + let gId = UUID() + let group = makeGroup(id: gId, name: "G") + + #expect(wouldCreateCircle(movingGroupId: gId, toParentId: gId, groups: [group])) + } + + @Test("Move to direct child creates circle") + func wouldCreateCircle_directChild() { + let parentId = UUID() + let childId = UUID() + let parent = makeGroup(id: parentId, name: "Parent") + let child = makeGroup(id: childId, name: "Child", parentId: parentId) + + #expect(wouldCreateCircle(movingGroupId: parentId, toParentId: childId, groups: [parent, child])) + } + + @Test("Move to deep descendant creates circle") + func wouldCreateCircle_deepDescendant() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let g1 = makeGroup(id: id1, name: "G1") + let g2 = makeGroup(id: id2, name: "G2", parentId: id1) + let g3 = makeGroup(id: id3, name: "G3", parentId: id2) + + #expect(wouldCreateCircle(movingGroupId: id1, toParentId: id3, groups: [g1, g2, g3])) + } + + @Test("Move to sibling does not create circle") + func wouldCreateCircle_sibling() { + let id1 = UUID() + let id2 = UUID() + let g1 = makeGroup(id: id1, name: "G1") + let g2 = makeGroup(id: id2, name: "G2") + + #expect(!wouldCreateCircle(movingGroupId: id1, toParentId: id2, groups: [g1, g2])) + } + + @Test("Move to unrelated group does not create circle") + func wouldCreateCircle_unrelated() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let g1 = makeGroup(id: id1, name: "G1") + let g2 = makeGroup(id: id2, name: "G2", parentId: id1) + let g3 = makeGroup(id: id3, name: "G3") + + #expect(!wouldCreateCircle(movingGroupId: id1, toParentId: id3, groups: [g1, g2, g3])) + } + + // MARK: - depthOf + + @Test("nil groupId returns depth 0") + func depthOf_nilGroupId() { + #expect(depthOf(groupId: nil, groups: []) == 0) + } + + @Test("Top-level group returns depth 1") + func depthOf_topLevel() { + let gId = UUID() + let group = makeGroup(id: gId, name: "Top") + + #expect(depthOf(groupId: gId, groups: [group]) == 1) + } + + @Test("Nested group at depth 3 returns 3") + func depthOf_nestedDepth3() { + let id1 = UUID() + let id2 = UUID() + let id3 = UUID() + let g1 = makeGroup(id: id1, name: "L1") + let g2 = makeGroup(id: id2, name: "L2", parentId: id1) + let g3 = makeGroup(id: id3, name: "L3", parentId: id2) + + #expect(depthOf(groupId: id3, groups: [g1, g2, g3]) == 3) + } + + // MARK: - connectionCount + + @Test("Direct connections only returns correct count") + func connectionCount_directOnly() { + let gId = UUID() + let group = makeGroup(id: gId, name: "G") + let c1 = makeConnection(name: "C1", groupId: gId) + let c2 = makeConnection(name: "C2", groupId: gId) + let c3 = makeConnection(name: "C3") + + let count = connectionCount(in: gId, connections: [c1, c2, c3], groups: [group]) + #expect(count == 2) + } + + @Test("Nested groups include all descendant connections") + func connectionCount_withNestedGroups() { + let parentId = UUID() + let childId = UUID() + let parent = makeGroup(id: parentId, name: "Parent") + let child = makeGroup(id: childId, name: "Child", parentId: parentId) + let c1 = makeConnection(name: "In parent", groupId: parentId) + let c2 = makeConnection(name: "In child", groupId: childId) + let c3 = makeConnection(name: "Ungrouped") + + let count = connectionCount(in: parentId, connections: [c1, c2, c3], groups: [parent, child]) + #expect(count == 2) + } +} From b5ec940ba938aaa01216913cc53e8b23770fcaaf Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 28 Mar 2026 18:07:19 +0700 Subject: [PATCH 2/3] docs: update documentation for nested groups feature --- docs/customization/settings.mdx | 2 +- docs/databases/overview.mdx | 6 +++++- docs/features/connection-sharing.mdx | 7 +++++-- docs/features/icloud-sync.mdx | 2 +- docs/features/keyboard-shortcuts.mdx | 4 ++-- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/customization/settings.mdx b/docs/customization/settings.mdx index 33d79856..456d2284 100644 --- a/docs/customization/settings.mdx +++ b/docs/customization/settings.mdx @@ -457,7 +457,7 @@ Manage iCloud sync from the **Sync** tab in Settings. This is a Pro feature. | Setting | Default | Description | |---------|---------|-------------| | **Connections** | On | Sync connection details (not passwords) | -| **Groups & Tags** | On | Sync connection organization | +| **Groups & Tags** | On | Sync connection organization, including nested group hierarchy | | **App Settings** | On | Sync all 8 settings categories | | **Query History** | On | Sync query history | diff --git a/docs/databases/overview.mdx b/docs/databases/overview.mdx index 2baaec83..28045646 100644 --- a/docs/databases/overview.mdx +++ b/docs/databases/overview.mdx @@ -246,7 +246,11 @@ SSL/TLS is not available for SQLite connections (file-based, no network involved Colors tint the toolbar when you connect (red for production, green for development). Tags group connections in the sidebar. -Create connection groups by right-clicking in the connection list or using the folder icon. Groups collapse/expand and persist between sessions. +Create connection groups by right-clicking in the connection list or using the folder icon. Groups collapse/expand with native macOS disclosure triangles and persist between sessions. + +### Nested Groups + +Groups support up to 3 levels of nesting. Right-click a group to create a subgroup, move it under another group, or delete it. Deleting a parent removes all subgroups — connections inside are ungrouped, not deleted. The connection form shows the full hierarchy when picking a group. {/* Screenshot: Connection form showing color picker */} diff --git a/docs/features/connection-sharing.mdx b/docs/features/connection-sharing.mdx index 6e002df3..8b2fc08f 100644 --- a/docs/features/connection-sharing.mdx +++ b/docs/features/connection-sharing.mdx @@ -139,12 +139,15 @@ JSON. Required fields: `name`, `host`, `type`. "tagName": "production" } ], - "groups": [{ "name": "Backend", "color": "Blue" }], + "groups": [ + { "name": "Backend", "color": "Blue" }, + { "name": "Staging", "color": "Green", "parentGroupName": "Backend" } + ], "tags": [{ "name": "production", "color": "Red" }] } ``` -Groups and tags match by name. Missing ones are created. Paths use `~/` for portability. +Groups and tags match by name. Missing ones are created. Nested groups use `parentGroupName` to preserve hierarchy. Paths use `~/` for portability. ## Sharing vs iCloud Sync diff --git a/docs/features/icloud-sync.mdx b/docs/features/icloud-sync.mdx index 1d4d1283..e71530c5 100644 --- a/docs/features/icloud-sync.mdx +++ b/docs/features/icloud-sync.mdx @@ -13,7 +13,7 @@ TablePro syncs your connections, groups, settings, and query history across all |------|--------|-------| | **Connections** | Yes | Host, port, username, database type, SSH/SSL config | | **Passwords** | Optional | Opt-in via iCloud Keychain (end-to-end encrypted) | -| **Groups & Tags** | Yes | Full connection organization | +| **Groups & Tags** | Yes | Full connection organization, including nested group hierarchy (parent-child relationships and sort order) | | **App Settings** | Yes | All 8 categories (General, Appearance, Editor, Data Grid, History, Tabs, Keyboard, AI) | | **Query History** | Yes | Configurable limit: 100, 500, 1,000, or unlimited | diff --git a/docs/features/keyboard-shortcuts.mdx b/docs/features/keyboard-shortcuts.mdx index bd87ce2a..414c9e26 100644 --- a/docs/features/keyboard-shortcuts.mdx +++ b/docs/features/keyboard-shortcuts.mdx @@ -186,8 +186,8 @@ For keyboards without dedicated arrow keys (e.g., HHKB), Ctrl+HJKL works as arro |----------|--------|-------| | `Ctrl+J` / `Ctrl+N` | Move down / Next item | Data grid, connection list, quick switcher, database switcher | | `Ctrl+K` / `Ctrl+P` | Move up / Previous item | Data grid, connection list, quick switcher, database switcher | -| `Ctrl+H` | Move left / Collapse group | Data grid (column left), welcome panel (collapse group), onboarding (previous page) | -| `Ctrl+L` | Move right / Expand group | Data grid (column right), welcome panel (expand group), onboarding (next page) | +| `Ctrl+H` | Move left / Collapse group | Data grid (column left), welcome panel (collapse group at any nesting level), onboarding (previous page) | +| `Ctrl+L` | Move right / Expand group | Data grid (column right), welcome panel (expand group at any nesting level), onboarding (next page) | | `Ctrl+Shift+J` | Extend selection down | Data grid | | `Ctrl+Shift+K` | Extend selection up | Data grid | From 522fda3d6a96f499361254d0d5fb25035d6b63c1 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 28 Mar 2026 18:54:36 +0700 Subject: [PATCH 3/3] fix: address review issues - missing rebuildTree, onMove index safety, cycle tests --- TablePro/ViewModels/WelcomeViewModel.swift | 3 +++ .../Views/Connection/WelcomeWindowView.swift | 7 +++--- .../Models/ConnectionGroupTreeTests.swift | 24 +++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/TablePro/ViewModels/WelcomeViewModel.swift b/TablePro/ViewModels/WelcomeViewModel.swift index 61a3ec14..6738db9b 100644 --- a/TablePro/ViewModels/WelcomeViewModel.swift +++ b/TablePro/ViewModels/WelcomeViewModel.swift @@ -307,6 +307,7 @@ final class WelcomeViewModel { updated.name = newName groupStorage.updateGroup(updated) groups = groupStorage.loadGroups() + rebuildTree() renameGroupTarget = nil } @@ -315,6 +316,7 @@ final class WelcomeViewModel { updated.color = color groupStorage.updateGroup(updated) groups = groupStorage.loadGroups() + rebuildTree() } func moveConnections(_ targets: [DatabaseConnection], toGroup groupId: UUID) { @@ -350,6 +352,7 @@ final class WelcomeViewModel { updated.parentId = newParentId groupStorage.updateGroup(updated) groups = groupStorage.loadGroups() + rebuildTree() } // MARK: - Import / Export diff --git a/TablePro/Views/Connection/WelcomeWindowView.swift b/TablePro/Views/Connection/WelcomeWindowView.swift index 22dba8d3..81579f61 100644 --- a/TablePro/Views/Connection/WelcomeWindowView.swift +++ b/TablePro/Views/Connection/WelcomeWindowView.swift @@ -301,7 +301,8 @@ struct WelcomeWindowView: View { // MARK: - Tree Rendering private func treeRows(_ items: [ConnectionGroupTreeNode], parentGroupId: UUID? = nil) -> AnyView { - AnyView( + let allConnections = !items.contains { if case .group = $0 { return true } else { return false } } + return AnyView( ForEach(items) { item in switch item { case .connection(let conn): @@ -314,14 +315,14 @@ struct WelcomeWindowView: View { } } } - .onMove { from, to in + .onMove(perform: allConnections ? { from, to in guard vm.searchText.isEmpty else { return } if let parentGroupId, let group = vm.groups.first(where: { $0.id == parentGroupId }) { vm.moveGroupedConnections(in: group, from: from, to: to) } else { vm.moveUngroupedConnections(from: from, to: to) } - } + } : nil) ) } diff --git a/TableProTests/Models/ConnectionGroupTreeTests.swift b/TableProTests/Models/ConnectionGroupTreeTests.swift index ac5e20cd..b03cb6cc 100644 --- a/TableProTests/Models/ConnectionGroupTreeTests.swift +++ b/TableProTests/Models/ConnectionGroupTreeTests.swift @@ -474,4 +474,28 @@ struct ConnectionGroupTreeTests { let count = connectionCount(in: parentId, connections: [c1, c2, c3], groups: [parent, child]) #expect(count == 2) } + + // MARK: - Cycle Guard + + @Test("Cyclic parentId data does not cause infinite recursion in collectAllDescendantGroupIds") + func collectAllDescendantGroupIds_cyclicData() { + let idA = UUID() + let idB = UUID() + let a = ConnectionGroup(id: idA, name: "A", parentId: idB) + let b = ConnectionGroup(id: idB, name: "B", parentId: idA) + + let result = collectAllDescendantGroupIds(groupId: idA, groups: [a, b]) + #expect(result.count <= 2) + } + + @Test("Cyclic parentId data does not cause infinite recursion in depthOf") + func depthOf_cyclicData() { + let idA = UUID() + let idB = UUID() + let a = ConnectionGroup(id: idA, name: "A", parentId: idB) + let b = ConnectionGroup(id: idB, name: "B", parentId: idA) + + let depth = depthOf(groupId: idA, groups: [a, b]) + #expect(depth <= 2) + } }