feat(webapp): upload and crop a profile picture from the account page - #4802
feat(webapp): upload and crop a profile picture from the account page#4802kathiekiwi wants to merge 25 commits into
Conversation
|
WalkthroughAdds profile photo upload, cropping, removal, and avatar preview handling. Adds validated avatar storage with dedicated object-store configuration, cleanup, presigned downloads, and raw byte reads. Adds authenticated avatar resource actions and loaders with request-size limits. Updates API avatar URLs and CSP image sources. Adds Storybook states, styling, environment examples, local MinIO setup, and tests. Merge Risk: 🟡 Moderate · up to The PR adds S3-backed avatar upload and retrieval, but the current implementation can expose avatar access over unencrypted HTTP, leave removed images reachable after cleanup failures, or break the current avatar during overlapping updates. These concrete security and reliability issues should be fixed or explicitly accepted before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
Observability mapAs of 20/100 over 451 measured of 469 entry points (base 20, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/webapp/app/services/dashboardAgentBodyCap.server.ts (1)
82-90: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd required crumb instrumentation to the changed flows.
apps/webapp/app/services/dashboardAgentBodyCap.server.ts#L82-L90: add crumbs for declared-length rejection and request teardown.apps/webapp/app/routes/resources.account.avatar.ts#L18-L25: add crumbs for the impersonation mutation rejection.apps/webapp/test/dashboardAgentBodyCap.test.ts#L174-L187: add crumbs for the stalled-client teardown test path.apps/webapp/app/components/ProfilePhotoEditor.tsx#L94-L97: add crumbs for saved-avatar fallback state.Use an approved crumb namespace where required. As per coding guidelines: “Add crumbs as you write code — not just when debugging.”
Source: Coding guidelines
apps/webapp/app/components/ProfilePhotoEditor.tsx (1)
116-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject selected files over the 5 MiB limit.
selectFilechecks onlyfile.type. A source image over 5 MiB can still be decoded and cropped into a smaller PNG that passes the server upload limit. Checkfile.sizeagainst the sharedMAX_AVATAR_SIZE_IN_BYTESbefore creating the object URL.Proposed fix
if (!ACCEPTED_TYPES.includes(file.type)) { setError("Choose a PNG, JPEG or WebP image."); return; } + if (file.size > MAX_AVATAR_SIZE_IN_BYTES) { + setError("Choose an image smaller than 5 MB."); + return; + } + setCrop(CENTER);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e56b1115-c91b-44a3-9c6d-8bc5d32226bd
📒 Files selected for processing (5)
apps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/routes/storybook.profile-photo-editor/route.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: internal / 🧪 Unit Tests: Internal
🧰 Additional context used
📓 Path-based instructions (14)
Use Remix flat-file route conventions with dot-separated segments; for example, `api.v1.tasks.$taskId.trigger.ts` maps to `/api/v1/tasks/:taskId/trigger`.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/routes/resources.account.avatar.ts
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/services/dashboardAgentBodyCap.server.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/dashboardAgentBodyCap.test.ts
Test files must not import `app/env.server.ts`; pass configuration as options instead.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/test/dashboardAgentBodyCap.test.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/dashboardAgentBodyCap.test.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/dashboardAgentBodyCap.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/components/ProfilePhotoEditor.tsxapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/routes/resources.account.avatar.tsapps/webapp/app/services/dashboardAgentBodyCap.server.tsapps/webapp/test/dashboardAgentBodyCap.test.ts
🪛 ast-grep (0.45.2)
apps/webapp/test/dashboardAgentBodyCap.test.ts
[warning] 25-25: Express application should use Helmet
Context: express()
Note: [CWE-693] Protection Mechanism Failure (Express app without Helmet security headers).
(missing-helmet-typescript)
|
Preview Deployment
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/webapp/test/userAvatar.test.ts (1)
204-210: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIsolate
AVATARS_OBJECT_STORE_SERVICEin avatar tests.A runner-provided service value can remain active because neither suite resets this new setting. The MinIO test then does not guarantee S3 signing.
apps/webapp/test/userAvatar.test.ts#L204-L210: includeAVATARS_OBJECT_STORE_SERVICEin the saved and reset avatar environment state, then set"s3"for isolated test cases.apps/webapp/test/objectStore.test.ts#L855-L878: save and restore this value, and set it to"s3"before the MinIO upload.Proposed fix
const AVATAR_ENV_KEYS = [ "AVATARS_OBJECT_STORE_BASE_URL", "AVATARS_OBJECT_STORE_BUCKET", "AVATARS_OBJECT_STORE_ACCESS_KEY_ID", "AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY", "AVATARS_OBJECT_STORE_REGION", + "AVATARS_OBJECT_STORE_SERVICE", ] as const;const original = { // existing settings + service: env.AVATARS_OBJECT_STORE_SERVICE, }; afterAll(() => { // existing restores + env.AVATARS_OBJECT_STORE_SERVICE = original.service; }); // MinIO setup +env.AVATARS_OBJECT_STORE_SERVICE = "s3";apps/webapp/app/routes/account._index/route.tsx (1)
457-548: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd crumb instrumentation to the new profile-photo flow.
The new
ChangeProfilePhotoButtonupload and removal flow has no//@Crumbsmarker or `// `#region` `@crumbsblock. Add a crumb around this behavior so development tracing is available beforeagentcrumbs stripremoves it.As per coding guidelines:
**/*: Add crumbs as you write code — not just when debugging.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eba5b1a5-ff6e-456e-b53b-d0807e378893
📒 Files selected for processing (5)
apps/webapp/app/env.server.tsapps/webapp/app/routes/account._index/route.tsxapps/webapp/app/services/userAvatar.server.tsapps/webapp/test/objectStore.test.tsapps/webapp/test/userAvatar.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (38)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: 🛡️ E2E Auth Tests (full)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (13)
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/env.server.tsapps/webapp/app/services/userAvatar.server.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/objectStore.test.tsapps/webapp/test/userAvatar.test.ts
Test files must not import `app/env.server.ts`; pass configuration as options instead.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/test/objectStore.test.tsapps/webapp/test/userAvatar.test.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/app/env.server.tsapps/webapp/app/services/userAvatar.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/objectStore.test.tsapps/webapp/test/userAvatar.test.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/objectStore.test.tsapps/webapp/test/userAvatar.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/routes/account._index/route.tsxapps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/test/objectStore.test.tsapps/webapp/app/env.server.tsapps/webapp/test/userAvatar.test.tsapps/webapp/app/services/userAvatar.server.ts
🧠 Learnings (2)
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.
Applied to files:
apps/webapp/test/objectStore.test.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
apps/webapp/test/userAvatar.test.ts
🔇 Additional comments (1)
apps/webapp/app/routes/account._index/route.tsx (1)
11-11: LGTM!Also applies to: 37-37, 95-95, 267-271, 882-883, 1003-1007
|
Linear have a nice feature that lets you rotate the uploaded image by 45 degrees if it uploads in the wrong rotation. Like a little rotate icon top right of the image. It would be nice to include this too as it would be a big pain to fix outside the UI. |
2 participants
Summary
You can now change your profile picture from the account page. Hovering the avatar shows "Change your profile picture"; clicking opens a modal where you pick an image (png, jpeg or webp, up to 5MB), crop it with a circular mask and zoom, and save. Uploads go to S3.