Skip to content

Commit 6a44cf2

Browse files
zwickRossTarrant
zwick
and
RossTarrant
authored
Paginate project item lookup (#2914)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a67631b-dc18-448a-8be5-81110bfd543a Co-authored-by: Ross Tarrant <rosstarrant@github.com>
1 parent 9184f77 commit 6a44cf2

2 files changed

Lines changed: 242 additions & 18 deletions

File tree

‎pkg/github/projects_resolver.go‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -271,18 +271,20 @@ func resolveProjectItemIDByIssueNumber(ctx context.Context, gqlClient *githubv4.
271271
return 0, err
272272
}
273273

274-
var query struct {
274+
type projectItemsConnection struct {
275+
Nodes []struct {
276+
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
277+
Project struct {
278+
ID githubv4.ID
279+
}
280+
}
281+
PageInfo PageInfoFragment
282+
}
283+
284+
var firstPageQuery struct {
275285
Repository struct {
276286
Issue struct {
277-
ProjectItems struct {
278-
Nodes []struct {
279-
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
280-
Project struct {
281-
ID githubv4.ID
282-
}
283-
}
284-
PageInfo PageInfoFragment
285-
} `graphql:"projectItems(first: 50, includeArchived: true)"`
287+
ProjectItems projectItemsConnection `graphql:"projectItems(first: 50, includeArchived: true)"`
286288
} `graphql:"issue(number: $issueNumber)"`
287289
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
288290
}
@@ -293,18 +295,38 @@ func resolveProjectItemIDByIssueNumber(ctx context.Context, gqlClient *githubv4.
293295
"issueNumber": githubv4.Int(int32(issueNumber)), //nolint:gosec // Issue numbers are small
294296
}
295297

296-
if err := gqlClient.Query(ctx, &query, vars); err != nil {
298+
if err := gqlClient.Query(ctx, &firstPageQuery, vars); err != nil {
297299
return 0, fmt.Errorf("failed to resolve project item for %s/%s#%d: %w", issueOwner, issueRepo, issueNumber, err)
298300
}
299301

300-
for _, item := range query.Repository.Issue.ProjectItems.Nodes {
301-
if item.Project.ID == projectID {
302-
itemID, parseErr := parseInt64(string(item.FullDatabaseID))
303-
if parseErr != nil {
304-
return 0, fmt.Errorf("project item ID %q is not an integer: %w", string(item.FullDatabaseID), parseErr)
302+
projectItems := firstPageQuery.Repository.Issue.ProjectItems
303+
for {
304+
for _, item := range projectItems.Nodes {
305+
if item.Project.ID == projectID {
306+
itemID, parseErr := parseInt64(string(item.FullDatabaseID))
307+
if parseErr != nil {
308+
return 0, fmt.Errorf("project item ID %q is not an integer: %w", string(item.FullDatabaseID), parseErr)
309+
}
310+
return itemID, nil
305311
}
306-
return itemID, nil
307312
}
313+
314+
if !projectItems.PageInfo.HasNextPage {
315+
break
316+
}
317+
318+
var nextPageQuery struct {
319+
Repository struct {
320+
Issue struct {
321+
ProjectItems projectItemsConnection `graphql:"projectItems(first: 50, after: $after, includeArchived: true)"`
322+
} `graphql:"issue(number: $issueNumber)"`
323+
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
324+
}
325+
vars["after"] = projectItems.PageInfo.EndCursor
326+
if err := gqlClient.Query(ctx, &nextPageQuery, vars); err != nil {
327+
return 0, fmt.Errorf("failed to resolve project item for %s/%s#%d: %w", issueOwner, issueRepo, issueNumber, err)
328+
}
329+
projectItems = nextPageQuery.Repository.Issue.ProjectItems
308330
}
309331

310332
return 0, ghErrors.NewStructuredResolutionError(

‎pkg/github/projects_resolver_test.go‎

Lines changed: 203 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,32 @@ type resolveItemByIssueQuery struct {
215215
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
216216
}
217217

218+
type resolveItemByIssuePageQuery struct {
219+
Repository struct {
220+
Issue struct {
221+
ProjectItems struct {
222+
Nodes []struct {
223+
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
224+
Project struct {
225+
ID githubv4.ID
226+
}
227+
}
228+
PageInfo PageInfoFragment
229+
} `graphql:"projectItems(first: 50, after: $after, includeArchived: true)"`
230+
} `graphql:"issue(number: $issueNumber)"`
231+
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
232+
}
233+
234+
type requestCountingTransport struct {
235+
inner http.RoundTripper
236+
count int
237+
}
238+
239+
func (t *requestCountingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
240+
t.count++
241+
return t.inner.RoundTrip(req)
242+
}
243+
218244
func Test_ResolveProjectItemIDByIssueNumber_Success(t *testing.T) {
219245
mocked := githubv4mock.NewMockedHTTPClient(
220246
// project node id lookup (org)
@@ -279,6 +305,91 @@ func Test_ResolveProjectItemIDByIssueNumber_Success(t *testing.T) {
279305
assert.Equal(t, int64(4242), itemID)
280306
}
281307

308+
func Test_ResolveProjectItemIDByIssueNumber_TargetOnSecondPage(t *testing.T) {
309+
mocked := githubv4mock.NewMockedHTTPClient(
310+
githubv4mock.NewQueryMatcher(
311+
struct {
312+
Organization struct {
313+
ProjectV2 struct {
314+
ID githubv4.ID
315+
} `graphql:"projectV2(number: $projectNumber)"`
316+
} `graphql:"organization(login: $owner)"`
317+
}{},
318+
map[string]any{
319+
"owner": githubv4.String("octo-org"),
320+
"projectNumber": githubv4.Int(1),
321+
},
322+
githubv4mock.DataResponse(map[string]any{
323+
"organization": map[string]any{
324+
"projectV2": map[string]any{"id": "PVT_project1"},
325+
},
326+
}),
327+
),
328+
githubv4mock.NewQueryMatcher(
329+
resolveItemByIssueQuery{},
330+
map[string]any{
331+
"issueOwner": githubv4.String("octo-issue-owner"),
332+
"issueRepo": githubv4.String("repo"),
333+
"issueNumber": githubv4.Int(123),
334+
},
335+
githubv4mock.DataResponse(map[string]any{
336+
"repository": map[string]any{
337+
"issue": map[string]any{
338+
"projectItems": map[string]any{
339+
"nodes": []any{
340+
map[string]any{
341+
"fullDatabaseId": "9999",
342+
"project": map[string]any{"id": "PVT_other"},
343+
},
344+
},
345+
"pageInfo": map[string]any{
346+
"hasNextPage": true,
347+
"hasPreviousPage": false,
348+
"startCursor": "first",
349+
"endCursor": "page-one",
350+
},
351+
},
352+
},
353+
},
354+
}),
355+
),
356+
githubv4mock.NewQueryMatcher(
357+
resolveItemByIssuePageQuery{},
358+
map[string]any{
359+
"issueOwner": githubv4.String("octo-issue-owner"),
360+
"issueRepo": githubv4.String("repo"),
361+
"issueNumber": githubv4.Int(123),
362+
"after": githubv4.String("page-one"),
363+
},
364+
githubv4mock.DataResponse(map[string]any{
365+
"repository": map[string]any{
366+
"issue": map[string]any{
367+
"projectItems": map[string]any{
368+
"nodes": []any{
369+
map[string]any{
370+
"fullDatabaseId": "4242",
371+
"project": map[string]any{"id": "PVT_project1"},
372+
},
373+
},
374+
"pageInfo": map[string]any{
375+
"hasNextPage": false,
376+
"hasPreviousPage": true,
377+
"startCursor": "page-two",
378+
"endCursor": "page-two",
379+
},
380+
},
381+
},
382+
},
383+
}),
384+
),
385+
)
386+
gql := githubv4.NewClient(mocked)
387+
388+
itemID, err := resolveProjectItemIDByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
389+
require.NoError(t, err)
390+
assert.Equal(t, int64(4242), itemID)
391+
}
392+
282393
func Test_ResolveProjectItemIDByIssueNumber_NotInProject(t *testing.T) {
283394
mocked := githubv4mock.NewMockedHTTPClient(
284395
githubv4mock.NewQueryMatcher(
@@ -340,6 +451,97 @@ func Test_ResolveProjectItemIDByIssueNumber_NotInProject(t *testing.T) {
340451
assert.Equal(t, "item_not_in_project", msg["error"])
341452
}
342453

454+
func Test_ResolveProjectItemIDByIssueNumber_NotInProjectAfterMultiplePages(t *testing.T) {
455+
mocked := githubv4mock.NewMockedHTTPClient(
456+
githubv4mock.NewQueryMatcher(
457+
struct {
458+
Organization struct {
459+
ProjectV2 struct {
460+
ID githubv4.ID
461+
} `graphql:"projectV2(number: $projectNumber)"`
462+
} `graphql:"organization(login: $owner)"`
463+
}{},
464+
map[string]any{
465+
"owner": githubv4.String("octo-org"),
466+
"projectNumber": githubv4.Int(1),
467+
},
468+
githubv4mock.DataResponse(map[string]any{
469+
"organization": map[string]any{
470+
"projectV2": map[string]any{"id": "PVT_project1"},
471+
},
472+
}),
473+
),
474+
githubv4mock.NewQueryMatcher(
475+
resolveItemByIssueQuery{},
476+
map[string]any{
477+
"issueOwner": githubv4.String("octo-issue-owner"),
478+
"issueRepo": githubv4.String("repo"),
479+
"issueNumber": githubv4.Int(123),
480+
},
481+
githubv4mock.DataResponse(map[string]any{
482+
"repository": map[string]any{
483+
"issue": map[string]any{
484+
"projectItems": map[string]any{
485+
"nodes": []any{
486+
map[string]any{
487+
"fullDatabaseId": "9999",
488+
"project": map[string]any{"id": "PVT_other"},
489+
},
490+
},
491+
"pageInfo": map[string]any{
492+
"hasNextPage": true,
493+
"hasPreviousPage": false,
494+
"startCursor": "first",
495+
"endCursor": "page-one",
496+
},
497+
},
498+
},
499+
},
500+
}),
501+
),
502+
githubv4mock.NewQueryMatcher(
503+
resolveItemByIssuePageQuery{},
504+
map[string]any{
505+
"issueOwner": githubv4.String("octo-issue-owner"),
506+
"issueRepo": githubv4.String("repo"),
507+
"issueNumber": githubv4.Int(123),
508+
"after": githubv4.String("page-one"),
509+
},
510+
githubv4mock.DataResponse(map[string]any{
511+
"repository": map[string]any{
512+
"issue": map[string]any{
513+
"projectItems": map[string]any{
514+
"nodes": []any{
515+
map[string]any{
516+
"fullDatabaseId": "8888",
517+
"project": map[string]any{"id": "PVT_another"},
518+
},
519+
},
520+
"pageInfo": map[string]any{
521+
"hasNextPage": false,
522+
"hasPreviousPage": true,
523+
"startCursor": "page-two",
524+
"endCursor": "page-two",
525+
},
526+
},
527+
},
528+
},
529+
}),
530+
),
531+
)
532+
countingTransport := &requestCountingTransport{inner: mocked.Transport}
533+
mocked.Transport = countingTransport
534+
gql := githubv4.NewClient(mocked)
535+
536+
_, err := resolveProjectItemIDByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
537+
require.Error(t, err)
538+
assert.Equal(t, 3, countingTransport.count)
539+
540+
var msg map[string]any
541+
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
542+
assert.Equal(t, "item_not_in_project", msg["error"])
543+
}
544+
343545
func Test_ResolveFieldNamesToIDs_Success(t *testing.T) {
344546
mocked := githubv4mock.NewMockedHTTPClient(
345547
githubv4mock.NewQueryMatcher(
@@ -358,7 +560,7 @@ func Test_ResolveFieldNamesToIDs_Success(t *testing.T) {
358560
assert.Equal(t, []int64{100, 200}, ids)
359561
}
360562

361-
// Field and single-select option name matching is case-insensitive so agents passing lowercase
563+
// Field and single-select option name matching is case-insensitive so agents passing lowercase
362564
// names like "status" or "in progress" resolve to "Status" and "In Progress" respectively.
363565
func Test_ResolveProjectFieldByName_CaseInsensitive(t *testing.T) {
364566
mocked := githubv4mock.NewMockedHTTPClient(

0 commit comments

Comments
 (0)