diff --git a/.changeset/witty-wasps-kiss.md b/.changeset/witty-wasps-kiss.md new file mode 100644 index 0000000000..ef51ef7bfd --- /dev/null +++ b/.changeset/witty-wasps-kiss.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Update the `github:publish` action to allow passing wether to dismiss stale reviews on the protected default branch. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts index fc39e03cbc..dfd3297b87 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.test.ts @@ -495,4 +495,70 @@ describe('github:repo:push', () => { expect(enableBranchProtectionOnDefaultRepoBranch).not.toHaveBeenCalled(); }); + + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of dismissStaleReviews', async () => { + mockOctokit.rest.repos.get.mockResolvedValue({ + data: { + clone_url: 'https://github.com/clone/url.git', + html_url: 'https://github.com/html/url', + }, + }); + + await action.handler(mockContext); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: false, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + dismissStaleReviews: true, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: true, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + dismissStaleReviews: false, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repository', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: false, + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts index 1694e37710..4c931e029c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoPush.ts @@ -49,6 +49,7 @@ export function createGithubRepoPushAction(options: { gitAuthorName?: string; gitAuthorEmail?: string; requireCodeOwnerReviews?: boolean; + dismissStaleReviews?: boolean; bypassPullRequestAllowances?: | { users?: string[]; @@ -71,6 +72,7 @@ export function createGithubRepoPushAction(options: { properties: { repoUrl: inputProps.repoUrl, requireCodeOwnerReviews: inputProps.requireCodeOwnerReviews, + dismissStaleReviews: inputProps.dismissStaleReviews, requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, bypassPullRequestAllowances: inputProps.bypassPullRequestAllowances, requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, @@ -102,6 +104,7 @@ export function createGithubRepoPushAction(options: { gitAuthorName, gitAuthorEmail, requireCodeOwnerReviews = false, + dismissStaleReviews = false, bypassPullRequestAllowances, requiredStatusCheckContexts = [], requireBranchesToBeUpToDate = true, @@ -148,6 +151,7 @@ export function createGithubRepoPushAction(options: { gitCommitMessage, gitAuthorName, gitAuthorEmail, + dismissStaleReviews, ); ctx.output('remoteUrl', remoteUrl); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts index fe29378e2e..4dc7034f18 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -263,6 +263,7 @@ export async function initRepoPushAndProtect( gitCommitMessage?: string, gitAuthorName?: string, gitAuthorEmail?: string, + dismissStaleReviews?: boolean, ) { const gitAuthorInfo = { name: gitAuthorName @@ -303,6 +304,7 @@ export async function initRepoPushAndProtect( requiredStatusCheckContexts, requireBranchesToBeUpToDate, enforceAdmins: protectEnforceAdmins, + dismissStaleReviews: dismissStaleReviews, }); } catch (e) { assertError(e); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts index e832c3dbfa..6c3bebf47e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/inputProperties.ts @@ -38,6 +38,12 @@ const requireCodeOwnerReviews = { 'Require an approved review in PR including files with a designated Code Owner', type: 'boolean', }; +const dismissStaleReviews = { + title: 'Dismiss Stale Reviews', + description: + 'New reviewable commits pushed to a matching branch will dismiss pull request review approvals.', + type: 'boolean', +}; const requiredStatusCheckContexts = { title: 'Required Status Check Contexts', description: @@ -207,6 +213,7 @@ export { bypassPullRequestAllowances }; export { repoUrl }; export { repoVisibility }; export { requireCodeOwnerReviews }; +export { dismissStaleReviews }; export { requiredStatusCheckContexts }; export { requireBranchesToBeUpToDate }; export { sourcePath }; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts index 5afe51cc35..a7d29ec756 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/helpers.ts @@ -193,6 +193,7 @@ type BranchProtectionOptions = { requireBranchesToBeUpToDate?: boolean; defaultBranch?: string; enforceAdmins?: boolean; + dismissStaleReviews?: boolean; }; export const enableBranchProtectionOnDefaultRepoBranch = async ({ @@ -206,6 +207,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ requireBranchesToBeUpToDate = true, defaultBranch = 'master', enforceAdmins = true, + dismissStaleReviews = false, }: BranchProtectionOptions): Promise => { const tryOnce = async () => { try { @@ -233,6 +235,7 @@ export const enableBranchProtectionOnDefaultRepoBranch = async ({ required_approving_review_count: 1, require_code_owner_reviews: requireCodeOwnerReviews, bypass_pull_request_allowances: bypassPullRequestAllowances, + dismiss_stale_reviews: dismissStaleReviews, }, }); } catch (e) { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts index cebc968669..6c4cd55d98 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts @@ -922,4 +922,73 @@ describe('publish:github', () => { names: ['node.js'], }); }); + + it('should call enableBranchProtectionOnDefaultRepoBranch with the correct values of dismissStaleReviews', async () => { + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { type: 'User' }, + }); + + mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ + data: { + name: 'repo', + }, + }); + + await action.handler(mockContext); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: false, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + dismissStaleReviews: true, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: true, + }); + + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + dismissStaleReviews: false, + }, + }); + + expect(enableBranchProtectionOnDefaultRepoBranch).toHaveBeenCalledWith({ + owner: 'owner', + client: mockOctokit, + repoName: 'repo', + logger: mockContext.logger, + defaultBranch: 'master', + requireCodeOwnerReviews: false, + requiredStatusCheckContexts: [], + requireBranchesToBeUpToDate: true, + enforceAdmins: true, + dismissStaleReviews: false, + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index be4cbb1dad..1d3cce8067 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -68,6 +68,7 @@ export function createPublishGithubAction(options: { } | undefined; requireCodeOwnerReviews?: boolean; + dismissStaleReviews?: boolean; requiredStatusCheckContexts?: string[]; requireBranchesToBeUpToDate?: boolean; repoVisibility?: 'private' | 'internal' | 'public'; @@ -103,6 +104,7 @@ export function createPublishGithubAction(options: { access: inputProps.access, bypassPullRequestAllowances: inputProps.bypassPullRequestAllowances, requireCodeOwnerReviews: inputProps.requireCodeOwnerReviews, + dismissStaleReviews: inputProps.dismissStaleReviews, requiredStatusCheckContexts: inputProps.requiredStatusCheckContexts, requireBranchesToBeUpToDate: inputProps.requireBranchesToBeUpToDate, repoVisibility: inputProps.repoVisibility, @@ -138,6 +140,7 @@ export function createPublishGithubAction(options: { homepage, access, requireCodeOwnerReviews = false, + dismissStaleReviews = false, bypassPullRequestAllowances, requiredStatusCheckContexts = [], requireBranchesToBeUpToDate = true, @@ -213,6 +216,7 @@ export function createPublishGithubAction(options: { gitCommitMessage, gitAuthorName, gitAuthorEmail, + dismissStaleReviews, ); ctx.output('remoteUrl', remoteUrl);