diff --git a/api/queries_pr_review.go b/api/queries_pr_review.go index 3d0132cbd..a6fa34f9e 100644 --- a/api/queries_pr_review.go +++ b/api/queries_pr_review.go @@ -407,8 +407,8 @@ func RequestReviewsByLogin(client *Client, repo ghrepo.Interface, prID string, u // Returns the candidates, a MoreResults count, and an error. func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, query string) ([]ReviewerCandidate, int, error) { // Fetch 10 from each source to allow cascading quota to fill from available results. - // Use a single query that includes organization.teams - if the owner is not an org, - // we'll get a "Could not resolve to an Organization" error which we handle gracefully. + // Organization teams are fetched via repository.owner inline fragment, which + // gracefully returns empty data for personal (User-owned) repos. // We also fetch unfiltered total counts via aliases for the "X more" display. type responseData struct { Node struct { @@ -435,6 +435,19 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, } `graphql:"... on PullRequest"` } `graphql:"node(id: $id)"` Repository struct { + Owner struct { + TypeName string `graphql:"__typename"` + Organization struct { + Teams struct { + Nodes []struct { + Slug string + } + } `graphql:"teams(first: 10, query: $query)"` + TeamsTotalCount struct { + TotalCount int + } `graphql:"teamsTotalCount: teams(first: 0)"` + } `graphql:"... on Organization"` + } Collaborators struct { Nodes []struct { Login string @@ -445,16 +458,6 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, TotalCount int } `graphql:"collaboratorsTotalCount: collaborators(first: 0)"` } `graphql:"repository(owner: $owner, name: $name)"` - Organization struct { - Teams struct { - Nodes []struct { - Slug string - } - } `graphql:"teams(first: 10, query: $query)"` - TeamsTotalCount struct { - TotalCount int - } `graphql:"teamsTotalCount: teams(first: 0)"` - } `graphql:"organization(login: $owner)"` } variables := map[string]interface{}{ @@ -466,9 +469,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, var result responseData err := client.Query(repo.RepoHost(), "SuggestedReviewerActors", &result, variables) - // Handle the case where the owner is not an organization - the query still returns - // partial data (repository, node), so we can continue processing. - if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) { + if err != nil { return nil, 0, err } @@ -531,7 +532,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, teamsQuota := baseQuota + (collaboratorsQuota - collaboratorsAdded) teamsAdded := 0 ownerName := repo.RepoOwner() - for _, t := range result.Organization.Teams.Nodes { + for _, t := range result.Repository.Owner.Organization.Teams.Nodes { if teamsAdded >= teamsQuota { break } @@ -547,7 +548,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, } // MoreResults uses unfiltered total counts (teams will be 0 for personal repos) - moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount + moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Repository.Owner.Organization.TeamsTotalCount.TotalCount return candidates, moreResults, nil } @@ -584,6 +585,19 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query } `graphql:"suggestedReviewerActors(first: 10)"` } } `graphql:"pullRequests(first: 1, states: [OPEN])"` + Owner struct { + TypeName string `graphql:"__typename"` + Organization struct { + Teams struct { + Nodes []struct { + Slug string + } + } `graphql:"teams(first: 10, query: $query)"` + TeamsTotalCount struct { + TotalCount int + } `graphql:"teamsTotalCount: teams(first: 0)"` + } `graphql:"... on Organization"` + } Collaborators struct { Nodes []struct { Login string @@ -594,16 +608,6 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query TotalCount int } `graphql:"collaboratorsTotalCount: collaborators(first: 0)"` } `graphql:"repository(owner: $owner, name: $name)"` - Organization struct { - Teams struct { - Nodes []struct { - Slug string - } - } `graphql:"teams(first: 10, query: $query)"` - TeamsTotalCount struct { - TotalCount int - } `graphql:"teamsTotalCount: teams(first: 0)"` - } `graphql:"organization(login: $owner)"` } variables := map[string]interface{}{ @@ -614,9 +618,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query var result responseData err := client.Query(repo.RepoHost(), "SuggestedReviewerActorsForRepo", &result, variables) - // Handle the case where the owner is not an organization - the query still returns - // partial data (repository), so we can continue processing. - if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) { + if err != nil { return nil, 0, err } @@ -661,7 +663,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query teamsQuota := baseQuota + (baseQuota - collaboratorsAdded) teamsAdded := 0 ownerName := repo.RepoOwner() - for _, t := range result.Organization.Teams.Nodes { + for _, t := range result.Repository.Owner.Organization.Teams.Nodes { if teamsAdded >= teamsQuota { break } @@ -677,7 +679,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query } // MoreResults uses unfiltered total counts (teams will be 0 for personal repos) - moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount + moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Repository.Owner.Organization.TeamsTotalCount.TotalCount return candidates, moreResults, nil } diff --git a/api/queries_pr_test.go b/api/queries_pr_test.go index 8fff120bb..633b9a8c3 100644 --- a/api/queries_pr_test.go +++ b/api/queries_pr_test.go @@ -162,16 +162,14 @@ func mockReviewerResponse(suggestions, collabs, teams, totalCollabs, totalTeams "data": { "node": {"author": {"login": "testauthor"}, "suggestedReviewerActors": {"nodes": [%s]}}, "repository": { + "owner": {"__typename": "Organization", "teams": {"nodes": [%s]}, "teamsTotalCount": {"totalCount": %d}}, "collaborators": {"nodes": [%s]}, "collaboratorsTotalCount": {"totalCount": %d} - }, - "organization": { - "teams": {"nodes": [%s]}, - "teamsTotalCount": {"totalCount": %d} } } - }`, strings.Join(suggestionNodes, ","), strings.Join(collabNodes, ","), totalCollabs, - strings.Join(teamNodes, ","), totalTeams) + }`, strings.Join(suggestionNodes, ","), + strings.Join(teamNodes, ","), totalTeams, + strings.Join(collabNodes, ","), totalCollabs) } func TestSuggestedReviewerActors(t *testing.T) { @@ -241,12 +239,9 @@ func TestSuggestedReviewerActors(t *testing.T) { {"isAuthor": false, "reviewer": {"__typename": "User", "login": "s2", "name": "S2"}} ]}}, "repository": { + "owner": {"__typename": "Organization", "teams": {"nodes": [{"slug": "team1"}]}, "teamsTotalCount": {"totalCount": 3}}, "collaborators": {"nodes": [{"login": "c1", "name": "C1"}]}, "collaboratorsTotalCount": {"totalCount": 5} - }, - "organization": { - "teams": {"nodes": [{"slug": "team1"}]}, - "teamsTotalCount": {"totalCount": 3} } } }`)) @@ -271,10 +266,6 @@ func TestSuggestedReviewerActors(t *testing.T) { {"login": "c1", "name": "C1"} ]}, "collaboratorsTotalCount": {"totalCount": 5} - }, - "organization": { - "teams": {"nodes": []}, - "teamsTotalCount": {"totalCount": 0} } } }`)) @@ -295,15 +286,12 @@ func TestSuggestedReviewerActors(t *testing.T) { {"isAuthor": false, "reviewer": {"__typename": "User", "login": "shareduser", "name": "Shared"}} ]}}, "repository": { + "owner": {"__typename": "Organization", "teams": {"nodes": [{"slug": "team1"}]}, "teamsTotalCount": {"totalCount": 5}}, "collaborators": {"nodes": [ {"login": "shareduser", "name": "Shared"}, {"login": "c1", "name": "C1"} ]}, "collaboratorsTotalCount": {"totalCount": 10} - }, - "organization": { - "teams": {"nodes": [{"slug": "team1"}]}, - "teamsTotalCount": {"totalCount": 5} } } }`)) @@ -323,12 +311,11 @@ func TestSuggestedReviewerActors(t *testing.T) { {"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}} ]}}, "repository": { + "owner": {"__typename": "User"}, "collaborators": {"nodes": [{"login": "c1", "name": "C1"}]}, "collaboratorsTotalCount": {"totalCount": 3} - }, - "organization": null - }, - "errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}] + } + } }`)) }, expectedCount: 2, @@ -347,12 +334,9 @@ func TestSuggestedReviewerActors(t *testing.T) { {"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}} ]}}, "repository": { + "owner": {"__typename": "Organization", "teams": {"nodes": []}, "teamsTotalCount": {"totalCount": 0}}, "collaborators": {"nodes": []}, "collaboratorsTotalCount": {"totalCount": 5} - }, - "organization": { - "teams": {"nodes": []}, - "teamsTotalCount": {"totalCount": 0} } } }`)) @@ -421,16 +405,14 @@ func mockReviewerResponseForRepoWithCopilot(collabs, teams, totalCollabs, totalT "viewer": {"login": "testuser"}, "repository": { %s, + "owner": {"__typename": "Organization", "teams": {"nodes": [%s]}, "teamsTotalCount": {"totalCount": %d}}, "collaborators": {"nodes": [%s]}, "collaboratorsTotalCount": {"totalCount": %d} - }, - "organization": { - "teams": {"nodes": [%s]}, - "teamsTotalCount": {"totalCount": %d} } } - }`, pullRequestsJSON, strings.Join(collabNodes, ","), totalCollabs, - strings.Join(teamNodes, ","), totalTeams) + }`, pullRequestsJSON, + strings.Join(teamNodes, ","), totalTeams, + strings.Join(collabNodes, ","), totalCollabs) } func TestSuggestedReviewerActorsForRepo(t *testing.T) { @@ -484,12 +466,11 @@ func TestSuggestedReviewerActorsForRepo(t *testing.T) { "data": { "repository": { "pullRequests": {"nodes": []}, + "owner": {"__typename": "User"}, "collaborators": {"nodes": [{"login": "c1", "name": "C1"}]}, "collaboratorsTotalCount": {"totalCount": 3} - }, - "organization": null - }, - "errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}] + } + } }`)) }, expectedCount: 1, @@ -541,16 +522,13 @@ func TestSuggestedReviewerActorsForRepo(t *testing.T) { "viewer": {"login": "c2"}, "repository": { "pullRequests": {"nodes": []}, + "owner": {"__typename": "Organization", "teams": {"nodes": []}, "teamsTotalCount": {"totalCount": 0}}, "collaborators": {"nodes": [ {"login": "c1", "name": "C1"}, {"login": "c2", "name": "C2"}, {"login": "c3", "name": "C3"} ]}, "collaboratorsTotalCount": {"totalCount": 3} - }, - "organization": { - "teams": {"nodes": []}, - "teamsTotalCount": {"totalCount": 0} } } }`))