Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Enable pagination for AI enabled repos #1101

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions graphql_api/tests/test_owner.py
Original file line number Diff line number Diff line change
Expand Up @@ -1202,3 +1202,46 @@ def test_fetch_available_plans_is_enterprise_plan(self):
]
}
}

@patch("services.self_hosted.get_config")
def test_ai_enabled_repositories(self, get_config_mock):
current_org = OwnerFactory(
username="random-plan-user",
service="github",
)

get_config_mock.return_value = [
{"service": "github", "ai_features_app_id": 12345},
]

query = """{
owner(username: "%s") {
aiEnabledRepos
}
}

""" % (current_org.username)
data = self.gql_request(query, owner=current_org)
assert data["owner"]["aiEnabledRepos"] is None


@patch("services.self_hosted.get_config")
def test_ai_enabled_repositories_app_not_configured(self, get_config_mock):
current_org = OwnerFactory(
username="random-plan-user",
service="github",
)

get_config_mock.return_value = [
{"service": "github", "ai_features_app_id": 12345},
]

query = """{
owner(username: "%s") {
aiEnabledRepos
}
}

""" % (current_org.username)
data = self.gql_request(query, owner=current_org)
assert data["owner"]["aiEnabledRepos"] is None
Comment on lines +1205 to +1247
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test cases for aiEnabledRepos and aiEnabledRepositories are good additions to cover the new functionality. However, there are a few improvements we can make:

  1. The test cases are using the same mock configuration. Consider creating a setup method or fixture to avoid repetition.
  2. The assertions could be more specific. Instead of just checking if the result is None, we could assert the exact error or behavior expected when the app is not configured.
  3. Consider adding a test case for when the ai_features_app_id is not present in the config.

8 changes: 8 additions & 0 deletions graphql_api/types/owner/owner.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ type Owner {
yaml: String
aiFeaturesEnabled: Boolean!
aiEnabledRepos: [String]
aiEnabledRepositories(
ordering: RepositoryOrdering
orderingDirection: OrderingDirection
first: Int
after: String
last: Int
before: String
Comment on lines +42 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new aiEnabledRepositories field is a good addition. However, there are a couple of things to consider:

  1. The @cost directive is a good practice for managing query complexity. Make sure the complexity value (25) is appropriate for your use case.
  2. Consider adding a description for this field to provide more context for API users.
Suggested change
aiEnabledRepositories(
ordering: RepositoryOrdering
orderingDirection: OrderingDirection
first: Int
after: String
last: Int
before: String
aiEnabledRepositories(
ordering: RepositoryOrdering
orderingDirection: OrderingDirection
first: Int
after: String
last: Int
before: String
): RepositoryConnection! @cost(complexity: 25, multipliers: ["first", "last"]) @deprecated(description: "Description of the new field and why it's being added")

): RepositoryConnection! @cost(complexity: 25, multipliers: ["first", "last"])
uploadTokenRequired: Boolean
activatedUserCount: Int
}
30 changes: 30 additions & 0 deletions graphql_api/types/owner/owner.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,3 +396,33 @@
@require_shared_account_or_part_of_org
def resolve_activated_user_count(owner: Owner, info: GraphQLResolveInfo) -> int:
return owner.activated_user_count

@owner_bindable.field("aiEnabledRepositories")
def resolve_ai_enabled_repositories(
owner: Owner,
info: GraphQLResolveInfo,
ordering: Optional[RepositoryOrdering] = RepositoryOrdering.ID,
ordering_direction: Optional[OrderingDirection] = OrderingDirection.ASC,
**kwargs: Any,
) -> Coroutine[Any, Any, Connection]:
ai_features_app_install = GithubAppInstallation.objects.filter(

Check warning on line 408 in graphql_api/types/owner/owner.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/owner/owner.py#L408

Added line #L408 was not covered by tests
app_id=AI_FEATURES_GH_APP_ID, owner=owner
).first()

if not ai_features_app_install:
return None

Check warning on line 413 in graphql_api/types/owner/owner.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/owner/owner.py#L412-L413

Added lines #L412 - L413 were not covered by tests

current_owner = info.context["request"].current_owner
queryset = Repository.objects.filter(author=owner).viewable_repos(current_owner)

Check warning on line 416 in graphql_api/types/owner/owner.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/owner/owner.py#L415-L416

Added lines #L415 - L416 were not covered by tests

if ai_features_app_install.repository_service_ids:
queryset = queryset.filter(

Check warning on line 419 in graphql_api/types/owner/owner.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/owner/owner.py#L418-L419

Added lines #L418 - L419 were not covered by tests
service_id__in=ai_features_app_install.repository_service_ids
)

return queryset_to_connection(

Check warning on line 423 in graphql_api/types/owner/owner.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphql_api/types/owner/owner.py#L423

Added line #L423 was not covered by tests
queryset,
ordering=(ordering, RepositoryOrdering.ID),
ordering_direction=ordering_direction,
**kwargs,
)
Loading