Image uploads handling - #81
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 10 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR introduces a complete image upload workflow using Cloudflare R2 presigned URLs. It adds backend infrastructure for presigned-URL generation, frontend image compression, and a React component for image selection and upload management. ChangesImage Upload with Presigned URLs
Sequence Diagram(s)sequenceDiagram
participant User
participant Gallery as ImageUploadGallery
participant Hook as useImageUploadGallery
participant Compress as compressImage
participant Handler as handleImageUpload
participant API as POST /presigned-url
participant R2
User->>Gallery: Select image(s)
Gallery->>Hook: handleFilesSelected(files)
Hook->>Compress: Compress each file
Compress->>Hook: Compressed File
Hook->>Handler: (accessToken, compressed file)
Handler->>API: Request presigned URL
API->>Handler: uploadUrl, imageUrl, fileType
Handler->>R2: PUT compressed file
R2->>Handler: Success
Handler->>Hook: imageUrl, fileType, fileSizeKb
Hook->>Gallery: Update image state to done
Gallery->>User: Display thumbnail + success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
foundit-ui/components/uploadImage.tsx (1)
1-30: 💤 Low valueUnused hook return values.
The hook returns
inputRef(line 29) andhasImages, but neither is used in this component.inputRefis re-declared internally, andimages.length > 0is used instead ofhasImages.Consider removing unused destructured values or utilizing them:
- const { images, canAddMore, maxImages, handleFilesSelected, handleRemove } = + const { images, inputRef, canAddMore, hasImages, maxImages, handleFilesSelected, handleRemove } = useImageUploadGallery({ accessToken, onChange });Then use
inputRefwith the input element and replaceimages.length > 0withhasImageson line 67.🤖 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 `@foundit-ui/components/uploadImage.tsx` around lines 1 - 30, The component currently destructures inputRef and hasImages from useImageUploadGallery but doesn't use them (and re-declares inputRef internally) — update ImageUploadGallery to either stop destructuring them or actually use them: remove the internal redeclaration of inputRef, attach the hook-provided inputRef to the file input element used by handleFilesSelected, and replace any checks of images.length > 0 with hasImages; keep handleFilesSelected and handleRemove as-is to wire selection/removal to the hook.foundit-ui/hooks/useImageUploadGallery.ts (1)
94-102: ⚖️ Poor tradeoffConsider canceling in-flight uploads when removing images.
When a user removes an image with
status === 'uploading', the upload continues in the background, wasting bandwidth and potentially causing a state update on an unmounted component.You could refactor to use
AbortControllerto cancel the fetch requests inhandleImageUpload, though this adds complexity for a relatively minor optimization.🤖 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 `@foundit-ui/hooks/useImageUploadGallery.ts` around lines 94 - 102, The remove handler currently revokes the preview URL but doesn't stop in-flight uploads; update handleImageUpload to create and attach an AbortController to the image object (e.g., add an abortController field when starting upload) and modify handleRemove to detect if the removed image has status === 'uploading' and call removed.abortController.abort() (and cleanup the controller reference) before revoking the URL and filtering state; ensure setImages still calls notifyChange(next) and that any fetch handlers respect the AbortSignal to avoid state updates after abort.backend/package.json (1)
13-14: AWS SDK v3.1065.0 is published;^3.1065.0will already float to newer 3.x releases
@aws-sdk/client-s3@3.1065.0and@aws-sdk/s3-request-presigner@3.1065.0are both published (released June 9, 2026). As of June 13, 2026, the latest@aws-sdk/client-s3is3.1067.0; since the dependency uses^3.1065.0, you’ll already allow updates to later3.xversions without changing this range (subject to your usual compatibility testing).🤖 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 `@backend/package.json` around lines 13 - 14, The package.json entries for "`@aws-sdk/client-s3`" and "`@aws-sdk/s3-request-presigner`" use ^3.1065.0 which already allows newer 3.x releases, so either leave them as-is or explicitly pin; if you intended to lock to exactly 3.1065.0 remove the caret from both dependency values (change "^3.1065.0" to "3.1065.0") for "`@aws-sdk/client-s3`" and "`@aws-sdk/s3-request-presigner`", otherwise revert any accidental change and keep the caret to continue allowing patch/minor updates.
🤖 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 `@backend/src/lib/r2.ts`:
- Around line 3-10: Validate R2 environment variables before creating the
S3Client: check process.env.R2_ENDPOINT, process.env.R2_ACCESS_KEY_ID, and
process.env.R2_SECRET_ACCESS_KEY and fail fast with a clear error (throw or
process.exit(1) after logging) if any are missing, then instantiate the S3Client
(exported as r2) using the validated values; place the validation block
immediately before the new S3Client(...) creation to mirror the JWT secret
startup check pattern found in backend/src/index.ts.
In `@backend/src/routes/uploads.ts`:
- Line 6: The presigned URL endpoint is currently unauthenticated because the
requireAuth import was commented out; re-enable and import requireAuth and apply
it to the presigned URL route (e.g., router.post('/presigned-url', requireAuth,
async (req, res) => { ... })) so the middleware runs before the handler, and
update the handler to use authenticated context (req.user or req.session) to
validate/authorize the upload request and return 401/403 for
unauthenticated/unauthorized callers; ensure the backend consumes the
Authorization header the frontend sends and does not allow unauthenticated
generation of presigned URLs.
- Around line 77-79: The presigned URL expiry is set to 5 hours when calling
getSignedUrl (variable uploadUrl); change the expiresIn value to a much shorter
TTL (e.g., 15 * 60 for 15 minutes) in the getSignedUrl call to limit exposure,
and optionally make this TTL configurable (e.g., UPLOAD_PRESIGNED_TTL) so
handlers using r2 and command can adjust to 30–60 minutes for very large uploads
if needed.
- Around line 59-66: The current size check uses the untrusted fileSizeKb from
req.body (variable fileSizeKb) in the uploads route, so update the presigned
upload generation to enforce size on the server: when creating the presigned
PutObject URL (the code that constructs a PutObjectCommand / calls
getSignedUrl), add a ContentLengthRange condition (ContentLengthRange: [0, 5 *
1024 * 1024]) to the request parameters so the upload is rejected if the client
declares a larger Content-Length; additionally implement a post-upload
validation step in the same uploads route or a new webhook/lifecycle handler to
fetch the uploaded object's actual size (headObject or equivalent) and
delete/reject objects exceeding 5MB to cover clients that spoof headers.
- Line 72: The handler that builds the PutObjectCommand uses
process.env.R2_BUCKET! (seen in uploads.ts) which can be undefined and will
cause getSignedUrl to fail; add centralized environment validation at server
startup to assert R2_BUCKET is present and throw a clear error if missing, then
remove non-null assertions in the upload code (replace process.env.R2_BUCKET!
usage with the validated config value or a Config.getR2Bucket() accessor) so
PutObjectCommand always receives a defined Bucket; ensure the startup validation
runs before any route registration so endpoints like the upload route and
functions constructing PutObjectCommand/getSignedUrl cannot run without a valid
R2_BUCKET.
In `@foundit-ui/components/uploadImage.tsx`:
- Around line 34-64: The Label's text color is fixed to "blue.500" even when
error is true; update the Label's color prop to mirror the other error-based
styles (e.g., color={error ? 'red.500' : 'blue.500'}) so text and icons reflect
the error state consistently; adjust any related hover/_hover styles if needed
to prevent blue hover on error. Use the Label element and its color,
borderColor, bg, and _hover props to implement this change.
- Around line 70-131: Fix the typo in the thumbnail Box width: replace the
invalid w="80%%" with a single-percent value (e.g., w="80%") in the
GridItem/Image preview block (look for GridItem using img.previewUrl, Box that
sets w/h and Image that uses img.previewUrl). Also review the height value
(h="80%") for consistency—consider using equal width/height or an aspectRatio on
the same Box to ensure consistent thumbnails; keep the existing CloseButton
handler handleRemove unchanged.
In `@foundit-ui/hooks/useImageUploadGallery.ts`:
- Around line 50-92: The hook creates object URLs in the file loop (previewUrl
via URL.createObjectURL) but doesn't revoke them on component unmount; add a
React useEffect cleanup in useImageUploadGallery that on unmount iterates the
current images state and calls URL.revokeObjectURL(img.previewUrl) for each
image (and optionally revoke when an image status changes to ensure no leaks),
import useEffect from React, and keep existing handleRemove revocation logic
intact so all created object URLs are revoked either when removed or when the
component unmounts.
- Around line 50-77: The hook currently compresses files via compressImage
before calling handleImageUpload, but handleImageUpload also compresses
internally causing double compression and quality loss; fix by removing internal
compression from handleImageUpload (foundit-ui/utils/handleImageUpload.ts):
change its signature to accept an already-compressed File/Blob, delete the
internal call to compressImage, and ensure it uses the provided file directly
when building the upload payload; update any callers (e.g.,
useImageUploadGallery's call to handleImageUpload and its type expectations) and
adjust types/tests if needed.
In `@foundit-ui/utils/handleImageUpload.ts`:
- Line 2: The file handleImageUpload.ts imports the PresignedUrlResponse type
from the backend src tree which breaks module boundaries; to fix, remove the
cross-repo import and instead define or import a local frontend duplicate of the
interface (e.g., create foundit-ui/types/uploads.ts exporting
PresignedUrlResponse with fields uploadUrl, imageUrl, fileType, fileSizeKb) and
update handleImageUpload.ts to import PresignedUrlResponse from that local
frontend types module so the function(s) that reference PresignedUrlResponse use
the local type rather than ../../backend/src/types/uploads.
---
Nitpick comments:
In `@backend/package.json`:
- Around line 13-14: The package.json entries for "`@aws-sdk/client-s3`" and
"`@aws-sdk/s3-request-presigner`" use ^3.1065.0 which already allows newer 3.x
releases, so either leave them as-is or explicitly pin; if you intended to lock
to exactly 3.1065.0 remove the caret from both dependency values (change
"^3.1065.0" to "3.1065.0") for "`@aws-sdk/client-s3`" and
"`@aws-sdk/s3-request-presigner`", otherwise revert any accidental change and keep
the caret to continue allowing patch/minor updates.
In `@foundit-ui/components/uploadImage.tsx`:
- Around line 1-30: The component currently destructures inputRef and hasImages
from useImageUploadGallery but doesn't use them (and re-declares inputRef
internally) — update ImageUploadGallery to either stop destructuring them or
actually use them: remove the internal redeclaration of inputRef, attach the
hook-provided inputRef to the file input element used by handleFilesSelected,
and replace any checks of images.length > 0 with hasImages; keep
handleFilesSelected and handleRemove as-is to wire selection/removal to the
hook.
In `@foundit-ui/hooks/useImageUploadGallery.ts`:
- Around line 94-102: The remove handler currently revokes the preview URL but
doesn't stop in-flight uploads; update handleImageUpload to create and attach an
AbortController to the image object (e.g., add an abortController field when
starting upload) and modify handleRemove to detect if the removed image has
status === 'uploading' and call removed.abortController.abort() (and cleanup the
controller reference) before revoking the URL and filtering state; ensure
setImages still calls notifyChange(next) and that any fetch handlers respect the
AbortSignal to avoid state updates after abort.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ddd6a7e-1dd9-4065-9ba8-04e15afae383
⛔ Files ignored due to path filters (2)
backend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfoundit-ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
backend/.env.examplebackend/package.jsonbackend/src/index.tsbackend/src/lib/r2.tsbackend/src/routes/uploads.tsbackend/src/types/uploads.tsfoundit-ui/components/uploadImage.tsxfoundit-ui/hooks/useImageUploadGallery.tsfoundit-ui/package.jsonfoundit-ui/utils/handleImageUpload.tsfoundit-ui/utils/imageCompression.ts
Comment on lines
+34
to
+64
| <Label | ||
| htmlFor="image-upload-input" | ||
| w="100%" | ||
| h="40px" | ||
| display="flex" | ||
| alignItems="center" | ||
| justifyContent="center" | ||
| gap={2} | ||
| border="1px dashed" | ||
| borderColor={error ? 'red.500' : 'blue.300'} | ||
| bg={error ? 'red.50' : 'blue.50'} | ||
| color="blue.500" | ||
| borderRadius="sm" | ||
| cursor={canAddMore ? 'pointer' : 'not-allowed'} | ||
| opacity={canAddMore ? 1 : 0.6} | ||
| fontWeight={600} | ||
| transition="background 0.15s ease" | ||
| _hover={canAddMore ? { bg: 'blue.100' } : undefined} | ||
| > | ||
| <LuUpload size={16} /> | ||
| <Text fontSize="sm">Upload Picture</Text> | ||
| <input | ||
| id="image-upload-input" | ||
| type="file" | ||
| accept="image/jpeg,image/png,image/webp" | ||
| multiple | ||
| hidden | ||
| disabled={!canAddMore} | ||
| onChange={handleFilesSelected} | ||
| /> | ||
| </Label> |
There was a problem hiding this comment.
Text color should be conditional on error state.
Lines 43-44 apply red border and background when error is truthy, but line 45 keeps color="blue.500" regardless. This creates a visual inconsistency.
🎨 Proposed fix
bg={error ? 'red.50' : 'blue.50'}
- color="blue.500"
+ color={error ? 'red.500' : 'blue.500'}
borderRadius="sm"📝 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.
Suggested change
| <Label | |
| htmlFor="image-upload-input" | |
| w="100%" | |
| h="40px" | |
| display="flex" | |
| alignItems="center" | |
| justifyContent="center" | |
| gap={2} | |
| border="1px dashed" | |
| borderColor={error ? 'red.500' : 'blue.300'} | |
| bg={error ? 'red.50' : 'blue.50'} | |
| color="blue.500" | |
| borderRadius="sm" | |
| cursor={canAddMore ? 'pointer' : 'not-allowed'} | |
| opacity={canAddMore ? 1 : 0.6} | |
| fontWeight={600} | |
| transition="background 0.15s ease" | |
| _hover={canAddMore ? { bg: 'blue.100' } : undefined} | |
| > | |
| <LuUpload size={16} /> | |
| <Text fontSize="sm">Upload Picture</Text> | |
| <input | |
| id="image-upload-input" | |
| type="file" | |
| accept="image/jpeg,image/png,image/webp" | |
| multiple | |
| hidden | |
| disabled={!canAddMore} | |
| onChange={handleFilesSelected} | |
| /> | |
| </Label> | |
| <Label | |
| htmlFor="image-upload-input" | |
| w="100%" | |
| h="40px" | |
| display="flex" | |
| alignItems="center" | |
| justifyContent="center" | |
| gap={2} | |
| border="1px dashed" | |
| borderColor={error ? 'red.500' : 'blue.300'} | |
| bg={error ? 'red.50' : 'blue.50'} | |
| color={error ? 'red.500' : 'blue.500'} | |
| borderRadius="sm" | |
| cursor={canAddMore ? 'pointer' : 'not-allowed'} | |
| opacity={canAddMore ? 1 : 0.6} | |
| fontWeight={600} | |
| transition="background 0.15s ease" | |
| _hover={canAddMore ? { bg: 'blue.100' } : undefined} | |
| > | |
| <LuUpload size={16} /> | |
| <Text fontSize="sm">Upload Picture</Text> | |
| <input | |
| id="image-upload-input" | |
| type="file" | |
| accept="image/jpeg,image/png,image/webp" | |
| multiple | |
| hidden | |
| disabled={!canAddMore} | |
| onChange={handleFilesSelected} | |
| /> | |
| </Label> |
🤖 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 `@foundit-ui/components/uploadImage.tsx` around lines 34 - 64, The Label's text
color is fixed to "blue.500" even when error is true; update the Label's color
prop to mirror the other error-based styles (e.g., color={error ? 'red.500' :
'blue.500'}) so text and icons reflect the error state consistently; adjust any
related hover/_hover styles if needed to prevent blue hover on error. Use the
Label element and its color, borderColor, bg, and _hover props to implement this
change.
86unj
approved these changes
Jun 13, 2026
Merged
This was referenced Jul 6, 2026
This was referenced Jul 31, 2026
Merged
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
2 participants
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 by CodeRabbit