Fix: Implement filter and auth credentials persistence on page refresh (#291)#293
Fix: Implement filter and auth credentials persistence on page refresh (#291)#293surjeetkumar8006 wants to merge 7 commits into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAuth hook reads/writes credentials to sessionStorage and exposes getOctokit(), error, and logout; data hook exposes clearData(); Tracker page persists tabs/filters/pagination to localStorage, shows Logout when credentials exist, and clears state/localStorage on reset or logout; tsconfig ordering adjusted. ChangesState persistence
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @surjeetkumar8006 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}
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 `@src/hooks/useGitHubAuth.ts`:
- Around line 5-6: The hook useGitHubAuth is storing sensitive GitHub
credentials in localStorage via the state initializers for username and token
(the useState calls that read 'tracker_username' and 'tracker_token'), which is
insecure; replace this client-side persistence with a server-side session flow
where the PAT is never stored in browser storage and is instead kept in an
httpOnly cookie or backend session and accessed via authenticated API endpoints
from the frontend; if a backend session cannot be implemented immediately,
change the storage fallback to sessionStorage (read/write
'tracker_username'/'tracker_token' from sessionStorage) as a temporary
mitigation or implement client-side encryption before persisting, but prefer
updating the authentication flow in useGitHubAuth to delegate storage and token
refresh to the server.
🪄 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: da78dd20-77df-4f11-86fd-cc0aa3ed58e1
📒 Files selected for processing (3)
src/hooks/useGitHubAuth.tssrc/pages/Tracker/Tracker.tsxtsconfig.app.json
|
Updated the PR based on review feedback:
All checks are now passing. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubAuth.ts (1)
24-30:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore
errorin the hook return shape to avoid consumer breakage.
useGitHubAuth()no longer returnserror, butTracker.tsxreads it at Line 58. This causes a TypeScript compile error (Property 'error' does not exist ...).Proposed fix
return { username, setUsername, token, setToken, + error, getOctokit, };🤖 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 `@src/hooks/useGitHubAuth.ts` around lines 24 - 30, The hook useGitHubAuth was changed to stop returning error which breaks consumers like Tracker.tsx that read error; restore an error state (e.g., const [error, setError] = useState<Error | null>(null)) inside useGitHubAuth, ensure any failures (such as in getOctokit) set this error via setError, and include error in the returned object along with username, setUsername, token, setToken, and getOctokit so existing callers continue to compile.
🤖 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.
Outside diff comments:
In `@src/hooks/useGitHubAuth.ts`:
- Around line 24-30: The hook useGitHubAuth was changed to stop returning error
which breaks consumers like Tracker.tsx that read error; restore an error state
(e.g., const [error, setError] = useState<Error | null>(null)) inside
useGitHubAuth, ensure any failures (such as in getOctokit) set this error via
setError, and include error in the returned object along with username,
setUsername, token, setToken, and getOctokit so existing callers continue to
compile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d68ef73-dec8-43ae-9c39-234731372d35
📒 Files selected for processing (2)
src/hooks/useGitHubAuth.tssrc/pages/Tracker/Tracker.tsx
|
Hi maintainers, All major checks are passing now. Please review when convenient. Thanks! |
|
|
Hi @GitMetricsLab, I have addressed the CodeRabbit review feedback:
All checks are passing successfully now. Please review and merge the PR when convenient. Thank you! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubAuth.ts (1)
9-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
logout()key removal is being undone by the persistence effect.Line 9–Line 12 always calls
setItem, so after Line 32–Line 33removeItem, the next render writes both keys back as empty strings. Persist with remove-on-empty instead.Proposed fix
useEffect(() => { - sessionStorage.setItem('tracker_username', username); - sessionStorage.setItem('tracker_token', token); + if (username) { + sessionStorage.setItem('tracker_username', username); + } else { + sessionStorage.removeItem('tracker_username'); + } + + if (token) { + sessionStorage.setItem('tracker_token', token); + } else { + sessionStorage.removeItem('tracker_token'); + } }, [username, token]);Also applies to: 28-34
🤖 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 `@src/hooks/useGitHubAuth.ts` around lines 9 - 12, The effect in useGitHubAuth.ts that writes sessionStorage keys ('tracker_username' and 'tracker_token') in the useEffect is overwriting logout()'s removeItem calls; update the effect in the useGitHubAuth hook so it sets each key only if the value is non-empty (or non-null/undefined) and calls sessionStorage.removeItem for empty values instead of sessionStorage.setItem with an empty string; target the useEffect block that currently calls sessionStorage.setItem for username and token so it conditionally uses setItem or removeItem for each key.
🤖 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 `@src/hooks/useGitHubData.ts`:
- Around line 242-248: clearData currently only resets lists and totals but
leaves the rateLimited flag and any in-flight request lifecycle intact; update
clearData to also clear rateLimited and abort/reset any active request tracker
(e.g., abortController, activeRequest or inFlightRequest ref) so future fetches
aren’t blocked and stale responses can’t repopulate cleared state. Find the
clearData function and add steps to set rateLimited = false and to cancel or
null out the in-flight request handle used by the fetch logic (the same
controller/ref used around the fetch at or near the earlier fetch logic on line
~99).
---
Outside diff comments:
In `@src/hooks/useGitHubAuth.ts`:
- Around line 9-12: The effect in useGitHubAuth.ts that writes sessionStorage
keys ('tracker_username' and 'tracker_token') in the useEffect is overwriting
logout()'s removeItem calls; update the effect in the useGitHubAuth hook so it
sets each key only if the value is non-empty (or non-null/undefined) and calls
sessionStorage.removeItem for empty values instead of sessionStorage.setItem
with an empty string; target the useEffect block that currently calls
sessionStorage.setItem for username and token so it conditionally uses setItem
or removeItem for each key.
🪄 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: 495714b9-bf49-4dc1-95dc-e3830899e3d5
📒 Files selected for processing (3)
src/hooks/useGitHubAuth.tssrc/hooks/useGitHubData.tssrc/pages/Tracker/Tracker.tsx
surjeetkumar8006
force-pushed
the
fix-filter-persistence
branch
from
945cc48 to
fc405b6
Compare
…ionStorage values
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Description
This PR resolves issue #291 by implementing
localStoragepersistence for the tracking dashboard filters and GitHub authentication credentials.Previously, refreshing the page would reset all selected filters and require the user to re-enter their GitHub username and Personal Access Token.
Changes Made:
src/pages/Tracker/Tracker.tsx: Initialized filter states (tab, page, issue/PR state filters, search queries, and dates) usinglocalStorage.getItemand added auseEffectto sync them.src/hooks/useGitHubAuth.ts: Ensured the GitHub username and Token are also stored and retrieved fromlocalStorageso users stay authenticated on reload.tsconfig.app.json: Removed an unsupported TypeScript compiler option to resolve VS Code syntax warnings.Related Issue
Fixes #291
Checklist
Summary by CodeRabbit
New Features
Chores