feat(about): combine separate GraphQL queries into single request#3604
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConsolidates multiple About-page GraphQL queries into a single Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/__tests__/e2e/pages/About.spec.ts (1)
7-29: Ensure non‑matching GraphQL requests are continued.Playwright route handlers must explicitly handle all intercepted requests by calling
continue(),fulfill(),abort(), orfallback(). Currently, any GraphQL operation other thanGetAboutPageDatais left unhandled and will hang indefinitely, causing test timeouts if any additional query fires (e.g., from future layout or header changes).✅ Fix
if (postData.operationName === 'GetAboutPageData') { const leaders = ['arkid15r', 'kasya', 'mamicidal'] const leaderData = leaders.reduce( (acc, leader, index) => ({ ...acc, [`leader${index + 1}`]: mockAboutData.users[leader], }), {} ) - await route.fulfill({ + return route.fulfill({ status: 200, json: { data: { project: mockAboutData.project, topContributors: mockAboutData.topContributors, ...leaderData, }, }, }) } + return route.continue()
🧹 Nitpick comments (2)
frontend/src/server/queries/aboutQueries.ts (1)
3-82: Consider extracting a shared fragment for leader fields.This selection set is repeated three times; a fragment keeps future changes in sync.
♻️ Possible refactor
export const GET_ABOUT_PAGE_DATA = gql` + fragment LeaderUserFields on User { + id + avatarUrl + login + name + badgeCount + badges { + cssClass + description + id + name + weight + } + } query GetAboutPageData( $projectKey: String! @@ - leader1: user(login: $leader1) { - id - avatarUrl - login - name - badgeCount - badges { - cssClass - description - id - name - weight - } - } - leader2: user(login: $leader2) { - id - avatarUrl - login - name - badgeCount - badges { - cssClass - description - id - name - weight - } - } - leader3: user(login: $leader3) { - id - avatarUrl - login - name - badgeCount - badges { - cssClass - description - id - name - weight - } - } + leader1: user(login: $leader1) { + ...LeaderUserFields + } + leader2: user(login: $leader2) { + ...LeaderUserFields + } + leader3: user(login: $leader3) { + ...LeaderUserFields + } } `frontend/src/app/about/page.tsx (1)
14-84: Guard leader descriptions and names against missing mappings.If a leader login isn’t in the
leadersmap or the name is missing, the UI can receiveundefined. Add nullish fallbacks.🛠️ Suggested tweak
- const leadersData = [data?.leader1, data?.leader2, data?.leader3].filter(Boolean).map((user) => ({ - description: user?.login ? leaders[user.login as keyof typeof leaders] : '', - memberName: user?.name || user?.login, - member: user, - })) + const leadersData = [data?.leader1, data?.leader2, data?.leader3].filter(Boolean).map((user) => ({ + description: user?.login ? leaders[user.login as keyof typeof leaders] ?? '' : '', + memberName: user?.name ?? user?.login ?? '', + member: user, + }))
|
@arkid15r , please review it and let me know if any changes are required. |
|
@arkid15r , please review it and let me know if any changes are required. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3604 +/- ##
==========================================
- Coverage 87.70% 87.69% -0.01%
==========================================
Files 462 462
Lines 14287 14271 -16
Branches 1905 1900 -5
==========================================
- Hits 12530 12515 -15
Misses 1344 1344
+ Partials 413 412 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@cubic-dev-ai review this pull request |
@arkid15r I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
2 participants
Proposed change
Resolves #3592
This PR optimizes the About page performance by combining 5 separate API requests into a single GraphQL query.
Changes:
GetAboutPageDataquery to fetch project, contributors, and leaders data in one go.About pagecomponent to use this single query.Checklist
make check-testlocally: all warnings addressed, tests passed