Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ import {
allowsSinglePullRequestMerge,
canDeletePullRequestBranch,
isPullRequestConflicting,
pullRequestDetailPatchAfterAction,
resolveBaseFreshness,
resolvePullRequestMergeMethod,
type PullRequestFinding,
Expand Down Expand Up @@ -347,6 +348,7 @@ export function PullRequestDetailPanel({
const [threadPickerOpen, setThreadPickerOpen] = useState(false);
const [openReviewRequest, setOpenReviewRequest] = useState(0);
const {
recordActionOutcome,
canWriteSourceControl,
detailQuery,
activityQuery,
Expand Down Expand Up @@ -687,6 +689,8 @@ export function PullRequestDetailPanel({
onActed?.(action, "failed");
return false;
}
const outcome = pullRequestDetailPatchAfterAction(action);
if (outcome !== null) recordActionOutcome(outcome);
if (action === "revert") {
toastManager.add({ type: "success", title: `Revert ${terminology.singular} opened` });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ import {
pullRequestPanelContext,
threadPullRequestPanelTarget,
latestPullRequestReviewOutcomes,
applyPullRequestDetailOverride,
expirePullRequestDetailOverride,
pullRequestDetailPatchAfterAction,
recordPullRequestDetailOverride,
settlePullRequestDetailOverride,
newestPullRequestCommitAt,
mergePullRequestThreadComments,
orderPullRequestComments,
Expand Down Expand Up @@ -315,6 +320,95 @@ describe("review verdicts", () => {
});
});

describe("confirmed action outcome on the detail", () => {
const scope = "env:project:host:repo#1";
const detail = { state: "open" as const, isDraft: true, autoMergeEnabled: false };
const at = 1_000_000;
const record = (patch: NonNullable<ReturnType<typeof pullRequestDetailPatchAfterAction>>) =>
recordPullRequestDetailOverride(null, scope, patch, at);

it("maps each action to the state it confirms, and leaves merge and the rest alone", () => {
expect(
(
["ready", "draft", "close", "reopen", "enable-auto-merge", "disable-auto-merge"] as const
).map(pullRequestDetailPatchAfterAction),
).toEqual([
{ isDraft: false },
{ isDraft: true },
{ state: "closed" },
{ state: "open" },
{ autoMergeEnabled: true },
{ autoMergeEnabled: false },
]);
expect(pullRequestDetailPatchAfterAction("merge")).toBeNull();
});

it("shows the outcome over the fetched detail", () => {
expect(applyPullRequestDetailOverride(detail, record({ isDraft: false }))).toMatchObject({
state: "open",
isDraft: false,
});
expect(applyPullRequestDetailOverride(detail, record({ state: "closed" })).state).toBe(
"closed",
);
expect(applyPullRequestDetailOverride(detail, null)).toBe(detail);
});

it("accumulates outcomes within a pull request and starts over for another", () => {
const first = record({ isDraft: false });
expect(recordPullRequestDetailOverride(first, scope, { state: "closed" }, at + 1)).toEqual({
scope,
at: { isDraft: at, state: at + 1 },
isDraft: false,
state: "closed",
});
expect(recordPullRequestDetailOverride(first, "other", { state: "closed" }, at + 1)).toEqual({
scope: "other",
at: { state: at + 1 },
state: "closed",
});
});

it("retires a field once the fetched detail agrees", () => {
const override = recordPullRequestDetailOverride(
record({ isDraft: false }),
scope,
{ autoMergeEnabled: true },
at,
);
expect(settlePullRequestDetailOverride(override, detail, scope)).toBe(override);
expect(settlePullRequestDetailOverride(override, { ...detail, isDraft: false }, scope)).toEqual(
{ scope, at: { autoMergeEnabled: at }, autoMergeEnabled: true },
);
expect(
settlePullRequestDetailOverride(
override,
{ ...detail, isDraft: false, autoMergeEnabled: true },
scope,
),
).toBeNull();
expect(settlePullRequestDetailOverride(override, detail, "other")).toBeNull();
expect(settlePullRequestDetailOverride(override, null, scope)).toBe(override);
});

it("expires each field on its own deadline", () => {
const override = recordPullRequestDetailOverride(
record({ isDraft: false }),
scope,
{ autoMergeEnabled: true },
at + 1,
);
expect(expirePullRequestDetailOverride(override, at)).toEqual({
scope,
at: { autoMergeEnabled: at + 1 },
autoMergeEnabled: true,
});
expect(
expirePullRequestDetailOverride(expirePullRequestDetailOverride(override, at), at + 1),
).toBeNull();
});
});

describe("pull request timeline", () => {
it("orders host timestamps by instant across offsets and fractional seconds", () => {
const events = buildPullRequestTimeline({
Expand Down
114 changes: 114 additions & 0 deletions apps/web/src/components/pullRequest/pullRequestDetail.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1403,3 +1403,117 @@ export function resolveDisplayedPullRequestDetail(input: {
? input.cached
: null;
}

export interface PullRequestDetailPatch {
readonly state?: PullRequestState;
readonly isDraft?: boolean;
readonly autoMergeEnabled?: boolean;
}

export interface PullRequestDetailOverride extends PullRequestDetailPatch {
readonly scope: string;
readonly at: Partial<Record<keyof PullRequestDetailPatch, number>>;
}

const PATCH_FIELDS = ["state", "isDraft", "autoMergeEnabled"] as const;

export function pullRequestDetailPatchAfterAction(
action: PullRequestAction,
): PullRequestDetailPatch | null {
switch (action) {
case "ready":
return { isDraft: false };
case "draft":
return { isDraft: true };
case "close":
return { state: "closed" };
case "reopen":
return { state: "open" };
case "enable-auto-merge":
return { autoMergeEnabled: true };
case "disable-auto-merge":
return { autoMergeEnabled: false };
default:
return null;
}
}

export function recordPullRequestDetailOverride(
previous: PullRequestDetailOverride | null,
scope: string,
patch: PullRequestDetailPatch,
now: number,
): PullRequestDetailOverride {
const base = previous?.scope === scope ? previous : null;
return {
...base,
...patch,
scope,
at: {
...base?.at,
...Object.fromEntries(
PATCH_FIELDS.filter((f) => patch[f] !== undefined).map((f) => [f, now]),
),
},
};
}

function retainPullRequestDetailOverride(
override: PullRequestDetailOverride,
keep: (field: (typeof PATCH_FIELDS)[number]) => boolean,
): PullRequestDetailOverride | null {
const held = PATCH_FIELDS.filter((f) => override[f] !== undefined);
const kept = held.filter(keep);
if (kept.length === 0) return null;
if (kept.length === held.length) return override;
return {
scope: override.scope,
...Object.fromEntries(kept.map((f) => [f, override[f]])),
at: Object.fromEntries(kept.map((f) => [f, override.at[f]])),
};
}

/** Drop the fields stamped at `at`, leaving fields recorded by later actions. */
export function expirePullRequestDetailOverride(
override: PullRequestDetailOverride | null,
at: number,
): PullRequestDetailOverride | null {
return override === null
? null
: retainPullRequestDetailOverride(override, (f) => override.at[f] !== at);
}

type OverridableDetail = Pick<PullRequestDetail, "state" | "isDraft" | "autoMergeEnabled">;

export function applyPullRequestDetailOverride<Detail extends OverridableDetail>(
detail: Detail,
override: PullRequestDetailOverride | null,
): Detail {
if (override === null) return detail;
const { state, isDraft, autoMergeEnabled } = override;
return {
...detail,
...(state === undefined ? {} : { state }),
...(isDraft === undefined ? {} : { isDraft }),
...(autoMergeEnabled === undefined ? {} : { autoMergeEnabled }),
};
}

/** Drop what the fetched detail now agrees with; the rest holds until it expires. */
export function settlePullRequestDetailOverride(
override: PullRequestDetailOverride | null,
detail: OverridableDetail | null,
scope: string,
): PullRequestDetailOverride | null {
if (override === null) return null;
if (override.scope !== scope) return null;
if (detail === null) return override;
const { state, isDraft, autoMergeEnabled } = override;
const open = {
state: state !== undefined && detail.state !== state,
isDraft: isDraft !== undefined && detail.isDraft !== isDraft,
autoMergeEnabled:
autoMergeEnabled !== undefined && detail.autoMergeEnabled !== autoMergeEnabled,
};
return retainPullRequestDetailOverride(override, (f) => open[f]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ export function reusePullRequestEntries<Entry extends PullRequestListEntry>(
: out;
}

const PULL_REQUEST_OVERRIDE_TRUST_MS = 60_000;
export const PULL_REQUEST_OVERRIDE_TRUST_MS = 60_000;

/** Retire overrides only when returned rows agree; partial-feed absence proves nothing. */
export function settlePullRequestOverrides<Entry extends PullRequestListEntry>(
Expand Down
24 changes: 23 additions & 1 deletion apps/web/src/components/pullRequest/usePullRequestDetail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@ import {
useSharedPullRequestSummary,
} from "~/state/pullRequests";
import {
applyPullRequestDetailOverride,
expirePullRequestDetailOverride,
readPullRequestDetailSnapshot,
recordPullRequestDetailOverride,
resolveDisplayedPullRequestDetail,
settlePullRequestDetailOverride,
type PullRequestDetailOverride,
type PullRequestDetailPatch,
} from "./pullRequestDetail.logic";
import { PULL_REQUEST_OVERRIDE_TRUST_MS } from "./pullRequestList.logic";

export function pullRequestRefKey(reference: PullRequestRef) {
return `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`;
Expand Down Expand Up @@ -88,6 +95,7 @@ export function usePullRequestDetail(
} else if (detailQuery.data !== null && detailQuery.data !== cachedDetail) {
setCachedDetail(detailQuery.data);
}
const [override, setOverride] = useState<PullRequestDetailOverride | null>(null);
const observedSummary = useSharedPullRequestSummary(environmentId, reference);
const activity = activityQuery.data;
const resolvedCoreDetail = resolveDisplayedPullRequestDetail({
Expand Down Expand Up @@ -128,7 +136,7 @@ export function usePullRequestDetail(
: sharedSummary.mergedAt,
isDraft: sharedSummary.isDraft ?? resolvedCoreDetail.isDraft,
};
const detail =
const fetchedDetail =
coreDetail === null
? null
: {
Expand Down Expand Up @@ -169,7 +177,21 @@ export function usePullRequestDetail(
commits: activity?.commits ?? [],
reactions: activity?.reactions ?? [],
};
const settledOverride = settlePullRequestDetailOverride(override, fetchedDetail, scopeKey);
if (settledOverride !== override) setOverride(settledOverride);
const detail =
fetchedDetail === null ? null : applyPullRequestDetailOverride(fetchedDetail, settledOverride);
const recordActionOutcome = (patch: PullRequestDetailPatch) => {
const at = Date.now();
setOverride((previous) => recordPullRequestDetailOverride(previous, scopeKey, patch, at));
// A fetch that never agrees must not pin the confirmed outcome forever.
window.setTimeout(
() => setOverride((previous) => expirePullRequestDetailOverride(previous, at)),
PULL_REQUEST_OVERRIDE_TRUST_MS,
);
};
return {
recordActionOutcome,
canWriteSourceControl,
detailQuery,
activityQuery,
Expand Down
Loading