fix: backfill files by commit for rebased PRs (#2141)

* fix: backfill files by commit for rebased PRs

Fixes #2140

* fix: always set commit PR from associated PRs

---------

Co-authored-by: Jeff Ching <chingor@google.com>

Author: lukekarrys

__snapshots__/github.js

@@ -469,17 +469,42 @@ export class GitHub {
     }
     const history = response.repository.ref.target.history;
     const commits = (history.nodes || []) as GraphQLCommit[];
+    // Count the number of pull requests associated with each merge commit. This is
+    // used in the next step to make sure we only find pull requests with a
+    // merge commit that contain 1 merged commit.
+    const mergeCommitCount: Record<string, number> = {};
+    for (const commit of commits) {
+      for (const pr of commit.associatedPullRequests.nodes) {
+        if (pr.mergeCommit?.oid) {
+          mergeCommitCount[pr.mergeCommit.oid] ??= 0;
+          mergeCommitCount[pr.mergeCommit.oid]++;
+        }
+      }
+    }
     const commitData: Commit[] = [];
     for (const graphCommit of commits) {
       const commit: Commit = {
         sha: graphCommit.sha,
         message: graphCommit.message,
       };
-      const pullRequest = graphCommit.associatedPullRequests.nodes.find(pr => {
-        return pr.mergeCommit && pr.mergeCommit.oid === graphCommit.sha;
-      });
+      const mergePullRequest = graphCommit.associatedPullRequests.nodes.find(
+        pr => {
+          return (
+            // Only match the pull request with a merge commit if there is a
+            // single merged commit in the PR. This means merge commits and squash
+            // merges will be matched, but rebase merged PRs will only be matched
+            // if they contain a single commit. This is so PRs that are rebased
+            // and merged will have ßSfiles backfilled from each commit instead of
+            // the whole PR.
+            pr.mergeCommit &&
+            pr.mergeCommit.oid === graphCommit.sha &&
+            mergeCommitCount[pr.mergeCommit.oid] === 1
+          );
+        }
+      );
+      const pullRequest =
+        mergePullRequest || graphCommit.associatedPullRequests.nodes[0];
       if (pullRequest) {
-        const files = (pullRequest.files?.nodes || []).map(node => node.path);
         commit.pullRequest = {
           sha: commit.sha,
           number: pullRequest.number,
@@ -488,17 +513,24 @@ export class GitHub {
           title: pullRequest.title,
           body: pullRequest.body,
           labels: pullRequest.labels.nodes.map(node => node.name),
-          files,
+          files: (pullRequest.files?.nodes || []).map(node => node.path),
         };
-        if (pullRequest.files?.pageInfo?.hasNextPage && options.backfillFiles) {
+      }
+      if (mergePullRequest) {
+        if (
+          mergePullRequest.files?.pageInfo?.hasNextPage &&
+          options.backfillFiles
+        ) {
           this.logger.info(
-            `PR #${pullRequest.number} has many files, backfilling`
+            `PR #${mergePullRequest.number} has many files, backfilling`
           );
           commit.files = await this.getCommitFiles(graphCommit.sha);
         } else {
           // We cannot directly fetch files on commits via graphql, only provide file
           // information for commits with associated pull requests
-          commit.files = files;
+          commit.files = (mergePullRequest.files?.nodes || []).map(
+            node => node.path
+          );
         }
       } else if (options.backfillFiles) {
         // In this case, there is no squashed merge commit. This could be a simple

src/github.ts

@@ -0,0 +1,88 @@
+{
+  "repository": {
+    "ref": {
+      "target": {
+        "history": {
+          "nodes": [
+            {
+              "associatedPullRequests": {
+                "nodes": [
+                  {
+                    "number": 7,
+                    "title": "feat: feature that will be rebase merged",
+                    "baseRefName": "main",
+                    "headRefName": "feature-branch-rebase-merge",
+                    "labels": {
+                      "nodes": []
+                    },
+                    "body": "",
+                    "mergeCommit": {
+                      "oid": "b29149f890e6f76ee31ed128585744d4c598924c"
+                    },
+                    "files": {
+                      "nodes": []
+                    }
+                  }
+                ]
+              },
+              "sha": "b29149f890e6f76ee31ed128585744d4c598924c",
+              "message": "feat: feature-branch-rebase-merge commit 1"
+            },
+            {
+              "associatedPullRequests": {
+                "nodes": [
+                  {
+                    "number": 7,
+                    "title": "feat: feature that will be rebase merged",
+                    "baseRefName": "main",
+                    "headRefName": "feature-branch-rebase-merge",
+                    "labels": {
+                      "nodes": []
+                    },
+                    "body": "",
+                    "mergeCommit": {
+                      "oid": "b29149f890e6f76ee31ed128585744d4c598924c"
+                    },
+                    "files": {
+                      "nodes": []
+                    }
+                  }
+                ]
+              },
+              "sha": "27d7d7232e2e312d1380e906984f0823f5decf61",
+              "message": "feat: feature-branch-rebase-merge commit 2"
+            },
+            {
+              "associatedPullRequests": {
+                "nodes": [
+                  {
+                    "number": 6,
+                    "title": "feat: other pr",
+                    "baseRefName": "main",
+                    "headRefName": "feature-branch-other",
+                    "labels": {
+                      "nodes": []
+                    },
+                    "body": "",
+                    "mergeCommit": {
+                      "oid": "2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5"
+                    },
+                    "files": {
+                      "nodes": []
+                    }
+                  }
+                ]
+              },
+              "sha": "2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5",
+              "message": "fix: feature-branch-other"
+            }
+          ],
+          "pageInfo": {
+            "hasNextPage": false,
+            "endCursor": "e6daec403626c9987c7af0d97b34f324cd84320a 12"
+          }
+        }
+      }
+    }
+  }
+}

test/fixtures/commits-since-rebase.json

@@ -546,6 +546,39 @@ describe('GitHub', () => {
       snapshot(commitsSinceSha);
       req.done();
     });
+
+    it('backfills commit files for pull requests rebased and merged', async () => {
+      const graphql = JSON.parse(
+        readFileSync(resolve(fixturesPath, 'commits-since-rebase.json'), 'utf8')
+      );
+      req
+        .post('/graphql')
+        .reply(200, {
+          data: graphql,
+        })
+        .get(
+          '/repos/fake/fake/commits/b29149f890e6f76ee31ed128585744d4c598924c'
+        )
+        .reply(200, {files: [{filename: 'abc'}]})
+        .get(
+          '/repos/fake/fake/commits/27d7d7232e2e312d1380e906984f0823f5decf61'
+        )
+        .reply(200, {files: [{filename: 'def'}]});
+      const targetBranch = 'main';
+      const commitsSinceSha = await github.commitsSince(
+        targetBranch,
+        commit => {
+          // this commit is the 3rd most recent
+          return commit.sha === '2b4e0b3be2e231cd87cc44c411bd8f84b4587ab5';
+        },
+        {backfillFiles: true}
+      );
+      expect(commitsSinceSha.length).to.eql(2);
+      expect(commitsSinceSha[0].files).to.eql(['abc']);
+      expect(commitsSinceSha[1].files).to.eql(['def']);
+      snapshot(commitsSinceSha);
+      req.done();
+    });
   });
 
   describe('mergeCommitIterator', () => {