🧪 [Test] Handle empty history in fetchContributions gracefully#358
🧪 [Test] Handle empty history in fetchContributions gracefully#358is0692vs wants to merge 24 commits into
Conversation
…y to return null Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughmostActiveDay を nullable にし、型定義、集計ロジック、UI 表示、テスト、API モックを連鎖的に null 対応へ更新したリファクタリング。 ChangesmostActiveDay null 対応化
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the mostActiveDay field to be nullable (string | null) across types, utility functions, and components, ensuring that empty contribution histories are handled correctly by returning null instead of an empty string or a default day. Corresponding updates were made to the API route tests and component rendering logic. However, the test file fetchContributions.test.ts contains three duplicate test cases for handling empty history (two in Japanese and one in English) which should be cleaned up to avoid redundancy.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptile review |
…252113264336071474
|
@greptile review |
…y to return null Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
…y to return null Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/lib/github.ts (1)
91-107: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
calculateMostActiveDayを共通ユーティリティに統一してください。Line 91-107 は
src/lib/yearInReviewUtils.tsのgetMostActiveDayFromCalendarと責務が重複しており、すでに「無効日付の扱い」「0 以下カウントの扱い」で実装差が出ています。ここを一本化しておくと、mostActiveDay契約変更時の再乖離を防げます。♻️ 提案差分
+import { getMostActiveDayFromCalendar } from "`@/lib/yearInReviewUtils`"; ... function calculateMostActiveDay(calendar: { date: string; count: number }[]): string | null { - const weekdayNames = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]; - const weekdayTotals = Array.from({ length: 7 }, () => 0); - - for (const day of calendar) { - if (day.count === 0) { - continue; - } - const weekday = new Date(`${day.date}T00:00:00Z`).getUTCDay(); - weekdayTotals[weekday] += day.count; - } - - const maxWeekdayTotal = Math.max(...weekdayTotals); - return maxWeekdayTotal > 0 - ? weekdayNames[weekdayTotals.findIndex((count) => count === maxWeekdayTotal)] - : null; + return getMostActiveDayFromCalendar(calendar); }🤖 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/lib/github.ts` around lines 91 - 107, calculateMostActiveDay duplicates logic already implemented in getMostActiveDayFromCalendar; extract the common logic into a single shared utility (e.g., export function getMostActiveDayFromCalendar or mostActiveDayFromCalendar) that normalizes invalid dates, ignores non-positive counts, computes weekday totals and returns the weekday name or null, then replace the calculateMostActiveDay implementation with an import/use of that shared utility in place of the local function (update callers that used calculateMostActiveDay and the existing getMostActiveDayFromCalendar to reference the single exported function and remove the duplicate implementation).
🤖 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/lib/github.ts`:
- Around line 91-107: calculateMostActiveDay duplicates logic already
implemented in getMostActiveDayFromCalendar; extract the common logic into a
single shared utility (e.g., export function getMostActiveDayFromCalendar or
mostActiveDayFromCalendar) that normalizes invalid dates, ignores non-positive
counts, computes weekday totals and returns the weekday name or null, then
replace the calculateMostActiveDay implementation with an import/use of that
shared utility in place of the local function (update callers that used
calculateMostActiveDay and the existing getMostActiveDayFromCalendar to
reference the single exported function and remove the duplicate implementation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b0996ad-9e2a-45d9-83b4-292509ff3c70
📒 Files selected for processing (11)
src/app/api/dashboard/year/route.test.tssrc/components/ContributionsCard.tsxsrc/components/YearInReviewCarousel.test.tsxsrc/components/YearInReviewCarousel.tsxsrc/components/YearInReviewSlide.test.tsxsrc/components/YearInReviewSlide.tsxsrc/lib/__tests__/github/fetchContributions.test.tssrc/lib/__tests__/yearInReviewUtils.test.tssrc/lib/github.tssrc/lib/types.tssrc/lib/yearInReviewUtils.ts
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/ContributionsCard.tsx`:
- Around line 154-156: Add a test case in
src/components/__tests__/ContributionsCard.test.tsx that passes
contributions.mostActiveDay = null to the ContributionsCard component and
asserts that MostActiveDayCard is not rendered; locate the ContributionsCard
component (rendering conditional: {contributions.mostActiveDay &&
<MostActiveDayCard ... />}) and update or add a test using the existing test
harness (render/mount utilities already in the file) to render ContributionsCard
with mostActiveDay: null and verify via queryByText/queryByTestId or container
that MostActiveDayCard does not appear.
In `@src/lib/yearInReviewUtils.ts`:
- Line 99: Update the JSDoc for getMostActiveDay (the function returning string
| null) to explicitly document the null case: change the `@returns` line to state
it returns the weekday name (e.g., "Monday") or null when there are no
contributions/activities to determine a most active day; ensure the doc matches
the function signature string | null and mentions the condition that causes null
to be returned.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0c38363-dd1f-48e3-a04a-72b719a00eec
📒 Files selected for processing (3)
src/components/ContributionsCard.tsxsrc/lib/__tests__/github/fetchContributions.test.tssrc/lib/yearInReviewUtils.ts
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
@greptile review |
1 similar comment
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
…story-test-2252113264336071474' into pr-358-review-fix
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
@greptile review |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
Superseded by #359. The Jules branch kept adding commits that reverted review fixes, so the completed empty-history handling was moved to a stable branch. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
🎯 What: The
fetchContributionsfunction previously lacked test coverage for scenarios where the GitHub GraphQL response contained an empty contribution history.📊 Coverage: A new test case was added to
src/lib/__tests__/github/fetchContributions.test.tsto mock an empty history response and ensure calculations gracefully return 0/null/empty arrays.✨ Result: Enhanced the resilience of the application by fixing underlying bugs where
mostActiveDaydefaulted improperly. Changed the signature to returnstring | nullinstead of""or"Sunday", and implemented the corresponding type and UI rendering updates insrc/components/ContributionsCard.tsx. Full test suite passing.PR created automatically by Jules for task 2252113264336071474 started by @is0692vs
Greptile Summary
このPRは、コントリビューション履歴が空の場合に
fetchContributionsがmostActiveDayを空文字列""ではなくnullとして返すよう修正し、型定義・UIコンポーネント・テスト全体に伝播させるものです。ContributionData/YearInReviewDataのmostActiveDay型をstring → string | nullに変更し、ContributionsCard・YearInReviewSlide・YearInReviewCarouselの各コンポーネントでnullの場合の表示を適切にガードfetchContributionsの空履歴ケースをカバーする新テスト追加、yearInReviewUtils.tsの JSDoc 更新、およびvalidators.test.tsのテスト構造リファクタリングConfidence Score: 5/5
変更はテスト追加と null ガードの追加のみで、既存のデータフローを壊すリスクはほぼありません。
型変更（string → string | null）が ContributionData・YearInReviewData・UIコンポーネント・テストまで一貫して適用されており、null によるレンダリングバグも適切に防止されています。ロジックの正確性・テスト網羅性ともに問題なく、本番動作に影響するリスクは見当たりません。
特に注意が必要なファイルはありません。fetchContributions.test.ts のテスト名と実際のモックデータの不一致は軽微なドキュメント上の問題にとどまります。
Important Files Changed
calculateMostActiveDayの戻り値型を `stringContributionDataとYearInReviewDataのmostActiveDayフィールドを `string → stringgetMostActiveDayFromCalendarを `stringshowMostActiveDay変数を廃止しnulltruthy チェックに置き換え。型安全になり null 時に MostActiveDayCard が非表示になる。it.eachスタイルから個別itブロックへのリファクタリング。一部の境界値ケースが脱落しているが主要カバレッジは維持されている。Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[GitHub GraphQL Response] --> B{contributionDays empty or all zero?} B -->|Yes| C[calendar = empty array] B -->|No| D[calendar = date/count entries] C --> E[calculateMostActiveDay: weekdayTotals all zeros] D --> F[calculateMostActiveDay: accumulate weekday totals] E --> G[returns null] F --> H{maxWeekdayTotal > 0?} H -->|Yes| I[returns weekday name] H -->|No| G G --> J[mostActiveDay = null] I --> K[mostActiveDay = weekday string] J --> L[ContributionsCard: hide MostActiveDayCard] K --> M[ContributionsCard: show MostActiveDayCard] J --> N[YearInReviewSlide: hide badge] J --> P[YearInReviewCarousel: show fallback caption] K --> Q[YearInReviewCarousel: show normal caption]Prompt To Fix All With AI
Reviews (11): Last reviewed commit: "test: address minor review feedbacks" | Re-trigger Greptile