| author | |
| committer | |
| log | 339179f50a84dc599cce373dbb48f172f182f44e |
| tree | 45470dd9a1796a403f1c9fcd503f1e07f4ef9be6 |
| parent | e536bfd33bc3c351997141d93790df75eeded7b9 |
| signature | Signed by PGP key B5690EEEBB952194 |
24 files changed, 963 insertions(+), 13 deletions(-)
apps/mobile/src/components/AndroidScreenHeader.tsx+8-4| ... | @@ -6,6 +6,7 @@ import { AppText as Text } from "./AppText"; | ... | @@ -6,6 +6,7 @@ import { AppText as Text } from "./AppText"; |
| 6 | import { cn } from "../lib/cn"; | 6 | import { cn } from "../lib/cn"; |
| 7 | import { MaterialIconButton } from "./MaterialIconButton"; | 7 | import { MaterialIconButton } from "./MaterialIconButton"; |
| 8 | import { AndroidAnchoredMenu } from "./AndroidAnchoredMenu"; | 8 | import { AndroidAnchoredMenu } from "./AndroidAnchoredMenu"; |
| 9 | import { splitAndroidHeaderActions } from "./headerMenu.android"; | ||
| 9 | import { useScaledTextRole } from "../features/settings/appearance/useScaledTextRole"; | 10 | import { useScaledTextRole } from "../features/settings/appearance/useScaledTextRole"; |
| 10 | import { useMaterialToolbarLayout } from "./useMaterialToolbarLayout"; | 11 | import { useMaterialToolbarLayout } from "./useMaterialToolbarLayout"; |
| 11 | 12 | ||
| ... | @@ -16,6 +17,8 @@ export interface AndroidHeaderAction { | ... | @@ -16,6 +17,8 @@ export interface AndroidHeaderAction { |
| 16 | readonly disabled?: boolean; | 17 | readonly disabled?: boolean; |
| 17 | readonly loading?: boolean; | 18 | readonly loading?: boolean; |
| 18 | readonly selected?: boolean; | 19 | readonly selected?: boolean; |
| 20 | readonly destructive?: boolean; | ||
| 21 | readonly overflowOnly?: boolean; | ||
| 19 | } | 22 | } |
| 20 | 23 | ||
| 21 | export function AndroidHeaderIconButton(props: { | 24 | export function AndroidHeaderIconButton(props: { |
| ... | @@ -45,10 +48,10 @@ export function AndroidScreenHeader(props: { | ... | @@ -45,10 +48,10 @@ export function AndroidScreenHeader(props: { |
| 45 | props.embedded, | 48 | props.embedded, |
| 46 | ); | 49 | ); |
| 47 | const [headerWidth, setHeaderWidth] = useState(0); | 50 | const [headerWidth, setHeaderWidth] = useState(0); |
| 48 | const actions = props.actions ?? []; | 51 | const { visibleActions, overflowActions } = splitAndroidHeaderActions( |
| 49 | const directCount = actions.length > 2 ? (headerWidth >= 600 ? 3 : 1) : actions.length; | 52 | props.actions ?? [], |
| 50 | const visibleActions = actions.slice(0, directCount); | 53 | headerWidth >= 600, |
| 51 | const overflowActions = actions.slice(directCount); | 54 | ); |
| 52 | 55 | ||
| 53 | return ( | 56 | return ( |
| 54 | <View | 57 | <View |
| ... | @@ -115,6 +118,7 @@ export function AndroidScreenHeader(props: { | ... | @@ -115,6 +118,7 @@ export function AndroidScreenHeader(props: { |
| 115 | title: action.accessibilityLabel, | 118 | title: action.accessibilityLabel, |
| 116 | attributes: { | 119 | attributes: { |
| 117 | disabled: Boolean(action.disabled), | 120 | disabled: Boolean(action.disabled), |
| 121 | destructive: action.destructive, | ||
| 118 | state: action.selected ? "on" : undefined, | 122 | state: action.selected ? "on" : undefined, |
| 119 | }, | 123 | }, |
| 120 | }))} | 124 | }))} |
apps/mobile/src/components/headerMenu.android.ts+13| ... | @@ -41,3 +41,16 @@ export function findHeaderMenuAction( | ... | @@ -41,3 +41,16 @@ export function findHeaderMenuAction( |
| 41 | } | 41 | } |
| 42 | return undefined; | 42 | return undefined; |
| 43 | } | 43 | } |
| 44 | |||
| 45 | export function splitAndroidHeaderActions<Action extends { readonly overflowOnly?: boolean }>( | ||
| 46 | actions: ReadonlyArray<Action>, | ||
| 47 | wide: boolean, | ||
| 48 | ) { | ||
| 49 | const inline = actions.filter((action) => !action.overflowOnly); | ||
| 50 | const directCount = actions.length > 2 ? (wide ? 3 : 1) : inline.length; | ||
| 51 | const visibleActions = inline.slice(0, directCount); | ||
| 52 | return { | ||
| 53 | visibleActions, | ||
| 54 | overflowActions: actions.filter((action) => !visibleActions.includes(action)), | ||
| 55 | }; | ||
| 56 | } |
apps/mobile/src/components/headerMenu.test.ts+37-1| ... | @@ -1,5 +1,9 @@ | ... | @@ -1,5 +1,9 @@ |
| 1 | import { describe, expect, it } from "vite-plus/test"; | 1 | import { describe, expect, it } from "vite-plus/test"; |
| 2 | import { androidHeaderMenuActions, findHeaderMenuAction } from "./headerMenu.android"; | 2 | import { |
| 3 | androidHeaderMenuActions, | ||
| 4 | findHeaderMenuAction, | ||
| 5 | splitAndroidHeaderActions, | ||
| 6 | } from "./headerMenu.android"; | ||
| 3 | import type { ScreenHeaderMenuItem } from "./ScreenHeader.types"; | 7 | import type { ScreenHeaderMenuItem } from "./ScreenHeader.types"; |
| 4 | 8 | ||
| 5 | const items: ReadonlyArray<ScreenHeaderMenuItem> = [ | 9 | const items: ReadonlyArray<ScreenHeaderMenuItem> = [ |
| ... | @@ -63,4 +67,36 @@ describe("header menus", () => { | ... | @@ -63,4 +67,36 @@ describe("header menus", () => { |
| 63 | expect(findHeaderMenuAction(items, "sessions")).toBeUndefined(); | 67 | expect(findHeaderMenuAction(items, "sessions")).toBeUndefined(); |
| 64 | expect(findHeaderMenuAction(items, "removed-session")).toBeUndefined(); | 68 | expect(findHeaderMenuAction(items, "removed-session")).toBeUndefined(); |
| 65 | }); | 69 | }); |
| 70 | |||
| 71 | it("keeps overflow-only Android header actions in the overflow menu", () => { | ||
| 72 | const action = (accessibilityLabel: string, overflowOnly?: boolean) => ({ | ||
| 73 | accessibilityLabel, | ||
| 74 | icon: "trash" as const, | ||
| 75 | onPress: () => {}, | ||
| 76 | overflowOnly, | ||
| 77 | }); | ||
| 78 | const labels = ( | ||
| 79 | split: ReturnType<typeof splitAndroidHeaderActions<ReturnType<typeof action>>>, | ||
| 80 | ) => ({ | ||
| 81 | visible: split.visibleActions.map((item) => item.accessibilityLabel), | ||
| 82 | overflow: split.overflowActions.map((item) => item.accessibilityLabel), | ||
| 83 | }); | ||
| 84 | const open = action("Open"); | ||
| 85 | const remove = action("Delete", true); | ||
| 86 | expect(labels(splitAndroidHeaderActions([open, action("Comment")], false))).toEqual({ | ||
| 87 | visible: ["Open", "Comment"], | ||
| 88 | overflow: [], | ||
| 89 | }); | ||
| 90 | expect(labels(splitAndroidHeaderActions([open, remove], true))).toEqual({ | ||
| 91 | visible: ["Open"], | ||
| 92 | overflow: ["Delete"], | ||
| 93 | }); | ||
| 94 | expect(labels(splitAndroidHeaderActions([remove], true))).toEqual({ | ||
| 95 | visible: [], | ||
| 96 | overflow: ["Delete"], | ||
| 97 | }); | ||
| 98 | expect( | ||
| 99 | labels(splitAndroidHeaderActions([action("Review"), action("Comment"), open, remove], true)), | ||
| 100 | ).toEqual({ visible: ["Review", "Comment", "Open"], overflow: ["Delete"] }); | ||
| 101 | }); | ||
| 66 | }); | 102 | }); |
apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx+47-1| ... | @@ -17,7 +17,12 @@ import { | ... | @@ -17,7 +17,12 @@ import { |
| 17 | PR_ROW_EXIT, | 17 | PR_ROW_EXIT, |
| 18 | PR_POSITION_LAYOUT, | 18 | PR_POSITION_LAYOUT, |
| 19 | } from "./pullRequestAnimations"; | 19 | } from "./pullRequestAnimations"; |
| 20 | import { openOnProviderLabel } from "./pullRequestActions"; | 20 | import { |
| 21 | canPerformPullRequestAction, | ||
| 22 | openOnProviderLabel, | ||
| 23 | PR_ACTION_ICONS, | ||
| 24 | PR_ACTION_LABELS, | ||
| 25 | } from "./pullRequestActions"; | ||
| 21 | import * as Clipboard from "expo-clipboard"; | 26 | import * as Clipboard from "expo-clipboard"; |
| 22 | import { ControlPillMenu } from "../../components/ControlPillMenu"; | 27 | import { ControlPillMenu } from "../../components/ControlPillMenu"; |
| 23 | import { MaterialIconButton } from "../../components/MaterialIconButton"; | 28 | import { MaterialIconButton } from "../../components/MaterialIconButton"; |
| ... | @@ -25,6 +30,7 @@ import { MaterialButton } from "../../components/MaterialButton"; | ... | @@ -25,6 +30,7 @@ import { MaterialButton } from "../../components/MaterialButton"; |
| 25 | import { usePullRequestReviewDraft } from "./usePullRequestReviewDraft"; | 30 | import { usePullRequestReviewDraft } from "./usePullRequestReviewDraft"; |
| 26 | import { usePullRequestFilesViewed } from "./usePullRequestFilesViewed"; | 31 | import { usePullRequestFilesViewed } from "./usePullRequestFilesViewed"; |
| 27 | import { usePullRequestPermissions } from "./usePullRequestPermissions"; | 32 | import { usePullRequestPermissions } from "./usePullRequestPermissions"; |
| 33 | import { usePullRequestAction } from "./usePullRequestAction"; | ||
| 28 | import { annotatePullRequestDiff, reviewPosition } from "./pullRequestDiffComments"; | 34 | import { annotatePullRequestDiff, reviewPosition } from "./pullRequestDiffComments"; |
| 29 | import { usePullRequestFileFilter } from "./usePullRequestFileFilter"; | 35 | import { usePullRequestFileFilter } from "./usePullRequestFileFilter"; |
| 30 | import { isPullRequestTestFile } from "./pullRequestFileFilters"; | 36 | import { isPullRequestTestFile } from "./pullRequestFileFilters"; |
| ... | @@ -341,6 +347,15 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP | ... | @@ -341,6 +347,15 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP |
| 341 | const summary = params.initialSummary; | 347 | const summary = params.initialSummary; |
| 342 | const identity = pr ?? summary; | 348 | const identity = pr ?? summary; |
| 343 | const permissions = usePullRequestPermissions(params, pr); | 349 | const permissions = usePullRequestPermissions(params, pr); |
| 350 | const action = usePullRequestAction(params, pr); | ||
| 351 | const showsDeleteBranch = pr !== null && canPerformPullRequestAction(pr, "delete-branch"); | ||
| 352 | const confirmDeleteBranch = () => | ||
| 353 | pr && | ||
| 354 | action.confirm( | ||
| 355 | "delete-branch", | ||
| 356 | PR_ACTION_LABELS["delete-branch"], | ||
| 357 | `Delete ${pr.headBranch} from ${pr.headRepositoryNameWithOwner ?? pr.repository}? Local branches and worktrees are not changed.`, | ||
| 358 | ); | ||
| 344 | const stack = usePullRequestStack(params, pr); | 359 | const stack = usePullRequestStack(params, pr); |
| 345 | const refreshActivity = useAtomRefresh(composerPullRequests.activity(requestTarget(params))); | 360 | const refreshActivity = useAtomRefresh(composerPullRequests.activity(requestTarget(params))); |
| 346 | const refreshGuide = useAtomRefresh(composerPullRequests.reviewGuide(requestTarget(params))); | 361 | const refreshGuide = useAtomRefresh(composerPullRequests.reviewGuide(requestTarget(params))); |
| ... | @@ -417,6 +432,18 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP | ... | @@ -417,6 +432,18 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP |
| 417 | }, | 432 | }, |
| 418 | ] | 433 | ] |
| 419 | : []), | 434 | : []), |
| 435 | ...(showsDeleteBranch | ||
| 436 | ? [ | ||
| 437 | { | ||
| 438 | accessibilityLabel: PR_ACTION_LABELS["delete-branch"], | ||
| 439 | icon: PR_ACTION_ICONS["delete-branch"], | ||
| 440 | disabled: !action.available("delete-branch"), | ||
| 441 | destructive: true, | ||
| 442 | overflowOnly: true, | ||
| 443 | onPress: confirmDeleteBranch, | ||
| 444 | }, | ||
| 445 | ] | ||
| 446 | : []), | ||
| 420 | ]} | 447 | ]} |
| 421 | /> | 448 | /> |
| 422 | ) : null} | 449 | ) : null} |
| ... | @@ -456,6 +483,25 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP | ... | @@ -456,6 +483,25 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP |
| 456 | </NativeHeaderToolbar.Label> | 483 | </NativeHeaderToolbar.Label> |
| 457 | </NativeHeaderToolbar.MenuAction> | 484 | </NativeHeaderToolbar.MenuAction> |
| 458 | ) : null} | 485 | ) : null} |
| 486 | {showsDeleteBranch ? ( | ||
| 487 | <NativeHeaderToolbar.MenuAction | ||
| 488 | icon={PR_ACTION_ICONS["delete-branch"]} | ||
| 489 | destructive | ||
| 490 | disabled={!action.available("delete-branch")} | ||
| 491 | subtitle={ | ||
| 492 | action.busy | ||
| 493 | ? "Deleting…" | ||
| 494 | : connected | ||
| 495 | ? pr.headBranch | ||
| 496 | : "Reconnect to delete this branch." | ||
| 497 | } | ||
| 498 | onPress={confirmDeleteBranch} | ||
| 499 | > | ||
| 500 | <NativeHeaderToolbar.Label> | ||
| 501 | {PR_ACTION_LABELS["delete-branch"]} | ||
| 502 | </NativeHeaderToolbar.Label> | ||
| 503 | </NativeHeaderToolbar.MenuAction> | ||
| 504 | ) : null} | ||
| 459 | </NativeHeaderToolbar.Menu> | 505 | </NativeHeaderToolbar.Menu> |
| 460 | </NativeHeaderToolbar> | 506 | </NativeHeaderToolbar> |
| 461 | <View | 507 | <View |
apps/mobile/src/features/pull-requests/pullRequestActions.test.ts+13| ... | @@ -112,6 +112,19 @@ describe("mobile pull request actions", () => { | ... | @@ -112,6 +112,19 @@ describe("mobile pull request actions", () => { |
| 112 | expect(mergeSummary(unknown, true)).toBe("Squash"); | 112 | expect(mergeSummary(unknown, true)).toBe("Squash"); |
| 113 | }); | 113 | }); |
| 114 | 114 | ||
| 115 | it("offers deleting the source branch only once merged or closed on a host that supports it", () => { | ||
| 116 | const capabilities = { ...detail().capabilities, deleteBranch: true }; | ||
| 117 | expect(canPerformPullRequestAction(detail({ capabilities }), "delete-branch")).toBe(false); | ||
| 118 | for (const state of ["merged", "closed"] as const) { | ||
| 119 | expect(canPerformPullRequestAction(detail({ state, capabilities }), "delete-branch")).toBe( | ||
| 120 | true, | ||
| 121 | ); | ||
| 122 | expect(canPerformPullRequestAction(detail({ state }), "delete-branch")).toBe(false); | ||
| 123 | } | ||
| 124 | const refused = detail({ state: "merged", capabilities, viewerPermissions: { actions: [] } }); | ||
| 125 | expect(canPerformPullRequestAction(refused, "delete-branch")).toBe(false); | ||
| 126 | }); | ||
| 127 | |||
| 115 | it("cannot merge when the repository has disabled every offered strategy", () => { | 128 | it("cannot merge when the repository has disabled every offered strategy", () => { |
| 116 | const pr = detail({ mergeCapabilities: { merge: false, squash: false, rebase: false } }); | 129 | const pr = detail({ mergeCapabilities: { merge: false, squash: false, rebase: false } }); |
| 117 | expect(canPerformPullRequestAction(pr, "merge")).toBe(false); | 130 | expect(canPerformPullRequestAction(pr, "merge")).toBe(false); |
apps/mobile/src/features/pull-requests/pullRequestActions.ts+6-1| ... | @@ -13,7 +13,7 @@ type ActionDetail = Pick< | ... | @@ -13,7 +13,7 @@ type ActionDetail = Pick< |
| 13 | > & { | 13 | > & { |
| 14 | readonly capabilities: Pick< | 14 | readonly capabilities: Pick< |
| 15 | PullRequestDetail["capabilities"], | 15 | PullRequestDetail["capabilities"], |
| 16 | "actions" | "mergeMethods" | "updateMethods" | 16 | "actions" | "mergeMethods" | "updateMethods" | "deleteBranch" |
| 17 | >; | 17 | >; |
| 18 | readonly viewerPermissions: Pick< | 18 | readonly viewerPermissions: Pick< |
| 19 | PullRequestDetail["viewerPermissions"], | 19 | PullRequestDetail["viewerPermissions"], |
| ... | @@ -41,6 +41,7 @@ export const PR_ACTION_LABELS = { | ... | @@ -41,6 +41,7 @@ export const PR_ACTION_LABELS = { |
| 41 | "enable-auto-merge": "Enable auto-merge", | 41 | "enable-auto-merge": "Enable auto-merge", |
| 42 | "disable-auto-merge": "Disable auto-merge", | 42 | "disable-auto-merge": "Disable auto-merge", |
| 43 | revert: "Create revert pull request", | 43 | revert: "Create revert pull request", |
| 44 | "delete-branch": "Delete branch", | ||
| 44 | "approve-workflows": "Approve workflows", | 45 | "approve-workflows": "Approve workflows", |
| 45 | } as const satisfies Record<PullRequestAction, string>; | 46 | } as const satisfies Record<PullRequestAction, string>; |
| 46 | 47 | ||
| ... | @@ -54,6 +55,7 @@ export const PR_ACTION_ICONS = { | ... | @@ -54,6 +55,7 @@ export const PR_ACTION_ICONS = { |
| 54 | "enable-auto-merge": "clock", | 55 | "enable-auto-merge": "clock", |
| 55 | "disable-auto-merge": "xmark", | 56 | "disable-auto-merge": "xmark", |
| 56 | revert: "arrow.uturn.backward", | 57 | revert: "arrow.uturn.backward", |
| 58 | "delete-branch": "trash", | ||
| 57 | "approve-workflows": "checkmark", | 59 | "approve-workflows": "checkmark", |
| 58 | } as const satisfies Record<PullRequestAction, AppSymbolName>; | 60 | } as const satisfies Record<PullRequestAction, AppSymbolName>; |
| 59 | 61 | ||
| ... | @@ -68,6 +70,7 @@ export const ALL_PR_ACTIONS: readonly PullRequestAction[] = [ | ... | @@ -68,6 +70,7 @@ export const ALL_PR_ACTIONS: readonly PullRequestAction[] = [ |
| 68 | "close", | 70 | "close", |
| 69 | "reopen", | 71 | "reopen", |
| 70 | "revert", | 72 | "revert", |
| 73 | "delete-branch", | ||
| 71 | ]; | 74 | ]; |
| 72 | 75 | ||
| 73 | export function allowedMergeMethods(pr: ActionDetail) { | 76 | export function allowedMergeMethods(pr: ActionDetail) { |
| ... | @@ -86,6 +89,8 @@ export function canPerformPullRequestAction(pr: ActionDetail, action: PullReques | ... | @@ -86,6 +89,8 @@ export function canPerformPullRequestAction(pr: ActionDetail, action: PullReques |
| 86 | return false; | 89 | return false; |
| 87 | if (action === "reopen") return pr.state === "closed"; | 90 | if (action === "reopen") return pr.state === "closed"; |
| 88 | if (action === "revert") return pr.state === "merged"; | 91 | if (action === "revert") return pr.state === "merged"; |
| 92 | if (action === "delete-branch") | ||
| 93 | return pr.state !== "open" && pr.capabilities.deleteBranch === true; | ||
| 89 | if (pr.state !== "open") return false; | 94 | if (pr.state !== "open") return false; |
| 90 | switch (action) { | 95 | switch (action) { |
| 91 | case "update-branch": | 96 | case "update-branch": |
apps/mobile/src/features/pull-requests/usePullRequestAction.ts+3-3| ... | @@ -14,7 +14,7 @@ import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff" | ... | @@ -14,7 +14,7 @@ import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff" |
| 14 | import { usePullRequestPermissions } from "./usePullRequestPermissions"; | 14 | import { usePullRequestPermissions } from "./usePullRequestPermissions"; |
| 15 | 15 | ||
| 16 | /** Small row actions stay in place, preserving expanded rows and scroll position. */ | 16 | /** Small row actions stay in place, preserving expanded rows and scroll position. */ |
| 17 | export function usePullRequestAction(params: PullRequestParams, pr: PullRequestDetail) { | 17 | export function usePullRequestAction(params: PullRequestParams, pr: PullRequestDetail | null) { |
| 18 | const target = pullRequestTarget(params); | 18 | const target = pullRequestTarget(params); |
| 19 | const permissions = usePullRequestPermissions(params, pr); | 19 | const permissions = usePullRequestPermissions(params, pr); |
| 20 | const run = useAtomCommand(composerPullRequests.runAction, { reportFailure: false }); | 20 | const run = useAtomCommand(composerPullRequests.runAction, { reportFailure: false }); |
| ... | @@ -23,7 +23,7 @@ export function usePullRequestAction(params: PullRequestParams, pr: PullRequestD | ... | @@ -23,7 +23,7 @@ export function usePullRequestAction(params: PullRequestParams, pr: PullRequestD |
| 23 | const pending = useRef(false); | 23 | const pending = useRef(false); |
| 24 | const [busy, setBusy] = useState(false); | 24 | const [busy, setBusy] = useState(false); |
| 25 | const available = (action: PullRequestAction) => | 25 | const available = (action: PullRequestAction) => |
| 26 | !busy && permissions.canWrite && canPerformPullRequestAction(pr, action); | 26 | pr !== null && !busy && permissions.canWrite && canPerformPullRequestAction(pr, action); |
| 27 | const perform = async (action: PullRequestAction, updateMethod?: PullRequestUpdateMethod) => { | 27 | const perform = async (action: PullRequestAction, updateMethod?: PullRequestUpdateMethod) => { |
| 28 | if (pending.current || !available(action)) return; | 28 | if (pending.current || !available(action)) return; |
| 29 | pending.current = true; | 29 | pending.current = true; |
| ... | @@ -51,7 +51,7 @@ export function usePullRequestAction(params: PullRequestParams, pr: PullRequestD | ... | @@ -51,7 +51,7 @@ export function usePullRequestAction(params: PullRequestParams, pr: PullRequestD |
| 51 | { text: "Cancel", style: "cancel" }, | 51 | { text: "Cancel", style: "cancel" }, |
| 52 | { | 52 | { |
| 53 | text: title, | 53 | text: title, |
| 54 | style: action === "close" ? "destructive" : "default", | 54 | style: action === "close" || action === "delete-branch" ? "destructive" : "default", |
| 55 | onPress: () => void perform(action), | 55 | onPress: () => void perform(action), |
| 56 | }, | 56 | }, |
| 57 | ]); | 57 | ]); |
apps/server/src/pullRequest/PullRequestService.test.ts+198| ... | @@ -13,7 +13,9 @@ import * as Exit from "effect/Exit"; | ... | @@ -13,7 +13,9 @@ import * as Exit from "effect/Exit"; |
| 13 | import * as Fiber from "effect/Fiber"; | 13 | import * as Fiber from "effect/Fiber"; |
| 14 | import * as Layer from "effect/Layer"; | 14 | import * as Layer from "effect/Layer"; |
| 15 | import * as Option from "effect/Option"; | 15 | import * as Option from "effect/Option"; |
| 16 | import * as Redacted from "effect/Redacted"; | ||
| 16 | import * as Stream from "effect/Stream"; | 17 | import * as Stream from "effect/Stream"; |
| 18 | import { HttpClient, HttpClientResponse } from "effect/http"; | ||
| 17 | import * as TestClock from "effect/testing/TestClock"; | 19 | import * as TestClock from "effect/testing/TestClock"; |
| 18 | import type { | 20 | import type { |
| 19 | OrchestrationProjectShell, | 21 | OrchestrationProjectShell, |
| ... | @@ -34,6 +36,12 @@ import * as SourceControlProviderRegistry from "../sourceControl/SourceControlPr | ... | @@ -34,6 +36,12 @@ import * as SourceControlProviderRegistry from "../sourceControl/SourceControlPr |
| 34 | import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; | 36 | import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; |
| 35 | import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli"; | 37 | import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli"; |
| 36 | import * as ForgejoPullRequestProvider from "@t3tools/source-control-forgejo/server/ForgejoPullRequestProvider"; | 38 | import * as ForgejoPullRequestProvider from "@t3tools/source-control-forgejo/server/ForgejoPullRequestProvider"; |
| 39 | import * as GitHubApi from "@t3tools/source-control-github/server/GitHubApi"; | ||
| 40 | import * as GitHubCredentials from "@t3tools/source-control-github/server/GitHubCredentials"; | ||
| 41 | import * as GitHubPullRequestApi from "@t3tools/source-control-github/server/GitHubPullRequestApi"; | ||
| 42 | import * as GitHubPullRequestProvider from "@t3tools/source-control-github/server/GitHubPullRequestProvider"; | ||
| 43 | import * as GitHubQuota from "@t3tools/source-control-github/server/GitHubQuota"; | ||
| 44 | import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; | ||
| 37 | import { | 45 | import { |
| 38 | PullRequestProviderError, | 46 | PullRequestProviderError, |
| 39 | type ProviderChangeRequest, | 47 | type ProviderChangeRequest, |
| ... | @@ -1624,6 +1632,196 @@ it.effect("refreshes every reader before a queued merge confirmation finishes", | ... | @@ -1624,6 +1632,196 @@ it.effect("refreshes every reader before a queued merge confirmation finishes", |
| 1624 | ), | 1632 | ), |
| 1625 | ); | 1633 | ); |
| 1626 | 1634 | ||
| 1635 | it.effect("runs branch deletion only when the host and viewer support it, never for a stack", () => | ||
| 1636 | Effect.gen(function* () { | ||
| 1637 | let actions = 0; | ||
| 1638 | let supported = false; | ||
| 1639 | let allowed = true; | ||
| 1640 | const base = fakeProvider(SourceControlProviderKind.make("github")); | ||
| 1641 | const service = yield* makeService({ | ||
| 1642 | projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], | ||
| 1643 | providers: [ | ||
| 1644 | { | ||
| 1645 | ...base, | ||
| 1646 | capabilities: { | ||
| 1647 | ...base.capabilities, | ||
| 1648 | actions: ["delete-branch"], | ||
| 1649 | stackActions: true, | ||
| 1650 | get deleteBranch() { | ||
| 1651 | return supported; | ||
| 1652 | }, | ||
| 1653 | }, | ||
| 1654 | getViewerPermissions: () => | ||
| 1655 | Effect.succeed({ | ||
| 1656 | actions: allowed ? ["delete-branch"] : [], | ||
| 1657 | comment: true, | ||
| 1658 | resolve: true, | ||
| 1659 | verdicts: [], | ||
| 1660 | requestReviewers: false, | ||
| 1661 | }), | ||
| 1662 | runAction: (input) => | ||
| 1663 | Effect.sync(() => { | ||
| 1664 | assert.strictEqual(input.action, "delete-branch"); | ||
| 1665 | actions++; | ||
| 1666 | }), | ||
| 1667 | }, | ||
| 1668 | ], | ||
| 1669 | }); | ||
| 1670 | const input = { | ||
| 1671 | projectId: "p1" as ProjectId, | ||
| 1672 | repository: "acme/web", | ||
| 1673 | number: 1, | ||
| 1674 | action: "delete-branch" as const, | ||
| 1675 | }; | ||
| 1676 | assert.strictEqual( | ||
| 1677 | (yield* Effect.flip(service.runAction(input)))._tag, | ||
| 1678 | "PullRequestOperationError", | ||
| 1679 | ); | ||
| 1680 | assert.strictEqual(actions, 0); | ||
| 1681 | supported = true; | ||
| 1682 | assert.strictEqual( | ||
| 1683 | (yield* Effect.flip( | ||
| 1684 | service.runAction({ | ||
| 1685 | ...input, | ||
| 1686 | stackNumber: 2, | ||
| 1687 | expectedStackHeads: [{ number: 1, headSha: "abc123" }], | ||
| 1688 | }), | ||
| 1689 | ))._tag, | ||
| 1690 | "PullRequestOperationError", | ||
| 1691 | ); | ||
| 1692 | assert.strictEqual(actions, 0); | ||
| 1693 | allowed = false; | ||
| 1694 | assert.strictEqual( | ||
| 1695 | (yield* Effect.flip(service.runAction(input)))._tag, | ||
| 1696 | "PullRequestOperationError", | ||
| 1697 | ); | ||
| 1698 | assert.strictEqual(actions, 0); | ||
| 1699 | allowed = true; | ||
| 1700 | yield* service.runAction(input); | ||
| 1701 | assert.strictEqual(actions, 1); | ||
| 1702 | }), | ||
| 1703 | ); | ||
| 1704 | |||
| 1705 | it.effect("keeps branch deletion on the credential that passed source repository permissions", () => | ||
| 1706 | Effect.gen(function* () { | ||
| 1707 | let activeToken = "account-a"; | ||
| 1708 | const requests: Array<{ operation: string; authorization: string | undefined }> = []; | ||
| 1709 | const credentials = Layer.succeed( | ||
| 1710 | GitHubCredentials.GitHubCredentials, | ||
| 1711 | GitHubCredentials.GitHubCredentials.of({ | ||
| 1712 | get: (host) => | ||
| 1713 | Effect.sync(() => ({ | ||
| 1714 | host, | ||
| 1715 | token: Redacted.make(activeToken), | ||
| 1716 | source: "gh" as const, | ||
| 1717 | fingerprint: `${host}:${activeToken}`, | ||
| 1718 | })), | ||
| 1719 | invalidate: () => Effect.void, | ||
| 1720 | }), | ||
| 1721 | ); | ||
| 1722 | const http = Layer.succeed( | ||
| 1723 | HttpClient.HttpClient, | ||
| 1724 | HttpClient.make((request) => | ||
| 1725 | Effect.sync(() => { | ||
| 1726 | const operation = request.url.endsWith("/user") | ||
| 1727 | ? "user" | ||
| 1728 | : request.body._tag === "Uint8Array" | ||
| 1729 | ? new TextDecoder().decode(request.body.body) | ||
| 1730 | : ""; | ||
| 1731 | requests.push({ operation, authorization: request.headers["authorization"] }); | ||
| 1732 | let body; | ||
| 1733 | if (operation === "user") { | ||
| 1734 | body = { id: activeToken === "account-a" ? 101 : 202, login: activeToken }; | ||
| 1735 | } else if (operation.includes("query PullRequestBranchToDelete")) { | ||
| 1736 | body = { | ||
| 1737 | data: { | ||
| 1738 | repository: { | ||
| 1739 | pullRequest: { | ||
| 1740 | state: "MERGED", | ||
| 1741 | headRefName: "feature", | ||
| 1742 | baseRefName: "main", | ||
| 1743 | headRefOid: "abc123", | ||
| 1744 | headRepository: { id: "FORK_REPO" }, | ||
| 1745 | }, | ||
| 1746 | }, | ||
| 1747 | }, | ||
| 1748 | }; | ||
| 1749 | } else if (operation.includes("query PullRequestSourceBranch")) { | ||
| 1750 | body = { | ||
| 1751 | data: { | ||
| 1752 | node: { | ||
| 1753 | defaultBranchRef: { name: "main" }, | ||
| 1754 | ref: { target: { oid: "abc123" } }, | ||
| 1755 | }, | ||
| 1756 | }, | ||
| 1757 | }; | ||
| 1758 | } else if (operation.includes("updateRefs(")) { | ||
| 1759 | body = { data: { updateRefs: {} } }; | ||
| 1760 | } else { | ||
| 1761 | activeToken = "account-b"; | ||
| 1762 | body = { | ||
| 1763 | data: { | ||
| 1764 | repository: { | ||
| 1765 | mergeCommitAllowed: true, | ||
| 1766 | squashMergeAllowed: true, | ||
| 1767 | rebaseMergeAllowed: true, | ||
| 1768 | viewerPermission: "READ", | ||
| 1769 | pullRequest: { | ||
| 1770 | viewerCanUpdate: false, | ||
| 1771 | viewerDidAuthor: false, | ||
| 1772 | headRepository: { viewerPermission: "WRITE" }, | ||
| 1773 | }, | ||
| 1774 | }, | ||
| 1775 | }, | ||
| 1776 | }; | ||
| 1777 | } | ||
| 1778 | return HttpClientResponse.fromWeb(request, new Response(JSON.stringify(body))); | ||
| 1779 | }), | ||
| 1780 | ), | ||
| 1781 | ); | ||
| 1782 | const provider = yield* GitHubPullRequestProvider.make.pipe( | ||
| 1783 | Effect.provide( | ||
| 1784 | GitHubPullRequestApi.layer.pipe( | ||
| 1785 | Layer.provide( | ||
| 1786 | GitHubApi.layer.pipe( | ||
| 1787 | Layer.provide(Layer.mergeAll(credentials, http)), | ||
| 1788 | Layer.provide(GitHubQuota.layer), | ||
| 1789 | Layer.provide(SourceControlRateLimit.layer), | ||
| 1790 | ), | ||
| 1791 | ), | ||
| 1792 | Layer.provide(TestSourceControlHost.layer()), | ||
| 1793 | Layer.provide(NodeServices.layer), | ||
| 1794 | ), | ||
| 1795 | ), | ||
| 1796 | ); | ||
| 1797 | const service = yield* makeService({ | ||
| 1798 | projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], | ||
| 1799 | providers: [provider], | ||
| 1800 | }); | ||
| 1801 | yield* service.runAction({ | ||
| 1802 | projectId: "p1" as ProjectId, | ||
| 1803 | repository: "acme/web", | ||
| 1804 | number: 1, | ||
| 1805 | action: "delete-branch", | ||
| 1806 | }); | ||
| 1807 | assert.strictEqual(activeToken, "account-b"); | ||
| 1808 | assert.strictEqual( | ||
| 1809 | requests.filter(({ operation }) => operation.includes("updateRefs(")).length, | ||
| 1810 | 1, | ||
| 1811 | ); | ||
| 1812 | assert.deepStrictEqual( | ||
| 1813 | requests.map(({ authorization }) => authorization), | ||
| 1814 | [ | ||
| 1815 | "Bearer account-a", | ||
| 1816 | "Bearer account-a", | ||
| 1817 | "Bearer account-a", | ||
| 1818 | "Bearer account-a", | ||
| 1819 | "Bearer account-a", | ||
| 1820 | ], | ||
| 1821 | ); | ||
| 1822 | }), | ||
| 1823 | ); | ||
| 1824 | |||
| 1627 | it.effect("refuses an action this viewer may not take, and says what access it takes", () => | 1825 | it.effect("refuses an action this viewer may not take, and says what access it takes", () => |
| 1628 | Effect.gen(function* () { | 1826 | Effect.gen(function* () { |
| 1629 | let ran: string | null = null; | 1827 | let ran: string | null = null; |
apps/server/src/pullRequest/PullRequestService.ts+21-1| ... | @@ -356,6 +356,7 @@ const ACTION_ACCESS_REFUSALS: Record<PullRequestAction, string> = { | ... | @@ -356,6 +356,7 @@ const ACTION_ACCESS_REFUSALS: Record<PullRequestAction, string> = { |
| 356 | "disable-auto-merge": | 356 | "disable-auto-merge": |
| 357 | "You need write access on this repository to stop it being merged for you once it is ready.", | 357 | "You need write access on this repository to stop it being merged for you once it is ready.", |
| 358 | revert: "You need write access on this repository to open a revert pull request.", | 358 | revert: "You need write access on this repository to open a revert pull request.", |
| 359 | "delete-branch": "You need write access on the source repository to delete its branch.", | ||
| 359 | "approve-workflows": | 360 | "approve-workflows": |
| 360 | "You need write access on this repository to approve workflows from a fork pull request.", | 361 | "You need write access on this repository to approve workflows from a fork pull request.", |
| 361 | }; | 362 | }; |
| ... | @@ -2019,6 +2020,14 @@ export const make = Effect.gen(function* () { | ... | @@ -2019,6 +2020,14 @@ export const make = Effect.gen(function* () { |
| 2019 | const runAction = (input: PullRequestActionInput): Effect.Effect<string, PullRequestError> => | 2020 | const runAction = (input: PullRequestActionInput): Effect.Effect<string, PullRequestError> => |
| 2020 | requireProject(input).pipe( | 2021 | requireProject(input).pipe( |
| 2021 | Effect.flatMap((project): Effect.Effect<string, PullRequestError> => { | 2022 | Effect.flatMap((project): Effect.Effect<string, PullRequestError> => { |
| 2023 | if (input.action === "delete-branch" && project.api.capabilities.deleteBranch !== true) { | ||
| 2024 | return Effect.fail( | ||
| 2025 | new PullRequestOperationError({ | ||
| 2026 | operation: "runAction", | ||
| 2027 | detail: "This host cannot delete a change request's source branch.", | ||
| 2028 | }), | ||
| 2029 | ); | ||
| 2030 | } | ||
| 2022 | if ( | 2031 | if ( |
| 2023 | input.stackNumber !== undefined && | 2032 | input.stackNumber !== undefined && |
| 2024 | (project.api.capabilities.stackActions !== true || | 2033 | (project.api.capabilities.stackActions !== true || |
| ... | @@ -2073,7 +2082,7 @@ export const make = Effect.gen(function* () { | ... | @@ -2073,7 +2082,7 @@ export const make = Effect.gen(function* () { |
| 2073 | // What the host can do and what this account may ask of it are two questions, and both | 2082 | // What the host can do and what this account may ask of it are two questions, and both |
| 2074 | // have to say yes. The second is asked last, because it costs a request and the checks | 2083 | // have to say yes. The second is asked last, because it costs a request and the checks |
| 2075 | // above do not. | 2084 | // above do not. |
| 2076 | return viewerPermissionsOf( | 2085 | const operation = viewerPermissionsOf( |
| 2077 | project, | 2086 | project, |
| 2078 | input, | 2087 | input, |
| 2079 | "runAction", | 2088 | "runAction", |
| ... | @@ -2158,6 +2167,17 @@ export const make = Effect.gen(function* () { | ... | @@ -2158,6 +2167,17 @@ export const make = Effect.gen(function* () { |
| 2158 | ); | 2167 | ); |
| 2159 | }), | 2168 | }), |
| 2160 | ); | 2169 | ); |
| 2170 | return input.action === "delete-branch" && project.api.withVerifiedCredential !== undefined | ||
| 2171 | ? project.api | ||
| 2172 | .withVerifiedCredential( | ||
| 2173 | { cwd: project.project.workspaceRoot, host: project.host }, | ||
| 2174 | () => operation.pipe(Effect.result), | ||
| 2175 | ) | ||
| 2176 | .pipe( | ||
| 2177 | Effect.mapError(toPullRequestError("runAction")), | ||
| 2178 | Effect.flatMap(Effect.fromResult), | ||
| 2179 | ) | ||
| 2180 | : operation; | ||
| 2161 | }), | 2181 | }), |
| 2162 | ); | 2182 | ); |
| 2163 | 2183 |
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+48| ... | @@ -45,6 +45,7 @@ import { | ... | @@ -45,6 +45,7 @@ import { |
| 45 | PlayIcon, | 45 | PlayIcon, |
| 46 | RotateCcwIcon, | 46 | RotateCcwIcon, |
| 47 | SparklesIcon, | 47 | SparklesIcon, |
| 48 | Trash2Icon, | ||
| 48 | TriangleAlertIcon, | 49 | TriangleAlertIcon, |
| 49 | } from "lucide-react"; | 50 | } from "lucide-react"; |
| 50 | import { | 51 | import { |
| ... | @@ -161,6 +162,7 @@ import { | ... | @@ -161,6 +162,7 @@ import { |
| 161 | resolveDisplayedPullRequestDetail, | 162 | resolveDisplayedPullRequestDetail, |
| 162 | resolvePullRequestPrimaryControl, | 163 | resolvePullRequestPrimaryControl, |
| 163 | allowsSinglePullRequestMerge, | 164 | allowsSinglePullRequestMerge, |
| 165 | canDeletePullRequestBranch, | ||
| 164 | resolveBaseFreshness, | 166 | resolveBaseFreshness, |
| 165 | resolvePullRequestMergeMethod, | 167 | resolvePullRequestMergeMethod, |
| 166 | type PullRequestFinding, | 168 | type PullRequestFinding, |
| ... | @@ -204,6 +206,7 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { | ... | @@ -204,6 +206,7 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { |
| 204 | "enable-auto-merge": "Could not turn on auto-merge", | 206 | "enable-auto-merge": "Could not turn on auto-merge", |
| 205 | "disable-auto-merge": "Could not turn off auto-merge", | 207 | "disable-auto-merge": "Could not turn off auto-merge", |
| 206 | revert: "Could not open a revert pull request", | 208 | revert: "Could not open a revert pull request", |
| 209 | "delete-branch": "Could not delete this branch", | ||
| 207 | "approve-workflows": "Could not approve workflows", | 210 | "approve-workflows": "Could not approve workflows", |
| 208 | }; | 211 | }; |
| 209 | 212 | ||
| ... | @@ -228,6 +231,8 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { | ... | @@ -228,6 +231,8 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { |
| 228 | "The host refused it. Check that you have write access, and that the merge has not already happened.", | 231 | "The host refused it. Check that you have write access, and that the merge has not already happened.", |
| 229 | revert: | 232 | revert: |
| 230 | "The host refused it. Check that you have write access and that this pull request was merged on the host.", | 233 | "The host refused it. Check that you have write access and that this pull request was merged on the host.", |
| 234 | "delete-branch": | ||
| 235 | "The host refused it. Check that you have write access to the source repository and that the branch has not moved since this page loaded.", | ||
| 231 | "approve-workflows": | 236 | "approve-workflows": |
| 232 | "The host refused it. Check that you have Actions write access and that these workflow runs are still awaiting approval.", | 237 | "The host refused it. Check that you have Actions write access and that these workflow runs are still awaiting approval.", |
| 233 | }; | 238 | }; |
| ... | @@ -1030,6 +1035,9 @@ export function PullRequestDetailPanel({ | ... | @@ -1030,6 +1035,9 @@ export function PullRequestDetailPanel({ |
| 1030 | if (action === "revert") { | 1035 | if (action === "revert") { |
| 1031 | toastManager.add({ type: "success", title: "Revert pull request opened" }); | 1036 | toastManager.add({ type: "success", title: "Revert pull request opened" }); |
| 1032 | } | 1037 | } |
| 1038 | if (action === "delete-branch") { | ||
| 1039 | toastManager.add({ type: "success", title: "Branch deleted" }); | ||
| 1040 | } | ||
| 1033 | // A branch update moves the head commit, which leaves the diff atom pointed at a comparison | 1041 | // A branch update moves the head commit, which leaves the diff atom pointed at a comparison |
| 1034 | // that no longer exists — the same staleness the manual refresh button fixes, so it goes | 1042 | // that no longer exists — the same staleness the manual refresh button fixes, so it goes |
| 1035 | // through that path rather than a second one. Every other action here only changes metadata; | 1043 | // through that path rather than a second one. Every other action here only changes metadata; |
| ... | @@ -1639,6 +1647,22 @@ export function PullRequestDetailPanel({ | ... | @@ -1639,6 +1647,22 @@ export function PullRequestDetailPanel({ |
| 1639 | const statePresentation = detail | 1647 | const statePresentation = detail |
| 1640 | ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) | 1648 | ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) |
| 1641 | : null; | 1649 | : null; |
| 1650 | const showsDeleteBranch = detail !== null && canDeletePullRequestBranch(detail); | ||
| 1651 | const environmentConnected = environments.some( | ||
| 1652 | (environment) => | ||
| 1653 | environment.environmentId === environmentId && environment.connection.phase === "connected", | ||
| 1654 | ); | ||
| 1655 | const confirmDeleteBranch = async () => { | ||
| 1656 | if (detail === null) return; | ||
| 1657 | const confirmed = await readLocalApi()?.dialogs.confirm( | ||
| 1658 | [ | ||
| 1659 | `Delete branch ${detail.headBranch}?`, | ||
| 1660 | `This deletes ${detail.headBranch} from ${detail.headRepositoryNameWithOwner ?? detail.repository}. Local branches and worktrees are not changed.`, | ||
| 1661 | ].join("\n"), | ||
| 1662 | { variant: "destructive" }, | ||
| 1663 | ); | ||
| 1664 | if (confirmed) void perform("delete-branch"); | ||
| 1665 | }; | ||
| 1642 | const showsApproveWorkflows = | 1666 | const showsApproveWorkflows = |
| 1643 | workflowApprovalsRequired > 0 && !checksStale && can("approve-workflows"); | 1667 | workflowApprovalsRequired > 0 && !checksStale && can("approve-workflows"); |
| 1644 | const checksSummary = checksStale | 1668 | const checksSummary = checksStale |
| ... | @@ -2424,6 +2448,30 @@ export function PullRequestDetailPanel({ | ... | @@ -2424,6 +2448,30 @@ export function PullRequestDetailPanel({ |
| 2424 | </MenuItem> | 2448 | </MenuItem> |
| 2425 | </> | 2449 | </> |
| 2426 | ) : null} | 2450 | ) : null} |
| 2451 | {showsDeleteBranch ? ( | ||
| 2452 | <> | ||
| 2453 | <MenuSeparator /> | ||
| 2454 | <MenuItem | ||
| 2455 | variant="destructive" | ||
| 2456 | disabled={actionPending || !environmentConnected} | ||
| 2457 | onClick={() => void confirmDeleteBranch()} | ||
| 2458 | > | ||
| 2459 | <Trash2Icon className="mt-1 size-3.5 shrink-0 self-start" /> | ||
| 2460 | <span className="flex min-w-0 flex-col"> | ||
| 2461 | <span> | ||
| 2462 | {pendingAction === "delete-branch" | ||
| 2463 | ? "Deleting branch..." | ||
| 2464 | : "Delete branch"} | ||
| 2465 | </span> | ||
| 2466 | <span className="truncate text-xs text-muted-foreground"> | ||
| 2467 | {environmentConnected | ||
| 2468 | ? detail.headBranch | ||
| 2469 | : "Reconnect to delete this branch"} | ||
| 2470 | </span> | ||
| 2471 | </span> | ||
| 2472 | </MenuItem> | ||
| 2473 | </> | ||
| 2474 | ) : null} | ||
| 2427 | </MenuPopup> | 2475 | </MenuPopup> |
| 2428 | </Menu> | 2476 | </Menu> |
| 2429 | </TooltipProvider> | 2477 | </TooltipProvider> |
apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts+30| ... | @@ -10,6 +10,7 @@ import { | ... | @@ -10,6 +10,7 @@ import { |
| 10 | type PullRequestDetailView, | 10 | type PullRequestDetailView, |
| 11 | type PullRequestRef, | 11 | type PullRequestRef, |
| 12 | type PullRequestReviewThread, | 12 | type PullRequestReviewThread, |
| 13 | type PullRequestState, | ||
| 13 | type RepositoryIdentity, | 14 | type RepositoryIdentity, |
| 14 | type ThreadPullRequestLink, | 15 | type ThreadPullRequestLink, |
| 15 | } from "@t3tools/contracts"; | 16 | } from "@t3tools/contracts"; |
| ... | @@ -28,6 +29,7 @@ import { | ... | @@ -28,6 +29,7 @@ import { |
| 28 | buildExplainPullRequestHandoff, | 29 | buildExplainPullRequestHandoff, |
| 29 | buildPullRequestReferenceContext, | 30 | buildPullRequestReferenceContext, |
| 30 | buildFixFindingHandoff, | 31 | buildFixFindingHandoff, |
| 32 | canDeletePullRequestBranch, | ||
| 31 | buildFixFindingsHandoff, | 33 | buildFixFindingsHandoff, |
| 32 | groupPullRequestTimelineConversations, | 34 | groupPullRequestTimelineConversations, |
| 33 | handoffPrompt, | 35 | handoffPrompt, |
| ... | @@ -1654,12 +1656,40 @@ describe("which actions need the host read again after they run", () => { | ... | @@ -1654,12 +1656,40 @@ describe("which actions need the host read again after they run", () => { |
| 1654 | "disable-auto-merge", | 1656 | "disable-auto-merge", |
| 1655 | "merge", | 1657 | "merge", |
| 1656 | "revert", | 1658 | "revert", |
| 1659 | "delete-branch", | ||
| 1657 | ] as const) { | 1660 | ] as const) { |
| 1658 | expect(pullRequestActionNeedsHostRefresh(action)).toBe(false); | 1661 | expect(pullRequestActionNeedsHostRefresh(action)).toBe(false); |
| 1659 | } | 1662 | } |
| 1660 | }); | 1663 | }); |
| 1661 | }); | 1664 | }); |
| 1662 | 1665 | ||
| 1666 | describe("when the source branch can be deleted", () => { | ||
| 1667 | const branchDetail = ( | ||
| 1668 | state: PullRequestState, | ||
| 1669 | overrides: { deleteBranch?: boolean; allowed?: boolean } = {}, | ||
| 1670 | ) => ({ | ||
| 1671 | state, | ||
| 1672 | capabilities: { | ||
| 1673 | deleteBranch: overrides.deleteBranch ?? true, | ||
| 1674 | actions: ["delete-branch" as const], | ||
| 1675 | }, | ||
| 1676 | viewerPermissions: { | ||
| 1677 | actions: overrides.allowed === false ? [] : (["delete-branch"] as const), | ||
| 1678 | }, | ||
| 1679 | }); | ||
| 1680 | |||
| 1681 | it("offers it only after the pull request is merged or closed", () => { | ||
| 1682 | expect(canDeletePullRequestBranch(branchDetail("merged"))).toBe(true); | ||
| 1683 | expect(canDeletePullRequestBranch(branchDetail("closed"))).toBe(true); | ||
| 1684 | expect(canDeletePullRequestBranch(branchDetail("open"))).toBe(false); | ||
| 1685 | }); | ||
| 1686 | |||
| 1687 | it("needs both the host capability and the viewer's permission", () => { | ||
| 1688 | expect(canDeletePullRequestBranch(branchDetail("merged", { deleteBranch: false }))).toBe(false); | ||
| 1689 | expect(canDeletePullRequestBranch(branchDetail("merged", { allowed: false }))).toBe(false); | ||
| 1690 | }); | ||
| 1691 | }); | ||
| 1692 | |||
| 1663 | describe("the compact row's single action slot", () => { | 1693 | describe("the compact row's single action slot", () => { |
| 1664 | const check = (status: PullRequestCheck["status"]): PullRequestCheck => ({ | 1694 | const check = (status: PullRequestCheck["status"]): PullRequestCheck => ({ |
| 1665 | name: "ci", | 1695 | name: "ci", |
apps/web/src/components/pullRequest/pullRequestDetail.logic.ts+15| ... | @@ -1339,6 +1339,7 @@ const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { | ... | @@ -1339,6 +1339,7 @@ const ACTION_NEEDS_HOST_REFRESH: Record<PullRequestAction, boolean> = { |
| 1339 | "enable-auto-merge": false, | 1339 | "enable-auto-merge": false, |
| 1340 | "disable-auto-merge": false, | 1340 | "disable-auto-merge": false, |
| 1341 | revert: false, | 1341 | revert: false, |
| 1342 | "delete-branch": false, | ||
| 1342 | "approve-workflows": true, | 1343 | "approve-workflows": true, |
| 1343 | }; | 1344 | }; |
| 1344 | 1345 | ||
| ... | @@ -1346,6 +1347,20 @@ export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): bo | ... | @@ -1346,6 +1347,20 @@ export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): bo |
| 1346 | return ACTION_NEEDS_HOST_REFRESH[action]; | 1347 | return ACTION_NEEDS_HOST_REFRESH[action]; |
| 1347 | } | 1348 | } |
| 1348 | 1349 | ||
| 1350 | export function canDeletePullRequestBranch( | ||
| 1351 | detail: Pick<PullRequestDetail, "state"> & { | ||
| 1352 | readonly capabilities: Pick<PullRequestDetail["capabilities"], "actions" | "deleteBranch">; | ||
| 1353 | readonly viewerPermissions: Pick<PullRequestDetail["viewerPermissions"], "actions">; | ||
| 1354 | }, | ||
| 1355 | ): boolean { | ||
| 1356 | return ( | ||
| 1357 | detail.state !== "open" && | ||
| 1358 | detail.capabilities.deleteBranch === true && | ||
| 1359 | detail.capabilities.actions.includes("delete-branch") && | ||
| 1360 | detail.viewerPermissions.actions.includes("delete-branch") | ||
| 1361 | ); | ||
| 1362 | } | ||
| 1363 | |||
| 1349 | type SnapshotStorage = Pick<Storage, "getItem" | "setItem">; | 1364 | type SnapshotStorage = Pick<Storage, "getItem" | "setItem">; |
| 1350 | 1365 | ||
| 1351 | export function resolvePullRequestReferenceHost( | 1366 | export function resolvePullRequestReferenceHost( |
apps/web/src/components/pullRequest/usePullRequestActions.ts+4| ... | @@ -87,6 +87,7 @@ const ACTION_SUCCESS_LABELS: Record<PullRequestAction, string> = { | ... | @@ -87,6 +87,7 @@ const ACTION_SUCCESS_LABELS: Record<PullRequestAction, string> = { |
| 87 | "enable-auto-merge": "Auto-merge enabled", | 87 | "enable-auto-merge": "Auto-merge enabled", |
| 88 | "disable-auto-merge": "Auto-merge disabled", | 88 | "disable-auto-merge": "Auto-merge disabled", |
| 89 | revert: "Revert pull request opened", | 89 | revert: "Revert pull request opened", |
| 90 | "delete-branch": "Branch deleted", | ||
| 90 | "approve-workflows": "Workflows approved", | 91 | "approve-workflows": "Workflows approved", |
| 91 | }; | 92 | }; |
| 92 | 93 | ||
| ... | @@ -101,6 +102,7 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { | ... | @@ -101,6 +102,7 @@ const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = { |
| 101 | "enable-auto-merge": "Could not enable auto-merge", | 102 | "enable-auto-merge": "Could not enable auto-merge", |
| 102 | "disable-auto-merge": "Could not disable auto-merge", | 103 | "disable-auto-merge": "Could not disable auto-merge", |
| 103 | revert: "Could not open a revert pull request", | 104 | revert: "Could not open a revert pull request", |
| 105 | "delete-branch": "Could not delete this branch", | ||
| 104 | "approve-workflows": "Could not approve workflows", | 106 | "approve-workflows": "Could not approve workflows", |
| 105 | }; | 107 | }; |
| 106 | 108 | ||
| ... | @@ -120,6 +122,8 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { | ... | @@ -120,6 +122,8 @@ const ACTION_FAILURE_HINTS: Record<PullRequestAction, string> = { |
| 120 | "disable-auto-merge": "The host refused it. Check that you have write access to this repository.", | 122 | "disable-auto-merge": "The host refused it. Check that you have write access to this repository.", |
| 121 | revert: | 123 | revert: |
| 122 | "The host refused it. Check that you have write access and that this pull request was merged on the host.", | 124 | "The host refused it. Check that you have write access and that this pull request was merged on the host.", |
| 125 | "delete-branch": | ||
| 126 | "The host refused it. Check that you have write access to the source repository and that the branch has not moved since this page loaded.", | ||
| 123 | "approve-workflows": | 127 | "approve-workflows": |
| 124 | "The host refused it. Check that you have Actions write access and that these workflow runs are still awaiting approval.", | 128 | "The host refused it. Check that you have Actions write access and that these workflow runs are still awaiting approval.", |
| 125 | }; | 129 | }; |
packages/contracts/src/pullRequest.test.ts+15| ... | @@ -220,6 +220,16 @@ describe("leaving a merge for the host to make once it is ready", () => { | ... | @@ -220,6 +220,16 @@ describe("leaving a merge for the host to make once it is ready", () => { |
| 220 | }); | 220 | }); |
| 221 | 221 | ||
| 222 | describe("reverting a merged pull request", () => { | 222 | describe("reverting a merged pull request", () => { |
| 223 | it("carries a standalone branch deletion action", () => { | ||
| 224 | expect( | ||
| 225 | decodeAction({ | ||
| 226 | projectId: "project-1", | ||
| 227 | repository: "acme/web", | ||
| 228 | number: 7, | ||
| 229 | action: "delete-branch", | ||
| 230 | }).action, | ||
| 231 | ).toBe("delete-branch"); | ||
| 232 | }); | ||
| 223 | it("carries the revert action without merge options", () => { | 233 | it("carries the revert action without merge options", () => { |
| 224 | const action = decodeAction({ | 234 | const action = decodeAction({ |
| 225 | projectId: "project-1", | 235 | projectId: "project-1", |
| ... | @@ -261,6 +271,11 @@ describe("PullRequestCapabilities", () => { | ... | @@ -261,6 +271,11 @@ describe("PullRequestCapabilities", () => { |
| 261 | it("decodes a server that says nothing about reactions as a server with none", () => { | 271 | it("decodes a server that says nothing about reactions as a server with none", () => { |
| 262 | expect(decodeCapabilities(base).reactions).toBeUndefined(); | 272 | expect(decodeCapabilities(base).reactions).toBeUndefined(); |
| 263 | }); | 273 | }); |
| 274 | |||
| 275 | it("only offers branch deletion when the server advertises it", () => { | ||
| 276 | expect(decodeCapabilities(base).deleteBranch).toBeUndefined(); | ||
| 277 | expect(decodeCapabilities({ ...base, deleteBranch: true }).deleteBranch).toBe(true); | ||
| 278 | }); | ||
| 264 | }); | 279 | }); |
| 265 | 280 | ||
| 266 | describe("naming the reader as the author to narrow by", () => { | 281 | describe("naming the reader as the author to narrow by", () => { |
packages/contracts/src/pullRequest.ts+2| ... | @@ -83,6 +83,7 @@ export const PullRequestAction = Schema.Literals([ | ... | @@ -83,6 +83,7 @@ export const PullRequestAction = Schema.Literals([ |
| 83 | "draft", | 83 | "draft", |
| 84 | "close", | 84 | "close", |
| 85 | "reopen", | 85 | "reopen", |
| 86 | "delete-branch", | ||
| 86 | /** Bring the base branch's commits into this one, which is what unblocks a stale branch. */ | 87 | /** Bring the base branch's commits into this one, which is what unblocks a stale branch. */ |
| 87 | "update-branch", | 88 | "update-branch", |
| 88 | /** | 89 | /** |
| ... | @@ -418,6 +419,7 @@ export const PullRequestCapabilities = Schema.Struct({ | ... | @@ -418,6 +419,7 @@ export const PullRequestCapabilities = Schema.Struct({ |
| 418 | actions: Schema.Array(PullRequestAction), | 419 | actions: Schema.Array(PullRequestAction), |
| 419 | /** Merge strategies the provider itself offers, before repository settings narrow them. */ | 420 | /** Merge strategies the provider itself offers, before repository settings narrow them. */ |
| 420 | mergeMethods: Schema.Array(PullRequestMergeMethod), | 421 | mergeMethods: Schema.Array(PullRequestMergeMethod), |
| 422 | deleteBranch: Schema.optional(Schema.Boolean), | ||
| 421 | /** | 423 | /** |
| 422 | * How this host can bring a stale branch up to date. Absent where it cannot at all, which is | 424 | * How this host can bring a stale branch up to date. Absent where it cannot at all, which is |
| 423 | * every host that has not said otherwise — so a provider that says nothing offers nothing. | 425 | * every host that has not said otherwise — so a provider that says nothing offers nothing. |
packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts+1| ... | @@ -325,6 +325,7 @@ function actionArgs( | ... | @@ -325,6 +325,7 @@ function actionArgs( |
| 325 | // Never reached: this host does not declare the action, so the service refuses it first. | 325 | // Never reached: this host does not declare the action, so the service refuses it first. |
| 326 | case "revert": | 326 | case "revert": |
| 327 | case "approve-workflows": | 327 | case "approve-workflows": |
| 328 | case "delete-branch": | ||
| 328 | throw new Error(`Azure DevOps pull request action ${action} is unsupported`); | 329 | throw new Error(`Azure DevOps pull request action ${action} is unsupported`); |
| 329 | } | 330 | } |
| 330 | } | 331 | } |
packages/source-control-github/src/client/definition.ts+1| ... | @@ -72,6 +72,7 @@ export const definition = defineSourceControlClient({ | ... | @@ -72,6 +72,7 @@ export const definition = defineSourceControlClient({ |
| 72 | "draft", | 72 | "draft", |
| 73 | "close", | 73 | "close", |
| 74 | "reopen", | 74 | "reopen", |
| 75 | "delete-branch", | ||
| 75 | "update-branch", | 76 | "update-branch", |
| 76 | "enable-auto-merge", | 77 | "enable-auto-merge", |
| 77 | "disable-auto-merge", | 78 | "disable-auto-merge", |
packages/source-control-github/src/server/GitHubPullRequestApi.test.ts+246| ... | @@ -18,6 +18,7 @@ import * as GitHubQuota from "./GitHubQuota.ts"; | ... | @@ -18,6 +18,7 @@ import * as GitHubQuota from "./GitHubQuota.ts"; |
| 18 | import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; | 18 | import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; |
| 19 | import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; | 19 | import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; |
| 20 | import * as GitHubPullRequestApi from "./GitHubPullRequestApi.ts"; | 20 | import * as GitHubPullRequestApi from "./GitHubPullRequestApi.ts"; |
| 21 | import * as GitHubPullRequestProvider from "./GitHubPullRequestProvider.ts"; | ||
| 21 | import { KnownWorkflowRuns } from "./gitHubConditionalChecks.ts"; | 22 | import { KnownWorkflowRuns } from "./gitHubConditionalChecks.ts"; |
| 22 | 23 | ||
| 23 | const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); | 24 | const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); |
| ... | @@ -2226,6 +2227,249 @@ layer("GitHubPullRequestApi.layer", (it) => { | ... | @@ -2226,6 +2227,249 @@ layer("GitHubPullRequestApi.layer", (it) => { |
| 2226 | }), | 2227 | }), |
| 2227 | ); | 2228 | ); |
| 2228 | 2229 | ||
| 2230 | it.effect.each([ | ||
| 2231 | { | ||
| 2232 | description: "base-only write", | ||
| 2233 | basePermission: "WRITE", | ||
| 2234 | sourcePermission: "READ", | ||
| 2235 | headRef: { id: "REF" }, | ||
| 2236 | expected: false, | ||
| 2237 | actionAllowed: false, | ||
| 2238 | }, | ||
| 2239 | { | ||
| 2240 | description: "source-only write", | ||
| 2241 | basePermission: "READ", | ||
| 2242 | sourcePermission: "WRITE", | ||
| 2243 | headRef: { id: "REF" }, | ||
| 2244 | expected: true, | ||
| 2245 | actionAllowed: true, | ||
| 2246 | }, | ||
| 2247 | { | ||
| 2248 | description: "a deleted branch", | ||
| 2249 | basePermission: "READ", | ||
| 2250 | sourcePermission: "WRITE", | ||
| 2251 | headRef: null, | ||
| 2252 | expected: false, | ||
| 2253 | actionAllowed: true, | ||
| 2254 | }, | ||
| 2255 | { | ||
| 2256 | description: "an unavailable source repository", | ||
| 2257 | basePermission: "WRITE", | ||
| 2258 | sourcePermission: null, | ||
| 2259 | headRef: { id: "REF" }, | ||
| 2260 | expected: false, | ||
| 2261 | actionAllowed: false, | ||
| 2262 | }, | ||
| 2263 | ])( | ||
| 2264 | "uses source branch access in both permission reads for $description", | ||
| 2265 | ({ basePermission, sourcePermission, headRef, expected, actionAllowed }) => | ||
| 2266 | Effect.gen(function* () { | ||
| 2267 | const answer = coreResponse({ | ||
| 2268 | headRepository: sourcePermission === null ? null : { viewerPermission: sourcePermission }, | ||
| 2269 | headRef, | ||
| 2270 | }); | ||
| 2271 | answer.data.repository.viewerPermission = basePermission; | ||
| 2272 | mockedExecute.mockReturnValue(Effect.succeed(output(encodeJson(answer)))); | ||
| 2273 | const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; | ||
| 2274 | const input = { cwd: "/w", repository: "acme/web", host: "github.com", number: 7 }; | ||
| 2275 | expect((yield* cli.getViewerAccess(input)).canDeleteBranch).toBe(actionAllowed); | ||
| 2276 | expect((yield* cli.getPullRequestDetail(input)).viewerAccess.canDeleteBranch).toBe( | ||
| 2277 | expected, | ||
| 2278 | ); | ||
| 2279 | for (const [call] of mockedExecute.mock.calls) { | ||
| 2280 | if ( | ||
| 2281 | call.kind === "graphql" && | ||
| 2282 | !call.query.includes("query PullRequestStackMemberships") | ||
| 2283 | ) { | ||
| 2284 | if (call.query.includes("$headRef: String!")) { | ||
| 2285 | expect(call.query).toContain("headRepository { nameWithOwner viewerPermission }"); | ||
| 2286 | expect(call.query).toContain("headRef { id }"); | ||
| 2287 | } else { | ||
| 2288 | expect(call.query).toContain("headRepository { viewerPermission }"); | ||
| 2289 | } | ||
| 2290 | } | ||
| 2291 | } | ||
| 2292 | }), | ||
| 2293 | ); | ||
| 2294 | |||
| 2295 | it.effect("preserves the actual source fork name in provider detail", () => | ||
| 2296 | Effect.gen(function* () { | ||
| 2297 | const answer = coreResponse({ | ||
| 2298 | state: "MERGED", | ||
| 2299 | isCrossRepository: true, | ||
| 2300 | headRepository: { nameWithOwner: "contributor/web-fork", viewerPermission: "WRITE" }, | ||
| 2301 | headRef: { id: "REF" }, | ||
| 2302 | }); | ||
| 2303 | mockedExecute.mockReturnValue(Effect.succeed(output(encodeJson(answer)))); | ||
| 2304 | const provider = yield* GitHubPullRequestProvider.make; | ||
| 2305 | const detail = yield* provider.getChangeRequest({ | ||
| 2306 | cwd: "/w", | ||
| 2307 | repository: "acme/web", | ||
| 2308 | host: "github.com", | ||
| 2309 | number: 7, | ||
| 2310 | }); | ||
| 2311 | expect(detail.headRepositoryNameWithOwner).toBe("contributor/web-fork"); | ||
| 2312 | expect(detail.viewerPermissions.actions).toContain("delete-branch"); | ||
| 2313 | expect(queryAt(0)).toContain("headRepository { nameWithOwner viewerPermission }"); | ||
| 2314 | }), | ||
| 2315 | ); | ||
| 2316 | |||
| 2317 | const branchToDelete = (pullRequest: Readonly<Record<string, unknown>> = {}) => ({ | ||
| 2318 | data: { | ||
| 2319 | repository: { | ||
| 2320 | pullRequest: { | ||
| 2321 | state: "MERGED", | ||
| 2322 | headRefName: "feat/delete", | ||
| 2323 | baseRefName: "main", | ||
| 2324 | headRefOid: "abc123", | ||
| 2325 | headRepository: { id: "FORK_REPO" }, | ||
| 2326 | ...pullRequest, | ||
| 2327 | }, | ||
| 2328 | }, | ||
| 2329 | }, | ||
| 2330 | }); | ||
| 2331 | const sourceBranch = (repository: Readonly<Record<string, unknown>> = {}) => ({ | ||
| 2332 | data: { | ||
| 2333 | node: { | ||
| 2334 | defaultBranchRef: { name: "main" }, | ||
| 2335 | ref: { target: { oid: "abc123" } }, | ||
| 2336 | ...repository, | ||
| 2337 | }, | ||
| 2338 | }, | ||
| 2339 | }); | ||
| 2340 | |||
| 2341 | it.effect.each(["MERGED", "CLOSED"])("deletes the exact remote fork branch of a %s PR", (state) => | ||
| 2342 | Effect.gen(function* () { | ||
| 2343 | route( | ||
| 2344 | ["query PullRequestBranchToDelete", branchToDelete({ state })], | ||
| 2345 | ["query PullRequestSourceBranch", sourceBranch()], | ||
| 2346 | ); | ||
| 2347 | const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; | ||
| 2348 | yield* cli.runPullRequestAction({ | ||
| 2349 | cwd: "/w", | ||
| 2350 | repository: "acme/web", | ||
| 2351 | host: "github.enterprise.test", | ||
| 2352 | number: 7, | ||
| 2353 | action: "delete-branch", | ||
| 2354 | }); | ||
| 2355 | expect(variablesOf("query PullRequestBranchToDelete")).toEqual([ | ||
| 2356 | { owner: "acme", name: "web", number: 7 }, | ||
| 2357 | ]); | ||
| 2358 | expect(variablesOf("query PullRequestSourceBranch")).toEqual([ | ||
| 2359 | { repositoryId: "FORK_REPO", ref: "refs/heads/feat/delete" }, | ||
| 2360 | ]); | ||
| 2361 | expect(variablesOf("updateRefs(")).toEqual([ | ||
| 2362 | { | ||
| 2363 | input: { | ||
| 2364 | repositoryId: "FORK_REPO", | ||
| 2365 | refUpdates: [ | ||
| 2366 | { | ||
| 2367 | name: "refs/heads/feat/delete", | ||
| 2368 | beforeOid: "abc123", | ||
| 2369 | afterOid: "0000000000000000000000000000000000000000", | ||
| 2370 | }, | ||
| 2371 | ], | ||
| 2372 | }, | ||
| 2373 | }, | ||
| 2374 | ]); | ||
| 2375 | expect(mockedExecute.mock.calls.at(-1)?.[0].host).toBe("github.enterprise.test"); | ||
| 2376 | expect(variablesOf("mergePullRequest(")).toEqual([]); | ||
| 2377 | expect(variablesOf("closePullRequest(")).toEqual([]); | ||
| 2378 | }), | ||
| 2379 | ); | ||
| 2380 | |||
| 2381 | it.effect.each([ | ||
| 2382 | { description: "an open PR", head: { state: "OPEN" }, source: {} }, | ||
| 2383 | { description: "a missing source repository", head: { headRepository: null }, source: {} }, | ||
| 2384 | { description: "the base branch", head: { headRefName: "main" }, source: {} }, | ||
| 2385 | { | ||
| 2386 | description: "the default branch", | ||
| 2387 | head: {}, | ||
| 2388 | source: { defaultBranchRef: { name: "feat/delete" } }, | ||
| 2389 | }, | ||
| 2390 | { description: "a missing default branch", head: {}, source: { defaultBranchRef: null } }, | ||
| 2391 | { | ||
| 2392 | description: "a changed branch", | ||
| 2393 | head: {}, | ||
| 2394 | source: { ref: { target: { oid: "new-commit" } } }, | ||
| 2395 | }, | ||
| 2396 | { description: "an unavailable source repository", head: {}, source: null }, | ||
| 2397 | ])("refuses to delete $description", ({ head, source }) => | ||
| 2398 | Effect.gen(function* () { | ||
| 2399 | route( | ||
| 2400 | ["query PullRequestBranchToDelete", branchToDelete(head)], | ||
| 2401 | [ | ||
| 2402 | "query PullRequestSourceBranch", | ||
| 2403 | source === null ? { data: { node: null } } : sourceBranch(source), | ||
| 2404 | ], | ||
| 2405 | ); | ||
| 2406 | const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; | ||
| 2407 | const error = yield* Effect.flip( | ||
| 2408 | cli.runPullRequestAction({ | ||
| 2409 | cwd: "/w", | ||
| 2410 | repository: "acme/web", | ||
| 2411 | host: "github.com", | ||
| 2412 | number: 7, | ||
| 2413 | action: "delete-branch", | ||
| 2414 | }), | ||
| 2415 | ); | ||
| 2416 | expect(error._tag).toBe("GitHubBranchDeletionRefusedError"); | ||
| 2417 | expect(variablesOf("updateRefs(")).toEqual([]); | ||
| 2418 | }), | ||
| 2419 | ); | ||
| 2420 | |||
| 2421 | it.effect("accepts an already deleted source branch", () => | ||
| 2422 | Effect.gen(function* () { | ||
| 2423 | route( | ||
| 2424 | ["query PullRequestBranchToDelete", branchToDelete()], | ||
| 2425 | ["query PullRequestSourceBranch", sourceBranch({ ref: null })], | ||
| 2426 | ); | ||
| 2427 | const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; | ||
| 2428 | yield* cli.runPullRequestAction({ | ||
| 2429 | cwd: "/w", | ||
| 2430 | repository: "acme/web", | ||
| 2431 | host: "github.com", | ||
| 2432 | number: 7, | ||
| 2433 | action: "delete-branch", | ||
| 2434 | }); | ||
| 2435 | expect(variablesOf("updateRefs(")).toEqual([]); | ||
| 2436 | }), | ||
| 2437 | ); | ||
| 2438 | |||
| 2439 | it.effect("keeps the source branch when GitHub refuses deletion", () => | ||
| 2440 | Effect.gen(function* () { | ||
| 2441 | route( | ||
| 2442 | ["query PullRequestBranchToDelete", branchToDelete()], | ||
| 2443 | ["query PullRequestSourceBranch", sourceBranch()], | ||
| 2444 | ); | ||
| 2445 | const answer = mockedExecute.getMockImplementation()!; | ||
| 2446 | mockedExecute.mockImplementation((call) => | ||
| 2447 | call.kind === "graphql" && call.query.includes("updateRefs(") | ||
| 2448 | ? Effect.fail( | ||
| 2449 | new GitHubApi.GitHubApiResponseError({ | ||
| 2450 | host: call.host, | ||
| 2451 | operation: call.operation, | ||
| 2452 | status: 422, | ||
| 2453 | }), | ||
| 2454 | ) | ||
| 2455 | : answer(call), | ||
| 2456 | ); | ||
| 2457 | const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; | ||
| 2458 | const error = yield* Effect.flip( | ||
| 2459 | cli.runPullRequestAction({ | ||
| 2460 | cwd: "/w", | ||
| 2461 | repository: "acme/web", | ||
| 2462 | host: "github.com", | ||
| 2463 | number: 7, | ||
| 2464 | action: "delete-branch", | ||
| 2465 | }), | ||
| 2466 | ); | ||
| 2467 | expect(error._tag).toBe("GitHubApiResponseError"); | ||
| 2468 | expect(variablesOf("mergePullRequest(")).toEqual([]); | ||
| 2469 | expect(variablesOf("closePullRequest(")).toEqual([]); | ||
| 2470 | }), | ||
| 2471 | ); | ||
| 2472 | |||
| 2229 | it.effect.each(["merge", "enable-auto-merge"] as const)( | 2473 | it.effect.each(["merge", "enable-auto-merge"] as const)( |
| 2230 | "removes agent credits from the proposed message for %s", | 2474 | "removes agent credits from the proposed message for %s", |
| 2231 | (action) => | 2475 | (action) => |
| ... | @@ -4103,6 +4347,7 @@ layer("GitHubPullRequestApi.layer", (it) => { | ... | @@ -4103,6 +4347,7 @@ layer("GitHubPullRequestApi.layer", (it) => { |
| 4103 | expect(access).toEqual({ | 4347 | expect(access).toEqual({ |
| 4104 | mergeCapabilities: { merge: true, squash: false, rebase: true }, | 4348 | mergeCapabilities: { merge: true, squash: false, rebase: true }, |
| 4105 | canWrite: false, | 4349 | canWrite: false, |
| 4350 | canDeleteBranch: false, | ||
| 4106 | canTriage: false, | 4351 | canTriage: false, |
| 4107 | canUpdate: true, | 4352 | canUpdate: true, |
| 4108 | didAuthor: true, | 4353 | didAuthor: true, |
| ... | @@ -4213,6 +4458,7 @@ layer("GitHubPullRequestApi.layer", (it) => { | ... | @@ -4213,6 +4458,7 @@ layer("GitHubPullRequestApi.layer", (it) => { |
| 4213 | expect(access).toEqual({ | 4458 | expect(access).toEqual({ |
| 4214 | mergeCapabilities: { merge: true, squash: false, rebase: true }, | 4459 | mergeCapabilities: { merge: true, squash: false, rebase: true }, |
| 4215 | canWrite: false, | 4460 | canWrite: false, |
| 4461 | canDeleteBranch: false, | ||
| 4216 | canTriage: false, | 4462 | canTriage: false, |
| 4217 | canUpdate: true, | 4463 | canUpdate: true, |
| 4218 | didAuthor: true, | 4464 | didAuthor: true, |
packages/source-control-github/src/server/GitHubPullRequestApi.ts+178| ... | @@ -311,7 +311,37 @@ export class GitHubWorkflowApprovalHeadChangedError extends Schema.TaggedError<G | ... | @@ -311,7 +311,37 @@ export class GitHubWorkflowApprovalHeadChangedError extends Schema.TaggedError<G |
| 311 | } | 311 | } |
| 312 | } | 312 | } |
| 313 | 313 | ||
| 314 | export class GitHubBranchDeletionRefusedError extends Schema.TaggedError<GitHubBranchDeletionRefusedError>()( | ||
| 315 | "GitHubBranchDeletionRefusedError", | ||
| 316 | { | ||
| 317 | number: Schema.Int, | ||
| 318 | reason: Schema.Literals([ | ||
| 319 | "open-pr", | ||
| 320 | "stack", | ||
| 321 | "source-unavailable", | ||
| 322 | "unsafe-branch", | ||
| 323 | "head-changed", | ||
| 324 | ]), | ||
| 325 | }, | ||
| 326 | ) { | ||
| 327 | override get message(): string { | ||
| 328 | switch (this.reason) { | ||
| 329 | case "open-pr": | ||
| 330 | return `Close or merge pull request #${this.number} before deleting its source branch.`; | ||
| 331 | case "stack": | ||
| 332 | return "Branch deletion is not supported for a stack."; | ||
| 333 | case "source-unavailable": | ||
| 334 | return `GitHub did not report a complete source branch for pull request #${this.number}.`; | ||
| 335 | case "unsafe-branch": | ||
| 336 | return "The source branch is a default or base branch and cannot be deleted here."; | ||
| 337 | case "head-changed": | ||
| 338 | return "The source branch has new commits and was not deleted."; | ||
| 339 | } | ||
| 340 | } | ||
| 341 | } | ||
| 342 | |||
| 314 | export type GitHubPullRequestApiError = | 343 | export type GitHubPullRequestApiError = |
| 344 | | GitHubBranchDeletionRefusedError | ||
| 315 | | GitHubStackActionError | 345 | | GitHubStackActionError |
| 316 | | GitHubApi.GitHubApiError | 346 | | GitHubApi.GitHubApiError |
| 317 | | GitHubPullRequestReadError | 347 | | GitHubPullRequestReadError |
| ... | @@ -1062,6 +1092,60 @@ const MERGE_PULL_REQUEST_GRAPHQL_MUTATION = `mutation($input: MergePullRequestIn | ... | @@ -1062,6 +1092,60 @@ const MERGE_PULL_REQUEST_GRAPHQL_MUTATION = `mutation($input: MergePullRequestIn |
| 1062 | const ENABLE_AUTO_MERGE_GRAPHQL_MUTATION = `mutation($input: EnablePullRequestAutoMergeInput!) { | 1092 | const ENABLE_AUTO_MERGE_GRAPHQL_MUTATION = `mutation($input: EnablePullRequestAutoMergeInput!) { |
| 1063 | enablePullRequestAutoMerge(input: $input) { clientMutationId } | 1093 | enablePullRequestAutoMerge(input: $input) { clientMutationId } |
| 1064 | }`; | 1094 | }`; |
| 1095 | |||
| 1096 | const BRANCH_TO_DELETE_GRAPHQL_QUERY = `query PullRequestBranchToDelete($owner: String!, $name: String!, $number: Int!) { | ||
| 1097 | repository(owner: $owner, name: $name) { | ||
| 1098 | pullRequest(number: $number) { | ||
| 1099 | state | ||
| 1100 | headRefName | ||
| 1101 | baseRefName | ||
| 1102 | headRefOid | ||
| 1103 | headRepository { id } | ||
| 1104 | } | ||
| 1105 | } | ||
| 1106 | }`; | ||
| 1107 | const decodeBranchToDelete = Schema.decodeUnknownResult( | ||
| 1108 | Schema.fromJsonString( | ||
| 1109 | Schema.Struct({ | ||
| 1110 | data: Schema.Struct({ | ||
| 1111 | repository: Schema.Struct({ | ||
| 1112 | pullRequest: Schema.Struct({ | ||
| 1113 | state: Schema.Literals(["OPEN", "CLOSED", "MERGED"]), | ||
| 1114 | headRefName: Schema.String, | ||
| 1115 | baseRefName: Schema.String, | ||
| 1116 | headRefOid: Schema.String, | ||
| 1117 | headRepository: Schema.NullOr(Schema.Struct({ id: Schema.String })), | ||
| 1118 | }), | ||
| 1119 | }), | ||
| 1120 | }), | ||
| 1121 | }), | ||
| 1122 | ), | ||
| 1123 | ); | ||
| 1124 | const SOURCE_BRANCH_GRAPHQL_QUERY = `query PullRequestSourceBranch($repositoryId: ID!, $ref: String!) { | ||
| 1125 | node(id: $repositoryId) { | ||
| 1126 | ... on Repository { | ||
| 1127 | defaultBranchRef { name } | ||
| 1128 | ref(qualifiedName: $ref) { target { oid } } | ||
| 1129 | } | ||
| 1130 | } | ||
| 1131 | }`; | ||
| 1132 | const decodeSourceBranch = Schema.decodeUnknownResult( | ||
| 1133 | Schema.fromJsonString( | ||
| 1134 | Schema.Struct({ | ||
| 1135 | data: Schema.Struct({ | ||
| 1136 | node: Schema.NullOr( | ||
| 1137 | Schema.Struct({ | ||
| 1138 | defaultBranchRef: Schema.NullOr(Schema.Struct({ name: Schema.String })), | ||
| 1139 | ref: Schema.NullOr(Schema.Struct({ target: Schema.Struct({ oid: Schema.String }) })), | ||
| 1140 | }), | ||
| 1141 | ), | ||
| 1142 | }), | ||
| 1143 | }), | ||
| 1144 | ), | ||
| 1145 | ); | ||
| 1146 | const DELETE_BRANCH_GRAPHQL_MUTATION = `mutation($input: UpdateRefsInput!) { | ||
| 1147 | updateRefs(input: $input) { clientMutationId } | ||
| 1148 | }`; | ||
| 1065 | const DISABLE_AUTO_MERGE_GRAPHQL_MUTATION = `mutation($pullRequestId: ID!) { | 1149 | const DISABLE_AUTO_MERGE_GRAPHQL_MUTATION = `mutation($pullRequestId: ID!) { |
| 1066 | disablePullRequestAutoMerge(input: { pullRequestId: $pullRequestId }) { clientMutationId } | 1150 | disablePullRequestAutoMerge(input: { pullRequestId: $pullRequestId }) { clientMutationId } |
| 1067 | }`; | 1151 | }`; |
| ... | @@ -2494,6 +2578,100 @@ export const make = Effect.gen(function* () { | ... | @@ -2494,6 +2578,100 @@ export const make = Effect.gen(function* () { |
| 2494 | }, | 2578 | }, |
| 2495 | 2579 | ||
| 2496 | runPullRequestAction: (input) => { | 2580 | runPullRequestAction: (input) => { |
| 2581 | if (input.action === "delete-branch") { | ||
| 2582 | return Effect.gen(function* () { | ||
| 2583 | if (input.stackNumber !== undefined) { | ||
| 2584 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2585 | number: input.number, | ||
| 2586 | reason: "stack", | ||
| 2587 | }); | ||
| 2588 | } | ||
| 2589 | const { owner, name } = parseRepositorySelector(input.repository); | ||
| 2590 | const response = yield* graphqlRead({ | ||
| 2591 | cwd: input.cwd, | ||
| 2592 | host: input.host, | ||
| 2593 | operation: "deletePullRequestBranch", | ||
| 2594 | allowReserve: true, | ||
| 2595 | query: BRANCH_TO_DELETE_GRAPHQL_QUERY, | ||
| 2596 | variables: { owner, name, number: input.number }, | ||
| 2597 | decode: decodeBranchToDelete, | ||
| 2598 | }); | ||
| 2599 | const branch = response.data.repository.pullRequest; | ||
| 2600 | if (branch.state === "OPEN") { | ||
| 2601 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2602 | number: input.number, | ||
| 2603 | reason: "open-pr", | ||
| 2604 | }); | ||
| 2605 | } | ||
| 2606 | if ( | ||
| 2607 | branch.headRepository === null || | ||
| 2608 | !branch.headRepository.id || | ||
| 2609 | !branch.headRefName || | ||
| 2610 | !branch.headRefOid | ||
| 2611 | ) { | ||
| 2612 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2613 | number: input.number, | ||
| 2614 | reason: "source-unavailable", | ||
| 2615 | }); | ||
| 2616 | } | ||
| 2617 | if (branch.headRefName === branch.baseRefName) { | ||
| 2618 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2619 | number: input.number, | ||
| 2620 | reason: "unsafe-branch", | ||
| 2621 | }); | ||
| 2622 | } | ||
| 2623 | const ref = `refs/heads/${branch.headRefName}`; | ||
| 2624 | const source = yield* graphqlRead({ | ||
| 2625 | cwd: input.cwd, | ||
| 2626 | host: input.host, | ||
| 2627 | operation: "deletePullRequestBranch", | ||
| 2628 | allowReserve: true, | ||
| 2629 | query: SOURCE_BRANCH_GRAPHQL_QUERY, | ||
| 2630 | variables: { repositoryId: branch.headRepository.id, ref }, | ||
| 2631 | decode: decodeSourceBranch, | ||
| 2632 | }); | ||
| 2633 | const repository = source.data.node; | ||
| 2634 | if (repository === null) { | ||
| 2635 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2636 | number: input.number, | ||
| 2637 | reason: "source-unavailable", | ||
| 2638 | }); | ||
| 2639 | } | ||
| 2640 | if (repository.ref === null) return; | ||
| 2641 | if ( | ||
| 2642 | repository.defaultBranchRef === null || | ||
| 2643 | branch.headRefName === repository.defaultBranchRef.name | ||
| 2644 | ) { | ||
| 2645 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2646 | number: input.number, | ||
| 2647 | reason: "unsafe-branch", | ||
| 2648 | }); | ||
| 2649 | } | ||
| 2650 | if (repository.ref.target.oid !== branch.headRefOid) { | ||
| 2651 | return yield* new GitHubBranchDeletionRefusedError({ | ||
| 2652 | number: input.number, | ||
| 2653 | reason: "head-changed", | ||
| 2654 | }); | ||
| 2655 | } | ||
| 2656 | yield* graphql({ | ||
| 2657 | host: input.host, | ||
| 2658 | operation: "deletePullRequestBranch", | ||
| 2659 | query: DELETE_BRANCH_GRAPHQL_MUTATION, | ||
| 2660 | variables: { | ||
| 2661 | input: { | ||
| 2662 | repositoryId: branch.headRepository.id, | ||
| 2663 | refUpdates: [ | ||
| 2664 | { | ||
| 2665 | name: ref, | ||
| 2666 | beforeOid: branch.headRefOid, | ||
| 2667 | afterOid: "0000000000000000000000000000000000000000", | ||
| 2668 | }, | ||
| 2669 | ], | ||
| 2670 | }, | ||
| 2671 | }, | ||
| 2672 | }); | ||
| 2673 | }); | ||
| 2674 | } | ||
| 2497 | if (input.stackNumber !== undefined) | 2675 | if (input.stackNumber !== undefined) |
| 2498 | return runGitHubStackAction({ ...input, stackNumber: input.stackNumber }).pipe( | 2676 | return runGitHubStackAction({ ...input, stackNumber: input.stackNumber }).pipe( |
| 2499 | Effect.provideService(GitHubApi.GitHubApi, api), | 2677 | Effect.provideService(GitHubApi.GitHubApi, api), |
packages/source-control-github/src/server/GitHubPullRequestProvider.test.ts+44| ... | @@ -64,6 +64,48 @@ it.effect("maps credential verification failures without relabeling operation fa | ... | @@ -64,6 +64,48 @@ it.effect("maps credential verification failures without relabeling operation fa |
| 64 | }), | 64 | }), |
| 65 | ); | 65 | ); |
| 66 | 66 | ||
| 67 | it.effect("advertises standalone branch deletion for source repository writers", () => | ||
| 68 | Effect.gen(function* () { | ||
| 69 | let action: string | undefined; | ||
| 70 | const provider = yield* make.pipe( | ||
| 71 | Effect.provide( | ||
| 72 | Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({ | ||
| 73 | runPullRequestAction: (input) => | ||
| 74 | Effect.sync(() => { | ||
| 75 | action = input.action; | ||
| 76 | }), | ||
| 77 | }), | ||
| 78 | ), | ||
| 79 | ); | ||
| 80 | expect(provider.capabilities.deleteBranch).toBe(true); | ||
| 81 | expect(provider.capabilities.actions).toContain("delete-branch"); | ||
| 82 | yield* provider.runAction({ | ||
| 83 | cwd: "/w", | ||
| 84 | repository: "acme/web", | ||
| 85 | host: "github.com", | ||
| 86 | number: 7, | ||
| 87 | action: "delete-branch", | ||
| 88 | }); | ||
| 89 | expect(action).toBe("delete-branch"); | ||
| 90 | const access = { | ||
| 91 | canWrite: false, | ||
| 92 | canTriage: false, | ||
| 93 | canUpdate: false, | ||
| 94 | didAuthor: false, | ||
| 95 | canDeleteBranch: true, | ||
| 96 | }; | ||
| 97 | expect(gitHubViewerPermissions(access).actions).toContain("delete-branch"); | ||
| 98 | expect( | ||
| 99 | gitHubViewerPermissions({ | ||
| 100 | ...access, | ||
| 101 | canWrite: true, | ||
| 102 | didAuthor: true, | ||
| 103 | canDeleteBranch: false, | ||
| 104 | }).actions, | ||
| 105 | ).not.toContain("delete-branch"); | ||
| 106 | }), | ||
| 107 | ); | ||
| 108 | |||
| 67 | it.effect("refreshes checks without permissions or comparison reads", () => | 109 | it.effect("refreshes checks without permissions or comparison reads", () => |
| 68 | Effect.gen(function* () { | 110 | Effect.gen(function* () { |
| 69 | let reads = 0; | 111 | let reads = 0; |
| ... | @@ -219,6 +261,7 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -219,6 +261,7 @@ describe("gitHubViewerPermissions", () => { |
| 219 | expect( | 261 | expect( |
| 220 | gitHubViewerPermissions({ | 262 | gitHubViewerPermissions({ |
| 221 | canWrite: true, | 263 | canWrite: true, |
| 264 | canDeleteBranch: true, | ||
| 222 | canTriage: true, | 265 | canTriage: true, |
| 223 | canUpdate: true, | 266 | canUpdate: true, |
| 224 | didAuthor: false, | 267 | didAuthor: false, |
| ... | @@ -226,6 +269,7 @@ describe("gitHubViewerPermissions", () => { | ... | @@ -226,6 +269,7 @@ describe("gitHubViewerPermissions", () => { |
| 226 | ).toEqual({ | 269 | ).toEqual({ |
| 227 | // Arming a merge for later is the merge, so it travels with it. | 270 | // Arming a merge for later is the merge, so it travels with it. |
| 228 | actions: [ | 271 | actions: [ |
| 272 | "delete-branch", | ||
| 229 | "merge", | 273 | "merge", |
| 230 | "enable-auto-merge", | 274 | "enable-auto-merge", |
| 231 | "disable-auto-merge", | 275 | "disable-auto-merge", |
packages/source-control-github/src/server/GitHubPullRequestProvider.ts+2| ... | @@ -27,6 +27,7 @@ const CAPABILITIES: PullRequestCapabilities = { | ... | @@ -27,6 +27,7 @@ const CAPABILITIES: PullRequestCapabilities = { |
| 27 | comment: true, | 27 | comment: true, |
| 28 | actions: [...definition.changeRequestActions], | 28 | actions: [...definition.changeRequestActions], |
| 29 | mergeMethods: ["merge", "squash", "rebase"], | 29 | mergeMethods: ["merge", "squash", "rebase"], |
| 30 | deleteBranch: true, | ||
| 30 | updateMethods: ["merge", "rebase"], | 31 | updateMethods: ["merge", "rebase"], |
| 31 | search: true, | 32 | search: true, |
| 32 | reactions: true, | 33 | reactions: true, |
| ... | @@ -65,6 +66,7 @@ export function gitHubViewerPermissions(access: GitHubViewerAccess): PullRequest | ... | @@ -65,6 +66,7 @@ export function gitHubViewerPermissions(access: GitHubViewerAccess): PullRequest |
| 65 | return { | 66 | return { |
| 66 | ...(access.canWrite ? { stackRebase: true } : {}), | 67 | ...(access.canWrite ? { stackRebase: true } : {}), |
| 67 | actions: [ | 68 | actions: [ |
| 69 | ...(access.canDeleteBranch === true ? (["delete-branch"] as const) : []), | ||
| 68 | // Arming a merge and taking the arming back are the merge, deferred: whoever may not | 70 | // Arming a merge and taking the arming back are the merge, deferred: whoever may not |
| 69 | // merge here may not leave an instruction to merge later either. | 71 | // merge here may not leave an instruction to merge later either. |
| 70 | ...(access.canWrite | 72 | ...(access.canWrite |
packages/source-control-github/src/server/gitHubPullRequestJson.test.ts+3| ... | @@ -1055,6 +1055,7 @@ describe("viewer permission decoding", () => { | ... | @@ -1055,6 +1055,7 @@ describe("viewer permission decoding", () => { |
| 1055 | canTriage: false, | 1055 | canTriage: false, |
| 1056 | canUpdate: true, | 1056 | canUpdate: true, |
| 1057 | didAuthor: true, | 1057 | didAuthor: true, |
| 1058 | canDeleteBranch: false, | ||
| 1058 | }); | 1059 | }); |
| 1059 | }); | 1060 | }); |
| 1060 | 1061 | ||
| ... | @@ -1074,6 +1075,7 @@ describe("viewer permission decoding", () => { | ... | @@ -1074,6 +1075,7 @@ describe("viewer permission decoding", () => { |
| 1074 | canTriage: false, | 1075 | canTriage: false, |
| 1075 | canUpdate: false, | 1076 | canUpdate: false, |
| 1076 | didAuthor: false, | 1077 | didAuthor: false, |
| 1078 | canDeleteBranch: false, | ||
| 1077 | }); | 1079 | }); |
| 1078 | }); | 1080 | }); |
| 1079 | 1081 | ||
| ... | @@ -1087,6 +1089,7 @@ describe("viewer permission decoding", () => { | ... | @@ -1087,6 +1089,7 @@ describe("viewer permission decoding", () => { |
| 1087 | canTriage: false, | 1089 | canTriage: false, |
| 1088 | canUpdate: true, | 1090 | canUpdate: true, |
| 1089 | didAuthor: false, | 1091 | didAuthor: false, |
| 1092 | canDeleteBranch: false, | ||
| 1090 | }); | 1093 | }); |
| 1091 | }); | 1094 | }); |
| 1092 | 1095 |
packages/source-control-github/src/server/gitHubPullRequestJson.ts+27-2| ... | @@ -558,6 +558,18 @@ const RawViewerFieldsSchema = Schema.Struct({ | ... | @@ -558,6 +558,18 @@ const RawViewerFieldsSchema = Schema.Struct({ |
| 558 | viewerDidAuthor: Schema.optional(Schema.Boolean), | 558 | viewerDidAuthor: Schema.optional(Schema.Boolean), |
| 559 | }); | 559 | }); |
| 560 | 560 | ||
| 561 | const RawHeadBranchAccessSchema = Schema.Struct({ | ||
| 562 | headRepository: Schema.optional( | ||
| 563 | Schema.NullOr( | ||
| 564 | Schema.Struct({ | ||
| 565 | nameWithOwner: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 566 | viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), | ||
| 567 | }), | ||
| 568 | ), | ||
| 569 | ), | ||
| 570 | headRef: Schema.optional(Schema.NullOr(Schema.Struct({ id: Schema.String }))), | ||
| 571 | }); | ||
| 572 | |||
| 561 | const RawThreadCommentsSchema = Schema.Struct({ | 573 | const RawThreadCommentsSchema = Schema.Struct({ |
| 562 | totalCount: Schema.optional(Schema.Int), | 574 | totalCount: Schema.optional(Schema.Int), |
| 563 | pageInfo: Schema.optional(RawPageInfoSchema), | 575 | pageInfo: Schema.optional(RawPageInfoSchema), |
| ... | @@ -722,6 +734,7 @@ const RawCoreSchema = Schema.Struct({ | ... | @@ -722,6 +734,7 @@ const RawCoreSchema = Schema.Struct({ |
| 722 | pullRequest: Schema.Struct({ | 734 | pullRequest: Schema.Struct({ |
| 723 | ...RawDetailSchema.fields, | 735 | ...RawDetailSchema.fields, |
| 724 | ...RawViewerFieldsSchema.fields, | 736 | ...RawViewerFieldsSchema.fields, |
| 737 | ...RawHeadBranchAccessSchema.fields, | ||
| 725 | viewerCanUpdateBranch: Schema.Boolean, | 738 | viewerCanUpdateBranch: Schema.Boolean, |
| 726 | baseRef: Schema.NullOr( | 739 | baseRef: Schema.NullOr( |
| 727 | Schema.Struct({ | 740 | Schema.Struct({ |
| ... | @@ -842,6 +855,8 @@ export const pullRequestCoreGraphQlQuery = (host: string) => { | ... | @@ -842,6 +855,8 @@ export const pullRequestCoreGraphQlQuery = (host: string) => { |
| 842 | additions deletions changedFiles createdAt updatedAt mergedAt closedAt | 855 | additions deletions changedFiles createdAt updatedAt mergedAt closedAt |
| 843 | headRefName baseRefName headRefOid isCrossRepository | 856 | headRefName baseRefName headRefOid isCrossRepository |
| 844 | headRepositoryOwner { login } | 857 | headRepositoryOwner { login } |
| 858 | headRepository { nameWithOwner viewerPermission } | ||
| 859 | headRef { id } | ||
| 845 | author { login avatarUrl ... on User { id name } } | 860 | author { login avatarUrl ... on User { id name } } |
| 846 | autoMergeRequest { mergeMethod } | 861 | autoMergeRequest { mergeMethod } |
| 847 | viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch | 862 | viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch |
| ... | @@ -1353,6 +1368,7 @@ export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { | ... | @@ -1353,6 +1368,7 @@ export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { |
| 1353 | readonly isCrossRepository?: boolean; | 1368 | readonly isCrossRepository?: boolean; |
| 1354 | /** The owner of the head branch's repository; null where `gh` did not say. */ | 1369 | /** The owner of the head branch's repository; null where `gh` did not say. */ |
| 1355 | readonly headRepositoryOwner: string | null; | 1370 | readonly headRepositoryOwner: string | null; |
| 1371 | readonly headRepositoryNameWithOwner?: string | null; | ||
| 1356 | readonly headSha?: string | null; | 1372 | readonly headSha?: string | null; |
| 1357 | readonly body: string; | 1373 | readonly body: string; |
| 1358 | readonly changedFiles: number; | 1374 | readonly changedFiles: number; |
| ... | @@ -2358,8 +2374,10 @@ export function decodePullRequestCoreJson( | ... | @@ -2358,8 +2374,10 @@ export function decodePullRequestCoreJson( |
| 2358 | labels: pr.labels.nodes, | 2374 | labels: pr.labels.nodes, |
| 2359 | statusCheckRollup: toCheckContexts(contexts?.nodes ?? []), | 2375 | statusCheckRollup: toCheckContexts(contexts?.nodes ?? []), |
| 2360 | }), | 2376 | }), |
| 2377 | headRepositoryNameWithOwner: trimmed(pr.headRepository?.nameWithOwner), | ||
| 2361 | viewerAccess: { | 2378 | viewerAccess: { |
| 2362 | canWrite: toCanWrite(repository.viewerPermission), | 2379 | canWrite: toCanWrite(repository.viewerPermission), |
| 2380 | canDeleteBranch: toCanWrite(pr.headRepository?.viewerPermission) && pr.headRef !== null, | ||
| 2363 | canTriage: toCanTriage(repository.viewerPermission), | 2381 | canTriage: toCanTriage(repository.viewerPermission), |
| 2364 | ...toPullRequestViewerFields(pr), | 2382 | ...toPullRequestViewerFields(pr), |
| 2365 | mergeCapabilities: { | 2383 | mergeCapabilities: { |
| ... | @@ -3181,6 +3199,7 @@ export function buildLabelRequest(labels: ReadonlyArray<string>): { | ... | @@ -3181,6 +3199,7 @@ export function buildLabelRequest(labels: ReadonlyArray<string>): { |
| 3181 | */ | 3199 | */ |
| 3182 | export interface GitHubViewerAccess { | 3200 | export interface GitHubViewerAccess { |
| 3183 | readonly canWrite: boolean; | 3201 | readonly canWrite: boolean; |
| 3202 | readonly canDeleteBranch?: boolean; | ||
| 3184 | /** | 3203 | /** |
| 3185 | * The viewer's role reaches triage, which is the least that may label. Everyone who can write | 3204 | * The viewer's role reaches triage, which is the least that may label. Everyone who can write |
| 3186 | * can triage; a triager is the one role that can label without being able to merge. | 3205 | * can triage; a triager is the one role that can label without being able to merge. |
| ... | @@ -3201,7 +3220,10 @@ export interface GitHubViewerAccess { | ... | @@ -3201,7 +3220,10 @@ export interface GitHubViewerAccess { |
| 3201 | export const VIEWER_PERMISSIONS_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { | 3220 | export const VIEWER_PERMISSIONS_GRAPHQL_QUERY = `query($owner: String!, $name: String!, $number: Int!) { |
| 3202 | repository(owner: $owner, name: $name) { | 3221 | repository(owner: $owner, name: $name) { |
| 3203 | mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission | 3222 | mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission |
| 3204 | pullRequest(number: $number) { viewerCanUpdate viewerDidAuthor } | 3223 | pullRequest(number: $number) { |
| 3224 | viewerCanUpdate viewerDidAuthor | ||
| 3225 | headRepository { viewerPermission } | ||
| 3226 | } | ||
| 3205 | } | 3227 | } |
| 3206 | }`; | 3228 | }`; |
| 3207 | 3229 | ||
| ... | @@ -3210,7 +3232,9 @@ const RawViewerPermissionsSchema = Schema.Struct({ | ... | @@ -3210,7 +3232,9 @@ const RawViewerPermissionsSchema = Schema.Struct({ |
| 3210 | repository: Schema.Struct({ | 3232 | repository: Schema.Struct({ |
| 3211 | ...RawRepositoryAccessSchema.fields, | 3233 | ...RawRepositoryAccessSchema.fields, |
| 3212 | /** Null for a number that names no pull request the viewer can see. */ | 3234 | /** Null for a number that names no pull request the viewer can see. */ |
| 3213 | pullRequest: Schema.NullOr(RawViewerFieldsSchema), | 3235 | pullRequest: Schema.NullOr( |
| 3236 | Schema.Struct({ ...RawViewerFieldsSchema.fields, ...RawHeadBranchAccessSchema.fields }), | ||
| 3237 | ), | ||
| 3214 | }), | 3238 | }), |
| 3215 | }), | 3239 | }), |
| 3216 | }); | 3240 | }); |
| ... | @@ -3232,6 +3256,7 @@ export function decodeViewerPermissionsJson( | ... | @@ -3232,6 +3256,7 @@ export function decodeViewerPermissionsJson( |
| 3232 | rebase: repository.rebaseMergeAllowed, | 3256 | rebase: repository.rebaseMergeAllowed, |
| 3233 | }, | 3257 | }, |
| 3234 | canWrite: toCanWrite(repository.viewerPermission), | 3258 | canWrite: toCanWrite(repository.viewerPermission), |
| 3259 | canDeleteBranch: toCanWrite(repository.pullRequest?.headRepository?.viewerPermission), | ||
| 3235 | canTriage: toCanTriage(repository.viewerPermission), | 3260 | canTriage: toCanTriage(repository.viewerPermission), |
| 3236 | ...toPullRequestViewerFields(repository.pullRequest), | 3261 | ...toPullRequestViewerFields(repository.pullRequest), |
| 3237 | }); | 3262 | }); |
packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts+1| ... | @@ -524,6 +524,7 @@ function actionArgs( | ... | @@ -524,6 +524,7 @@ function actionArgs( |
| 524 | // Never reached: this host does not declare the action, so the service refuses it first. | 524 | // Never reached: this host does not declare the action, so the service refuses it first. |
| 525 | case "revert": | 525 | case "revert": |
| 526 | case "approve-workflows": | 526 | case "approve-workflows": |
| 527 | case "delete-branch": | ||
| 527 | throw new Error(`GitLab merge request action ${action} is unsupported`); | 528 | throw new Error(`GitLab merge request action ${action} is unsupported`); |
| 528 | } | 529 | } |
| 529 | } | 530 | } |