Skip to content

Commit f31f0e7

Browse files
FelixTJDietrichFelixTJDietrich
authored
feat(workspace): sort workspaces by display name (#1054)
1 parent de80402 commit f31f0e7

8 files changed

Lines changed: 310 additions & 7 deletions

File tree

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/workspace/WorkspaceQueryService.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import de.tum.in.www1.hephaestus.gitprovider.user.User;
1010
import de.tum.in.www1.hephaestus.gitprovider.user.UserRepository;
1111
import de.tum.in.www1.hephaestus.workspace.dto.WorkspaceProvidersDTO;
12+
import java.util.Comparator;
1213
import java.util.LinkedHashMap;
1314
import java.util.List;
1415
import java.util.Optional;
@@ -32,6 +33,11 @@
3233
@WorkspaceAgnostic("Manages workspaces themselves - operates at admin/registry level")
3334
public class WorkspaceQueryService {
3435

36+
private static final Comparator<Workspace> ACCESSIBLE_WORKSPACE_COMPARATOR = Comparator.comparing(
37+
Workspace::getDisplayName,
38+
String.CASE_INSENSITIVE_ORDER
39+
).thenComparing(Workspace::getWorkspaceSlug, String.CASE_INSENSITIVE_ORDER);
40+
3541
private final WorkspaceRepository workspaceRepository;
3642
private final WorkspaceMembershipRepository workspaceMembershipRepository;
3743
private final RepositoryToMonitorRepository repositoryToMonitorRepository;
@@ -118,7 +124,7 @@ List<Workspace> findAccessibleWorkspaces(Optional<User> currentUser) {
118124
);
119125

120126
if (currentUser.isEmpty()) {
121-
return publicWorkspaces;
127+
return publicWorkspaces.stream().sorted(ACCESSIBLE_WORKSPACE_COMPARATOR).toList();
122128
}
123129

124130
// Fetch memberships for the current user and load workspaces by ID
@@ -140,6 +146,7 @@ List<Workspace> findAccessibleWorkspaces(Optional<User> currentUser) {
140146
)
141147
.values()
142148
.stream()
149+
.sorted(ACCESSIBLE_WORKSPACE_COMPARATOR)
143150
.toList();
144151
}
145152

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/workspace/WorkspaceControllerIntegrationTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,59 @@ void createWorkspaceEndpointAssignsOwnerMembershipAndListsWorkspace() {
202202
assertThat(membership.getRole()).isEqualTo(WorkspaceMembership.WorkspaceRole.OWNER);
203203
}
204204

205+
@Test
206+
@WithAdminUser
207+
void listWorkspacesIsSortedByDisplayName() {
208+
User ownerAlpha = persistUser("sorted-alpha-owner");
209+
User ownerZulu = persistUser("sorted-zulu-owner");
210+
User ownerBravo = persistUser("sorted-bravo-owner");
211+
212+
Workspace workspaceZulu = createWorkspace(
213+
"sorted-zulu",
214+
"Zulu Workspace",
215+
"sorted-zulu",
216+
AccountType.ORG,
217+
ownerZulu
218+
);
219+
Workspace workspaceAlpha = createWorkspace(
220+
"sorted-alpha",
221+
"Alpha Workspace",
222+
"sorted-alpha",
223+
AccountType.ORG,
224+
ownerAlpha
225+
);
226+
Workspace workspaceBravo = createWorkspace(
227+
"sorted-bravo",
228+
"Bravo Workspace",
229+
"sorted-bravo",
230+
AccountType.ORG,
231+
ownerBravo
232+
);
233+
234+
ensureAdminMembership(workspaceZulu);
235+
ensureAdminMembership(workspaceAlpha);
236+
ensureAdminMembership(workspaceBravo);
237+
238+
List<WorkspaceListItemDTO> workspaces = webTestClient
239+
.get()
240+
.uri("/workspaces")
241+
.headers(TestAuthUtils.withCurrentUser())
242+
.exchange()
243+
.expectStatus()
244+
.isOk()
245+
.expectBodyList(WorkspaceListItemDTO.class)
246+
.returnResult()
247+
.getResponseBody();
248+
249+
assertThat(workspaces)
250+
.isNotNull()
251+
.filteredOn(workspace ->
252+
List.of("sorted-zulu", "sorted-alpha", "sorted-bravo").contains(workspace.workspaceSlug())
253+
)
254+
.extracting(WorkspaceListItemDTO::workspaceSlug)
255+
.containsExactly("sorted-alpha", "sorted-bravo", "sorted-zulu");
256+
}
257+
205258
@Test
206259
@WithAdminUser
207260
void repositoryListingAndDeletionAreScopedByWorkspaceSlug() {

‎server/application-server/src/test/java/de/tum/in/www1/hephaestus/workspace/WorkspaceQueryServiceTest.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package de.tum.in.www1.hephaestus.workspace;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.Mockito.when;
5+
6+
import de.tum.in.www1.hephaestus.feature.FeatureFlagService;
7+
import de.tum.in.www1.hephaestus.gitprovider.common.gitlab.GitLabProperties;
8+
import de.tum.in.www1.hephaestus.gitprovider.github.GitHubProperties;
9+
import de.tum.in.www1.hephaestus.gitprovider.user.User;
10+
import de.tum.in.www1.hephaestus.gitprovider.user.UserRepository;
11+
import de.tum.in.www1.hephaestus.testconfig.BaseUnitTest;
12+
import java.util.List;
13+
import java.util.Optional;
14+
import org.junit.jupiter.api.Test;
15+
import org.mockito.Mock;
16+
17+
class WorkspaceQueryServiceTest extends BaseUnitTest {
18+
19+
@Mock
20+
private WorkspaceRepository workspaceRepository;
21+
22+
@Mock
23+
private WorkspaceMembershipRepository workspaceMembershipRepository;
24+
25+
@Mock
26+
private RepositoryToMonitorRepository repositoryToMonitorRepository;
27+
28+
@Mock
29+
private UserRepository userRepository;
30+
31+
@Mock
32+
private GitHubProperties gitHubProperties;
33+
34+
@Mock
35+
private GitLabProperties gitLabProperties;
36+
37+
@Mock
38+
private FeatureFlagService featureFlagService;
39+
40+
@Test
41+
void findAccessibleWorkspacesSortsByDisplayNameAndDeduplicatesMemberships() {
42+
Workspace alphaWorkspace = workspace(1L, "alpha-space", "Alpha Workspace", true);
43+
Workspace bravoWorkspace = workspace(2L, "bravo-space", "Bravo Workspace", false);
44+
Workspace zuluWorkspace = workspace(3L, "zulu-space", "Zulu Workspace", true);
45+
46+
User currentUser = new User();
47+
currentUser.setId(42L);
48+
49+
WorkspaceMembership bravoMembership = membership(bravoWorkspace);
50+
WorkspaceMembership alphaMembership = membership(alphaWorkspace);
51+
52+
WorkspaceQueryService service = new WorkspaceQueryService(
53+
workspaceRepository,
54+
workspaceMembershipRepository,
55+
repositoryToMonitorRepository,
56+
userRepository,
57+
gitHubProperties,
58+
gitLabProperties,
59+
featureFlagService
60+
);
61+
62+
when(workspaceRepository.findByStatusAndIsPubliclyViewableTrue(Workspace.WorkspaceStatus.ACTIVE)).thenReturn(
63+
List.of(zuluWorkspace, alphaWorkspace)
64+
);
65+
when(workspaceMembershipRepository.findByUser_Id(42L)).thenReturn(List.of(bravoMembership, alphaMembership));
66+
when(workspaceRepository.findAllById(List.of(2L, 1L))).thenReturn(List.of(bravoWorkspace, alphaWorkspace));
67+
68+
List<Workspace> workspaces = service.findAccessibleWorkspaces(Optional.of(currentUser));
69+
70+
assertThat(workspaces)
71+
.extracting(Workspace::getWorkspaceSlug)
72+
.containsExactly("alpha-space", "bravo-space", "zulu-space");
73+
}
74+
75+
@Test
76+
void findAccessibleWorkspacesSortsPublicWorkspacesForAnonymousUsers() {
77+
Workspace zuluWorkspace = workspace(3L, "zulu-space", "Zulu Workspace", true);
78+
Workspace alphaWorkspace = workspace(1L, "alpha-space", "Alpha Workspace", true);
79+
80+
WorkspaceQueryService service = new WorkspaceQueryService(
81+
workspaceRepository,
82+
workspaceMembershipRepository,
83+
repositoryToMonitorRepository,
84+
userRepository,
85+
gitHubProperties,
86+
gitLabProperties,
87+
featureFlagService
88+
);
89+
90+
when(workspaceRepository.findByStatusAndIsPubliclyViewableTrue(Workspace.WorkspaceStatus.ACTIVE)).thenReturn(
91+
List.of(zuluWorkspace, alphaWorkspace)
92+
);
93+
94+
List<Workspace> workspaces = service.findAccessibleWorkspaces(Optional.empty());
95+
96+
assertThat(workspaces).extracting(Workspace::getWorkspaceSlug).containsExactly("alpha-space", "zulu-space");
97+
}
98+
99+
private Workspace workspace(Long id, String slug, String displayName, boolean publiclyViewable) {
100+
Workspace workspace = new Workspace();
101+
workspace.setId(id);
102+
workspace.setWorkspaceSlug(slug);
103+
workspace.setDisplayName(displayName);
104+
workspace.setIsPubliclyViewable(publiclyViewable);
105+
workspace.setStatus(Workspace.WorkspaceStatus.ACTIVE);
106+
return workspace;
107+
}
108+
109+
private WorkspaceMembership membership(Workspace workspace) {
110+
WorkspaceMembership membership = new WorkspaceMembership();
111+
membership.setWorkspace(workspace);
112+
membership.setId(new WorkspaceMembership.Id(workspace.getId(), 42L));
113+
return membership;
114+
}
115+
}

‎webapp/src/hooks/use-active-workspace.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { useWorkspaceStore } from "@/stores/workspace-store";
1313
* this hook automatically redirects to another available workspace.
1414
*/
1515
export function useActiveWorkspaceSlug() {
16-
const { selectedSlug, setSelectedSlug } = useWorkspaceStore();
16+
const { selectedSlug, hasHydrated, setSelectedSlug } = useWorkspaceStore();
1717
const { isAuthenticated, isLoading: authLoading } = useAuth();
1818
const navigate = useNavigate();
1919
const query = useQuery({
@@ -25,6 +25,7 @@ export function useActiveWorkspaceSlug() {
2525
});
2626
const workspaces = Array.isArray(query.data) ? query.data : [];
2727
const location = useLocation();
28+
const workspaceSelectionLoading = isAuthenticated && !authLoading && !hasHydrated;
2829

2930
// Track if we've already attempted a redirect to prevent infinite loops
3031
const hasAttemptedRedirect = useRef(false);
@@ -42,6 +43,10 @@ export function useActiveWorkspaceSlug() {
4243
return isValidSlug(slugFromPath) ? slugFromPath : undefined;
4344
}
4445

46+
if (!hasHydrated) {
47+
return undefined;
48+
}
49+
4550
if (isValidSlug(selectedSlug)) {
4651
return selectedSlug;
4752
}
@@ -107,10 +112,10 @@ export function useActiveWorkspaceSlug() {
107112
// so we must wait until we have workspace data before syncing.
108113
useEffect(() => {
109114
const workspacesLoaded = !query.isLoading && query.data !== undefined;
110-
if (workspacesLoaded && activeSlug !== selectedSlug) {
115+
if (hasHydrated && workspacesLoaded && activeSlug !== selectedSlug) {
111116
setSelectedSlug(activeSlug);
112117
}
113-
}, [activeSlug, selectedSlug, setSelectedSlug, query.isLoading, query.data]);
118+
}, [activeSlug, hasHydrated, selectedSlug, setSelectedSlug, query.isLoading, query.data]);
114119

115120
// Clear persisted selection when the user logs out to avoid cross-user leakage
116121
useEffect(() => {
@@ -126,7 +131,7 @@ export function useActiveWorkspaceSlug() {
126131
workspaces,
127132
providerType: activeWorkspace?.providerType ?? "GITHUB",
128133
selectWorkspace: setSelectedSlug,
129-
isLoading: query.isLoading,
134+
isLoading: query.isLoading || workspaceSelectionLoading,
130135
error: query.error as Error | null,
131136
};
132137
}

‎webapp/src/routes/_authenticated/-index.test.tsx‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { render, waitFor } from "@testing-library/react";
2+
import { afterEach, describe, expect, it, vi } from "vitest";
3+
4+
const mockNavigate = vi.fn();
5+
const mockUseActiveWorkspaceSlug = vi.fn();
6+
const mockUseAuth = vi.fn();
7+
8+
vi.mock("@tanstack/react-router", () => ({
9+
createFileRoute: () => (options: unknown) => options,
10+
useNavigate: () => mockNavigate,
11+
}));
12+
13+
vi.mock("@/hooks/use-active-workspace", () => ({
14+
useActiveWorkspaceSlug: () => mockUseActiveWorkspaceSlug(),
15+
}));
16+
17+
vi.mock("@/integrations/auth/AuthContext", () => ({
18+
useAuth: () => mockUseAuth(),
19+
}));
20+
21+
vi.mock("@/components/workspace/NoWorkspace", () => ({
22+
NoWorkspace: () => <div>No Workspace</div>,
23+
}));
24+
25+
import { RedirectToWorkspace } from "./index";
26+
27+
describe("RedirectToWorkspace", () => {
28+
afterEach(() => {
29+
vi.clearAllMocks();
30+
});
31+
32+
it("redirects authenticated users to the last selected workspace", async () => {
33+
const selectWorkspace = vi.fn();
34+
35+
mockUseAuth.mockReturnValue({
36+
isAuthenticated: true,
37+
});
38+
mockUseActiveWorkspaceSlug.mockReturnValue({
39+
workspaceSlug: "prompt-edu",
40+
workspaces: [{ workspaceSlug: "ls1intum" }, { workspaceSlug: "prompt-edu" }],
41+
selectWorkspace,
42+
isLoading: false,
43+
});
44+
45+
render(<RedirectToWorkspace />);
46+
47+
await waitFor(() => {
48+
expect(selectWorkspace).toHaveBeenCalledWith("prompt-edu");
49+
expect(mockNavigate).toHaveBeenCalledWith({
50+
to: "/w/$workspaceSlug",
51+
params: { workspaceSlug: "prompt-edu" },
52+
replace: true,
53+
});
54+
});
55+
});
56+
57+
it("waits for workspace selection hydration before redirecting", () => {
58+
const selectWorkspace = vi.fn();
59+
60+
mockUseAuth.mockReturnValue({
61+
isAuthenticated: true,
62+
});
63+
mockUseActiveWorkspaceSlug.mockReturnValue({
64+
workspaceSlug: undefined,
65+
workspaces: [{ workspaceSlug: "ls1intum" }],
66+
selectWorkspace,
67+
isLoading: true,
68+
});
69+
70+
render(<RedirectToWorkspace />);
71+
72+
expect(selectWorkspace).not.toHaveBeenCalled();
73+
expect(mockNavigate).not.toHaveBeenCalled();
74+
});
75+
});

‎webapp/src/routes/_authenticated/index.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const Route = createFileRoute("/_authenticated/")({
88
component: RedirectToWorkspace,
99
});
1010

11-
function RedirectToWorkspace() {
11+
export function RedirectToWorkspace() {
1212
const navigate = useNavigate();
1313
const { isAuthenticated } = useAuth();
1414
const { workspaceSlug, workspaces, selectWorkspace, isLoading } = useActiveWorkspaceSlug();

‎webapp/src/stores/workspace-store.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
2+
import { useWorkspaceStore } from "./workspace-store";
3+
4+
const STORAGE_KEY = "hephaestus-workspace-selection";
5+
6+
describe("workspace-store", () => {
7+
beforeEach(() => {
8+
localStorage.clear();
9+
useWorkspaceStore.setState({ selectedSlug: undefined, hasHydrated: false });
10+
});
11+
12+
afterEach(() => {
13+
localStorage.clear();
14+
useWorkspaceStore.setState({ selectedSlug: undefined, hasHydrated: false });
15+
});
16+
17+
it("persists the selected workspace slug in localStorage", () => {
18+
useWorkspaceStore.getState().setSelectedSlug("prompt-edu");
19+
20+
const persisted = JSON.parse(localStorage.getItem(STORAGE_KEY) ?? "{}");
21+
22+
expect(persisted.state).toEqual({ selectedSlug: "prompt-edu" });
23+
});
24+
25+
it("rehydrates the selected workspace slug from localStorage", async () => {
26+
localStorage.setItem(
27+
STORAGE_KEY,
28+
JSON.stringify({
29+
state: { selectedSlug: "prompt-edu" },
30+
version: 0,
31+
}),
32+
);
33+
34+
await useWorkspaceStore.persist.rehydrate();
35+
36+
expect(useWorkspaceStore.getState().selectedSlug).toBe("prompt-edu");
37+
expect(useWorkspaceStore.getState().hasHydrated).toBe(true);
38+
});
39+
});

0 commit comments

Comments
 (0)