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: add ability to set custom comment content #15

Merged
merged 2 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
78 changes: 68 additions & 10 deletions __tests__/comment-upserter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {RestEndpointMethodTypes} from '@octokit/plugin-rest-endpoint-methods'
import {RestEndpointMethods} from '@octokit/plugin-rest-endpoint-methods/dist-types/generated/method-types.d'
import {EqualMatchingInjectorConfig, It, Mock, Times} from 'moq.ts'

import {CommentUpserterImpl, HEADER} from '../src/comment-upserter'
import {CommentUpserterImpl, DEFAULT_COMMENT_PREAMBLE, HEADER} from '../src/comment-upserter'
import {Repo} from '../src/github-types'

describe('CommentUpserterImpl', () => {
Expand Down Expand Up @@ -35,13 +35,6 @@ describe('CommentUpserterImpl', () => {
}
]

const commentBody =
HEADER +
[
'| db/migrate/\\*\\* | @cto, @dba |',
'| .github/\\*\\*<br>spec/\\*.rb | @ci |'
].join('\n')

const stubListComments = (comments: string[]): void => {
const listResponse = {
data: comments.map((comment, index) => ({id: index + 1, body: comment}))
Expand Down Expand Up @@ -74,6 +67,16 @@ describe('CommentUpserterImpl', () => {
})

it('creates a comment', async () => {
const expectedCommentBody =
HEADER +
[
DEFAULT_COMMENT_PREAMBLE,
'| File Patterns | Mentions |',
'| - | - |',
'| db/migrate/\\*\\* | @cto, @dba |',
'| .github/\\*\\*<br>spec/\\*.rb | @ci |'
].join('\n')

issuesMock
.setup(instance => instance.createComment(It.IsAny()))
.returnsAsync(
Expand All @@ -88,7 +91,42 @@ describe('CommentUpserterImpl', () => {
instance.createComment({
...repo,
issue_number: pullNumber,
body: commentBody
body: expectedCommentBody
})
)
})

it('creates a comment with custom comment content', async () => {
const customContent = {
preamble: 'Added you as a subscriber.',
epilogue: '> [CodeMention](https://github.com/tobyhs/codemention)'
}
const expectedCommentBody =
HEADER +
[
customContent.preamble,
'| File Patterns | Mentions |',
'| - | - |',
'| db/migrate/\\*\\* | @cto, @dba |',
'| .github/\\*\\*<br>spec/\\*.rb | @ci |',
`\n${customContent.epilogue}`
].join('\n')

issuesMock
.setup(instance => instance.createComment(It.IsAny()))
.returnsAsync(
new Mock<
RestEndpointMethodTypes['issues']['createComment']['response']
>().object()
)

await upserter.upsert(repo, pullNumber, rules, customContent)

issuesMock.verify(instance =>
instance.createComment({
...repo,
issue_number: pullNumber,
body: expectedCommentBody
})
)
})
Expand All @@ -97,6 +135,16 @@ describe('CommentUpserterImpl', () => {
describe('when a codemention comment exists', () => {
describe('and the comment is different', () => {
it('updates the comment', async () => {
const expectedCommentBody =
HEADER +
[
DEFAULT_COMMENT_PREAMBLE,
'| File Patterns | Mentions |',
'| - | - |',
'| db/migrate/\\*\\* | @cto, @dba |',
'| .github/\\*\\*<br>spec/\\*.rb | @ci |'
].join('\n')

const existingComment = HEADER + '| config/brakeman.yml | @security |'
stubListComments(['First', existingComment])

Expand All @@ -114,14 +162,24 @@ describe('CommentUpserterImpl', () => {
instance.updateComment({
...repo,
comment_id: 2,
body: commentBody
body: expectedCommentBody
})
)
})
})

describe('and the comment is the same', () => {
it('does not update the comment', async () => {
const commentBody =
HEADER +
[
DEFAULT_COMMENT_PREAMBLE,
'| File Patterns | Mentions |',
'| - | - |',
'| db/migrate/\\*\\* | @cto, @dba |',
'| .github/\\*\\*<br>spec/\\*.rb | @ci |'
].join('\n')

stubListComments(['First', commentBody])
await upserter.upsert(repo, pullNumber, rules)

Expand Down
3 changes: 3 additions & 0 deletions __tests__/fixtures/codemention.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
commentConfiguration:
preamble: 'testing preamble'
epilogue: 'testing epilogue'
rules:
- patterns: ['config/**']
mentions: ['sysadmin']
Expand Down
2 changes: 1 addition & 1 deletion __tests__/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('Runner', () => {
}
]
commentUpserterMock.verify(instance =>
instance.upsert(repo, prNumber, matchingRules)
instance.upsert(repo, prNumber, matchingRules, {preamble: 'testing preamble', epilogue: 'testing epilogue'})
)
})
})
Expand Down
58 changes: 38 additions & 20 deletions src/comment-upserter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ import * as core from '@actions/core'
import {RestEndpointMethods} from '@octokit/plugin-rest-endpoint-methods/dist-types/generated/method-types.d'
import markdownEscape from 'markdown-escape'

import {MentionRule} from './configuration'
import {CommentConfiguration, MentionRule} from './configuration'
import {Repo} from './github-types'

export const HEADER = [
'<!-- codemention header -->',
'[CodeMention](https://github.com/tobyhs/codemention):\n',
'| File Patterns | Mentions |',
'| - | - |\n'
].join('\n')
export const HEADER = '<!-- codemention header -->'
Copy link
Owner

Choose a reason for hiding this comment

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

This is probably an extremely nitpicky thing, but I believe this comment header will no longer have a newline character separating it from the content afterwards. It probably doesn't make a difference when the content is rendered, but I do prefer a newline afterwards.

Copy link
Contributor Author

@wschurman wschurman Oct 8, 2024

Choose a reason for hiding this comment

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

Nice catch! Removing this is a hypothesis about what is responsible for the extra newline behavior we were seeing in the iOS notification:

img_0757_720-2

It doesn't seem to affect the comment on the web, just the one in a notification.

Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, here's another github app notification for a normal review request:
Screenshot 2024-10-07 at 2 27 06 PM

(note that the newlines between "Why" and the rest of the content are expected and part of the summary)

Copy link
Owner

Choose a reason for hiding this comment

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

In that case, then I think it makes sense to not have the newline after the comment.

Copy link
Owner

Choose a reason for hiding this comment

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

Actually, after merging and testing this, it looks like the link doesn't render correctly on tobyhs/codemention-test#8 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. Yep, definitely something up. Feel free to revert and I'll test more tomorrow and find a fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: #16

(going to move the header into a footer to fix both issues in a follow-up)


export const DEFAULT_COMMENT_PREAMBLE =
'[CodeMention](https://github.com/tobyhs/codemention):'

/**
* @see {@link upsert}
Expand All @@ -22,8 +20,14 @@ export interface CommentUpserter {
* @param repo - repository that the pull request is in
* @param pullNumber - number that identifies the pull request
* @param rules - mention rules to use in the comment
* @param commentContent - comment content to print above matching rules table
wschurman marked this conversation as resolved.
Show resolved Hide resolved
*/
upsert(repo: Repo, pullNumber: number, rules: MentionRule[]): Promise<void>
upsert(
repo: Repo,
pullNumber: number,
rules: MentionRule[],
commentConfiguration?: CommentConfiguration
): Promise<void>
}

export class CommentUpserterImpl implements CommentUpserter {
Expand All @@ -36,7 +40,8 @@ export class CommentUpserterImpl implements CommentUpserter {
async upsert(
repo: Repo,
pullNumber: number,
rules: MentionRule[]
rules: MentionRule[],
commentConfiguration?: CommentConfiguration
): Promise<void> {
const issuesApi = this.octokitRest.issues
const listResponse = await issuesApi.listComments({
Expand All @@ -47,7 +52,7 @@ export class CommentUpserterImpl implements CommentUpserter {
const existingComment = listResponse.data.find(
c => c.body !== undefined && c.body.startsWith(HEADER)
)
const commentBody = this.createCommentBody(rules)
const commentBody = this.createCommentBody(rules, commentConfiguration)

if (existingComment === undefined) {
if (rules.length > 0) {
Expand Down Expand Up @@ -76,18 +81,31 @@ export class CommentUpserterImpl implements CommentUpserter {

/**
* @param rules - mention rules to use in the comment
* @param commentContent - comment content to print above matching rules table
* @returns text to be used in a GitHub pull request comment body
*/
private createCommentBody(rules: MentionRule[]): string {
const body = rules
.map(rule => {
const patterns = rule.patterns
.map(pattern => markdownEscape(pattern, ['slashes']))
.join('<br>')
const mentions = rule.mentions.map(name => `@${name}`).join(', ')
return `| ${patterns} | ${mentions} |`
})
private createCommentBody(
rules: MentionRule[],
commentConfiguration?: CommentConfiguration
): string {
const mentionsTableRows = rules.map(rule => {
const patterns = rule.patterns
.map(pattern => markdownEscape(pattern, ['slashes']))
.join('<br>')
const mentions = rule.mentions.map(name => `@${name}`).join(', ')
return `| ${patterns} | ${mentions} |`
})
const content = [
commentConfiguration?.preamble ?? DEFAULT_COMMENT_PREAMBLE,
'| File Patterns | Mentions |',
'| - | - |',
...mentionsTableRows,
commentConfiguration?.epilogue
? `\n${commentConfiguration.epilogue}` // need two line breaks to finish table before epilogue
: undefined
]
.filter(elem => elem !== undefined)
.join('\n')
return `${HEADER}${body}`
return `${HEADER}${content}`
}
}
12 changes: 12 additions & 0 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,22 @@ export interface MentionRule {
mentions: string[]
}

/**
* A set of configuration items for the comment posted by the bot
*/
export interface CommentConfiguration {
/** Comment content to print above matching rules table */
preamble?: string
/** Comment content to print below matching rules table */
epilogue?: string
}

/**
* Configuration for the GitHub Action
*/
export interface Configuration {
/** Rules for mentioning */
rules: MentionRule[]
/** Configuration for comment */
commentConfiguration?: CommentConfiguration
}
19 changes: 10 additions & 9 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,18 @@ export default class Runner {
return
}

const configuration = await this.configurationReader.read(
repo,
pullRequest.base.sha
)
const filesChanged = await this.filesChangedReader.read(
repo,
pullRequest.number
)
const [configuration, filesChanged] = await Promise.all([
this.configurationReader.read(repo, pullRequest.base.sha),
this.filesChangedReader.read(repo, pullRequest.number)
])
const matchingRules = configuration.rules.filter(
rule => micromatch(filesChanged, rule.patterns).length > 0
)
await this.commentUpserter.upsert(repo, pullRequest.number, matchingRules)
await this.commentUpserter.upsert(
repo,
pullRequest.number,
matchingRules,
configuration.commentConfiguration
)
}
}
Loading