feat(task): auto-fill task name from PR title when pasting a PR URL (#1027)
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import {
|
||||
useAutoFillTaskNameFromPR,
|
||||
useBranchAutoSelectEffect,
|
||||
useWorkflowAgentProfileEffect,
|
||||
} from "./task-create-dialog-effects";
|
||||
@@ -187,3 +188,132 @@ describe("useBranchAutoSelectEffect", () => {
|
||||
expect(fs.setGitHubBranch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
const PR_1_TITLE = "PR #1: first";
|
||||
|
||||
type AutoFillFake = Pick<DialogFormState, "taskName" | "setTaskName" | "setHasTitle">;
|
||||
function makeAutoFillFs(overrides: Partial<AutoFillFake> = {}): DialogFormState {
|
||||
return {
|
||||
taskName: "",
|
||||
setTaskName: vi.fn(),
|
||||
setHasTitle: vi.fn(),
|
||||
...overrides,
|
||||
} as unknown as DialogFormState;
|
||||
}
|
||||
|
||||
describe("useAutoFillTaskNameFromPR", () => {
|
||||
it("fills the task name with 'PR #N: <title>' when the title is empty", () => {
|
||||
const fs = makeAutoFillFs({ taskName: "" });
|
||||
const { result } = renderHook(() => useAutoFillTaskNameFromPR(fs));
|
||||
result.current(971, "feat/omp-acp-agent");
|
||||
expect(fs.setTaskName).toHaveBeenCalledWith("PR #971: feat/omp-acp-agent");
|
||||
expect(fs.setHasTitle).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it("fills when the title contains only whitespace", () => {
|
||||
// Regression guard: hasTitle treats whitespace-only as empty, so the
|
||||
// auto-fill check must too — otherwise pasting a PR URL after typing a
|
||||
// stray space would silently keep the spaces.
|
||||
const fs = makeAutoFillFs({ taskName: " " });
|
||||
const { result } = renderHook(() => useAutoFillTaskNameFromPR(fs));
|
||||
result.current(7, "fix");
|
||||
expect(fs.setTaskName).toHaveBeenCalledWith("PR #7: fix");
|
||||
});
|
||||
|
||||
it("does NOT overwrite a title the user typed themselves", () => {
|
||||
const fs = makeAutoFillFs({ taskName: "my custom title" });
|
||||
const { result } = renderHook(() => useAutoFillTaskNameFromPR(fs));
|
||||
result.current(42, "something");
|
||||
expect(fs.setTaskName).not.toHaveBeenCalled();
|
||||
expect(fs.setHasTitle).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("replaces a previously auto-filled title when a different PR URL is pasted", () => {
|
||||
// Re-paste flow: the user pastes PR #1 (autofills "PR #1: foo"), then
|
||||
// realizes wrong URL and pastes PR #2. Without the lastAutoFilled-ref
|
||||
// tracking, the second paste would see a non-empty title and bail.
|
||||
let currentName = "";
|
||||
const fs = makeAutoFillFs({
|
||||
taskName: currentName,
|
||||
setTaskName: vi.fn((v: string) => {
|
||||
currentName = v;
|
||||
}),
|
||||
});
|
||||
const { result, rerender } = renderHook(() => {
|
||||
// Re-read taskName from the closure on every render so the ref-mirror
|
||||
// effect sees the latest value.
|
||||
fs.taskName = currentName;
|
||||
return useAutoFillTaskNameFromPR(fs);
|
||||
});
|
||||
result.current(1, "first");
|
||||
expect(currentName).toBe(PR_1_TITLE);
|
||||
|
||||
rerender();
|
||||
result.current(2, "second");
|
||||
expect(currentName).toBe("PR #2: second");
|
||||
expect(fs.setTaskName).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("does NOT replace once the user edits the auto-filled title", () => {
|
||||
let currentName = "";
|
||||
const fs = makeAutoFillFs({
|
||||
taskName: currentName,
|
||||
setTaskName: vi.fn((v: string) => {
|
||||
currentName = v;
|
||||
}),
|
||||
});
|
||||
const { result, rerender } = renderHook(() => {
|
||||
fs.taskName = currentName;
|
||||
return useAutoFillTaskNameFromPR(fs);
|
||||
});
|
||||
result.current(1, "first");
|
||||
expect(currentName).toBe(PR_1_TITLE);
|
||||
|
||||
// User edits the title manually.
|
||||
currentName = "my edits";
|
||||
rerender();
|
||||
|
||||
result.current(2, "second");
|
||||
// Setter is still only called once (the initial auto-fill); the manual
|
||||
// edit is preserved.
|
||||
expect(fs.setTaskName).toHaveBeenCalledTimes(1);
|
||||
expect(currentName).toBe("my edits");
|
||||
});
|
||||
|
||||
it("clears the sentinel when taskName resets to empty, preventing a re-typed auto-fill from being overwritten", () => {
|
||||
// Regression guard for the mounted-across-open/close edge case:
|
||||
// after a dialog reset clears taskName, the user manually types the same
|
||||
// text that was previously auto-filled. A second PR paste must NOT
|
||||
// overwrite it because the sentinel was cleared on the empty transition.
|
||||
let currentName = "";
|
||||
const fs = makeAutoFillFs({
|
||||
taskName: currentName,
|
||||
setTaskName: vi.fn((v: string) => {
|
||||
currentName = v;
|
||||
}),
|
||||
});
|
||||
const { result, rerender } = renderHook(() => {
|
||||
fs.taskName = currentName;
|
||||
return useAutoFillTaskNameFromPR(fs);
|
||||
});
|
||||
result.current(1, "first");
|
||||
expect(currentName).toBe(PR_1_TITLE);
|
||||
|
||||
// Simulate React flushing the setTaskName state update back into the hook
|
||||
// so the ref-mirror effect sees taskName = PR_1_TITLE.
|
||||
rerender();
|
||||
|
||||
// Dialog resets taskName to empty (sentinel should clear).
|
||||
currentName = "";
|
||||
rerender();
|
||||
|
||||
// User re-types the exact previously auto-filled value.
|
||||
currentName = PR_1_TITLE;
|
||||
rerender();
|
||||
|
||||
// A second paste with a different PR should NOT overwrite.
|
||||
result.current(2, "second");
|
||||
expect(fs.setTaskName).toHaveBeenCalledTimes(1); // only the first auto-fill
|
||||
expect(currentName).toBe(PR_1_TITLE);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -408,6 +408,39 @@ function parseGitHubUrl(url: string): { owner: string; repo: string; prNumber?:
|
||||
return { owner: match[1], repo: match[2] };
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a callback that auto-fills the task name with `PR #N: <title>` when
|
||||
* a PR URL is pasted, leaving anything the user typed themselves alone. The
|
||||
* callback reads the latest taskName via a ref so the fetch effect doesn't
|
||||
* need to list taskName as a dep (which would re-fire the branches/PR fetch on
|
||||
* every keystroke in the title input).
|
||||
*
|
||||
* NOTE: Callers must omit the returned callback from `useEffect` dependency
|
||||
* arrays — including it causes the fetch to re-fire on every keystroke,
|
||||
* defeating the purpose of the ref. The call site uses
|
||||
* `eslint-disable react-hooks/exhaustive-deps` intentionally.
|
||||
*/
|
||||
export function useAutoFillTaskNameFromPR(fs: DialogFormState) {
|
||||
const { taskName, setTaskName, setHasTitle } = fs;
|
||||
const lastAutoFilledTitleRef = useRef("");
|
||||
const taskNameRef = useRef(taskName);
|
||||
useEffect(() => {
|
||||
taskNameRef.current = taskName;
|
||||
if (!taskName.trim()) {
|
||||
lastAutoFilledTitleRef.current = "";
|
||||
}
|
||||
}, [taskName]);
|
||||
return (prNumber: number, prTitle: string) => {
|
||||
const next = `PR #${prNumber}: ${prTitle}`;
|
||||
const current = taskNameRef.current;
|
||||
if (!current.trim() || current === lastAutoFilledTitleRef.current) {
|
||||
lastAutoFilledTitleRef.current = next;
|
||||
setTaskName(next);
|
||||
setHasTitle(true);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
|
||||
const {
|
||||
useGitHubUrl,
|
||||
@@ -418,6 +451,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
|
||||
setGitHubPrHeadBranch,
|
||||
setGitHubPrBaseBranch,
|
||||
} = fs;
|
||||
const autoFillTitle = useAutoFillTaskNameFromPR(fs);
|
||||
useEffect(() => {
|
||||
if (!open || !useGitHubUrl) {
|
||||
setGitHubBranchesLoading(false);
|
||||
@@ -464,6 +498,7 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
|
||||
if (prInfo) {
|
||||
setGitHubPrHeadBranch(prInfo.head_branch);
|
||||
setGitHubPrBaseBranch(prInfo.base_branch);
|
||||
autoFillTitle(prInfo.number, prInfo.title);
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
@@ -488,6 +523,10 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
|
||||
clearTimeout(timeoutId);
|
||||
controller.abort();
|
||||
};
|
||||
// autoFillTitle is intentionally omitted: it's a fresh closure each render
|
||||
// but reads the latest taskName via ref, so excluding it keeps the fetch
|
||||
// from re-firing on every keystroke.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [
|
||||
open,
|
||||
useGitHubUrl,
|
||||
|
||||
Reference in New Issue
Block a user