feat(groups): support nested connection groups with cascade delete

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<ConnectionGroup> {
+        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<ConnectionGroup> = 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<ConnectionGroup> = 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<ConnectionGroup> = 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");
+    }
 }

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());
+    }
+}

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,