fixup! feat(ng-dev): create pr takeover command

Author: josephperrott

ng-dev/BUILD.bazel

@@ -58,7 +58,7 @@ esbuild_esm_bundle(
         # to launch these files/scripts dynamically (through e.g. `spawn` or `fork`).
         "//ng-dev/release/build:build-worker.ts",
         "//ng-dev/pr/merge:strategies/commit-message-filter.ts",
-        "//ng-dev/pr/takeover-pr:commit-message-filter.ts",
+        "//ng-dev/pr/checkout:commit-message-filter.ts",
     ],
     external = NG_DEV_EXTERNALS,
     splitting = True,

ng-dev/pr/BUILD.bazel

@@ -10,7 +10,6 @@ ts_library(
         "//ng-dev/pr/discover-new-conflicts",
         "//ng-dev/pr/merge",
         "//ng-dev/pr/rebase",
-        "//ng-dev/pr/takeover-pr",
         "@npm//@types/yargs",
     ],
 )

ng-dev/pr/checkout/BUILD.bazel

@@ -1,12 +1,17 @@
 load("//tools:defaults.bzl", "ts_library")
 
+exports_files([
+    "commit-message-filter.ts",
+])
+
 ts_library(
     name = "checkout",
     srcs = glob(["*.ts"]),
     visibility = ["//ng-dev:__subpackages__"],
     deps = [
         "//ng-dev/pr/common",
         "//ng-dev/utils",
+        "@npm//@types/node",
         "@npm//@types/yargs",
     ],
 )

ng-dev/pr/checkout/checkout.ts

@@ -0,0 +1,101 @@
+import {dirname, join} from 'path';
+import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client.js';
+import {Prompt} from '../../utils/prompt.js';
+import {Log, bold, green} from '../../utils/logging.js';
+import {checkOutPullRequestLocally} from '../common/checkout-pr.js';
+import {fileURLToPath} from 'url';
+
+/** List of accounts that are supported for takeover. */
+const takeoverAccounts = ['angular-robot'];
+
+export interface CheckoutPullRequestParams {
+  pr: number;
+  takeover?: boolean;
+}
+
+export async function checkoutPullRequest(params: CheckoutPullRequestParams): Promise<void> {
+  const {pr, takeover} = params;
+  /** An authenticated git client. */
+  const git = await AuthenticatedGitClient.get();
+  /** The branch name used for the takeover change. */
+  const branchName = `pr-takeover-${pr}`;
+
+  // Make sure the local repository is clean.
+  if (git.hasUncommittedChanges()) {
+    Log.error(
+      ` ✘ Local working repository not clean. Please make sure there are no uncommitted changes`,
+    );
+    return;
+  }
+
+  const {resetGitState, pullRequest, pushToUpstreamCommand} = await checkOutPullRequestLocally(pr, {
+    allowIfMaintainerCannotModify: true,
+  });
+
+  // if maintainer can modify is false or if takeover is provided do takeover
+
+  if (pullRequest.maintainerCanModify === false || takeover) {
+    if (takeover !== true) {
+      Log.info('The author of this pull request does not allow maintainers to modify the pull');
+      Log.info('request. Since you will not be able to push changes to the original pull request');
+      Log.info('you will instead need to perform a "takeover." In a takeover the original pull');
+      Log.info('request will be checked out, the commits are modified to close the original on');
+      Log.info('merge of the newly created branch.\n');
+
+      if (!(await Prompt.confirm(`Would you like to create a takeover pull request?`, true))) {
+        Log.info('Aborting takeover..');
+        await resetGitState();
+        return;
+      }
+    }
+
+    if (git.runGraceful(['rev-parse', '-q', '--verify', branchName]).status === 0) {
+      Log.error(` ✘ Expected branch name \`${branchName}\` already exists locally`);
+      return;
+    }
+
+    // Confirm that the takeover request is being done on a valid pull request.
+    if (!takeoverAccounts.includes(pullRequest.author.login)) {
+      Log.warn(
+        ` ⚠ ${bold(pullRequest.author.login)} is not an account fully supported for takeover.`,
+      );
+      Log.warn(`   Supported accounts: ${bold(takeoverAccounts.join(', '))}`);
+      if (await Prompt.confirm(`Continue with pull request takeover anyway?`, true)) {
+        Log.debug('Continuing per user confirmation in prompt');
+      } else {
+        Log.info('Aborting takeover..');
+        await resetGitState();
+        return;
+      }
+    }
+
+    Log.info(`Setting local branch name based on the pull request`);
+    git.run(['checkout', '-q', '-b', branchName]);
+
+    Log.info('Updating commit messages to close previous pull request');
+    git.run([
+      'filter-branch',
+      '-f',
+      '--msg-filter',
+      `${getCommitMessageFilterScriptPath()} ${pr}`,
+      `${pullRequest.baseRefOid}..HEAD`,
+    ]);
+
+    Log.info(` ${green('✔')} Checked out pull request #${pr} into branch: ${branchName}`);
+    return;
+  }
+
+  Log.info(`Checked out the remote branch for pull request #${pr}\n`);
+  Log.info('To push the checked out branch back to its PR, run the following command:');
+  Log.info(`  $ ${pushToUpstreamCommand}`);
+}
+
+/** Gets the absolute file path to the commit-message filter script. */
+function getCommitMessageFilterScriptPath(): string {
+  // This file is getting bundled and ends up in `<pkg-root>/bundles/<chunk>`. We also
+  // bundle the commit-message-filter script as another entry-point and can reference
+  // it relatively as the path is preserved inside `bundles/`.
+  // *Note*: Relying on package resolution is problematic within ESM and with `local-dev.sh`
+  const bundlesDir = dirname(fileURLToPath(import.meta.url));
+  return join(bundlesDir, './pr/checkout/commit-message-filter.mjs');
+}

ng-dev/pr/checkout/cli.ts

@@ -7,31 +7,32 @@
  */
 
 import {Argv, Arguments, CommandModule} from 'yargs';
-import {Log} from '../../utils/logging.js';
 
 import {addGithubTokenOption} from '../../utils/git/github-yargs.js';
-import {checkOutPullRequestLocally} from '../common/checkout-pr.js';
-
-export interface CheckoutOptions {
-  pr: number;
-}
+import {checkoutPullRequest, CheckoutPullRequestParams} from './checkout.js';
 
 /** Builds the checkout pull request command. */
 function builder(yargs: Argv) {
-  return addGithubTokenOption(yargs).positional('pr', {type: 'number', demandOption: true});
+  return addGithubTokenOption(yargs)
+    .positional('pr', {
+      type: 'number',
+      demandOption: true,
+      describe: 'The pull request number for the pull request to checkout',
+    })
+    .option('takeover', {
+      type: 'boolean',
+      demandOption: false,
+      describe: 'Check out the pull request to perform a takeover',
+    });
 }
 
 /** Handles the checkout pull request command. */
-async function handler({pr}: Arguments<CheckoutOptions>) {
-  const options = {allowIfMaintainerCannotModify: true, branchName: `pr-${pr}`};
-  const {pushToUpstreamCommand} = await checkOutPullRequestLocally(pr, options);
-  Log.info(`Checked out the remote branch for pull request #${pr}\n`);
-  Log.info('To push the checked out branch back to its PR, run the following command:');
-  Log.info(`  $ ${pushToUpstreamCommand}`);
+async function handler({pr, takeover}: Arguments<CheckoutPullRequestParams>) {
+  await checkoutPullRequest({pr, takeover});
 }
 
 /** yargs command module for checking out a PR  */
-export const CheckoutCommandModule: CommandModule<{}, CheckoutOptions> = {
+export const CheckoutCommandModule: CommandModule<{}, CheckoutPullRequestParams> = {
   handler,
   builder,
   command: 'checkout <pr>',

ng-dev/pr/takeover-pr/commit-message-filter.ts renamed to ng-dev/pr/checkout/commit-message-filter.ts

@@ -39,6 +39,6 @@ function main() {
 
 function rewriteCommitMessage(message: string, prNumber: string) {
   const lines = message.split(/\n/);
-  lines.push(`Closes #${prNumber} as this was created using the \`ng-dev pr takeover\` tooling`);
+  lines.push(`Closes #${prNumber} as a pr takeover`);
   return lines.join('\n');
 }

ng-dev/pr/cli.ts

@@ -13,7 +13,6 @@ import {CheckoutCommandModule} from './checkout/cli.js';
 import {DiscoverNewConflictsCommandModule} from './discover-new-conflicts/cli.js';
 import {MergeCommandModule} from './merge/cli.js';
 import {RebaseCommandModule} from './rebase/cli.js';
-import {TakeoverPrCommandModule} from './takeover-pr/cli.js';
 
 /** Build the parser for pull request commands. */
 export function buildPrParser(localYargs: Argv) {
@@ -25,6 +24,5 @@ export function buildPrParser(localYargs: Argv) {
     .command(RebaseCommandModule)
     .command(MergeCommandModule)
     .command(CheckoutCommandModule)
-    .command(TakeoverPrCommandModule)
     .command(CheckTargetBranchesModule);
 }

ng-dev/pr/common/checkout-pr.ts

@@ -8,13 +8,15 @@
 
 import {types as graphqlTypes} from 'typed-graphqlify';
 
-import {Log} from '../../utils/logging.js';
 import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client.js';
 import {addTokenToGitHttpsUrl} from '../../utils/git/github-urls.js';
 import {getPr} from '../../utils/github.js';
 
 /* Graphql schema for the response body for a pending PR. */
 const PR_SCHEMA = {
+  author: {
+    login: graphqlTypes.string,
+  },
   state: graphqlTypes.string,
   maintainerCanModify: graphqlTypes.boolean,
   viewerDidAuthor: graphqlTypes.boolean,
@@ -26,6 +28,7 @@ const PR_SCHEMA = {
       nameWithOwner: graphqlTypes.string,
     },
   },
+  baseRefOid: graphqlTypes.string,
   baseRef: {
     name: graphqlTypes.string,
     repository: {
@@ -107,7 +110,6 @@ export async function checkOutPullRequestLocally(
 
   try {
     // Fetch the branch at the commit of the PR, and check it out in a detached state.
-    Log.info(`Checking out PR #${prNumber} from ${fullHeadRef}`);
     git.run(['fetch', '-q', headRefUrl, headRefName]);
     git.run(['checkout', '--detach', 'FETCH_HEAD']);
   } catch (e) {
@@ -132,5 +134,6 @@ export async function checkOutPullRequestLocally(
     },
     pushToUpstreamCommand: `git push ${pr.headRef.repository.url} HEAD:${headRefName} ${forceWithLeaseFlag}`,
     resetGitStateCommand: `git rebase --abort && git reset --hard && git checkout ${previousBranchOrRevision}`,
+    pullRequest: pr,
   };
 }