Skip to content

Commit 666abc3

Browse files
MatissJanisMatissJanis
authored
Merge pull request #34 from actualbudget/matiss/auto-assign
feat: Auto-assign maintainers to PRs on comment/review
2 parents 2298b07 + 9eebac4 commit 666abc3

11 files changed

+458
-1
lines changed

‎src/classes/PullRequest.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { Context, ProbotOctokit } from 'probot';
22

33
import { labels } from '../labels.js';
44

5-
type TContext = Context<'pull_request' | 'pull_request_review'>;
5+
type TContext = Context<
6+
'pull_request' | 'pull_request_review' | 'pull_request_review_comment'
7+
>;
68
type ReviewStatus =
79
| 'changesRequested'
810
| 'needsMoreApprovals'
@@ -195,4 +197,25 @@ export default class PullRequest {
195197

196198
return 'readyForReview';
197199
}
200+
201+
async isOrgMember(username: string): Promise<boolean> {
202+
try {
203+
await this.octokit.orgs.checkMembershipForUser({
204+
org: 'actualbudget',
205+
username,
206+
});
207+
return true;
208+
} catch {
209+
return false;
210+
}
211+
}
212+
213+
async addAssignee(username: string) {
214+
await this.octokit.issues.addAssignees({
215+
owner: this.owner,
216+
repo: this.repo,
217+
issue_number: this.number,
218+
assignees: [username],
219+
});
220+
}
198221
}

‎src/handlers/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@ import { Probot } from 'probot';
22

33
import CheckSuite from './checkSuite.js';
44
import Installation from './installation.js';
5+
import IssueComment from './issueComment.js';
56
import PullRequest from './pullRequest.js';
67
import PullRequestReview from './pullRequestReview.js';
8+
import PullRequestReviewComment from './pullRequestReviewComment.js';
79

810
export default (app: Probot) => {
911
CheckSuite(app);
1012
Installation(app);
13+
IssueComment(app);
1114
PullRequest(app);
1215
PullRequestReview(app);
16+
PullRequestReviewComment(app);
1317
};

‎src/handlers/issueComment.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { Probot } from 'probot';
2+
3+
import PullRequest from '../classes/PullRequest.js';
4+
5+
export default (app: Probot) => {
6+
app.on(['issue_comment.created'], async context => {
7+
// Only handle comments on pull requests (not issues)
8+
if (!context.payload.issue.pull_request) return;
9+
10+
const commenter = context.payload.comment.user?.login;
11+
if (!commenter) return;
12+
13+
const pr = await PullRequest.getFromNumber(
14+
context,
15+
context.payload.issue.number,
16+
);
17+
18+
const isMember = await pr.isOrgMember(commenter);
19+
if (!isMember) return;
20+
21+
await pr.addAssignee(commenter);
22+
});
23+
};

‎src/handlers/pullRequestReview.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,19 @@ export default (app: Probot) => {
66
app.on(['pull_request_review'], async context => {
77
const pr = new PullRequest(context);
88

9+
// Auto-assign reviewer if they are an org member
10+
const reviewer = context.payload.review.user?.login;
11+
if (reviewer) {
12+
const isMember = await pr.isOrgMember(reviewer);
13+
if (isMember) {
14+
try {
15+
await pr.addAssignee(reviewer);
16+
} catch {
17+
// Ignore errors - don't break review labeling if assignment fails
18+
}
19+
}
20+
}
21+
922
if (pr.wip || pr.data.draft) return;
1023

1124
const reviewStatus = await pr.getReviewStatus();

‎src/handlers/pullRequestReviewComment.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { Probot } from 'probot';
2+
3+
import PullRequest from '../classes/PullRequest.js';
4+
5+
export default (app: Probot) => {
6+
app.on(['pull_request_review_comment.created'], async context => {
7+
const commenter = context.payload.comment.user?.login;
8+
if (!commenter) return;
9+
10+
const pr = new PullRequest(context);
11+
12+
const isMember = await pr.isOrgMember(commenter);
13+
if (!isMember) return;
14+
15+
await pr.addAssignee(commenter);
16+
});
17+
};

‎test/autoAssign/issueComment.test.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { describe, beforeEach, afterEach, test, expect } from 'vitest';
2+
import fs from 'fs';
3+
import nock from 'nock';
4+
import path from 'path';
5+
6+
import { setupProbot, teardownProbot } from '../testHelpers';
7+
8+
const issueCommentPayload = JSON.parse(
9+
fs.readFileSync(
10+
path.join(__dirname, '../fixtures/issue_comment.json'),
11+
'utf-8',
12+
),
13+
);
14+
15+
const pullRequestPayload = JSON.parse(
16+
fs.readFileSync(
17+
path.join(__dirname, '../fixtures/pull_request.json'),
18+
'utf-8',
19+
),
20+
);
21+
22+
describe('Issue Comment Auto-Assign', () => {
23+
let probot: any;
24+
25+
beforeEach(() => {
26+
probot = setupProbot();
27+
});
28+
29+
afterEach(() => {
30+
teardownProbot();
31+
});
32+
33+
test('assigns maintainer when they comment on a PR', async () => {
34+
const mock = nock('https://api.github.com')
35+
.post('/app/installations/2/access_tokens')
36+
.reply(200, {
37+
token: 'test',
38+
permissions: {
39+
pull_requests: 'write',
40+
},
41+
})
42+
.get('/repos/your-repo/your-repo-name/pulls/1')
43+
.reply(200, pullRequestPayload.pull_request)
44+
.get('/orgs/actualbudget/members/maintainer-user')
45+
.reply(204)
46+
.post(
47+
'/repos/your-repo/your-repo-name/issues/1/assignees',
48+
(body: any) => {
49+
expect(body).toMatchObject({ assignees: ['maintainer-user'] });
50+
return true;
51+
},
52+
)
53+
.reply(200);
54+
55+
await probot.receive({
56+
name: 'issue_comment',
57+
payload: issueCommentPayload,
58+
});
59+
60+
expect(mock.pendingMocks()).toStrictEqual([]);
61+
});
62+
63+
test('does not assign non-maintainer when they comment on a PR', async () => {
64+
const mock = nock('https://api.github.com')
65+
.post('/app/installations/2/access_tokens')
66+
.reply(200, {
67+
token: 'test',
68+
permissions: {
69+
pull_requests: 'write',
70+
},
71+
})
72+
.get('/repos/your-repo/your-repo-name/pulls/1')
73+
.reply(200, pullRequestPayload.pull_request)
74+
.get('/orgs/actualbudget/members/maintainer-user')
75+
.reply(404);
76+
77+
const errorMock = nock('https://api.github.com')
78+
.post('/repos/your-repo/your-repo-name/issues/1/assignees')
79+
.reply(() => {
80+
throw new Error('Assignee should not be called for non-maintainers');
81+
});
82+
83+
await probot.receive({
84+
name: 'issue_comment',
85+
payload: issueCommentPayload,
86+
});
87+
88+
expect(mock.pendingMocks()).toStrictEqual([]);
89+
expect(errorMock.isDone()).toBe(false);
90+
});
91+
92+
test('ignores comments on issues (not PRs)', async () => {
93+
const issueOnlyPayload = {
94+
...issueCommentPayload,
95+
issue: {
96+
...issueCommentPayload.issue,
97+
pull_request: undefined,
98+
},
99+
};
100+
101+
const errorMock = nock('https://api.github.com')
102+
.get('/orgs/actualbudget/members/maintainer-user')
103+
.reply(() => {
104+
throw new Error('Org membership should not be checked for issues');
105+
});
106+
107+
await probot.receive({
108+
name: 'issue_comment',
109+
payload: issueOnlyPayload,
110+
});
111+
112+
expect(errorMock.isDone()).toBe(false);
113+
});
114+
});

‎test/autoAssign/pullRequestReview.test.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, beforeEach, afterEach, test, expect } from 'vitest';
2+
import fs from 'fs';
3+
import nock from 'nock';
4+
import path from 'path';
5+
6+
import { setupProbot, teardownProbot } from '../testHelpers';
7+
8+
const pullRequestReviewPayload = JSON.parse(
9+
fs.readFileSync(
10+
path.join(__dirname, '../fixtures/pull_request_review.json'),
11+
'utf-8',
12+
),
13+
);
14+
15+
describe('Pull Request Review Auto-Assign', () => {
16+
let probot: any;
17+
18+
beforeEach(() => {
19+
probot = setupProbot();
20+
});
21+
22+
afterEach(() => {
23+
teardownProbot();
24+
});
25+
26+
test('assigns maintainer when they submit a review', async () => {
27+
const mock = nock('https://api.github.com')
28+
.post('/app/installations/2/access_tokens')
29+
.reply(200, {
30+
token: 'test',
31+
permissions: {
32+
pull_requests: 'write',
33+
},
34+
})
35+
.get('/orgs/actualbudget/members/maintainer-user')
36+
.reply(204)
37+
.post(
38+
'/repos/your-repo/your-repo-name/issues/1/assignees',
39+
(body: any) => {
40+
expect(body).toMatchObject({ assignees: ['maintainer-user'] });
41+
return true;
42+
},
43+
)
44+
.reply(200)
45+
.get('/repos/your-repo/your-repo-name/pulls/1/reviews')
46+
.reply(200, [])
47+
.put('/repos/your-repo/your-repo-name/issues/1/labels')
48+
.reply(200);
49+
50+
await probot.receive({
51+
name: 'pull_request_review',
52+
payload: pullRequestReviewPayload,
53+
});
54+
55+
expect(mock.pendingMocks()).toStrictEqual([]);
56+
});
57+
58+
test('does not assign non-maintainer when they submit a review', async () => {
59+
const mock = nock('https://api.github.com')
60+
.post('/app/installations/2/access_tokens')
61+
.reply(200, {
62+
token: 'test',
63+
permissions: {
64+
pull_requests: 'write',
65+
},
66+
})
67+
.get('/orgs/actualbudget/members/maintainer-user')
68+
.reply(404)
69+
.get('/repos/your-repo/your-repo-name/pulls/1/reviews')
70+
.reply(200, [])
71+
.put('/repos/your-repo/your-repo-name/issues/1/labels')
72+
.reply(200);
73+
74+
const errorMock = nock('https://api.github.com')
75+
.post('/repos/your-repo/your-repo-name/issues/1/assignees')
76+
.reply(() => {
77+
throw new Error('Assignee should not be called for non-maintainers');
78+
});
79+
80+
await probot.receive({
81+
name: 'pull_request_review',
82+
payload: pullRequestReviewPayload,
83+
});
84+
85+
expect(mock.pendingMocks()).toStrictEqual([]);
86+
expect(errorMock.isDone()).toBe(false);
87+
});
88+
});

0 commit comments

Comments
 (0)