diff --git a/src/lib/github/webhookHandlers.js b/src/lib/github/webhookHandlers.js index 9e024ba..1a5a956 100644 --- a/src/lib/github/webhookHandlers.js +++ b/src/lib/github/webhookHandlers.js @@ -78,21 +78,15 @@ async function sendOpenerInitialStateMessage(opener, reviewers, prObj) { async function requestReviewersAndAssignees(users, body) { try { const githubUsers = users.map(user => user.github); + const owner = body.pull_request.base.repo.owner.login; + const repo = body.pull_request.base.repo.name; + const number = body.pull_request.number; - // Should probably look at the results to check if reviewers are there. - const reviewRequests = await octokit.pullRequests.createReviewRequest({ - owner: body.pull_request.base.repo.owner.login, - repo: body.pull_request.base.repo.name, - number: body.pull_request.number, - reviewers: githubUsers, - }); - - const assignees = await octokit.issues.addAssignees({ - owner: body.pull_request.base.repo.owner.login, - repo: body.pull_request.base.repo.name, - number: body.pull_request.number, - assignees: githubUsers, - }); + // Should probably look at the results to check if reviewres are there. + const [reviewRequests, assignees] = await Promise.all([ + octokit.pullRequests.createReviewRequest({ owner, repo, number, reviewers: githubUsers }), + octokit.issues.addAssignees({ owner, repo, number, assignees: githubUsers }), + ]); logger.info(`[Add Users to PR] Repo: ${body.pull_request.base.repo.name}. ` + `Assigned and Request reviews from: ${githubUsers}`); @@ -105,9 +99,13 @@ async function requestReviewersAndAssignees(users, body) { async function requestReviewByGithubName(body) { const logId = shortid.generate(); - const opener = await findByGithubName(body.pull_request.user.login, logId); + + const [opener, requestedReviewer] = await Promise.all([ + findByGithubName(body.pull_request.user.login, logId), + findByGithubName(body.requested_reviewer.login, logId), + ]); + const openerName = opener ? opener.name : body.pull_request.user.login; - const requestedReviewer = await findByGithubName(body.requested_reviewer.login, logId); if (requestedReviewer && requestedReviewer.slack) { return await sendReviewRequestMessage(openerName, requestedReviewer, body); } @@ -132,9 +130,10 @@ async function prOpened(body) { const numReviewersAlready = body.pull_request.assignees.length; const numReviewersToRandomlySelect = NUM_REVIEWERS - numReviewersAlready; - const preselectedUsers = await Promise.all(body.pull_request.assignees.map(user => { - return findByGithubName(user.login, logId); - })); + const preselectedUsers = await Promise.all( + body.pull_request.assignees.map(user => findByGithubName(user.login, logId)) + ); + const notTheseUsers = opener ? preselectedUsers.concat(opener.github) : preselectedUsers; const randomUsers = await selectRandomGithubUsers(notTheseUsers, body.repository.full_name, numReviewersToRandomlySelect); const users = preselectedUsers.concat(randomUsers); @@ -195,8 +194,11 @@ async function prReviewed(body) { let reviewer, coder; const logId = shortid.generate(); try { - reviewer = await findByGithubName(body.review.user.login, logId); - coder = await findByGithubName(body.pull_request.user.login, logId); + const [reviewer, coder] = await Promise.all( + findByGithubName(body.review.user.login, logId), + findByGithubName(body.pull_request.user.login, logId), + ); + if (!reviewer) throw new Error(`[github.prReviewed:${logId}] Reviewer not registered with git slackin`); if (!coder) throw new Error(`[github.prReviewed:${logId}] Coder not registered with git slackin`); } catch (e) { @@ -237,13 +239,16 @@ async function prReviewed(body) { try { // let coder know its been done - await send(coder.slack.id, message); - let shouldNotify = await checkForReviews({ - owner: body.repository.owner.login, - repo: body.repository.name, - number: body.pull_request.number }); + const [, passedReviewThreshold] = await Promise.all([ + send(coder.slack.id, message), + checkForReviews({ + owner: body.repository.owner.login, + repo: body.repository.name, + number: body.pull_request.number, + }), + ]); - shouldNotify = shouldNotify && body.review.state.toUpperCase() === 'APPROVED'; + const shouldNotify = passedReviewThreshold && body.review.state.toUpperCase() === 'APPROVED'; if (shouldNotify) { const mergerMessage = @@ -297,11 +302,14 @@ function sendPrUpdatedMessage(openerName, users, body) { async function prSynchronize(body) { const logId = shortid.generate(); - const opener = await findByGithubName(body.pull_request.user.login, logId); + const reviewer_list = body.pull_request.requested_reviewers; + + const [opener, ...reviewers] = await Promise.all( + findByGithubName(body.pull_request.user.login, logId), + ...reviewer_list.map(user => findByGithubName(user.login, logId)), + ); + const openerName = opener ? opener.name : body.pull_request.user.login; - const reviewers = await Promise.all(body.pull_request.requested_reviewers.map(user => { - return findByGithubName(user.login, logId); - })); return await sendPrUpdatedMessage(openerName, reviewers, body); } diff --git a/src/lib/github/webhookRouter.js b/src/lib/github/webhookRouter.js index 58fa712..9532b70 100644 --- a/src/lib/github/webhookRouter.js +++ b/src/lib/github/webhookRouter.js @@ -34,6 +34,7 @@ async function routeIt(body, { signature }) { throw new Error('Signatures do not match!'); } } + logger.info(`[RouteIt] ${body.action} on ${body.pull_request.base.repo.name}`); try { diff --git a/src/lib/slack/common.js b/src/lib/slack/common.js index 481d964..5c5dac6 100644 --- a/src/lib/slack/common.js +++ b/src/lib/slack/common.js @@ -5,30 +5,23 @@ const simpleGit = require('simple-git/promise')(appRoot.path); const users = require('../users'); async function generateAndSendBootMessage(channel = null, { msgText = null } = {}) { - const { available, benched } = await users.listAllUserNamesByAvailability(); - const SHA = await simpleGit.revparse(['HEAD']); + const [{ available, benched }, SHA] = await Promise.all([ + users.listAllUserNamesByAvailability(), + simpleGit.revparse(['HEAD']), + ]); + const messageObject = { text: msgText || `Git Slackin: ONLINE. SHA \`${SHA.trim()}\``, attachments: [ { text: '', color: 'good', - fields: [ - { - title: 'Available Users', - value: available, - }, - ], + fields: [{ title: 'Available Users', value: available }], }, { text: '', color: 'warning', - fields: [ - { - title: 'Benched Users', - value: benched, - }, - ], + fields: [{ title: 'Benched Users', value: benched }], }, ], }; diff --git a/src/lib/slack/eventHandlers.js b/src/lib/slack/eventHandlers.js index 7441106..aa1098b 100644 --- a/src/lib/slack/eventHandlers.js +++ b/src/lib/slack/eventHandlers.js @@ -30,8 +30,9 @@ async function updateConfigurations(configOverrides) { // especially if changing branches. async function updateGitSlackin(theEvent, branch = 'master') { let updateResult = null; + try { - // Let's discard these changes first. + // Discard any unstaged changes. await simpleGit.stash(); await simpleGit.stash(['drop']); @@ -43,17 +44,17 @@ async function updateGitSlackin(theEvent, branch = 'master') { } const triggeringUser = await findBySlackUserId(theEvent.user); - return sendToChannel(theEvent.channel, `Update trigger by ${triggeringUser.name}. Be back shortly! :wave:\n` + - `Changes: ${updateResult}`) - .then(() => { - // this works since we've already pulled so restarting should work. - return process.exit(0); - }); + + const message = `Update trigger by ${triggeringUser.name}. Be back shortly! :wave:\nChanges: ${updateResult}`; + await sendToChannel(theEvent.channel, message); + + // This works since we've already pulled so restarting should work. + return process.exit(0); } async function handleAdminCommands(command, theEvent, res, logId) { - if (!config.has('slack_manager_ids') - || !config.get('slack_manager_ids').includes(theEvent.user)) { + if (!config.has('slack_manager_ids') || + !config.get('slack_manager_ids').includes(theEvent.user)) { return sendEphemeralMessage(theEvent.channel, theEvent.user, 'This command is Admin-only or does not exist.'); } @@ -71,10 +72,8 @@ async function handleAdminCommands(command, theEvent, res, logId) { try { const newConfig = JSON.parse(setConfigRegexResult[1]); await updateConfigurations(newConfig); - return await sendToChannel(theEvent.channel, 'Updated config, restarting Git Slackin...') - .then(() => { - return process.exit(0); - }); + await sendToChannel(theEvent.channel, 'Updated config, restarting Git Slackin...'); + return process.exit(0); } catch (e) { return sendEphemeralMessage(theEvent.channel, theEvent.user, 'Error updating configuration'); } @@ -90,26 +89,33 @@ async function handleAdminCommands(command, theEvent, res, logId) { if (!slackUserIdToBench) { logger.warn(`[commands.admin.bench:${logId}] Could not find user to user ${slackUserIdToBench}`); - return await sendEphemeralMessage(theEvent.channel, theEvent.user, - `:whatsgoingon: I could not find the user '<@${slackUserIdToBench}>' to bench. ` + - `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``); + const failureMessage = `:whatsgoingon: I could not find the user '<@${slackUserIdToBench}>' to bench. ` + + `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``; + + return await sendEphemeralMessage(theEvent.channel, theEvent.user, failureMessage); } const success = await benchUserBySlackId(slackUserIdToBench, logId); if (success) { - await send(slackUserIdToBench, `You have been benched by <@${theEvent.user}>. ` + - 'Send me, Git Slackin, `start` to start receiving Review Requests again.'); + const responseMessage = `I have benched <@${slackUserIdToBench}> as requested.`; + const benchedUserMessage = `You have been benched by <@${theEvent.user}>. ` + + 'Send me, Git Slackin, `start` to start receiving Review Requests again.'; - return await sendEphemeralMessage(theEvent.channel, theEvent.user, - `I have benched <@${slackUserIdToBench}> as requested.`); + const [, result] = await Promise.all([ + send(slackUserIdToBench, benchedUserMessage), + sendEphemeralMessage(theEvent.channel, theEvent.user, responseMessage), + ]); + + return result; } else { const logId = shortid.generate(); logger.warn(`[commands.admin.bench:${logId}] Could not bench user ${slackUserIdToBench}`); - return await sendEphemeralMessage(theEvent.channel, theEvent.user, - `:whatsgoingon: I could not bench <@${slackUserIdToBench}> as requested. ` + - `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``); + const failureMessage = `:whatsgoingon: I could not bench <@${slackUserIdToBench}> as requested. ` + + `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``; + + return await sendEphemeralMessage(theEvent.channel, theEvent.user, failureMessage); } } @@ -118,26 +124,33 @@ async function handleAdminCommands(command, theEvent, res, logId) { if (!slackUserIdToUnbench) { logger.warn(`[commands.admin.bench:${logId}] Could not find user to user ${slackUserIdToUnbench}`); - return await sendEphemeralMessage(theEvent.channel, theEvent.user, - `:whatsgoingon: I could not find the user '<@${slackUserIdToUnbench}>' to unbench. ` + - `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``); + const failureMessage = `:whatsgoingon: I could not find the user '<@${slackUserIdToUnbench}>'. ` + + `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``; + + return await sendEphemeralMessage(theEvent.channel, theEvent.user, failureMessage); } const success = await activateUserBySlackId(slackUserIdToUnbench, logId); if (success) { - send(slackUserIdToUnbench, `You have been unbenched by <@${theEvent.user}>. ` + - 'Send me, Git Slackin, `start` to start receiving Review Requests again.'); + const responseMessage = `I have unbenched <@${slackUserIdToUnbench}> as requested.`; + const unbenchedUserMessage = `You have been unbenched by <@${theEvent.user}>. ` + + 'Send me, Git Slackin, `start` to start receiving Review Requests again.'; - return sendEphemeralMessage(theEvent.channel, theEvent.user, - `I have unbenched <@${slackUserIdToUnbench}> as requested.`); + const [, result] = await Promise.all([ + send(slackUserIdToUnbench, unbenchedUserMessage), + sendEphemeralMessage(theEvent.channel, theEvent.user, responseMessage), + ]); + + return result; } else { const logId = shortid.generate(); logger.warn(`[commands.admin.unbench:${logId}] Could not unbench user: ${slackUserIdToUnbench}`); - return await sendEphemeralMessage(theEvent.channel, theEvent.user, - `:whatsgoingon: I could not unbench <@${slackUserIdToUnbench}> as requested. ` + - `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``); + const failureMessage = `:whatsgoingon: I could not unbench <@${slackUserIdToUnbench}> as requested. ` + + `Please inform James if you think this is a bug. And refer to log code: \`${logId}\``; + + return await sendEphemeralMessage(theEvent.channel, theEvent.user, failureMessage); } } @@ -151,10 +164,7 @@ async function handleAdminCommands(command, theEvent, res, logId) { if (command === 'shutdown') { logger.info(`[ADMIN Event] ${theEvent.user} requested shutdown`); - return sendToChannel(theEvent.channel, 'Shutting down!') - .then(() => { - return process.exit(0); - }); + return sendToChannel(theEvent.channel, 'Shutting down!').then(() => process.exit(0)); } } @@ -259,7 +269,9 @@ async function handleCommands(text, theEvent, res, logId = 'NoId') { } if (smallText === 'status') { const user = await findBySlackUserId(theEvent.user); + logger.info(`[commands.user.status:${logId}] ${theEvent.user} requested their status.`); + return sendEphemeralMessage(theEvent.channel, theEvent.user, `You are <@${user.slack.id}> here and ` + ` on GitHub.\n` + `Your current Git Slackin' status is: ${user.requestable ? 'Requestable :yes:' : 'UnRequestable :no:'}.\n` + diff --git a/src/lib/users.js b/src/lib/users.js index a698358..3eba6b8 100644 --- a/src/lib/users.js +++ b/src/lib/users.js @@ -110,7 +110,9 @@ async function benchUserBySlackId(id, logId) { } return user; }); + await synchronizeUserList(); + if (updated) { logger.info(`[users.benchUserBySlackId:${logId}] Benched user: ${id}. user_list file`); } else { @@ -131,7 +133,9 @@ async function activateUserBySlackId(id, logId) { } return user; }); + await synchronizeUserList(); + if (updated) { logger.info(`[users.activateUserBySlackId:${logId}] Benched user: ${id}. user_list file`); } else { @@ -147,9 +151,12 @@ async function muteNotificationsBySlackId(id, logId) { if (user.slack && user.slack.id.toLowerCase() === id.toLowerCase()) { user.notifications = false; } + return user; }); + await synchronizeUserList(); + logger.info('[USERS] Update user_list file'); return users; } @@ -161,24 +168,29 @@ async function unmuteNotificationsBySlackId(id, logId) { if (user.slack && user.slack.id.toLowerCase() === id.toLowerCase()) { user.notifications = true; } + return user; }); + await synchronizeUserList(); + return users; } async function listAllUserNamesByAvailability() { - const availableUsers = await listAvailableUsers(true); - const benchedUsers = await listBenchedUsers(true); - - let availableUsersString = availableUsers.join(); - let benchedUsersString = benchedUsers.join(); - if (availableUsersString.length === 0) availableUsersString = 'None'; - if (benchedUsersString.length === 0) benchedUsersString = 'None'; + const [available, benched] = await Promise.all([ + listAvailableUsers(true), + listBenchedUsers(true), + ]); + + const toString = (arr) => { + if (arr.length === 0) return 'None'; + return arr.join(); + }; return { - available: availableUsersString, - benched: benchedUsersString, + available: toString(available), + benched: toString(benched), }; } diff --git a/src/server.js b/src/server.js index eaf56b4..d8ec02e 100644 --- a/src/server.js +++ b/src/server.js @@ -48,9 +48,11 @@ app.use(bodyParser.urlencoded({ extended: true })); // Basic web server to handle payloads app.post('/payload', (req, res) => { - if (req.headers['x-github-event'] === 'pull_request' || - req.headers['x-github-event'] === 'pull_request_review' || - req.headers['x-github-event'] === 'pull_request_review_comment') { + const eventType = req.headers['x-github-event']; + + if (eventType === 'pull_request' || + eventType === 'pull_request_review' || + eventType === 'pull_request_review_comment') { return githubWebhooks.handle(req.body, { signature: req.headers['x-hub-signature'], webhookId: req.headers['x-github-delivery'],