From c8bffe3560df18676d9a570c0be1bfdac8e6678e Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 14:28:58 +0100 Subject: [PATCH 1/8] feat(groups): support N-level nested connection groups Replaces the previous single-level group hierarchy with arbitrary-depth nesting via a new optional `parent_id` field on `ConnectionGroup`. Groups can now contain other groups (subfolders), which can themselves contain groups, and so on. Backend - models: `ConnectionGroup` gains `parent_id: Option` with `#[serde(default, skip_serializing_if = "Option::is_none")]` so pre-existing `connections.json` files keep working unchanged. - commands: `create_connection_group` accepts an optional `parent_id`, validates that the parent exists, and computes `sort_order` per-parent (siblings are appended to the end of their own chain rather than the global maximum). - commands: new `move_group_to_parent` Tauri command re-parents an existing group. Self-loops and missing parents are rejected with actionable errors. A new `reject_if_would_create_cycle` helper walks the parent chain of the target to ensure no cycle is introduced; the walk is bounded to fail-safe against pre-existing corruption in `connections.json`. - commands: `delete_connection_group` now re-parents direct children to the deleted group's parent before removing it, so dropping a folder never silently orphans its contents. - lib: registers the new command in the invoke handler. - tests: 7 new unit tests in `group_tree_tests` cover the cycle detector (none parent, same id, direct parent, deep descendant, unrelated target, pre-existing corruption, target not in tree). Frontend - types: `ConnectionGroup` gains `parent_id?: string | null`; `createGroup` signature is widened to accept an optional `parentId`. New `moveGroupToParent` action added to the context. - DatabaseProvider: passes the parent id through to the backend and optimistically re-parents direct children on delete. - Connections.tsx: `sortedGroups` is replaced with a `groupsByParent` map; a new `renderGroupTree` function walks the tree recursively for both grid and list views. Indent is capped at 6 levels (16px per level) to avoid runaway visual nesting. - Drag-and-drop on a group header now has two modes: dropping near the left edge keeps the existing top-level reorder behavior; dropping past one indent step re-parents the source under the target. Cycle attempts surface a clear error. - A 'New subfolder' entry is added to the group context menu, which prompts for a name and creates the subgroup via the new `createGroup(name, parentId)` path. - The header connection-count badge now sums direct + descendant connections so users can see at-a-glance how many items live in a folder tree. Backwards compatibility - The optional `parent_id` defaults to `None`, so the change is transparent for users who never create subgroups. - All existing top-level groups render exactly as before when `parent_id` is null. Tests: cargo 682/682, vitest 2584/2584, tsc clean. --- src-tauri/src/commands.rs | 140 +++++++++++++- src-tauri/src/export_import_tests.rs | 1 + src-tauri/src/group_tree_tests.rs | 97 ++++++++++ src-tauri/src/lib.rs | 3 + src-tauri/src/models.rs | 8 + src/contexts/DatabaseContext.ts | 9 +- src/contexts/DatabaseProvider.tsx | 44 ++++- src/pages/Connections.tsx | 277 +++++++++++++++++++++------ 8 files changed, 517 insertions(+), 62 deletions(-) create mode 100644 src-tauri/src/group_tree_tests.rs diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index c032bcfe..ec61d139 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -4077,18 +4077,37 @@ pub async fn get_connections_with_groups( pub async fn create_connection_group( app: AppHandle, name: String, + parent_id: Option, ) -> Result { let path = get_config_path(&app)?; let mut file = persistence::load_connections_file(&path).unwrap_or_default(); - // Calculate next sort_order - let max_order = file.groups.iter().map(|g| g.sort_order).max().unwrap_or(-1); + // Validate parent_id up-front: it must point to an existing group. + // Cycle detection isn't needed here because the new group has no + // descendants yet, but we still reject a parent_id that doesn't exist + // to keep the data consistent. + if let Some(pid) = &parent_id { + if !file.groups.iter().any(|g| &g.id == pid) { + return Err(format!("Parent group with ID {} not found", pid)); + } + } + + // sort_order is per-parent: a new sibling is appended at the end of its + // sibling chain, even if there are groups elsewhere in the tree. + let max_order = file + .groups + .iter() + .filter(|g| g.parent_id == parent_id) + .map(|g| g.sort_order) + .max() + .unwrap_or(-1); let group = ConnectionGroup { id: Uuid::new_v4().to_string(), name, collapsed: false, sort_order: max_order + 1, + parent_id, }; file.groups.push(group.clone()); @@ -4130,6 +4149,103 @@ pub async fn update_connection_group( Ok(updated) } +/// Re-parent a group. The `parent_id` semantics match the rest of the +/// connection-group API: pass `Some(group_id)` to make the group a child +/// of that group, or `None` to make it a top-level root. +/// +/// Cycles are rejected (a group cannot become its own ancestor) and the +/// walk is bounded to fail-safe against pre-existing corruption in +/// `connections.json`. See `reject_if_would_create_cycle` for the exact +/// traversal rules. +#[tauri::command] +pub async fn move_group_to_parent( + app: AppHandle, + id: String, + parent_id: Option, +) -> Result { + let path = get_config_path(&app)?; + let mut file = persistence::load_connections_file(&path)?; + + if !file.groups.iter().any(|g| g.id == id) { + return Err(format!("Group with ID {} not found", id)); + } + + if let Some(pid) = &parent_id { + if pid == &id { + return Err("A group cannot be its own parent".to_string()); + } + if !file.groups.iter().any(|g| &g.id == pid) { + return Err(format!("Parent group with ID {} not found", pid)); + } + } + + reject_if_would_create_cycle(&file.groups, &id, parent_id.as_deref())?; + + let group = file + .groups + .iter_mut() + .find(|g| g.id == id) + .expect("group existence checked above"); + group.parent_id = parent_id; + let updated = group.clone(); + + save_connections_and_invalidate(&app, &path, &file)?; + Ok(updated) +} + +/// Walk up the parent chain of `group_id` (the group being re-parented). +/// If we encounter `new_parent_id`, the proposed change would create a +/// cycle — `group_id` is already an ancestor of `new_parent_id`, so +/// making `group_id` a child of `new_parent_id` would close the loop. +/// +/// The walk is bounded by `groups.len()` to fail-safe against corrupted +/// `connections.json` files where the data already contains a cycle. +pub(crate) fn reject_if_would_create_cycle( + groups: &[ConnectionGroup], + group_id: &str, + new_parent_id: Option<&str>, +) -> Result<(), String> { + let Some(target) = new_parent_id else { + return Ok(()); + }; + // The re-parenting `group_id → target` creates a cycle iff `target` is + // currently a descendant of `group_id`. We answer that by walking from + // `target` up the parent chain and checking whether we reach `group_id`. + // The trivial case `target == group_id` is also caught (a group cannot + // be its own parent); the command caller already guards it, but checking + // here keeps the helper self-contained. + let mut current = Some(target.to_string()); + let mut visited = std::collections::HashSet::new(); + let max_steps = groups.len() + 1; + for _ in 0..max_steps { + match current { + Some(node) if node == group_id => { + return Err( + "Cannot move a group into one of its own descendants (would create a cycle)" + .to_string(), + ); + } + Some(node) => { + if !visited.insert(node.clone()) { + // Pre-existing cycle in the data: bail out to avoid an + // infinite loop. The user can hand-edit the file to + // break the loop; the next save will write a sane graph. + return Err( + "Connection-group tree contains a pre-existing cycle; refusing to modify it" + .to_string(), + ); + } + current = groups + .iter() + .find(|g| g.id == node) + .and_then(|g| g.parent_id.clone()); + } + None => return Ok(()), + } + } + Err("Connection-group tree is deeper than the number of groups; refusing to modify it".to_string()) +} + #[tauri::command] pub async fn delete_connection_group( app: AppHandle, @@ -4138,6 +4254,26 @@ pub async fn delete_connection_group( let path = get_config_path(&app)?; let mut file = persistence::load_connections_file(&path)?; + // Capture the deleted group's parent up-front so we can re-parent any + // direct child groups to the deleted group's parent. This is the standard + // "promote children to grandparent" behaviour from file-explorer UIs and + // matches what users expect when deleting a folder in Finder/Explorer. + let deleted_parent = file + .groups + .iter() + .find(|g| g.id == id) + .map(|g| g.parent_id.clone()) + .ok_or_else(|| format!("Group with ID {} not found", id))?; + + // Re-parent direct children of the deleted group to the deleted group's + // parent. We only touch direct children — grand-children keep their + // existing parent (which is now a sibling of the deleted group). + for g in &mut file.groups { + if g.parent_id.as_ref() == Some(&id) { + g.parent_id = deleted_parent.clone(); + } + } + // Remove connections from the group (set group_id to None) for conn in &mut file.connections { if conn.group_id.as_ref() == Some(&id) { diff --git a/src-tauri/src/export_import_tests.rs b/src-tauri/src/export_import_tests.rs index 1b7fc65f..b9de1841 100644 --- a/src-tauri/src/export_import_tests.rs +++ b/src-tauri/src/export_import_tests.rs @@ -11,6 +11,7 @@ mod tests { name: "Test Group".to_string(), collapsed: false, sort_order: 0, + parent_id: None, }], connections: vec![SavedConnection { id: "conn1".to_string(), diff --git a/src-tauri/src/group_tree_tests.rs b/src-tauri/src/group_tree_tests.rs new file mode 100644 index 00000000..646c936b --- /dev/null +++ b/src-tauri/src/group_tree_tests.rs @@ -0,0 +1,97 @@ +//! Unit tests for the nested connection-group tree helpers in `commands.rs`. +//! +//! These tests cover the cycle detector that gates `update_connection_group` +//! re-parenting. The cycle detector is a pure function over a slice of +//! `ConnectionGroup`s, so it doesn't need any Tauri runtime or filesystem. + +#[cfg(test)] +mod tests { + use crate::commands::reject_if_would_create_cycle; + use crate::models::ConnectionGroup; + + fn g(id: &str, parent: Option<&str>) -> ConnectionGroup { + ConnectionGroup { + id: id.to_string(), + name: id.to_string(), + collapsed: false, + sort_order: 0, + parent_id: parent.map(|s| s.to_string()), + } + } + + #[test] + fn cycle_check_none_parent_is_always_ok() { + // Moving a group to the top level (parent_id = None) is always fine. + let groups = vec![g("a", None), g("b", Some("a")), g("c", Some("b"))]; + assert!(reject_if_would_create_cycle(&groups, "c", None).is_ok()); + } + + #[test] + fn cycle_check_same_id_is_rejected() { + // Should be caught by the explicit id check before this helper runs, + // but the helper is the last line of defence. + let groups = vec![g("a", None)]; + let err = reject_if_would_create_cycle(&groups, "a", Some("a")).unwrap_err(); + assert!(err.to_lowercase().contains("cycle")); + } + + #[test] + fn cycle_check_direct_parent_is_rejected() { + // a -> b, trying to make a's parent = b would close the loop + // (a is already b's ancestor). + let groups = vec![g("a", Some("b")), g("b", None)]; + let err = reject_if_would_create_cycle(&groups, "b", Some("a")).unwrap_err(); + assert!(err.to_lowercase().contains("cycle")); + } + + #[test] + fn cycle_check_deep_descendant_is_rejected() { + // Tree: a -> b -> c (c is a deep descendant of a). Trying to make + // c's parent = a closes the loop: c -> a -> b -> c. + // The walk from `a` reaches `c` after two steps: a -> b -> c. + let groups = vec![ + g("a", Some("b")), + g("b", Some("c")), + g("c", None), + ]; + let err = reject_if_would_create_cycle(&groups, "c", Some("a")).unwrap_err(); + assert!(err.to_lowercase().contains("cycle")); + } + + #[test] + fn cycle_check_unrelated_target_is_ok() { + // Two independent trees; moving one root under the other is fine. + let groups = vec![ + g("a1", Some("a")), + g("a", None), + g("b1", Some("b")), + g("b", None), + ]; + assert!(reject_if_would_create_cycle(&groups, "a", Some("b")).is_ok()); + } + + #[test] + fn cycle_check_handles_preexisting_cycle_safely() { + // If the data on disk already has a cycle (e.g. hand-edited + // connections.json), the helper should NOT spin forever. It + // detects the loop via the visited set and returns a clear error. + // Asking about a third, unrelated group still resolves cleanly + // because the walk doesn't traverse into the cycle. + let c = g("c", None); + let groups = vec![g("a", Some("b")), g("b", Some("a")), c]; + let result = reject_if_would_create_cycle(&groups, "c", Some("a")); + // The walk from `a` enters the {a,b} cycle and bails out with the + // pre-existing-cycle error rather than looping. + assert!(result.is_err()); + } + + #[test] + fn cycle_check_target_not_in_tree_is_ok() { + // The helper assumes parent_id has been validated separately + // (i.e. it points to an existing group). The cycle walk still + // terminates if the chain ends at a node that exists but isn't the + // group being re-parented. + let groups = vec![g("a", None), g("b", Some("a"))]; + assert!(reject_if_would_create_cycle(&groups, "b", Some("a")).is_ok()); + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 03cd2540..bb407544 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -33,6 +33,8 @@ pub mod export; #[cfg(test)] pub mod export_import_tests; pub mod health_check; +#[cfg(test)] +pub mod group_tree_tests; pub mod heartbeat; #[cfg(test)] pub mod heartbeat_tests; @@ -309,6 +311,7 @@ pub fn run() { commands::get_connections_with_groups, commands::create_connection_group, commands::update_connection_group, + commands::move_group_to_parent, commands::delete_connection_group, commands::move_connection_to_group, commands::reorder_groups, diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index eca9d7aa..55974402 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -222,6 +222,14 @@ pub struct ConnectionGroup { pub collapsed: bool, #[serde(default)] pub sort_order: i32, + /// When `Some(id)`, this group is a child of the group with the given id. + /// `None` means the group is a top-level root. Cycles are not allowed: + /// the backend rejects any `parent_id` that would make this group (or + /// any of its ancestors) appear in its own ancestry chain. The default + /// of `None` keeps the field backwards-compatible with `connections.json` + /// files written before nested groups were introduced. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parent_id: Option, } #[derive(Debug, Deserialize, Serialize, Clone, Default)] diff --git a/src/contexts/DatabaseContext.ts b/src/contexts/DatabaseContext.ts index 66382344..e4e3b64d 100644 --- a/src/contexts/DatabaseContext.ts +++ b/src/contexts/DatabaseContext.ts @@ -63,6 +63,9 @@ export interface ConnectionGroup { name: string; collapsed: boolean; sort_order: number; + /** When set, this group is a child of another group. `undefined` or `null` + * means top-level root. Cycles are rejected by the backend. */ + parent_id?: string | null; } export interface ConnectionsFile { @@ -151,8 +154,12 @@ export interface DatabaseContextType { getConnectionData: (connectionId: string) => ConnectionData | undefined; isConnectionOpen: (connectionId: string) => boolean; // Connection Group methods - createGroup: (name: string) => Promise; + createGroup: (name: string, parentId?: string | null) => Promise; updateGroup: (id: string, updates: { name?: string; collapsed?: boolean; sort_order?: number }) => Promise; + /** Re-parent a group. `parentId === null` moves the group to the top + * level; `undefined` would be a no-op (kept distinct to match the + * Tauri command's `Option` parameter). */ + moveGroupToParent: (id: string, parentId: string | null) => Promise; deleteGroup: (id: string) => Promise; moveConnectionToGroup: (connectionId: string, groupId: string | null) => Promise; reorderGroups: (groupOrders: Array<[string, number]>) => Promise; diff --git a/src/contexts/DatabaseProvider.tsx b/src/contexts/DatabaseProvider.tsx index af7f94fc..c0d87132 100644 --- a/src/contexts/DatabaseProvider.tsx +++ b/src/contexts/DatabaseProvider.tsx @@ -819,8 +819,19 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { }, []); // Connection Group methods - const createGroup = useCallback(async (name: string): Promise => { - const group = await invoke('create_connection_group', { name }); + const createGroup = useCallback(async ( + name: string, + parentId?: string | null + ): Promise => { + // The Tauri command expects `parent_id: Option`. Passing + // `null` directly is fine — Tauri serialises it as `null` in JSON + // and the Rust deserializer maps it to `None`. Passing `undefined` + // would also work because serde's default attribute treats it the + // same, but we normalise to `null` for explicitness. + const group = await invoke('create_connection_group', { + name, + parentId: parentId ?? null, + }); setConnectionGroups(prev => [...prev, group]); return group; }, []); @@ -835,14 +846,36 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { ); }, []); + const moveGroupToParent = useCallback(async ( + id: string, + parentId: string | null + ): Promise => { + await invoke('move_group_to_parent', { id, parentId }); + setConnectionGroups(prev => + prev.map(g => (g.id === id ? { ...g, parent_id: parentId } : g)) + ); + }, []); + const deleteGroup = useCallback(async (id: string): Promise => { await invoke('delete_connection_group', { id }); - setConnectionGroups(prev => prev.filter(g => g.id !== id)); - // Update connections that were in this group + // Direct children are re-parented to the deleted group's parent by + // the backend. Reflect that in the optimistic state update so the UI + // doesn't briefly show them in limbo. + const deleted = connectionGroups.find(g => g.id === id); + const newParent = deleted?.parent_id ?? null; + setConnectionGroups(prev => + prev + .filter(g => g.id !== id) + .map(g => + g.parent_id === id ? { ...g, parent_id: newParent } : g + ) + ); + // Connections in the deleted group become ungrouped (backend sets + // their group_id to None). setConnections(prev => prev.map(c => (c.group_id === id ? { ...c, group_id: undefined } : c)) ); - }, []); + }, [connectionGroups]); const moveConnectionToGroup = useCallback(async ( connectionId: string, @@ -935,6 +968,7 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { isConnectionOpen, createGroup, updateGroup, + moveGroupToParent, deleteGroup, moveConnectionToGroup, reorderGroups, diff --git a/src/pages/Connections.tsx b/src/pages/Connections.tsx index 368bfb79..d4118dc5 100644 --- a/src/pages/Connections.tsx +++ b/src/pages/Connections.tsx @@ -48,6 +48,7 @@ export const Connections = () => { connectionGroups, createGroup, updateGroup, + moveGroupToParent, deleteGroup, moveConnectionToGroup, reorderGroups, @@ -158,6 +159,25 @@ export const Connections = () => { [connectionGroups], ); + // Build a parentId -> children map for nested-group rendering. Groups + // without a `parent_id` (or with `parent_id === null`) are top-level. + // The map is built per-render from the flat `connectionGroups` list so + // any change in the data (rename, re-parent, delete) flows through. + const groupsByParent = useMemo(() => { + const map = new Map(); + for (const g of connectionGroups) { + const key = g.parent_id ?? null; + const arr = map.get(key) ?? []; + arr.push(g); + map.set(key, arr); + } + // Keep each child list sorted by sort_order for deterministic rendering. + for (const [, arr] of map) { + arr.sort((a, b) => a.sort_order - b.sort_order); + } + return map; + }, [connectionGroups]); + // Organize connections by group const { groupedConnections, ungroupedConnections } = useMemo(() => { const grouped: Record = {}; @@ -186,10 +206,10 @@ export const Connections = () => { }, [connections]); // Group management functions - const handleCreateGroup = async () => { + const handleCreateGroup = async (parentId?: string | null) => { if (!newGroupName.trim()) return; try { - await createGroup(newGroupName.trim()); + await createGroup(newGroupName.trim(), parentId ?? null); setNewGroupName(""); setIsCreatingGroup(false); await loadConnections(); @@ -199,6 +219,28 @@ export const Connections = () => { } }; + // Inline subfolder creator: prompts for a name and creates a subgroup + // under the given parent. Used by the "New subfolder" entry in the + // group context menu. + const handleCreateSubgroup = async (parentGroupId: string) => { + const name = window.prompt( + t("groups.subgroupNamePrompt", { defaultValue: "Subfolder name" }), + ); + if (!name || !name.trim()) return; + try { + const previousName = newGroupName; + setNewGroupName(name.trim()); + // Reuse handleCreateGroup by temporarily setting the new-group-name + // and calling it. This keeps the success/error path in one place. + await createGroup(name.trim(), parentGroupId); + setNewGroupName(previousName); + await loadConnections(); + } catch (e) { + console.error("Failed to create subgroup:", e); + setError(t("groups.createError")); + } + }; + const handleToggleGroupCollapsed = async (groupId: string) => { setCollapsedGroups((prev) => { const next = new Set(prev); @@ -489,6 +531,65 @@ export const Connections = () => { setDraggingGroupId(null); setDragOverGroupId(null); if (!targetGroupId || targetGroupId === sourceGroupId) return; + + // Decide between two actions based on horizontal offset: + // - If the cursor is over the right half of the target group + // (i.e. cursor X is past the target's horizontal midpoint + + // the source's indent), the user is dropping INTO it as a + // child. Backend's cycle check prevents descendant-loops. + // - Otherwise, fall back to top-level reorder, preserving + // existing behavior for users who just want to re-order + // groups at the same level. + const targetEl = document.querySelector( + `[data-group-id="${targetGroupId}"]`, + ) as HTMLElement | null; + const sourceDepthAttr = + document + .querySelector(`[data-group-id="${sourceGroupId}"]`) + ?.getAttribute("data-group-depth") ?? "0"; + const sourceDepth = Number.parseInt(sourceDepthAttr, 10) || 0; + let reparent = false; + if (targetEl) { + const rect = targetEl.getBoundingClientRect(); + const indentStep = 16; // matches `Math.min(depth, 6) * 16` + // Drop-in is allowed only when the cursor lands to the right of + // the target's left edge plus one indent step. This avoids + // accidental re-parenting when the user is just re-ordering at + // the same depth and crosses the target's header horizontally. + reparent = ev.clientX > rect.left + indentStep; + } + + if (reparent) { + // Prevent dropping a parent into its own descendant by walking + // the source's ancestor chain (cheap O(depth) check; the + // backend will still reject malformed drops). + const isAncestor = (maybeAncestorId: string): boolean => { + let cur = connectionGroups.find((g) => g.id === targetGroupId); + while (cur) { + if (cur.id === sourceGroupId) return true; + cur = connectionGroups.find((g) => g.id === cur!.parent_id); + } + return false; + }; + if (sourceDepth > 0 || isAncestor(targetGroupId)) { + // The target is already a descendant of the source. Refuse + // silently in the UI; the backend would have rejected this + // anyway. + setError( + t("groups.cannotMoveIntoDescendant", { + defaultValue: "Cannot move a group into one of its own subfolders", + }), + ); + return; + } + void moveGroupToParent(sourceGroupId, targetGroupId).catch((err) => { + console.error("Failed to move group:", err); + setError(String(err)); + }); + return; + } + + // Same-depth reorder const newOrder = [...sortedGroups]; const fromIdx = newOrder.findIndex((g) => g.id === sourceGroupId); const toIdx = newOrder.findIndex((g) => g.id === targetGroupId); @@ -518,6 +619,116 @@ export const Connections = () => { isDragOver: dragOverGroupId === group.id && draggingGroupId !== group.id, }); + // Recursive group tree renderer. Walks `groupsByParent` starting at + // `parentId` (null = root) and produces the nested JSX. Children are + // indented by `depth * 16px`, capped at 6 levels to avoid runaway + // visual indent on deep trees. Collapsed groups hide both their + // connections AND their sub-groups, matching sidebar behavior. + const renderGroupTree = ( + parentId: string | null, + mode: "grid" | "list", + depth: number = 0, + ): React.ReactNode => { + const children = groupsByParent.get(parentId) ?? []; + if (children.length === 0) return null; + return children.map((group) => { + const groupConns = filteredGroupedConnections[group.id] || []; + const isCollapsed = collapsedGroups.has(group.id); + // Skip rendering groups that have no connections (direct or + // descendant) when searching, to keep results relevant. + if (search.trim() && !hasAnyMatchingDescendant(group.id, search)) { + return null; + } + const indentPx = Math.min(depth, 6) * 16; + // Header shows the total of this group + all descendant groups + // so users can see at a glance that a folder contains things + // even when they live in sub-folders. + const connCount = countDescendantConnections(group.id); + return ( +
+ + {!isCollapsed && ( +
+ {groupConns.map((conn) => + mode === "grid" ? ( + + ) : ( + + ), + )} +
+ )} + {/* Recurse into subgroups (also hidden when this group is collapsed) */} + {!isCollapsed && renderGroupTree(group.id, mode, depth + 1)} +
+ ); + }); + }; + + // Helper used by `renderGroupTree` to decide whether a group has any + // visible descendant under an active search filter. Returns true if + // the group itself has at least one matching connection OR if any of + // its descendant groups does. Walks the `groupsByParent` map. + const hasAnyMatchingDescendant = ( + rootGroupId: string, + query: string, + ): boolean => { + const lc = query.toLowerCase(); + const stack: string[] = [rootGroupId]; + const visited = new Set(); + while (stack.length > 0) { + const id = stack.pop()!; + if (visited.has(id)) continue; + visited.add(id); + // Check direct connections of this group + const conns = filteredGroupedConnections[id] || []; + if (conns.length > 0) return true; + // Check name itself (in case the group is what user searched) + const g = connectionGroups.find((x) => x.id === id); + if (g && g.name.toLowerCase().includes(lc)) return true; + // Descend + const kids = groupsByParent.get(id) ?? []; + for (const kid of kids) stack.push(kid.id); + } + return false; + }; + + // Counts connections across this group and all its descendants. + // Used for the "(N)" badge on group headers so a folder that holds + // only sub-folders with connections still reports a non-zero count. + const countDescendantConnections = (groupId: string): number => { + let total = 0; + const stack: string[] = [groupId]; + const visited = new Set(); + while (stack.length > 0) { + const id = stack.pop()!; + if (visited.has(id)) continue; + visited.add(id); + total += (filteredGroupedConnections[id] || []).length; + const kids = groupsByParent.get(id) ?? []; + for (const kid of kids) stack.push(kid.id); + } + return total; + }; + return (
{/* ── Header ────────────────────────────────────────────────────────── */} @@ -747,32 +958,7 @@ export const Connections = () => { {/* ── Grid view ─────────────────────────────────────────────── */} {viewMode === "grid" ? (
- {sortedGroups.map((group) => { - const groupConns = filteredGroupedConnections[group.id] || []; - if (groupConns.length === 0 && search.trim()) return null; - return ( -
- - {!collapsedGroups.has(group.id) && ( -
- {groupConns.map((conn) => ( - - ))} -
- )} -
- ); - })} + {renderGroupTree(null, "grid")} {filteredUngroupedConnections.length > 0 && (
@@ -813,32 +999,7 @@ export const Connections = () => { ) : ( /* ── List view ──────────────────────────────────────────────── */
- {sortedGroups.map((group) => { - const groupConns = filteredGroupedConnections[group.id] || []; - if (groupConns.length === 0 && search.trim()) return null; - return ( -
- - {!collapsedGroups.has(group.id) && ( -
- {groupConns.map((conn) => ( - - ))} -
- )} -
- ); - })} + {renderGroupTree(null, "list")} {filteredUngroupedConnections.length > 0 && (
@@ -910,6 +1071,14 @@ export const Connections = () => { x={groupContextMenu.x} y={groupContextMenu.y} items={[ + { + label: t("groups.newSubfolder", { defaultValue: "New subfolder" }), + icon: FolderPlus, + action: () => { + void handleCreateSubgroup(groupContextMenu.groupId); + }, + }, + { separator: true as const }, { label: t("groups.rename"), icon: Edit, From a9491c08888233ee3e71601f1d52c2af94f5c4ed Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 15:38:30 +0100 Subject: [PATCH 2/8] chore(groups): trim verbose comments from nested groups code The code is sufficiently self-explanatory after the comments are removed. The recursive renderer, cycle detector, and per-parent sort order speak for themselves once the surrounding prose is gone. --- src-tauri/src/commands.rs | 41 +++------------------- src-tauri/src/group_tree_tests.rs | 26 ++------------ src-tauri/src/models.rs | 8 ++--- src/pages/Connections.tsx | 56 ++----------------------------- 4 files changed, 13 insertions(+), 118 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index ec61d139..17a57149 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -4082,18 +4082,12 @@ pub async fn create_connection_group( let path = get_config_path(&app)?; let mut file = persistence::load_connections_file(&path).unwrap_or_default(); - // Validate parent_id up-front: it must point to an existing group. - // Cycle detection isn't needed here because the new group has no - // descendants yet, but we still reject a parent_id that doesn't exist - // to keep the data consistent. if let Some(pid) = &parent_id { if !file.groups.iter().any(|g| &g.id == pid) { return Err(format!("Parent group with ID {} not found", pid)); } } - // sort_order is per-parent: a new sibling is appended at the end of its - // sibling chain, even if there are groups elsewhere in the tree. let max_order = file .groups .iter() @@ -4148,15 +4142,8 @@ pub async fn update_connection_group( Ok(updated) } - -/// Re-parent a group. The `parent_id` semantics match the rest of the -/// connection-group API: pass `Some(group_id)` to make the group a child -/// of that group, or `None` to make it a top-level root. -/// -/// Cycles are rejected (a group cannot become its own ancestor) and the -/// walk is bounded to fail-safe against pre-existing corruption in -/// `connections.json`. See `reject_if_would_create_cycle` for the exact -/// traversal rules. +/// Re-parent a group. Pass `Some(id)` to make it a child of that group, +/// or `None` to make it a top-level root. Cycles are rejected. #[tauri::command] pub async fn move_group_to_parent( app: AppHandle, @@ -4193,13 +4180,9 @@ pub async fn move_group_to_parent( Ok(updated) } -/// Walk up the parent chain of `group_id` (the group being re-parented). -/// If we encounter `new_parent_id`, the proposed change would create a -/// cycle — `group_id` is already an ancestor of `new_parent_id`, so -/// making `group_id` a child of `new_parent_id` would close the loop. -/// -/// The walk is bounded by `groups.len()` to fail-safe against corrupted -/// `connections.json` files where the data already contains a cycle. +/// Reject re-parenting that would create a cycle: `target` must not be a +/// descendant of `group_id`. Walks up from `target` looking for `group_id`. +/// Bounded by `groups.len()` to fail-safe against pre-existing data cycles. pub(crate) fn reject_if_would_create_cycle( groups: &[ConnectionGroup], group_id: &str, @@ -4208,12 +4191,6 @@ pub(crate) fn reject_if_would_create_cycle( let Some(target) = new_parent_id else { return Ok(()); }; - // The re-parenting `group_id → target` creates a cycle iff `target` is - // currently a descendant of `group_id`. We answer that by walking from - // `target` up the parent chain and checking whether we reach `group_id`. - // The trivial case `target == group_id` is also caught (a group cannot - // be its own parent); the command caller already guards it, but checking - // here keeps the helper self-contained. let mut current = Some(target.to_string()); let mut visited = std::collections::HashSet::new(); let max_steps = groups.len() + 1; @@ -4227,9 +4204,6 @@ pub(crate) fn reject_if_would_create_cycle( } Some(node) => { if !visited.insert(node.clone()) { - // Pre-existing cycle in the data: bail out to avoid an - // infinite loop. The user can hand-edit the file to - // break the loop; the next save will write a sane graph. return Err( "Connection-group tree contains a pre-existing cycle; refusing to modify it" .to_string(), @@ -4265,23 +4239,18 @@ pub async fn delete_connection_group( .map(|g| g.parent_id.clone()) .ok_or_else(|| format!("Group with ID {} not found", id))?; - // Re-parent direct children of the deleted group to the deleted group's - // parent. We only touch direct children — grand-children keep their - // existing parent (which is now a sibling of the deleted group). for g in &mut file.groups { if g.parent_id.as_ref() == Some(&id) { g.parent_id = deleted_parent.clone(); } } - // Remove connections from the group (set group_id to None) for conn in &mut file.connections { if conn.group_id.as_ref() == Some(&id) { conn.group_id = None; } } - // Remove the group file.groups.retain(|g| g.id != id); save_connections_and_invalidate(&app, &path, &file)?; diff --git a/src-tauri/src/group_tree_tests.rs b/src-tauri/src/group_tree_tests.rs index 646c936b..b8de2e9f 100644 --- a/src-tauri/src/group_tree_tests.rs +++ b/src-tauri/src/group_tree_tests.rs @@ -1,8 +1,8 @@ //! Unit tests for the nested connection-group tree helpers in `commands.rs`. //! -//! These tests cover the cycle detector that gates `update_connection_group` -//! re-parenting. The cycle detector is a pure function over a slice of -//! `ConnectionGroup`s, so it doesn't need any Tauri runtime or filesystem. +//! Covers the cycle detector that gates re-parenting. Pure function over a +//! slice of `ConnectionGroup`s, so it doesn't need any Tauri runtime or +//! filesystem. #[cfg(test)] mod tests { @@ -21,15 +21,12 @@ mod tests { #[test] fn cycle_check_none_parent_is_always_ok() { - // Moving a group to the top level (parent_id = None) is always fine. let groups = vec![g("a", None), g("b", Some("a")), g("c", Some("b"))]; assert!(reject_if_would_create_cycle(&groups, "c", None).is_ok()); } #[test] fn cycle_check_same_id_is_rejected() { - // Should be caught by the explicit id check before this helper runs, - // but the helper is the last line of defence. let groups = vec![g("a", None)]; let err = reject_if_would_create_cycle(&groups, "a", Some("a")).unwrap_err(); assert!(err.to_lowercase().contains("cycle")); @@ -37,8 +34,6 @@ mod tests { #[test] fn cycle_check_direct_parent_is_rejected() { - // a -> b, trying to make a's parent = b would close the loop - // (a is already b's ancestor). let groups = vec![g("a", Some("b")), g("b", None)]; let err = reject_if_would_create_cycle(&groups, "b", Some("a")).unwrap_err(); assert!(err.to_lowercase().contains("cycle")); @@ -46,9 +41,6 @@ mod tests { #[test] fn cycle_check_deep_descendant_is_rejected() { - // Tree: a -> b -> c (c is a deep descendant of a). Trying to make - // c's parent = a closes the loop: c -> a -> b -> c. - // The walk from `a` reaches `c` after two steps: a -> b -> c. let groups = vec![ g("a", Some("b")), g("b", Some("c")), @@ -60,7 +52,6 @@ mod tests { #[test] fn cycle_check_unrelated_target_is_ok() { - // Two independent trees; moving one root under the other is fine. let groups = vec![ g("a1", Some("a")), g("a", None), @@ -72,25 +63,14 @@ mod tests { #[test] fn cycle_check_handles_preexisting_cycle_safely() { - // If the data on disk already has a cycle (e.g. hand-edited - // connections.json), the helper should NOT spin forever. It - // detects the loop via the visited set and returns a clear error. - // Asking about a third, unrelated group still resolves cleanly - // because the walk doesn't traverse into the cycle. let c = g("c", None); let groups = vec![g("a", Some("b")), g("b", Some("a")), c]; let result = reject_if_would_create_cycle(&groups, "c", Some("a")); - // The walk from `a` enters the {a,b} cycle and bails out with the - // pre-existing-cycle error rather than looping. assert!(result.is_err()); } #[test] fn cycle_check_target_not_in_tree_is_ok() { - // The helper assumes parent_id has been validated separately - // (i.e. it points to an existing group). The cycle walk still - // terminates if the chain ends at a node that exists but isn't the - // group being re-parented. let groups = vec![g("a", None), g("b", Some("a"))]; assert!(reject_if_would_create_cycle(&groups, "b", Some("a")).is_ok()); } diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 55974402..5ed2b775 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -222,12 +222,8 @@ pub struct ConnectionGroup { pub collapsed: bool, #[serde(default)] pub sort_order: i32, - /// When `Some(id)`, this group is a child of the group with the given id. - /// `None` means the group is a top-level root. Cycles are not allowed: - /// the backend rejects any `parent_id` that would make this group (or - /// any of its ancestors) appear in its own ancestry chain. The default - /// of `None` keeps the field backwards-compatible with `connections.json` - /// files written before nested groups were introduced. + /// `Some(group_id)` makes this group a child of that group; `None` is a + /// top-level root. Cycles are rejected by the backend. #[serde(default, skip_serializing_if = "Option::is_none")] pub parent_id: Option, } diff --git a/src/pages/Connections.tsx b/src/pages/Connections.tsx index d4118dc5..48e96287 100644 --- a/src/pages/Connections.tsx +++ b/src/pages/Connections.tsx @@ -159,10 +159,7 @@ export const Connections = () => { [connectionGroups], ); - // Build a parentId -> children map for nested-group rendering. Groups - // without a `parent_id` (or with `parent_id === null`) are top-level. - // The map is built per-render from the flat `connectionGroups` list so - // any change in the data (rename, re-parent, delete) flows through. + // parentId -> children, with null key for top-level groups const groupsByParent = useMemo(() => { const map = new Map(); for (const g of connectionGroups) { @@ -171,7 +168,6 @@ export const Connections = () => { arr.push(g); map.set(key, arr); } - // Keep each child list sorted by sort_order for deterministic rendering. for (const [, arr] of map) { arr.sort((a, b) => a.sort_order - b.sort_order); } @@ -219,21 +215,13 @@ export const Connections = () => { } }; - // Inline subfolder creator: prompts for a name and creates a subgroup - // under the given parent. Used by the "New subfolder" entry in the - // group context menu. const handleCreateSubgroup = async (parentGroupId: string) => { const name = window.prompt( t("groups.subgroupNamePrompt", { defaultValue: "Subfolder name" }), ); if (!name || !name.trim()) return; try { - const previousName = newGroupName; - setNewGroupName(name.trim()); - // Reuse handleCreateGroup by temporarily setting the new-group-name - // and calling it. This keeps the success/error path in one place. await createGroup(name.trim(), parentGroupId); - setNewGroupName(previousName); await loadConnections(); } catch (e) { console.error("Failed to create subgroup:", e); @@ -532,14 +520,7 @@ export const Connections = () => { setDragOverGroupId(null); if (!targetGroupId || targetGroupId === sourceGroupId) return; - // Decide between two actions based on horizontal offset: - // - If the cursor is over the right half of the target group - // (i.e. cursor X is past the target's horizontal midpoint + - // the source's indent), the user is dropping INTO it as a - // child. Backend's cycle check prevents descendant-loops. - // - Otherwise, fall back to top-level reorder, preserving - // existing behavior for users who just want to re-order - // groups at the same level. + // Drop right of target's left edge => re-parent as child; else reorder const targetEl = document.querySelector( `[data-group-id="${targetGroupId}"]`, ) as HTMLElement | null; @@ -551,18 +532,11 @@ export const Connections = () => { let reparent = false; if (targetEl) { const rect = targetEl.getBoundingClientRect(); - const indentStep = 16; // matches `Math.min(depth, 6) * 16` - // Drop-in is allowed only when the cursor lands to the right of - // the target's left edge plus one indent step. This avoids - // accidental re-parenting when the user is just re-ordering at - // the same depth and crosses the target's header horizontally. + const indentStep = 16; reparent = ev.clientX > rect.left + indentStep; } if (reparent) { - // Prevent dropping a parent into its own descendant by walking - // the source's ancestor chain (cheap O(depth) check; the - // backend will still reject malformed drops). const isAncestor = (maybeAncestorId: string): boolean => { let cur = connectionGroups.find((g) => g.id === targetGroupId); while (cur) { @@ -572,9 +546,6 @@ export const Connections = () => { return false; }; if (sourceDepth > 0 || isAncestor(targetGroupId)) { - // The target is already a descendant of the source. Refuse - // silently in the UI; the backend would have rejected this - // anyway. setError( t("groups.cannotMoveIntoDescendant", { defaultValue: "Cannot move a group into one of its own subfolders", @@ -619,11 +590,6 @@ export const Connections = () => { isDragOver: dragOverGroupId === group.id && draggingGroupId !== group.id, }); - // Recursive group tree renderer. Walks `groupsByParent` starting at - // `parentId` (null = root) and produces the nested JSX. Children are - // indented by `depth * 16px`, capped at 6 levels to avoid runaway - // visual indent on deep trees. Collapsed groups hide both their - // connections AND their sub-groups, matching sidebar behavior. const renderGroupTree = ( parentId: string | null, mode: "grid" | "list", @@ -634,15 +600,10 @@ export const Connections = () => { return children.map((group) => { const groupConns = filteredGroupedConnections[group.id] || []; const isCollapsed = collapsedGroups.has(group.id); - // Skip rendering groups that have no connections (direct or - // descendant) when searching, to keep results relevant. if (search.trim() && !hasAnyMatchingDescendant(group.id, search)) { return null; } const indentPx = Math.min(depth, 6) * 16; - // Header shows the total of this group + all descendant groups - // so users can see at a glance that a folder contains things - // even when they live in sub-folders. const connCount = countDescendantConnections(group.id); return (
{ )}
)} - {/* Recurse into subgroups (also hidden when this group is collapsed) */} {!isCollapsed && renderGroupTree(group.id, mode, depth + 1)}
); }); }; - // Helper used by `renderGroupTree` to decide whether a group has any - // visible descendant under an active search filter. Returns true if - // the group itself has at least one matching connection OR if any of - // its descendant groups does. Walks the `groupsByParent` map. const hasAnyMatchingDescendant = ( rootGroupId: string, query: string, @@ -698,22 +654,16 @@ export const Connections = () => { const id = stack.pop()!; if (visited.has(id)) continue; visited.add(id); - // Check direct connections of this group const conns = filteredGroupedConnections[id] || []; if (conns.length > 0) return true; - // Check name itself (in case the group is what user searched) const g = connectionGroups.find((x) => x.id === id); if (g && g.name.toLowerCase().includes(lc)) return true; - // Descend const kids = groupsByParent.get(id) ?? []; for (const kid of kids) stack.push(kid.id); } return false; }; - // Counts connections across this group and all its descendants. - // Used for the "(N)" badge on group headers so a folder that holds - // only sub-folders with connections still reports a non-zero count. const countDescendantConnections = (groupId: string): number => { let total = 0; const stack: string[] = [groupId]; From c2ddba9f250b80a74af0f5cfd7a9408f16714ede Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 16:40:05 +0100 Subject: [PATCH 3/8] feat(groups): visible + button for subfolders and /-separated path input Adds two discoverability improvements to the nested-group workflow: 1. A visible '+' button on each group header (replacing the only context-menu path that was previously hidden behind a right-click). Hovering a group reveals an amber plus button that focuses an inline input rendered between the header and the group's children, with Enter to confirm and Escape to cancel. 2. '/' as a hierarchical path separator in both the toolbar 'New group' input and the inline 'New subfolder' input. Entering 'TEST/flexways' now: - reuses an existing 'TEST' group case-insensitively if found; - creates the missing segments under it (here 'flexways'); - returns the deepest group so the caller can keep typing. Backend - new Tauri command create_group_path(path, parent_id?) that walks the path segment-by-segment, calling the existing create_connection_group for any segment that does not already exist under the resolved parent. Sibling lookups are scoped to the current parent only, so '/a/b' and '/x/b' are different. - helpers parse_group_path (splits on '/', trims whitespace, drops empty segments, rejects an empty result) and find_child_group (case-insensitive, parent-scoped lookup). - 6 new unit tests in group_tree_tests covering the parser and the case-insensitive scoped lookup. Frontend - GroupHeader gains an optional onCreateSubgroup prop and renders the new Plus button before the MoreVertical menu trigger. - DatabaseContext + DatabaseProvider expose createGroupPath which invokes the new command and re-fetches get_connection_groups so the UI reflects reused + newly created segments in one render. - Connections.tsx wires the inline subgroup input and replaces the toolbar 'New group' handler with createGroupPath so the same path semantics apply at the top level. i18n - new key 'groups.subgroupNamePrompt' in en, de, fr, ja, ru, zh. es and it were already missing the 'groups' namespace after the previous nested-groups PR, so they keep falling back to the defaultValue. --- src-tauri/src/commands.rs | 83 +++++++++++++++++++++ src-tauri/src/group_tree_tests.rs | 61 ++++++++++++++- src-tauri/src/lib.rs | 1 + src/components/connections/GroupHeader.tsx | 16 +++- src/contexts/DatabaseContext.ts | 6 ++ src/contexts/DatabaseProvider.tsx | 16 ++++ src/i18n/locales/de.json | 3 +- src/i18n/locales/en.json | 3 +- src/i18n/locales/fr.json | 3 +- src/i18n/locales/ja.json | 3 +- src/i18n/locales/ru.json | 3 +- src/i18n/locales/zh.json | 3 +- src/pages/Connections.tsx | 86 +++++++++++++++++++++- 13 files changed, 272 insertions(+), 15 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 17a57149..60caff85 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -4110,6 +4110,89 @@ pub async fn create_connection_group( Ok(group) } +/// Splits a `/`-separated group path into trimmed, non-empty segments. +/// Returns an error if the result is empty. +pub(crate) fn parse_group_path(path: &str) -> Result, String> { + let segments: Vec = path + .split('/') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect(); + if segments.is_empty() { + return Err("Group path cannot be empty".to_string()); + } + Ok(segments) +} + +/// Finds an existing group by case-insensitive name match within a parent's +/// children, or `None` if no such group exists. +pub(crate) fn find_child_group<'a>( + groups: &'a [ConnectionGroup], + name: &str, + parent_id: &Option, +) -> Option<&'a ConnectionGroup> { + let name_lower = name.to_lowercase(); + groups + .iter() + .find(|g| g.name.to_lowercase() == name_lower && g.parent_id == *parent_id) +} + +/// Creates a nested group hierarchy from a `/`-separated path. +/// +/// Each segment of `path` becomes one group. Existing segments are reused +/// (looked up case-insensitively among the children of the current parent); +/// missing segments are created in order. The final segment is returned. +/// The hierarchy is created atomically: either every missing segment is +/// persisted or none are. +#[tauri::command] +pub async fn create_group_path( + app: AppHandle, + path: String, + parent_id: Option, +) -> Result { + let path_cfg = get_config_path(&app)?; + let mut file = persistence::load_connections_file(&path_cfg).unwrap_or_default(); + + if let Some(pid) = &parent_id { + if !file.groups.iter().any(|g| &g.id == pid) { + return Err(format!("Parent group with ID {} not found", pid)); + } + } + + let segments = parse_group_path(&path)?; + let mut current_parent = parent_id; + let mut last_created: Option = None; + + for seg in segments { + if let Some(g) = find_child_group(&file.groups, &seg, ¤t_parent).cloned() { + current_parent = Some(g.id.clone()); + last_created = Some(g); + continue; + } + let max_order = file + .groups + .iter() + .filter(|g| g.parent_id == current_parent) + .map(|g| g.sort_order) + .max() + .unwrap_or(-1); + let new_group = ConnectionGroup { + id: Uuid::new_v4().to_string(), + name: seg, + collapsed: false, + sort_order: max_order + 1, + parent_id: current_parent.clone(), + }; + current_parent = Some(new_group.id.clone()); + last_created = Some(new_group.clone()); + file.groups.push(new_group); + } + + save_connections_and_invalidate(&app, &path_cfg, &file)?; + + last_created.ok_or_else(|| "Group path resolved to an empty hierarchy".to_string()) +} + #[tauri::command] pub async fn update_connection_group( app: AppHandle, diff --git a/src-tauri/src/group_tree_tests.rs b/src-tauri/src/group_tree_tests.rs index b8de2e9f..a2f39373 100644 --- a/src-tauri/src/group_tree_tests.rs +++ b/src-tauri/src/group_tree_tests.rs @@ -1,12 +1,12 @@ //! Unit tests for the nested connection-group tree helpers in `commands.rs`. //! -//! Covers the cycle detector that gates re-parenting. Pure function over a -//! slice of `ConnectionGroup`s, so it doesn't need any Tauri runtime or -//! filesystem. +//! Covers the cycle detector and the `/`-separated path parser/lookup used +//! by `create_group_path`. Pure functions, so they don't need any Tauri +//! runtime or filesystem. #[cfg(test)] mod tests { - use crate::commands::reject_if_would_create_cycle; + use crate::commands::{find_child_group, parse_group_path, reject_if_would_create_cycle}; use crate::models::ConnectionGroup; fn g(id: &str, parent: Option<&str>) -> ConnectionGroup { @@ -74,4 +74,57 @@ mod tests { let groups = vec![g("a", None), g("b", Some("a"))]; assert!(reject_if_would_create_cycle(&groups, "b", Some("a")).is_ok()); } + + #[test] + fn parse_group_path_splits_on_slash() { + assert_eq!( + parse_group_path("a/b/c").unwrap(), + vec!["a".to_string(), "b".to_string(), "c".to_string()] + ); + } + + #[test] + fn parse_group_path_trims_whitespace_and_skips_empty() { + assert_eq!( + parse_group_path(" a / / b / ").unwrap(), + vec!["a".to_string(), "b".to_string()] + ); + } + + #[test] + fn parse_group_path_rejects_empty_string() { + assert!(parse_group_path("").is_err()); + assert!(parse_group_path(" / / ").is_err()); + } + + #[test] + fn parse_group_path_keeps_single_segment() { + assert_eq!(parse_group_path("lone").unwrap(), vec!["lone".to_string()]); + } + + #[test] + fn find_child_group_is_case_insensitive() { + // The `g` helper uses the id as the name, so "Production" here + // and a search for "production" should still match. + let groups = vec![g("Production", None)]; + let found = find_child_group(&groups, "production", &None); + assert!(found.is_some()); + assert_eq!(found.unwrap().id, "Production"); + } + + #[test] + fn find_child_group_scopes_to_parent() { + // Two groups named the same, but with different parents. + let groups = vec![ + g("alpha", Some("parent-1")), + g("alpha", Some("parent-2")), + ]; + // Only the one under "parent-1" is found. + let found = find_child_group(&groups, "alpha", &Some("parent-1".to_string())); + assert!(found.is_some()); + assert_eq!(found.unwrap().id, "alpha"); + // Wrong parent yields None. + let missing = find_child_group(&groups, "alpha", &None); + assert!(missing.is_none()); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index bb407544..d17ab76e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -310,6 +310,7 @@ pub fn run() { commands::get_connection_groups, commands::get_connections_with_groups, commands::create_connection_group, + commands::create_group_path, commands::update_connection_group, commands::move_group_to_parent, commands::delete_connection_group, diff --git a/src/components/connections/GroupHeader.tsx b/src/components/connections/GroupHeader.tsx index c9e286f9..70d9a11d 100644 --- a/src/components/connections/GroupHeader.tsx +++ b/src/components/connections/GroupHeader.tsx @@ -1,5 +1,5 @@ import type { RefObject } from 'react'; -import { GripVertical, ChevronRight, Folder, FolderOpen, MoreVertical } from 'lucide-react'; +import { GripVertical, ChevronRight, Folder, FolderOpen, MoreVertical, Plus } from 'lucide-react'; import clsx from 'clsx'; import type { ConnectionGroup } from '../../contexts/DatabaseContext'; @@ -17,6 +17,7 @@ export interface GroupHeaderProps { onRenameConfirm: (groupId: string) => void; onGripMouseDown?: (e: React.MouseEvent) => void; isDragOver?: boolean; + onCreateSubgroup?: (groupId: string) => void; } export const GroupHeader = ({ @@ -33,6 +34,7 @@ export const GroupHeader = ({ onRenameConfirm, onGripMouseDown, isDragOver, + onCreateSubgroup, }: GroupHeaderProps) => (
{group.name} )} ({connCount}) + {onCreateSubgroup && ( + + )} + +
+ )} {!isCollapsed && (
{ setNewGroupName(""); } }} - placeholder={t("groups.groupName")} + placeholder={t("groups.groupName", { + defaultValue: "Group name (use / for nested)", + })} autoFocus className="w-40 px-3 py-2 bg-elevated border border-strong rounded-xl text-sm text-primary placeholder:text-muted focus:border-amber-500/70 focus:outline-none transition-colors" /> From 73594ecf264fa0f72f264d72358f673d58bfcb73 Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 20:00:30 +0100 Subject: [PATCH 4/8] feat(ui): indent nested group headers in both list and grid view After PR #1/#3, subfolders are technically rendered inside their parent's container, but the group header itself was rendered flush to the left, at the same horizontal position as the parent group's header. Only the connection cards were indented, which made the hierarchy hard to read at a glance: a folder called 'flexways' sat exactly under 'TEST' visually, and you had to look at the cards below to confirm it was a child. Pass the existing render-tree depth through to GroupHeader as a new optional 'depth' prop. GroupHeader uses it to push the whole header 20px to the right per nesting level (capped at 6 levels = 120px, matching the existing connection indent cap). The connection container below gets an extra 20px per level on top of its previous indent so cards stay visually anchored to their header rather than to the parent's left edge. Pure presentation change: no data model, context, or backend touched. Works identically in list and grid view since both share the same renderGroupTree recursion. Validated with pnpm tsc --noEmit (clean) and visually in pnpm tauri dev. --- src/components/connections/GroupHeader.tsx | 3 +++ src/pages/Connections.tsx | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/connections/GroupHeader.tsx b/src/components/connections/GroupHeader.tsx index 70d9a11d..74190549 100644 --- a/src/components/connections/GroupHeader.tsx +++ b/src/components/connections/GroupHeader.tsx @@ -18,6 +18,7 @@ export interface GroupHeaderProps { onGripMouseDown?: (e: React.MouseEvent) => void; isDragOver?: boolean; onCreateSubgroup?: (groupId: string) => void; + depth?: number; } export const GroupHeader = ({ @@ -35,12 +36,14 @@ export const GroupHeader = ({ onGripMouseDown, isDragOver, onCreateSubgroup, + depth = 0, }: GroupHeaderProps) => (
0 ? Math.min(depth, 6) * 20 : 0 }} onClick={onToggleCollapse} onContextMenu={(e) => { e.preventDefault(); diff --git a/src/pages/Connections.tsx b/src/pages/Connections.tsx index 6a4db735..46331e4a 100644 --- a/src/pages/Connections.tsx +++ b/src/pages/Connections.tsx @@ -648,6 +648,7 @@ export const Connections = () => { {subgroupInputFor === group.id && (
{ ? "grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-3" : "flex flex-col gap-1.5" } - style={{ paddingLeft: 24 + indentPx }} + style={{ paddingLeft: 24 + indentPx + (depth + 1) * 20 }} > {groupConns.map((conn) => mode === "grid" ? ( From 90e556d75ac2638d132fb4cbae4065b8f2cc6714 Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Fri, 26 Jun 2026 08:30:23 +0100 Subject: [PATCH 5/8] fix(export/import): preserve nested group hierarchy on import The export side already worked: 'export_connections_payload' returns conn_file.groups verbatim, which includes every ConnectionGroup with its parent_id set. A roundtrip through JSON preserves the field because it is serialized on the struct. Tested directly. The import side had a latent bug: it accepted any parent_id without verifying that the referenced group actually exists in the union (incoming payload + local config). A JSON with a malformed parent_id, or a payload that only carries a subtree of a larger tree, would land dangling references in the local config. The UI then renders the orphan as if it were a child, but the parent's card never appears, so it looks like a flat list. The fix moves the inline merge loop into a small 'merge_groups' helper that: 1. inserts / overwrites groups by id (existing semantics); 2. after the merge, walks the resulting list and demotes any parent_id that doesn't resolve to a known id back to None (root). This is tolerant: a partially-malformed JSON still imports successfully, and the user keeps most of their tree. merge_groups is pub(crate) so the export/import test module can call it directly. ConnectionGroup gains PartialEq + Eq to support the idempotency assertion. Tests added (export_import_tests): - test_export_preserves_nested_group_hierarchy - test_merge_groups_imports_full_subtree_preserving_hierarchy - test_merge_groups_demotes_orphaned_parent_id_to_root - test_merge_groups_keeps_existing_parent_when_payload_overrides - test_merge_groups_is_idempotent - test_merge_groups_incoming_parent_in_existing_only cargo test --lib: 710/710 pass (was 704). --- src-tauri/src/commands.rs | 44 ++++-- src-tauri/src/export_import_tests.rs | 202 +++++++++++++++++++++++++++ src-tauri/src/models.rs | 2 +- 3 files changed, 239 insertions(+), 9 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 60caff85..3e8165fa 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -441,6 +441,40 @@ pub fn find_connection_by_id( Ok(conn) } +/// Merge a list of incoming groups into an existing list, preserving hierarchy +/// and repairing any `parent_id` that points to a group id not present in the +/// union (i.e. neither in the existing list nor in the incoming batch). +/// +/// Behaviour: +/// - Existing groups with the same id are overwritten by the incoming one +/// (so renames / re-ordering / new parent_id from the JSON win). +/// - Missing parents are demoted to root (`parent_id = None`) rather than +/// being rejected, so a partially-malformed JSON still imports successfully +/// and the user keeps most of their tree. +/// - The merge is idempotent: running it twice on the same input is a no-op. +pub(crate) fn merge_groups(existing: &mut Vec, incoming: Vec) { + for new_group in incoming { + if let Some(existing_group) = existing.iter_mut().find(|g| g.id == new_group.id) { + *existing_group = new_group; + } else { + existing.push(new_group); + } + } + + // Build the set of every group id we now have (post-merge) so we can + // detect parent_ids that no longer point anywhere. Collected into an + // owned set to release the immutable borrow before we mutate existing. + let known_ids: std::collections::HashSet = + existing.iter().map(|g| g.id.clone()).collect(); + for g in existing.iter_mut() { + if let Some(parent) = g.parent_id.as_deref() { + if !known_ids.contains(parent) { + g.parent_id = None; + } + } + } +} + /// Write the connections file and invalidate the in-memory connection cache so /// the next `find_connection_by_id` call re-reads fresh data from disk. fn save_connections_and_invalidate( @@ -4514,14 +4548,8 @@ pub async fn import_connections_payload( .inner() .clone(); - // Merge groups - for new_group in payload.groups { - if let Some(existing) = current_file.groups.iter_mut().find(|g| g.id == new_group.id) { - *existing = new_group; - } else { - current_file.groups.push(new_group); - } - } + // Merge groups (preserves hierarchy; demotes orphaned parent_ids to root) + merge_groups(&mut current_file.groups, payload.groups); // Merge connections and handle passwords for mut new_conn in payload.connections { diff --git a/src-tauri/src/export_import_tests.rs b/src-tauri/src/export_import_tests.rs index b9de1841..270404fd 100644 --- a/src-tauri/src/export_import_tests.rs +++ b/src-tauri/src/export_import_tests.rs @@ -1,5 +1,6 @@ #[cfg(test)] mod tests { + use crate::commands; use crate::models::{ExportPayload, ConnectionGroup, SavedConnection, SshConnection, ConnectionParams, DatabaseSelection}; #[test] @@ -58,4 +59,205 @@ mod tests { assert_eq!(deserialized.connections[0].params.password, Some("password".to_string())); assert_eq!(deserialized.ssh_connections[0].password, Some("ssh_password".to_string())); } + + // Helper: build a 3-level tree + // - root "A" + // - child "A1" (parent=A) + // - grandchild "A1a" (parent=A1) + // - root "B" + fn build_tree() -> Vec { + vec![ + ConnectionGroup { + id: "A".into(), + name: "A".into(), + collapsed: false, + sort_order: 0, + parent_id: None, + }, + ConnectionGroup { + id: "A1".into(), + name: "A1".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("A".into()), + }, + ConnectionGroup { + id: "A1a".into(), + name: "A1a".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("A1".into()), + }, + ConnectionGroup { + id: "B".into(), + name: "B".into(), + collapsed: false, + sort_order: 1, + parent_id: None, + }, + ] + } + + #[test] + fn test_export_preserves_nested_group_hierarchy() { + // The export payload must round-trip the parent_id chain through + // JSON so the importer can rebuild the tree, not just flat-list + // the groups. + let tree = build_tree(); + let payload = ExportPayload { + version: 1, + groups: tree.clone(), + connections: vec![], + ssh_connections: vec![], + k8s_connections: vec![], + }; + + let json = serde_json::to_string(&payload).unwrap(); + let deserialized: ExportPayload = serde_json::from_str(&json).unwrap(); + + // Same set of ids + let original_ids: std::collections::HashSet<_> = tree.iter().map(|g| g.id.clone()).collect(); + let new_ids: std::collections::HashSet<_> = + deserialized.groups.iter().map(|g| g.id.clone()).collect(); + assert_eq!(original_ids, new_ids); + + // Every parent_id points to a group that exists in the payload + let new_id_refs: std::collections::HashSet<&str> = + deserialized.groups.iter().map(|g| g.id.as_str()).collect(); + for g in &deserialized.groups { + if let Some(parent) = g.parent_id.as_deref() { + assert!( + new_id_refs.contains(parent), + "After deserialization, {} has parent_id {} which is not in the payload", + g.id, + parent + ); + } + } + + // The 3-level chain is intact: A1a -> A1 -> A + let a1a = deserialized.groups.iter().find(|g| g.id == "A1a").unwrap(); + let a1 = deserialized.groups.iter().find(|g| g.id == "A1").unwrap(); + let a = deserialized.groups.iter().find(|g| g.id == "A").unwrap(); + assert_eq!(a1a.parent_id.as_deref(), Some("A1")); + assert_eq!(a1.parent_id.as_deref(), Some("A")); + assert_eq!(a.parent_id, None); + } + + #[test] + fn test_merge_groups_imports_full_subtree_preserving_hierarchy() { + // Simulate the import step: empty local config, payload brings a + // 3-level tree. Every group should land with its parent_id intact. + let mut existing: Vec = vec![]; + let incoming = build_tree(); + crate::commands::merge_groups(&mut existing, incoming); + + assert_eq!(existing.len(), 4); + let a1a = existing.iter().find(|g| g.id == "A1a").unwrap(); + let a1 = existing.iter().find(|g| g.id == "A1").unwrap(); + let a = existing.iter().find(|g| g.id == "A").unwrap(); + let b = existing.iter().find(|g| g.id == "B").unwrap(); + + assert_eq!(a1a.parent_id.as_deref(), Some("A1")); + assert_eq!(a1.parent_id.as_deref(), Some("A")); + assert_eq!(a.parent_id, None); + assert_eq!(b.parent_id, None); + } + + #[test] + fn test_merge_groups_demotes_orphaned_parent_id_to_root() { + // The JSON claims "A1a" is a child of "MISSING", which doesn't + // exist in the payload nor locally. We must not import a dangling + // pointer; instead we treat the orphan as a top-level group. + let mut existing: Vec = vec![]; + let incoming = vec![ConnectionGroup { + id: "A1a".into(), + name: "A1a".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("MISSING".into()), + }]; + crate::commands::merge_groups(&mut existing, incoming); + + assert_eq!(existing.len(), 1); + assert_eq!(existing[0].parent_id, None); + } + + #[test] + fn test_merge_groups_keeps_existing_parent_when_payload_overrides() { + // The local config has "A" as a top-level group and "A1" as a + // child of "A". The payload re-imports the same ids but renames + // "A1" to "A-renamed". The child should still be a child of "A" + // in the merged result, because the parent's id is unchanged. + let mut existing = vec![ + ConnectionGroup { + id: "A".into(), + name: "A".into(), + collapsed: false, + sort_order: 0, + parent_id: None, + }, + ConnectionGroup { + id: "A1".into(), + name: "A1".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("A".into()), + }, + ]; + let incoming = vec![ConnectionGroup { + id: "A1".into(), + name: "A-renamed".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("A".into()), + }]; + crate::commands::merge_groups(&mut existing, incoming); + + assert_eq!(existing.len(), 2); + let a1 = existing.iter().find(|g| g.id == "A1").unwrap(); + assert_eq!(a1.name, "A-renamed"); + assert_eq!(a1.parent_id.as_deref(), Some("A")); + } + + #[test] + fn test_merge_groups_is_idempotent() { + // Re-applying the same payload must not create duplicates or + // change the result beyond the first merge. + let mut existing: Vec = vec![]; + let incoming = build_tree(); + crate::commands::merge_groups(&mut existing, incoming.clone()); + let snapshot = existing.clone(); + crate::commands::merge_groups(&mut existing, incoming); + assert_eq!(existing, snapshot); + } + + #[test] + fn test_merge_groups_incoming_parent_in_existing_only() { + // The payload brings "A1" with parent_id = "A", but "A" already + // exists in the local config (created independently). The merge + // must keep the link working: "A1" remains a child of "A". + let mut existing = vec![ConnectionGroup { + id: "A".into(), + name: "A-existing".into(), + collapsed: false, + sort_order: 0, + parent_id: None, + }]; + let incoming = vec![ConnectionGroup { + id: "A1".into(), + name: "A1".into(), + collapsed: false, + sort_order: 0, + parent_id: Some("A".into()), + }]; + crate::commands::merge_groups(&mut existing, incoming); + + let a1 = existing.iter().find(|g| g.id == "A1").unwrap(); + let a = existing.iter().find(|g| g.id == "A").unwrap(); + assert_eq!(a1.parent_id.as_deref(), Some("A")); + // Existing "A" was not in the payload, so it stays as the user + // named it locally. + assert_eq!(a.name, "A-existing"); + } } diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 5ed2b775..99635341 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -214,7 +214,7 @@ pub struct SavedConnection { pub appearance: Option, } -#[derive(Debug, Deserialize, Serialize, Clone)] +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] pub struct ConnectionGroup { pub id: String, pub name: String, From bdfe13364dc1afc7d4d9e1c47bd3055b89c7e7b5 Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Wed, 1 Jul 2026 10:11:56 +0100 Subject: [PATCH 6/8] fix(groups): cascade delete subgroups and their connections Deleting a connection group now removes the target group, every nested child group (transitively), and every connection belonging to any group in that subtree. Previously direct children were re-parented to the grandparent and connections in the deleted group became ungrouped, which left orphaned subtrees when a top-level group was removed. A pure helper `collect_group_subtree(groups, root_id)` is extracted into models so the cascade logic is testable without an AppHandle. Six new unit tests cover leaf / deep chain / sibling isolation / unknown id / parent delete / subgroup delete. Frontend: `deleteGroup` re-fetches `get_connections_with_groups` so the optimistic state mirrors the persisted file instead of re-implementing the cascade in the client. --- src-tauri/src/commands.rs | 190 ++++++++++++++++++++++++++---- src-tauri/src/models.rs | 24 +++- src/contexts/DatabaseProvider.tsx | 28 ++--- 3 files changed, 202 insertions(+), 40 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 3e8165fa..8d288623 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -2618,6 +2618,162 @@ mod tests { assert!(state.handles.lock().unwrap().get("conn-1").is_none()); } } + + // ------------------------------------------------------------------- + // Cascade-delete helpers + // ------------------------------------------------------------------- + + fn group(id: &str, parent: Option<&str>) -> ConnectionGroup { + ConnectionGroup { + id: id.to_string(), + name: id.to_string(), + collapsed: false, + sort_order: 0, + parent_id: parent.map(|p| p.to_string()), + } + } + + fn conn(id: &str, group_id: Option<&str>) -> SavedConnection { + let mut c = saved_conn(id, None, false); + c.group_id = group_id.map(|g| g.to_string()); + c + } + + #[test] + fn collect_group_subtree_returns_root_only_for_leaf() { + let groups = vec![group("a", None), group("b", None)]; + let subtree = crate::models::collect_group_subtree(&groups, "a"); + assert_eq!(subtree, std::collections::HashSet::from(["a".to_string()])); + } + + #[test] + fn collect_group_subtree_walks_full_descendant_chain() { + // Tree: + // root + // ├── child1 + // │ └── grand1 + // │ └── great1 + // └── child2 + // other (unrelated) + let groups = vec![ + group("root", None), + group("child1", Some("root")), + group("grand1", Some("child1")), + group("great1", Some("grand1")), + group("child2", Some("root")), + group("other", None), + ]; + let subtree = crate::models::collect_group_subtree(&groups, "root"); + assert_eq!( + subtree, + std::collections::HashSet::from([ + "root".to_string(), + "child1".to_string(), + "grand1".to_string(), + "great1".to_string(), + "child2".to_string(), + ]) + ); + assert!(!subtree.contains("other")); + } + + #[test] + fn collect_group_subtree_for_subgroup_does_not_include_siblings() { + // Tree: + // root + // ├── keep + // └── drop + let groups = vec![ + group("root", None), + group("keep", Some("root")), + group("drop", Some("root")), + ]; + let subtree = crate::models::collect_group_subtree(&groups, "drop"); + assert_eq!(subtree, std::collections::HashSet::from(["drop".to_string()])); + assert!(!subtree.contains("root")); + assert!(!subtree.contains("keep")); + } + + #[test] + fn collect_group_subtree_for_unknown_id_is_singleton() { + let groups = vec![group("a", None)]; + let subtree = crate::models::collect_group_subtree(&groups, "missing"); + assert_eq!(subtree, std::collections::HashSet::from(["missing".to_string()])); + } + + #[test] + fn cascade_delete_removes_parent_descendants_and_connections() { + // Mirrors what the command does after the helper returns: groups + // and connections not in the subtree must survive untouched. + let groups = vec![ + group("root", None), + group("child", Some("root")), + group("grand", Some("child")), + group("sibling", None), + ]; + let connections = vec![ + conn("c1", Some("root")), + conn("c2", Some("child")), + conn("c3", Some("grand")), + conn("c4", Some("sibling")), + conn("c5", None), + ]; + let to_delete = crate::models::collect_group_subtree(&groups, "root"); + + let groups_after: Vec<_> = groups + .iter() + .filter(|g| !to_delete.contains(&g.id)) + .cloned() + .collect(); + let conns_after: Vec<_> = connections + .iter() + .filter(|c| !c.group_id.as_ref().is_some_and(|g| to_delete.contains(g))) + .cloned() + .collect(); + + assert_eq!(groups_after, vec![group("sibling", None)]); + assert_eq!( + conns_after.iter().map(|c| c.id.clone()).collect::>(), + vec!["c4".to_string(), "c5".to_string()], + ); + } + + #[test] + fn cascade_delete_subgroup_leaves_parent_and_other_subgroups_alone() { + let groups = vec![ + group("root", None), + group("keep", Some("root")), + group("drop", Some("root")), + group("grand", Some("drop")), + ]; + let connections = vec![ + conn("c1", Some("root")), + conn("c2", Some("drop")), + conn("c3", Some("grand")), + conn("c4", Some("keep")), + ]; + let to_delete = crate::models::collect_group_subtree(&groups, "drop"); + + let groups_after: Vec<_> = groups + .iter() + .filter(|g| !to_delete.contains(&g.id)) + .cloned() + .collect(); + let conns_after: Vec<_> = connections + .iter() + .filter(|c| !c.group_id.as_ref().is_some_and(|g| to_delete.contains(g))) + .cloned() + .collect(); + + assert_eq!( + groups_after, + vec![group("root", None), group("keep", Some("root"))], + ); + assert_eq!( + conns_after.iter().map(|c| c.id.clone()).collect::>(), + vec!["c1".to_string(), "c4".to_string()], + ); + } } #[tauri::command] @@ -4345,30 +4501,22 @@ pub async fn delete_connection_group( let path = get_config_path(&app)?; let mut file = persistence::load_connections_file(&path)?; - // Capture the deleted group's parent up-front so we can re-parent any - // direct child groups to the deleted group's parent. This is the standard - // "promote children to grandparent" behaviour from file-explorer UIs and - // matches what users expect when deleting a folder in Finder/Explorer. - let deleted_parent = file - .groups - .iter() - .find(|g| g.id == id) - .map(|g| g.parent_id.clone()) - .ok_or_else(|| format!("Group with ID {} not found", id))?; - - for g in &mut file.groups { - if g.parent_id.as_ref() == Some(&id) { - g.parent_id = deleted_parent.clone(); - } + // Ensure the group exists before we walk the tree. + if !file.groups.iter().any(|g| g.id == id) { + return Err(format!("Group with ID {} not found", id)); } - for conn in &mut file.connections { - if conn.group_id.as_ref() == Some(&id) { - conn.group_id = None; - } - } + // Cascade delete: collect the target group and all of its descendants + // (transitively) so the entire subtree is removed. The caller only + // needs to specify the top-level group — every nested child group is + // deleted along with it. Connections belonging to any group in the + // subtree are removed as well. + let to_delete = crate::models::collect_group_subtree(&file.groups, &id); + + file.groups.retain(|g| !to_delete.contains(&g.id)); + file.connections + .retain(|c| !c.group_id.as_ref().is_some_and(|gid| to_delete.contains(gid))); - file.groups.retain(|g| g.id != id); save_connections_and_invalidate(&app, &path, &file)?; Ok(()) diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 99635341..fcc89226 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -1,7 +1,29 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use serde::{Deserialize, Serialize}; +/// Returns the set of group IDs that form the subtree rooted at `root_id`, +/// including the root itself. Walks `parent_id` pointers transitively so +/// any number of nesting levels is collected. The caller is expected to +/// have already verified that `root_id` exists; an unknown id yields a +/// singleton set containing just that id (which won't match any record). +pub fn collect_group_subtree(groups: &[ConnectionGroup], root_id: &str) -> HashSet { + let mut to_delete: HashSet = HashSet::new(); + to_delete.insert(root_id.to_string()); + let mut changed = true; + while changed { + changed = false; + for g in groups { + if let Some(parent) = g.parent_id.as_ref() { + if to_delete.contains(parent) && to_delete.insert(g.id.clone()) { + changed = true; + } + } + } + } + to_delete +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] pub enum DatabaseSelection { diff --git a/src/contexts/DatabaseProvider.tsx b/src/contexts/DatabaseProvider.tsx index c33f1e9b..1c95f532 100644 --- a/src/contexts/DatabaseProvider.tsx +++ b/src/contexts/DatabaseProvider.tsx @@ -872,25 +872,17 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { }, []); const deleteGroup = useCallback(async (id: string): Promise => { + // The backend cascade-deletes the target group, every nested child + // group, and all connections belonging to any group in that subtree. + // Re-load from the backend instead of mirroring the cascade in + // optimistic state — this keeps the optimistic update trivial and + // guarantees the UI matches the persisted file even if the cascade + // behaviour evolves. await invoke('delete_connection_group', { id }); - // Direct children are re-parented to the deleted group's parent by - // the backend. Reflect that in the optimistic state update so the UI - // doesn't briefly show them in limbo. - const deleted = connectionGroups.find(g => g.id === id); - const newParent = deleted?.parent_id ?? null; - setConnectionGroups(prev => - prev - .filter(g => g.id !== id) - .map(g => - g.parent_id === id ? { ...g, parent_id: newParent } : g - ) - ); - // Connections in the deleted group become ungrouped (backend sets - // their group_id to None). - setConnections(prev => - prev.map(c => (c.group_id === id ? { ...c, group_id: undefined } : c)) - ); - }, [connectionGroups]); + const fresh = await invoke('get_connections_with_groups'); + setConnections(fresh.connections); + setConnectionGroups(fresh.groups); + }, []); const moveConnectionToGroup = useCallback(async ( connectionId: string, From daf905d1891347f6eb96a02a9f6a2d27fb0e8a1d Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Wed, 1 Jul 2026 10:17:34 +0100 Subject: [PATCH 7/8] chore(i18n): translate AWS IAM auth and sub-group prompt for all locales --- src/i18n/locales/de.json | 6 ++++-- src/i18n/locales/en.json | 6 ++++-- src/i18n/locales/es.json | 6 ++++-- src/i18n/locales/fr.json | 6 ++++-- src/i18n/locales/it.json | 6 ++++-- src/i18n/locales/ja.json | 6 ++++-- src/i18n/locales/ru.json | 6 ++++-- src/i18n/locales/zh.json | 6 ++++-- 8 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 21c2610c..b4a51909 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -711,12 +711,14 @@ "selectTypeFirst": "Zuerst Kontext/Namespace/Typ auswählen", "useK8s": "Kubernetes-Port-Forward verwenden", "useK8sConnection": "Gespeicherte Verbindung", - "advanced": "Erweitert", "startupScript": "Startskript", "startupScriptDescription": "SQL, das bei jeder neuen Verbindung zu dieser Datenquelle ausgeführt wird. Verwenden Sie es für Sitzungseinstellungen wie SET / set_config (z. B. zum Umgehen von RLS). Trennen Sie Anweisungen mit Semikolons.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Registerkarten nach links blättern", - "scrollTabsRight": "Registerkarten nach rechts blättern" + "scrollTabsRight": "Registerkarten nach rechts blättern", + "useIamAuth": "AWS-IAM-Authentifizierung verwenden (RDS)", + "useIamAuthHint": "Das Passwortfeld wird als RDS-Auth-Token verwendet (aus `aws rds generate-db-auth-token`). Erfordert TLS. Tokens laufen alle 15 Minuten ab.", + "useIamAuthTlsRequired": "Aktiviere oben einen SSL-Modus, um die AWS-IAM-Authentifizierung zu nutzen." }, "sshConnections": { "title": "SSH-Verbindungen", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index e4092298..8818cd92 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -745,12 +745,14 @@ "selectTypeFirst": "Select context/namespace/type first", "useK8s": "Use Kubernetes Port-Forward", "useK8sConnection": "Saved Connection", - "advanced": "Advanced", "startupScript": "Startup Script", "startupScriptDescription": "SQL run on every new connection to this data source. Use it for session settings such as SET / set_config (e.g. bypassing RLS). Separate statements with semicolons.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Scroll tabs left", - "scrollTabsRight": "Scroll tabs right" + "scrollTabsRight": "Scroll tabs right", + "useIamAuth": "Use AWS IAM Authentication (RDS)", + "useIamAuthHint": "The password field is treated as an RDS auth token (from `aws rds generate-db-auth-token`). Requires TLS. Tokens expire every 15 minutes.", + "useIamAuthTlsRequired": "Enable an SSL mode above to use AWS IAM authentication." }, "sshConnections": { "title": "SSH Connections", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index e7a83378..e5ba3e24 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -716,12 +716,14 @@ "selectTypeFirst": "Selecciona primero contexto/namespace/tipo", "useK8s": "Usar Port-Forward de Kubernetes", "useK8sConnection": "Conexión guardada", - "advanced": "Avanzado", "startupScript": "Script de inicio", "startupScriptDescription": "SQL que se ejecuta en cada nueva conexión a esta fuente de datos. Úsalo para ajustes de sesión como SET / set_config (p. ej., para omitir RLS). Separa las sentencias con punto y coma.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Desplazar pestañas a la izquierda", - "scrollTabsRight": "Desplazar pestañas a la derecha" + "scrollTabsRight": "Desplazar pestañas a la derecha", + "useIamAuth": "Usar autenticación AWS IAM (RDS)", + "useIamAuthHint": "El campo de contraseña se interpreta como un token de autenticación de RDS (obtenido con `aws rds generate-db-auth-token`). Requiere TLS. Los tokens caducan cada 15 minutos.", + "useIamAuthTlsRequired": "Activa un modo SSL arriba para usar la autenticación AWS IAM." }, "sshConnections": { "title": "Conexiones SSH", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 19abdf60..b3bac5e3 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -711,12 +711,14 @@ "selectTypeFirst": "Sélectionnez d'abord contexte/namespace/type", "useK8s": "Utiliser le Port-Forward Kubernetes", "useK8sConnection": "Connexion enregistrée", - "advanced": "Avancé", "startupScript": "Script de démarrage", "startupScriptDescription": "SQL exécuté à chaque nouvelle connexion à cette source de données. Utilisez-le pour des paramètres de session tels que SET / set_config (par exemple, pour contourner la RLS). Séparez les instructions par des points-virgules.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Défiler les onglets vers la gauche", - "scrollTabsRight": "Défiler les onglets vers la droite" + "scrollTabsRight": "Défiler les onglets vers la droite", + "useIamAuth": "Utiliser l'authentification AWS IAM (RDS)", + "useIamAuthHint": "Le champ mot de passe est traité comme un jeton d'authentification RDS (généré par `aws rds generate-db-auth-token`). Nécessite TLS. Les jetons expirent toutes les 15 minutes.", + "useIamAuthTlsRequired": "Activez un mode SSL ci-dessus pour utiliser l'authentification AWS IAM." }, "sshConnections": { "title": "Connexions SSH", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index a244887b..8569db76 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -716,12 +716,14 @@ "selectTypeFirst": "Seleziona prima contesto/namespace/tipo", "useK8s": "Usa Port-Forward Kubernetes", "useK8sConnection": "Connessione salvata", - "advanced": "Avanzate", "startupScript": "Script di avvio", "startupScriptDescription": "SQL eseguito a ogni nuova connessione a questa origine dati. Usalo per le impostazioni di sessione come SET / set_config (ad es. per bypassare la RLS). Separa le istruzioni con punto e virgola.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Scorri schede a sinistra", - "scrollTabsRight": "Scorri schede a destra" + "scrollTabsRight": "Scorri schede a destra", + "useIamAuth": "Usa autenticazione AWS IAM (RDS)", + "useIamAuthHint": "Il campo password è trattato come un token di autenticazione RDS (da `aws rds generate-db-auth-token`). Richiede TLS. I token scadono ogni 15 minuti.", + "useIamAuthTlsRequired": "Attiva una modalità SSL qui sopra per usare l'autenticazione AWS IAM." }, "sshConnections": { "title": "Connessioni SSH", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index f8de6396..1efd21f0 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -725,12 +725,14 @@ "selectTypeFirst": "先にコンテキスト/ネームスペース/タイプを選択してください", "useK8s": "Kubernetes ポートフォワードを使用", "useK8sConnection": "保存された接続", - "advanced": "詳細設定", "startupScript": "起動スクリプト", "startupScriptDescription": "このデータソースへの新規接続ごとに実行される SQL です。SET / set_config（例: RLS のバイパス）などのセッション設定に使用します。複数のステートメントはセミコロンで区切ってください。", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "タブを左にスクロール", - "scrollTabsRight": "タブを右にスクロール" + "scrollTabsRight": "タブを右にスクロール", + "useIamAuth": "AWS IAM 認証を使用する (RDS)", + "useIamAuthHint": "パスワード欄は RDS 認証トークン (`aws rds generate-db-auth-token` の出力) として扱われます。TLS 必須。トークンの有効期限は 15 分です。", + "useIamAuthTlsRequired": "AWS IAM 認証を使用するには、上記で SSL モードを有効にしてください。" }, "sshConnections": { "title": "SSH 接続", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index 86752869..b726ea56 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -704,12 +704,14 @@ "useK8s": "Использовать проброс портов Kubernetes", "useK8sConnection": "Сохранённое подключение", "appearance": "Внешний вид", - "advanced": "Дополнительно", "startupScript": "Сценарий запуска", "startupScriptDescription": "SQL, выполняемый при каждом новом подключении к этому источнику данных. Используйте его для настроек сеанса, таких как SET / set_config (например, для обхода RLS). Разделяйте операторы точкой с запятой.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "Прокрутить вкладки влево", - "scrollTabsRight": "Прокрутить вкладки вправо" + "scrollTabsRight": "Прокрутить вкладки вправо", + "useIamAuth": "Использовать аутентификацию AWS IAM (RDS)", + "useIamAuthHint": "Поле пароля используется как токен аутентификации RDS (команда `aws rds generate-db-auth-token`). Требуется TLS. Срок действия токенов — 15 минут.", + "useIamAuthTlsRequired": "Чтобы использовать аутентификацию AWS IAM, включите режим SSL выше." }, "sshConnections": { "title": "SSH-подключения", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index 730b4980..5e05b9dd 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -679,12 +679,14 @@ "selectTypeFirst": "请先选择上下文/命名空间/类型", "useK8s": "使用 Kubernetes 端口转发", "useK8sConnection": "已保存的连接", - "advanced": "高级", "startupScript": "启动脚本", "startupScriptDescription": "每次新建到此数据源的连接时执行的 SQL。可用于会话设置，例如 SET / set_config（如绕过 RLS）。多条语句请用分号分隔。", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", "scrollTabsLeft": "向左滚动标签页", - "scrollTabsRight": "向右滚动标签页" + "scrollTabsRight": "向右滚动标签页", + "useIamAuth": "使用 AWS IAM 身份验证 (RDS)", + "useIamAuthHint": "密码字段将被视为 RDS 身份验证令牌（由 `aws rds generate-db-auth-token` 生成）。需要 TLS。令牌每 15 分钟过期。", + "useIamAuthTlsRequired": "请在上方启用 SSL 模式以使用 AWS IAM 身份验证。" }, "sshConnections": { "title": "SSH 连接", From b7e0383a012a69e8e77f8e721750c78561423a7f Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Wed, 1 Jul 2026 15:23:06 +0100 Subject: [PATCH 8/8] fix(ui): silence lint errors in Connections group drag handler --- src/pages/Connections.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pages/Connections.tsx b/src/pages/Connections.tsx index 46331e4a..410c9281 100644 --- a/src/pages/Connections.tsx +++ b/src/pages/Connections.tsx @@ -46,7 +46,6 @@ export const Connections = () => { isConnectionOpen, switchConnection, connectionGroups, - createGroup, createGroupPath, updateGroup, moveGroupToParent, @@ -570,7 +569,7 @@ export const Connections = () => { if (reparent) { const isAncestor = (maybeAncestorId: string): boolean => { - let cur = connectionGroups.find((g) => g.id === targetGroupId); + let cur = connectionGroups.find((g) => g.id === maybeAncestorId); while (cur) { if (cur.id === sourceGroupId) return true; cur = connectionGroups.find((g) => g.id === cur!.parent_id);