Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 8 additions & 3 deletions apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ import {
PULL_REQUEST_MERGE_METHOD_LABELS,
readableFailure,
readPullRequestDetailSnapshot,
resolvePullRequestReferenceHost,
resolveDisplayedPullRequestDetail,
resolvePullRequestPrimaryControl,
allowsSinglePullRequestMerge,
Expand Down Expand Up @@ -523,18 +524,23 @@ export function PullRequestDetailPanel({
onBack?: (() => void) | undefined;
}) {
const environmentConfigs = useServerConfigs();
const projects = useProjects();
const repositoryIdentity = projects.find(
(project) =>
project.id === requestedReference.projectId && project.environmentId === environmentId,
)?.repositoryIdentity;
const supportsThreadPullRequests =
environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true;
const reference = useMemo(
() =>
supportsThreadPullRequests
? requestedReference
? resolvePullRequestReferenceHost(requestedReference, repositoryIdentity)
: {
projectId: requestedReference.projectId,
repository: requestedReference.repository,
number: requestedReference.number,
},
[requestedReference, supportsThreadPullRequests],
[requestedReference, repositoryIdentity, supportsThreadPullRequests],
);
const pullRequestKey = `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`;
const matchingListEntry =
Expand Down Expand Up @@ -881,7 +887,6 @@ export function PullRequestDetailPanel({
const newThread = useNewThreadHandler();
const { environments } = useEnvironments();
const primaryEnvironmentId = usePrimaryEnvironmentId();
const projects = useProjects();
const unavailableGitHubUrl = useMemo(() => {
const identity = projects.find(
(project) => project.id === reference.projectId && project.environmentId === environmentId,
Expand Down
111 changes: 108 additions & 3 deletions apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { resolvePlanFollowUpSubmission } from "../../proposedPlan";
import { serializeLegacyContextMessage } from "@t3tools/shared/composerContextLegacySend";
import {
ProjectId,
PullRequestAction,
type PullRequestCheck,
type PullRequestComment,
Expand Down Expand Up @@ -40,6 +41,7 @@ import {
readableFailure,
readPullRequestDetailSnapshot,
resolveDisplayedPullRequestDetail,
resolvePullRequestReferenceHost,
resolvePullRequestPrimaryControl,
allowsSinglePullRequestMerge,
shouldRefreshPullRequestActivity,
Expand Down Expand Up @@ -1440,7 +1442,7 @@ describe("which actions need the host read again after they run", () => {
});

describe("cached pull request detail", () => {
const reference = { projectId: "project-1", repository: "acme/web", number: 7 };
const reference = { projectId: ProjectId.make("project-1"), repository: "acme/web", number: 7 };
const detail = (overrides: Partial<PullRequestDetail> = {}): PullRequestDetail =>
({
provider: "github",
Expand Down Expand Up @@ -1511,6 +1513,106 @@ describe("cached pull request detail", () => {
expect(snapshot?.deletions).toBe(3);
});

it("reuses a host-qualified snapshot when reopening a thread link without a host", () => {
const storage = makeStorage();
writePullRequestDetailSnapshot(
storage,
"env-1",
{ ...reference, host: "github.com" },
detail(),
);
const resolved = resolvePullRequestReferenceHost(reference, {
canonicalKey: "github.com/acme/web",
locator: {
source: "git-remote",
remoteName: "origin",
remoteUrl: "https://github.com/acme/web.git",
},
provider: "github",
});
expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe(
"Cache the title",
);
const explicit = { ...reference, host: "github.example.com" };
expect(
resolvePullRequestReferenceHost(explicit, {
canonicalKey: "github.com/acme/web",
locator: {
source: "git-remote",
remoteName: "origin",
remoteUrl: "https://github.com/acme/web.git",
},
}),
).toBe(explicit);
});

it("leaves server-resolved Azure SSH references unchanged", () => {
expect(
resolvePullRequestReferenceHost(reference, {
canonicalKey: "ssh.dev.azure.com/v3/org/project/web",
locator: {
source: "git-remote",
remoteName: "origin",
remoteUrl: "git@ssh.dev.azure.com:v3/org/project/web",
},
provider: "azure-devops",
}),
).toBe(reference);
expect(resolvePullRequestReferenceHost(reference, undefined)).toBe(reference);
});

it("hydrates legacy hostless snapshots only for the matching host", () => {
const storage = makeStorage();
writePullRequestDetailSnapshot(storage, "env-1", reference, detail());
expect(
readPullRequestDetailSnapshot(storage, "env-1", { ...reference, host: "github.com" })?.title,
).toBe("Cache the title");
expect(
readPullRequestDetailSnapshot(storage, "env-1", {
...reference,
host: "github.example.com",
}),
).toBeNull();
});

it("keeps Forgejo ports isolated when recovering legacy snapshots", () => {
const storage = makeStorage();
const cached = detail({
provider: "forgejo",
url: "https://forge.example:8443/acme/web/pulls/7",
});
writePullRequestDetailSnapshot(storage, "env-1", reference, cached);
const resolved = { ...reference, host: "forge.example:8443" };
expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe(cached.title);
expect(
readPullRequestDetailSnapshot(storage, "env-1", {
...reference,
host: "forge.example:9443",
}),
).toBeNull();
expect(
readPullRequestDetailSnapshot(storage, "env-1", {
...reference,
host: "forge.example",
}),
).toBeNull();
});

it.each(["github", "gitlab"] as const)(
"retains portless %s snapshot identities for custom web ports",
(provider) => {
const storage = makeStorage();
const host = `${provider}.example.com`;
const hosted = { ...reference, host };
const cached = detail({
provider,
url: `https://${host}:8443/acme/web/${provider === "github" ? "pull" : "-/merge_requests"}/7`,
});
writePullRequestDetailSnapshot(storage, "env-1", hosted, cached);
expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)?.title).toBe(cached.title);
},
);

it("keeps a cached tab painted while the live read replaces the counts", () => {
const cached = detail();
const live = detail({ additions: 40, deletions: 9, title: "Cache the title" });
Expand All @@ -1534,11 +1636,11 @@ describe("cached pull request detail", () => {
it("isolates stored and displayed details between hosts with the same repository and number", () => {
const storage = makeStorage();
const publicRef = { ...reference, host: "github.com" };
const enterpriseRef = { ...reference, host: "github.example.com" };
const enterpriseRef = { ...reference, host: "ghe.example.com" };
const publicDetail = detail();
const enterpriseDetail = detail({
title: "Enterprise change",
url: "https://github.example.com/acme/web/pull/7",
url: "https://ghe.example.com/acme/web/pull/7",
});
writePullRequestDetailSnapshot(storage, "env-1", publicRef, publicDetail);
expect(readPullRequestDetailSnapshot(storage, "env-1", enterpriseRef)).toBeNull();
Expand Down Expand Up @@ -1572,6 +1674,9 @@ describe("cached pull request detail", () => {
storage.setItem("t3.pullRequests.detail:env-1:project-1:acme/web#7", "{not json");
expect(readPullRequestDetailSnapshot(storage, "env-1", reference)).toBeNull();
expect(readPullRequestDetailSnapshot(undefined, "env-1", reference)).toBeNull();
const hosted = { ...reference, host: "github.com" };
writePullRequestDetailSnapshot(storage, "env-1", hosted, detail({ url: "invalid url" }));
expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)).toBeNull();
});
});

Expand Down
45 changes: 35 additions & 10 deletions apps/web/src/components/pullRequest/pullRequestDetail.logic.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as Schema from "effect/Schema";
import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl";

import {
PullRequestDetail,
pullRequestHostOf,
type PullRequestAction,
type PullRequestActor,
type PullRequestBaseComparison,
Expand All @@ -15,6 +15,8 @@ import {
type PullRequestMergeability,
type PullRequestMergeMethod,
type PullRequestReaction,
type PullRequestRef,
type RepositoryIdentity,
type PullRequestReviewThread,
type PullRequestState,
type PullRequestUpdateMethod,
Expand Down Expand Up @@ -1092,6 +1094,15 @@ export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): bo

type SnapshotStorage = Pick<Storage, "getItem" | "setItem">;

export function resolvePullRequestReferenceHost(
reference: PullRequestRef,
identity: RepositoryIdentity | null | undefined,
): PullRequestRef {
// Other providers may resolve an SSH remote to a different web authority on the server.
if (reference.host !== undefined || identity?.provider !== "github") return reference;
return { ...reference, host: pullRequestHostOf(identity, "github") };
}

export interface PullRequestDetailSnapshotRef {
readonly host?: string | undefined;
readonly projectId: string;
Expand Down Expand Up @@ -1121,7 +1132,13 @@ export function readPullRequestDetailSnapshot(
reference: PullRequestDetailSnapshotRef,
): PullRequestDetail | null {
try {
const raw = storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference));
const raw =
storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference)) ??
(reference.host === undefined
? null
: storage?.getItem(
pullRequestDetailSnapshotKey(environmentId, { ...reference, host: undefined }),
));
if (!raw) return null;
const decoded = decodeDetailSnapshot(JSON.parse(raw));
return decoded._tag === "Some"
Expand Down Expand Up @@ -1157,14 +1174,22 @@ export function resolveDisplayedPullRequestDetail(input: {
}): PullRequestDetail | null {
if (input.live !== null) return input.live;
if (
input.cached !== null &&
input.cached.projectId === input.reference.projectId &&
input.cached.repository.toLowerCase() === input.reference.repository.toLowerCase() &&
input.cached.number === input.reference.number &&
(input.reference.host === undefined ||
parseChangeRequestUrl(input.cached.url)?.host === input.reference.host.toLowerCase())
input.cached === null ||
input.cached.projectId !== input.reference.projectId ||
input.cached.repository.toLowerCase() !== input.reference.repository.toLowerCase() ||
input.cached.number !== input.reference.number
) {
return input.cached;
return null;
}
if (input.reference.host === undefined) return input.cached;
try {
const url = new URL(input.cached.url);
const host = input.cached.provider === "forgejo" ? url.host : url.hostname;
return (url.protocol === "https:" || url.protocol === "http:") &&
host.toLowerCase() === input.reference.host.toLowerCase()
? input.cached
: null;
} catch {
return null;
}
return null;
}
39 changes: 25 additions & 14 deletions apps/web/src/hooks/useOpenPanelPullRequestUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useMemo } from "react";
import {
readPullRequestDetailSnapshot,
resolveDisplayedPullRequestDetail,
resolvePullRequestReferenceHost,
} from "../components/pullRequest/pullRequestDetail.logic";
import { gitHubPullRequestBrowserUrl } from "../lib/openPullRequestLink";
import { selectActiveRightPanelSurface, useRightPanelStore } from "../rightPanelStore";
Expand All @@ -17,28 +18,38 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) {
const surface = useRightPanelStore((state) =>
selectActiveRightPanelSurface(state.byThreadKey, threadRef),
);
const reference = surface?.kind === "pull-request" ? surface : null;
const environmentId = reference?.environmentId
? EnvironmentId.make(reference.environmentId)
const requestedReference = surface?.kind === "pull-request" ? surface : null;
const environmentId = requestedReference?.environmentId
? EnvironmentId.make(requestedReference.environmentId)
: threadRef?.environmentId;
const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId ?? null);
const project = useProject(
reference && environmentId
? scopeProjectRef(environmentId, ProjectId.make(reference.projectId))
requestedReference && environmentId
? scopeProjectRef(environmentId, ProjectId.make(requestedReference.projectId))
: null,
);
const reference = useMemo(() => {
if (requestedReference === null) return null;
const input = {
projectId: ProjectId.make(requestedReference.projectId),
repository: requestedReference.repository,
number: requestedReference.number,
};
return supportsMultiplePullRequests
? resolvePullRequestReferenceHost(
{
...input,
...(requestedReference.host === undefined ? {} : { host: requestedReference.host }),
},
project?.repositoryIdentity,
)
: input;
}, [requestedReference, supportsMultiplePullRequests, project?.repositoryIdentity]);
const detail = useEnvironmentQuery(
reference && environmentId
? pullRequestEnvironment.detail({
environmentId,
input: {
projectId: ProjectId.make(reference.projectId),
...(supportsMultiplePullRequests && reference.host !== undefined
? { host: reference.host }
: {}),
repository: reference.repository,
number: reference.number,
},
input: reference,
})
: null,
).data;
Expand All @@ -55,7 +66,7 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) {
);
return reference
? (resolveDisplayedPullRequestDetail({ live: detail, cached: cachedDetail, reference })?.url ??
reference.url ??
requestedReference?.url ??
gitHubPullRequestBrowserUrl(
project?.repositoryIdentity,
reference.repository,
Expand Down
Loading