From 3e9bfa25d351dc70eeae4dcbfa1cf38703ee96b7 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 4 Apr 2021 13:08:08 +0400 Subject: [PATCH] feat(github): Time threshold for PR reopening (#9392) --- .../github/__snapshots__/index.spec.ts.snap | 51 +++++++++++++++++++ lib/platform/github/index.spec.ts | 31 +++++++++++ lib/platform/github/index.ts | 17 ++++++- lib/platform/types.ts | 1 + 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/lib/platform/github/__snapshots__/index.spec.ts.snap b/lib/platform/github/__snapshots__/index.spec.ts.snap index ea29ddd14c88dc..222091c07e42f4 100644 --- a/lib/platform/github/__snapshots__/index.spec.ts.snap +++ b/lib/platform/github/__snapshots__/index.spec.ts.snap @@ -1873,6 +1873,57 @@ Array [ ] `; +exports[`platform/github getBranchPr(branchName) aborts reopen if PR is too old 1`] = ` +Array [ + Object { + "graphql": Object { + "query": Object { + "repository": Object { + "__args": Object { + "name": "repo", + "owner": "some", + }, + "defaultBranchRef": Object { + "name": null, + "target": Object { + "oid": null, + }, + }, + "isArchived": null, + "isFork": null, + "mergeCommitAllowed": null, + "nameWithOwner": null, + "rebaseMergeAllowed": null, + "squashMergeAllowed": null, + }, + }, + }, + "headers": Object { + "accept": "application/vnd.github.v3+json", + "accept-encoding": "gzip, deflate", + "authorization": "token abc123", + "content-length": "330", + "content-type": "application/json", + "host": "api.github.com", + "user-agent": "https://github.com/renovatebot/renovate", + }, + "method": "POST", + "url": "https://api.github.com/graphql", + }, + Object { + "headers": Object { + "accept": "application/vnd.github.v3+json", + "accept-encoding": "gzip, deflate", + "authorization": "token abc123", + "host": "api.github.com", + "user-agent": "https://github.com/renovatebot/renovate", + }, + "method": "GET", + "url": "https://api.github.com/repos/some/repo/pulls?per_page=100&state=all", + }, +] +`; + exports[`platform/github getBranchPr(branchName) aborts reopening if PR reopening fails 1`] = ` Array [ Object { diff --git a/lib/platform/github/index.spec.ts b/lib/platform/github/index.spec.ts index d85d0c818fed4b..6140a1b1b309eb 100644 --- a/lib/platform/github/index.spec.ts +++ b/lib/platform/github/index.spec.ts @@ -1,4 +1,5 @@ import fs from 'fs-extra'; +import { DateTime } from 'luxon'; import * as httpMock from '../../../test/http-mock'; import { mocked } from '../../../test/util'; import { @@ -540,6 +541,9 @@ describe('platform/github', () => { head: { ref: 'somebranch', repo: { full_name: 'some/repo' } }, title: 'old title - autoclosed', state: PrState.Closed, + closed_at: DateTime.now() + .minus({ days: 6, hours: 23, minutes: 59, seconds: 59 }) + .toISO(), }, ]) .post('/repos/some/repo/git/refs') @@ -566,6 +570,31 @@ describe('platform/github', () => { expect(pr).toMatchSnapshot(); expect(httpMock.getTrace()).toMatchSnapshot(); }); + it('aborts reopen if PR is too old', async () => { + const scope = httpMock.scope(githubApiHost); + initRepoMock(scope, 'some/repo'); + scope.get('/repos/some/repo/pulls?per_page=100&state=all').reply(200, [ + { + number: 90, + head: { ref: 'somebranch', repo: { full_name: 'other/repo' } }, + state: PrState.Open, + }, + { + number: 91, + head: { ref: 'somebranch', repo: { full_name: 'some/repo' } }, + title: 'old title - autoclosed', + state: PrState.Closed, + closed_at: DateTime.now().minus({ days: 7 }).toISO(), + }, + ]); + + await github.initRepo({ + repository: 'some/repo', + } as any); + const pr = await github.getBranchPr('somebranch'); + expect(pr).toBeNull(); + expect(httpMock.getTrace()).toMatchSnapshot(); + }); it('aborts reopening if branch recreation fails', async () => { const scope = httpMock.scope(githubApiHost); initRepoMock(scope, 'some/repo'); @@ -577,6 +606,7 @@ describe('platform/github', () => { head: { ref: 'somebranch', repo: { full_name: 'some/repo' } }, title: 'old title - autoclosed', state: PrState.Closed, + closed_at: DateTime.now().minus({ minutes: 10 }).toISO(), }, ]) .post('/repos/some/repo/git/refs') @@ -602,6 +632,7 @@ describe('platform/github', () => { head: { ref: 'somebranch', repo: { full_name: 'some/repo' } }, title: 'old title - autoclosed', state: PrState.Closed, + closed_at: DateTime.now().minus({ minutes: 10 }).toISO(), }, ]) .post('/repos/some/repo/git/refs') diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts index 6fa8b3e047dc88..909987b336b823 100644 --- a/lib/platform/github/index.ts +++ b/lib/platform/github/index.ts @@ -1,6 +1,7 @@ import URL from 'url'; import is from '@sindresorhus/is'; import delay from 'delay'; +import { DateTime } from 'luxon'; import { PLATFORM_INTEGRATION_UNAUTHORIZED, REPOSITORY_ACCESS_FORBIDDEN, @@ -759,7 +760,7 @@ export async function getPrList(): Promise { ? /* istanbul ignore next */ PrState.Merged : pr.state, createdAt: pr.created_at, - closed_at: pr.closed_at, + closedAt: pr.closed_at, sourceRepo: pr.head?.repo?.full_name, } as never) ); @@ -788,6 +789,8 @@ export async function findPr({ return pr; } +const REOPEN_THRESHOLD_MILLIS = 1000 * 60 * 60 * 24 * 7; + // Returns the Pull Request for a branch. Null if not exists. export async function getBranchPr(branchName: string): Promise { // istanbul ignore if @@ -807,7 +810,17 @@ export async function getBranchPr(branchName: string): Promise { branchName, state: PrState.Closed, }); - if (autoclosedPr?.title?.endsWith(' - autoclosed')) { + if ( + autoclosedPr?.title?.endsWith(' - autoclosed') && + autoclosedPr?.closedAt + ) { + const closedMillisAgo = DateTime.fromISO(autoclosedPr.closedAt) + .diffNow() + .negate() + .toMillis(); + if (closedMillisAgo > REOPEN_THRESHOLD_MILLIS) { + return null; + } logger.debug({ autoclosedPr }, 'Found autoclosed PR for branch'); const { sha, number } = autoclosedPr; try { diff --git a/lib/platform/types.ts b/lib/platform/types.ts index ad73807f6e6758..c72e89da171e63 100644 --- a/lib/platform/types.ts +++ b/lib/platform/types.ts @@ -47,6 +47,7 @@ export interface Pr { canMerge?: boolean; canMergeReason?: string; createdAt?: string; + closedAt?: string; displayNumber?: string; hasAssignees?: boolean; hasReviewers?: boolean;