diff --git a/lib/github-client.js b/lib/github-client.js index 76f39443..93f0b5b9 100644 --- a/lib/github-client.js +++ b/lib/github-client.js @@ -1,8 +1,10 @@ 'use strict' +require('dotenv').load({ silent: true }) const GitHub = require('github') const githubClient = new GitHub({ + debug: require('debug')('github').enabled, version: '3.0.0', protocol: 'https', host: 'api.github.com', @@ -17,4 +19,30 @@ githubClient.authenticate({ token: process.env.GITHUB_TOKEN || 'invalid-placeholder-token' }) +// Gets all pages of a paged response +// example: +// github.orgs.getTeams({ org:'nodejs' }) +// .then(github.allPages) +// .then(console.log) +githubClient.allPages = (res1, out = []) => + githubClient.hasNextPage(res1) + ? githubClient.getNextPage(res1) + .then((res2) => githubClient.allPages(res2, out.concat(res1))) + : out.concat(res1) + +// Pages through responses until the predicate matches. Returns matched item. +// example: +// github.orgs.getTeams({ org:'nodejs' }) +// .then(github.find(t => t.name === 'addon-api')) +// .then(console.log) +githubClient.find = (predicate) => { + const finder = (res) => { + var found = res.find(predicate) + if (typeof found !== 'undefined') return found + + return githubClient.hasNextPage(res) && githubClient.getNextPage(res).then(finder) + } + return finder +} + module.exports = githubClient diff --git a/package.json b/package.json index ba78b5f4..9faf95f7 100644 --- a/package.json +++ b/package.json @@ -19,9 +19,11 @@ "bunyan-middleware": "^0.3.1", "debug": "^2.2.0", "dotenv": "^2.0.0", + "effd": "^1.4.8", "express": "^4.13.4", "github": "^7.0.1", "glob": "^7.0.3", + "mentions-regex": "^2.0.3", "lru-cache": "^4.0.1", "travis-ci": "^2.1.0" }, diff --git a/scripts/manage-members.js b/scripts/manage-members.js new file mode 100644 index 00000000..9292c170 --- /dev/null +++ b/scripts/manage-members.js @@ -0,0 +1,177 @@ +'use strict' + +const P = require('effd') +const request = require('request') +const debug = require('debug')('manage-members') +const logger = require('../lib/logger.js') + +var login +const github = require('../lib/github-client.js') +// store/cache info about the authenticated user +github.users.get({}).then((user) => { + debug('authenticated as:', user.login) + login = user.login +}) +.catch((err) => { + logger.error('authentication failed', err) +}) + +// we need this RegExp to have the global flag +const mentionsRegex = new RegExp(require('mentions-regex')().source, 'g') +const exclude = (a, b) => a.filter((x) => !b.find((y) => y === x)) +const README = (file) => /^readme\.md$/i.test(file.filename || file) // array of strings or file objects + +const ignoredErrors = new Set([ + 'Not Applicable', + 'No members changed', + 'Members section not found', + 'Push is on non-default branch' +]) +const logError = (err) => { + if (ignoredErrors.has(err && err.message)) { + return debug(err.message) + } + logger.error(err) +} + +module.exports = function (app) { + app.on('push', (e, owner, repo) => onPush(e, owner, repo).catch(logError)) + app.on('pull_request.opened', (e, owner, repo) => onPullRequest(e, owner, repo).catch(logError)) + app.on('pull_request.synchronize', (e, owner, repo) => onPullRequest(e, owner, repo).catch(logError)) +} + +function onPullRequest (event, owner, repo) { + const number = event.pull_request.number + debug('onPullRequest %s %s %s', owner, repo, number) + + // ignore if not to the default branch + if (event.pull_request.base.ref !== event.repository.default_branch) { + return P.error('PR is to non-default branch') + } + + return github.pullRequests.getFiles({ owner, repo, number, per_page: 100 }) + .then(github.find(README)) + .then((readme) => !readme ? P.error('README not modified') : readme.raw_url) + .then(download) + .then(parseTeamSection) + .then((team) => findChangedMembers(owner, team.name, team.mentions)) + .then(createMessageBody) + .then((body) => + github.issues.getComments({ owner, repo, number, per_page: 100 }) + .then(github.find((comment) => comment.user.login === login)) + .then((comment) => + comment + ? github.issues.editComment({ owner, repo, id: comment.id, body }) + : github.issues.createComment({ owner, repo, number, body }) + ) + .then(() => body) + ) +} + +function onPush (event, org, repo) { + debug('onPush %s %s', org, repo) + + // ignore if not to the default branch + if (event.ref !== 'refs/heads/' + event.repository.default_branch) { + return P.error('Push is on non-default branch') + } + + // ignore if the readme wasn't changed + // missed edge case: a push directly to master (no PR, so no merge) + // and readme change was not in the head commit + const head = event.head_commit + const readme = head.modified.find(README) || head.added.find(README) + if (!readme) return P.error('README not modified') + + return download(`https://github.com/${org}/${repo}/raw/${head.id}/${readme}`) + .then(parseTeamSection) + .then((team) => findChangedMembers(org, team.name, team.mentions)) + .then(updateMembers) +} + +function download (url) { + debug('downloading %s', url) + return P((Ø) => { + request(url, (err, res, body) => + err + ? Ø.error(err) + : res.statusCode !== 200 + ? Ø.error(`Download Failed: ${res.statusCode} ${res.statusMessage}`) + : Ø.done(body) + ) + }) +} + +function updateMembers ({ id, added, removed }) { + debug('updateMembers %s %j %j', id, added, removed) + + return P.all([].concat( + added.map((username) => + github.orgs.addTeamMembership({ id, username }).catch(logError) // do not let errors stop processing + ), + removed.map((username) => + github.orgs.removeTeamMembership({ id, username }).catch(logError) // do not let errors stop processing + ))) + .then(() => { + // don't return anything to the promise chain. nothing should need/want it. + }) +} + +function findChangedMembers (org, teamName, mentions) { + // get the id of the team listed in the README + return github.orgs.getTeams({ org, per_page: 100 }) + .then(github.find((t) => t.name === teamName)) + .then((team) => + !team + ? P.error('Team Not Found: ' + teamName) + // get the usernames of the team members + : github.orgs.getTeamMembers({ id: team.id, per_page: 100 }) + .then(github.allPages) + .then((members) => members.map((member) => member.login.toLocaleLowerCase())) + .then((members) => ({ + org, + id: team.id, + team: teamName, + added: exclude(mentions, members), + removed: exclude(members, mentions) + })) + ) + .then((changes) => + !changes.added.length && !changes.removed.length + ? P.error('No members changed') + : changes + ) +} + +function createMessageBody ({org, team, added, removed}) { + var message = `This merge, if accepted, will cause changes to the @${org}/${team} team.\n` + if (added.length) message += `- Add: @${added.join(', @')}\n` + if (removed.length) message += `- Remove: @${removed.join(', @')}\n` + debug(message) + return message +} + +function parseTeamSection (readmeContent) { + const start = readmeContent.search(/', start) + if (start === -1 || end === -1) return P.error('Members section not found') + + const section = readmeContent.substring(start, end) + const mentions = section.match(mentionsRegex) || [] + const result = { + name: readmeContent.match(/ + +html should be OK in here! +Mr A (@a) +Mx B (@b) +@ c (not valid) +wm@gm.co (ignored) +and @e! + + + +# License +MIT diff --git a/test/integration/node-labels-webhook.test.js b/test/integration/node-labels-webhook.test.js index 56034921..3ff026b2 100644 --- a/test/integration/node-labels-webhook.test.js +++ b/test/integration/node-labels-webhook.test.js @@ -17,6 +17,8 @@ const testStubs = { } } +// only load the script being tested +process.env.SCRIPTS = './scripts/node-subsystem-label.js' const app = proxyquire('../../app', testStubs) const readFixture = require('../read-fixture') diff --git a/test/integration/ping.test.js b/test/integration/ping.test.js index 104095a7..ef8804ad 100644 --- a/test/integration/ping.test.js +++ b/test/integration/ping.test.js @@ -3,6 +3,8 @@ const tap = require('tap') const request = require('request') +// only load the script being tested +process.env.SCRIPTS = './scripts/ping.js' const app = require('../../app') tap.test('GET /ping responds with status 200 / "pong"', (t) => { diff --git a/test/integration/push-jenkins-update.test.js b/test/integration/push-jenkins-update.test.js index 39a49868..2b721dd8 100644 --- a/test/integration/push-jenkins-update.test.js +++ b/test/integration/push-jenkins-update.test.js @@ -5,6 +5,8 @@ const url = require('url') const nock = require('nock') const supertest = require('supertest') +// only load the script being tested +process.env.SCRIPTS = './scripts/node-jenkins-status.js' const app = require('../../app') const readFixture = require('../read-fixture') diff --git a/test/read-fixture.js b/test/read-fixture.js index dcf9597c..ea6cb668 100644 --- a/test/read-fixture.js +++ b/test/read-fixture.js @@ -2,8 +2,11 @@ const fs = require('fs') const path = require('path') +const raw = (filename) => fs.readFileSync(path.join(__dirname, '_fixtures', filename)).toString() module.exports = function readFixture (fixtureName) { - const content = fs.readFileSync(path.join(__dirname, '_fixtures', fixtureName)).toString() + const content = raw(fixtureName) return JSON.parse(content) } + +module.exports.raw = raw diff --git a/test/unit/manage-members.test.js b/test/unit/manage-members.test.js new file mode 100644 index 00000000..9757beb2 --- /dev/null +++ b/test/unit/manage-members.test.js @@ -0,0 +1,312 @@ +const proxyquire = require('proxyquire') +const tap = require('tap') +const P = require('effd') +const readFixture = require('../read-fixture') +const asyncish = (cb) => setTimeout(cb, Math.random() * 10) + +const teamsMembers = { + '222': [ 'aaa', 'Bbb', 'cCC', 'DDD' ].map((x) => ({ login: x })), + '333': [ 'alpha', 'omega', 'phi' ].map((x) => ({ login: x })), + '444': [ 'a', 'b', 'e' ].map((x) => ({ login: x })) +} +const prFiles = { + '12': [{ filename: 'docs/README.md' }], + '34': [{ filename: 'app.js' }, { filename: 'package.json' }, { filename: 'readme/md' }], + '56': [{ filename: 'reaDme.Md', raw_url: 'readme' }] +} +const manageMembers = proxyquire('../../scripts/manage-members.js', { + '../lib/github-client.js': { + issues: { + getComments: () => P([]), + createComment: () => P() + }, + pullRequests: { + getFiles: (options) => { + return P(prFiles[options.number]) + } + }, + orgs: { + getTeams: () => Promise.resolve([ + { name: 'foo', id: 111 }, + { name: 'bar', id: 222 }, + { name: 'botsters', id: 444 }, + { name: 'baz', id: 333 } + ]), + getTeamMembers: (options) => Promise.resolve(teamsMembers[options.id]), + addTeamMembership: (options) => P((Ø) => { + const members = teamsMembers[options.id] + asyncish(() => { + if (!members.find((m) => m.login === options.username)) { + members.push({ login: options.username }) + } + Ø.done() + }) + }), + removeTeamMembership: (options) => P((Ø) => { + const members = teamsMembers[options.id] + asyncish(() => { + const index = members.findIndex((m) => m.login === options.username) + if (index > -1) members.splice(index, 1) + Ø.done() + }) + }) + }, + users: { + get: () => Promise.resolve({ login: 'nodejs-github-bot' }) + }, + find: (x) => (array) => Promise.resolve(array.find(x)), + allPages: (x) => Promise.resolve(x), + '@noCallThru': true + }, + 'request': (url, cb) => { + if (url === 'http://404') return cb(null, { statusCode: 404 }) + if (url === 'http://error') return cb(new Error()) + if (url === 'readme') return cb(null, { statusCode: 200 }, readFixture.raw('README.md')) + if (url === 'https://github.com/undefined/undefined/raw/undefined/README.md') { + return cb(null, { statusCode: 200 }, readFixture.raw('README.md')) + } + + cb(null, { statusCode: 200 }, url) + } +}) + +// ... the tests: + +// download + +tap.test('download: returns content', (t) => + manageMembers.download('http://someurl') + .then((content) => { + t.equal(content, 'http://someurl') + }) +) +tap.test('download: rejects non 200', (t) => + manageMembers.download('http://404') + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.match(err.message, /^Download Failed:/) + }) +) +tap.test('download: rejects on error', (t) => + manageMembers.download('http://error') + .then(t.fail) + .catch((err) => { + t.type(err, Error) + }) +) + +// updateMembers + +tap.test('updateMembers: adds & removes the changed memebers', (t) => + manageMembers.updateMembers({ id: 333, added: [ 'bob', 'omega', 'alice' ], removed: [ 'alpha', 'idontexist' ] }) + .then(() => { // omega, phi, bob, alice + const members = teamsMembers[333].map((m) => m.login) + t.equal(members.length, 4) + t.true(members.includes('omega')) + t.true(members.includes('bob')) + t.true(members.includes('alice')) + t.true(members.includes('phi')) + t.end() + }) +) + +// findChangedMembers + +tap.test('findChangedMembers: rejects if Team Not Found', (t) => + manageMembers.findChangedMembers('nodejs', 'botfolks', [ 'xray', 'yarn', 'zulu' ]) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.match(err.message, /^Team Not Found/) + }) +) +tap.test('findChangedMembers: determines additions and removals', (t) => + manageMembers.findChangedMembers('nodejs', 'bar', [ 'xray', 'aaa', 'yarn', 'zulu' ]) + .then((changes) => { + t.equal(changes.org, 'nodejs') + t.equal(changes.id, 222) + t.strictSame(changes.added, [ 'xray', 'yarn', 'zulu' ]) + t.strictSame(changes.removed, [ 'bbb', 'ccc', 'ddd' ]) + t.end() + }) +) + +// createMessageBody + +tap.test('createMessageBody: returns undefined if no changes', (t) => { + var messageBody = manageMembers.createMessageBody({ org: 'nodejs', team: 'bot', added: [ 'a', 'b' ], removed: [ 'd', 'e' ] }) + t.equal(messageBody, 'This merge, if accepted, will cause changes to the @nodejs/bot team.\n- Add: @a, @b\n- Remove: @d, @e\n') + t.end() +}) + +// parseTeamSection + +tap.test('parseTeamSection: rejects if special comment not found', (t) => + manageMembers.parseTeamSection("## This README doesn't have the special `team` comment") + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'Members section not found') + }) +) +tap.test("parseTeamSection: rejects if special comment isn't JUST right", (t) => + manageMembers.parseTeamSection( + '## Hello\n ## Members \nThe spacing is off\n\n# License\nMIT\n' + ) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'Members section not found') + }) +) +tap.test('parseTeamSection: returns empty array if no mentions found', (t) => + manageMembers.parseTeamSection( + '## Hello\n ## Members \nno mentions\n\n# License\nMIT\n' + ) + .then((data) => { + t.type(data, Object) + t.type(data.mentions, Array) + t.equal(data.name, 'botsters') + t.equal(data.mentions.length, 0) + t.end() + }) +) +tap.test('parseTeamSection: returns array of mentions found', (t) => + manageMembers.parseTeamSection(readFixture.raw('README.md')) + .then((data) => { + t.type(data, Object) + t.type(data.mentions, Array) + t.equal(data.name, 'botsters') + t.strictSame(data.mentions, [ 'a', 'b', 'e' ]) + t.end() + }) +) + +// onPullRequest + +tap.test('onPullRequest: not default branch', (t) => + P(readFixture('pull-request-opened.json')) + .then((pr) => (pr.repository.default_branch = 'not-master') && pr) + .then(manageMembers.onPullRequest) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal('PR is to non-default branch', err.message) + t.end() + }) +) +tap.test('onPullRequest: ignores non-root level README', (t) => + // bare min event data to get it to the list with a non-root readme + P({ pull_request: { number: 12, base: {} }, repository: {} }) + .then(manageMembers.onPullRequest) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'README not modified') + t.end() + }) +) +tap.test('onPullRequest: ignored if README not in changed files', (t) => + // bare min event data to get it to the list without a readme.md + P({ pull_request: { number: 34, base: {} }, repository: {} }) + .then(manageMembers.onPullRequest) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'README not modified') + t.end() + }) +) +tap.test('onPullRequest: README changed- but members did not', (t) => + // bare min event data to get it to the list with a readme.md change + P({ pull_request: { number: 56, base: {} }, repository: {} }) + .then(manageMembers.onPullRequest) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'No members changed') + t.end() + }) +) +tap.test('onPullRequest: README changed- members added & removed', (t) => { + teamsMembers[444] = [ 'a', 'x', 'e', 'y' ].map((x) => ({ login: x })) + t.teardown(() => { + teamsMembers[444] = [ 'a', 'b', 'e' ].map((x) => ({ login: x })) + }) + // bare min event data to get it to the list with a readme.md change + return P({ pull_request: { number: 56, base: {} }, repository: {} }) + .then(manageMembers.onPullRequest) + .then((commentBody) => { + const expected = + 'This merge, if accepted, will cause changes to the @undefined/botsters team.\n' + + '- Add: @b\n' + + '- Remove: @x, @y\n' + t.equal(commentBody, expected) + t.end() + }) +}) + +// onPush + +tap.test('onPush: not default branch', (t) => + P({ ref: 'refs/heads/non-master', repository: { default_branch: 'master' } }) + .then((pr) => (pr.repository.default_branch = 'not-master') && pr) + .then(manageMembers.onPush) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal('Push is on non-default branch', err.message) + t.end() + }) +) +tap.test('onPush: ignored if README not in changed files', (t) => + // bare min event data without a readme.md change + P({ ref: 'refs/heads/undefined', repository: {}, head_commit: { modified: [], added: [] } }) + .then(manageMembers.onPush) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'README not modified') + t.end() + }) +) +tap.test('onPush: ignores non-root level README changes', (t) => + // bare min event data with a non-root readme.md change + P({ ref: 'refs/heads/undefined', repository: {}, head_commit: { modified: [ 'docs/README.md' ], added: [] } }) + .then(manageMembers.onPush) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'README not modified') + t.end() + }) +) +tap.test('onPush: README changed- but members did not', (t) => + // bare min event data with a readme.md change + P({ ref: 'refs/heads/undefined', repository: {}, head_commit: { modified: [ 'README.md' ], added: [] } }) + .then(manageMembers.onPush) + .then(t.fail) + .catch((err) => { + t.type(err, Error) + t.equal(err.message, 'No members changed') + t.end() + }) +) +tap.test('onPush: README changed- members added & removed', (t) => { + teamsMembers[444] = [ 'a', 'x', 'e', 'y' ].map((x) => ({ login: x })) + t.teardown(() => { + teamsMembers[444] = [ 'a', 'b', 'e' ].map((x) => ({ login: x })) + }) + // bare min event data with a readme.md change + return P({ ref: 'refs/heads/undefined', repository: {}, head_commit: { modified: [ 'README.md' ], added: [] } }) + .then(manageMembers.onPush) + .then(() => { + t.equal(teamsMembers[444].length, 3) + const members = teamsMembers[444].map((m) => m.login) + t.true(members.includes('a')) + t.true(members.includes('b')) + t.true(members.includes('e')) + t.end() + }) +})