feat: integrate image upload API for application#1142
Conversation
WalkthroughThis PR adds API-based image upload functionality to the new-join-steps component. Changes include a new profile image upload endpoint constant, an async image upload handler with FormData integration, 2MB file size validation, robust error handling, and comprehensive integration tests. Changes
Sequence DiagramsequenceDiagram
actor User
participant Component as NewStepOne<br/>Component
participant API as API Server
participant Toast as Toast Service
User->>Component: Select image file
Component->>Component: Validate file type
alt Invalid type
Component->>Toast: Show error (invalid type)
else Invalid size (>2MB)
Component->>Toast: Show error (size limit)
else Valid file
Component->>Component: Set isImageUploading=true
Component->>API: POST FormData with image
alt API Success
API-->>Component: Return image URL
Component->>Component: Convert to data URL<br/>Update imageUrl
Component->>Component: Set data-test attr
Component->>Toast: Show success toast
else API Error
API-->>Component: Error response
Component->>Toast: Show error (API message)
end
Component->>Component: Set isImageUploading=false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
Deploying www-rds with
|
| Latest commit: |
1709989
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a4d316cf.www-rds.pages.dev |
| Branch Preview URL: | https://feat-image-upload.www-rds.pages.dev |
ec7767a to
3a396b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/new-join-steps/new-step-one.js`:
- Around line 116-130: The success toast uses TOAST_OPTIONS but the error toasts
do not, causing inconsistent toast behavior; update all toast calls in this
component (this.toast.success and both this.toast.error usages inside the upload
flow) to pass the same TOAST_OPTIONS argument so success and error messages
share the same display configuration.
- Line 129: Remove the debug console.error call and replace it with the
application's proper logging/observability mechanism: locate the
console.error('Image upload error:', error) statement in the image upload
handler inside new-step-one.js and either remove it or call the centralized
logger/observability utility (e.g., logger.error or Sentry.captureException)
with the error and contextual metadata (e.g., "image upload" or relevant
identifiers); ensure to import the logging utility if not already imported and
preserve existing error-handling flow (rethrow or surface user-facing error)
after logging.
- Around line 95-133: The code uses FileReader/readAsDataURL causing a race and
risks never saving the server-returned URL; remove the FileReader usage in the
upload flow inside the try block (delete reader, reader.onload and
reader.readAsDataURL), and after parsing response.json() in the apiRequest call
(APPLICATION_PROFILE_IMAGE_URL) immediately validate that data.url ||
data.picture is present (assign to imageUrl); if missing, treat as error
(toast.error and skip success path); otherwise set this.imagePreview = imageUrl
and call this.updateFieldValue?.('imageUrl', imageUrl) immediately, then show
the success toast; keep the existing catch/finally to reset
this.isImageUploading but ensure the success path happens before finally exits.
In `@tests/integration/components/new-join-steps/new-step-one-test.js`:
- Around line 40-44: The test uses this.toast.error.calledWith(...) which only
partially matches arguments; update each toast assertion in new-step-one-test
(all four occurrences referencing this.toast.error and similar toast methods) to
assert exact arguments by replacing calledWith with calledWithExactly or by
including the expected title and options object in the call (e.g., include the
title string and the options object alongside the message) so the test verifies
both title and options rather than only the first argument.
- Around line 67-93: The test "imagePreview and imageUrl are updated on
successful upload" currently only asserts the preview src but not that the
returned server URL is persisted; update the test to verify that the component
saves the URL by stubbing or spying on the form update method (e.g.,
updateFieldValue) or by accessing the component's data (data.imageUrl) after the
fake fetch resolves, and assert it was called/updated with
'https://example.com/photo.jpg' when NewJoinSteps::NewStepOne handles the
upload.
- Line 92: Remove the redundant manual restores for the fetch stub: delete the
in-test calls to fetchStub.restore() (the ones inside the two test bodies)
because the suite-level afterEach hook already calls sinon.restore(), which
restores fetchStub; leave the afterEach(sinon.restore) intact and ensure only
the fetchStub variable is used in tests without manual restore calls.
tests/integration/components/new-join-steps/new-step-one-test.js
Outdated
Show resolved
Hide resolved
032a1a6 to
5624407
Compare
|
@coderabbitai not writing poems these days? |
|
Tip For best results, initiate chat on the files or code changes.
In the land of forms where applications dwell, FormData packages your file with care, If success arrives with a URL so bright, Tests now guard this feature with pride, So here's to uploads, both large and small, 🐰✨ Poetically yours! 🎭 |
| TOAST_OPTIONS, | ||
| ); | ||
| } else { | ||
| const errorData = await response.json(); |
There was a problem hiding this comment.
at response.json()
If the server returns a non-JSON error response (e.g., a 502 Bad Gateway with an HTML body), await response.json() will throw, get swallowed by the catch, and display a cryptic message like "Unexpected token '<'" instead of a meaningful error.
Date: 01-02-26
Developer Name: @MayankBansal12
Issue Ticket Number:-
Description:
Is Under Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Tested in staging?
Add relevant Screenshot below ( e.g test coverage etc. )
screencast
Screencast.from.2026-02-21.19-06-10.mp4
tests