feat(frontend): css optimization in frontend #156
Conversation
|
Warning Rate limit exceeded@autofix-ci[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 33 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 (4)
WalkthroughThis pull request simplifies and refines several components across the frontend. In the homepage, redundant conditional logic in the project creation flow is removed. The sidebar and its items receive layout and styling adjustments, including changes to spacing and positioning. The GraphQL schema is extended with new types, mutations, and queries related to email confirmation and project limits. Additionally, the AuthProvider now features further inline comments to clarify token management and authentication processes. Changes
Sequence Diagram(s)Project Creation Flow in HomePagesequenceDiagram
participant User
participant HomePage
participant ProjectService
participant PromptForm
User->>HomePage: Submits project creation form
HomePage->>ProjectService: createProjectFromPrompt()
ProjectService-->>HomePage: Returns result
HomePage->>PromptForm: clearMessage()
Sidebar New Project Button FlowsequenceDiagram
participant User
participant Sidebar
participant Router
participant Modal
User->>Sidebar: Clicks New Project Button
alt Sidebar Collapsed
Sidebar->>Router: Navigate to HomePage
else Sidebar Expanded
Sidebar->>Modal: Open New Project Modal
end
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 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 (
|
![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 (13)
frontend/src/components/root/prompt-form.tsx (1)
214-214: Good UI improvement replacing fixed width with min-w-maxReplacing the fixed width
w-[117px]withmin-w-maxallows the SelectTrigger component to dynamically resize based on its content. This prevents text truncation when longer model names are displayed and provides a better user experience.For even better responsiveness, consider adding a max-width constraint with overflow handling to prevent the component from becoming too wide on smaller screens:
-'h-6 border-0 focus:ring-0 hover:bg-gray-100 dark:hover:bg-gray-600 pl-1 min-w-max', +'h-6 border-0 focus:ring-0 hover:bg-gray-100 dark:hover:bg-gray-600 pl-1 min-w-max max-w-[200px] text-ellipsis overflow-hidden',frontend/src/components/chat/chat-layout.tsx (2)
18-20: Authentication check added for project data fetchingGood implementation of conditional data fetching for user projects. This ensures that only authenticated users can fetch project data, which is important for data privacy and security.
Minor formatting suggestion - add a space after the comma for better readability:
-const { refetch } = useQuery(GET_USER_PROJECTS,{ +const { refetch } = useQuery(GET_USER_PROJECTS, { skip: !isAuthorized, });
23-31:❓ Verification inconclusive
Verify if redirect can be moved to middleware
The component implements a redirect for unauthenticated users, which is good for security. However, consider if this check could be implemented at the middleware level for better performance and to avoid momentary flashes of unauthorized content.
If no middleware is handling authentication redirects, consider creating one to move this logic out of the component.
🏁 Script executed:
#!/bin/bash # Check if authentication middleware exists echo "Looking for middleware files that might handle authentication..." fd "middleware" --type f --extension ts --extension tsx --extension js --extension jsx echo "\nLooking for auth-related redirects in existing middleware..." rg "router.push\('\/'\)" --type=ts --type=tsxLength of output: 514
Action: Re-check middleware for authentication redirect
The suggestion to move the redirect into middleware is worth considering. However, our initial search for middleware files and auth-related redirects using the command resulted in a regex error. Please re-run the following updated shell script to verify if any middleware handling authentication already exists:
#!/bin/bash # Search for middleware files related to authentication echo "Looking for middleware files with 'middleware' in the name..." fd -i "middleware" --type f --extension ts --extension tsx --extension js --extension jsx echo "" echo "Searching for auth-related redirects using router.push('/') in ts/tsx files..." rg "router\.push\('/'\)" --type ts --type tsxBased on the results of this script, if no middleware is handling the authentication redirect, consider moving the redirect logic from the component (as shown below) into a middleware for improved performance and to avoid flashes of unauthorized content:
useEffect(() => { if (!isAuthorized) { - router.push('/'); + // Redirect to home; consider moving this logic to a middleware for better performance. + router.push('/'); } }, [isAuthorized, router]); if (!isAuthorized) { return null; }Please run the above script and verify its output. Adjust the implementation accordingly if you decide to implement a new middleware.
frontend/src/components/sign-in-modal.tsx (1)
46-48: Improved toast notification placement.Setting the toast position to 'bottom-right' creates a more consistent user experience by:
- Keeping notifications out of the main content area
- Placing them in a standard location users expect to see notifications
- Ensuring they don't interfere with the sign-in form
For consistency, consider applying this same positioning to all toast notifications throughout the application.
You may want to create a wrapper function or utility that applies consistent positioning to all toast notifications:
-toast.success('Login successful!', { - position: 'bottom-right', -}); +import { showToast } from '@/utils/toast'; +showToast.success('Login successful!');Then in a new utils/toast.ts file:
import { toast, ToastOptions } from 'sonner'; const defaultOptions: ToastOptions = { position: 'bottom-right', }; export const showToast = { success: (message: string, options?: ToastOptions) => toast.success(message, { ...defaultOptions, ...options }), error: (message: string, options?: ToastOptions) => toast.error(message, { ...defaultOptions, ...options }), // Add other toast types as needed };frontend/src/components/chat/code-engine/project-context.tsx (1)
52-52: Fix the indentation for consistencyThe line is not properly indented compared to surrounding code, which breaks the consistent formatting pattern.
-const { isAuthorized } = useAuthContext(); + const { isAuthorized } = useAuthContext();frontend/src/app/(main)/page.tsx (1)
17-19: Proper state setup for modal managementThe state variables are appropriately defined to manage the visibility of authentication modals.
However, I notice that while
routeris imported on line 13 and initialized on line 17, it's not used anywhere in this component. Consider removing unused imports.-import { useRouter } from 'next/navigation';- const router = useRouter();frontend/src/providers/AuthProvider.tsx (7)
49-49: Consider using English for code commentsThe comment uses Chinese characters. For international teams and consistency with the rest of the codebase (which appears to be in English), consider using English for all comments.
- // 验证本地 token 是否有效 + // Validate if the local token is valid
72-72: Maintain language consistency in commentsSame issue as above - the comment is in Chinese while the codebase appears to use English.
- // 获取当前用户信息 + // Fetch current user information
87-87: Use consistent language for commentsMaintain English language consistency throughout the codebase.
- // 刷新 token + // Refresh token
120-120: Use English for code commentsContinue using English for comments for better international collaboration.
- // 登录时写入 token 并获取用户信息 + // Store tokens and fetch user info on login
127-127: Remove or enhance debugging console logThis console log statement appears to be for debugging purposes. Either remove it for production or enhance it to be a conditional debug log.
- console.log('Saved token:', accessToken); + // Only log in development environment + if (process.env.NODE_ENV === 'development') { + console.log('Saved token:', accessToken); + }
134-134: Maintain language consistency in commentsUse English for all comments.
- // 登出 + // Logout
143-167: Good implementation of authentication initialization flowThe authentication flow is well-structured with clear comments explaining each step of the process:
- Check for stored token
- Validate the token if present
- Attempt to refresh if validation fails
- Set authentication state based on the result
The comments help understand the flow, though they should be in English for consistency.
Translate the comments to English for better international collaboration:
- // 如果本地根本没有 accessToken，就直接判定未登录 + // If there's no accessToken stored locally, immediately set as not logged in - // 有本地 token，先验证 + // If local token exists, validate it first - // 如果验证失败，再试图刷新 + // If validation fails, attempt to refresh the token - // 最终判断 + // Final determination
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/public/map.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
frontend/src/app/(main)/page.tsx(3 hunks)frontend/src/components/auth-choice-modal.tsx(3 hunks)frontend/src/components/chat/chat-layout.tsx(1 hunks)frontend/src/components/chat/code-engine/project-context.tsx(3 hunks)frontend/src/components/chat/index.tsx(2 hunks)frontend/src/components/root/nav.tsx(1 hunks)frontend/src/components/root/prompt-form.tsx(1 hunks)frontend/src/components/sidebar-item.tsx(2 hunks)frontend/src/components/sidebar.tsx(4 hunks)frontend/src/components/sign-in-modal.tsx(1 hunks)frontend/src/graphql/schema.gql(2 hunks)frontend/src/hooks/useChatList.ts(1 hunks)frontend/src/hooks/useModels.ts(2 hunks)frontend/src/providers/AuthProvider.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/graphql/schema.gql
🔇 Additional comments (26)
frontend/src/hooks/useChatList.ts (1)
5-5:❓ Verification inconclusive
Security improvement by conditional query execution
Good implementation of conditional data fetching based on authentication status. This ensures that GraphQL queries are only executed when the user is authorized, preventing unnecessary network requests and potential data leakage.
Let's verify if this pattern is consistently applied across other query hooks in the application:
Also applies to: 9-9, 17-17
🏁 Script executed:
#!/bin/bash # Check for other useQuery calls in the codebase to ensure they also have proper skip conditions for authorization echo "Searching for all GraphQL useQuery calls in the codebase..." rg "useQuery\(" --type=ts --type=tsx echo "\nChecking specifically for skip conditions related to authorization..." rg "skip:.*!isAuthorized" --type=ts --type=tsxLength of output: 425
I'll re-run the verification command with an updated file type configuration to include TSX files. This should ensure we capture all relevant GraphQL query calls and their associated skip conditions. Please run the following script:
#!/bin/bash echo "Re-checking for all GraphQL useQuery calls in the codebase including TSX files..." rg "useQuery\(" --type-add "tsx:*.tsx" --type=ts --type=tsx echo "\nRe-checking specifically for skip conditions related to authorization..." rg "skip:.*!isAuthorized" --type-add "tsx:*.tsx" --type=ts --type=tsxOnce you get the output, verify that:
- All
useQuerycalls across both TypeScript and TSX files are being reported.- The expected skip conditions (
skip: ... !isAuthorized) are present where needed.After you verify the results using this updated script, please update the review comment if necessary.
frontend/src/components/chat/index.tsx (1)
20-20: Good security implementation for chat history fetchingProperly implementing authentication checks before fetching chat history data. The skip condition with
!isAuthorized || !chatIdensures queries are only executed when both conditions are met: the user is authenticated and there's a valid chat ID.This approach protects user data and prevents unnecessary network requests, enhancing both security and performance.
Also applies to: 24-24, 41-41
frontend/src/hooks/useModels.ts (2)
14-14: Added authorization check for model selection.The addition of
isAuthorizedfrom the auth context is used to conditionally fetch models based on user authentication state. This is a good security practice to ensure model data is only fetched for authorized users.
53-53: Appropriate query control based on authorization status.Adding
!isAuthorizedto the skip condition ensures the query doesn't run for unauthorized users, which:
- Prevents unnecessary network requests
- Adds a security layer by not exposing model data to unauthenticated users
- Improves performance by avoiding wasted queries
This change aligns with how other components in the application handle authorization-dependent queries.
frontend/src/components/sidebar-item.tsx (3)
107-107: Improved sidebar item container layout.The addition of
relativepositioning andpx-2padding creates a better layout for the sidebar item, giving the content proper spacing while establishing a positioning context for its children.
111-111: Enhanced content truncation with proper spacing.The changes to this div's classes improve text truncation behavior by:
- Adding
min-w-0to ensure the flex item can shrink below its content size- Using
mr-12to provide space for the absolutely positioned button- Removing
gap-3to tighten up the layoutThis ensures that long text properly truncates rather than overflowing.
122-122: Fixed button positioning for better UI consistency.The button's class now uses absolute positioning with specific coordinates to ensure it stays properly aligned regardless of the content length. The transform for vertical centering and specific dimensions create a consistent interaction target.
frontend/src/components/root/nav.tsx (2)
177-177: Changed "Codefox Journey" tab behavior to show "Coming Soon" alert.This change is appropriate for a feature that's still in development, clearly indicating to users that the functionality isn't available yet rather than directing them to the GitHub repository.
185-185:❓ Verification inconclusive
Removed fixed positioning from the navbar container.
Removing the
fixedclass from the navbar container allows it to flow with the document layout rather than being fixed to the viewport. This may affect how the navbar behaves during scrolling.Please verify that this change is intentional and doesn't negatively impact the user experience when scrolling through pages with varying content heights. Consider the following checks:
🏁 Script executed:
#!/bin/bash # Check if there are any CSS classes in other files that might be positioning this navbar rg -A 3 -B 3 "FloatingNavbar" --type tsx --type jsx # Look for any scroll event handlers that might interact with the navbar rg "scroll" --type tsx --type jsx | grep -B 5 -A 5 "navbar|nav|header"Length of output: 178
Below is a revised verification script to better inspect the potential impacts on the navbar behavior. Please run these commands and then verify manually that:
- No other CSS classes (such as ones referencing “FloatingNavbar”) are inadvertently affecting the navbar’s positioning.
- There are no scroll event handlers (e.g., via
onScroll) tying the navbar behavior to scrolling.Please verify the output of these commands manually.
Action Required: Verify Navbar Positioning and Scroll Behavior
- The
fixedclass removal is intended, shifting the navbar to flow with the document layout.- Ensure this change does not cause unintended layout issues on pages with varying content heights.
- Confirm that no other CSS classes or scroll event handlers interfere with the navbar’s expected behavior.
🏁 Script executed:
#!/bin/bash # Search for references to "FloatingNavbar" in TSX and JSX files manually find . -type f \( -iname "*.tsx" -o -iname "*.jsx" \) -exec grep -H "FloatingNavbar" {} \; # Search for scroll event handlers (using "onScroll") in TSX and JSX files, # showing 5 lines of context to check for any navbar or header references. find . -type f \( -iname "*.tsx" -o -iname "*.jsx" \) -exec grep -H -C 5 "onScroll" {} \;Length of output: 900
Action Required: Confirm Navbar Responsiveness
After running the updated verification scripts, we noted that:
- References to
FloatingNavbarappear as expected in both./frontend/src/components/root/nav.tsxand./frontend/src/components/root/nav-layout.tsx.- No scroll event handlers (like
onScroll) tied to navbar or header behaviors were detected.These findings indicate that the removal of the
fixedclass—making the navbar flow with the document layout—was an intentional change. However, since the navbar behavior during scroll can be subtle, please manually verify in the deployed environment that the navbar behaves as expected on pages with varying content heights.frontend/src/components/auth-choice-modal.tsx (2)
38-49: Solid implementation of modal transitionThe use of
setTimeoutto provide a smooth transition between modals is a good approach. It ensures the current modal has time to close before opening the sign-in modal, preventing visual jarring.
53-62: Consistent pattern for modal transitionsThe implementation mirrors the pattern used for the sign-in button, maintaining consistency in the user experience for both authentication options.
frontend/src/components/chat/code-engine/project-context.tsx (2)
21-21: Proper authentication integrationAdding the auth context import enables conditional data fetching based on user authentication status.
89-89: Auth-conditional query skipping is implemented correctlyUsing the
skipproperty to prevent queries when users aren't authenticated is a good practice. This prevents unnecessary network requests and potential errors from unauthorized API calls.frontend/src/app/(main)/page.tsx (3)
11-13: Good import organization for authentication componentsThe necessary authentication-related components and hooks are properly imported.
97-105: Consistent modal transition implementationThe implementation matches the pattern in the AuthChoiceModal component, ensuring consistent behavior between the components.
109-111: Complete authentication flow integrationThe SignInModal and SignUpModal components are properly integrated with appropriate props for opening and closing.
frontend/src/components/sidebar.tsx (10)
11-11: Import of routing capabilities added correctly.The addition of
useRouterfrom Next.js navigation library is appropriate for implementing the redirection functionality needed for the "New Project" feature in the sidebar.
50-50: Router initialized properly.The router is correctly initialized using the
useRouterhook from Next.js, following best practices for client-side navigation in Next.js applications.
67-67: Simplified logging for better debugging.The console logging has been simplified to display only the essential state information, which is a good practice for maintaining clean development logs.
72-73: Background styling improvements.The background styling updates properly ensure consistency between light and dark modes using utility classes, improving the overall visual appearance of the sidebar.
76-112: Improved header layout with responsive design.The header section is now properly structured with a flex container that adjusts based on the sidebar's collapsed state. The positioning of the logo and sidebar trigger is well-implemented for both states.
116-116: Clear visual separation with divider.The divider provides a clean visual separation between the header and content sections of the sidebar.
118-160: Enhanced "New Project" button with context-aware behavior.The button now intelligently handles different states:
- When the sidebar is collapsed: Redirects to the home page using router navigation
- When the sidebar is expanded: Opens the modal for creating a new project
This is a good UX improvement that maximizes the utility of the sidebar in both states.
166-167: Optimization for collapsed state.Good performance optimization by conditionally rendering the chat list only when the sidebar is expanded, avoiding unnecessary DOM elements when they wouldn't be visible anyway.
193-195: Responsive footer implementation.The footer styling is now properly adjusted based on the sidebar's collapsed state, ensuring consistent appearance in both states.
147-152: SVG transformation for correct icon orientation.The SVG has been properly transformed to ensure the icon is displayed in the correct orientation, which is a good attention to detail for visual consistency.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **Refactor** - Improved chat session navigation for smoother transitions and a more integrated user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (29)
frontend/src/components/chat/code-engine/web-view.tsx (3)
137-147: Use iframeRef instead of document.getElementById.The
reloadIframefunction retrieves the iframe element usingdocument.getElementByIdinstead of using the existingiframeRef. This is inconsistent with the rest of the component and could lead to issues if the ID is changed.const reloadIframe = () => { - const iframe = document.getElementById('myIframe') as HTMLIFrameElement; - if (iframe) { + if (iframeRef.current) { + const iframe = iframeRef.current; const src = iframe.src; iframe.src = 'about:blank'; setTimeout(() => { iframe.src = src; setScale(0.7); }, 50); } };
149-151: Translate comments to English for consistency.The comment in Chinese should be translated to English to maintain code consistency.
const zoomIn = () => { - setScale((prevScale) => Math.min(prevScale + 0.1, 2)); // 最大缩放比例为 2 + setScale((prevScale) => Math.min(prevScale + 0.1, 2)); // Maximum zoom scale is 2 };
153-155: Translate comments to English for consistency.The comment in Chinese should be translated to English to maintain code consistency.
const zoomOut = () => { - setScale((prevScale) => Math.max(prevScale - 0.1, 0.5)); // 最小缩放比例为 0.5 + setScale((prevScale) => Math.max(prevScale - 0.1, 0.5)); // Minimum zoom scale is 0.5 };backend/src/app.module.ts (1)
20-23: Consider extracting environment check function.The
isProductionfunction defined inline with a TODO comment could be moved to a utility file as suggested by the comment.You could create a new file like
src/utils/environment.ts:export function isProduction(): boolean { return process.env.NODE_ENV === 'production'; } export function isDevelopment(): boolean { return process.env.NODE_ENV === 'development'; } export function isTest(): boolean { return process.env.NODE_ENV === 'test'; }Then import and use it in this file:
- // TODO(Sma1lboy): move to a separate file - function isProduction(): boolean { - return process.env.NODE_ENV === 'production'; - } + import { isProduction } from './utils/environment';frontend/src/app/api/file/route.ts (1)
62-63: Good addition of TypeScript React file type support.Adding the TypeScript React file extension mapping improves the application's ability to properly handle
.tsxfiles. The TODO comment also serves as a useful reminder for future enhancements.Consider adding other common React and modern web development file types that might be missing, such as:
//TODO: Add more file types tsx: 'typescriptreact', +jsx: 'javascriptreact', +vue: 'vue', +svelte: 'svelte', txt: 'text',backend/src/user/dto/resend-email.input.ts (1)
1-1: Fix incorrect file path in comment.The comment indicates this file is at
src/auth/dto/resend-email.input.ts, but the actual path isbackend/src/user/dto/resend-email.input.ts. Update the comment to match the actual file location.-// src/auth/dto/resend-email.input.ts +// src/user/dto/resend-email.input.tsbackend/src/mail/mail.module.ts (1)
15-41: Consider adding logging for mail connection issues.While the configuration is correct, consider adding error handling or logging for mail connection failures to help with debugging in production.
useFactory: async (config: ConfigService) => ({ + // Add debug option to help troubleshoot mail connection issues + transport: { host: config.get('MAIL_HOST'), port: config.get<number>('MAIL_PORT'), secure: false, auth: { user: config.get('MAIL_USER'), pass: config.get('MAIL_PASSWORD'), }, + debug: config.get('NODE_ENV') !== 'production', + logger: config.get('NODE_ENV') !== 'production', },backend/src/config/env.validation.ts (1)
1-51: Well-structured environment variables validation implementation.This is a well-organized validation class for environment variables using class-validator decorators. The class correctly defines required and optional variables with appropriate validation rules.
Consider adding:
- Documentation comments for each environment variable explaining its purpose, acceptable values, and any specific formatting requirements
- Default values where appropriate
- Transformation decorators (e.g., @Transform) for variables that need type conversion from string to number
Example enhancement:
import { IsOptional, IsString, IsNumber, IsIn } from 'class-validator'; +import { Transform } from 'class-transformer'; export class EnvironmentVariables { + /** + * Port number for the server to listen on + * @example 3000 + */ @IsNumber() + @Transform(({ value }) => parseInt(value, 10)) PORT: number; + /** + * Environment mode for the application + * @example 'DEV' | 'PROD' | 'TEST' + */ @IsString() @IsIn(['DEV', 'PROD', 'TEST']) NODE_ENV: string;frontend/src/lib/client.ts (1)
93-93: TypeScript casting is necessary but could be handled better.The cast to
unknownfirst and then toApolloLinkis a workaround for TypeScript type compatibility issues.Consider using a more explicit type assertion or adding a type declaration file for the apollo-upload-client to improve type safety:
- uploadLink as unknown as ApolloLink, // Cast to ApolloLink to satisfy TypeScript + uploadLink as ApolloLink, // Cast to ApolloLink with proper type definitionsfrontend/src/app/api/runProject/route.ts (4)
131-133: Add context about build operations.While these logging statements are helpful, consider adding context about the environment (e.g., whether triggered locally or in CI) or which project this build corresponds to, for quicker debugging in multi-project scenarios.
153-153: Be mindful of logging sensitive information.Printing the entire run command can be useful for debugging but could potentially log sensitive details or environment-specific data. Ensure no secrets or tokens appear here.
158-198: Consider concurrency checks for container removal and re-run.The logic correctly removes an existing container upon conflict and retries the run, but if two processes trigger the same run concurrently, a race condition may occur (e.g., both detect conflict, remove, then both re-run). Consider adding locking mechanisms or verifying that only one container is needed per project path.
251-282: Validate after removing stale containers.When removing a non-running container, you rely on an asynchronous command and keep no immediate check to ensure removal success before proceeding. If a subsequent request arrives before the removal completes, concurrency issues or stale data might persist. Consider awaiting removal or exploring a more robust approach (e.g., building a queue or job system).
frontend/src/app/auth/confirm/page.tsx (1)
16-23: Use descriptive status states or an enum for clarity.You're using string union types for
'loading' | 'success' | 'error'. For large projects, consider an enum or constants to improve maintainability and prevent typos.backend/src/upload/upload.service.ts (3)
42-63: Validate fallback endpoint usage.The step-by-step logic for determining the endpoint is good. If the fallback endpoint is unlikely to be used, consider logging a warning or throwing an error to catch misconfiguration sooner.
64-160: Centralize logic for S3 vs. local to simplify future changes.These conditionals for uploading to either S3 or local could be abstracted in smaller helper functions (e.g.,
uploadToS3,uploadToLocal). That way, future changes for one storage type won't risk breaking the other.
162-176: Efficiently converting streams to buffers.The approach is straightforward, though for very large files, this can consume memory. In future expansions, consider streaming directly to S3 instead of buffering the entire content if memory usage becomes an issue.
frontend/src/components/sidebar.tsx (2)
77-114: Header layout implementation looks good, but consider translating Chinese comments.The header layout uses appropriate flex patterns to handle both collapsed and expanded states. However, there's a Chinese comment that should be translated to English for consistency.
- {/* SidebarTrigger 保证在 CodeFox 按钮的中间 */} + {/* SidebarTrigger positioned in the middle of the CodeFox button */}
119-161: Improved "New Project" button with responsive behavior.The button implementation now correctly handles both collapsed and expanded states, with appropriate navigation logic. One small suggestion: consider extracting the SVG into a separate component to improve readability.
Consider extracting the SVG into a component or using an SVG file import to reduce the complexity of this component:
- <svg - data-name="Layer 1" - viewBox="0 0 32 32" - preserveAspectRatio="xMidYMid meet" - xmlns="http://www.w3.org/2000/svg" - className={ - isCollapsed - ? 'w-8 h-8 min-w-[32px] min-h-[32px] ml-[-5px] mt-[-10px]' - : 'w-10 h-10 min-w-[32px] min-h-[32px] mr-1' - } - strokeWidth="0.1" - > - <g transform="scale(-1,1) translate(-32,0)"> - <path - d="M5,8A1,1,0,0,0,7,8V7H8A1,1,0,0,0,8,5H7V4A1,1,0,0,0,5,4V5H4A1,1,0,0,0,4,7H5ZM18,5H12a1,1,0,0,0,0,2h6a1,1,0,0,1,1,1v9.72l-1.57-1.45a1,1,0,0,0-.68-.27H8a1,1,0,0,1-1-1V12a1,1,0,0,0-2,0v3a3,3,0,0,0,3,3h8.36l3,2.73A1,1,0,0,0,20,21a1.1,1.1,0,0,0,.4-.08A1,1,0,0,0,21,20V8A3,3,0,0,0,18,5Z" - fill="#808080" - /> - </g> - </svg> + <NewProjectIcon isCollapsed={isCollapsed} />backend/src/mail/mail.service.ts (2)
28-42: Password reset email has duplicate user information in context.The method works correctly, but there's redundancy in the context data where both
nameandfirstNameare set to the same value (user.username).context: { name: user.username, - firstName: user.username, url, },
44-58: Consider removing commented code or adding an explanation.There's a commented-out alternative implementation of
sendConfirmationEmailthat should either be removed or have a clear comment explaining why it's kept.If this is kept as a reference or for future implementation, add a comment explaining why:
+ // Alternative implementation for confirmation emails with more context data. + // Currently not in use, but kept for reference. // async sendConfirmationEmail(user: User, token: string) { // const frontendUrl = this.configService.get('FRONTEND_URL'); // const url = `${frontendUrl}/confirm-email?token=${token}`;backend/src/project/project.resolver.ts (1)
151-162: Outdated TODO comment about rate limiting.The TODO comment on line 153 mentions implementing rate limiting later, but you've already implemented project creation rate limiting in this PR.
/** * Fetch public projects with limittation - * TODO(Sma1lboy): handle Rate limit later - each MAC shouldn't exceed 20 requests per minute + * TODO(Sma1lboy): handle API Rate limit - each MAC shouldn't exceed 20 requests per minute + * (Note: Project creation rate limiting has been implemented separately) * @param input the inputs * @returns return some projects */frontend/src/components/sign-up-modal.tsx (2)
105-118: Timer-based cooldown logic is clear and effective.
The countdown approach andclearTimeoutusage are correct. You could optionally convert the condition to use optional chaining on line 113 (e.g.,resendMessage?.includes(...)) but the current null-check is sufficient.🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
213-282: Sign-up form design is solid.
Form fields and error states are handled thoroughly. As a potential enhancement, consider adding a “Confirm Password” field to reduce accidental typos.frontend/src/graphql/type.tsx (1)
340-343:UpdateProjectPhotoInputis well-structured.
AcceptingfileandprojectIdis straightforward. Ensure you handle file size limits and security checks downstream in the resolver or service.backend/src/auth/auth.service.ts (1)
111-142:resendVerificationEmailenforces a cooldown.
Ensures multiple emails are not triggered in quick succession. You might add logging for cooldown rejections to monitor suspicious activity.Do you want me to provide a quick snippet for logging or metrics on repeated resend attempts?
backend/src/project/project.service.ts (3)
48-48: Constructor injection of UploadService.Adding
uploadServiceas a constructor parameter is straightforward. Ensure the service is properly mocked or injected in tests to verify file uploads.
481-501: File upload handling.The
try/catchstructure around the upload ensures comprehensive error logging. Consider any additional validations (e.g., scanning, size limits) before uploading possibly untrusted files.
663-685: Method duplication with canCreateProject.
getRemainingProjectLimitrepeats date computations. Consider refactoring common logic (e.g., retrieving today’s project count) into a shared helper to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (51)
.gitignore(1 hunks)backend/.env(0 hunks)backend/.env.development(0 hunks)backend/.env.example(1 hunks)backend/.gitignore(1 hunks)backend/package.json(3 hunks)backend/src/app.module.ts(2 hunks)backend/src/auth/auth.module.ts(2 hunks)backend/src/auth/auth.resolver.ts(2 hunks)backend/src/auth/auth.service.ts(4 hunks)backend/src/config/config.module.ts(1 hunks)backend/src/config/config.service.ts(1 hunks)backend/src/config/env.validation.ts(1 hunks)backend/src/guard/project.guard.ts(1 hunks)backend/src/mail/mail.module.ts(1 hunks)backend/src/mail/mail.service.ts(1 hunks)backend/src/mail/templates/confirmation.hbs(1 hunks)backend/src/mail/templates/passwordReset.hbs(1 hunks)backend/src/main.ts(2 hunks)backend/src/project/dto/project.input.ts(2 hunks)backend/src/project/project-limits.ts(1 hunks)backend/src/project/project.module.ts(1 hunks)backend/src/project/project.resolver.ts(3 hunks)backend/src/project/project.service.ts(7 hunks)backend/src/upload/upload.module.ts(1 hunks)backend/src/upload/upload.service.ts(1 hunks)backend/src/user/dto/resend-email.input.ts(1 hunks)backend/src/user/user.model.ts(2 hunks)backend/src/user/user.module.ts(1 hunks)backend/src/user/user.resolver.ts(2 hunks)frontend/next.config.mjs(1 hunks)frontend/package.json(1 hunks)frontend/src/app/(main)/page.tsx(3 hunks)frontend/src/app/api/file/route.ts(1 hunks)frontend/src/app/api/runProject/route.ts(3 hunks)frontend/src/app/auth/confirm/page.tsx(1 hunks)frontend/src/components/chat/chat-layout.tsx(2 hunks)frontend/src/components/chat/code-engine/code-engine.tsx(3 hunks)frontend/src/components/chat/code-engine/project-context.tsx(4 hunks)frontend/src/components/chat/code-engine/web-view.tsx(8 hunks)frontend/src/components/root/nav.tsx(1 hunks)frontend/src/components/root/root-layout.tsx(1 hunks)frontend/src/components/sidebar.tsx(6 hunks)frontend/src/components/sign-in-modal.tsx(1 hunks)frontend/src/components/sign-up-modal.tsx(5 hunks)frontend/src/graphql/mutations/auth.ts(2 hunks)frontend/src/graphql/request.ts(0 hunks)frontend/src/graphql/schema.gql(6 hunks)frontend/src/graphql/type.tsx(9 hunks)frontend/src/lib/client.ts(3 hunks)llm-server/.env(0 hunks)
💤 Files with no reviewable changes (4)
- backend/.env.development
- llm-server/.env
- frontend/src/graphql/request.ts
- backend/.env
✅ Files skipped from review due to trivial changes (5)
- backend/.gitignore
- backend/src/guard/project.guard.ts
- .gitignore
- backend/.env.example
- backend/src/mail/templates/passwordReset.hbs
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/components/root/nav.tsx
- frontend/src/components/chat/chat-layout.tsx
- frontend/src/components/chat/code-engine/project-context.tsx
- frontend/src/app/(main)/page.tsx
- frontend/src/components/sign-in-modal.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/components/sign-up-modal.tsx
[error] 113-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/components/sidebar.tsx
[error] 23-23: Shouldn't redeclare 'useRouter'. Consider to delete it or rename it.
'useRouter' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (125)
frontend/src/components/chat/code-engine/web-view.tsx (6)
10-12: Icon imports added correctly.Icons for refresh and zoom controls are imported correctly, following the existing pattern in the file.
21-21: Good choice for default scale.The default scale of 0.7 is a reasonable starting point that allows users to see more content in the preview while maintaining readability.
56-56: Reduced timeout improves responsiveness.Decreasing the timeout from 200ms to 100ms improves the UI responsiveness when loading previews.
181-188: Reload button is well implemented.The reload button implementation follows the existing UI patterns and provides the user with a useful way to refresh the iframe content.
205-222: Zoom controls are well implemented.The zoom in and zoom out buttons follow the existing UI patterns and provide the user with a useful way to adjust the preview size.
248-258: Iframe styling for zoom works well.The implementation of iframe scaling is good. The transform origin set to 'top left' ensures consistent zooming behavior, and the dynamic adjustment of width and height based on scale maintains the proper display of content.
backend/src/auth/auth.module.ts (2)
12-12: Mail functionality integration looks good.Adding the MailModule import is a good step for supporting email functionality in the auth module.
27-27: MailModule properly included in imports array.MailModule is correctly added to the imports array, which will make email services available for authentication-related features like account verification and password resets.
backend/src/project/project.module.ts (2)
12-13: Good addition of required modules.Adding AppConfigModule and UploadModule imports will enable configuration access and file upload functionality within the project module.
18-20: Well-organized module imports.The modules are added properly to the imports array. This change ensures that configuration settings, authentication, and file upload services are available within the ProjectModule.
backend/src/app.module.ts (2)
18-18: Mail module import properly added.Adding the MailModule import is appropriate for enabling email functionality across the application.
51-51: MailModule correctly registered in AppModule.MailModule is properly added to the imports array in the AppModule, making email services available throughout the application.
backend/src/upload/upload.module.ts (1)
1-10: Well-structured NestJS module implementation.This module follows the standard NestJS structure with appropriate imports, providers, and exports. The organization is clean and aligns with NestJS best practices.
backend/src/user/user.module.ts (1)
9-17: Clean integration of MailModule.The addition of MailModule to the UserModule's imports is appropriate and follows NestJS best practices. The refactoring of imports into a multiline format improves readability, especially as the list of imports grows.
backend/src/main.ts (3)
6-6: Good addition of GraphQL upload capability.The import of
graphqlUploadExpressis appropriate for enabling file upload functionality in the GraphQL API.
22-22: Proper CORS configuration for Apollo GraphQL.Adding the 'Apollo-Require-Preflight' header to the allowed headers list is necessary for Apollo Client to properly handle file uploads.
27-30:✅ Verification successful
Well-configured file upload middleware.
The file upload middleware configuration includes appropriate limits for file size (50MB) and number of files (10). However, consider if these limits align with your application's requirements and server capabilities.
🏁 Script executed:
#!/bin/bash # Check if there are any environment variables or configuration that might override these limits grep -r "maxFileSize\|maxFiles" --include="*.ts" --include="*.js" .Length of output: 979
File Upload Middleware Configuration Verified
The file upload middleware in backend/src/main.ts (lines 27–30) is properly configured with explicit limits of 50MB per file and a maximum of 10 files per request. Our review confirms that there are no additional configurations or environment variables overriding these settings. Please double-check that these limits align with your application's requirements and server capabilities.
backend/src/mail/templates/confirmation.hbs (5)
1-6: Well-structured HTML document setup.The HTML template has proper DOCTYPE declaration, appropriate meta tags, and a clear title that explains the purpose of the email.
7-8: Good responsive design approach.The body styling uses a responsive layout with appropriate padding, font family, and a centered container with a maximum width, which works well across different email clients.
9-12: Clear and welcoming message.The welcome message is friendly and concise, clearly communicating the purpose of the email to the user.
13-17: Accessible call-to-action button.The confirmation button is styled as a prominent call-to-action with good color contrast and padding. The template correctly uses a Handlebars variable
{{confirmUrl}}for the dynamic confirmation link.
18-20: Important security information included.Good practice to mention the expiration time of the confirmation link and what to do if the user didn't request this email.
frontend/src/graphql/mutations/auth.ts (3)
31-38: Well-structured GraphQL mutation for email confirmation.The
CONFIRM_EMAIL_MUTATIONis properly defined and follows the existing pattern in the file. It accepts a token parameter and returns appropriate response fields.
40-47: GraphQL mutation for resending confirmation email is properly implemented.The
RESEND_CONFIRMATION_EMAIL_MUTATIONis correctly defined using the newResendEmailInputtype and returns the appropriate response fields. This aligns well with the corresponding backend implementation.
31-47:✅ Verification successful
Verify that the mutation responses align with backend resolver implementations.
Ensure that the backend resolvers for these mutations return objects with exactly the fields specified here (
messageandsuccess).
🏁 Script executed:
#!/bin/bash # Check the resolver implementations for these mutations echo "Checking for confirmEmail resolver:" rg -A 10 "confirmEmail\(" --type=ts echo -e "\nChecking for resendConfirmationEmail resolver:" rg -A 10 "resendConfirmationEmail\(" --type=tsLength of output: 5066
Mutation response verification: After running the verification scripts, it’s confirmed that both the
confirmEmailandresendConfirmationEmailresolvers return objects containing themessageandsuccessfields as expected. No further changes are required in the GraphQL mutation definitions infrontend/src/graphql/mutations/auth.ts.backend/src/user/dto/resend-email.input.ts (3)
2-4: Appropriate imports for DTO implementation.The necessary decorators and validators are properly imported from the correct packages.
5-11: Well-defined DTO with proper validation.The
ResendEmailInputclass is properly decorated with@InputType()for GraphQL and includes appropriate validation decorators for the email field. This ensures that only valid email addresses are accepted.
1-11:✅ Verification successful
Verify usage of this DTO in the resolvers.
Confirm that this DTO is correctly imported and used in the corresponding resolver functions.
🏁 Script executed:
#!/bin/bash # Check where ResendEmailInput is being imported and used rg -A 5 "ResendEmailInput" --type=tsLength of output: 1496
DTO Usage Verification Confirmed
- The
ResendEmailInputis correctly imported inbackend/src/user/user.resolver.tsand is used as the expected input for the resolver function.- Its usage is also consistent in the frontend GraphQL mutation definition in
frontend/src/graphql/mutations/auth.ts.No further changes are required.
frontend/src/components/root/root-layout.tsx (4)
15-31: Well-implemented authorization check for sidebar visibility.The component correctly uses the
isAuthorizedstate from the auth context and updates the sidebar visibility accordingly through theuseEffecthook. This ensures that the sidebar is only shown to authorized users.
33-59: Clean responsive sidebar implementation with animations.The sidebar uses Framer Motion for smooth animations and handles both collapsed and expanded states well. The conditional rendering based on
showSidebaris properly implemented.
61-68: Effective layout adjustment based on sidebar state.The main content area properly adjusts its margin based on whether the sidebar is shown and whether it's collapsed or expanded, creating a responsive layout.
45-57:✅ Verification successful
ProjectProvider wrapper removed from ChatSideBar.
The
ProjectProviderwrapper has been removed, which simplifies the component hierarchy. However, we should verify thatChatSideBardoesn't depend on any context provided byProjectProvider.
🏁 Script executed:
#!/bin/bash # Check if ChatSideBar uses useProjectContext or similar hooks echo "Checking for Project context usage in ChatSideBar:" cat $(fd "sidebar" --type f --extension tsx --extension jsx | grep -v test) | grep -i "useProject\|ProjectContext" || echo "No direct references found" # Check if ProjectProvider is used elsewhere for ChatSideBar echo -e "\nChecking for other ChatSideBar + ProjectProvider usages:" rg "<ProjectProvider.*?>\s*.*?<ChatSideBar" -U --multilineLength of output: 760
Simplify ChatSideBar by removing unnecessary ProjectProvider dependency
The verification indicates that there are no direct references to ProjectContext or any related hooks within the ChatSideBar component. Although one of the sidebar components (e.g. file-sidebar.tsx) uses ProjectContext, there were no findings of ChatSideBar being wrapped with or relying on ProjectProvider. This confirms that removing the ProjectProvider wrapper in frontend/src/components/root/root-layout.tsx is safe.
- Verified Areas:
- No direct use of
useProjectContextorProjectContextinside ChatSideBar.- No instances where
<ProjectProvider>wraps<ChatSideBar>were found in the codebase.backend/src/auth/auth.resolver.ts (2)
21-28: LGTM! Well-structured email confirmation response type.The new
EmailConfirmationResponseGraphQL object type is properly defined with required message field and optional success field.
46-51: Email confirmation mutation correctly implemented.The mutation is well-structured, follows the established pattern in the resolver class, and properly passes the token to the service method.
backend/src/config/config.module.ts (3)
1-7: LGTM! Proper imports for the configuration module.All necessary dependencies are imported, including NestJS modules and validation utilities.
8-22: Well-implemented configuration validation function.The validation function properly transforms and validates environment variables, with appropriate error handling.
24-34: LGTM! Configuration module setup follows best practices.The module is correctly set up with:
- Global configuration availability
- Proper validation
- Service provider and export
This implementation follows NestJS best practices for configuration modules.
backend/src/user/user.model.ts (2)
14-14: LGTM! Import of UpdateDateColumn for tracking email timestamps.The import is properly added to support the new
lastEmailSendTimefield.
36-42: Email confirmation tracking fields correctly implemented.The two new fields are well-structured:
isEmailConfirmed: Boolean with default falselastEmailSendTime: Auto-updating timestampBoth fields are properly decorated for GraphQL and TypeORM.
backend/src/mail/mail.module.ts (4)
1-10: LGTM! Comprehensive imports for the mail module.All necessary dependencies are imported, including the mailer module, handlebars adapter, and supporting services.
11-43: Mail module configuration is comprehensive and follows best practices.The email service configuration is well-structured with:
- Async factory pattern for configuration
- Environment variable integration
- Proper template setup with Handlebars
- Secure authentication handling
44-47: Mail service properly provided and exported.The module correctly provides and exports the MailService, making it available to other modules that depend on it.
19-28:❓ Verification inconclusive
Verify mail server credentials in environment variables.
Ensure that the required environment variables (
MAIL_HOST,MAIL_PORT,MAIL_USER,MAIL_PASSWORD) are properly set in your deployment environment.
🏁 Script executed:
#!/bin/bash # Check if mail-related environment variables are defined in .env files echo "Checking for mail configuration variables..." if [ -f .env ]; then echo "Examining .env file:" grep -E "MAIL_(HOST|PORT|USER|PASSWORD|FROM)" .env fi if [ -f .env.example ]; then echo "Examining .env.example file:" grep -E "MAIL_(HOST|PORT|USER|PASSWORD|FROM)" .env.example fi echo "Checking if these variables are documented:" if [ -f README.md ]; then echo "Searching in README.md:" grep -E "MAIL_(HOST|PORT|USER|PASSWORD|FROM)" README.md fiLength of output: 377
Mail Credentials Configuration Check
The mail module uses environment variables (
MAIL_HOST,MAIL_PORT,MAIL_USER, andMAIL_PASSWORD) to configure the transport settings. However, our initial check didn’t detect these variables in the repository’s.env,.env.example, orREADME.md. Please verify manually that these variables are indeed set in your deployment environment or through your configuration management system.
- File:
backend/src/mail/mail.module.ts(lines 19-28)- Action Required: Confirm that the mail server credentials are properly configured in your runtime environment even if they aren’t defined in the repository files.
frontend/package.json (3)
40-40: Added TypeScript definitions for Apollo upload client.This addition is appropriate for enhancing type safety when working with file uploads in your GraphQL operations.
45-45: The dependency was temporarily removed and re-added.Based on the diff, it seems that the "codefox-common" dependency was removed and then added back with the same version constraint. This doesn't cause any issues, but it's worth noting.
42-42:✅ Verification successful
Added Apollo upload client for file upload support.
The
apollo-upload-clientlibrary extends Apollo Client to support file uploads through GraphQL mutations, which is essential for enabling project photo uploads.Make sure that your GraphQL client configuration is updated to use this library. Typically, you need to replace the default Apollo link with a createUploadLink from this package:
🏁 Script executed:
#!/bin/bash # Check if the Apollo client is properly configured for file uploads # Look for createUploadLink usage in the codebase rg "createUploadLink|apolloUploadClient" --type tsLength of output: 218
GraphQL Client Configuration Verified
The file upload support is correctly implemented. We confirmed that
createUploadLinkfrom theapollo-upload-clientpackage is properly imported and used infrontend/src/lib/client.ts.
- Verify that
frontend/src/lib/client.tsimportscreateUploadLinkfrom'apollo-upload-client/createUploadLink.mjs'and initializes the upload link accordingly.backend/package.json (5)
50-50: Added class-transformer for object transformation.This library works with class-validator to handle object transformation between plain objects and class instances, which is useful for request validation and data transformation.
63-63: Added nodemailer for email handling.This is the underlying email sending library used by the @nestjs-modules/mailer package.
31-31:✅ Verification successful
Added AWS S3 client for file storage.
This addition supports file uploads to Amazon S3 or compatible services (like Cloudflare R2 mentioned in the environment variables).
Ensure your S3 service integration is properly set up. You'll need:
- A service that configures the S3 client
- Proper configuration of environment variables
🏁 Script executed:
#!/bin/bash # Check if S3 service is properly configured # Look for S3 client implementation rg "new S3Client|@aws-sdk/client-s3" --type tsLength of output: 224
S3 Client Dependency and Integration Verified
The AWS S3 client dependency in
backend/package.jsonis correctly added and the integration is confirmed inbackend/src/upload/upload.service.tsthrough the proper import and instantiation ofS3Client. Please ensure that the relevant environment variables for AWS credentials and region are configured as expected.
34-34:✅ Verification successful
Added NestJS mailer module for email functionality.
This package is appropriate for implementing email sending features like confirmation emails, password resets, etc.
Check if the mailer module is properly configured in your application:
🏁 Script executed:
#!/bin/bash # Check if MailerModule is imported and configured # Look for MailerModule imports and configuration rg "MailerModule.forRoot|MailerModule.forRootAsync" --type tsLength of output: 127
Mailer Module Integration Confirmed
The dependency on
@nestjs-modules/mailerinbackend/package.jsonis appropriate, and the module is correctly imported and configured usingMailerModule.forRootAsyncinbackend/src/mail/mail.module.ts. No further modifications are required.
59-59:✅ Verification successful
Added GraphQL upload support for file handling.
This enables file upload functionality in GraphQL resolvers, which is necessary for features like project photo uploads.
Check if GraphQL uploads are properly configured in your application:
🏁 Script executed:
#!/bin/bash # Check if graphql-upload is configured in main.ts or similar files # Look for graphql-upload configuration rg "graphqlUploadExpress|processRequest" --type tsLength of output: 222
GraphQL Upload Support Verified
- The new dependency
"graphql-upload-minimal": "^1.6.1"inbackend/package.jsonis correctly integrated.- The configuration was confirmed in
backend/src/main.tswheregraphqlUploadExpressis both imported and initialized with the expected settings (maxFileSize: 50000000, maxFiles: 10).- No further changes are necessary.
backend/src/user/user.resolver.ts (3)
19-20: Added necessary imports for email confirmation functionality.These imports support the newly implemented email resend functionality.
38-43: Removed commented-out code for clarity.Good practice to remove commented-out code rather than leaving it in the codebase. The implementation has been replaced with a cleaner version.
45-50:✅ Verification successful
Implemented email confirmation resend functionality.
This mutation allows users to request a new confirmation email if they didn't receive the original one or if it expired.
Ensure that:
- The
ResendEmailInputDTO properly validates the email field- The
authService.resendVerificationEmailmethod is properly implemented
🏁 Script executed:
#!/bin/bash # Verify the ResendEmailInput implementation and authService method # Check ResendEmailInput DTO echo "Checking ResendEmailInput implementation:" fd "resend-email.input.ts" -x cat {} # Check authService implementation of resendVerificationEmail echo -e "\nChecking authService.resendVerificationEmail implementation:" rg -A 10 "resendVerificationEmail" --type tsLength of output: 2365
Approved: Email Confirmation Resend Functionality Verified
- The
ResendEmailInputDTO insrc/auth/dto/resend-email.input.tscorrectly validates the@IsEmailand@IsNotEmpty.- The
authService.resendVerificationEmailmethod implementation inbackend/src/auth/auth.service.tsproperly handles checking for an existing user and returning an appropriate message when the email is already confirmed.- The mutation in
backend/src/user/user.resolver.ts(lines 45-50) correctly calls the service method.No further changes are needed.
frontend/src/lib/client.ts (4)
14-14: Proper import for file upload support.Good addition of the Apollo upload client which is specifically designed to handle file uploads with GraphQL.
17-25: Upload link setup looks good.The implementation correctly configures an upload link to handle file uploads with the appropriate headers. Setting 'Apollo-Require-Preflight' to 'true' is required to ensure the Apollo client sends multipart/form-data requests properly.
88-94: Well-structured HTTP link chain.Good refactoring to use the
fromutility to compose multiple links in the correct order. The links are properly organized with error handling first, followed by logging, authentication, and the upload link.
96-109: Link splitting logic looks good.The code correctly maintains the split between WebSocket for subscriptions and HTTP for queries/mutations while now using the enhanced HTTP link with upload capabilities.
backend/src/project/dto/project.input.ts (2)
5-5: Appropriate import for file upload support.Good choice using the
graphql-upload-minimalpackage which is a maintained alternative for handling file uploads in GraphQL.
123-132:✅ Verification successful
Well-structured input type for file uploads.
The new
UpdateProjectPhotoInputclass is well-defined with proper decorators for GraphQL and validation. The file field is correctly marked as optional using@IsOptional()and properly typed withGraphQLUpload.However, verify that your NestJS application is configured to handle multipart form data for file uploads:
🏁 Script executed:
#!/bin/bash # Check if the NestJS app is configured with the graphql-upload middleware grep -r "graphqlUploadExpress" backend/src/main.ts || grep -r "graphqlUploadExpress" backend/src/app.module.tsLength of output: 180
File Upload Input and Middleware Configuration Verified
The new
UpdateProjectPhotoInputclass is well-defined, with correct decorators and proper typing for handling file uploads. Additionally, the NestJS application inbackend/src/main.tsis correctly configured with thegraphqlUploadExpressmiddleware (usinggraphql-upload-minimal), confirming that multipart form data handling is enabled.frontend/src/components/chat/code-engine/code-engine.tsx (5)
85-88: Improved error handling with proper braces and logging.The addition of braces around the conditional return statement improves code clarity. The console log message helps with debugging when no project path is found.
94-94: Helpful debug logging.Adding debug logs for important operations like loading the file structure helps with troubleshooting issues.
277-277: Fix for content overflow issues.Changing from
overflow: hiddentooverflow-scrollallows users to scroll when content exceeds the container size, improving usability.
282-282: Adjusted content height calculation.The height calculation has been modified from
calc(100vh-48px-2rem)tocalc(100vh-48px-4rem), which reduces the available height. Ensure this doesn't cut off important content on smaller screens.Consider testing this on various screen sizes to confirm that the height adjustment doesn't negatively impact the user experience on smaller viewports.
318-318:✅ Verification successful
Updated default language for the code editor.
Changing the default language from
typescripttotypescriptreactis appropriate if most of the edited files are React TypeScript files (.tsx).
🏁 Script executed:
#!/bin/bash # Check the distribution of TypeScript file types in the project echo "Regular TypeScript files (.ts):" find frontend/src -name "*.ts" | grep -v ".d.ts" | wc -l echo "React TypeScript files (.tsx):" find frontend/src -name "*.tsx" | wc -lLength of output: 248
Default Editor Language Update Verified
- The change in
frontend/src/components/chat/code-engine/code-engine.tsxfromtypescripttotypescriptreactis confirmed by the file distribution (75 React TSX files vs. 20 regular TS files).- This adjustment is appropriate given the predominant usage of React TypeScript files.
backend/src/config/config.service.ts (5)
1-4: Good imports with proper typing.The imports are appropriate with a properly typed NestConfigService using EnvironmentVariables for type safety.
5-8: Well-structured service class with proper injection.The AppConfigService class is properly decorated with @Injectable() and uses dependency injection to access the NestConfigService.
9-42: Clean configuration getters with good documentation.The getters for configuration values are well-documented with JSDoc comments and provide type-safe access to environment variables.
44-57: Comprehensive S3 configuration object.The s3Config getter returns a complete object with all necessary S3/Cloudflare R2 configuration parameters, making it easy to access these related values as a group.
59-70:✅ Verification successful
Useful validation for S3 configuration.
The hasS3Configured getter provides a convenient way to check if all required S3 configuration parameters are present, which helps prevent runtime errors when attempting to use S3 functionality.
Confirm that this validation is actually used before attempting S3 operations:
🏁 Script executed:
#!/bin/bash # Check if the hasS3Configured method is being used before S3 operations rg "hasS3Configured" --type ts backend/srcLength of output: 344
S3 Configuration Check Confirmed
The
hasS3Configuredgetter is effectively used before any S3 operations, as shown inbackend/src/upload/upload.service.ts. The validation properly confirms all necessary parameters, and no further changes are required.frontend/src/app/api/runProject/route.ts (1)
147-151: Ensure correct shell escaping or validation for injected variables.When constructing a long command string with multiple arguments, ensure the domain and other variables are properly escaped or validated. Malicious or malformed input could break the command or create security concerns.
frontend/src/app/auth/confirm/page.tsx (4)
1-2: Usage of 'use client' is appropriate for Next.js routes that require client functionality.No issues spotted; this ensures the component runs in the client environment.
24-44: Good use of Apollo error handling.You handle both
onCompletedandonErrorproperly, ensuring the user sees the correct status. This is clean and effective.
46-62: Check token validity server-side if feasible.You handle the absence of a token gracefully. However, if there's any heavy logic or token validation needed, consider performing it server-side for security.
63-102: UI feedback flow is well structured.You provide clear visual feedback for loading, success, and error states. The 3-second redirect is a positive user experience. No concerns here.
backend/src/upload/upload.service.ts (3)
1-15: Keep imports minimal and organized.The imports and the
UploadResultinterface look appropriate. No concerns here, but ensure you only import what you use to avoid bloating the bundle in larger projects.
16-41: Constructor approach is clear and well-structured.Your logic for conditional S3 client initialization and media directory creation is well-organized. This ensures local fallback when S3 config is absent.
177-193: Check bucket name validity behavior.If the bucket name is missing or invalid, you might want to provide an immediate error instead of constructing a partially valid URL. Consider validating
s3Config.bucketNameat the start or throwing an error if missing.backend/src/project/project-limits.ts (3)
4-4: Project creation limit is well-defined with a clear constant.The
PROJECT_DAILY_LIMITconstant is appropriately defined at the module level, making it easy to adjust in the future.
6-8: Good use of enum for error code standardization.Using an enum for the error code provides type safety and consistency in error handling across the application.
10-26: Well-implemented exception class with proper GraphQL integration.The
ProjectRateLimitExceptionclass extendsForbiddenExceptioncorrectly and provides a clear error message to users. ThegetGraphQLError()method is particularly valuable for consistent error handling in the GraphQL context, including appropriate status codes and metadata.I especially like that you're returning
HttpStatus.TOO_MANY_REQUESTSrather than just the default forbidden status, which is more semantically appropriate for rate limiting.frontend/src/components/sidebar.tsx (2)
181-181: Router navigation implementation is correct.Using
router.pushfor navigation is the correct approach in Next.js applications.
192-194: Good responsive footer implementation.The footer layout correctly adjusts based on the sidebar's collapsed state.
backend/src/mail/mail.service.ts (2)
8-15: Well-structured service class with proper dependency injection.The
MailServiceclass follows NestJS best practices with proper dependency injection through constructor parameters.
17-26: Confirmation email implementation looks good.The method correctly uses the configuration service to build the confirmation URL and sends the email with appropriate context.
backend/src/project/project.resolver.ts (1)
165-170: Good implementation of the remaining project limit query.This query method works well with the new rate limiting functionality, allowing clients to check limits before attempting to create projects.
frontend/src/components/sign-up-modal.tsx (7)
21-24: Imports for authentication mutations look good.
No issues found with these new imports.
27-28: Icon and effect imports are correctly added.
These imports fromlucide-reactand React'suseEffectprovide the necessary UI icons and lifecycle hooks for your feature.
42-44: New state variables are well-defined.
StoringregistrationSuccess,resendCooldown, andresendMessageensures clear separation of concerns for each piece of state.
55-55: Good practice updating the registration success state.
This assignment helps conditionally render the success message upon registration completion.
83-103: Mutation for resend confirmation email is well-structured.
Error handling, success message updates, and cooldown assignment are handled nicely. Consider logging different error codes if needed to provide more context for debugging.
[approve]
120-131: Resend handler is straightforward.
Prevents the operation during cooldown or while loading. Looks good overall.
145-211: Success message UI is user-friendly.
Displaying the verification email instructions and providing a clear call to action to resend or close is well done. The icons and color feedback are helpful visual cues.frontend/src/graphql/type.tsx (5)
39-40: NewUploadscalar is declared properly.
Ensures your schema can handle file uploads.
159-159: Renamed mutationupdateProjectPhotois consistent.
This name better reflects the mutation purpose.
215-216: New args type strongly typed.
MutationUpdateProjectPhotoArgsreferencingUpdateProjectPhotoInputis clearer and helps maintain a strict schema contract.
501-502: Extended resolvers correctly include newUploadand input types.
No issues spotted here. The typing is aligned with the rest of the schema definitions.
913-917:UploadScalarConfigextendsGraphQLScalarTypeConfigproperly.
Fine addition for custom scalars. Consider verifyingUploadusage in your server to handle large files safely.backend/src/auth/auth.service.ts (10)
22-25: Importing new resolver types is accurate.
These additions tie in well with your updated email confirmation logic.
26-26: InjectingMailServicesupports email workflow.
Ensures your email logic remains in a dedicated service.
30-30:isMailEnabledproperty introduced for feature toggling.
A good approach to conditionally control email-based flows.
38-38: Injecting mailService as a constructor param is correct.
Complements theisMailEnabledlogic for your email features.
45-50: ReadingMAIL_ENABLEDenvironment variable is a neat toggle approach.
This ensures your service gracefully adapts to environments without mail capability.
52-89:confirmEmailfunction handles token verification cleanly.
UsesjwtService.verifyAsync, checks the user record, and updatesisEmailConfirmed. Good error handling for expired or invalid tokens.
91-109:sendVerificationEmailis well-structured.
Generates a 30-minute token and updateslastEmailSendTime. The separation into a dedicated function fosters clarity and reusability.
158-173: Differentiated user creation for mail-enabled vs non-mail setups.
Automatically settingisEmailConfirmedto false or true is practical for different deployments.
175-180: Sending verification email right after registration is consistent.
Makes sense to initiate the process upon successful user creation.
195-197: Blocking login for unconfirmed emails is appropriate.
This upholds security, ensuring only verified accounts can gain system access.backend/src/project/project.service.ts (7)
10-10: Minor Import Addition.The inclusion of
Betweenand other imports from TypeORM is appropriate for the new date-based queries. No issues found here.
29-29: New UploadService dependency.Injecting the
UploadServicewill facilitate file uploads. Looks good.
30-33: Project limits import acknowledged.The import of
PROJECT_DAILY_LIMITandProjectRateLimitExceptionseems clear and cohesive with the creation-limit functionality introduced below.
129-150: Daily project creation limit logic.This logic correctly counts projects for “today” using time boundaries. However, consider potential edge cases with different time zones (UTC vs. server local time). Be sure this aligns with business requirements for “daily” project limits.
157-161: Conditional rate-limiting check.Good job ensuring the user’s limit is checked before creating a project. This is a sensible place to throw an exception, preventing wasted database operations.
203-205: GraphQL error handling for rate limit exception.Re-throwing the custom exception as a GraphQL error is a neat approach. Confirm there's a test covering this scenario to ensure the client receives the proper error object.
473-475: Signature updated for file upload parameters.The expanded signature includes file info. Ensure validation (e.g., file size checks, MIME type checks) is implemented if needed for security and consistency.
frontend/src/graphql/schema.gql (10)
57-57: Docstring re-labeled.No functional impact here.
60-63: New EmailConfirmationResponse type.Defining a dedicated type for email confirmation responses is clear and explicit.
109-109: confirmEmail mutation.Straightforward addition for handling email confirmation. Looks good.
119-119: resendConfirmationEmail mutation.Provides a simple way to trigger another confirmation email. No issues found.
123-123: updateProjectPhoto mutation signature change.Renaming from
updateProjectPhotoUrltoupdateProjectPhotoaligns well with passing a file object. Good design choice.
179-179: New getRemainingProjectLimit query.Consistent with the backend’s method. This helps clients display remaining project creation capacity.
199-201: ResendEmailInput introduced.Defines email input for resending confirmation. Concise and clear.
223-226: UpdateProjectPhotoInput structure.Accepts a file and project ID. Matches the updated backend signature.
228-229: New Upload scalar.Standard approach for handling file uploads in GraphQL.
238-239: Additional user fields.Adding
isEmailConfirmedandlastEmailSendTimeto theUsertype is logical for tracking email confirmation status.
| const checkUrlStatus = async (url: string) => { | ||
| let status = 0; | ||
| while (status !== 200) { | ||
| try { | ||
| const res = await fetch(url, { method: 'HEAD' }); | ||
| status = res.status; | ||
| if (status !== 200) { | ||
| console.log(`URL status: ${status}. Retrying...`); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| } | ||
| } catch (err) { | ||
| console.error('Error checking URL status:', err); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const baseUrl = `http://${json.domain}`; | ||
| await checkUrlStatus(baseUrl); | ||
|
|
||
| setBaseUrl(baseUrl); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a maximum retry limit to URL status check.
The checkUrlStatus function is a good addition that ensures the iframe only loads after confirming the URL is available. However, it lacks a maximum retry limit, which could potentially lead to an infinite loop if the URL never returns a 200 status.
Consider adding a maximum retry count to prevent indefinite looping:
const checkUrlStatus = async (url: string) => {
let status = 0;
+ let retryCount = 0;
+ const maxRetries = 30; // 30 seconds timeout
while (status !== 200) {
+ if (retryCount >= maxRetries) {
+ console.error('URL status check timed out after', maxRetries, 'attempts');
+ break;
+ }
try {
const res = await fetch(url, { method: 'HEAD' });
status = res.status;
if (status !== 200) {
console.log(`URL status: ${status}. Retrying...`);
await new Promise((resolve) => setTimeout(resolve, 1000));
+ retryCount++;
}
} catch (err) {
console.error('Error checking URL status:', err);
await new Promise((resolve) => setTimeout(resolve, 1000));
+ retryCount++;
}
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkUrlStatus = async (url: string) => { | |
| let status = 0; | |
| while (status !== 200) { | |
| try { | |
| const res = await fetch(url, { method: 'HEAD' }); | |
| status = res.status; | |
| if (status !== 200) { | |
| console.log(`URL status: ${status}. Retrying...`); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| } catch (err) { | |
| console.error('Error checking URL status:', err); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| } | |
| }; | |
| const baseUrl = `http://${json.domain}`; | |
| await checkUrlStatus(baseUrl); | |
| setBaseUrl(baseUrl); | |
| const checkUrlStatus = async (url: string) => { | |
| let status = 0; | |
| let retryCount = 0; | |
| const maxRetries = 30; // 30 seconds timeout | |
| while (status !== 200) { | |
| if (retryCount >= maxRetries) { | |
| console.error('URL status check timed out after', maxRetries, 'attempts'); | |
| break; | |
| } | |
| try { | |
| const res = await fetch(url, { method: 'HEAD' }); | |
| status = res.status; | |
| if (status !== 200) { | |
| console.log(`URL status: ${status}. Retrying...`); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| retryCount++; | |
| } | |
| } catch (err) { | |
| console.error('Error checking URL status:', err); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| retryCount++; | |
| } | |
| } | |
| }; | |
| const baseUrl = `http://${json.domain}`; | |
| await checkUrlStatus(baseUrl); | |
| setBaseUrl(baseUrl); |
frontend/next.config.mjs
Outdated
| @@ -28,7 +28,7 @@ const nextConfig = { | |||
| remotePatterns: [ | |||
| { | |||
| protocol: 'https', | |||
| hostname: 'picsum.photos', | |||
| hostname: '*', | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern with wildcard image source.
Changing the hostname pattern from 'picsum.photos' to '*' allows loading images from any domain, which could lead to security vulnerabilities like:
- Cross-Site Scripting (XSS) if the images contain malicious code
- Cross-Origin Resource Sharing (CORS) issues
- Potential for bandwidth theft (hotlinking)
- Malicious use of user credentials in some contexts
Consider restricting this to only the specific domains you need instead of allowing all domains.
- hostname: '*',
+ hostname: '*.yoursite.com',Or add multiple specific patterns:
- hostname: '*',
+ },
+ {
+ protocol: 'https',
+ hostname: 'picsum.photos',
+ pathname: '/**',
+ },
+ {
+ protocol: 'https',
+ hostname: 'your-backend-storage.com',
+ pathname: '/**',🌐 Web query:
Next.js image security best practices for remotePatterns
💡 Result:
Next.js provides robust image optimization capabilities through its <Image> component, but securing external image sources requires careful configuration. Here are the key security best practices for remotePatterns:
Core Security Principles
- Avoid wildcards in
remotePatternsconfigurations to prevent Server-Side Request Forgery (SSRF) attacks[2][9] - Be specific with protocols, domains, and paths to limit attack surfaces[1][7]
- Combine with security headers like Content-Security-Policy for defense in depth[4][8][10]
Secure Configuration Patterns
Basic Allowlist
// next.config.js
module.exports = {
images: {
remotePatterns: [
{
protocol: 'https',
hostname: 'assets.example.com',
pathname: '/images/**',
port: ''
}
]
}
}Advanced Restrictions
// next.config.js
module.exports = {
images: {
remotePatterns: [
{
protocol: 'https',
hostname: 'cdn.example.net',
pathname: '/user-uploads/**',
search: '?verified=true'
}
],
formats: ['image/webp'],
dangerouslyAllowSVG: false,
contentSecurityPolicy: "default-src 'self'"
}
}Critical Security Measures
-
Path Validation
- Use explicit path patterns (e.g.,
/blog-images/**) - Avoid empty pathname values[1][7]
- Use explicit path patterns (e.g.,
-
Protocol Enforcement
- Restrict to HTTPS only[1][7]
protocol: 'https' -
Port Specification
- Explicitly define ports when required
port: '8080' -
Query Parameter Control
- Validate required query parameters
search: '?v=verified123'
Security Risks to Mitigate
| Risk | Mitigation Strategy | Impact |
|---|---|---|
| SSRF Attacks[2][9] | Strict hostname patterns | High |
| Malicious SVG Execution[1][9] | dangerouslyAllowSVG: false |
Critical |
| Open Redirects | Validate pathname & search params | Medium |
| Data Exfiltration | Restrict image formats | Medium |
Maintenance Best Practices
- Regular audits of allowed domains[7][10]
- Monitor Next.js updates for security improvements[7]
- Use automated scanning tools to detect misconfigurations[2][10]
- Implement rate limiting for image optimization API[3]
For maximum security, combine remotePatterns with:
- Content Security Policy headers[8][10]
- CSRF protection[8]
- Input validation middleware[4]
- Security-focused CI/CD pipelines[10]
Remember that complete wildcard patterns (hostname: '*') should never be used in production environments due to SSRF risks[2][5][9]. Always prefer explicit allowlisting over broad permissions.
Citations:
- 1: https://nextjs.org/docs/pages/api-reference/components/image
- 2: https://www.acunetix.com/vulnerabilities/web/next-js-image-blind-ssrf/
- 3: With all remotePatterns allowed for images, some image URLs are still rejected vercel/next.js#60483
- 4: https://blog.openreplay.com/best-practices-for-security-in-nextjs/
- 5: https://dev.to/joodi/allow-all-domains-for-images-in-nextjs-hpj
- 6: https://nextjs.org/docs/pages/building-your-application/optimizing/images
- 7: https://www.timsanteford.com/posts/migrating-next-js-image-domains-to-remote-patterns/
- 8: https://www.vintasoftware.com/blog/security-nextjs-applications
- 9: Allowing all remotePatterns when using next/image - is it safe? vercel/next.js#71843
- 10: https://blog.arcjet.com/next-js-security-checklist/
Security Update Required: Avoid Wildcard for Remote Image Sources
The current configuration in frontend/next.config.mjs (line 31) uses a wildcard (hostname: '*'), which exposes the application to significant security risks—such as SSRF, potential XSS via malicious image content, unintended CORS issues, and hotlinking. According to Next.js image security best practices, wildcards should be avoided in favor of explicit allowlisting.
Please update the configuration by either:
- Restricting the hostname to your trusted domain (e.g., replacing
'*'with'*.yoursite.com'), or - Defining multiple, specific remote patterns to cover all required hosts.
Example diff snippets:
- hostname: '*',
+ hostname: '*.yoursite.com',Or, if multiple sources are needed:
- hostname: '*',
+ },
+ {
+ protocol: 'https',
+ hostname: 'picsum.photos',
+ pathname: '/**',
+ },
+ {
+ protocol: 'https',
+ hostname: 'your-backend-storage.com',
+ pathname: '/**',This change adheres to Next.js best practices by using explicit protocol, domain, and path restrictions to mitigate potential security vulnerabilities.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hostname: '*', | |
| hostname: '*.yoursite.com', |
| <p style="font-size: 14px; color: #888;"> | ||
| Need help? Contact our support team at <a href="mailto:support@codefox.net">support@codefox.net</a>. | ||
| </p> |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify the support email address.
Ensure that the support email address is valid and monitored. Consider using a configuration variable instead of hardcoding the email address for easier maintenance across environments.
🏁 Script executed:
#!/bin/bash
# Check if the support email is defined elsewhere in the codebase as a variable
rg -i "support@codefox.net" --type=ts --type=jsLength of output: 47
Action Required: Refactor Hardcoded Support Email Address
- The support email
support@codefox.netappears only in the template file (backend/src/mail/templates/confirmation.hbs, lines 21–23) and is not defined as a configuration variable elsewhere in the codebase. - Please verify that this email is accurate and actively monitored.
- For easier maintenance across environments, consider externalizing the support email (e.g., storing it in a configuration or environment variable) rather than hardcoding it in the template.
frontend/src/components/sidebar.tsx
Outdated
| import SidebarSkeleton from './sidebar-skeleton'; | ||
| import UserSettings from './user-settings'; | ||
| import { SideBarItem } from './sidebar-item'; | ||
| import { Chat } from '@/graphql/type'; | ||
| import { EventEnum } from '../const/EventEnum'; | ||
| import { useRouter } from 'next/navigation'; |
There was a problem hiding this comment.
Remove duplicate import of useRouter.
The useRouter hook from Next.js is imported twice, which creates confusion and could lead to bugs.
import { EventEnum } from '../const/EventEnum';
import { useRouter } from 'next/navigation';
import {
SidebarContent,
SidebarGroup,
SidebarGroupContent,
SidebarTrigger,
Sidebar,
SidebarRail,
SidebarFooter,
} from './ui/sidebar';
import { ProjectContext } from './chat/code-engine/project-context';
-import { useRouter } from 'next/navigation';Also applies to: 23-23
| async updateProjectPhoto( | ||
| @GetUserIdFromToken() userId: string, | ||
| @Args('projectId', { type: () => ID }) projectId: string, | ||
| @Args('photoUrl') photoUrl: string, | ||
| @Args('input') input: UpdateProjectPhotoInput, | ||
| ): Promise<Project> { | ||
| this.logger.log( | ||
| `User ${userId} updating photo URL for project ${projectId}`, | ||
| ); | ||
| const { projectId, file } = input; | ||
| this.logger.log(`User ${userId} uploading photo for project ${projectId}`); | ||
|
|
||
| // Extract the file data | ||
| const { createReadStream, mimetype } = await file; | ||
|
|
||
| // Buffer the file content | ||
| const chunks = []; | ||
| for await (const chunk of createReadStream()) { | ||
| chunks.push(chunk); | ||
| } | ||
| const buffer = Buffer.concat(chunks); | ||
|
|
||
| // Call the service with the extracted buffer and mimetype | ||
| return this.projectService.updateProjectPhotoUrl( | ||
| userId, | ||
| projectId, | ||
| photoUrl, | ||
| buffer, | ||
| mimetype, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improved project photo update with proper file handling.
The mutation has been enhanced to handle file uploads directly through GraphQL, processing the file stream correctly. However, there are a few areas that could be improved:
- Missing error handling for file processing
- No validation for file size or type
Consider adding error handling and validation:
async updateProjectPhoto(
@GetUserIdFromToken() userId: string,
@Args('input') input: UpdateProjectPhotoInput,
): Promise<Project> {
const { projectId, file } = input;
this.logger.log(`User ${userId} uploading photo for project ${projectId}`);
// Extract the file data
const { createReadStream, mimetype } = await file;
+
+ // Validate file type
+ const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/gif'];
+ if (!allowedMimeTypes.includes(mimetype)) {
+ throw new Error(`Unsupported file type: ${mimetype}. Allowed types: ${allowedMimeTypes.join(', ')}`);
+ }
// Buffer the file content
const chunks = [];
- for await (const chunk of createReadStream()) {
- chunks.push(chunk);
- }
- const buffer = Buffer.concat(chunks);
+ try {
+ for await (const chunk of createReadStream()) {
+ chunks.push(chunk);
+ }
+ const buffer = Buffer.concat(chunks);
+
+ // Check file size (e.g., 5MB limit)
+ if (buffer.length > 5 * 1024 * 1024) {
+ throw new Error('File size exceeds 5MB limit');
+ }
+
+ // Call the service with the extracted buffer and mimetype
+ return this.projectService.updateProjectPhotoUrl(
+ userId,
+ projectId,
+ buffer,
+ mimetype,
+ );
+ } catch (error) {
+ this.logger.error(`Error processing file upload: ${error.message}`, error.stack);
+ throw new Error(`Failed to process file: ${error.message}`);
+ }
-
- // Call the service with the extracted buffer and mimetype
- return this.projectService.updateProjectPhotoUrl(
- userId,
- projectId,
- buffer,
- mimetype,
- );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async updateProjectPhoto( | |
| @GetUserIdFromToken() userId: string, | |
| @Args('projectId', { type: () => ID }) projectId: string, | |
| @Args('photoUrl') photoUrl: string, | |
| @Args('input') input: UpdateProjectPhotoInput, | |
| ): Promise<Project> { | |
| this.logger.log( | |
| `User ${userId} updating photo URL for project ${projectId}`, | |
| ); | |
| const { projectId, file } = input; | |
| this.logger.log(`User ${userId} uploading photo for project ${projectId}`); | |
| // Extract the file data | |
| const { createReadStream, mimetype } = await file; | |
| // Buffer the file content | |
| const chunks = []; | |
| for await (const chunk of createReadStream()) { | |
| chunks.push(chunk); | |
| } | |
| const buffer = Buffer.concat(chunks); | |
| // Call the service with the extracted buffer and mimetype | |
| return this.projectService.updateProjectPhotoUrl( | |
| userId, | |
| projectId, | |
| photoUrl, | |
| buffer, | |
| mimetype, | |
| ); | |
| } | |
| async updateProjectPhoto( | |
| @GetUserIdFromToken() userId: string, | |
| @Args('input') input: UpdateProjectPhotoInput, | |
| ): Promise<Project> { | |
| const { projectId, file } = input; | |
| this.logger.log(`User ${userId} uploading photo for project ${projectId}`); | |
| // Extract the file data | |
| const { createReadStream, mimetype } = await file; | |
| // Validate file type | |
| const allowedMimeTypes = ['image/jpeg', 'image/png', 'image/gif']; | |
| if (!allowedMimeTypes.includes(mimetype)) { | |
| throw new Error(`Unsupported file type: ${mimetype}. Allowed types: ${allowedMimeTypes.join(', ')}`); | |
| } | |
| // Buffer the file content | |
| const chunks = []; | |
| try { | |
| for await (const chunk of createReadStream()) { | |
| chunks.push(chunk); | |
| } | |
| const buffer = Buffer.concat(chunks); | |
| // Check file size (e.g., 5MB limit) | |
| if (buffer.length > 5 * 1024 * 1024) { | |
| throw new Error('File size exceeds 5MB limit'); | |
| } | |
| // Call the service with the extracted buffer and mimetype | |
| return this.projectService.updateProjectPhotoUrl( | |
| userId, | |
| projectId, | |
| buffer, | |
| mimetype, | |
| ); | |
| } catch (error) { | |
| this.logger.error(`Error processing file upload: ${error.message}`, error.stack); | |
| throw new Error(`Failed to process file: ${error.message}`); | |
| } | |
| } |
Co-authored-by: pengyu <pengyuchen01@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
PengyuChen01
force-pushed
the
fixing-css-new-project-in-sidebar
branch
from
5a995a8 to
67961b5
Compare
PengyuChen01
force-pushed
the
fixing-css-new-project-in-sidebar
branch
from
aef7676 to
10f0cde
Compare
Sma1lboy
changed the title
Summary by CodeRabbit