Skip to content

Commit d745f5c

Browse files
mattdhollowaymattdholloway
committed
add GraphQL pagination support for discussion comments and categories
1 parent d464ac1 commit d745f5c

File tree

2 files changed

+102
-71
lines changed

2 files changed

+102
-71
lines changed

‎pkg/github/discussions.go‎

Lines changed: 59 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
286286
mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")),
287287
mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")),
288288
mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")),
289+
WithGraphQLPagination(),
289290
),
290291
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
291292
// Decode params
@@ -298,6 +299,17 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
298299
return mcp.NewToolResultError(err.Error()), nil
299300
}
300301

302+
paginationParams, err := OptionalGraphQLPaginationParams(request)
303+
if err != nil {
304+
return mcp.NewToolResultError(err.Error()), nil
305+
}
306+
307+
// Use default of 100 if neither first nor last is specified
308+
if paginationParams.First == nil && paginationParams.Last == nil {
309+
defaultFirst := int32(100)
310+
paginationParams.First = &defaultFirst
311+
}
312+
301313
client, err := getGQLClient(ctx)
302314
if err != nil {
303315
return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil
@@ -310,24 +322,39 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
310322
Nodes []struct {
311323
Body githubv4.String
312324
}
313-
} `graphql:"comments(first:100)"`
325+
PageInfo struct {
326+
HasNextPage githubv4.Boolean
327+
EndCursor githubv4.String
328+
}
329+
} `graphql:"comments(first: $first, after: $after)"`
314330
} `graphql:"discussion(number: $discussionNumber)"`
315331
} `graphql:"repository(owner: $owner, name: $repo)"`
316332
}
317333
vars := map[string]interface{}{
318334
"owner": githubv4.String(params.Owner),
319335
"repo": githubv4.String(params.Repo),
320336
"discussionNumber": githubv4.Int(params.DiscussionNumber),
337+
"first": (*githubv4.Int)(paginationParams.First),
338+
"after": (*githubv4.String)(paginationParams.After),
321339
}
322340
if err := client.Query(ctx, &q, vars); err != nil {
323341
return mcp.NewToolResultError(err.Error()), nil
324342
}
343+
325344
var comments []*github.IssueComment
326345
for _, c := range q.Repository.Discussion.Comments.Nodes {
327346
comments = append(comments, &github.IssueComment{Body: github.Ptr(string(c.Body))})
328347
}
329348

330-
out, err := json.Marshal(comments)
349+
result := map[string]interface{}{
350+
"comments": comments,
351+
"pageInfo": map[string]interface{}{
352+
"hasNextPage": bool(q.Repository.Discussion.Comments.PageInfo.HasNextPage),
353+
"endCursor": string(q.Repository.Discussion.Comments.PageInfo.EndCursor),
354+
},
355+
}
356+
357+
out, err := json.Marshal(result)
331358
if err != nil {
332359
return nil, fmt.Errorf("failed to marshal comments: %w", err)
333360
}
@@ -351,80 +378,75 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
351378
mcp.Required(),
352379
mcp.Description("Repository name"),
353380
),
354-
mcp.WithNumber("first",
355-
mcp.Description("Number of categories to return per page (min 1, max 100)"),
356-
mcp.Min(1),
357-
mcp.Max(100),
358-
),
359-
mcp.WithNumber("last",
360-
mcp.Description("Number of categories to return from the end (min 1, max 100)"),
361-
mcp.Min(1),
362-
mcp.Max(100),
363-
),
364-
mcp.WithString("after",
365-
mcp.Description("Cursor for pagination, use the 'after' field from the previous response"),
366-
),
367-
mcp.WithString("before",
368-
mcp.Description("Cursor for pagination, use the 'before' field from the previous response"),
369-
),
381+
WithGraphQLPagination(),
370382
),
371383
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
372384
// Decode params
373385
var params struct {
374-
Owner string
375-
Repo string
376-
First int32
377-
Last int32
378-
After string
379-
Before string
386+
Owner string
387+
Repo string
380388
}
381389
if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {
382390
return mcp.NewToolResultError(err.Error()), nil
383391
}
384392

385-
// Validate pagination parameters
386-
if params.First != 0 && params.Last != 0 {
387-
return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil
388-
}
389-
if params.After != "" && params.Before != "" {
390-
return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil
391-
}
392-
if params.After != "" && params.Last != 0 {
393-
return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil
393+
pagination, err := OptionalGraphQLPaginationParams(request)
394+
if err != nil {
395+
return mcp.NewToolResultError(err.Error()), nil
394396
}
395-
if params.Before != "" && params.First != 0 {
396-
return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil
397+
398+
// Use default of 100 if neither first nor last is specified
399+
if pagination.First == nil && pagination.Last == nil {
400+
defaultFirst := int32(100)
401+
pagination.First = &defaultFirst
397402
}
398403

399404
client, err := getGQLClient(ctx)
400405
if err != nil {
401406
return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil
402407
}
408+
403409
var q struct {
404410
Repository struct {
405411
DiscussionCategories struct {
406412
Nodes []struct {
407413
ID githubv4.ID
408414
Name githubv4.String
409415
}
410-
} `graphql:"discussionCategories(first: 100)"`
416+
PageInfo struct {
417+
HasNextPage githubv4.Boolean
418+
EndCursor githubv4.String
419+
}
420+
} `graphql:"discussionCategories(first: $first, after: $after)"`
411421
} `graphql:"repository(owner: $owner, name: $repo)"`
412422
}
413423
vars := map[string]interface{}{
414424
"owner": githubv4.String(params.Owner),
415425
"repo": githubv4.String(params.Repo),
426+
"first": (*githubv4.Int)(pagination.First),
427+
"after": (*githubv4.String)(pagination.After),
416428
}
417429
if err := client.Query(ctx, &q, vars); err != nil {
418430
return mcp.NewToolResultError(err.Error()), nil
419431
}
432+
420433
var categories []map[string]string
421434
for _, c := range q.Repository.DiscussionCategories.Nodes {
422435
categories = append(categories, map[string]string{
423436
"id": fmt.Sprint(c.ID),
424437
"name": string(c.Name),
425438
})
426439
}
427-
out, err := json.Marshal(categories)
440+
441+
result := map[string]interface{}{
442+
"categories": categories,
443+
"pageInfo": map[string]interface{}{
444+
"hasNextPage": bool(q.Repository.DiscussionCategories.PageInfo.HasNextPage),
445+
"endCursor": string(q.Repository.DiscussionCategories.PageInfo.EndCursor),
446+
},
447+
}
448+
449+
out, err := json.Marshal(result)
428450
if err != nil {
429451
return nil, fmt.Errorf("failed to marshal discussion categories: %w", err)
430452
}

‎pkg/github/discussions_test.go‎

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -294,22 +294,18 @@ func Test_GetDiscussionComments(t *testing.T) {
294294
assert.Contains(t, toolDef.InputSchema.Properties, "discussionNumber")
295295
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"})
296296

297-
var q struct {
298-
Repository struct {
299-
Discussion struct {
300-
Comments struct {
301-
Nodes []struct {
302-
Body githubv4.String
303-
}
304-
} `graphql:"comments(first:100)"`
305-
} `graphql:"discussion(number: $discussionNumber)"`
306-
} `graphql:"repository(owner: $owner, name: $repo)"`
307-
}
297+
// Use exact string query that matches implementation output
298+
qGetComments := "query($after:String$discussionNumber:Int!$first:Int$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){comments(first: $first, after: $after){nodes{body},pageInfo{hasNextPage,endCursor}}}}}"
299+
300+
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
308301
vars := map[string]interface{}{
309302
"owner": githubv4.String("owner"),
310303
"repo": githubv4.String("repo"),
311304
"discussionNumber": githubv4.Int(1),
305+
"first": float64(100), // Default value when no pagination specified
306+
"after": nil,
312307
}
308+
313309
mockResponse := githubv4mock.DataResponse(map[string]any{
314310
"repository": map[string]any{
315311
"discussion": map[string]any{
@@ -318,11 +314,15 @@ func Test_GetDiscussionComments(t *testing.T) {
318314
{"body": "This is the first comment"},
319315
{"body": "This is the second comment"},
320316
},
317+
"pageInfo": map[string]any{
318+
"hasNextPage": false,
319+
"endCursor": "",
320+
},
321321
},
322322
},
323323
},
324324
})
325-
matcher := githubv4mock.NewQueryMatcher(q, vars, mockResponse)
325+
matcher := githubv4mock.NewQueryMatcher(qGetComments, vars, mockResponse)
326326
httpClient := githubv4mock.NewMockedHTTPClient(matcher)
327327
gqlClient := githubv4.NewClient(httpClient)
328328
_, handler := GetDiscussionComments(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper)
@@ -338,42 +338,47 @@ func Test_GetDiscussionComments(t *testing.T) {
338338

339339
textContent := getTextResult(t, result)
340340

341-
var returnedComments []*github.IssueComment
342-
err = json.Unmarshal([]byte(textContent.Text), &returnedComments)
341+
var response struct {
342+
Comments []*github.IssueComment `json:"comments"`
343+
PageInfo map[string]interface{} `json:"pageInfo"`
344+
}
345+
err = json.Unmarshal([]byte(textContent.Text), &response)
343346
require.NoError(t, err)
344-
assert.Len(t, returnedComments, 2)
347+
assert.Len(t, response.Comments, 2)
345348
expectedBodies := []string{"This is the first comment", "This is the second comment"}
346-
for i, comment := range returnedComments {
349+
for i, comment := range response.Comments {
347350
assert.Equal(t, expectedBodies[i], *comment.Body)
348351
}
352+
assert.Equal(t, false, response.PageInfo["hasNextPage"])
349353
}
350354

351355
func Test_ListDiscussionCategories(t *testing.T) {
352-
var q struct {
353-
Repository struct {
354-
DiscussionCategories struct {
355-
Nodes []struct {
356-
ID githubv4.ID
357-
Name githubv4.String
358-
}
359-
} `graphql:"discussionCategories(first: 100)"`
360-
} `graphql:"repository(owner: $owner, name: $repo)"`
361-
}
356+
// Use exact string query that matches implementation output
357+
qListCategories := "query($after:String$first:Int$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first, after: $after){nodes{id,name},pageInfo{hasNextPage,endCursor}}}}"
358+
359+
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
362360
vars := map[string]interface{}{
363361
"owner": githubv4.String("owner"),
364362
"repo": githubv4.String("repo"),
363+
"first": float64(100), // Default value when no pagination specified
364+
"after": nil,
365365
}
366+
366367
mockResp := githubv4mock.DataResponse(map[string]any{
367368
"repository": map[string]any{
368369
"discussionCategories": map[string]any{
369370
"nodes": []map[string]any{
370371
{"id": "123", "name": "CategoryOne"},
371372
{"id": "456", "name": "CategoryTwo"},
372373
},
374+
"pageInfo": map[string]any{
375+
"hasNextPage": false,
376+
"endCursor": "",
377+
},
373378
},
374379
},
375380
})
376-
matcher := githubv4mock.NewQueryMatcher(q, vars, mockResp)
381+
matcher := githubv4mock.NewQueryMatcher(qListCategories, vars, mockResp)
377382
httpClient := githubv4mock.NewMockedHTTPClient(matcher)
378383
gqlClient := githubv4.NewClient(httpClient)
379384

@@ -389,11 +394,15 @@ func Test_ListDiscussionCategories(t *testing.T) {
389394
require.NoError(t, err)
390395

391396
text := getTextResult(t, result).Text
392-
var categories []map[string]string
393-
require.NoError(t, json.Unmarshal([]byte(text), &categories))
394-
assert.Len(t, categories, 2)
395-
assert.Equal(t, "123", categories[0]["id"])
396-
assert.Equal(t, "CategoryOne", categories[0]["name"])
397-
assert.Equal(t, "456", categories[1]["id"])
398-
assert.Equal(t, "CategoryTwo", categories[1]["name"])
397+
var response struct {
398+
Categories []map[string]string `json:"categories"`
399+
PageInfo map[string]interface{} `json:"pageInfo"`
400+
}
401+
require.NoError(t, json.Unmarshal([]byte(text), &response))
402+
assert.Len(t, response.Categories, 2)
403+
assert.Equal(t, "123", response.Categories[0]["id"])
404+
assert.Equal(t, "CategoryOne", response.Categories[0]["name"])
405+
assert.Equal(t, "456", response.Categories[1]["id"])
406+
assert.Equal(t, "CategoryTwo", response.Categories[1]["name"])
407+
assert.Equal(t, false, response.PageInfo["hasNextPage"])
399408
}

0 commit comments

Comments
 (0)