fix(client): persist workspace brand color, retention, and logo settings - #257
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughChangesWorkspace settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceSettingsForm
participant LogoUploadRoute
participant VercelBlob
participant PrismaOrganization
WorkspaceSettingsForm->>LogoUploadRoute: Upload validated logo
LogoUploadRoute->>VercelBlob: Store image
VercelBlob-->>LogoUploadRoute: Return public URL
LogoUploadRoute->>PrismaOrganization: Save logoUrl
LogoUploadRoute-->>WorkspaceSettingsForm: Return logo URL
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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 |
Contributor
📦 Affected Packages
Diff: +10560 / -3460 lines
|
Contributor
Lighthouse CI — Marketing Site
Core Web Vitals (Mobile)
|
iDorgham
marked this pull request as ready for review
August 12, 2026 10:16
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.env.example (1)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the quotes to satisfy
dotenv-linter.The configured linter reports
QuoteCharacterfor this value.-BLOB_READ_WRITE_TOKEN="vercel_blob_rw_..." +BLOB_READ_WRITE_TOKEN=vercel_blob_rw_...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example at line 43, Update the BLOB_READ_WRITE_TOKEN example in the environment configuration to remove the surrounding double quotes while preserving its placeholder value.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/client-dashboard/src/app/api/workspace/logo/route.ts`:
- Around line 33-41: Update the logo upload validation around the ALLOWED_TYPES
check and the upload flow near the public upload: do not trust file.type alone,
inspect raster file bytes for valid PNG, JPEG, or WebP signatures before
publishing, and reject mismatches. For SVG uploads, sanitize the XML content
before upload or reject SVG files entirely, ensuring only validated image bytes
reach the public upload with their content type.
In `@apps/client-dashboard/src/app/api/workspace/settings/route.ts`:
- Around line 95-99: Update the settings payload construction in the route’s
workspace update flow so domain is included only when the request provides it,
preserving the existing domain when omitted. Replace the unconditional domain
assignment with the same conditional-property pattern used for accentColor and
logoUrl, while retaining null conversion when an explicit empty value is
submitted.
- Around line 27-35: Update both handlers in
apps/client-dashboard/src/app/api/workspace/settings/route.ts (lines 27-35) and
apps/client-dashboard/src/app/api/workspace/logo/route.ts (lines 14-22) to
require claims.permissions?.['workspace:manage'] in addition to claims.orgId.
Reject unauthorized callers before parsing request data or applying organization
changes, preserving the existing unauthorized response.
- Around line 73-75: Update the three organization lookups and both API update
predicates to use the tenant boundary id plus deletedAt: null:
apps/client-dashboard/src/app/api/workspace/settings/route.ts lines 73-75,
apps/client-dashboard/src/app/[locale]/dashboard/organizations/[orgId]/settings/workspace/page.tsx
lines 8-20, and apps/client-dashboard/src/app/api/workspace/logo/route.ts lines
51-75. In the API routes, treat a conditional update that affects no active
organization as a not-found response.
In `@apps/client-dashboard/src/components/settings/workspace-form.tsx`:
- Around line 116-117: Update the accentColor initialization in the workspace
form to use a resolved six-digit hexadecimal fallback instead of
token('color.background.brand.bold'). Preserve initialData?.accentColor when
provided, and ensure the default satisfies workspaceSchema validation.
---
Nitpick comments:
In @.env.example:
- Line 43: Update the BLOB_READ_WRITE_TOKEN example in the environment
configuration to remove the surrounding double quotes while preserving its
placeholder value.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 259bd33b-5465-40ef-a770-d2d27401d160
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.env.exampleapps/client-dashboard/package.jsonapps/client-dashboard/src/app/[locale]/dashboard/organizations/[orgId]/settings/workspace/page.tsxapps/client-dashboard/src/app/api/workspace/logo/route.tsapps/client-dashboard/src/app/api/workspace/settings/route.tsapps/client-dashboard/src/components/settings/workspace-form.tsxpackages/db/prisma/migrations/20260812120000_add_org_accent_color/migration.sqlpackages/db/prisma/schema.prisma
Contributor
Fixes Applied SuccessfullyFixed 4 file(s) based on 5 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
iDorgham
added a commit
that referenced
this pull request
Aug 12, 2026
…ettings Manual click-through of PR #257 surfaced four real bugs beyond the original scope, all now fixed: - Brand Color's default value came from an unresolved @atlaskit/tokens token() call (returned the raw CSS var string, not a hex color), which failed the accentColor hex regex and blocked the ENTIRE form from saving for any org without a stored color — i.e. every org today. Replaced with a literal default (#ED4B00) and stopped feeding native <input type=color> a possibly-uppercase value, which the browser was silently substituting. - The settings form's PATCH request 403'd with "CSRF token missing" — it used a raw fetch() instead of this app's csrfFetch()/x-csrf-token convention. Fixed for both the settings PATCH and the logo upload POST. - Settings page subtitle showed the literal string "{{orgName}}" instead of the org name — SettingsLayout called t() without passing orgName, despite the translation string requiring it. Now threaded through from requireAuth()'s org data. - The settings sidebar always highlighted "Workspaces" regardless of the active tab — SETTINGS_TABS_DEFS hrefs were hardcoded to a route shape (/dashboard/settings/*) that no longer matches where these pages actually live (/dashboard/organizations/[orgId]/settings/*), so the href-match always missed and silently fell back to the first tab. Hrefs are now derived from the actual pathname at render time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/client-dashboard/src/app/api/workspace/logo/route.ts`:
- Around line 119-139: The logo upload flow leaves an unreferenced blob when
organization persistence fails. In the route handler around put and the
organization.updateMany call, invoke del(blob.url) when updateMany returns zero
affected rows or throws, while never deleting the existing organization.logoUrl;
preserve the current 404 response and rethrow or propagate persistence errors
after cleanup.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5804406c-76d7-41f9-b4f7-e79bc7f74435
📒 Files selected for processing (7)
apps/client-dashboard/src/app/[locale]/dashboard/organizations/[orgId]/settings/layout.tsxapps/client-dashboard/src/app/[locale]/dashboard/organizations/[orgId]/settings/workspace/page.tsxapps/client-dashboard/src/app/api/workspace/logo/route.tsapps/client-dashboard/src/app/api/workspace/settings/route.tsapps/client-dashboard/src/components/settings/settings-layout.tsxapps/client-dashboard/src/components/settings/workspace-form.tsxpackages/i18n/src/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/client-dashboard/src/app/[locale]/dashboard/organizations/[orgId]/settings/workspace/page.tsx
- apps/client-dashboard/src/components/settings/workspace-form.tsx
Comment on lines
+119
to
+139
| const extension = file.type.split('/')[1]; | ||
| const blob = await put( | ||
| `org-logos/${organizationId}-${Date.now()}.${extension}`, | ||
| fileBuffer, | ||
| { | ||
| access: 'public', | ||
| contentType: file.type, | ||
| } | ||
| ); | ||
|
|
||
| const updateResult = await prisma.organization.updateMany({ | ||
| where: { id: organizationId, deletedAt: null }, | ||
| data: { logoUrl: blob.url }, | ||
| }); | ||
|
|
||
| if (updateResult.count === 0) { | ||
| return NextResponse.json( | ||
| { success: false, message: 'Organization not found' }, | ||
| { status: 404 } | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For @vercel/blobversion 2.8.0, does the server-sidedelAPI accept a blob URL returned byput, and what errors can it throw when deleting a public blob?
💡 Result:
Yes, the server-side del API in @vercel/blob version 2.8.0 accepts a blob URL returned by put [1][2][3]. The del function is designed to accept either a string (representing a single URL or pathname) or an array of strings [1][3]. When deleting a blob, including a public one, the following errors and behaviors may occur: 1. Authentication Errors: If the BLOB_READ_WRITE_TOKEN is not configured or not passed via the token option, the operation will throw an error indicating that no token was found [4]. This is a common requirement even for public stores, as the SDK needs permission to perform the deletion [4][5]. 2. Conditional Delete Errors: If you use the ifMatch option to perform a conditional delete and the ETag provided does not match the current ETag of the blob, the operation will throw a BlobPreconditionFailedError [1][3]. This option is only supported for single-URL deletions [1][3]. 3. Other Failures: While the del operation itself is idempotent—meaning it will not throw an error if the blob URL does not exist—it may throw generic errors related to network failures, rate limiting (e.g., BlobServiceRateLimited), or invalid credentials [3][6]. Note that del returns a void response; successful deletion occurs if no error is thrown [3][7].
Citations:
- 1: https://github.com/vercel/storage/blob/f23cb891/packages/blob/src/del.ts
- 2: https://github.com/vercel/storage/blob/main/packages/blob/CHANGELOG.md
- 3: https://vercel.com/docs/vercel-blob/using-blob-sdk
- 4: https://community.vercel.com/t/unable-to-delete-file-stored-on-public-blob-vercel-storage-com/4399
- 5: https://vercel.com/docs/vercel-blob/public-storage.md
- 6: https://vercel.com/docs/vercel-blob/examples
- 7: https://examples.vercel.com/docs/vercel-blob/using-blob-sdk
🏁 Script executed:
#!/bin/bash
set -eu
route=$(fd -t f 'route\.ts$' apps/client-dashboard/src/app/api/workspace/logo 2>/dev/null | head -n 1)
printf '%s\n' "ROUTE=$route"
cat -n "$route" | sed -n '1,190p'
printf '\nPACKAGE REFERENCES\n'
rg -n '"`@vercel/blob`"|from .@vercel/blob.|del\(' --glob 'package.json' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' .Repository: iDorgham/Gateflow
Length of output: 6140
Delete the new blob when persistence does not complete.
If updateMany affects zero rows or throws after put succeeds, call del(blob.url) to remove the unreferenced public blob. Do not delete the existing organization.logoUrl, which can contain a non-Vercel URL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/client-dashboard/src/app/api/workspace/logo/route.ts` around lines 119 -
139, The logo upload flow leaves an unreferenced blob when organization
persistence fails. In the route handler around put and the
organization.updateMany call, invoke del(blob.url) when updateMany returns zero
affected rows or throws, while never deleting the existing organization.logoUrl;
preserve the current 404 response and rethrow or propagate persistence errors
after cleanup.
…logo settings Brand Color, Data Retention, and Logo Upload silently no-op'd on save. Adds Organization.accentColor (+ migration), replaces the single days slider with the four schema-matching *RetentionMonths controls plus a legal-hold toggle, and wires real logo upload via Vercel Blob instead of base64-in-state. Also fixes the settings form posting to a nonexistent /organizations/:id route with mismatched field names, which was silently dropping every save (including name/email edits). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
…ettings Manual click-through of PR #257 surfaced four real bugs beyond the original scope, all now fixed: - Brand Color's default value came from an unresolved @atlaskit/tokens token() call (returned the raw CSS var string, not a hex color), which failed the accentColor hex regex and blocked the ENTIRE form from saving for any org without a stored color — i.e. every org today. Replaced with a literal default (#ED4B00) and stopped feeding native <input type=color> a possibly-uppercase value, which the browser was silently substituting. - The settings form's PATCH request 403'd with "CSRF token missing" — it used a raw fetch() instead of this app's csrfFetch()/x-csrf-token convention. Fixed for both the settings PATCH and the logo upload POST. - Settings page subtitle showed the literal string "{{orgName}}" instead of the org name — SettingsLayout called t() without passing orgName, despite the translation string requiring it. Now threaded through from requireAuth()'s org data. - The settings sidebar always highlighted "Workspaces" regardless of the active tab — SETTINGS_TABS_DEFS hrefs were hardcoded to a route shape (/dashboard/settings/*) that no longer matches where these pages actually live (/dashboard/organizations/[orgId]/settings/*), so the href-match always missed and silently fell back to the first tab. Hrefs are now derived from the actual pathname at render time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
iDorgham
force-pushed
the
fix/workspace-settings-persistence
branch
from
August 12, 2026 14:53
c03f7bb to
a0d2035
Compare
github-actions
Bot
added
size/XL
Contributor
Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
github-actions
Bot
added
size/XL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Organization.accentColor(+ hand-written migration) so Brand Color has somewhere to persist.*RetentionMonthsfields, plus a Legal Hold toggle./api/workspace/logo) instead of base64-encoding into ephemeral local state./organizations/:idroute with a mismatched field name (adminEmailvsemail) — this was silently dropping every save, including plain name/email edits.Bugs found and fixed during manual verification
Live click-through (login → edit → save → reload) surfaced four more real bugs, all fixed in this PR:
@atlaskit/tokenstoken()call (returned the raw CSS var string, not a hex color), which failed theaccentColorhex regex — so any org without a stored color (i.e. every org today) couldn't save name/email/retention/anything. Replaced with a literal default and stopped feeding native<input type=color>a possibly-uppercase value, which the browser was silently substituting (and which was leaking back into the paired text field via a shared form binding).fetch()instead of this app'scsrfFetch()/x-csrf-tokenconvention, so every submit 403'd with "CSRF token missing."{{orgName}}shown literally in the settings page subtitle —SettingsLayoutcalledt()without passingorgName. Now threaded through fromrequireAuth()'s org data./dashboard/settings/*, a route shape these pages no longer live at (they're under/dashboard/organizations/[orgId]/settings/*), so the href-match always missed and silently fell back to the first tab. Hrefs are now derived from the actual pathname at render time.CodeRabbit auto-fixes
CodeRabbit's bot pushed a commit to this branch with real security hardening, since rebased in cleanly:
workspace:managepermission gate on both the settings PATCH and logo upload POST (previously any authenticated org member could hit them).updateMany/findFirstwithdeletedAtguards instead of separate read-then-write (closes a TOCTOU race).One conflict on rebase: CodeRabbit's fix for the broken Brand Color default used a generic placeholder (
#0052CC); kept GateFlow's actual brand accent (#ED4B00) instead.Rebased onto master
masterindependently landed4e3e0f68while this PR was in flight — a narrower fix for the same active-tab-highlight bug, plus a fix for the broken workspace-settings endpoint that only handlesname/email(explicitly deferringaccentColor/retention/logoUrlto "tracked separately" — i.e. this PR). Rebased on top and reconciled:settingsRoot, exact-match for thegeneraltab) — functionally identical to what I'd written independently, no reason to keep two implementations of the same fix.apiClient.patch(), which never attaches the CSRF token — it would still 403 in practice. This PR'scsrfFetch()-based version (already verified working end-to-end) handles all fields and CSRF correctly, so it supersedes master's narrower fix rather than merging with it.Re-verified end-to-end after the rebase: fresh login, edit, save → clean
200 OKon the first request (no CSRF retry needed), confirmed via a fresh DB read.Test plan
tsc --noEmitclean on client-dashboardeslintclean on all touched filesprisma validate/prisma generateclean on the new schema field + migrationpnpm preflight(lint + typecheck + test, all packages) passed pre-push, twice (before and after the CodeRabbit rebase)accentColor: "#ED4B00",scanLogRetentionMonths: 18){{orgName}}interpolation fixes visuallyput()call with a clean error (No blob credentials found— expected,BLOB_READ_WRITE_TOKENisn't set locally)400 "SVG uploads are not supported for security reasons"image/png) →400 "File content does not match declared file type"401 "Unauthorized"BLOB_READ_WRITE_TOKENis set in the Vercel project (Blob store must be linked) before this ships to an environment where logo upload will be exercised — that's the only remaining unverified step, and it's an infra/env prerequisite, not a code path🤖 Generated with Claude Code