From 5b647238a3d6385d05cb55d8b834688c88045340 Mon Sep 17 00:00:00 2001 From: James James Date: Wed, 16 Dec 2020 00:01:45 +0100 Subject: [PATCH] Fix #170: Ping and assign release team lead when new jobs get created (#172) --- actions_build/index.js | 4 ++-- lib/checkCriticalPullRequest.js | 14 ++++++-------- lib/checkPullRequestJob.js | 17 +++++++++++------ lib/checkPullRequestLabels.js | 8 ++++---- spec/checkCriticalPullRequestSpec.js | 14 +++++--------- spec/checkPullRequestJobSpec.js | 17 ++++++++--------- userWhitelist.json | 2 +- 7 files changed, 37 insertions(+), 39 deletions(-) diff --git a/actions_build/index.js b/actions_build/index.js index 3535dde2..d11ffc8b 100644 --- a/actions_build/index.js +++ b/actions_build/index.js @@ -316,7 +316,7 @@ module.exports = require("https"); /***/ 35: /***/ (function(module) { -module.exports = {"releaseCoordinators":["nithusha21","aks681","vojtechjelinek","ankita240796","DubeySandeep","BenHenning","kevintab95"],"goodFirstIssue":["U8NWXD","kevintab95","seanlip","ankita240796","DubeySandeep","bansalnitish","vojtechjelinek","marianazangrossi","brianrodri","nithusha21","aks681","srijanreddy98"],"teamLeads":{"onboardingTeam":"vojtechjelinek","releaseTeam":"ankita240796","welfareTeam":"DubeySandeep"},"oppiaMaintainers":"oppia/core-maintainers","oppiaReleaseCoordinators":"oppia/release-coordinators","oppiaDevWorkflowTeam":"oppia/dev-workflow-team","SERVER_JOBS_ADMIN":"seanlip"}; +module.exports = {"releaseCoordinators":["nithusha21","aks681","vojtechjelinek","ankita240796","DubeySandeep","BenHenning","kevintab95"],"goodFirstIssue":["U8NWXD","kevintab95","seanlip","ankita240796","DubeySandeep","bansalnitish","vojtechjelinek","marianazangrossi","brianrodri","nithusha21","aks681","srijanreddy98"],"teamLeads":{"onboardingTeam":"vojtechjelinek","releaseTeam":"ankita240796","welfareTeam":"DubeySandeep"},"oppiaMaintainers":"oppia/core-maintainers","oppiaReleaseCoordinators":"oppia/release-coordinators","oppiaDevWorkflowTeam":"oppia/dev-workflow-team","serverJobAdmins":"seanlip"}; /***/ }), @@ -25792,4 +25792,4 @@ function onceStrict (fn) { /***/ }) -/******/ }); \ No newline at end of file +/******/ }); diff --git a/lib/checkCriticalPullRequest.js b/lib/checkCriticalPullRequest.js index 89a2c7c7..ad996f85 100644 --- a/lib/checkCriticalPullRequest.js +++ b/lib/checkCriticalPullRequest.js @@ -27,9 +27,7 @@ const { } = require('./utils'); const { - teamLeads, - oppiaMaintainers, - SERVER_JOBS_ADMIN + serverJobAdmins } = require('../userWhitelist.json'); const STORAGE_DIR_PREFIX = 'core/storage/'; const MODEL_REGEX = new RegExp( @@ -92,14 +90,14 @@ const getCommentBody = (newModelFiles) => { // This function will never be called when there are no model files. if (newModelFiles.length === 1) { - const message = 'Hi @' + SERVER_JOBS_ADMIN + ' and @' + - teamLeads.releaseTeam + ', PTAL at this PR, it adds a model that ' + + const message = 'Hi @' + serverJobAdmins.join(' and @') + + ', PTAL at this PR, it adds a model that ' + 'needs to be validated.' + modelNameString + newLineFeed + 'Thanks!'; return message; } else { - const message = 'Hi @' + SERVER_JOBS_ADMIN + ' and @' + - teamLeads.releaseTeam + ', PTAL at this PR, it adds new models that ' + + const message = 'Hi @' + serverJobAdmins.join(' and @') + + ', PTAL at this PR, it adds new models that ' + 'need to be validated.' + modelNameString + newLineFeed + 'Thanks!'; return message; @@ -139,7 +137,7 @@ const checkIfPRAffectsDatastoreLayer = async (context) => { await pingAndAssignUsers( context, pullRequest, - [teamLeads.releaseTeam, SERVER_JOBS_ADMIN], + serverJobAdmins, commentBody ); } diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js index 568c8ab6..0909987e 100644 --- a/lib/checkPullRequestJob.js +++ b/lib/checkPullRequestJob.js @@ -12,7 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -const { SERVER_JOBS_ADMIN } = require('../userWhitelist.json'); +const { + serverJobAdmins +} = require('../userWhitelist.json'); const { DATASTORE_LABEL, getAllChangedFiles, @@ -141,8 +143,10 @@ const getCommentBody = (newJobFiles, changedFiles, prAuthor) => { // This function will never be called when there are no job files. if (newJobFiles.length === 1) { - const serverAdminPing = 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + - 'it adds a new one off job.' + jobNameString; + const serverAdminPing = ( + 'Hi @' + serverJobAdmins.join(', @') + + ', PTAL at this PR, ' + 'it adds a new one off job.' + jobNameString + ); const prAuthorPing = 'Also @' + prAuthor + registryReminder + 'please make sure to fill in the ' + serverJobsForm + @@ -153,8 +157,9 @@ const getCommentBody = (newJobFiles, changedFiles, prAuthor) => { return (serverAdminPing + newLineFeed + prAuthorPing + newLineFeed + 'Thanks!'); } else { - const serverAdminPing = 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + - 'it adds new one off jobs.' + jobNameString; + const serverAdminPing = ( + 'Hi @' + serverJobAdmins.join(', @') + + ', PTAL at this PR, ' + 'it adds new one off jobs.' + jobNameString); const prAuthorPing = 'Also @' + prAuthor + registryReminder + 'please make sure to fill in the ' + serverJobsForm + @@ -194,7 +199,7 @@ const checkForNewJob = async (context) => { await pingAndAssignUsers( context, pullRequest, - [SERVER_JOBS_ADMIN], + serverJobAdmins, commentBody ); diff --git a/lib/checkPullRequestLabels.js b/lib/checkPullRequestLabels.js index b738564f..ae71af4b 100644 --- a/lib/checkPullRequestLabels.js +++ b/lib/checkPullRequestLabels.js @@ -24,10 +24,10 @@ const { pingAndAssignUsers } = require('./utils'); const { - releaseCoordinators: RELEASE_COORDINATORS, + releaseCoordinators, oppiaReleaseCoordinators, oppiaDevWorkflowTeam, - SERVER_JOBS_ADMIN, + serverJobAdmins, } = require('../userWhitelist.json'); const DEFAULT_CHANGELOG_LABEL = 'REVIEWERS: Please add changelog label'; const PR_LABELS = ['dependencies', 'stale', DEFAULT_CHANGELOG_LABEL]; @@ -276,8 +276,8 @@ module.exports.checkChangelogLabel = async function (context) { */ const isUserAllowedToRemoveCriticalLabel = function (username) { return ( - username === SERVER_JOBS_ADMIN || - RELEASE_COORDINATORS.includes(username)); + serverJobAdmins.includes(username) || + releaseCoordinators.includes(username)); }; /** diff --git a/spec/checkCriticalPullRequestSpec.js b/spec/checkCriticalPullRequestSpec.js index 59a34c27..20d239b2 100644 --- a/spec/checkCriticalPullRequestSpec.js +++ b/spec/checkCriticalPullRequestSpec.js @@ -31,11 +31,7 @@ const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); -const { - teamLeads, - oppiaMaintainers, - SERVER_JOBS_ADMIN, -} = require('../userWhitelist.json'); + let payloadData = JSON.parse( JSON.stringify(require('../fixtures/pullRequestPayload.json')) ); @@ -244,7 +240,7 @@ describe('Critical Pull Request Spec', () => { owner: payloadData.payload.repository.owner.login, issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ' and @' + teamLeads.releaseTeam + + 'Hi @seanlip and @vojtechjelinek' + ', PTAL at this PR, it adds a model that needs to be validated. ' + 'The name of the model is ' + firstModel + '.
Thanks!', }); @@ -256,7 +252,7 @@ describe('Critical Pull Request Spec', () => { repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, issue_number: payloadData.payload.pull_request.number, - assignees: [teamLeads.releaseTeam, SERVER_JOBS_ADMIN], + assignees: ['seanlip', 'vojtechjelinek'], }); }); @@ -306,7 +302,7 @@ describe('Critical Pull Request Spec', () => { owner: payloadData.payload.repository.owner.login, issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ' and @' + teamLeads.releaseTeam + + 'Hi @seanlip and @vojtechjelinek' + ', PTAL at this PR, it adds new models that need to be validated. ' + 'The models are ' + firstModel + ', ' + secondModels + '.
Thanks!' }); @@ -318,7 +314,7 @@ describe('Critical Pull Request Spec', () => { repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, issue_number: payloadData.payload.pull_request.number, - assignees: [teamLeads.releaseTeam, SERVER_JOBS_ADMIN], + assignees: ['seanlip', 'vojtechjelinek'], }); }); diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 7b8333c0..9b0f7f4d 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -31,7 +31,6 @@ const scheduler = require('../lib/scheduler'); let payloadData = JSON.parse( JSON.stringify(require('../fixtures/pullRequestPayload.json')) ); -const { SERVER_JOBS_ADMIN } = require('../userWhitelist.json'); describe('Pull Request Job Spec', () => { /** @@ -512,7 +511,7 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + + 'Hi @seanlip, @vojtechjelinek, PTAL at this PR, ' + 'it adds a new one off job. The name of the job is ' + jobNameLink + '.' + newLineFeed + 'Also @' + author + ', please add the new job ' + 'to the ' + jobRegistryLink + @@ -532,7 +531,7 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, - assignees: [SERVER_JOBS_ADMIN] + assignees: ['seanlip', 'vojtechjelinek'] }); }); @@ -596,7 +595,7 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + + 'Hi @seanlip, @vojtechjelinek, PTAL at this PR, ' + 'it adds new one off jobs. The jobs are ' + firstJobNameLink + ', ' + secondJobNameLink + '.' + newLineFeed + 'Also @' + author + ', please add the new jobs ' + 'to the ' + @@ -616,7 +615,7 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, - assignees: [SERVER_JOBS_ADMIN] + assignees: ['seanlip', 'vojtechjelinek'] }); }); @@ -674,7 +673,7 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + + 'Hi @seanlip, @vojtechjelinek, PTAL at this PR, ' + 'it adds a new one off job. The name of the job is ' + jobNameLink + '.' + newLineFeed + 'Also @' + author + ', please make sure to fill in the ' + formText + @@ -693,7 +692,7 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, - assignees: [SERVER_JOBS_ADMIN] + assignees: ['seanlip', 'vojtechjelinek'] }); }); @@ -754,7 +753,7 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, body: - 'Hi @' + SERVER_JOBS_ADMIN + ', PTAL at this PR, ' + + 'Hi @seanlip, @vojtechjelinek, PTAL at this PR, ' + 'it adds a new one off job. The name of the job is ' + jobNameLink + '.' + newLineFeed + 'Also @' + author + ', please add the new job ' + 'to the ' + jobRegistryLink + ' and please make sure to fill ' + @@ -773,7 +772,7 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, - assignees: [SERVER_JOBS_ADMIN] + assignees: ['seanlip', 'vojtechjelinek'] }); }); diff --git a/userWhitelist.json b/userWhitelist.json index d260d893..bc5933a7 100644 --- a/userWhitelist.json +++ b/userWhitelist.json @@ -30,5 +30,5 @@ "oppiaMaintainers": "oppia/core-maintainers", "oppiaReleaseCoordinators": "oppia/release-coordinators", "oppiaDevWorkflowTeam": "oppia/dev-workflow-team", - "SERVER_JOBS_ADMIN": "seanlip" + "serverJobAdmins": ["seanlip", "vojtechjelinek"] }