| author | |
| committer | |
| log | 7068e86f76e908c77ff04d1a684840257eab3a8a |
| tree | 81e043adca77b92bf054e545cfaf2b4e12dec9b3 |
| parent | f276e632c5a7952655e2455aef3b86c08164b988 |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 218 insertions(+), 13 deletions(-)
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+12-2| ... | @@ -51,7 +51,7 @@ import { | ... | @@ -51,7 +51,7 @@ import { |
| 51 | import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; | 51 | import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; |
| 52 | import { useNewThreadHandler } from "~/hooks/useHandleNewThread"; | 52 | import { useNewThreadHandler } from "~/hooks/useHandleNewThread"; |
| 53 | import { useCopyToClipboard, writeTextToClipboard } from "~/hooks/useCopyToClipboard"; | 53 | import { useCopyToClipboard, writeTextToClipboard } from "~/hooks/useCopyToClipboard"; |
| 54 | import { changeRequestRepositoryUrl } from "~/lib/openPullRequestLink"; | 54 | import { changeRequestRepositoryUrl, gitHubPullRequestBrowserUrl } from "~/lib/openPullRequestLink"; |
| 55 | import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions"; | 55 | import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions"; |
| 56 | import { cn } from "~/lib/utils"; | 56 | import { cn } from "~/lib/utils"; |
| 57 | import { readLocalApi } from "~/localApi"; | 57 | import { readLocalApi } from "~/localApi"; |
| ... | @@ -590,6 +590,12 @@ export function PullRequestDetailPanel({ | ... | @@ -590,6 +590,12 @@ export function PullRequestDetailPanel({ |
| 590 | const newThread = useNewThreadHandler(); | 590 | const newThread = useNewThreadHandler(); |
| 591 | const { environments } = useEnvironments(); | 591 | const { environments } = useEnvironments(); |
| 592 | const projects = useProjects(); | 592 | const projects = useProjects(); |
| 593 | const unavailableGitHubUrl = useMemo(() => { | ||
| 594 | const identity = projects.find( | ||
| 595 | (project) => project.id === reference.projectId && project.environmentId === environmentId, | ||
| 596 | )?.repositoryIdentity; | ||
| 597 | return gitHubPullRequestBrowserUrl(identity, reference.repository, reference.number); | ||
| 598 | }, [environmentId, projects, reference.number, reference.projectId, reference.repository]); | ||
| 593 | // Beside a thread there is nothing to pick: the hand-offs land in that thread's composer, and | 599 | // Beside a thread there is nothing to pick: the hand-offs land in that thread's composer, and |
| 594 | // the thread is already on one server's copy of the branch. | 600 | // the thread is already on one server's copy of the branch. |
| 595 | const pickableEnvironments = useMemo( | 601 | const pickableEnvironments = useMemo( |
| ... | @@ -1906,7 +1912,11 @@ export function PullRequestDetailPanel({ | ... | @@ -1906,7 +1912,11 @@ export function PullRequestDetailPanel({ |
| 1906 | }} | 1912 | }} |
| 1907 | > | 1913 | > |
| 1908 | {detailQuery.error && !detail ? ( | 1914 | {detailQuery.error && !detail ? ( |
| 1909 | <PullRequestsUnavailableState error={detailQuery.error} onRetry={refreshDetail} /> | 1915 | <PullRequestsUnavailableState |
| 1916 | error={detailQuery.error} | ||
| 1917 | onRetry={refreshDetail} | ||
| 1918 | {...(unavailableGitHubUrl ? { gitHubUrl: unavailableGitHubUrl } : {})} | ||
| 1919 | /> | ||
| 1910 | ) : detail ? ( | 1920 | ) : detail ? ( |
| 1911 | <> | 1921 | <> |
| 1912 | {mountedTabs.has("summary") ? ( | 1922 | {mountedTabs.has("summary") ? ( |
apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx+44-3| ... | @@ -1,4 +1,5 @@ | ... | @@ -1,4 +1,5 @@ |
| 1 | import { isValidElement, type ReactElement, type ReactNode } from "react"; | 1 | import { isValidElement, type ReactElement, type ReactNode } from "react"; |
| 2 | import { renderToStaticMarkup } from "react-dom/server"; | ||
| 2 | import { describe, expect, it } from "vite-plus/test"; | 3 | import { describe, expect, it } from "vite-plus/test"; |
| 3 | 4 | ||
| 4 | import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; | 5 | import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; |
| ... | @@ -25,8 +26,48 @@ describe("PullRequestsUnavailableState", () => { | ... | @@ -25,8 +26,48 @@ describe("PullRequestsUnavailableState", () => { |
| 25 | }); | 26 | }); |
| 26 | 27 | ||
| 27 | it("retains the retry for transient load failures", () => { | 28 | it("retains the retry for transient load failures", () => { |
| 28 | expect( | 29 | const html = renderToStaticMarkup( |
| 29 | textOf(PullRequestsUnavailableState({ error: "GitHub did not answer.", onRetry: () => {} })), | 30 | <PullRequestsUnavailableState |
| 30 | ).toContain("Retry"); | 31 | error="GitHub did not answer." |
| 32 | onRetry={() => {}} | ||
| 33 | gitHubUrl="https://github.com/pingdotgg/t3code/pull/42" | ||
| 34 | />, | ||
| 35 | ); | ||
| 36 | |||
| 37 | expect(html).toContain("Retry"); | ||
| 38 | expect(html).toContain("Open on GitHub"); | ||
| 39 | expect(html).toContain('href="https://github.com/pingdotgg/t3code/pull/42"'); | ||
| 40 | expect(html).toContain('target="_blank"'); | ||
| 41 | expect(html).toContain('rel="noopener noreferrer"'); | ||
| 42 | }); | ||
| 43 | |||
| 44 | it("can offer the browser without offering a retry", () => { | ||
| 45 | const html = renderToStaticMarkup( | ||
| 46 | <PullRequestsUnavailableState | ||
| 47 | error="This server cannot read the pull request." | ||
| 48 | gitHubUrl="https://github.com/pingdotgg/t3code/pull/9" | ||
| 49 | />, | ||
| 50 | ); | ||
| 51 | |||
| 52 | expect(html).toContain("Open on GitHub"); | ||
| 53 | expect(html).not.toContain("Retry"); | ||
| 54 | }); | ||
| 55 | |||
| 56 | it("can offer a retry without offering GitHub", () => { | ||
| 57 | const html = renderToStaticMarkup( | ||
| 58 | <PullRequestsUnavailableState error="The host did not answer." onRetry={() => {}} />, | ||
| 59 | ); | ||
| 60 | |||
| 61 | expect(html).toContain("Retry"); | ||
| 62 | expect(html).not.toContain("Open on GitHub"); | ||
| 63 | }); | ||
| 64 | |||
| 65 | it("renders no action content without a retry or browser target", () => { | ||
| 66 | const html = renderToStaticMarkup( | ||
| 67 | <PullRequestsUnavailableState error="This project has no known remote." />, | ||
| 68 | ); | ||
| 69 | |||
| 70 | expect(html).not.toContain('data-slot="empty-content"'); | ||
| 71 | expect(html).not.toContain("href="); | ||
| 31 | }); | 72 | }); |
| 32 | }); | 73 | }); |
apps/web/src/components/pullRequest/PullRequestsUnavailableState.tsx+21-7| ... | @@ -1,4 +1,4 @@ | ... | @@ -1,4 +1,4 @@ |
| 1 | import { GitPullRequestIcon, RefreshCwIcon } from "lucide-react"; | 1 | import { ExternalLinkIcon, GitPullRequestIcon, RefreshCwIcon } from "lucide-react"; |
| 2 | 2 | ||
| 3 | import { Button } from "../ui/button"; | 3 | import { Button } from "../ui/button"; |
| 4 | import { | 4 | import { |
| ... | @@ -14,10 +14,12 @@ export function PullRequestsUnavailableState({ | ... | @@ -14,10 +14,12 @@ export function PullRequestsUnavailableState({ |
| 14 | title = "Could not load pull requests", | 14 | title = "Could not load pull requests", |
| 15 | error, | 15 | error, |
| 16 | onRetry, | 16 | onRetry, |
| 17 | gitHubUrl, | ||
| 17 | }: { | 18 | }: { |
| 18 | title?: string; | 19 | title?: string; |
| 19 | error: string; | 20 | error: string; |
| 20 | onRetry?: () => void; | 21 | onRetry?: () => void; |
| 22 | gitHubUrl?: string; | ||
| 21 | }) { | 23 | }) { |
| 22 | return ( | 24 | return ( |
| 23 | <Empty className="px-4 py-16 md:px-4"> | 25 | <Empty className="px-4 py-16 md:px-4"> |
| ... | @@ -30,12 +32,24 @@ export function PullRequestsUnavailableState({ | ... | @@ -30,12 +32,24 @@ export function PullRequestsUnavailableState({ |
| 30 | shows its message rather than trying to infer one from the failure text. */} | 32 | shows its message rather than trying to infer one from the failure text. */} |
| 31 | <EmptyDescription>{error}</EmptyDescription> | 33 | <EmptyDescription>{error}</EmptyDescription> |
| 32 | </EmptyHeader> | 34 | </EmptyHeader> |
| 33 | {onRetry ? ( | 35 | {onRetry || gitHubUrl ? ( |
| 34 | <EmptyContent> | 36 | <EmptyContent className="flex-row flex-wrap justify-center gap-2"> |
| 35 | <Button size="sm" variant="outline" onClick={onRetry}> | 37 | {onRetry ? ( |
| 36 | <RefreshCwIcon className="size-3.5" /> | 38 | <Button size="sm" variant="outline" onClick={onRetry}> |
| 37 | Retry | 39 | <RefreshCwIcon className="size-3.5" /> |
| 38 | </Button> | 40 | Retry |
| 41 | </Button> | ||
| 42 | ) : null} | ||
| 43 | {gitHubUrl ? ( | ||
| 44 | <Button | ||
| 45 | size="sm" | ||
| 46 | variant="outline" | ||
| 47 | render={<a href={gitHubUrl} target="_blank" rel="noopener noreferrer" />} | ||
| 48 | > | ||
| 49 | <ExternalLinkIcon aria-hidden className="size-3.5" /> | ||
| 50 | Open on GitHub | ||
| 51 | </Button> | ||
| 52 | ) : null} | ||
| 39 | </EmptyContent> | 53 | </EmptyContent> |
| 40 | ) : null} | 54 | ) : null} |
| 41 | </Empty> | 55 | </Empty> |
apps/web/src/lib/openPullRequestLink.test.ts+102-1| ... | @@ -3,13 +3,114 @@ import { describe, expect, it, vi } from "vite-plus/test"; | ... | @@ -3,13 +3,114 @@ import { describe, expect, it, vi } from "vite-plus/test"; |
| 3 | import { | 3 | import { |
| 4 | changeRequestRepositoryUrl, | 4 | changeRequestRepositoryUrl, |
| 5 | findProjectForChangeRequest, | 5 | findProjectForChangeRequest, |
| 6 | gitHubPullRequestBrowserUrl, | ||
| 6 | matchesLinkedPullRequestUrl, | 7 | matchesLinkedPullRequestUrl, |
| 7 | openPullRequestLink, | 8 | openPullRequestLink, |
| 8 | parseChangeRequestUrl, | 9 | parseChangeRequestUrl, |
| 9 | PullRequestLinkOpenError, | 10 | PullRequestLinkOpenError, |
| 10 | shouldOpenPullRequestExternally, | 11 | shouldOpenPullRequestExternally, |
| 11 | } from "./openPullRequestLink"; | 12 | } from "./openPullRequestLink"; |
| 12 | import { ProjectId } from "@t3tools/contracts"; | 13 | import { ProjectId, type RepositoryIdentity } from "@t3tools/contracts"; |
| 14 | |||
| 15 | function repositoryIdentity( | ||
| 16 | provider: string, | ||
| 17 | canonicalKey: string, | ||
| 18 | remoteUrl: string, | ||
| 19 | ): RepositoryIdentity { | ||
| 20 | return { | ||
| 21 | canonicalKey, | ||
| 22 | provider, | ||
| 23 | locator: { source: "git-remote", remoteName: "origin", remoteUrl }, | ||
| 24 | }; | ||
| 25 | } | ||
| 26 | |||
| 27 | describe("gitHubPullRequestBrowserUrl", () => { | ||
| 28 | it("uses the requested GitHub repository instead of the project's default repository", () => { | ||
| 29 | const identity = repositoryIdentity( | ||
| 30 | "github", | ||
| 31 | "github.com/acme/default", | ||
| 32 | "https://github.com/acme/default.git", | ||
| 33 | ); | ||
| 34 | |||
| 35 | expect(gitHubPullRequestBrowserUrl(identity, "acme/other", 42)).toBe( | ||
| 36 | "https://github.com/acme/other/pull/42", | ||
| 37 | ); | ||
| 38 | }); | ||
| 39 | |||
| 40 | it("preserves a custom GitHub HTTP origin without its credentials", () => { | ||
| 41 | const identity = repositoryIdentity( | ||
| 42 | "github", | ||
| 43 | "github.acme.test/team/default", | ||
| 44 | "http://token@github.acme.test:8443/team/default.git", | ||
| 45 | ); | ||
| 46 | |||
| 47 | expect(gitHubPullRequestBrowserUrl(identity, "platform/api", 7)).toBe( | ||
| 48 | "http://github.acme.test:8443/platform/api/pull/7", | ||
| 49 | ); | ||
| 50 | }); | ||
| 51 | |||
| 52 | it.each([ | ||
| 53 | { | ||
| 54 | name: "SSH", | ||
| 55 | remoteUrl: "git@github.acme.test:team/default.git", | ||
| 56 | }, | ||
| 57 | { | ||
| 58 | name: "git protocol", | ||
| 59 | remoteUrl: "git://github.acme.test/team/default.git", | ||
| 60 | }, | ||
| 61 | ])("uses the normalized host for a $name remote", ({ remoteUrl }) => { | ||
| 62 | const identity = repositoryIdentity("github", "github.acme.test/team/default", remoteUrl); | ||
| 63 | |||
| 64 | expect(gitHubPullRequestBrowserUrl(identity, "platform/api", 9)).toBe( | ||
| 65 | "https://github.acme.test/platform/api/pull/9", | ||
| 66 | ); | ||
| 67 | }); | ||
| 68 | |||
| 69 | it("returns null for missing or invalid GitHub data", () => { | ||
| 70 | expect(gitHubPullRequestBrowserUrl(null, "acme/repository", 1)).toBeNull(); | ||
| 71 | expect( | ||
| 72 | gitHubPullRequestBrowserUrl( | ||
| 73 | repositoryIdentity("github", "github.com/acme/repository", "https://github.com/a/b"), | ||
| 74 | "acme", | ||
| 75 | 1, | ||
| 76 | ), | ||
| 77 | ).toBeNull(); | ||
| 78 | expect( | ||
| 79 | gitHubPullRequestBrowserUrl( | ||
| 80 | repositoryIdentity("github", "github.com/acme/repository", "https://github.com/a/b"), | ||
| 81 | "../repository", | ||
| 82 | 1, | ||
| 83 | ), | ||
| 84 | ).toBeNull(); | ||
| 85 | expect( | ||
| 86 | gitHubPullRequestBrowserUrl( | ||
| 87 | repositoryIdentity("github", "github.com/acme/repository", "https://github.com/a/b"), | ||
| 88 | "acme/repository", | ||
| 89 | 0, | ||
| 90 | ), | ||
| 91 | ).toBeNull(); | ||
| 92 | expect( | ||
| 93 | gitHubPullRequestBrowserUrl( | ||
| 94 | repositoryIdentity("github", "bad host/acme/repository", "not a remote"), | ||
| 95 | "acme/repository", | ||
| 96 | 1, | ||
| 97 | ), | ||
| 98 | ).toBeNull(); | ||
| 99 | }); | ||
| 100 | |||
| 101 | it.each(["gitlab", "bitbucket", "azure-devops", "unknown"])( | ||
| 102 | "does not build a fallback for %s", | ||
| 103 | (provider) => { | ||
| 104 | expect( | ||
| 105 | gitHubPullRequestBrowserUrl( | ||
| 106 | repositoryIdentity(provider, "github.com/acme/repository", "https://github.com/a/b"), | ||
| 107 | "acme/repository", | ||
| 108 | 1, | ||
| 109 | ), | ||
| 110 | ).toBeNull(); | ||
| 111 | }, | ||
| 112 | ); | ||
| 113 | }); | ||
| 13 | 114 | ||
| 14 | describe("changeRequestRepositoryUrl", () => { | 115 | describe("changeRequestRepositoryUrl", () => { |
| 15 | it("preserves repository path casing", () => { | 116 | it("preserves repository path casing", () => { |
apps/web/src/lib/openPullRequestLink.ts+37| ... | @@ -1,6 +1,7 @@ | ... | @@ -1,6 +1,7 @@ |
| 1 | import type { | 1 | import type { |
| 2 | EnvironmentId, | 2 | EnvironmentId, |
| 3 | LocalApi, | 3 | LocalApi, |
| 4 | RepositoryIdentity, | ||
| 4 | ScopedThreadRef, | 5 | ScopedThreadRef, |
| 5 | ThreadLinkedPullRequest, | 6 | ThreadLinkedPullRequest, |
| 6 | } from "@t3tools/contracts"; | 7 | } from "@t3tools/contracts"; |
| ... | @@ -53,6 +54,42 @@ export async function openPullRequestLink( | ... | @@ -53,6 +54,42 @@ export async function openPullRequestLink( |
| 53 | } | 54 | } |
| 54 | } | 55 | } |
| 55 | 56 | ||
| 57 | /** Builds a GitHub URL that remains available when the pull request API cannot be read. */ | ||
| 58 | export function gitHubPullRequestBrowserUrl( | ||
| 59 | identity: RepositoryIdentity | null | undefined, | ||
| 60 | repository: string, | ||
| 61 | number: number, | ||
| 62 | ): string | null { | ||
| 63 | if (identity?.provider !== "github" || !Number.isSafeInteger(number) || number < 1) return null; | ||
| 64 | const repositoryPath = repository.split("/"); | ||
| 65 | if ( | ||
| 66 | repositoryPath.length !== 2 || | ||
| 67 | repositoryPath.some((segment) => segment.length === 0 || segment === "." || segment === "..") | ||
| 68 | ) { | ||
| 69 | return null; | ||
| 70 | } | ||
| 71 | |||
| 72 | let origin: string | null = null; | ||
| 73 | try { | ||
| 74 | const remoteUrl = new URL(identity.locator.remoteUrl.trim()); | ||
| 75 | if (remoteUrl.protocol === "http:" || remoteUrl.protocol === "https:") { | ||
| 76 | origin = remoteUrl.origin; | ||
| 77 | } | ||
| 78 | } catch { | ||
| 79 | // SCP-style remotes are read from their normalized identity below. | ||
| 80 | } | ||
| 81 | const hostname = identity.canonicalKey.split("/")[0]; | ||
| 82 | if (origin === null && !hostname) return null; | ||
| 83 | |||
| 84 | try { | ||
| 85 | const url = new URL(origin ?? `https://${hostname}`); | ||
| 86 | url.pathname = `/${repositoryPath.join("/")}/pull/${number}`; | ||
| 87 | return url.toString(); | ||
| 88 | } catch { | ||
| 89 | return null; | ||
| 90 | } | ||
| 91 | } | ||
| 92 | |||
| 56 | /** | 93 | /** |
| 57 | * A change request the page can open, named the way the page names one: the host below which the | 94 | * A change request the page can open, named the way the page names one: the host below which the |
| 58 | * repository is addressed, the repository path as that host writes it, and the number. | 95 | * repository is addressed, the repository path as that host writes it, and the number. |
docs/user/source-control.md+2| ... | @@ -42,6 +42,8 @@ T3 Code works with the platforms your team already uses: | ... | @@ -42,6 +42,8 @@ T3 Code works with the platforms your team already uses: |
| 42 | - While working in a thread, open linked reviews in the same compact right-panel tabs without | 42 | - While working in a thread, open linked reviews in the same compact right-panel tabs without |
| 43 | leaving the conversation | 43 | leaving the conversation |
| 44 | - Open the review directly in your browser with one click | 44 | - Open the review directly in your browser with one click |
| 45 | - If T3 Code cannot load a GitHub pull request, including when GitHub rate limits requests, use | ||
| 46 | **Open on GitHub** in the error view | ||
| 45 | - Command-click (Control-click on Windows and Linux) a pull request number in the sidebar to open it in your browser instead of in T3 Code | 47 | - Command-click (Control-click on Windows and Linux) a pull request number in the sidebar to open it in your browser instead of in T3 Code |
| 46 | - Check out a teammate's branch to review code locally | 48 | - Check out a teammate's branch to review code locally |
| 47 | 49 |