feat: add applicant image upload functionality#2564
feat: add applicant image upload functionality#2564AnujChhikara wants to merge 4 commits intodevelopfrom
Conversation
WalkthroughA new application picture upload feature is added, including error message constants, Cloudinary configuration, a POST endpoint with file validation middleware, image service integration for Cloudinary uploads, and comprehensive test coverage for both success and failure scenarios. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant Authenticate as Auth Middleware
participant ValidatePic as Validator Middleware
participant Controller
participant ImageService
participant Cloudinary
Client->>Router: POST /applications/picture (file, auth token)
Router->>Authenticate: Check authentication
Authenticate->>Router: User verified
Router->>ValidatePic: Validate file
ValidatePic->>ValidatePic: Check file presence & MIME type
ValidatePic->>Router: Validation passed
Router->>Controller: Forward request
Controller->>ImageService: uploadApplicationImage({file, userId})
ImageService->>ImageService: Encode to Base64 Data URI
ImageService->>Cloudinary: Upload with public_id
Cloudinary-->>ImageService: Return publicId & URL
ImageService-->>Controller: Return {publicId, url}
Controller-->>Client: 201 Created {message, image}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
| ); | ||
| router.post( | ||
| "/picture", | ||
| authenticate, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 20 hours ago
In general, to fix this problem you introduce a rate-limiting middleware so that expensive or security-sensitive endpoints cannot be called arbitrarily often. In an Express app, this is typically done with a library such as express-rate-limit, configuring a reasonable window and maximum number of requests, and then inserting that middleware either globally or on specific routes.
The best fix here, without changing existing functionality, is to add express-rate-limit in this router file and apply a limiter specifically to the /picture POST route (and optionally other modifying endpoints if desired). This keeps the existing authentication, validation, upload, and controller behavior intact, but ensures that a single client cannot flood the server with repeated picture uploads. Concretely: in routes/applications.ts, add a require("express-rate-limit") at the top, define a limiter instance (e.g., const pictureRateLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 100 }); or a stricter value suitable for file uploads), and then insert pictureRateLimiter as a middleware in the router.post("/picture", ...) chain immediately after the path and before authenticate. No other routes or behavior need to be changed.
| @@ -1,4 +1,5 @@ | ||
| const express = require("express"); | ||
| const rateLimit = require("express-rate-limit"); | ||
| const { SUPERUSER } = require("../constants/roles"); | ||
| const authenticate = require("../middlewares/authenticate"); | ||
| const authorizeRoles = require("../middlewares/authorizeRoles"); | ||
| @@ -9,6 +10,11 @@ | ||
|
|
||
| const router = express.Router(); | ||
|
|
||
| const pictureRateLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 picture upload requests per windowMs | ||
| }); | ||
|
|
||
| router.get( | ||
| "/", | ||
| authenticate, | ||
| @@ -18,6 +24,7 @@ | ||
| ); | ||
| router.post( | ||
| "/picture", | ||
| pictureRateLimiter, | ||
| authenticate, | ||
| upload.single("profile"), | ||
| applicationValidator.validateApplicationPicture, |
| @@ -42,7 +42,8 @@ | ||
| "passport-github2": "0.1.12", | ||
| "passport-google-oauth20": "^2.0.0", | ||
| "rate-limiter-flexible": "5.0.3", | ||
| "winston": "3.13.0" | ||
| "winston": "3.13.0", | ||
| "express-rate-limit": "^8.2.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/chai": "4.3.16", |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
|
@coderabbitai review bro |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@routes/applications.ts`:
- Around line 19-25: The POST /picture route lacks rate limiting—add the
existing commonRateLimiter middleware into the router.post middleware chain
before the upload handler so requests hit rate limits; specifically update the
router.post call that currently lists authenticate, upload.single("profile"),
applicationValidator.validateApplicationPicture, and
applications.postUserPicture to include commonRateLimiter (imported from
middlewares/rateLimiting.js) in the middleware sequence (e.g., after
authenticate and before upload.single) so the route uses commonRateLimiter and
still calls applicationValidator.validateApplicationPicture and
applications.postUserPicture.
In `@test/integration/application.test.ts`:
- Around line 361-397: Add integration tests for the validation/error paths
around the POST /applications/picture route: add a test that sends no attachment
and asserts the response is the expected validation error from
validateApplicationPicture (e.g., 400 and appropriate error message), a test
that attaches an invalid file type (e.g., .txt or .gif) and asserts the
middleware returns the correct validation response, and a test where
sinon.stub(imageService, "uploadApplicationImage") is made to reject to assert
the route returns the service error (status/code and message) and is handled
correctly; place these alongside the existing tests in application.test.ts and
reuse the same auth cookie setup (cookieName/secondUserJwt) and
mockUploadResponse reference but adjust stubbing/attachment to trigger each
path.
- Around line 381-396: The test stubs imageService.uploadApplicationImage but
never restores it, risking test pollution; update the test suite to restore the
stub after the case by adding an afterEach that calls sinon.restore() (or
explicitly calls uploadApplicationImage.restore() if you capture the stub
variable) so the stub created in the "should return 201 and image data when
authenticated with valid image" test is removed before other tests run; locate
the stub usage in the test that posts to "/applications/picture" and add the
afterEach cleanup alongside other per-test cleanup blocks.
36bf39e to
856f32d
Compare
856f32d to
3b9eca5
Compare
Date: 17 Feb, 2026
Developer Name: @AnujChhikara
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Screen.Recording.2026-02-03.at.1.50.48.AM.mov
Test Coverage
Screenshot 1
Additional Notes