chore: minor improvements #21
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@macintushar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 46 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 (1)
WalkthroughThis pull request refactors database and UI logic across several files. In the database module, a new constant replaces a hardcoded table name and the function signature of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PagesComponent as Pages Component
participant AuthModule as getLocalUser
participant Database as getPages API
User->>PagesComponent: Opens Pages view
PagesComponent->>AuthModule: getLocalUser()
AuthModule-->>PagesComponent: Returns session (or error)
alt Valid Session
PagesComponent->>Database: getPages(user_id)
Database-->>PagesComponent: Returns pages data
else Invalid Session/Error
PagesComponent->>PagesComponent: Display error toast
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
src/db/draw.ts (1)
24-31: 🛠️ Refactor suggestionConsider adding user validation to other database operations.
While
getPagesnow validates the user, other functions likegetDrawData,setDrawData, anddeletePagedon't verify if the requesting user owns the resource.Apply this pattern to other functions to prevent unauthorized access:
export async function getDrawData(id: string): Promise<DBResponse> { + const { data: profile } = await supabase.auth.getUser(); + const user_id = profile?.user?.id; + if (!user_id) { + return { data: null, error: new Error("Unauthorized") }; + } const { data, error } = await supabase .from(DB_NAME) .select() - .eq("page_id", id); + .eq("page_id", id) + .eq("user_id", user_id); return { data, error }; }Also applies to: 33-45, 47-60, 62-69
🧹 Nitpick comments (2)
src/views/Pages.tsx (1)
53-63: Improve error handling in queryFn.While the session validation is good, the error handling could be improved:
- The error message "Something went wrong!" is not descriptive.
- Returning
nullon error might cause type issues.Consider this improved implementation:
queryFn: async() => { const user_session = await getLocalUser(); if (!user_session.error) { if (!user_session.data.session) { - toast.error("Something went wrong!"); - return { data: null, error: null }; + const error = new Error("User session not found"); + toast.error(error.message); + return { data: null, error }; } return getPages(user_session?.data.session.user?.id ?? ""); }; - return null; + toast.error(user_session.error.message); + return { data: null, error: user_session.error }; },src/views/Page.tsx (1)
146-148: Remove redundant conditional rendering.The inner condition
{isLoading ? <Loader /> : <WelcomeScreen />}is redundant as there's already an outer condition checkingisLoadingat line 108.Consider this simplified implementation:
<Excalidraw excalidrawAPI={(api) => setExcalidrawAPI(api)} renderTopRightUI={() => ( // ... existing UI code ... )} theme={theme === "dark" ? "dark" : "light"} autoFocus > - {isLoading ? <Loader /> : <WelcomeScreen />} + <WelcomeScreen /> </Excalidraw>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockbun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (3)
src/db/draw.ts(4 hunks)src/views/Page.tsx(3 hunks)src/views/Pages.tsx(2 hunks)
🔇 Additional comments (3)
src/db/draw.ts (2)
11-12: LGTM! Good use of constant.The introduction of
DB_NAMEconstant improves maintainability by centralizing the table name.
13-19:✅ Verification successful
Verify all callers of
getPagesare updated.The function signature change from
getPages()togetPages(user_id: string)is a breaking change.
🏁 Script executed:
#!/bin/bash # Description: Find all callers of getPages to verify they've been updated with the user_id parameter # Search for getPages function calls rg -A 5 'getPages\(' --type tsLength of output: 626
Verification Successful: Caller Update Confirmed
The search output confirms that the only usage of
getPagesoutside its definition is insrc/views/Pages.tsx, where the function is correctly called with auser_idparameter. No other call sites were found that still use the old signature.src/views/Page.tsx (1)
125-125: LGTM! Good use of asChild prop.The
asChildprop onTooltipTriggeris a good practice as it ensures proper accessibility and event handling.
Summary by CodeRabbit
New Features
Bug Fixes