From f932ab0b0ca61b0fcd0af2406e7a73ba3e7513a8 Mon Sep 17 00:00:00 2001 From: Brad Herrmann Date: Wed, 7 Aug 2024 11:26:29 -0500 Subject: [PATCH] feat(manage-merge-queue): allow only maintainers to jump the queue --- src/helpers/is-user-in-team.ts | 2 +- src/helpers/manage-merge-queue.ts | 17 ++++++++-- test/helpers/is-user-in-team.test.ts | 11 +++++- test/helpers/manage-merge-queue.test.ts | 45 +++++++++++++++++++++++-- 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/helpers/is-user-in-team.ts b/src/helpers/is-user-in-team.ts index 9110cf7e8..6f05ff368 100644 --- a/src/helpers/is-user-in-team.ts +++ b/src/helpers/is-user-in-team.ts @@ -16,7 +16,7 @@ import { context } from '@actions/github'; import { octokit } from '../octokit'; export class IsUserInTeam extends HelperInputs { - login = ''; + login? = ''; team = ''; } diff --git a/src/helpers/manage-merge-queue.ts b/src/helpers/manage-merge-queue.ts index a80398826..789d2b5f5 100644 --- a/src/helpers/manage-merge-queue.ts +++ b/src/helpers/manage-merge-queue.ts @@ -24,13 +24,14 @@ import { PullRequest, PullRequestList } from '../types/github'; import { context } from '@actions/github'; import { notifyUser } from '../utils/notify-user'; import { octokit, octokitGraphql } from '../octokit'; -import { removeLabelIfExists } from './remove-label'; +import {removeLabel, removeLabelIfExists} from './remove-label'; import { setCommitStatus } from './set-commit-status'; import { updateMergeQueue } from '../utils/update-merge-queue'; import { paginateAllOpenPullRequests } from '../utils/paginate-open-pull-requests'; import { updatePrWithDefaultBranch } from './prepare-queued-pr-for-merge'; import { approvalsSatisfied } from './approvals-satisfied'; import { createPrComment } from './create-pr-comment'; +import { isUserInTeam } from './is-user-in-team'; import { join } from 'path'; export class ManageMergeQueue extends HelperInputs { @@ -38,9 +39,11 @@ export class ManageMergeQueue extends HelperInputs { login?: string; slack_webhook_url?: string; skip_auto_merge?: string; + maintainers_team?: string; + only_maintainers_can_jump?: boolean; } -export const manageMergeQueue = async ({ max_queue_size, login, slack_webhook_url, skip_auto_merge }: ManageMergeQueue = {}) => { +export const manageMergeQueue = async ({ max_queue_size, login, slack_webhook_url, skip_auto_merge, maintainers_team = '', only_maintainers_can_jump = false }: ManageMergeQueue = {}) => { const { data: pullRequest } = await octokit.pulls.get({ pull_number: context.issue.number, ...context.repo }); if (pullRequest.merged || !pullRequest.labels.find(label => label.name === READY_FOR_MERGE_PR_LABEL)) { core.info('This PR is not in the merge queue.'); @@ -62,6 +65,16 @@ export const manageMergeQueue = async ({ max_queue_size, login, slack_webhook_ur return removePrFromQueue(pullRequest); } if (pullRequest.labels.find(label => label.name === JUMP_THE_QUEUE_PR_LABEL)) { + if (only_maintainers_can_jump) { + const isMaintainer = await isUserInTeam({team: maintainers_team}) + if (isMaintainer != true) { + await removeLabel({label: JUMP_THE_QUEUE_PR_LABEL}) + return await createPrComment({ + body: `Only core maintainers can jump the queue. Please have a core maintainer jump the queue for you` + }); + } + } + return updateMergeQueue(queuedPrs); } if (!pullRequest.labels.find(label => label.name?.startsWith(QUEUED_FOR_MERGE_PREFIX))) { diff --git a/test/helpers/is-user-in-team.test.ts b/test/helpers/is-user-in-team.test.ts index cda44935e..dfdea6443 100644 --- a/test/helpers/is-user-in-team.test.ts +++ b/test/helpers/is-user-in-team.test.ts @@ -18,7 +18,7 @@ import { Mocktokit } from '../types'; jest.mock('@actions/core'); jest.mock('@actions/github', () => ({ - context: { repo: { repo: 'repo', owner: 'owner' }, issue: { number: 123 } }, + context: { repo: { repo: 'repo', owner: 'owner' }, issue: { number: 123 }, actor: 'admin' }, getOctokit: jest.fn(() => ({ rest: { teams: { listMembersInOrg: jest.fn() } } })) })); @@ -38,6 +38,15 @@ describe('isUserInTeam', () => { expect(response).toBe(true); }); + it('should call isUserInTeam with correct params and find user in team for context actor', async () => { + const response = await isUserInTeam({ team: 'users' }); + expect(octokit.teams.listMembersInOrg).toHaveBeenCalledWith({ + org: context.actor, + team_slug: 'users' + }); + expect(response).toBe(true); + }); + it('should call isUserInTeam with correct params and find user not in team', async () => { const response = await isUserInTeam({ login, team: 'core' }); expect(octokit.teams.listMembersInOrg).toHaveBeenCalledWith({ diff --git a/test/helpers/manage-merge-queue.test.ts b/test/helpers/manage-merge-queue.test.ts index ad39b1f22..d57640db5 100644 --- a/test/helpers/manage-merge-queue.test.ts +++ b/test/helpers/manage-merge-queue.test.ts @@ -17,17 +17,19 @@ import { context } from '@actions/github'; import { manageMergeQueue } from '../../src/helpers/manage-merge-queue'; import { notifyUser } from '../../src/utils/notify-user'; import { octokit, octokitGraphql } from '../../src/octokit'; -import { removeLabelIfExists } from '../../src/helpers/remove-label'; +import {removeLabel, removeLabelIfExists} from '../../src/helpers/remove-label'; import { setCommitStatus } from '../../src/helpers/set-commit-status'; import { updateMergeQueue } from '../../src/utils/update-merge-queue'; import { updatePrWithDefaultBranch } from '../../src/helpers/prepare-queued-pr-for-merge'; import { approvalsSatisfied } from '../../src/helpers/approvals-satisfied'; import { createPrComment } from '../../src/helpers/create-pr-comment'; +import {isUserInTeam} from "../../src/helpers/is-user-in-team"; jest.mock('../../src/helpers/remove-label'); jest.mock('../../src/helpers/set-commit-status'); jest.mock('../../src/utils/notify-user'); jest.mock('../../src/utils/update-merge-queue'); +jest.mock('../../src/helpers/is-user-in-team'); jest.mock('../../src/helpers/approvals-satisfied'); jest.mock('../../src/helpers/create-pr-comment'); jest.mock('../../src/utils/../../src/helpers/prepare-queued-pr-for-merge'); @@ -45,6 +47,10 @@ jest.mock('@actions/github', () => ({ })) })); +(isUserInTeam as jest.Mock).mockImplementation(({ team }) => { + return team === 'team' +}) + describe('manageMergeQueue', () => { describe('pr merged case', () => { const queuedPrs = [{ labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }]; @@ -336,7 +342,7 @@ describe('manageMergeQueue', () => { { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] }, { labels: [{ name: READY_FOR_MERGE_PR_LABEL }] } ]; - beforeEach(async () => { + beforeEach(() => { (octokit.pulls.list as unknown as Mocktokit).mockImplementation(async ({ page }) => ({ data: page === 1 ? queuedPrs : [] })); @@ -348,10 +354,43 @@ describe('manageMergeQueue', () => { } })); (approvalsSatisfied as jest.Mock).mockResolvedValue(true); + }); + + it('should call updateMergeQueue with correct params', async() => { await manageMergeQueue(); + + expect(isUserInTeam).toHaveBeenCalledTimes(0) + expect(removeLabel).toHaveBeenCalledTimes(0) + expect(createPrComment).toHaveBeenCalledTimes(0) + expect(updateMergeQueue).toHaveBeenCalledWith(queuedPrs); }); - it('should call updateMergeQueue with correct params', () => { + it('should call updateMergeQueue with correct params when not checking maintainers group', async() => { + await manageMergeQueue({ only_maintainers_can_jump: false }); + + expect(isUserInTeam).toHaveBeenCalledTimes(0) + expect(removeLabel).toHaveBeenCalledTimes(0) + expect(createPrComment).toHaveBeenCalledTimes(0) + expect(updateMergeQueue).toHaveBeenCalledWith(queuedPrs); + }); + + it('should call updateMergeQueue when user in maintainers group', async() => { + + + await manageMergeQueue({ maintainers_team: 'team', only_maintainers_can_jump: true }); + + expect(isUserInTeam).toHaveBeenCalled() + expect(removeLabel).toHaveBeenCalledTimes(0) + expect(createPrComment).toHaveBeenCalledTimes(0) + expect(updateMergeQueue).toHaveBeenCalledWith(queuedPrs); + }); + + it('should not call updateMergeQueue when user not in maintainers group', async() => { + await manageMergeQueue({ maintainers_team: 'not_team', only_maintainers_can_jump: true }); + + expect(isUserInTeam).toHaveBeenCalled() + expect(removeLabel).toHaveBeenCalled() + expect(createPrComment).toHaveBeenCalled() expect(updateMergeQueue).toHaveBeenCalledWith(queuedPrs); }); });