diff --git a/index.js b/index.js index d73dcd99..85f46aee 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,7 @@ const scheduler = require('./lib/scheduler'); const apiForSheetsModule = require('./lib/apiForSheets'); const checkMergeConflictsModule = require('./lib/checkMergeConflicts'); const checkPullRequestLabelsModule = require('./lib/checkPullRequestLabels'); +const checkPullRequestBranchModule = require('./lib/checkPullRequestBranch'); const checkWIPModule = require('./lib/checkWipDraftPR'); const whitelistedAccounts = ( @@ -28,6 +29,7 @@ module.exports = (oppiabot) => { if (whitelistedAccounts.includes(context.repo().owner.toLowerCase())) { await apiForSheetsModule.checkClaStatus(context); await checkPullRequestLabelsModule.checkChangelogLabel(context); + await checkPullRequestBranchModule.checkBranch(context); await checkWIPModule.checkWIP(context); } }); @@ -35,6 +37,8 @@ module.exports = (oppiabot) => { oppiabot.on('pull_request.reopened', async context => { if (whitelistedAccounts.includes(context.repo().owner.toLowerCase())) { await checkPullRequestLabelsModule.checkChangelogLabel(context); + // Prevent user from reopening the PR. + await checkPullRequestBranchModule.checkBranch(context); await checkWIPModule.checkWIP(context); } }); diff --git a/lib/checkPullRequestBranch.js b/lib/checkPullRequestBranch.js new file mode 100644 index 00000000..8100ced4 --- /dev/null +++ b/lib/checkPullRequestBranch.js @@ -0,0 +1,56 @@ +// Copyright 2020 The Oppia Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS-IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +const DISSALLOWED_BRANCH_PREFIXES = ['develop', 'release-', 'test-']; + +/** + * @param {import('probot').Context} context + */ +const checkBranch = async (context) => { + /** + * @type {import('probot').Octokit.PullsGetResponse} pullRequest + */ + const pullRequest = context.payload.pull_request; + const branchName = pullRequest.head.ref; + const branchIsInvalid = DISSALLOWED_BRANCH_PREFIXES.some((prefix) => { + return branchName.startsWith(prefix); + }); + + if (branchIsInvalid) { + const prAuthor = pullRequest.user.login; + // Comment on the pull request. + const wiki = 'wiki'.link( + 'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' + + 'instructions-for-making-a-code-change'); + const commentParams = context.repo({ + issue_number: pullRequest.number, + body: + 'Hi @' + prAuthor + ', PRs made from develop, release or test ' + + 'branches are not allowed. So this PR is being closed. Please make ' + + 'your changes in another branch and send in the PR. To learn more ' + + 'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!' + }); + await context.github.issues.createComment(commentParams); + // Close the pull request. + const closeIssueParams = context.repo({ + issue_number: pullRequest.number, + state: 'closed', + }); + await context.github.issues.update(closeIssueParams); + } +}; + +module.exports = { + checkBranch, +}; diff --git a/spec/checkPullRequestBranchSpec.js b/spec/checkPullRequestBranchSpec.js new file mode 100644 index 00000000..36f144a1 --- /dev/null +++ b/spec/checkPullRequestBranchSpec.js @@ -0,0 +1,211 @@ +// Copyright 2020 The Oppia Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS-IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +require('dotenv').config(); +const { createProbot } = require('probot'); +// The plugin refers to the actual app in index.js. +const oppiaBot = require('../index'); +const checkWipDraftPRModule = require('../lib/checkWipDraftPR'); +const scheduler = require('../lib/scheduler'); +const pullRequestPayload = require('../fixtures/pullRequestPayload.json'); +const apiForSheetsModule = require('../lib/apiForSheets'); +const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); +const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); + +describe('Pull Request Branch Check', () => { + /** + * @type {import('probot').Probot} robot + */ + let robot; + + /** + * @type {import('probot').Octokit} github + */ + let github; + + /** + * @type {import('probot').Application} app + */ + let app; + + beforeEach(() => { + spyOn(scheduler, 'createScheduler').and.callFake(() => {}); + + // Spy on other modules that will be triggered by the payload. + spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => {}); + spyOn( + checkPullRequestLabelsModule, + 'checkChangelogLabel' + ).and.callFake(() => {}); + spyOn(checkWipDraftPRModule, 'checkWIP').and.callFake(() => {}); + + github = { + issues: { + createComment: jasmine.createSpy('createComment').and.returnValue({}), + update: jasmine.createSpy('update').and.resolveTo({}), + }, + }; + + robot = createProbot({ + id: 1, + cert: 'test', + githubToken: 'test', + }); + + app = robot.load(oppiaBot); + spyOn(app, 'auth').and.resolveTo(github); + spyOn(checkPullRequestBranchModule, 'checkBranch').and.callThrough(); + }); + + describe('Invalid branch name', () => { + describe('develop branch check', () => { + beforeEach(async () => { + pullRequestPayload.payload.pull_request.head.ref = 'develop'; + await app.receive(pullRequestPayload); + }); + + it('should call appropriate module', async () => { + expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled(); + }); + + it('should create appropriate comment', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = pullRequestPayload.payload.pull_request.user.login; + const wiki = 'wiki'.link( + 'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' + + 'instructions-for-making-a-code-change'); + const commentBody = ( + 'Hi @' + author + ', PRs made from develop, release or test ' + + 'branches are not allowed. So this PR is being closed. Please make ' + + 'your changes in another branch and send in the PR. To learn more ' + + 'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!'); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + body: commentBody + }); + }); + + it('should close pull request', () => { + expect(github.issues.update).toHaveBeenCalled(); + expect(github.issues.update).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + state: 'closed', + }); + }); + }); + + describe('release branch check', () => { + beforeEach(async () => { + pullRequestPayload.payload.pull_request.head.ref = 'release-2'; + await app.receive(pullRequestPayload); + }); + + it('should call appropriate module', async () => { + expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled(); + }); + + it('should create appropriate comment', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = pullRequestPayload.payload.pull_request.user.login; + const wiki = 'wiki'.link( + 'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' + + 'instructions-for-making-a-code-change'); + const commentBody = ( + 'Hi @' + author + ', PRs made from develop, release or test ' + + 'branches are not allowed. So this PR is being closed. Please make ' + + 'your changes in another branch and send in the PR. To learn more ' + + 'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!'); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + body: commentBody + }); + }); + + it('should close pull request', () => { + expect(github.issues.update).toHaveBeenCalled(); + expect(github.issues.update).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + state: 'closed', + }); + }); + }); + + describe('test branch check', () => { + beforeEach(async () => { + pullRequestPayload.payload.pull_request.head.ref = 'test-2'; + await app.receive(pullRequestPayload); + }); + + it('should call appropriate module', async () => { + expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled(); + }); + + it('should create appropriate comment', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = pullRequestPayload.payload.pull_request.user.login; + const wiki = 'wiki'.link( + 'https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#' + + 'instructions-for-making-a-code-change'); + const commentBody = ( + 'Hi @' + author + ', PRs made from develop, release or test ' + + 'branches are not allowed. So this PR is being closed. Please make ' + + 'your changes in another branch and send in the PR. To learn more ' + + 'about contributing to Oppia, take a look at our ' + wiki + '. Thanks!'); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + body: commentBody + }); + }); + + it('should close pull request', () => { + expect(github.issues.update).toHaveBeenCalled(); + expect(github.issues.update).toHaveBeenCalledWith({ + issue_number: pullRequestPayload.payload.pull_request.number, + owner: pullRequestPayload.payload.repository.owner.login, + repo: pullRequestPayload.payload.repository.name, + state: 'closed', + }); + }); + }); + }); + + describe('Valid branch name', () => { + beforeEach(async () => { + pullRequestPayload.payload.pull_request.head.ref = 'valid-branch'; + await app.receive(pullRequestPayload); + }); + + it('should call appropriate module', async () => { + expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled(); + }); + + it('should not create comment', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + + it('should not close pull request', () => { + expect(github.issues.update).not.toHaveBeenCalled(); + }); + }); +});