Skip to content

Commit 9d1a665

Browse files
LuluBeatsonLuluBeatson
committed
Complete merge by re-introducing pagination to ListDiscussions
1 parent 9f38405 commit 9d1a665

2 files changed

Lines changed: 188 additions & 40 deletions

File tree

‎pkg/github/discussions.go‎

Lines changed: 116 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,62 @@ import (
1515

1616
const DefaultGraphQLPageSize = 30
1717

18+
// Common interface for all discussion query types
19+
type DiscussionQueryResult interface {
20+
GetDiscussionNodes() []DiscussionFragment
21+
GetPageInfo() PageInfoFragment
22+
GetTotalCount() githubv4.Int
23+
}
24+
25+
// Implement the interface for all query types
26+
func (q *BasicNoOrder) GetDiscussionNodes() []DiscussionFragment {
27+
return q.Repository.Discussions.Nodes
28+
}
29+
30+
func (q *BasicNoOrder) GetPageInfo() PageInfoFragment {
31+
return q.Repository.Discussions.PageInfo
32+
}
33+
34+
func (q *BasicNoOrder) GetTotalCount() githubv4.Int {
35+
return q.Repository.Discussions.TotalCount
36+
}
37+
38+
func (q *BasicWithOrder) GetDiscussionNodes() []DiscussionFragment {
39+
return q.Repository.Discussions.Nodes
40+
}
41+
42+
func (q *BasicWithOrder) GetPageInfo() PageInfoFragment {
43+
return q.Repository.Discussions.PageInfo
44+
}
45+
46+
func (q *BasicWithOrder) GetTotalCount() githubv4.Int {
47+
return q.Repository.Discussions.TotalCount
48+
}
49+
50+
func (q *WithCategoryAndOrder) GetDiscussionNodes() []DiscussionFragment {
51+
return q.Repository.Discussions.Nodes
52+
}
53+
54+
func (q *WithCategoryAndOrder) GetPageInfo() PageInfoFragment {
55+
return q.Repository.Discussions.PageInfo
56+
}
57+
58+
func (q *WithCategoryAndOrder) GetTotalCount() githubv4.Int {
59+
return q.Repository.Discussions.TotalCount
60+
}
61+
62+
func (q *WithCategoryNoOrder) GetDiscussionNodes() []DiscussionFragment {
63+
return q.Repository.Discussions.Nodes
64+
}
65+
66+
func (q *WithCategoryNoOrder) GetPageInfo() PageInfoFragment {
67+
return q.Repository.Discussions.PageInfo
68+
}
69+
70+
func (q *WithCategoryNoOrder) GetTotalCount() githubv4.Int {
71+
return q.Repository.Discussions.TotalCount
72+
}
73+
1874
type DiscussionFragment struct {
1975
Number githubv4.Int
2076
Title githubv4.String
@@ -29,35 +85,50 @@ type DiscussionFragment struct {
2985
URL githubv4.String `graphql:"url"`
3086
}
3187

88+
type PageInfoFragment struct {
89+
HasNextPage bool
90+
HasPreviousPage bool
91+
StartCursor githubv4.String
92+
EndCursor githubv4.String
93+
}
94+
3295
type BasicNoOrder struct {
3396
Repository struct {
3497
Discussions struct {
35-
Nodes []DiscussionFragment
36-
} `graphql:"discussions(first: 100)"`
98+
Nodes []DiscussionFragment
99+
PageInfo PageInfoFragment
100+
TotalCount githubv4.Int
101+
} `graphql:"discussions(first: $first, after: $after)"`
37102
} `graphql:"repository(owner: $owner, name: $repo)"`
38103
}
39104

40105
type BasicWithOrder struct {
41106
Repository struct {
42107
Discussions struct {
43-
Nodes []DiscussionFragment
44-
} `graphql:"discussions(first: 100, orderBy: { field: $orderByField, direction: $orderByDirection })"`
108+
Nodes []DiscussionFragment
109+
PageInfo PageInfoFragment
110+
TotalCount githubv4.Int
111+
} `graphql:"discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection })"`
45112
} `graphql:"repository(owner: $owner, name: $repo)"`
46113
}
47114

48115
type WithCategoryAndOrder struct {
49116
Repository struct {
50117
Discussions struct {
51-
Nodes []DiscussionFragment
52-
} `graphql:"discussions(first: 100, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection })"`
118+
Nodes []DiscussionFragment
119+
PageInfo PageInfoFragment
120+
TotalCount githubv4.Int
121+
} `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection })"`
53122
} `graphql:"repository(owner: $owner, name: $repo)"`
54123
}
55124

56125
type WithCategoryNoOrder struct {
57126
Repository struct {
58127
Discussions struct {
59-
Nodes []DiscussionFragment
60-
} `graphql:"discussions(first: 100, categoryId: $categoryId)"`
128+
Nodes []DiscussionFragment
129+
PageInfo PageInfoFragment
130+
TotalCount githubv4.Int
131+
} `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId)"`
61132
} `graphql:"repository(owner: $owner, name: $repo)"`
62133
}
63134

@@ -116,6 +187,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
116187
mcp.Description("Order direction."),
117188
mcp.Enum("ASC", "DESC"),
118189
),
190+
WithCursorPagination(),
119191
),
120192
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
121193
owner, err := RequiredParam[string](request, "owner")
@@ -142,6 +214,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
142214
return mcp.NewToolResultError(err.Error()), nil
143215
}
144216

217+
// Get pagination parameters and convert to GraphQL format
218+
pagination, err := OptionalCursorPaginationParams(request)
219+
if err != nil {
220+
return nil, err
221+
}
222+
paginationParams, err := pagination.ToGraphQLParams()
223+
if err != nil {
224+
return nil, err
225+
}
226+
145227
client, err := getGQLClient(ctx)
146228
if err != nil {
147229
return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil
@@ -156,6 +238,12 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
156238
vars := map[string]interface{}{
157239
"owner": githubv4.String(owner),
158240
"repo": githubv4.String(repo),
241+
"first": githubv4.Int(*paginationParams.First),
242+
}
243+
if paginationParams.After != nil {
244+
vars["after"] = githubv4.String(*paginationParams.After)
245+
} else {
246+
vars["after"] = (*githubv4.String)(nil)
159247
}
160248

161249
// this is an extra check in case the tool description is misinterpreted, because
@@ -170,38 +258,36 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
170258
vars["categoryId"] = *categoryID
171259
}
172260

173-
var discussions []*github.Discussion
174261
discussionQuery := getQueryType(useOrdering, categoryID)
175-
176262
if err := client.Query(ctx, discussionQuery, vars); err != nil {
177263
return mcp.NewToolResultError(err.Error()), nil
178264
}
179265

180-
// we need to check what user inputs we received at runtime, and use the
181-
// most appropriate query based on that
182-
switch queryType := discussionQuery.(type) {
183-
case *WithCategoryAndOrder:
184-
for _, node := range queryType.Repository.Discussions.Nodes {
185-
discussions = append(discussions, fragmentToDiscussion(node))
186-
}
187-
188-
case *WithCategoryNoOrder:
189-
for _, node := range queryType.Repository.Discussions.Nodes {
190-
discussions = append(discussions, fragmentToDiscussion(node))
191-
}
192-
193-
case *BasicWithOrder:
194-
for _, node := range queryType.Repository.Discussions.Nodes {
266+
// Extract and convert all discussion nodes using the common interface
267+
var discussions []*github.Discussion
268+
var pageInfo PageInfoFragment
269+
var totalCount githubv4.Int
270+
if queryResult, ok := discussionQuery.(DiscussionQueryResult); ok {
271+
for _, node := range queryResult.GetDiscussionNodes() {
195272
discussions = append(discussions, fragmentToDiscussion(node))
196273
}
274+
pageInfo = queryResult.GetPageInfo()
275+
totalCount = queryResult.GetTotalCount()
276+
}
197277

198-
case *BasicNoOrder:
199-
for _, node := range queryType.Repository.Discussions.Nodes {
200-
discussions = append(discussions, fragmentToDiscussion(node))
201-
}
278+
// Create response with pagination info
279+
response := map[string]interface{}{
280+
"discussions": discussions,
281+
"pageInfo": map[string]interface{}{
282+
"hasNextPage": pageInfo.HasNextPage,
283+
"hasPreviousPage": pageInfo.HasPreviousPage,
284+
"startCursor": pageInfo.StartCursor,
285+
"endCursor": pageInfo.EndCursor,
286+
},
287+
"totalCount": totalCount,
202288
}
203289

204-
out, err := json.Marshal(discussions)
290+
out, err := json.Marshal(response)
205291
if err != nil {
206292
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
207293
}

‎pkg/github/discussions_test.go‎

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,27 +71,72 @@ var (
7171

7272
mockResponseListAll = githubv4mock.DataResponse(map[string]any{
7373
"repository": map[string]any{
74-
"discussions": map[string]any{"nodes": discussionsAll},
74+
"discussions": map[string]any{
75+
"nodes": discussionsAll,
76+
"pageInfo": map[string]any{
77+
"hasNextPage": false,
78+
"hasPreviousPage": false,
79+
"startCursor": "",
80+
"endCursor": "",
81+
},
82+
"totalCount": 3,
83+
},
7584
},
7685
})
7786
mockResponseListGeneral = githubv4mock.DataResponse(map[string]any{
7887
"repository": map[string]any{
79-
"discussions": map[string]any{"nodes": discussionsGeneral},
88+
"discussions": map[string]any{
89+
"nodes": discussionsGeneral,
90+
"pageInfo": map[string]any{
91+
"hasNextPage": false,
92+
"hasPreviousPage": false,
93+
"startCursor": "",
94+
"endCursor": "",
95+
},
96+
"totalCount": 2,
97+
},
8098
},
8199
})
82100
mockResponseOrderedCreatedAsc = githubv4mock.DataResponse(map[string]any{
83101
"repository": map[string]any{
84-
"discussions": map[string]any{"nodes": discussionsOrderedCreatedAsc},
102+
"discussions": map[string]any{
103+
"nodes": discussionsOrderedCreatedAsc,
104+
"pageInfo": map[string]any{
105+
"hasNextPage": false,
106+
"hasPreviousPage": false,
107+
"startCursor": "",
108+
"endCursor": "",
109+
},
110+
"totalCount": 3,
111+
},
85112
},
86113
})
87114
mockResponseOrderedUpdatedDesc = githubv4mock.DataResponse(map[string]any{
88115
"repository": map[string]any{
89-
"discussions": map[string]any{"nodes": discussionsOrderedUpdatedDesc},
116+
"discussions": map[string]any{
117+
"nodes": discussionsOrderedUpdatedDesc,
118+
"pageInfo": map[string]any{
119+
"hasNextPage": false,
120+
"hasPreviousPage": false,
121+
"startCursor": "",
122+
"endCursor": "",
123+
},
124+
"totalCount": 3,
125+
},
90126
},
91127
})
92128
mockResponseGeneralOrderedDesc = githubv4mock.DataResponse(map[string]any{
93129
"repository": map[string]any{
94-
"discussions": map[string]any{"nodes": discussionsGeneralOrderedDesc},
130+
"discussions": map[string]any{
131+
"nodes": discussionsGeneralOrderedDesc,
132+
"pageInfo": map[string]any{
133+
"hasNextPage": false,
134+
"hasPreviousPage": false,
135+
"startCursor": "",
136+
"endCursor": "",
137+
},
138+
"totalCount": 2,
139+
},
95140
},
96141
})
97142
mockErrorRepoNotFound = githubv4mock.ErrorResponse("repository not found")
@@ -108,20 +153,27 @@ func Test_ListDiscussions(t *testing.T) {
108153
assert.Contains(t, toolDef.InputSchema.Properties, "direction")
109154
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"})
110155

156+
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
111157
varsListAll := map[string]interface{}{
112158
"owner": githubv4.String("owner"),
113159
"repo": githubv4.String("repo"),
160+
"first": float64(30),
161+
"after": (*string)(nil),
114162
}
115163

116164
varsRepoNotFound := map[string]interface{}{
117165
"owner": githubv4.String("owner"),
118166
"repo": githubv4.String("nonexistent-repo"),
167+
"first": float64(30),
168+
"after": (*string)(nil),
119169
}
120170

121171
varsDiscussionsFiltered := map[string]interface{}{
122172
"owner": githubv4.String("owner"),
123173
"repo": githubv4.String("repo"),
124174
"categoryId": githubv4.ID("DIC_kwDOABC123"),
175+
"first": float64(30),
176+
"after": (*string)(nil),
125177
}
126178

127179
varsOrderByCreatedAsc := map[string]interface{}{
@@ -310,20 +362,30 @@ func Test_ListDiscussions(t *testing.T) {
310362
}
311363
require.NoError(t, err)
312364

313-
var returnedDiscussions []*github.Discussion
314-
err = json.Unmarshal([]byte(text), &returnedDiscussions)
365+
// Parse the structured response with pagination info
366+
var response struct {
367+
Discussions []*github.Discussion `json:"discussions"`
368+
PageInfo struct {
369+
HasNextPage bool `json:"hasNextPage"`
370+
HasPreviousPage bool `json:"hasPreviousPage"`
371+
StartCursor string `json:"startCursor"`
372+
EndCursor string `json:"endCursor"`
373+
} `json:"pageInfo"`
374+
TotalCount int `json:"totalCount"`
375+
}
376+
err = json.Unmarshal([]byte(text), &response)
315377
require.NoError(t, err)
316378

317-
assert.Len(t, returnedDiscussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(returnedDiscussions))
379+
assert.Len(t, response.Discussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(response.Discussions))
318380

319381
// Verify order if verifyOrder function is provided
320382
if tc.verifyOrder != nil {
321-
tc.verifyOrder(t, returnedDiscussions)
383+
tc.verifyOrder(t, response.Discussions)
322384
}
323385

324386
// Verify that all returned discussions have a category if filtered
325387
if _, hasCategory := tc.reqParams["category"]; hasCategory {
326-
for _, discussion := range returnedDiscussions {
388+
for _, discussion := range response.Discussions {
327389
require.NotNil(t, discussion.DiscussionCategory, "Discussion should have category")
328390
assert.NotEmpty(t, *discussion.DiscussionCategory.Name, "Discussion should have category name")
329391
}

0 commit comments

Comments
 (0)