Stop auto-populating create PR fields (#2660)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
69728c9904
commit
8ad76f7b37
|
|
@ -358,23 +358,13 @@ describe('getHostedReviewCreationEligibility', () => {
|
|||
blockedReason: null,
|
||||
nextAction: null,
|
||||
defaultBaseRef: 'origin/main',
|
||||
head: 'feature/create-pr',
|
||||
title: 'Feature title',
|
||||
body: 'Feature title'
|
||||
head: 'feature/create-pr'
|
||||
})
|
||||
})
|
||||
|
||||
it('resolves remote eligibility through SSH repo metadata', async () => {
|
||||
it('resolves remote eligibility through SSH repo metadata without generating PR copy', async () => {
|
||||
const remoteGit = {
|
||||
exec: vi.fn(async (args: string[]) => {
|
||||
if (args[0] === 'log' && args.includes('--pretty=%s')) {
|
||||
return { stdout: 'Remote title\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'log') {
|
||||
return { stdout: '- Remote title\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
exec: vi.fn(async () => ({ stdout: '', stderr: '' }))
|
||||
}
|
||||
getSshGitProviderMock.mockReturnValue(remoteGit)
|
||||
|
||||
|
|
@ -392,8 +382,7 @@ describe('getHostedReviewCreationEligibility', () => {
|
|||
).resolves.toMatchObject({
|
||||
provider: 'github',
|
||||
canCreate: true,
|
||||
title: 'Remote title',
|
||||
body: '- Remote title'
|
||||
head: 'feature/create-pr'
|
||||
})
|
||||
|
||||
expect(getProjectSlugMock).toHaveBeenCalledWith('/remote/repo', 'ssh-1')
|
||||
|
|
@ -401,7 +390,7 @@ describe('getHostedReviewCreationEligibility', () => {
|
|||
expect(getHostedReviewForBranchMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ repoPath: '/remote/repo', connectionId: 'ssh-1' })
|
||||
)
|
||||
expect(remoteGit.exec).toHaveBeenCalledWith(['log', '-1', '--pretty=%s'], '/remote/repo')
|
||||
expect(remoteGit.exec).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('offers push as the next action for authenticated branches with local-only commits', async () => {
|
||||
|
|
|
|||
|
|
@ -34,15 +34,6 @@ function stripRefPrefix(ref: string): string {
|
|||
return normalizeHostedReviewHeadRef(ref)
|
||||
}
|
||||
|
||||
function branchToTitle(branch: string): string {
|
||||
const lastSegment = branch.split('/').filter(Boolean).at(-1) ?? branch
|
||||
return lastSegment
|
||||
.replace(/[-_]+/g, ' ')
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim()
|
||||
.replace(/\b\w/g, (char) => char.toUpperCase())
|
||||
}
|
||||
|
||||
async function detectHostedReviewProvider(
|
||||
repoPath: string,
|
||||
connectionId?: string | null
|
||||
|
|
@ -100,44 +91,6 @@ async function runGitForHostedReview(
|
|||
return gitExecFileAsync(args, { cwd: repoPath })
|
||||
}
|
||||
|
||||
async function getLatestCommitSubject(
|
||||
repoPath: string,
|
||||
connectionId?: string | null
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await runGitForHostedReview(
|
||||
repoPath,
|
||||
['log', '-1', '--pretty=%s'],
|
||||
connectionId
|
||||
)
|
||||
const subject = stdout.trim()
|
||||
return subject || null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
async function getCommitSummaryBody(
|
||||
repoPath: string,
|
||||
base: string | null,
|
||||
connectionId?: string | null
|
||||
): Promise<string | null> {
|
||||
if (!base) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
const { stdout } = await runGitForHostedReview(
|
||||
repoPath,
|
||||
['log', '--pretty=format:- %s', '--max-count=20', `${base}..HEAD`],
|
||||
connectionId
|
||||
)
|
||||
const body = stdout.trim()
|
||||
return body || null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
async function getDefaultBaseRef(
|
||||
repoPath: string,
|
||||
connectionId?: string | null
|
||||
|
|
@ -340,16 +293,11 @@ export async function getHostedReviewCreationEligibility(
|
|||
connectionId: args.connectionId ?? null
|
||||
})
|
||||
|
||||
const title =
|
||||
(await getLatestCommitSubject(args.repoPath, args.connectionId)) ?? branchToTitle(branch)
|
||||
const body = await getCommitSummaryBody(args.repoPath, defaultBaseRef ?? null, args.connectionId)
|
||||
const baseResult = {
|
||||
provider,
|
||||
review: review ? { number: review.number, url: review.url } : null,
|
||||
defaultBaseRef,
|
||||
head: branch || null,
|
||||
title,
|
||||
body
|
||||
head: branch || null
|
||||
}
|
||||
|
||||
if (!branch || branch === 'HEAD') {
|
||||
|
|
|
|||
|
|
@ -306,6 +306,7 @@ export function CreatePullRequestDialog({
|
|||
id="create-pr-title"
|
||||
value={title}
|
||||
onChange={(event) => setTitle(event.target.value)}
|
||||
placeholder="Title"
|
||||
aria-invalid={!title.trim()}
|
||||
/>
|
||||
</div>
|
||||
|
|
@ -317,18 +318,19 @@ export function CreatePullRequestDialog({
|
|||
value={body}
|
||||
onChange={(event) => setBody(event.target.value)}
|
||||
rows={6}
|
||||
placeholder="Description (optional)"
|
||||
className="w-full resize-none rounded-md border border-border bg-background px-3 py-2 text-sm text-foreground outline-none placeholder:text-muted-foreground/70 focus-visible:ring-1 focus-visible:ring-ring"
|
||||
/>
|
||||
</div>
|
||||
|
||||
<label className="flex items-center gap-2 text-sm text-foreground">
|
||||
<label className="flex items-center gap-2 rounded-md border border-border bg-background px-3 py-2 text-sm text-foreground transition-colors hover:bg-accent hover:text-accent-foreground">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={draft}
|
||||
onChange={(event) => setDraft(event.target.checked)}
|
||||
className="size-4 rounded border-border accent-primary"
|
||||
className="size-4 shrink-0 rounded border-border accent-primary"
|
||||
/>
|
||||
Draft
|
||||
<span className="min-w-0 flex-1 truncate">Create as draft</span>
|
||||
</label>
|
||||
|
||||
{stripBaseRef(base).toLowerCase() === stripBaseRef(branch).toLowerCase() ? (
|
||||
|
|
|
|||
|
|
@ -4475,23 +4475,26 @@ function PullRequestComposer({
|
|||
aria-hidden="true"
|
||||
/>
|
||||
</div>
|
||||
<label
|
||||
className={cn(
|
||||
'inline-flex shrink-0 items-center gap-1.5 text-[11px]',
|
||||
fieldsLocked ? 'cursor-not-allowed opacity-60' : 'cursor-pointer text-foreground'
|
||||
)}
|
||||
>
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={draft}
|
||||
disabled={fieldsLocked}
|
||||
onChange={(event) => setDraft(event.target.checked)}
|
||||
className="size-3.5 rounded border-border accent-primary"
|
||||
/>
|
||||
Draft
|
||||
</label>
|
||||
</div>
|
||||
|
||||
<label
|
||||
className={cn(
|
||||
'flex h-7 items-center gap-2 rounded-md border border-border bg-background px-2 text-xs text-foreground transition-colors',
|
||||
fieldsLocked
|
||||
? 'cursor-not-allowed opacity-60'
|
||||
: 'cursor-pointer hover:bg-accent hover:text-accent-foreground'
|
||||
)}
|
||||
>
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={draft}
|
||||
disabled={fieldsLocked}
|
||||
onChange={(event) => setDraft(event.target.checked)}
|
||||
className="size-3.5 shrink-0 rounded border-border accent-primary"
|
||||
/>
|
||||
<span className="min-w-0 flex-1 truncate">Create as draft</span>
|
||||
</label>
|
||||
|
||||
{baseResults.length > 0 ? (
|
||||
<div className="max-h-28 overflow-auto rounded-md border border-border p-1 scrollbar-sleek">
|
||||
{baseResults.map((ref) => (
|
||||
|
|
|
|||
|
|
@ -122,8 +122,10 @@ export function useCreatePullRequestDialogFields({
|
|||
initializedFromEligibilityRef.current = initializationKey
|
||||
const initialBase = eligibility.defaultBaseRef ?? ''
|
||||
setBase(stripBaseRef(initialBase))
|
||||
setTitle(eligibility.title ?? '')
|
||||
setBody(eligibility.body ?? '')
|
||||
// Why: PR reviewer-facing copy should be explicit or AI-generated on
|
||||
// request; commit-subject summaries were too noisy as automatic defaults.
|
||||
setTitle('')
|
||||
setBody('')
|
||||
setDraft(false)
|
||||
setBaseQuery('')
|
||||
setBaseResults([])
|
||||
|
|
|
|||
|
|
@ -64,9 +64,7 @@ export async function seedCreatePrComposer(page: Page): Promise<{
|
|||
blockedReason: null,
|
||||
nextAction: null,
|
||||
defaultBaseRef: primaryBranch,
|
||||
head: branch,
|
||||
title: 'Seed PR title',
|
||||
body: 'Seed PR body'
|
||||
head: branch
|
||||
}
|
||||
|
||||
store.setState((current) => ({
|
||||
|
|
|
|||
|
|
@ -110,9 +110,7 @@ async function seedCreatePREligibleBranch(
|
|||
blockedReason: null,
|
||||
nextAction: null,
|
||||
defaultBaseRef: 'origin/main',
|
||||
head: branch,
|
||||
title: 'Create PR from E2E',
|
||||
body: '- Initial commit for E2E'
|
||||
head: branch
|
||||
}
|
||||
|
||||
;(window as unknown as { __createPRPayloads: CreatePRPayload[] }).__createPRPayloads = []
|
||||
|
|
@ -187,16 +185,19 @@ test.describe('Source Control create pull request', () => {
|
|||
|
||||
const createButton = orcaPage.getByRole('button', { name: 'Create PR' })
|
||||
await expect(createButton).toBeVisible({ timeout: 10_000 })
|
||||
await expect(createButton).toBeEnabled()
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request title' })).toHaveValue(
|
||||
'Create PR from E2E'
|
||||
)
|
||||
await expect(createButton).toBeDisabled()
|
||||
const titleInput = orcaPage.getByRole('textbox', { name: 'Pull request title' })
|
||||
const descriptionInput = orcaPage.getByRole('textbox', {
|
||||
name: 'Pull request description'
|
||||
})
|
||||
await expect(titleInput).toHaveValue('')
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request base branch' })).toHaveValue(
|
||||
'main'
|
||||
)
|
||||
await expect(orcaPage.getByRole('textbox', { name: 'Pull request description' })).toHaveValue(
|
||||
'- Initial commit for E2E'
|
||||
)
|
||||
await expect(descriptionInput).toHaveValue('')
|
||||
await titleInput.fill('Create PR from E2E')
|
||||
await descriptionInput.fill('- Initial commit for E2E')
|
||||
await expect(createButton).toBeEnabled()
|
||||
await createButton.click()
|
||||
|
||||
await expect
|
||||
|
|
|
|||
Loading…
Reference in New Issue