feat: MongoDB auth, session history, and full-width UI#231
Open
CodeByAlok24 wants to merge 5 commits into
Open
feat: MongoDB auth, session history, and full-width UI#231CodeByAlok24 wants to merge 5 commits into
CodeByAlok24 wants to merge 5 commits into
Conversation
❌ Deploy Preview for github-spy failed.
|
Contributor
📝 WalkthroughWalkthroughThis PR restructures the project to a frontend-first layout, moving ChangesFrontend Project Structure Reorganization
GitHub Activity Tracker with Search History
Contributors Discovery and Landing Page
Backend Security and Configuration
Documentation and Test Infrastructure
Sequence Diagram(s)sequenceDiagram
participant User as User Browser
participant Frontend as Frontend App
participant Backend as Backend API
participant GitHub as GitHub API
participant DB as MongoDB
User->>Frontend: Login & Enter GitHub Token
Frontend->>Backend: POST /auth/login (credentials)
Backend->>DB: Verify User
DB-->>Backend: User Found
Backend-->>Frontend: Session Cookie Set
Frontend->>Frontend: Redirect to /track
User->>Frontend: Enter Username to Search
Frontend->>Frontend: Load History from Backend
Backend->>DB: GET User Tracker History
DB-->>Backend: Return Last 10 Searches
Backend-->>Frontend: Return History
Frontend->>GitHub: Search Issues/PRs (token)
GitHub-->>Frontend: Return Results (paginated)
Frontend->>Frontend: Filter by State/Repo/Date
User->>Frontend: Select Repository & Filters
Frontend->>Frontend: Recompute Filtered Results
Frontend->>User: Display Filtered Items + Pagination
Frontend->>Backend: POST New Search (username, timestamp)
Backend->>DB: Update User.trackerHistory
DB-->>Backend: Deduplicate & Truncate to 10
Backend-->>Frontend: Return Updated History
Frontend->>Frontend: Update localStorage & UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @CodeByAlok24 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
backend/models/User.js (1)
19-22: ⚡ Quick winConsider defining a more specific schema for trackerHistory.
The
trackerHistoryfield uses a genericArraytype, which provides no validation or documentation of the expected structure. Consider defining a subdocument schema or at minimum specifying the array item type.💡 Example of a more specific schema
+const TrackerHistoryItemSchema = new mongoose.Schema({ + username: { type: String, required: true }, + searchedAt: { type: Date, default: Date.now } +}, { _id: false }); + const UserSchema = new mongoose.Schema({ username: { type: String, required: true, unique: true, }, email: { type: String, required: true, unique: true, }, password: { type: String, required: true, }, trackerHistory: { - type: Array, + type: [TrackerHistoryItemSchema], default: [], },Adjust the subdocument fields based on what's actually stored in the tracker history.
🤖 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/models/User.js` around lines 19 - 22, The trackerHistory field in User.js is declared as a generic Array which provides no validation; replace it with a specific subdocument schema (e.g., create a TrackerHistorySchema or inline schema) and use an array of that schema for trackerHistory (for example an array of objects with fields like event:String, details:Mixed or Object, timestamp:Date, userAgent:String as appropriate); update the User schema to use trackerHistory: { type: [TrackerHistorySchema], default: [] } (or equivalent inline definition) so Mongoose validates items and documents the expected structure.
🤖 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/routes/auth.js`:
- Around line 60-74: Validate and sanitize req.body.username and
req.body.searchedAt before using them: ensure username exists and is a non-empty
string, and ensure searchedAt is a valid date (or omit/replace with Date.now()
if invalid). In the route where you fetch User (User.findById and then mutate
user.trackerHistory), add checks that username is typeof string and trim() !==
"" and that new Date(searchedAt) is valid (isNaN(Date.parse(...)) check) and
return a 400 with a clear message if validation fails; only then perform the
existing trackerHistory filtering (user.trackerHistory.filter(...)) and unshift
with a normalized { username: username.trim(), searchedAt: new Date(validDate) }
entry.
In `@backend/server.js`:
- Around line 26-30: Update the session cookie and proxy settings to be
environment-aware: call app.set('trust proxy', 1) during server setup, and in
the session middleware's cookie config (the cookie object currently containing
httpOnly/secure/sameSite) set secure = (process.env.NODE_ENV === 'production')
and set sameSite = 'none' when in production and FRONTEND_URL is cross-origin
(otherwise keep 'lax'); ensure secure is true in production so cookies require
HTTPS and adjust sameSite to 'none' when sending credentials across origins. Use
the existing session middleware cookie object and the app.set('trust proxy', 1)
symbol names to locate where to apply these changes.
In `@frontend/package.json`:
- Line 49: Remove the backend runtime packages mistakenly declared in frontend's
devDependencies: delete the entries for "express", "mongoose",
"express-session", "passport", "passport-local", "bcryptjs", and "supertest"
from frontend/package.json so only frontend-related packages remain; locate the
devDependencies block in frontend/package.json and remove those package keys
(ensure you don't touch backend/package.json where these belong) and then run
npm/yarn install to update lockfile.
- Line 10: The frontend package.json currently defines the "test" script
("test": "jasmine") and lists jasmine and `@types/jasmine` in devDependencies
though those tests are backend specs; remove the "test" script and
jasmine-related devDependencies from frontend/package.json and add them into
backend/package.json instead: add the "test": "jasmine" script to
backend/package.json and move jasmine and `@types/jasmine` into backend's
devDependencies so backend runs the Jasmine specs (refer to the "test" script
and the jasmine/@types/jasmine entries when updating the two package.json
files).
In `@frontend/src/hooks/useGitHubData.ts`:
- Around line 77-83: The hook useGitHubData currently sets rateLimited to true
on a 403 and the existing guard then blocks all future fetches with no recovery;
modify the 403 handling to parse a Retry-After header or X-RateLimit-Reset (or
fall back to a sensible default like 60s), call setRateLimited(true) but
schedule a timer to call setRateLimited(false) after that delay, and ensure the
timer is stored in a ref and cleared on unmount or when a new rate-limit event
occurs; this uses the existing symbols rateLimited, setRateLimited, and the 403
catch branch so the guard that prevents fetches will automatically resume after
the timeout.
In `@frontend/src/pages/Signup/Signup.tsx`:
- Around line 36-38: The redirect currently depends on response.data.message
string match; change the check to use an explicit status or boolean from the API
(e.g., response.status === 201 or response.data.success === true) before calling
navigate("/login") in the signup submit handler (the block that currently
inspects response.data.message and calls navigate). Update the API contract
usage to prefer response.status or response.data.success and handle non-success
cases accordingly.
In `@frontend/src/pages/Tracker/Tracker.tsx`:
- Around line 192-193: The end-date check in matchesEnd uses new Date(endDate)
which is midnight and thus excludes items later that day; update the logic to
compare createdAt against the end-of-day timestamp (e.g., compute an endOfDay
from endDate by setting hours to 23:59:59.999 or by creating nextDay = new
Date(endDate) + 1 day and using createdAt < nextDay) so matchesEnd becomes true
for any createdAt on the selected endDate; keep matchesStart as-is using
startDate and createdAt.
- Around line 160-164: The effect currently re-runs on every username (and tab)
change causing API calls on each keystroke; change it to only run for deliberate
fetches by introducing a stable "submittedUsername" state (e.g.,
submittedUsername) that is set in the form submit handler instead of using raw
username, then update the effect to depend on submittedUsername and page (and
not username/tab) and call fetchData(submittedUsername, page + 1,
ROWS_PER_PAGE); alternatively implement a debounce that sets submittedUsername
after a short pause—update handlers that trigger fetches to set
submittedUsername so fetches only occur on submit/debounced value, not on every
keystroke.
In `@README.md`:
- Around line 92-96: The README currently contains a hardcoded MongoDB
connection string (MONGO_URI) with embedded credentials; replace that value with
a placeholder (e.g.,
MONGO_URI=mongodb+srv://<DB_USER>:<DB_PASS>@<HOST>/<DB_NAME>?retryWrites=true&w=majority)
and update the example block so no real secrets appear, and add a short note to
rotate/revoke the exposed credentials immediately; locate the example using the
MONGO_URI token and update it accordingly.
---
Nitpick comments:
In `@backend/models/User.js`:
- Around line 19-22: The trackerHistory field in User.js is declared as a
generic Array which provides no validation; replace it with a specific
subdocument schema (e.g., create a TrackerHistorySchema or inline schema) and
use an array of that schema for trackerHistory (for example an array of objects
with fields like event:String, details:Mixed or Object, timestamp:Date,
userAgent:String as appropriate); update the User schema to use trackerHistory:
{ type: [TrackerHistorySchema], default: [] } (or equivalent inline definition)
so Mongoose validates items and documents the expected structure.
🪄 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
Run ID: d5596297-a7d9-4847-8620-672de32ed83d
⛔ Files ignored due to path filters (4)
frontend/public/crl-icon.pngis excluded by!**/*.pngfrontend/public/crl.pngis excluded by!**/*.pngfrontend/public/vite.svgis excluded by!**/*.svgfrontend/src/assets/react.svgis excluded by!**/*.svg
📒 Files selected for processing (52)
.env.exampleREADME.mdbackend/.env.samplebackend/models/User.jsbackend/routes/auth.jsbackend/server.jsdocker-compose.ymlfrontend/Dockerfile.devfrontend/Dockerfile.prodfrontend/eslint.config.jsfrontend/index.htmlfrontend/package.jsonfrontend/postcss.config.cjsfrontend/public/_redirectsfrontend/src/App.cssfrontend/src/App.tsxfrontend/src/Routes/Router.tsxfrontend/src/components/Features.tsxfrontend/src/components/Footer.tsxfrontend/src/components/Hero.tsxfrontend/src/components/HowItWorks.tsxfrontend/src/components/Navbar.tsxfrontend/src/components/ScrollProgressBar.tsxfrontend/src/context/ThemeContext.tsxfrontend/src/context/theme.tsfrontend/src/hooks/useGitHubAuth.tsfrontend/src/hooks/useGitHubData.tsfrontend/src/index.cssfrontend/src/main.tsxfrontend/src/pages/About/About.tsxfrontend/src/pages/Contact/Contact.tsxfrontend/src/pages/ContributorProfile/ContributorProfile.tsxfrontend/src/pages/Contributors/Contributors.tsxfrontend/src/pages/Home/Home.tsxfrontend/src/pages/Login/Login.tsxfrontend/src/pages/Signup/Signup.tsxfrontend/src/pages/Tracker/Tracker.tsxfrontend/src/utils/constants.tsfrontend/src/vite-env.d.tsfrontend/tailwind.config.jsfrontend/tsconfig.app.jsonfrontend/tsconfig.jsonfrontend/tsconfig.node.jsonfrontend/vite.config.tsspec/auth.routes.spec.cjsspec/user.model.spec.cjssrc/components/Features.tsxsrc/components/Footer.tsxsrc/components/Hero.tsxsrc/components/HowItWorks.tsxsrc/pages/Contributors/Contributors.tsxsrc/pages/Tracker/Tracker.tsx
💤 Files with no reviewable changes (8)
- backend/.env.sample
- src/components/Features.tsx
- src/components/Hero.tsx
- .env.example
- src/components/Footer.tsx
- src/components/HowItWorks.tsx
- src/pages/Tracker/Tracker.tsx
- src/pages/Contributors/Contributors.tsx
Comment on lines
+60
to
+74
| const { username, searchedAt } = req.body; | ||
| try { | ||
| const user = await User.findById(req.user.id); | ||
| if (!user) { | ||
| return res.status(404).json({ message: 'User not found' }); | ||
| } | ||
|
|
||
| // Remove duplicate if exists (keep only unique searches) | ||
| user.trackerHistory = user.trackerHistory.filter( | ||
| item => item.username.toLowerCase() !== username.toLowerCase() | ||
| ); | ||
|
|
||
| // Add new search at the beginning | ||
| user.trackerHistory.unshift({ username, searchedAt: new Date(searchedAt) }); | ||
|
|
Contributor
There was a problem hiding this comment.
Validate username and searchedAt before processing history.
The route can throw when username is missing/non-string, and invalid searchedAt is not rejected. This turns client input errors into 500s.
Suggested patch
const { username, searchedAt } = req.body;
+ if (typeof username !== "string" || !username.trim()) {
+ return res.status(400).json({ message: "username is required" });
+ }
+ const parsedSearchedAt = searchedAt ? new Date(searchedAt) : new Date();
+ if (Number.isNaN(parsedSearchedAt.getTime())) {
+ return res.status(400).json({ message: "searchedAt must be a valid date" });
+ }
try {
const user = await User.findById(req.user.id);
if (!user) {
return res.status(404).json({ message: 'User not found' });
}
// Remove duplicate if exists (keep only unique searches)
user.trackerHistory = user.trackerHistory.filter(
- item => item.username.toLowerCase() !== username.toLowerCase()
+ item => item.username?.toLowerCase() !== username.toLowerCase()
);
// Add new search at the beginning
- user.trackerHistory.unshift({ username, searchedAt: new Date(searchedAt) });
+ user.trackerHistory.unshift({ username, searchedAt: parsedSearchedAt });📝 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
| const { username, searchedAt } = req.body; | |
| try { | |
| const user = await User.findById(req.user.id); | |
| if (!user) { | |
| return res.status(404).json({ message: 'User not found' }); | |
| } | |
| // Remove duplicate if exists (keep only unique searches) | |
| user.trackerHistory = user.trackerHistory.filter( | |
| item => item.username.toLowerCase() !== username.toLowerCase() | |
| ); | |
| // Add new search at the beginning | |
| user.trackerHistory.unshift({ username, searchedAt: new Date(searchedAt) }); | |
| const { username, searchedAt } = req.body; | |
| if (typeof username !== "string" || !username.trim()) { | |
| return res.status(400).json({ message: "username is required" }); | |
| } | |
| const parsedSearchedAt = searchedAt ? new Date(searchedAt) : new Date(); | |
| if (Number.isNaN(parsedSearchedAt.getTime())) { | |
| return res.status(400).json({ message: "searchedAt must be a valid date" }); | |
| } | |
| try { | |
| const user = await User.findById(req.user.id); | |
| if (!user) { | |
| return res.status(404).json({ message: 'User not found' }); | |
| } | |
| // Remove duplicate if exists (keep only unique searches) | |
| user.trackerHistory = user.trackerHistory.filter( | |
| item => item.username?.toLowerCase() !== username.toLowerCase() | |
| ); | |
| // Add new search at the beginning | |
| user.trackerHistory.unshift({ username, searchedAt: parsedSearchedAt }); |
🤖 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/routes/auth.js` around lines 60 - 74, Validate and sanitize
req.body.username and req.body.searchedAt before using them: ensure username
exists and is a non-empty string, and ensure searchedAt is a valid date (or
omit/replace with Date.now() if invalid). In the route where you fetch User
(User.findById and then mutate user.trackerHistory), add checks that username is
typeof string and trim() !== "" and that new Date(searchedAt) is valid
(isNaN(Date.parse(...)) check) and return a 400 with a clear message if
validation fails; only then perform the existing trackerHistory filtering
(user.trackerHistory.filter(...)) and unshift with a normalized { username:
username.trim(), searchedAt: new Date(validDate) } entry.
Comment on lines
+26
to
+30
| cookie: { | ||
| httpOnly: true, | ||
| secure: false, // set to true in production with HTTPS | ||
| sameSite: 'lax' | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current session/proxy configuration in backend
rg -n "app\\.set\\(['\"]trust proxy['\"]|session\\(|secure:|sameSite:" backend/server.js
rg -n "NODE_ENV|FRONTEND_URL|SESSION_SECRET" backend/.env .env 2>/dev/null || trueRepository: GitMetricsLab/github_tracker
Length of output: 218
Add environment-aware session cookie security and trust proxy configuration.
The session cookie has secure: false hardcoded (line 28), allowing production sessions to transmit over non-HTTPS. Additionally, app.set('trust proxy', 1) is missing, which breaks session handling when deployed behind a reverse proxy.
Set secure based on the environment and configure proxy trust. For cross-origin scenarios (FRONTEND_URL with credentials), also adjust sameSite to 'none' in production.
Suggested patch
+app.set('trust proxy', 1);
+
app.use(session({
secret: process.env.SESSION_SECRET,
resave: false,
saveUninitialized: false,
cookie: {
httpOnly: true,
- secure: false, // set to true in production with HTTPS
- sameSite: 'lax'
+ secure: process.env.NODE_ENV === 'production',
+ sameSite: process.env.NODE_ENV === 'production' ? 'none' : 'lax'
}
}));🤖 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/server.js` around lines 26 - 30, Update the session cookie and proxy
settings to be environment-aware: call app.set('trust proxy', 1) during server
setup, and in the session middleware's cookie config (the cookie object
currently containing httpOnly/secure/sameSite) set secure =
(process.env.NODE_ENV === 'production') and set sameSite = 'none' when in
production and FRONTEND_URL is cross-origin (otherwise keep 'lax'); ensure
secure is true in production so cookies require HTTPS and adjust sameSite to
'none' when sending credentials across origins. Use the existing session
middleware cookie object and the app.set('trust proxy', 1) symbol names to
locate where to apply these changes.
| "dev": "vite --host", | ||
| "build": "vite build", | ||
| "lint": "eslint .", | ||
| "test": "jasmine", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Jasmine spec files
fd -e cjs -e js "spec\."
# Check jasmine configuration
fd -t f "jasmine.json"Repository: GitMetricsLab/github_tracker
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Check repository structure
echo "=== Repository structure ==="
ls -la | head -30
# Check if there are separate frontend and backend directories
echo -e "\n=== Checking for frontend/backend directories ==="
[ -d "frontend" ] && echo "frontend/ exists" || echo "frontend/ does NOT exist"
[ -d "backend" ] && echo "backend/ exists" || echo "backend/ does NOT exist"
# Check location of spec files - are they at root or in backend?
echo -e "\n=== Full paths of spec files ==="
find . -name "*.spec.cjs" -o -name "*.spec.js" 2>/dev/null | head -20
# Read frontend/package.json if it exists
echo -e "\n=== frontend/package.json (lines 1-20) ==="
[ -f "frontend/package.json" ] && head -20 frontend/package.json || echo "frontend/package.json not found"
# Read backend/package.json if it exists
echo -e "\n=== backend/package.json (lines 1-20) ==="
[ -f "backend/package.json" ] && head -20 backend/package.json || echo "backend/package.json not found"
# Check if spec directory exists at root
echo -e "\n=== Check spec/ directory ==="
[ -d "spec" ] && echo "spec/ exists at root" && ls -la spec/ || echo "spec/ does NOT exist at root"Repository: GitMetricsLab/github_tracker
Length of output: 2655
🏁 Script executed:
#!/bin/bash
# Check for root-level package.json
echo "=== Root-level package.json ==="
[ -f "package.json" ] && cat package.json || echo "No package.json at root"
# Check for jasmine configuration files
echo -e "\n=== Jasmine configuration files ==="
fd -t f -i "jasmine" 2>/dev/null | head -20
# Check if frontend has jasmine config
echo -e "\n=== Frontend directory contents ==="
ls -la frontend/ | grep -E "(jasmine|spec|test)"
# Check if there's a jasmine.json or similar
echo -e "\n=== Looking for test config files ==="
find . -maxdepth 2 -type f \( -name "*jasmine*" -o -name "*.spec.js" -o -name "*.spec.cjs" -o -name "jest.config.*" \) 2>/dev/nullRepository: GitMetricsLab/github_tracker
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Read jasmine configuration
echo "=== spec/support/jasmine.mjs ==="
cat spec/support/jasmine.mjs
# Check what dependencies are in frontend vs backend for jasmine
echo -e "\n=== Jasmine in frontend/package.json dependencies ==="
grep -i "jasmine" frontend/package.json || echo "No jasmine in frontend dependencies"
echo -e "\n=== Jasmine in backend/package.json dependencies ==="
grep -i "jasmine" backend/package.json || echo "No jasmine in backend dependencies"
# Check full devDependencies in both
echo -e "\n=== Frontend devDependencies ==="
grep -A 20 '"devDependencies"' frontend/package.json | head -25
echo -e "\n=== Backend devDependencies ==="
grep -A 20 '"devDependencies"' backend/package.json | head -25Repository: GitMetricsLab/github_tracker
Length of output: 1379
Move the test script to backend/package.json.
The "test": "jasmine" script in frontend/package.json is configured to run backend tests (spec/auth.routes.spec.cjs, spec/user.model.spec.cjs), not frontend tests. This script and its dependencies should be in backend/package.json to maintain proper separation between frontend and backend. Additionally, the jasmine package and @types/jasmine should be moved to backend's devDependencies, not frontend's.
🤖 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/package.json` at line 10, The frontend package.json currently
defines the "test" script ("test": "jasmine") and lists jasmine and
`@types/jasmine` in devDependencies though those tests are backend specs; remove
the "test" script and jasmine-related devDependencies from frontend/package.json
and add them into backend/package.json instead: add the "test": "jasmine" script
to backend/package.json and move jasmine and `@types/jasmine` into backend's
devDependencies so backend runs the Jasmine specs (refer to the "test" script
and the jasmine/@types/jasmine entries when updating the two package.json
files).
| "eslint-plugin-react": "^7.37.2", | ||
| "eslint-plugin-react-hooks": "^5.0.0", | ||
| "eslint-plugin-react-refresh": "^0.4.14", | ||
| "express": "^5.2.1", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if backend has its own package.json
fd -t f "package.json" backend/
# If it exists, check its dependencies
if [ -f backend/package.json ]; then
echo "=== backend/package.json dependencies ==="
cat backend/package.json | jq '.dependencies, .devDependencies'
fiRepository: GitMetricsLab/github_tracker
Length of output: 393
🏁 Script executed:
cat -n frontend/package.jsonRepository: GitMetricsLab/github_tracker
Length of output: 2227
Remove backend dependencies from frontend package.json devDependencies.
The following backend runtime dependencies are misplaced in frontend/package.json and should not appear there at all: express, mongoose, express-session, passport, passport-local, bcryptjs, and supertest. These packages already exist in backend/package.json where they belong. Their presence in the frontend package.json will:
- Unnecessarily bloat the frontend bundle
- Create confusion in dependency management
- Violate the separation of concerns established by the project structure
Remove lines 49, 50, 53-55 (and bcryptjs on line 44 and supertest on line 56) from devDependencies.
🤖 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/package.json` at line 49, Remove the backend runtime packages
mistakenly declared in frontend's devDependencies: delete the entries for
"express", "mongoose", "express-session", "passport", "passport-local",
"bcryptjs", and "supertest" from frontend/package.json so only frontend-related
packages remain; locate the devDependencies block in frontend/package.json and
remove those package keys (ensure you don't touch backend/package.json where
these belong) and then run npm/yarn install to update lockfile.
Comment on lines
+77
to
83
| } catch (err: unknown) { | ||
| const githubError = err as GitHubApiError; | ||
|
|
||
| if (githubError.status === 403) { | ||
| setError('GitHub API rate limit exceeded. Please wait or use a token.'); | ||
| setRateLimited(true); // Prevent further fetches | ||
| } else { |
Contributor
There was a problem hiding this comment.
Rate-limit state can permanently lock out further fetch attempts.
After Line 82 sets rateLimited to true, the guard at Line 62 prevents all future requests with no recovery path in this hook instance.
♻️ Proposed fix
- if (!octokit || !username || rateLimited) return;
+ if (!octokit || !username) return;
setLoading(true);
setError('');
try {
const [issueRes, prRes] = await Promise.all([
fetchPaginated(octokit, username, 'issue', page, perPage),
fetchPaginated(octokit, username, 'pr', page, perPage),
]);
+ setRateLimited(false);
setIssues(issueRes.items);
setPrs(prRes.items);🤖 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/hooks/useGitHubData.ts` around lines 77 - 83, The hook
useGitHubData currently sets rateLimited to true on a 403 and the existing guard
then blocks all future fetches with no recovery; modify the 403 handling to
parse a Retry-After header or X-RateLimit-Reset (or fall back to a sensible
default like 60s), call setRateLimited(true) but schedule a timer to call
setRateLimited(false) after that delay, and ensure the timer is stored in a ref
and cleared on unmount or when a new rate-limit event occurs; this uses the
existing symbols rateLimited, setRateLimited, and the 403 catch branch so the
guard that prevents fetches will automatically resume after the timeout.
Comment on lines
+36
to
+38
| if (response.data.message === "User created successfully") { | ||
| navigate("/login"); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid redirect logic tied to response text.
Routing on an exact message string is brittle. Prefer status-code (or explicit boolean) checks.
Suggested patch
- if (response.data.message === "User created successfully") {
+ if (response.status === 201) {
navigate("/login");
}📝 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
| if (response.data.message === "User created successfully") { | |
| navigate("/login"); | |
| } | |
| if (response.status === 201) { | |
| navigate("/login"); | |
| } |
🤖 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/pages/Signup/Signup.tsx` around lines 36 - 38, The redirect
currently depends on response.data.message string match; change the check to use
an explicit status or boolean from the API (e.g., response.status === 201 or
response.data.success === true) before calling navigate("/login") in the signup
submit handler (the block that currently inspects response.data.message and
calls navigate). Update the API contract usage to prefer response.status or
response.data.success and handle non-success cases accordingly.
Comment on lines
+160
to
+164
| useEffect(() => { | ||
| if (username) { | ||
| fetchData(username, page + 1, ROWS_PER_PAGE); | ||
| } | ||
| }, [fetchData, page, tab, username]); |
Contributor
There was a problem hiding this comment.
Auto-fetch effect triggers API calls on every username keystroke.
Because this effect depends on username (and tab), each edit in the input can trigger network requests before form submit, which can quickly hit rate limits.
✅ Suggested direction
+ const [submittedUsername, setSubmittedUsername] = useState("");
useEffect(() => {
- if (username) {
- fetchData(username, page + 1, ROWS_PER_PAGE);
+ if (submittedUsername) {
+ fetchData(submittedUsername, page + 1, ROWS_PER_PAGE);
}
- }, [fetchData, page, tab, username]);
+ }, [fetchData, page, submittedUsername]);
const handleSubmit = async (
e: React.FormEvent<HTMLFormElement>,
): Promise<void> => {
e.preventDefault();
setPage(0);
+ const normalized = username.trim();
+ setSubmittedUsername(normalized);
- await saveLookupHistory(username);
- await fetchData(username, 1, ROWS_PER_PAGE);
+ await saveLookupHistory(normalized);
+ await fetchData(normalized, 1, ROWS_PER_PAGE);
};🤖 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/pages/Tracker/Tracker.tsx` around lines 160 - 164, The effect
currently re-runs on every username (and tab) change causing API calls on each
keystroke; change it to only run for deliberate fetches by introducing a stable
"submittedUsername" state (e.g., submittedUsername) that is set in the form
submit handler instead of using raw username, then update the effect to depend
on submittedUsername and page (and not username/tab) and call
fetchData(submittedUsername, page + 1, ROWS_PER_PAGE); alternatively implement a
debounce that sets submittedUsername after a short pause—update handlers that
trigger fetches to set submittedUsername so fetches only occur on
submit/debounced value, not on every keystroke.
Comment on lines
+192
to
+193
| const matchesStart = !startDate || createdAt >= new Date(startDate); | ||
| const matchesEnd = !endDate || createdAt <= new Date(endDate); |
Contributor
There was a problem hiding this comment.
End-date filtering is currently exclusive for most of the selected day.
new Date(endDate) resolves to midnight, so items created later on the same end date are filtered out.
🗓️ Proposed fix
- const matchesEnd = !endDate || createdAt <= new Date(endDate);
+ const endBoundary = endDate
+ ? new Date(`${endDate}T23:59:59.999`)
+ : null;
+ const matchesEnd = !endBoundary || createdAt <= endBoundary;📝 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
| const matchesStart = !startDate || createdAt >= new Date(startDate); | |
| const matchesEnd = !endDate || createdAt <= new Date(endDate); | |
| const matchesStart = !startDate || createdAt >= new Date(startDate); | |
| const endBoundary = endDate | |
| ? new Date(`${endDate}T23:59:59.999`) | |
| : null; | |
| const matchesEnd = !endBoundary || createdAt <= endBoundary; |
🤖 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/pages/Tracker/Tracker.tsx` around lines 192 - 193, The end-date
check in matchesEnd uses new Date(endDate) which is midnight and thus excludes
items later that day; update the logic to compare createdAt against the
end-of-day timestamp (e.g., compute an endOfDay from endDate by setting hours to
23:59:59.999 or by creating nextDay = new Date(endDate) + 1 day and using
createdAt < nextDay) so matchesEnd becomes true for any createdAt on the
selected endDate; keep matchesStart as-is using startDate and createdAt.
Comment on lines
+92
to
+96
| ``` | ||
| MONGO_URI=mongodb+srv://alokhacs222729_db_user:IvORbhhzQg71OF2q@cluster0.gevcwty.mongodb.net/?appName=Cluster0 | ||
| PORT=5000 | ||
| SESSION_SECRET=your_session_secret_key_here | ||
| ``` |
Contributor
There was a problem hiding this comment.
Remove hardcoded MongoDB credentials from documentation immediately.
Line 93 exposes a full connection string with embedded credentials. This must be replaced with placeholders, and the exposed credentials should be rotated/revoked.
🔒 Proposed fix
-MONGO_URI=mongodb+srv://alokhacs222729_db_user:IvORbhhzQg71OF2q@cluster0.gevcwty.mongodb.net/?appName=Cluster0
+MONGO_URI=mongodb+srv://<db_user>:<db_password>@cluster0.gevcwty.mongodb.net/<db_name>?retryWrites=true&w=majority
PORT=5000
SESSION_SECRET=your_session_secret_key_here🧰 Tools
🪛 Betterleaks (1.2.0)
[high] 93-93: Detected a MongoDB connection string with embedded credentials, potentially exposing direct database access and sensitive application data.
(mongodb-connection-string)
🪛 markdownlint-cli2 (0.22.1)
[warning] 92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@README.md` around lines 92 - 96, The README currently contains a hardcoded
MongoDB connection string (MONGO_URI) with embedded credentials; replace that
value with a placeholder (e.g.,
MONGO_URI=mongodb+srv://<DB_USER>:<DB_PASS>@<HOST>/<DB_NAME>?retryWrites=true&w=majority)
and update the example block so no real secrets appear, and add a short note to
rotate/revoke the exposed credentials immediately; locate the example using the
MONGO_URI token and update it accordingly.
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.
Related Issue
N/A - New feature addition
Description
frontend/andbackend/foldersHow Has This Been Tested?
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit
New Features
Documentation
Security