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
98 changes: 98 additions & 0 deletions src/backend/pullrequests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
PullRequestError,
requestChangesOnPullRequest,
unapprovePullRequest,
updatePullRequest,
withdrawRequestChanges,
} from "./index.ts";

Expand Down Expand Up @@ -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<string, any> | null = null;
server.use(
http.put(PR_DETAIL_PATH(42), async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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<string, any> | null = null;
server.use(
http.put(PR_DETAIL_PATH(42), async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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<string, any> | null = null;
server.use(
http.put(PR_DETAIL_PATH(42), async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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`;
Expand Down
52 changes: 52 additions & 0 deletions src/backend/pullrequests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PullRequestDetail> {
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
Expand Down
61 changes: 61 additions & 0 deletions src/commands/pullrequest/edit.test.ts
Original file line number Diff line number Diff line change
@@ -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",
});
});
});
164 changes: 164 additions & 0 deletions src/commands/pullrequest/edit.ts
Original file line number Diff line number Diff line change
@@ -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<PullRequestState> = 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<void> {
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 };
}
Loading
Loading