Skip to content

Commit 030f9f5

Browse files
ConnorYohConnorYoh
authored
feat(editor): adopt TanStack Query, convert three read-only fetches (#7264)
# Description of Changes ## The problem The editor has no query client. ~295 `apiClient` call sites, each mount refetching what the last one just got, and three module-level caches reimplementing dedupe, retry and invalidation by hand — each shaped differently. The Processor (`frontend/editor/src/portal`) has run on TanStack Query since #7135. The editor never got it. ## End state The editor has a query client, and the three read-only fetch sites that convert safely now use it. `@tanstack/react-query` is already a dependency — no new package. **Foundation** | File | | |---|---| | `core/query/queryClient.ts` | `baseQueryOptions` + client factory. The portal now builds its client from the same options. `networkMode: "always"` — `navigator.onLine` describes internet reachability, which says nothing about a bundled backend on 127.0.0.1 or a self-hosted server on the LAN. | | `core/query/keys.ts` | `["editor", resource, ...params]` | | `core/query/staleTime.ts` + `desktop/query/staleTime.ts` | Config staleTime: `Infinity` on web, 5 min on desktop | | `core/api/config.ts`, `core/api/users.ts` | Fetch functions, mirroring `portal/api/*` | | `core/tests/utils/TestQueryProvider.tsx` | | | `desktop/components/DesktopQueryCacheReset.tsx` | | `QueryClientProvider` mounts at the top of `core/components/AppProviders.tsx`. That diff looks large but is one wrapper plus the reindent underneath it. **Converted.** All three keep their existing return shape, so no consumer changes. | | Before | |---|---| | `useFooterInfo` | Fetched twice — Footer and admin legal section | | `useGroupEnabled` | Refetched on every mount | | `UserSelector` | Refetched the whole roster on each of two mount sites, and again whenever `t` or `user` changed identity | **Desktop needs more than the provider.** `operationRouter` resolves the same relative path to the local bundled backend, a self-hosted server, or the SaaS backend. Query caches by key, not by resolved URL, so a cached entry can outlive the backend that filled it. `group-enabled` routes this way, so this PR introduces the hazard and carries the fix: `DesktopQueryCacheReset` calls `resetQueries()` when the connection mode changes or the self-hosted server goes up or down, and `CONFIG_STALE_TIME` is finite on desktop as a backstop. **Behaviour changes** - All three sites now retry once on failure (client default). None retried before, so a failing request sits in `loading` for one extra attempt plus backoff. - `staleTime: Infinity` on web means admin edits to legal links no longer appear on remount within a session. Saving those already prompts a restart, so this is accepted rather than incidental. - Desktop `useGroupEnabled` shows the *translated* offline reason on first render. The old code showed raw English for one render. - `UserSelector` drops three `console.log`s that were dumping user records to the console. ## Decisions **1. The foundation doesn't ship alone.** A provider nothing consumes gives a reviewer nothing to react to and rots if the follow-up stalls, so it lands with the cheapest safe conversions. **2. Hooks keep their existing return shape.** The alternative is switching to `{ data, isPending, error }` and updating consumers now. Cost of my choice: we carry a `loading`-shaped façade indefinitely, and consumers don't get `isFetching`/`refetch` without a second pass. Taken because it's what keeps each later migration a one-file diff. **3. Shared defaults, separate instances.** The editor and the Processor mount as *sibling* routes, not nested — they never coexist in one tree. Both clients now come from the same `baseQueryOptions`, so behaviour can't drift. A single shared instance would only buy cache surviving navigation between the two products, which is worth little while they share no keys, and it breaks the contract three portal tests rely on (`createPortalQueryClient()` returning a fresh client per test). That belongs in the collapse PR. Consequence meanwhile: the desktop reset covers the editor client only — harmless, since the portal isn't in desktop builds. **4. The desktop reset is wholesale.** A mode switch already remounts the SaaS provider tree, so there's nothing to preserve, and an allowlist of "mode-sensitive" keys would be a trap every new query has to remember to join. ## Coming next Ordered by consumers per line changed. | PR | Scope | |---|---| | 2 | `AppConfigContext` + `useEndpointConfig` — ~80 consumers, deletes ~200 lines of hand-rolled cache, retry and dedupe | | 3 | `useAdminSettings` (20 consumers) and the config sections | | 4 | Polling loops → `refetchInterval` | | 5 | Finish the Processor's remaining files, collapse to one client | | 6 | Tool execution — mutation state only, narrowly scoped | Not in scope, deliberately: `usePdfLibLinks` (its cache is a refcounted ArrayBuffer lifetime manager), thumbnail hooks, watched-folder IndexedDB reads, the desktop health monitors. Unifying `endpointAvailabilityService` / `saasAppConfigService` with the query cache would mean handing `operationRouter` a query client — its own PR if a second reason appears. ## Testing `task frontend:check` green: 1666 tests across 191 files, typecheck on all five flavours, eslint `--max-warnings=0`, dpdm, prettier. New tests cover request de-duplication, per-group key isolation, the desktop offline short-circuit, and the cache reset. The reset test was verified to fail against the `clear()` implementation it replaced. `UserSelector` has no test beyond its existing stories. One existing test needed a wrapper: `Login.test.tsx` renders `<Login />` in isolation, and `AuthLayout` → `Footer` → `useFooterInfo` now needs a client. The real `/login` route is already inside `AppProviders`, so this is test isolation, not a runtime gap. Rollback is a clean revert — nothing persists outside the React tree.
1 parent 921bdac commit 030f9f5

21 files changed

Lines changed: 638 additions & 272 deletions

‎frontend/editor/src/core/api/config.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import apiClient from "@app/services/apiClient";
2+
3+
export interface FooterInfo {
4+
analyticsEnabled?: boolean;
5+
termsAndConditions?: string;
6+
privacyPolicy?: string;
7+
accessibilityStatement?: string;
8+
cookiePolicy?: string;
9+
impressum?: string;
10+
}
11+
12+
/** Public — no authentication required. */
13+
export async function fetchFooterInfo(): Promise<FooterInfo> {
14+
try {
15+
const response = await apiClient.get<FooterInfo>(
16+
"/api/v1/ui-data/footer-info",
17+
{ suppressErrorToast: true },
18+
);
19+
return response.data;
20+
} catch (error) {
21+
// Toasts are suppressed here, so the failure would otherwise be silent.
22+
console.error("[api/config] footer-info failed:", error);
23+
throw error;
24+
}
25+
}
26+
27+
export async function fetchGroupEnabled(group: string): Promise<boolean> {
28+
const response = await apiClient.get<boolean>(
29+
`/api/v1/config/group-enabled?group=${encodeURIComponent(group)}`,
30+
);
31+
return response.data;
32+
}

‎frontend/editor/src/core/api/users.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import apiClient from "@app/services/apiClient";
2+
import { UserSummary } from "@app/types/signingSession";
3+
4+
export async function fetchUsers(): Promise<UserSummary[]> {
5+
const response = await apiClient.get<UserSummary[]>("/api/v1/user/users");
6+
// A proxy can answer 200 with an HTML login page; callers assume an array.
7+
return Array.isArray(response.data) ? response.data : [];
8+
}

‎frontend/editor/src/core/components/AppProviders.tsx‎

Lines changed: 69 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { ReactNode, useEffect } from "react";
1+
import { ReactNode, useEffect, useState } from "react";
2+
import { QueryClientProvider } from "@tanstack/react-query";
3+
import { createAppQueryClient } from "@app/query/queryClient";
24
import { ThemeProvider } from "@app/components/shared/ThemeProvider";
35
import { FileContextProvider } from "@app/contexts/FileContext";
46
import { NavigationProvider } from "@app/contexts/NavigationContext";
@@ -119,70 +121,73 @@ export function AppProviders({
119121
appConfigRetryOptions,
120122
appConfigProviderProps,
121123
}: AppProvidersProps) {
124+
const [queryClient] = useState(createAppQueryClient);
122125
return (
123-
<PreferencesProvider>
124-
<ThemeProvider>
125-
<ErrorBoundary>
126-
<BannerProvider>
127-
<AppConfigProvider
128-
retryOptions={appConfigRetryOptions}
129-
{...appConfigProviderProps}
130-
>
131-
<PosthogTrackingInitializer />
132-
<ScarfTrackingInitializer />
133-
<AppConfigLoader />
134-
<ServerDefaultsSync />
135-
{/* Auto-popup on startup when a newer Stirling-PDF release is available.
136-
No-ops inside Tauri — the desktop popup handles that flow. */}
137-
<UpdateStartupPopup />
138-
<FileContextProvider
139-
enableUrlSync={true}
140-
enablePersistence={true}
126+
<QueryClientProvider client={queryClient}>
127+
<PreferencesProvider>
128+
<ThemeProvider>
129+
<ErrorBoundary>
130+
<BannerProvider>
131+
<AppConfigProvider
132+
retryOptions={appConfigRetryOptions}
133+
{...appConfigProviderProps}
141134
>
142-
<FolderProvider>
143-
<AppInitializer />
144-
<BrandingAssetManager />
145-
<ToolRegistryProvider>
146-
<NavigationProvider>
147-
<FilesModalProvider>
148-
<ToolWorkflowProvider>
149-
<HotkeyProvider>
150-
<SidebarProvider>
151-
<ViewerProvider>
152-
<PageEditorProvider>
153-
<SignatureProvider>
154-
<SigningOverlayProvider>
155-
<RedactionProvider>
156-
<FormFillProvider>
157-
<AnnotationProvider>
158-
<WorkbenchBarProvider>
159-
<TourOrchestrationProvider>
160-
<AdminTourOrchestrationProvider>
161-
<FolderFileContextProvider>
162-
{children}
163-
</FolderFileContextProvider>
164-
</AdminTourOrchestrationProvider>
165-
</TourOrchestrationProvider>
166-
</WorkbenchBarProvider>
167-
</AnnotationProvider>
168-
</FormFillProvider>
169-
</RedactionProvider>
170-
</SigningOverlayProvider>
171-
</SignatureProvider>
172-
</PageEditorProvider>
173-
</ViewerProvider>
174-
</SidebarProvider>
175-
</HotkeyProvider>
176-
</ToolWorkflowProvider>
177-
</FilesModalProvider>
178-
</NavigationProvider>
179-
</ToolRegistryProvider>
180-
</FolderProvider>
181-
</FileContextProvider>
182-
</AppConfigProvider>
183-
</BannerProvider>
184-
</ErrorBoundary>
185-
</ThemeProvider>
186-
</PreferencesProvider>
135+
<PosthogTrackingInitializer />
136+
<ScarfTrackingInitializer />
137+
<AppConfigLoader />
138+
<ServerDefaultsSync />
139+
{/* Auto-popup on startup when a newer Stirling-PDF release is available.
140+
No-ops inside Tauri — the desktop popup handles that flow. */}
141+
<UpdateStartupPopup />
142+
<FileContextProvider
143+
enableUrlSync={true}
144+
enablePersistence={true}
145+
>
146+
<FolderProvider>
147+
<AppInitializer />
148+
<BrandingAssetManager />
149+
<ToolRegistryProvider>
150+
<NavigationProvider>
151+
<FilesModalProvider>
152+
<ToolWorkflowProvider>
153+
<HotkeyProvider>
154+
<SidebarProvider>
155+
<ViewerProvider>
156+
<PageEditorProvider>
157+
<SignatureProvider>
158+
<SigningOverlayProvider>
159+
<RedactionProvider>
160+
<FormFillProvider>
161+
<AnnotationProvider>
162+
<WorkbenchBarProvider>
163+
<TourOrchestrationProvider>
164+
<AdminTourOrchestrationProvider>
165+
<FolderFileContextProvider>
166+
{children}
167+
</FolderFileContextProvider>
168+
</AdminTourOrchestrationProvider>
169+
</TourOrchestrationProvider>
170+
</WorkbenchBarProvider>
171+
</AnnotationProvider>
172+
</FormFillProvider>
173+
</RedactionProvider>
174+
</SigningOverlayProvider>
175+
</SignatureProvider>
176+
</PageEditorProvider>
177+
</ViewerProvider>
178+
</SidebarProvider>
179+
</HotkeyProvider>
180+
</ToolWorkflowProvider>
181+
</FilesModalProvider>
182+
</NavigationProvider>
183+
</ToolRegistryProvider>
184+
</FolderProvider>
185+
</FileContextProvider>
186+
</AppConfigProvider>
187+
</BannerProvider>
188+
</ErrorBoundary>
189+
</ThemeProvider>
190+
</PreferencesProvider>
191+
</QueryClientProvider>
187192
);
188193
}

‎frontend/editor/src/core/components/shared/UserSelector.tsx‎

Lines changed: 43 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import { useEffect, useState } from "react";
1+
import { useEffect, useMemo, useState } from "react";
22
import { useTranslation } from "react-i18next";
3+
import { useQuery } from "@tanstack/react-query";
34
import { MultiSelect, Loader, Text, Stack } from "@mantine/core";
45
import { Button } from "@app/ui/Button";
56
import { useNavigate } from "react-router-dom";
67
import { alert } from "@app/components/toast";
7-
import { UserSummary } from "@app/types/signingSession";
8-
import apiClient from "@app/services/apiClient";
8+
import { fetchUsers } from "@app/api/users";
99
import { useAuth } from "@app/auth/UseSession";
10+
import { qk } from "@app/query/keys";
1011
import { Z_INDEX_OVER_FILE_MANAGER_MODAL } from "@app/styles/zIndex";
1112

1213
interface UserSelectorProps {
@@ -30,79 +31,58 @@ const UserSelector = ({
3031
const { t } = useTranslation();
3132
const { user } = useAuth();
3233
const navigate = useNavigate();
33-
const [selectData, setSelectData] = useState<GroupedData[]>([]);
34-
const [loading, setLoading] = useState(true);
3534
const [stringValue, setStringValue] = useState<string[]>([]);
3635

37-
useEffect(() => {
38-
const fetchUsers = async () => {
39-
try {
40-
const response = await apiClient.get("/api/v1/user/users");
41-
console.log("Users API response:", response.data);
42-
const fetchedUsers = response.data || [];
43-
44-
// Process selectData inside useEffect - group by team
45-
const usersByTeam: Record<string, SelectItem[]> = {};
46-
const currentUserId = user?.id ? parseInt(user.id, 10) : null;
36+
const {
37+
data: users,
38+
isPending: loading,
39+
error,
40+
} = useQuery({ queryKey: qk.users(), queryFn: fetchUsers });
4741

48-
fetchedUsers
49-
.filter((u: UserSummary) => u && u.userId && u.username)
50-
.filter((u: UserSummary) => u.userId !== currentUserId) // Exclude current user
51-
.filter((u: UserSummary) => u.teamName?.toLowerCase() !== "internal") // Exclude internal users
52-
.forEach((user: UserSummary) => {
53-
const teamName =
54-
user.teamName ||
55-
t("certSign.collab.userSelector.noTeam", "No Team");
56-
if (!usersByTeam[teamName]) {
57-
usersByTeam[teamName] = [];
58-
}
59-
const displayName = user.displayName || user.username || "Unknown";
60-
const username = user.username || "unknown";
61-
const label =
62-
displayName !== username
63-
? `${displayName} (@${username})`
64-
: displayName;
65-
usersByTeam[teamName].push({
66-
value: String(user.userId),
67-
label,
68-
});
69-
});
42+
useEffect(() => {
43+
if (!error) return;
44+
alert({
45+
alertType: "error",
46+
title: t("common.error"),
47+
body: t("certSign.collab.userSelector.loadError", "Failed to load users"),
48+
});
49+
}, [error, t]);
7050

71-
// Convert to Mantine's grouped format
72-
const processed: GroupedData[] = Object.entries(usersByTeam).map(
73-
([teamName, items]) => ({
74-
group: teamName,
75-
items: items.sort((a, b) => a.label.localeCompare(b.label)),
76-
}),
77-
);
51+
const selectData = useMemo<GroupedData[]>(() => {
52+
const usersByTeam: Record<string, SelectItem[]> = {};
53+
const currentUserId = user?.id ? parseInt(user.id, 10) : null;
7854

79-
console.log("Processed selectData:", processed);
80-
setSelectData(processed);
81-
} catch (error) {
82-
console.error("Failed to load users:", error);
83-
alert({
84-
alertType: "error",
85-
title: t("common.error"),
86-
body: t(
87-
"certSign.collab.userSelector.loadError",
88-
"Failed to load users",
89-
),
90-
});
91-
} finally {
92-
setLoading(false);
93-
}
94-
};
55+
(users ?? [])
56+
.filter((u) => u && u.userId && u.username)
57+
.filter((u) => u.userId !== currentUserId)
58+
.filter((u) => u.teamName?.toLowerCase() !== "internal")
59+
.forEach((u) => {
60+
const teamName =
61+
u.teamName || t("certSign.collab.userSelector.noTeam", "No Team");
62+
if (!usersByTeam[teamName]) {
63+
usersByTeam[teamName] = [];
64+
}
65+
const displayName = u.displayName || u.username || "Unknown";
66+
const username = u.username || "unknown";
67+
const label =
68+
displayName !== username
69+
? `${displayName} (@${username})`
70+
: displayName;
71+
usersByTeam[teamName].push({ value: String(u.userId), label });
72+
});
9573

96-
fetchUsers();
97-
}, [t, user]);
74+
return Object.entries(usersByTeam).map(([teamName, items]) => ({
75+
group: teamName,
76+
items: items.sort((a, b) => a.label.localeCompare(b.label)),
77+
}));
78+
}, [users, user, t]);
9879

9980
// Process stringValue when value prop changes
10081
useEffect(() => {
10182
const safeValue = Array.isArray(value) ? value : [];
10283
const result = safeValue
10384
.map((id) => (id != null ? id.toString() : ""))
10485
.filter(Boolean);
105-
console.log("stringValue for MultiSelect:", result);
10686
setStringValue(result);
10787
}, [value]);
10888

‎frontend/editor/src/core/hooks/useFooterInfo.test.tsx‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { renderHook, waitFor } from "@testing-library/react";
3+
import { TestQueryProvider } from "@app/tests/utils/TestQueryProvider";
4+
import { useFooterInfo } from "@app/hooks/useFooterInfo";
5+
import { fetchFooterInfo } from "@app/api/config";
6+
7+
vi.mock("@app/api/config", () => ({ fetchFooterInfo: vi.fn() }));
8+
9+
const mockFetch = vi.mocked(fetchFooterInfo);
10+
11+
describe("useFooterInfo", () => {
12+
beforeEach(() => {
13+
vi.clearAllMocks();
14+
});
15+
16+
it("returns the server's footer config", async () => {
17+
mockFetch.mockResolvedValue({
18+
analyticsEnabled: true,
19+
privacyPolicy: "/privacy",
20+
});
21+
22+
const { result } = renderHook(() => useFooterInfo(), {
23+
wrapper: TestQueryProvider,
24+
});
25+
26+
expect(result.current.loading).toBe(true);
27+
await waitFor(() => expect(result.current.loading).toBe(false));
28+
expect(result.current.footerInfo).toEqual({
29+
analyticsEnabled: true,
30+
privacyPolicy: "/privacy",
31+
});
32+
});
33+
34+
it("falls back to analytics-off rather than null when the fetch fails", async () => {
35+
mockFetch.mockRejectedValue(new Error("offline"));
36+
37+
const { result } = renderHook(() => useFooterInfo(), {
38+
wrapper: TestQueryProvider,
39+
});
40+
41+
await waitFor(() => expect(result.current.error).toBeTruthy());
42+
expect(result.current.footerInfo).toEqual({ analyticsEnabled: false });
43+
});
44+
45+
it("shares one request between the footer and the legal section", async () => {
46+
mockFetch.mockResolvedValue({ analyticsEnabled: false });
47+
48+
const { result } = renderHook(
49+
() => ({ footer: useFooterInfo(), legal: useFooterInfo() }),
50+
{ wrapper: TestQueryProvider },
51+
);
52+
53+
await waitFor(() => expect(result.current.footer.loading).toBe(false));
54+
expect(mockFetch).toHaveBeenCalledTimes(1);
55+
});
56+
});

0 commit comments

Comments
 (0)