From 196099eddb48e487a8e7aaaf18bbaa9a3a80cfb0 Mon Sep 17 00:00:00 2001 From: Jan Hesse Date: Tue, 18 Jun 2024 23:51:25 +0200 Subject: [PATCH] feat: Support 'issue_comment' event as trigger for actions (#754) --- __fixtures__/unit/helper.js | 2 +- __tests__/unit/actions/assign.test.js | 33 +++++++++++++---- __tests__/unit/actions/checks.test.js | 37 +++++++++++++++++-- __tests__/unit/actions/close.test.js | 22 +++++++++++ __tests__/unit/actions/comment.test.js | 28 ++++++++++++-- __tests__/unit/actions/labels.test.js | 25 +++++++++++++ .../transformers/v2Config.test.js | 36 ++++++++++++++++++ docs/actions/assign.rst | 4 +- docs/actions/{check.rst => checks.rst} | 6 ++- docs/actions/close.rst | 4 +- docs/actions/comment.rst | 4 +- docs/actions/labels.rst | 2 +- docs/actions/merge.rst | 4 +- docs/actions/request_review.rst | 2 +- docs/changelog.rst | 1 + docs/configuration.rst | 2 +- lib/actions/assign.js | 3 +- lib/actions/checks.js | 13 +++++-- lib/actions/close.js | 1 + lib/actions/comment.js | 1 + lib/actions/labels.js | 1 + lib/configuration/transformers/v2Config.js | 4 +- 22 files changed, 203 insertions(+), 32 deletions(-) rename docs/actions/{check.rst => checks.rst} (95%) diff --git a/__fixtures__/unit/helper.js b/__fixtures__/unit/helper.js index fc7bdb11..081bb10e 100644 --- a/__fixtures__/unit/helper.js +++ b/__fixtures__/unit/helper.js @@ -182,7 +182,7 @@ module.exports = { }, requestReviewers: jest.fn().mockReturnValue(options.requestReviewers || 'request review success'), merge: jest.fn().mockReturnValue(options.merge || 'merged'), - get: jest.fn() + get: jest.fn().mockReturnValue({ data: { head: { ref: 'test', sha: 'sha1' } } }) }, paginate: jest.fn(async (fn, cb) => { return fn.then(cb) diff --git a/__tests__/unit/actions/assign.test.js b/__tests__/unit/actions/assign.test.js index ed3fc9e1..6f828ff7 100644 --- a/__tests__/unit/actions/assign.test.js +++ b/__tests__/unit/actions/assign.test.js @@ -1,15 +1,32 @@ const Assign = require('../../../lib/actions/assign') const Helper = require('../../../__fixtures__/unit/helper') +test.each([ + undefined, + 'pull_request', + 'issues', + 'issue_comment' +])('check that assign is called for %s events', async (eventName) => { + const settings = { + assignees: [] + } + + const assign = new Assign() + const context = createMockContext(eventName) + + await assign.afterValidate(context, settings) + expect(context.octokit.issues.addAssignees.mock.calls.length).toBe(1) +}) + test('check that assignees are added when afterValidate is called with proper parameter', async () => { const settings = { assignees: ['testuser1', 'testuser2'] } - const comment = new Assign() + const assign = new Assign() const context = createMockContext() - await comment.afterValidate(context, settings) + await assign.afterValidate(context, settings) expect(context.octokit.issues.addAssignees.mock.calls.length).toBe(1) expect(context.octokit.issues.addAssignees.mock.calls[0][0].assignees[0]).toBe('testuser1') expect(context.octokit.issues.addAssignees.mock.calls[0][0].assignees[1]).toBe('testuser2') @@ -20,10 +37,10 @@ test('check that creator is added when assignee is @author', async () => { assignees: ['@author'] } - const comment = new Assign() + const assign = new Assign() const context = createMockContext() - await comment.afterValidate(context, settings) + await assign.afterValidate(context, settings) expect(context.octokit.issues.addAssignees.mock.calls.length).toBe(1) expect(context.octokit.issues.addAssignees.mock.calls[0][0].assignees[0]).toBe('creator') }) @@ -33,7 +50,7 @@ test('check only authorized users are added as assignee ', async () => { assignees: ['testuser1', 'testuser2'] } - const comment = new Assign() + const assign = new Assign() const context = createMockContext() context.octokit.issues.checkUserCanBeAssigned = (input) => { @@ -45,14 +62,14 @@ test('check only authorized users are added as assignee ', async () => { }) } - await comment.afterValidate(context, settings) + await assign.afterValidate(context, settings) expect(context.octokit.issues.addAssignees.mock.calls.length).toBe(1) expect(context.octokit.issues.addAssignees.mock.calls[0][0].assignees[0]).toBe('testuser1') expect(context.octokit.issues.addAssignees.mock.calls[0][0].assignees[1]).toBeUndefined() }) -const createMockContext = () => { - const context = Helper.mockContext() +const createMockContext = (eventName = undefined) => { + const context = Helper.mockContext({ eventName }) context.octokit.issues.addAssignees = jest.fn() return context diff --git a/__tests__/unit/actions/checks.test.js b/__tests__/unit/actions/checks.test.js index d07c3f14..0bfb75d0 100644 --- a/__tests__/unit/actions/checks.test.js +++ b/__tests__/unit/actions/checks.test.js @@ -2,14 +2,43 @@ const MetaData = require('../../../lib/metaData') const Checks = require('../../../lib/actions/checks') const Helper = require('../../../__fixtures__/unit/helper') -test('run', async () => { +test.each([ + undefined, + 'pull_request', + 'pull_request_review', + 'issue_comment' +])('that checks is called for %s events', async (eventName) => { + const checks = new Checks() + const context = createMockContext(eventName) + const result = {} + const settings = { + payload: {}, + state: 'completed', + status: 'success' + } + + const name = undefined + + checks.checkRunResult = new Map() + + checks.checkRunResult.set(name, { + data: { + id: '3' + } + }) + + await checks.afterValidate(context, settings, name, result) + expect(context.octokit.checks.update.mock.calls.length).toBe(1) +}) + +test('that run calls create api', async () => { const checks = new Checks() const context = createMockContext() await checks.run({ context, payload: {} }) expect(context.octokit.checks.create.mock.calls.length).toBe(1) }) -test('check that checks created when doPostAction is called with proper parameter', async () => { +test('that checks created when doPostAction is called with proper parameter', async () => { const checks = new Checks() const context = createMockContext() const settings = { name: 'test' } @@ -183,8 +212,8 @@ test('that correct name is used afterValidate payload', async () => { expect(MetaData.exists(output.text)).toBe(true) }) -const createMockContext = () => { - const context = Helper.mockContext() +const createMockContext = (eventName = undefined) => { + const context = Helper.mockContext({ eventName }) context.payload.action = 'actionName' context.octokit.checks.create = jest.fn() context.octokit.checks.update = jest.fn() diff --git a/__tests__/unit/actions/close.test.js b/__tests__/unit/actions/close.test.js index a7c74b06..682db3cd 100644 --- a/__tests__/unit/actions/close.test.js +++ b/__tests__/unit/actions/close.test.js @@ -1,6 +1,28 @@ const Close = require('../../../lib/actions/close') const Helper = require('../../../__fixtures__/unit/helper') +test.each([ + undefined, + 'pull_request', + 'issues', + 'issue_comment', + 'schedule' +])('check that close is called for %s events', async (eventName) => { + const close = new Close() + const context = Helper.mockContext({ eventName: eventName }) + const schedulerResult = { + validationSuites: [{ + schedule: { + issues: [{ number: 1, user: { login: 'scheduler' } }], + pulls: [] + } + }] + } + + await close.afterValidate(context, {}, '', schedulerResult) + expect(context.octokit.issues.update.mock.calls.length).toBe(1) +}) + test('check that issue is closed', async () => { const close = new Close() const context = Helper.mockContext() diff --git a/__tests__/unit/actions/comment.test.js b/__tests__/unit/actions/comment.test.js index f8703fd8..3871005b 100644 --- a/__tests__/unit/actions/comment.test.js +++ b/__tests__/unit/actions/comment.test.js @@ -15,6 +15,27 @@ const result = { }] } +test.each([ + undefined, + 'pull_request', + 'issues', + 'issue_comment', + 'schedule' +])('check that comment is called for %s events', async (eventName) => { + const comment = new Comment() + const context = createMockContext([], eventName) + const schedulerResult = { ...result } + schedulerResult.validationSuites = [{ + schedule: { + issues: [{ number: 1, user: { login: 'scheduler' } }], + pulls: [] + } + }] + + await comment.afterValidate(context, settings, '', schedulerResult) + expect(context.octokit.issues.createComment.mock.calls.length).toBe(1) +}) + test('check that comment created when afterValidate is called with proper parameter', async () => { const comment = new Comment() const context = createMockContext() @@ -34,8 +55,7 @@ test('check that comment created when afterValidate is called with proper parame test('that comment is created three times when result contain three issues found to be acted on', async () => { const comment = new Comment() - const context = createMockContext([], 'repository') - context.eventName = 'schedule' + const context = createMockContext([], 'schedule', 'repository') const schedulerResult = { ...result } schedulerResult.validationSuites = [{ schedule: { @@ -236,8 +256,8 @@ test('error handling includes removing old error comments and creating new error expect(context.octokit.issues.createComment.mock.calls[0][0].body).toBe('creator , do something!') }) -const createMockContext = (listComments, event = undefined) => { - const context = Helper.mockContext({ listComments, event }) +const createMockContext = (listComments, eventName = undefined, event = undefined) => { + const context = Helper.mockContext({ listComments, eventName, event }) context.octokit.issues.createComment = jest.fn() context.octokit.issues.deleteComment = jest.fn() diff --git a/__tests__/unit/actions/labels.test.js b/__tests__/unit/actions/labels.test.js index 0e67fbd8..396a66b8 100644 --- a/__tests__/unit/actions/labels.test.js +++ b/__tests__/unit/actions/labels.test.js @@ -2,6 +2,31 @@ const Labels = require('../../../lib/actions/labels') const Helper = require('../../../__fixtures__/unit/helper') const UnSupportedSettingError = require('../../../lib/errors/unSupportedSettingError') +test.each([ + undefined, + 'pull_request', + 'issues', + 'issue_comment', + 'schedule' +])('check that close is called for %s events', async (eventName) => { + const labels = new Labels() + const context = createMockContext([], eventName) + const settings = { + add: ['a label'] + } + const schedulerResult = { + validationSuites: [{ + schedule: { + issues: [{ number: 1, user: { login: 'scheduler' } }], + pulls: [] + } + }] + } + + await labels.afterValidate(context, settings, '', schedulerResult) + expect(context.octokit.issues.setLabels.mock.calls.length).toBe(1) +}) + test('check that replace replaces existing labels', async () => { const labels = new Labels() const context = createMockContext(['drop_label']) diff --git a/__tests__/unit/configuration/transformers/v2Config.test.js b/__tests__/unit/configuration/transformers/v2Config.test.js index 2fa81bcd..757fa4ef 100644 --- a/__tests__/unit/configuration/transformers/v2Config.test.js +++ b/__tests__/unit/configuration/transformers/v2Config.test.js @@ -92,6 +92,42 @@ test('pass, fail, error defaults will load when pull_request is mixed with other expect(transformed.mergeable[0].error).toEqual(constants.DEFAULT_PR_ERROR) }) +test('pass, fail, error defaults will load when issue_comment event is specified.', () => { + const config = ` + version: 2 + mergeable: + - when: issue_comment.* + validate: + - do: lastComment + must_exclude: + regex: 'wip|work in progress' + ` + const transformed = V2Config.transform(yaml.safeLoad(config)) + + expect(transformed.mergeable[0].pass).toEqual(constants.DEFAULT_PR_PASS) + expect(transformed.mergeable[0].fail).toEqual(constants.DEFAULT_PR_FAIL) + expect(transformed.mergeable[0].error).toEqual(constants.DEFAULT_PR_ERROR) +}) + +test('pass, fail, error defaults will load when pull_request_review event is specified.', () => { + const config = ` + version: 2 + mergeable: + - when: pull_request_review.* + validate: + - do: payload + review: + state: + must_exclude: + regex: 'changes_requested' + ` + const transformed = V2Config.transform(yaml.safeLoad(config)) + + expect(transformed.mergeable[0].pass).toEqual(constants.DEFAULT_PR_PASS) + expect(transformed.mergeable[0].fail).toEqual(constants.DEFAULT_PR_FAIL) + expect(transformed.mergeable[0].error).toEqual(constants.DEFAULT_PR_ERROR) +}) + test('only pass, fail defaults ignore recipes that are not for pull_requests', () => { const config = ` version: 2 diff --git a/docs/actions/assign.rst b/docs/actions/assign.rst index a433792b..7da8c0be 100644 --- a/docs/actions/assign.rst +++ b/docs/actions/assign.rst @@ -1,6 +1,8 @@ Assign ^^^^^^^^ +You can assign specific people to a pull request or issue. + :: - do: assign @@ -9,4 +11,4 @@ Assign Supported Events: :: - 'pull_request.*', 'issues.*' \ No newline at end of file + 'pull_request.*', 'issues.*', 'issue_comment.*' \ No newline at end of file diff --git a/docs/actions/check.rst b/docs/actions/checks.rst similarity index 95% rename from docs/actions/check.rst rename to docs/actions/checks.rst index 11e85e0e..32f1ceeb 100644 --- a/docs/actions/check.rst +++ b/docs/actions/checks.rst @@ -1,6 +1,8 @@ -Check +Checks ^^^^^^^^ +You can add check runs to a pull request to enforce that it can only be merged once all checks returned with success. + .. note:: The logic for whether checks will be added by default is as follows: 1. If no action is provided in either pass, fail or error , add `checks` as default (to be backward compatible) @@ -74,4 +76,4 @@ The `pull_request.closed` event is not supported since it does not have meaningf :: - 'pull_request.assigned', 'pull_request.auto_merge_disabled', 'pull_request.auto_merge_enabled', 'pull_request.converted_to_draft', 'pull_request.demilestoned', 'pull_request.dequeued', 'pull_request.edited', 'pull_request.enqueued', 'pull_request.labeled', 'pull_request.locked', 'pull_request.milestoned', 'pull_request.opened', 'pull_request.push_synchronize', 'pull_request.ready_for_review', 'pull_request.reopened', 'pull_request.review_request_removed', 'pull_request.review_requested', 'pull_request.synchronize', 'pull_request.unassigned', 'pull_request.unlabeled', 'pull_request.unlocked', 'pull_request_review.dismissed', 'pull_request_review.edited', 'pull_request_review.submitted' + 'pull_request.assigned', 'pull_request.auto_merge_disabled', 'pull_request.auto_merge_enabled', 'pull_request.converted_to_draft', 'pull_request.demilestoned', 'pull_request.dequeued', 'pull_request.edited', 'pull_request.enqueued', 'pull_request.labeled', 'pull_request.locked', 'pull_request.milestoned', 'pull_request.opened', 'pull_request.push_synchronize', 'pull_request.ready_for_review', 'pull_request.reopened', 'pull_request.review_request_removed', 'pull_request.review_requested', 'pull_request.synchronize', 'pull_request.unassigned', 'pull_request.unlabeled', 'pull_request.unlocked', 'pull_request_review.dismissed', 'pull_request_review.edited', 'pull_request_review.submitted', 'issue_comment.*' diff --git a/docs/actions/close.rst b/docs/actions/close.rst index 1f99c61a..078f01c2 100644 --- a/docs/actions/close.rst +++ b/docs/actions/close.rst @@ -1,6 +1,8 @@ Close ^^^^^^^^ +You can close a pull request or issue. + :: - do: close @@ -8,4 +10,4 @@ Close Supported Events: :: - 'schedule.repository', 'pull_request.*', 'issues.*' + 'schedule.repository', 'pull_request.*', 'issues.*', 'issue_comment.*' diff --git a/docs/actions/comment.rst b/docs/actions/comment.rst index 9a4b2f49..df4f129f 100644 --- a/docs/actions/comment.rst +++ b/docs/actions/comment.rst @@ -1,6 +1,8 @@ Comment ^^^^^^^^ +You can add a comment to a pull request or issue. + :: - do: comment @@ -12,4 +14,4 @@ Comment Supported Events: :: - 'schedule.repository', 'pull_request.*', 'issues.*' + 'schedule.repository', 'pull_request.*', 'issues.*', 'issue_comment.*' diff --git a/docs/actions/labels.rst b/docs/actions/labels.rst index da6863aa..edbce49a 100644 --- a/docs/actions/labels.rst +++ b/docs/actions/labels.rst @@ -83,4 +83,4 @@ Note that the glob functionality is powered by the minimatch library. Please see Supported Events: :: - 'schedule.repository', 'pull_request.*', 'issues.*' + 'schedule.repository', 'pull_request.*', 'issues.*', 'issue_comment.*' diff --git a/docs/actions/merge.rst b/docs/actions/merge.rst index 3cded983..ec06bb56 100644 --- a/docs/actions/merge.rst +++ b/docs/actions/merge.rst @@ -1,6 +1,8 @@ Merge ^^^^^^^^ +You can merge a pull request and specify the merge method used. + :: - do: merge @@ -14,4 +16,4 @@ Merge Supported Events: :: - 'pull_request.*', 'pull_request_review.*', 'status.*', 'check_suite.*' + 'pull_request.*', 'pull_request_review.*', 'status.*', 'check_suite.*', 'issue_comment.*' diff --git a/docs/actions/request_review.rst b/docs/actions/request_review.rst index aa7c4113..93a0d79b 100644 --- a/docs/actions/request_review.rst +++ b/docs/actions/request_review.rst @@ -1,7 +1,7 @@ Request Review ^^^^^^^^^^^^^^^ -You can request specific reviews from specific reviewers, teams, or both +You can request specific reviews from specific reviewers, teams, or both for a pull request. :: diff --git a/docs/changelog.rst b/docs/changelog.rst index 8f4b7902..50640b04 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,5 +1,6 @@ CHANGELOG ===================================== +| June 12, 2024: feat: Support `issue_comment` event as trigger for actions `#754 `_ | June 10, 2024: fix: Docker image not working `#753 `_ | June 10, 2024: feat: publish multi arch docker image to dockerhub `#751 `_ | April 29, 2024: fix: Always allow assigning author `#744 `_ diff --git a/docs/configuration.rst b/docs/configuration.rst index 5cf9fd3b..b0b88e7b 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -174,7 +174,7 @@ Actions that mergeable is currently able to perform. .. toctree:: actions/assign.rst - actions/check.rst + actions/checks.rst actions/close.rst actions/comment.rst actions/merge.rst diff --git a/lib/actions/assign.js b/lib/actions/assign.js index 41616894..9631cdcc 100644 --- a/lib/actions/assign.js +++ b/lib/actions/assign.js @@ -6,7 +6,8 @@ class Assign extends Action { super('assign') this.supportedEvents = [ 'pull_request.*', - 'issues.*' + 'issues.*', + 'issue_comment.*' ] } diff --git a/lib/actions/checks.js b/lib/actions/checks.js index a2fa7418..8ff479b1 100644 --- a/lib/actions/checks.js +++ b/lib/actions/checks.js @@ -15,6 +15,11 @@ const createChecks = async (context, payload, actionObj) => { if (context.payload.checksuite) { params.head_branch = context.payload.checksuite.head_branch params.head_sha = context.payload.checksuite.head_sha + } else if (context.eventName === 'issue_comment') { + const issueNumber = context.payload.issue.number + const pullRequest = (await actionObj.githubAPI.getPR(context, issueNumber)).data + params.head_branch = pullRequest.head.ref + params.head_sha = pullRequest.head.sha } else { params.head_branch = context.payload.pull_request.head.ref params.head_sha = context.payload.pull_request.head.sha @@ -92,7 +97,8 @@ class Checks extends Action { 'pull_request.unlocked', 'pull_request_review.dismissed', 'pull_request_review.edited', - 'pull_request_review.submitted' + 'pull_request_review.submitted', + 'issue_comment.*' ] this.checkRunResult = new Map() } @@ -126,10 +132,9 @@ class Checks extends Action { async afterValidate (context, settings, name, results) { const checkRunResult = this.checkRunResult.get(name) + const payload = settings.payload ? this.populatePayloadWithResult(settings.payload, results, context) : undefined - const payload = this.populatePayloadWithResult(settings.payload, results, context) - - if (payload.text !== undefined) { + if (payload && payload.text !== undefined) { payload.text += MetaData.serialize({ id: checkRunResult.data.id, eventName: context.eventName, diff --git a/lib/actions/close.js b/lib/actions/close.js index 2b3c37e8..92bfcd4e 100644 --- a/lib/actions/close.js +++ b/lib/actions/close.js @@ -6,6 +6,7 @@ class Close extends Action { this.supportedEvents = [ 'pull_request.*', 'issues.*', + 'issue_comment.*', 'schedule.repository' ] } diff --git a/lib/actions/comment.js b/lib/actions/comment.js index 7d466e3c..ea89f03f 100644 --- a/lib/actions/comment.js +++ b/lib/actions/comment.js @@ -29,6 +29,7 @@ class Comment extends Action { this.supportedEvents = [ 'pull_request.*', 'issues.*', + 'issue_comment.*', 'schedule.repository' ] } diff --git a/lib/actions/labels.js b/lib/actions/labels.js index 1c929548..baa40623 100644 --- a/lib/actions/labels.js +++ b/lib/actions/labels.js @@ -23,6 +23,7 @@ class Labels extends Action { this.supportedEvents = [ 'pull_request.*', 'issues.*', + 'issue_comment.*', 'schedule.repository' ] } diff --git a/lib/configuration/transformers/v2Config.js b/lib/configuration/transformers/v2Config.js index 77e67c44..8fb1a4bc 100644 --- a/lib/configuration/transformers/v2Config.js +++ b/lib/configuration/transformers/v2Config.js @@ -33,7 +33,9 @@ const checkAndSetDefault = (ruleSet, defaultValue) => { const setPullRequestDefault = (config) => { config.mergeable.forEach((recipe) => { - if (recipe.when.includes('pull_request')) { + if (recipe.when.includes('pull_request') || + recipe.when.includes('pull_request_review') || + recipe.when.includes('issue_comment')) { if (!checkIfDefaultShouldBeApplied(recipe)) { if (recipe.pass === undefined) { recipe.pass = []