Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(contribution-types): support plural forms, multi word types and any case #53

Merged
merged 8 commits into from
Jan 18, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ A bot for automatically adding all-contributors.
### Adding contributions
Comment on Issue or Pull Request with text:
```
@all-contributors please add @jakebolam for infrastructure, test and code
@all-contributors please add @jakebolam for infrastructure, tests and code
```

The bot will then create a Pull Request to add the contributor, then reply with the pull request details.
Expand Down
94 changes: 74 additions & 20 deletions src/utils/parse-comment/index.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
const nlp = require('compromise')

// Types that are valid (multi words must all be lower case)
const validContributionTypes = [
'blog',
'bug',
'code',
'design',
'doc',
'eventOrganizing',
'eventorganizing',
'example',
'financial',
'fundingFinding',
'fundingfinding',
'ideas',
'infra',
'platform',
Expand All @@ -22,16 +23,58 @@ const validContributionTypes = [
'tool',
'translation',
'tutorial',
'userTesting',
'usertesting',
'video',
]

// Types that are valid multi words, that we need to re map back to there camelCase parts
const validMultiContributionTypesMapping = {
eventorganizing: 'eventOrganizing',
fundingfinding: 'fundingFinding',
usertesting: 'userTesting',
}

// Additional terms to match to types (plurals, aliases etc)
Copy link
Contributor Author

@jakebolam jakebolam Jan 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I investigated using compromise for plurals and it fell quite short (got 15 of our needed 21) also missed some plural forms.

const contributionTypeMappings = {
blogs: 'blog',
blogging: 'blog',
bugs: 'bug',
codes: 'code',
coding: 'code',
designing: 'design',
desigs: 'design',
doc: 'doc',
docs: 'doc',
documenting: 'doc',
documentation: 'doc',
examples: 'example',
finance: 'financial',
financials: 'financial',
funds: 'fundingfinding',
idea: 'ideas',
infras: 'infra',
infrastructure: 'infra',
platforms: 'platform',
plugins: 'plugin',
questions: 'question',
reviews: 'review',
securing: 'security',
talks: 'talk',
tests: 'test',
testing: 'test',
tools: 'tool',
tooling: 'tool',
translations: 'translation',
tutorials: 'tutorial',
videoes: 'video',
}

// Additional terms to match to types (plurals, aliases etc) that are multi word
const contributionTypeMultiWordMapping = {
'event organizing': 'eventOrganizing',
'fund finding': 'fundingFinding',
'funding finding': 'fundingFinding',
'user testing': 'userTesting',
documentation: 'doc',
infrastructure: 'infra',
}

const Contributions = {}
Expand All @@ -49,15 +92,12 @@ const plugin = {
...Contributions,
add: 'Action',
},
patterns: {
// 'add #person for #Contribution': 'AddContributor',
// "i can't (log|sign|get) in to my? #Software": 'LoginIssue'
},
}

nlp.plugin(plugin)

function parseAddComment(doc, action) {
const whoMatched = doc
function parseAddComment(message, action) {
const whoMatched = nlp(message)
.match(`${action} [.]`)
.normalize({
whitespace: true, // remove hyphens, newlines, and force one space between words
Expand All @@ -77,20 +117,33 @@ function parseAddComment(doc, action) {

const who = whoMatched.startsWith('@') ? whoMatched.substr(1) : whoMatched

// TODO: handle plurals (e.g. some said docs)
let contributions = doc
// Contributions
const doc = nlp(message).toLowerCase()
// This is to support multi word 'matches' (altho the compromise docs say it supports this *confused*)
Object.entries(contributionTypeMultiWordMapping).forEach(
([multiWordType, singleWordType]) => {
doc.replace(multiWordType, singleWordType)
},
)
const contributions = doc
.match('#Contribution')
.lump()
.data()
.map(data => {
// This removes whitespace, commas etc
return data.normal
})

contributions = contributions.map(type => {
if (contributionTypeMappings[type])
return contributionTypeMappings[type]
return type
})
.map(type => {
if (contributionTypeMappings[type])
return contributionTypeMappings[type]
return type
})
.map(type => {
// Convert usertesting to userTesting for the node api
if (validMultiContributionTypesMapping[type])
return validMultiContributionTypesMapping[type]
return type
})

return {
action: 'add',
Expand All @@ -103,12 +156,13 @@ function parseComment(message) {
const doc = nlp(message)

const action = doc
.toLowerCase()
.match('#Action')
.normalize()
.out('string')

if (action === 'add') {
return parseAddComment(doc, action)
return parseAddComment(message, action)
}

return {
Expand Down
102 changes: 77 additions & 25 deletions test/utils/parse-comment/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ describe('parseComment', () => {
})
})

test('Basic intent to add - ignore case (for action and contributions, NOT for user)', () => {
expect(
parseComment(
`@${testBotName} please Add jakeBolam for DOC, inFra and coDe`,
),
).toEqual({
action: 'add',
who: 'jakeBolam',
contributions: ['doc', 'infra', 'code'],
})
})

test('Basic intent to add - non name username', () => {
expect(
parseComment(`@${testBotName} please add tbenning for design`),
Expand All @@ -35,11 +47,13 @@ describe('parseComment', () => {
})
})

test('Intent unknown', () => {
test('Basic intent to add - with plurals', () => {
expect(
parseComment(`@${testBotName} please lollmate for tool`),
parseComment(`@${testBotName} please add dat2 for docs`),
).toEqual({
action: false,
action: 'add',
who: 'dat2',
contributions: ['doc'],
})
})

Expand Down Expand Up @@ -67,27 +81,65 @@ describe('parseComment', () => {
})
})

// TODO: make it so this works
// test('Support split words (like user testing)', () => {
// expect(
// parseComment(
// `@${testBotName} please add jakebolam for infrastructure, user testing`,
// ),
// ).toEqual({
// action: 'add',
// who: 'jakebolam',
// contributions: ['infra', 'userTesting'],
// })
// })
test('Support alternative sentences', () => {
expect(
parseComment(`@${testBotName} add @sinchang for infrastructure`),
).toEqual({
action: 'add',
who: 'sinchang',
contributions: ['infra'],
})

expect(
parseComment(
`Jane you're crushing it in documentation and infrastructure, let's add jane.doe23 for her contributions. cc @${testBotName}`,
),
).toEqual({
action: 'add',
who: 'jane.doe23',
contributions: ['doc', 'infra'],
})
})

test('Support split words (like user testing)', () => {
expect(
parseComment(
`@${testBotName} please add jakebolam for infrastructure, fund finding`,
),
).toEqual({
action: 'add',
who: 'jakebolam',
contributions: ['infra', 'fundingFinding'],
})

expect(
parseComment(
`@${testBotName} please add jakebolam for infrastructure, user testing and testing`,
),
).toEqual({
action: 'add',
who: 'jakebolam',
contributions: ['infra', 'userTesting', 'test'],
})
})

test('Support split words types that are referenced via other terms (e.g. a plural split word)', () => {
expect(
parseComment(
`@${testBotName} please add @jakebolam for infrastructure, funds`,
),
).toEqual({
action: 'add',
who: 'jakebolam',
contributions: ['infra', 'fundingFinding'],
})
})

//
// test('Basic intent to add (with plurals)', () => {
// expect(
// parseComment(`@${testBotName} please add dat2 for docs`),
// ).toEqual({
// action: 'add',
// who: 'dat2',
// contributions: ['doc'],
// })
// })
test('Intent unknown', () => {
expect(
parseComment(`@${testBotName} please lollmate for tool`),
).toEqual({
action: false,
})
})
})