feat: add recent search history for tracked GitHub profiles#660
feat: add recent search history for tracked GitHub profiles#660PalashKulkarni wants to merge 1 commit into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe PR adds a recent search history feature to the GitHub Tracker. The hook ChangesRecent Search History Feature
Sequence DiagramsequenceDiagram
participant User
participant Tracker
participant useGitHubData
participant localStorage
User->>Tracker: Enter username or click recent search
Tracker->>Tracker: Normalize username, reset page
Tracker->>useGitHubData: Call fetchData(username)
useGitHubData->>useGitHubData: Validate Octokit, check rate limit
useGitHubData->>useGitHubData: Fetch issues/PRs via GitHub API
useGitHubData->>Tracker: Return true (success) or false (failure)
alt Fetch successful
Tracker->>localStorage: Save username to recent searches
end
Tracker->>localStorage: Load recent searches on mount
User->>Tracker: Click recent search button
Tracker->>Tracker: Trigger fetch with stored username
Tracker->>User: Display results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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
added
gssoc:approved
level:intermediate
quality:clean
type:accessibility
labels
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @PalashKulkarni for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
|
Hi maintainers 👋 I've submitted a PR for Issue #659 that adds a Recent Search History feature to the Tracker page. What was implemented:
I've tested the feature locally and ensured it integrates with the existing search workflow without requiring backend changes. I would appreciate it if you could review the PR and merge it if everything looks good. I'm happy to make any requested changes or improvements. Thank you for your time and feedback! 🚀 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubData.ts (1)
143-189:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve error classification/rate-limit gating for per-request failures in
Promise.allSettledIn
src/hooks/useGitHubData.ts(around lines 143-189),Promise.allSettled(requests)prevents individual request rejections from reaching the outercatchwhere 403/rate-limit and other specific errors are classified; instead the code only sets the generic partial-failure message and then unconditionally callssetRateLimited(false).Possible fix
+ const applyFetchError = (err: { status?: number; message?: string }) => { + const errorMessage = err.message?.toLowerCase() || ''; + + if (err.status === 403) { + setError( + 'GitHub API rate limit exceeded. Please provide a PAT to continue.' + ); + setRateLimited(true); + } else if (errorMessage.includes('do not exist')) { + setError('User not found. Please check the GitHub username.'); + } else if (errorMessage.includes('validation failed')) { + setError('Invalid GitHub username or insufficient permissions.'); + } else if (err.status === 401 || errorMessage.includes('permission')) { + setError('Private repository detected. Please provide a PAT.'); + } else if (err.status === 404) { + setError('Resource not found.'); + } else { + setError( + 'Unable to fetch GitHub data. Please verify the username, token, or network connection.' + ); + } + }; + const results = await Promise.allSettled(requests); @@ - const hasRejected = results.some( - (result) => result.status === 'rejected' - ); - - if (hasRejected) { - setError( - 'Some GitHub data could not be fetched completely.' - ); - } + const rejected = results.find( + (result): result is PromiseRejectedResult => + result.status === 'rejected' + ); + + if (rejected) { + applyFetchError(rejected.reason as { + status?: number; + message?: string; + }); + return false; + } setRateLimited(false); - return !hasRejected; + return true;🤖 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/useGitHubData.ts` around lines 143 - 189, Promise.allSettled(requests) swallows per-request errors, so restore error classification by inspecting rejected results and either rethrowing or handling rate-limit specifically. After computing results and before calling setRateLimited(false), scan results for rejected entries; if any rejection’s reason indicates 403/rate-limit, trigger the existing path by throwing that error so the outer catch classifies it (and sets setRateLimited(true)), otherwise setError with the generic partial-failure message. Only call setRateLimited(false) when all requests are fulfilled; do not reset it to false if any rejection occurred. Use existing symbols: Promise.allSettled(requests), setError, setRateLimited, shouldFetchIssues/shouldFetchPrs, setIssues/setPrs setters to locate the block.
🤖 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/pages/Tracker/Tracker.tsx`:
- Around line 147-151: The duplicate fetches are caused by calling fetchData
inside handleSubmit/handleRecentSearchClick while also relying on the
useEffect([tab, page]) to fetch; fix by moving fetch control entirely into the
effect: add username to the useEffect dependency array (useEffect(() => { if
(username) fetchData(username, page + 1, ROWS_PER_PAGE); }, [tab, page,
username])) and remove the immediate await fetchData(...) calls from
handleSubmit and handleRecentSearchClick (keep their setUsername and setPage(0)
calls so the effect drives the fetch). This consolidates fetch logic into
useEffect and prevents double requests.
---
Outside diff comments:
In `@src/hooks/useGitHubData.ts`:
- Around line 143-189: Promise.allSettled(requests) swallows per-request errors,
so restore error classification by inspecting rejected results and either
rethrowing or handling rate-limit specifically. After computing results and
before calling setRateLimited(false), scan results for rejected entries; if any
rejection’s reason indicates 403/rate-limit, trigger the existing path by
throwing that error so the outer catch classifies it (and sets
setRateLimited(true)), otherwise setError with the generic partial-failure
message. Only call setRateLimited(false) when all requests are fulfilled; do not
reset it to false if any rejection occurred. Use existing symbols:
Promise.allSettled(requests), setError, setRateLimited,
shouldFetchIssues/shouldFetchPrs, setIssues/setPrs setters to locate the block.
🪄 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: de33127e-7426-4b90-8460-e87822269448
📒 Files selected for processing (2)
src/hooks/useGitHubData.tssrc/pages/Tracker/Tracker.tsx
| useEffect(() => { | ||
| if (username) { | ||
| fetchData(username, page + 1, ROWS_PER_PAGE); | ||
| } | ||
| }, [tab, page]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '147,197p' src/pages/Tracker/Tracker.tsxRepository: GitMetricsLab/github_tracker
Length of output: 1165
Duplicate API calls when searching from non-zero pages
Both handleSubmit and handleRecentSearchClick call setPage(0) then immediately await fetchData(..., 1, ...). Setting page to 0 triggers the [tab, page] effect, which fetches the same page 1 data again. This wastes GitHub API quota on every search initiated from pages beyond the first.
Move all fetch logic into the effect, or add a flag to prevent the effect from running when handlers have already fetched.
Affected code
useEffect(() => {
if (username) {
fetchData(username, page + 1, ROWS_PER_PAGE);
}
}, [tab, page]);
const handleSubmit = async (e: React.FormEvent<HTMLFormElement>): Promise<void> => {
// ...
setUsername(normalizedUsername);
setPage(0);
const wasSuccessful = await fetchData(
normalizedUsername,
1,
ROWS_PER_PAGE
);
// ...
};
const handleRecentSearchClick = async (value: string) => {
// ...
setUsername(normalizedUsername);
setPage(0);
const wasSuccessful = await fetchData(
normalizedUsername,
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 `@src/pages/Tracker/Tracker.tsx` around lines 147 - 151, The duplicate fetches
are caused by calling fetchData inside handleSubmit/handleRecentSearchClick
while also relying on the useEffect([tab, page]) to fetch; fix by moving fetch
control entirely into the effect: add username to the useEffect dependency array
(useEffect(() => { if (username) fetchData(username, page + 1, ROWS_PER_PAGE);
}, [tab, page, username])) and remove the immediate await fetchData(...) calls
from handleSubmit and handleRecentSearchClick (keep their setUsername and
setPage(0) calls so the effect drives the fetch). This consolidates fetch logic
into useEffect and prevents double requests.
1 participant
Related Issue
Description
This PR adds a Recent Search History feature to the Tracker page, allowing users to quickly revisit previously searched GitHub profiles.
Changes Made
localStorageBenefits
How Has This Been Tested?
localStorageScreenshots (if applicable)
N/A – This feature primarily adds functionality and persistence behavior. UI changes are minimal and integrated into the existing Tracker page.
Type of Change
Summary by CodeRabbit