From 656ddd29e7aa837723d19c0888f6a37e88971892 Mon Sep 17 00:00:00 2001 From: Jake Bolam Date: Sun, 13 Jan 2019 22:30:44 +0800 Subject: [PATCH] fix: in some cases usernames are not correctly parsed (#20) * test: add test cases that are failing * wip --- serverless.yml | 4 +-- src/processIssueComment.js | 24 +++++---------- src/utils/isMessageForBot.js | 8 +++++ src/utils/parse-comment/index.js | 29 ++++++++++++++---- test/setup.js | 2 +- test/utils/isMessageForBot.test.js | 31 +++++++++++++++++++ test/utils/parse-comment/index.test.js | 41 +++++++++++++++++++++----- 7 files changed, 107 insertions(+), 32 deletions(-) create mode 100644 src/utils/isMessageForBot.js create mode 100644 test/utils/isMessageForBot.test.js diff --git a/serverless.yml b/serverless.yml index 2f8c671d..70440997 100644 --- a/serverless.yml +++ b/serverless.yml @@ -10,8 +10,8 @@ custom: dev: '23178' prod: '23186' botName: - dev: '@AllContributorsDev' - prod: '@AllContributorsBot' + dev: 'AllContributorsDev' + prod: 'AllContributorsBot' logLevel: dev: debug prod: debug # TODO change to info soon diff --git a/src/processIssueComment.js b/src/processIssueComment.js index b555f63d..f1a72d08 100644 --- a/src/processIssueComment.js +++ b/src/processIssueComment.js @@ -6,6 +6,7 @@ const ContentFiles = require('./ContentFiles') const getUserDetails = require('./utils/getUserDetails') const parseComment = require('./utils/parse-comment') +const isMessageForBot = require('./utils/isMessageForBot') const { GIHUB_BOT_NAME } = require('./utils/settings') const { AllContributorBotError } = require('./utils/errors') @@ -69,13 +70,6 @@ async function processIssueComment({ context, commentReply }) { const commentBody = context.payload.comment.body const parsedComment = parseComment(commentBody) - if (!parsedComment.action) { - commentReply.reply(`I could not determine your intention.`) - commentReply.reply( - `Basic usage: @${GIHUB_BOT_NAME} please add jakebolam for code, doc`, - ) - return - } if (parsedComment.action === 'add') { await processAddContributor({ @@ -89,26 +83,24 @@ async function processIssueComment({ context, commentReply }) { return } - commentReply.reply(`I'm not sure how to ${parsedComment.action}`) + commentReply.reply(`I could not determine your intention.`) commentReply.reply( - `Basic usage: @${GIHUB_BOT_NAME} please add jakebolam for code, doc`, + `Basic usage: @${GIHUB_BOT_NAME} please add jakebolam for code, doc and infra`, + ) + commentReply( + `For other usage see the [documentation](https://github.com/all-contributors/all-contributors-bot#usage)`, ) return } -function hasMentionedBotName(context) { - const commentBody = context.payload.comment.body - const hasMentionedBotName = commentBody.includes(GIHUB_BOT_NAME) - return hasMentionedBotName -} - async function processIssueCommentSafe({ context }) { if (context.isBot) { context.log.debug('From a bot, exiting') return } - if (!hasMentionedBotName(context)) { + const commentBody = context.payload.comment.body + if (!isMessageForBot(commentBody)) { context.log.debug('Message not for us, exiting') return } diff --git a/src/utils/isMessageForBot.js b/src/utils/isMessageForBot.js new file mode 100644 index 00000000..ab809e9f --- /dev/null +++ b/src/utils/isMessageForBot.js @@ -0,0 +1,8 @@ +const { GIHUB_BOT_NAME } = require('./settings') + +function isMessageForBot(message) { + const isMessageForBot = message.includes(`@${GIHUB_BOT_NAME}`) + return isMessageForBot +} + +module.exports = isMessageForBot diff --git a/src/utils/parse-comment/index.js b/src/utils/parse-comment/index.js index 430596a9..95dc5575 100644 --- a/src/utils/parse-comment/index.js +++ b/src/utils/parse-comment/index.js @@ -42,6 +42,7 @@ validContributionTypes.forEach(type => { const plugin = { words: { ...Contributions, + add: 'Action', }, patterns: { // 'add #person for #Contribution': 'AddContributor', @@ -51,7 +52,23 @@ const plugin = { nlp.plugin(plugin) function parseAddComment(doc, action) { - const who = doc.match(`${action} [.]`).data()[0].normal + const who = doc + .match(`${action} [.]`) + .normalize({ + whitespace: true, // remove hyphens, newlines, and force one space between words + case: false, // keep only first-word, and 'entity' titlecasing + numbers: false, // turn 'seven' to '7' + punctuation: true, // remove commas, semicolons - but keep sentence-ending punctuation + unicode: false, // visually romanize/anglicize 'Björk' into 'Bjork'. + contractions: false, // turn "isn't" to "is not" + acronyms: false, //remove periods from acronyms, like 'F.B.I.' + parentheses: false, //remove words inside brackets (like these) + possessives: false, // turn "Google's tax return" to "Google tax return" + plurals: false, // turn "batmobiles" into "batmobile" + verbs: false, // turn all verbs into Infinitive form - "I walked" → "I walk" + honorifics: false, //turn 'Vice Admiral John Smith' to 'John Smith' + }) + .data()[0].text // TODO: handle plurals (e.g. some said docs) const contributions = doc @@ -72,17 +89,17 @@ function parseAddComment(doc, action) { function parseComment(message) { const doc = nlp(message) - if (doc.verbs().data().length === 0) { - return {} - } + const action = doc + .match('#Action') + .normalize() + .out('string') - const action = doc.verbs().data()[0].normal if (action === 'add') { return parseAddComment(doc, action) } return { - action, + action: false, } } diff --git a/test/setup.js b/test/setup.js index 64930022..405285ba 100644 --- a/test/setup.js +++ b/test/setup.js @@ -2,4 +2,4 @@ const nock = require('nock') nock.disableNetConnect() -process.env.BOT_NAME = '@AllContributorsBotTest' +process.env.BOT_NAME = 'AllContributorsBotTest' diff --git a/test/utils/isMessageForBot.test.js b/test/utils/isMessageForBot.test.js new file mode 100644 index 00000000..c65bc0c3 --- /dev/null +++ b/test/utils/isMessageForBot.test.js @@ -0,0 +1,31 @@ +const isMessageForBot = require('../../src/utils/isMessageForBot') + +describe('isMessageForBot', () => { + const testBotName = 'AllContributorsBotTest' + + test('For us', () => { + expect( + isMessageForBot( + `@${testBotName} please add jakebolam for doc, infra and code`, + ), + ).toBe(true) + + expect( + isMessageForBot( + `@AllContributorsBotTest please add jakebolam for doc, infra and code`, + ), + ).toBe(true) + }) + + test('Not for us', () => { + expect( + isMessageForBot( + `${testBotName} please add jakebolam for doc, infra and code`, + ), + ).toBe(false) + + expect( + isMessageForBot(`Please add jakebolam for doc, infra and code`), + ).toBe(false) + }) +}) diff --git a/test/utils/parse-comment/index.test.js b/test/utils/parse-comment/index.test.js index fcdb4c21..a4a5c361 100644 --- a/test/utils/parse-comment/index.test.js +++ b/test/utils/parse-comment/index.test.js @@ -1,11 +1,12 @@ const parseComment = require('../../../src/utils/parse-comment') -const { GIHUB_BOT_NAME } = require('../../../src//utils/settings') describe('parseComment', () => { + const testBotName = 'AllContributorsBotTest' + test('Basic intent to add', () => { expect( parseComment( - `@${GIHUB_BOT_NAME} please add jakebolam for doc, infra and code`, + `@${testBotName} please add jakebolam for doc, infra and code`, ), ).toEqual({ action: 'add', @@ -13,16 +14,42 @@ describe('parseComment', () => { contributions: ['doc', 'infra', 'code'], }) }) + + test('Basic intent to add - non name username', () => { + expect( + parseComment(`@${testBotName} please add tbenning for design`), + ).toEqual({ + action: 'add', + who: 'tbenning', + contributions: ['design'], + }) + }) + + test('Basic intent to add - captialized username', () => { + expect( + parseComment(`@${testBotName} please add Rbot25_RULES for tool`), + ).toEqual({ + action: 'add', + who: 'Rbot25_RULES', + contributions: ['tool'], + }) + }) + + test('Intent unknown', () => { + expect( + parseComment(`@${testBotName} please lollmate for tool`), + ).toEqual({ + action: false, + }) + }) // // test('Basic intent to add (with plurals)', () => { // expect( - // parseComment( - // `@${GIHUB_BOT_NAME} please add jakebolam for docs, infra and code`, - // ), + // parseComment(`@${testBotName} please add dat2 for docs`), // ).toEqual({ // action: 'add', - // who: 'jakebolam', - // contributions: ['doc', 'infra', 'code'], + // who: 'dat2', + // contributions: ['doc'], // }) // }) })