fix: updating a pull request uses overflow handler if body is too lar…

…ge (#1702)

* test: failing test for updating pull request with overflow handler

* fix: updating a pull request uses overflow handler if body is too large

* test: add tests for updatePullRequest handling overflow

src/github.ts

@@ -49,6 +49,7 @@ import {
 import {Logger} from 'code-suggester/build/src/types';
 import {HttpsProxyAgent} from 'https-proxy-agent';
 import {HttpProxyAgent} from 'http-proxy-agent';
+import {PullRequestOverflowHandler} from './util/pull-request-overflow-handler';
 
 // Extract some types from the `request` package.
 type RequestBuilderType = typeof request;
@@ -1162,7 +1163,12 @@ export class GitHub {
    * Update a pull request's title and body.
    * @param {number} number The pull request number
    * @param {ReleasePullRequest} releasePullRequest Pull request data to update
-   * @param {}
+   * @param {string} targetBranch The target branch of the pull request
+   * @param {string} options.signoffUser Optional. Commit signoff message
+   * @param {boolean} options.fork Optional. Whether to open the pull request from
+   *   a fork or not. Defaults to `false`
+   * @param {PullRequestOverflowHandler} options.pullRequestOverflowHandler Optional.
+   *   Handles extra large pull request body messages.
    */
   updatePullRequest = wrapAsync(
     async (
@@ -1172,6 +1178,7 @@ export class GitHub {
       options?: {
         signoffUser?: string;
         fork?: boolean;
+        pullRequestOverflowHandler?: PullRequestOverflowHandler;
       }
     ): Promise<PullRequest> => {
       //  Update the files for the release if not already supplied
@@ -1184,7 +1191,13 @@ export class GitHub {
         message = signoffCommitMessage(message, options.signoffUser);
       }
       const title = releasePullRequest.title.toString();
-      const body = releasePullRequest.body
+      const body = (
+        options?.pullRequestOverflowHandler
+          ? await options.pullRequestOverflowHandler.handleOverflow(
+              releasePullRequest
+            )
+          : releasePullRequest.body
+      )
         .toString()
         .slice(0, MAX_ISSUE_BODY_SIZE);
       const prNumber = await createPullRequest(this.octokit, changes, {

src/manifest.ts

@@ -975,6 +975,7 @@ export class Manifest {
       {
         fork: this.fork,
         signoffUser: this.signoffUser,
+        pullRequestOverflowHandler: this.pullRequestOverflowHandler,
       }
     );
     return updatedPullRequest;
@@ -999,6 +1000,7 @@ export class Manifest {
       {
         fork: this.fork,
         signoffUser: this.signoffUser,
+        pullRequestOverflowHandler: this.pullRequestOverflowHandler,
       }
     );
     // TODO: consider leaving the snooze label

test/github.ts

@@ -40,6 +40,7 @@ import {RawContent} from '../src/updaters/raw-content';
 import {HttpsProxyAgent} from 'https-proxy-agent';
 import {HttpProxyAgent} from 'http-proxy-agent';
 import {Commit} from '../src/commit';
+import {mockReleaseData, MockPullRequestOverflowHandler} from './helpers';
 
 const fixturesPath = './test/fixtures';
 const sandbox = sinon.createSandbox();
@@ -1141,4 +1142,38 @@ describe('GitHub', () => {
       expect(url).to.eql('https://github.com/fake/fake/blob/new-file.txt');
     });
   });
+
+  describe('updatePullRequest', () => {
+    it('handles a PR body that is too big', async () => {
+      req = req.patch('/repos/fake/fake/pulls/123').reply(200, {
+        number: 123,
+        title: 'updated-title',
+        body: 'updated body',
+        labels: [],
+        head: {
+          ref: 'abc123',
+        },
+        base: {
+          ref: 'def234',
+        },
+      });
+      const pullRequest = {
+        title: PullRequestTitle.ofTargetBranch('main'),
+        body: new PullRequestBody(mockReleaseData(1000), {useComponents: true}),
+        labels: [],
+        headRefName: 'release-please--branches--main',
+        draft: false,
+        updates: [],
+      };
+      const pullRequestOverflowHandler = new MockPullRequestOverflowHandler();
+      const handleOverflowStub = sandbox
+        .stub(pullRequestOverflowHandler, 'handleOverflow')
+        .resolves('overflow message');
+      await github.updatePullRequest(123, pullRequest, 'main', {
+        pullRequestOverflowHandler,
+      });
+      sinon.assert.calledOnce(handleOverflowStub);
+      req.done();
+    });
+  });
 });

test/helpers.ts

@@ -27,14 +27,17 @@ import {expect} from 'chai';
 import {CandidateReleasePullRequest} from '../src/manifest';
 import {Version} from '../src/version';
 import {PullRequestTitle} from '../src/util/pull-request-title';
-import {PullRequestBody} from '../src/util/pull-request-body';
+import {PullRequestBody, ReleaseData} from '../src/util/pull-request-body';
 import {BranchName} from '../src/util/branch-name';
 import {ReleaseType} from '../src/factory';
 import {
   GitHubFileContents,
   DEFAULT_FILE_MODE,
 } from '@google-automations/git-file-utils';
 import {CompositeUpdater} from '../src/updaters/composite';
+import {PullRequestOverflowHandler} from '../src/util/pull-request-overflow-handler';
+import {ReleasePullRequest} from '../src/release-pull-request';
+import {PullRequest} from '../src/pull-request';
 
 export function stubSuggesterWithSnapshot(
   sandbox: sinon.SinonSandbox,
@@ -372,3 +375,32 @@ export function mockTags(
   }
   return sandbox.stub(github, 'tagIterator').returns(fakeGenerator());
 }
+
+export function mockReleaseData(count: number): ReleaseData[] {
+  const releaseData: ReleaseData[] = [];
+  const version = Version.parse('1.2.3');
+  for (let i = 0; i < count; i++) {
+    releaseData.push({
+      component: `component${i}`,
+      version,
+      notes: `release notes for component${i}`,
+    });
+  }
+  return releaseData;
+}
+
+export class MockPullRequestOverflowHandler
+  implements PullRequestOverflowHandler
+{
+  async handleOverflow(
+    pullRequest: ReleasePullRequest,
+    _maxSize?: number | undefined
+  ): Promise<string> {
+    return pullRequest.body.toString();
+  }
+  async parseOverflow(
+    pullRequest: PullRequest
+  ): Promise<PullRequestBody | undefined> {
+    return PullRequestBody.parse(pullRequest.body);
+  }
+}

test/manifest.ts

@@ -28,6 +28,7 @@ import {
   mockReleases,
   mockTags,
   assertNoHasUpdate,
+  mockReleaseData,
 } from './helpers';
 import {expect} from 'chai';
 import * as assert from 'assert';
@@ -3735,6 +3736,174 @@ describe('Manifest', () => {
       expect(pullRequestNumbers).lengthOf(1);
     });
 
+    describe('with an overflowing body', () => {
+      const body = new PullRequestBody(mockReleaseData(1000), {
+        useComponents: true,
+      });
+
+      it('updates an existing pull request', async function () {
+        sandbox
+          .stub(github, 'getFileContentsOnBranch')
+          .withArgs('README.md', 'main')
+          .resolves(buildGitHubFileRaw('some-content'));
+        stubSuggesterWithSnapshot(sandbox, this.test!.fullTitle());
+        mockPullRequests(
+          github,
+          [
+            {
+              number: 22,
+              title: 'pr title1',
+              body: pullRequestBody('release-notes/single.txt'),
+              headBranchName: 'release-please/branches/main',
+              baseBranchName: 'main',
+              labels: ['autorelease: pending'],
+              files: [],
+            },
+          ],
+          []
+        );
+        sandbox
+          .stub(github, 'updatePullRequest')
+          .withArgs(
+            22,
+            sinon.match.any,
+            sinon.match.any,
+            sinon.match.has('pullRequestOverflowHandler', sinon.match.truthy)
+          )
+          .resolves({
+            number: 22,
+            title: 'pr title1',
+            body: 'pr body1',
+            headBranchName: 'release-please/branches/main',
+            baseBranchName: 'main',
+            labels: [],
+            files: [],
+          });
+        const manifest = new Manifest(
+          github,
+          'main',
+          {
+            'path/a': {
+              releaseType: 'node',
+              component: 'pkg1',
+            },
+            'path/b': {
+              releaseType: 'node',
+              component: 'pkg2',
+            },
+          },
+          {
+            'path/a': Version.parse('1.0.0'),
+            'path/b': Version.parse('0.2.3'),
+          },
+          {
+            separatePullRequests: true,
+            plugins: ['node-workspace'],
+          }
+        );
+        sandbox.stub(manifest, 'buildPullRequests').resolves([
+          {
+            title: PullRequestTitle.ofTargetBranch('main'),
+            body,
+            updates: [
+              {
+                path: 'README.md',
+                createIfMissing: false,
+                updater: new RawContent('some raw content'),
+              },
+            ],
+            labels: [],
+            headRefName: 'release-please/branches/main',
+            draft: false,
+          },
+        ]);
+        const pullRequestNumbers = await manifest.createPullRequests();
+        expect(pullRequestNumbers).lengthOf(1);
+      });
+
+      it('ignores an existing pull request if there are no changes', async function () {
+        sandbox
+          .stub(github, 'getFileContentsOnBranch')
+          .withArgs('README.md', 'main')
+          .resolves(buildGitHubFileRaw('some-content'))
+          .withArgs('release-notes.md', 'my-head-branch--release-notes')
+          .resolves(buildGitHubFileRaw(body.toString()));
+        stubSuggesterWithSnapshot(sandbox, this.test!.fullTitle());
+        mockPullRequests(
+          github,
+          [
+            {
+              number: 22,
+              title: 'pr title1',
+              body: pullRequestBody('release-notes/overflow.txt'),
+              headBranchName: 'release-please/branches/main',
+              baseBranchName: 'main',
+              labels: ['autorelease: pending'],
+              files: [],
+            },
+          ],
+          []
+        );
+        sandbox
+          .stub(github, 'updatePullRequest')
+          .withArgs(
+            22,
+            sinon.match.any,
+            sinon.match.any,
+            sinon.match.has('pullRequestOverflowHandler', sinon.match.truthy)
+          )
+          .resolves({
+            number: 22,
+            title: 'pr title1',
+            body: 'pr body1',
+            headBranchName: 'release-please/branches/main',
+            baseBranchName: 'main',
+            labels: [],
+            files: [],
+          });
+        const manifest = new Manifest(
+          github,
+          'main',
+          {
+            'path/a': {
+              releaseType: 'node',
+              component: 'pkg1',
+            },
+            'path/b': {
+              releaseType: 'node',
+              component: 'pkg2',
+            },
+          },
+          {
+            'path/a': Version.parse('1.0.0'),
+            'path/b': Version.parse('0.2.3'),
+          },
+          {
+            separatePullRequests: true,
+            plugins: ['node-workspace'],
+          }
+        );
+        sandbox.stub(manifest, 'buildPullRequests').resolves([
+          {
+            title: PullRequestTitle.ofTargetBranch('main'),
+            body,
+            updates: [
+              {
+                path: 'README.md',
+                createIfMissing: false,
+                updater: new RawContent('some raw content'),
+              },
+            ],
+            labels: [],
+            headRefName: 'release-please/branches/main',
+            draft: false,
+          },
+        ]);
+        const pullRequestNumbers = await manifest.createPullRequests();
+        expect(pullRequestNumbers).lengthOf(0);
+      });
+    });
+
     it('updates an existing snapshot pull request', async function () {
       sandbox
         .stub(github, 'getFileContentsOnBranch')