From c593ab4a680c3eada4a276ce7c32663d2056a156 Mon Sep 17 00:00:00 2001
From: RahulGautamSingh <rahultesnik@gmail.com>
Date: Wed, 7 Sep 2022 20:44:51 +0530
Subject: [PATCH] feat: recreate merged PRs when updates are available (#16898)
---
.../repository/update/branch/commit.ts | 2 +-
.../update/branch/handle-existing.ts | 2 -
.../repository/update/branch/index.spec.ts | 69 +++++++++++++++++--
lib/workers/repository/update/branch/index.ts | 57 ++++++++++++---
lib/workers/types.ts | 1 +
5 files changed, 112 insertions(+), 19 deletions(-)
diff --git a/lib/workers/repository/update/branch/commit.ts b/lib/workers/repository/update/branch/commit.ts
index 827d03c5ee..78ccab0d79 100644
--- a/lib/workers/repository/update/branch/commit.ts
+++ b/lib/workers/repository/update/branch/commit.ts
@@ -55,7 +55,7 @@ export function commitFilesToBranch(
branchName: config.branchName,
files: updatedFiles,
message: config.commitMessage!,
- force: !!config.forceCommit,
+ force: config.recreateMergedPr ?? !!config.forceCommit,
platformCommit: !!config.platformCommit,
});
}
diff --git a/lib/workers/repository/update/branch/handle-existing.ts b/lib/workers/repository/update/branch/handle-existing.ts
index 735ce2b242..c031be82fa 100644
--- a/lib/workers/repository/update/branch/handle-existing.ts
+++ b/lib/workers/repository/update/branch/handle-existing.ts
@@ -41,7 +41,5 @@ export async function handlepr(config: BranchConfig, pr: Pr): Promise<void> {
await deleteBranch(config.branchName);
}
}
- } else if (pr.state === PrState.Merged) {
- logger.debug({ pr: pr.number }, 'Merged PR is blocking this branch');
}
}
diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts
index 4da4fa0c7d..e2b5dbc6c5 100644
--- a/lib/workers/repository/update/branch/index.spec.ts
+++ b/lib/workers/repository/update/branch/index.spec.ts
@@ -281,22 +281,81 @@ describe('workers/repository/update/branch/index', () => {
expect(reuse.shouldReuseExistingBranch).toHaveBeenCalledTimes(0);
});
- it('skips branch if merged PR found', async () => {
- schedule.isScheduledNow.mockReturnValueOnce(false);
+ it('recreates pr using old branch when it exists for a merged pr', async () => {
+ schedule.isScheduledNow.mockReturnValueOnce(true);
git.branchExists.mockReturnValue(true);
checkExisting.prAlreadyExisted.mockResolvedValueOnce({
number: 13,
state: PrState.Merged,
} as Pr);
- await branchWorker.processBranch(config);
- expect(reuse.shouldReuseExistingBranch).toHaveBeenCalledTimes(0);
+ git.getBranchCommit.mockReturnValue('target_branch_sha');
+ getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
+ updatedPackageFiles: [{}],
+ } as PackageFilesResult);
+ npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
+ artifactErrors: [{}],
+ updatedArtifacts: [{}],
+ } as WriteExistingFilesResult);
+ platform.getBranchPr.mockResolvedValueOnce({
+ state: PrState.Open,
+ } as Pr);
+ checkExisting.prAlreadyExisted.mockResolvedValueOnce({
+ state: PrState.Merged,
+ } as Pr);
+ config.automerge = true;
+ const processBranchResult = await branchWorker.processBranch(config);
+ expect(processBranchResult).toEqual({
+ branchExists: true,
+ commitSha: '123test',
+ prNo: undefined,
+ result: 'done',
+ });
+ expect(logger.debug).not.toHaveBeenCalledWith(
+ { prTitle: config.prTitle },
+ 'Merged PR already exists. Creating a fresh PR'
+ );
+ expect(logger.debug).toHaveBeenCalledWith(
+ 'Disable automerge to prevent re-created PRs from being merged automatically.'
+ );
+ });
+
+ it('recreates pr using new branch when merged pr exists but branch is deleted', async () => {
+ schedule.isScheduledNow.mockReturnValueOnce(true);
+ git.branchExists.mockReturnValue(false);
+ checkExisting.prAlreadyExisted.mockResolvedValueOnce({
+ number: 13,
+ state: PrState.Merged,
+ } as Pr);
+ getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
+ updatedPackageFiles: [{}],
+ } as PackageFilesResult);
+ npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
+ artifactErrors: [{}],
+ updatedArtifacts: [{}],
+ } as WriteExistingFilesResult);
+ git.getBranchCommit.mockReturnValue('target_branch_sha');
+ platform.getBranchPr.mockResolvedValueOnce(null);
+ checkExisting.prAlreadyExisted.mockResolvedValueOnce({
+ state: PrState.Merged,
+ } as Pr);
+ const processBranchResult = await branchWorker.processBranch(config);
+ expect(processBranchResult).toEqual({
+ branchExists: true,
+ commitSha: '123test',
+ prNo: undefined,
+ result: 'pr-created',
+ });
+ expect(logger.debug).toHaveBeenCalledWith(
+ { prTitle: config.prTitle },
+ 'Merged PR already exists. Creating a fresh PR'
+ );
});
it('throws error if closed PR found', async () => {
schedule.isScheduledNow.mockReturnValueOnce(false);
git.branchExists.mockReturnValue(true);
platform.getBranchPr.mockResolvedValueOnce({
- state: PrState.Merged,
+ state: PrState.Closed,
} as Pr);
git.isBranchModified.mockResolvedValueOnce(true);
await expect(branchWorker.processBranch(config)).rejects.toThrow(
diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts
index a0692ecae0..57b0942953 100644
--- a/lib/workers/repository/update/branch/index.ts
+++ b/lib/workers/repository/update/branch/index.ts
@@ -83,6 +83,18 @@ export interface ProcessBranchResult {
commitSha?: string | null;
}
+interface RecreateMergedPrConfig {
+ recreateMergedPr: boolean;
+ automerge: boolean;
+}
+
+function recreateMergedPrConfig(branchPr: Pr | null): RecreateMergedPrConfig {
+ return {
+ recreateMergedPr: !branchPr,
+ automerge: false,
+ };
+}
+
export async function processBranch(
branchConfig: BranchConfig
): Promise<ProcessBranchResult> {
@@ -114,19 +126,37 @@ export async function processBranch(
const artifactErrorTopic = emojify(':warning: Artifact update problem');
try {
// Check if branch already existed
- const existingPr = branchPr ? undefined : await prAlreadyExisted(config);
+ const existingPr = await prAlreadyExisted(config);
if (existingPr && !dependencyDashboardCheck) {
- logger.debug(
- { prTitle: config.prTitle },
- 'Closed PR already exists. Skipping branch.'
- );
- await handlepr(config, existingPr);
- return {
- branchExists: false,
- prNo: existingPr.number,
- result: BranchResult.AlreadyExisted,
- };
+ // Recreates pr if merged pr already exists
+ if (existingPr.state === 'merged') {
+ if (config.automerge) {
+ logger.debug(
+ 'Disable automerge to prevent re-created PRs from being merged automatically.'
+ );
+ }
+
+ config = { ...config, ...recreateMergedPrConfig(branchPr) };
+ if (!branchPr) {
+ logger.debug(
+ { prTitle: config.prTitle },
+ 'Merged PR already exists. Creating a fresh PR'
+ );
+ }
+ } else {
+ logger.debug(
+ { prTitle: config.prTitle },
+ 'Closed PR already exists. Skipping branch.'
+ );
+ await handlepr(config, existingPr);
+ return {
+ branchExists: false,
+ prNo: existingPr.number,
+ result: BranchResult.AlreadyExisted,
+ };
+ }
}
+
// istanbul ignore if
if (!branchExists && config.dependencyDashboardApproval) {
if (dependencyDashboardCheck) {
@@ -385,6 +415,11 @@ export async function processBranch(
prNo: branchPr?.number,
result: BranchResult.NoWork,
};
+ } else if (config.recreateMergedPr) {
+ logger.debug(
+ 'Rebase branch cause the commits from old branch have already been merged.'
+ );
+ config.reuseExistingBranch = false;
} else {
config = { ...config, ...(await shouldReuseExistingBranch(config)) };
}
diff --git a/lib/workers/types.ts b/lib/workers/types.ts
index 1da19fe992..ade345c7c3 100644
--- a/lib/workers/types.ts
+++ b/lib/workers/types.ts
@@ -126,6 +126,7 @@ export interface BranchConfig
prNo?: number;
stopUpdating?: boolean;
isConflicted?: boolean;
+ recreateMergedPr?: boolean;
branchFingerprint?: string;
skipBranchUpdate?: boolean;
}
--
GitLab