diff --git a/src/backend/pullrequests/index.test.ts b/src/backend/pullrequests/index.test.ts index e30b5f4..2ff5007 100644 --- a/src/backend/pullrequests/index.test.ts +++ b/src/backend/pullrequests/index.test.ts @@ -15,6 +15,7 @@ import { PullRequestError, requestChangesOnPullRequest, unapprovePullRequest, + updatePullRequest, withdrawRequestChanges, } from "./index.ts"; @@ -921,6 +922,103 @@ describe("review action endpoints (approve / unapprove / request-changes / unreq }); }); +describe("updatePullRequest", () => { + test("PUTs title only and returns the updated detail", async () => { + let seenBody: Record | null = null; + server.use( + http.put(PR_DETAIL_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 42, title: "new title" })); + }), + ); + + const result = await updatePullRequest(creds, ref, 42, { + title: "new title", + }); + + expect(seenBody!).toEqual({ type: "pullrequest", title: "new title" }); + expect(seenBody!).not.toHaveProperty("description"); + expect(result.id).toBe(42); + expect(result.title).toBe("new title"); + }); + + test("PUTs description only", async () => { + let seenBody: Record | null = null; + server.use( + http.put(PR_DETAIL_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json( + makePrDetail({ id: 42, summary: { raw: "new description" } }), + ); + }), + ); + + await updatePullRequest(creds, ref, 42, { + description: "new description", + }); + + expect(seenBody!).toEqual({ + type: "pullrequest", + description: "new description", + }); + expect(seenBody!).not.toHaveProperty("title"); + }); + + test("PUTs title and description together when both provided", async () => { + let seenBody: Record | null = null; + server.use( + http.put(PR_DETAIL_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 42 })); + }), + ); + + await updatePullRequest(creds, ref, 42, { + title: "t", + description: "d", + }); + + expect(seenBody!).toEqual({ + type: "pullrequest", + title: "t", + description: "d", + }); + }); + + test("surfaces 4xx cleanly (e.g. editing a closed PR)", async () => { + server.use( + http.put(PR_DETAIL_PATH(42), () => + HttpResponse.json( + { type: "error", error: { message: "PR is not open" } }, + { status: 400 }, + ), + ), + ); + + const err = await updatePullRequest(creds, ref, 42, { + title: "x", + }).catch((e) => e); + + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(400); + }); + + test("surfaces 404 when the PR does not exist", async () => { + server.use( + http.put(PR_DETAIL_PATH(99), () => + HttpResponse.json({ type: "error" }, { status: 404 }), + ), + ); + + const err = await updatePullRequest(creds, ref, 99, { + title: "x", + }).catch((e) => e); + + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(404); + }); +}); + describe("getPullRequestDiff", () => { const DIFF_PATH = (id: number) => `${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/diff`; diff --git a/src/backend/pullrequests/index.ts b/src/backend/pullrequests/index.ts index 0d7236f..23e1a01 100644 --- a/src/backend/pullrequests/index.ts +++ b/src/backend/pullrequests/index.ts @@ -265,6 +265,58 @@ export async function createPullRequest( return toPullRequestDetail(data as RawPullRequest); } +export type UpdatePullRequestInput = { + title?: string; + description?: string; +}; + +/** + * PUTs a partial update to a pull request. Bitbucket's spec types the body + * as the full `pullrequest` schema and the doc comment only formally + * describes "branches or description" updates, but the API accepts partial + * bodies in practice for common mutable fields (title, description) — see + * docs/bb-notes.md → "Updating a PR". + * + * The caller is responsible for making sure the PR is in a mutable state + * (i.e. OPEN). Non-mutable states surface here as a `PullRequestError`. + */ +export async function updatePullRequest( + credentials: Credentials, + ref: { workspace: string; slug: string }, + pullRequestId: number, + input: UpdatePullRequestInput, +): Promise { + const client = createBitbucketClient(credentials); + const { data, response } = await client.PUT( + "/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}", + { + params: { + path: { + workspace: ref.workspace, + repo_slug: ref.slug, + pull_request_id: pullRequestId, + }, + }, + body: { + type: "pullrequest", + ...(input.title !== undefined ? { title: input.title } : {}), + ...(input.description !== undefined + ? { description: input.description } + : {}), + }, + }, + ); + + if (!response.ok || !data) { + throw new PullRequestError( + `Failed to update pull request #${pullRequestId}: HTTP ${response.status}.`, + response.status, + ); + } + + return toPullRequestDetail(data as RawPullRequest); +} + /** * Fetches a single pull request by id. Single typed call — the overlay * (BBC2-38) gives us typed path params and response shape, so we stay on diff --git a/src/commands/pullrequest/edit.test.ts b/src/commands/pullrequest/edit.test.ts new file mode 100644 index 0000000..114a81c --- /dev/null +++ b/src/commands/pullrequest/edit.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from "bun:test"; +import { parseMessage } from "./edit.ts"; + +describe("parseMessage", () => { + test("first line becomes the title; rest after blank line becomes the description", () => { + expect(parseMessage("Hello world\n\nA description here.\n")).toEqual({ + title: "Hello world", + description: "A description here.", + }); + }); + + test("handles multi-paragraph descriptions verbatim", () => { + expect(parseMessage("Title\n\nPara one.\n\nPara two.\n\n* list\n")).toEqual( + { + title: "Title", + description: "Para one.\n\nPara two.\n\n* list", + }, + ); + }); + + test("no description yields an empty string", () => { + expect(parseMessage("Just a title\n")).toEqual({ + title: "Just a title", + description: "", + }); + }); + + test("leading empty lines are skipped", () => { + expect(parseMessage("\n\nTitle\n\nBody\n")).toEqual({ + title: "Title", + description: "Body", + }); + }); + + test("trailing blank lines on the title are tolerated (multiple separators)", () => { + expect(parseMessage("Title\n\n\n\nBody\n")).toEqual({ + title: "Title", + description: "Body", + }); + }); + + test("CRLF line endings are handled", () => { + expect(parseMessage("Title\r\n\r\nBody\r\n")).toEqual({ + title: "Title", + description: "Body", + }); + }); + + test("empty input returns null (caller treats as abort)", () => { + expect(parseMessage("")).toBeNull(); + expect(parseMessage("\n\n\n")).toBeNull(); + expect(parseMessage(" \n\t\n")).toBeNull(); + }); + + test("title is trimmed", () => { + expect(parseMessage(" Spacey title \n\nbody\n")).toEqual({ + title: "Spacey title", + description: "body", + }); + }); +}); diff --git a/src/commands/pullrequest/edit.ts b/src/commands/pullrequest/edit.ts new file mode 100644 index 0000000..3123ad6 --- /dev/null +++ b/src/commands/pullrequest/edit.ts @@ -0,0 +1,164 @@ +import { + getPullRequest, + type PullRequestDetail, + PullRequestError, + type PullRequestState, + updatePullRequest, +} from "../../backend/pullrequests/index.ts"; +import { loadConfigOrExit } from "../../shared/config/index.ts"; +import { EditorError, openEditor } from "../../shared/editor/index.ts"; +import type { Renderer } from "../../shared/renderer/index.ts"; +import { + RepositoryResolutionError, + resolveRepository, +} from "../../shared/repository/index.ts"; +import { resolveCurrentPullRequestId } from "./current.ts"; + +export type PullRequestEditOptions = { + repository?: string; + title?: string; + description?: string; +}; + +const IMMUTABLE_STATES: ReadonlySet = new Set([ + "MERGED", + "DECLINED", + "SUPERSEDED", +]); + +/** + * Updates a PR's title and/or description. Without flags, falls through to + * $EDITOR pre-filled with the PR's current title (first line), a blank + * separator, and the current description. Classic git-style message layout. + * + * No-op guard: if the resolved new values match the existing ones, skip + * the PUT entirely. Saves a round trip and avoids a pointless "updated" + * confirmation when the editor was closed without changes. + */ +export async function runPullRequestEdit( + renderer: Renderer, + idArg: string | undefined, + options: PullRequestEditOptions, +): Promise { + const config = await loadConfigOrExit(renderer); + + try { + const ref = await resolveRepository({ override: options.repository }); + const id = await resolveCurrentPullRequestId(idArg, { + renderer, + config, + ref, + commandName: "edit", + }); + + const current = await getPullRequest(config, ref, id); + if (IMMUTABLE_STATES.has(current.state)) { + renderer.error( + `Pull request #${id} is ${current.state.toLowerCase()}; cannot edit.`, + ); + process.exit(1); + } + + const resolved = await resolveFields(renderer, current, options); + if (resolved === null) { + renderer.message(`No changes to pull request #${id}.`); + return; + } + + const updated = await updatePullRequest(config, ref, id, resolved); + + if (renderer.json) { + renderer.detail(updated, []); + } else { + renderer.message(`Updated pull request #${id}: ${updated.url}`); + } + } catch (err) { + if ( + err instanceof RepositoryResolutionError || + err instanceof PullRequestError || + err instanceof EditorError + ) { + renderer.error(err.message); + process.exit(1); + } + throw err; + } +} + +/** + * Resolves the fields to PUT. + * + * - One or both flags set → use them; only include fields that actually + * differ from the current PR (so e.g. `--title "same as before"` is a + * no-op and doesn't generate a PUT). + * - No flags → open editor pre-filled; parse back the same way. Aborts + * with a clear error on empty output. + * + * Returns `null` when there's nothing to send (either both flags matched + * current state, or the editor came back unchanged). + */ +async function resolveFields( + renderer: Renderer, + current: PullRequestDetail, + options: PullRequestEditOptions, +): Promise<{ title?: string; description?: string } | null> { + if (options.title !== undefined || options.description !== undefined) { + const patch: { title?: string; description?: string } = {}; + if (options.title !== undefined && options.title !== current.title) { + patch.title = options.title; + } + if ( + options.description !== undefined && + options.description !== current.description + ) { + patch.description = options.description; + } + return Object.keys(patch).length > 0 ? patch : null; + } + + // Editor fallback. + const initial = `${current.title}\n\n${current.description}`; + const raw = await openEditor(initial); + const parsed = parseMessage(raw); + if (parsed === null) { + renderer.error("Edit aborted: title is empty."); + process.exit(1); + } + + const patch: { title?: string; description?: string } = {}; + if (parsed.title !== current.title) patch.title = parsed.title; + if (parsed.description !== current.description) { + patch.description = parsed.description; + } + return Object.keys(patch).length > 0 ? patch : null; +} + +/** + * Parses a git-style message buffer: first non-empty line is the title, + * everything after the first blank line is the description. Returns null + * when the title ends up empty, which the caller treats as "abort". + * + * Preserves trailing content verbatim so multi-paragraph descriptions + * survive the round trip. + */ +export function parseMessage( + raw: string, +): { title: string; description: string } | null { + // Find the first non-empty line → that's the title. Everything after + // the first blank line following the title is the description. + const lines = raw.split(/\r?\n/); + let i = 0; + while (i < lines.length && lines[i]!.trim() === "") i++; + if (i >= lines.length) return null; + + const title = lines[i]!.trim(); + if (!title) return null; + + // Skip the title line and any immediately following blank separator + // lines so the description doesn't start with an empty line. + i++; + while (i < lines.length && lines[i]!.trim() === "") i++; + + const description = lines.slice(i).join("\n").trimEnd(); + return { title, description }; +} diff --git a/src/commands/pullrequest/index.ts b/src/commands/pullrequest/index.ts index eb4197f..9581a45 100644 --- a/src/commands/pullrequest/index.ts +++ b/src/commands/pullrequest/index.ts @@ -3,6 +3,7 @@ import { withRenderer } from "../../shared/renderer/commander.ts"; import { runPullRequestComment } from "./comment.ts"; import { runPullRequestCreate } from "./create.ts"; import { runPullRequestDiff } from "./diff.ts"; +import { runPullRequestEdit } from "./edit.ts"; import { runPullRequestList } from "./list.ts"; import { runPullRequestReview } from "./review.ts"; import { runPullRequestView } from "./view.ts"; @@ -86,6 +87,19 @@ export function registerPullRequestCommands(program: Command): void { ) .action(withRenderer(runPullRequestDiff)); + pr.command("edit") + .description( + "Update a pull request's title and/or description (defaults to the PR for the current branch)", + ) + .argument("[id]", "Pull request number") + .option( + "-R, --repository ", + "Override repository detection", + ) + .option("-t, --title ", "New title") + .option("-d, --description <description>", "New description") + .action(withRenderer(runPullRequestEdit)); + pr.command("review") .description( "Submit a review on a pull request (defaults to the PR for the current branch)",