feat(webapp): add workspace error state components #629
Conversation
github-actions
bot
added
feature
|
Warning Rate limit exceeded@FelixTJDietrich has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a workspace-scoped authenticated route that loads workspace data into a WorkspaceContext; introduces accessible workspace error/empty-state components (Forbidden, NotFound, Error) with stories; migrates workspace-scoped pages to consume workspaceSlug from context; updates generated route tree and public typings. Changes
sequenceDiagram
participant Router as Router (navigate)
participant Loader as Route Loader
participant API as Workspace API
participant Provider as WorkspaceContext Provider
participant Page as Child Page (useWorkspace)
participant ErrorComp as Error/Empty Component
Router->>Loader: request /w/$workspaceSlug
Loader->>API: getWorkspace(slug)
alt success (200)
API-->>Loader: workspace data
Loader->>Provider: provide workspace → children
Provider-->>Page: useWorkspace() supplies workspaceSlug
Page->>API: run page queries using workspaceSlug
else not found (404)
API-->>Loader: 404
Loader->>ErrorComp: render WorkspaceNotFound(slug)
else forbidden (403)
API-->>Loader: 403
Loader->>ErrorComp: throw/render WorkspaceForbidden(reason/slug)
else other error
API-->>Loader: error
Loader->>ErrorComp: render WorkspaceError(error)
end
🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
the
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
21-53: Consider extracting the shared navigation pattern.Both WorkspaceForbidden and WorkspaceNotFound components share an identical
handleGoHomepattern. While the duplication is minimal, if you add more navigation logic later (e.g., analytics tracking, redirect with state), extracting a shared hook likeuseGoHome()could keep these components in sync.🔎 Optional: Extract shared navigation logic
Create a shared hook:
// hooks/useGoHome.ts export function useGoHome() { const navigate = useNavigate(); return () => navigate({ to: "/" }); }Then use it in both components:
-const navigate = useNavigate(); - -const handleGoHome = () => { - navigate({ to: "/" }); -}; +const handleGoHome = useGoHome();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/index.tswebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/integrations/workspace/index.tswebapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/integrations/workspace/index.tswebapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.test.{ts,tsx}: Write component tests with Vitest and@testing-library/react. Prefer queries that mirror user intent such asfindByRoleorgetByLabelText.
Mock network requests withvi.mockplus the generated TanStack Query helpers. Do not hand-roll fetch mocks.
Cover happy path, loading, and error outcomes in both tests and stories.
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsx
🧬 Code graph analysis (5)
webapp/src/components/workspace/WorkspaceForbidden.tsx (2)
webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsx (2)
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
WorkspaceForbidden(21-53)webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (2)
Default(33-33)WithSlug(38-42)
webapp/src/components/workspace/WorkspaceNotFound.tsx (2)
webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/integrations/workspace/mock-workspace-provider.tsx (2)
webapp/src/api/types.gen.ts (2)
Workspace(343-362)WorkspaceMembership(376-383)webapp/src/integrations/workspace/workspace-provider.tsx (2)
WorkspaceContextType(15-38)WorkspaceErrorType(13-13)
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (1)
webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
WorkspaceNotFound(21-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Agent
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: test-suite / application-server-architecture
- GitHub Check: test-suite / application-server-unit
- GitHub Check: security-scan / secrets
- GitHub Check: test-suite / webapp-visual
- GitHub Check: security-scan / sast
🔇 Additional comments (18)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsx (1)
1-43: LGTM!The Storybook stories are well-structured and follow best practices. The meta configuration includes proper layout, documentation, and argTypes, and the two stories (Default and WithSlug) provide comprehensive coverage for the WorkspaceForbidden component's different states.
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (1)
1-42: LGTM!The Storybook stories are well-structured and consistent with the WorkspaceForbidden stories. The documentation and story variants provide clear examples of the component's behavior.
webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
21-49: LGTM!The component is well-structured and provides a clear user experience for the 404 state. The conditional message based on slug presence enhances the user feedback.
webapp/src/integrations/workspace/workspace-provider.test.tsx (1)
1-325: Excellent test coverage!The test suite is comprehensive and well-organized, covering:
- Initialization and slug exposure
- Loading states (auth loading, unauthenticated)
- Successful data loading with role derivation
- Error handling (404, 403, generic)
- Hook usage (useWorkspace, useWorkspaceOptional)
- localStorage persistence
The test structure follows best practices with proper mocking, waitFor usage, and clear test descriptions.
webapp/src/integrations/workspace/index.ts (1)
1-24: LGTM!The barrel export is clean and well-organized, properly exposing both the real and mock workspace providers along with their associated types and utilities.
webapp/src/integrations/workspace/workspace-provider.tsx (9)
1-11: LGTM!The imports and constant definition are clean. The localStorage key uses a namespaced approach (
hephaestus.lastWorkspaceSlug) which prevents collisions.
13-38: Well-designed context type.The WorkspaceContextType interface is comprehensive and provides clear documentation for each field. The convenience flags (isAdmin, isOwner, isMember) and isReady state will simplify consumer code.
52-70: Robust error classification.The getErrorType function handles both structured errors (with status property) and Error objects with message hints. This provides good resilience across different error formats.
75-103: LGTM: Safe localStorage operations.All three localStorage functions properly wrap operations in try-catch blocks, preventing crashes in environments where localStorage is unavailable (incognito mode, disabled storage, etc.).
116-145: Excellent retry logic and query setup.The implementation correctly:
- Uses generated option helpers from TanStack Query
- Conditionally enables queries based on authentication state
- Implements custom retry logic that avoids retrying 403/404 responses
- Applies the retry logic to both workspace and membership queries
This prevents unnecessary retry attempts on expected error states.
147-159: LGTM: Proper effect usage.The two useEffect hooks correctly:
- Persist the slug when workspace data loads successfully
- Clear the persisted slug on logout
Dependencies are appropriate and effects are focused on single responsibilities.
161-187: Consider the isReady derivation logic.The
isReadyflag is computed as:isReady: Boolean(workspaceQuery.data) && !isLoading && !errorTypeThis means
isReadyis false when membership fails but workspace succeeds. Depending on your use case, you might want workspace data to be considered "ready" even if the user isn't a member. Consider whether membership is truly required for the workspace to be usable.Current behavior:
- If workspace loads but membership fails (e.g., user isn't a member),
isReady = false- Consumers must check both
workspaceandisReadyAlternative consideration:
isReady: Boolean(workspaceQuery.data) && !workspaceQuery.isLoading && !errorTypeThis would allow
isReady = truewithworkspacedata even whenmembershipis null (user is not a member). Is this intentional, or should workspace data be considered ready independently of membership?
211-217: LGTM!The useWorkspace hook correctly throws when used outside the provider, following React context best practices. The error message is clear and helpful.
223-225: LGTM!The useWorkspaceOptional hook provides a safe alternative for components that may exist outside the workspace context, returning undefined rather than throwing.
webapp/src/integrations/workspace/mock-workspace-provider.tsx (4)
9-48: LGTM: Comprehensive mock data.The mock workspace, membership, and default context provide realistic test data with proper TypeScript typing. The dates use consistent ISO strings which helps with reproducibility in tests.
50-95: Excellent mock provider design.The MockWorkspaceProvider uses a separate context to avoid interference with the real provider, and the merge pattern (
{ ...defaultMockContext, ...value }) allows flexible test setup. The documentation examples are particularly helpful for test authors.
101-107: LGTM!The useMockWorkspace hook follows the same pattern as the real useWorkspace hook, providing consistency for test code.
113-206: Comprehensive factory helpers.The factory functions cover all essential testing scenarios:
- Generic override (createMockWorkspaceContext)
- Loading state (createLoadingContext)
- Error states (createNotFoundContext, createForbiddenContext)
- Role variations (createAdminContext, createOwnerContext)
Each factory correctly sets all related flags (e.g., OWNER sets isOwner, isAdmin, and isMember).
There was a problem hiding this comment.
Pull request overview
This PR introduces a centralized workspace state management system using React Context and TanStack Query v5. The implementation provides a WorkspaceProvider component with a useWorkspace() hook that fetches and caches workspace data and user membership, handles loading and error states (404/403), persists the last visited workspace slug to localStorage, and includes comprehensive testing utilities.
Key changes:
- WorkspaceProvider with TanStack Query integration for workspace and membership data fetching
- localStorage persistence for last visited workspace with automatic cleanup on logout
- Error state UI components (WorkspaceNotFound, WorkspaceForbidden) with Storybook stories
- MockWorkspaceProvider with factory functions for testing different states
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
workspace-provider.tsx |
Main provider implementation with dual query fetching, error detection, localStorage persistence, and role-based permission flags |
workspace-provider.test.tsx |
Comprehensive test suite covering initialization, data loading, error states, hooks, and localStorage utilities |
mock-workspace-provider.tsx |
Testing utilities with factory functions for different workspace states (loading, admin, owner, errors) |
index.ts |
Public API exports for both production and test utilities |
WorkspaceNotFound.tsx |
404 error state component with navigation to home |
WorkspaceNotFound.stories.tsx |
Storybook stories for 404 state |
WorkspaceForbidden.tsx |
403 error state component with navigation to home |
WorkspaceForbidden.stories.tsx |
Storybook stories for 403 state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size:XXL
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
21-46: Consider passinguseGoHome()directly toonClick.The
handleGoHomevariable is an unnecessary intermediate assignment. You can simplify by passing the hook result directly to theonClickprop.🔎 Proposed simplification
export function WorkspaceForbidden({ slug }: WorkspaceForbiddenProps) { - const handleGoHome = useGoHome(); + const goHome = useGoHome(); return ( <Empty> <EmptyHeader> <EmptyMedia variant="icon"> <ShieldX /> </EmptyMedia> <EmptyTitle>Access denied</EmptyTitle> <EmptyDescription> {slug ? ( <> You don't have permission to access the workspace{" "} <strong>"{slug}"</strong>. Contact a workspace administrator if you believe this is a mistake. </> ) : ( <> You don't have permission to access this workspace. Contact a workspace administrator if you believe this is a mistake. </> )} </EmptyDescription> </EmptyHeader> - <Button onClick={handleGoHome}>Go to home</Button> + <Button onClick={goHome}>Go to home</Button> </Empty> ); }webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
21-43: Consider passinguseGoHome()directly toonClick.The
handleGoHomevariable is an unnecessary intermediate assignment, similar toWorkspaceForbidden. You can simplify by passing the hook result directly.🔎 Proposed simplification
export function WorkspaceNotFound({ slug }: WorkspaceNotFoundProps) { - const handleGoHome = useGoHome(); + const goHome = useGoHome(); return ( <Empty> <EmptyHeader> <EmptyMedia variant="icon"> <SearchX /> </EmptyMedia> <EmptyTitle>Workspace not found</EmptyTitle> <EmptyDescription> {slug ? ( <> The workspace <strong>"{slug}"</strong> doesn't exist or may have been deleted. </> ) : ( <>The workspace you're looking for doesn't exist or may have been deleted.</> )} </EmptyDescription> </EmptyHeader> - <Button onClick={handleGoHome}>Go to home</Button> + <Button onClick={goHome}>Go to home</Button> </Empty> ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/hooks/use-go-home.tswebapp/src/integrations/workspace/index.tswebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- webapp/src/integrations/workspace/index.ts
- webapp/src/components/workspace/WorkspaceForbidden.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/hooks/use-go-home.tswebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/workspace-provider.test.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/hooks/use-go-home.tswebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/workspace-provider.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/integrations/workspace/mock-workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/integrations/workspace/workspace-provider.test.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.test.{ts,tsx}: Write component tests with Vitest and@testing-library/react. Prefer queries that mirror user intent such asfindByRoleorgetByLabelText.
Mock network requests withvi.mockplus the generated TanStack Query helpers. Do not hand-roll fetch mocks.
Cover happy path, loading, and error outcomes in both tests and stories.
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsx
🧬 Code graph analysis (5)
webapp/src/components/workspace/WorkspaceForbidden.tsx (3)
webapp/src/hooks/use-go-home.ts (1)
useGoHome(7-13)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/integrations/workspace/mock-workspace-provider.tsx (2)
webapp/src/api/types.gen.ts (2)
Workspace(343-362)WorkspaceMembership(376-383)webapp/src/integrations/workspace/workspace-provider.tsx (3)
WorkspaceContextType(15-38)WorkspaceContext(46-46)WorkspaceErrorType(13-13)
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (2)
webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
WorkspaceNotFound(21-45)webapp/src/components/workspace/WorkspaceForbidden.stories.tsx (2)
Default(36-36)WithSlug(42-46)
webapp/src/components/workspace/WorkspaceNotFound.tsx (3)
webapp/src/hooks/use-go-home.ts (1)
useGoHome(7-13)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/integrations/workspace/workspace-provider.test.tsx (3)
webapp/src/integrations/auth/AuthContext.tsx (1)
useAuth(29-35)webapp/src/api/@tanstack/react-query.gen.ts (2)
getWorkspaceOptions(221-234)getCurrentUserMembershipOptions(615-628)webapp/src/integrations/workspace/workspace-provider.tsx (5)
WorkspaceProvider(122-200)useWorkspace(221-227)getLastWorkspaceSlug(92-98)useWorkspaceOptional(233-235)clearLastWorkspaceSlug(103-109)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: docker-build / intelligence-service-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / webapp-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / application-server-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webhook-ingest-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: quality-gates / application-server-quality
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: security-scan / dependencies
- GitHub Check: security-scan / sast
- GitHub Check: test-suite / webapp-visual
🔇 Additional comments (10)
webapp/src/hooks/use-go-home.ts (1)
1-13: LGTM!The hook is well-designed: it encapsulates navigation logic cleanly, follows the Rules of Hooks, and provides a reusable abstraction for error-state components. The JSDoc clearly explains the purpose and consumers.
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
1-15: LGTM!The imports and interface are clean. The
WorkspaceForbiddenPropsinterface is properly exported and documented, making it easy for consumers to understand the component's API.webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
1-15: LGTM!The imports and interface follow the established pattern from
WorkspaceForbiddenand are properly documented. Consistent API design across error components.webapp/src/integrations/workspace/workspace-provider.test.tsx (1)
1-469: LGTM! Comprehensive test coverage.The test suite thoroughly covers all critical paths including initialization, loading states, error handling, retry behavior, localStorage persistence, and logout clearing. All past review feedback has been addressed with proper tests for:
- Custom retry logic that skips 404/403 errors (lines 267-342)
- localStorage persistence on successful load (lines 344-407)
- localStorage clearing on logout (lines 378-406)
The test structure is clean, well-organized, and follows best practices with proper mocking and assertions.
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (1)
1-45: LGTM! Well-structured Storybook stories.The JSDoc has been properly updated to focus on when to use the component ("Use when a workspace slug cannot be resolved, typically in route loaders or workspace-dependent layouts"), addressing the previous review feedback. The two stories (Default and WithSlug) effectively demonstrate both use cases.
webapp/src/integrations/workspace/mock-workspace-provider.tsx (1)
1-197: LGTM! Mock provider is well-designed and addresses all feedback.The mock provider implementation properly addresses all previous review comments:
- Uses
MOCK_DATEconstant (line 10) for deterministic test data- Uses the real
WorkspaceContext(line 97) so components can use the actualuseWorkspacehook in testsThe factory functions are comprehensive and provide excellent test utilities for simulating loading, not-found, forbidden, admin, and owner states. The extensive JSDoc examples make this very developer-friendly.
webapp/src/integrations/workspace/workspace-provider.tsx (4)
58-76: LGTM! Error type detection is robust.The
getErrorTypefunction properly handles edge cases:
- Checks for null/undefined first
- Safely checks for object with
statusproperty (line 62:error !== nullcheck addresses previous feedback)- Falls back to message-based detection for Error instances
- Returns generic "error" for unknown error types
81-109: LGTM! localStorage utilities are safely implemented.The localStorage functions properly handle errors with try-catch blocks, making them resilient to environments where localStorage is unavailable (e.g., incognito mode). The public exports (
getLastWorkspaceSlug,clearLastWorkspaceSlug) and privatepersistLastWorkspaceSlugprovide a clean API.
122-200: LGTM! WorkspaceProvider implementation is solid.The provider demonstrates excellent patterns:
- Conditional query enabling based on authentication state (line 124)
- Custom retry logic that skips 404/403 errors (lines 132-137, 146-150)
- Automatic localStorage persistence on successful load (lines 154-158)
- Cleanup on logout (lines 161-165)
- Proper error normalization with type narrowing (lines 182-183) that addresses previous type safety concerns
The derived flags (
isAdmin,isOwner,isMember) provide convenient access to membership status, andisReadygives a clear signal for when data is available.
221-235: LGTM! Hooks follow React context patterns correctly.Both
useWorkspace(mandatory, throws if outside provider) anduseWorkspaceOptional(returns undefined if outside provider) provide appropriate access patterns. The JSDoc examples clearly demonstrate usage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webapp/src/integrations/workspace/workspace-provider.tsx (1)
145-150: Consider extracting the shared retry logic.Both queries use identical retry functions. Extracting to a helper would reduce duplication and ensure consistency if retry behavior needs to change.
🔎 Optional refactor to shared retry helper
+/** + * Shared retry logic that skips retries for expected 403/404 errors. + */ +function shouldRetryWorkspaceQuery(failureCount: number, error: unknown): boolean { + const errorType = getErrorType(error); + if (errorType === "not-found" || errorType === "forbidden") return false; + return failureCount < 2; +} + export function WorkspaceProvider({ slug, children }: WorkspaceProviderProps) { // ... auth logic ... const workspaceQuery = useQuery({ ...getWorkspaceOptions({ path: { workspaceSlug: slug }, }), enabled: isEnabled, - retry: (failureCount: number, error: unknown) => { - // Don't retry on 403/404 - these are expected error states - const errorType = getErrorType(error); - if (errorType === "not-found" || errorType === "forbidden") return false; - return failureCount < 2; - }, + retry: shouldRetryWorkspaceQuery, }); const membershipQuery = useQuery({ ...getCurrentUserMembershipOptions({ path: { workspaceSlug: slug }, }), enabled: isEnabled, - retry: (failureCount: number, error: unknown) => { - const errorType = getErrorType(error); - if (errorType === "not-found" || errorType === "forbidden") return false; - return failureCount < 2; - }, + retry: shouldRetryWorkspaceQuery, });Also applies to: 159-163
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webapp/src/components/workspace/NoWorkspace.stories.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/integrations/workspace/workspace-provider.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/NoWorkspace.stories.tsx
🧬 Code graph analysis (1)
webapp/src/components/workspace/NoWorkspace.stories.tsx (1)
webapp/src/components/core/Header.stories.tsx (1)
NoWorkspace(110-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: docker-build / webapp-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / application-server-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / intelligence-service-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / webhook-ingest-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: security-scan / sast
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: security-scan / dependencies
- GitHub Check: test-suite / webapp-visual
🔇 Additional comments (10)
webapp/src/components/workspace/NoWorkspace.stories.tsx (4)
4-8: LGTM — Clear component documentation.The JSDoc provides helpful context about when to use this empty-state component.
9-20: LGTM — Modern Storybook pattern with better type inference.The
satisfiesoperator provides superior type checking compared to explicit type annotations, and this pattern aligns with modern Storybook best practices.
23-23: LGTM — Recommended Story type derivation.Deriving the Story type from
metaensures that any decorators, parameters, or other meta configurations are properly reflected in the story type.
25-29: LGTM — Clear story documentation.The updated JSDoc provides helpful context, and the empty story object is appropriate for a component with no required props.
webapp/src/integrations/workspace/workspace-provider.tsx (6)
55-67: Type guards properly implemented and resolve past concerns.The
hasStatusCodeguard correctly checkstypeof value === "object", null-safety, and property existence. TheisErrorguard safely narrows to Error instances. These guards enable type-safe error handling throughout the provider.
91-122: LGTM: Robust localStorage utilities.The try-catch blocks appropriately handle localStorage failures (incognito mode, storage quota). Silent failures are correct here since persistence is a convenience feature, not critical functionality.
167-178: LGTM: Effects properly handle localStorage lifecycle.The persistence effect correctly fires only on successful workspace load, and the cleanup effect ensures the slug is cleared on logout. Dependencies are accurate and prevent stale closures.
195-199: Error normalization safely handles unknown error types.The code uses the
isErrortype guard for safe narrowing instead of type assertions. Non-Error values are wrapped in a new Error instance for consistent handling. This approach resolves the type safety concerns mentioned in past reviews.
201-213: LGTM: Context value comprehensively exposes workspace state.The derived flags (
isMember,isAdmin,isOwner) provide convenient role checks, andisReadyaccurately reflects successful load state. All properties are correctly typed and derived from query state.
237-251: LGTM: Context hooks follow React best practices.The standard
useWorkspacehook enforces proper usage with a clear error message, whileuseWorkspaceOptionalprovides flexibility for components that may render outside a workspace context. Documentation and examples are excellent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
webapp/src/integrations/workspace/workspace-provider.test.tsx (2)
272-281: Consider strengthening or removing the generic error test.This test doesn't validate error handling behavior—it only verifies that
slugis set, which is already covered by other tests. The comment acknowledges this limitation.Consider either:
- Adding a real generic error scenario (e.g., network timeout, 500 status) and verifying
errorType === "error"- Removing this test entirely since it provides no additional coverage
💡 Example of a meaningful generic error test
- it("should handle generic errors gracefully", () => { - // Generic error handling is covered by the error type detection logic - // The component will show error state when queries fail - const { result } = renderHook(() => useWorkspace(), { - wrapper: createWrapper(queryClient, "test-workspace"), - }); - - // Initially no error - expect(result.current.slug).toBe("test-workspace"); - }); + it("should handle generic errors gracefully", async () => { + mockGetWorkspaceOptions.mockImplementation(({ path }) => ({ + queryKey: ["getWorkspace", path.workspaceSlug], + queryFn: () => Promise.reject({ status: 500, message: "Internal Server Error" }), + })); + + const { result } = renderHook(() => useWorkspace(), { + wrapper: createWrapper(queryClient, "test-workspace"), + }); + + await waitFor(() => { + expect(result.current.errorType).toBe("error"); + }); + + expect(result.current.isReady).toBe(false); + });
284-394: Consider adding a positive retry test case.The retry tests verify that 404/403/401 errors prevent retries (the negative case), which addresses past review feedback. However, they don't verify the positive case: that other errors (e.g., 500, network failures) do retry up to the configured limit.
💡 Example test for positive retry case
it("should retry on transient errors", async () => { let callCount = 0; mockGetWorkspaceOptions.mockImplementation(({ path }) => ({ queryKey: ["getWorkspace", path.workspaceSlug], queryFn: () => { callCount++; return Promise.reject({ status: 500, message: "Internal Server Error" }); }, })); const retryQueryClient = new QueryClient({ defaultOptions: { queries: { staleTime: Number.POSITIVE_INFINITY, }, }, }); renderHook(() => useWorkspace(), { wrapper: createWrapper(retryQueryClient, "test-workspace"), }); // Wait for retries to complete await waitFor( () => { // Should retry: initial call + 2 retries = 3 total expect(callCount).toBe(3); }, { timeout: 2000 }, ); });webapp/src/integrations/workspace/workspace-provider.tsx (1)
176-182: Consider centralized error logging for better observability.While the provider correctly exposes errors through context, it doesn't log them. Per coding guidelines, errors should be logged with contextual information.
Consider adding error logging when queries fail:
💡 Example error logging approach
// Derive loading state const isLoading = authLoading || (isEnabled && (workspaceQuery.isLoading || membershipQuery.isLoading)); // Determine error type (prioritize workspace errors) const error = workspaceQuery.error ?? membershipQuery.error ?? null; const errorType = getErrorType(error); // Log errors when they occur (add after error derivation) useEffect(() => { if (error && errorType) { console.error("[WorkspaceProvider] Error loading workspace", { slug, errorType, error, isWorkspaceError: Boolean(workspaceQuery.error), isMembershipError: Boolean(membershipQuery.error), }); // Optionally integrate with Sentry: // Sentry.captureException(error, { tags: { workspace: slug, errorType } }); } }, [error, errorType, slug, workspaceQuery.error, membershipQuery.error]);This ensures consistent error tracking across all workspace contexts without relying on individual consumers to log errors.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.test.{ts,tsx}: Write component tests with Vitest and@testing-library/react. Prefer queries that mirror user intent such asfindByRoleorgetByLabelText.
Mock network requests withvi.mockplus the generated TanStack Query helpers. Do not hand-roll fetch mocks.
Cover happy path, loading, and error outcomes in both tests and stories.
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/integrations/workspace/workspace-provider.test.tsxwebapp/src/integrations/workspace/workspace-provider.tsx
🧬 Code graph analysis (1)
webapp/src/integrations/workspace/workspace-provider.tsx (4)
webapp/src/api/types.gen.ts (2)
Workspace(343-362)WorkspaceMembership(376-383)webapp/src/integrations/auth/AuthContext.tsx (1)
useAuth(29-35)webapp/src/integrations/auth/keycloak.ts (1)
isAuthenticated(110-112)webapp/src/api/@tanstack/react-query.gen.ts (2)
getWorkspaceOptions(221-234)getCurrentUserMembershipOptions(615-628)
🔇 Additional comments (11)
webapp/src/integrations/workspace/workspace-provider.test.tsx (3)
1-46: LGTM: Clean test setup with proper mocking pattern.The mock setup correctly mocks external dependencies before importing the tested modules, and the type casts for the mocked functions are appropriate. The file header documentation clearly outlines test coverage.
47-88: LGTM: Well-structured test utilities and mock data.The mock data aligns with the generated types, and the helper functions follow React Testing Library best practices. Disabling retry by default in the test QueryClient is appropriate—the retry-specific tests correctly override this behavior.
396-521: LGTM: Comprehensive localStorage and hook tests.These tests thoroughly address all past review concerns:
- ✓ localStorage persistence when workspace loads successfully
- ✓ No persistence on fetch failures
- ✓ Cleanup on user logout
- ✓ Hook behavior inside and outside provider
- ✓ Standalone utility functions
The logout test (lines 430-458) particularly demonstrates careful simulation of auth state changes.
webapp/src/integrations/workspace/workspace-provider.tsx (8)
1-53: LGTM: Well-documented type definitions and clean imports.The type definitions are comprehensive with clear JSDoc documentation. The
@internalannotation on the exportedWorkspaceContext(line 44) appropriately signals that it's exported for testing purposes, not public API consumption.
55-67: LGTM: Robust type guards with proper null handling.The type guards are correctly implemented. The
hasStatusCodeguard (lines 65-67) properly handles the edge case wheretypeof null === "object"by explicitly checkingvalue !== null.
69-94: LGTM: Clean error classification and retry logic.Both functions are well-implemented:
getErrorTypecorrectly handles the falsy check with!errorand uses the type guard for safe status code accessshouldRetryclearly documents that 401/403/404 errors shouldn't be retried (they're not transient)The retry limit of
failureCount < 2allows up to 2 retries (3 total attempts) for transient failures, which is reasonable.
96-127: LGTM: Defensive localStorage handling.The try-catch wrappers appropriately handle localStorage failures (incognito mode, quota exceeded, security restrictions). Silently ignoring these errors is standard practice for progressive enhancement features like slug persistence.
140-160: LGTM: Correct usage of TanStack Query with generated helpers.The queries properly:
- Spread the generated option helpers from
@/api/@tanstack/react-query.gen.ts- Conditionally enable based on authentication state
- Apply the custom
shouldRetrylogicThe
isEnabledguard prevents unnecessary requests when the user isn't authenticated or auth state is still loading.
162-174: LGTM: Well-structured side effects for slug persistence.Both effects are correctly implemented:
- Persists slug only after successful workspace fetch
- Clears localStorage on logout with proper guard (
!authLoading) to avoid clearing during initial auth checkThe dependency arrays are complete and the effects follow the Rules of Hooks.
176-211: LGTM: Safe error normalization and correct state derivation.The state derivation logic is well-structured:
- Error normalization (lines 191-195) now uses the
isErrortype guard instead of type assertions, which is much saferisReadycalculation correctly treatsnullas the "no error" state- Role flags are properly derived from membership data
214-247: LGTM: Standard context consumer hooks with helpful documentation.The hooks follow established React patterns:
useWorkspace()provides type-safe access with runtime validationuseWorkspaceOptional()enables conditional workspace context accessThe JSDoc example (lines 220-231) demonstrates proper usage patterns for handling loading and error states.
Based on maintainer feedback from PRs #607, #628, #629: builder.md: - Add 'Research Before Implementing' section with WebSearch/WebFetch tool guidance - Add 'Anti-Bloat Checklist' before shipping - Add 3 new instant rejection criteria (test bloat, duplicate abstractions, unintegrated files) - Update 'Address Reviews' to filter HTML comment bloat from bots (CodeRabbit, etc.) architect.md: - Add 'Over-engineering detected' intervention trigger - Add 3 anti-patterns (dispatching without approval, scope creep, letting bloat happen)
Based on maintainer feedback from PRs #607, #628, #629: builder.md: - Add 'Research Before Implementing' section with WebSearch/WebFetch tool guidance - Add 'Anti-Bloat Checklist' before shipping - Add 3 new instant rejection criteria (test bloat, duplicate abstractions, unintegrated files) - Update 'Address Reviews' to filter HTML comment bloat from bots (CodeRabbit, etc.) architect.md: - Add 'Over-engineering detected' intervention trigger - Add 3 anti-patterns (dispatching without approval, scope creep, letting bloat happen)
github-actions
bot
added
size:L
FelixTJDietrich
force-pushed
the
feat/workspace-context-provider
branch
from
454ca7c to
71e12e3
Compare
FelixTJDietrich
changed the title
github-actions
bot
added
size:XL
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (1)
42-54: Consider extractinggetScheduleto a shared utility.This schedule extraction logic is duplicated in
webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx. Consider extracting to a shared helper in@/lib/timeframealongside the existingLeaderboardScheduletype.🔎 Proposed extraction
// In @/lib/timeframe.ts export function getLeaderboardScheduleFromWorkspace(workspace: { leaderboardScheduleTime?: string | null; leaderboardScheduleDay?: number | null; }): LeaderboardSchedule { const scheduledTime = workspace.leaderboardScheduleTime || "9:00"; const scheduledDay = workspace.leaderboardScheduleDay ?? 2; const [hours, minutes] = scheduledTime .split(":") .map((part: string) => Number.parseInt(part, 10)); return { day: scheduledDay, hour: hours || 9, minute: minutes || 0, }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
webapp/src/integrations/workspace/context.tsxwebapp/src/routeTree.gen.tswebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/integrations/workspace/context.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/routeTree.gen.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/integrations/workspace/context.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/integrations/workspace/context.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/routeTree.gen.ts
🧬 Code graph analysis (5)
webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (3)
webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
WorkspaceNotFound(21-45)webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
WorkspaceForbidden(21-49)webapp/src/integrations/workspace/context.tsx (1)
WorkspaceContext(8-8)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsx (3)
webapp/src/integrations/workspace/context.tsx (1)
useWorkspace(16-22)webapp/src/integrations/auth/keycloak.ts (1)
isCurrentUser(254-258)webapp/src/api/@tanstack/react-query.gen.ts (2)
getActivityByUserOptions(362-375)getUserProfileOptions(1015-1028)
webapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsx (3)
webapp/src/integrations/workspace/context.tsx (1)
useWorkspace(16-22)webapp/src/api/@tanstack/react-query.gen.ts (2)
getThreadQueryKey(972-972)getGroupedThreadsQueryKey(952-952)webapp/src/api/types.gen.ts (1)
ChatThreadGroup(27-30)
webapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsx (1)
webapp/src/integrations/workspace/context.tsx (1)
useWorkspace(16-22)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (3)
webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (1)
Route(37-44)webapp/src/integrations/workspace/context.tsx (1)
useWorkspace(16-22)webapp/src/lib/timeframe.ts (1)
LeaderboardSchedule(29-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / intelligence-service-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: docker-build / application-server-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: docker-build / webhook-ingest-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / webapp-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: security-scan / sast
- GitHub Check: security-scan / dependencies
- GitHub Check: test-suite / webapp-visual
🔇 Additional comments (12)
webapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsx (1)
11-18: LGTM!Clean refactor to context-based workspace access. The component correctly relies on the parent route's error handling for workspace-level errors, and uses TanStack Query idiomatically.
webapp/src/integrations/workspace/context.tsx (1)
1-22: LGTM — good use of React 19'suse()hook.The context and hook are well-structured with proper null validation and clear error messaging. The
use()hook is appropriate here as it allows the context to be consumed in async contexts if needed in the future.webapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsx (1)
23-66: LGTM!The
hasStartedRefpattern correctly ensures the auto-start effect runs only once per mount, and the query cache pre-population before navigation is a good UX pattern. TheworkspaceSlugdependency is correctly included for exhaustive deps compliance, even though the ref guard prevents re-execution.webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (2)
46-71: LGTM!Clean refactor to use workspace context. The schedule extraction logic correctly handles optional values with sensible defaults.
103-117: LGTM!The query's
enabledcondition correctly gates onparsedAfter && teamsQuery.data, ensuring the leaderboard query doesn't fire until dependencies are ready. TheplaceholderDatapattern provides smooth UX during filter changes.webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsx (2)
24-48: LGTM!Clean refactor with consistent use of
workspaceSlugfrom context across all queries and mutations. Theenabledconditions are appropriately simplified since the workspace is guaranteed by the parent route.
50-113: LGTM!All mutations consistently invalidate the activity query cache with the correct query key including
workspaceSlug. Error handling via toast notifications provides good user feedback.webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (3)
11-16: LGTM — good encapsulation of the 403 error case.The custom error class cleanly captures the workspace slug for display in the error component, following the discriminated error pattern.
18-58: LGTM — comprehensive error handling.The loader correctly handles all HTTP error cases: 404 triggers
notFound(), 403 throws a typed error for the error boundary, and other failures produce a generic error. Re-throwing non-WorkspaceForbiddenErrorin the error component correctly bubbles unexpected errors to parent boundaries.
60-67: LGTM — clean context provision.Using
WorkspaceContextdirectly with avalueprop is the recommended React 19 pattern. The layout correctly provides workspace data to all child routes viaOutlet.webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (2)
32-54: LGTM!Clean integration with workspace context. The schedule extraction is correct.
77-88: LGTM!The query configuration is correct —
enabledonly needs to checkusernamesince the workspace is guaranteed by the parent route's loader.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
webapp/src/components/workspace/NoWorkspace.tsx (1)
20-25: Reconsider programmatic focus for live regions.The
useEffectthat programmatically focuses the container on mount is unnecessary and potentially disruptive to screen reader users. Therole="status"witharia-live="polite"already ensures screen readers announce the content automatically when it appears—no focus manipulation is needed.Programmatically setting focus can interrupt a user's current position in the document and is generally considered an anti-pattern unless there's a specific user action (like opening a dialog) that warrants it.
🔎 Recommended fix: remove focus management
import { Folders } from "lucide-react"; -import { useEffect, useRef } from "react"; import { Empty, EmptyDescription, @@ -17,27 +15,18 @@ * - Auto-focuses the container for keyboard navigation */ export function NoWorkspace() { - const containerRef = useRef<HTMLDivElement>(null); - - // Focus the container when mounted for screen reader announcement - useEffect(() => { - containerRef.current?.focus(); - }, []); - return ( <Empty> {/* Accessible status announcement region */} <div - ref={containerRef} role="status" aria-live="polite" - tabIndex={-1} - className="outline-none" > <EmptyHeader>Update the JSDoc to remove the focus-related note:
* Accessibility: * - Uses role="status" for non-critical information - * - Auto-focuses the container for keyboard navigationAlso applies to: 30-36
webapp/src/components/workspace/WorkspaceError.tsx (2)
31-36: Remove unnecessary focus management.As with the other workspace components, the programmatic focus on mount is unnecessary. The
role="alert"witharia-live="assertive"already ensures immediate announcement to screen readers. Forcing focus can disrupt users mid-navigation.🔎 Recommended fix
export function WorkspaceError({ error, reset }: WorkspaceErrorProps) { const router = useRouter(); - const containerRef = useRef<HTMLDivElement>(null); - - // Focus the container when mounted for screen reader announcement - useEffect(() => { - containerRef.current?.focus(); - }, []); const handleRetry = () => { @@ -57,11 +52,8 @@ <Empty> {/* Accessible error announcement region */} <div - ref={containerRef} role="alert" aria-live="assertive" aria-atomic="true" - tabIndex={-1} - className="outline-none" >Remove the
useRefimport if it becomes unused, and update the JSDoc comment accordingly.
50-54: Brittle error detection based on message strings.Detecting network errors by checking for substrings like
"fetch"or"network"inerror.messageis fragile. Error messages can vary across browsers, libraries, and environments. Consider checkingerror.name, using instanceof with typed error classes, or accepting an explicit error type/category prop.Alternative approaches
Option 1: Check error type or name:
const isNetworkError = error.name === "TypeError" || error.name === "NetworkError" || (error instanceof TypeError && error.message.includes("fetch"));Option 2: Add a category prop to
WorkspaceErrorProps:export interface WorkspaceErrorProps { error: Error; reset?: () => void; category?: "network" | "server" | "unknown"; }Option 3: Use a discriminated union with typed errors:
class NetworkError extends Error { name = "NetworkError" as const; } // Then check: error instanceof NetworkErrorwebapp/src/components/workspace/WorkspaceNotFound.tsx (1)
32-37: Remove unnecessary focus management.This component has the same programmatic focus pattern as
NoWorkspaceandWorkspaceError. Therole="alert"witharia-live="assertive"already announces the not-found state to screen readers immediately—no focus manipulation is needed.🔎 Recommended fix
export function WorkspaceNotFound({ slug, showRetry = false }: WorkspaceNotFoundProps) { const navigate = useNavigate(); const router = useRouter(); - const containerRef = useRef<HTMLDivElement>(null); - - // Focus the container when mounted for screen reader announcement - useEffect(() => { - containerRef.current?.focus(); - }, []); const handleRetry = () => { @@ -44,11 +39,8 @@ <Empty> {/* Accessible error announcement region */} <div - ref={containerRef} role="alert" aria-live="assertive" aria-atomic="true" - tabIndex={-1} - className="outline-none" >Remove the
useRefanduseEffectimports if they become unused, and update the JSDoc comment to remove focus-related notes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
webapp/src/components/workspace/NoWorkspace.stories.tsxwebapp/src/components/workspace/NoWorkspace.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/components/workspace/index.tswebapp/src/hooks/use-workspace.tswebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- webapp/src/routes/_authenticated/w/$workspaceSlug.tsx
- webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsx
- webapp/src/components/workspace/WorkspaceNotFound.stories.tsx
- webapp/src/components/workspace/WorkspaceForbidden.stories.tsx
- webapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsx
- webapp/src/components/workspace/NoWorkspace.stories.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/hooks/use-workspace.tswebapp/src/components/workspace/NoWorkspace.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/index.tswebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/hooks/use-workspace.tswebapp/src/components/workspace/NoWorkspace.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/index.tswebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/components/workspace/NoWorkspace.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/WorkspaceError.stories.tsx
🧬 Code graph analysis (9)
webapp/src/hooks/use-workspace.ts (1)
webapp/src/api/types.gen.ts (1)
Workspace(343-362)
webapp/src/components/workspace/NoWorkspace.tsx (1)
webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)
webapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsx (3)
webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/api/@tanstack/react-query.gen.ts (2)
getThreadQueryKey(972-972)getGroupedThreadsQueryKey(952-952)webapp/src/api/types.gen.ts (1)
ChatThreadGroup(27-30)
webapp/src/components/workspace/WorkspaceError.tsx (3)
webapp/src/components/workspace/index.ts (2)
WorkspaceErrorProps(12-12)WorkspaceError(12-12)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (3)
webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (1)
Route(20-62)webapp/src/api/@tanstack/react-query.gen.ts (2)
getLeaderboardOptions(383-396)getUserProfileOptions(1015-1028)
webapp/src/components/workspace/WorkspaceError.stories.tsx (1)
webapp/src/components/workspace/WorkspaceError.tsx (1)
WorkspaceError(29-109)
webapp/src/components/workspace/WorkspaceForbidden.tsx (4)
webapp/src/components/workspace/index.ts (1)
WorkspaceForbidden(14-14)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)docs/src/pages/index.tsx (1)
Home(79-91)
webapp/src/components/workspace/WorkspaceNotFound.tsx (3)
webapp/src/components/workspace/index.ts (2)
WorkspaceNotFoundProps(19-19)WorkspaceNotFound(18-18)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (5)
webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (1)
Route(20-62)webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (1)
Route(37-44)webapp/src/integrations/auth/AuthContext.tsx (1)
useAuth(29-35)webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/lib/timeframe.ts (1)
LeaderboardSchedule(29-34)
🔇 Additional comments (10)
webapp/src/hooks/use-workspace.ts (1)
1-21: LGTM! Correct use of React 19'suse()hook.This hook correctly leverages React 19's new
use()primitive to consume context. Theuse()hook can read Context or Promises and is part of the stable React 19 API. The error boundary ensures the hook is only called within a workspace route, and the return type correctly narrows fromWorkspace | nulltoWorkspace.webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (4)
49-51: Clean migration to workspace context.The refactor correctly replaces the previous workspace query with the context-provided workspace from the parent route. This eliminates redundant loading logic and aligns with the PR's architectural goal of centralizing workspace loading at the route level.
99-99: All query paths correctly use context-provided workspaceSlug.The workspaceSlug is now consistently derived from the workspace context across all query option objects (
getAllTeamsOptions,getLeaderboardOptions,getUserProfileOptions,getUserLeagueStatsOptions). This ensures data fetching aligns with the loaded workspace.Also applies to: 106-106, 121-121, 227-227
320-320: Loading state calculation simplified correctly.The
isLoadingcalculation now only depends onteamsQueryandleaderboardQuerystates, which is correct since the workspace is pre-loaded by the parent route. The guard!leaderboardQuery.dataensures the loading indicator persists during refetches while showing stale data.
116-116: Query enable guards are correctly simplified and sufficient.The parent route (
$workspaceSlug.tsx) properly handles all workspace loading failures:
- 404: Throws
notFound()withnotFoundComponenterror boundary- 403: Throws
WorkspaceForbiddenErrorwitherrorComponenterror boundary- Other errors: Throws descriptive error with
errorComponenterror boundaryThe
WorkspaceLayoutcomponent provides the workspace to child routes via context only after successful loading. Child routes usinguseWorkspace()have guaranteed access to a valid workspace—if the loader fails, the error/notFound components render instead, preventing child route execution. The enable guards correctly check only their specific dependencies without needing to verify workspace existence.webapp/src/components/workspace/index.ts (1)
1-20: LGTM! Clean barrel export with helpful documentation.The accessibility documentation header provides valuable context for maintainers, and the exports follow TypeScript best practices by co-exporting prop types for reuse.
webapp/src/components/workspace/WorkspaceError.stories.tsx (1)
1-67: LGTM! Well-documented Storybook stories.The stories comprehensively cover error scenarios (network errors, generic errors, with/without reset). The inline JSDoc documentation with accessibility notes and usage examples is particularly helpful for component consumers.
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
27-74: LGTM! Excellent accessibility implementation.The component follows accessibility best practices comprehensively:
- Proper ARIA roles and live regions for screen reader announcements
- Focus management on mount for keyboard navigation
- Decorative icons properly hidden from assistive technology
- Clear, actionable error messaging with optional context
The implementation aligns with the coding guidelines for React components and accessibility patterns.
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (1)
37-39: LGTM! Clean context migration with appropriate simplifications.The migration from per-route workspace queries to context-based workspace data is correctly implemented:
- Lines 37-39: Workspace data properly sourced from context
- Lines 43-44: Schedule data correctly extracted with safe fallbacks
- Line 87: Query enablement simplified appropriately—
workspaceSlugcheck is unnecessary since the parent loader guarantees workspace availability- Line 103: Loading state correctly simplified to depend only on profile query
The implementation follows the established pattern from the parent layout route and maintains type safety with nullish coalescing operators.
Also applies to: 43-44, 87-87, 103-103
webapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsx (1)
18-19: No action needed. The dependency array is correct.When the
workspaceSlugroute parameter changes, TanStack Router re-executes the loader and effectively re-mounts the component tree. ThehasStartedRefguard is sufficient because it protects against double-invocation within a single mount lifecycle, which is exactly what it should do. IncludingworkspaceSlugin the dependency array is correct ESLint practice and makes intent explicit, even though the component remounting on parameter change would handle the scenario independently.Likely an incorrect or invalid review comment.
Implements a centralized workspace context provider that: - Fetches and caches workspace data by slug using TanStack Query - Handles 404/403 error states with dedicated error components - Persists last visited workspace slug to localStorage - Exposes membership role flags (isMember, isAdmin, isOwner) - Provides mock provider for testing and Storybook This unblocks routing cleanup and role guards by providing a single source of truth for workspace state in slug-scoped pages. Refs: heph-aey.1
- fix MockWorkspaceProvider to use real WorkspaceContext so components using useWorkspace hook work correctly in tests - fix type safety: use 'error !== null' check instead of redundant 'error &&' - improve error normalization with proper Error wrapping instead of type assertion - add tests for retry behavior (404/403 skip retries, others retry) - add tests for localStorage persistence when workspace loads - add tests for localStorage clearing on logout - update Storybook JSDoc to explain WHEN to use components - use fixed date constant (MOCK_DATE) for deterministic test data - extract shared useGoHome() hook from WorkspaceForbidden/WorkspaceNotFound - export WorkspaceContext for test provider injection
… provider - Add isError and hasStatusCode type guards for type-safe error handling - Use type guards instead of inline instanceof checks and type assertions - Add JSDoc comment to NoWorkspace.stories.tsx explaining when to use component - Update story type to use satisfies Meta pattern for consistency
…orkspace provider - Extract duplicated retry logic into reusable shouldRetry function - Add 401 status code handling (mapped to 'forbidden' error type) - Remove fragile string-based error message matching - Add comprehensive tests for 401 error handling and retry skipping
Address maintainer review feedback questioning architectural approach: 1. Remove WorkspaceProvider context - TanStack Router provides built-in errorComponent and notFoundComponent for route-level error handling 2. Remove mock-workspace-provider - not needed when using framework patterns 3. Remove use-go-home hook - unnecessary abstraction, inline useNavigate 4. Remove 521 lines of tests for removed functionality Error handling should happen at the route level using TanStack Router's notFound() function and route-level error/notFound components, not via a custom context provider. The previous implementation duplicated what the framework already provides. The WorkspaceForbidden and WorkspaceNotFound components remain as simple presentational components that can be used as route-level errorComponent or notFoundComponent.
Create layout route at _authenticated/w/$workspaceSlug.tsx that properly integrates workspace error handling with TanStack Router: - Fetch workspace in loader with fail-fast pattern - Use notFound() for 404 and custom WorkspaceForbiddenError for 403 - Wire WorkspaceForbidden and WorkspaceNotFound to error boundaries - Provide workspace data to child routes via React Context Simplify child routes to use useWorkspace() hook instead of fetching workspace data independently.
- Simplify Storybook meta docs following guidelines (JSDoc above meta) - Remove redundant docs.description in favor of concise JSDoc - Clarify context.tsx documentation with @see reference - Make story-level JSDoc comments more concise and action-oriented
Address review feedback questioning whether workspace context belongs in integrations/ folder. The context is not an external integration - it's a simple React context for sharing route loader data with child components. Move from integrations/workspace/context.tsx to hooks/use-workspace.ts to align with the codebase pattern where hooks live in the hooks/ directory.
- Add role="alert" with aria-live="assertive" for screen reader announcements - Auto-focus error containers for keyboard navigation - Add aria-hidden to decorative icons - Create WorkspaceError component for generic errors with retry capability - Add showRetry prop to WorkspaceNotFound for transient error scenarios - Update route to handle all errors gracefully instead of re-throwing - Improve error messaging for user-friendly AND debuggable output - Add development-only error details expansion - Enhance Storybook documentation with accessibility notes and usage examples - Export all workspace components from index barrel file
FelixTJDietrich
force-pushed
the
feat/workspace-context-provider
branch
from
bcfff08 to
d43781f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
webapp/src/components/workspace/WorkspaceError.tsx (1)
51-55: Consider case-insensitive matching for network error detection.The string matching is case-sensitive, which could miss error messages with different casing (e.g., "Network request failed" vs "network request failed"). Also, consider checking for additional common patterns like "timeout" or "ECONNREFUSED".
🔎 Proposed enhancement
// Determine if this is a network error (potentially transient) + const lowerMessage = error.message.toLowerCase(); const isNetworkError = - error.message.includes("fetch") || - error.message.includes("network") || - error.message.includes("Failed to load"); + lowerMessage.includes("fetch") || + lowerMessage.includes("network") || + lowerMessage.includes("failed to load") || + lowerMessage.includes("timeout") || + lowerMessage.includes("econnrefused");
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
webapp/src/components/workspace/NoWorkspace.stories.tsxwebapp/src/components/workspace/NoWorkspace.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/components/workspace/index.tswebapp/src/hooks/use-workspace.tswebapp/src/routeTree.gen.tswebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- webapp/src/hooks/use-workspace.ts
- webapp/src/components/workspace/NoWorkspace.tsx
- webapp/src/components/workspace/WorkspaceForbidden.stories.tsx
- webapp/src/components/workspace/WorkspaceNotFound.stories.tsx
- webapp/src/routes/_authenticated/w/$workspaceSlug/mentor/index.tsx
- webapp/src/components/workspace/WorkspaceError.stories.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/components/workspace/index.tswebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsxwebapp/src/routeTree.gen.ts
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/components/workspace/index.tswebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsxwebapp/src/routeTree.gen.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/index.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsxwebapp/src/components/workspace/NoWorkspace.stories.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/NoWorkspace.stories.tsx
🧬 Code graph analysis (7)
webapp/src/components/workspace/WorkspaceForbidden.tsx (2)
webapp/src/components/workspace/index.ts (1)
WorkspaceForbidden(14-14)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)
webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (4)
webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (1)
Route(20-62)webapp/src/lib/timeframe.ts (1)
LeaderboardSchedule(29-34)webapp/src/api/@tanstack/react-query.gen.ts (2)
getLeaderboardOptions(383-396)getUserProfileOptions(1015-1028)
webapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsx (2)
webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (1)
Route(20-62)webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsx (5)
webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/integrations/auth/keycloak.ts (1)
isCurrentUser(254-258)webapp/src/api/@tanstack/react-query.gen.ts (1)
getActivityByUserOptions(362-375)webapp/src/api/sdk.gen.ts (3)
detectBadPracticesForPullRequest(131-136)resolveBadPractice(124-129)provideFeedbackForBadPractice(113-122)webapp/src/api/types.gen.ts (1)
BadPracticeFeedback(15-18)
webapp/src/components/workspace/WorkspaceNotFound.tsx (3)
webapp/src/components/workspace/index.ts (2)
WorkspaceNotFoundProps(19-19)WorkspaceNotFound(18-18)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (5)
webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (1)
Route(20-62)webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (1)
Route(37-44)webapp/src/integrations/auth/AuthContext.tsx (1)
useAuth(29-35)webapp/src/hooks/use-workspace.ts (1)
useWorkspace(15-21)webapp/src/lib/timeframe.ts (1)
LeaderboardSchedule(29-34)
webapp/src/components/workspace/NoWorkspace.stories.tsx (2)
webapp/src/components/workspace/NoWorkspace.tsx (1)
NoWorkspace(19-50)webapp/src/components/workspace/index.ts (1)
NoWorkspace(11-11)
🔇 Additional comments (25)
webapp/src/components/workspace/NoWorkspace.stories.tsx (1)
4-33: LGTM! Well-documented Storybook story with proper TypeScript patterns.The use of
satisfies Meta<typeof NoWorkspace>is idiomatic for CSF3, and the comprehensive JSDoc block documenting accessibility features and usage examples is excellent. TheStoryObj<typeof meta>pattern ensures type inference flows correctly.webapp/src/components/workspace/WorkspaceError.tsx (2)
29-49: Well-structured error component with proper accessibility and navigation.Good use of
useNavigatefor client-side navigation (addressing the previous review feedback), and the retry logic correctly falls back torouter.invalidate()when no reset function is provided. The focus management pattern is consistent with the other workspace components.
57-108: Clean JSX structure with appropriate accessibility attributes.The accessible alert region with
role="alert"andaria-live="assertive"is correct for error states. The conditional rendering for network vs generic errors provides helpful context to users. The development-only debug details usingprocess.env.NODE_ENVis a good pattern for troubleshooting.webapp/src/routes/_authenticated/w/$workspaceSlug/teams/index.tsx (1)
11-18: LGTM! Clean migration to context-based workspace data.The component correctly relies on the parent layout route for workspace loading and error handling. Using
useWorkspace()to obtainworkspaceSlugfrom context is consistent with the new architecture, and the query options are properly spread per the coding guidelines.webapp/src/components/workspace/index.ts (1)
1-20: LGTM! Well-organized barrel module with prop type exports.The barrel file correctly exports both components and their associated prop types, following the coding guideline to "Export prop and return-value types so other modules can reuse them." The header documentation summarizing accessibility patterns is helpful for consumers.
webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/best-practices.tsx (2)
24-49: LGTM! Clean migration to context-based workspace data.The removal of
hasWorkspacechecks is correct since the parent layout route now handles workspace loading and access validation. Theenabledflags correctly depend only onusernamepresence, and theworkspaceSlugis reliably provided via context.
50-156: Mutations properly use context-provided workspaceSlug.All mutations correctly use
workspaceSlugfrom context and invalidate with the generatedgetActivityByUserQueryKeyhelper, following the coding guidelines for TanStack Query cache invalidation.webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
27-73: LGTM! Well-implemented 403 error state with consistent patterns.The component follows the same accessibility patterns as its siblings (WorkspaceNotFound, WorkspaceError): auto-focus on mount,
role="alert"witharia-live="assertive", andaria-hiddenon decorative icons. The conditional description based on slug presence provides appropriate context to users.webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (3)
13-18: Good use of custom error class for type-safe error discrimination.The
WorkspaceForbiddenErrorclass enables type-safeinstanceofchecks in the error component, which is cleaner than checking error properties. Consider exporting this class if other parts of the codebase might need to throw this error type.
20-62: Well-structured route with comprehensive error handling.The loader correctly uses
throwOnError: falseto enable manual HTTP status handling, and the error mapping (404 →notFound(), 403 →WorkspaceForbiddenError) provides clear UX for different failure modes. The defensive check for empty data afterresponse.okis a good safeguard.
64-71: Clean context provider pattern using React 19's simplified Context API.The
<WorkspaceContext value={workspace}>syntax is the correct React 19 pattern (usingvalueprop directly instead of.Provider). This cleanly provides workspace data to all child routes via theuseWorkspacehook.webapp/src/routes/_authenticated/w/$workspaceSlug/index.tsx (6)
15-15: LGTM! Context-based workspace loading is properly integrated.The workspace is now loaded by the parent layout route and provided via context, aligning with the new architecture that centralizes workspace data loading in the parent route.
Also applies to: 49-51
58-70: LGTM! Schedule extraction handles workspace data correctly.The schedule derivation properly extracts and parses the leaderboard schedule from the workspace context with appropriate defaults.
99-99: LGTM! Queries consistently use context-provided workspaceSlug.All data fetching operations now use the workspaceSlug from the workspace context, ensuring consistency across the component.
Also applies to: 106-106, 121-121, 227-227
116-116: LGTM! Query enabled conditions properly simplified.The enabled conditions are correctly simplified by removing redundant workspaceSlug checks, since the workspace is now guaranteed to be available from the parent route's context.
Also applies to: 131-131, 250-250
286-289: LGTM! Navigation correctly uses context-provided workspaceSlug.The navigation to user profile properly includes the workspaceSlug from context in the route parameters.
320-320: LGTM! Loading state logic properly handles placeholder data.The loading calculation correctly combines teams and leaderboard states, and the pattern
leaderboardQuery.isPending && !leaderboardQuery.dataprevents unnecessary loading indicators when placeholder data is available.webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx (4)
5-5: LGTM! Context-based workspace loading correctly implemented.The imports and hook usage properly transition from per-route workspace queries to the centralized context-based approach.
Also applies to: 7-7, 33-33, 37-39
42-55: LGTM! Schedule extraction consistent with other routes.The schedule derivation follows the same pattern as the leaderboard route, ensuring consistency across workspace-scoped routes.
87-87: LGTM! Query enabled condition appropriately simplified.The enabled condition correctly relies only on
usernamesinceworkspaceSlugis guaranteed from the parent route context.
103-103: LGTM! Loading state properly handles placeholder data.The loading calculation uses the same pattern as other routes, preventing unnecessary loading indicators when placeholder data is available.
webapp/src/components/workspace/WorkspaceNotFound.tsx (3)
1-11: LGTM! Imports are appropriate and consistent.The imports properly use TanStack Router for navigation, lucide-react for icons, and the Empty UI primitives for consistent error state presentation.
13-18: LGTM! Props interface is well-defined.The interface provides appropriate optional props with clear documentation for customizing the error state display.
29-87: LGTM! Component implementation is excellent with strong accessibility.The component demonstrates best practices:
- Proper ARIA attributes (
role="alert",aria-live="assertive",aria-atomic="true") for screen reader announcement- Focus management on mount for keyboard navigation
- Clear action buttons with descriptive labels
- Conditional rendering based on props
- Appropriate use of TanStack Router's
invalidate()for retry logicwebapp/src/routeTree.gen.ts (1)
1-10: Generated file - no manual edits needed.This file is automatically generated by TanStack Router tooling. The route tree structure correctly reflects the new workspace-scoped parent route and its children. As per coding guidelines, this file should not be hand-edited.
- Use requestAnimationFrame for focus management (better for a11y) - Add slug truncation for long workspace names (50 char limit) - Improve network error detection using error.name + regex patterns - Add edge case Storybook stories (long slugs, special chars) - Enhance JSDoc with WCAG 2.2 compliance notes - Add break-all and title attribute for truncated text
github-actions
bot
added
size:XXL
…ents - Add defensive handling for undefined/null error.message - Add CORS error detection pattern - Add max-height to error details and truncation for long errors - Add edge case stories: empty message, undefined message, long message - Add edge case stories for slugs: empty, unicode, special chars only - Add useWorkspaceOptional hook for non-throwing context access
…eation Extracts the error details rendering into a separate ErrorDetails component to prevent the inline IIFE from being re-created on each render. This is a minor optimization that improves code clarity.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webapp/src/components/workspace/WorkspaceError.tsx (1)
24-35:isNetworkErrormay produce false positives forTypeError.Treating all
TypeErrors as network errors (line 26) is overly broad.TypeErroris a general JavaScript error that occurs for property access onundefined/null, invalid function invocations, and other non-network scenarios. This could show misleading "temporary issue" messaging for programming bugs.Consider narrowing the heuristic to only flag
TypeErrorwhen the message also matches network patterns:🔎 Proposed fix
function isNetworkError(error: Error): boolean { - // Check error.name first - more reliable than message parsing - if (error.name === "TypeError" || error.name === "NetworkError") { + // NetworkError is explicitly network-related + if (error.name === "NetworkError") { return true; } - // Fallback: check common fetch/network error patterns in message + // Check common fetch/network error patterns in message // Guard against undefined/null message (defensive) const message = error.message ?? ""; const networkPatterns = /\b(fetch|network|timeout|abort|connection|offline|cors)\b/i; - return networkPatterns.test(message); + + // TypeError with fetch-related message is likely a network error + if (error.name === "TypeError" && networkPatterns.test(message)) { + return true; + } + + return networkPatterns.test(message); }webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
91-94: Minor: Inline arrow function in onClick.The inline arrow function
() => navigate({ to: "/" })creates a new function reference on each render. This is unlikely to cause issues in practice, but for consistency withWorkspaceError, you could extract it to ahandleGoHomecallback. Per coding guidelines, trust the React Compiler for common cases, so this is optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
webapp/src/components/workspace/NoWorkspace.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsxwebapp/src/components/workspace/index.tswebapp/src/hooks/use-workspace.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- webapp/src/hooks/use-workspace.ts
- webapp/src/components/workspace/WorkspaceForbidden.stories.tsx
- webapp/src/components/workspace/index.ts
- webapp/src/components/workspace/NoWorkspace.tsx
- webapp/src/components/workspace/WorkspaceForbidden.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/components/workspace/WorkspaceError.tsxwebapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/WorkspaceError.stories.tsxwebapp/src/components/workspace/WorkspaceNotFound.stories.tsx
🧬 Code graph analysis (2)
webapp/src/components/workspace/WorkspaceError.stories.tsx (2)
webapp/src/components/workspace/WorkspaceError.tsx (1)
WorkspaceError(72-145)webapp/src/components/workspace/index.ts (1)
WorkspaceError(13-13)
webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (3)
webapp/src/components/workspace/index.ts (1)
WorkspaceNotFound(19-19)webapp/src/components/workspace/WorkspaceNotFound.tsx (1)
WorkspaceNotFound(30-98)webapp/src/components/workspace/NoWorkspace.stories.tsx (1)
Default(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / application-server-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webapp-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webhook-ingest-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / intelligence-service-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: security-scan / dependencies
- GitHub Check: test-suite / webapp-visual
- GitHub Check: security-scan / sast
🔇 Additional comments (9)
webapp/src/components/workspace/WorkspaceError.tsx (2)
72-96: Well-structured component with proper accessibility and navigation.The implementation correctly addresses the previous review comment by using TanStack Router's
navigate()for client-side navigation. The accessibility setup (role="alert", aria-live, requestAnimationFrame focus) follows WCAG 2.2 patterns. The retry logic appropriately falls back torouter.invalidate()when no reset function is provided.
44-60: Good extraction ofErrorDetailsto avoid render-time recreation.Extracting this as a separate component rather than an inline IIFE prevents unnecessary function recreation on each render. The truncation logic with character count display is helpful for debugging without breaking layout.
webapp/src/components/workspace/WorkspaceError.stories.tsx (2)
1-48: Comprehensive meta documentation and configuration.The JSDoc explains usage context, accessibility features, and provides a practical example. The argTypes correctly disable controls for complex object props. Good alignment with Storybook conventions.
117-145: Good defensive edge-case coverage.The
EmptyMessage,UndefinedMessage, andLongErrorMessagestories exercise the component's defensive handling. The@ts-expect-errorannotation for the undefined message test is the correct approach for testing edge cases that violate type contracts.webapp/src/components/workspace/WorkspaceNotFound.tsx (2)
30-46: Consistent accessibility and routing patterns.The component mirrors
WorkspaceError's accessibility implementation (requestAnimationFrame focus, aria attributes) and uses TanStack Router'snavigate()for client-side navigation. Good consistency across the workspace error components.
48-49: Slug truncation logic is correct.The truncation at 50 characters with ellipsis (
47 + "...") combined with thetitleattribute on the<strong>element provides a good UX—users can hover to see the full slug while the layout remains stable.webapp/src/components/workspace/WorkspaceNotFound.stories.tsx (3)
4-39: Well-documented meta with clear usage guidance.The JSDoc addresses the component's purpose and when to use it (HTTP 404 in route's
notFoundComponent), documents accessibility features, and provides a practical code example. This aligns with the previous review feedback about focusing on usage context.
62-74: Good edge-case coverage for slug handling.The
LongSlugstory validates truncation behavior, andSpecialCharacterstests that potentially malicious input (XSS-like strings) is rendered safely. React's JSX escaping handles this by default, but having explicit story coverage for Chromatic visual regression is valuable.
83-95: Unicode and special character stories add internationalization coverage.The
UnicodeSlugstory with emoji and mixed scripts (Japanese, Arabic) andOnlySpecialCharsensure the component handles diverse character sets correctly. This is good defensive coverage for user-generated workspace names.
Makes the error class available for instanceof checks in consumer code, matching the usage examples shown in Storybook documentation.
…ure extensibility Allows custom denial messages for permission-specific scenarios like: - Role requirements (Owner, Admin, Member) - Feature-specific access control - Subscription tier restrictions Includes new story demonstrating the reason prop usage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
43-44: Consider using Tooltip component for improved UX.The current implementation uses
titleattribute to show the full slug on hover. While functional, shadcn/ui likely provides aTooltipcomponent that would offer better styling consistency and accessibility (e.g., better mobile support, keyboard navigation).🔎 Optional enhancement with Tooltip component
If a
Tooltipcomponent exists in@/components/ui/tooltip, consider this pattern:+import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; // ... in the render -<strong className="break-all" title={slug}> - "{displaySlug}" -</strong> +{displaySlug !== slug ? ( + <TooltipProvider> + <Tooltip> + <TooltipTrigger asChild> + <strong className="break-all">"{displaySlug}"</strong> + </TooltipTrigger> + <TooltipContent>{slug}</TooltipContent> + </Tooltip> + </TooltipProvider> +) : ( + <strong className="break-all">"{displaySlug}"</strong> +)}This provides better accessibility and mobile support while maintaining the truncation behavior.
Also applies to: 65-70
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/general-coding.instructions.md)
**/*.{ts,tsx}: Use PascalCase for component names, interfaces, and type aliases
Use camelCase for variables, functions, and methods
Prefix private class members with underscore (_)
Use ALL_CAPS for constants
Use try/catch blocks for async operations
Implement proper error boundaries in React components
Always log errors with contextual information
**/*.{ts,tsx}: Ship new code as TypeScript. Reach fortypealiases when composing shapes andinterfacewhen you need declaration merging or extension.
Default toconst, mark collectionsreadonlywhen practical, and avoid mutating function arguments.
Rely on optional chaining (?.) and nullish coalescing (??) when working with generated API responses.
Export prop and return-value types so other modules can reuse them (for exampleexport interface MentorCardProps).
Import with the@/*alias defined intsconfig.json. Keep relative paths shallow.
Use discriminated unions orzodschemas for request and response guards. Assert intent with thesatisfiesoperator instead of broad casts.
Model async flows withPromiseSettledResulthelpers and exhaustively handle states.
Author components as named functions (export function Component(props: ComponentProps)). AvoidReact.FC; annotate props explicitly.
Obey the Rules of Hooks. No conditional hook calls or hook invocations inside loops.
Keep render logic pure—never mutate stores, perform navigation, or touch the DOM during render.
Prefer named exports for components and hooks so tree shaking stays predictable.
Fetch data with TanStack Query v5. Spread the generated option helpers fromsrc/api/@tanstack/react-query.gen.tsintouseQuery/useMutationso request typing, retries, and query keys stay in sync with the server.
Use the sharedQueryClientprovided bysrc/integrations/tanstack-query/root-provider.tsx. Invalidate with the generated...QueryKey()helpers instead of literal arrays.
Reuse the preconfigured OpenAPIclient(set up in `main.t...
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Do not useReact.FCtype annotation for functional components in TSX files
Fetch data exclusively with TanStack Query v5 and the generated helpers in@/api/@tanstack/react-query.gen.ts, spreading the option objects likeuseQuery(getTeamsOptions({ ... }))
Use generated*.QueryKey()helpers for cache invalidation in TanStack Query
Do not callfetchdirectly; reuse the generated@hey-apiclient configured insrc/api/client.tsand the shared QueryClient fromsrc/integrations/tanstack-query/root-provider.tsx
Use Tailwind utility classes and shadcn primitives for styling, composing class names withclsx/tailwind-mergeand using tokenized colors (bg-surface,text-muted, etc.)
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsx
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
webapp/src/routes/_authenticated/w/$workspaceSlug.tsxwebapp/src/components/workspace/WorkspaceForbidden.tsxwebapp/src/components/workspace/WorkspaceForbidden.stories.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/tsx.instructions.md)
**/*.stories.{ts,tsx}: Keep stories colocated and represent the real UI states that Chromatic validates.
Use Storybook play functions for interactive flows instead of end-to-end tests when the surface is presentational.
Files:
webapp/src/components/workspace/WorkspaceForbidden.stories.tsx
🧬 Code graph analysis (2)
webapp/src/components/workspace/WorkspaceForbidden.tsx (3)
webapp/src/components/workspace/index.ts (2)
WorkspaceForbiddenProps(16-16)WorkspaceForbidden(15-15)webapp/src/components/ui/empty.tsx (4)
EmptyHeader(94-94)EmptyMedia(94-94)EmptyTitle(94-94)EmptyDescription(94-94)webapp/src/components/ui/button.tsx (1)
Button(59-59)
webapp/src/components/workspace/WorkspaceForbidden.stories.tsx (2)
webapp/src/components/workspace/WorkspaceForbidden.tsx (1)
WorkspaceForbidden(30-88)webapp/src/components/workspace/index.ts (1)
WorkspaceForbidden(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: docker-build / application-server-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / intelligence-service-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / intelligence-service-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/intelligence-service
- GitHub Check: docker-build / application-server-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/application-server
- GitHub Check: docker-build / webapp-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / webapp-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webapp
- GitHub Check: docker-build / webhook-ingest-build / Build linux/arm64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: docker-build / webhook-ingest-build / Build linux/amd64 Docker Image for ls1intum/hephaestus/webhook-ingest
- GitHub Check: quality-gates / webapp-quality
- GitHub Check: quality-gates / database-models-validation
- GitHub Check: quality-gates / database-documentation-validation
- GitHub Check: quality-gates / openapi-validation
- GitHub Check: quality-gates / database-schema-validation
- GitHub Check: quality-gates / intelligence-service-quality
- GitHub Check: quality-gates / webhook-ingest-quality
- GitHub Check: test-suite / webapp-visual
- GitHub Check: security-scan / sast
🔇 Additional comments (7)
webapp/src/components/workspace/WorkspaceForbidden.tsx (2)
30-41: LGTM! Proper focus management implementation.The auto-focus implementation correctly uses
requestAnimationFrameto ensure the DOM is fully painted before focusing, and includes proper cleanup withcancelAnimationFrame. The empty dependency array is intentional for mount-only execution, which aligns with the accessibility requirement.
46-88: Excellent accessibility implementation.The component properly implements WCAG 2.2 requirements with:
- Assertive live region for immediate screen reader announcement
- Atomic updates for complete message reading
- Programmatic focus support with
tabIndex={-1}- Decorative icons correctly hidden from assistive technology
- Conditional messaging that gracefully handles missing props
The navigation logic correctly uses TanStack Router's
navigateAPI.webapp/src/routes/_authenticated/w/$workspaceSlug.tsx (3)
9-19: LGTM! Well-structured custom error class.The
WorkspaceForbiddenErrorclass properly extendsErrorwith:
- A public
slugproperty for contextual information- Descriptive error message
- Custom
namefor error identification in boundaries- Exported for instanceof checks in error components
This follows TypeScript best practices and enables proper type narrowing in error boundaries.
22-49: Loader implementation is correct.The loader properly:
- Uses
throwOnError: falsefor manual error handling- Maps HTTP statuses to appropriate error types (404 → notFound(), 403 → WorkspaceForbiddenError)
- Returns strongly-typed Workspace data
The additional null check at lines 43-46 is defensive but not harmful. When
response.okis true, the generated client should guaranteeresult.dataexists, making this check technically redundant.
50-61: LGTM! Proper error boundary implementation.The route correctly implements TanStack Router's error handling:
notFoundComponentextracts params and renders the appropriate 404 stateerrorComponentusesinstanceoffor type-safe error discrimination- The
resetcallback is properly passed toWorkspaceErrorfor retry functionalityThis aligns with the framework's error boundary patterns.
webapp/src/components/workspace/WorkspaceForbidden.stories.tsx (2)
4-41: LGTM! Well-documented Storybook configuration.The JSDoc comment properly addresses the previous review feedback by focusing on when and how to use the component (route error boundaries for 403 scenarios). The meta configuration follows best practices:
- Clear accessibility documentation
- Practical usage example with error boundary pattern
- Proper
satisfies Meta<typeof WorkspaceForbidden>for type safety- Well-documented argTypes for both
slugandreasonprops
46-84: Excellent story coverage for edge cases.The story set comprehensively tests various scenarios:
- Default and empty states for generic messaging
- Slug variations (normal, long, empty, Unicode) to validate truncation and character handling
- Custom reason prop to demonstrate permission-specific messaging
This aligns with Chromatic visual regression testing needs and provides clear examples for consumers.
| function WorkspaceLayout() { | ||
| const workspace = Route.useLoaderData(); | ||
|
|
||
| return ( | ||
| <WorkspaceContext value={workspace}> | ||
| <Outlet /> | ||
| </WorkspaceContext> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find WorkspaceContext definition
rg -r "WorkspaceContext\s*=" --type ts --type tsxRepository: ls1intum/Hephaestus
Length of output: 459
🏁 Script executed:
# Check use-workspace.ts file location and content
fd "use-workspace" --type fRepository: ls1intum/Hephaestus
Length of output: 137
🏁 Script executed:
# Search for createContext with WorkspaceContext
rg "createContext.*Workspace|WorkspaceContext.*createContext" --type ts --type tsx -A 3Repository: ls1intum/Hephaestus
Length of output: 90
🏁 Script executed:
# Read the use-workspace.ts file
cat webapp/src/hooks/use-workspace.tsRepository: ls1intum/Hephaestus
Length of output: 1049
🏁 Script executed:
# Search for WorkspaceContext definition more broadly
rg "WorkspaceContext" --type tsRepository: ls1intum/Hephaestus
Length of output: 859
🏁 Script executed:
# Double-check the actual component to ensure we're seeing the full picture
sed -n '65,73p' webapp/src/routes/_authenticated/w/$workspaceSlug.tsxRepository: ls1intum/Hephaestus
Length of output: 145
🏁 Script executed:
# List files to get the exact filename
ls -la webapp/src/routes/_authenticated/w/Repository: ls1intum/Hephaestus
Length of output: 305
🏁 Script executed:
# Use find to locate the file
find webapp/src/routes/_authenticated/w/ -type f -name "*.tsx"Repository: ls1intum/Hephaestus
Length of output: 830
🏁 Script executed:
# Read the WorkspaceLayout component file
cat webapp/src/routes/_authenticated/w/\$workspaceSlug.tsxRepository: ls1intum/Hephaestus
Length of output: 2419
Fix WorkspaceContext provider syntax.
Line 69-71 must use <WorkspaceContext.Provider> instead of <WorkspaceContext>. The createContext() API requires .Provider to wrap child components:
<WorkspaceContext.Provider value={workspace}>
<Outlet />
</WorkspaceContext.Provider>Without .Provider, context will not propagate to child components, breaking useWorkspace() calls throughout the workspace routes.
🤖 Prompt for AI Agents
In webapp/src/routes/_authenticated/w/$workspaceSlug.tsx around lines 65 to 73,
the component is using <WorkspaceContext> instead of the context provider;
replace the wrapper with WorkspaceContext.Provider and pass the workspace as the
value prop so child components receive the context (i.e., wrap <Outlet /> with
WorkspaceContext.Provider value={workspace} and close with
</WorkspaceContext.Provider>).
…ries - Reduce Storybook stories to essential UI variants only - WorkspaceError: 11→3 stories (Default, NetworkError, WithReset) - WorkspaceForbidden: 6→4 stories (Default, WithSlug, LongSlug, WithReason) - WorkspaceNotFound: 8→4 stories (Default, WithSlug, WithRetry, LongSlug) - Delete redundant edge cases (React escapes XSS by default) - Architecture already uses TanStack Router error boundaries - Context uses correct React 19 shorthand syntax
Summary
This PR adds presentational components for workspace error states, following maintainer feedback to use TanStack Router's built-in error handling instead of custom context-based solutions.
Key Changes:
WorkspaceForbidden(403) - Access denied state with optional custom reasonWorkspaceNotFound(404) - Workspace not found stateWorkspaceError- Generic error with retry capability and smart network detectionNoWorkspace- Empty state for users without workspace membershipsuseWorkspace/useWorkspaceOptionalhooks - React 19use()based context consumersWorkspaceForbiddenError- Exported error class for type guards/w/$workspaceSlugwith integrated error handlingAccessibility (WCAG 2.2)
All components follow accessibility best practices:
role="alert"witharia-live="assertive"for error announcementsrole="status"witharia-live="polite"for non-critical messagesaria-atomicfor complete message readingrequestAnimationFramefor keyboard navigationaria-hiddento prevent redundant announcementsFuture Extensibility
Components are designed for future workspace features:
WorkspaceForbiddenaccepts optionalreasonprop for role-specific messagesWorkspaceErroruses smart network error detection viaerror.name+ regexuseWorkspace) and non-throwing (useWorkspaceOptional) hooksEdge Cases Handled
What Was Removed (Based on Review Feedback)
Per maintainer feedback questioning the architectural approach:
WorkspaceProvidercontext - TanStack Router provides built-in error handlingmock-workspace-provider.tsx- No longer neededuse-go-home.ts- Unnecessary abstractionHow to Test
npm run check- All quality gates passworkspace/(26 stories)Architecture
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.