Skip to content

Commit 80b1e05

Browse files
tommaso-moroLuluBeatson
tommaso-moro
and
LuluBeatson
authored
feat: list_discussions sort by updatedAt & createdAt, return updatedAt and author (github#690)
* added updatedAt and Author (aka User) login to query and payload * added initial support for orderby and direction * sort by created at instead of updated at by default * remove unused code * refactor to map to most suitable query based on user inputs at runtime * updated readme with new description * restore original categoryID code, simplify vars management * quick fix * update tests to account for recent changes (author login, updated at date) * use switch statement for better readability * remove comment * linting * refactored logic, simplified switch statement * linting * use original queries from discussions list tool for testing * linting * remove logging * Complete merge by re-introducing pagination to ListDiscussions * fix unit tests * refactor: less repetitive interface --------- Co-authored-by: LuluBeatson <[email protected]> Co-authored-by: Lulu <[email protected]>
1 parent 445f958 commit 80b1e05

File tree

3 files changed

+396
-153
lines changed

3 files changed

+396
-153
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,8 @@ The following sets of tools are available (all are on by default):
462462
- **list_discussions** - List discussions
463463
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
464464
- `category`: Optional filter by discussion category ID. If provided, only discussions with this category are listed. (string, optional)
465+
- `direction`: Order direction. (string, optional)
466+
- `orderBy`: Order discussions by field. If provided, the 'direction' also needs to be provided. (string, optional)
465467
- `owner`: Repository owner (string, required)
466468
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
467469
- `repo`: Repository name (string, required)

‎pkg/github/discussions.go‎

Lines changed: 169 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,108 @@ import (
1515

1616
const DefaultGraphQLPageSize = 30
1717

18+
// Common interface for all discussion query types
19+
type DiscussionQueryResult interface {
20+
GetDiscussionFragment() DiscussionFragment
21+
}
22+
23+
// Implement the interface for all query types
24+
func (q *BasicNoOrder) GetDiscussionFragment() DiscussionFragment {
25+
return q.Repository.Discussions
26+
}
27+
28+
func (q *BasicWithOrder) GetDiscussionFragment() DiscussionFragment {
29+
return q.Repository.Discussions
30+
}
31+
32+
func (q *WithCategoryAndOrder) GetDiscussionFragment() DiscussionFragment {
33+
return q.Repository.Discussions
34+
}
35+
36+
func (q *WithCategoryNoOrder) GetDiscussionFragment() DiscussionFragment {
37+
return q.Repository.Discussions
38+
}
39+
40+
type DiscussionFragment struct {
41+
Nodes []NodeFragment
42+
PageInfo PageInfoFragment
43+
TotalCount githubv4.Int
44+
}
45+
46+
type NodeFragment struct {
47+
Number githubv4.Int
48+
Title githubv4.String
49+
CreatedAt githubv4.DateTime
50+
UpdatedAt githubv4.DateTime
51+
Author struct {
52+
Login githubv4.String
53+
}
54+
Category struct {
55+
Name githubv4.String
56+
} `graphql:"category"`
57+
URL githubv4.String `graphql:"url"`
58+
}
59+
60+
type PageInfoFragment struct {
61+
HasNextPage bool
62+
HasPreviousPage bool
63+
StartCursor githubv4.String
64+
EndCursor githubv4.String
65+
}
66+
67+
type BasicNoOrder struct {
68+
Repository struct {
69+
Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after)"`
70+
} `graphql:"repository(owner: $owner, name: $repo)"`
71+
}
72+
73+
type BasicWithOrder struct {
74+
Repository struct {
75+
Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection })"`
76+
} `graphql:"repository(owner: $owner, name: $repo)"`
77+
}
78+
79+
type WithCategoryAndOrder struct {
80+
Repository struct {
81+
Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection })"`
82+
} `graphql:"repository(owner: $owner, name: $repo)"`
83+
}
84+
85+
type WithCategoryNoOrder struct {
86+
Repository struct {
87+
Discussions DiscussionFragment `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId)"`
88+
} `graphql:"repository(owner: $owner, name: $repo)"`
89+
}
90+
91+
func fragmentToDiscussion(fragment NodeFragment) *github.Discussion {
92+
return &github.Discussion{
93+
Number: github.Ptr(int(fragment.Number)),
94+
Title: github.Ptr(string(fragment.Title)),
95+
HTMLURL: github.Ptr(string(fragment.URL)),
96+
CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time},
97+
UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time},
98+
User: &github.User{
99+
Login: github.Ptr(string(fragment.Author.Login)),
100+
},
101+
DiscussionCategory: &github.DiscussionCategory{
102+
Name: github.Ptr(string(fragment.Category.Name)),
103+
},
104+
}
105+
}
106+
107+
func getQueryType(useOrdering bool, categoryID *githubv4.ID) any {
108+
if categoryID != nil && useOrdering {
109+
return &WithCategoryAndOrder{}
110+
}
111+
if categoryID != nil && !useOrdering {
112+
return &WithCategoryNoOrder{}
113+
}
114+
if categoryID == nil && useOrdering {
115+
return &BasicWithOrder{}
116+
}
117+
return &BasicNoOrder{}
118+
}
119+
18120
func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
19121
return mcp.NewTool("list_discussions",
20122
mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")),
@@ -33,10 +135,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
33135
mcp.WithString("category",
34136
mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."),
35137
),
138+
mcp.WithString("orderBy",
139+
mcp.Description("Order discussions by field. If provided, the 'direction' also needs to be provided."),
140+
mcp.Enum("CREATED_AT", "UPDATED_AT"),
141+
),
142+
mcp.WithString("direction",
143+
mcp.Description("Order direction."),
144+
mcp.Enum("ASC", "DESC"),
145+
),
36146
WithCursorPagination(),
37147
),
38148
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
39-
// Required params
40149
owner, err := RequiredParam[string](request, "owner")
41150
if err != nil {
42151
return mcp.NewToolResultError(err.Error()), nil
@@ -46,12 +155,21 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
46155
return mcp.NewToolResultError(err.Error()), nil
47156
}
48157

49-
// Optional params
50158
category, err := OptionalParam[string](request, "category")
51159
if err != nil {
52160
return mcp.NewToolResultError(err.Error()), nil
53161
}
54162

163+
orderBy, err := OptionalParam[string](request, "orderBy")
164+
if err != nil {
165+
return mcp.NewToolResultError(err.Error()), nil
166+
}
167+
168+
direction, err := OptionalParam[string](request, "direction")
169+
if err != nil {
170+
return mcp.NewToolResultError(err.Error()), nil
171+
}
172+
55173
// Get pagination parameters and convert to GraphQL format
56174
pagination, err := OptionalCursorPaginationParams(request)
57175
if err != nil {
@@ -67,155 +185,69 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
67185
return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil
68186
}
69187

70-
// If category filter is specified, use it as the category ID for server-side filtering
71188
var categoryID *githubv4.ID
72189
if category != "" {
73190
id := githubv4.ID(category)
74191
categoryID = &id
75192
}
76193

77-
var out []byte
78-
79-
var discussions []*github.Discussion
80-
if categoryID != nil {
81-
// Query with category filter (server-side filtering)
82-
var query struct {
83-
Repository struct {
84-
Discussions struct {
85-
Nodes []struct {
86-
Number githubv4.Int
87-
Title githubv4.String
88-
CreatedAt githubv4.DateTime
89-
Category struct {
90-
Name githubv4.String
91-
} `graphql:"category"`
92-
URL githubv4.String `graphql:"url"`
93-
}
94-
PageInfo struct {
95-
HasNextPage bool
96-
HasPreviousPage bool
97-
StartCursor string
98-
EndCursor string
99-
}
100-
TotalCount int
101-
} `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId)"`
102-
} `graphql:"repository(owner: $owner, name: $repo)"`
103-
}
104-
vars := map[string]interface{}{
105-
"owner": githubv4.String(owner),
106-
"repo": githubv4.String(repo),
107-
"categoryId": *categoryID,
108-
"first": githubv4.Int(*paginationParams.First),
109-
}
110-
if paginationParams.After != nil {
111-
vars["after"] = githubv4.String(*paginationParams.After)
112-
} else {
113-
vars["after"] = (*githubv4.String)(nil)
114-
}
115-
if err := client.Query(ctx, &query, vars); err != nil {
116-
return mcp.NewToolResultError(err.Error()), nil
117-
}
118-
119-
// Map nodes to GitHub Discussion objects
120-
for _, n := range query.Repository.Discussions.Nodes {
121-
di := &github.Discussion{
122-
Number: github.Ptr(int(n.Number)),
123-
Title: github.Ptr(string(n.Title)),
124-
HTMLURL: github.Ptr(string(n.URL)),
125-
CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time},
126-
DiscussionCategory: &github.DiscussionCategory{
127-
Name: github.Ptr(string(n.Category.Name)),
128-
},
129-
}
130-
discussions = append(discussions, di)
131-
}
194+
vars := map[string]interface{}{
195+
"owner": githubv4.String(owner),
196+
"repo": githubv4.String(repo),
197+
"first": githubv4.Int(*paginationParams.First),
198+
}
199+
if paginationParams.After != nil {
200+
vars["after"] = githubv4.String(*paginationParams.After)
201+
} else {
202+
vars["after"] = (*githubv4.String)(nil)
203+
}
132204

133-
// Create response with pagination info
134-
response := map[string]interface{}{
135-
"discussions": discussions,
136-
"pageInfo": map[string]interface{}{
137-
"hasNextPage": query.Repository.Discussions.PageInfo.HasNextPage,
138-
"hasPreviousPage": query.Repository.Discussions.PageInfo.HasPreviousPage,
139-
"startCursor": query.Repository.Discussions.PageInfo.StartCursor,
140-
"endCursor": query.Repository.Discussions.PageInfo.EndCursor,
141-
},
142-
"totalCount": query.Repository.Discussions.TotalCount,
143-
}
205+
// this is an extra check in case the tool description is misinterpreted, because
206+
// we shouldn't use ordering unless both a 'field' and 'direction' are provided
207+
useOrdering := orderBy != "" && direction != ""
208+
if useOrdering {
209+
vars["orderByField"] = githubv4.DiscussionOrderField(orderBy)
210+
vars["orderByDirection"] = githubv4.OrderDirection(direction)
211+
}
144212

145-
out, err = json.Marshal(response)
146-
if err != nil {
147-
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
148-
}
149-
} else {
150-
// Query without category filter
151-
var query struct {
152-
Repository struct {
153-
Discussions struct {
154-
Nodes []struct {
155-
Number githubv4.Int
156-
Title githubv4.String
157-
CreatedAt githubv4.DateTime
158-
Category struct {
159-
Name githubv4.String
160-
} `graphql:"category"`
161-
URL githubv4.String `graphql:"url"`
162-
}
163-
PageInfo struct {
164-
HasNextPage bool
165-
HasPreviousPage bool
166-
StartCursor string
167-
EndCursor string
168-
}
169-
TotalCount int
170-
} `graphql:"discussions(first: $first, after: $after)"`
171-
} `graphql:"repository(owner: $owner, name: $repo)"`
172-
}
173-
vars := map[string]interface{}{
174-
"owner": githubv4.String(owner),
175-
"repo": githubv4.String(repo),
176-
"first": githubv4.Int(*paginationParams.First),
177-
}
178-
if paginationParams.After != nil {
179-
vars["after"] = githubv4.String(*paginationParams.After)
180-
} else {
181-
vars["after"] = (*githubv4.String)(nil)
182-
}
183-
if err := client.Query(ctx, &query, vars); err != nil {
184-
return mcp.NewToolResultError(err.Error()), nil
185-
}
213+
if categoryID != nil {
214+
vars["categoryId"] = *categoryID
215+
}
186216

187-
// Map nodes to GitHub Discussion objects
188-
for _, n := range query.Repository.Discussions.Nodes {
189-
di := &github.Discussion{
190-
Number: github.Ptr(int(n.Number)),
191-
Title: github.Ptr(string(n.Title)),
192-
HTMLURL: github.Ptr(string(n.URL)),
193-
CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time},
194-
DiscussionCategory: &github.DiscussionCategory{
195-
Name: github.Ptr(string(n.Category.Name)),
196-
},
197-
}
198-
discussions = append(discussions, di)
199-
}
217+
discussionQuery := getQueryType(useOrdering, categoryID)
218+
if err := client.Query(ctx, discussionQuery, vars); err != nil {
219+
return mcp.NewToolResultError(err.Error()), nil
220+
}
200221

201-
// Create response with pagination info
202-
response := map[string]interface{}{
203-
"discussions": discussions,
204-
"pageInfo": map[string]interface{}{
205-
"hasNextPage": query.Repository.Discussions.PageInfo.HasNextPage,
206-
"hasPreviousPage": query.Repository.Discussions.PageInfo.HasPreviousPage,
207-
"startCursor": query.Repository.Discussions.PageInfo.StartCursor,
208-
"endCursor": query.Repository.Discussions.PageInfo.EndCursor,
209-
},
210-
"totalCount": query.Repository.Discussions.TotalCount,
222+
// Extract and convert all discussion nodes using the common interface
223+
var discussions []*github.Discussion
224+
var pageInfo PageInfoFragment
225+
var totalCount githubv4.Int
226+
if queryResult, ok := discussionQuery.(DiscussionQueryResult); ok {
227+
fragment := queryResult.GetDiscussionFragment()
228+
for _, node := range fragment.Nodes {
229+
discussions = append(discussions, fragmentToDiscussion(node))
211230
}
231+
pageInfo = fragment.PageInfo
232+
totalCount = fragment.TotalCount
233+
}
212234

213-
out, err = json.Marshal(response)
214-
if err != nil {
215-
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
216-
}
235+
// Create response with pagination info
236+
response := map[string]interface{}{
237+
"discussions": discussions,
238+
"pageInfo": map[string]interface{}{
239+
"hasNextPage": pageInfo.HasNextPage,
240+
"hasPreviousPage": pageInfo.HasPreviousPage,
241+
"startCursor": string(pageInfo.StartCursor),
242+
"endCursor": string(pageInfo.EndCursor),
243+
},
244+
"totalCount": totalCount,
217245
}
218246

247+
out, err := json.Marshal(response)
248+
if err != nil {
249+
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
250+
}
219251
return mcp.NewToolResultText(string(out)), nil
220252
}
221253
}

0 commit comments

Comments
 (0)