feat: dashboard k6 - 대시보드 리펙토링 버전#1
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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 refactors the dashboard to use a consolidated BFF (Backend For Frontend) endpoint for fetching summary data, replacing multiple individual queries for users, progress, and goals. It introduces a centralized DASHBOARD_STALE_TIME and updates performance test scripts to support the new API structure. Feedback focuses on a potential UX bug where initialData causes the GitHub connection modal to appear prematurely, a regression in query options for GitHub repositories, and a performance concern regarding the removal of prefetching for the goals list which may cause client-side waterfalls.
| await queryClient.prefetchQuery({ | ||
| queryKey: dashboardKeys.summary(), | ||
| queryFn: async () => (await getDashboardSummary()).data, | ||
| }); |
There was a problem hiding this comment.
goalQueries.list()에 대한 prefetch가 제거되었습니다. DashboardDetail 컴포넌트에서 해당 쿼리를 사용하고 있으므로, 서버 사이드에서 미리 데이터를 가져오지 않으면 클라이언트에서 추가적인 네트워크 요청이 발생하여 렌더링 지연(Waterfall)이 생길 수 있습니다. 사용자 경험 향상을 위해 dashboardKeys.summary()와 함께 병렬로 prefetch하고, DashboardDetail을 HydrationBoundary 안으로 포함시키는 것을 권장합니다.
There was a problem hiding this comment.
Pull request overview
This PR refactors the dashboard to use a consolidated “dashboard summary” data source, adds a Next.js BFF route for that summary, and updates performance (k6) scripts to benchmark the new request pattern.
Changes:
- Added
dashboardKeys/dashboardQueries.summary()and integrated a unified dashboard summary query. - Implemented a Next.js route handler at
/api/dashboard/summaryand updated client/server fetching behavior infetchDashboard. - Updated the dashboard k6 scenario to optionally use the new summary endpoint.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/lib/query/queryKeys.ts | Adds dashboard query + introduces shared staleTime constant; modifies existing query options. |
| src/shared/lib/query/keyFactory.ts | Adds dashboardKeys for the new summary query key. |
| src/shared/lib/api/fetchDashboard.ts | Implements client-side fetch to /api/dashboard/summary while keeping server-side aggregation. |
| src/perf/dashboard-http.js | Adds SUMMARY_FETCH_MODE and summary-endpoint scenario path for k6. |
| src/features/dashboard/components/DashboardSummary/index.tsx | Switches dashboard summary UI to read from the new summary query and passes data down to cards. |
| src/features/dashboard/components/DashboardDetail/index.tsx | Removes unused useEffect import. |
| src/app/api/dashboard/summary/route.ts | Adds BFF route for aggregated dashboard summary. |
| src/app/(main)/dashboard/page.tsx | Prefetches dashboard summary only; adds DataBoundary with skeleton fallback around detail section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const githubGoals = dashboardSummaryData?.todos?.filter((goal) => goal.source === 'GITHUB') ?? []; | ||
| const isGithubDisconnectedSession = | ||
| typeof window !== 'undefined' && window.sessionStorage.getItem(GITHUB_DISCONNECTED_SESSION_KEY) === 'true'; |
There was a problem hiding this comment.
githubGoals is now derived from dashboardSummaryData.todos (recent todos) rather than from the goal list. This changes the connect-modal condition to “no recent GitHub-sourced todos” instead of “no GitHub goals”, which can incorrectly open the GitHub connect modal even when GitHub goals exist but recent todos are all MANUAL. Consider keeping the goals query for this check, or extend the dashboard summary API to include GitHub goal existence/count and use that instead.
| @@ -99,10 +115,6 @@ export const githubQueries = { | |||
| queryOptions({ | |||
| queryKey: githubKeys.repositories(), | |||
| queryFn: fetchGithubIntegrations.getRepositories, | |||
There was a problem hiding this comment.
The githubQueries.repositories options were removed (staleTime=5m, retry=false, refetchOnWindowFocus=false). With the current QueryClient defaults (staleTime 60s, retry 3), this query will refetch more frequently and retry on failures, which can increase GitHub API traffic and make the connect modal feel noisy (esp. on window focus). If the previous behavior was intentional, reintroduce these options here or set an equivalent default specifically for this query key.
| queryFn: fetchGithubIntegrations.getRepositories, | |
| queryFn: fetchGithubIntegrations.getRepositories, | |
| staleTime: DASHBOARD_STALE_TIME, | |
| retry: false, | |
| refetchOnWindowFocus: false, |
| const DASHBOARD_STALE_TIME = 1000 * 60 * 5; | ||
| // goal queries |
There was a problem hiding this comment.
DASHBOARD_STALE_TIME is applied to goal/todo/user queries used across multiple non-dashboard pages (e.g., calendar, sidebar, todo modals). The name is misleading and makes it harder to reason about caching scope. Consider renaming to a generic constant (e.g., DEFAULT_STALE_TIME_MS) or scoping it only to dashboard-specific queries.
| <DataBoundary suspenseFallback={<DashboardDetailSkeleton />}> | ||
| <DashboardDetail /> | ||
| </div> | ||
| </HydrationBoundary> | ||
| </DataBoundary> |
There was a problem hiding this comment.
DataBoundary's suspenseFallback will only render if a child suspends. DashboardDetail uses useQuery (not useSuspenseQuery / suspense: true), so it won’t suspend and DashboardDetailSkeleton likely never appears. If the intent is to show this skeleton during query loading, enable React Query suspense for the queries used by DashboardDetail (or render the skeleton based on isLoading instead).
| export async function GET() { | ||
| try { | ||
| const result = await fetchDashboard.getDashboardSummaryResult(); | ||
|
|
||
| return NextResponse.json(result, { | ||
| status: result.hasAnySuccess ? 200 : 502, | ||
| headers: { | ||
| 'Cache-Control': 'no-store', | ||
| }, | ||
| }); | ||
| } catch { | ||
| return NextResponse.json({ message: 'Failed to fetch dashboard summary' }, { status: 502 }); |
There was a problem hiding this comment.
The route returns 502 when hasAnySuccess is false, which will also happen for auth failures (e.g., upstream 401 for user/progress/todos when no/expired token). That masks authentication problems as a gateway error and makes it harder for the client to handle login/refresh flows correctly. Consider detecting ApiError rejections with status 401/403 inside getDashboardSummaryResult (or in this route) and returning that status (or a structured error) instead of 502.
| export async function GET() { | |
| try { | |
| const result = await fetchDashboard.getDashboardSummaryResult(); | |
| return NextResponse.json(result, { | |
| status: result.hasAnySuccess ? 200 : 502, | |
| headers: { | |
| 'Cache-Control': 'no-store', | |
| }, | |
| }); | |
| } catch { | |
| return NextResponse.json({ message: 'Failed to fetch dashboard summary' }, { status: 502 }); | |
| function findAuthStatus(value: unknown, seen = new WeakSet<object>()): 401 | 403 | undefined { | |
| if (!value || (typeof value !== 'object' && typeof value !== 'function')) { | |
| return undefined; | |
| } | |
| const candidate = value as { | |
| status?: unknown; | |
| statusCode?: unknown; | |
| response?: unknown; | |
| cause?: unknown; | |
| error?: unknown; | |
| errors?: unknown; | |
| }; | |
| if (candidate.status === 401 || candidate.statusCode === 401) { | |
| return 401; | |
| } | |
| if (candidate.status === 403 || candidate.statusCode === 403) { | |
| return 403; | |
| } | |
| if (seen.has(value as object)) { | |
| return undefined; | |
| } | |
| seen.add(value as object); | |
| const nestedValues = [ | |
| candidate.response, | |
| candidate.cause, | |
| candidate.error, | |
| candidate.errors, | |
| ...Object.values(candidate), | |
| ]; | |
| for (const nestedValue of nestedValues) { | |
| const authStatus = findAuthStatus(nestedValue, seen); | |
| if (authStatus) { | |
| return authStatus; | |
| } | |
| } | |
| return undefined; | |
| } | |
| export async function GET() { | |
| try { | |
| const result = await fetchDashboard.getDashboardSummaryResult(); | |
| const authStatus = !result.hasAnySuccess ? findAuthStatus(result) : undefined; | |
| return NextResponse.json(result, { | |
| status: authStatus ?? (result.hasAnySuccess ? 200 : 502), | |
| headers: { | |
| 'Cache-Control': 'no-store', | |
| }, | |
| }); | |
| } catch (error) { | |
| const authStatus = findAuthStatus(error); | |
| return NextResponse.json( | |
| { message: 'Failed to fetch dashboard summary' }, | |
| { status: authStatus ?? 502 }, | |
| ); |
No description provided.