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 @@ -110,14 +110,16 @@ export function PullRequestMergeControls(props: {
? { label: "Draft", icon: PullRequestGlyph.draft }
: pr.mergeability === "conflicting"
? { label: "Resolve conflicts to merge", icon: PullRequestGlyph.conflicting }
: pr.autoMergeEnabled
? {
label: `Auto-merge pending${pr.autoMergeMethod ? ` (${pr.autoMergeMethod})` : ""}`,
icon: "clock",
}
: checkingStack && !busy
? { label: "Checking stack…", icon: PullRequestGlyph.merged }
: null;
: pr.isInMergeQueue
? { label: "In merge queue", icon: PullRequestGlyph.merged }
: pr.autoMergeEnabled
? {
label: `Auto-merge pending${pr.autoMergeMethod ? ` (${pr.autoMergeMethod})` : ""}`,
icon: "clock",
}
: checkingStack && !busy
? { label: "Checking stack…", icon: PullRequestGlyph.merged }
: null;
const autoAction = pr.autoMergeEnabled ? "disable-auto-merge" : "enable-auto-merge";
const canAutoMerge =
!busy &&
Expand Down Expand Up @@ -232,7 +234,9 @@ export function PullRequestMergeControls(props: {
id,
title: MERGE_METHODS[id].title,
state: id === method ? ("on" as const) : ("off" as const),
attributes: { disabled: !canMerge && !canAutoMerge },
attributes: {
disabled: pr.isInMergeQueue === true || (!canMerge && !canAutoMerge),
},
})),
...(canAutoMerge
? [
Expand Down
15 changes: 12 additions & 3 deletions apps/mobile/src/features/pull-requests/pullRequestActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,19 @@ describe("mobile pull request actions", () => {
expect(mergeUnavailableReason(detail(), false, GITHUB)).toContain("read-only");
});

it("blocks a conflicting PR's immediate merge and explains why", () => {
const pr = detail({ mergeability: "conflicting" });
it.each([
{ fields: { mergeability: "conflicting" as const }, reason: "conflicts" },
{ fields: { isInMergeQueue: true }, reason: "merge queue" },
])("blocks an immediate merge and explains why: $reason", ({ fields, reason }) => {
const pr = detail(fields);
expect(canPerformPullRequestAction(pr, "merge")).toBe(false);
expect(mergeUnavailableReason(pr, true, GITHUB)).toContain("conflicts");
expect(mergeUnavailableReason(pr, true, GITHUB)).toContain(reason);
});

it("offers no auto-merge for a pull request the merge queue already holds", () => {
expect(canPerformPullRequestAction(detail({ isInMergeQueue: true }), "enable-auto-merge")).toBe(
false,
);
});

it("names the change request with its host's term", () => {
Expand Down
23 changes: 20 additions & 3 deletions apps/mobile/src/features/pull-requests/pullRequestActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import { PullRequestGlyph } from "./pullRequestGlyphs";

type ActionDetail = Pick<
PullRequestDetail,
"state" | "isDraft" | "mergeability" | "autoMergeEnabled" | "workflowApprovalsRequired"
| "state"
| "isDraft"
| "isInMergeQueue"
| "mergeability"
| "autoMergeEnabled"
| "workflowApprovalsRequired"
> & {
readonly capabilities: Pick<
PullRequestDetail["capabilities"],
Expand Down Expand Up @@ -107,9 +112,19 @@ export function canPerformPullRequestAction(pr: ActionDetail, action: PullReques
case "draft":
return !pr.isDraft;
case "merge":
return !pr.isDraft && pr.mergeability !== "conflicting" && allowedMergeMethods(pr).length > 0;
return (
!pr.isDraft &&
pr.isInMergeQueue !== true &&
pr.mergeability !== "conflicting" &&
allowedMergeMethods(pr).length > 0
);
case "enable-auto-merge":
return !pr.isDraft && pr.autoMergeEnabled === false && allowedMergeMethods(pr).length > 0;
return (
!pr.isDraft &&
pr.isInMergeQueue !== true &&
pr.autoMergeEnabled === false &&
allowedMergeMethods(pr).length > 0
);
case "disable-auto-merge":
return pr.autoMergeEnabled === true;
default:
Expand All @@ -130,6 +145,8 @@ export function mergeUnavailableReason(
if (!pr.viewerPermissions.actions.includes("merge"))
return `Your account cannot merge this ${terminology.singular}.`;
if (pr.isDraft) return "Mark this draft ready for review before merging.";
if (pr.isInMergeQueue === true)
return `This ${terminology.singular} is already in the merge queue.`;
if (pr.mergeability === "conflicting") return "Resolve the branch conflicts before merging.";
if (!allowedMergeMethods(pr).length) return "This repository has no available merge methods.";
return null;
Expand Down
16 changes: 14 additions & 2 deletions apps/mobile/src/features/pull-requests/pullRequestStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ const STATES = {
text: "text-danger-foreground",
background: "bg-danger",
},
queued: {
label: "In merge queue",
icon: PullRequestGlyph.merged,
tint: "accent-adaptive-amber-700-300",
text: "text-adaptive-amber-700-300",
background: "bg-adaptive-amber-500-a12-a16",
},
draft: {
label: "Draft",
icon: PullRequestGlyph.draft,
Expand All @@ -43,8 +50,13 @@ const STATES = {
{ label: string; icon: AppSymbolName; tint: string; text: string; background: string }
>;

export function pullRequestStatus(pr: Pick<PullRequestListEntry, "state" | "isDraft">) {
return STATES[pr.isDraft && pr.state === "open" ? "draft" : pr.state];
export function pullRequestStatus(
pr: Pick<PullRequestListEntry, "state" | "isDraft"> & {
readonly isInMergeQueue?: boolean | undefined;
},
) {
if (pr.state !== "open") return STATES[pr.state];
return STATES[pr.isDraft ? "draft" : pr.isInMergeQueue === true ? "queued" : "open"];
}

export function pullRequestConflict(pr: {
Expand Down
18 changes: 12 additions & 6 deletions apps/mobile/src/features/threads/thread-list-v2-items.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1293,17 +1293,23 @@ export const ThreadListV2Row = memo(function ThreadListV2Row(props: {
? PullRequestGlyph.stack
: pr.state === null
? PullRequestGlyph.pullRequest
: pullRequestStatus({ state: pr.state, isDraft: pr.isDraft }).icon
: pullRequestStatus({
state: pr.state,
isDraft: pr.isDraft,
isInMergeQueue: pr.isInMergeQueue,
}).icon
}
size={12}
tintColorClassName={
pr.state === null || pr.isDraft
? rowAppearance.mutedIconTintClassName
: pr.state === "open"
? "accent-adaptive-emerald-600-400"
: pr.state === "closed"
? "accent-adaptive-rose-600-400"
: "accent-adaptive-violet-600-400"
: pr.isInMergeQueue
? "accent-adaptive-amber-700-300"
: pr.state === "open"
? "accent-adaptive-emerald-600-400"
: pr.state === "closed"
? "accent-adaptive-rose-600-400"
: "accent-adaptive-violet-600-400"
}
/>
<Text
Expand Down
25 changes: 19 additions & 6 deletions apps/mobile/src/state/thread-pr-presentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface ThreadPrPresentation {
readonly kind: "pull-request" | "stack";
readonly others: number;
readonly isDraft: boolean;
readonly isInMergeQueue: boolean;
readonly updatedAt: string | null;
readonly url: string;
readonly changeRequest: ChangeRequestTerminology;
Expand All @@ -31,25 +32,32 @@ const PR_STATE_TEXT_CLASS: Record<ThreadPr["state"], string> = {
merged: "text-adaptive-violet-600-400",
closed: "text-foreground-muted",
};
const MERGE_QUEUE_TEXT_CLASS = "text-adaptive-amber-700-300";

export function presentThreadPr(
pr: ThreadPr,
provider: VcsStatusResult["sourceControlProvider"] | null | undefined,
): ThreadPrPresentation {
const { changeRequest } = sourceControlClients.get(provider?.kind);
const isDraft = pr.state === "open" && pr.isDraft === true;
const isInMergeQueue = pr.state === "open" && !isDraft && pr.isInMergeQueue === true;
return {
kind: "pull-request",
others: 0,
number: pr.number,
state: pr.state,
isDraft,
isInMergeQueue,
updatedAt: pr.updatedAt ?? null,
url: pr.url,
changeRequest,
label: String(pr.number),
accessibilityLabel: `#${pr.number} ${changeRequest.singular} ${isDraft ? "draft" : pr.state}`,
textClassName: isDraft ? "text-foreground-muted" : PR_STATE_TEXT_CLASS[pr.state],
accessibilityLabel: `#${pr.number} ${changeRequest.singular} ${isDraft ? "draft" : isInMergeQueue ? "in merge queue" : pr.state}`,
textClassName: isDraft
? "text-foreground-muted"
: isInMergeQueue
? MERGE_QUEUE_TEXT_CLASS
: PR_STATE_TEXT_CLASS[pr.state],
};
}

Expand All @@ -72,6 +80,8 @@ export function presentThreadLinkedPullRequests(
const isDraft = isMultiple
? badge.state === "draft"
: snapshot?.isDraft === true && state === "open";
const isInMergeQueue =
!isMultiple && !isDraft && state === "open" && snapshot?.isInMergeQueue === true;
const label =
badge.kind === "stack"
? String(badge.layers)
Expand All @@ -84,6 +94,7 @@ export function presentThreadLinkedPullRequests(
number: link.number,
state,
isDraft,
isInMergeQueue,
updatedAt: snapshot?.updatedAt ?? null,
url: link.url,
changeRequest,
Expand All @@ -93,13 +104,15 @@ export function presentThreadLinkedPullRequests(
? `${badge.layers} ${changeRequest.singular}s in stack, ${isDraft ? "draft" : (state ?? "status pending")}`
: linkedCount !== null
? `${linkedCount} linked ${changeRequest.plural}, overall ${badge.state}`
: `#${link.number} ${changeRequest.singular} ${state === null ? "status pending" : isDraft ? "draft" : state}`,
: `#${link.number} ${changeRequest.singular} ${state === null ? "status pending" : isDraft ? "draft" : isInMergeQueue ? "in merge queue" : state}`,
textClassName:
state === null || isDraft
? "text-foreground-muted"
: isMultiple && state === "closed"
? "text-adaptive-rose-600-400"
: PR_STATE_TEXT_CLASS[state],
: isInMergeQueue
? MERGE_QUEUE_TEXT_CLASS
: isMultiple && state === "closed"
? "text-adaptive-rose-600-400"
: PR_STATE_TEXT_CLASS[state],
};
}

Expand Down
102 changes: 52 additions & 50 deletions apps/server/src/git/GitManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1021,60 +1021,62 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
}),
);

it.effect("branch PR lookup notifies linked threads when draft status changes", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "feature/draft-status"]);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
yield* runGit(repoDir, ["push", "-u", "origin", "feature/draft-status"]);
const drafts = [false, true, true, false, false, true];
const pullRequest: FakeChangeRequest = {
number: 1,
title: "Draft status",
url: "https://github.com/pingdotgg/codething-mvp/pull/1",
baseRefName: "main",
headRefName: "feature/draft-status",
state: "open",
};
const { manager } = yield* makeManager({
providerScenario: {
prListSequence: [
...drafts.map((isDraft) => [{ ...pullRequest, isDraft }]),
[
{
...pullRequest,
number: 2,
url: "https://github.com/pingdotgg/codething-mvp/pull/2",
state: "merged",
},
it.effect.each(["isDraft", "isInMergeQueue"] as const)(
"branch PR lookup notifies linked threads when %s changes",
(field) =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "feature/draft-status"]);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
yield* runGit(repoDir, ["push", "-u", "origin", "feature/draft-status"]);
const readings = [false, true, true, false, false, true];
const pullRequest: FakeChangeRequest = {
number: 1,
title: "Draft status",
url: "https://github.com/pingdotgg/codething-mvp/pull/1",
baseRefName: "main",
headRefName: "feature/draft-status",
state: "open",
};
const { manager } = yield* makeManager({
providerScenario: {
prListSequence: [
...readings.map((reading) => [{ ...pullRequest, [field]: reading }]),
[
{
...pullRequest,
number: 2,
url: "https://github.com/pingdotgg/codething-mvp/pull/2",
state: "merged",
},
],
],
],
},
});
const changes = yield* manager.subscribePullRequestStateChanges;
for (const isDraft of drafts) {
const pr = yield* manager.branchPullRequest(
},
});
const changes = yield* manager.subscribePullRequestStateChanges;
for (const reading of readings) {
const pr = yield* manager.branchPullRequest(
{ cwd: repoDir, branch: "feature/draft-status" },
{ refresh: true },
);
expect(pr?.[field] === true).toBe(reading);
}
// A different PR ends collection without waiting on a background stream consumer.
yield* manager.branchPullRequest(
{ cwd: repoDir, branch: "feature/draft-status" },
{ refresh: true },
);
expect(pr?.isDraft === true).toBe(isDraft);
}
// A different PR ends collection without waiting on a background stream consumer.
yield* manager.branchPullRequest(
{ cwd: repoDir, branch: "feature/draft-status" },
{ refresh: true },
);
const events = yield* Stream.runCollect(
changes.pipe(Stream.takeUntil((event) => event.number === 2, { excludeLast: true })),
);
expect(events).toEqual([
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
]);
}),
const events = yield* Stream.runCollect(
changes.pipe(Stream.takeUntil((event) => event.number === 2, { excludeLast: true })),
);
expect(events).toEqual([
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
{ host: "github.com", repository: "pingdotgg/codething-mvp", number: 1 },
]);
}),
);

it.effect("branch PR lookup uses a saved tracked branch without changing checkout", () =>
Expand Down
Loading
Loading