fix: resolve all pre-existing backend eslint errors and add eslint config - #843
fix: resolve all pre-existing backend eslint errors and add eslint config#843Rudra-clrscr wants to merge 10 commits into
Conversation
…nfig
Adds backend/eslint.config.js (none existed before) and fixes every error
it surfaces except middleware/fileValidation.js (separately tracked, in
progress). Several of these were one-off typos or single duplicate
lines; a few turned out to be much larger merge-corruption issues.
Small fixes:
- config/index.js: `configs.development` -> `config.development` typo.
- utils/emailRules.js: `matchingRule.type` -> `matchedType` (the actual
variable holding the matched rule's type in that scope).
- utils/fileValidation.js: drop an unnecessary regex escape.
- utils/utils/emailTransporter.js -> utils/emailTransporter.js: fix a
duplicated-directory path bug (authController.js requires
'../utils/emailTransporter', which never existed at that path).
- server.js: fix an empty catch block (log the error instead of
silently swallowing it) and remove a require for
'./routes/visualRoutes', a file that never existed anywhere in the
repo (dead "VBSF routes" stub, not implemented on the Node side).
Larger fixes (duplicate/corrupted code from bad merges):
- middleware/adversarialGuard.js: removed a duplicated
`detectAdversarialPatterns` call, a duplicated `adversarialGuard`
function stub, a duplicated outer `if` in `monitorConfidence`, and two
`catch` clauses attached to the same `try` - all leftover copies with
a working original alongside them.
- server.js: removed a broken, never-closed `logStartupTime` stub and
its duplicated EvoMail/startup-timer requires that preceded the real,
complete versions; moved the EvoMail/visual route `app.use()` calls to
after `const app = express()` is defined (they previously ran before
`app` existed, which would throw at runtime once the file parsed).
- routes/authRoutes.js: this file had ~11 controller functions
(register, login, logout, getMe, googleLogin, updateAvatar,
forgotPassword, resetPassword, changePassword, updateWebhook,
getSessionStatus) each duplicated 2-3x (verified byte-identical
copies), plus assignRole/getUserPermissions/getRolesAndPermissions
whose bodies were corrupted/incomplete in every copy. More importantly,
despite creating an Express Router, the file never once called
router.get/post() - it only exported bare handler functions - while
server.js did `app.use("/api/v1/auth", authRoutes)`, passing a plain
object where Express requires a router/middleware function. The whole
auth API was not actually routed. Rewrote the file as a thin router
that imports the (already complete, working) handlers from
controllers/authController.js and wires each one to its documented
`@route` (recovered from the file's own JSDoc comments) with the
appropriate validator/rate-limiter/protect middleware, then exports
the router. Deleted routes/prediction.route.js, an orphaned, never-
required duplicate of predictionRoutes.js's /stats route that
referenced a `Prediction` model that doesn't exist anywhere in the
codebase; fixed predictionRoutes.js's own /stats route (same bug) to
use the History model it already imports.
Verified: `node --check` on every touched file, `npx eslint .` (1
remaining error is middleware/fileValidation.js, tracked separately),
a full server.js boot with dummy env vars (previously impossible - it
couldn't get past its first require), and the existing Jest (16/16) and
node:test (26/26) suites still pass.
Left as pre-existing, separately-tracked issues (not touched here):
- middleware/fileValidation.js's own parsing error (your in-progress
work).
- services/otpService.js exports rate limiters instead of OTP
functions - unrelated corruption discovered while tracing
authRoutes.js's dead requestOTP/verifyOTP/getOTPStatus imports (no
working implementation of those three exists anywhere, so they were
dropped from the new router rather than inventing the feature).
- 63 remaining eslint warnings (mostly unused vars/imports across many
files) left non-blocking, consistent with the frontend cleanup PR.
|
Someone is attempting to deploy a commit to the Aditya Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Userunknown84 this is an extra PR needed for solving CI issue |
There was a problem hiding this comment.
Pull request overview
This PR introduces a backend ESLint flat config and fixes a large set of pre-existing backend issues surfaced by linting and merge corruption, including making auth routing functional and removing dead/broken server code.
Changes:
- Add
backend/eslint.config.js, add a backendlintscript, and introduce ESLint dependencies. - Repair backend runtime wiring issues (auth router export/wiring, server startup ordering, dead route removal).
- Fix correctness issues in prediction stats and various utility/middleware cleanups.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/utils/fileValidation.js | Regex cleanup for CSV formula-injection guard. |
| backend/utils/emailTransporter.js | Adds a centralized Nodemailer transporter module. |
| backend/utils/emailRules.js | Fixes rule-applied field to use the matched rule type variable. |
| backend/server.js | Removes broken/duplicated startup stubs, fixes route registration order, improves error logging. |
| backend/routes/predictionRoutes.js | Fixes /stats to use History model fields; minor handler restructuring. |
| backend/routes/prediction.route.js | Deletes an orphaned duplicate stats route file. |
| backend/routes/authRoutes.js | Replaces corrupted/duplicated handlers with a proper Express router wiring controllers + middleware. |
| backend/package.json | Adds lint script and ESLint-related devDependencies. |
| backend/package-lock.json | Locks ESLint-related dependencies. |
| backend/middleware/adversarialGuard.js | Removes duplicated/corrupted code and replaces an empty catch. |
| backend/eslint.config.js | Adds ESLint flat config for backend. |
| backend/config/index.js | Fixes fallback config typo (configs.development → config.development). |
Files not reviewed (1)
- backend/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const multer = require('multer'); | ||
| const upload = multer(); |
| res.status(500).json({ error: 'Server error. Please try again.' }); | ||
| } | ||
| }; | ||
| router.post('/avatar', protect, upload.single('avatar'), updateAvatar); |
| "devDependencies": { | ||
| "@eslint/js": "^10.0.1", | ||
| "eslint": "^10.7.0", | ||
| "globals": "^17.7.0", | ||
| "jest": "^30.4.2", | ||
| "supertest": "^7.2.2" | ||
| } |
| } | ||
| } | ||
| } catch (err) { | ||
| logger.error('[DB Pool] Failed to log connection pool stats:', err.message); |
The container's Flask process defaults to FLASK_HOST=127.0.0.1 (a deliberate secure default for bare-metal runs), so inside the CI container it never listens on an interface reachable through the -p 5000:5000 port mapping, and the smoke test's curl always times out. docker-compose.yml already sets FLASK_HOST=0.0.0.0 for the same reason; the CI workflow's docker run step needs the same override.
main's copies of these files have a duplicate 'function App()' declaration and a stray '#Manipulation' line before an import respectively - both invalid JS that break 'vite build' in the frontend Docker image stage. Only fixed inside the open fix/frontend-eslint-errors PR branch, never merged to main, so any branch cut from main (including this one) inherits them. Pulling in the already-verified fix.
App.jsx and ManipulationIndex.jsx changes pulled in a Docker-CI-only fix from an unrelated PR, making this PR's diff show large unrelated deletions. Reverting so review stays focused on this PR's actual scope. The Docker frontend-build check will fail again until the frontend-eslint-errors (or automated-test-suite) PR merges first - expected, since main itself still carries the parse bugs those PRs fix.
…ibility eslint 10.x's dependency chain declares Node >=20.19.0 or >=22.13.0. '20.x' in setup-node doesn't pin a minimum patch, so a runner could in principle resolve an older 20.x than required and fail npm ci/lint in a way that's runner-dependent rather than reproducible. Pinning to 22.x and declaring engines.node in package.json makes the requirement explicit for both CI and contributors. Left continue-on-error: true on the lint steps as-is per discussion - main's lint baseline isn't clean yet (that's what Userunknown84#842/Userunknown84#843 fix), so hard-failing now would block every PR until those merge.
- predictionRoutes.js: destructuring req.body before the try block crashed with a TypeError (bypassing error handling, returning an unhandled 500) whenever the body was missing/non-JSON. Moved the destructure inside try, declared the vars ahead of it so the catch block's Sentry context still has access to them. - authRoutes.js: the /avatar route used a bare multer() instance, bypassing the size/MIME/content-sniffing checks already implemented in middleware/avatarUpload.js. Wired up handleAvatarUpload instead. That middleware requires 'file-type', which was never declared in package.json - added it (Node 22's require(esm) support, matching this PR's own Node pin below, means the plain require() still works despite file-type now being ESM-only upstream). - backend-ci.yml: dropped EOL Node 18.x from the Jest matrix (added 24.x) and declared engines.node in package.json, since this PR's own eslint 10.x devDependency requires Node >=20.19.0 or >=22.13.0 and would have been the first thing to break the 18.x leg. - server.js: pool-monitor error logging dropped the stack via err.message; log the full error instead so failures are diagnosable.
…errors # Conflicts: # .github/workflows/docker.yml # backend/package-lock.json
…I parse errors The earlier fix for the Docker-build App.jsx parse error replaced this file with a version that also deleted a large amount of unrelated, working functionality (streak/badge gamification, dark mode toggle state, detectURLs helper, ResultBadge import, etc.) - flagged by @Userunknown84 as unwanted deletions, not a cleanup this PR asked for. Restored all of that content. The actual parse-breaking bugs on main were narrower than first diagnosed: - A stray, erroneous 'function App() {' line (left over from an old 'Add Back to Top button' merge) split handlePredict's try/finally in half instead of opening a real second component. Removed the stray line and restored handlePredict's own missing closing braces. - The outermost wrapping <div> was missing its closing tag at the very end of the file (3 closing </div> where 4 were needed around <Footer>/<Chatbot>). Added it back. Also fixed two more pre-existing, genuine bugs uncovered while getting this to parse and actually render correctly: - Emoji sentiment analysis called analyzeEmojis(text), but the function is defined as analyzeEmojiSentiment - a ReferenceError crash any time a result renders. Fixed the call sites to match, and memoized the (now correctly-named) call via useMemo instead of invoking it up to 8 times per render. - The emoji-detection regex was missing the 'u' flag, so astral-plane emoji (e.g. U+1F600) never matched the surrogate-pair character class.
…pp.jsx state Matches the same eslint-disable-next-line comments added on the frontend-eslint-errors PR, keeping App.jsx byte-identical across branches so merging any subset of these PRs stays conflict-free.
Userunknown84
left a comment
There was a problem hiding this comment.
AuthRoutes.js ka code delete ho rhaa h rest is fine
|
@Rudra-clrscr fix the conflicts. |
The middleware was tracked under two case-variant paths (fileValidation.js and filevalidation.js) left over from an earlier merge, which are the same file on case-insensitive filesystems and caused spurious local diffs on checkout. Keep the single canonical lowercase path (matching upstream/main) with the clean, deduplicated content, and fix the one test import that referenced the camelCase path so it resolves correctly on case-sensitive filesystems too.
…errors # Conflicts: # backend/package.json # backend/routes/predictionRoutes.js # backend/server.js # backend/tests/fileValidation.test.js # frontend/src/pages/App.jsx
Adds backend/eslint.config.js (none existed before) and fixes every error it surfaces except middleware/fileValidation.js (separately tracked, in progress). Several of these were one-off typos or single duplicate lines; a few turned out to be much larger merge-corruption issues.
Small fixes:
configs.development->config.developmenttypo.matchingRule.type->matchedType(the actual variable holding the matched rule's type in that scope).Larger fixes (duplicate/corrupted code from bad merges):
detectAdversarialPatternscall, a duplicatedadversarialGuardfunction stub, a duplicated outerifinmonitorConfidence, and twocatchclauses attached to the sametry- all leftover copies with a working original alongside them.logStartupTimestub and its duplicated EvoMail/startup-timer requires that preceded the real, complete versions; moved the EvoMail/visual routeapp.use()calls to afterconst app = express()is defined (they previously ran beforeappexisted, which would throw at runtime once the file parsed).app.use("/api/v1/auth", authRoutes), passing a plain object where Express requires a router/middleware function. The whole auth API was not actually routed. Rewrote the file as a thin router that imports the (already complete, working) handlers from controllers/authController.js and wires each one to its documented@route(recovered from the file's own JSDoc comments) with the appropriate validator/rate-limiter/protect middleware, then exports the router. Deleted routes/prediction.route.js, an orphaned, never- required duplicate of predictionRoutes.js's /stats route that referenced aPredictionmodel that doesn't exist anywhere in the codebase; fixed predictionRoutes.js's own /stats route (same bug) to use the History model it already imports.Verified:
node --checkon every touched file,npx eslint .(1 remaining error is middleware/fileValidation.js, tracked separately), a full server.js boot with dummy env vars (previously impossible - it couldn't get past its first require), and the existing Jest (16/16) and node:test (26/26) suites still pass.Left as pre-existing, separately-tracked issues (not touched here):