fix: resolve GitHub profile pictures and recent projects refresh issues #2773
Conversation
…perty - Fixed camelCase vs snake_case mismatch in user metadata - GitHub OAuth provides avatar_url, not avatarUrl - Resolves profile pictures showing only initials instead of actual images
|
@binu-baiju is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUpdates local Supabase startup to exclude mailpit, logflare, and vector; changes Supabase ports and OAuth redirect URIs; updates Drizzle default DB port. Adds a protected Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Web Client
participant API as projectRouter
participant DB as Database
User->>Client: Open project page
Client->>API: trackAccess({ projectId })
note right of API: protectedProcedure verifies membership
API->>DB: UPDATE projects SET updatedAt = now() WHERE id = projectId
DB-->>API: OK
API-->>Client: { success: true }
note right of Client: invalidate project list cache -> refetch recent projects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 3
🧹 Nitpick comments (6)
apps/web/client/src/server/api/routers/user/user.ts (1)
25-26: Harden avatar derivation with broader fallbacksSome IdPs store avatar under different keys. Add a short fallback chain to be resilient across providers.
Apply this diff:
- avatarUrl: user.avatarUrl ?? authUser.user_metadata.avatar_url, + avatarUrl: + user.avatarUrl + ?? authUser.user_metadata?.avatar_url + ?? authUser.user_metadata?.avatarUrl + ?? authUser.user_metadata?.picture + ?? null,- avatarUrl: input.avatarUrl ?? authUser.user_metadata.avatar_url, + avatarUrl: + input.avatarUrl + ?? authUser.user_metadata?.avatar_url + ?? authUser.user_metadata?.avatarUrl + ?? authUser.user_metadata?.picture + ?? null,Also applies to: 59-60
apps/backend/supabase/config.toml (1)
10-15: Consider adding 127.0.0.1 app URLs for local dev flexibilityIf you sometimes run the web app at 127.0.0.1:3000 (not localhost), add it to allowed redirects to avoid blocked post-auth redirects.
Apply this diff:
additional_redirect_urls = [ "http://localhost:3000", "http://localhost:3000/auth/callback", + "http://127.0.0.1:3000", + "http://127.0.0.1:3000/auth/callback", ]apps/backend/package.json (1)
5-5: Excluding logflare/vector may conflict with analytics settingsYou’re starting Supabase without logflare/vector, while
analytics.enabled = trueremains in config.toml. Validate local startup health and decide whether to:
- keep exclude flags and set
[analytics] enabled = false, or- include the services during
supabase start.apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (1)
39-44: Surface mutation errors to the userAdd
onErrorto toast/log failures so access-tracking issues aren’t silent.Apply this diff:
- const { mutateAsync: trackProjectAccess } = api.project.trackAccess.useMutation({ - onSuccess: () => { + const { mutateAsync: trackProjectAccess } = api.project.trackAccess.useMutation({ + onSuccess: () => { // Invalidate project list to refresh recent projects apiUtils.project.list.invalidate(); - }, + }, + onError: (err) => { + console.error('Failed to track project access', err); + toast.message('Could not update recent projects', { + description: err instanceof Error ? err.message : 'Unknown error', + }); + }, });apps/web/client/src/server/api/routers/project/project.ts (2)
164-181: Sort in SQL by last access (or updatedAt) to reduce app-side work and align semantics.Move sorting to the query and, if adopting lastAccessedAt, order by it (fallback to projects.updatedAt).
Example (Drizzle-style; adjust imports for desc/sql):
- const fetchedUserProjects = await ctx.db.query.userProjects.findMany({ - where: input?.excludeProjectId ? and( - eq(userProjects.userId, ctx.user.id), - ne(userProjects.projectId, input.excludeProjectId), - ) : eq(userProjects.userId, ctx.user.id), - with: { - project: true, - }, - limit: input?.limit, - }); - return fetchedUserProjects.map((userProject) => toProject(userProject.project)).sort((a, b) => new Date(b.metadata.updatedAt).getTime() - new Date(a.metadata.updatedAt).getTime()); + const rows = await ctx.db + .select() + .from(userProjects) + .leftJoin(projects, eq(userProjects.projectId, projects.id)) + .where( + input?.excludeProjectId + ? and(eq(userProjects.userId, ctx.user.id), ne(userProjects.projectId, input.excludeProjectId)) + : eq(userProjects.userId, ctx.user.id), + ) + // If lastAccessedAt exists, prefer it; otherwise order by projects.updatedAt + .orderBy(sql`COALESCE(${userProjects.lastAccessedAt}, ${projects.updatedAt}) DESC`) + .limit(input?.limit ?? undefined); + return rows.map((r) => toProject(r.projects));Additional imports (top of file):
import { desc, sql } from 'drizzle-orm';
406-412: Track access per user instead of mutating project.updatedAt
updatedAt should reflect content changes; using it for “recent” access affects all members and conflates semantics. Add alastAccessedAttimestamp to youruser_projectstable and update that here:await ctx.db .update(userProjects) .set({ lastAccessedAt: new Date() }) .where(and( eq(userProjects.userId, ctx.user.id), eq(userProjects.projectId, input.projectId), ));You’ll need to add the
lastAccessedAtcolumn in the Drizzle schema and create a migration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/backend/package.json(1 hunks)apps/backend/supabase/config.toml(2 hunks)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(1 hunks)apps/web/client/src/server/api/routers/project/project.ts(1 hunks)apps/web/client/src/server/api/routers/user/user.ts(2 hunks)packages/db/drizzle.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (1)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/server/api/routers/project/project.ts (2)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(130-149)packages/db/src/schema/project/project.ts (1)
projects(12-31)
🔇 Additional comments (5)
apps/web/client/src/server/api/routers/user/user.ts (1)
25-26: Good fix: use snake_caseavatar_urlfrom Supabase metadataThis resolves the GitHub avatar mismatch where
avatarUrlwas undefined.apps/backend/supabase/config.toml (2)
5-5: Ports alignment looks consistent with Drizzle configAPI=55321, DB=55432, Studio=55323. Matches packages/db/drizzle.config.ts.
Also applies to: 19-19, 22-22
28-29: Verify OAuth app callback URLs are updated to 127.0.0.1:55321Ensure GitHub and Google developer console apps include the exact callback:
A mismatch will block local sign-in.
Also applies to: 34-35
packages/db/drizzle.config.ts (1)
3-3: Drizzle fallback DB URL updated to 55432 — matches Supabase DB portThis keeps local migrations consistent with the new Supabase config.
apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx (1)
39-44: Nice: track access + invalidate list to refresh recentsThis should surface the accessed project at the top without manual refresh.
| // Track project access to update "recent projects" list | ||
| trackProjectAccess({ projectId: project.id }).catch(console.error); | ||
| } | ||
| }, [project]); | ||
| }, [project, trackProjectAccess]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against duplicate tracking on re-renders
Depending on the identity stability of trackProjectAccess, the effect can fire more than once per project. Guard with a ref keyed by project.id and narrow the dependency to the project id.
Apply this diff in the nearest appropriate scopes:
+import { useEffect, useRef, useState } from 'react';- useEffect(() => {
- if (project) {
- startSandbox(project);
- editorEngine.screenshot.lastScreenshotAt = project.metadata.updatedPreviewImgAt;
- // Track project access to update "recent projects" list
- trackProjectAccess({ projectId: project.id }).catch(console.error);
- }
- }, [project, trackProjectAccess]);
+ const lastTrackedProjectIdRef = useRef<string | null>(null);
+ useEffect(() => {
+ if (project) {
+ startSandbox(project);
+ editorEngine.screenshot.lastScreenshotAt = project.metadata.updatedPreviewImgAt;
+ // Track project access to update "recent projects" list (once per project id)
+ if (lastTrackedProjectIdRef.current !== project.id) {
+ lastTrackedProjectIdRef.current = project.id;
+ void trackProjectAccess({ projectId: project.id }).catch(console.error);
+ }
+ }
+ }, [project?.id]);📝 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.
| // Track project access to update "recent projects" list | |
| trackProjectAccess({ projectId: project.id }).catch(console.error); | |
| } | |
| }, [project]); | |
| }, [project, trackProjectAccess]); | |
| // At the top of the file, ensure you have React hooks imported: | |
| import React, { useEffect, useRef, useState } from 'react'; | |
| // …inside your component… | |
| - useEffect(() => { | |
| - if (project) { | |
| - startSandbox(project); | |
| - editorEngine.screenshot.lastScreenshotAt = project.metadata.updatedPreviewImgAt; | |
| - // Track project access to update "recent projects" list | |
| - trackProjectAccess({ projectId: project.id }).catch(console.error); | |
| - } | |
| // Ref to remember which project ID we've already tracked | |
| const lastTrackedProjectIdRef = useRef<string | null>(null); | |
| useEffect(() => { | |
| if (project) { | |
| startSandbox(project); | |
| editorEngine.screenshot.lastScreenshotAt = project.metadata.updatedPreviewImgAt; | |
| // Track project access to update "recent projects" list (once per project id) | |
| if (lastTrackedProjectIdRef.current !== project.id) { | |
| lastTrackedProjectIdRef.current = project.id; | |
| void trackProjectAccess({ projectId: project.id }).catch(console.error); | |
| } | |
| } | |
| }, [project?.id]); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/client/src/server/api/routers/project/project.ts (1)
407-430: IDOR closed with membership check + UUID validation; consider atomic update/returning and access-specific timestamp.Well done on adding protectedProcedure, z.string().uuid(), and the membership check. To harden further and improve UX semantics:
- Make the update atomic and verify one row changed (handles race where project is deleted between check and update) and use DB time to avoid clock skew.
- Optional: avoid overloading updatedAt for “recently accessed.” Add a lastAccessedAt column and sort by it to prevent confusing modification history.
Apply within these lines:
- await ctx.db.update(projects).set({ - updatedAt: new Date(), - }).where(eq(projects.id, input.projectId)); - - return { success: true }; + const updated = await ctx.db + .update(projects) + .set({ updatedAt: sql`now()` }) + .where(eq(projects.id, input.projectId)) + .returning({ id: projects.id }); + + if (updated.length === 0) { + // Project was likely removed between membership check and update + throw new TRPCError({ code: 'NOT_FOUND' }); + } + return { success: true };Outside the selected range, add sql to the drizzle import:
import { and, eq, ne, sql } from 'drizzle-orm';If you opt for a dedicated access timestamp later, I can propose the migration + list query change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/client/src/server/api/routers/project/project.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/client/src/server/api/routers/project/project.ts (3)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(130-149)packages/db/src/schema/user/user-project.ts (1)
userProjects(10-23)packages/db/src/schema/project/project.ts (1)
projects(12-31)
🔇 Additional comments (1)
apps/web/client/src/server/api/routers/project/project.ts (1)
3-3: Good import.Importing TRPCError is appropriate and used below.
| displayName: user.displayName ?? displayName, | ||
| email: user.email ?? authUser.email, | ||
| avatarUrl: user.avatarUrl ?? authUser.user_metadata.avatarUrl, | ||
| avatarUrl: user.avatarUrl ?? authUser.user_metadata.avatar_url, |
There was a problem hiding this comment.
@binu-baiju Thanks for the PR! There seems to be a lot of your personal config changes. I think this is the only thing we need. Could you remove the rest? Thanks
|
Closing this PR to create a cleaner version without config changes as requested by @Kitenite. New clean PR: [https://github.com//pull/2778] |
2 participants
Description
Fixed two critical bugs in the Onlook web application that were affecting user experience:
GitHub Profile Picture Bug: Users' GitHub profile pictures were not displaying correctly, showing only initials ("bb") instead of their actual GitHub avatar after OAuth login.
Recent Projects Refresh Bug: The recent projects list was not updating when users accessed projects, meaning recently opened projects did not move to the top of the list as expected.
Root Causes Identified:
avatarUrl(camelCase) andavatar_url(snake_case) from GitHub OAuth metadataSolutions Implemented:
apps/web/client/src/server/api/routers/user/user.tsto correctly mapauthUser.user_metadata.avatar_urltrackAccessmutation and integrated it with project loading to update timestamps and refresh UI cacheRelated Issues
Fixes GitHub profile picture display issue
Fixes recent projects list refresh issue
Type of Change
Testing
Manual Testing Performed:
GitHub Profile Pictures:
Recent Projects Refresh:
Environment:
Screenshots (if applicable)
Additional Notes
Technical Details:
bun formatfor code formatting consistencybun lintto ensure code quality standardsDevelopment Process:
Files Modified:
apps/web/client/src/server/api/routers/user/user.ts(2 lines changed)apps/web/client/src/server/api/routers/project/project.ts(added trackAccess mutation)apps/web/client/src/app/project/[id]/_hooks/use-start-project.tsx(integrated tracking)Both issues were affecting real user experience, and these fixes provide immediate value to the Onlook community.
Important
Fix GitHub avatar display and project list refresh, update Supabase config and database URL.
authUser.user_metadata.avatar_urlinuser.ts.trackAccessmutation inproject.tsto update project timestamps and refresh UI cache.config.toml.DEFAULT_DATABASE_URLindrizzle.config.tsto new port.onlook-preload-script.js.This description was created by
for 1189d05. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Chores