From fe81a1d27b3ec4ccb609a9708338d2f81889639f Mon Sep 17 00:00:00 2001
From: Eric Morphis <meorphis@gmail.com>
Date: Wed, 31 Jan 2024 16:25:42 -0500
Subject: [PATCH] automerge filters: fix early return in conventional commit
 filter

---
 src/manifest.ts  |  6 ++--
 test/manifest.ts | 76 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/src/manifest.ts b/src/manifest.ts
index f465c9bd7..5c4bfa82d 100644
--- a/src/manifest.ts
+++ b/src/manifest.ts
@@ -1689,6 +1689,7 @@ export class Manifest {
           filter => `${filter.type}:${filter.scope ? filter.scope : '*'}`
         )
       );
+      // return true if every commit matches the filter - false otherwise
       if (conventionalCommitFilter!.matchBehaviour === 'match-all') {
         for (const commit of pullRequest.conventionalCommits) {
           if (
@@ -1702,11 +1703,12 @@ export class Manifest {
             );
             return false;
           }
-          return true;
         }
+        return true;
       } else if (
         conventionalCommitFilter!.matchBehaviour === 'match-at-least-one'
       ) {
+        // return true if any commit matches the filter - false otherwise
         for (const commit of pullRequest.conventionalCommits) {
           if (
             filterSet.has(`${commit.type}:${commit.scope}`) ||
@@ -1720,8 +1722,8 @@ export class Manifest {
             );
             return true;
           }
-          return false;
         }
+        return false;
       }
       return false;
     };
diff --git a/test/manifest.ts b/test/manifest.ts
index 4e294e726..37fd7ede7 100644
--- a/test/manifest.ts
+++ b/test/manifest.ts
@@ -5051,6 +5051,38 @@ version = "3.0.0"
               },
             ],
           },
+          {
+            title: PullRequestTitle.ofTargetBranch('main', 'main'),
+            body: new PullRequestBody([]),
+            updates: [],
+            labels: [],
+            headRefName: 'release-please/branches/main/components/b',
+            draft: false,
+            version: Version.parse('1.1.0'), // minor bump, match filter
+            previousVersion: Version.parse('1.0.0'),
+            conventionalCommits: [
+              {
+                type: 'fix', // type match filter
+                scope: 'api', // scope match filter
+                notes: [],
+                references: [],
+                sha: 'commit123',
+                message: 'fix(api): something',
+                bareMessage: 'something',
+                breaking: false,
+              },
+              {
+                type: 'feat(client)', // type does not match filter
+                scope: 'api', // scope match filter
+                notes: [],
+                references: [],
+                sha: 'commit123',
+                message: 'fix(api): something',
+                bareMessage: 'something',
+                breaking: false,
+              },
+            ],
+          },
           {
             title: PullRequestTitle.ofTargetBranch('main', 'main'),
             body: new PullRequestBody([]),
@@ -5125,11 +5157,11 @@ version = "3.0.0"
 
       const pullRequestNumbers = await manifest.createPullRequests();
 
-      expect(pullRequestNumbers).lengthOf(5);
+      expect(pullRequestNumbers).lengthOf(6);
       sinon.assert.calledOnce(getLabelsStub);
       sinon.assert.calledOnce(createLabelsStub);
 
-      expect(createPullRequestStub.callCount).to.equal(5);
+      expect(createPullRequestStub.callCount).to.equal(6);
       sinon.assert.calledWith(
         createPullRequestStub,
         sinon.match.has('headBranchName', sinon.match.string),
@@ -5143,7 +5175,7 @@ version = "3.0.0"
       expect(enablePullRequestAutoMergeStub.callCount).to.equal(1);
 
       // only called when not auto-merged
-      expect(addPullRequestReviewersStub.callCount).to.equal(4);
+      expect(addPullRequestReviewersStub.callCount).to.equal(5);
     });
 
     it('enables auto-merge when filters are provided (filters: only commit type, match-all)', async () => {
@@ -5453,6 +5485,38 @@ version = "3.0.0"
               },
             ],
           },
+          {
+            title: PullRequestTitle.ofTargetBranch('main', 'main'),
+            body: new PullRequestBody([]),
+            updates: [],
+            labels: [],
+            headRefName: 'release-please/branches/main/components/a',
+            draft: false,
+            version: Version.parse('1.0.1'), // version bump match filter
+            previousVersion: Version.parse('1.0.0'),
+            conventionalCommits: [
+              {
+                type: 'ci', // first type does not match filter
+                scope: 'something',
+                notes: [],
+                references: [],
+                sha: 'commit123',
+                message: 'ci(something): something',
+                bareMessage: 'something',
+                breaking: false,
+              },
+              {
+                type: 'fix', // type match filter
+                scope: 'something',
+                notes: [],
+                references: [],
+                sha: 'commit123',
+                message: 'fix(something): something',
+                bareMessage: 'something',
+                breaking: false,
+              },
+            ],
+          },
           {
             title: PullRequestTitle.ofTargetBranch('main', 'main'),
             body: new PullRequestBody([]),
@@ -5621,11 +5685,11 @@ version = "3.0.0"
 
       const pullRequestNumbers = await manifest.createPullRequests();
 
-      expect(pullRequestNumbers).lengthOf(6);
+      expect(pullRequestNumbers).lengthOf(7);
       sinon.assert.calledOnce(getLabelsStub);
       sinon.assert.calledOnce(createLabelsStub);
 
-      expect(createPullRequestStub.callCount).to.equal(6);
+      expect(createPullRequestStub.callCount).to.equal(7);
       sinon.assert.calledWith(
         createPullRequestStub,
         sinon.match.has('headBranchName', sinon.match.string),
@@ -5636,7 +5700,7 @@ version = "3.0.0"
         sinon.match.object
       );
 
-      expect(enablePullRequestAutoMergeStub.callCount).to.equal(3);
+      expect(enablePullRequestAutoMergeStub.callCount).to.equal(4);
       // only called when not auto-merged
       expect(addPullRequestReviewersStub.callCount).to.equal(3);
     });