Add profile picture upload for users and organizations#704
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds shared profile-picture validation, cropping, compression, GCS delivery, and deletion flows for users and organizations. Updates organization creation/settings, user profile editing, welcome completion, organization branding displays, and upload dropzone styling. ChangesProfile picture foundations
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProfilePictureManager
participant ProfileAction
participant GCS
User->>ProfilePictureManager: Select and crop image
ProfilePictureManager->>ProfileAction: Submit cropped multipart data
ProfileAction->>GCS: Upload image object
ProfileAction->>ProfileAction: Update user or organization image reference
ProfileAction->>GCS: Delete previous object
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@fdm-app/app/components/blocks/organization/form.tsx`:
- Around line 178-187: Add type="button" to the logo-removal Button that calls
setProfilePictureFiles([]), ensuring it only clears the selected files and does
not submit the surrounding form.
In `@fdm-app/app/components/blocks/profile/detect-existing.server.ts`:
- Around line 15-20: Update the URL-to-key logic in detect-existing.server.ts
around the types loop to remove the query string from imageUrl before extracting
the path suffix. Ensure the returned profile_picture_${type} key uses only the
pathname portion, so cache-busting parameters are excluded and the result
matches buildObjectKey output.
In `@fdm-app/app/components/blocks/profile/profile-info-form.tsx`:
- Around line 71-78: Remove the stray “Verwijderen” submit Button from the
profile info form, leaving only the “Opslaan” button for submitting the
firstname/surname update. Preserve the existing isSubmitting disabled state and
spinner behavior on the remaining button.
- Line 35: Remove the leftover console.log(form.formState.errors) statement from
the profile info form component, leaving the surrounding form rendering and
validation logic unchanged.
In `@fdm-app/app/components/custom/image-cropper.tsx`:
- Around line 176-179: The crop rectangle callback in the frame-position update
handler must use the newly computed nextX, nextY, and nextScale values rather
than the stale framePosition values. In the imageData reset useEffect, after
resetting to the default frame position, also compute and emit the corresponding
rectangle through onFrameRectangleChange so untouched newly selected images
submit accurate crop bounds.
In `@fdm-app/app/routes/api.profile-picture`.$type.$file_name.ts:
- Around line 42-44: Update the Cache-Control header in the profile-picture
route to use private caching or disable shared caching instead of public
caching. Preserve the existing max-age only if compatible with the chosen
private policy, ensuring session and principal authorization checks remain
enforced for every client.
- Around line 38-46: Update the response handling after generateSignedReadUrl in
the profile-picture route to use react-router’s redirect helper with the signed
URL, ensuring a Location header is emitted. Use a temporary redirect status such
as 302 or 307 rather than 301, while preserving the existing cache-control
behavior.
In `@fdm-app/app/routes/organization`.$slug.settings.tsx:
- Around line 255-266: Update the rollback logic in the organization settings
route’s deleteObject failure path to restore the logo unconditionally, removing
the invalid currentOrganization.metadata.data guard because metadata is raw JSON
there. Keep the existing auth.api.updateOrganization call and payload unchanged,
matching the unconditional rollback behavior of the update_profile_picture
branch.
- Around line 107-111: Gebruik in de ProfilePictureManager-aanroep
loaderData.organization.logo als currentPicture in plaats van
organization.metadata.data?.image. Pas ook de rollback-logica van
delete_profile_picture aan zodat deze de raw organization gebruikt en niet
currentOrganization.metadata.data; behoud de bestaande upload-, delete- en
fallbackstatussen.
In `@fdm-app/app/routes/organization.new.tsx`:
- Line 141: Update the uploadObject calls in the organization new flow and
welcome flow to pass the image buffer (organizationImage.buffer and the
corresponding profile-picture buffer) instead of the MD5 hash, while preserving
the existing object key and MIME arguments.
In `@fdm-app/app/routes/user.settings.profile.edit.tsx`:
- Around line 189-207: Update the delete_profile_picture branch in the action
handler to return a response after completing the deletion flow, including the
same success toast/redirect behavior used by the other intent branches. Ensure
both successful deletion and the existing restoration path after deleteObject
failure produce a valid response instead of falling through with undefined.
- Around line 158-182: In the profile-picture update flow, move the
auth.api.updateUser call that sets the new image URL until after uploadObject
succeeds, matching the upload-then-pointer ordering used by the equivalent
welcome flow. Preserve cleanup by deleting the previous object only after the
pointer update succeeds, and ensure failures remove the newly uploaded object
where needed without reverting a pointer that was never updated.
In `@fdm-app/app/routes/welcome.tsx`:
- Around line 327-340: Update the uploadObject call in the profilePicture
handling block to pass profilePicture.buffer as the object body instead of
profilePicture.hash, while preserving the existing object key and detected MIME
type arguments.
- Around line 327-348: Update the profilePicture upload flow in the welcome
route to determine the existing profile-picture object key and delete it when
the new detected extension differs, reusing the cleanup behavior from the
profile edit route. Ensure the old object is removed after the new upload
succeeds and preserve the existing rollback of the newly uploaded object when
the operation fails.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 35e0dfda-0e01-4db2-a96a-e854ed6ec5e3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/organization/form.tsxfdm-app/app/components/blocks/organization/schema.tsfdm-app/app/components/blocks/profile/detect-existing.server.tsfdm-app/app/components/blocks/profile/profile-info-form.tsxfdm-app/app/components/blocks/profile/profile-info-schema.tsfdm-app/app/components/blocks/profile/profile-picture-manager.tsxfdm-app/app/components/blocks/profile/profile-picture-schema.tsfdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/custom/dropzone.tsxfdm-app/app/components/custom/image-cropper.tsxfdm-app/app/components/ui/slider.tsxfdm-app/app/lib/image-upload.client.tsfdm-app/app/lib/organization-helpers.tsfdm-app/app/routes/api.profile-picture.$type.$file_name.tsfdm-app/app/routes/organization.$slug.settings.tsxfdm-app/app/routes/organization.new.tsxfdm-app/app/routes/user.settings.profile._index.tsxfdm-app/app/routes/user.settings.profile.edit.tsxfdm-app/app/routes/welcome.tsxfdm-app/package.json
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 @.changeset/gold-actors-type.md:
- Line 5: Update the changelog sentence in gold-actors-type.md to scope the
social-login deletion claim to user profile pictures only; retain the deletion
claim for organization logos without implying they can originate from social
login.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5443851a-81b3-4a46-a3c6-1101b1dd9083
📒 Files selected for processing (10)
.changeset/gold-actors-type.mdfdm-app/app/components/blocks/organization/form.tsxfdm-app/app/components/blocks/profile/detect-existing.server.tsfdm-app/app/components/blocks/profile/profile-info-form.tsxfdm-app/app/components/custom/image-cropper.tsxfdm-app/app/routes/api.profile-picture.$type.$file_name.tsfdm-app/app/routes/organization.$slug.settings.tsxfdm-app/app/routes/organization.new.tsxfdm-app/app/routes/user.settings.profile.edit.tsxfdm-app/app/routes/welcome.tsx
💤 Files with no reviewable changes (1)
- fdm-app/app/components/blocks/profile/profile-info-form.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- fdm-app/app/components/blocks/profile/detect-existing.server.ts
- fdm-app/app/routes/api.profile-picture.$type.$file_name.ts
- fdm-app/app/routes/user.settings.profile.edit.tsx
- fdm-app/app/routes/organization.new.tsx
- fdm-app/app/components/blocks/organization/form.tsx
- fdm-app/app/routes/welcome.tsx
- fdm-app/app/components/custom/image-cropper.tsx
- fdm-app/app/routes/organization.$slug.settings.tsx
| "@nmi-agro/fdm-app": minor | ||
| --- | ||
|
|
||
| Users can now upload their profile picture and organizations can now upload their logo, which will be stored in the GCS and displayed using the existing user.image and organization.logo database record fields. The profile pictures and logos, including those retrieved previously from social login, can also be deleted. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the social-login deletion claim to user profile pictures.
The supplied flow demonstrates removal of a user’s imported social image, but not social-login-derived organization logos. Reword this sentence to avoid implying that organization logos can originate from social login.
🤖 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 @.changeset/gold-actors-type.md at line 5, Update the changelog sentence in
gold-actors-type.md to scope the social-login deletion claim to user profile
pictures only; retain the deletion claim for organization logos without implying
they can originate from social login.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@fdm-app/app/components/blocks/organization/organization-card.tsx`:
- Around line 28-31: Update the OrganizationAvatar alt text in
fdm-app/app/components/blocks/organization/organization-card.tsx (lines 28-31)
to use organization.name ?? "organisatie"; apply the same fallback in
fdm-app/app/routes/organization.$slug._index.tsx (lines 156-162) using
loaderData.organization.name ?? "organisatie".
In `@fdm-app/app/routes/organization`.$slug._index.tsx:
- Around line 156-162: Update the organization data flow feeding the rightNode
OrganizationAvatar to resolve rawOrganization.logo through the existing
signed-URL helper/API before rendering. Ensure the avatar receives the signed
browser URL rather than the persisted private GCS object key, while preserving
the existing alt text and styling.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5096ec43-5f6e-4ef6-87f3-529ba2ba6f4b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
fdm-app/app/components/blocks/organization/organization-avatar.tsxfdm-app/app/components/blocks/organization/organization-card.tsxfdm-app/app/routes/organization.$slug._index.tsxfdm-app/app/routes/organization.tsxfdm-app/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/package.json
| }, | ||
| }) | ||
| } catch (err) { | ||
| if (uploaded) { |
New features
Closes #664
Summary by CodeRabbit