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: support adding multiple users in a single message #127

Merged
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
96 changes: 64 additions & 32 deletions src/tasks/processIssueComment/probot-processIssueComment.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,55 @@ const {

const getSafeRef = require('./utils/git/getSafeRef')

async function processAddContributor({
async function processAddContributors({
context,
commentReply,
repository,
optionsConfig,
who,
contributions,
contributors,
branchName,
}) {
if (contributions.length === 0) {
context.log.debug('No contributions')
return commentReply.reply(
`I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use [valid contribution names](https://allcontributors.org/docs/en/emoji-key).`,
)

const usersAdded = []
const usersMissed = []

async function addContributor(who, contributions) {
if (contributions.length === 0) {
context.log.debug(`No contributions for ${who}`)
usersMissed.push(who)
return
}

// TODO: wrap this blog in a try catch, if one user fails, don't fail the whole pull request
const { name, avatar_url, profile } = await getUserDetails({
github: context.github,
username: who,
})

await optionsConfig.addContributor({
login: who,
contributions,
name,
avatar_url,
profile,
})

usersAdded.push(who)
}
const { name, avatar_url, profile } = await getUserDetails({
github: context.github,
username: who,
})

await optionsConfig.addContributor({
login: who,
contributions,
name,
avatar_url,
profile,
// TODO: throttle max paralle requests
const addContributorPromises = contributors.map(function(contributor) {
return addContributor((contributor.who, contributor.contributions))
})

await Promise.all(addContributorPromises)

if (usersAdded === 0) {
return commentReply.reply(
`I couldn't determine any contributions for ${usersAdded.join(', ')}.
Did you specify any contributions? Please make sure to use [valid contribution names](https://allcontributors.org/docs/en/emoji-key).`,
)
tenshiAMD marked this conversation as resolved.
Show resolved Hide resolved

const contentFiles = new ContentFiles({
repository,
})
Expand All @@ -58,27 +78,36 @@ async function processAddContributor({
originalSha: optionsConfig.getOriginalSha(),
}

const pullRequestBodyAdds = `Adds the following contributors:
- ${usersAdded.join('\n- ')}\n\n`

const pullRequestBodyMissed = `Unable to determine contributions for the following contributors, these were excluded from this PR:
- ${usersMissed.join('\\n- ')}\n\n`

const pullRequestBodyRequester = `This was requested by ${commentReply.replyingToWho()} [in this comment](${commentReply.replyingToWhere()})`

const {
pullRequestURL,
pullCreated,
} = await repository.createPullRequestFromFiles({
title: `docs: add ${who} as a contributor`,
body: `Adds @${who} as a contributor for ${contributions.join(
', ',
)}.\n\nThis was requested by ${commentReply.replyingToWho()} [in this comment](${commentReply.replyingToWhere()})`,
title: `docs: add new contributors`,
body: `${pullRequestBodyAdds}.\n\n
${usersMissed > 0 ? pullRequestBodyMissed : ''}
${pullRequestBodyRequester}
`,
filesByPath: filesByPathToUpdate,
branchName,
})

if (pullCreated) {
commentReply.reply(
`I've put up [a pull request](${pullRequestURL}) to add @${who}! :tada:`,
`I've put up [a pull request](${pullRequestURL}) to add new contributors! :tada:`,
)
return
}
// Updated
commentReply.reply(
`I've updated [the pull request](${pullRequestURL}) to add @${who}! :tada:`,
`I've updated [the pull request](${pullRequestURL}) to add contributors! :tada:`,
)
}

Expand Down Expand Up @@ -133,14 +162,18 @@ async function probotProcessIssueComment({ context, commentReply, analytics }) {
analytics.track('processComment', {
commentBody: commentBody,
})
const { who, action, contributions } = parseComment(commentBody)
const { action, contributors } = parseComment(commentBody)

if (action === 'add') {
analytics.track('addContributor', {
analytics.track('addContributors', {
who: commentBody,
contributions,
contributors,
})
const safeWho = getSafeRef(who)

const whos = contributors.map(({ who }) => who).join('--')
const safeWho = getSafeRef(whos)
// TODO: max length on branch name?
Copy link
Member

Choose a reason for hiding this comment

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

If we were to have a max length, what would it be?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest 255 chars. While it's true that people could limit themselves to using GitHub's ui for everything, and with recent changes, it's possible to renaming existing branches, branch names are generally stored in file systems and can be subject to the whims of file systems, some of whom don't particularly like very long file names (I'm not suggesting an 11 character limit...).

I suspect that would give you ~16-20 contributors, which is quite a lot.

Copy link
Member

Choose a reason for hiding this comment

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

@jsoref That makes sense, yeah 255 chars seems good to me.

// TODO: should this be the branch name
const branchName = `all-contributors/add-${safeWho}`

const repository = await setupRepository({
Expand All @@ -149,13 +182,12 @@ async function probotProcessIssueComment({ context, commentReply, analytics }) {
})
const optionsConfig = await setupOptionsConfig({ repository })

await processAddContributor({
await processAddContributors({
context,
commentReply,
repository,
optionsConfig,
who,
contributions,
contributors,
branchName,
})
analytics.track('processCommentSuccess')
Expand Down
63 changes: 56 additions & 7 deletions src/tasks/processIssueComment/utils/parse-comment/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,9 @@ const plugin = {

nlp.plugin(plugin)

function parseAddComment(message, action) {
const whoMatched = nlp(message)
.match(`${action} [.]`)
.normalize({
function findWho(message, action) {
function findWhoSafe(match) {
const whoNormalizeSettings = {
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'
Expand All @@ -126,8 +125,40 @@ function parseAddComment(message, action) {
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
}
const matchedSet = nlp(message)
.match(match)
.normalize(whoNormalizeSettings)
.data()

if (matchedSet.length > 0) {
return matchedSet[0].text
}
}

const whoMatchedMention = findWhoSafe(`@[.]`)
if (whoMatchedMention) {
return whoMatchedMention
}

const whoMatchedByAction = findWhoSafe(`${action} [.]`)
if (whoMatchedByAction) {
return whoMatchedByAction
}

const whoMatchedByFor = findWhoSafe(`[.] for`)
if (whoMatchedByFor) {
return whoMatchedByFor
}
}

function parseAddSentence(message, action) {
const whoMatched = findWho(message, action)
if (!whoMatched) {
return {
who: undefined,
}
}

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

Expand Down Expand Up @@ -159,12 +190,30 @@ function parseAddComment(message, action) {
})

return {
action: 'add',
who,
contributions,
}
}

function parseAddComment(message, action) {
const contributors = {}

const sentences = nlp(message).sentences()
sentences.forEach(function(sentence) {
const sentenceRaw = sentence.data()[0].text
const { who, contributions } = parseAddSentence(sentenceRaw, action)

if (who) {
contributors[who] = contributions
}
})

return {
action: 'add',
contributors,
}
}

function parseComment(message) {
const doc = nlp(message)

Expand Down
Loading