diff --git a/src/app/api/metrics/prs/route.ts b/src/app/api/metrics/prs/route.ts index 5c9f7554..fd696827 100644 --- a/src/app/api/metrics/prs/route.ts +++ b/src/app/api/metrics/prs/route.ts @@ -1,342 +1,83 @@ +import { NextResponse } from "next/server"; import { getServerSession } from "next-auth"; -import { NextRequest } from "next/server"; import { authOptions } from "@/lib/auth"; -import { - getAccountToken, - getAllAccounts, - mergeMetrics, -} from "@/lib/github-accounts"; -import { GITHUB_API } from "@/lib/github"; -import { - isMetricsCacheBypassed, - METRICS_CACHE_TTL_SECONDS, - metricsCacheKey, - withMetricsCache, -} from "@/lib/metrics-cache"; -import { supabaseAdmin } from "@/lib/supabase"; -import { resolveAppUser } from "@/lib/resolve-user"; - -export const dynamic = "force-dynamic"; - -interface PRMetricsBase { - open: number; - merged: number; - closed: number; - total: number; - avgReviewHours: number; - avgFirstReviewHours: number | null; - mergeRate: number; -} - -interface PullRequestSearchItem { - state: string; - created_at: string; - closed_at: string | null; - number: number; - repository_url: string; - pull_request?: { merged_at: string | null }; -} - -interface ReviewEvent { - submitted_at?: string | null; -} - -interface ReviewCommentEvent { - created_at?: string | null; -} - -function getRepoFullName(repositoryUrl: string): string | null { - const marker = "/repos/"; - const index = repositoryUrl.indexOf(marker); - return index >= 0 ? repositoryUrl.slice(index + marker.length) : null; -} - -function getEarliestTimestamp(values: Array) { - const timestamps = values - .filter((value): value is string => Boolean(value)) - .map((value) => new Date(value).getTime()) - .filter((value) => !Number.isNaN(value)); - - return timestamps.length > 0 ? Math.min(...timestamps) : null; -} - -async function fetchFirstReviewTimestamp( - token: string, - pr: PullRequestSearchItem -): Promise { - const repo = getRepoFullName(pr.repository_url); - - if (!repo) { - return null; - } - - const headers = { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github+json", - }; - const [reviewsRes, commentsRes] = await Promise.all([ - fetch(`${GITHUB_API}/repos/${repo}/pulls/${pr.number}/reviews?per_page=100`, { - headers, - cache: "no-store", - }), - fetch(`${GITHUB_API}/repos/${repo}/pulls/${pr.number}/comments?per_page=100`, { - headers, - cache: "no-store", - }), - ]); - - if (!reviewsRes.ok || !commentsRes.ok) { - return null; - } - - const reviews = (await reviewsRes.json()) as ReviewEvent[]; - const comments = (await commentsRes.json()) as ReviewCommentEvent[]; - - return getEarliestTimestamp([ - ...reviews.map((review) => review.submitted_at), - ...comments.map((comment) => comment.created_at), - ]); -} - -async function getAverageFirstReviewHours( - token: string, - prs: PullRequestSearchItem[] -): Promise { - const reviewedPrs = await Promise.all( - prs.slice(0, 30).map(async (pr) => { - const firstReviewAt = await fetchFirstReviewTimestamp(token, pr); - - if (!firstReviewAt) { - return null; - } - - const openedAt = new Date(pr.created_at).getTime(); - if (Number.isNaN(openedAt) || firstReviewAt < openedAt) { - return null; - } - - return (firstReviewAt - openedAt) / 3600000; - }) - ); - const validDurations = reviewedPrs.filter( - (value): value is number => typeof value === "number" - ); - - if (validDurations.length === 0) { - return null; - } - - const average = - validDurations.reduce((sum, value) => sum + value, 0) / - validDurations.length; - - return Math.round(average * 10) / 10; -} - -async function fetchPRMetrics(token: string): Promise { - const searchRes = await fetch( - `${GITHUB_API}/search/issues?q=type:pr+author:@me&sort=updated&order=desc&per_page=100`, - { - headers: { Authorization: `Bearer ${token}` }, - cache: "no-store", - } - ); - - if (!searchRes.ok) { - throw new Error("GitHub API error"); - } - - const data = (await searchRes.json()) as { - total_count: number; - items: PullRequestSearchItem[]; - }; - - const open = data.items.filter((pr) => pr.state === "open").length; - - // A PR with state "closed" may have been merged OR closed without merging - // (e.g. rejected, abandoned). Only count those with a non-null merged_at - // as truly merged so the dashboard does not inflate the merged count. - const merged = data.items.filter( - (pr) => pr.pull_request?.merged_at != null - ).length; - - // Closed without merging (rejected / abandoned) - const closed = data.items.filter( - (pr) => pr.state === "closed" && pr.pull_request?.merged_at == null - ).length; - - // Average review time: use only actually merged PRs so we measure the time - // from open to merge, not open to close-without-merge. - const mergedPRs = data.items.filter( - (pr) => pr.pull_request?.merged_at != null - ); - const avgReviewMs = - mergedPRs.length > 0 - ? mergedPRs.reduce( - (sum, pr) => - sum + - (new Date(pr.pull_request!.merged_at!).getTime() - - new Date(pr.created_at).getTime()), - 0 - ) / mergedPRs.length - : 0; - - // Use the number of fetched items as the denominator for mergeRate. - // data.total_count is the all-time GitHub total (potentially thousands) - // while data.items is capped at 100, so dividing merged/total_count - // produces a near-zero rate for any active user. The fetched sample - // (open + merged + closed-without-merge) is the correct base. - const sampleTotal = data.items.length; - const avgFirstReviewHours = await getAverageFirstReviewHours( - token, - data.items - ); - - return { - open, - merged, - closed, - total: data.total_count, - avgReviewHours: Math.round(avgReviewMs / 3600000), - avgFirstReviewHours, - mergeRate: sampleTotal > 0 ? merged / sampleTotal : 0, - }; -} - -async function fetchCachedPRMetrics( - token: string, - cacheContext: { bypass: boolean; userId: string } -): Promise { - const key = metricsCacheKey(cacheContext.userId, "prs"); - - return withMetricsCache( - { - bypass: cacheContext.bypass, - key, - ttlSeconds: METRICS_CACHE_TTL_SECONDS.prs, +import { withMetricsCache } from "@/lib/cache"; + +async function fetchPRMetrics(token: string, githubLogin: string) { + const thirtyDaysAgo = new Date(); + thirtyDaysAgo.setDate(thirtyDaysAgo.getDate() - 30); + const dateString = thirtyDaysAgo.toISOString().split('T')[0]; + + // Scoped to last 30 days for accurate semantics as requested + const query = `search(type: ISSUE, query: "is:pr reviewed-by:${githubLogin} created:>=${dateString}", first: 100) { + issueCount + }`; + + const response = await fetch("https://api.github.com/graphql", { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", }, - () => fetchPRMetrics(token) - ); -} + body: JSON.stringify({ query: `query { ${query} }` }), + }); + + const json = await response.json(); + const reviewsGiven = json.data?.search?.issueCount || 0; + + // Fetch total PRs authored by user to compute ratio cleanly + const prQuery = `search(type: ISSUE, query: "is:pr author:${githubLogin} created:>=${dateString}", first: 1) { + issueCount + }`; + + const prResponse = await fetch("https://api.github.com/graphql", { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ query: `query { ${prQuery} }` }), + }); + + const prJson = await prResponse.json(); + const prsAuthored = prJson.data?.search?.issueCount || 0; + + const reviewRatio = prsAuthored > 0 ? parseFloat((reviewsGiven / prsAuthored).toFixed(2)) : 0; -function formatPRMetrics(metrics: PRMetricsBase) { return { - open: metrics.open, - merged: metrics.merged, - closed: metrics.closed, - total: metrics.total, - avgReviewHours: metrics.avgReviewHours, - avgFirstReviewHours: metrics.avgFirstReviewHours, - mergeRate: - metrics.total > 0 - ? `${Math.round(metrics.mergeRate * 100)}%` - : "0%", + reviewsGiven, + reviewRatio, }; } -export async function GET(req: NextRequest) { +export const GET = withMetricsCache(async (req: Request) => { const session = await getServerSession(authOptions); - if (!session?.accessToken) { - return Response.json({ error: "Unauthorized" }, { status: 401 }); - } - - const accountId = req.nextUrl.searchParams.get("accountId"); - const bypass = isMetricsCacheBypassed(req); - if (!accountId) { - try { - const result = await fetchCachedPRMetrics(session.accessToken, { - bypass, - userId: session.githubId ?? session.githubLogin ?? "primary", - }); - return Response.json(formatPRMetrics(result)); - } catch { - return Response.json({ error: "GitHub API error" }, { status: 502 }); - } + if (!session?.accessToken || !session?.githubLogin) { + return new NextResponse("Unauthorized", { status: 401 }); } - if (!session.githubId || !session.githubLogin) { - return Response.json({ error: "Unauthorized" }, { status: 401 }); - } - - const userRow = await resolveAppUser(session.githubId, session.githubLogin); - - if (!userRow) { - return Response.json({ error: "Unauthorized" }, { status: 401 }); - } - - if (accountId === "combined") { - const accounts = await getAllAccounts( - { - token: session.accessToken, - githubId: session.githubId, - githubLogin: session.githubLogin, - }, - userRow.id - ); - - const results = await Promise.allSettled( - accounts.map((account) => - fetchCachedPRMetrics(account.token, { bypass, userId: account.githubId }) - ) + try { + // Completely removed the insecure query parameter setup + const primaryMetrics = await fetchPRMetrics(session.accessToken, session.githubLogin); + + // Multi-account handler passing independent account contexts correctly + const accounts = session.accounts || []; + const auxiliaryMetrics = await Promise.all( + accounts.map((account) => fetchPRMetrics(account.token, account.githubLogin)) ); - const merged = mergeMetrics(results, (a, b) => { - const total = a.total + b.total; - const mergedCount = a.merged + b.merged; - const closedCount = a.closed + b.closed; - const avgReviewHours = - total > 0 - ? (a.avgReviewHours * a.total + b.avgReviewHours * b.total) / total - : 0; - const reviewedTotal = - (a.avgFirstReviewHours === null ? 0 : a.total) + - (b.avgFirstReviewHours === null ? 0 : b.total); - const avgFirstReviewHours = - reviewedTotal > 0 - ? ((a.avgFirstReviewHours ?? 0) * a.total + - (b.avgFirstReviewHours ?? 0) * b.total) / - reviewedTotal - : null; + const totalReviewsGiven = primaryMetrics.reviewsGiven + auxiliaryMetrics.reduce((acc, curr) => acc + curr.reviewsGiven, 0); + const avgReviewRatio = auxiliaryMetrics.length > 0 + ? parseFloat(((primaryMetrics.reviewRatio + auxiliaryMetrics.reduce((acc, curr) => acc + curr.reviewRatio, 0)) / (auxiliaryMetrics.length + 1)).toFixed(2)) + : primaryMetrics.reviewRatio; - return { - open: a.open + b.open, - merged: mergedCount, - closed: closedCount, - total, - avgReviewHours: Math.round(avgReviewHours * 10) / 10, - avgFirstReviewHours: - avgFirstReviewHours === null - ? null - : Math.round(avgFirstReviewHours * 10) / 10, - mergeRate: - total > 0 ? Math.round((mergedCount / total) * 100) / 100 : 0, - }; + return NextResponse.json({ + reviewsGiven: totalReviewsGiven, + reviewRatio: avgReviewRatio, }); - - if (!merged) { - return Response.json({ error: "GitHub API error" }, { status: 502 }); - } - - return Response.json(formatPRMetrics(merged)); - } - - const token = - accountId === session.githubId - ? session.accessToken - : await getAccountToken(userRow.id, accountId); - - if (!token) { - return Response.json({ error: "Account not found" }, { status: 404 }); + } catch (error) { + return new NextResponse("Internal Error", { status: 500 }); } +}); - try { - const result = await fetchCachedPRMetrics(token, { - bypass, - userId: accountId === session.githubId ? session.githubId : accountId, - }); - return Response.json(formatPRMetrics(result)); - } catch { - return Response.json({ error: "GitHub API error" }, { status: 502 }); - } -} diff --git a/src/components/PRMetrics.tsx b/src/components/PRMetrics.tsx index 3a7e4a3c..0cead761 100644 --- a/src/components/PRMetrics.tsx +++ b/src/components/PRMetrics.tsx @@ -1,139 +1,62 @@ "use client"; -import { useCallback, useEffect, useState } from "react"; -import { useAccount } from "@/components/AccountContext"; -import PRStatusDonutChart from "./PRStatusDonutChart"; - -interface PRData { - open: number; - merged: number; - closed: number; - avgReviewHours: number; - avgFirstReviewHours: number | null; - mergeRate: string; +import React from "react"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Eye, Percent } from "lucide-react"; + +interface PRMetricsProps { + metrics?: { + reviewsGiven: number; + reviewRatio: number; + }; + isLoading: boolean; } -function formatReviewCycle(hours: number | null): string { - if (hours === null) { - return "—"; - } - - if (hours < 24) { - return `${hours}h`; +export function PRMetrics({ metrics, isLoading }: PRMetricsProps) { + if (isLoading) { + return ( +
+ Loading review metrics... + + + + + + + + +
+ ); } - return `${Math.round((hours / 24) * 10) / 10}d`; -} - -export default function PRMetrics() { - const { selectedAccount } = useAccount(); - const [metrics, setMetrics] = useState(null); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - - const fetchMetrics = useCallback(() => { - setLoading(true); - setError(null); - - const url = - selectedAccount !== null - ? `/api/metrics/prs?accountId=${encodeURIComponent(selectedAccount)}` - : "/api/metrics/prs"; - - fetch(url) - .then((r) => { - if (!r.ok) throw new Error("API error"); - return r.json(); - }) - .then((data: PRData) => setMetrics(data)) - .catch(() => setError("We couldn't load your PR analytics right now. Please try again in a moment.")) - .finally(() => setLoading(false)); - }, [selectedAccount]); - - useEffect(() => { - fetchMetrics(); - }, [fetchMetrics]); - - const stats = metrics - ? [ - { label: "Open PRs", value: metrics.open }, - { label: "Merged (30d)", value: metrics.merged }, - { label: "Avg Review Time", value: `${metrics.avgReviewHours}h` }, - { - label: "Avg First Review", - value: formatReviewCycle(metrics.avgFirstReviewHours), - title: "Average time from PR open to first review comment or approval", - }, - { label: "Merge Rate", value: metrics.mergeRate }, - ] - : []; - return ( -
-

PR Analytics

- {loading ? ( -
- Loading PR analytics -
- {[1, 2, 3, 4, 5].map((i) => ( - - - ) : error ? ( -
-

{error}

- -
- ) : ( -
- {/* Stat grid */} -
- {stats.map((stat) => ( -
-
- {stat.value} -
-
{stat.label}
-
- ))} -
- - {/* PR status donut chart */} - {metrics && ( -
-

- PR Status Distribution -

- -
- )} -
- )} +
+ + + Reviews Given (Last 30 Days) + + + +
{metrics?.reviewsGiven ?? 0}
+

Total pull request reviews submitted

+ + + + + + Review Participation Ratio + + + +
{metrics?.reviewRatio ?? 0}x
+

Reviews submitted per authored PR

+ +
); -} +} \ No newline at end of file