Skip to content

Commit

Permalink
Fix #170: Ping and assign release team lead when new jobs get created (
Browse files Browse the repository at this point in the history
  • Loading branch information
jameesjohn authored Dec 15, 2020
1 parent 5aebb78 commit 5b64723
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 39 deletions.
4 changes: 2 additions & 2 deletions actions_build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"};

/***/ }),

Expand Down Expand Up @@ -25792,4 +25792,4 @@ function onceStrict (fn) {

/***/ })

/******/ });
/******/ });
14 changes: 6 additions & 8 deletions lib/checkCriticalPullRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -139,7 +137,7 @@ const checkIfPRAffectsDatastoreLayer = async (context) => {
await pingAndAssignUsers(
context,
pullRequest,
[teamLeads.releaseTeam, SERVER_JOBS_ADMIN],
serverJobAdmins,
commentBody
);
}
Expand Down
17 changes: 11 additions & 6 deletions lib/checkPullRequestJob.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 +
Expand All @@ -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 +
Expand Down Expand Up @@ -194,7 +199,7 @@ const checkForNewJob = async (context) => {
await pingAndAssignUsers(
context,
pullRequest,
[SERVER_JOBS_ADMIN],
serverJobAdmins,
commentBody
);

Expand Down
8 changes: 4 additions & 4 deletions lib/checkPullRequestLabels.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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));
};

/**
Expand Down
14 changes: 5 additions & 9 deletions spec/checkCriticalPullRequestSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
);
Expand Down Expand Up @@ -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 + '.<br>Thanks!',
});
Expand All @@ -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'],
});
});

Expand Down Expand Up @@ -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 + '.<br>Thanks!'
});
Expand All @@ -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'],
});
});

Expand Down
17 changes: 8 additions & 9 deletions spec/checkPullRequestJobSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
/**
Expand Down Expand Up @@ -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 +
Expand All @@ -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']
});
});

Expand Down Expand Up @@ -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 ' +
Expand All @@ -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']
});
});

Expand Down Expand Up @@ -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 +
Expand All @@ -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']
});
});

Expand Down Expand Up @@ -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 ' +
Expand All @@ -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']
});
});

Expand Down
2 changes: 1 addition & 1 deletion userWhitelist.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@
"oppiaMaintainers": "oppia/core-maintainers",
"oppiaReleaseCoordinators": "oppia/release-coordinators",
"oppiaDevWorkflowTeam": "oppia/dev-workflow-team",
"SERVER_JOBS_ADMIN": "seanlip"
"serverJobAdmins": ["seanlip", "vojtechjelinek"]
}

0 comments on commit 5b64723

Please sign in to comment.