Skip to content

Commit f7f217a

Browse files
DanielMicrosoftDanielMicrosoft
committed
fix: auto expand bug
1 parent b80125f commit f7f217a

File tree

4 files changed

+269
-10
lines changed

4 files changed

+269
-10
lines changed

‎src/web/src/__tests__/components/WorkspaceSelector.test.tsx‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,29 +311,23 @@ describe("Workspace Management", () => {
311311

312312
render(<WorkspaceSelector name="Select Workspace" />);
313313

314-
// Wait for workspaces to load
315314
await waitFor(() => {
316315
expect(workspaceApi.getWorkspaces).toHaveBeenCalled();
317316
});
318317

319-
// Type a new workspace name to trigger create option
320318
const autocomplete = screen.getByLabelText("Select Workspace");
321319
await user.click(autocomplete);
322320
await user.type(autocomplete, "new-test-workspace");
323321

324-
// Click on the create option
325322
const createOption = await screen.findByText('Create "new-test-workspace"');
326323
await user.click(createOption);
327324

328-
// Wait for the dialog to appear
329325
await screen.findByText("Create a new workspace");
330326

331-
// Wait for the getPlanes API to be called
332327
await waitFor(() => {
333328
expect(specsApi.getPlanes).toHaveBeenCalled();
334329
});
335330

336-
// The plane dropdown should be populated with the first plane
337331
const planeDropdown = await screen.findByLabelText(/Plane/i);
338332
expect(planeDropdown).toHaveValue("Control plane");
339333
});

‎src/web/src/__tests__/hooks/useTreeState.test.ts‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
import { renderHook, act } from "@testing-library/react";
2+
import { useTreeState } from "../../views/workspace/hooks/useTreeState";
3+
import type { Command, CommandGroup } from "../../views/workspace/interfaces";
4+
5+
describe("useTreeState", () => {
6+
const mockCommandGroupMap = {
7+
"group:automanage": {
8+
id: "group:automanage",
9+
names: ["automanage"],
10+
} as CommandGroup,
11+
"group:automanage/configuration-profile": {
12+
id: "group:automanage/configuration-profile",
13+
names: ["automanage", "configuration-profile"],
14+
} as CommandGroup,
15+
"group:automanage/configuration-profile/assignment": {
16+
id: "group:automanage/configuration-profile/assignment",
17+
names: ["automanage", "configuration-profile", "assignment"],
18+
} as CommandGroup,
19+
"group:storage": {
20+
id: "group:storage",
21+
names: ["storage"],
22+
} as CommandGroup,
23+
"group:storage/account": {
24+
id: "group:storage/account",
25+
names: ["storage", "account"],
26+
} as CommandGroup,
27+
};
28+
29+
const mockCommandMap = {
30+
"command:automanage/configuration-profile/assignment/create": {
31+
id: "command:automanage/configuration-profile/assignment/create",
32+
names: ["automanage", "configuration-profile", "assignment", "create"],
33+
} as Command,
34+
};
35+
36+
const mockCommandTree = [
37+
{
38+
id: "group:automanage",
39+
names: ["automanage"],
40+
canDelete: true,
41+
nodes: [
42+
{
43+
id: "group:automanage/configuration-profile",
44+
names: ["automanage", "configuration-profile"],
45+
canDelete: true,
46+
nodes: [
47+
{
48+
id: "group:automanage/configuration-profile/assignment",
49+
names: ["automanage", "configuration-profile", "assignment"],
50+
canDelete: true,
51+
leaves: [
52+
{
53+
id: "command:automanage/configuration-profile/assignment/create",
54+
names: ["automanage", "configuration-profile", "assignment", "create"],
55+
},
56+
],
57+
},
58+
],
59+
},
60+
],
61+
},
62+
{
63+
id: "group:storage",
64+
names: ["storage"],
65+
canDelete: true,
66+
nodes: [
67+
{
68+
id: "group:storage/account",
69+
names: ["storage", "account"],
70+
canDelete: true,
71+
},
72+
],
73+
},
74+
];
75+
76+
describe("updateExpanded with autoExpandAll", () => {
77+
it("should expand all command groups when autoExpandAll is true", () => {
78+
const { result } = renderHook(() => useTreeState(mockCommandMap, {}, mockCommandTree));
79+
80+
act(() => {
81+
result.current.updateExpanded(mockCommandGroupMap, undefined, true);
82+
});
83+
84+
const expandedArray = Array.from(result.current.expanded);
85+
86+
// Should include all command groups
87+
expect(expandedArray).toContain("group:automanage");
88+
expect(expandedArray).toContain("group:automanage/configuration-profile");
89+
expect(expandedArray).toContain("group:automanage/configuration-profile/assignment");
90+
expect(expandedArray).toContain("group:storage");
91+
expect(expandedArray).toContain("group:storage/account");
92+
93+
// Should include all parent paths for hierarchy
94+
expect(expandedArray).toContain("group:automanage");
95+
expect(expandedArray).toContain("group:automanage/configuration-profile");
96+
expect(expandedArray).toContain("group:storage");
97+
98+
// Total count should be all unique paths
99+
expect(expandedArray).toHaveLength(5);
100+
});
101+
102+
it("should expand all groups regardless of existing expanded state when autoExpandAll is true", () => {
103+
const { result } = renderHook(() => useTreeState(mockCommandMap, {}, mockCommandTree));
104+
105+
// First, manually expand only one group
106+
act(() => {
107+
result.current.updateExpanded({ "group:storage": mockCommandGroupMap["group:storage"] }, undefined, false);
108+
});
109+
110+
// Should only have storage expanded initially
111+
expect(Array.from(result.current.expanded)).toEqual(["group:storage"]);
112+
113+
// Now call with autoExpandAll=true
114+
act(() => {
115+
result.current.updateExpanded(mockCommandGroupMap, undefined, true);
116+
});
117+
118+
const expandedArray = Array.from(result.current.expanded);
119+
120+
// Should now include ALL groups, not just storage
121+
expect(expandedArray).toContain("group:automanage");
122+
expect(expandedArray).toContain("group:automanage/configuration-profile");
123+
expect(expandedArray).toContain("group:automanage/configuration-profile/assignment");
124+
expect(expandedArray).toContain("group:storage");
125+
expect(expandedArray).toContain("group:storage/account");
126+
});
127+
128+
it("should only expand new groups when autoExpandAll is false", () => {
129+
const initialCommandGroupMap = {
130+
"group:storage": mockCommandGroupMap["group:storage"],
131+
};
132+
133+
const { result } = renderHook(() => useTreeState(mockCommandMap, initialCommandGroupMap, mockCommandTree));
134+
135+
// First, expand storage (which already exists in initial map)
136+
act(() => {
137+
result.current.updateExpanded(initialCommandGroupMap, undefined, false);
138+
});
139+
140+
// Should be empty since storage already existed in the initial map
141+
expect(Array.from(result.current.expanded)).toHaveLength(0);
142+
143+
// Now add new groups with autoExpandAll=false
144+
act(() => {
145+
result.current.updateExpanded(mockCommandGroupMap, undefined, false);
146+
});
147+
148+
const expandedArray = Array.from(result.current.expanded);
149+
150+
// Should only include new groups (not storage since it existed before)
151+
expect(expandedArray).toContain("group:automanage");
152+
expect(expandedArray).toContain("group:automanage/configuration-profile");
153+
expect(expandedArray).toContain("group:automanage/configuration-profile/assignment");
154+
expect(expandedArray).toContain("group:storage/account"); // This is new
155+
156+
// Should NOT include storage since it existed in the original commandGroupMap
157+
expect(expandedArray).not.toContain("group:storage");
158+
});
159+
160+
it("should include parent paths for deeply nested groups", () => {
161+
const deeplyNestedMap = {
162+
"group:level1/level2/level3/level4": {
163+
id: "group:level1/level2/level3/level4",
164+
names: ["level1", "level2", "level3", "level4"],
165+
} as CommandGroup,
166+
};
167+
168+
const { result } = renderHook(() => useTreeState(mockCommandMap, {}, mockCommandTree));
169+
170+
act(() => {
171+
result.current.updateExpanded(deeplyNestedMap, undefined, true);
172+
});
173+
174+
const expandedArray = Array.from(result.current.expanded);
175+
176+
// Should include the group itself
177+
expect(expandedArray).toContain("group:level1/level2/level3/level4");
178+
179+
// Should include all parent paths for proper hierarchy
180+
expect(expandedArray).toContain("group:level1/level2");
181+
expect(expandedArray).toContain("group:level1/level2/level3");
182+
});
183+
184+
it("should preserve existing expanded state when adding new groups with autoExpandAll=true", () => {
185+
const { result } = renderHook(() => useTreeState(mockCommandMap, {}, mockCommandTree));
186+
187+
// Start with some manual expansion
188+
act(() => {
189+
result.current.handleCommandTreeToggle(["group:storage"]);
190+
});
191+
192+
expect(Array.from(result.current.expanded)).toEqual(["group:storage"]);
193+
194+
// Now call updateExpanded with autoExpandAll=true
195+
act(() => {
196+
result.current.updateExpanded(mockCommandGroupMap, undefined, true);
197+
});
198+
199+
const expandedArray = Array.from(result.current.expanded);
200+
201+
// Should still contain the manually expanded group
202+
expect(expandedArray).toContain("group:storage");
203+
204+
// Plus all the auto-expanded groups
205+
expect(expandedArray).toContain("group:automanage");
206+
expect(expandedArray).toContain("group:automanage/configuration-profile");
207+
expect(expandedArray).toContain("group:automanage/configuration-profile/assignment");
208+
expect(expandedArray).toContain("group:storage/account");
209+
});
210+
});
211+
212+
describe("basic functionality", () => {
213+
it("should initialize and auto-select first group with its path expanded", () => {
214+
const { result } = renderHook(() => useTreeState(mockCommandMap, mockCommandGroupMap, mockCommandTree));
215+
216+
// Should auto-select the first group from commandTree
217+
expect(result.current.selected?.id).toBe("group:automanage");
218+
219+
// Should auto-expand the path to the selected group
220+
expect(result.current.expanded.size).toBe(1);
221+
expect(Array.from(result.current.expanded)).toContain("group:automanage");
222+
});
223+
224+
it("should handle command tree toggle", () => {
225+
const { result } = renderHook(() => useTreeState(mockCommandMap, mockCommandGroupMap, mockCommandTree));
226+
227+
act(() => {
228+
result.current.handleCommandTreeToggle(["group:storage", "group:automanage"]);
229+
});
230+
231+
const expandedArray = Array.from(result.current.expanded);
232+
expect(expandedArray).toContain("group:storage");
233+
expect(expandedArray).toContain("group:automanage");
234+
expect(expandedArray).toHaveLength(2);
235+
});
236+
});
237+
});

‎src/web/src/views/workspace/components/WSEditor/WSEditor.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ const WSEditor = ({ params }: WSEditorProps) => {
6464
}
6565
}, [workspace.reloadTimestamp]);
6666

67+
useEffect(() => {
68+
if (Object.keys(workspace.commandGroupMap).length > 0) {
69+
treeState.updateExpanded(workspace.commandGroupMap, undefined, true);
70+
}
71+
}, [workspace.commandGroupMap, treeState.updateExpanded]);
72+
6773
const handleSwaggerReloadDialogClose = useCallback(
6874
async (reloaded: boolean) => {
6975
if (reloaded) {

‎src/web/src/views/workspace/hooks/useTreeState.ts‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ interface UseTreeStateReturn {
1414
expanded: Set<string>;
1515
handleCommandTreeSelect: (nodeId: string) => void;
1616
handleCommandTreeToggle: (nodeIds: string[]) => void;
17-
updateExpanded: (commandGroupMap: CommandGroupMap, selected?: Command | CommandGroup | null) => void;
17+
updateExpanded: (
18+
commandGroupMap: CommandGroupMap,
19+
selected?: Command | CommandGroup | null,
20+
autoExpandAll?: boolean,
21+
) => void;
1822
setSelected: (selected: Command | CommandGroup | null) => void;
1923
}
2024

@@ -58,7 +62,7 @@ export function useTreeState(
5862
}, []);
5963

6064
const updateExpanded = useCallback(
61-
(newCommandGroupMap: CommandGroupMap, newSelected?: Command | CommandGroup | null) => {
65+
(newCommandGroupMap: CommandGroupMap, newSelected?: Command | CommandGroup | null, autoExpandAll?: boolean) => {
6266
setExpanded((prevExpanded) => {
6367
const newExpanded = new Set<string>();
6468

@@ -68,9 +72,27 @@ export function useTreeState(
6872
}
6973
});
7074

71-
for (const groupId in newCommandGroupMap) {
72-
if (!(groupId in commandGroupMap)) {
75+
if (autoExpandAll) {
76+
for (const groupId in newCommandGroupMap) {
7377
newExpanded.add(groupId);
78+
79+
const parts = groupId.split("/");
80+
for (let i = 1; i < parts.length; i++) {
81+
const parentPath = parts.slice(0, i + 1).join("/");
82+
newExpanded.add(parentPath);
83+
}
84+
}
85+
} else {
86+
for (const groupId in newCommandGroupMap) {
87+
if (!(groupId in commandGroupMap)) {
88+
newExpanded.add(groupId);
89+
90+
const parts = groupId.split("/");
91+
for (let i = 1; i < parts.length; i++) {
92+
const parentPath = parts.slice(0, i + 1).join("/");
93+
newExpanded.add(parentPath);
94+
}
95+
}
7496
}
7597
}
7698

0 commit comments

Comments
 (0)