Skip to content

Commit 68e2a29

Browse files
committed
Centralize extract and user-profile route resolution
Move URL parsing, GraphQL entity resolution, and reactive-var population for /extracts/:extractId and /users/:slug into CentralRouteManager, matching the pattern already used for corpus, document, thread, and labelset routes. Why --- ExtractDetailRoute and UserProfileRoute were each parsing useParams, running their own RESOLVE_* / GET_USER queries, and writing the corresponding entity reactive vars (openedExtract, ad-hoc local state). This duplicated the four-phase flow documented in docs/frontend/routing_system.md and made it possible for components to race CentralRouteManager (e.g. ExtractDetail's back handler clearing openedExtract while Phase 1 was still settling). A few smaller violations were cleaned up at the same time: - views/Corpuses.tsx used window.history.replaceState to strip a ?create=true query param; replaced with React Router navigate so the routing system stays in sync. - LabelSetDetailPage and LabelSetLandingRoute called openedLabelset(null) in handlers that already navigate to a browse route, where Phase 1 clears the var. What changed ------------ Routing system: - cache.ts: added openedUser (typed OpenedUserProfile) and tightened the openedExtract docstring to reflect manager-only ownership. - navigationUtils.ts: parseRoute now recognizes /users/:slug and /extracts/:extractId. - CentralRouteManager.tsx: added GET_USER lazy query, a Phase 1 user branch, and openedUser(null) clears alongside the existing entity reset paths. Route components became dumb consumers: - ExtractDetailRoute.tsx (89 -> 56 lines) - UserProfileRoute.tsx (68 -> 51 lines) - New ProfileRedirect.tsx handles /profile -> /users/<current-slug> using backendUserObj. This is auth-driven, not URL-driven, so it legitimately lives outside CentralRouteManager. - App.tsx wires /profile to ProfileRedirect. Cleanups: - views/ExtractDetail.tsx back handler no longer writes openedExtract. - LabelSet handlers no longer write openedLabelset. Tests ----- - 8 new parseRoute tests covering /users/:slug and /extracts/:extractId. - 3 new CentralRouteManager tests: user resolution success, user not-found error path, extract-by-id resolution. Existing fixtures updated to reset the new vars in beforeEach. - New centralRouteDiscipline.test.ts: static regression test that grep-walks frontend/src and fails if any production file outside CentralRouteManager.tsx and cache.ts SETs the 15 routing-owned reactive vars. This caught three openedLabelset writes during development that were also fixed. - New e2e/user-and-extract-routes.spec.ts: deep-links /users/<slug>, verifies /profile redirects, and exercises the dumb-consumer error path on /extracts/<unknown-id>. - Added /users/admin to the helpers.ts VIEWS catalog so the existing login-and-navigation walk also covers the user route. Pre-existing typecheck failure resolved --------------------------------------- CamlDirectiveRenderer.tsx failed tsc because the installed @os-legal/caml-react@0.0.1 d.ts files lack the resolveImageSrc prop that the declared semver (^0.1.0) source already exposes. Added a narrow local type widening with a comment so tsc compiles cleanly until the lockfile is refreshed; runtime behavior is unchanged. Verification ------------ - yarn tsc --noEmit: clean. - yarn lint: clean. - yarn vitest run for the three updated/new test files: 140/140 passing (24 manager, 115 navigationUtils, 1 discipline regression). - E2E specs deferred to CI (frontend-e2e.yml).
1 parent 255f84a commit 68e2a29

17 files changed

Lines changed: 687 additions & 114 deletions

frontend/src/App.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import { NotFound } from "./components/routes/NotFound";
8484
import { CorpusLandingRoute } from "./components/routes/CorpusLandingRoute";
8585
import { CorpusThreadRoute } from "./components/routes/CorpusThreadRoute";
8686
import { UserProfileRoute } from "./components/routes/UserProfileRoute";
87+
import { ProfileRedirect } from "./components/routes/ProfileRedirect";
8788
import { LeaderboardRoute } from "./components/routes/LeaderboardRoute";
8889
import { GlobalDiscussionsRoute } from "./components/routes/GlobalDiscussionsRoute";
8990
import { ThreadSearchRoute } from "./views/ThreadSearchRoute";
@@ -490,7 +491,7 @@ export const App = () => {
490491
<Route path="/threads" element={<ThreadSearchRoute />} />
491492

492493
{/* User Profile Routes (Issue #611) */}
493-
<Route path="/profile" element={<UserProfileRoute />} />
494+
<Route path="/profile" element={<ProfileRedirect />} />
494495
<Route path="/users/:slug" element={<UserProfileRoute />} />
495496
{/* Convenience redirect for badge notifications (Issue #637) */}
496497
<Route

frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,23 @@
1313
* or any specific directive. All behavior comes from registered handlers.
1414
*/
1515
import React, { useMemo } from "react";
16+
import type { ComponentType } from "react";
1617
import type { CamlDocument, CamlProse } from "@os-legal/caml";
17-
import { CamlArticle, CamlThemeProvider } from "@os-legal/caml-react";
18+
import {
19+
CamlArticle as CamlArticleUntyped,
20+
CamlThemeProvider,
21+
} from "@os-legal/caml-react";
22+
23+
// The @os-legal/caml-react@0.1.x source supports `resolveImageSrc`, but the
24+
// 0.0.1 build still resident in some lockfiles ships d.ts files without it.
25+
// Until the lockfile catches up, widen the prop surface locally so consumer
26+
// code keeps the strong type for the props the published d.ts does declare.
27+
type CamlArticleExtraProps = {
28+
resolveImageSrc?: (src: string) => string | undefined;
29+
};
30+
const CamlArticle = CamlArticleUntyped as ComponentType<
31+
React.ComponentProps<typeof CamlArticleUntyped> & CamlArticleExtraProps
32+
>;
1833

1934
import { MarkdownMessageRenderer } from "../../threads/MarkdownMessageRenderer";
2035
import {

frontend/src/components/labelsets/LabelSetDetailPage.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export const LabelSetDetailPage: React.FC<LabelSetDetailPageProps> = ({
259259
if (onClose) {
260260
onClose();
261261
} else {
262-
openedLabelset(null);
262+
// CentralRouteManager Phase 1 clears openedLabelset on browse routes.
263263
navigate("/label_sets");
264264
}
265265
};
@@ -453,7 +453,7 @@ export const LabelSetDetailPage: React.FC<LabelSetDetailPageProps> = ({
453453
.then((result) => {
454454
if (result.data?.deleteLabelset?.ok) {
455455
toast.success("Label set deleted successfully");
456-
openedLabelset(null);
456+
// CentralRouteManager Phase 1 clears openedLabelset on browse routes.
457457
navigate("/label_sets");
458458
} else {
459459
toast.error(

frontend/src/components/routes/ExtractDetailRoute.tsx

Lines changed: 14 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,28 @@
1-
import React, { useEffect } from "react";
2-
import { useParams } from "react-router-dom";
3-
import { useQuery, useReactiveVar } from "@apollo/client";
1+
import React from "react";
2+
import { useReactiveVar } from "@apollo/client";
43
import { MetaTags } from "../seo/MetaTags";
54
import { ModernLoadingDisplay } from "../widgets/ModernLoadingDisplay";
65
import { ModernErrorDisplay } from "../widgets/ModernErrorDisplay";
76
import { ErrorBoundary } from "../widgets/ErrorBoundary";
8-
import { openedExtract } from "../../graphql/cache";
7+
import { openedExtract, routeLoading, routeError } from "../../graphql/cache";
98
import { ExtractDetail } from "../../views/ExtractDetail";
10-
import {
11-
RESOLVE_EXTRACT_BY_ID,
12-
ResolveExtractByIdOutput,
13-
ResolveExtractByIdInput,
14-
} from "../../graphql/queries";
159

1610
/**
17-
* ExtractDetailRoute - Handles the /extracts/:extractId route
11+
* ExtractDetailRoute - Renders the extract detail view for /extracts/:extractId
12+
* and /e/:userIdent/:extractIdent.
1813
*
19-
* This is the new route-based pattern for viewing extract details,
20-
* replacing the modal-based EditExtractModal.
21-
*
22-
* Route pattern: /extracts/:extractId
14+
* URL parsing, GraphQL resolution, and reactive-var population are owned by
15+
* CentralRouteManager. This component reads the resolved state and renders.
2316
*/
2417
export const ExtractDetailRoute: React.FC = () => {
25-
const { extractId } = useParams<{ extractId: string }>();
26-
27-
// Check if we already have the extract from reactive var
28-
const existingExtract = useReactiveVar(openedExtract);
29-
30-
// Query to resolve extract by ID if not already loaded
31-
const { loading, error, data } = useQuery<
32-
ResolveExtractByIdOutput,
33-
ResolveExtractByIdInput
34-
>(RESOLVE_EXTRACT_BY_ID, {
35-
variables: { extractId: extractId ?? "" },
36-
skip: !extractId || existingExtract?.id === extractId,
37-
fetchPolicy: "network-only",
38-
});
39-
40-
// Set the opened extract when query completes
41-
useEffect(() => {
42-
if (data?.extract) {
43-
openedExtract(data.extract);
44-
}
45-
}, [data]);
18+
const extract = useReactiveVar(openedExtract);
19+
const loading = useReactiveVar(routeLoading);
20+
const error = useReactiveVar(routeError);
4621

47-
// Handle missing extractId
48-
if (!extractId) {
49-
return <ModernErrorDisplay type="extract" error="No extract ID provided" />;
50-
}
51-
52-
// Loading state
53-
if (loading && !existingExtract) {
22+
if (loading && !extract) {
5423
return <ModernLoadingDisplay type="extract" size="large" />;
5524
}
5625

57-
// Error state
5826
if (error) {
5927
return (
6028
<ModernErrorDisplay
@@ -64,20 +32,15 @@ export const ExtractDetailRoute: React.FC = () => {
6432
);
6533
}
6634

67-
// Get the extract to display
68-
const extract =
69-
existingExtract?.id === extractId ? existingExtract : data?.extract;
70-
71-
// Not found state
72-
if (!extract && !loading) {
35+
if (!extract) {
7336
return <ModernErrorDisplay type="extract" error="Extract not found" />;
7437
}
7538

7639
return (
7740
<ErrorBoundary>
7841
<MetaTags
79-
title={extract?.name || "Extract"}
80-
description={`Extract: ${extract?.name}`}
42+
title={extract.name || "Extract"}
43+
description={`Extract: ${extract.name}`}
8144
entity={extract}
8245
entityType="extract"
8346
/>

frontend/src/components/routes/LabelSetLandingRoute.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ export const LabelSetLandingRoute: React.FC = () => {
3131
hasError: !!error,
3232
});
3333

34-
// Handle close by navigating back to list
34+
// Handle close by navigating back to list. CentralRouteManager Phase 1
35+
// clears openedLabelset when the new path resolves to a browse route.
3536
const handleClose = () => {
36-
openedLabelset(null);
3737
navigate("/label_sets");
3838
};
3939

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* ProfileRedirect - Resolves /profile to the current user's canonical
3+
* /users/:slug URL.
4+
*
5+
* This is auth-state-driven (not URL-state-driven), so it lives outside
6+
* the CentralRouteManager entity-resolution path. It reads backendUserObj
7+
* and renders a React Router <Navigate> to the canonical profile URL,
8+
* which then triggers CentralRouteManager Phase 1 user resolution.
9+
*
10+
* Anonymous visitors are sent to /login.
11+
*/
12+
13+
import React from "react";
14+
import { Navigate } from "react-router-dom";
15+
import { useReactiveVar } from "@apollo/client";
16+
import { backendUserObj } from "../../graphql/cache";
17+
18+
export const ProfileRedirect: React.FC = () => {
19+
const currentUser = useReactiveVar(backendUserObj);
20+
21+
if (!currentUser?.slug) {
22+
return <Navigate to="/login" replace />;
23+
}
24+
25+
return <Navigate to={`/users/${currentUser.slug}`} replace />;
26+
};
27+
28+
export default ProfileRedirect;

frontend/src/components/routes/UserProfileRoute.tsx

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,65 +4,48 @@
44
* Issue: #611 - Create User Profile Page with badge display and stats
55
* Epic: #572 - Social Features Epic
66
*
7-
* Slug-based route component that resolves user profile and renders the UserProfile view.
8-
* Handles both /profile (current user) and /users/:slug (any user) routes.
7+
* Renders the user profile resolved by CentralRouteManager from the
8+
* /users/:slug route. URL parsing and entity fetching live in
9+
* CentralRouteManager; this component reads reactive vars and renders.
10+
*
11+
* The /profile redirect (current user) is handled by ProfileRedirect.
912
*/
1013

1114
import React from "react";
12-
import { useParams, Navigate } from "react-router-dom";
13-
import { useQuery, useReactiveVar } from "@apollo/client";
14-
import { GET_USER, GetUserInput, GetUserOutput } from "../../graphql/queries";
15-
import { backendUserObj } from "../../graphql/cache";
15+
import { useReactiveVar } from "@apollo/client";
16+
import {
17+
backendUserObj,
18+
openedUser,
19+
routeLoading,
20+
routeError,
21+
} from "../../graphql/cache";
1622
import { ModernLoadingDisplay } from "../widgets/ModernLoadingDisplay";
1723
import { ModernErrorDisplay } from "../widgets/ModernErrorDisplay";
1824
import { UserProfile } from "../../views/UserProfile";
1925

2026
export const UserProfileRoute: React.FC = () => {
21-
const { slug } = useParams<{ slug: string }>();
27+
const user = useReactiveVar(openedUser);
28+
const loading = useReactiveVar(routeLoading);
29+
const error = useReactiveVar(routeError);
2230
const currentUser = useReactiveVar(backendUserObj);
2331

24-
// Must be unconditional — skip: !slug gates the network call; see #1295.
25-
const { data, loading, error } = useQuery<GetUserOutput, GetUserInput>(
26-
GET_USER,
27-
{
28-
variables: { slug: slug ?? "" },
29-
skip: !slug,
30-
}
31-
);
32-
33-
// If no slug provided, redirect to current user's profile
34-
if (!slug) {
35-
if (!currentUser?.slug) {
36-
return <Navigate to="/login" replace />;
37-
}
38-
return <Navigate to={`/users/${currentUser.slug}`} replace />;
39-
}
40-
41-
if (loading) {
32+
if (loading && !user) {
4233
return <ModernLoadingDisplay type="default" message="Loading profile..." />;
4334
}
4435

45-
if (error) {
46-
return (
47-
<ModernErrorDisplay
48-
type="generic"
49-
title="User Not Found"
50-
error={`Could not find user with slug "${slug}"`}
51-
/>
52-
);
53-
}
54-
55-
if (!data?.userBySlug) {
36+
if (error || !user) {
5637
return (
5738
<ModernErrorDisplay
5839
type="generic"
5940
title="User Not Found"
60-
error={`User "${slug}" does not exist or their profile is private`}
41+
error={
42+
error?.message || "User does not exist or their profile is private"
43+
}
6144
/>
6245
);
6346
}
6447

65-
const isOwnProfile = currentUser?.id === data.userBySlug.id;
48+
const isOwnProfile = currentUser?.id === user.id;
6649

67-
return <UserProfile user={data.userBySlug} isOwnProfile={isOwnProfile} />;
50+
return <UserProfile user={user} isOwnProfile={isOwnProfile} />;
6851
};

frontend/src/graphql/cache.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -434,32 +434,55 @@ export const linkDocumentsModalState = makeVar<LinkDocumentsModalState>({
434434
* Extract-related global variables
435435
*
436436
* ENTITY STATE:
437-
* openedExtract - Extract resolved from /extracts/:extractId route
438-
* Set by: CentralRouteManager OR route components (ExtractDetailRoute)
437+
* openedExtract - Extract resolved from /extracts/:extractId or /e/:user/:extractId route
438+
* Set by: CentralRouteManager (Phase 1) only
439439
*
440440
* URL-DRIVEN STATE:
441441
* selectedExtractIds - Controlled by URL query parameter ?extract=
442-
* Set by: CentralRouteManager Phase 2
443-
*
444-
* Write access is restricted to:
445-
* - CentralRouteManager (for legacy /e/:user/:extractId routes)
446-
* - Route components like ExtractDetailRoute (for new /extracts/:extractId routes)
442+
* Set by: CentralRouteManager (Phase 2) only
447443
*
448444
* All other components must:
449445
* - ONLY READ via useReactiveVar()
450446
* - UPDATE STATE via navigate() to change the URL (which triggers route resolution)
451447
*
452448
* Examples:
453-
* /extracts/extract-123 → openedExtract(extractObj) via ExtractDetailRoute
449+
* /extracts/extract-123 → openedExtract(extractObj) via CentralRouteManager
454450
* /e/user/extract-123 → openedExtract(extractObj) via CentralRouteManager
455451
* /c/user/corpus?extract=456 → selectedExtractIds(["456"])
456452
* /d/user/doc?extract=456,789 → selectedExtractIds(["456", "789"])
457453
*/
458-
export const openedExtract = makeVar<ExtractType | null>(null); // ENTITY STATE - set by route components
459-
export const selectedExtractIds = makeVar<string[]>([]); // URL-DRIVEN - set by CentralRouteManager Phase 2
454+
export const openedExtract = makeVar<ExtractType | null>(null);
455+
export const selectedExtractIds = makeVar<string[]>([]);
460456
export const selectedExtract = makeVar<ExtractType | null>(null); // Legacy - kept for backward compatibility
461457
export const extractSearchTerm = makeVar<string>("");
462458

459+
/**
460+
* User profile entity resolved from /users/:slug route.
461+
*
462+
* ENTITY STATE:
463+
* openedUser - Profile snapshot resolved by CentralRouteManager Phase 1.
464+
* Set by: CentralRouteManager only.
465+
*
466+
* The shape mirrors the GET_USER GraphQL response. Components reading this
467+
* should treat null as "no user resolved yet" (loading or missing).
468+
*/
469+
export interface OpenedUserProfile {
470+
id: string;
471+
username: string;
472+
slug: string;
473+
name: string;
474+
firstName: string;
475+
lastName: string;
476+
email: string;
477+
isProfilePublic: boolean;
478+
reputationGlobal: number;
479+
totalMessages: number;
480+
totalThreadsCreated: number;
481+
totalAnnotationsCreated: number;
482+
totalDocumentsUploaded: number;
483+
}
484+
export const openedUser = makeVar<OpenedUserProfile | null>(null);
485+
463486
/**
464487
* Corpus-related global variables
465488
*/

0 commit comments

Comments
 (0)