From a33307ce64b52c90b14817a97a1e511dc3b5a256 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Fri, 28 Mar 2025 15:56:53 -0700 Subject: [PATCH 1/3] fix: Fix query for is_first_pull_request --- graphql_api/tests/test_repository.py | 5 +++++ graphql_api/types/repository/repository.py | 21 +++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/graphql_api/tests/test_repository.py b/graphql_api/tests/test_repository.py index 5eb3d17afb..ad2cc38ece 100644 --- a/graphql_api/tests/test_repository.py +++ b/graphql_api/tests/test_repository.py @@ -513,6 +513,11 @@ def test_repository_is_first_pull_request(self) -> None: assert data["me"]["owner"]["repository"]["isFirstPullRequest"] == True + assert ( + repo.pull_requests.values("id")[:2].query + == 'SELECT "pull_requests"."id" FROM "pull_requests" WHERE "pull_requests"."repoid" = 1 ORDER BY "pull_requests"."pullid" DESC LIMIT 2' + ) + def test_repository_is_first_pull_request_compared_to_not_none(self) -> None: repo = RepositoryFactory( author=self.owner, diff --git a/graphql_api/types/repository/repository.py b/graphql_api/types/repository/repository.py index a220d0c638..9ef6e51602 100644 --- a/graphql_api/types/repository/repository.py +++ b/graphql_api/types/repository/repository.py @@ -14,9 +14,7 @@ from graphql_api.actions.commits import repo_commits from graphql_api.dataloader.commit import CommitLoader from graphql_api.dataloader.owner import OwnerLoader -from graphql_api.helpers.connection import ( - queryset_to_connection, -) +from graphql_api.helpers.connection import queryset_to_connection from graphql_api.types.coverage_analytics.coverage_analytics import ( CoverageAnalyticsProps, ) @@ -267,13 +265,16 @@ def resolve_repository_result_type(obj: Any, *_: Any) -> Optional[str]: def resolve_is_first_pull_request( repository: Repository, info: GraphQLResolveInfo ) -> bool: - has_one_pr = repository.pull_requests.count() == 1 - - if has_one_pr: - first_pr = repository.pull_requests.first() - return not first_pr.compared_to - - return False + try: + # SELECT "pull_requests"."id" FROM "pull_requests" WHERE "pull_requests"."repoid" = 1 ORDER BY "pull_requests"."pullid" DESC LIMIT 2 + pull_requests = repository.pull_requests.values("id")[:2] + return len(pull_requests) == 1 + except Exception as e: + log.error( + "Error checking is_first_pull_request", + extra=dict(repo_id=repository.repoid, error=str(e)), + ) + return False @repository_bindable.field("isGithubRateLimited") From aab3b8bacf7ac7eb1906a4c042489735720f16da Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Fri, 28 Mar 2025 16:26:31 -0700 Subject: [PATCH 2/3] fix test --- graphql_api/tests/test_repository.py | 5 ----- graphql_api/types/repository/repository.py | 11 +++++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/graphql_api/tests/test_repository.py b/graphql_api/tests/test_repository.py index ad2cc38ece..5eb3d17afb 100644 --- a/graphql_api/tests/test_repository.py +++ b/graphql_api/tests/test_repository.py @@ -513,11 +513,6 @@ def test_repository_is_first_pull_request(self) -> None: assert data["me"]["owner"]["repository"]["isFirstPullRequest"] == True - assert ( - repo.pull_requests.values("id")[:2].query - == 'SELECT "pull_requests"."id" FROM "pull_requests" WHERE "pull_requests"."repoid" = 1 ORDER BY "pull_requests"."pullid" DESC LIMIT 2' - ) - def test_repository_is_first_pull_request_compared_to_not_none(self) -> None: repo = RepositoryFactory( author=self.owner, diff --git a/graphql_api/types/repository/repository.py b/graphql_api/types/repository/repository.py index 9ef6e51602..897e503016 100644 --- a/graphql_api/types/repository/repository.py +++ b/graphql_api/types/repository/repository.py @@ -266,12 +266,15 @@ def resolve_is_first_pull_request( repository: Repository, info: GraphQLResolveInfo ) -> bool: try: - # SELECT "pull_requests"."id" FROM "pull_requests" WHERE "pull_requests"."repoid" = 1 ORDER BY "pull_requests"."pullid" DESC LIMIT 2 - pull_requests = repository.pull_requests.values("id")[:2] - return len(pull_requests) == 1 + # Get at most 2 PRs to determine if there's only one + pull_requests = repository.pull_requests.values("id", "compared_to")[:2] + if len(pull_requests) != 1: + return False + # For single PR, check if it's a valid first PR by verifying no compared_to + return pull_requests[0]["compared_to"] is None except Exception as e: log.error( - "Error checking is_first_pull_request", + "Error checking is_first_pull_request, assuming False", extra=dict(repo_id=repository.repoid, error=str(e)), ) return False From c59da64dd06abdb48c734e4535b0787655da2691 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Fri, 28 Mar 2025 16:45:10 -0700 Subject: [PATCH 3/3] use existing error handling --- graphql_api/types/repository/repository.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/graphql_api/types/repository/repository.py b/graphql_api/types/repository/repository.py index 897e503016..4dc7413843 100644 --- a/graphql_api/types/repository/repository.py +++ b/graphql_api/types/repository/repository.py @@ -265,19 +265,12 @@ def resolve_repository_result_type(obj: Any, *_: Any) -> Optional[str]: def resolve_is_first_pull_request( repository: Repository, info: GraphQLResolveInfo ) -> bool: - try: - # Get at most 2 PRs to determine if there's only one - pull_requests = repository.pull_requests.values("id", "compared_to")[:2] - if len(pull_requests) != 1: - return False - # For single PR, check if it's a valid first PR by verifying no compared_to - return pull_requests[0]["compared_to"] is None - except Exception as e: - log.error( - "Error checking is_first_pull_request, assuming False", - extra=dict(repo_id=repository.repoid, error=str(e)), - ) + # Get at most 2 PRs to determine if there's only one + pull_requests = repository.pull_requests.values("id", "compared_to")[:2] + if len(pull_requests) != 1: return False + # For single PR, check if it's a valid first PR by verifying no compared_to + return pull_requests[0]["compared_to"] is None @repository_bindable.field("isGithubRateLimited")