Skip to content

Commit e10d76a

Browse files
xcorailxcorail
committed
Add missing pagination parameters to ListDiscussions
1 parent 2f90a60 commit e10d76a

3 files changed

Lines changed: 86 additions & 2 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,9 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
623623
- `categoryId`: Filter by category ID (string, optional)
624624
- `since`: Filter by date (ISO 8601 timestamp) (string, optional)
625625
- `first`: Pagination - Number of records to retrieve (number, optional)
626+
- `last`: Pagination - Number of records to retrieve from the end (number, optional)
626627
- `after`: Pagination - Cursor to start with (string, optional)
628+
- `before`: Pagination - Cursor to end with (string, optional)
627629
- `sort`: Sort by ('CREATED_AT', 'UPDATED_AT') (string, optional)
628630
- `direction`: Sort direction ('ASC', 'DESC') (string, optional)
629631

‎pkg/github/discussions.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
5050
mcp.Min(1),
5151
mcp.Max(100),
5252
),
53+
mcp.WithNumber("last",
54+
mcp.Description("Number of discussions to return from the end (min 1, max 100)"),
55+
mcp.Min(1),
56+
mcp.Max(100),
57+
),
5358
mcp.WithString("after",
5459
mcp.Description("Cursor for pagination, use the 'after' field from the previous response"),
5560
),
61+
mcp.WithString("before",
62+
mcp.Description("Cursor for pagination, use the 'before' field from the previous response"),
63+
),
5664
),
5765
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
5866
// Decode params
@@ -64,11 +72,25 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
6472
Sort string
6573
Direction string
6674
First int32
75+
Last int32
6776
After string
77+
Before string
6878
}
6979
if err := mapstructure.Decode(request.Params.Arguments, &params); err != nil {
7080
return mcp.NewToolResultError(err.Error()), nil
7181
}
82+
if params.First != 0 && params.Last != 0 {
83+
return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil
84+
}
85+
if params.After != "" && params.Before != "" {
86+
return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil
87+
}
88+
if params.After != "" && params.Last != 0 {
89+
return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil
90+
}
91+
if params.Before != "" && params.First != 0 {
92+
return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil
93+
}
7294
// Get GraphQL client
7395
client, err := getGQLClient(ctx)
7496
if err != nil {
@@ -87,7 +109,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
87109
} `graphql:"category"`
88110
URL githubv4.String `graphql:"url"`
89111
}
90-
} `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"`
112+
} `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"`
91113
} `graphql:"repository(owner: $owner, name: $repo)"`
92114
}
93115
// Build query variables
@@ -98,7 +120,9 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
98120
"sort": githubv4.DiscussionOrderField(params.Sort),
99121
"direction": githubv4.OrderDirection(params.Direction),
100122
"first": githubv4.Int(params.First),
123+
"last": githubv4.Int(params.Last),
101124
"after": githubv4.String(params.After),
125+
"before": githubv4.String(params.Before),
102126
}
103127
// Execute query
104128
if err := client.Query(ctx, &q, vars); err != nil {

‎pkg/github/discussions_test.go‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func Test_ListDiscussions(t *testing.T) {
5555
} `graphql:"category"`
5656
URL githubv4.String `graphql:"url"`
5757
}
58-
} `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"`
58+
} `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"`
5959
} `graphql:"repository(owner: $owner, name: $repo)"`
6060
}
6161

@@ -66,7 +66,9 @@ func Test_ListDiscussions(t *testing.T) {
6666
"sort": githubv4.DiscussionOrderField(""),
6767
"direction": githubv4.OrderDirection(""),
6868
"first": githubv4.Int(0),
69+
"last": githubv4.Int(0),
6970
"after": githubv4.String(""),
71+
"before": githubv4.String(""),
7072
}
7173

7274
varsListInvalid := map[string]interface{}{
@@ -76,7 +78,9 @@ func Test_ListDiscussions(t *testing.T) {
7678
"sort": githubv4.DiscussionOrderField(""),
7779
"direction": githubv4.OrderDirection(""),
7880
"first": githubv4.Int(0),
81+
"last": githubv4.Int(0),
7982
"after": githubv4.String(""),
83+
"before": githubv4.String(""),
8084
}
8185

8286
varsListWithCategory := map[string]interface{}{
@@ -86,7 +90,9 @@ func Test_ListDiscussions(t *testing.T) {
8690
"sort": githubv4.DiscussionOrderField(""),
8791
"direction": githubv4.OrderDirection(""),
8892
"first": githubv4.Int(0),
93+
"last": githubv4.Int(0),
8994
"after": githubv4.String(""),
95+
"before": githubv4.String(""),
9096
}
9197

9298
tests := []struct {
@@ -144,6 +150,58 @@ func Test_ListDiscussions(t *testing.T) {
144150
expectError: false,
145151
expectedIds: []int64{2, 3},
146152
},
153+
{
154+
name: "both first and last parameters provided",
155+
vars: varsListAll, // vars don't matter since error occurs before GraphQL call
156+
reqParams: map[string]interface{}{
157+
"owner": "owner",
158+
"repo": "repo",
159+
"first": int32(10),
160+
"last": int32(5),
161+
},
162+
response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call
163+
expectError: true,
164+
errContains: "only one of 'first' or 'last' may be specified",
165+
},
166+
{
167+
name: "after with last parameters provided",
168+
vars: varsListAll, // vars don't matter since error occurs before GraphQL call
169+
reqParams: map[string]interface{}{
170+
"owner": "owner",
171+
"repo": "repo",
172+
"after": "cursor123",
173+
"last": int32(5),
174+
},
175+
response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call
176+
expectError: true,
177+
errContains: "'after' cannot be used with 'last'. Did you mean to use 'before' instead?",
178+
},
179+
{
180+
name: "before with first parameters provided",
181+
vars: varsListAll, // vars don't matter since error occurs before GraphQL call
182+
reqParams: map[string]interface{}{
183+
"owner": "owner",
184+
"repo": "repo",
185+
"before": "cursor456",
186+
"first": int32(10),
187+
},
188+
response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call
189+
expectError: true,
190+
errContains: "'before' cannot be used with 'first'. Did you mean to use 'after' instead?",
191+
},
192+
{
193+
name: "both after and before parameters provided",
194+
vars: varsListAll, // vars don't matter since error occurs before GraphQL call
195+
reqParams: map[string]interface{}{
196+
"owner": "owner",
197+
"repo": "repo",
198+
"after": "cursor123",
199+
"before": "cursor456",
200+
},
201+
response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call
202+
expectError: true,
203+
errContains: "only one of 'after' or 'before' may be specified",
204+
},
147205
}
148206

149207
for _, tc := range tests {

0 commit comments

Comments
 (0)