Skip to content

Commit 7382b86

Browse files
BagToadCopilot
BagToad
and
Copilot
committed
Fetch org teams via repository.owner inline fragment
Replace the top-level organization(login: $owner) query with repository.owner { ... on Organization { teams } }. This uses GraphQL inline fragments to conditionally fetch team data only when the repo owner is an Organization, eliminating the need to handle 'Could not resolve to an Organization' errors for personal repos. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 24fb765 commit 7382b86

2 files changed

Lines changed: 52 additions & 72 deletions

File tree

‎api/queries_pr_review.go‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,8 @@ func RequestReviewsByLogin(client *Client, repo ghrepo.Interface, prID string, u
407407
// Returns the candidates, a MoreResults count, and an error.
408408
func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, query string) ([]ReviewerCandidate, int, error) {
409409
// Fetch 10 from each source to allow cascading quota to fill from available results.
410-
// Use a single query that includes organization.teams - if the owner is not an org,
411-
// we'll get a "Could not resolve to an Organization" error which we handle gracefully.
410+
// Organization teams are fetched via repository.owner inline fragment, which
411+
// gracefully returns empty data for personal (User-owned) repos.
412412
// We also fetch unfiltered total counts via aliases for the "X more" display.
413413
type responseData struct {
414414
Node struct {
@@ -435,6 +435,19 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
435435
} `graphql:"... on PullRequest"`
436436
} `graphql:"node(id: $id)"`
437437
Repository struct {
438+
Owner struct {
439+
TypeName string `graphql:"__typename"`
440+
Organization struct {
441+
Teams struct {
442+
Nodes []struct {
443+
Slug string
444+
}
445+
} `graphql:"teams(first: 10, query: $query)"`
446+
TeamsTotalCount struct {
447+
TotalCount int
448+
} `graphql:"teamsTotalCount: teams(first: 0)"`
449+
} `graphql:"... on Organization"`
450+
}
438451
Collaborators struct {
439452
Nodes []struct {
440453
Login string
@@ -445,16 +458,6 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
445458
TotalCount int
446459
} `graphql:"collaboratorsTotalCount: collaborators(first: 0)"`
447460
} `graphql:"repository(owner: $owner, name: $name)"`
448-
Organization struct {
449-
Teams struct {
450-
Nodes []struct {
451-
Slug string
452-
}
453-
} `graphql:"teams(first: 10, query: $query)"`
454-
TeamsTotalCount struct {
455-
TotalCount int
456-
} `graphql:"teamsTotalCount: teams(first: 0)"`
457-
} `graphql:"organization(login: $owner)"`
458461
}
459462

460463
variables := map[string]interface{}{
@@ -466,9 +469,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
466469

467470
var result responseData
468471
err := client.Query(repo.RepoHost(), "SuggestedReviewerActors", &result, variables)
469-
// Handle the case where the owner is not an organization - the query still returns
470-
// partial data (repository, node), so we can continue processing.
471-
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
472+
if err != nil {
472473
return nil, 0, err
473474
}
474475

@@ -531,7 +532,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
531532
teamsQuota := baseQuota + (collaboratorsQuota - collaboratorsAdded)
532533
teamsAdded := 0
533534
ownerName := repo.RepoOwner()
534-
for _, t := range result.Organization.Teams.Nodes {
535+
for _, t := range result.Repository.Owner.Organization.Teams.Nodes {
535536
if teamsAdded >= teamsQuota {
536537
break
537538
}
@@ -547,7 +548,7 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
547548
}
548549

549550
// MoreResults uses unfiltered total counts (teams will be 0 for personal repos)
550-
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount
551+
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Repository.Owner.Organization.TeamsTotalCount.TotalCount
551552

552553
return candidates, moreResults, nil
553554
}
@@ -584,6 +585,19 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query
584585
} `graphql:"suggestedReviewerActors(first: 10)"`
585586
}
586587
} `graphql:"pullRequests(first: 1, states: [OPEN])"`
588+
Owner struct {
589+
TypeName string `graphql:"__typename"`
590+
Organization struct {
591+
Teams struct {
592+
Nodes []struct {
593+
Slug string
594+
}
595+
} `graphql:"teams(first: 10, query: $query)"`
596+
TeamsTotalCount struct {
597+
TotalCount int
598+
} `graphql:"teamsTotalCount: teams(first: 0)"`
599+
} `graphql:"... on Organization"`
600+
}
587601
Collaborators struct {
588602
Nodes []struct {
589603
Login string
@@ -594,16 +608,6 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query
594608
TotalCount int
595609
} `graphql:"collaboratorsTotalCount: collaborators(first: 0)"`
596610
} `graphql:"repository(owner: $owner, name: $name)"`
597-
Organization struct {
598-
Teams struct {
599-
Nodes []struct {
600-
Slug string
601-
}
602-
} `graphql:"teams(first: 10, query: $query)"`
603-
TeamsTotalCount struct {
604-
TotalCount int
605-
} `graphql:"teamsTotalCount: teams(first: 0)"`
606-
} `graphql:"organization(login: $owner)"`
607611
}
608612

609613
variables := map[string]interface{}{
@@ -614,9 +618,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query
614618

615619
var result responseData
616620
err := client.Query(repo.RepoHost(), "SuggestedReviewerActorsForRepo", &result, variables)
617-
// Handle the case where the owner is not an organization - the query still returns
618-
// partial data (repository), so we can continue processing.
619-
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
621+
if err != nil {
620622
return nil, 0, err
621623
}
622624

@@ -661,7 +663,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query
661663
teamsQuota := baseQuota + (baseQuota - collaboratorsAdded)
662664
teamsAdded := 0
663665
ownerName := repo.RepoOwner()
664-
for _, t := range result.Organization.Teams.Nodes {
666+
for _, t := range result.Repository.Owner.Organization.Teams.Nodes {
665667
if teamsAdded >= teamsQuota {
666668
break
667669
}
@@ -677,7 +679,7 @@ func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query
677679
}
678680

679681
// MoreResults uses unfiltered total counts (teams will be 0 for personal repos)
680-
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount
682+
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Repository.Owner.Organization.TeamsTotalCount.TotalCount
681683

682684
return candidates, moreResults, nil
683685
}

‎api/queries_pr_test.go‎

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -162,16 +162,14 @@ func mockReviewerResponse(suggestions, collabs, teams, totalCollabs, totalTeams
162162
"data": {
163163
"node": {"author": {"login": "testauthor"}, "suggestedReviewerActors": {"nodes": [%s]}},
164164
"repository": {
165+
"owner": {"__typename": "Organization", "teams": {"nodes": [%s]}, "teamsTotalCount": {"totalCount": %d}},
165166
"collaborators": {"nodes": [%s]},
166167
"collaboratorsTotalCount": {"totalCount": %d}
167-
},
168-
"organization": {
169-
"teams": {"nodes": [%s]},
170-
"teamsTotalCount": {"totalCount": %d}
171168
}
172169
}
173-
}`, strings.Join(suggestionNodes, ","), strings.Join(collabNodes, ","), totalCollabs,
174-
strings.Join(teamNodes, ","), totalTeams)
170+
}`, strings.Join(suggestionNodes, ","),
171+
strings.Join(teamNodes, ","), totalTeams,
172+
strings.Join(collabNodes, ","), totalCollabs)
175173
}
176174

177175
func TestSuggestedReviewerActors(t *testing.T) {
@@ -241,12 +239,9 @@ func TestSuggestedReviewerActors(t *testing.T) {
241239
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s2", "name": "S2"}}
242240
]}},
243241
"repository": {
242+
"owner": {"__typename": "Organization", "teams": {"nodes": [{"slug": "team1"}]}, "teamsTotalCount": {"totalCount": 3}},
244243
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
245244
"collaboratorsTotalCount": {"totalCount": 5}
246-
},
247-
"organization": {
248-
"teams": {"nodes": [{"slug": "team1"}]},
249-
"teamsTotalCount": {"totalCount": 3}
250245
}
251246
}
252247
}`))
@@ -271,10 +266,6 @@ func TestSuggestedReviewerActors(t *testing.T) {
271266
{"login": "c1", "name": "C1"}
272267
]},
273268
"collaboratorsTotalCount": {"totalCount": 5}
274-
},
275-
"organization": {
276-
"teams": {"nodes": []},
277-
"teamsTotalCount": {"totalCount": 0}
278269
}
279270
}
280271
}`))
@@ -295,15 +286,12 @@ func TestSuggestedReviewerActors(t *testing.T) {
295286
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "shareduser", "name": "Shared"}}
296287
]}},
297288
"repository": {
289+
"owner": {"__typename": "Organization", "teams": {"nodes": [{"slug": "team1"}]}, "teamsTotalCount": {"totalCount": 5}},
298290
"collaborators": {"nodes": [
299291
{"login": "shareduser", "name": "Shared"},
300292
{"login": "c1", "name": "C1"}
301293
]},
302294
"collaboratorsTotalCount": {"totalCount": 10}
303-
},
304-
"organization": {
305-
"teams": {"nodes": [{"slug": "team1"}]},
306-
"teamsTotalCount": {"totalCount": 5}
307295
}
308296
}
309297
}`))
@@ -323,12 +311,11 @@ func TestSuggestedReviewerActors(t *testing.T) {
323311
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}}
324312
]}},
325313
"repository": {
314+
"owner": {"__typename": "User"},
326315
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
327316
"collaboratorsTotalCount": {"totalCount": 3}
328-
},
329-
"organization": null
330-
},
331-
"errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}]
317+
}
318+
}
332319
}`))
333320
},
334321
expectedCount: 2,
@@ -347,12 +334,9 @@ func TestSuggestedReviewerActors(t *testing.T) {
347334
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}}
348335
]}},
349336
"repository": {
337+
"owner": {"__typename": "Organization", "teams": {"nodes": []}, "teamsTotalCount": {"totalCount": 0}},
350338
"collaborators": {"nodes": []},
351339
"collaboratorsTotalCount": {"totalCount": 5}
352-
},
353-
"organization": {
354-
"teams": {"nodes": []},
355-
"teamsTotalCount": {"totalCount": 0}
356340
}
357341
}
358342
}`))
@@ -421,16 +405,14 @@ func mockReviewerResponseForRepoWithCopilot(collabs, teams, totalCollabs, totalT
421405
"viewer": {"login": "testuser"},
422406
"repository": {
423407
%s,
408+
"owner": {"__typename": "Organization", "teams": {"nodes": [%s]}, "teamsTotalCount": {"totalCount": %d}},
424409
"collaborators": {"nodes": [%s]},
425410
"collaboratorsTotalCount": {"totalCount": %d}
426-
},
427-
"organization": {
428-
"teams": {"nodes": [%s]},
429-
"teamsTotalCount": {"totalCount": %d}
430411
}
431412
}
432-
}`, pullRequestsJSON, strings.Join(collabNodes, ","), totalCollabs,
433-
strings.Join(teamNodes, ","), totalTeams)
413+
}`, pullRequestsJSON,
414+
strings.Join(teamNodes, ","), totalTeams,
415+
strings.Join(collabNodes, ","), totalCollabs)
434416
}
435417

436418
func TestSuggestedReviewerActorsForRepo(t *testing.T) {
@@ -484,12 +466,11 @@ func TestSuggestedReviewerActorsForRepo(t *testing.T) {
484466
"data": {
485467
"repository": {
486468
"pullRequests": {"nodes": []},
469+
"owner": {"__typename": "User"},
487470
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
488471
"collaboratorsTotalCount": {"totalCount": 3}
489-
},
490-
"organization": null
491-
},
492-
"errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}]
472+
}
473+
}
493474
}`))
494475
},
495476
expectedCount: 1,
@@ -541,16 +522,13 @@ func TestSuggestedReviewerActorsForRepo(t *testing.T) {
541522
"viewer": {"login": "c2"},
542523
"repository": {
543524
"pullRequests": {"nodes": []},
525+
"owner": {"__typename": "Organization", "teams": {"nodes": []}, "teamsTotalCount": {"totalCount": 0}},
544526
"collaborators": {"nodes": [
545527
{"login": "c1", "name": "C1"},
546528
{"login": "c2", "name": "C2"},
547529
{"login": "c3", "name": "C3"}
548530
]},
549531
"collaboratorsTotalCount": {"totalCount": 3}
550-
},
551-
"organization": {
552-
"teams": {"nodes": []},
553-
"teamsTotalCount": {"totalCount": 0}
554532
}
555533
}
556534
}`))

0 commit comments

Comments
 (0)