Uh oh!
There was an error while loading. Please reload this page.
feat(workspaces): add workspace logo upload - #4136
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Introduces a new uploads context Extends the workspace API and client types/mutations to persist Reviewed by Cursor Bugbot for commit 473785c. Configure here. |
Greptile SummaryThis PR adds workspace logo upload support via a right-click context menu, introduces a dedicated The previous thread concerns — storage cleanup parity with profile-pictures and URL validation — have been acknowledged/addressed. All remaining findings are P2. Confidence Score: 5/5Safe to merge — all findings are P2 style/edge-case concerns with no blocking bugs or security issues Server-side authorization is correct (admin-only upload + PATCH, URL validation, public serve for logos). The DB migration is additive and backwards-compatible. Prior review concerns (storage cleanup parity, arbitrary URL injection) have been acknowledged or addressed. The two remaining findings are code duplication and an edge-case workspaceIdRef drift that only affects audit logs in an unlikely navigation sequence, neither of which blocks shipping. apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-logo-upload.ts — code duplication and workspaceIdRef drift Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant Sidebar
participant Hook as useWorkspaceLogoUpload
participant UploadAPI as POST /api/files/upload
participant Storage as workspace-logos bucket
participant PatchAPI as PATCH /api/workspaces/[id]
participant DB as workspace.logo_url
User->>Sidebar: Right-click workspace → Upload logo
Sidebar->>Hook: setTargetWorkspaceId(workspaceId)
Sidebar->>Sidebar: logoTargetWorkspaceIdRef = workspaceId
Sidebar->>User: Open file picker
User->>Sidebar: Select image file
Sidebar->>Hook: handleFileChange(file)
Hook->>Hook: validateFile (size ≤5MB, allowed type)
Hook->>Hook: setPreviewUrl(objectURL)
Hook->>UploadAPI: POST multipart (file, context=workspace-logos, workspaceId)
UploadAPI->>UploadAPI: Verify user is workspace admin
UploadAPI->>Storage: uploadFile(workspace-logos/ts-filename)
Storage-->>UploadAPI: fileInfo.path
UploadAPI->>UploadAPI: recordAudit + captureServerEvent
UploadAPI-->>Hook: fileInfo.path
Hook->>Hook: setPreviewUrl(serverUrl)
Hook->>Sidebar: onUpload(serverUrl)
Sidebar->>PatchAPI: PATCH { logoUrl: serverUrl }
PatchAPI->>PatchAPI: Verify user is workspace admin
PatchAPI->>PatchAPI: Validate logoUrl (starts with / or https://)
PatchAPI->>DB: UPDATE workspace SET logo_url = serverUrl
PatchAPI->>PatchAPI: recordAudit
PatchAPI-->>Sidebar: updated workspace
Sidebar->>Sidebar: invalidate workspaceKeys.lists()
Sidebar->>Sidebar: Re-render with new logoUrl in header
Reviews (6): Last reviewed commit: "refactor: replace raw fetch with useWork..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
7ca4af8 to
1d5656aComparewaleedlatif1
commented
Apr 13, 2026
Both security findings are now addressed:
|
waleedlatif1
commented
Apr 13, 2026
waleedlatif1
commented
Apr 13, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…t for workspace logo uploads
- Add image/webp to ACCEPTED_IMAGE_TYPES in useProfilePictureUpload - Add image/webp to file input accept attributes in whitelabeling settings - Refactor useProfilePictureUpload to use refs for onUpload, onError, and currentImage callbacks, matching the established codebase pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These files were accidentally regressed during rebase conflict resolution, reverting changes from #4027. Restoring to staging versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0e1b3a0 to
8794e12CompareCo-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
waleedlatif1
commented
Apr 13, 2026
waleedlatif1
commented
Apr 13, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Split the ref sync useEffect so workspaceIdRef only updates when the workspaceId prop changes, not when onUpload/onError callbacks get new references. Prevents setTargetWorkspaceId from being overwritten by a re-render before the file upload completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
waleedlatif1
commented
Apr 13, 2026
@cursor review |
waleedlatif1
commented
Apr 13, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Apr 13, 2026
waleedlatif1
commented
Apr 13, 2026
@cursor review |
Uh oh!
There was an error while loading. Please reload this page.
The shared Workspace type requires ownerId and other fields that aren't available from the workspaces API response mapping. Use a Pick type to accurately represent the subset of fields actually constructed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove useState + useEffect + fetch anti-pattern for loading workspaces. Use useWorkspacesQuery from React Query with inline filter for write/admin permissions. Eliminates ~30 lines of manual state management, any casts, and the Pick type workaround. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
waleedlatif1
commented
Apr 13, 2026
@cursor review |
waleedlatif1
commented
Apr 13, 2026
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f05046a. Configure here.
Summary
Type of Change
Testing
Tested manually
Checklist