| author | |
| committer | |
| log | d612d12b8bb278af97b0029f8d48b2403a5f9ee6 |
| tree | f576da861d48e52bff9876b7faff936136145c35 |
| parent | 19ee856ec099dddb94e28a3f933ec7c2575ed5a2 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>4 files changed, 176 insertions(+), 30 deletions(-)
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+8-3| ... | ... | @@ -145,6 +145,7 @@ import { |
| 145 | 145 | PULL_REQUEST_MERGE_METHOD_LABELS, |
| 146 | 146 | readableFailure, |
| 147 | 147 | readPullRequestDetailSnapshot, |
| 148 | resolvePullRequestReferenceHost, | |
| 148 | 149 | resolveDisplayedPullRequestDetail, |
| 149 | 150 | resolvePullRequestPrimaryControl, |
| 150 | 151 | allowsSinglePullRequestMerge, |
| ... | ... | @@ -523,18 +524,23 @@ export function PullRequestDetailPanel({ |
| 523 | 524 | onBack?: (() => void) | undefined; |
| 524 | 525 | }) { |
| 525 | 526 | const environmentConfigs = useServerConfigs(); |
| 527 | const projects = useProjects(); | |
| 528 | const repositoryIdentity = projects.find( | |
| 529 | (project) => | |
| 530 | project.id === requestedReference.projectId && project.environmentId === environmentId, | |
| 531 | )?.repositoryIdentity; | |
| 526 | 532 | const supportsThreadPullRequests = |
| 527 | 533 | environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true; |
| 528 | 534 | const reference = useMemo( |
| 529 | 535 | () => |
| 530 | 536 | supportsThreadPullRequests |
| 531 | ? requestedReference | |
| 537 | ? resolvePullRequestReferenceHost(requestedReference, repositoryIdentity) | |
| 532 | 538 | : { |
| 533 | 539 | projectId: requestedReference.projectId, |
| 534 | 540 | repository: requestedReference.repository, |
| 535 | 541 | number: requestedReference.number, |
| 536 | 542 | }, |
| 537 | [requestedReference, supportsThreadPullRequests], | |
| 543 | [requestedReference, repositoryIdentity, supportsThreadPullRequests], | |
| 538 | 544 | ); |
| 539 | 545 | const pullRequestKey = `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`; |
| 540 | 546 | const matchingListEntry = |
| ... | ... | @@ -881,7 +887,6 @@ export function PullRequestDetailPanel({ |
| 881 | 887 | const newThread = useNewThreadHandler(); |
| 882 | 888 | const { environments } = useEnvironments(); |
| 883 | 889 | const primaryEnvironmentId = usePrimaryEnvironmentId(); |
| 884 | const projects = useProjects(); | |
| 885 | 890 | const unavailableGitHubUrl = useMemo(() => { |
| 886 | 891 | const identity = projects.find( |
| 887 | 892 | (project) => project.id === reference.projectId && project.environmentId === environmentId, |
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts+108-3| ... | ... | @@ -1,6 +1,7 @@ |
| 1 | 1 | import { resolvePlanFollowUpSubmission } from "../../proposedPlan"; |
| 2 | 2 | import { serializeLegacyContextMessage } from "@t3tools/shared/composerContextLegacySend"; |
| 3 | 3 | import { |
| 4 | ProjectId, | |
| 4 | 5 | PullRequestAction, |
| 5 | 6 | type PullRequestCheck, |
| 6 | 7 | type PullRequestComment, |
| ... | ... | @@ -40,6 +41,7 @@ import { |
| 40 | 41 | readableFailure, |
| 41 | 42 | readPullRequestDetailSnapshot, |
| 42 | 43 | resolveDisplayedPullRequestDetail, |
| 44 | resolvePullRequestReferenceHost, | |
| 43 | 45 | resolvePullRequestPrimaryControl, |
| 44 | 46 | allowsSinglePullRequestMerge, |
| 45 | 47 | shouldRefreshPullRequestActivity, |
| ... | ... | @@ -1440,7 +1442,7 @@ describe("which actions need the host read again after they run", () => { |
| 1440 | 1442 | }); |
| 1441 | 1443 | |
| 1442 | 1444 | describe("cached pull request detail", () => { |
| 1443 | const reference = { projectId: "project-1", repository: "acme/web", number: 7 }; | |
| 1445 | const reference = { projectId: ProjectId.make("project-1"), repository: "acme/web", number: 7 }; | |
| 1444 | 1446 | const detail = (overrides: Partial<PullRequestDetail> = {}): PullRequestDetail => |
| 1445 | 1447 | ({ |
| 1446 | 1448 | provider: "github", |
| ... | ... | @@ -1511,6 +1513,106 @@ describe("cached pull request detail", () => { |
| 1511 | 1513 | expect(snapshot?.deletions).toBe(3); |
| 1512 | 1514 | }); |
| 1513 | 1515 | |
| 1516 | it("reuses a host-qualified snapshot when reopening a thread link without a host", () => { | |
| 1517 | const storage = makeStorage(); | |
| 1518 | writePullRequestDetailSnapshot( | |
| 1519 | storage, | |
| 1520 | "env-1", | |
| 1521 | { ...reference, host: "github.com" }, | |
| 1522 | detail(), | |
| 1523 | ); | |
| 1524 | const resolved = resolvePullRequestReferenceHost(reference, { | |
| 1525 | canonicalKey: "github.com/acme/web", | |
| 1526 | locator: { | |
| 1527 | source: "git-remote", | |
| 1528 | remoteName: "origin", | |
| 1529 | remoteUrl: "https://github.com/acme/web.git", | |
| 1530 | }, | |
| 1531 | provider: "github", | |
| 1532 | }); | |
| 1533 | expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe( | |
| 1534 | "Cache the title", | |
| 1535 | ); | |
| 1536 | const explicit = { ...reference, host: "github.example.com" }; | |
| 1537 | expect( | |
| 1538 | resolvePullRequestReferenceHost(explicit, { | |
| 1539 | canonicalKey: "github.com/acme/web", | |
| 1540 | locator: { | |
| 1541 | source: "git-remote", | |
| 1542 | remoteName: "origin", | |
| 1543 | remoteUrl: "https://github.com/acme/web.git", | |
| 1544 | }, | |
| 1545 | }), | |
| 1546 | ).toBe(explicit); | |
| 1547 | }); | |
| 1548 | ||
| 1549 | it("leaves server-resolved Azure SSH references unchanged", () => { | |
| 1550 | expect( | |
| 1551 | resolvePullRequestReferenceHost(reference, { | |
| 1552 | canonicalKey: "ssh.dev.azure.com/v3/org/project/web", | |
| 1553 | locator: { | |
| 1554 | source: "git-remote", | |
| 1555 | remoteName: "origin", | |
| 1556 | remoteUrl: "git@ssh.dev.azure.com:v3/org/project/web", | |
| 1557 | }, | |
| 1558 | provider: "azure-devops", | |
| 1559 | }), | |
| 1560 | ).toBe(reference); | |
| 1561 | expect(resolvePullRequestReferenceHost(reference, undefined)).toBe(reference); | |
| 1562 | }); | |
| 1563 | ||
| 1564 | it("hydrates legacy hostless snapshots only for the matching host", () => { | |
| 1565 | const storage = makeStorage(); | |
| 1566 | writePullRequestDetailSnapshot(storage, "env-1", reference, detail()); | |
| 1567 | expect( | |
| 1568 | readPullRequestDetailSnapshot(storage, "env-1", { ...reference, host: "github.com" })?.title, | |
| 1569 | ).toBe("Cache the title"); | |
| 1570 | expect( | |
| 1571 | readPullRequestDetailSnapshot(storage, "env-1", { | |
| 1572 | ...reference, | |
| 1573 | host: "github.example.com", | |
| 1574 | }), | |
| 1575 | ).toBeNull(); | |
| 1576 | }); | |
| 1577 | ||
| 1578 | it("keeps Forgejo ports isolated when recovering legacy snapshots", () => { | |
| 1579 | const storage = makeStorage(); | |
| 1580 | const cached = detail({ | |
| 1581 | provider: "forgejo", | |
| 1582 | url: "https://forge.example:8443/acme/web/pulls/7", | |
| 1583 | }); | |
| 1584 | writePullRequestDetailSnapshot(storage, "env-1", reference, cached); | |
| 1585 | const resolved = { ...reference, host: "forge.example:8443" }; | |
| 1586 | expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe(cached.title); | |
| 1587 | expect( | |
| 1588 | readPullRequestDetailSnapshot(storage, "env-1", { | |
| 1589 | ...reference, | |
| 1590 | host: "forge.example:9443", | |
| 1591 | }), | |
| 1592 | ).toBeNull(); | |
| 1593 | expect( | |
| 1594 | readPullRequestDetailSnapshot(storage, "env-1", { | |
| 1595 | ...reference, | |
| 1596 | host: "forge.example", | |
| 1597 | }), | |
| 1598 | ).toBeNull(); | |
| 1599 | }); | |
| 1600 | ||
| 1601 | it.each(["github", "gitlab"] as const)( | |
| 1602 | "retains portless %s snapshot identities for custom web ports", | |
| 1603 | (provider) => { | |
| 1604 | const storage = makeStorage(); | |
| 1605 | const host = `${provider}.example.com`; | |
| 1606 | const hosted = { ...reference, host }; | |
| 1607 | const cached = detail({ | |
| 1608 | provider, | |
| 1609 | url: `https://${host}:8443/acme/web/${provider === "github" ? "pull" : "-/merge_requests"}/7`, | |
| 1610 | }); | |
| 1611 | writePullRequestDetailSnapshot(storage, "env-1", hosted, cached); | |
| 1612 | expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)?.title).toBe(cached.title); | |
| 1613 | }, | |
| 1614 | ); | |
| 1615 | ||
| 1514 | 1616 | it("keeps a cached tab painted while the live read replaces the counts", () => { |
| 1515 | 1617 | const cached = detail(); |
| 1516 | 1618 | const live = detail({ additions: 40, deletions: 9, title: "Cache the title" }); |
| ... | ... | @@ -1534,11 +1636,11 @@ describe("cached pull request detail", () => { |
| 1534 | 1636 | it("isolates stored and displayed details between hosts with the same repository and number", () => { |
| 1535 | 1637 | const storage = makeStorage(); |
| 1536 | 1638 | const publicRef = { ...reference, host: "github.com" }; |
| 1537 | const enterpriseRef = { ...reference, host: "github.example.com" }; | |
| 1639 | const enterpriseRef = { ...reference, host: "ghe.example.com" }; | |
| 1538 | 1640 | const publicDetail = detail(); |
| 1539 | 1641 | const enterpriseDetail = detail({ |
| 1540 | 1642 | title: "Enterprise change", |
| 1541 | url: "https://github.example.com/acme/web/pull/7", | |
| 1643 | url: "https://ghe.example.com/acme/web/pull/7", | |
| 1542 | 1644 | }); |
| 1543 | 1645 | writePullRequestDetailSnapshot(storage, "env-1", publicRef, publicDetail); |
| 1544 | 1646 | expect(readPullRequestDetailSnapshot(storage, "env-1", enterpriseRef)).toBeNull(); |
| ... | ... | @@ -1572,6 +1674,9 @@ describe("cached pull request detail", () => { |
| 1572 | 1674 | storage.setItem("t3.pullRequests.detail:env-1:project-1:acme/web#7", "{not json"); |
| 1573 | 1675 | expect(readPullRequestDetailSnapshot(storage, "env-1", reference)).toBeNull(); |
| 1574 | 1676 | expect(readPullRequestDetailSnapshot(undefined, "env-1", reference)).toBeNull(); |
| 1677 | const hosted = { ...reference, host: "github.com" }; | |
| 1678 | writePullRequestDetailSnapshot(storage, "env-1", hosted, detail({ url: "invalid url" })); | |
| 1679 | expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)).toBeNull(); | |
| 1575 | 1680 | }); |
| 1576 | 1681 | }); |
| 1577 | 1682 |
apps/web/src/components/pullRequest/pullRequestDetail.logic.ts+35-10| ... | ... | @@ -1,8 +1,8 @@ |
| 1 | 1 | import * as Schema from "effect/Schema"; |
| 2 | import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; | |
| 3 | 2 | |
| 4 | 3 | import { |
| 5 | 4 | PullRequestDetail, |
| 5 | pullRequestHostOf, | |
| 6 | 6 | type PullRequestAction, |
| 7 | 7 | type PullRequestActor, |
| 8 | 8 | type PullRequestBaseComparison, |
| ... | ... | @@ -15,6 +15,8 @@ import { |
| 15 | 15 | type PullRequestMergeability, |
| 16 | 16 | type PullRequestMergeMethod, |
| 17 | 17 | type PullRequestReaction, |
| 18 | type PullRequestRef, | |
| 19 | type RepositoryIdentity, | |
| 18 | 20 | type PullRequestReviewThread, |
| 19 | 21 | type PullRequestState, |
| 20 | 22 | type PullRequestUpdateMethod, |
| ... | ... | @@ -1092,6 +1094,15 @@ export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): bo |
| 1092 | 1094 | |
| 1093 | 1095 | type SnapshotStorage = Pick<Storage, "getItem" | "setItem">; |
| 1094 | 1096 | |
| 1097 | export function resolvePullRequestReferenceHost( | |
| 1098 | reference: PullRequestRef, | |
| 1099 | identity: RepositoryIdentity | null | undefined, | |
| 1100 | ): PullRequestRef { | |
| 1101 | // Other providers may resolve an SSH remote to a different web authority on the server. | |
| 1102 | if (reference.host !== undefined || identity?.provider !== "github") return reference; | |
| 1103 | return { ...reference, host: pullRequestHostOf(identity, "github") }; | |
| 1104 | } | |
| 1105 | ||
| 1095 | 1106 | export interface PullRequestDetailSnapshotRef { |
| 1096 | 1107 | readonly host?: string | undefined; |
| 1097 | 1108 | readonly projectId: string; |
| ... | ... | @@ -1121,7 +1132,13 @@ export function readPullRequestDetailSnapshot( |
| 1121 | 1132 | reference: PullRequestDetailSnapshotRef, |
| 1122 | 1133 | ): PullRequestDetail | null { |
| 1123 | 1134 | try { |
| 1124 | const raw = storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference)); | |
| 1135 | const raw = | |
| 1136 | storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference)) ?? | |
| 1137 | (reference.host === undefined | |
| 1138 | ? null | |
| 1139 | : storage?.getItem( | |
| 1140 | pullRequestDetailSnapshotKey(environmentId, { ...reference, host: undefined }), | |
| 1141 | )); | |
| 1125 | 1142 | if (!raw) return null; |
| 1126 | 1143 | const decoded = decodeDetailSnapshot(JSON.parse(raw)); |
| 1127 | 1144 | return decoded._tag === "Some" |
| ... | ... | @@ -1157,14 +1174,22 @@ export function resolveDisplayedPullRequestDetail(input: { |
| 1157 | 1174 | }): PullRequestDetail | null { |
| 1158 | 1175 | if (input.live !== null) return input.live; |
| 1159 | 1176 | if ( |
| 1160 | input.cached !== null && | |
| 1161 | input.cached.projectId === input.reference.projectId && | |
| 1162 | input.cached.repository.toLowerCase() === input.reference.repository.toLowerCase() && | |
| 1163 | input.cached.number === input.reference.number && | |
| 1164 | (input.reference.host === undefined || | |
| 1165 | parseChangeRequestUrl(input.cached.url)?.host === input.reference.host.toLowerCase()) | |
| 1177 | input.cached === null || | |
| 1178 | input.cached.projectId !== input.reference.projectId || | |
| 1179 | input.cached.repository.toLowerCase() !== input.reference.repository.toLowerCase() || | |
| 1180 | input.cached.number !== input.reference.number | |
| 1166 | 1181 | ) { |
| 1167 | return input.cached; | |
| 1182 | return null; | |
| 1183 | } | |
| 1184 | if (input.reference.host === undefined) return input.cached; | |
| 1185 | try { | |
| 1186 | const url = new URL(input.cached.url); | |
| 1187 | const host = input.cached.provider === "forgejo" ? url.host : url.hostname; | |
| 1188 | return (url.protocol === "https:" || url.protocol === "http:") && | |
| 1189 | host.toLowerCase() === input.reference.host.toLowerCase() | |
| 1190 | ? input.cached | |
| 1191 | : null; | |
| 1192 | } catch { | |
| 1193 | return null; | |
| 1168 | 1194 | } |
| 1169 | return null; | |
| 1170 | 1195 | } |
apps/web/src/hooks/useOpenPanelPullRequestUrl.ts+25-14| ... | ... | @@ -5,6 +5,7 @@ import { useMemo } from "react"; |
| 5 | 5 | import { |
| 6 | 6 | readPullRequestDetailSnapshot, |
| 7 | 7 | resolveDisplayedPullRequestDetail, |
| 8 | resolvePullRequestReferenceHost, | |
| 8 | 9 | } from "../components/pullRequest/pullRequestDetail.logic"; |
| 9 | 10 | import { gitHubPullRequestBrowserUrl } from "../lib/openPullRequestLink"; |
| 10 | 11 | import { selectActiveRightPanelSurface, useRightPanelStore } from "../rightPanelStore"; |
| ... | ... | @@ -17,28 +18,38 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) { |
| 17 | 18 | const surface = useRightPanelStore((state) => |
| 18 | 19 | selectActiveRightPanelSurface(state.byThreadKey, threadRef), |
| 19 | 20 | ); |
| 20 | const reference = surface?.kind === "pull-request" ? surface : null; | |
| 21 | const environmentId = reference?.environmentId | |
| 22 | ? EnvironmentId.make(reference.environmentId) | |
| 21 | const requestedReference = surface?.kind === "pull-request" ? surface : null; | |
| 22 | const environmentId = requestedReference?.environmentId | |
| 23 | ? EnvironmentId.make(requestedReference.environmentId) | |
| 23 | 24 | : threadRef?.environmentId; |
| 24 | 25 | const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId ?? null); |
| 25 | 26 | const project = useProject( |
| 26 | reference && environmentId | |
| 27 | ? scopeProjectRef(environmentId, ProjectId.make(reference.projectId)) | |
| 27 | requestedReference && environmentId | |
| 28 | ? scopeProjectRef(environmentId, ProjectId.make(requestedReference.projectId)) | |
| 28 | 29 | : null, |
| 29 | 30 | ); |
| 31 | const reference = useMemo(() => { | |
| 32 | if (requestedReference === null) return null; | |
| 33 | const input = { | |
| 34 | projectId: ProjectId.make(requestedReference.projectId), | |
| 35 | repository: requestedReference.repository, | |
| 36 | number: requestedReference.number, | |
| 37 | }; | |
| 38 | return supportsMultiplePullRequests | |
| 39 | ? resolvePullRequestReferenceHost( | |
| 40 | { | |
| 41 | ...input, | |
| 42 | ...(requestedReference.host === undefined ? {} : { host: requestedReference.host }), | |
| 43 | }, | |
| 44 | project?.repositoryIdentity, | |
| 45 | ) | |
| 46 | : input; | |
| 47 | }, [requestedReference, supportsMultiplePullRequests, project?.repositoryIdentity]); | |
| 30 | 48 | const detail = useEnvironmentQuery( |
| 31 | 49 | reference && environmentId |
| 32 | 50 | ? pullRequestEnvironment.detail({ |
| 33 | 51 | environmentId, |
| 34 | input: { | |
| 35 | projectId: ProjectId.make(reference.projectId), | |
| 36 | ...(supportsMultiplePullRequests && reference.host !== undefined | |
| 37 | ? { host: reference.host } | |
| 38 | : {}), | |
| 39 | repository: reference.repository, | |
| 40 | number: reference.number, | |
| 41 | }, | |
| 52 | input: reference, | |
| 42 | 53 | }) |
| 43 | 54 | : null, |
| 44 | 55 | ).data; |
| ... | ... | @@ -55,7 +66,7 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) { |
| 55 | 66 | ); |
| 56 | 67 | return reference |
| 57 | 68 | ? (resolveDisplayedPullRequestDetail({ live: detail, cached: cachedDetail, reference })?.url ?? |
| 58 | reference.url ?? | |
| 69 | requestedReference?.url ?? | |
| 59 | 70 | gitHubPullRequestBrowserUrl( |
| 60 | 71 | project?.repositoryIdentity, |
| 61 | 72 | reference.repository, |