diff --git a/frontend/src/components/issues/GitHubIssueState.module.css b/frontend/src/components/issues/GitHubIssueState.module.css index cd2a56d51..e218a77bd 100644 --- a/frontend/src/components/issues/GitHubIssueState.module.css +++ b/frontend/src/components/issues/GitHubIssueState.module.css @@ -9,3 +9,6 @@ .github-issue-not-planned { color: var(--fgColor-muted); } +.github-pull-request-closed { + color: var(--fgColor-closed); +} diff --git a/frontend/src/components/issues/GitHubPullRequestState.tsx b/frontend/src/components/issues/GitHubPullRequestState.tsx new file mode 100644 index 000000000..77f337eb8 --- /dev/null +++ b/frontend/src/components/issues/GitHubPullRequestState.tsx @@ -0,0 +1,43 @@ +import { GitMergeIcon, GitPullRequestClosedIcon, GitPullRequestIcon } from "lucide-preact"; +import type { PullRequestStateEnum } from "@/api/generated/models"; +import styles from "./GitHubIssueState.module.css"; + +type Props = { + state: readonly PullRequestStateEnum[]; +}; + +function gitHubPullRequestStateLabel(state: readonly PullRequestStateEnum[]): string { + if (!state.includes("CLOSED")) { + return "Open"; + } + if (state.includes("MERGED")) { + return "Merged"; + } + return "Closed"; +} + +function GitHubPullRequestStateIcon({ state }: Props) { + if (!state.includes("CLOSED")) { + return ; + } + if (state.includes("MERGED")) { + return ; + } + return ; +} + +export function GitHubPullRequestState({ state }: Props) { + const label = gitHubPullRequestStateLabel(state); + return ( + + + + ); +} diff --git a/frontend/src/components/issues/Issue.tsx b/frontend/src/components/issues/Issue.tsx index 9ec1ae34b..958278335 100644 --- a/frontend/src/components/issues/Issue.tsx +++ b/frontend/src/components/issues/Issue.tsx @@ -13,8 +13,13 @@ import { issuePath } from "@/routes/paths"; import { ExternalLink } from "../ui/ExternalLink"; import { LegendCard } from "../ui/LegendCard"; import { GitHubIssueState } from "./GitHubIssueState"; +import { GitHubPullRequestState } from "./GitHubPullRequestState"; import { IssueViewToggle } from "./IssueViewToggle"; +function gitHubNumber(url: string): string { + return `#${url.split("/").pop()}`; +} + type Props = { issue: IssueType; inheritedViewMode?: IssueViewMode; @@ -28,7 +33,7 @@ export function Issue({ allowViewModeClear = false, inheritedSuggestionViewMode = DEFAULT_SUGGESTION_VIEW_MODE, }: Props) { - const { code, title, github_issue_url, github_issue_state } = issue; + const { code, title, github_issue_url, github_issue_state, github_closing_pull_requests } = issue; const [ownViewMode, setOwnViewMode] = useState(undefined); const [ownSuggestionViewMode, setOwnSuggestionViewMode] = useState< @@ -46,15 +51,6 @@ export function Issue({ {code} - {github_issue_url && ( - /* FIXME(@fricklerhandwerk): Move the GitHub issue state into the card once the issue activity log lands, so the legend carries no variable data. */ -
- {github_issue_state && ( - - )} - GitHub -
- )} ); @@ -72,32 +68,72 @@ export function Issue({ return ( -
- {viewMode === "collapsed" && title && ( - {title} +
+ {viewMode !== "collapsed" && github_issue_url && ( +
+ GitHub: + + {github_issue_state && ( + + )} + {gitHubNumber(github_issue_url)} + + {github_closing_pull_requests.length > 0 && closed by} + {github_closing_pull_requests.map((pullRequest) => ( + + + {gitHubNumber(pullRequest.url)} + + ))} +
)} - {viewMode !== "collapsed" && title && ( -
- {title} + {viewMode === "collapsed" && ( +
+
+ {github_issue_state && ( + + )} + {github_issue_url && ( + + {gitHubNumber(github_issue_url)} + + )} +
+ {title && {title}} +
+ )} + {viewMode === "expanded" && ( +
+ {title && ( +
+ {title} +
+ )} + {issue.suggestions.length > 1 && ( +
+ + +
+ )}
)}
{viewMode === "expanded" && issue.suggestions.length > 0 && (
- {issue.suggestions.length > 1 && ( -
- - -
- )} {issue.suggestions.map((suggestion) => typeof suggestion === "number" ? null : ( list[str]: + """ + Names of all flags occurring in valid event combinations for the given item type. + """ + valid = [EventType(v) for v in range(1, 2 * max(EventType)) if EventType.valid(v)] + return [ + cast(str, flag.name) + for flag in EventType + if any(flag in v for v in valid if item_type in v) + ] + + +class GitHubPullRequestSerializer(serializers.ModelSerializer): + """A GitHub pull request and its current state.""" + + url = serializers.URLField(read_only=True) + pull_request_state = serializers.SerializerMethodField() + + class Meta: + model = GitHubItem + fields = ["url", "pull_request_state"] + + @extend_schema_field( + serializers.ListField( + child=serializers.ChoiceField(choices=state_names(EventType.PULL_REQUEST)) + ) + ) + def get_pull_request_state(self, obj: GitHubItem) -> list[str]: + return [cast(str, flag.name) for flag in obj.state] class IssueSerializer(serializers.ModelSerializer): @@ -14,6 +45,7 @@ class IssueSerializer(serializers.ModelSerializer): github_issue_url = serializers.SerializerMethodField() github_issue_state = serializers.SerializerMethodField() + github_closing_pull_requests = serializers.SerializerMethodField() suggestions = serializers.SerializerMethodField() class Meta: @@ -27,6 +59,7 @@ class Meta: "updated_at", "github_issue_url", "github_issue_state", + "github_closing_pull_requests", "suggestions", ] # `title`/`status` have model defaults, but always present on read. @@ -42,7 +75,7 @@ def get_github_issue_url(self, obj: NixpkgsIssue) -> str | None: @extend_schema_field( serializers.ListField( - child=serializers.ChoiceField(choices=[flag.name for flag in EventType]), + child=serializers.ChoiceField(choices=state_names(EventType.ISSUE)), allow_null=True, ) ) @@ -50,6 +83,22 @@ def get_github_issue_state(self, obj: NixpkgsIssue) -> list[str] | None: item = obj.github_issue return [cast(str, flag.name) for flag in item.state] if item else None + @extend_schema_field(GitHubPullRequestSerializer(many=True)) + def get_github_closing_pull_requests(self, obj: NixpkgsIssue) -> ReturnList: + item = obj.github_issue + pull_requests = ( + sorted( + (reference.pull_request for reference in item.closed_by.all()), + key=lambda pull_request: pull_request.number, + ) + if item + else [] + ) + return cast( + ReturnList, + GitHubPullRequestSerializer(pull_requests, many=True).data, + ) + @extend_schema_field( { "type": "array", diff --git a/src/api/issues/views.py b/src/api/issues/views.py index 5a135c66d..dfaec530f 100644 --- a/src/api/issues/views.py +++ b/src/api/issues/views.py @@ -42,6 +42,7 @@ def get_queryset(self) -> QuerySet[NixpkgsIssue]: # pyright: ignore[reportIncom "suggestions__cached", "suggestions__cve__container__references__tags", "github_items__events", + "github_items__closed_by__pull_request__events", ) .order_by("-created_at") .distinct() diff --git a/src/api/tests/test_issues.py b/src/api/tests/test_issues.py index 801e52668..0d6e64741 100644 --- a/src/api/tests/test_issues.py +++ b/src/api/tests/test_issues.py @@ -174,9 +174,41 @@ def test_issue_detail_github_issue_state( assert response.data["github_issue_state"] == ["ISSUE", "CLOSED", "COMPLETED"] +def test_issue_detail_github_closing_pull_requests( + make_issue: Callable[..., NixpkgsIssue], + make_github_item: Callable[..., GitHubItem], +) -> None: + client = APIClient() + issue = make_issue() + github_issue = make_github_item( + 1, + GitHubItem.Kind.ISSUE, + EventType.ISSUE | EventType.OPENED, + EventType.ISSUE | EventType.CLOSED | EventType.COMPLETED, + ) + issue.github_items.add(github_issue) + pull_request = make_github_item( + 2, + GitHubItem.Kind.PULL_REQUEST, + EventType.PULL_REQUEST | EventType.OPENED, + EventType.PULL_REQUEST | EventType.CLOSED | EventType.MERGED, + ) + pull_request.closes.create(issue=github_issue) + + response = client.get(detail_url(issue.code)) + assert response.status_code == 200 + assert response.data["github_closing_pull_requests"] == [ + { + "url": "https://github.com/NixOS/nixpkgs/pull/2", + "pull_request_state": ["PULL_REQUEST", "CLOSED", "MERGED"], + } + ] + + def test_list_issues_query_count_independent_of_issue_count( make_container: Callable[..., Container], make_issue: Callable[..., NixpkgsIssue], + make_github_item: Callable[..., GitHubItem], ) -> None: client = APIClient() @@ -192,6 +224,11 @@ def add_closed_issue(number: int) -> None: event_type=EventType.ISSUE | EventType.CLOSED | EventType.COMPLETED, occurred_at=timezone.now(), ) + make_github_item( + 100 + number, + GitHubItem.Kind.PULL_REQUEST, + EventType.PULL_REQUEST | EventType.OPENED, + ).closes.create(issue=item) def count_queries() -> int: with CaptureQueriesContext(connection) as queries: diff --git a/src/shared/github_events.py b/src/shared/github_events.py index 6b5acc49b..97fe65a4e 100644 --- a/src/shared/github_events.py +++ b/src/shared/github_events.py @@ -8,6 +8,7 @@ from collections.abc import Iterable, Iterator from functools import cache +from typing import Any import github_graphql as schema from django.utils.functional import classproperty @@ -16,6 +17,120 @@ from shared.models.issue import EventType, GitHubEvent +class TruncatedError(Exception): + def __init__(self, number: int, what: str) -> None: + super().__init__(f"{what} of #{number}") + self.number = number + self.what = what + + +def nodes(page: Any, number: int, what: str) -> list[Any]: + # FIXME(@fricklerhandwerk): Fetch the remaining pages here instead of giving up, once lists that long occur. + if page.page_info.has_next_page: + raise TruncatedError(number, what) + return page.nodes or [] + + +class PullRequest: + @classproperty + @cache + def closed(cls) -> Fragment: + fragment = Fragment(schema.ClosedEvent, "PullRequestClosed") + fragment.created_at() + return fragment + + @classproperty + @cache + def reopened(cls) -> Fragment: + fragment = Fragment(schema.ReopenedEvent, "PullRequestReopened") + fragment.created_at() + return fragment + + @classproperty + @cache + def merged(cls) -> Fragment: + fragment = Fragment(schema.MergedEvent, "PullRequestMerged") + fragment.created_at() + return fragment + + @classproperty + @cache + def fragment(cls) -> Fragment: + fragment = Fragment(schema.PullRequest, "PullRequestHistory") + fragment.number() + fragment.created_at() + fragment.base_repository.name_with_owner() + timeline = fragment.timeline_items( + first=20, + item_types=["CLOSED_EVENT", "REOPENED_EVENT", "MERGED_EVENT"], + ) + timeline.page_info.has_next_page() + nodes = timeline.nodes + nodes.__fragment__(cls.closed) + nodes.__fragment__(cls.reopened) + nodes.__fragment__(cls.merged) + return fragment + + def __init__(self, data: schema.PullRequest) -> None: + self.data = data + + @property + def number(self) -> int: + return self.data.number + + @property + def repository(self) -> str | None: + if repository := self.data.base_repository: + return repository.name_with_owner + return None + + def events(self) -> Iterator[GitHubEvent]: + yield GitHubEvent( + event_type=EventType.PULL_REQUEST | EventType.OPENED, + occurred_at=self.data.created_at, + ) + timeline = nodes(self.data.timeline_items, self.number, "timeline") + # GitHub records a merge as both a merge and a close, the close at the same moment or shortly after. + # Recording both would leave two latest events in unknown order, and a merged pull request may end up showing as merely closed. + # We therefore filter those cases. + # A merged pull request can't be reopened, so the merge's close is its last one. + merge_close = ( + next( + ( + node + for node in reversed(timeline) + if isinstance(node, schema.ClosedEvent) + ), + None, + ) + if any(isinstance(node, schema.MergedEvent) for node in timeline) + else None + ) + for node in timeline: + match node: + case schema.MergedEvent(): + yield GitHubEvent( + event_type=EventType.PULL_REQUEST + | EventType.CLOSED + | EventType.MERGED, + occurred_at=node.created_at, + ) + case schema.ClosedEvent() if node is merge_close: + continue + case schema.ClosedEvent(): + yield GitHubEvent( + event_type=EventType.PULL_REQUEST | EventType.CLOSED, + occurred_at=node.created_at, + ) + case schema.ReopenedEvent(): + yield GitHubEvent( + event_type=EventType.PULL_REQUEST | EventType.OPENED, + occurred_at=node.created_at, + ) + case _: + raise ValueError(f"Unexpected timeline item: {type(node).__name__}") + + class Issue: @classproperty @cache @@ -38,12 +153,19 @@ def fragment(cls) -> Fragment: fragment = Fragment(schema.Issue, "IssueHistory") fragment.number() fragment.created_at() - nodes = fragment.timeline_items( - first=100, + timeline = fragment.timeline_items( + first=20, item_types=["CLOSED_EVENT", "REOPENED_EVENT"], - ).nodes + ) + timeline.page_info.has_next_page() + nodes = timeline.nodes nodes.__fragment__(cls.closed) nodes.__fragment__(cls.reopened) + closing = fragment.closed_by_pull_requests_references( + first=10, include_closed_prs=True + ) + closing.page_info.has_next_page() + closing.nodes.__fragment__(PullRequest.fragment) return fragment def __init__(self, data: schema.Issue) -> None: @@ -58,7 +180,7 @@ def events(self) -> Iterator[GitHubEvent]: event_type=EventType.ISSUE | EventType.OPENED, occurred_at=self.data.created_at, ) - for node in self.data.timeline_items.nodes or []: + for node in nodes(self.data.timeline_items, self.number, "timeline"): match node: case schema.ClosedEvent(): match node.state_reason: @@ -84,6 +206,14 @@ def events(self) -> Iterator[GitHubEvent]: case _: raise ValueError(f"Unexpected timeline item: {type(node).__name__}") + def closing_pull_requests(self) -> Iterator[PullRequest]: + for node in nodes( + self.data.closed_by_pull_requests_references, + self.number, + "closing pull requests", + ): + yield PullRequest(node) + class Repository: @staticmethod diff --git a/src/shared/ingest_github_events.py b/src/shared/ingest_github_events.py index 90e574e26..0a12ada2a 100644 --- a/src/shared/ingest_github_events.py +++ b/src/shared/ingest_github_events.py @@ -10,8 +10,13 @@ from github import Github, GithubException from sgqlc.operation import Operation -from shared.github_events import Repository -from shared.models.issue import EventType, GitHubItem, GitHubSyncError +from shared.github_events import Repository, TruncatedError +from shared.models.issue import ( + EventType, + GitHubClosingReference, + GitHubItem, + GitHubSyncError, +) logger = logging.getLogger(__name__) @@ -29,6 +34,7 @@ def sync_github_events(github: Github) -> None: ) .distinct() .order_by("owner", "repository", "number") + .prefetch_related("nixpkgs_issues") ) repositories = items.order_by().values("owner", "repository").distinct().count() logger.info( @@ -78,7 +84,23 @@ def sync_github_events(github: Github) -> None: found = [] events = 0 for issue in Repository(data.repository).issues(batch): - issue_events = list(issue.events()) + item = by_number[issue.number] + try: + issue_events = list(issue.events()) + closing_pull_requests = [ + (closing_pr, list(closing_pr.events())) + for closing_pr in issue.closing_pull_requests() + if closing_pr.repository == f"{owner}/{name}" + ] + except TruncatedError as e: + message = f"Query result has more items than could be fetched in one page: {e.what} of {owner}/{name}#{e.number}" + logger.error(f"Skipping {owner}/{name}#{issue.number}: {message}") + GitHubSyncError.objects.update_or_create( + item=item, + error_type="TRUNCATED", + defaults={"message": message}, + ) + continue logger.debug( f"{owner}/{name}#{issue.number}: " + ( @@ -89,9 +111,29 @@ def sync_github_events(github: Github) -> None: or "no events" ) ) - by_number[issue.number].add_events(issue_events) + item.add_events(issue_events) events += len(issue_events) - found.append(by_number[issue.number]) + pull_requests = [] + for closing_pr, pr_events in closing_pull_requests: + pull_request, _ = GitHubItem.objects.get_or_create( + owner=owner, + repository=name, + number=closing_pr.number, + defaults={"kind": GitHubItem.Kind.PULL_REQUEST}, + ) + pull_request.add_events(pr_events) + pull_requests.append(pull_request) + GitHubClosingReference.objects.filter(issue=item).exclude( + pull_request__in=pull_requests + ).delete() + GitHubClosingReference.objects.bulk_create( + [ + GitHubClosingReference(pull_request=pull_request, issue=item) + for pull_request in pull_requests + ], + ignore_conflicts=True, + ) + found.append(item) GitHubSyncError.objects.filter(item__in=found).delete() logger.info( f"{owner}/{name}: {len(found)} of {len(batch)} issues synced, {len(batch) - len(found)} failed, {events} events fetched" diff --git a/src/shared/migrations/0111_github_closing_references.py b/src/shared/migrations/0111_github_closing_references.py new file mode 100644 index 000000000..19cf17cf2 --- /dev/null +++ b/src/shared/migrations/0111_github_closing_references.py @@ -0,0 +1,25 @@ +# Generated by Django 5.2.17 on 2026-10-21 22:23 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('shared', '0110_githubsyncerror'), + ] + + operations = [ + migrations.CreateModel( + name='GitHubClosingReference', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('issue', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='closed_by', to='shared.githubitem')), + ('pull_request', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='closes', to='shared.githubitem')), + ], + options={ + 'constraints': [models.UniqueConstraint(fields=('pull_request', 'issue'), name='githubclosingreference_unique')], + }, + ), + ] diff --git a/src/shared/models/issue.py b/src/shared/models/issue.py index 9a97c9c67..19b9d6c40 100644 --- a/src/shared/models/issue.py +++ b/src/shared/models/issue.py @@ -66,7 +66,10 @@ def url(self) -> str: def state(self) -> "EventType": latest = max(self.events.all(), key=lambda e: e.occurred_at, default=None) if latest is None: - return EventType.ISSUE | EventType.OPENED + return { + self.Kind.ISSUE: EventType.ISSUE, + self.Kind.PULL_REQUEST: EventType.PULL_REQUEST, + }[self.Kind(self.kind)] | EventType.OPENED return EventType(latest.event_type) def add_events(self, events: Iterable["GitHubEvent"]) -> None: @@ -279,6 +282,23 @@ class Meta: # type: ignore[override] ] +class GitHubClosingReference(models.Model): + pull_request = models.ForeignKey( + GitHubItem, on_delete=models.CASCADE, related_name="closes" + ) + issue = models.ForeignKey( + GitHubItem, on_delete=models.CASCADE, related_name="closed_by" + ) + + class Meta: # type: ignore[override] + constraints = [ + models.UniqueConstraint( + fields=["pull_request", "issue"], + name="githubclosingreference_unique", + ), + ] + + class GitHubSyncError(models.Model): item = models.ForeignKey( GitHubItem, on_delete=models.CASCADE, related_name="sync_errors" diff --git a/src/shared/tests/conftest.py b/src/shared/tests/conftest.py index 2cb24458b..dd2ef148e 100644 --- a/src/shared/tests/conftest.py +++ b/src/shared/tests/conftest.py @@ -26,7 +26,7 @@ Tag, Version, ) -from shared.models.issue import NixpkgsIssue +from shared.models.issue import EventType, GitHubItem, NixpkgsIssue from shared.models.linkage import ( CVEDerivationClusterProposal, DerivationClusterProposalLink, @@ -477,6 +477,30 @@ def issue(make_issue: Callable[..., NixpkgsIssue]) -> NixpkgsIssue: return make_issue() +@pytest.fixture +def make_github_item(db: None) -> Callable[..., GitHubItem]: + def wrapped( + number: int, + kind: GitHubItem.Kind, + *events: EventType, + ) -> GitHubItem: + item = GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=number, + kind=kind, + ) + now = timezone.now() + for age, event_type in enumerate(reversed(events)): + item.events.create( + event_type=event_type, + occurred_at=now - timedelta(days=age), + ) + return item + + return wrapped + + @pytest.fixture def make_user( django_user_model: type[User], diff --git a/src/shared/tests/fake_github.py b/src/shared/tests/fake_github.py index 685aa89ab..6e0292216 100644 --- a/src/shared/tests/fake_github.py +++ b/src/shared/tests/fake_github.py @@ -4,7 +4,21 @@ from importlib.resources import files from typing import Any, Literal -from graphql import GraphQLError, GraphQLResolveInfo, build_schema, graphql_sync +from graphql import ( + DocumentNode, + FieldNode, + FragmentDefinitionNode, + FragmentSpreadNode, + GraphQLError, + GraphQLResolveInfo, + InlineFragmentNode, + IntValueNode, + OperationDefinitionNode, + SelectionSetNode, + build_schema, + graphql_sync, + parse, +) from graphql.pyutils import camel_to_snake from requests import PreparedRequest @@ -15,6 +29,54 @@ # PyGithub's connection fills in the default port, so requests carry it explicitly. GRAPHQL_URL = "https://api.github.com:443/graphql" +MAX_NODES = 500_000 + + +def node_count(document: DocumentNode) -> int: + """ + Count the nodes a query may return, the way GitHub does to enforce its limit. + """ + fragments = { + definition.name.value: definition + for definition in document.definitions + if isinstance(definition, FragmentDefinitionNode) + } + + def count(selection_set: SelectionSetNode | None, multiplier: int) -> int: + if selection_set is None: + return 0 + total = 0 + for selection in selection_set.selections: + if isinstance(selection, FragmentSpreadNode): + total += count( + fragments[selection.name.value].selection_set, multiplier + ) + elif isinstance(selection, InlineFragmentNode): + total += count(selection.selection_set, multiplier) + elif isinstance(selection, FieldNode): + size = next( + ( + int(argument.value.value) + for argument in selection.arguments or [] + if argument.name.value in ("first", "last") + and isinstance(argument.value, IntValueNode) + ), + None, + ) + if size is None: + total += count(selection.selection_set, multiplier) + else: + total += multiplier * size + count( + selection.selection_set, multiplier * size + ) + return total + + return sum( + count(definition.selection_set, 1) + for definition in document.definitions + if isinstance(definition, OperationDefinitionNode) + ) + def page( nodes: list[Any], @@ -48,11 +110,34 @@ class ReopenedEvent: @dataclass -class Issue: +class MergedEvent: + created_at: datetime + + +def timeline_page( + timeline: list[Any], item_types: list[str] | None, **args: Any +) -> dict[str, Any]: + # GitHub names the filter value for each item type by converting the type name, e.g. `CLOSED_EVENT` for `ClosedEvent`. + nodes = [ + node + for node in timeline + if item_types is None + or camel_to_snake(type(node).__name__).upper() in item_types + ] + return page(nodes, **args) + + +@dataclass +class Item: repository: str number: int created_at: datetime + + +@dataclass +class Issue(Item): timeline: list[ClosedEvent | ReopenedEvent] = field(default_factory=list) + closed_by: list["PullRequest"] = field(default_factory=list) def timeline_items( self, @@ -60,14 +145,31 @@ def timeline_items( item_types: list[str] | None = None, **args: Any, ) -> dict[str, Any]: - # GitHub names the filter value for each item type by converting the type name, e.g. `CLOSED_EVENT` for `ClosedEvent`. - nodes = [ - node - for node in self.timeline - if item_types is None - or camel_to_snake(type(node).__name__).upper() in item_types - ] - return page(nodes, **args) + return timeline_page(self.timeline, item_types, **args) + + def closed_by_pull_requests_references( + self, _info: GraphQLResolveInfo, **args: Any + ) -> dict[str, Any]: + return page(self.closed_by, **args) + + +@dataclass +class PullRequest(Item): + timeline: list[ClosedEvent | ReopenedEvent | MergedEvent] = field( + default_factory=list + ) + + def timeline_items( + self, + _info: GraphQLResolveInfo, + item_types: list[str] | None = None, + **args: Any, + ) -> dict[str, Any]: + return timeline_page(self.timeline, item_types, **args) + + @property + def base_repository(self) -> dict[str, Any]: + return {"nameWithOwner": self.repository} class NotFoundError(GraphQLError): @@ -77,14 +179,21 @@ def __init__(self, message: str) -> None: @dataclass class Repository: - items: dict[int, Issue] + items: dict[int, Issue | PullRequest] def issue(self, _info: GraphQLResolveInfo, number: int) -> Issue: - if number not in self.items: + item = self.items.get(number) + if not isinstance(item, Issue): raise NotFoundError( f"Could not resolve to an Issue with the number of {number}." ) - return self.items[number] + return item + + def pull_request( + self, _info: GraphQLResolveInfo, number: int + ) -> PullRequest | None: + item = self.items.get(number) + return item if isinstance(item, PullRequest) else None def resolve_field(source: Any, info: GraphQLResolveInfo, **args: Any) -> Any: @@ -101,7 +210,7 @@ def resolve_field(source: Any, info: GraphQLResolveInfo, **args: Any) -> Any: class FakeGitHub: def __init__(self) -> None: - self.items: list[Issue] = [] + self.items: list[Issue | PullRequest] = [] def repository( self, _info: GraphQLResolveInfo, owner: str, name: str, **_: Any @@ -127,6 +236,16 @@ def rate_limit(self, _info: GraphQLResolveInfo, **_: Any) -> dict[str, Any]: def handle(self, request: PreparedRequest) -> tuple[int, dict[str, str], str]: assert isinstance(request.body, str | bytes) body = json.loads(request.body) + if (nodes := node_count(parse(body["query"]))) > MAX_NODES: + error = { + "type": "MAX_NODE_LIMIT_EXCEEDED", + "message": f"This query requests up to {nodes:,} possible nodes which exceeds the maximum limit of {MAX_NODES:,}.", + } + return ( + 200, + {"Content-Type": "application/json"}, + json.dumps({"errors": [error]}), + ) result = graphql_sync( GITHUB_SCHEMA, body["query"], diff --git a/src/shared/tests/test_github_events.py b/src/shared/tests/test_github_events.py index 3be97108a..be5990db8 100644 --- a/src/shared/tests/test_github_events.py +++ b/src/shared/tests/test_github_events.py @@ -1,11 +1,11 @@ from collections.abc import Iterator -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta import pytest import responses from github import Github -from shared.ingest_github_events import sync_github_events +from shared.ingest_github_events import BATCH_SIZE, sync_github_events from shared.models.issue import ( EventType, GitHubEvent, @@ -18,6 +18,8 @@ ClosedEvent, FakeGitHub, Issue, + MergedEvent, + PullRequest, ReopenedEvent, ) @@ -158,6 +160,66 @@ def test_missing_issues_do_not_abort_sync( assert GitHubSyncError.objects.count() == len(missing) +def test_full_batch_stays_within_node_limit( + issue: NixpkgsIssue, fake_github: FakeGitHub +) -> None: + numbers = range(1, BATCH_SIZE + 1) + for number in numbers: + GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=number, + kind=GitHubItem.Kind.ISSUE, + ).nixpkgs_issues.add(issue) + fake_github.items = [ + Issue("NixOS/nixpkgs", number, created_at=datetime(2026, 10, 1)) + for number in numbers + ] + + sync_github_events(Github()) + + assert GitHubEvent.objects.count() == len(numbers) + + +def test_truncated_timeline_skips_issue( + issue: NixpkgsIssue, fake_github: FakeGitHub +) -> None: + for number in (1, 2): + GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=number, + kind=GitHubItem.Kind.ISSUE, + ).nixpkgs_issues.add(issue) + fake_github.items = [ + Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + timeline=[ + ClosedEvent( + created_at=datetime(2026, 10, 1) + timedelta(hours=n), + state_reason="COMPLETED", + ) + if n % 2 + else ReopenedEvent( + created_at=datetime(2026, 10, 1) + timedelta(hours=n) + ) + for n in range(1, 102) + ], + ), + Issue("NixOS/nixpkgs", 2, created_at=datetime(2026, 10, 1)), + ] + + sync_github_events(Github()) + + assert set(GitHubEvent.objects.values_list("item__number", flat=True)) == {2} + assert set(GitHubSyncError.objects.values_list("item__number", "error_type")) == { + (1, "TRUNCATED") + } + assert "timeline of NixOS/nixpkgs#1" in GitHubSyncError.objects.get().message + + def test_reappearing_issue_clears_error( issue: NixpkgsIssue, fake_github: FakeGitHub ) -> None: @@ -177,3 +239,207 @@ def test_reappearing_issue_clears_error( ] sync_github_events(Github()) assert not GitHubSyncError.objects.filter(item=item).exists() + + +def test_closing_pull_request_is_tracked( + issue: NixpkgsIssue, fake_github: FakeGitHub +) -> None: + item = GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=1, + kind=GitHubItem.Kind.ISSUE, + ) + item.nixpkgs_issues.add(issue) + pull_request = PullRequest("NixOS/nixpkgs", 2, created_at=datetime(2026, 10, 2)) + github_issue = Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + closed_by=[pull_request], + ) + fake_github.items = [github_issue, pull_request] + + sync_github_events(Github()) + + closing = GitHubItem.objects.get(number=2) + assert closing.kind == GitHubItem.Kind.PULL_REQUEST + assert set(GitHubItem.objects.filter(closes__issue=item)) == {closing} + + github_issue.closed_by = [] + sync_github_events(Github()) + + assert not item.closed_by.exists() + assert GitHubItem.objects.filter(number=2).exists() + + +@pytest.mark.parametrize("close_delay", [timedelta(0), timedelta(seconds=1)]) +def test_closing_pull_request_records_history( + issue: NixpkgsIssue, fake_github: FakeGitHub, close_delay: timedelta +) -> None: + GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=1, + kind=GitHubItem.Kind.ISSUE, + ).nixpkgs_issues.add(issue) + pull_request = PullRequest( + "NixOS/nixpkgs", + 2, + created_at=datetime(2026, 10, 2), + timeline=[ + ClosedEvent(created_at=datetime(2026, 10, 3), state_reason="COMPLETED"), + ReopenedEvent(created_at=datetime(2026, 10, 4)), + MergedEvent(created_at=datetime(2026, 10, 5)), + ClosedEvent( + created_at=datetime(2026, 10, 5) + close_delay, + state_reason="COMPLETED", + ), + ], + ) + fake_github.items = [ + Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + closed_by=[pull_request], + ), + pull_request, + ] + + sync_github_events(Github()) + + assert list( + GitHubEvent.objects.filter(item__number=2) + .order_by("occurred_at") + .values_list("event_type", "occurred_at") + ) == [ + (EventType.PULL_REQUEST | EventType.OPENED, datetime(2026, 10, 2, tzinfo=UTC)), + (EventType.PULL_REQUEST | EventType.CLOSED, datetime(2026, 10, 3, tzinfo=UTC)), + (EventType.PULL_REQUEST | EventType.OPENED, datetime(2026, 10, 4, tzinfo=UTC)), + ( + EventType.PULL_REQUEST | EventType.CLOSED | EventType.MERGED, + datetime(2026, 10, 5, tzinfo=UTC), + ), + ] + + +def test_foreign_closing_pull_request_is_ignored( + issue: NixpkgsIssue, fake_github: FakeGitHub +) -> None: + item = GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=1, + kind=GitHubItem.Kind.ISSUE, + ) + item.nixpkgs_issues.add(issue) + foreign = PullRequest("someone/elsewhere", 2, created_at=datetime(2026, 10, 2)) + fake_github.items = [ + Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + closed_by=[foreign], + ), + foreign, + ] + + sync_github_events(Github()) + + assert not item.closed_by.exists() + assert not GitHubItem.objects.filter(number=2).exists() + + +def test_failed_issue_keeps_closing_pull_requests( + issue: NixpkgsIssue, fake_github: FakeGitHub +) -> None: + item = GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=1, + kind=GitHubItem.Kind.ISSUE, + ) + item.nixpkgs_issues.add(issue) + pull_request = PullRequest("NixOS/nixpkgs", 2, created_at=datetime(2026, 10, 2)) + fake_github.items = [ + Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + closed_by=[pull_request], + ), + pull_request, + ] + sync_github_events(Github()) + + fake_github.items = [] + sync_github_events(Github()) + + assert set(GitHubItem.objects.filter(closes__issue=item)) == { + GitHubItem.objects.get(number=2) + } + + +@pytest.mark.parametrize( + ("closed_by", "where"), + [ + ( + [ + PullRequest("NixOS/nixpkgs", n, created_at=datetime(2026, 10, 2)) + for n in range(2, 103) + ], + "closing pull requests of NixOS/nixpkgs#1", + ), + ( + [ + PullRequest( + "NixOS/nixpkgs", + 2, + created_at=datetime(2026, 10, 2), + timeline=[ + ClosedEvent( + created_at=datetime(2026, 10, 2) + timedelta(hours=n), + state_reason="COMPLETED", + ) + if n % 2 + else ReopenedEvent( + created_at=datetime(2026, 10, 2) + timedelta(hours=n) + ) + for n in range(1, 102) + ], + ) + ], + "timeline of NixOS/nixpkgs#2", + ), + ], +) +def test_truncated_closing_data_skips_issue( + issue: NixpkgsIssue, + fake_github: FakeGitHub, + closed_by: list[PullRequest], + where: str, +) -> None: + item = GitHubItem.objects.create( + owner="NixOS", + repository="nixpkgs", + number=1, + kind=GitHubItem.Kind.ISSUE, + ) + item.nixpkgs_issues.add(issue) + fake_github.items = [ + Issue( + "NixOS/nixpkgs", + 1, + created_at=datetime(2026, 10, 1), + closed_by=closed_by, + ), + *closed_by, + ] + + sync_github_events(Github()) + + assert not item.closed_by.exists() + error = GitHubSyncError.objects.get(item=item) + assert error.error_type == "TRUNCATED" + assert where in error.message diff --git a/src/shared/tests/test_github_state.py b/src/shared/tests/test_github_state.py index 30fc74dc9..530cb7c6d 100644 --- a/src/shared/tests/test_github_state.py +++ b/src/shared/tests/test_github_state.py @@ -1,5 +1,6 @@ from datetime import timedelta +import pytest from django.utils import timezone from shared.models.issue import EventType, GitHubItem @@ -25,12 +26,21 @@ def test_closed_issue(db: None) -> None: assert item.state == EventType.ISSUE | EventType.CLOSED | EventType.COMPLETED -def test_issue_without_events_is_open(db: None) -> None: +@pytest.mark.parametrize( + "kind, flag", + [ + (GitHubItem.Kind.ISSUE, EventType.ISSUE), + (GitHubItem.Kind.PULL_REQUEST, EventType.PULL_REQUEST), + ], +) +def test_item_without_events_is_open( + db: None, kind: GitHubItem.Kind, flag: EventType +) -> None: item = GitHubItem.objects.create( owner="NixOS", repository="nixpkgs", number=1, - kind=GitHubItem.Kind.ISSUE, + kind=kind, ) - assert item.state == EventType.ISSUE | EventType.OPENED + assert item.state == flag | EventType.OPENED diff --git a/src/webview/tests/frontend/test_issue_detail.py b/src/webview/tests/frontend/test_issue_detail.py index ab0537543..fddf39478 100644 --- a/src/webview/tests/frontend/test_issue_detail.py +++ b/src/webview/tests/frontend/test_issue_detail.py @@ -1,3 +1,5 @@ +from collections.abc import Callable + from django.utils import timezone from playwright.sync_api import Page, expect from pytest_django.live_server_helper import LiveServer @@ -41,7 +43,7 @@ def test_issue_detail_shows_github_issue_link( kind=GitHubItem.Kind.ISSUE, ) page.goto(live_server.url + ISSUE_DETAIL + f"/{issue.code}") - expect(page.get_by_role("link", name="GitHub")).to_have_attribute( + expect(page.get_by_role("link", name="#1")).to_have_attribute( "href", "https://github.com/NixOS/nixpkgs/issues/1" ) @@ -90,3 +92,30 @@ def test_issue_detail_shows_unsynced_github_issue_as_open( expect( page.get_by_test_id(f"issue-{issue.code}-github-state") ).to_have_accessible_name("Open") + + +def test_issue_detail_shows_closing_pull_request( + live_server: LiveServer, + page: Page, + issue: NixpkgsIssue, + make_github_item: Callable[..., GitHubItem], +) -> None: + github_issue = make_github_item( + 1, + GitHubItem.Kind.ISSUE, + EventType.ISSUE | EventType.OPENED, + EventType.ISSUE | EventType.CLOSED | EventType.COMPLETED, + ) + issue.github_items.add(github_issue) + make_github_item( + 2, + GitHubItem.Kind.PULL_REQUEST, + EventType.PULL_REQUEST | EventType.OPENED, + EventType.PULL_REQUEST | EventType.CLOSED | EventType.MERGED, + ).closes.create(issue=github_issue) + + page.goto(live_server.url + ISSUE_DETAIL + f"/{issue.code}") + expect(page.get_by_role("link", name="#2")).to_have_attribute( + "href", "https://github.com/NixOS/nixpkgs/pull/2" + ) + expect(page.get_by_role("img", name="Merged")).to_be_visible()