refactor: enhance GitHub API client with commit data handling and improve caching strategy #67
Conversation
…r issue retrieval
…rove caching strategy
WalkthroughThis set of changes refines error handling, type definitions, and user analytics functionality across multiple components and utilities. Chart components now display a "No Data Available" message when data is missing. The GitHub API client is overhauled to fetch real analytics data, improve caching, and remove fallback/demo data. Several UI components and hooks are updated for improved clarity, error handling, and formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchPage
participant GitHubAPIClient
participant GitHubAPI
User->>SearchPage: Initiate user search
SearchPage->>GitHubAPIClient: loadUserAnalytics(username)
GitHubAPIClient->>GitHubAPI: Fetch user profile
GitHubAPI-->>GitHubAPIClient: Return profile data
GitHubAPIClient->>GitHubAPI: Fetch user repositories
GitHubAPI-->>GitHubAPIClient: Return repositories list
GitHubAPIClient->>GitHubAPI: Fetch commit/activity data per repo
GitHubAPI-->>GitHubAPIClient: Return commit/activity data
GitHubAPIClient->>SearchPage: Return analytics (profile, overview, languages, behavior)
SearchPage->>User: Display analytics or show "No Data Available" in charts if missing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
src/components/charts/BarChart.tsx (1)
33-44: Avoid duplication; extract a shared “no data” option builder and consider disabling interactions.You repeat the same “No Data Available” option across charts. Centralize (e.g., buildNoDataOption()) and optionally disable interactions for clarity.
Example helper:
+// utils/charts.ts +import type { EChartsOption } from 'echarts'; +export function buildNoDataOption(text = 'No Data Available'): EChartsOption { + return { + title: { + text, + left: 'center', + top: 'middle', + textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } + }, + tooltip: { show: false }, + legend: { show: false } + }; +}Then:
- const option: EChartsOption = { /* ... */ }; + const option = buildNoDataOption();Happy to add this helper and update all chart components in this PR.
src/components/charts/AreaChart.tsx (1)
29-44: Centralize the empty-state option and disable interactions for clarity.Same “no data” block appears across charts; extract a shared helper (e.g., buildNoDataOption) and set
tooltip/legend.show = false.Minimal change here:
- const option: EChartsOption = { - title: { text: 'No Data Available', left: 'center', top: 'middle', textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } } - }; + const option: EChartsOption = { + title: { text: 'No Data Available', left: 'center', top: 'middle', textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } }, + tooltip: { show: false }, + legend: { show: false } + };I can add a tiny charts util and update usages.
src/components/charts/PieChart.tsx (1)
36-47: DRY the “No Data Available” option and disable interactions.Use a shared helper for the empty state and set
tooltip/legend.show = falseto avoid hover/legend noise.Example:
- const option: EChartsOption = { /* ...title... */ }; + const option: EChartsOption = { + title: { text: 'No Data Available', left: 'center', top: 'middle', textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } }, + tooltip: { show: false }, + legend: { show: false } + };Can implement a shared helper and apply across chart components.
src/components/charts/LineChart.tsx (1)
38-53: Unify empty-state handling; disable interactions for the placeholder.Keep the UX consistent with other charts and reduce duplication by extracting a shared
buildNoDataOption(); also settooltip/legend.show = false.Example change:
- const option: EChartsOption = { - title: { text: 'No Data Available', left: 'center', top: 'middle', textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } } - }; + const option: EChartsOption = { + title: { text: 'No Data Available', left: 'center', top: 'middle', textStyle: { fontSize: 16, fontWeight: 'normal', color: '#9ca3af' } }, + tooltip: { show: false }, + legend: { show: false } + };I can add a tiny util and update all chart components in this PR.
eslint.config.mjs (1)
14-28: Rule overrides may hide legitimate issuesTurning off
react/no-unescaped-entitiesglobally and suppressing all unused-error variables via"caughtErrors": "none"silences potentially useful warnings.
Consider restricting these overrides to specific directories/files or using ESLint “overrides” blocks so that real mistakes (e.g. accidently swallowed errors, missed HTML escapes) are still surfaced where they matter.src/app/api/auth/logout/route.ts (1)
27-31: Silent catch drops diagnostic informationThe empty
catch { … }returns a 500 but discards the underlying exception.
At minimum, log the error with a structured logger (orconsole.errorserver-side) before responding so operational issues can be traced.src/lib/api/github-api-client.ts (2)
43-50:created_at/updated_atshould be requiredGitHub always returns
created_atandupdated_atfor user objects.
Marking them optional (?) forces extra null-checks downstream and risks hiding API-usage errors.
593-613: Cache clearing misses repo-scoped endpoints
clearUserCache()only wipes keys containing/users/${username}; repo and commit endpoints (/repos/${username}/…) stay cached, so fresh analytics may show stale commit counts.
Extend the key match or provide a per-user namespace in the cache.src/app/auth/callback/page.tsx (1)
24-39: Swallowed error obscures auth failures
catch { router.replace('/') }hides the original problem, complicating debugging.
Log the exception or surface a toast so users/devs know why callback handling failed.src/app/settings/page.tsx (1)
15-18: KeeptempOrgNamein sync withorgData
useState(orgData?.orgName || '')captures the value only on first render.
IforgDatais populated asynchronously, the input shows an empty string until refresh.const [tempOrgName, setTempOrgName] = useState(''); +useEffect(() => { + setTempOrgName(orgData?.orgName ?? ''); +}, [orgData?.orgName]);src/components/quick-wins/hooks/useQuickWins.ts (1)
90-92: Ensure consistentfetch*API usage
refreshGoodIssues/EasyFixescallfetchX(true)whereas the initial fetch above is invoked without an argument.
Verify the function signature—if the boolean controls “force refresh”, document it; otherwise keep the call site uniform to avoid confusion.src/app/search/page.tsx (1)
220-220: Effect calls performSearch but it’s not in the deps — consider memoizing and adding itTo satisfy exhaustive-deps and avoid future stale-closure issues, memoize performSearch and include it in the dependency array (or add a targeted eslint-disable with rationale).
Example:
const performSearch = useCallback(async (query: string, type: "users" | "repos") => { setSearchResults((prev) => ({ ...prev, loading: true, error: null })); try { if (type === "users") { const users = await githubAPIClient.searchUsers(query, "all", 20); setSearchResults({ repos: [], users: users || [], loading: false, error: null }); } else { const repos = await githubAPIClient.searchRepositories(query, "stars", 20); setSearchResults({ repos: repos || [], users: [], loading: false, error: null }); } } catch { setSearchResults({ repos: [], users: [], loading: false, error: "Search failed. Please try again." }); } }, []); // ... useEffect(() => { // ... performSearch(query, type); // ... }, [userParam, repoParam, setCurrentQuery, setCurrentSearchType, loadUserAnalytics, performSearch]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
eslint.config.mjs(1 hunks)src/app/action-required/page.tsx(4 hunks)src/app/api/auth/logout/route.ts(2 hunks)src/app/auth/callback/page.tsx(2 hunks)src/app/quick-wins/page.tsx(4 hunks)src/app/search/page.tsx(3 hunks)src/app/settings/page.tsx(2 hunks)src/components/charts/AreaChart.tsx(1 hunks)src/components/charts/BarChart.tsx(1 hunks)src/components/charts/LineChart.tsx(1 hunks)src/components/charts/PieChart.tsx(1 hunks)src/components/quick-wins/hooks/useQuickWins.ts(2 hunks)src/components/search/SearchModal.tsx(2 hunks)src/lib/api/github-api-client.ts(8 hunks)src/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/components/search/SearchModal.tsx (1)
src/components/ui/dialog.tsx (1)
DialogDescription(136-136)
src/app/settings/page.tsx (1)
src/hooks/useAuth.ts (1)
useRequireAuth(22-30)
src/components/quick-wins/hooks/useQuickWins.ts (1)
src/lib/api/github-api-client.ts (1)
githubAPIClient(893-893)
src/app/action-required/page.tsx (1)
src/components/layout/PageHeader.tsx (1)
PageHeader(6-38)
src/lib/api/github-api-client.ts (1)
src/types/api.ts (1)
GitHubSearchResponse(126-130)
🔇 Additional comments (5)
src/lib/api/github-api-client.ts (1)
186-189: Verify auth scheme change
Authorization: Bearer <token>works for OAuth and fine-grained PATs, but classic PATs are documented with thetokenscheme.
Confirm that all supported token types authenticate correctly (403 would signal a mismatch).src/app/quick-wins/page.tsx (1)
31-35: Optional-chaining addition looks goodSafer access to
searchParams.get('tab')without altering behaviour.src/components/search/SearchModal.tsx (1)
8-9: Nice accessibility enhancementImporting and using
DialogDescriptionimproves semantics & screen-reader support.Also applies to: 176-178
src/app/search/page.tsx (1)
67-73: Types aligned to GitHub API (nullables) — good changeSwitching bio, location, and company to string | null removes undefined/null ambiguity and matches API payloads. Existing truthy checks still work as expected.
src/types/index.ts (1)
64-64: New/modified re-export: verifyContributorWithRepossource and update summaryLine 64 re-exports
ContributorWithReposfrom./github. This contradicts the AI summary claiming only comment removals. Please verify the symbol exists and is correctly exported fromsrc/types/github(or its index) to avoid TS errors at consumers. Also confirm no naming drift (e.g.,ContributorWithRepositories) and no duplicate symbol across other barrels.Run this to validate presence and catch typos/duplicates:
#!/bin/bash set -euo pipefail echo "Locate github types module(s):" fd -a -g "src/**/types/**/github.ts" || true fd -a -g "src/**/types/**/github/index.ts" || true echo "Check for definition (type alias or interface) of ContributorWithRepos:" rg -n -S $'^export\s+(type|interface)\s+ContributorWithRepos\b' -A 2 || true echo "Check for similarly named symbols to catch typos:" rg -n -S 'ContributorWithRepo' -A 1 || true echo "Check for any other exports of ContributorWithRepos (to avoid duplicate barrels):" rg -n -S $'^export\s+\{[^}]*\bContributorWithRepos\b[^}]*\}\s+from' || trueLikely an incorrect or invalid review comment.
| refreshData().catch(() => { | ||
| // Silent error handling for refresh data | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Silent error swallowing obscures failures
Catching and ignoring all errors from refreshData() removes any trace for debugging or UI feedback.
Consider at least logging (console.error) or surfacing an error state.
🤖 Prompt for AI Agents
In src/app/action-required/page.tsx around lines 50 to 52, the current code
silently catches and ignores all errors from the refreshData() call, which hides
failures and complicates debugging. Modify the catch block to log the error
using console.error or update the UI with an error state to make failures
visible and easier to diagnose.
| commits = allCommits.filter(commit => | ||
| commit.author?.login === username || | ||
| commit.commit?.author?.name?.toLowerCase().includes(username.toLowerCase()) || | ||
| commit.commit?.author?.email?.includes(username.toLowerCase()) | ||
| ); |
There was a problem hiding this comment.
Null-safety bug when calling .includes
commit.commit?.author?.name?.toLowerCase().includes(...) and the similar email check can throw if name/email is undefined because .includes executes on an undefined result.
- commit.commit?.author?.name?.toLowerCase().includes(username.toLowerCase())
+ (commit.commit?.author?.name?.toLowerCase() ?? '').includes(username.toLowerCase())Apply the same pattern to the email branch.
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 798 to 802, the code calls
.includes on commit.commit?.author?.name?.toLowerCase() and
commit.commit?.author?.email without ensuring these values are defined, which
can cause runtime errors. To fix this, add null checks before calling
.toLowerCase() and .includes() on both name and email properties, ensuring these
methods are only called on defined strings.
…ile return type in GitHubAPIClient
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/api/github-api-client.ts (1)
729-732: Null-safety bug when filtering commits (issue previously flagged)
commit.commit?.author?.name?.toLowerCase().includes(username.toLowerCase())can throw whennameisundefined. Guard with a fallback string, just as you did ingetUserCommitsLastWeek.- commit.commit?.author?.name?.toLowerCase().includes(username.toLowerCase()) + (commit.commit?.author?.name?.toLowerCase() ?? '').includes(username.toLowerCase())
🧹 Nitpick comments (2)
src/lib/api/github-api-client.ts (1)
5-5: Remove unused & misleadingerrorimport
import { error } from 'console'is never referenced and shadows the ubiquitouserroridentifier used incatchblocks. Drop the import to avoid confusion.src/components/quick-wins/hooks/useQuickWins.ts (1)
40-52: Dead code:initializeDatais no longer invokedThe
initializeDatacallback was left behind after the refactor and is never called. Delete it (and the accompanyingisInitializedref) to reduce noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/quick-wins/page.tsx(4 hunks)src/app/search/page.tsx(3 hunks)src/components/charts/BarChart.tsx(1 hunks)src/components/charts/PieChart.tsx(1 hunks)src/components/quick-wins/hooks/useQuickWins.ts(2 hunks)src/lib/api/github-api-client.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/charts/PieChart.tsx
- src/components/charts/BarChart.tsx
- src/app/search/page.tsx
- src/app/quick-wins/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/quick-wins/hooks/useQuickWins.ts (1)
src/lib/api/github-api-client.ts (1)
githubAPIClient(903-903)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/quick-wins/hooks/useQuickWins.ts (1)
106-109: Stabilise refresh callbacks.The inline arrow functions recreate on every render and can trigger needless re-renders downstream. Wrap them in
useCallback:-import { useCallback } from 'react' +import { useCallback } from 'react' ... -const refreshGoodIssues = () => fetchGoodIssues(true) -const refreshEasyFixes = () => fetchEasyFixes(true) +const refreshGoodIssues = useCallback(() => fetchGoodIssues(true), [fetchGoodIssues]) +const refreshEasyFixes = useCallback(() => fetchEasyFixes(true), [fetchEasyFixes])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/quick-wins/hooks/useQuickWins.ts(3 hunks)
🔇 Additional comments (2)
src/components/quick-wins/hooks/useQuickWins.ts (2)
6-6: Import added – issue resolved.
githubAPIClientis now correctly imported, fixing the compile-time error flagged in the previous review.
103-103:needsTokenmay become stale after token change.
hasValidToken()is evaluated once per render and isn’t reactive to changes outside React state (e.g. localStorage updates done by another component).
If the token can be set after this hook mounts, consider exposing token state through a store oruseSyncExternalStoreso the UI re-evaluates automatically.
|
|
||
| // Load from cache first | ||
| loadFromCache() | ||
|
|
||
| // Then check if we need to fetch fresh data | ||
| if (isQuickWinsCacheExpired()) { | ||
| fetchGoodIssues() | ||
| fetchEasyFixes() | ||
| } | ||
| }, [loadFromCache, isQuickWinsCacheExpired, fetchGoodIssues, fetchEasyFixes]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redundant cache load & orphaned initializeData.
This useEffect duplicates the behaviour already encapsulated in initializeData, yet initializeData is no longer invoked anywhere, leaving dead code and executing loadFromCache() twice on the initial render.
Consolidate the bootstrap logic into a single effect or call initializeData here, then delete the unused function to avoid confusion.
🤖 Prompt for AI Agents
In src/components/quick-wins/hooks/useQuickWins.ts around lines 90 to 99, the
useEffect duplicates the data loading logic already present in the unused
initializeData function, causing loadFromCache() to run twice on initial render.
Refactor by either calling initializeData inside this useEffect and removing the
duplicated code or by consolidating all bootstrap logic into this useEffect and
then deleting the unused initializeData function to eliminate dead code and
avoid confusion.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/search/page.tsx (1)
111-147: Critical: Duplicate function declaration causes the first implementation to be overridden.Two
loadUserAnalyticsfunctions are declared, which means the second one (lines 219-238) will completely override the first one (lines 111-147). This makes the data normalization logic in the first function unreachable.The static analysis tool correctly flagged this as a redeclaration error. You need to either:
- Remove the first function if you only want the race-condition-safe version, or
- Merge both implementations to have both data normalization AND race condition protection
Apply this diff to merge both implementations:
- const loadUserAnalytics = useCallback(async () => { - if (!userParam) return; - - setLoadingAnalytics(true); - try { - const analytics = await githubAPIClient.getUserAnalytics(userParam); - if (analytics) { - // Convert GitHubUserDetailed to the expected profile format - const convertedAnalytics: UserAnalytics = { - profile: analytics.profile ? { - avatar_url: analytics.profile.avatar_url, - login: analytics.profile.login, - type: analytics.profile.type, - bio: analytics.profile.bio, - public_repos: analytics.profile.public_repos, - followers: analytics.profile.followers, - following: analytics.profile.following, - location: analytics.profile.location, - company: analytics.profile.company, - html_url: analytics.profile.html_url - } : undefined, - overview: analytics.overview, - languages: analytics.languages, - behavior: analytics.behavior - }; - setUserAnalytics(convertedAnalytics); - } else { - setUserAnalytics(null); - } - } catch (error) { - console.error("Analytics error:", error); - // Fallback to null if API fails - setUserAnalytics(null); - } finally { - setLoadingAnalytics(false); - } - }, [userParam]); const loadUserAnalytics = useCallback(async () => { if (!userParam) return; const requestedUser = userParam; setLoadingAnalytics(true); try { const analytics = await githubAPIClient.getUserAnalytics(requestedUser); // Ignore if param changed while awaiting if (requestedUser !== userParam) return; - setUserAnalytics(analytics); + + if (analytics) { + // Convert GitHubUserDetailed to the expected profile format + const convertedAnalytics: UserAnalytics = { + profile: analytics.profile ? { + avatar_url: analytics.profile.avatar_url, + login: analytics.profile.login, + type: analytics.profile.type, + bio: analytics.profile.bio, + public_repos: analytics.profile.public_repos, + followers: analytics.profile.followers, + following: analytics.profile.following, + location: analytics.profile.location, + company: analytics.profile.company, + html_url: analytics.profile.html_url + } : undefined, + overview: analytics.overview, + languages: analytics.languages, + behavior: analytics.behavior + }; + setUserAnalytics(convertedAnalytics); + } else { + setUserAnalytics(null); + } } catch { // Fallback to null if API fails if (requestedUser !== userParam) return; setUserAnalytics(null); } finally { if (requestedUser === userParam) { setLoadingAnalytics(false); } } }, [userParam]);Also applies to: 219-238
src/lib/api/github-api-client.ts (1)
796-797: Null safety issue properly addressed.The previous review comment about null safety in commit filtering has been correctly fixed using the nullish coalescing operator (
?? ''). This prevents runtime errors whencommit.commit?.author?.nameis undefined.
🧹 Nitpick comments (1)
src/app/api/search/route.ts (1)
18-23: Consider extracting common search logic to reduce duplication.The search logic is identical between GET and POST handlers. Consider extracting this into a shared function to improve maintainability.
+async function performSearch(query: string, type: string, limit: number) { + let results + if (type === 'users') { + results = await githubAPIClient.searchUsers(query, 'all', limit) + } else { + results = await githubAPIClient.searchRepositories(query, 'stars', limit) + } + return results +} export async function GET(request: NextRequest) { try { // ... validation code ... - let results - if (type === 'users') { - results = await githubAPIClient.searchUsers(query, 'all', limit) - } else { - results = await githubAPIClient.searchRepositories(query, 'stars', limit) - } + const results = await performSearch(query, type, limit)Also applies to: 52-57
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/api/search/route.ts(1 hunks)src/app/search/page.tsx(4 hunks)src/components/quick-wins/hooks/useQuickWins.ts(4 hunks)src/lib/api/github-api-client.ts(11 hunks)src/lib/api/github-graphql-client.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/quick-wins/hooks/useQuickWins.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/api/search/route.ts (2)
src/lib/api/github-graphql-client.ts (1)
query(64-104)src/lib/api/github-api-client.ts (1)
githubAPIClient(884-884)
src/lib/api/github-api-client.ts (2)
src/types/github.ts (2)
GitHubUserDetailed(73-106)GitHubRepositoryDetailed(108-195)src/types/api.ts (1)
GitHubSearchResponse(126-130)
🪛 Biome (2.1.2)
src/app/search/page.tsx
[error] 219-219: Shouldn't redeclare 'loadUserAnalytics'. Consider to delete it or rename it.
'loadUserAnalytics' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (16)
src/lib/api/github-graphql-client.ts (3)
38-38: Good null safety improvement for stargazerCount.Updating the interface to reflect that
stargazerCountcan be null aligns with GitHub's actual API behavior and improves type safety.
172-172: Excellent null safety in filtering logic.The explicit checks for
issue.repository?.stargazerCountbefore comparing to 5 prevent potential runtime errors when the API returns null values. This defensive programming approach is much safer than the previous direct comparison.Also applies to: 262-262
282-282: Proper null handling in mapping operation.Using the nullish coalescing operator (
|| 0) provides a sensible default whenstargazerCountis null, ensuring the mappedGitHubIssuealways has a valid numericstarsvalue.src/app/api/search/route.ts (2)
4-38: Well-implemented GET handler with proper validation.The handler correctly validates the required query parameter, provides sensible defaults for optional parameters, and handles both user and repository searches appropriately. Error handling with logging and structured responses is good for API debugging.
40-72: POST handler mirrors GET functionality effectively.The POST implementation maintains consistency with the GET handler while properly parsing JSON request bodies. The parameter destructuring with defaults is clean and readable.
src/app/search/page.tsx (2)
67-72: Good type safety improvement with explicit null types.Making
bio,location, andcompanyexplicitlystring | nullinstead of optional provides better type safety and aligns with the actual GitHub API response structure.
264-264: Good dependency array update.Including
loadUserAnalyticsin the useEffect dependency array ensures the effect re-runs when the callback changes, maintaining proper React behavior.src/lib/api/github-api-client.ts (9)
5-8: Good type imports for enhanced data handling.Adding imports for
GitHubUserDetailedandGitHubRepositoryDetailedsupports the improved analytics functionality and provides better typing throughout the client.
110-111: Excellent granular cache timeout strategy.Separating cache timeouts for regular data (10 minutes) and commit data (30 minutes) optimizes performance by caching expensive commit operations longer while keeping other data relatively fresh.
176-180: Improved cache method with commit data optimization.The
isCommitDataparameter allows the cache to apply appropriate timeouts based on data type, which is a smart optimization for expensive GitHub API operations.
193-193: Correct authorization header format.Changing from
tokentoBearerfollows the current GitHub API authentication best practices.
214-215: Improved error handling removes unreliable fallbacks.Throwing errors instead of returning fallback data ensures the application handles failures appropriately rather than masking issues with potentially misleading demo data. This improves data integrity.
Also applies to: 231-232
545-551: Cleaner error handling in getUserProfile.Returning
nullon failure instead of fallback data is more honest about API failures and allows calling code to handle missing data appropriately.
614-616: Better error handling in getUserAnalytics.Throwing an error when no profile is found provides clear feedback about failures instead of silently returning demo data. This improves debugging and error detection.
Also applies to: 640-642
643-735: Excellent implementation of real commit counting.The
getRepositoryCommitCountmethod implements a sophisticated fallback strategy:
- Try GitHub's stats API first (most accurate)
- Fall back to paginated commit counting
- Final fallback with author name matching
This provides accurate commit data while handling various API limitations gracefully.
736-874: Comprehensive weekly behavior data implementation.The new methods for fetching real weekly commit, PR, and issue data provide accurate analytics instead of random demo data. The day-of-week mapping and rate limiting are well implemented.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Style
Chores