Skip to content

Commit d463236

Browse files
Add scope validation to mention decorator
1 parent 86ba7b2 commit d463236

File tree

4 files changed

+291
-0
lines changed

4 files changed

+291
-0
lines changed

src/django_github_app/commands.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,25 @@ def check_event_for_mention(
8989
return True
9090

9191
return any(mention.command == command.lower() for mention in mentions)
92+
93+
94+
def check_event_scope(
95+
event_type: str, event_data: dict[str, Any], scope: CommandScope | None
96+
) -> bool:
97+
if scope is None:
98+
return True
99+
100+
# For issue_comment events, we need to distinguish between issues and PRs
101+
if event_type == "issue_comment":
102+
issue = event_data.get("issue", {})
103+
is_pull_request = "pull_request" in issue and issue["pull_request"] is not None
104+
105+
# If scope is ISSUE, we only want actual issues (not PRs)
106+
if scope == CommandScope.ISSUE:
107+
return not is_pull_request
108+
# If scope is PR, we only want pull requests
109+
elif scope == CommandScope.PR:
110+
return is_pull_request
111+
112+
scope_events = scope.get_events()
113+
return any(event_action.event == event_type for event_action in scope_events)

src/django_github_app/routing.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from ._typing import override
1717
from .commands import CommandScope
1818
from .commands import check_event_for_mention
19+
from .commands import check_event_scope
1920

2021
AsyncCallback = Callable[..., Awaitable[None]]
2122
SyncCallback = Callable[..., None]
@@ -83,6 +84,10 @@ async def async_wrapper(
8384
if not check_event_for_mention(event.data, command, username):
8485
return
8586

87+
# Check if the event matches the specified scope
88+
if not check_event_scope(event.event, event.data, scope):
89+
return
90+
8691
# TODO: Check permissions
8792
# For now, just call through
8893
await func(event, *args, **wrapper_kwargs) # type: ignore[func-returns-value]
@@ -97,6 +102,10 @@ def sync_wrapper(
97102
if not check_event_for_mention(event.data, command, username):
98103
return
99104

105+
# Check if the event matches the specified scope
106+
if not check_event_scope(event.event, event.data, scope):
107+
return
108+
100109
# TODO: Check permissions
101110
# For now, just call through
102111
func(event, *args, **wrapper_kwargs)

tests/test_commands.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
from __future__ import annotations
22

3+
from django_github_app.commands import CommandScope
34
from django_github_app.commands import check_event_for_mention
5+
from django_github_app.commands import check_event_scope
46
from django_github_app.commands import parse_mentions
57

68

@@ -194,3 +196,87 @@ def test_multiple_mentions(self):
194196
assert check_event_for_mention(event, "help", "bot") is True
195197
assert check_event_for_mention(event, "deploy", "bot") is True
196198
assert check_event_for_mention(event, "test", "bot") is False
199+
200+
201+
class TestCheckEventScope:
202+
def test_no_scope_allows_all_events(self):
203+
# When no scope is specified, all events should pass
204+
assert check_event_scope("issue_comment", {"issue": {}}, None) is True
205+
assert check_event_scope("pull_request_review_comment", {}, None) is True
206+
assert check_event_scope("commit_comment", {}, None) is True
207+
208+
def test_issue_scope_on_issue_comment(self):
209+
# Issue comment on an actual issue (no pull_request field)
210+
issue_event = {"issue": {"title": "Bug report"}}
211+
assert (
212+
check_event_scope("issue_comment", issue_event, CommandScope.ISSUE) is True
213+
)
214+
215+
# Issue comment on a pull request (has pull_request field)
216+
pr_event = {"issue": {"title": "PR title", "pull_request": {"url": "..."}}}
217+
assert check_event_scope("issue_comment", pr_event, CommandScope.ISSUE) is False
218+
219+
def test_pr_scope_on_issue_comment(self):
220+
# Issue comment on an actual issue (no pull_request field)
221+
issue_event = {"issue": {"title": "Bug report"}}
222+
assert check_event_scope("issue_comment", issue_event, CommandScope.PR) is False
223+
224+
# Issue comment on a pull request (has pull_request field)
225+
pr_event = {"issue": {"title": "PR title", "pull_request": {"url": "..."}}}
226+
assert check_event_scope("issue_comment", pr_event, CommandScope.PR) is True
227+
228+
def test_pr_scope_allows_pr_specific_events(self):
229+
# PR scope should allow pull_request_review_comment
230+
assert (
231+
check_event_scope("pull_request_review_comment", {}, CommandScope.PR)
232+
is True
233+
)
234+
235+
# PR scope should allow pull_request_review
236+
assert check_event_scope("pull_request_review", {}, CommandScope.PR) is True
237+
238+
# PR scope should not allow commit_comment
239+
assert check_event_scope("commit_comment", {}, CommandScope.PR) is False
240+
241+
def test_commit_scope_allows_commit_comment_only(self):
242+
# Commit scope should allow commit_comment
243+
assert check_event_scope("commit_comment", {}, CommandScope.COMMIT) is True
244+
245+
# Commit scope should not allow issue_comment
246+
assert (
247+
check_event_scope("issue_comment", {"issue": {}}, CommandScope.COMMIT)
248+
is False
249+
)
250+
251+
# Commit scope should not allow PR events
252+
assert (
253+
check_event_scope("pull_request_review_comment", {}, CommandScope.COMMIT)
254+
is False
255+
)
256+
257+
def test_issue_scope_disallows_non_issue_events(self):
258+
# Issue scope should not allow pull_request_review_comment
259+
assert (
260+
check_event_scope("pull_request_review_comment", {}, CommandScope.ISSUE)
261+
is False
262+
)
263+
264+
# Issue scope should not allow commit_comment
265+
assert check_event_scope("commit_comment", {}, CommandScope.ISSUE) is False
266+
267+
def test_pull_request_field_none_treated_as_issue(self):
268+
# If pull_request field exists but is None, treat as issue
269+
event_with_none_pr = {"issue": {"title": "Issue", "pull_request": None}}
270+
assert (
271+
check_event_scope("issue_comment", event_with_none_pr, CommandScope.ISSUE)
272+
is True
273+
)
274+
assert (
275+
check_event_scope("issue_comment", event_with_none_pr, CommandScope.PR)
276+
is False
277+
)
278+
279+
def test_missing_issue_data(self):
280+
# If issue data is missing entirely, default behavior
281+
assert check_event_scope("issue_comment", {}, CommandScope.ISSUE) is True
282+
assert check_event_scope("issue_comment", {}, CommandScope.PR) is False

tests/test_routing.py

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,3 +273,177 @@ def sync_handler(event, *args, **kwargs):
273273
test_router.dispatch(event, None)
274274

275275
assert handler_called
276+
277+
def test_scope_validation_issue_comment_on_issue(self, test_router):
278+
"""Test that ISSUE scope works for actual issues."""
279+
handler_called = False
280+
281+
@test_router.mention(command="issue-only", scope=CommandScope.ISSUE)
282+
def issue_handler(event, *args, **kwargs):
283+
nonlocal handler_called
284+
handler_called = True
285+
286+
# Issue comment on an actual issue (no pull_request field)
287+
event = sansio.Event(
288+
{
289+
"action": "created",
290+
"issue": {"title": "Bug report", "number": 123},
291+
"comment": {"body": "@bot issue-only"},
292+
},
293+
event="issue_comment",
294+
delivery_id="123",
295+
)
296+
test_router.dispatch(event, None)
297+
298+
assert handler_called
299+
300+
def test_scope_validation_issue_comment_on_pr(self, test_router):
301+
"""Test that ISSUE scope rejects PR comments."""
302+
handler_called = False
303+
304+
@test_router.mention(command="issue-only", scope=CommandScope.ISSUE)
305+
def issue_handler(event, *args, **kwargs):
306+
nonlocal handler_called
307+
handler_called = True
308+
309+
# Issue comment on a pull request (has pull_request field)
310+
event = sansio.Event(
311+
{
312+
"action": "created",
313+
"issue": {
314+
"title": "PR title",
315+
"number": 456,
316+
"pull_request": {"url": "https://api.github.com/..."},
317+
},
318+
"comment": {"body": "@bot issue-only"},
319+
},
320+
event="issue_comment",
321+
delivery_id="123",
322+
)
323+
test_router.dispatch(event, None)
324+
325+
assert not handler_called
326+
327+
def test_scope_validation_pr_scope_on_pr(self, test_router):
328+
"""Test that PR scope works for pull requests."""
329+
handler_called = False
330+
331+
@test_router.mention(command="pr-only", scope=CommandScope.PR)
332+
def pr_handler(event, *args, **kwargs):
333+
nonlocal handler_called
334+
handler_called = True
335+
336+
# Issue comment on a pull request
337+
event = sansio.Event(
338+
{
339+
"action": "created",
340+
"issue": {
341+
"title": "PR title",
342+
"number": 456,
343+
"pull_request": {"url": "https://api.github.com/..."},
344+
},
345+
"comment": {"body": "@bot pr-only"},
346+
},
347+
event="issue_comment",
348+
delivery_id="123",
349+
)
350+
test_router.dispatch(event, None)
351+
352+
assert handler_called
353+
354+
def test_scope_validation_pr_scope_on_issue(self, test_router):
355+
"""Test that PR scope rejects issue comments."""
356+
handler_called = False
357+
358+
@test_router.mention(command="pr-only", scope=CommandScope.PR)
359+
def pr_handler(event, *args, **kwargs):
360+
nonlocal handler_called
361+
handler_called = True
362+
363+
# Issue comment on an actual issue
364+
event = sansio.Event(
365+
{
366+
"action": "created",
367+
"issue": {"title": "Bug report", "number": 123},
368+
"comment": {"body": "@bot pr-only"},
369+
},
370+
event="issue_comment",
371+
delivery_id="123",
372+
)
373+
test_router.dispatch(event, None)
374+
375+
assert not handler_called
376+
377+
def test_scope_validation_commit_scope(self, test_router):
378+
"""Test that COMMIT scope works for commit comments."""
379+
handler_called = False
380+
381+
@test_router.mention(command="commit-only", scope=CommandScope.COMMIT)
382+
def commit_handler(event, *args, **kwargs):
383+
nonlocal handler_called
384+
handler_called = True
385+
386+
# Commit comment event
387+
event = sansio.Event(
388+
{
389+
"action": "created",
390+
"comment": {"body": "@bot commit-only"},
391+
"commit": {"sha": "abc123"},
392+
},
393+
event="commit_comment",
394+
delivery_id="123",
395+
)
396+
test_router.dispatch(event, None)
397+
398+
assert handler_called
399+
400+
def test_scope_validation_no_scope(self, test_router):
401+
"""Test that no scope allows all comment types."""
402+
call_count = 0
403+
404+
@test_router.mention(command="all-contexts")
405+
def all_handler(event, *args, **kwargs):
406+
nonlocal call_count
407+
call_count += 1
408+
409+
# Test on issue
410+
event = sansio.Event(
411+
{
412+
"action": "created",
413+
"issue": {"title": "Issue", "number": 1},
414+
"comment": {"body": "@bot all-contexts"},
415+
},
416+
event="issue_comment",
417+
delivery_id="123",
418+
)
419+
test_router.dispatch(event, None)
420+
421+
# Test on PR
422+
event = sansio.Event(
423+
{
424+
"action": "created",
425+
"issue": {
426+
"title": "PR",
427+
"number": 2,
428+
"pull_request": {"url": "..."},
429+
},
430+
"comment": {"body": "@bot all-contexts"},
431+
},
432+
event="issue_comment",
433+
delivery_id="124",
434+
)
435+
test_router.dispatch(event, None)
436+
437+
# Test on commit
438+
event = sansio.Event(
439+
{
440+
"action": "created",
441+
"comment": {"body": "@bot all-contexts"},
442+
"commit": {"sha": "abc123"},
443+
},
444+
event="commit_comment",
445+
delivery_id="125",
446+
)
447+
test_router.dispatch(event, None)
448+
449+
assert call_count == 3

0 commit comments

Comments
 (0)