Repository: intuit/auto (ts)

Issue:
Committing with a string like "First Commit #1" gets parsed as separate changelog line
**Describe the bug**

If the commits in a PR (say #32) has the following commits:

- all done
- working
- first commit #1 

the Changelog will include both #32 and #1, which is wrong

Pull request description:
omit commit+prs that: aren't found in repo, are issues, or have the "released" label
# What Changed

Better handling of situations where the parsed commit message might have a PR number that shouldn't be included. Situations covered:

1. PR number that doesn't exist in the repo yet (ex: 4000, but max PR number is 30)
2. Number is an issue and not a pr
3. Pr already has the "released" label attached

# Why

more fixes for #536 

this should also close #465

Todo:

- [x] Add tests

<!-- GITHUB_RELEASE PR BODY: canary-version -->
Published PR with canary version: `7.3.6-canary.538.7039.0`
<!-- GITHUB_RELEASE PR BODY: canary-version -->


Patch:
diff --git a/packages/core/src/auto.ts b/packages/core/src/auto.ts
index 638d705f..378d6400 100644
--- a/packages/core/src/auto.ts
+++ b/packages/core/src/auto.ts
@@ -292,9 +292,7 @@ export default class Auto {
         });
       } catch (error) {
         throw new Error(
-          `Failed to post status to Pull Request with error code ${
-            error.status
-          }`
+          `Failed to post status to Pull Request with error code ${error.status}`
         );
       }
 
@@ -387,9 +385,7 @@ export default class Auto {
         this.logger.log.success('Posted status to Pull Request.');
       } catch (error) {
         throw new Error(
-          `Failed to post status to Pull Request with error code ${
-            error.status
-          }`
+          `Failed to post status to Pull Request with error code ${error.status}`
         );
       }
     } else {
@@ -944,7 +940,10 @@ If a command fails manually run:
    * Apply all of the plugins in the config.
    */
   private loadPlugins(config: IAutoConfig) {
-    const pluginsPaths = config.plugins || ['npm'];
+    const pluginsPaths = [
+      require.resolve('./plugins/filter-non-pull-request'),
+      ...(config.plugins || ['npm'])
+    ];
 
     pluginsPaths
       .map(plugin =>
diff --git a/packages/core/src/plugins/filter-non-pull-request.ts b/packages/core/src/plugins/filter-non-pull-request.ts
new file mode 100644
index 00000000..d403643b
--- /dev/null
+++ b/packages/core/src/plugins/filter-non-pull-request.ts
@@ -0,0 +1,37 @@
+import on from 'await-to-js';
+
+import { Auto, IPlugin } from '../auto';
+
+export default class FilterNonPullRequestPlugin implements IPlugin {
+  name = 'Filter Non Pull Request';
+
+  apply(auto: Auto) {
+    auto.hooks.onCreateLogParse.tap(this.name, logParse => {
+      logParse.hooks.omitCommit.tapPromise(this.name, async commit => {
+        // tslint:disable-next-line early-exit
+        if (commit.pullRequest && commit.pullRequest.number) {
+          const { number: prNumber } = commit.pullRequest;
+          const [err, info] = await on(auto.git!.getPr(prNumber));
+
+          // Omit PRs that don't exist on the repo
+          if (err && err.message.includes('Not Found')) {
+            return true;
+          }
+
+          if (err) {
+            throw err;
+          }
+
+          if (!info) {
+            throw new Error(`Could not find PR: ${prNumber}`);
+          }
+
+          // Omit issues
+          if (!info.data.pull_request) {
+            return true;
+          }
+        }
+      });
+    });
+  }
+}
diff --git a/plugins/released/src/index.ts b/plugins/released/src/index.ts
index c65f95a4..4a98199b 100644
--- a/plugins/released/src/index.ts
+++ b/plugins/released/src/index.ts
@@ -38,6 +38,14 @@ export default class ReleasedLabelPlugin implements IPlugin {
       return config;
     });
 
+    auto.hooks.onCreateLogParse.tap(this.name, logParse => {
+      logParse.hooks.omitCommit.tapPromise(this.name, async commit => {
+        if (commit.labels.includes(this.options.label)) {
+          return true;
+        }
+      });
+    });
+
     auto.hooks.afterRelease.tapPromise(
       this.name,
       async ({ newVersion, commits }) => {
diff --git a/yarn.lock b/yarn.lock
index 0995c84c..05a48234 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -10009,9 +10009,9 @@ mississippi@^3.0.0:
     through2 "^2.0.0"
 
 mixin-deep@^1.2.0:
-  version "1.3.2"
-  resolved "https://registry.yarnpkg.com/mixin-deep/-/mixin-deep-1.3.2.tgz#1120b43dc359a785dce65b55b82e257ccf479566"
-  integrity sha512-WRoDn//mXBiJ1H40rqa3vH0toePwSsGb45iInWlTySa+Uu4k3tYUSxa2v1KqAiLtvlrSzaExqS1gtk96A9zvEA==
+  version "1.3.1"
+  resolved "https://registry.yarnpkg.com/mixin-deep/-/mixin-deep-1.3.1.tgz#a49e7268dce1a0d9698e45326c5626df3543d0fe"
+  integrity sha512-8ZItLHeEgaqEvd5lYBXfm4EZSFCX29Jb9K+lAHhDKzReKBQKj3R+7NOF6tjqYi9t4oI8VUfaWITJQm86wnXGNQ==
   dependencies:
     for-in "^1.0.2"
     is-extendable "^1.0.1"


Test patch:
diff --git a/packages/core/src/__tests__/__snapshots__/log-parse.test.ts.snap b/packages/core/src/__tests__/__snapshots__/log-parse.test.ts.snap
index 17102856..0d156a13 100644
--- a/packages/core/src/__tests__/__snapshots__/log-parse.test.ts.snap
+++ b/packages/core/src/__tests__/__snapshots__/log-parse.test.ts.snap
@@ -14,6 +14,7 @@ Array [
     "hash": "foo",
     "labels": Array [],
     "packages": undefined,
+    "pullRequest": undefined,
     "subject": "First",
   },
   Object {
@@ -28,6 +29,7 @@ Array [
     "hash": "foo",
     "labels": Array [],
     "packages": undefined,
+    "pullRequest": undefined,
     "subject": "Second",
   },
   Object {
@@ -42,6 +44,7 @@ Array [
     "hash": "foo",
     "labels": Array [],
     "packages": undefined,
+    "pullRequest": undefined,
     "subject": "Third",
   },
 ]
diff --git a/packages/core/src/__tests__/__snapshots__/release.test.ts.snap b/packages/core/src/__tests__/__snapshots__/release.test.ts.snap
index da53187f..aa83cc10 100644
--- a/packages/core/src/__tests__/__snapshots__/release.test.ts.snap
+++ b/packages/core/src/__tests__/__snapshots__/release.test.ts.snap
@@ -304,6 +304,7 @@ Array [
     "hash": "foo",
     "labels": Array [],
     "packages": undefined,
+    "pullRequest": undefined,
     "subject": "First",
   },
   Object {
@@ -355,6 +356,7 @@ Array [
     "hash": "foo",
     "labels": Array [],
     "packages": undefined,
+    "pullRequest": undefined,
     "subject": "Third",
   },
 ]
diff --git a/packages/core/src/__tests__/make-commit-from-msg.ts b/packages/core/src/__tests__/make-commit-from-msg.ts
index 56161e55..f8dd6390 100644
--- a/packages/core/src/__tests__/make-commit-from-msg.ts
+++ b/packages/core/src/__tests__/make-commit-from-msg.ts
@@ -9,6 +9,9 @@ const makeCommitFromMsg = (
     labels?: string[];
     username?: string;
     packages?: string[];
+    pullRequest?: {
+      number: number;
+    };
   } = {}
 ): IExtendedCommit => ({
   hash: options.hash || 'foo',
@@ -23,7 +26,8 @@ const makeCommitFromMsg = (
     }
   ],
   subject,
-  packages: options.packages
+  packages: options.packages,
+  pullRequest: options.pullRequest
 });
 
 export default makeCommitFromMsg;
diff --git a/packages/core/src/plugins/__tests__/filter-non-pull-request.test.ts b/packages/core/src/plugins/__tests__/filter-non-pull-request.test.ts
new file mode 100644
index 00000000..540b2638
--- /dev/null
+++ b/packages/core/src/plugins/__tests__/filter-non-pull-request.test.ts
@@ -0,0 +1,74 @@
+import makeCommitFromMsg from '../../__tests__/make-commit-from-msg';
+import Auto from '../../auto';
+import Git from '../../git';
+import LogParse from '../../log-parse';
+import { makeHooks, makeLogParseHooks } from '../../utils/make-hooks';
+
+import FilterNonPullRequestPlugin from '../filter-non-pull-request';
+
+const getPr = jest.fn();
+
+const setup = () => {
+  const plugin = new FilterNonPullRequestPlugin();
+  const hooks = makeHooks();
+  const logParseHooks = makeLogParseHooks();
+
+  plugin.apply({ hooks, git: ({ getPr } as unknown) as Git } as Auto);
+  hooks.onCreateLogParse.call({ hooks: logParseHooks } as LogParse);
+
+  return logParseHooks;
+};
+
+describe('Filter Non Pull Request Plugin', () => {
+  test('should do nothing for non-prs', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo');
+    expect(await hooks.omitCommit.promise(commit)).toBeUndefined();
+  });
+
+  test('should not filter bad PR numbers', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo', { pullRequest: { number: 404 } });
+
+    getPr.mockRejectedValueOnce(new Error('Not Found'));
+    expect(await hooks.omitCommit.promise(commit)).toBe(true);
+  });
+
+  test('should throw unknown errors', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo', { pullRequest: { number: 123 } });
+
+    getPr.mockRejectedValueOnce(new Error('Some error'));
+    await expect(hooks.omitCommit.promise(commit)).rejects.toBeInstanceOf(
+      Error
+    );
+  });
+
+  test('should with no PR return value', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo', { pullRequest: { number: 123 } });
+
+    getPr.mockReturnValueOnce(Promise.resolve());
+    await expect(hooks.omitCommit.promise(commit)).rejects.toBeInstanceOf(
+      Error
+    );
+  });
+
+  test('should filter issues', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo', { pullRequest: { number: 123 } });
+
+    getPr.mockReturnValueOnce(Promise.resolve({ data: {} }));
+    expect(await hooks.omitCommit.promise(commit)).toBe(true);
+  });
+
+  test('should not filter PRs', async () => {
+    const hooks = setup();
+    const commit = makeCommitFromMsg('foo', { pullRequest: { number: 123 } });
+
+    getPr.mockReturnValueOnce(
+      Promise.resolve({ data: { pull_request: true } })
+    );
+    expect(await hooks.omitCommit.promise(commit)).toBeUndefined();
+  });
+});
diff --git a/plugins/released/__tests__/released-label.test.ts b/plugins/released/__tests__/released-label.test.ts
index 72abdaca..7ea7aa9e 100644
--- a/plugins/released/__tests__/released-label.test.ts
+++ b/plugins/released/__tests__/released-label.test.ts
@@ -4,7 +4,10 @@ import Git from '@auto-it/core/dist/git';
 import LogParse from '@auto-it/core/dist/log-parse';
 import { defaultLabelDefinition } from '@auto-it/core/dist/release';
 import { dummyLog } from '@auto-it/core/dist/utils/logger';
-import { makeHooks } from '@auto-it/core/dist/utils/make-hooks';
+import {
+  makeHooks,
+  makeLogParseHooks
+} from '@auto-it/core/dist/utils/make-hooks';
 import { IAutoConfig } from '@auto-it/core/src/release';
 import ReleasedLabelPlugin from '../src';
 
@@ -39,7 +42,7 @@ describe('release label plugin', () => {
     lockIssue.mockClear();
   });
 
-  test('should so nothing without PRs', async () => {
+  test('should init label', async () => {
     const releasedLabel = new ReleasedLabelPlugin();
     const autoHooks = makeHooks();
     releasedLabel.apply({ hooks: autoHooks } as Auto);
@@ -56,7 +59,31 @@ describe('release label plugin', () => {
     });
   });
 
-  test('should so nothing without PRs', async () => {
+  test('should omit released PRs', async () => {
+    const releasedLabel = new ReleasedLabelPlugin();
+    const autoHooks = makeHooks();
+    const logParseHooks = makeLogParseHooks();
+
+    releasedLabel.apply(({
+      hooks: autoHooks,
+      labels: defaultLabelDefinition,
+      logger: dummyLog(),
+      options: {},
+      comment,
+      git
+    } as unknown) as Auto);
+    autoHooks.onCreateLogParse.call({ hooks: logParseHooks } as LogParse);
+
+    const included = makeCommitFromMsg('normal commit with no bump');
+    expect(await logParseHooks.omitCommit.promise(included)).toBeUndefined();
+
+    const omitted = makeCommitFromMsg('normal commit with no bump', {
+      labels: ['released']
+    });
+    expect(await logParseHooks.omitCommit.promise(omitted)).toBe(true);
+  });
+
+  test('should do nothing without PRs', async () => {
     const releasedLabel = new ReleasedLabelPlugin();
     const autoHooks = makeHooks();
     releasedLabel.apply(({