From 499ea9c74730d51d09487f5039b2667c5f928cb9 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Thu, 17 Oct 2024 18:14:55 +0530 Subject: [PATCH] feat(dashboard): on demand config migration (#31129) Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> Co-authored-by: Rhys Arkins Co-authored-by: Sergei Zharinov --- docs/usage/config-migration.md | 80 ++++ docs/usage/configuration-options.md | 6 +- .../config-migration/branch/index.spec.ts | 290 +++++++++++--- .../config-migration/branch/index.ts | 116 ++++-- .../config-migration/branch/rebase.spec.ts | 9 - .../config-migration/branch/rebase.ts | 4 - .../repository/config-migration/index.spec.ts | 66 +++- .../repository/config-migration/index.ts | 61 ++- .../repository/config-migration/pr/index.ts | 13 +- .../repository/dependency-dashboard.spec.ts | 372 ++++++++++++++++-- .../repository/dependency-dashboard.ts | 38 ++ lib/workers/repository/index.ts | 9 +- 12 files changed, 890 insertions(+), 174 deletions(-) create mode 100644 docs/usage/config-migration.md diff --git a/docs/usage/config-migration.md b/docs/usage/config-migration.md new file mode 100644 index 00000000000000..789e8ad5950d71 --- /dev/null +++ b/docs/usage/config-migration.md @@ -0,0 +1,80 @@ +# Config Migration + +As part of continuous improvement and refinement, the Renovate maintainers often rename, remove or combine configuration options. + +When the Renovate maintainers change configuration options, they add "config migration" code. +The migration code allows "legacy" config from users to keep working. +Config migration works by migrating legacy config internally, before the config is used. +If done right, config migration "just works" silently and legacy configs continue working indefinitely. +The only sign that "config migration" is needed is the debug message in the Renovate logs, noting the old and newly migrated configs. + +## Enabling config migration pull requests + +Even though Renovate allows you to keep using "legacy config", we recommend you update the configuration names in your config regularly. +Using the latest names: + +- makes it easier for you to understand the config +- helps you find the documentation for the config + +Renovate can create a config migration Pull Request, to migrate legacy config in your configuration file. +To get automated config migration Pull Requests from Renovate: set the [`configMigration`](./configuration-options.md#configmigration) config option to `true`. + +Config migration PRs are disabled by default. +But we recommend you enable config migration PRs, because: + +- the config migration PR "tells" you something changed +- up-to-date terms help you search the Renovate documentation +- up-to-date terms help you, and us, debug problems quicker + +## Config migration scenarios + +The scenarios for config migration are: + +- No config migration needed +- Config migration needed, and enabled +- Config migration needed, but disabled +- Config migration needed, but there is a previously closed migration PR + +### No config migration needed + +Renovate takes no action. + +### Config migration needed, and enabled + +Renovate will: + +1. Create a Config Migration PR +1. If the Dependency Dashboard issue is enabled, then Renovate puts a link to the Config Migration PR on the dashboard + +### Config migration needed, but disabled + +If config migration is needed, but disabled then Renovate adds a checkbox to the Dependency Dashboard (if the dashboard exists). +This is known as "on-demand" config migration, because migration PRs are only created at the request of the user by ticking the checkbox. + +The checkbox looks like this: + +``` +- [ ] Select this checkbox to let Renovate create an automated Config Migration PR. +``` + +When you select the checkbox: + +1. Renovate creates a config migration PR +2. Renovate replaces the checkbox with a link to the Config Migration PR + +For example: + +``` +See Config Migration PR: #1. +``` + +### Config migration needed, but there is a closed migration PR + +In this case, it does not matter if Config Migration is enabled, or not. +Renovate will: + +- Add a checkbox to the Dependency Dashboard issue (if enabled) +- When you select the checkbox on the dashboard, Renovate will: + 1. Delete the _old_ config migration branch + 1. Create a _new_ config migration PR + 1. Replace the checkbox with a link to the _new_ PR in the Dependency Dashboard issue diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index c3049b6f28fffa..4cadbd87afc72a 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -582,11 +582,7 @@ After we changed the [`baseBranches`](#basebranches) feature, the Renovate confi When downgrading JSON5 to JSON Renovate may also remove the JSON5 comments. This can happen because Renovate wrongly converts JSON5 to JSON, thus removing the comments. - -!!! note - When you close a config migration PR, Renovate ignores it forever. - This also means that Renovate won't create a config migration PR in future. - If you closed the PR by accident, find the closed PR and re-name the PR title to get a new PR. +For more details, read the [config migration documentation](./config-migration.md). ## configWarningReuseIssue diff --git a/lib/workers/repository/config-migration/branch/index.spec.ts b/lib/workers/repository/config-migration/branch/index.spec.ts index b0f9f6170c3785..c2b98b4687091f 100644 --- a/lib/workers/repository/config-migration/branch/index.spec.ts +++ b/lib/workers/repository/config-migration/branch/index.spec.ts @@ -37,24 +37,150 @@ describe('workers/repository/config-migration/branch/index', () => { config.branchPrefix = 'some/'; }); - it('Exits when Migration is not needed', async () => { + it('does nothing when migration disabled and checkbox unchecked', async () => { await expect( - checkConfigMigrationBranch(config, null), - ).resolves.toBeNull(); + checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'unchecked', + }, + }, + migratedData, + ), + ).resolves.toMatchObject({ result: 'no-migration-branch' }); expect(logger.debug).toHaveBeenCalledWith( - 'checkConfigMigrationBranch() Config does not need migration', + 'Config migration needed but config migration is disabled and checkbox not checked or not present.', ); }); - it('Updates migration branch & refresh PR', async () => { + it('creates migration branch when migration disabled but checkbox checked', async () => { + mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( + 'committed', + ); + await expect( + checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'checked', + }, + }, + migratedData, + ), + ).resolves.toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR'); + }); + + it('does not create a branch if migration branch is modified', async () => { + platform.getBranchPr.mockResolvedValue( + mock({ + number: 1, + }), + ); + scm.isBranchModified.mockResolvedValueOnce(true); + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'migration-pr-exists', + }, + }, + migratedData, + ); + // TODO: types (#22198) + expect(res).toMatchObject({ + result: 'migration-branch-modified', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.commitFiles).toHaveBeenCalledTimes(0); + expect(platform.refreshPr).toHaveBeenCalledTimes(0); + expect(logger.debug).toHaveBeenCalledWith( + 'Config Migration branch has been modified. Skipping branch rebase.', + ); + }); + + it('updates migration branch & refreshes pr when migration disabled but open pr exists', async () => { + platform.getBranchPr.mockResolvedValue( + mock({ + number: 1, + }), + ); + platform.refreshPr = jest.fn().mockResolvedValueOnce(null); + mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed'); + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'migration-pr-exists', + }, + }, + migratedData, + ); + // TODO: types (#22198) + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.commitFiles).toHaveBeenCalledTimes(0); + expect(platform.refreshPr).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( + 'Config Migration PR already exists', + ); + }); + + it('creates migration branch when migration enabled but no pr exists', async () => { + mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( + 'committed', + ); + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: true, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'no-checkbox', + }, + }, + migratedData, + ); + // TODO: types (#22198) + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.commitFiles).toHaveBeenCalledTimes(0); + expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR'); + }); + + it('updates migration branch & refresh PR when migration enabled and open pr exists', async () => { platform.getBranchPr.mockResolvedValue(mock()); - // platform.refreshPr is undefined as it is an optional function - // declared as: refreshPr?(number: number): Promise; platform.refreshPr = jest.fn().mockResolvedValueOnce(null); mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed'); - const res = await checkConfigMigrationBranch(config, migratedData); + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: true, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'migration-pr-exists', + }, + }, + migratedData, + ); // TODO: types (#22198) - expect(res).toBe(`${config.branchPrefix!}migrate-config`); + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); expect(git.commitFiles).toHaveBeenCalledTimes(0); expect(logger.debug).toHaveBeenCalledWith( @@ -68,23 +194,23 @@ describe('workers/repository/config-migration/branch/index', () => { }); platform.getBranchPr.mockResolvedValueOnce(mock()); mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed'); - const res = await checkConfigMigrationBranch(config, migratedData); - // TODO: types (#22198) - expect(res).toBe(`${config.branchPrefix!}migrate-config`); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); - expect(git.commitFiles).toHaveBeenCalledTimes(0); - }); - - it('Creates migration PR', async () => { - mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( - 'committed', + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: true, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'migration-pr-exists', + }, + }, + migratedData, ); - const res = await checkConfigMigrationBranch(config, migratedData); // TODO: types (#22198) - expect(res).toBe(`${config.branchPrefix!}migrate-config`); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); expect(git.commitFiles).toHaveBeenCalledTimes(0); - expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR'); }); it('Dry runs create migration PR', async () => { @@ -94,9 +220,20 @@ describe('workers/repository/config-migration/branch/index', () => { mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( 'committed', ); - const res = await checkConfigMigrationBranch(config, migratedData); + const res = await checkConfigMigrationBranch( + { + ...config, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'checked', + }, + }, + migratedData, + ); // TODO: types (#22198) - expect(res).toBe(`${config.branchPrefix!}migrate-config`); + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); expect(git.commitFiles).toHaveBeenCalledTimes(0); }); @@ -105,46 +242,95 @@ describe('workers/repository/config-migration/branch/index', () => { const title = 'PR title'; const pr = partial({ title, state: 'closed', number: 1 }); - it('skips branch when there is a closed one delete it and add an ignore PR message', async () => { + it('does not create a branch when migration is disabled but needed and a closed pr exists', async () => { platform.findPr.mockResolvedValueOnce(pr); platform.getBranchPr.mockResolvedValue(null); scm.branchExists.mockResolvedValueOnce(true); - const res = await checkConfigMigrationBranch(config, migratedData); - expect(res).toBeNull(); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - expect(scm.deleteBranch).toHaveBeenCalledTimes(1); - expect(logger.debug).toHaveBeenCalledWith( - { prTitle: title }, - 'Closed PR already exists. Skipping branch.', + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'no-checkbox', + }, + }, + migratedData, ); - expect(platform.ensureComment).toHaveBeenCalledTimes(1); - expect(platform.ensureComment).toHaveBeenCalledWith({ - content: - '\n\nIf you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR.', - topic: 'Renovate Ignore Notification', - number: 1, + expect(res).toMatchObject({ + result: 'no-migration-branch', + }); + }); + + it('deletes old branch and creates new migration branch when migration is disabled but needed, a closed pr exists and checkbox is checked', async () => { + platform.findPr.mockResolvedValueOnce(pr); + platform.getBranchPr.mockResolvedValue(null); + scm.branchExists.mockResolvedValueOnce(true); + mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( + 'committed', + ); + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'checked', + }, + }, + migratedData, + ); + expect(scm.deleteBranch).toHaveBeenCalledTimes(1); + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); }); - it('dryrun: skips branch when there is a closed one and add an ignore PR message', async () => { - GlobalConfig.set({ dryRun: 'full' }); + it('does not create a branch when migration is enabled and a closed pr exists', async () => { platform.findPr.mockResolvedValueOnce(pr); platform.getBranchPr.mockResolvedValue(null); scm.branchExists.mockResolvedValueOnce(true); - const res = await checkConfigMigrationBranch(config, migratedData); - expect(res).toBeNull(); - expect(logger.info).toHaveBeenCalledWith( - `DRY-RUN: Would ensure closed PR comment in PR #${pr.number}`, + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: true, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'no-checkbox', + }, + }, + migratedData, ); - expect(logger.info).toHaveBeenCalledWith( - 'DRY-RUN: Would delete branch ' + pr.sourceBranch, + expect(res).toMatchObject({ + result: 'no-migration-branch', + }); + }); + + it('dry run:deletes old branch and creates new migration branch when migration is disabled but needed, a closed pr exists and checkbox is checked', async () => { + GlobalConfig.set({ + dryRun: 'full', + }); + platform.findPr.mockResolvedValueOnce(pr); + platform.getBranchPr.mockResolvedValue(null); + scm.branchExists.mockResolvedValueOnce(true); + mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce( + 'committed', ); - expect(logger.debug).toHaveBeenCalledWith( - { prTitle: title }, - 'Closed PR already exists. Skipping branch.', + const res = await checkConfigMigrationBranch( + { + ...config, + configMigration: false, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'checked', + }, + }, + migratedData, ); - expect(platform.ensureComment).toHaveBeenCalledTimes(0); + expect(scm.deleteBranch).toHaveBeenCalledTimes(0); + expect(res).toMatchObject({ + result: 'migration-branch-exists', + migrationBranch: `${config.branchPrefix!}migrate-config`, + }); + expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); }); }); }); diff --git a/lib/workers/repository/config-migration/branch/index.ts b/lib/workers/repository/config-migration/branch/index.ts index 507fada5d197a9..3f14a180eecc76 100644 --- a/lib/workers/repository/config-migration/branch/index.ts +++ b/lib/workers/repository/config-migration/branch/index.ts @@ -1,9 +1,9 @@ +import is from '@sindresorhus/is'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; import type { FindPRConfig, Pr } from '../../../../modules/platform'; import { platform } from '../../../../modules/platform'; -import { ensureComment } from '../../../../modules/platform/comment'; import { scm } from '../../../../modules/platform/scm'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; @@ -11,21 +11,40 @@ import { createConfigMigrationBranch } from './create'; import type { MigratedData } from './migrated-data'; import { rebaseMigrationBranch } from './rebase'; +export type CheckConfigMigrationBranchResult = + | { result: 'no-migration-branch' } + | { + result: 'migration-branch-exists' | 'migration-branch-modified'; + migrationBranch: string; + }; + export async function checkConfigMigrationBranch( config: RenovateConfig, - migratedConfigData: MigratedData | null, -): Promise { + migratedConfigData: MigratedData, +): Promise { logger.debug('checkConfigMigrationBranch()'); - if (!migratedConfigData) { - logger.debug('checkConfigMigrationBranch() Config does not need migration'); - return null; + const configMigrationCheckboxState = + config.dependencyDashboardChecks?.configMigrationCheckboxState; + + if (!config.configMigration) { + if ( + is.undefined(configMigrationCheckboxState) || + configMigrationCheckboxState === 'no-checkbox' || + configMigrationCheckboxState === 'unchecked' + ) { + logger.debug( + 'Config migration needed but config migration is disabled and checkbox not checked or not present.', + ); + return { result: 'no-migration-branch' }; + } } + const configMigrationBranch = getMigrationBranchName(config); const branchPr = await migrationPrExists( configMigrationBranch, config.baseBranch, - ); // handles open/autoClosed PRs + ); // handles open/autoclosed PRs if (!branchPr) { const commitMessageFactory = new ConfigMigrationCommitMessageFactory( @@ -45,69 +64,84 @@ export async function checkConfigMigrationBranch( // found closed migration PR if (closedPr) { + logger.debug('Closed config migration PR found.'); + + // if a closed pr exists and the checkbox for config migration is not checked + // return no-migration-branch result so that the checkbox gets added again + // we only want to create a config migration pr if the checkbox is checked + if (configMigrationCheckboxState !== 'checked') { + logger.debug( + 'Config migration is enabled and needed. But a closed pr exists and checkbox is not checked. Skipping migration branch creation.', + ); + return { result: 'no-migration-branch' }; + } + logger.debug( - { prTitle: closedPr.title }, - 'Closed PR already exists. Skipping branch.', + 'Closed migration PR found and checkbox is checked. Try to delete this old branch and create a new one.', ); await handlePr(config, closedPr); - return null; } } + let result: CheckConfigMigrationBranchResult['result']; + if (branchPr) { logger.debug('Config Migration PR already exists'); - await rebaseMigrationBranch(config, migratedConfigData); - if (platform.refreshPr) { - const configMigrationPr = await platform.getBranchPr( - configMigrationBranch, - config.baseBranch, + + if (await isMigrationBranchModified(config, configMigrationBranch)) { + logger.debug( + 'Config Migration branch has been modified. Skipping branch rebase.', ); - if (configMigrationPr) { - await platform.refreshPr(configMigrationPr.number); + result = 'migration-branch-modified'; + } else { + await rebaseMigrationBranch(config, migratedConfigData); + if (platform.refreshPr) { + const configMigrationPr = await platform.getBranchPr( + configMigrationBranch, + config.baseBranch, + ); + if (configMigrationPr) { + await platform.refreshPr(configMigrationPr.number); + } } + result = 'migration-branch-exists'; } } else { logger.debug('Config Migration PR does not exist'); logger.debug('Need to create migration PR'); await createConfigMigrationBranch(config, migratedConfigData); + result = 'migration-branch-exists'; } if (!GlobalConfig.get('dryRun')) { await scm.checkoutBranch(configMigrationBranch); } - return configMigrationBranch; + return { + migrationBranch: configMigrationBranch, + result, + }; } export async function migrationPrExists( branchName: string, targetBranch?: string, -): Promise { - return !!(await platform.getBranchPr(branchName, targetBranch)); +): Promise { + return await platform.getBranchPr(branchName, targetBranch); } async function handlePr(config: RenovateConfig, pr: Pr): Promise { - if ( - pr.state === 'closed' && - !config.suppressNotifications!.includes('prIgnoreNotification') - ) { + if (await scm.branchExists(pr.sourceBranch)) { if (GlobalConfig.get('dryRun')) { - logger.info( - `DRY-RUN: Would ensure closed PR comment in PR #${pr.number}`, - ); + logger.info('DRY-RUN: Would delete branch ' + pr.sourceBranch); } else { - const content = - '\n\nIf you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR.'; - await ensureComment({ - number: pr.number, - topic: 'Renovate Ignore Notification', - content, - }); - } - if (await scm.branchExists(pr.sourceBranch)) { - if (GlobalConfig.get('dryRun')) { - logger.info('DRY-RUN: Would delete branch ' + pr.sourceBranch); - } else { - await scm.deleteBranch(pr.sourceBranch); - } + await scm.deleteBranch(pr.sourceBranch); } } } + +async function isMigrationBranchModified( + config: RenovateConfig, + configMigrationBranch: string, +): Promise { + const baseBranch = config.defaultBranch!; + return await scm.isBranchModified(configMigrationBranch, baseBranch); +} diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index cc8b1573b67576..4f6b9d9019d328 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -50,15 +50,6 @@ describe('workers/repository/config-migration/branch/rebase', () => { }; }); - it('does not rebase modified branch', async () => { - scm.isBranchModified.mockResolvedValueOnce(true); - - await rebaseMigrationBranch(config, migratedConfigData); - - expect(scm.checkoutBranch).toHaveBeenCalledTimes(0); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - }); - it.each([ ['renovate.json', renovateConfigJson], ['renovate.json5', renovateConfigJson5], diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 994e349e7908d7..32481a4239ad20 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -17,10 +17,6 @@ export async function rebaseMigrationBranch( logger.debug('Checking if migration branch needs rebasing'); const baseBranch = config.defaultBranch!; const branchName = getMigrationBranchName(config); - if (await scm.isBranchModified(branchName, baseBranch)) { - logger.debug('Migration branch has been edited and cannot be rebased'); - return null; - } const configFileName = migratedConfigData.filename; let contents = migratedConfigData.content; const existingContents = await getFile(configFileName, branchName); diff --git a/lib/workers/repository/config-migration/index.spec.ts b/lib/workers/repository/config-migration/index.spec.ts index 3067573bedbeb8..19ae2263eeb0f2 100644 --- a/lib/workers/repository/config-migration/index.spec.ts +++ b/lib/workers/repository/config-migration/index.spec.ts @@ -2,6 +2,7 @@ import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../test/fixtures'; import { mockedFunction, partial } from '../../../../test/util'; import { getConfig } from '../../../config/defaults'; +import type { Pr } from '../../../modules/platform/types'; import { checkConfigMigrationBranch } from './branch'; import { MigratedDataFactory } from './branch/migrated-data'; import { ensureConfigMigrationPr } from './pr'; @@ -29,32 +30,75 @@ describe('workers/repository/config-migration/index', () => { }); it('does nothing when in silent mode', async () => { - await configMigration({ ...config, mode: 'silent' }, []); + const res = await configMigration({ ...config, mode: 'silent' }, []); + expect(res).toMatchObject({ result: 'no-migration' }); expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); - it('does nothing when config migration is disabled', async () => { - await configMigration({ ...config, configMigration: false }, []); - expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); + it('skips pr creation when migration is not needed', async () => { + const branchList: string[] = []; + mockedFunction(MigratedDataFactory.getAsync).mockResolvedValue(null); + const res = await configMigration(config, branchList); + expect(res).toMatchObject({ result: 'no-migration' }); expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); - it('ensures config migration PR when migrated', async () => { + it('creates migration pr if needed', async () => { const branchList: string[] = []; - mockedFunction(checkConfigMigrationBranch).mockResolvedValue(branchName); - await configMigration(config, branchList); + mockedFunction(checkConfigMigrationBranch).mockResolvedValue({ + migrationBranch: branchName, + result: 'migration-branch-exists', + }); + mockedFunction(ensureConfigMigrationPr).mockResolvedValue( + partial({ number: 1 }), + ); + const res = await configMigration(config, branchList); + expect(res).toMatchObject({ result: 'pr-exists', prNumber: 1 }); expect(branchList).toContainEqual(branchName); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(1); }); - it('skips pr creation when migration is not needed', async () => { + it('returns add-checkbox if migration pr exists but is created by another user', async () => { + const branchList: string[] = []; + mockedFunction(checkConfigMigrationBranch).mockResolvedValue({ + migrationBranch: branchName, + result: 'migration-branch-exists', + }); + mockedFunction(ensureConfigMigrationPr).mockResolvedValue(null); + const res = await configMigration(config, branchList); + expect(res).toMatchObject({ result: 'add-checkbox' }); + expect(branchList).toContainEqual(branchName); + expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(1); + }); + + it('returns pr-modified incase the migration pr has been modified', async () => { const branchList: string[] = []; - mockedFunction(checkConfigMigrationBranch).mockResolvedValue(null); - await configMigration(config, branchList); - expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(1); + mockedFunction(checkConfigMigrationBranch).mockResolvedValue({ + migrationBranch: branchName, + result: 'migration-branch-modified', + }); + mockedFunction(ensureConfigMigrationPr).mockResolvedValue( + partial({ + number: 1, + }), + ); + const res = await configMigration(config, branchList); + expect(res).toMatchObject({ result: 'pr-modified', prNumber: 1 }); + expect(branchList).toContainEqual(branchName); + expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(1); + }); + + it('returns add-checkbox if migration is needed but not demanded', async () => { + const branchList: string[] = []; + mockedFunction(checkConfigMigrationBranch).mockResolvedValue({ + result: 'no-migration-branch', + }); + const res = await configMigration(config, branchList); + expect(res).toMatchObject({ result: 'add-checkbox' }); + expect(branchList).toBeEmptyArray(); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); }); diff --git a/lib/workers/repository/config-migration/index.ts b/lib/workers/repository/config-migration/index.ts index 856b0ceb277268..2574fdc4eacefb 100644 --- a/lib/workers/repository/config-migration/index.ts +++ b/lib/workers/repository/config-migration/index.ts @@ -4,26 +4,53 @@ import { checkConfigMigrationBranch } from './branch'; import { MigratedDataFactory } from './branch/migrated-data'; import { ensureConfigMigrationPr } from './pr'; +export type ConfigMigrationResult = + | { result: 'no-migration' } + | { result: 'add-checkbox' } + | { result: 'pr-exists' | 'pr-modified'; prNumber: number }; + export async function configMigration( config: RenovateConfig, branchList: string[], -): Promise { - if (config.configMigration) { - if (config.mode === 'silent') { - logger.debug( - 'Config migration issues are not created, updated or closed when mode=silent', - ); - return; - } - const migratedConfigData = await MigratedDataFactory.getAsync(); - const migrationBranch = await checkConfigMigrationBranch( - config, - migratedConfigData, - ); // null if migration not needed - if (migrationBranch) { - branchList.push(migrationBranch); - await ensureConfigMigrationPr(config, migratedConfigData!); - } +): Promise { + if (config.mode === 'silent') { + logger.debug( + 'Config migration issues are not created, updated or closed when mode=silent', + ); + return { result: 'no-migration' }; + } + + const migratedConfigData = await MigratedDataFactory.getAsync(); + if (!migratedConfigData) { + logger.debug('Config does not need migration'); + MigratedDataFactory.reset(); + return { result: 'no-migration' }; + } + + const res = await checkConfigMigrationBranch(config, migratedConfigData); + + // migration needed but not demanded by user + if (res.result === 'no-migration-branch') { MigratedDataFactory.reset(); + return { result: 'add-checkbox' }; } + + branchList.push(res.migrationBranch); + + const pr = await ensureConfigMigrationPr(config, migratedConfigData); + + // only happens incase a migration pr was created by another user + // for other cases in which a PR could not be found or created: we log warning and throw error from within the ensureConfigMigrationPr fn + if (!pr) { + MigratedDataFactory.reset(); + return { result: 'add-checkbox' }; + } + + MigratedDataFactory.reset(); + + return { + result: + res.result === 'migration-branch-exists' ? 'pr-exists' : 'pr-modified', + prNumber: pr.number, + }; } diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts index 53a3ccb57744cd..7239453af3cb52 100644 --- a/lib/workers/repository/config-migration/pr/index.ts +++ b/lib/workers/repository/config-migration/pr/index.ts @@ -2,6 +2,7 @@ import is from '@sindresorhus/is'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; +import type { Pr } from '../../../../modules/platform'; import { platform } from '../../../../modules/platform'; import { hashBody } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; @@ -19,7 +20,7 @@ import { getMigrationBranchName } from '../common'; export async function ensureConfigMigrationPr( config: RenovateConfig, migratedConfigData: MigratedData, -): Promise { +): Promise { logger.debug('ensureConfigMigrationPr()'); const docsLink = joinUrlParts( coerceString(config.productLinks?.documentation), @@ -74,7 +75,7 @@ ${ existingPr.title === prTitle ) { logger.debug(`Pr does not need updating, PrNo: ${existingPr.number}`); - return; + return existingPr; } // PR must need updating if (GlobalConfig.get('dryRun')) { @@ -87,7 +88,7 @@ ${ }); logger.info({ pr: existingPr.number }, 'Migration PR updated'); } - return; + return existingPr; } logger.debug('Creating migration PR'); const labels = prepareLabels(config); @@ -111,6 +112,8 @@ ${ if (pr) { await addParticipants(config, pr); } + + return pr; } } catch (err) { if ( @@ -124,8 +127,10 @@ ${ 'Migration PR already exists but cannot find it. It was probably created by a different user.', ); await scm.deleteBranch(branchName); - return; + return null; } throw err; } + + return null; } diff --git a/lib/workers/repository/dependency-dashboard.spec.ts b/lib/workers/repository/dependency-dashboard.spec.ts index 8a0c6edd53bf6b..4a67016793c32e 100644 --- a/lib/workers/repository/dependency-dashboard.spec.ts +++ b/lib/workers/repository/dependency-dashboard.spec.ts @@ -78,7 +78,12 @@ async function dryRun( ensureIssueCalls: number, ) { GlobalConfig.set({ dryRun: 'full' }); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes( ensureIssueClosingCalls, ); @@ -97,7 +102,9 @@ describe('workers/repository/dependency-dashboard', () => { }); await dependencyDashboard.readDashboardBody(conf); expect(conf).toEqual({ - dependencyDashboardChecks: {}, + dependencyDashboardChecks: { + configMigrationCheckboxState: 'no-checkbox', + }, dependencyDashboardAllPending: false, dependencyDashboardAllRateLimited: false, dependencyDashboardIssue: 1, @@ -125,6 +132,7 @@ describe('workers/repository/dependency-dashboard', () => { dependencyDashboardAllRateLimited: false, dependencyDashboardChecks: { branchName1: 'approve', + configMigrationCheckboxState: 'no-checkbox', }, dependencyDashboardIssue: 1, dependencyDashboardRebaseAllOpen: true, @@ -150,6 +158,7 @@ describe('workers/repository/dependency-dashboard', () => { dependencyDashboardChecks: { branch1: 'global-config', branch2: 'global-config', + configMigrationCheckboxState: 'no-checkbox', }, dependencyDashboardIssue: 1, dependencyDashboardRebaseAllOpen: false, @@ -174,6 +183,7 @@ describe('workers/repository/dependency-dashboard', () => { dependencyDashboardChecks: { branchName1: 'approve', branchName2: 'approve', + configMigrationCheckboxState: 'no-checkbox', }, dependencyDashboardIssue: 1, dependencyDashboardRebaseAllOpen: false, @@ -200,6 +210,7 @@ describe('workers/repository/dependency-dashboard', () => { dependencyDashboardChecks: { branchName5: 'unlimit', branchName6: 'unlimit', + configMigrationCheckboxState: 'no-checkbox', }, dependencyDashboardIssue: 1, dependencyDashboardRebaseAllOpen: false, @@ -210,6 +221,48 @@ describe('workers/repository/dependency-dashboard', () => { }); }); + it('reads dashboard body and config migration checkbox - checked', async () => { + const conf: RenovateConfig = {}; + conf.prCreation = 'approval'; + platform.findIssue.mockResolvedValueOnce({ + title: '', + number: 1, + body: '\n\n - [x] ', + }); + await dependencyDashboard.readDashboardBody(conf); + expect(conf.dependencyDashboardChecks).toEqual({ + configMigrationCheckboxState: 'checked', + }); + }); + + it('reads dashboard body and config migration checkbox - unchecked', async () => { + const conf: RenovateConfig = {}; + conf.prCreation = 'approval'; + platform.findIssue.mockResolvedValueOnce({ + title: '', + number: 1, + body: '\n\n - [ ] ', + }); + await dependencyDashboard.readDashboardBody(conf); + expect(conf.dependencyDashboardChecks).toEqual({ + configMigrationCheckboxState: 'unchecked', + }); + }); + + it('reads dashboard body and config migration pr link', async () => { + const conf: RenovateConfig = {}; + conf.prCreation = 'approval'; + platform.findIssue.mockResolvedValueOnce({ + title: '', + number: 1, + body: '\n\n ', + }); + await dependencyDashboard.readDashboardBody(conf); + expect(conf.dependencyDashboardChecks).toEqual({ + configMigrationCheckboxState: 'migration-pr-exists', + }); + }); + it('does not read dashboard body but applies checkedBranches regardless', async () => { const conf: RenovateConfig = {}; conf.dependencyDashboard = false; @@ -239,7 +292,12 @@ describe('workers/repository/dependency-dashboard', () => { it('does nothing if mode=silent', async () => { const branches: BranchConfig[] = []; config.mode = 'silent'; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(0); @@ -249,7 +307,12 @@ describe('workers/repository/dependency-dashboard', () => { it('do nothing if dependencyDashboard is disabled', async () => { const branches: BranchConfig[] = []; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); expect(platform.ensureIssue).toHaveBeenCalledTimes(0); @@ -269,7 +332,12 @@ describe('workers/repository/dependency-dashboard', () => { dependencyDashboardApproval: false, }, ]; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); expect(platform.ensureIssue).toHaveBeenCalledTimes(0); @@ -281,7 +349,12 @@ describe('workers/repository/dependency-dashboard', () => { const branches: BranchConfig[] = []; config.dependencyDashboard = true; config.dependencyDashboardAutoclose = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); expect(platform.ensureIssueClosing.mock.calls[0][0]).toBe( config.dependencyDashboardTitle, @@ -308,7 +381,12 @@ describe('workers/repository/dependency-dashboard', () => { ]; config.dependencyDashboard = true; config.dependencyDashboardAutoclose = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); expect(platform.ensureIssueClosing.mock.calls[0][0]).toBe( config.dependencyDashboardTitle, @@ -324,7 +402,12 @@ describe('workers/repository/dependency-dashboard', () => { config.dependencyDashboard = true; config.dependencyDashboardHeader = 'This is a header'; config.dependencyDashboardFooter = 'And this is a footer'; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -349,7 +432,12 @@ describe('workers/repository/dependency-dashboard', () => { 'This is a header for platform:{{platform}}'; config.dependencyDashboardFooter = 'And this is a footer for repository:{{repository}}'; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -435,7 +523,12 @@ describe('workers/repository/dependency-dashboard', () => { }, ]; config.dependencyDashboard = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -472,7 +565,12 @@ describe('workers/repository/dependency-dashboard', () => { }, ]; config.dependencyDashboard = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -517,7 +615,12 @@ describe('workers/repository/dependency-dashboard', () => { }, ]; config.dependencyDashboard = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -552,7 +655,12 @@ describe('workers/repository/dependency-dashboard', () => { }, ]; config.dependencyDashboard = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -602,7 +710,12 @@ describe('workers/repository/dependency-dashboard', () => { ]; config.dependencyDashboard = true; config.dependencyDashboardPrApproval = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].title).toBe( @@ -616,6 +729,130 @@ describe('workers/repository/dependency-dashboard', () => { await dryRun(branches, platform, 0, 1); }); + it('adds a checkbox for config migration', async () => { + const branches: BranchConfig[] = []; + config.repository = 'test'; + config.packageRules = [ + { + dependencyDashboardApproval: true, + }, + {}, + ]; + config.dependencyDashboardHeader = + 'This is a header for platform:{{platform}}'; + config.dependencyDashboardFooter = + 'And this is a footer for repository:{{repository}}'; + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + undefined, + { + result: 'add-checkbox', + }, + ); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0].title).toBe( + config.dependencyDashboardTitle, + ); + expect(platform.ensureIssue.mock.calls[0][0].body).toMatch( + ' - [ ] Select this checkbox to let Renovate create an automated Config Migration PR.', + ); + }); + + it('adds config migration pr link when it exists', async () => { + const branches: BranchConfig[] = []; + config.repository = 'test'; + config.packageRules = [ + { + dependencyDashboardApproval: true, + }, + {}, + ]; + config.dependencyDashboardHeader = + 'This is a header for platform:{{platform}}'; + config.dependencyDashboardFooter = + 'And this is a footer for repository:{{repository}}'; + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + undefined, + { + result: 'pr-exists', + prNumber: 1, + }, + ); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0].title).toBe( + config.dependencyDashboardTitle, + ); + expect(platform.ensureIssue.mock.calls[0][0].body).toMatch( + `## Config Migration Needed\n\n See Config Migration PR:`, + ); + }); + + it('adds related text when config migration pr has been modified', async () => { + const branches: BranchConfig[] = []; + config.repository = 'test'; + config.packageRules = [ + { + dependencyDashboardApproval: true, + }, + {}, + ]; + config.dependencyDashboardHeader = + 'This is a header for platform:{{platform}}'; + config.dependencyDashboardFooter = + 'And this is a footer for repository:{{repository}}'; + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + undefined, + { + result: 'pr-modified', + prNumber: 1, + }, + ); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0].title).toBe( + config.dependencyDashboardTitle, + ); + expect(platform.ensureIssue.mock.calls[0][0].body).toMatch( + 'The Config Migration branch exists but has been modified by another user. Renovate will not push to this branch unless it is first deleted.', + ); + }); + + it('does not add a config migration checkbox when not needed', async () => { + const branches: BranchConfig[] = []; + config.repository = 'test'; + config.packageRules = [ + { + dependencyDashboardApproval: true, + }, + {}, + ]; + config.dependencyDashboardHeader = + 'This is a header for platform:{{platform}}'; + config.dependencyDashboardFooter = + 'And this is a footer for repository:{{repository}}'; + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0].title).toBe( + config.dependencyDashboardTitle, + ); + expect(platform.ensureIssue.mock.calls[0][0].body).not.toMatch( + '## Config Migration Needed', + ); + }); + it('contains logged problems', async () => { const branches: BranchConfig[] = [ { @@ -660,7 +897,12 @@ describe('workers/repository/dependency-dashboard', () => { }, ]); config.dependencyDashboard = true; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); }); @@ -688,7 +930,12 @@ describe('workers/repository/dependency-dashboard', () => { repoProblemsHeader: 'platform is {{platform}}', }; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toContain( @@ -732,7 +979,12 @@ describe('workers/repository/dependency-dashboard', () => { - [ ] pr2 - [x] 🔐 **Create all pending approval PRs at once** 🔐`, }); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); const checkApprovePendingSelectAll = regEx( / - \[ ] /g, ); @@ -791,7 +1043,12 @@ describe('workers/repository/dependency-dashboard', () => { - [ ] pr1 - [ ] pr2`, }); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); const checkRateLimitedSelectAll = regEx( / - \[ ] /g, ); @@ -869,7 +1126,12 @@ describe('workers/repository/dependency-dashboard', () => { - [x] ' `, }); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); }); @@ -894,7 +1156,12 @@ None detected title: 'Dependency Dashboard', body: '', }); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).not.toHaveBeenCalled(); }); @@ -902,7 +1169,12 @@ None detected const branches: BranchConfig[] = []; config.dependencyDashboard = true; config.dependencyDashboardLabels = ['RenovateBot', 'Maintenance']; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].labels).toStrictEqual([ 'RenovateBot', @@ -934,7 +1206,12 @@ None detected it('add detected dependencies to the Dependency Dashboard body', async () => { const branches: BranchConfig[] = []; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -946,7 +1223,12 @@ None detected const branches: BranchConfig[] = []; PackageFiles.clear(); PackageFiles.add('main', {}); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -958,7 +1240,12 @@ None detected const branches: BranchConfig[] = []; PackageFiles.clear(); PackageFiles.add('main', null); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -969,7 +1256,12 @@ None detected it('shows different combinations of version+digest for a given dependency', async () => { const branches: BranchConfig[] = []; PackageFiles.add('main', packageFilesWithDigest); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -993,6 +1285,7 @@ None detected config, branches, packageFilesWithDeprecations, + { result: 'no-migration' }, ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toInclude( @@ -1018,7 +1311,12 @@ None detected it('add detected dependencies to the Dependency Dashboard body', async () => { const branches: BranchConfig[] = []; - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -1029,7 +1327,12 @@ None detected it('show default message in issues body when packageFiles is empty', async () => { const branches: BranchConfig[] = []; PackageFiles.add('main', {}); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -1040,7 +1343,12 @@ None detected it('show default message in issues body when when packageFiles is null', async () => { const branches: BranchConfig[] = []; PackageFiles.add('main', null); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); @@ -1053,7 +1361,12 @@ None detected const packageFilesBigRepo = genRandPackageFile(100, 700); PackageFiles.clear(); PackageFiles.add('main', packageFilesBigRepo); - await dependencyDashboard.ensureDependencyDashboard(config, branches); + await dependencyDashboard.ensureDependencyDashboard( + config, + branches, + {}, + { result: 'no-migration' }, + ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect( platform.ensureIssue.mock.calls[0][0].body.length < @@ -1092,6 +1405,7 @@ None detected config, branches, packageFiles, + { result: 'no-migration' }, ); expect(platform.ensureIssue).toHaveBeenCalledTimes(1); expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot(); diff --git a/lib/workers/repository/dependency-dashboard.ts b/lib/workers/repository/dependency-dashboard.ts index d23e5e70beb298..9c61879a877aa9 100644 --- a/lib/workers/repository/dependency-dashboard.ts +++ b/lib/workers/repository/dependency-dashboard.ts @@ -9,6 +9,7 @@ import { coerceString } from '../../util/string'; import * as template from '../../util/template'; import type { BranchConfig, SelectAllConfig } from '../types'; import { extractRepoProblems } from './common'; +import type { ConfigMigrationResult } from './config-migration'; import { getDepWarningsDashboard } from './errors-warnings'; import { PackageFiles } from './package-files'; import type { Vulnerability } from './process/types'; @@ -47,6 +48,24 @@ function checkRebaseAll(issueBody: string): boolean { return issueBody.includes(' - [x] '); } +function getConfigMigrationCheckboxState( + issueBody: string, +): 'no-checkbox' | 'checked' | 'unchecked' | 'migration-pr-exists' { + if (issueBody.includes('')) { + return 'migration-pr-exists'; + } + + if (issueBody.includes(' - [x] ')) { + return 'checked'; + } + + if (issueBody.includes(' - [ ] ')) { + return 'unchecked'; + } + + return 'no-checkbox'; +} + function selectAllRelevantBranches(issueBody: string): string[] { const checkedBranches = []; if (checkOpenAllRateLimitedPR(issueBody)) { @@ -92,6 +111,8 @@ function parseDashboardIssue(issueBody: string): DependencyDashboard { const dependencyDashboardAllPending = checkApproveAllPendingPR(issueBody); const dependencyDashboardAllRateLimited = checkOpenAllRateLimitedPR(issueBody); + dependencyDashboardChecks['configMigrationCheckboxState'] = + getConfigMigrationCheckboxState(issueBody); return { dependencyDashboardChecks, dependencyDashboardRebaseAllOpen, @@ -178,6 +199,7 @@ export async function ensureDependencyDashboard( config: SelectAllConfig, allBranches: BranchConfig[], packageFiles: Record = {}, + configMigrationRes: ConfigMigrationResult, ): Promise { logger.debug('ensureDependencyDashboard()'); if (config.mode === 'silent') { @@ -263,6 +285,22 @@ export async function ensureDependencyDashboard( return; } let issueBody = ''; + + if (configMigrationRes.result === 'pr-exists') { + issueBody += + '## Config Migration Needed\n\n' + + ` See Config Migration PR: #${configMigrationRes.prNumber}.\n\n`; + } else if (configMigrationRes?.result === 'pr-modified') { + issueBody += + '## Config Migration Needed (error)\n\n' + + ` The Config Migration branch exists but has been modified by another user. Renovate will not push to this branch unless it is first deleted. \n\n See Config Migration PR: #${configMigrationRes.prNumber}.\n\n`; + } else if (configMigrationRes?.result === 'add-checkbox') { + issueBody += + '## Config Migration Needed\n\n' + + ' - [ ] Select this checkbox to let Renovate create an automated Config Migration PR.' + + '\n\n'; + } + if (config.dependencyDashboardHeader?.length) { issueBody += template.compile(config.dependencyDashboardHeader, config) + '\n\n'; diff --git a/lib/workers/repository/index.ts b/lib/workers/repository/index.ts index 25536aad048604..81d244e7fa931b 100644 --- a/lib/workers/repository/index.ts +++ b/lib/workers/repository/index.ts @@ -102,8 +102,13 @@ export async function renovateRepository( } logger.debug(`Automerged but already retried once`); } else { - await configMigration(config, branchList); - await ensureDependencyDashboard(config, branches, packageFiles); + const configMigrationRes = await configMigration(config, branchList); + await ensureDependencyDashboard( + config, + branches, + packageFiles, + configMigrationRes, + ); } await finalizeRepo(config, branchList); // TODO #22198