diff --git a/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx b/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx index 5557dc38..627528bd 100644 --- a/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx +++ b/apps/web/src/components/pull-requests/PullRequestDetailPanel.browser.tsx @@ -261,6 +261,9 @@ describe("PullRequestDetailPanel", () => { // The merge dialog remembers this per computer, so one test's tick would // otherwise be the next one's default. window.localStorage.removeItem(`threadlines:pull-requests:delete-branch:v1:${ENVIRONMENT_ID}`); + window.localStorage.removeItem( + "threadlines:pull-requests:merge-method:v1:github:threadlines/threadlines", + ); }); it("renders the header, the checks that need attention, and the conversation", async () => { @@ -693,4 +696,34 @@ describe("PullRequestDetailPanel", () => { await rendered.cleanup(); }); + + it("wears the method last run on the repository and runs it next time", async () => { + const viewer = { canWrite: true, canReview: false, canManage: true }; + const first = await renderPanel({ detail: { viewer } }); + + // The repository lists squash first, so that is the button's own until a + // choice is made. The menu spells the methods out in full. + await expect.element(page.getByTestId("pull-request-merge")).toHaveTextContent("Squash"); + await userEvent.click(page.getByRole("button", { name: "Choose a merge method" })); + await userEvent.click(page.getByRole("menuitem", { name: "Create a merge commit" })); + const dialog = page.getByRole("alertdialog"); + await expect.element(dialog.getByText("Create a merge commit.")).toBeVisible(); + await userEvent.click(dialog.getByRole("button", { name: "Merge" })); + await vi.waitFor(() => { + expect(first.runAction).toHaveBeenCalledWith({ + ...REFERENCE, + action: "merge", + mergeMethod: "merge", + }); + }); + await first.cleanup(); + + const second = await renderPanel({ detail: { viewer } }); + await expect.element(page.getByTestId("pull-request-merge")).toHaveTextContent("Merge"); + await userEvent.click(page.getByTestId("pull-request-merge")); + await expect + .element(page.getByRole("alertdialog").getByText("Create a merge commit.")) + .toBeVisible(); + await second.cleanup(); + }); }); diff --git a/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx b/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx index e0790a86..d78e8b9d 100644 --- a/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx @@ -12,6 +12,7 @@ import type { ScopedThreadRef, SourceControlProviderKind, } from "@threadlines/contracts"; +import { PullRequestMergeMethod as PullRequestMergeMethodSchema } from "@threadlines/contracts"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; import * as Schema from "effect/Schema"; @@ -94,6 +95,7 @@ import { } from "./pullRequestPresentation"; import { PULL_REQUEST_MERGE_METHOD_LABELS, + PULL_REQUEST_MERGE_METHOD_WORDS, appendHandoffToDraft, buildReviewCommentHandoff, formatPullRequestBaseFreshness, @@ -1061,6 +1063,11 @@ type PullRequestConfirmation = /** Remembered per computer: whoever deletes merged branches always does. */ const DELETE_BRANCH_STORAGE_PREFIX = "threadlines:pull-requests:delete-branch:v1"; +/** Remembered per repository: the merge method last run on it becomes the + * Merge button's own, as the host's site does. */ +const MERGE_METHOD_STORAGE_PREFIX = "threadlines:pull-requests:merge-method:v1"; +const REMEMBERED_MERGE_METHOD_SCHEMA = Schema.NullOr(PullRequestMergeMethodSchema); + /** The three pieces the header hangs in three different places. */ interface PullRequestActionsView { /** The buttons, for the right of the header's first row. */ @@ -1104,6 +1111,11 @@ function usePullRequestActions({ false, Schema.Boolean, ); + const [rememberedMergeMethod, setRememberedMergeMethod] = useLocalStorage( + `${MERGE_METHOD_STORAGE_PREFIX}:${detail.provider}:${detail.repository}`, + null, + REMEMBERED_MERGE_METHOD_SCHEMA, + ); const isRunning = mutation.isPending; const runningAction = isRunning ? (mutation.variables?.action ?? null) : null; @@ -1144,7 +1156,7 @@ function usePullRequestActions({ const updateMethods = detail.capabilities.updateMethods; const mergeBlock = resolvePullRequestMergeBlock(detail); const mergeDisabled = isRunning || mergeBlock !== null; - const defaultMergeMethod = resolveDefaultMergeMethod(detail.mergeMethods); + const defaultMergeMethod = resolveDefaultMergeMethod(detail.mergeMethods, rememberedMergeMethod); const canUpdateBranch = canWrite && isOpen && allows("update-branch"); const isBehind = detail.baseComparison === "behind"; @@ -1180,7 +1192,9 @@ function usePullRequestActions({ data-testid="pull-request-merge" onClick={() => setConfirming({ action: "merge", mergeMethod: defaultMergeMethod })} > - {runningAction === "merge" ? RUNNING_ACTION_WORDS.merge : "Merge"} + {runningAction === "merge" + ? RUNNING_ACTION_WORDS.merge + : PULL_REQUEST_MERGE_METHOD_WORDS[defaultMergeMethod]} {detail.mergeMethods.length > 1 ? ( @@ -1329,7 +1343,7 @@ function usePullRequestActions({ setConfirming({ action: "merge", mergeMethod: defaultMergeMethod }) } > - Merge + {PULL_REQUEST_MERGE_METHOD_LABELS[defaultMergeMethod]} ) : null} {handoffs ? ( @@ -1456,6 +1470,7 @@ function usePullRequestActions({ onClick={() => { setConfirming(null); if (confirming.action === "merge") { + setRememberedMergeMethod(confirming.mergeMethod); run("merge", { mergeMethod: confirming.mergeMethod, ...(deleteBranch ? { deleteBranch: true } : {}), diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts index 7a19ad01..d6df2f26 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts @@ -37,6 +37,7 @@ import { pullRequestFiltersFromSearch, pullRequestFiltersToSearch, pullRequestLabelColor, + resolveDefaultMergeMethod, resolveNeedsYouReason, resolvePullRequestMergeBlock, resolvePullRequestReviewPosition, @@ -1221,3 +1222,15 @@ describe("applyPendingPullRequestReactions", () => { ).toEqual([{ content: "thumbs-up", count: 3, viewerReacted: true }]); }); }); + +describe("resolveDefaultMergeMethod", () => { + it("runs the method last used on the repository while it is still allowed", () => { + expect(resolveDefaultMergeMethod(["merge", "squash", "rebase"], "squash")).toBe("squash"); + }); + + it("falls back to the repository's first method when nothing is remembered or it is off", () => { + expect(resolveDefaultMergeMethod(["squash", "rebase"], "merge")).toBe("squash"); + expect(resolveDefaultMergeMethod(["squash", "rebase"])).toBe("squash"); + expect(resolveDefaultMergeMethod([], "squash")).toBe("merge"); + }); +}); diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.ts b/apps/web/src/components/pull-requests/pullRequests.logic.ts index 00c07ab4..a576758f 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.ts @@ -1328,15 +1328,28 @@ export const PULL_REQUEST_MERGE_METHOD_LABELS: Readonly> = { + merge: "Merge", + squash: "Squash", + rebase: "Rebase", +}; + /** - * The method the Merge button runs without being asked. The repository lists - * what it allows in its own order and the first one is its default; a host - * that reports nothing still gets a plain merge offered, and refuses it itself - * if it really is off. + * The method the Merge button runs without being asked: the one last used on + * this repository while the repository still allows it, the way the host's own + * site remembers a choice. Before any choice, the repository lists what it + * allows in its own order and the first one is its default; a host that reports + * nothing still gets a plain merge offered, and refuses it itself if it really + * is off. */ export function resolveDefaultMergeMethod( mergeMethods: readonly PullRequestMergeMethod[], + remembered: PullRequestMergeMethod | null = null, ): PullRequestMergeMethod { + if (remembered !== null && mergeMethods.includes(remembered)) { + return remembered; + } return mergeMethods[0] ?? "merge"; }