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
3 changes: 2 additions & 1 deletion apps/mobile/src/components/ComposerContextSheet.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { pullRequestStatus } from "../features/pull-requests/pullRequestStatus";
import { Button } from "./ui/Button";
import * as Dialog from "./ui/Dialog";
import { useOpenPullRequest } from "../native/useOpenPullRequest";
Expand Down Expand Up @@ -247,7 +248,7 @@ export function ComposerContextSheet(props: {
<>
{record.pullRequest ? (
<ContextField
label={`#${record.pullRequest.number} · ${record.pullRequest.isDraft ? "draft" : record.pullRequest.state}`}
label={`#${record.pullRequest.number} · ${pullRequestStatus(record.pullRequest).label}`}
value={`${record.pullRequest.title}\n${record.pullRequest.headBranch} → ${record.pullRequest.baseBranch}`}
/>
) : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export function PullRequestStackCard(props: {
const status = pullRequestStatus({
state: layer.state,
isDraft: layer.isDraft === true,
inMergeQueue: layer.inMergeQueue === true,
});
return (
<View key={layer.number} className={selected ? "bg-thread-selected" : undefined}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ export function PullRequestChecksRow(props: RowProps) {
return (
<Animated.View layout={PR_ROW_LAYOUT}>
<PullRequestCardRow
title={checks.title}
title="PR checks"
detail={checks.title}
icon={checks.icon}
tint={checks.tint}
value={checks.count}
compact
accessibilityLabel={`Checks, ${checks.label}`}
accessibilityLabel={`PR checks, ${checks.label}`}
expanded={expanded}
onPress={() => setExpanded(!expanded)}
/>
Expand Down Expand Up @@ -169,7 +170,7 @@ export function PullRequestChecksRow(props: RowProps) {
<Text className="px-4 py-3 text-xs text-foreground-muted">No checks reported.</Text>
) : null}
<PullRequestCardRow
title="Open checks on provider"
title="Open PR checks on provider"
icon="arrow.up.right"
onPress={() =>
void Linking.openURL(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ describe("mobile pull request actions", () => {
expect(mergeUnavailableReason(pr, true, GITHUB)).toContain("conflicts");
});

it("blocks manual and auto merge while queued", () => {
const pr = detail({ inMergeQueue: true });
expect(canPerformPullRequestAction(pr, "merge")).toBe(false);
expect(canPerformPullRequestAction(pr, "draft")).toBe(false);
expect(canPerformPullRequestAction(pr, "enable-auto-merge")).toBe(false);
expect(mergeUnavailableReason(pr, true, GITHUB)).toContain("queued to merge");
});

it("names the change request with its host's term", () => {
expect(pullRequestActionLabel("close", GITHUB)).toBe("Close pull request");
expect(pullRequestActionLabel("close", GITLAB)).toBe("Close merge request");
Expand Down
24 changes: 20 additions & 4 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"
| "inMergeQueue"
| "mergeability"
| "autoMergeEnabled"
| "workflowApprovalsRequired"
> & {
readonly capabilities: Pick<
PullRequestDetail["capabilities"],
Expand Down Expand Up @@ -105,11 +110,21 @@ export function canPerformPullRequestAction(pr: ActionDetail, action: PullReques
case "ready":
return pr.isDraft;
case "draft":
return !pr.isDraft;
return !pr.isDraft && !pr.inMergeQueue;
case "merge":
return !pr.isDraft && pr.mergeability !== "conflicting" && allowedMergeMethods(pr).length > 0;
return (
!pr.isDraft &&
!pr.inMergeQueue &&
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.inMergeQueue &&
pr.autoMergeEnabled === false &&
allowedMergeMethods(pr).length > 0
);
case "disable-auto-merge":
return pr.autoMergeEnabled === true;
default:
Expand All @@ -130,6 +145,7 @@ 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.inMergeQueue) return "This pull request is queued to merge.";
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
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ describe("recycled PR list rows", () => {
}),
).toBe(false);
});

it("recycles a row when its merge queue status changes", () => {
const original = scopedEntry();
expect(pullRequestListRowsAreEqual(original, { ...original, inMergeQueue: true })).toBe(false);
});
});

describe("mobile PR list pagination", () => {
Expand Down
1 change: 1 addition & 0 deletions apps/mobile/src/features/pull-requests/pullRequestList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export function pullRequestListRowsAreEqual(
previous.url === next.url &&
previous.state === next.state &&
previous.isDraft === next.isDraft &&
previous.inMergeQueue === next.inMergeQueue &&
previous.mergeability === next.mergeability &&
previous.baseBranch === next.baseBranch &&
previous.updatedAt === next.updatedAt &&
Expand Down
51 changes: 48 additions & 3 deletions apps/mobile/src/features/pull-requests/pullRequestStack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { describe, expect, it } from "vite-plus/test";
import {
pullRequestLayerParams,
pullRequestStackMerge,
pullRequestStackView,
savedPullRequestStack,
singlePullRequestMergeAllowed,
} from "./pullRequestStack";
Expand Down Expand Up @@ -101,6 +102,20 @@ describe("mobile stack merge scope", () => {
pullRequestStackMerge(stack, 99, sourceControlClients.get("gitlab").changeRequest).reason,
).toBe("This merge request is no longer in the stack. Refresh it.");
});
it("blocks merging any queued layer in the selected stack prefix", () => {
const queued = {
...stack,
layers: stack.layers.map((layer) =>
layer.number === 2 ? { ...layer, inMergeQueue: true } : layer,
),
};
expect(pullRequestStackMerge(queued, 3, GITHUB).reason).toContain("merge queue");
const exited = {
...queued,
layers: queued.layers.map((layer) => ({ ...layer, inMergeQueue: false })),
};
expect(pullRequestStackMerge(exited, 3, GITHUB).reason).toBeNull();
});
it("waits for successful discovery before allowing a single PR merge on modern servers", () => {
expect(
singlePullRequestMergeAllowed({
Expand Down Expand Up @@ -147,13 +162,43 @@ describe("mobile stack layer navigation", () => {

describe("mobile saved stack navigation", () => {
it("keeps all saved layers and enriches titles without carrying action revisions", () => {
const saved = savedPullRequestStack([link], reference);
const queuedLink = {
...link,
snapshot: { ...link.snapshot!, inMergeQueue: true },
};
const saved = savedPullRequestStack([queuedLink], reference);
expect(saved?.layers).toHaveLength(3);
expect(saved?.layers[1]).toMatchObject({ number: 2, title: "Middle layer", isDraft: false });
expect(saved?.layers[1]).toMatchObject({
number: 2,
title: "Middle layer",
isDraft: false,
inMergeQueue: true,
});
expect(saved?.layers.every((layer) => layer.headSha === undefined)).toBe(true);
expect(pullRequestStackMerge(saved!, 2, GITHUB).reason).toContain("latest revisions");
expect(pullRequestStackMerge(saved!, 2, GITHUB).reason).toContain("merge queue");
expect(savedPullRequestStack([link], { ...reference, number: 3 })?.number).toBe(10);
});
it("restores saved queue status only when fresh stack data omits it", () => {
const saved = savedPullRequestStack(
[{ ...link, snapshot: { ...link.snapshot!, inMergeQueue: true } }],
reference,
)!;
const fresh = {
...stack,
layers: stack.layers.map(({ inMergeQueue: _queue, ...layer }) => layer),
};
expect(pullRequestStackView(stack, stack)).toBe(stack);
expect(pullRequestStackView(fresh, saved)?.layers[1]?.inMergeQueue).toBe(true);
expect(
pullRequestStackView(
{
...fresh,
layers: fresh.layers.map((layer) => ({ ...layer, inMergeQueue: false })),
},
saved,
)?.layers[1]?.inMergeQueue,
).toBe(false);
});
it("never borrows membership from another host or repository", () => {
expect(savedPullRequestStack([link], { ...reference, host: "enterprise.example" })).toBeNull();
expect(savedPullRequestStack([link], { ...reference, repository: "other/web" })).toBeNull();
Expand Down
43 changes: 39 additions & 4 deletions apps/mobile/src/features/pull-requests/pullRequestStack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ export function pullRequestStackMerge(
? "Reopen the closed layers before merging this stack."
: layers.some((layer) => layer.isDraft)
? "Mark every layer being merged ready for review."
: heads.length !== layers.length
? "Refresh the stack to load its latest revisions before merging."
: null;
: layers.some((layer) => layer.inMergeQueue)
? "Wait for queued layers to leave the merge queue before merging this stack."
: heads.length !== layers.length
? "Refresh the stack to load its latest revisions before merging."
: null;
return { position: position + 1, layers, heads, reason };
}

Expand Down Expand Up @@ -61,7 +63,13 @@ export function savedPullRequestStack(
number: layer.number,
headBranch: layer.headBranch,
state: layer.state,
...(snapshot ? { title: snapshot.title, isDraft: snapshot.isDraft } : {}),
...(snapshot
? {
title: snapshot.title,
isDraft: snapshot.isDraft,
inMergeQueue: snapshot.inMergeQueue,
}
: {}),
};
}),
};
Expand All @@ -76,6 +84,33 @@ export function pullRequestLayerParams<Params extends { readonly initialSummary?
return { ...layer, number };
}

export function pullRequestStackView(
data: PullRequestStack | null,
saved: PullRequestStack | null,
) {
if (
data === null ||
saved === null ||
!data.layers.some(
(layer) =>
layer.inMergeQueue === undefined &&
saved.layers.some(
(candidate) => candidate.number === layer.number && candidate.inMergeQueue !== undefined,
),
)
)
return data;
return {
...data,
layers: data.layers.map((layer) => {
const savedLayer = saved.layers.find((candidate) => candidate.number === layer.number);
return layer.inMergeQueue !== undefined || savedLayer?.inMergeQueue === undefined
? layer
: { ...layer, inMergeQueue: savedLayer.inMergeQueue };
}),
};
}

export function singlePullRequestMergeAllowed(input: {
readonly supportsStackActions: boolean;
readonly hasStack: boolean;
Expand Down
21 changes: 21 additions & 0 deletions apps/mobile/src/features/pull-requests/pullRequestStatus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
normalizedReviewState,
pullRequestChecksSummary,
pullRequestReviewSummary,
pullRequestStatus,
} from "./pullRequestStatus";

const author = { login: "julius", name: null, avatarUrl: null };
Expand Down Expand Up @@ -99,3 +100,23 @@ describe("mobile overview check status", () => {
).toMatchObject({ title: "1 check failing", count: "1 of 3" });
});
});

describe("mobile pull request status", () => {
it("shows queued for open PRs before draft, and keeps merged and closed terminal", () => {
expect(pullRequestStatus({ state: "open", isDraft: false, inMergeQueue: true }).label).toBe(
"Queued",
);
expect(pullRequestStatus({ state: "open", isDraft: true, inMergeQueue: true }).label).toBe(
"Queued",
);
expect(pullRequestStatus({ state: "open", isDraft: true, inMergeQueue: false }).label).toBe(
"Draft",
);
expect(pullRequestStatus({ state: "merged", isDraft: false, inMergeQueue: true }).label).toBe(
"Merged",
);
expect(pullRequestStatus({ state: "closed", isDraft: false, inMergeQueue: true }).label).toBe(
"Closed",
);
});
});
15 changes: 13 additions & 2 deletions apps/mobile/src/features/pull-requests/pullRequestStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ const STATES = {
text: "text-adaptive-emerald-600-400",
background: "bg-adaptive-emerald-500-a12-a16",
},
queued: {
label: "Queued",
icon: "clock",
tint: "accent-warning-foreground",
text: "text-warning-foreground",
background: "bg-warning",
},
merged: {
label: "Merged",
icon: PullRequestGlyph.merged,
Expand All @@ -43,8 +50,12 @@ 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" | "inMergeQueue">,
) {
if (pr.state !== "open") return STATES[pr.state];
if (pr.inMergeQueue) return STATES.queued;
return STATES[pr.isDraft ? "draft" : "open"];
}

export function pullRequestConflict(pr: {
Expand Down
5 changes: 3 additions & 2 deletions apps/mobile/src/features/pull-requests/usePullRequestStack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useMemo } from "react";
import { useEnvironmentServerConfig, useNavigationThreadShells } from "../../state/entities";
import { pullRequestStackAtom } from "../../state/pull-requests";
import { useEnvironmentQuery } from "../../state/query";
import { savedPullRequestStack } from "./pullRequestStack";
import { pullRequestStackView, savedPullRequestStack } from "./pullRequestStack";
import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff";

export function usePullRequestStack(params: PullRequestParams, pr: PullRequestDetail | null) {
Expand Down Expand Up @@ -37,7 +37,8 @@ export function usePullRequestStack(params: PullRequestParams, pr: PullRequestDe
const query = useEnvironmentQuery(
supported ? pullRequestStackAtom({ ...target, input: reference }) : null,
);
const stack = query.isSuccess ? query.data : (query.data ?? saved);
const data = query.isSuccess ? query.data : (query.data ?? saved);
const stack = useMemo(() => pullRequestStackView(data, saved), [data, saved]);
return {
...query,
stack,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export function usePullRequestThreads(
baseBranch: pr.baseBranch,
state: pr.state,
isDraft: pr.isDraft,
inMergeQueue: pr.inMergeQueue,
},
uuidv4(),
filePath,
Expand Down
8 changes: 7 additions & 1 deletion apps/mobile/src/features/threads/git/GitOverviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
useLinkedPullRequestLines,
} from "../LinkedPullRequestList";
import { useLinkedPullRequestActions } from "../useLinkedPullRequestActions";
import { pullRequestStatus } from "../../pull-requests/pullRequestStatus";

const HEADER_SCROLL_EDGE_EFFECTS = nativeHeaderScrollEdgeEffects(Platform.OS, Platform.Version);

Expand Down Expand Up @@ -218,7 +219,12 @@ function GitOverviewSheetContent(props: GitOverviewSheetProps) {
return `${ahead} commit${ahead === 1 ? "" : "s"} ahead`;
}
if (item.kind === "open_pr" && status.pr?.number != null) {
return `${shortLabel} #${status.pr.number} ${status.pr.state ?? "open"}`;
const prStatus = pullRequestStatus({
state: status.pr.state ?? "open",
isDraft: status.pr.isDraft === true,
inMergeQueue: status.pr.inMergeQueue,
});
return `${shortLabel} #${status.pr.number} ${prStatus.label.toLowerCase()}`;
}
return undefined;
};
Expand Down
Loading
Loading