feat: octokit error typing, bot as flag

salesforcecli · Sep 14, 2022 · 367ea6c · 367ea6c
1 parent 7125730
commit 367ea6c
Show file tree

Hide file tree

Showing 10 changed files with 93 additions and 320 deletions.
diff --git a/messages/configure.repo.md b/messages/configure.repo.md
@@ -17,3 +17,7 @@ Make no changes
 # examples
 
 - <%= config.bin %> <%= command.id %> -r salesforcecli/testPackageRelease
+
+# flags.bot.summary
+
+The github login/username for the bot.
diff --git a/package.json b/package.json
@@ -6,14 +6,14 @@
   "bugs": "https://github.com/forcedotcom/cli/issues",
   "dependencies": {
     "@oclif/core": "^1.16.1",
+    "@octokit/rest": "^19.0.4",
     "@salesforce/core": "^3.26.3",
     "@salesforce/kit": "^1.6.0",
     "@salesforce/sf-plugins-core": "^1.14.1",
     "change-case": "^4.1.2",
     "fast-glob": "^3.2.12",
     "got": "^11.8.5",
     "js-yaml": "^4.1.0",
-    "octokit": "^2.0.7",
     "replace-in-file": "^6.3.2",
     "shelljs": "^0.8.5",
     "tslib": "^2",
@@ -129,4 +129,4 @@
   "publishConfig": {
     "access": "public"
   }
-}
+}
diff --git a/plugin-env b/plugin-env
diff --git a/plugin-org b/plugin-org
diff --git a/sfdx-core b/sfdx-core
diff --git a/src/commands/configure/repo.ts b/src/commands/configure/repo.ts
@@ -5,9 +5,12 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+// because github api isn't camelcased
+/* eslint-disable camelcase */
 import { SfCommand, Flags } from '@salesforce/sf-plugins-core';
 import { Messages } from '@salesforce/core';
-import { Octokit } from 'octokit';
+import { Octokit } from '@octokit/rest';
+import { OctokitError } from '../../types';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.load('@salesforce/plugin-dev', 'configure.repo', [
@@ -16,6 +19,7 @@ const messages = Messages.load('@salesforce/plugin-dev', 'configure.repo', [
   'examples',
   'flags.repository.summary',
   'flags.dryRun.summary',
+  'flags.bot.summary',
 ]);
 
 export type ConfigureRepoResult = {
@@ -25,8 +29,6 @@ export type ConfigureRepoResult = {
   prBypass: boolean;
 };
 
-const BOT_LOGIN = 'SF-CLI-BOT';
-
 export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
   public static summary = messages.getMessage('summary');
   public static description = messages.getMessage('description');
@@ -42,6 +44,11 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
       summary: messages.getMessage('flags.dryRun.summary'),
       char: 'c',
     }),
+    bot: Flags.string({
+      summary: messages.getMessage('flags.bot.summary'),
+      char: 'b',
+      default: 'SF-CLI-BOT',
+    }),
   };
 
   public async run(): Promise<ConfigureRepoResult> {
@@ -62,45 +69,46 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
     // assertion : bot has write+ access to repo
     const { data: permission } = await octokit.rest.repos.getCollaboratorPermissionLevel({
       ...repoBase,
-      username: BOT_LOGIN,
+      username: flags.bot,
     });
     if (permission.permission !== 'admin') {
       this.error(
-        `${BOT_LOGIN}  does not have "admin"  access to ${flags.repository}.  No further inspection is possible.`
+        `${flags.bot}  does not have "admin"  access to ${flags.repository}.  No further inspection is possible.`
       );
     } else {
-      this.logSuccess(`✓ ${BOT_LOGIN} has necessary access to ${flags.repository}`);
+      this.logSuccess(`✓ ${flags.bot} has necessary access to ${flags.repository}`);
       output.botAccess = true;
     }
 
     try {
       await octokit.rest.repos.getBranchProtection({ ...repoBase, branch: 'main' });
     } catch (e) {
-      if (e.response.data) {
-        if (!flags['dry-run'] && e.response.data.message === 'Branch not protected') {
+      const typedError = e as OctokitError;
+      if (typedError) {
+        if (!flags['dry-run'] && typedError.response.data.message === 'Branch not protected') {
           this.log('setting up basic branch protection');
 
           await octokit.rest.repos.updateBranchProtection({
             ...repoBase,
             branch: 'main',
             required_pull_request_reviews: {
               bypass_pull_request_allowances: {
-                users: [BOT_LOGIN],
+                users: [flags.bot],
               },
             },
             enforce_admins: false,
             required_status_checks: null,
             restrictions: {
-              users: [BOT_LOGIN],
+              users: [flags.bot],
               teams: [],
             },
           });
         } else {
-          this.warn(e.response.data.message);
-          this.log(`see ${e.response.data.documentation_url}`);
+          this.warn(typedError.response.data.message);
+          this.log(`see ${typedError.response.data.documentation_url}`);
         }
       } else {
-        console.log(e);
+        // console.log(e);
       }
     }
 
@@ -113,13 +121,13 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
       // repo requires PR reviews but doesn't allow the bot to bypass them
       protectedBranch.required_pull_request_reviews &&
       !protectedBranch.required_pull_request_reviews.bypass_pull_request_allowances?.users?.some(
-        (user) => user.login === BOT_LOGIN
+        (user) => user.login === flags.bot
       )
     ) {
       if (flags['dry-run']) {
-        this.warn(`${BOT_LOGIN} needs permissions to bypass pull request requirements`);
+        this.warn(`${flags.bot} needs permissions to bypass pull request requirements`);
       } else {
-        this.log(`giving ${BOT_LOGIN} pull request bypass permissions`);
+        this.log(`giving ${flags.bot} pull request bypass permissions`);
         const updatePayload = protectedBranch.required_pull_request_reviews.bypass_pull_request_allowances
           ? // maintain original, but append bot to users object
             {
@@ -131,7 +139,7 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
                 ...(protectedBranch.required_pull_request_reviews.bypass_pull_request_allowances.users?.map(
                   (u) => u.login
                 ) ?? []),
-                BOT_LOGIN,
+                flags.bot,
               ],
               teams:
                 protectedBranch.required_pull_request_reviews.bypass_pull_request_allowances.teams?.map(
@@ -140,7 +148,7 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
             }
           : // there was not an object, so create one
             {
-              users: [BOT_LOGIN],
+              users: [flags.bot],
             };
         await octokit.rest.repos.updatePullRequestReviewProtection({
           ...repoBase,
@@ -150,7 +158,7 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
         output.prBypass = true;
       }
     } else {
-      this.logSuccess(`✓ ${BOT_LOGIN} can bypass pull request requirements`);
+      this.logSuccess(`✓ ${flags.bot} can bypass pull request requirements`);
       output.prBypass = true;
     }
 
@@ -161,20 +169,20 @@ export default class ConfigureRepo extends SfCommand<ConfigureRepoResult> {
       })
     ).data;
 
-    if (protectedBranch.restrictions && !protectedBranch.restrictions?.users.some((user) => user.login === BOT_LOGIN)) {
+    if (protectedBranch.restrictions && !protectedBranch.restrictions?.users.some((user) => user.login === flags.bot)) {
       if (flags['dry-run']) {
         this.warn('SF-CLI-BOT needs permissions to push directly to main');
       } else {
-        this.log(`giving ${BOT_LOGIN} permission to push directly to main`);
+        this.log(`giving ${flags.bot} permission to push directly to main`);
         await octokit.rest.repos.addUserAccessRestrictions({
           ...repoBase,
           branch: 'main',
-          users: [BOT_LOGIN],
+          users: [flags.bot],
         });
       }
       output.prRestrictions = true;
     } else {
-      this.logSuccess(`✓ ${BOT_LOGIN} can push directly to main`);
+      this.logSuccess(`✓ ${flags.bot} can push directly to main`);
       output.prRestrictions = true;
     }
 

diff --git a/src/commands/configure/secrets.ts b/src/commands/configure/secrets.ts
@@ -1,24 +1,23 @@
-/*
- * Copyright (c) 2020, salesforce.com, inc.
- * All rights reserved.
- * Licensed under the BSD 3-Clause license.
- * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
- */
-
 /*
  * Copyright (c) 2022, salesforce.com, inc.
  * All rights reserved.
  * Licensed under the BSD 3-Clause license.
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+// because github api isn't camelcased
+/* eslint-disable camelcase */
+
 import * as fs from 'fs';
+import { CliUx } from '@oclif/core';
+
 import { SfCommand, Flags } from '@salesforce/sf-plugins-core';
 import { Messages } from '@salesforce/core';
 
-import { Octokit } from 'octokit';
+import { Octokit } from '@octokit/rest';
 import { exec } from 'shelljs';
 import * as yaml from 'js-yaml';
+import { OctokitError } from '../../types';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.load('@salesforce/plugin-dev', 'configure.secrets', [
@@ -177,9 +176,10 @@ const secretCheck = async (
       return 'overridden by repo';
     }
   } catch (e) {
-    // if (e.response.data) {
-    //   console.log(`check repo secrets for ${secretName}: ${e.response.data.message}`);
-    // }
+    const typedError = e as OctokitError;
+    if (typedError.response.data) {
+      CliUx.ux.log(`check repo secrets for ${secretName}: ${typedError.response.data.message}`);
+    }
     // secret doesn't exist locally, keep looking.
   }
 
@@ -208,8 +208,10 @@ const secretCheck = async (
       }
     }
   } catch (e) {
-    if (e.response.data) {
-      console.log(`check org secrets for ${secretName}: ${e.response.data.message}`);
+    const typedError = e as OctokitError;
+
+    if (typedError.response.data) {
+      CliUx.ux.log(`check org secrets for ${secretName}: ${typedError.response.data.message}`);
     }
     return 'does not exist in org';
   }

diff --git a/src/types.ts b/src/types.ts
@@ -71,3 +71,13 @@ export type FlagAnswers = {
   integerMax?: number;
   integerDefault?: number;
 };
+
+export type OctokitError = {
+  response: {
+    data: {
+      message: string;
+      // eslint-disable-next-line camelcase
+      documentation_url: string;
+    };
+  };
+};
diff --git a/test/commands/configure/repo.nut.ts b/test/commands/configure/repo.nut.ts