From f378454c921e7eff70d825a6acda58c7b58a5a3f Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Mon, 27 Aug 2018 12:07:35 +0300
Subject: [PATCH] fix(docs-infra): correctly check PR files on preview server
 (#25671)

According to the docs, the response of GitHub's [PR files API][1]
_"includes a maximum of 300 files"_. This means that if a PR contains
more files, it is possible that not all files are retrieved (which
could, for example, give a false negative for the "significant files
touched" check - not likely but possible).

This commit fixes it by using paginated requests to retrieve all changed
files.

[1]: https://developer.github.com/v3/pulls/#list-pull-requests-files

PR Close #25671
---
 .../scripts-js/lib/common/github-pull-requests.ts |  2 +-
 .../lib/verify-setup/mock-external-apis.ts        |  2 +-
 .../test/common/github-pull-requests.spec.ts      | 15 +++++++++------
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts
index 62d832fbf1..f429a3c2c3 100644
--- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts
+++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts
@@ -74,6 +74,6 @@ export class GithubPullRequests {
    */
   public fetchFiles(pr: number): Promise<FileInfo[]> {
     assert(pr > 0, `Invalid PR number: ${pr}`);
-    return this.api.get<FileInfo[]>(`/repos/${this.repoSlug}/pulls/${pr}/files`);
+    return this.api.getPaginated<FileInfo>(`/repos/${this.repoSlug}/pulls/${pr}/files`);
   }
 }
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts
index 7fb9220e0b..db39e3289a 100644
--- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts
+++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts
@@ -76,7 +76,7 @@ const GITHUB_PULLS_URL = `/repos/${AIO_GITHUB_ORGANIZATION}/${AIO_GITHUB_REPO}/p
 const GITHUB_TEAMS_URL = `/orgs/${AIO_GITHUB_ORGANIZATION}/teams`;
 
 const getIssueUrl = (prNum: number) => `${GITHUB_ISSUES_URL}/${prNum}`;
-const getFilesUrl = (prNum: number) => `${GITHUB_PULLS_URL}/${prNum}/files`;
+const getFilesUrl = (prNum: number, pageNum = 0) => `${GITHUB_PULLS_URL}/${prNum}/files?page=${pageNum}&per_page=100`;
 const getCommentUrl = (prNum: number) => `${getIssueUrl(prNum)}/comments`;
 const getTeamMembershipUrl = (teamId: number, username: string) => `/teams/${teamId}/memberships/${username}`;
 
diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts
index d6e3f31407..27e265b9f1 100644
--- a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts
+++ b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts
@@ -4,13 +4,13 @@ import {GithubPullRequests} from '../../lib/common/github-pull-requests';
 
 // Tests
 describe('GithubPullRequests', () => {
-
   let githubApi: jasmine.SpyObj<GithubApi>;
 
   beforeEach(() => {
     githubApi = jasmine.createSpyObj('githubApi', ['post', 'get', 'getPaginated']);
   });
 
+
   describe('constructor()', () => {
 
     it('should throw if \'githubOrg\' is missing or empty', () => {
@@ -95,6 +95,7 @@ describe('GithubPullRequests', () => {
         done();
       });
     });
+
   });
 
 
@@ -128,8 +129,10 @@ describe('GithubPullRequests', () => {
       githubApi.getPaginated.and.returnValue('Test');
       expect(prs.fetchAll() as any).toBe('Test');
     });
+
   });
 
+
   describe('fetchFiles()', () => {
     let prs: GithubPullRequests;
 
@@ -138,21 +141,21 @@ describe('GithubPullRequests', () => {
     });
 
 
-    it('should make a GET request to GitHub with the correct pathname', () => {
+    it('should make a paginated GET request to GitHub with the correct pathname', () => {
       prs.fetchFiles(42);
-      expect(githubApi.get).toHaveBeenCalledWith('/repos/foo/bar/pulls/42/files');
+      expect(githubApi.getPaginated).toHaveBeenCalledWith('/repos/foo/bar/pulls/42/files');
     });
 
 
     it('should resolve with the data returned from GitHub', done => {
-      const expected: any = [{ sha: 'ABCDE', filename: 'a/b/c'}, { sha: '12345', filename: 'x/y/z' }];
-      githubApi.get.and.callFake(() => Promise.resolve(expected));
+      const expected: any = [{sha: 'ABCDE', filename: 'a/b/c'}, {sha: '12345', filename: 'x/y/z'}];
+      githubApi.getPaginated.and.callFake(() => Promise.resolve(expected));
       prs.fetchFiles(42).then(data => {
         expect(data).toEqual(expected);
         done();
       });
     });
+
   });
 
-
 });