Feat/#37/search component and function#48
Conversation
…ub.com/pich-reamrachna/social-media into feat/#37/search-component-and-function
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds route-aware session loading in the server hook with dev diagnostics, enables a 5-minute cookie cache in auth config, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Hook as Server Hook\n(src/hooks.server.ts)
participant AuthAPI as Auth API\n(auth.api.getSession)
participant Svelte as SvelteKit Handler
Client->>Hook: HTTP request
Hook->>Hook: normalizeUrl -> pathname, label, cookie header
alt shouldLoadSession(pathname)
Hook->>AuthAPI: auth.api.getSession(request)
AuthAPI-->>Hook: session (or null)
Hook->>Hook: set event.locals.session / .user if session
else skip session
Hook-->>Hook: skip getSession (dev log)
end
Hook->>Svelte: call svelteKitHandler(event)
Svelte-->>Hook: Response
Hook->>Hook: dev logs (timings, set-cookie presence)
Hook-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/routes/profile/[username]/+page.svelte (1)
49-77: Solid lazy-loading implementation, but failures are silent.The loading guard pattern (line 50) and state management are well-implemented. However, on failure (line 72-73), the user receives no feedback—the tab simply shows empty. Consider adding a toast or error indicator:
Optional: Add user feedback on failure
} catch { profile_liked_posts = [] + // Optionally show error feedback to user + console.error('Failed to load liked posts') } finally { is_liked_posts_loading = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/profile/`[username]/+page.svelte around lines 49 - 77, The ensure_liked_posts_loaded function currently swallows all errors and leaves the UI silent; modify it to surface failures by setting an error state and/or invoking the app's toast/notification utility when the fetch or deserialization fails. Specifically, inside the catch block for ensure_liked_posts_loaded (which currently just sets profile_liked_posts = []), set a boolean like liked_posts_load_error (or call the existing toast function) and include a short, user-facing message; keep the existing guards (has_loaded_liked_posts, is_liked_posts_loading) and finally block unchanged so loading state is still cleared.src/routes/home/+page.server.ts (1)
250-255: Same stale count issue as profile route; consider DRY extraction.The returned
likesvalue is calculated from the pre-updatecurrent_post.likeCount, not the actual post-update value. Additionally, thistoggleLikelogic is nearly identical tosrc/routes/profile/[username]/+page.server.ts. Consider extracting to a shared utility in$lib/server/to reduce duplication.Proposed shared utility location
// $lib/server/actions/toggle-like.ts export async function toggleLike(userId: string, postId: string) { // Shared implementation with RETURNING clause }Then import and use in both route actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/home/`+page.server.ts around lines 250 - 255, The likes value is computed from the stale pre-update current_post.likeCount and duplicates logic used in the profile route; change the route action to call a shared server utility (e.g. export async function toggleLike(userId, postId) in $lib/server/actions/toggle-like.ts) that performs the DB toggle and returns the actual updated likeCount and the new liked state (use a DB UPDATE/INSERT/DELETE with RETURNING to get the new count), then replace the inline calculation of is_liked and likes (which currently uses existing_like and current_post.likeCount) with the values returned from toggleLike and import that utility into both +page.server.ts files to remove duplication.src/hooks.server.ts (1)
6-12: Share the auth-route matcher withsrc/routes/+layout.server.ts.
AUTH_ROUTES,PROTECTED_ROUTE_PREFIXES, andis_protected_route()are now a second copy of the auth-route policy already defined insrc/routes/+layout.server.ts, Lines 4-12. If one side changes without the other, hooks can skip session hydration while the layout still enforces redirects, which sends authenticated users back to/login. Export a single matcher/constants module and reuse it in both places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks.server.ts` around lines 6 - 12, The auth-route logic is duplicated—AUTH_ROUTES, PROTECTED_ROUTE_PREFIXES, and is_protected_route()—so extract those three symbols into a single shared module and export them, then replace the local definitions in both places with imports of AUTH_ROUTES, PROTECTED_ROUTE_PREFIXES, and is_protected_route; ensure the exported is_protected_route function preserves the same semantics (checking equality or startsWith(`${prefix}/`) for each prefix) and update any import sites to use the shared module so both the hook and the layout use the exact same matcher.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks.server.ts`:
- Around line 28-33: Normalize the pathname before matching auth routes: replace
direct uses of event.url.pathname in the should_load_session check with a
normalized path (use normalizeUrl from `@sveltejs/kit`) so that SvelteKit internal
suffixes like "__data.json" are stripped; update the logic around
should_load_session, AUTH_ROUTES lookups and any callsites that rely on
event.url.pathname (e.g., where getSession() is conditionally invoked) to use
the normalized pathname to ensure exact matches (e.g., "/login") succeed and
locals.user is populated for guard checks.
In `@src/routes/profile/`[username]/+page.server.ts:
- Around line 222-230: The code reads current_post via db.query.post.findFirst
and uses current_post.likeCount later to compute the returned likes, which can
be stale under concurrent updates; instead perform the like/unlike update using
a database update call with a RETURNING clause to fetch the post's updated
likeCount (e.g., replace the separate findFirst + later computation with a
single update that returns likeCount), then use that returned likeCount in the
response; reference the existing symbols current_post, db.query.post.findFirst,
post_id and likeCount when locating and replacing the logic so the response uses
the updated DB value rather than the pre-read value.
---
Nitpick comments:
In `@src/hooks.server.ts`:
- Around line 6-12: The auth-route logic is duplicated—AUTH_ROUTES,
PROTECTED_ROUTE_PREFIXES, and is_protected_route()—so extract those three
symbols into a single shared module and export them, then replace the local
definitions in both places with imports of AUTH_ROUTES,
PROTECTED_ROUTE_PREFIXES, and is_protected_route; ensure the exported
is_protected_route function preserves the same semantics (checking equality or
startsWith(`${prefix}/`) for each prefix) and update any import sites to use the
shared module so both the hook and the layout use the exact same matcher.
In `@src/routes/home/`+page.server.ts:
- Around line 250-255: The likes value is computed from the stale pre-update
current_post.likeCount and duplicates logic used in the profile route; change
the route action to call a shared server utility (e.g. export async function
toggleLike(userId, postId) in $lib/server/actions/toggle-like.ts) that performs
the DB toggle and returns the actual updated likeCount and the new liked state
(use a DB UPDATE/INSERT/DELETE with RETURNING to get the new count), then
replace the inline calculation of is_liked and likes (which currently uses
existing_like and current_post.likeCount) with the values returned from
toggleLike and import that utility into both +page.server.ts files to remove
duplication.
In `@src/routes/profile/`[username]/+page.svelte:
- Around line 49-77: The ensure_liked_posts_loaded function currently swallows
all errors and leaves the UI silent; modify it to surface failures by setting an
error state and/or invoking the app's toast/notification utility when the fetch
or deserialization fails. Specifically, inside the catch block for
ensure_liked_posts_loaded (which currently just sets profile_liked_posts = []),
set a boolean like liked_posts_load_error (or call the existing toast function)
and include a short, user-facing message; keep the existing guards
(has_loaded_liked_posts, is_liked_posts_loading) and finally block unchanged so
loading state is still cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69321eaf-8c96-4f55-91cc-023a5635e924
📒 Files selected for processing (7)
src/hooks.server.tssrc/lib/server/auth.tssrc/lib/server/db/post.tssrc/routes/home/+page.server.tssrc/routes/home/+page.sveltesrc/routes/profile/[username]/+page.server.tssrc/routes/profile/[username]/+page.svelte
| const current_post = await db.query.post.findFirst({ | ||
| where: eq(post.id, post_id), | ||
| columns: { | ||
| likeCount: true | ||
| } | ||
| }) | ||
| if (!current_post) { | ||
| return fail(404, { message: 'Post not found' }) | ||
| } |
There was a problem hiding this comment.
Returned likes count may be stale under concurrent updates.
The current_post.likeCount is read at line 222-227 before the update occurs. The returned likes (line 254) is calculated from this pre-read value, not the actual database value after the update. Under concurrent like/unlike operations, the client receives a potentially incorrect count.
Consider using RETURNING to get the actual post-update value:
Proposed fix using RETURNING clause
if (existing_like) {
await db.delete(like).where(and(eq(like.postId, post_id), eq(like.userId, viewer.id)))
- await db
+ const [updated] = await db
.update(post)
.set({
likeCount: sql`greatest(${post.likeCount} - 1, 0)`
})
.where(eq(post.id, post_id))
+ .returning({ likeCount: post.likeCount })
+ return { success: true, is_liked: false, likes: updated?.likeCount ?? 0 }
} else {
await db.insert(like).values({ userId: viewer.id, postId: post_id })
- await db
+ const [updated] = await db
.update(post)
.set({
likeCount: sql`${post.likeCount} + 1`
})
.where(eq(post.id, post_id))
+ .returning({ likeCount: post.likeCount })
+ return { success: true, is_liked: true, likes: updated?.likeCount ?? 0 }
}
- } catch {
+ } catch (err) {
+ console.error('toggleLike database error:', err)
return fail(500, { message: 'Failed to update like' })
}
-
- return { success: true }Also applies to: 250-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/profile/`[username]/+page.server.ts around lines 222 - 230, The
code reads current_post via db.query.post.findFirst and uses
current_post.likeCount later to compute the returned likes, which can be stale
under concurrent updates; instead perform the like/unlike update using a
database update call with a RETURNING clause to fetch the post's updated
likeCount (e.g., replace the separate findFirst + later computation with a
single update that returns likeCount), then use that returned likeCount in the
response; reference the existing symbols current_post, db.query.post.findFirst,
post_id and likeCount when locating and replacing the logic so the response uses
the updated DB value rather than the pre-read value.
|
The main problems were:
So the short answer is:
|
Summary by CodeRabbit
New Features
Performance
Bug Fixes
Improvements