-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat 137 project tags schema #258
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis PR introduces new API endpoints for managing project tags. Two route files are added: one for operations on individual tags (GET, PUT, DELETE) and another for listing and creating tags (GET, POST), each with appropriate validation and error handling. A new SQL migration file creates the necessary tables for project tags and their relationships. Additionally, the documentation is updated by removing the Passkey Authentication section and renumbering the following sections. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant API as API (Single Tag)
participant DB as Supabase
C->>API: GET/PUT/DELETE /api/tag/{tagId}
API->>DB: Query, update, or delete tag record
DB-->>API: Return tag data / confirmation or error
API-->>C: Return JSON response with proper status
sequenceDiagram
participant C as Client
participant API as API (Tag Collection)
participant DB as Supabase
C->>API: GET /api/tag?page=&pageSize=
API->>DB: Query paginated list of tags
DB-->>API: Return tag list and pagination metadata
API-->>C: Return JSON response with tag data
C->>API: POST /api/tag (new tag details)
API->>API: Validate input and generate color if missing
API->>DB: Insert new tag into database
DB-->>API: Return newly created tag or error
API-->>C: Return HTTP response (201 on success, error code otherwise)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/app/api/tag/route.ts (1)
4-7
: The color generation function works well but could be enhanced.The hash-based color generation is a nice approach for deterministic colors based on tag names. Consider adding a validation step to ensure the output is always a valid 6-character hex color code, as the current implementation might produce fewer than 6 characters in rare cases.
function generateColor(name: string): string { const hash = [...name].reduce((acc, c) => acc + c.charCodeAt(0), 0) - return `#${((hash * 2654435761) & 0xffffff).toString(16).padStart(6, '0')}` + const hexColor = ((hash * 2654435761) & 0xffffff).toString(16).padStart(6, '0') + return `#${hexColor}` }apps/web/app/api/tag/[tagId]/route.ts (1)
31-82
: PUT implementation could enhance error handling.The input validation is thorough with good sanitization. Consider adding specific handling for the 'not found' case similar to what's done in the GET method to return a 404 status when updating a non-existent tag.
if (error) { + if (error.code === 'PGRST116' || error.message.includes('not found')) { + return NextResponse.json({ error: 'Tag not found' }, { status: 404 }) + } return NextResponse.json({ error: error.message }, { status: 500 }) }Also, consider whether the character restrictions on tag names are appropriate for your use case - the current regex allows only alphanumeric characters, spaces, hyphens, and underscores.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/app/api/tag/[tagId]/route.ts
(1 hunks)apps/web/app/api/tag/route.ts
(1 hunks)apps/web/components/featured/projects-grid.tsx
(2 hunks)apps/web/components/sections/projects/projects-grid.tsx
(2 hunks)apps/web/components/shared/project-card.tsx
(10 hunks)apps/web/hooks/use-projects-filter.ts
(1 hunks)apps/web/lib/mock-data/featured-projects/mock-featured-projets.ts
(4 hunks)apps/web/lib/mock-data/mock-projects-view.tsx
(16 hunks)apps/web/lib/types/featured-projects/featured-projects.types.ts
(1 hunks)apps/web/lib/types/projects.types.ts
(1 hunks)apps/web/lib/types/user-dashboard.types.ts
(1 hunks)services/supabase/migrations/20250302053608_create_project_tags.sql
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- apps/web/lib/types/featured-projects/featured-projects.types.ts
- apps/web/hooks/use-projects-filter.ts
- apps/web/lib/mock-data/mock-projects-view.tsx
- apps/web/lib/types/projects.types.ts
- apps/web/components/shared/project-card.tsx
- apps/web/lib/mock-data/featured-projects/mock-featured-projets.ts
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts
: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/web/lib/types/user-dashboard.types.ts
apps/web/app/api/tag/[tagId]/route.ts
apps/web/app/api/tag/route.ts
`apps/web/app/api/**/*.ts`: Review API routes ensuring prope...
apps/web/app/api/**/*.ts
: Review API routes ensuring proper input validation and error handling. Verify security measures including rate limiting and authentication. Check response formatting and error states.
apps/web/app/api/tag/[tagId]/route.ts
apps/web/app/api/tag/route.ts
🧠 Learnings (1)
services/supabase/migrations/20250302053608_create_project_tags.sql (1)
Learnt from: AndlerRL
PR: kindfi-org/kindfi#147
File: services/supabase/migrations/20250211_update_projects_table.sql:2-8
Timestamp: 2025-02-12T08:42:03.870Z
Learning: In PostgreSQL schemas for the Kindfi project, update categories and similar predefined sets of values should be implemented using ENUM types instead of text arrays for better type safety and data consistency.
🔇 Additional comments (10)
apps/web/components/featured/projects-grid.tsx (2)
4-4
: Great improvement to TypeScript type imports!I appreciate the explicit use of the
type
keyword in the import statement. This clarifies thatProject
is used solely as a type and helps with type checking, which aligns with TypeScript best practices.
27-31
: Nice formatting enhancement for better readability.The filter logic has been nicely restructured with proper indentation, making the conditional logic easier to follow. I also notice the consistent use of single quotes for the empty string, which helps maintain style consistency throughout the codebase.
apps/web/components/sections/projects/projects-grid.tsx (2)
2-2
: Excellent TypeScript type import clarification.The addition of the
type
keyword for theProject
import provides better clarity on how it's being used. This type-only import approach is consistent with modern TypeScript best practices and improves code maintainability.
9-9
: Clean function signature formatting.The simplified function declaration maintains readability while reducing unnecessary line breaks. This concise approach to parameter destructuring makes the code more elegant without sacrificing clarity.
apps/web/lib/types/user-dashboard.types.ts (1)
2-2
: Well-implemented type-only import.The change to use
import type
for the Tag import is excellent practice. This clarifies that Tag is used solely for type checking purposes, which helps with tree-shaking and potentially reduces bundle size while preventing value imports where only types are needed.apps/web/app/api/tag/route.ts (1)
9-46
:❓ Verification inconclusive
Well-implemented GET endpoint with proper pagination.
The implementation includes excellent pagination handling with appropriate parameter validation and constraints. The response structure with pagination metadata is very clean and follows best practices.
Consider adding authentication checks if this endpoint should be restricted to authenticated users.
🏁 Script executed:
#!/bin/bash # Check if there's any authentication middleware in use for API routes fd "middleware|auth" --type f --extension ts,js apps/web/app/ -x grep -l "auth|authenticate|protected" {} \;Length of output: 113
Authentication Check Advisory for GET Endpoint
The GET endpoint is cleanly implemented with robust pagination handling and error management. However, our verification didn't uncover any authentication checks in the API routes. If this endpoint should be restricted to authenticated users, consider adding proper authentication middleware (e.g., session or JWT verification). Please manually verify whether the absence of such checks is intentional or if additional security measures are needed.
services/supabase/migrations/20250302053608_create_project_tags.sql (2)
1-7
: Well-designed project_tags table with appropriate constraints.The table structure is excellent with appropriate use of UUID as primary key and properly constrained fields. The color validation using a regex pattern ensures data integrity, and the name uniqueness constraint prevents duplicates. The timestamps will be useful for tracking creation and updates.
9-13
: Excellent implementation of many-to-many relationship.The junction table with cascading delete behavior is well-designed, maintaining referential integrity while allowing for efficient queries. The composite primary key on (project_id, tag_id) is the correct approach for this relationship.
apps/web/app/api/tag/[tagId]/route.ts (2)
4-29
: GET implementation with good error handling for 'not found' case.The endpoint correctly handles the 'not found' scenario with an appropriate 404 status code. The error handling approach is robust, distinguishing between different types of errors and providing appropriate status codes.
84-110
: DELETE implementation handles 'not found' case appropriately.The endpoint correctly verifies whether any records were deleted and returns a 404 status when the tag doesn't exist. The error handling is robust with appropriate status codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Villarley we noticed this branch is out-of-date with the base branch. Please rebase it
* feat: add ssr and replace express * docs: it closes #164 * refactor: move passkey validation to kyc server * chore: it closes #166 --------- Co-authored-by: Brandon Fernández <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/app/api/tag/[tagId]/route.ts (4)
5-7
: Consider removing the unused response parameter.
We could enhance consistency by removing or renaming the second parameter in each handler, as it is not being used. This helps prevent confusion and clarifies the function signatures.-export async function GET( - request: NextRequest, - response: NextResponse, - { params }: { params: { tagId: string } }, -) { +export async function GET( + request: NextRequest, + { params }: { params: { tagId: string } } +) { // ... }Also applies to: 33-36, 91-94
9-31
: Centralize repeated error handling.
We could improve maintainability by introducing a helper function or a shared error-handling utility. This prevents duplication of returning JSON errors in each handler and keeps the code more DRY.
46-51
: Consolidate input validation checks.
We are already validating the existence and format of the tag name. Consider extracting these checks into a small utility function to streamline the code and promote reusability.Also applies to: 62-67
96-123
: Consider verifying foreign key constraints and adding tests.
When deleting tags, ensure no active references break. We could return a more descriptive error if the tag is linked to existing records. Also, consider unit tests to confirm expected behaviors for all status codes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/kyc-server/src/routes/index.ts
(1 hunks)apps/web/app/api/tag/[tagId]/route.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/kyc-server/src/routes/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts
: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/web/app/api/tag/[tagId]/route.ts
`apps/web/app/api/**/*.ts`: Review API routes ensuring prope...
apps/web/app/api/**/*.ts
: Review API routes ensuring proper input validation and error handling. Verify security measures including rate limiting and authentication. Check response formatting and error states.
apps/web/app/api/tag/[tagId]/route.ts
28e3b22
to
e972688
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
apps/web/hooks/passkey/use-passkey-authentication.ts (1)
57-57
: 💡 Verification agent🧩 Analysis chain
Consider addressing the TODO comment for type safety
The TODO comment indicates missing type definitions for authenticationOptions. Consider defining an interface for this response to ensure type safety throughout the authentication flow.
🌐 Web query:
What is the typical TypeScript type for WebAuthn authentication options?
💡 Result:
The typical TypeScript type for WebAuthn authentication options in the SimpleWebAuthn ecosystem is
PublicKeyCredentialRequestOptionsJSON
. This type represents the structured authentication options required to initiate a WebAuthn authentication flow[1][7].Key properties of this type include:
challenge
: A Base64URL-encoded cryptographic challengerpId
: The relying party identifier (typically your domain)allowCredentials
: Array of allowed credential IDs and transportsuserVerification
: Specifies if biometric verification is required ("required"
,"preferred"
, or"discouraged"
)timeout
: Authentication attempt duration in millisecondsExample usage with SimpleWebAuthn:
import type { PublicKeyCredentialRequestOptionsJSON } from '@simplewebauthn/typescript-types'; const options: PublicKeyCredentialRequestOptionsJSON = await generateAuthenticationOptions({ rpID: 'example.com', allowCredentials: [{ id: 'credentialIdHere', transports: ['internal', 'hybrid'], type: 'public-key' }], userVerification: 'preferred' });This type is part of the
@simplewebauthn/typescript-types
package[6] and is used in conjunction with SimpleWebAuthn's server-sidegenerateAuthenticationOptions()
method[1][7]. It aligns with the WebAuthn specification's authentication requirements while providing TypeScript developers with strong typing guarantees[7][9].Citations:
- 1: https://simplewebauthn.dev/docs/packages/server
- 2: https://simplewebauthn.dev
- 3: https://www.telerik.com/blogs/integrate-biometric-authentication-nextjs-app-simplewebauthn-package-part-1
- 4: https://blog.logrocket.com/implementing-webauthn-for-passwordless-logins/
- 5: https://auth0.com/blog/webauthn-a-short-introduction/
- 6: https://classic.yarnpkg.com/en/package/@simplewebauthn/typescript-types
- 7: https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API
- 8: https://webauthn.guide
- 9: https://github.com/yackermann/awesome-webauthn
- 10: https://www.telerik.com/blogs/integrate-biometric-authentication-nextjs-app-simplewebauthn-package-part-2
Enhance Type Safety for WebAuthn Authentication Options
Consider updating the code to explicitly type the
authenticationOptions
variable. Based on our research, a good choice is to use thePublicKeyCredentialRequestOptionsJSON
type from the@simplewebauthn/typescript-types
package. This change will help enforce type safety and make the authentication flow more robust. For instance, you could adjust the code as follows:import type { PublicKeyCredentialRequestOptionsJSON } from '@simplewebauthn/typescript-types'; // ... const authenticationOptions: PublicKeyCredentialRequestOptionsJSON = await resp.json();This modification leverages well-established typings for WebAuthn authentication options, ensuring consistency and clarity throughout the codebase.
apps/web/hooks/stellar/use-stellar.ts (1)
87-102
: 🛠️ Refactor suggestionConsider safer implementation for disabled signing logic
While commenting out error checks might be necessary temporarily, we could enhance this by:
- Returning a mock result with appropriate structure instead of disabling checks
- Adding a clearer comment about when this should be re-enabled
- Implementing graceful fallbacks
This would maintain type safety while still allowing development to proceed.
🧹 Nitpick comments (66)
apps/kyc-server/docs/ROUTING.md (9)
1-12
: Introduction and OverviewThe introduction clearly explains the hybrid routing approach that combines SSR with client-side routing. Consider enhancing this section with a simple diagram or flowchart to visually represent the request lifecycle for new contributors. Also, review the use of compound adjectives (e.g., ensuring consistency with terms such as "server-side" and "client-side") as noted by static analysis.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...benefits, and smooth navigation without full page reloads. ## Server-Side Routing ### I...(EN_COMPOUND_ADJECTIVE_INTERNAL)
14-23
: Server-Side Routing OverviewThis section effectively identifies where server-side routing is implemented (i.e., in
src/index.tsx
andsrc/routes/react.tsx
). To further improve traceability, you might consider adding links or brief descriptions of these files.
24-50
: Route Definition Code BlockThe TypeScript snippet for
reactRoutes
is clear and well-structured. The use of a dedicated CORS wrapper (withConfiguredCORS
) and asynchronous GET handlers demonstrates a thoughtful approach to handling requests. A minor enhancement could be to include inline comments that explain why specific methods (likerenderToReadableStream
) are chosen.
73-85
: Server-Side Rendering SectionThe snippet conveying the use of
renderToReadableStream
for transforming React components into an HTML stream is succinct and clear. You might consider incorporating basic error handling or fallback mechanisms in scenarios where rendering might fail, to further increase robustness.
122-156
: Navigation Component ImplementationThe
App
component is implemented neatly with active link highlighting using theuseLocation
hook. For enhanced accessibility, consider adding ARIA attributes to the navigation elements. Moreover, ensure the component is tested across diverse devices to maintain responsiveness.
171-191
: How It Works TogetherThis section provides an excellent step-by-step breakdown of the routing lifecycle—from the initial request through hydration to client-side navigation. Including a visual aid or diagram here could further improve clarity for developers unfamiliar with the overall process.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~190-~190: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...nders the appropriate component - No full page reload occurs ## Benefits - **Fast In...(EN_COMPOUND_ADJECTIVE_INTERNAL)
193-198
: Benefits ListingThe benefits are clearly articulated, emphasizing fast initial loads, SEO advantages, and smooth navigation. As a minor note, double-check that all compound adjectives are consistently hyphenated per style guidelines.
200-204
: Considerations SectionThis section wisely outlines potential concerns such as hydration mismatches and increased HTML size. It may be beneficial to add references or links to additional resources that offer strategies for mitigating these issues.
206-213
: Advanced Techniques DiscussionThe advanced techniques section succinctly lists best practices like data loading, code splitting, prefetching, and transitions. Expanding this section with brief examples or links to further reading materials could provide additional guidance for developers looking to implement these techniques.
apps/kyc-server/src/components/App.tsx (3)
1-3
: Consider adding explicit type imports.The imports look good, but we could enhance type safety by explicitly importing types from 'react-router-dom'.
import React from 'react' -import { Link, Outlet, useLocation } from 'react-router-dom' +import { Link, Outlet, useLocation, Location } from 'react-router-dom'
4-6
: Consider adding type annotations for improved type safety.We could enhance this component with TypeScript type annotations for better type safety and documentation.
-export function App() { +export function App(): React.ReactElement { - const location = useLocation() + const location: Location = useLocation()
7-36
: Consider extracting navigation links to a separate component or configuration.The navigation logic works well, but we could improve maintainability by extracting the repeated pattern into a more scalable structure. This would make adding or modifying navigation items easier in the future.
+interface NavItem { + path: string; + label: string; +} + +const navItems: NavItem[] = [ + { path: '/', label: 'Home' }, + { path: '/react', label: 'React Demo' }, + { path: '/websocket', label: 'WebSocket Demo' } +]; + return ( <> <nav> <ul> - <li> - <Link to="/" className={location.pathname === '/' ? 'active' : ''}> - Home - </Link> - </li> - <li> - <Link - to="/react" - className={location.pathname === '/react' ? 'active' : ''} - > - React Demo - </Link> - </li> - <li> - <Link - to="/websocket" - className={location.pathname === '/websocket' ? 'active' : ''} - > - WebSocket Demo - </Link> - </li> + {navItems.map((item) => ( + <li key={item.path}> + <Link + to={item.path} + className={location.pathname === item.path ? 'active' : ''} + > + {item.label} + </Link> + </li> + ))} </ul> </nav> <Outlet /> </> )apps/kyc-server/src/routes/index.ts (1)
5-10
: Well-organized route consolidation approach.The approach of combining all routes into a single export is clean and maintainable. This pattern makes it easy to add new route modules in the future without changing the structure.
Consider adding TypeScript type annotations to make the API contract more explicit and provide better IDE support:
- export const routes = { + export const routes: Record<string, Record<string, (req: Request) => Response | Promise<Response>>> = { ...pingRoutes, ...passkeyRoutes, ...reactRoutes, }apps/kyc-server/src/components/About.tsx (1)
3-24
: Clean and well-structured component.The component follows React best practices with proper JSX structure and semantic HTML elements.
Consider extracting the inline styles to a separate CSS module or styled components for better maintainability as the application grows:
+ import styles from './About.module.css'; export function About() { return ( <div> <h2>About This App</h2> <p> This is a server-side rendered React application built with Bun. It demonstrates how to create a modern web application with: </p> - <ul style={{ marginLeft: '1.5rem', listStyleType: 'disc' }}> + <ul className={styles.featureList}> <li>Server-side rendering (SSR)</li> <li>Client-side hydration</li> <li>WebSocket communication</li> <li>Clean component architecture</li> <li>TypeScript for type safety</li> </ul> - <p style={{ marginTop: '1rem' }}> + <p className={styles.description}> The application uses a unified approach to rendering, ensuring that the server and client render the same content to avoid hydration errors. </p> </div> ) }apps/kyc-server/src/utils/error-handler.ts (1)
6-17
: Robust error handling utility with room for enhancement.The utility function handles errors consistently and distinguishes between application-specific errors and unexpected errors. The type of
unknown
for the error parameter is a good practice for type safety.We could enhance this by using more specific HTTP status codes based on the error type rather than using 500 for all errors:
export const handleError = (error: unknown) => { console.error('Error:', error) if (error instanceof InAppError) { - return Response.json({ error: ERROR_MESSAGES[error.code] }, { status: 500 }) + // Use appropriate status code based on the error type + const statusCode = error.httpStatusCode || 500 + return Response.json({ error: ERROR_MESSAGES[error.code] }, { status: statusCode }) } return Response.json( { error: 'An unexpected error occurred' }, { status: 500 }, ) }This would require adding an optional
httpStatusCode
property to theInAppError
class, but would provide more accurate error responses to clients.apps/kyc-server/src/routes/ping.ts (2)
4-8
: Well-encapsulated CORS handling for route handlers.The
withConfiguredCORS
function provides a clean way to wrap route handlers with CORS middleware.Consider adding a return type annotation for better type safety and documentation:
// Create a configured CORS handler const withConfiguredCORS = ( handler: (req: Request) => Response | Promise<Response>, -) => withCORS(handler, corsConfig) +): ((req: Request) => Promise<Response>) => withCORS(handler, corsConfig)
9-21
: Clear and functional ping route implementation.The ping route is well-structured with appropriate HTTP methods.
Consider enhancing the response with additional useful information:
async GET(req: Request) { return withConfiguredCORS(async () => { return Response.json({ message: 'pong', method: 'GET', + timestamp: new Date().toISOString(), + version: process.env.APP_VERSION || '1.0.0', }) })(req) },Also, it would be helpful to add explicit return types to make the API contract clearer:
- async GET(req: Request) { + async GET(req: Request): Promise<Response> {apps/kyc-server/src/components/WebSocketDemo.tsx (3)
19-58
: Consider implementing a reconnection strategy for WebSocketThe WebSocket connection implementation is good, with proper event handlers for all connection states. However, we could enhance this by implementing an automatic reconnection strategy when the connection unexpectedly closes.
useEffect(() => { + let reconnectTimeout: number; + let reconnectAttempts = 0; + const maxReconnectAttempts = 5; + const reconnectDelay = 2000; + + const setupWebSocket = () => { // Get the current host const host = window.location.host const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:' const wsUrl = `${protocol}//${host}/live` const ws = new WebSocket(wsUrl) ws.onopen = () => { setConnected(true) addMessage('Connected to server', 'server') + reconnectAttempts = 0; } ws.onmessage = (event) => { try { const data = JSON.parse(event.data) addMessage(`${data.message || data.text || event.data}`, 'server') } catch (e) { addMessage(`${event.data}`, 'server') } } ws.onclose = () => { setConnected(false) addMessage('Disconnected from server', 'server') + // Attempt to reconnect if not at max attempts + if (reconnectAttempts < maxReconnectAttempts) { + addMessage(`Attempting to reconnect (${reconnectAttempts + 1}/${maxReconnectAttempts})...`, 'server') + reconnectTimeout = window.setTimeout(() => { + reconnectAttempts++; + setupWebSocket(); + }, reconnectDelay); + } else { + addMessage('Maximum reconnection attempts reached. Please refresh the page.', 'server') + } } ws.onerror = (error) => { console.error('WebSocket error:', error) addMessage('Error connecting to server', 'server') } wsRef.current = ws + return ws; + }; + + const ws = setupWebSocket(); // Cleanup on unmount return () => { + window.clearTimeout(reconnectTimeout); ws.close() } }, [])
83-91
: Consider adding error handling for JSON.stringifyThe current implementation handles message sending well. We could enhance this by adding a try/catch block around the JSON.stringify operation to handle any potential serialization errors.
// Send a message const sendMessage = () => { if (!inputValue.trim() || !connected) return // Send message as JSON with proper format - wsRef.current?.send(JSON.stringify({ text: inputValue })) - addMessage(inputValue, 'user') - setInputValue('') + try { + wsRef.current?.send(JSON.stringify({ text: inputValue })) + addMessage(inputValue, 'user') + setInputValue('') + } catch (error) { + console.error('Error sending message:', error) + addMessage('Failed to send message', 'server') + } }
155-249
: Consider extracting styles to a separate CSS moduleThe inline styles work well, but for better maintainability and potential reuse, we could extract these styles to a separate CSS module file. This would also reduce the component's file size and improve readability.
apps/kyc-server/src/utils/contentMap.tsx (1)
15-20
: Consider adding TypeScript generic to improve type safetyThe
getContent
function works well, but we could enhance its type safety by using a TypeScript generic to maintain the return type. This would be especially helpful when consuming components are expecting specific return types.-export function getContent( - path: string, - fallback: ReactNode = 'Welcome to Kindfi KYC Server!', -): ReactNode { - return contentMap[path] || fallback -} +export function getContent<T extends ReactNode = ReactNode>( + path: string, + fallback: T = 'Welcome to Kindfi KYC Server!' as unknown as T, +): T | ReactNode { + return contentMap[path] || fallback +}apps/kyc-server/src/config/cors.ts (1)
22-28
: Consider adding more granular control for CORS headersThe current CORS configuration is solid. We could enhance security by implementing more specific controls for credentials and exposed headers.
// HTTP headers allowed allowedHeaders: ['Content-Type', 'Authorization'], +// Allow credentials (cookies, authorization headers) +allowCredentials: true, + +// Headers that browsers are allowed to access +exposedHeaders: ['Content-Length', 'X-Request-Id'], + // Cache duration for preflight requests in seconds (24 hours) maxAge: 86400,apps/web/hooks/passkey/use-passkey-authentication.ts (1)
37-50
: Well-implemented API endpoint updateGood implementation of the dynamic API endpoint using the base URL. Consider abstracting all API paths into constants to further enhance maintainability.
apps/web/hooks/stellar/use-stellar.ts (1)
90-96
: Improve TODO comments with specific criteriaThe TODO comment could be enhanced by adding specific criteria or a ticket reference for when the signing logic should be re-enabled. This would make it clearer when to revisit this code.
- // TODO: disable for now, enable the signing logic when the transaction is ready + // TODO: disable for now, enable the signing logic when transaction features are implemented in PR #XXXapps/kyc-server/src/client.tsx (2)
9-14
: Consider removing console.log statements before productionThe debug logging is helpful during development but should be removed or conditionally executed in production environments. Consider implementing a logging utility that respects environment settings.
- console.log( - 'Client-side JavaScript loaded. Available routes:', - routes.map((r) => r.path).join(', '), - ) - console.log('Current path:', currentPath) + // Only log in development environment + if (process.env.NODE_ENV === 'development') { + console.log( + 'Client-side JavaScript loaded. Available routes:', + routes.map((r) => r.path).join(', '), + ) + console.log('Current path:', currentPath) + }
25-28
: Consider removing debug log for navigation elementLogging React elements to the console can be verbose and doesn't provide significant value in normal operation. We could enhance the debugging experience by using more structured logging or only enabling it conditionally.
apps/kyc-server/src/routes/react.tsx (1)
1-46
: Consider adding type for the route pathsWe could enhance type safety by creating a string literal union type for route paths. This would provide autocomplete for valid routes and catch typos at compile time.
+ // Define a type for valid route paths + type RoutePath = '/about' | '/home' | '/websocket' | '/'; // Add all valid paths + - const routesObject = routes.reduce( + const routesObject = routes.reduce<Record<RoutePath, ReturnType<typeof createRoute>>>( (acc, route) => { acc[route.path] = createRoute( contentMap[route.path] || `Welcome to ${route.label}`, route.path, ) return acc }, - {} as Record<string, ReturnType<typeof createRoute>>, + {} as Record<RoutePath, ReturnType<typeof createRoute>>, )apps/kyc-server/src/libs/passkey/env.ts (2)
5-36
: Consider refactoring repetitive transformation logic.The transformation logic for
RP_ID
,RP_NAME
, andEXPECTED_ORIGIN
follows the same pattern. We could enhance this by extracting this logic into a reusable function to reduce duplication and improve maintainability.import { z } from 'zod' +const parseJsonArrayString = (errorMessage: string) => + z.string().transform((val) => { + try { + return JSON.parse(val) as string[] + } catch { + throw new Error(errorMessage) + } + }) const envSchema = z.object({ REDIS_URL: z.string().default(''), - RP_ID: z - .string() - .transform((val) => { - try { - return JSON.parse(val) as string[] - } catch { - throw new Error('Invalid RP_ID format. Expected a JSON array string.') - } - }) - .default('["localhost"]'), + RP_ID: parseJsonArrayString('Invalid RP_ID format. Expected a JSON array string.') + .default('["localhost"]'), - RP_NAME: z - .string() - .transform((val) => { - try { - return JSON.parse(val) as string[] - } catch { - throw new Error('Invalid RP_NAME format. Expected a JSON array string.') - } - }) - .default('["App"]'), + RP_NAME: parseJsonArrayString('Invalid RP_NAME format. Expected a JSON array string.') + .default('["App"]'), - EXPECTED_ORIGIN: z - .string() - .transform((val) => { - try { - return JSON.parse(val) as string[] - } catch { - throw new Error( - 'Invalid EXPECTED_ORIGIN format. Expected a JSON array string.', - ) - } - }) - .default('["http://localhost:3000"]'), + EXPECTED_ORIGIN: parseJsonArrayString('Invalid EXPECTED_ORIGIN format. Expected a JSON array string.') + .default('["http://localhost:3000"]'), CHALLENGE_TTL_SECONDS: z.coerce.number().default(60), })
9-9
: Consider using Zod's array validation instead of type assertion.The current implementation uses a type assertion (
as string[]
) after parsing JSON. We could enhance this by using Zod's array validation to ensure type safety at runtime.-return JSON.parse(val) as string[] +return z.array(z.string()).parse(JSON.parse(val))apps/kyc-server/src/utils/buildClient.ts (3)
20-22
: The hash generation approach could be strengthened.The current implementation generates a hash based on timestamp and random number, which works well for most cases. We could enhance this by using a more robust hashing function to further reduce the likelihood of collisions in high-volume environments.
// Generate a unique hash based on current timestamp -const hash = - Date.now().toString(36) + Math.random().toString(36).substring(2, 5) +const hash = await crypto.subtle.digest( + 'SHA-256', + new TextEncoder().encode(`${Date.now()}-${Math.random()}`) +).then(hashBuffer => { + return Array.from(new Uint8Array(hashBuffer)) + .map(b => b.toString(16).padStart(2, '0')) + .join('') + .substring(0, 8); +}); const clientFileName = `client-${hash}.js`
38-42
: Consider adding error handling for file operations.The file renaming operation looks good, but we could enhance it by adding proper error handling to gracefully manage potential filesystem issues.
// Rename the output file to include the hash const originalPath = join(publicDir, 'client.js') const newPath = join(publicDir, clientFileName) -await rename(originalPath, newPath) +try { + await rename(originalPath, newPath) +} catch (error) { + console.error(`❌ Failed to rename client file: ${error.message}`) + process.exit(1) +}
44-51
: Consider adding error handling for manifest file creation.The manifest file creation looks good, but we could enhance it by adding error handling to gracefully manage potential write failures.
// Create a manifest file with the current client filename const manifestPath = join(publicDir, 'manifest.json') const manifest = { clientJs: clientFileName, buildTime: new Date().toISOString(), } -await writeFile(manifestPath, JSON.stringify(manifest, null, 2)) +try { + await writeFile(manifestPath, JSON.stringify(manifest, null, 2)) +} catch (error) { + console.error(`❌ Failed to write manifest file: ${error.message}`) + // Continue execution as this is not critical + console.warn('Continuing without manifest file...') +}apps/kyc-server/docs/CORS.md (2)
40-40
: Add information about the CorsOptions type for clarity.The example is clear and helpful. We could enhance this by providing information about where the
CorsOptions
type comes from or how it's defined to help developers implement the configuration correctly.-export const corsConfig: CorsOptions = { +// Import the CorsOptions type from your middleware or define it locally +import { CorsOptions } from '../middleware/cors'; + +export const corsConfig: CorsOptions = {
29-33
: Enhance security considerations for wildcard usage.The wildcard usage note is helpful. We could enhance this by providing more detailed security implications and alternative approaches for development environments.
In development mode, you can use a wildcard to allow all origins:ALLOWED_ORIGINS=*
+ +**Note**: Using wildcards in development reduces security and can mask CORS-related issues that would occur in production. Consider these alternatives: + +1. Use explicit development origins: `["http://localhost:3000","http://localhost:8000"]` +2. Use a local development proxy to avoid CORS entirely +3. If using wildcards, implement additional request validation
apps/kyc-server/src/routes/passkey.ts (1)
16-94
: Consider extracting common route handling logic.The implementation of each route follows a similar pattern. We could enhance this by extracting the common structure into a reusable function to reduce duplication and improve maintainability.
+// Create a handler factory to reduce duplication +const createPasskeyHandler = ( + handlerFn: (requestData: any) => Promise<any> +) => { + return { + async POST(req: Request) { + return withConfiguredCORS(async () => { + try { + const requestData = await req.json() + const result = await handlerFn(requestData) + return Response.json(result) + } catch (error) { + return handleError(error) + } + })(req) + }, + OPTIONS: withConfiguredCORS(() => new Response(null)), + } +} export const passkeyRoutes = { - '/api/passkey/generate-registration-options': { - async POST(req: Request) { - return withConfiguredCORS(async () => { - try { - const { identifier, origin } = await req.json() - const options = await getRegistrationOptions({ - identifier, - origin, - }) - return Response.json(options) - } catch (error) { - return handleError(error) - } - })(req) - }, - OPTIONS: withConfiguredCORS(() => new Response(null)), - }, + '/api/passkey/generate-registration-options': createPasskeyHandler( + async ({ identifier, origin }) => { + return getRegistrationOptions({ + identifier, + origin, + }) + } + ), - '/api/passkey/verify-registration': { - async POST(req: Request) { - return withConfiguredCORS(async () => { - try { - const { identifier, origin, registrationResponse } = await req.json() - const result = await verifyRegistration({ - identifier, - registrationResponse, - origin, - }) - return Response.json(result) - } catch (error) { - return handleError(error) - } - })(req) - }, - OPTIONS: withConfiguredCORS(() => new Response(null)), - }, + '/api/passkey/verify-registration': createPasskeyHandler( + async ({ identifier, origin, registrationResponse }) => { + return verifyRegistration({ + identifier, + registrationResponse, + origin, + }) + } + ), // Similar refactoring for the remaining routes... }apps/kyc-server/docs/client-example.js (3)
10-11
: Consider environment-based configuration for API base URL.
We could enhance maintainability by loadingAPI_BASE_URL
from environment variables or a configuration file, reducing the need for manual updates in different environments.
28-31
: Be mindful of exposing PII in logs or network requests.
It's great that you're sending only the identifier, but if it’s personally identifiable information (like an email), consider obfuscating it or securing logs to safeguard user privacy.
107-110
: Check for WebAuthn API availability.
It’s good practice to confirmnavigator.credentials
is supported by the browser. We could add a condition or fallback for improved cross-browser robustness.apps/kyc-server/src/components/Navigation.tsx (2)
17-22
: Use conditional logging or debug modes.
We could enhance clarity and avoid leaking details by wrapping console logs in a debug check or removing them in production.
28-41
: Leverage dedicated link components for client-side routing.
We could enhance user experience by using specialized link components (e.g.,Link
from Next.js or React Router) for smoother navigation and route handling.apps/kyc-server/src/libs/passkey/errors.ts (1)
18-25
: Unify error naming for consistency.
Currently, the class is namedInAppError
butthis.name
is'PasskeyError'
. We could ensure naming alignment or rename one to clearly reflect its purpose.apps/kyc-server/src/index.tsx (2)
39-40
: Persist connections robustly for production use.
Storing WebSocket clients in memory is fine for demos, but consider a more resilient data store for real-world scenarios (e.g., when scaling across multiple servers).
70-81
: Include additional security headers for static file responses.
We could enhance security by adding headers likeX-Content-Type-Options: nosniff
andContent-Security-Policy
, helping protect against MIME sniffing and other threats.apps/kyc-server/src/middleware/cors.ts (1)
89-108
: Consider adding content type support for CORS headers.The actual request handling properly adds CORS headers to the response, but we could enhance this by adding Access-Control-Allow-Credentials if credentials need to be supported.
// Add CORS headers to the response const headers = new Headers(response.headers) headers.set('Access-Control-Allow-Origin', allowOrigin) headers.set( 'Access-Control-Allow-Methods', corsOptions.allowedMethods?.join(', ') || '', ) headers.set( 'Access-Control-Allow-Headers', corsOptions.allowedHeaders?.join(', ') || '', ) +// Add support for credentials if needed +if (corsOptions.allowCredentials) { + headers.set('Access-Control-Allow-Credentials', 'true') +}apps/kyc-server/src/components/Home.tsx (2)
13-24
: Good error handling in client filename retrieval.The
getClientFilename
function implements proper error handling with a try-catch block and provides a sensible fallback. Consider adding more specific error types for better debugging.function getClientFilename(): string { try { const manifestPath = join(process.cwd(), 'public', 'manifest.json') if (existsSync(manifestPath)) { const manifest = JSON.parse(readFileSync(manifestPath, 'utf-8')) return manifest.clientJs || 'client.js' } - } catch (error) { + } catch (error: unknown) { + const errorMessage = error instanceof Error ? error.message : String(error) + console.error('Error reading manifest:', errorMessage) - console.error('Error reading manifest:', error) } return 'client.js' }
30-96
: Consider extracting styles to a separate file for better maintainability.The component's implementation is clean and well-structured, but the inline styles could be extracted to a separate file or styled-components for better maintainability. The conditional rendering of the message based on its type is a nice touch.
Consider moving the styles to a separate CSS file or using a CSS-in-JS solution for better separation of concerns and maintainability.
apps/kyc-server/docs/SUMMARY.md (1)
48-74
: Helpful file structure visualization.The file structure visualization provides a clear overview of the project organization. Consider adding a language specification to the code block for proper syntax highlighting.
-``` +```bash kindfi/apps/kyc-server/ ├── build.ts # Standalone build script ├── docs/ # Documentation ...🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
apps/kyc-server/src/libs/passkey/passkey.ts (6)
11-16
: Great use of WebAuthn helper functions.
We could enhance maintainability by defining a local wrapper for repeated@simplewebauthn/server
calls, allowing more consistent error handling across these functions.
18-20
: Base64url usage is well-selected.
We could consider logging an error ifENV_PASSKEY
is incomplete, ensuring we catch misconfigured environments early in development.
36-49
: RP info retrieval is well-structured.
Consider supporting partial checks (e.g., subdomains) or trimming trailing slashes inorigin
to reduce user errors and provide more flexibility.
51-101
: Registration options logic looks comprehensive.
We could enhance this by adding logs or metrics to track how often credentials are excluded or how many credentials a user has. This would aid troubleshooting and usage analytics.
103-164
: Registration verification flow is solid.
Consider logging or auditable events for successful registrations to help detect malicious attempts or credential reuse. Otherwise, the approach to storing new credentials is commendable.
166-206
: Authentication options generation is clearly laid out.
We could enhance user experience by returning helpful error messages whencredentials
are empty or missing, guiding them to register first.apps/kyc-server/package.json (3)
10-11
: Scripts look practical.
We could enhance reliability by introducing a 'build' script for consistent bundling or static compilation, though Bun can handle dynamic needs.
14-31
: Modern dependencies for React stack.
Consider adding pinned versions for all key packages in production environments to prevent accidental breakage from future major releases.
34-37
: Dev dependencies for React are appropriate.
If there's a plan for testing, we might incorporate suitable frameworks (e.g., Vitest or Jest) along with relevant type definitions.apps/kyc-server/src/libs/passkey/redis.ts (6)
1-17
: Configurable Redis logic is straightforward.
We could enhance resilience by adding handling for connection failures or retry strategies, ensuring we gracefully degrade if Redis is unavailable.
34-45
: Saving challenges with TTL is a good approach.
We might add debug logs for saved challenges, helping trace any challenge mismatch issues.
47-56
: Challenge retrieval is concise.
Consider logging a warning whenevernull
is returned for an expected challenge, which would help track unusual usage.
69-99
: Credential type definitions look solid.
We could enhance maintainability by using a single source of truth for these types across the codebase to avoid duplication.
76-107
: User retrieval with fallback is well-implemented.
We could enhance logs when returning an empty credential list, especially for new or uninitialized users.
110-131
: User saving logic is robust.
Consider storing minimal user data in Redis to reduce duplication if there's a plan to persist in Postgres soon. This way, we won’t have to sync large data sets across two storage layers.apps/web/app/api/tag/[tagId]/route.ts (3)
33-36
: Consider using consistent parameter naming conventions.The parameter
Response
is capitalized, which is inconsistent with the other method parameters and TypeScript naming conventions. Consider renaming it to lowercaseresponse
to maintain consistency.export async function PUT( request: NextRequest, - Response: NextResponse, + response: NextResponse, context: { params: { tagId: string } }, ) {
69-82
: Consider adding existence check before update operation.The current implementation doesn't verify if the tag exists before attempting to update it. We could enhance this by first checking if the tag exists and returning a 404 if it doesn't, similar to how the GET and DELETE methods handle non-existent tags.
- const { data, error } = await supabase + // First check if the tag exists + const { data: existingTag, error: fetchError } = await supabase + .from('project_tags') + .select('id') + .eq('id', tagId) + .single() + + if (fetchError || !existingTag) { + return NextResponse.json({ error: 'Tag not found' }, { status: 404 }) + } + + const { data, error } = await supabase .from('project_tags') .update({ name: sanitizedName, updated_at: new Date(), }) .eq('id', tagId) .single()
62-67
: Consider using a more descriptive regular expression with better feedback.The current regex validation is good, but we could enhance it by providing more specific feedback about what characters are allowed in tag names.
- if (!/^[a-zA-Z0-9\s-_]+$/.test(sanitizedName)) { + if (!/^[a-zA-Z0-9\s\-_]+$/.test(sanitizedName)) { return NextResponse.json( - { error: 'Tag name contains invalid characters' }, + { error: 'Tag name may only contain letters, numbers, spaces, hyphens, and underscores' }, { status: 400 }, ) }apps/kyc-server/tsconfig.json (1)
3-26
: Modern TypeScript configuration with good organization.The updated configuration is well-structured with clear sections for different aspects of the TypeScript configuration. The move to ESNext for target and module settings, along with the bundler mode configuration, aligns well with modern TypeScript practices.
Consider enabling some of the stricter flags that are currently disabled (like
noUnusedLocals
andnoUnusedParameters
) once the codebase is ready for it, as they can help catch potential issues early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (55)
apps/kyc-server/.env-sample
(0 hunks)apps/kyc-server/.env.example
(1 hunks)apps/kyc-server/.gitignore
(1 hunks)apps/kyc-server/README.md
(1 hunks)apps/kyc-server/build.ts
(1 hunks)apps/kyc-server/docs/BUILD.md
(1 hunks)apps/kyc-server/docs/CORS.md
(1 hunks)apps/kyc-server/docs/PASSKEY.md
(5 hunks)apps/kyc-server/docs/ROUTING.md
(1 hunks)apps/kyc-server/docs/SUMMARY.md
(1 hunks)apps/kyc-server/docs/client-example.js
(1 hunks)apps/kyc-server/package.json
(1 hunks)apps/kyc-server/public/index.html
(0 hunks)apps/kyc-server/public/manifest.json
(1 hunks)apps/kyc-server/public/styles.css
(0 hunks)apps/kyc-server/src/app.ts
(0 hunks)apps/kyc-server/src/client.tsx
(1 hunks)apps/kyc-server/src/components/About.tsx
(1 hunks)apps/kyc-server/src/components/App.tsx
(1 hunks)apps/kyc-server/src/components/Home.tsx
(1 hunks)apps/kyc-server/src/components/Navigation.tsx
(1 hunks)apps/kyc-server/src/components/WebSocketDemo.tsx
(1 hunks)apps/kyc-server/src/config/cors.ts
(1 hunks)apps/kyc-server/src/controllers/index.ts
(0 hunks)apps/kyc-server/src/db/schema.ts
(0 hunks)apps/kyc-server/src/index.tsx
(1 hunks)apps/kyc-server/src/libs/passkey/env.ts
(1 hunks)apps/kyc-server/src/libs/passkey/errors.ts
(1 hunks)apps/kyc-server/src/libs/passkey/passkey.ts
(1 hunks)apps/kyc-server/src/libs/passkey/redis.ts
(1 hunks)apps/kyc-server/src/middleware/cors.ts
(1 hunks)apps/kyc-server/src/resolvers/index.ts
(0 hunks)apps/kyc-server/src/routes/index.ts
(1 hunks)apps/kyc-server/src/routes/passkey.ts
(1 hunks)apps/kyc-server/src/routes/ping.ts
(1 hunks)apps/kyc-server/src/routes/react.tsx
(1 hunks)apps/kyc-server/src/types/apollo-resolvers.ts
(0 hunks)apps/kyc-server/src/types/index.ts
(0 hunks)apps/kyc-server/src/utils/buildClient.ts
(1 hunks)apps/kyc-server/src/utils/contentMap.tsx
(1 hunks)apps/kyc-server/src/utils/error-handler.ts
(1 hunks)apps/kyc-server/tsconfig.json
(1 hunks)apps/web/.env.sample
(1 hunks)apps/web/.gitignore
(1 hunks)apps/web/app/(auth-pages)/sign-in/page.tsx
(1 hunks)apps/web/app/api/passkey/generate-authentication-options/route.ts
(0 hunks)apps/web/app/api/passkey/generate-registration-options/route.ts
(0 hunks)apps/web/app/api/passkey/verify-authentication/route.ts
(0 hunks)apps/web/app/api/passkey/verify-registration/route.ts
(0 hunks)apps/web/app/api/tag/[tagId]/route.ts
(1 hunks)apps/web/hooks/passkey/use-passkey-authentication.ts
(3 hunks)apps/web/hooks/passkey/use-passkey-registration.ts
(3 hunks)apps/web/hooks/passkey/use-passkey.configuration.ts
(1 hunks)apps/web/hooks/stellar/use-stellar.ts
(2 hunks)biome.json
(1 hunks)
💤 Files with no reviewable changes (13)
- apps/kyc-server/.env-sample
- apps/kyc-server/src/controllers/index.ts
- apps/kyc-server/src/resolvers/index.ts
- apps/kyc-server/src/db/schema.ts
- apps/kyc-server/src/types/index.ts
- apps/kyc-server/public/index.html
- apps/web/app/api/passkey/verify-registration/route.ts
- apps/web/app/api/passkey/verify-authentication/route.ts
- apps/web/app/api/passkey/generate-registration-options/route.ts
- apps/web/app/api/passkey/generate-authentication-options/route.ts
- apps/kyc-server/public/styles.css
- apps/kyc-server/src/types/apollo-resolvers.ts
- apps/kyc-server/src/app.ts
✅ Files skipped from review due to trivial changes (6)
- apps/web/.gitignore
- apps/web/app/(auth-pages)/sign-in/page.tsx
- apps/web/hooks/passkey/use-passkey.configuration.ts
- apps/kyc-server/.env.example
- apps/kyc-server/public/manifest.json
- apps/kyc-server/.gitignore
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.ts`: Review TypeScript files ensuring type safety with...
**/*.ts
: Review TypeScript files ensuring type safety with no 'any' usage unless justified. Verify named exports, proper error handling with typed errors, and pure functions. Check module encapsulation and consistent naming using camelCase for functions and PascalCase for types. Validate unit test coverage for utilities if required.
apps/kyc-server/src/routes/ping.ts
apps/kyc-server/build.ts
apps/web/hooks/stellar/use-stellar.ts
apps/kyc-server/src/routes/index.ts
apps/kyc-server/src/config/cors.ts
apps/web/hooks/passkey/use-passkey-registration.ts
apps/kyc-server/src/utils/error-handler.ts
apps/web/hooks/passkey/use-passkey-authentication.ts
apps/kyc-server/src/utils/buildClient.ts
apps/kyc-server/src/middleware/cors.ts
apps/kyc-server/src/routes/passkey.ts
apps/kyc-server/src/libs/passkey/redis.ts
apps/kyc-server/src/libs/passkey/env.ts
apps/web/app/api/tag/[tagId]/route.ts
apps/kyc-server/src/libs/passkey/passkey.ts
apps/kyc-server/src/libs/passkey/errors.ts
`**/*.md`: Review documentation for technical accuracy and c...
**/*.md
: Review documentation for technical accuracy and completeness. Verify code examples are correct and up-to-date. Ensure clear installation instructions and proper API documentation.
apps/kyc-server/docs/ROUTING.md
apps/kyc-server/docs/BUILD.md
apps/kyc-server/docs/SUMMARY.md
apps/kyc-server/docs/CORS.md
apps/kyc-server/docs/PASSKEY.md
apps/kyc-server/README.md
`apps/web/app/api/**/*.ts`: Review API routes ensuring prope...
apps/web/app/api/**/*.ts
: Review API routes ensuring proper input validation and error handling. Verify security measures including rate limiting and authentication. Check response formatting and error states.
apps/web/app/api/tag/[tagId]/route.ts
🪛 LanguageTool
apps/kyc-server/docs/ROUTING.md
[uncategorized] ~12-~12: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...benefits, and smooth navigation without full page reloads. ## Server-Side Routing ### I...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~190-~190: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...nders the appropriate component - No full page reload occurs ## Benefits - **Fast In...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/kyc-server/docs/SUMMARY.md
[uncategorized] ~16-~16: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...d - Links are intercepted to prevent full page reloads - The DOM is updated in-plac...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[duplication] ~87-~87: Possible typo: you repeated a word.
Context: ...- Server renders the React component to HTML - HTML is sent to the client with a link to th...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ation - The DOM is updated without a full page reload 5. Authentication Flow -...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/kyc-server/README.md
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Smooth navigation between pages without full page reloads - React 19: Leverages the l...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~33-~33: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Router handles the navigation without a full page reload ### Passkey Authentication The...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
apps/kyc-server/docs/SUMMARY.md
50-50: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (53)
apps/kyc-server/docs/ROUTING.md (3)
52-71
: Route Handling LogicThis snippet neatly illustrates the dynamic selection of a route based on the request path and method. The inclusion of the
@ts-expect-error
comment is an acceptable trade-off for dynamic access; however, consider documenting its necessity so future maintainers are aware of the design choice.
87-120
: Client-Side Routing ConfigurationThe router configuration using
createBrowserRouter
is well demonstrated, showcasing nested routes and an index route for default content. This snippet serves as a solid example of how to integrate client-side routing in the application.
158-169
: Hydration MechanismThe hydration snippet correctly waits for the DOM to load before invoking
hydrateRoot
, which is crucial for ensuring a smooth transition from SSR to a fully interactive application. This is a clean and effective implementation.apps/web/.env.sample (1)
14-16
: Clear and well-formatted environment variable addition.The new KYC API base URL environment variable is properly documented with a preceding comment and follows the established naming patterns in the file. The default value of
http://localhost:3001
provides a good starting point for local development.apps/kyc-server/build.ts (1)
1-12
: Well-structured build script with clear documentation.The build script is concise and follows good practices with appropriate JSDoc comments explaining its purpose and usage. The async/await pattern is correctly implemented, making the code flow clear and maintainable.
biome.json (1)
10-14
: Appropriate addition to ignore list.The addition of the KYC server's public directory to the ignore list makes sense, as this directory likely contains generated files that shouldn't be processed by Biome. This is consistent with the existing pattern of ignoring generated content.
apps/kyc-server/src/components/WebSocketDemo.tsx (2)
4-9
: Solid type definition for the Message interfaceThe Message interface is well-structured with clear type definitions for each property. This provides good type safety throughout the component.
61-69
: Good use of useLayoutEffect for smooth scrolling behaviorThe implementation of auto-scrolling with useLayoutEffect is well done. The comment explaining why useLayoutEffect is used instead of useEffect shows good attention to UI behavior details.
apps/web/hooks/passkey/use-passkey-registration.ts (2)
8-8
: Good refactoring to use a centralized API base URLUsing the imported
KYC_API_BASE_URL
constant for constructing API endpoints improves maintainability and follows the DRY principle. This change makes it easier to update API endpoints across the application.
37-37
: Consistent API endpoint construction with configuration variableThe implementation consistently uses the
KYC_API_BASE_URL
for constructing API endpoints. This approach ensures that all API calls reference the same base URL, which simplifies environment-specific configurations.Also applies to: 60-61
apps/kyc-server/src/utils/contentMap.tsx (1)
7-12
: Well-structured content map with appropriate component referencesThe content map is well-organized and provides a clear mapping between routes and their corresponding React components or string content. This centralized approach makes route content management more maintainable.
apps/kyc-server/src/config/cors.ts (1)
12-20
: Good environment-specific CORS configurationThe CORS configuration correctly differentiates between production and non-production environments. Using environment variables for allowed origins in production is a good practice for security and flexibility.
apps/web/hooks/passkey/use-passkey-authentication.ts (2)
6-6
: Nice enhancement with KYC_API_BASE_URL import!Adding the KYC_API_BASE_URL from a configuration file improves flexibility and maintainability, allowing the API endpoint to be configured once and reused across the application.
63-76
: Good consistency with dynamic endpoint patternThe verification endpoint follows the same pattern as the authentication options endpoint, maintaining consistency in the codebase.
apps/web/hooks/stellar/use-stellar.ts (1)
78-85
:❓ Verification inconclusive
Review commented-out error handling in prepareSign
The commented-out error checks for bundlerKey.current and deployee could lead to runtime issues if these values are null or undefined. Consider implementing safer checks or providing fallback behavior.
🏁 Script executed:
#!/bin/bash # Check if prepareSign is called from any components and whether the caller checks for errors rg -A 3 -B 3 "prepareSign\(\)" --type tsLength of output: 42
Double-check error handling in
prepareSign
- The grep search indicates that no components are currently calling
prepareSign()
. If this is intentional for staging purposes, please document its future integration.- The commented-out error checks for
bundlerKey.current
anddeployee
might lead to runtime issues if these values are expected to be valid once integrated. Consider implementing robust checks or providing fallback behavior to ensure safe operation when the transaction logic is enabled.apps/kyc-server/src/client.tsx (1)
32-36
: Well-structured conditional renderingThe conditional rendering of content is clean and effective. This approach properly handles both string and React node content types.
apps/kyc-server/src/routes/react.tsx (3)
12-16
: Great abstraction of CORS handlingThe creation of a configured CORS handler is an excellent approach that ensures consistent CORS handling across routes. This promotes maintainability and reduces the risk of security configuration errors.
18-31
: Well-designed route creator functionThe
createRoute
helper function is a strong abstraction that ensures consistency in route handling. This approach makes adding new routes simpler and less error-prone.
34-43
: Elegant route generation from route definitionsUsing reduce to transform the routes array into an object with route handlers is clean and maintainable. This approach provides a clear connection between route definitions and their implementations.
apps/kyc-server/src/routes/passkey.ts (2)
11-14
: The CORS handler creation is well-implemented.The
withConfiguredCORS
function is a great approach that promotes code reuse and consistency across routes. This higher-order function pattern keeps the routes clean and maintainable.
32-32
: Great handling of OPTIONS requests.The implementation correctly handles OPTIONS requests for CORS preflight checks, which is essential for cross-origin API access.
apps/kyc-server/src/middleware/cors.ts (6)
10-15
: Well-structured CORS type definition with flexible options.The
CorsOptions
type is clearly defined with appropriate documentation and flexible configuration options. The use of string arrays or wildcard for allowed origins provides good flexibility.
18-23
: Good default configuration for CORS.The default CORS configuration is well thought out with sensible defaults that support common use cases. Consider adding more context in comments about security implications of using wildcard origins in production environments.
28-47
: Excellent implementation of origin validation with wildcard subdomain support.The origin validation logic is thorough and handles multiple cases correctly, including exact matches and wildcard subdomain matching. The implementation is easy to follow with clear comments.
52-60
: Type-safe middleware implementation with good option handling.The CORS middleware function is well-implemented with proper TypeScript types and graceful option merging. This ensures configuration flexibility while maintaining type safety.
62-73
: Robust request origin handling with appropriate security measures.I appreciate the security-first approach by returning a 403 Forbidden status when origins are not allowed. The conditional logic for determining allowed origins is clear and comprehensive.
76-87
: Clear preflight request handling.The OPTIONS request handling is well-implemented, properly setting all required CORS headers for preflight requests.
apps/kyc-server/src/components/Home.tsx (2)
7-10
: Clear and well-typed component props interface.The
HomeProps
interface is well-defined with appropriate types and optional properties. The use ofReactNode
type for the message prop provides flexibility for rendering different content types.
26-29
: Clean component implementation with proper props destructuring.The
Home
component follows React best practices with appropriate props destructuring and initialization of default values.apps/kyc-server/README.md (9)
1-4
: Clear project introduction and description.The introduction clearly communicates the nature of the project as a server-side rendered React application with Passkey Authentication support. This sets proper expectations for developers.
5-15
: Comprehensive feature list with helpful categorization.The features section effectively breaks down the key capabilities of the application. Consider adding brief explanations of how some features benefit the end user or developer experience.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Smooth navigation between pages without full page reloads - React 19: Leverages the l...(EN_COMPOUND_ADJECTIVE_INTERNAL)
16-44
: Well-detailed architecture section with clear explanations.The architecture section provides excellent insights into how the application works, including SSR, client-side routing, and passkey authentication. The explanations are thorough and help developers understand the design decisions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Router handles the navigation without a full page reload ### Passkey Authentication The...(EN_COMPOUND_ADJECTIVE_INTERNAL)
45-63
: Helpful file structure overview.The file structure section gives developers a clear understanding of the project organization. Consider adding brief descriptions of what each component is responsible for to further enhance understanding.
64-88
: Clear routing explanation with server-side and client-side distinctions.The routing explanation clearly distinguishes between server-side and client-side routing, helping developers understand the hybrid approach. This section effectively bridges the gap between the architecture explanation and implementation details.
89-116
: Comprehensive development setup instructions.The development section provides clear and concise instructions for setting up and running the project. The prerequisites, setup, and development workflow sections are well-organized and easy to follow.
117-125
: Clear production deployment instructions.The production section provides concise instructions for building and deploying the application in a production environment. Consider adding more information about production-specific configurations or optimizations.
126-147
: Well-documented environment variables.The environment variables section clearly documents the available configuration options. Consider grouping related variables or adding more context about how they affect the application behavior.
149-159
: Comprehensive documentation section with links to detailed resources.The documentation section provides links to more detailed resources, which is very helpful for developers who need more specific information. This layered approach to documentation works well for different levels of familiarity with the project.
apps/kyc-server/docs/SUMMARY.md (6)
1-4
: Clear document purpose and scope.The introduction effectively sets expectations for what the document covers, providing a concise summary of the React SSR implementation with client-side routing.
5-47
: Comprehensive implementation overview with clear categorization.The implementation section provides a well-structured overview of the key features, making it easy for developers to understand what has been implemented. Each section is concise yet informative.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...d - Links are intercepted to prevent full page reloads - The DOM is updated in-plac...(EN_COMPOUND_ADJECTIVE_INTERNAL)
75-107
: Clear operational flow explanation.The "How It Works" section provides an excellent step-by-step explanation of the application flow, from server start to authentication. This helps developers understand the sequence of operations and how different components interact.
🧰 Tools
🪛 LanguageTool
[duplication] ~87-~87: Possible typo: you repeated a word.
Context: ...- Server renders the React component to HTML - HTML is sent to the client with a link to th...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ation - The DOM is updated without a full page reload 5. Authentication Flow -...(EN_COMPOUND_ADJECTIVE_INTERNAL)
108-114
: Effective benefits summary.The benefits section clearly articulates the advantages of the chosen architecture and implementation. This helps stakeholders understand the value of the technical decisions made.
115-122
: Well-organized documentation links.The documentation section provides clear links to more detailed resources, making it easy for developers to find specific information. The organization mirrors that in the README, creating a consistent documentation structure.
123-133
: Forward-looking next steps section.The next steps section provides a clear roadmap for future development, helping team members understand the planned enhancements. Consider prioritizing these items or adding estimated timelines for implementation.
apps/kyc-server/src/libs/passkey/passkey.ts (1)
1-9
: Imports appear well-organized.
Consider grouping type-only imports separately for clarity. Otherwise, this is an effective usage of external modules.apps/kyc-server/package.json (1)
4-8
: Metadata updates align with new stack goals.
This rebranding clarifies that Bun, React, and TailwindCSS power the app. Nicely done.apps/kyc-server/src/libs/passkey/redis.ts (3)
21-27
: Utility for byte-array conversion is nicely done.
This clarifies code and avoids repeating buffer logic.
29-32
: Key generation is simple and readable.
Make sure the final key structure remains consistent across all app layers. This approach is already quite straightforward.
58-67
: Challenge deletion is straightforward.
Well done. This ensures old challenges won't stay in Redis unnecessarily.apps/kyc-server/docs/BUILD.md (1)
1-141
: Well-structured build documentation with comprehensive details.This is a thorough and well-organized documentation of the build process. The structure flows logically from overview to specific implementation details, making it easy for developers to understand both the high-level process and specific implementation steps.
I appreciate the clear explanations of the build configuration options and the distinction between development and production environments.
apps/web/app/api/tag/[tagId]/route.ts (1)
102-116
: Good error handling for DELETE operation.I appreciate the thorough error handling in this DELETE endpoint. The code correctly checks for database errors and verifies that a record was actually deleted before returning a success response, preventing false positives.
apps/kyc-server/docs/PASSKEY.md (2)
9-15
: Clear and complete API documentation update for the origin parameter.The addition of the origin parameter to the API request bodies is well-documented with proper JSON formatting. This change aligns with the WebAuthn specification requirements.
173-201
: Excellent explanation of the origin parameter requirements.The newly added "Origin Parameter" section provides comprehensive documentation on why the origin parameter is required, its security implications, and how to use it. The example code showing how to dynamically include the current origin is particularly helpful for developers implementing clients for this API.
already |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Implemented Project Tags and the corresponding database schema in Supabase, along with the necessary API routes in Next.js.
#API Endpoints 🌐
Located in apps/web/app/api/tag/:
GET /api/tag → Fetch all tags.
POST /api/tag → Create a new tag.
GET /api/tag/:tagId → Retrieve a tag.
PUT /api/tag/:tagId → Update a tag.
DELETE /api/tag/:tagId → Remove a tag.
Evidence 🧪
Summary by CodeRabbit
New Features
Documentation