Fix empty contribution history active-day handling#359
Conversation
…y to return null Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
…252113264336071474
…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>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
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
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
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? |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Review limit reached
More reviews will be available in 17 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ 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 |
|
@greptile review |
![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) instead of a default string when there are no contributions, adjusting the underlying calculation logic, types, UI components, and tests accordingly. The reviewer points out that refactoring the isValidGitHubUsername tests from it.each to individual it blocks in validators.test.ts increases boilerplate and accidentally removes several critical test cases, recommending a revert to the parameterized it.each pattern.
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.
| // ---------- 有効なユーザー名 ---------- | ||
| it("英数字のみのユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("testuser")).toBe(true); | ||
| }); | ||
|
|
||
| it("1文字のユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("a")).toBe(true); | ||
| }); | ||
|
|
||
| it("数字のみのユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("12345")).toBe(true); | ||
| }); | ||
|
|
||
| it("ハイフンを含むユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("test-user")).toBe(true); | ||
| }); | ||
|
|
||
| it("複数ハイフンを含むユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("my-test-user")).toBe(true); | ||
| }); | ||
|
|
||
| it("39文字のユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("a".repeat(39))).toBe(true); | ||
| }); | ||
|
|
||
| it("大文字を含むユーザー名は有効", () => { | ||
| expect(isValidGitHubUsername("TestUser")).toBe(true); | ||
| }); | ||
|
|
||
| // ---------- 無効なユーザー名 ---------- | ||
| it("空文字列は無効", () => { | ||
| expect(isValidGitHubUsername("")).toBe(false); | ||
| }); | ||
|
|
||
| it("null/undefined 入力は実行時に無効", () => { | ||
| expect(isValidGitHubUsername(null as unknown as string)).toBe(false); | ||
| expect(isValidGitHubUsername(undefined as unknown as string)).toBe(false); | ||
| }); | ||
|
|
||
| it("ハイフンで始まるユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("-testuser")).toBe(false); | ||
| }); | ||
|
|
||
| it("ハイフンで終わるユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("testuser-")).toBe(false); | ||
| }); | ||
|
|
||
| it("連続ハイフンを含むユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("test--user")).toBe(false); | ||
| }); | ||
|
|
||
| it("40文字以上のユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("a".repeat(40))).toBe(false); | ||
| }); | ||
|
|
||
| it("特殊文字を含むユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("test@user")).toBe(false); | ||
| expect(isValidGitHubUsername("test.user")).toBe(false); | ||
| expect(isValidGitHubUsername("test_user")).toBe(false); | ||
| expect(isValidGitHubUsername("test user")).toBe(false); | ||
| }); | ||
|
|
||
| it("スラッシュを含むユーザー名は無効 (パストラバーサル防止)", () => { | ||
| expect(isValidGitHubUsername("test/user")).toBe(false); | ||
| expect(isValidGitHubUsername("../etc/passwd")).toBe(false); | ||
| }); | ||
|
|
||
| it("SQLインジェクション的な文字列は無効", () => { | ||
| expect(isValidGitHubUsername("'; DROP TABLE users; --")).toBe(false); | ||
| }); | ||
|
|
||
| it("マルチバイト文字（日本語や絵文字）を含むユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername("テスト")).toBe(false); | ||
| expect(isValidGitHubUsername("user😀")).toBe(false); | ||
| expect(isValidGitHubUsername("déjà-vu")).toBe(false); | ||
| }); | ||
|
|
||
| it("空白文字や制御文字を含むユーザー名は無効", () => { | ||
| expect(isValidGitHubUsername(" testuser")).toBe(false); | ||
| expect(isValidGitHubUsername("testuser ")).toBe(false); | ||
| expect(isValidGitHubUsername("test\nuser")).toBe(false); | ||
| expect(isValidGitHubUsername("test\tuser")).toBe(false); | ||
| expect(isValidGitHubUsername("test\0user")).toBe(false); | ||
| }); | ||
|
|
||
| it("極端に長い文字列は無効 (長さ上限の確認)", () => { | ||
| expect(isValidGitHubUsername("a".repeat(1000))).toBe(false); | ||
| }); | ||
|
|
||
| }); |
There was a problem hiding this comment.
The rewrite of isValidGitHubUsername tests from it.each to individual it blocks significantly increases boilerplate code and reduces maintainability. More importantly, it resulted in a loss of several critical test cases (e.g., '1文字の数字', '大文字とハイフン', '39文字の数字', '38文字の英字', 'ハイフンが複数あるが連続していない', '39文字でハイフンを含む', '40文字（ハイフン含む）', '先頭がアンダースコア', and '末尾がアンダースコア'). Reverting to the parameterized it.each pattern keeps the test suite concise, clean, and ensures full test coverage is preserved.
describe("有効なユーザー名 (Valid usernames)", () => {
it.each([
["英数字のみ", "testuser"],
["1文字の英字", "a"],
["1文字の数字", "1"],
["数字のみ", "12345"],
["ハイフンを含む", "test-user"],
["複数ハイフンを含む", "my-test-user"],
["大文字を含む", "TestUser"],
["大文字とハイフン", "Test-User"],
["39文字の英字", "a".repeat(39)],
["39文字の数字", "1".repeat(39)],
["38文字の英字", "a".repeat(38)],
["ハイフンが複数あるが連続していない", "a-b-c-d-e"],
["39文字でハイフンを含む", "a" + "b".repeat(36) + "-c"],
])("%s: %p", (_, username) => {
expect(isValidGitHubUsername(username)).toBe(true);
});
});
describe("無効なユーザー名 (Invalid usernames)", () => {
it.each([
["空文字列", ""],
["ハイフンで始まる", "-testuser"],
["ハイフンで終わる", "testuser-"],
["連続ハイフンを含む", "test--user"],
["40文字", "a".repeat(40)],
["40文字（ハイフン含む）", "a" + "b".repeat(37) + "-c"],
["先頭がアンダースコア", "_testuser"],
["末尾がアンダースコア", "testuser_"],
["特殊文字を含む(@)", "test@user"],
["特殊文字を含む(.)", "test.user"],
["特殊文字を含む(_)", "test_user"],
["空白を含む", "test user"],
["先頭に空白", " testuser"],
["末尾に空白", "testuser "],
["制御文字(改行)を含む", "test\\nuser"],
["制御文字(タブ)を含む", "test\\tuser"],
["ヌル文字を含む", "test\\0user"],
["パストラバーサル", "../etc/passwd"],
["パストラバーサル(/)", "test/user"],
["SQLインジェクション", "'; DROP TABLE users; --"],
["日本語", "テスト"],
["絵文字", "user😀"],
["アクセント記号", "déjà-vu"],
["極端に長い文字列", "a".repeat(1000)],
["undefined (型キャスト)", undefined as unknown as string],
["null (型キャスト)", null as unknown as string],
])("%s: %p", (_, username) => {
expect(isValidGitHubUsername(username)).toBe(false);
});
});
|
@greptile review |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Supersedes #358.\n\nThis keeps the empty-history fix and review follow-ups in a stable branch:\n- mostActiveDay returns null for empty history\n- null-safe ContributionsCard and YearInReview UI\n- duplicate empty-history tests removed\n- runtime username guard tests restored\n\nLocal verification:\n- npm run test -- src/components/YearInReviewCarousel.test.tsx src/components/YearInReviewSlide.test.tsx src/components/tests/ContributionsCard.test.tsx src/lib/tests/validators.test.ts src/lib/tests/github/fetchContributions.test.ts
Greptile Summary
このPRは、コントリビューション履歴が空の場合に
mostActiveDayが""ではなくnullを返すように修正し、UIコンポーネント側での null-safe レンダリングも合わせて対応しています。ContributionDataおよびYearInReviewDataの型をmostActiveDay: string | nullに変更し、calculateMostActiveDay/getMostActiveDayFromCalendarの両関数で空時はnullを返すよう修正。ContributionsCard、YearInReviewSlide、YearInReviewCarouselの各コンポーネントを null チェック対応に更新し、空履歴時のフォールバック表示を追加。nullに合わせて更新。Confidence Score: 5/5
変更はすべて空コントリビューション履歴の null ハンドリングに絞られており、既存の正常パスには影響しない。
修正対象の関数・コンポーネントすべてが一貫して null を扱うよう更新されており、各変更に対応するテストも追加されている。ロジックの抜け漏れや型の不整合は見当たらない。
特に注意が必要なファイルはない。
Important Files Changed
ContributionDataとYearInReviewDataのmostActiveDayを `stringcalculateMostActiveDayの戻り値を `stringgetMostActiveDayFromCalendarを `stringmostActiveDay.length > 0チェックをmostActiveDayの truthy チェックに置き換え。null-safe になり、型変更と整合している。mostActiveDayが null の場合のフォールバックキャプションを追加。"Most active on null" が表示されるバグを修正している。mostActiveDayが null の場合にバッジを非表示にする条件分岐を追加。null が文字列として描画されるバグを修正。Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[GitHub API レスポンス] --> B[fetchContributions] B --> C{contributionDays に\nコントリビューションあり?} C -- あり --> D["calculateMostActiveDay(calendar)\n→ string"] C -- なし/全て0 --> E["calculateMostActiveDay(calendar)\n→ null"] D --> F[mostActiveDay: string] E --> G[mostActiveDay: null] F --> H{UI コンポーネント} G --> H H -- truthy --> I[バッジ・キャプション表示] H -- null --> J[バッジ非表示 / フォールバック表示]Comments Outside Diff (2)
src/lib/__tests__/validators.test.ts, line 14-107 (link)it.eachから個別itへの書き直しで、いくつかの境界テストケースが削除されています。具体的には"1"（1文字の数字）、"a".repeat(38)（38文字）、"a-b-c-d-e"（複数ハイフン非連続）、"a" + "b".repeat(36) + "-c"（39文字・ハイフン含む）などがなくなっています。いずれも既存バリデーターロジックで正しく処理されるため実害はありませんが、将来的なリグレッションを防ぐためにこれらのケースを追加し直すことを検討してください。Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
src/components/__tests__/ContributionsCard.test.tsx, line 19 (link)mostActiveDayの型がstring | nullに変更されましたが、このテストフィクスチャでは旧来の空文字列""が残っています。空文字列は falsy なので動作上の問題はありませんが、このPRの意図（空コントリビューション履歴にはnullを使う）と意味的に不一致です。一貫性のためにnullに更新することを推奨します。Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (3): Last reviewed commit: "test: restore username validator boundar..." | Re-trigger Greptile