diff --git a/apps/dashboard/src/components/compare/compare-diff-view.tsx b/apps/dashboard/src/components/compare/compare-diff-view.tsx index 4b42b24..a551fab 100644 --- a/apps/dashboard/src/components/compare/compare-diff-view.tsx +++ b/apps/dashboard/src/components/compare/compare-diff-view.tsx @@ -37,11 +37,13 @@ const noopEdit = (_a: PendingComment, _b: string) => {}; export function CompareDiffView({ commits, files, + filesTruncated = false, owner, repo, }: { commits: PullCommit[]; files: PullFile[]; + filesTruncated?: boolean; owner: string; repo: string; }) { @@ -52,13 +54,10 @@ export function CompareDiffView({ const loadMoreRef = useRef(null); useEffect(() => { - setVisibleCount((prev) => - Math.min( - files.length, - Math.max(files.length === 0 ? 0 : INITIAL_VISIBLE_COUNT, prev), - ), + setVisibleCount( + Math.min(files.length, files.length === 0 ? 0 : INITIAL_VISIBLE_COUNT), ); - }, [files.length]); + }, [files]); useEffect(() => { if (visibleCount >= files.length) return; @@ -202,6 +201,13 @@ export function CompareDiffView({ ) : null} + {filesTruncated ? ( +

+ This comparison includes more than {files.length} files; GitHub only + returns the first {files.length}. Push fewer changes or open the PR + to view the full set. +

+ ) : null} {files.length === 0 ? (

No files changed. diff --git a/apps/dashboard/src/components/compare/compare-header.tsx b/apps/dashboard/src/components/compare/compare-header.tsx index a1add82..7facf29 100644 --- a/apps/dashboard/src/components/compare/compare-header.tsx +++ b/apps/dashboard/src/components/compare/compare-header.tsx @@ -65,16 +65,16 @@ export function CompareHeader({ {aheadBy === 1 ? "commit" : "commits"} ahead ) : null} + {aheadBy > 0 && behindBy > 0 ? ( + · + ) : null} {behindBy > 0 ? ( - <> - · - - - {behindBy} - - {behindBy === 1 ? "commit" : "commits"} behind + + + {behindBy} - + {behindBy === 1 ? "commit" : "commits"} behind + ) : null} {status === "identical" ? Branches are identical : null} diff --git a/apps/dashboard/src/components/compare/compare-page.tsx b/apps/dashboard/src/components/compare/compare-page.tsx index 99e99e3..88d37ba 100644 --- a/apps/dashboard/src/components/compare/compare-page.tsx +++ b/apps/dashboard/src/components/compare/compare-page.tsx @@ -1,4 +1,5 @@ import type { MentionCandidate } from "@diffkit/ui/components/markdown-editor"; +import { toast } from "@diffkit/ui/components/sonner"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { useRouter } from "@tanstack/react-router"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; @@ -179,19 +180,27 @@ export function ComparePage({ }); if (result.ok) { + const INVALIDATE_SEGMENTS = new Set([ + "pulls", + "overview", + "recentPushableBranch", + "branchComparison", + "compareDetail", + ]); await queryClient.invalidateQueries({ predicate: (query) => { const key = query.queryKey; - return ( - Array.isArray(key) && - key.some( - (k) => - typeof k === "string" && - (k.includes("pulls") || k.includes("repoMeta")), - ) + if (!Array.isArray(key)) return false; + return key.some( + (k) => typeof k === "string" && INVALIDATE_SEGMENTS.has(k), ); }, }); + if (result.warnings && result.warnings.length > 0) { + toast.warning("Pull request created with some issues", { + description: result.warnings.join("\n"), + }); + } router.navigate({ to: "/$owner/$repo/pull/$pullId", params: { owner, repo, pullId: String(result.pullNumber) }, @@ -223,7 +232,27 @@ export function ComparePage({ if (overviewQuery.error) throw overviewQuery.error; if (compareQuery.error) throw compareQuery.error; - if (!repoData || !compare) return ; + if (overviewQuery.isPending || compareQuery.isPending) { + return ; + } + if (!repoData) return ; + if (!compare) { + return ( +

+

Nothing to compare

+

+ + {base} + {" "} + and{" "} + + {head} + {" "} + are identical, or one of them doesn't exist. +

+
+ ); + } return (
@@ -280,6 +309,7 @@ export function ComparePage({ diff --git a/apps/dashboard/src/components/compare/compare-sidebar.tsx b/apps/dashboard/src/components/compare/compare-sidebar.tsx index f8703cb..4066fc1 100644 --- a/apps/dashboard/src/components/compare/compare-sidebar.tsx +++ b/apps/dashboard/src/components/compare/compare-sidebar.tsx @@ -6,7 +6,7 @@ import { } from "@diffkit/ui/components/popover"; import { cn } from "@diffkit/ui/lib/utils"; import { useQuery, useQueryClient } from "@tanstack/react-query"; -import { useCallback, useMemo, useRef, useState } from "react"; +import { forwardRef, useCallback, useMemo, useRef, useState } from "react"; import { DetailSidebar } from "#/components/details/detail-sidebar"; import { type GitHubQueryScope, @@ -94,18 +94,39 @@ function SidebarSectionHeader({ ); } -function PickerTrigger({ onPrefetch }: { onPrefetch?: () => void }) { +const PickerTrigger = forwardRef< + HTMLButtonElement, + React.ButtonHTMLAttributes & { + label: string; + onPrefetch?: () => void; + } +>(function PickerTrigger( + { label, onPrefetch, onMouseEnter, onFocus, className, ...props }, + ref, +) { return ( ); -} +}); function PickerSearchInput({ value, @@ -222,9 +243,7 @@ function LabelsPicker({ }} > - - - + - - - + - - - + diff --git a/apps/dashboard/src/lib/github.functions.ts b/apps/dashboard/src/lib/github.functions.ts index b0e0b23..8153166 100644 --- a/apps/dashboard/src/lib/github.functions.ts +++ b/apps/dashboard/src/lib/github.functions.ts @@ -7195,7 +7195,7 @@ export type CreatePullRequestInput = { }; export type CreatePullRequestResult = - | { ok: true; pullNumber: number } + | { ok: true; pullNumber: number; warnings?: string[] } | { ok: false; error: string; installUrl?: string }; export const createPullRequest = createServerFn({ method: "POST" }) @@ -7219,43 +7219,60 @@ export const createPullRequest = createServerFn({ method: "POST" }) const pullNumber = response.data.number; - const followUps: Array> = []; + const followUps: Array<{ name: string; promise: Promise }> = []; if (data.labels && data.labels.length > 0) { - followUps.push( - context.octokit.rest.issues.addLabels({ + followUps.push({ + name: "addLabels", + promise: context.octokit.rest.issues.addLabels({ owner: data.owner, repo: data.repo, issue_number: pullNumber, labels: data.labels, }), - ); + }); } if (data.assignees && data.assignees.length > 0) { - followUps.push( - context.octokit.rest.issues.addAssignees({ + followUps.push({ + name: "addAssignees", + promise: context.octokit.rest.issues.addAssignees({ owner: data.owner, repo: data.repo, issue_number: pullNumber, assignees: data.assignees, }), - ); + }); } if ( (data.reviewers && data.reviewers.length > 0) || (data.teamReviewers && data.teamReviewers.length > 0) ) { - followUps.push( - context.octokit.rest.pulls.requestReviewers({ + followUps.push({ + name: "requestReviewers", + promise: context.octokit.rest.pulls.requestReviewers({ owner: data.owner, repo: data.repo, pull_number: pullNumber, reviewers: data.reviewers ?? [], team_reviewers: data.teamReviewers ?? [], }), - ); + }); } + const warnings: string[] = []; if (followUps.length > 0) { - await Promise.allSettled(followUps); + const results = await Promise.allSettled( + followUps.map((f) => f.promise), + ); + for (let i = 0; i < results.length; i++) { + const r = results[i]; + if (r.status === "rejected") { + const reason = r.reason; + const msg = + reason instanceof Error ? reason.message : String(reason); + const name = followUps[i].name; + console.error(`[create pull request] ${name} failed`, reason); + warnings.push(`${name}: ${msg}`); + } + } } await bumpGitHubCacheNamespaces([ @@ -7266,10 +7283,21 @@ export const createPullRequest = createServerFn({ method: "POST" }) }), ]); - return { ok: true, pullNumber }; + return { + ok: true, + pullNumber, + ...(warnings.length > 0 ? { warnings } : {}), + }; } catch (error) { const result = toMutationError("create pull request", error); - return { ok: false, error: result.ok ? "" : result.error }; + if (result.ok) { + return { ok: false, error: "" }; + } + return { + ok: false, + error: result.error, + ...(result.installUrl ? { installUrl: result.installUrl } : {}), + }; } }); @@ -8614,6 +8642,9 @@ export const getRecentPushableBranch = createServerFn({ method: "GET" }) const context = await getGitHubContextForRepository(data); if (!context) return null; + const userContext = await getGitHubUserContextForRepository(data); + if (!userContext) return null; + const repoCodeKey = githubRevalidationSignalKeys.repoCode(data); const repoMetaKey = githubRevalidationSignalKeys.repoMeta(data); @@ -8627,7 +8658,7 @@ export const getRecentPushableBranch = createServerFn({ method: "GET" }) namespaceKeys: [repoCodeKey, repoMetaKey], cacheMode: "split", fetcher: async () => { - const viewer = await getViewer(context); + const viewer = await getViewer(userContext); if (!viewer.login) { return { kind: "success", @@ -8649,14 +8680,15 @@ export const getRecentPushableBranch = createServerFn({ method: "GET" }) }; } - const activities = await context.octokit.rest.repos.listActivities({ - owner: data.owner, - repo: data.repo, - actor: viewer.login, - activity_type: "push", - time_period: "day", - per_page: 10, - }); + const activities = + await userContext.octokit.rest.repos.listActivities({ + owner: data.owner, + repo: data.repo, + actor: viewer.login, + activity_type: "push", + time_period: "day", + per_page: 10, + }); const seen = new Set(); for (const activity of activities.data) { @@ -8719,8 +8751,14 @@ export type CompareDetail = { totalCommits: number; commits: PullCommit[]; files: PullFile[]; + /** True when the file list hit GitHub's 300-file cap for this endpoint. */ + filesTruncated: boolean; }; +const COMPARE_COMMITS_MAX_PAGES = 3; +const COMPARE_COMMITS_PER_PAGE = 100; +const COMPARE_FILES_CAP = 300; + export const getCompareDetail = createServerFn({ method: "GET" }) .inputValidator(identityValidator) .handler(async ({ data }): Promise => { @@ -8728,50 +8766,87 @@ export const getCompareDetail = createServerFn({ method: "GET" }) const context = await getGitHubContextForRepository(data); if (!context) return null; - return getCachedGitHubRequest< - Awaited< - ReturnType - >["data"], - CompareDetail - >({ - context, - resource: "repo.compareDetail.v1", - params: data, - freshForMs: githubCachePolicy.detail.staleTimeMs, - signalKeys: [githubRevalidationSignalKeys.repoCode(data)], - namespaceKeys: [githubRevalidationSignalKeys.repoCode(data)], - cacheMode: "split", - request: (headers) => - context.octokit.rest.repos.compareCommitsWithBasehead({ - owner: data.owner, - repo: data.repo, - basehead: `${data.base}...${data.head}`, - per_page: 100, - headers, - }), - mapData: (comparison) => ({ - aheadBy: comparison.ahead_by, - behindBy: comparison.behind_by, - status: comparison.status as CompareDetail["status"], - totalCommits: comparison.total_commits, - commits: comparison.commits.map((c) => ({ - sha: c.sha, - message: c.commit.message, - createdAt: c.commit.committer?.date ?? c.commit.author?.date ?? "", - author: mapActor(c.author), - })), - files: (comparison.files ?? []).map((f) => ({ - sha: f.sha ?? null, - filename: f.filename, - status: f.status as PullFile["status"], - additions: f.additions, - deletions: f.deletions, - changes: f.changes, - patch: f.patch ?? null, - previousFilename: f.previous_filename ?? null, - })), - }), - }).catch(() => null); + try { + return await getOrRevalidateGitHubResource({ + userId: context.session.user.id, + resource: "repo.compareDetail.v2", + params: data, + freshForMs: githubCachePolicy.detail.staleTimeMs, + signalKeys: [githubRevalidationSignalKeys.repoCode(data)], + namespaceKeys: [githubRevalidationSignalKeys.repoCode(data)], + cacheMode: "split", + fetcher: async () => { + const first = + await context.octokit.rest.repos.compareCommitsWithBasehead({ + owner: data.owner, + repo: data.repo, + basehead: `${data.base}...${data.head}`, + per_page: COMPARE_COMMITS_PER_PAGE, + page: 1, + }); + + const commitsRaw: (typeof first.data.commits)[number][] = [ + ...first.data.commits, + ]; + const totalCommits = first.data.total_commits; + + // Paginate additional commit pages — compareCommitsWithBasehead + // returns files only on page 1, so we only collect commits here. + for (let page = 2; page <= COMPARE_COMMITS_MAX_PAGES; page++) { + if (commitsRaw.length >= totalCommits) break; + const next = + await context.octokit.rest.repos.compareCommitsWithBasehead({ + owner: data.owner, + repo: data.repo, + basehead: `${data.base}...${data.head}`, + per_page: COMPARE_COMMITS_PER_PAGE, + page, + }); + if (next.data.commits.length === 0) break; + commitsRaw.push(...next.data.commits); + } + + const files = first.data.files ?? []; + const filesTruncated = files.length >= COMPARE_FILES_CAP; + if (filesTruncated) { + console.warn( + `[compare detail] file list truncated at ${COMPARE_FILES_CAP} for ${data.owner}/${data.repo} ${data.base}...${data.head}`, + ); + } + + return { + kind: "success", + data: { + aheadBy: first.data.ahead_by, + behindBy: first.data.behind_by, + status: first.data.status as CompareDetail["status"], + totalCommits, + commits: commitsRaw.map((c) => ({ + sha: c.sha, + message: c.commit.message, + createdAt: + c.commit.committer?.date ?? c.commit.author?.date ?? "", + author: mapActor(c.author), + })), + files: files.map((f) => ({ + sha: f.sha ?? null, + filename: f.filename, + status: f.status as PullFile["status"], + additions: f.additions, + deletions: f.deletions, + changes: f.changes, + patch: f.patch ?? null, + previousFilename: f.previous_filename ?? null, + })), + filesTruncated, + }, + metadata: createGitHubResponseMetadata(200, {}), + }; + }, + }); + } catch { + return null; + } }); // --------------------------------------------------------------------------- diff --git a/apps/dashboard/src/lib/github.types.ts b/apps/dashboard/src/lib/github.types.ts index 590fbf6..d1c528c 100644 --- a/apps/dashboard/src/lib/github.types.ts +++ b/apps/dashboard/src/lib/github.types.ts @@ -574,6 +574,11 @@ export type BranchComparison = { totalCommits: number; }; +export type CompareFilesMeta = { + /** True when the GitHub API capped the file list (max 300). */ + truncated: boolean; +}; + export type RepoContributor = { login: string; avatarUrl: string;