Skip to content

Commit 9d032f5

Browse files
menessmeness
committed
fix: fix guards helpers to become more strict and handle exceptions
1 parent 997940b commit 9d032f5

File tree

6 files changed

+412
-203
lines changed

6 files changed

+412
-203
lines changed

‎src/handlers/chat.handler.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,15 @@ import { toneDetails } from '~common/consts';
1111
import { Handler } from '~common/interfaces';
1212
import { Config } from '~common/types';
1313
import { aiConfig } from '~configs';
14-
import { isDiscussion, isDiscussionComment, isPullRequestReviewComment } from '~helpers';
14+
import { SystemError } from '~errors';
15+
import {
16+
isDiscussion,
17+
isDiscussionComment,
18+
isInlineComment,
19+
isIssue,
20+
isIssueComment,
21+
isPullRequestComment
22+
} from '~helpers';
1523
import { AppService, CacheService, GitHubService, OpenAiService, QueueService, TemplateService } from '~services';
1624
import { chatCommentTemplate, chatPromptTemplate, chatSystemTemplate } from '~templates';
1725
import { ChatSkipConditions } from '~utils';
@@ -84,7 +92,7 @@ export class ChatHandler implements Handler<CommentEvent> {
8492
language: config.language
8593
}),
8694
model: this.openAiService.getModel(
87-
isPullRequestReviewComment(context.payload) ? aiConfig.model.inlineComment : aiConfig.model.comment,
95+
isInlineComment(context.payload) ? aiConfig.model.inlineComment : aiConfig.model.comment,
8896
apiKey
8997
),
9098
prompt: TemplateService.render(chatPromptTemplate, { username: comment.user.login, body: contextBody })
@@ -101,16 +109,18 @@ export class ChatHandler implements Handler<CommentEvent> {
101109
private async createComment(context: Context<CommentEvent>, replyContent: string): Promise<void> {
102110
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
103111
await this.queueService.schedule(() => GitHubService.createDiscussionComment(context, replyContent));
104-
} else if (isPullRequestReviewComment(context.payload)) {
112+
} else if (isInlineComment(context.payload)) {
105113
await this.queueService.schedule(() => GitHubService.createPullRequestReviewComment(context, replyContent));
106-
} else {
114+
} else if (isIssue(context.payload) || isIssueComment(context.payload) || isPullRequestComment(context.payload)) {
107115
await this.queueService.schedule(() => GitHubService.createIssueComment(context, replyContent));
116+
} else {
117+
throw new SystemError('Unhandled case');
108118
}
109119
}
110120

111121
private async getContextBody(context: Context<CommentEvent>): Promise<string | null> {
112122
return this.queueService.schedule(async () => {
113-
if (isPullRequestReviewComment(context.payload) && context.payload.comment.in_reply_to_id) {
123+
if (isInlineComment(context.payload) && context.payload.comment.in_reply_to_id) {
114124
const reviewComment = await GitHubService.getReviewComment(context, context.payload.comment.in_reply_to_id);
115125

116126
return reviewComment.data.body;

‎src/helpers/guard.helper.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
import { Context } from 'probot';
22
import { BaseError, SystemError, UserError } from '~errors';
33

4-
export const isDiscussionComment = (payload: Context['payload']): payload is Context<'discussion_comment'>['payload'] =>
5-
'comment' in payload && 'discussion' in payload;
6-
74
export const isDiscussion = (payload: Context['payload']): payload is Context<'discussion'>['payload'] =>
85
'discussion' in payload;
96

10-
export const isPullRequestReviewComment = (
7+
export const isDiscussionComment = (payload: Context['payload']): payload is Context<'discussion_comment'>['payload'] =>
8+
'comment' in payload && 'discussion' in payload;
9+
10+
export const isInlineComment = (
1111
payload: Context['payload']
12-
): payload is Context<'pull_request_review_comment'>['payload'] => 'pull_request' in payload;
12+
): payload is Context<'pull_request_review_comment'>['payload'] =>
13+
'comment' in payload && 'pull_request' in payload && !!payload.pull_request;
1314

1415
export const isIssueComment = (payload: Context['payload']): payload is Context<'issue_comment'>['payload'] =>
15-
'comment' in payload && 'issue' in payload;
16+
'comment' in payload && 'issue' in payload && (!('pull_request' in payload) || !payload.pull_request);
17+
18+
export const isIssue = (payload: Context['payload']): payload is Context<'issues'>['payload'] => 'issue' in payload;
1619

1720
export const isPullRequestComment = (payload: Context['payload']): payload is Context<'issue_comment'>['payload'] =>
18-
'comment' in payload && 'pull_request' in payload;
21+
'comment' in payload && 'issue' in payload && 'pull_request' in payload && !!payload.pull_request;
1922

2023
export const isError = (error: unknown): error is Error => error instanceof Error;
2124

‎src/utils/chat-config.util.ts‎

Lines changed: 60 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
import { Context } from 'probot';
22
import { Config } from '~common/types';
3-
import { isDiscussion, isDiscussionComment, isPullRequestReviewComment } from '~helpers';
3+
import { SystemError } from '~errors';
4+
import {
5+
isDiscussion,
6+
isDiscussionComment,
7+
isInlineComment,
8+
isIssue,
9+
isIssueComment,
10+
isPullRequestComment
11+
} from '~helpers';
412

513
type EventfulContext = Context<'issue_comment' | 'discussion_comment' | 'pull_request_review_comment'>;
614

@@ -11,10 +19,12 @@ export class ChatConfig {
1119
): Config['chat']['auto']['discussions' | 'issues' | 'pull-requests'] {
1220
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
1321
return config.chat['auto'].discussions;
14-
} else if (isPullRequestReviewComment(context.payload)) {
22+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
1523
return config.chat['auto']['pull-requests'];
24+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
25+
return config.chat['auto'].issues;
1626
} else {
17-
return context.payload.issue.pull_request ? config.chat['auto']['pull-requests'] : config.chat['auto'].issues;
27+
throw new SystemError('Unhandled case');
1828
}
1929
}
2030

@@ -28,12 +38,12 @@ export class ChatConfig {
2838
): Config['chat']['stop-on']['discussions' | 'issues' | 'pull-requests']['locked'] {
2939
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
3040
return config.chat['stop-on'].discussions.locked;
31-
} else if (isPullRequestReviewComment(context.payload)) {
41+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
3242
return config.chat['stop-on']['pull-requests'].locked;
43+
} else if (isIssueComment(context.payload) || isIssue(context.payload)) {
44+
return config.chat['stop-on'].issues.locked;
3345
} else {
34-
return (
35-
context.payload.issue.pull_request ? config.chat['stop-on']['pull-requests'] : config.chat['stop-on'].issues
36-
).locked;
46+
throw new SystemError('Unhandled case');
3747
}
3848
}
3949

@@ -43,12 +53,12 @@ export class ChatConfig {
4353
): Config['chat']['stop-on']['discussions' | 'issues' | 'pull-requests']['closed'] {
4454
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
4555
return config.chat['stop-on'].discussions.closed;
46-
} else if (isPullRequestReviewComment(context.payload)) {
56+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
4757
return config.chat['stop-on']['pull-requests'].closed;
58+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
59+
return config.chat['stop-on'].issues.locked;
4860
} else {
49-
return (
50-
context.payload.issue.pull_request ? config.chat['stop-on']['pull-requests'] : config.chat['stop-on'].issues
51-
).closed;
61+
throw new SystemError('Unhandled case');
5262
}
5363
}
5464

@@ -58,10 +68,12 @@ export class ChatConfig {
5868
): Config['chat']['allow']['discussions' | 'issues' | 'pull-requests'] {
5969
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
6070
return config.chat.allow.discussions;
61-
} else if (isPullRequestReviewComment(context.payload)) {
71+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
6272
return config.chat.allow['pull-requests'];
73+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
74+
return config.chat.allow.issues;
6375
} else {
64-
return context.payload.issue.pull_request ? config.chat.allow['pull-requests'] : config.chat.allow.issues;
76+
throw new SystemError('Unhandled case');
6577
}
6678
}
6779

@@ -71,28 +83,31 @@ export class ChatConfig {
7183
): Config['chat']['ignore']['discussions' | 'issues' | 'pull-requests']['titles'] {
7284
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
7385
return config.chat.ignore.discussions.titles;
74-
}
75-
76-
if (isPullRequestReviewComment(context.payload)) {
86+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
7787
return config.chat.ignore['pull-requests'].titles;
88+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
89+
return config.chat.ignore.issues.titles;
90+
} else {
91+
throw new SystemError('Unhandled case');
7892
}
79-
80-
return config.chat.ignore.issues.titles;
8193
}
8294

8395
static getChatIgnoreBranch(
8496
context: EventfulContext,
8597
config: Config
8698
): Config['chat']['ignore']['pull-requests']['branches'] {
87-
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
99+
if (
100+
isDiscussionComment(context.payload) ||
101+
isDiscussion(context.payload) ||
102+
isIssue(context.payload) ||
103+
isIssueComment(context.payload)
104+
) {
88105
return [];
89-
}
90-
91-
if (isPullRequestReviewComment(context.payload)) {
106+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
92107
return config.chat.ignore['pull-requests'].branches;
108+
} else {
109+
throw new SystemError('Unhandled case');
93110
}
94-
95-
return context.payload.issue.pull_request ? config.chat.ignore['pull-requests'].branches : [];
96111
}
97112

98113
static getChatIgnoreLabel(
@@ -101,14 +116,13 @@ export class ChatConfig {
101116
): Config['chat']['ignore']['pull-requests' | 'issues']['labels'] {
102117
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
103118
return [];
104-
}
105-
106-
if (isPullRequestReviewComment(context.payload)) {
119+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
107120
return config.chat.ignore['pull-requests'].labels;
121+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
122+
return config.chat.ignore.issues.labels;
123+
} else {
124+
throw new SystemError('Unhandled case');
108125
}
109-
110-
return (context.payload.issue.pull_request ? config.chat.ignore['pull-requests'] : config.chat.ignore.issues)
111-
.labels;
112126
}
113127

114128
static getChatStopOnDraft(
@@ -117,31 +131,30 @@ export class ChatConfig {
117131
): Config['chat']['stop-on']['pull-requests' | 'issues']['draft'] {
118132
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
119133
return false;
120-
} else if (isPullRequestReviewComment(context.payload)) {
134+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
121135
return config.chat['stop-on']['pull-requests'].draft;
136+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
137+
return config.chat['stop-on'].issues.draft;
122138
} else {
123-
const { issue } = context.payload;
124-
125-
return (issue.pull_request ? config.chat['stop-on']['pull-requests'] : config.chat['stop-on'].issues).draft;
139+
throw new SystemError('Unhandled case');
126140
}
127141
}
128142

129143
static getChatStopOnMerged(
130144
context: EventfulContext,
131145
config: Config
132146
): Config['chat']['stop-on']['pull-requests']['merged'] {
133-
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
147+
if (
148+
isDiscussionComment(context.payload) ||
149+
isDiscussion(context.payload) ||
150+
isIssue(context.payload) ||
151+
isIssueComment(context.payload)
152+
) {
134153
return false;
135-
} else if (isPullRequestReviewComment(context.payload)) {
154+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
136155
return config.chat['stop-on']['pull-requests'].merged;
137156
} else {
138-
const { issue } = context.payload;
139-
140-
if (issue.pull_request) {
141-
return config.chat['stop-on']['pull-requests'].merged;
142-
}
143-
144-
return false;
157+
throw new SystemError('Unhandled case');
145158
}
146159
}
147160

@@ -151,16 +164,12 @@ export class ChatConfig {
151164
): Config['chat']['enabled']['discussions' | 'issues' | 'pull-requests'] {
152165
if (isDiscussionComment(context.payload) || isDiscussion(context.payload)) {
153166
return config.chat.enabled.discussions;
154-
} else if (isPullRequestReviewComment(context.payload)) {
167+
} else if (isInlineComment(context.payload) || isPullRequestComment(context.payload)) {
155168
return config.chat.enabled['pull-requests'];
156-
} else {
157-
const { issue } = context.payload;
158-
159-
if (issue.pull_request) {
160-
return config.chat.enabled['pull-requests'];
161-
}
162-
169+
} else if (isIssue(context.payload) || isIssueComment(context.payload)) {
163170
return config.chat.enabled.issues;
171+
} else {
172+
throw new SystemError('Unhandled case');
164173
}
165174
}
166175
}

0 commit comments

Comments
 (0)