Skip to content

feat: add stale PR indicator #589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Saniya30pawar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add stale PR indicator #589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 97 additions & 13 deletions src/app/api/metrics/prs/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { getServerSession } from "next-auth";
import { NextRequest } from "next/server";
import { authOptions } from "@/lib/auth";
import {
getAccountToken,
getAllAccounts,
mergeMetrics,
} from "@/lib/github-accounts";
Expand All @@ -18,13 +17,19 @@ import { resolveAppUser } from "@/lib/resolve-user";

export const dynamic = "force-dynamic";

const STALE_THRESHOLD_OPTIONS = [7, 14, 30] as const;
const DEFAULT_STALE_THRESHOLD_DAYS = 7;

interface PRMetricsBase {
open: number;
merged: number;
total: number;
avgReviewHours: number;
avgFirstReviewHours: number | null;
mergeRate: number;
staleCount: number;
staleThresholdDays: number;
staleSearchUrl: string | null;
}

interface PullRequestSearchItem {
Expand Down Expand Up @@ -59,6 +64,35 @@ function getEarliestTimestamp(values: Array<string | null | undefined>) {
return timestamps.length > 0 ? Math.min(...timestamps) : null;
}

function getStaleThresholdDays(req: NextRequest): number {
const requestedThreshold = Number(
req.nextUrl.searchParams.get("staleThresholdDays") ??
DEFAULT_STALE_THRESHOLD_DAYS
);

return STALE_THRESHOLD_OPTIONS.includes(
requestedThreshold as (typeof STALE_THRESHOLD_OPTIONS)[number]
)
? requestedThreshold
: DEFAULT_STALE_THRESHOLD_DAYS;
}

function getStaleSearchUrl(
githubLogin: string | null | undefined,
staleCutoffMs: number
): string | null {
if (!githubLogin) {
return null;
}

const cutoffDate = new Date(staleCutoffMs).toISOString().slice(0, 10);
const params = new URLSearchParams({
q: `is:pr is:open author:${githubLogin} created:<${cutoffDate}`,
});

return `https://github.com/pulls?${params.toString()}`;
}

async function fetchFirstReviewTimestamp(
token: string,
pr: PullRequestSearchItem
Expand Down Expand Up @@ -132,7 +166,10 @@ async function getAverageFirstReviewHours(
return Math.round(average * 10) / 10;
}

async function fetchPRMetrics(token: string): Promise<PRMetricsBase> {
async function fetchPRMetrics(
token: string,
options: { staleThresholdDays: number; githubLogin?: string | null }
): Promise<PRMetricsBase> {
const searchRes = await fetch(
`${GITHUB_API}/search/issues?q=type:pr+author:@me&sort=updated&order=desc&per_page=100`,
{
Expand All @@ -151,6 +188,16 @@ async function fetchPRMetrics(token: string): Promise<PRMetricsBase> {
};

const open = data.items.filter((pr) => pr.state === "open").length;
const staleCutoffMs =
Date.now() - options.staleThresholdDays * 24 * 60 * 60 * 1000;
const staleCount = data.items.filter((pr) => {
if (pr.state !== "open") {
return false;
}

const createdAt = new Date(pr.created_at).getTime();
return !Number.isNaN(createdAt) && createdAt < staleCutoffMs;
}).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
Expand Down Expand Up @@ -193,22 +240,36 @@ async function fetchPRMetrics(token: string): Promise<PRMetricsBase> {
avgReviewHours: Math.round(avgReviewMs / 3600000),
avgFirstReviewHours,
mergeRate: sampleTotal > 0 ? merged / sampleTotal : 0,
staleCount,
staleThresholdDays: options.staleThresholdDays,
staleSearchUrl: getStaleSearchUrl(options.githubLogin, staleCutoffMs),
};
}

async function fetchCachedPRMetrics(
token: string,
cacheContext: { bypass: boolean; userId: string }
cacheContext: {
bypass: boolean;
githubLogin?: string | null;
staleThresholdDays: number;
userId: string;
}
): Promise<PRMetricsBase> {
const key = metricsCacheKey(cacheContext.userId, "prs");
const key = metricsCacheKey(cacheContext.userId, "prs", {
staleThresholdDays: cacheContext.staleThresholdDays,
});

return withMetricsCache(
{
bypass: cacheContext.bypass,
key,
ttlSeconds: METRICS_CACHE_TTL_SECONDS.prs,
},
() => fetchPRMetrics(token)
() =>
fetchPRMetrics(token, {
githubLogin: cacheContext.githubLogin,
staleThresholdDays: cacheContext.staleThresholdDays,
})
);
}

Expand All @@ -219,6 +280,9 @@ function formatPRMetrics(metrics: PRMetricsBase) {
total: metrics.total,
avgReviewHours: metrics.avgReviewHours,
avgFirstReviewHours: metrics.avgFirstReviewHours,
staleCount: metrics.staleCount,
staleThresholdDays: metrics.staleThresholdDays,
staleSearchUrl: metrics.staleSearchUrl,
mergeRate:
metrics.total > 0
? `${Math.round(metrics.mergeRate * 100)}%`
Expand All @@ -234,11 +298,14 @@ export async function GET(req: NextRequest) {

const accountId = req.nextUrl.searchParams.get("accountId");
const bypass = isMetricsCacheBypassed(req);
const staleThresholdDays = getStaleThresholdDays(req);

if (!accountId) {
try {
const result = await fetchCachedPRMetrics(session.accessToken, {
bypass,
githubLogin: session.githubLogin,
staleThresholdDays,
userId: session.githubId ?? session.githubLogin ?? "primary",
});
return Response.json(formatPRMetrics(result));
Expand Down Expand Up @@ -269,7 +336,12 @@ export async function GET(req: NextRequest) {

const results = await Promise.allSettled(
accounts.map((account) =>
fetchCachedPRMetrics(account.token, { bypass, userId: account.githubId })
fetchCachedPRMetrics(account.token, {
bypass,
githubLogin: account.githubLogin,
staleThresholdDays,
userId: account.githubId,
})
)
);

Expand Down Expand Up @@ -299,6 +371,9 @@ export async function GET(req: NextRequest) {
avgFirstReviewHours === null
? null
: Math.round(avgFirstReviewHours * 10) / 10,
staleCount: a.staleCount + b.staleCount,
staleThresholdDays,
staleSearchUrl: null,
mergeRate:
total > 0 ? Math.round((mergedCount / total) * 100) / 100 : 0,
};
Expand All @@ -311,19 +386,28 @@ export async function GET(req: NextRequest) {
return Response.json(formatPRMetrics(merged));
}

const token =
accountId === session.githubId
? session.accessToken
: await getAccountToken(userRow.id, accountId);
const accounts = await getAllAccounts(
{
token: session.accessToken,
githubId: session.githubId,
githubLogin: session.githubLogin,
},
userRow.id
);
const selectedAccount = accounts.find(
(account) => account.githubId === accountId
);

if (!token) {
if (!selectedAccount) {
return Response.json({ error: "Account not found" }, { status: 404 });
}

try {
const result = await fetchCachedPRMetrics(token, {
const result = await fetchCachedPRMetrics(selectedAccount.token, {
bypass,
userId: accountId === session.githubId ? session.githubId : accountId,
githubLogin: selectedAccount.githubLogin,
staleThresholdDays,
userId: selectedAccount.githubId,
});
return Response.json(formatPRMetrics(result));
} catch {
Expand Down
110 changes: 89 additions & 21 deletions src/components/PRMetrics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ interface PRData {
avgReviewHours: number;
avgFirstReviewHours: number | null;
mergeRate: string;
staleCount: number;
staleThresholdDays: number;
staleSearchUrl: string | null;
}

interface PRStat {
label: string;
value: string | number;
href?: string | null;
title?: string;
warning?: boolean;
}

function formatReviewCycle(hours: number | null): string {
Expand All @@ -26,17 +37,23 @@ function formatReviewCycle(hours: number | null): string {
export default function PRMetrics() {
const { selectedAccount } = useAccount();
const [metrics, setMetrics] = useState<PRData | null>(null);
const [staleThresholdDays, setStaleThresholdDays] = useState(7);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);

const fetchMetrics = useCallback(() => {
setLoading(true);
setError(null);

const url =
selectedAccount !== null
? `/api/metrics/prs?accountId=${encodeURIComponent(selectedAccount)}`
: "/api/metrics/prs";
const params = new URLSearchParams({
staleThresholdDays: String(staleThresholdDays),
});

if (selectedAccount !== null) {
params.set("accountId", selectedAccount);
}

const url = `/api/metrics/prs?${params.toString()}`;

fetch(url)
.then((r) => {
Expand All @@ -46,15 +63,22 @@ export default function PRMetrics() {
.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]);
}, [selectedAccount, staleThresholdDays]);

useEffect(() => {
fetchMetrics();
}, [fetchMetrics]);

const stats = metrics
const stats: PRStat[] = metrics
? [
{ label: "Open PRs", value: metrics.open },
{
label: `Stale > ${metrics.staleThresholdDays}d`,
value: metrics.staleCount,
href: metrics.staleSearchUrl,
title: `${metrics.staleCount} open PRs are older than ${metrics.staleThresholdDays} days`,
warning: metrics.staleCount > 0,
},
{ label: "Merged (30d)", value: metrics.merged },
{ label: "Avg Review Time", value: `${metrics.avgReviewHours}h` },
{
Expand All @@ -68,16 +92,32 @@ export default function PRMetrics() {

return (
<div className="rounded-xl border border-[var(--border)] bg-[var(--card)] p-6 shadow-sm">
<h2 className="mb-4 text-lg font-semibold text-[var(--card-foreground)]">PR Analytics</h2>
<div className="mb-4 flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between">
<h2 className="text-lg font-semibold text-[var(--card-foreground)]">PR Analytics</h2>
<label className="flex items-center gap-2 text-xs font-medium text-[var(--muted-foreground)]">
Stale after
<select
value={staleThresholdDays}
onChange={(event) => setStaleThresholdDays(Number(event.target.value))}
className="rounded-md border border-[var(--border)] bg-[var(--control)] px-2 py-1 text-sm text-[var(--foreground)] outline-none transition-colors focus:border-[var(--accent)]"
>
{[7, 14, 30].map((days) => (
<option key={days} value={days}>
{days} days
</option>
))}
</select>
</label>
</div>
{loading ? (
<div
role="status"
aria-live="polite"
aria-busy="true"
className="grid grid-cols-2 md:grid-cols-4 gap-4"
className="grid grid-cols-2 md:grid-cols-3 xl:grid-cols-6 gap-4"
>
<span className="sr-only">Loading PR analytics</span>
{[1, 2, 3, 4].map((i) => (
{[1, 2, 3, 4, 5, 6].map((i) => (
<div
key={i}
aria-hidden="true"
Expand All @@ -97,19 +137,47 @@ export default function PRMetrics() {
</button>
</div>
) : (
<div className="grid grid-cols-2 md:grid-cols-5 gap-4">
{stats.map((stat) => (
<div
key={stat.label}
className="rounded-lg bg-[var(--control)] p-4 text-center min-w-0"
title={stat.title}
>
<div className="truncate text-2xl font-bold text-[var(--accent)]">
{stat.value}
<div className="grid grid-cols-2 md:grid-cols-3 xl:grid-cols-6 gap-4">
{stats.map((stat) => {
const content = (
<>
<div
className={`truncate text-2xl font-bold ${
stat.warning ? "text-orange-300" : "text-[var(--accent)]"
}`}
>
{stat.value}
</div>
<div className="truncate mt-1 text-sm text-[var(--muted-foreground)]">{stat.label}</div>
</>
);
const className = `rounded-lg p-4 text-center min-w-0 transition-colors ${
stat.warning
? "border border-orange-400/30 bg-orange-500/10 hover:bg-orange-500/15"
: "bg-[var(--control)]"
}`;

return stat.href ? (
<a
key={stat.label}
href={stat.href}
target="_blank"
rel="noreferrer"
className={className}
title={stat.title}
>
{content}
</a>
) : (
<div
key={stat.label}
className={className}
title={stat.title}
>
{content}
</div>
<div className="truncate mt-1 text-sm text-[var(--muted-foreground)]">{stat.label}</div>
</div>
))}
);
})}
</div>
)}
</div>
Expand Down
Loading