diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 959996a8..f44439c3 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( @@ -2584,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] @@ -4217,18 +4407,31 @@ 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); + 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 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()); @@ -4237,6 +4440,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, @@ -4269,6 +4555,83 @@ pub async fn update_connection_group( Ok(updated) } +/// 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, + 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) +} + +/// 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, + new_parent_id: Option<&str>, +) -> Result<(), String> { + let Some(target) = new_parent_id else { + return Ok(()); + }; + 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()) { + 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( @@ -4278,15 +4641,22 @@ pub async fn delete_connection_group( let path = get_config_path(&app)?; let mut file = persistence::load_connections_file(&path)?; - // 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; - } + // 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)); } - // Remove the group - file.groups.retain(|g| g.id != id); + // 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))); + save_connections_and_invalidate(&app, &path, &file)?; Ok(()) @@ -4466,14 +4836,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 1b7fc65f..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] @@ -11,6 +12,7 @@ mod tests { name: "Test Group".to_string(), collapsed: false, sort_order: 0, + parent_id: None, }], connections: vec![SavedConnection { id: "conn1".to_string(), @@ -57,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/group_tree_tests.rs b/src-tauri/src/group_tree_tests.rs new file mode 100644 index 00000000..a2f39373 --- /dev/null +++ b/src-tauri/src/group_tree_tests.rs @@ -0,0 +1,130 @@ +//! Unit tests for the nested connection-group tree helpers in `commands.rs`. +//! +//! 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::{find_child_group, parse_group_path, 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() { + 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() { + 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() { + 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() { + 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() { + 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() { + 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")); + assert!(result.is_err()); + } + + #[test] + fn cycle_check_target_not_in_tree_is_ok() { + 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 61f4f8d2..448d59b5 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; @@ -308,7 +310,9 @@ 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, commands::move_connection_to_group, commands::reorder_groups, diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index f33e1551..c5be93be 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 { @@ -218,7 +240,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, @@ -226,6 +248,10 @@ pub struct ConnectionGroup { pub collapsed: bool, #[serde(default)] pub sort_order: i32, + /// `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, } #[derive(Debug, Deserialize, Serialize, Clone, Default)] diff --git a/src/components/connections/GroupHeader.tsx b/src/components/connections/GroupHeader.tsx index c9e286f9..74190549 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,8 @@ export interface GroupHeaderProps { onRenameConfirm: (groupId: string) => void; onGripMouseDown?: (e: React.MouseEvent) => void; isDragOver?: boolean; + onCreateSubgroup?: (groupId: string) => void; + depth?: number; } export const GroupHeader = ({ @@ -33,12 +35,15 @@ export const GroupHeader = ({ onRenameConfirm, onGripMouseDown, isDragOver, + onCreateSubgroup, + depth = 0, }: GroupHeaderProps) => (
0 ? Math.min(depth, 6) * 20 : 0 }} onClick={onToggleCollapse} onContextMenu={(e) => { e.preventDefault(); @@ -87,6 +92,18 @@ export const GroupHeader = ({ {group.name} )} ({connCount}) + {onCreateSubgroup && ( + + )} + +
+ )} + {!isCollapsed && ( +
+ {groupConns.map((conn) => + mode === "grid" ? ( + + ) : ( + + ), + )} +
+ )} + {!isCollapsed && renderGroupTree(group.id, mode, depth + 1)} + + ); + }); + }; + + 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); + const conns = filteredGroupedConnections[id] || []; + if (conns.length > 0) return true; + const g = connectionGroups.find((x) => x.id === id); + if (g && g.name.toLowerCase().includes(lc)) return true; + const kids = groupsByParent.get(id) ?? []; + for (const kid of kids) stack.push(kid.id); + } + return false; + }; + + 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 ────────────────────────────────────────────────────────── */} @@ -663,7 +900,9 @@ export const Connections = () => { 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" /> @@ -747,32 +986,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 +1027,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 +1099,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,