-
Notifications
You must be signed in to change notification settings - Fork 3
fix(backend): backend project rate limit and feat(frontend) project limit pop up #194
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request updates the project creation flow across backend and frontend components. In the backend, the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant PS as ProjectService
participant R as Repository
C->>PS: Call createProject
PS->>PS: Instantiate Project entity with properties
PS->>PS: Process project packages (try-catch)
alt Package processing error
PS->>PS: Log error and set empty packages
end
PS->>R: Save Project and log ID
PS->>PS: Call createProjectInBackground(savedProject)
PS->>PS: Retrieve project path in background
alt Path retrieved
PS->>R: Update and save project with new path
PS->>PS: Log update
else
PS->>PS: Log error for missing path
end
sequenceDiagram
participant U as User
participant H as HomePage
participant PC as ProjectContext
U->>H: Submit project creation prompt
H->>PC: Call createProjectFromPrompt(prompt, isPublic, model)
PC-->>H: Return CreateProjectResult (success or rate limit)
alt Rate Limit Reached
H->>H: Set state with rate limit flag and limit number
H->>U: Display RateLimitModal
else Success
H->>H: Clear message and navigate to chat page
end
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
frontend/src/components/chat/code-engine/project-context.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/components/rate-limit-modal.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/app/(main)/page.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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 (
|
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: 6
🧹 Nitpick comments (1)
frontend/src/components/chat/code-engine/project-context.tsx (1)
145-146: Fix React Hook cleanup dependencies warningThe effect cleanup function might use stale ref values. Store the ref values in variables inside the effect.
useEffect(() => { + const chatProjectCacheRef = chatProjectCache.current; + const pendingOperationsRef = pendingOperations.current; return () => { isMounted.current = false; - chatProjectCache.current.clear(); - pendingOperations.current.clear(); + chatProjectCacheRef.clear(); + pendingOperationsRef.clear(); }; }, []);🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 145-145: The ref value 'chatProjectCache.current' will likely have changed by the time this effect cleanup function runs. If this ref points to a node rendered by React, copy 'chatProjectCache.current' to a variable inside the effect, and use that variable in the cleanup function react-hooks/exhaustive-deps
[warning] 146-146: The ref value 'pendingOperations.current' will likely have changed by the time this effect cleanup function runs. If this ref points to a node rendered by React, copy 'pendingOperations.current' to a variable inside the effect, and use that variable in the cleanup function react-hooks/exhaustive-deps
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/project/project.service.ts(3 hunks)frontend/src/app/(main)/page.tsx(2 hunks)frontend/src/components/chat/code-engine/project-context.tsx(6 hunks)frontend/src/components/rate-limit-modal.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
backend/src/project/project.service.ts (1)
frontend/src/graphql/type.tsx (1) (1)
Project(255:277)
frontend/src/components/chat/code-engine/project-context.tsx (1)
frontend/src/app/log/logger.ts (1) (1)
logger(3:19)
🪛 GitHub Actions: autofix.ci
frontend/src/app/(main)/page.tsx
[warning] 56-56: Unexpected console statement no-console
backend/src/project/project.service.ts
[warning] 393-393: 'error' is defined but never used unused-imports/no-unused-vars
frontend/src/components/rate-limit-modal.tsx
[error] 35-35: 'can be escaped with', ‘, ', ’` react/no-unescaped-entities
frontend/src/components/chat/code-engine/project-context.tsx
[warning] 145-145: The ref value 'chatProjectCache.current' will likely have changed by the time this effect cleanup function runs. If this ref points to a node rendered by React, copy 'chatProjectCache.current' to a variable inside the effect, and use that variable in the cleanup function react-hooks/exhaustive-deps
[warning] 146-146: The ref value 'pendingOperations.current' will likely have changed by the time this effect cleanup function runs. If this ref points to a node rendered by React, copy 'pendingOperations.current' to a variable inside the effect, and use that variable in the cleanup function react-hooks/exhaustive-deps
[warning] 276-276: React Hook useEffect has a missing dependency: 'refetch'. Either include it or remove the dependency array react-hooks/exhaustive-deps
[warning] 411-411: 'data' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars
[warning] 412-412: Unexpected console statement no-console
🪛 GitHub Check: autofix
frontend/src/components/rate-limit-modal.tsx
[failure] 35-35:
' can be escaped with ', ‘, ', ’
🔇 Additional comments (14)
frontend/src/components/rate-limit-modal.tsx (4)
1-19: Well-structured component interfaceThe props interface is clear and provides good flexibility with the optional limit parameter and required callbacks.
30-43: LGTM: Clear heading and descriptionThe dialog header provides clear information about the rate limit situation with appropriate icon and description.
🧰 Tools
🪛 GitHub Check: autofix
[failure] 35-35:
'can be escaped with',‘,',’🪛 GitHub Actions: autofix.ci
[error] 35-35: '
can be escaped with',‘,',’` react/no-unescaped-entities
44-50: Good use of color-coding for emphasized informationThe amber-colored alert box effectively highlights the alternative options for the user.
52-54: Simple and clear action buttonThe footer button provides a straightforward way for users to acknowledge the message.
frontend/src/app/(main)/page.tsx (2)
10-14: Good import organizationThe imports are well-organized with the new type import properly grouped.
19-19: Good component importThe RateLimitModal import is correctly added.
backend/src/project/project.service.ts (4)
198-220: Good structured project creation approachThe code now creates a proper Project entity with all required properties before saving, which is a better practice than directly using the repository.save() with an object literal.
207-215: Improved error handling for package processingAdding a try-catch block for package processing allows the project creation to continue even if package transformation fails, with proper logging.
222-227: Better parameter passing for background processingUsing the saved project entity instead of just the user ID allows for proper updates to the project in the background process.
259-272: Good error handling and logging for project path updatesThe code properly handles the project path update with clear logging for both success and failure cases.
frontend/src/components/chat/code-engine/project-context.tsx (4)
26-29: Well-defined result type for project creationThe new
CreateProjectResulttype provides a clear contract for what can be returned from the project creation process, making error handling more robust.
110-112: Good use of refs for rate limit trackingUsing refs to track rate limit status is appropriate as it doesn't need to trigger re-renders but needs to be accessible across function calls.
416-430: Improved error handling for rate limitsThe enhanced error handling properly identifies and processes rate limit errors from GraphQL, setting the appropriate refs.
739-756: Well-structured result handlingThe code now properly checks for rate limits and returns structured results that match the CreateProjectResult type, improving error handling throughout the application.
| export const RateLimitModal = ({ | ||
| isOpen, | ||
| onClose, | ||
| limit = 3, | ||
| }: RateLimitModalProps) => { | ||
| return ( | ||
| <Dialog open={isOpen} onOpenChange={(open) => !open && onClose()}> | ||
| <DialogContent className="sm:max-w-md"> | ||
| <DialogHeader> | ||
| <div className="flex items-center gap-2"> | ||
| <AlertTriangle className="h-5 w-5 text-amber-500" /> | ||
| <DialogTitle>Daily Limit Reached</DialogTitle> | ||
| </div> | ||
| <DialogDescription> | ||
| You've reached your daily project creation limit. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
|
|
||
| <div className="py-4"> | ||
| <p className="mb-4"> | ||
| Your current plan allows creating up to {limit} projects per day. | ||
| This limit resets at midnight UTC. | ||
| </p> | ||
| <div className="bg-amber-50 dark:bg-amber-950/30 p-3 rounded-md border border-amber-200 dark:border-amber-800"> | ||
| <p className="text-sm text-amber-800 dark:text-amber-300"> | ||
| Try editing or reusing one of your existing projects, or wait | ||
| until tomorrow to create a new one. | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <DialogFooter> | ||
| <Button onClick={onClose}>I understand</Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ); | ||
| }; |
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.
Fix unescaped apostrophe in JSX content
The JSX content contains an unescaped apostrophe that's causing a pipeline failure.
- You've reached your daily project creation limit.
+ You've reached your daily project creation limit.📝 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.
| export const RateLimitModal = ({ | |
| isOpen, | |
| onClose, | |
| limit = 3, | |
| }: RateLimitModalProps) => { | |
| return ( | |
| <Dialog open={isOpen} onOpenChange={(open) => !open && onClose()}> | |
| <DialogContent className="sm:max-w-md"> | |
| <DialogHeader> | |
| <div className="flex items-center gap-2"> | |
| <AlertTriangle className="h-5 w-5 text-amber-500" /> | |
| <DialogTitle>Daily Limit Reached</DialogTitle> | |
| </div> | |
| <DialogDescription> | |
| You've reached your daily project creation limit. | |
| </DialogDescription> | |
| </DialogHeader> | |
| <div className="py-4"> | |
| <p className="mb-4"> | |
| Your current plan allows creating up to {limit} projects per day. | |
| This limit resets at midnight UTC. | |
| </p> | |
| <div className="bg-amber-50 dark:bg-amber-950/30 p-3 rounded-md border border-amber-200 dark:border-amber-800"> | |
| <p className="text-sm text-amber-800 dark:text-amber-300"> | |
| Try editing or reusing one of your existing projects, or wait | |
| until tomorrow to create a new one. | |
| </p> | |
| </div> | |
| </div> | |
| <DialogFooter> | |
| <Button onClick={onClose}>I understand</Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> | |
| ); | |
| }; | |
| export const RateLimitModal = ({ | |
| isOpen, | |
| onClose, | |
| limit = 3, | |
| }: RateLimitModalProps) => { | |
| return ( | |
| <Dialog open={isOpen} onOpenChange={(open) => !open && onClose()}> | |
| <DialogContent className="sm:max-w-md"> | |
| <DialogHeader> | |
| <div className="flex items-center gap-2"> | |
| <AlertTriangle className="h-5 w-5 text-amber-500" /> | |
| <DialogTitle>Daily Limit Reached</DialogTitle> | |
| </div> | |
| <DialogDescription> | |
| You've reached your daily project creation limit. | |
| </DialogDescription> | |
| </DialogHeader> | |
| <div className="py-4"> | |
| <p className="mb-4"> | |
| Your current plan allows creating up to {limit} projects per day. | |
| This limit resets at midnight UTC. | |
| </p> | |
| <div className="bg-amber-50 dark:bg-amber-950/30 p-3 rounded-md border border-amber-200 dark:border-amber-800"> | |
| <p className="text-sm text-amber-800 dark:text-amber-300"> | |
| Try editing or reusing one of your existing projects, or wait | |
| until tomorrow to create a new one. | |
| </p> | |
| </div> | |
| </div> | |
| <DialogFooter> | |
| <Button onClick={onClose}>I understand</Button> | |
| </DialogFooter> | |
| </DialogContent> | |
| </Dialog> | |
| ); | |
| }; |
🧰 Tools
🪛 GitHub Check: autofix
[failure] 35-35:
' can be escaped with ', ‘, ', ’
🪛 GitHub Actions: autofix.ci
[error] 35-35: 'can be escaped with', ‘, ', ’` react/no-unescaped-entities
| const [showRateLimitModal, setShowRateLimitModal] = useState(false); | ||
| const [rateLimiNumber, setRateLimiNumber] = useState<number | undefined>( | ||
| undefined | ||
| ); |
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.
🛠️ Refactor suggestion
Fix typo in state variable name
There's a typo in the state variable name which should be corrected for consistency.
- const [rateLimiNumber, setRateLimiNumber] = useState<number | undefined>(
+ const [rateLimitNumber, setRateLimitNumber] = useState<number | undefined>(
undefined
);📝 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 [showRateLimitModal, setShowRateLimitModal] = useState(false); | |
| const [rateLimiNumber, setRateLimiNumber] = useState<number | undefined>( | |
| undefined | |
| ); | |
| const [showRateLimitModal, setShowRateLimitModal] = useState(false); | |
| const [rateLimitNumber, setRateLimitNumber] = useState<number | undefined>( | |
| undefined | |
| ); |
| <RateLimitModal | ||
| isOpen={showRateLimitModal} | ||
| limit={rateLimiNumber ?? 3} | ||
| onClose={() => setShowRateLimitModal(false)} | ||
| /> |
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.
🛠️ Refactor suggestion
Fix variable name in component props
Fix the variable name in the RateLimitModal component props to match the state variable name change.
<RateLimitModal
isOpen={showRateLimitModal}
- limit={rateLimiNumber ?? 3}
+ limit={rateLimitNumber ?? 3}
onClose={() => setShowRateLimitModal(false)}
/>📝 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.
| <RateLimitModal | |
| isOpen={showRateLimitModal} | |
| limit={rateLimiNumber ?? 3} | |
| onClose={() => setShowRateLimitModal(false)} | |
| /> | |
| <RateLimitModal | |
| isOpen={showRateLimitModal} | |
| limit={rateLimitNumber ?? 3} | |
| onClose={() => setShowRateLimitModal(false)} | |
| /> |
| } else if (isRateLimitResult(result) && result.rateLimit) { | ||
| setShowRateLimitModal(true); | ||
| setRateLimiNumber(result.limitNumber); | ||
| console.log('Rate limit reached ' + result.limitNumber); | ||
| } |
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.
Remove console.log and fix state setter name
There's a console.log that should be removed, and the state setter name needs to be fixed.
} else if (isRateLimitResult(result) && result.rateLimit) {
setShowRateLimitModal(true);
- setRateLimiNumber(result.limitNumber);
- console.log('Rate limit reached ' + result.limitNumber);
+ setRateLimitNumber(result.limitNumber);
}📝 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.
| } else if (isRateLimitResult(result) && result.rateLimit) { | |
| setShowRateLimitModal(true); | |
| setRateLimiNumber(result.limitNumber); | |
| console.log('Rate limit reached ' + result.limitNumber); | |
| } | |
| } else if (isRateLimitResult(result) && result.rateLimit) { | |
| setShowRateLimitModal(true); | |
| setRateLimitNumber(result.limitNumber); | |
| } |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 56-56: Unexpected console statement no-console
| function isRateLimitResult( | ||
| result: CreateProjectResult | ||
| ): result is { success: false; rateLimit?: boolean; limitNumber?: number } { | ||
| return result.success === false; | ||
| } |
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.
🛠️ Refactor suggestion
Improve type guard function
The isRateLimitResult type guard is checking the success property, but does not specifically check for rate limit conditions, which can lead to incorrect behavior.
function isRateLimitResult(
result: CreateProjectResult
): result is { success: false; rateLimit?: boolean; limitNumber?: number } {
- return result.success === false;
+ return result.success === false && result.rateLimit === true;
}📝 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.
| function isRateLimitResult( | |
| result: CreateProjectResult | |
| ): result is { success: false; rateLimit?: boolean; limitNumber?: number } { | |
| return result.success === false; | |
| } | |
| function isRateLimitResult( | |
| result: CreateProjectResult | |
| ): result is { success: false; rateLimit?: boolean; limitNumber?: number } { | |
| - return result.success === false; | |
| + return result.success === false && result.rateLimit === true; | |
| } |
| // Create project mutation | ||
| const [createProject] = useMutation(CREATE_PROJECT, { | ||
| onCompleted: (data) => { | ||
| console.log('createProject have yes'); |
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.
Remove console.log statement
There's an unnecessary console.log statement that should be removed.
- console.log('createProject have yes');📝 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.
| console.log('createProject have yes'); |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 412-412: Unexpected console statement no-console
|
Hi! I'm the It looks like you correctly set up a CI job that uses the autofix.ci GitHub Action, but the autofix.ci GitHub App has not been installed for this repository. This means that autofix.ci unfortunately does not have the permissions to fix this pull request. If you are the repository owner, please install the app and then restart the CI workflow! 😃 |
Summary by CodeRabbit