refactor: extract shared utilities from duplicated code patterns#2
Open
devin-ai-integration[bot] wants to merge 3 commits into
Open
refactor: extract shared utilities from duplicated code patterns#2devin-ai-integration[bot] wants to merge 3 commits into
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent 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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds standardized response helpers and an async wrapper, a request validation middleware, integrates them into auth and ticket controllers/routes and the health endpoint; extracts a reusable IconLink component and updates App.jsx to render documentation and social links with it. ChangesBackend Error Handling & Auth Refactoring
Frontend Link Component Extraction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (3)
frontend/src/App.jsx (2)
62-64: ⚡ Quick winPrefer a stable key over
label.Using
link.labelas the React key works since labels are currently unique, but it's fragile—if labels change for internationalization or become non-unique, React's reconciliation will break. Uselink.href(guaranteed unique per destination) or the array index instead.♻️ Proposed refactor
- {socialLinks.map((link) => ( - <IconLink key={link.label} {...link} /> + {socialLinks.map((link) => ( + <IconLink key={link.href} {...link} />🤖 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 `@frontend/src/App.jsx` around lines 62 - 64, The React list is using link.label as the key in the socialLinks.map when rendering <IconLink>, which is fragile; change the key to a stable unique identifier such as link.href (or fallback to the array index) by updating the map callback to use key={link.href} (or key={i} if you must) so IconLink renders with a stable unique key.
8-13: SVG sprite icon IDs are present; minor key stability tweak
frontend/public/icons.svgdefinesgithub-icon,discord-icon,x-icon, andbluesky-icon, so the social link icons should render correctly.- In
frontend/src/App.jsx, use a more stable Reactkeythanlink.labelforsocialLinks.map(e.g.,href).🤖 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 `@frontend/src/App.jsx` around lines 8 - 13, The socialLinks array in frontend/src/App.jsx is mapped to render icons using SVG IDs but the React list key uses link.label which can be unstable; update the map in the component that iterates socialLinks (the socialLinks.map callback) to use a more stable unique key such as link.href (or another unique identifier) instead of link.label so React list reconciliation is reliable.backend/src/utils/catchAsync.js (1)
3-8: ⚡ Quick winGuard against
res.headersSentbefore sending the 500.If a wrapped handler throws after it has already started/sent a response, the
.catchwill attempt a second write throughsendError, triggeringERR_HTTP_HEADERS_SENTand masking the original error. Forward to Express in that case.♻️ Proposed guard
const catchAsync = (fn) => (req, res, next) => { Promise.resolve(fn(req, res, next)).catch((error) => { console.error(`${fn.name || 'Handler'} error:`, error); + if (res.headersSent) { + return next(error); + } sendError(res, 500, 'An internal server error occurred.'); }); };🤖 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/src/utils/catchAsync.js` around lines 3 - 8, The catchAsync wrapper should check res.headersSent before calling sendError to avoid writing headers twice; modify catchAsync (the Promise.resolve(...).catch block) to, on error, log the error as it currently does and then if res.headersSent call next(error) to forward to Express, otherwise call sendError(res, 500, 'An internal server error occurred.'); ensure you reference the existing fn (handler) and next parameters so the error is forwarded when headers were already sent.
🤖 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/middleware/validate.js`:
- Line 4: The validation currently does requiredFields.filter((field) =>
!req.body[field]) and will throw if req.body is undefined; change the middleware
(the validate function/middleware where requiredFields.filter is used) to guard
by defaulting req.body to an empty object (e.g., const body = req.body || {})
and use body[field] in the filter so missing/empty request bodies return the
expected 400 instead of causing a 500.
In `@frontend/src/components/IconLink.jsx`:
- Line 5: The anchor in IconLink.jsx opens external links with target="_blank"
but omits rel attributes; update the <a> element used in the IconLink component
to include rel="noopener noreferrer" alongside target="_blank" (ensuring this is
applied wherever the component renders the anchor and when props.href points to
external URLs) so you prevent window.opener tabnabbing and referrer leakage.
---
Nitpick comments:
In `@backend/src/utils/catchAsync.js`:
- Around line 3-8: The catchAsync wrapper should check res.headersSent before
calling sendError to avoid writing headers twice; modify catchAsync (the
Promise.resolve(...).catch block) to, on error, log the error as it currently
does and then if res.headersSent call next(error) to forward to Express,
otherwise call sendError(res, 500, 'An internal server error occurred.'); ensure
you reference the existing fn (handler) and next parameters so the error is
forwarded when headers were already sent.
In `@frontend/src/App.jsx`:
- Around line 62-64: The React list is using link.label as the key in the
socialLinks.map when rendering <IconLink>, which is fragile; change the key to a
stable unique identifier such as link.href (or fallback to the array index) by
updating the map callback to use key={link.href} (or key={i} if you must) so
IconLink renders with a stable unique key.
- Around line 8-13: The socialLinks array in frontend/src/App.jsx is mapped to
render icons using SVG IDs but the React list key uses link.label which can be
unstable; update the map in the component that iterates socialLinks (the
socialLinks.map callback) to use a more stable unique key such as link.href (or
another unique identifier) instead of link.label so React list reconciliation is
reliable.
🪄 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: ab811171-f143-4df0-aa74-db9eb7cbdcdc
📒 Files selected for processing (8)
backend/src/app.jsbackend/src/controllers/auth.controller.jsbackend/src/middleware/validate.jsbackend/src/routes/auth.routes.jsbackend/src/utils/catchAsync.jsbackend/src/utils/response.jsfrontend/src/App.jsxfrontend/src/components/IconLink.jsx
Backend: - Add utils/response.js (sendSuccess, sendError) to centralize API responses - Add utils/catchAsync.js to eliminate duplicated try-catch error handling - Add middleware/validate.js (validateFields) to DRY input validation - Refactor auth.controller.js and auth.routes.js to use shared utilities - Refactor app.js health check to use response utility Frontend: - Add components/IconLink.jsx for reusable icon+link pattern - Refactor App.jsx to use IconLink component, removing 6 repetitive blocks - Extract social links data into a declarative array Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
Bot
force-pushed
the
devin/1780488315-refactor-shared-utilities
branch
from
June 4, 2026 17:43
dc5f697 to
a72b181
Compare
- Guard against undefined req.body in validateFields middleware - Add rel='noopener noreferrer' to IconLink for security - Use link.href as stable React key instead of link.label Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
Author
|
Addressed all CodeRabbit feedback in f7d4d1c:
|
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 `@backend/src/controllers/ticket.controller.js`:
- Line 2: Add 'uuid' as a direct runtime dependency in backend/package.json
because ticket.controller.js requires it (const { v4: uuidv4 } =
require('uuid')). Update the "dependencies" section to include "uuid" (pick a
compatible semver, e.g. ^8.3.2 or the latest stable), then run the package
manager (npm install --save uuid or yarn add uuid) to update package-lock.json;
this ensures require('uuid') in ticket.controller.js will always resolve.
🪄 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: 6011cb32-7989-431e-b6f4-ed141f9cd8a1
📒 Files selected for processing (11)
backend/src/app.jsbackend/src/controllers/auth.controller.jsbackend/src/controllers/ticket.controller.jsbackend/src/middleware/auth.jsbackend/src/middleware/validate.jsbackend/src/routes/auth.routes.jsbackend/src/routes/ticket.routes.jsbackend/src/utils/catchAsync.jsbackend/src/utils/response.jsfrontend/src/App.jsxfrontend/src/components/IconLink.jsx
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/src/utils/catchAsync.js
- backend/src/utils/response.js
- backend/src/middleware/validate.js
- frontend/src/App.jsx
- backend/src/routes/auth.routes.js
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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
1 participant
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
Eliminates duplicated code across backend controllers, middleware, and frontend by extracting shared utilities. All 100 existing tests pass.
Backend — three new modules replace repeated inline patterns across all controllers and middleware:
utils/response.js→sendSuccess(res, code, msg, data)/sendError(res, code, msg)replaces every manualres.status().json({ success, message })call (~15 occurrences acrossauth.controller,ticket.controller,authmiddleware, andapp.js)utils/catchAsync.js→ wraps async handlers with configurable error messages, eliminating duplicated try/catch +console.error+ 500-response boilerplate in every controller (5 handlers)middleware/validate.js→validateFields(fields)route-level middleware for declarative field validationRefactored files:
auth.controller.js,ticket.controller.js,auth.routes.js,ticket.routes.js,middleware/auth.js,app.jsFrontend — reusable
IconLinkcomponent:components/IconLink.jsxextracts the repeated<li><a><svg|img>...</a></li>pattern (6 occurrences → 1 component + declarativesocialLinksarray)App.jsx: 122 → 75 linesLink to Devin session: https://app.devin.ai/sessions/8015016401f24d2698c63a0bddf8495d
Requested by: @0xpritam
Summary by CodeRabbit
New Features
Bug Fixes
UI/Style
Chores