From 90449b51970c05fe07ed5270ee75578c3f8568d4 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Thu, 2 Apr 2026 12:53:30 -0500 Subject: [PATCH 01/21] Implement `gh discussion list` command Add the discussion list command with full support for: - Listing discussions with state, category, answered, and order filters - Searching discussions by author and labels (uses Search API) - Category resolution by slug or name (case-insensitive) - TTY and non-TTY table output with colored IDs and labels - JSON output with totalCount envelope - Web mode (--web) to open discussions in browser - Pagination for large result sets Implement List, Search, and ListCategories methods on the DiscussionClient. List and Search use plain text GraphQL for flexible variable handling. ListCategories uses safely typed GraphQL via shurcooL/githubv4. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 351 ++++++++++++++++- pkg/cmd/discussion/discussion.go | 5 + pkg/cmd/discussion/list/list.go | 311 +++++++++++++++ pkg/cmd/discussion/list/list_test.go | 466 +++++++++++++++++++++++ 4 files changed, 1127 insertions(+), 6 deletions(-) create mode 100644 pkg/cmd/discussion/list/list.go create mode 100644 pkg/cmd/discussion/list/list_test.go diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index e5db32dd5..567cce2af 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -3,9 +3,12 @@ package client import ( "fmt" "net/http" + "strings" + "time" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/shurcooL/githubv4" ) type discussionClient struct { @@ -19,12 +22,314 @@ func NewDiscussionClient(httpClient *http.Client) DiscussionClient { } } -func (c *discussionClient) List(_ ghrepo.Interface, _ ListFilters, _ int) ([]Discussion, int, error) { - return nil, 0, fmt.Errorf("not implemented") +// discussionNode is the shared GraphQL response shape for a single discussion, +// used by both List and Search to avoid duplicating the field mapping. +type discussionNode struct { + ID string `json:"id"` + Number int `json:"number"` + Title string `json:"title"` + URL string `json:"url"` + State string `json:"state"` + StateReason string `json:"stateReason"` + Author struct { + Login string `json:"login"` + } `json:"author"` + Category struct { + ID string `json:"id"` + Name string `json:"name"` + Slug string `json:"slug"` + Emoji string `json:"emoji"` + IsAnswerable bool `json:"isAnswerable"` + } `json:"category"` + Labels struct { + Nodes []struct { + ID string `json:"id"` + Name string `json:"name"` + Color string `json:"color"` + } `json:"nodes"` + } `json:"labels"` + IsAnswered bool `json:"isAnswered"` + AnswerChosenAt time.Time `json:"answerChosenAt"` + AnswerChosenBy *struct { + Login string `json:"login"` + } `json:"answerChosenBy"` + ReactionGroups []struct { + Content string `json:"content"` + Users struct { + TotalCount int `json:"totalCount"` + } `json:"users"` + } `json:"reactionGroups"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + ClosedAt time.Time `json:"closedAt"` + Locked bool `json:"locked"` } -func (c *discussionClient) Search(_ ghrepo.Interface, _ SearchFilters, _ int) ([]Discussion, int, error) { - return nil, 0, fmt.Errorf("not implemented") +// mapDiscussion converts a GraphQL discussionNode response into the domain Discussion type. +func mapDiscussion(n discussionNode) Discussion { + d := Discussion{ + ID: n.ID, + Number: n.Number, + Title: n.Title, + URL: n.URL, + State: n.State, + StateReason: n.StateReason, + Author: DiscussionAuthor{Login: n.Author.Login}, + Category: DiscussionCategory{ + ID: n.Category.ID, + Name: n.Category.Name, + Slug: n.Category.Slug, + Emoji: n.Category.Emoji, + IsAnswerable: n.Category.IsAnswerable, + }, + Answered: n.IsAnswered, + AnswerChosenAt: n.AnswerChosenAt, + CreatedAt: n.CreatedAt, + UpdatedAt: n.UpdatedAt, + ClosedAt: n.ClosedAt, + Locked: n.Locked, + } + + if n.AnswerChosenBy != nil { + d.AnswerChosenBy = &DiscussionAuthor{Login: n.AnswerChosenBy.Login} + } + + d.Labels = make([]DiscussionLabel, len(n.Labels.Nodes)) + for i, l := range n.Labels.Nodes { + d.Labels[i] = DiscussionLabel{ID: l.ID, Name: l.Name, Color: l.Color} + } + + d.ReactionGroups = make([]ReactionGroup, len(n.ReactionGroups)) + for i, rg := range n.ReactionGroups { + d.ReactionGroups[i] = ReactionGroup{Content: rg.Content, TotalCount: rg.Users.TotalCount} + } + + return d +} + +// discussionFields is the GraphQL fragment selecting fields for discussion queries. +// It is shared by both List (repository.discussions) and Search queries. +const discussionFields = ` + id number title url state stateReason + author { login } + category { id name slug emoji isAnswerable } + labels(first: 20) { nodes { id name color } } + isAnswered answerChosenAt answerChosenBy { login } + reactionGroups { content users { totalCount } } + createdAt updatedAt closedAt locked +` + +func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) { + type response struct { + Repository struct { + HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` + Discussions struct { + TotalCount int `json:"totalCount"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + Nodes []discussionNode `json:"nodes"` + } `json:"discussions"` + } `json:"repository"` + } + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + } + + orderField := "UPDATED_AT" + orderDir := "DESC" + if filters.OrderBy != "" { + orderField = strings.ToUpper(filters.OrderBy) + "_AT" + } + if filters.Direction != "" { + orderDir = strings.ToUpper(filters.Direction) + } + variables["orderBy"] = map[string]string{ + "field": orderField, + "direction": orderDir, + } + + if filters.CategoryID != "" { + variables["categoryId"] = filters.CategoryID + } + + switch strings.ToLower(filters.State) { + case "open": + variables["states"] = []string{"OPEN"} + case "closed": + variables["states"] = []string{"CLOSED"} + } + + if filters.Answered != nil { + variables["answered"] = *filters.Answered + } + + // Build optional parameter declarations + paramParts := []string{ + "$owner: String!", + "$name: String!", + "$first: Int!", + "$after: String", + "$orderBy: DiscussionOrder", + } + argParts := []string{ + "first: $first", + "after: $after", + "orderBy: $orderBy", + } + if filters.CategoryID != "" { + paramParts = append(paramParts, "$categoryId: ID") + argParts = append(argParts, "categoryId: $categoryId") + } + if _, ok := variables["states"]; ok { + paramParts = append(paramParts, "$states: [DiscussionState!]") + argParts = append(argParts, "states: $states") + } + if filters.Answered != nil { + paramParts = append(paramParts, "$answered: Boolean") + argParts = append(argParts, "answered: $answered") + } + + query := fmt.Sprintf(`query DiscussionList(%s) { + repository(owner: $owner, name: $name) { + hasDiscussionsEnabled + discussions(%s) { + totalCount + pageInfo { hasNextPage endCursor } + nodes { %s } + } + } + }`, strings.Join(paramParts, ", "), strings.Join(argParts, ", "), discussionFields) + + var discussions []Discussion + var totalCount int + pageLimit := limit + + for { + perPage := pageLimit + if perPage > 100 { + perPage = 100 + } + variables["first"] = perPage + + var data response + if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { + return nil, 0, err + } + + if !data.Repository.HasDiscussionsEnabled { + return nil, 0, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + } + + totalCount = data.Repository.Discussions.TotalCount + for _, n := range data.Repository.Discussions.Nodes { + discussions = append(discussions, mapDiscussion(n)) + } + + pageLimit -= len(data.Repository.Discussions.Nodes) + if pageLimit <= 0 || !data.Repository.Discussions.PageInfo.HasNextPage { + break + } + variables["after"] = data.Repository.Discussions.PageInfo.EndCursor + } + + return discussions, totalCount, nil +} + +func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) { + type response struct { + Search struct { + DiscussionCount int `json:"discussionCount"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + Nodes []discussionNode `json:"nodes"` + } `json:"search"` + } + + searchTerms := []string{fmt.Sprintf("repo:%s/%s", repo.RepoOwner(), repo.RepoName())} + + switch strings.ToLower(filters.State) { + case "open": + searchTerms = append(searchTerms, "state:open") + case "closed": + searchTerms = append(searchTerms, "state:closed") + } + + if filters.Author != "" { + searchTerms = append(searchTerms, fmt.Sprintf("author:%s", filters.Author)) + } + for _, l := range filters.Labels { + searchTerms = append(searchTerms, fmt.Sprintf("label:%q", l)) + } + if filters.Category != "" { + searchTerms = append(searchTerms, fmt.Sprintf("category:%q", filters.Category)) + } + if filters.Answered != nil { + if *filters.Answered { + searchTerms = append(searchTerms, "is:answered") + } else { + searchTerms = append(searchTerms, "is:unanswered") + } + } + + orderField := "updated" + orderDir := "desc" + if filters.OrderBy != "" { + orderField = strings.ToLower(filters.OrderBy) + } + if filters.Direction != "" { + orderDir = strings.ToLower(filters.Direction) + } + searchTerms = append(searchTerms, fmt.Sprintf("sort:%s-%s", orderField, orderDir)) + + searchQuery := strings.Join(searchTerms, " ") + + query := fmt.Sprintf(`query DiscussionSearch($query: String!, $first: Int!, $after: String) { + search(query: $query, type: DISCUSSION, first: $first, after: $after) { + discussionCount + pageInfo { hasNextPage endCursor } + nodes { ... on Discussion { %s } } + } + }`, discussionFields) + + variables := map[string]interface{}{ + "query": searchQuery, + } + + var discussions []Discussion + var totalCount int + pageLimit := limit + + for { + perPage := pageLimit + if perPage > 100 { + perPage = 100 + } + variables["first"] = perPage + + var data response + if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { + return nil, 0, err + } + + totalCount = data.Search.DiscussionCount + for _, n := range data.Search.Nodes { + discussions = append(discussions, mapDiscussion(n)) + } + + pageLimit -= len(data.Search.Nodes) + if pageLimit <= 0 || !data.Search.PageInfo.HasNextPage { + break + } + variables["after"] = data.Search.PageInfo.EndCursor + } + + return discussions, totalCount, nil } func (c *discussionClient) GetByNumber(_ ghrepo.Interface, _ int) (*Discussion, error) { @@ -35,8 +340,42 @@ func (c *discussionClient) GetWithComments(_ ghrepo.Interface, _ int, _ int, _ s return nil, fmt.Errorf("not implemented") } -func (c *discussionClient) ListCategories(_ ghrepo.Interface) ([]DiscussionCategory, error) { - return nil, fmt.Errorf("not implemented") +func (c *discussionClient) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) { + var query struct { + Repository struct { + DiscussionCategories struct { + Nodes []struct { + ID string + Name string + Slug string + Emoji string + IsAnswerable bool + } + } `graphql:"discussionCategories(first: 100)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + } + + if err := c.gql.Query(repo.RepoHost(), "DiscussionCategoryList", &query, variables); err != nil { + return nil, err + } + + categories := make([]DiscussionCategory, len(query.Repository.DiscussionCategories.Nodes)) + for i, n := range query.Repository.DiscussionCategories.Nodes { + categories[i] = DiscussionCategory{ + ID: n.ID, + Name: n.Name, + Slug: n.Slug, + Emoji: n.Emoji, + IsAnswerable: n.IsAnswerable, + } + } + + return categories, nil } func (c *discussionClient) Create(_ ghrepo.Interface, _ CreateDiscussionInput) (*Discussion, error) { diff --git a/pkg/cmd/discussion/discussion.go b/pkg/cmd/discussion/discussion.go index 6db07c5d8..bd724fe69 100644 --- a/pkg/cmd/discussion/discussion.go +++ b/pkg/cmd/discussion/discussion.go @@ -2,6 +2,7 @@ package discussion import ( "github.com/MakeNowJust/heredoc" + cmdList "github.com/cli/cli/v2/pkg/cmd/discussion/list" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -29,5 +30,9 @@ func NewCmdDiscussion(f *cmdutil.Factory) *cobra.Command { cmdutil.EnableRepoOverride(cmd, f) + cmdutil.AddGroup(cmd, "General commands", + cmdList.NewCmdList(f, nil), + ) + return cmd } diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go new file mode 100644 index 000000000..f82cc9806 --- /dev/null +++ b/pkg/cmd/discussion/list/list.go @@ -0,0 +1,311 @@ +package list + +import ( + "fmt" + "net/url" + "strings" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/tableprinter" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/discussion/client" + "github.com/cli/cli/v2/pkg/cmd/discussion/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +// ListOptions holds the configuration for the discussion list command. +type ListOptions struct { + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + Client func() (client.DiscussionClient, error) + + Author string + Category string + Labels []string + State string + Limit int + Answered *bool + Order string + + WebMode bool + Exporter cmdutil.Exporter + Now func() time.Time +} + +// NewCmdList creates the "discussion list" command. +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + Browser: f.Browser, + Now: time.Now, + } + + cmd := &cobra.Command{ + Use: "list", + Short: "List discussions in a repository", + Long: heredoc.Doc(` + List discussions in a GitHub repository. By default, only open discussions + are shown. + `), + Example: heredoc.Doc(` + # List open discussions + $ gh discussion list + + # List discussions with a specific category + $ gh discussion list --category "General" + + # List closed discussions by author + $ gh discussion list --state closed --author monalisa + + # List answered discussions as JSON + $ gh discussion list --answered --json number,title,url + `), + Aliases: []string{"ls"}, + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + opts.BaseRepo = f.BaseRepo + opts.Client = shared.DiscussionClientFunc(f) + + if opts.Limit < 1 { + return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) + } + + if runF != nil { + return runF(opts) + } + return listRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Author, "author", "A", "", "Filter by author") + cmd.Flags().StringVarP(&opts.Category, "category", "c", "", "Filter by category name or slug") + cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by label") + cmdutil.StringEnumFlag(cmd, &opts.State, "state", "s", "open", []string{"open", "closed", "all"}, "Filter by state") + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of discussions to fetch") + cmdutil.NilBoolFlag(cmd, &opts.Answered, "answered", "", "Filter by answered state") + cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "updated", []string{"created", "updated"}, "Order by field") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List discussions in the web browser") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) + + return cmd +} + +func listRun(opts *ListOptions) error { + repo, err := opts.BaseRepo() + if err != nil { + return err + } + + if opts.WebMode { + return openInBrowser(opts, repo) + } + + dc, err := opts.Client() + if err != nil { + return err + } + + var categoryID string + var categorySlug string + if opts.Category != "" { + categories, err := dc.ListCategories(repo) + if err != nil { + return err + } + cat, err := matchCategory(opts.Category, categories) + if err != nil { + return err + } + categoryID = cat.ID + categorySlug = cat.Slug + } + + var discussions []client.Discussion + var totalCount int + + useSearch := opts.Author != "" || len(opts.Labels) > 0 + if useSearch { + filters := client.SearchFilters{ + Author: opts.Author, + Labels: opts.Labels, + State: opts.State, + Category: categorySlug, + Answered: opts.Answered, + OrderBy: opts.Order, + } + discussions, totalCount, err = dc.Search(repo, filters, opts.Limit) + } else { + filters := client.ListFilters{ + State: opts.State, + CategoryID: categoryID, + Answered: opts.Answered, + OrderBy: opts.Order, + } + discussions, totalCount, err = dc.List(repo, filters, opts.Limit) + } + if err != nil { + return err + } + + if opts.Exporter != nil { + envelope := map[string]interface{}{ + "totalCount": totalCount, + "discussions": discussions, + } + return opts.Exporter.Write(opts.IO, envelope) + } + + if len(discussions) == 0 { + return noResults(repo, opts.State) + } + + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + + isTerminal := opts.IO.IsStdoutTTY() + if isTerminal { + title := listHeader(ghrepo.FullName(repo), len(discussions), totalCount, opts.State) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) + } + + printDiscussions(opts, discussions, totalCount) + return nil +} + +func openInBrowser(opts *ListOptions, repo ghrepo.Interface) error { + discussionsURL := ghrepo.GenerateRepoURL(repo, "discussions") + + var queryParts []string + if opts.State != "" && opts.State != "all" { + queryParts = append(queryParts, "is:"+opts.State) + } + if opts.Author != "" { + queryParts = append(queryParts, "author:"+opts.Author) + } + for _, l := range opts.Labels { + queryParts = append(queryParts, fmt.Sprintf("label:%q", l)) + } + if opts.Category != "" { + queryParts = append(queryParts, fmt.Sprintf("category:%q", opts.Category)) + } + if opts.Answered != nil { + if *opts.Answered { + queryParts = append(queryParts, "is:answered") + } else { + queryParts = append(queryParts, "is:unanswered") + } + } + + if len(queryParts) > 0 { + discussionsURL += "?" + url.Values{"q": {strings.Join(queryParts, " ")}}.Encode() + } + + if opts.IO.IsStderrTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(discussionsURL)) + } + return opts.Browser.Browse(discussionsURL) +} + +func matchCategory(input string, categories []client.DiscussionCategory) (*client.DiscussionCategory, error) { + for i := range categories { + if strings.EqualFold(categories[i].Slug, input) { + return &categories[i], nil + } + } + for i := range categories { + if strings.EqualFold(categories[i].Name, input) { + return &categories[i], nil + } + } + + var available strings.Builder + for _, c := range categories { + fmt.Fprintf(&available, " %s (%s)\n", c.Slug, c.Name) + } + return nil, fmt.Errorf("category not found: %s\n\nAvailable categories:\n%s", input, available.String()) +} + +func noResults(repo ghrepo.Interface, state string) error { + stateQualifier := "" + switch state { + case "open": + stateQualifier = " open" + case "closed": + stateQualifier = " closed" + } + return cmdutil.NewNoResultsError(fmt.Sprintf("no%s discussions match your search in %s", stateQualifier, ghrepo.FullName(repo))) +} + +func listHeader(repoName string, count, total int, state string) string { + stateQualifier := "" + switch state { + case "open": + stateQualifier = " open" + case "closed": + stateQualifier = " closed" + } + return fmt.Sprintf("Showing %d of %d%s discussions in %s", count, total, stateQualifier, repoName) +} + +func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalCount int) { + isTerminal := opts.IO.IsStdoutTTY() + cs := opts.IO.ColorScheme() + now := opts.Now() + + headers := []string{"ID", "TITLE", "CATEGORY", "LABELS", "ANSWERED", "UPDATED"} + if !isTerminal { + headers = []string{"ID", "STATE", "TITLE", "CATEGORY", "LABELS", "ANSWERED", "UPDATED"} + } + tp := tableprinter.New(opts.IO, tableprinter.WithHeader(headers...)) + + for _, d := range discussions { + if isTerminal { + idColor := cs.Green + if strings.EqualFold(d.State, "CLOSED") { + idColor = cs.Gray + } + tp.AddField(fmt.Sprintf("#%d", d.Number), tableprinter.WithColor(idColor)) + } else { + tp.AddField(fmt.Sprintf("%d", d.Number)) + tp.AddField(d.State) + } + + tp.AddField(text.RemoveExcessiveWhitespace(d.Title)) + tp.AddField(d.Category.Name) + + labelNames := make([]string, len(d.Labels)) + for i, l := range d.Labels { + if isTerminal { + labelNames[i] = cs.Label(l.Color, l.Name) + } else { + labelNames[i] = l.Name + } + } + tp.AddField(strings.Join(labelNames, ", "), tableprinter.WithTruncate(nil)) + + if d.Answered { + tp.AddField("✓") + } else { + tp.AddField("") + } + + tp.AddTimeField(now, d.UpdatedAt, cs.Muted) + tp.EndRow() + } + + _ = tp.Render() + + remaining := totalCount - len(discussions) + if remaining > 0 { + fmt.Fprintf(opts.IO.Out, cs.Muted("And %d more\n"), remaining) + } +} diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go new file mode 100644 index 000000000..f4e6f57ab --- /dev/null +++ b/pkg/cmd/discussion/list/list_test.go @@ -0,0 +1,466 @@ +package list + +import ( + "bytes" + "testing" + "time" + + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/discussion/client" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func fixedTime() time.Time { + return time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC) +} + +func sampleDiscussions() []client.Discussion { + return []client.Discussion{ + { + Number: 42, + Title: "Bug report discussion", + URL: "https://github.com/OWNER/REPO/discussions/42", + State: "OPEN", + Author: client.DiscussionAuthor{Login: "monalisa"}, + Category: client.DiscussionCategory{ + ID: "CAT1", + Name: "General", + Slug: "general", + }, + Labels: []client.DiscussionLabel{ + {ID: "L1", Name: "bug", Color: "d73a4a"}, + }, + Answered: true, + UpdatedAt: time.Date(2025, 2, 28, 12, 0, 0, 0, time.UTC), + }, + { + Number: 41, + Title: "Feature request", + URL: "https://github.com/OWNER/REPO/discussions/41", + State: "OPEN", + Author: client.DiscussionAuthor{Login: "octocat"}, + Category: client.DiscussionCategory{ + ID: "CAT2", + Name: "Ideas", + Slug: "ideas", + }, + Labels: []client.DiscussionLabel{}, + Answered: false, + UpdatedAt: time.Date(2025, 2, 20, 12, 0, 0, 0, time.UTC), + }, + } +} + +func sampleCategories() []client.DiscussionCategory { + return []client.DiscussionCategory{ + {ID: "CAT1", Name: "General", Slug: "general", IsAnswerable: true}, + {ID: "CAT2", Name: "Ideas", Slug: "ideas", IsAnswerable: false}, + {ID: "CAT3", Name: "Show and tell", Slug: "show-and-tell", IsAnswerable: false}, + } +} + +func TestListRun_tty(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + return sampleDiscussions(), 2, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + + assert.Equal(t, "", stderr.String()) + out := stdout.String() + assert.Contains(t, out, "Showing 2 of 2 open discussions in OWNER/REPO") + assert.Contains(t, out, "#42") + assert.Contains(t, out, "Bug report discussion") + assert.Contains(t, out, "General") + assert.Contains(t, out, "✓") + assert.Contains(t, out, "#41") + assert.Contains(t, out, "Feature request") +} + +func TestListRun_nontty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + return sampleDiscussions(), 2, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + + out := stdout.String() + // Non-TTY output should not contain # prefix or header + assert.NotContains(t, out, "Showing") + assert.Contains(t, out, "42") + assert.Contains(t, out, "OPEN") + assert.Contains(t, out, "Bug report discussion") +} + +func TestListRun_json(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + return sampleDiscussions(), 2, nil + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields([]string{"number", "title"}) + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "open", + Limit: 30, + Exporter: exporter, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, `"totalCount"`) + assert.Contains(t, out, `"discussions"`) +} + +func TestListRun_web(t *testing.T) { + ios, _, _, stderr := iostreams.Test() + ios.SetStderrTTY(true) + + br := &browser.Stub{} + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Browser: br, + WebMode: true, + State: "open", + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + + assert.Contains(t, stderr.String(), "Opening") + assert.Contains(t, br.BrowsedURL(), "github.com/OWNER/REPO/discussions") +} + +func TestListRun_noResults(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + return []client.Discussion{}, 0, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.Error(t, err) + var noResultsErr cmdutil.NoResultsError + assert.ErrorAs(t, err, &noResultsErr) +} + +func TestListRun_categoryFilter(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + ListCategoriesFunc: func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { + return sampleCategories(), nil + }, + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + assert.Equal(t, "CAT1", filters.CategoryID) + return sampleDiscussions()[:1], 1, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Category: "general", + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "Bug report discussion") +} + +func TestListRun_categoryNotFound(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + mockClient := &client.DiscussionClientMock{ + ListCategoriesFunc: func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { + return sampleCategories(), nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Category: "nonexistent", + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.Error(t, err) + assert.Contains(t, err.Error(), "category not found: nonexistent") + assert.Contains(t, err.Error(), "general (General)") +} + +func TestListRun_authorFilter(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, limit int) ([]client.Discussion, int, error) { + assert.Equal(t, "monalisa", filters.Author) + return sampleDiscussions()[:1], 1, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Author: "monalisa", + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "Bug report discussion") +} + +func TestListRun_labelFilter(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, limit int) ([]client.Discussion, int, error) { + assert.Equal(t, []string{"bug", "docs"}, filters.Labels) + return sampleDiscussions()[:1], 1, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Labels: []string{"bug", "docs"}, + State: "open", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "Bug report discussion") +} + +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + args string + wantsErr bool + }{ + { + name: "no flags", + args: "", + }, + { + name: "state flag", + args: "--state closed", + }, + { + name: "label flag", + args: "--label bug --label docs", + }, + { + name: "author flag", + args: "--author monalisa", + }, + { + name: "category flag", + args: "--category general", + }, + { + name: "limit flag", + args: "--limit 10", + }, + { + name: "invalid limit", + args: "--limit 0", + wantsErr: true, + }, + { + name: "web flag", + args: "--web", + }, + { + name: "order flag", + args: "--order created", + }, + { + name: "invalid state", + args: "--state invalid", + wantsErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + Browser: &browser.Stub{}, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + } + + var gotOpts *ListOptions + cmd := NewCmdList(f, func(o *ListOptions) error { + gotOpts = o + return nil + }) + + argv := []string{} + if tt.args != "" { + argv = splitArgs(tt.args) + } + cmd.SetArgs(argv) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err := cmd.ExecuteC() + + if tt.wantsErr { + require.Error(t, err) + return + } + require.NoError(t, err) + _ = gotOpts + }) + } +} + +func splitArgs(s string) []string { + var args []string + for _, part := range splitRespectingQuotes(s) { + if part != "" { + args = append(args, part) + } + } + return args +} + +func splitRespectingQuotes(s string) []string { + var result []string + var current []byte + inQuote := false + for i := 0; i < len(s); i++ { + if s[i] == '"' { + inQuote = !inQuote + continue + } + if s[i] == ' ' && !inQuote { + result = append(result, string(current)) + current = nil + continue + } + current = append(current, s[i]) + } + result = append(result, string(current)) + return result +} + +func TestListRun_closedState(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + closed := []client.Discussion{ + { + Number: 10, + Title: "Old discussion", + State: "CLOSED", + Category: client.DiscussionCategory{Name: "General"}, + Labels: []client.DiscussionLabel{}, + UpdatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + }, + } + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + return closed, 1, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "closed", + Limit: 30, + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "closed discussions") + assert.Contains(t, out, "Old discussion") + // Verify the # prefix is present (TTY mode) + assert.Contains(t, out, "#10") +} From e84ecb1585371594112ef1570a8c88b2d1968fda Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 13 Apr 2026 10:20:09 -0500 Subject: [PATCH 02/21] Address review feedback on discussion list PR - Export domain consts (FilterStateOpen/Closed, OrderByCreated/Updated, OrderDirectionAsc/Desc) in types.go - Change State fields to *string (nil = all states) - Add DiscussionListResult type with NextCursor for pagination - Update List/Search signatures: add after param, return result type - Add limit <= 0 guard clauses in client methods - Replace strings.ToUpper/ToLower with switch statements + default errors - Rename pageLimit to remaining, hoist hasDiscussionsEnabled check - Use qualifier/keyword terminology in search query building - Quote author values with %q for whitespace safety - Add Keywords string field (single string, not []string) - Split --order into --sort {created|updated} and --order {asc|desc} - Add --search/-S and --after flags - Add next field in JSON output envelope - Extract defaultLimit const - Add toFilterState helper for CLI-to-domain mapping - Move matchCategory to shared/categories.go with godoc - Use single-line error messages with sorted slugs - Add (preview) annotations to Short docs - Update Use to "list [flags]" - Add examples for --answered=false, --state all, multi-label - Update tests for new signatures and new flags Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client.go | 4 +- pkg/cmd/discussion/client/client_impl.go | 173 ++++++++++++++------ pkg/cmd/discussion/client/client_mock.go | 28 +++- pkg/cmd/discussion/client/types.go | 33 +++- pkg/cmd/discussion/discussion.go | 6 +- pkg/cmd/discussion/list/list.go | 101 +++++++----- pkg/cmd/discussion/list/list_test.go | 193 ++++++++++++++++++++--- pkg/cmd/discussion/shared/categories.go | 32 ++++ 8 files changed, 439 insertions(+), 131 deletions(-) create mode 100644 pkg/cmd/discussion/shared/categories.go diff --git a/pkg/cmd/discussion/client/client.go b/pkg/cmd/discussion/client/client.go index b698dc896..7f5fdfd4b 100644 --- a/pkg/cmd/discussion/client/client.go +++ b/pkg/cmd/discussion/client/client.go @@ -9,8 +9,8 @@ import "github.com/cli/cli/v2/internal/ghrepo" // DiscussionClient defines operations for interacting with the GitHub Discussions API. type DiscussionClient interface { - List(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) - Search(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) + List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) + Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 567cce2af..761913d2a 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -119,7 +119,11 @@ const discussionFields = ` createdAt updatedAt closedAt locked ` -func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) { +func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { + if limit <= 0 { + return DiscussionListResult{}, fmt.Errorf("limit argument must be positive: %v", limit) + } + type response struct { Repository struct { HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` @@ -142,10 +146,24 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limi orderField := "UPDATED_AT" orderDir := "DESC" if filters.OrderBy != "" { - orderField = strings.ToUpper(filters.OrderBy) + "_AT" + switch filters.OrderBy { + case OrderByCreated: + orderField = "CREATED_AT" + case OrderByUpdated: + orderField = "UPDATED_AT" + default: + return DiscussionListResult{}, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) + } } if filters.Direction != "" { - orderDir = strings.ToUpper(filters.Direction) + switch filters.Direction { + case OrderDirectionAsc: + orderDir = "ASC" + case OrderDirectionDesc: + orderDir = "DESC" + default: + return DiscussionListResult{}, fmt.Errorf("unknown order direction: %q", filters.Direction) + } } variables["orderBy"] = map[string]string{ "field": orderField, @@ -156,11 +174,15 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limi variables["categoryId"] = filters.CategoryID } - switch strings.ToLower(filters.State) { - case "open": - variables["states"] = []string{"OPEN"} - case "closed": - variables["states"] = []string{"CLOSED"} + if filters.State != nil { + switch *filters.State { + case FilterStateOpen: + variables["states"] = []string{"OPEN"} + case FilterStateClosed: + variables["states"] = []string{"CLOSED"} + default: + return DiscussionListResult{}, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) + } } if filters.Answered != nil { @@ -204,12 +226,20 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limi } }`, strings.Join(paramParts, ", "), strings.Join(argParts, ", "), discussionFields) + if after != "" { + variables["after"] = after + } + var discussions []Discussion var totalCount int - pageLimit := limit + var nextCursor string + remaining := limit + + // Check hasDiscussionsEnabled on first request only + firstPage := true for { - perPage := pageLimit + perPage := remaining if perPage > 100 { perPage = 100 } @@ -217,29 +247,41 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, limi var data response if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { - return nil, 0, err + return DiscussionListResult{}, err } - if !data.Repository.HasDiscussionsEnabled { - return nil, 0, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + if firstPage && !data.Repository.HasDiscussionsEnabled { + return DiscussionListResult{}, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } + firstPage = false totalCount = data.Repository.Discussions.TotalCount for _, n := range data.Repository.Discussions.Nodes { discussions = append(discussions, mapDiscussion(n)) } - pageLimit -= len(data.Repository.Discussions.Nodes) - if pageLimit <= 0 || !data.Repository.Discussions.PageInfo.HasNextPage { + remaining -= len(data.Repository.Discussions.Nodes) + if remaining <= 0 || !data.Repository.Discussions.PageInfo.HasNextPage { + if data.Repository.Discussions.PageInfo.HasNextPage { + nextCursor = data.Repository.Discussions.PageInfo.EndCursor + } break } variables["after"] = data.Repository.Discussions.PageInfo.EndCursor } - return discussions, totalCount, nil + return DiscussionListResult{ + Discussions: discussions, + TotalCount: totalCount, + NextCursor: nextCursor, + }, nil } -func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) { +func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { + if limit <= 0 { + return DiscussionListResult{}, fmt.Errorf("limit argument must be positive: %v", limit) + } + type response struct { Search struct { DiscussionCount int `json:"discussionCount"` @@ -251,43 +293,60 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, } `json:"search"` } - searchTerms := []string{fmt.Sprintf("repo:%s/%s", repo.RepoOwner(), repo.RepoName())} + qualifiers := []string{fmt.Sprintf("repo:%s/%s", repo.RepoOwner(), repo.RepoName())} - switch strings.ToLower(filters.State) { - case "open": - searchTerms = append(searchTerms, "state:open") - case "closed": - searchTerms = append(searchTerms, "state:closed") - } - - if filters.Author != "" { - searchTerms = append(searchTerms, fmt.Sprintf("author:%s", filters.Author)) - } - for _, l := range filters.Labels { - searchTerms = append(searchTerms, fmt.Sprintf("label:%q", l)) - } - if filters.Category != "" { - searchTerms = append(searchTerms, fmt.Sprintf("category:%q", filters.Category)) - } - if filters.Answered != nil { - if *filters.Answered { - searchTerms = append(searchTerms, "is:answered") - } else { - searchTerms = append(searchTerms, "is:unanswered") + if filters.State != nil { + switch *filters.State { + case FilterStateOpen: + qualifiers = append(qualifiers, "state:open") + case FilterStateClosed: + qualifiers = append(qualifiers, "state:closed") + default: + return DiscussionListResult{}, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) } } - orderField := "updated" - orderDir := "desc" + if filters.Author != "" { + qualifiers = append(qualifiers, fmt.Sprintf("author:%q", filters.Author)) + } + for _, l := range filters.Labels { + qualifiers = append(qualifiers, fmt.Sprintf("label:%q", l)) + } + if filters.Category != "" { + qualifiers = append(qualifiers, fmt.Sprintf("category:%q", filters.Category)) + } + if filters.Answered != nil { + if *filters.Answered { + qualifiers = append(qualifiers, "is:answered") + } else { + qualifiers = append(qualifiers, "is:unanswered") + } + } + + orderField := OrderByUpdated + orderDir := OrderDirectionDesc if filters.OrderBy != "" { - orderField = strings.ToLower(filters.OrderBy) + switch filters.OrderBy { + case OrderByCreated, OrderByUpdated: + orderField = filters.OrderBy + default: + return DiscussionListResult{}, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) + } } if filters.Direction != "" { - orderDir = strings.ToLower(filters.Direction) + switch filters.Direction { + case OrderDirectionAsc, OrderDirectionDesc: + orderDir = filters.Direction + default: + return DiscussionListResult{}, fmt.Errorf("unknown order direction: %q", filters.Direction) + } } - searchTerms = append(searchTerms, fmt.Sprintf("sort:%s-%s", orderField, orderDir)) + qualifiers = append(qualifiers, fmt.Sprintf("sort:%s-%s", orderField, orderDir)) - searchQuery := strings.Join(searchTerms, " ") + searchQuery := strings.Join(qualifiers, " ") + if filters.Keywords != "" { + searchQuery += " " + filters.Keywords + } query := fmt.Sprintf(`query DiscussionSearch($query: String!, $first: Int!, $after: String) { search(query: $query, type: DISCUSSION, first: $first, after: $after) { @@ -301,12 +360,17 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, "query": searchQuery, } + if after != "" { + variables["after"] = after + } + var discussions []Discussion var totalCount int - pageLimit := limit + var nextCursor string + remaining := limit for { - perPage := pageLimit + perPage := remaining if perPage > 100 { perPage = 100 } @@ -314,7 +378,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, var data response if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { - return nil, 0, err + return DiscussionListResult{}, err } totalCount = data.Search.DiscussionCount @@ -322,14 +386,21 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, discussions = append(discussions, mapDiscussion(n)) } - pageLimit -= len(data.Search.Nodes) - if pageLimit <= 0 || !data.Search.PageInfo.HasNextPage { + remaining -= len(data.Search.Nodes) + if remaining <= 0 || !data.Search.PageInfo.HasNextPage { + if data.Search.PageInfo.HasNextPage { + nextCursor = data.Search.PageInfo.EndCursor + } break } variables["after"] = data.Search.PageInfo.EndCursor } - return discussions, totalCount, nil + return DiscussionListResult{ + Discussions: discussions, + TotalCount: totalCount, + NextCursor: nextCursor, + }, nil } func (c *discussionClient) GetByNumber(_ ghrepo.Interface, _ int) (*Discussion, error) { diff --git a/pkg/cmd/discussion/client/client_mock.go b/pkg/cmd/discussion/client/client_mock.go index 4f8227d5f..589258b4f 100644 --- a/pkg/cmd/discussion/client/client_mock.go +++ b/pkg/cmd/discussion/client/client_mock.go @@ -33,7 +33,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { // panic("mock out the GetWithComments method") // }, -// ListFunc: func(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) { +// ListFunc: func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { // panic("mock out the List method") // }, // ListCategoriesFunc: func(repo ghrepo.Interface) ([]DiscussionCategory, error) { @@ -48,7 +48,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // ReopenFunc: func(repo ghrepo.Interface, id string) (*Discussion, error) { // panic("mock out the Reopen method") // }, -// SearchFunc: func(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) { +// SearchFunc: func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { // panic("mock out the Search method") // }, // UnlockFunc: func(repo ghrepo.Interface, id string) error { @@ -83,7 +83,7 @@ type DiscussionClientMock struct { GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) // ListFunc mocks the List method. - ListFunc func(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) + ListFunc func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) // ListCategoriesFunc mocks the ListCategories method. ListCategoriesFunc func(repo ghrepo.Interface) ([]DiscussionCategory, error) @@ -98,7 +98,7 @@ type DiscussionClientMock struct { ReopenFunc func(repo ghrepo.Interface, id string) (*Discussion, error) // SearchFunc mocks the Search method. - SearchFunc func(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) + SearchFunc func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) // UnlockFunc mocks the Unlock method. UnlockFunc func(repo ghrepo.Interface, id string) error @@ -162,6 +162,8 @@ type DiscussionClientMock struct { Repo ghrepo.Interface // Filters is the filters argument value. Filters ListFilters + // After is the after argument value. + After string // Limit is the limit argument value. Limit int } @@ -199,6 +201,8 @@ type DiscussionClientMock struct { Repo ghrepo.Interface // Filters is the filters argument value. Filters SearchFilters + // After is the after argument value. + After string // Limit is the limit argument value. Limit int } @@ -441,23 +445,25 @@ func (mock *DiscussionClientMock) GetWithCommentsCalls() []struct { } // List calls ListFunc. -func (mock *DiscussionClientMock) List(repo ghrepo.Interface, filters ListFilters, limit int) ([]Discussion, int, error) { +func (mock *DiscussionClientMock) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { if mock.ListFunc == nil { panic("DiscussionClientMock.ListFunc: method is nil but DiscussionClient.List was just called") } callInfo := struct { Repo ghrepo.Interface Filters ListFilters + After string Limit int }{ Repo: repo, Filters: filters, + After: after, Limit: limit, } mock.lockList.Lock() mock.calls.List = append(mock.calls.List, callInfo) mock.lockList.Unlock() - return mock.ListFunc(repo, filters, limit) + return mock.ListFunc(repo, filters, after, limit) } // ListCalls gets all the calls that were made to List. @@ -467,11 +473,13 @@ func (mock *DiscussionClientMock) List(repo ghrepo.Interface, filters ListFilter func (mock *DiscussionClientMock) ListCalls() []struct { Repo ghrepo.Interface Filters ListFilters + After string Limit int } { var calls []struct { Repo ghrepo.Interface Filters ListFilters + After string Limit int } mock.lockList.RLock() @@ -625,23 +633,25 @@ func (mock *DiscussionClientMock) ReopenCalls() []struct { } // Search calls SearchFunc. -func (mock *DiscussionClientMock) Search(repo ghrepo.Interface, filters SearchFilters, limit int) ([]Discussion, int, error) { +func (mock *DiscussionClientMock) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { if mock.SearchFunc == nil { panic("DiscussionClientMock.SearchFunc: method is nil but DiscussionClient.Search was just called") } callInfo := struct { Repo ghrepo.Interface Filters SearchFilters + After string Limit int }{ Repo: repo, Filters: filters, + After: after, Limit: limit, } mock.lockSearch.Lock() mock.calls.Search = append(mock.calls.Search, callInfo) mock.lockSearch.Unlock() - return mock.SearchFunc(repo, filters, limit) + return mock.SearchFunc(repo, filters, after, limit) } // SearchCalls gets all the calls that were made to Search. @@ -651,11 +661,13 @@ func (mock *DiscussionClientMock) Search(repo ghrepo.Interface, filters SearchFi func (mock *DiscussionClientMock) SearchCalls() []struct { Repo ghrepo.Interface Filters SearchFilters + After string Limit int } { var calls []struct { Repo ghrepo.Interface Filters SearchFilters + After string Limit int } mock.lockSearch.RLock() diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 9870416ad..15cd45936 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -225,10 +225,37 @@ const ( CloseReasonDuplicate CloseReason = "DUPLICATE" ) +// Domain-level filter constants for state. +const ( + FilterStateOpen = "open" + FilterStateClosed = "closed" +) + +// Domain-level constants for order-by field. +const ( + OrderByCreated = "created" + OrderByUpdated = "updated" +) + +// Domain-level constants for order direction. +const ( + OrderDirectionAsc = "asc" + OrderDirectionDesc = "desc" +) + +// DiscussionListResult holds the result of a List or Search call, +// including the discussions, total count, and pagination cursor. +type DiscussionListResult struct { + Discussions []Discussion + TotalCount int + NextCursor string +} + // ListFilters holds parameters for the repository.discussions query. // CategoryID must be resolved by the caller before passing to List. +// A nil State indicates no state filtering (all states). type ListFilters struct { - State string + State *string CategoryID string Answered *bool OrderBy string @@ -237,12 +264,14 @@ type ListFilters struct { // SearchFilters holds parameters for the search query used when // author or label filtering is required. +// A nil State indicates no state filtering (all states). type SearchFilters struct { Author string Labels []string - State string + State *string Category string Answered *bool + Keywords string OrderBy string Direction string } diff --git a/pkg/cmd/discussion/discussion.go b/pkg/cmd/discussion/discussion.go index bd724fe69..a9bf9b981 100644 --- a/pkg/cmd/discussion/discussion.go +++ b/pkg/cmd/discussion/discussion.go @@ -11,8 +11,10 @@ import ( func NewCmdDiscussion(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "discussion ", - Short: "Manage discussions", - Long: "Work with GitHub Discussions.", + Short: "Work with GitHub Discussions (preview)", + Long: heredoc.Doc(` + Working with discussions in the GitHub CLI is in preview and subject to change without notice. + `), Example: heredoc.Doc(` $ gh discussion list $ gh discussion create --category "General" --title "Hello" diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index f82cc9806..4a1c5b509 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -18,6 +18,8 @@ import ( "github.com/spf13/cobra" ) +const defaultLimit = 30 + // ListOptions holds the configuration for the discussion list command. type ListOptions struct { IO *iostreams.IOStreams @@ -31,7 +33,10 @@ type ListOptions struct { State string Limit int Answered *bool + Sort string Order string + Search string + After string WebMode bool Exporter cmdutil.Exporter @@ -47,8 +52,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } cmd := &cobra.Command{ - Use: "list", - Short: "List discussions in a repository", + Use: "list [flags]", + Short: "List discussions in a repository (preview)", Long: heredoc.Doc(` List discussions in a GitHub repository. By default, only open discussions are shown. @@ -58,13 +63,19 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman $ gh discussion list # List discussions with a specific category - $ gh discussion list --category "General" + $ gh discussion list --category General # List closed discussions by author $ gh discussion list --state closed --author monalisa + # List all discussions (closed or open) by label + $ gh discussion list --state all --label bug,enhancement + # List answered discussions as JSON $ gh discussion list --answered --json number,title,url + + # List unanswered discussions as JSON + $ gh discussion list --answered=false --json number,title,url `), Aliases: []string{"ls"}, Args: cmdutil.NoArgsQuoteReminder, @@ -87,15 +98,33 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Category, "category", "c", "", "Filter by category name or slug") cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by label") cmdutil.StringEnumFlag(cmd, &opts.State, "state", "s", "open", []string{"open", "closed", "all"}, "Filter by state") - cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of discussions to fetch") + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, fmt.Sprintf("Maximum number of discussions to fetch (default %d)", defaultLimit)) cmdutil.NilBoolFlag(cmd, &opts.Answered, "answered", "", "Filter by answered state") - cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "updated", []string{"created", "updated"}, "Order by field") + cmdutil.StringEnumFlag(cmd, &opts.Sort, "sort", "", "updated", []string{"created", "updated"}, "Sort by field") + cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "desc", []string{"asc", "desc"}, "Order of results") + cmd.Flags().StringVarP(&opts.Search, "search", "S", "", "Search discussions with `query`") + cmd.Flags().StringVar(&opts.After, "after", "", "Cursor for the next page of results") cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List discussions in the web browser") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) return cmd } +// toFilterState maps CLI state strings to domain-level filter state pointers. +// "all" maps to nil (no state filter). +func toFilterState(v string) *string { + switch v { + case "open": + s := client.FilterStateOpen + return &s + case "closed": + s := client.FilterStateClosed + return &s + default: + return nil + } +} + func listRun(opts *ListOptions) error { repo, err := opts.BaseRepo() if err != nil { @@ -118,7 +147,7 @@ func listRun(opts *ListOptions) error { if err != nil { return err } - cat, err := matchCategory(opts.Category, categories) + cat, err := shared.MatchCategory(opts.Category, categories) if err != nil { return err } @@ -126,28 +155,32 @@ func listRun(opts *ListOptions) error { categorySlug = cat.Slug } - var discussions []client.Discussion - var totalCount int + state := toFilterState(opts.State) - useSearch := opts.Author != "" || len(opts.Labels) > 0 + var result client.DiscussionListResult + + useSearch := opts.Author != "" || len(opts.Labels) > 0 || opts.Search != "" if useSearch { filters := client.SearchFilters{ - Author: opts.Author, - Labels: opts.Labels, - State: opts.State, - Category: categorySlug, - Answered: opts.Answered, - OrderBy: opts.Order, + Author: opts.Author, + Labels: opts.Labels, + State: state, + Category: categorySlug, + Answered: opts.Answered, + Keywords: opts.Search, + OrderBy: opts.Sort, + Direction: opts.Order, } - discussions, totalCount, err = dc.Search(repo, filters, opts.Limit) + result, err = dc.Search(repo, filters, opts.After, opts.Limit) } else { filters := client.ListFilters{ - State: opts.State, + State: state, CategoryID: categoryID, Answered: opts.Answered, - OrderBy: opts.Order, + OrderBy: opts.Sort, + Direction: opts.Order, } - discussions, totalCount, err = dc.List(repo, filters, opts.Limit) + result, err = dc.List(repo, filters, opts.After, opts.Limit) } if err != nil { return err @@ -155,13 +188,14 @@ func listRun(opts *ListOptions) error { if opts.Exporter != nil { envelope := map[string]interface{}{ - "totalCount": totalCount, - "discussions": discussions, + "totalCount": result.TotalCount, + "discussions": result.Discussions, + "next": result.NextCursor, } return opts.Exporter.Write(opts.IO, envelope) } - if len(discussions) == 0 { + if len(result.Discussions) == 0 { return noResults(repo, opts.State) } @@ -173,11 +207,11 @@ func listRun(opts *ListOptions) error { isTerminal := opts.IO.IsStdoutTTY() if isTerminal { - title := listHeader(ghrepo.FullName(repo), len(discussions), totalCount, opts.State) + title := listHeader(ghrepo.FullName(repo), len(result.Discussions), result.TotalCount, opts.State) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } - printDiscussions(opts, discussions, totalCount) + printDiscussions(opts, result.Discussions, result.TotalCount) return nil } @@ -215,25 +249,6 @@ func openInBrowser(opts *ListOptions, repo ghrepo.Interface) error { return opts.Browser.Browse(discussionsURL) } -func matchCategory(input string, categories []client.DiscussionCategory) (*client.DiscussionCategory, error) { - for i := range categories { - if strings.EqualFold(categories[i].Slug, input) { - return &categories[i], nil - } - } - for i := range categories { - if strings.EqualFold(categories[i].Name, input) { - return &categories[i], nil - } - } - - var available strings.Builder - for _, c := range categories { - fmt.Fprintf(&available, " %s (%s)\n", c.Slug, c.Name) - } - return nil, fmt.Errorf("category not found: %s\n\nAvailable categories:\n%s", input, available.String()) -} - func noResults(repo ghrepo.Interface, state string) error { stateQualifier := "" switch state { diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go index f4e6f57ab..9404ac806 100644 --- a/pkg/cmd/discussion/list/list_test.go +++ b/pkg/cmd/discussion/list/list_test.go @@ -55,6 +55,13 @@ func sampleDiscussions() []client.Discussion { } } +func sampleResult() client.DiscussionListResult { + return client.DiscussionListResult{ + Discussions: sampleDiscussions(), + TotalCount: 2, + } +} + func sampleCategories() []client.DiscussionCategory { return []client.DiscussionCategory{ {ID: "CAT1", Name: "General", Slug: "general", IsAnswerable: true}, @@ -69,8 +76,8 @@ func TestListRun_tty(t *testing.T) { ios.SetStderrTTY(true) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { - return sampleDiscussions(), 2, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + return sampleResult(), nil }, } @@ -80,6 +87,8 @@ func TestListRun_tty(t *testing.T) { Client: func() (client.DiscussionClient, error) { return mockClient, nil }, State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -102,8 +111,8 @@ func TestListRun_nontty(t *testing.T) { ios.SetStdoutTTY(false) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { - return sampleDiscussions(), 2, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + return sampleResult(), nil }, } @@ -113,6 +122,8 @@ func TestListRun_nontty(t *testing.T) { Client: func() (client.DiscussionClient, error) { return mockClient, nil }, State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -120,7 +131,6 @@ func TestListRun_nontty(t *testing.T) { require.NoError(t, err) out := stdout.String() - // Non-TTY output should not contain # prefix or header assert.NotContains(t, out, "Showing") assert.Contains(t, out, "42") assert.Contains(t, out, "OPEN") @@ -131,8 +141,12 @@ func TestListRun_json(t *testing.T) { ios, _, stdout, _ := iostreams.Test() mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { - return sampleDiscussions(), 2, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + return client.DiscussionListResult{ + Discussions: sampleDiscussions(), + TotalCount: 2, + NextCursor: "CURSOR123", + }, nil }, } @@ -145,6 +159,8 @@ func TestListRun_json(t *testing.T) { Client: func() (client.DiscussionClient, error) { return mockClient, nil }, State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Exporter: exporter, Now: fixedTime, } @@ -155,6 +171,7 @@ func TestListRun_json(t *testing.T) { out := stdout.String() assert.Contains(t, out, `"totalCount"`) assert.Contains(t, out, `"discussions"`) + assert.Contains(t, out, `"next"`) } func TestListRun_web(t *testing.T) { @@ -184,8 +201,8 @@ func TestListRun_noResults(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { - return []client.Discussion{}, 0, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + return client.DiscussionListResult{}, nil }, } @@ -195,6 +212,8 @@ func TestListRun_noResults(t *testing.T) { Client: func() (client.DiscussionClient, error) { return mockClient, nil }, State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -212,9 +231,12 @@ func TestListRun_categoryFilter(t *testing.T) { ListCategoriesFunc: func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { return sampleCategories(), nil }, - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { assert.Equal(t, "CAT1", filters.CategoryID) - return sampleDiscussions()[:1], 1, nil + return client.DiscussionListResult{ + Discussions: sampleDiscussions()[:1], + TotalCount: 1, + }, nil }, } @@ -225,6 +247,8 @@ func TestListRun_categoryFilter(t *testing.T) { Category: "general", State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -249,13 +273,14 @@ func TestListRun_categoryNotFound(t *testing.T) { Category: "nonexistent", State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } err := listRun(opts) require.Error(t, err) - assert.Contains(t, err.Error(), "category not found: nonexistent") - assert.Contains(t, err.Error(), "general (General)") + assert.Contains(t, err.Error(), `unknown category: "nonexistent"`) } func TestListRun_authorFilter(t *testing.T) { @@ -263,9 +288,12 @@ func TestListRun_authorFilter(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, limit int) ([]client.Discussion, int, error) { + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { assert.Equal(t, "monalisa", filters.Author) - return sampleDiscussions()[:1], 1, nil + return client.DiscussionListResult{ + Discussions: sampleDiscussions()[:1], + TotalCount: 1, + }, nil }, } @@ -276,6 +304,8 @@ func TestListRun_authorFilter(t *testing.T) { Author: "monalisa", State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -289,9 +319,12 @@ func TestListRun_labelFilter(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, limit int) ([]client.Discussion, int, error) { + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { assert.Equal(t, []string{"bug", "docs"}, filters.Labels) - return sampleDiscussions()[:1], 1, nil + return client.DiscussionListResult{ + Discussions: sampleDiscussions()[:1], + TotalCount: 1, + }, nil }, } @@ -302,6 +335,8 @@ func TestListRun_labelFilter(t *testing.T) { Labels: []string{"bug", "docs"}, State: "open", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -310,6 +345,64 @@ func TestListRun_labelFilter(t *testing.T) { assert.Contains(t, stdout.String(), "Bug report discussion") } +func TestListRun_searchFilter(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { + assert.Equal(t, "some keywords", filters.Keywords) + return client.DiscussionListResult{ + Discussions: sampleDiscussions()[:1], + TotalCount: 1, + }, nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Search: "some keywords", + State: "open", + Limit: 30, + Sort: "updated", + Order: "desc", + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "Bug report discussion") +} + +func TestListRun_afterCursor(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + mockClient := &client.DiscussionClientMock{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + assert.Equal(t, "CURSOR_ABC", after) + return sampleResult(), nil + }, + } + + opts := &ListOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + State: "open", + Limit: 30, + Sort: "updated", + Order: "desc", + After: "CURSOR_ABC", + Now: fixedTime, + } + + err := listRun(opts) + require.NoError(t, err) +} + func TestNewCmdList(t *testing.T) { tests := []struct { name string @@ -349,15 +442,41 @@ func TestNewCmdList(t *testing.T) { name: "web flag", args: "--web", }, + { + name: "sort flag", + args: "--sort created", + }, { name: "order flag", - args: "--order created", + args: "--order asc", + }, + { + name: "sort and order flags", + args: "--sort created --order asc", + }, + { + name: "search flag", + args: "--search \"some query\"", + }, + { + name: "after flag", + args: "--after CURSOR123", }, { name: "invalid state", args: "--state invalid", wantsErr: true, }, + { + name: "invalid sort", + args: "--sort invalid", + wantsErr: true, + }, + { + name: "invalid order", + args: "--order invalid", + wantsErr: true, + }, } for _, tt := range tests { @@ -365,8 +484,8 @@ func TestNewCmdList(t *testing.T) { ios, _, _, _ := iostreams.Test() f := &cmdutil.Factory{ IOStreams: ios, - Browser: &browser.Stub{}, - BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Browser: &browser.Stub{}, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, } var gotOpts *ListOptions @@ -395,6 +514,30 @@ func TestNewCmdList(t *testing.T) { } } +func TestToFilterState(t *testing.T) { + tests := []struct { + input string + want *string + }{ + {"open", strPtr(client.FilterStateOpen)}, + {"closed", strPtr(client.FilterStateClosed)}, + {"all", nil}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := toFilterState(tt.input) + if tt.want == nil { + assert.Nil(t, got) + } else { + require.NotNil(t, got) + assert.Equal(t, *tt.want, *got) + } + }) + } +} + +func strPtr(s string) *string { return &s } + func splitArgs(s string) []string { var args []string for _, part := range splitRespectingQuotes(s) { @@ -441,8 +584,11 @@ func TestListRun_closedState(t *testing.T) { } mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, limit int) ([]client.Discussion, int, error) { - return closed, 1, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + return client.DiscussionListResult{ + Discussions: closed, + TotalCount: 1, + }, nil }, } @@ -452,6 +598,8 @@ func TestListRun_closedState(t *testing.T) { Client: func() (client.DiscussionClient, error) { return mockClient, nil }, State: "closed", Limit: 30, + Sort: "updated", + Order: "desc", Now: fixedTime, } @@ -461,6 +609,5 @@ func TestListRun_closedState(t *testing.T) { out := stdout.String() assert.Contains(t, out, "closed discussions") assert.Contains(t, out, "Old discussion") - // Verify the # prefix is present (TTY mode) assert.Contains(t, out, "#10") } diff --git a/pkg/cmd/discussion/shared/categories.go b/pkg/cmd/discussion/shared/categories.go new file mode 100644 index 000000000..a7893f229 --- /dev/null +++ b/pkg/cmd/discussion/shared/categories.go @@ -0,0 +1,32 @@ +package shared + +import ( + "fmt" + "slices" + "strings" + + "github.com/cli/cli/v2/pkg/cmd/discussion/client" +) + +// MatchCategory finds a category by name or slug (case-insensitive). +// It prefers an exact slug match over a name match, so users are +// encouraged to use slugs for unambiguous lookups. +func MatchCategory(input string, categories []client.DiscussionCategory) (*client.DiscussionCategory, error) { + for i := range categories { + if strings.EqualFold(categories[i].Slug, input) { + return &categories[i], nil + } + } + for i := range categories { + if strings.EqualFold(categories[i].Name, input) { + return &categories[i], nil + } + } + + slugs := make([]string, len(categories)) + for i, c := range categories { + slugs[i] = c.Slug + } + slices.Sort(slugs) + return nil, fmt.Errorf("unknown category: %q; must be one of %v", input, slugs) +} From a08f5f22f03210699e551a8a909a74e46dba2556 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 13 Apr 2026 10:24:52 -0500 Subject: [PATCH 03/21] Fix gofmt alignment in test file Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/list/list_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go index 9404ac806..20702a2a5 100644 --- a/pkg/cmd/discussion/list/list_test.go +++ b/pkg/cmd/discussion/list/list_test.go @@ -484,8 +484,8 @@ func TestNewCmdList(t *testing.T) { ios, _, _, _ := iostreams.Test() f := &cmdutil.Factory{ IOStreams: ios, - Browser: &browser.Stub{}, - BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Browser: &browser.Stub{}, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, } var gotOpts *ListOptions From 17238050d7a45497a02817166f361ce1fadc03ec Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 13 Apr 2026 10:38:51 -0500 Subject: [PATCH 04/21] Check hasDiscussionsEnabled in ListCategories When --category is used, ListCategories runs before the List query. On repos with discussions disabled, it silently returns empty categories, leading to a confusing "must be one of []" error. Now it checks hasDiscussionsEnabled and returns the standard "discussions disabled" error, matching the behavior of List and Search. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 761913d2a..60c02c8d5 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -414,7 +414,8 @@ func (c *discussionClient) GetWithComments(_ ghrepo.Interface, _ int, _ int, _ s func (c *discussionClient) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) { var query struct { Repository struct { - DiscussionCategories struct { + HasDiscussionsEnabled bool + DiscussionCategories struct { Nodes []struct { ID string Name string @@ -435,6 +436,10 @@ func (c *discussionClient) ListCategories(repo ghrepo.Interface) ([]DiscussionCa return nil, err } + if !query.Repository.HasDiscussionsEnabled { + return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + } + categories := make([]DiscussionCategory, len(query.Repository.DiscussionCategories.Nodes)) for i, n := range query.Repository.DiscussionCategories.Nodes { categories[i] = DiscussionCategory{ From 0687a29e51631e5c3f2e448716cee159d27f76ab Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Mon, 13 Apr 2026 11:51:22 -0500 Subject: [PATCH 05/21] Fix GQL schema: Discussion uses closed bool, not state string The GraphQL Discussion type has a `closed` boolean field, not a `state` string. Updated the API response struct and GQL fragment to query `closed` instead of `state`, and derive the domain-level State string ("OPEN"/"CLOSED") from the boolean in mapDiscussion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 60c02c8d5..8c6825785 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -29,7 +29,7 @@ type discussionNode struct { Number int `json:"number"` Title string `json:"title"` URL string `json:"url"` - State string `json:"state"` + Closed bool `json:"closed"` StateReason string `json:"stateReason"` Author struct { Login string `json:"login"` @@ -67,12 +67,17 @@ type discussionNode struct { // mapDiscussion converts a GraphQL discussionNode response into the domain Discussion type. func mapDiscussion(n discussionNode) Discussion { + state := "OPEN" + if n.Closed { + state = "CLOSED" + } + d := Discussion{ ID: n.ID, Number: n.Number, Title: n.Title, URL: n.URL, - State: n.State, + State: state, StateReason: n.StateReason, Author: DiscussionAuthor{Login: n.Author.Login}, Category: DiscussionCategory{ @@ -110,7 +115,7 @@ func mapDiscussion(n discussionNode) Discussion { // discussionFields is the GraphQL fragment selecting fields for discussion queries. // It is shared by both List (repository.discussions) and Search queries. const discussionFields = ` - id number title url state stateReason + id number title url closed stateReason author { login } category { id name slug emoji isAnswerable } labels(first: 20) { nodes { id name color } } From 3f52503a67a74dab7fe551cf6d2a15366cb4d44e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 11:45:55 +0100 Subject: [PATCH 06/21] fix(discussion list): use separate list of fields Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 4a1c5b509..e45f784a8 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -20,6 +20,27 @@ import ( const defaultLimit = 30 +var discussionListFields = []string{ + "id", + "number", + "title", + "body", + "url", + "state", + "stateReason", + "author", + "category", + "labels", + "answered", + "answerChosenAt", + "answerChosenBy", + "reactionGroups", + "createdAt", + "updatedAt", + "closedAt", + "locked", +} + // ListOptions holds the configuration for the discussion list command. type ListOptions struct { IO *iostreams.IOStreams @@ -105,7 +126,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Search, "search", "S", "", "Search discussions with `query`") cmd.Flags().StringVar(&opts.After, "after", "", "Cursor for the next page of results") cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List discussions in the web browser") - cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) + cmdutil.AddJSONFlags(cmd, &opts.Exporter, discussionListFields) return cmd } From 236224dc44b88a40eafed0a67b9c2f7b17a4faff Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 11:52:52 +0100 Subject: [PATCH 07/21] fix(discussion list): replace state with closed in domain types Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 7 +------ pkg/cmd/discussion/client/types.go | 6 +++--- pkg/cmd/discussion/list/list.go | 13 ++++++++++--- pkg/cmd/discussion/list/list_test.go | 4 +--- pkg/cmd/discussion/shared/fields.go | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 8c6825785..597303bef 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -67,17 +67,12 @@ type discussionNode struct { // mapDiscussion converts a GraphQL discussionNode response into the domain Discussion type. func mapDiscussion(n discussionNode) Discussion { - state := "OPEN" - if n.Closed { - state = "CLOSED" - } - d := Discussion{ ID: n.ID, Number: n.Number, Title: n.Title, URL: n.URL, - State: state, + Closed: n.Closed, StateReason: n.StateReason, Author: DiscussionAuthor{Login: n.Author.Login}, Category: DiscussionCategory{ diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 15cd45936..d0fc0fadf 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -10,7 +10,7 @@ type Discussion struct { Title string Body string URL string - State string + Closed bool StateReason string Author DiscussionAuthor Category DiscussionCategory @@ -43,8 +43,8 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { data[f] = d.Body case "url": data[f] = d.URL - case "state": - data[f] = d.State + case "closed": + data[f] = d.Closed case "stateReason": data[f] = d.StateReason case "author": diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index e45f784a8..119d3ff9d 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -20,13 +20,16 @@ import ( const defaultLimit = 30 +// discussionListFields lists the field names available for --json output +// on the discussion list command. This excludes fields like "comments" +// that are only populated by the view command. var discussionListFields = []string{ "id", "number", "title", "body", "url", - "state", + "closed", "stateReason", "author", "category", @@ -306,13 +309,17 @@ func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalC for _, d := range discussions { if isTerminal { idColor := cs.Green - if strings.EqualFold(d.State, "CLOSED") { + if d.Closed { idColor = cs.Gray } tp.AddField(fmt.Sprintf("#%d", d.Number), tableprinter.WithColor(idColor)) } else { tp.AddField(fmt.Sprintf("%d", d.Number)) - tp.AddField(d.State) + if d.Closed { + tp.AddField("CLOSED") + } else { + tp.AddField("OPEN") + } } tp.AddField(text.RemoveExcessiveWhitespace(d.Title)) diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go index 20702a2a5..72cc92a89 100644 --- a/pkg/cmd/discussion/list/list_test.go +++ b/pkg/cmd/discussion/list/list_test.go @@ -24,7 +24,6 @@ func sampleDiscussions() []client.Discussion { Number: 42, Title: "Bug report discussion", URL: "https://github.com/OWNER/REPO/discussions/42", - State: "OPEN", Author: client.DiscussionAuthor{Login: "monalisa"}, Category: client.DiscussionCategory{ ID: "CAT1", @@ -41,7 +40,6 @@ func sampleDiscussions() []client.Discussion { Number: 41, Title: "Feature request", URL: "https://github.com/OWNER/REPO/discussions/41", - State: "OPEN", Author: client.DiscussionAuthor{Login: "octocat"}, Category: client.DiscussionCategory{ ID: "CAT2", @@ -576,7 +574,7 @@ func TestListRun_closedState(t *testing.T) { { Number: 10, Title: "Old discussion", - State: "CLOSED", + Closed: true, Category: client.DiscussionCategory{Name: "General"}, Labels: []client.DiscussionLabel{}, UpdatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), diff --git a/pkg/cmd/discussion/shared/fields.go b/pkg/cmd/discussion/shared/fields.go index 47d750314..0feb789d9 100644 --- a/pkg/cmd/discussion/shared/fields.go +++ b/pkg/cmd/discussion/shared/fields.go @@ -8,7 +8,7 @@ var DiscussionFields = []string{ "title", "body", "url", - "state", + "closed", "stateReason", "author", "category", From 15d3eeae06126f2773d7de87d0a851eb6fc36bac Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 11:53:36 +0100 Subject: [PATCH 08/21] fix(discussion/client): fetch body in list/search Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 597303bef..5f4117cf9 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -110,7 +110,7 @@ func mapDiscussion(n discussionNode) Discussion { // discussionFields is the GraphQL fragment selecting fields for discussion queries. // It is shared by both List (repository.discussions) and Search queries. const discussionFields = ` - id number title url closed stateReason + id number title body url closed stateReason author { login } category { id name slug emoji isAnswerable } labels(first: 20) { nodes { id name color } } From 034e38f0e751c36d6e302caa331084f75caea7aa Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 12:11:00 +0100 Subject: [PATCH 09/21] fix(discussion/client): fetch author/answerChosenBy fields Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 20 ++++++++++++++++---- pkg/cmd/discussion/client/types.go | 12 ++++++------ pkg/cmd/discussion/list/list_test.go | 4 ++-- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 5f4117cf9..276009221 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -33,6 +33,8 @@ type discussionNode struct { StateReason string `json:"stateReason"` Author struct { Login string `json:"login"` + ID string `json:"id"` + Name string `json:"name"` } `json:"author"` Category struct { ID string `json:"id"` @@ -52,6 +54,8 @@ type discussionNode struct { AnswerChosenAt time.Time `json:"answerChosenAt"` AnswerChosenBy *struct { Login string `json:"login"` + ID string `json:"id"` + Name string `json:"name"` } `json:"answerChosenBy"` ReactionGroups []struct { Content string `json:"content"` @@ -74,7 +78,11 @@ func mapDiscussion(n discussionNode) Discussion { URL: n.URL, Closed: n.Closed, StateReason: n.StateReason, - Author: DiscussionAuthor{Login: n.Author.Login}, + Author: DiscussionActor{ + ID: n.Author.ID, + Login: n.Author.Login, + Name: n.Author.Name, + }, Category: DiscussionCategory{ ID: n.Category.ID, Name: n.Category.Name, @@ -91,7 +99,11 @@ func mapDiscussion(n discussionNode) Discussion { } if n.AnswerChosenBy != nil { - d.AnswerChosenBy = &DiscussionAuthor{Login: n.AnswerChosenBy.Login} + d.AnswerChosenBy = &DiscussionActor{ + ID: n.AnswerChosenBy.ID, + Login: n.AnswerChosenBy.Login, + Name: n.AnswerChosenBy.Name, + } } d.Labels = make([]DiscussionLabel, len(n.Labels.Nodes)) @@ -111,10 +123,10 @@ func mapDiscussion(n discussionNode) Discussion { // It is shared by both List (repository.discussions) and Search queries. const discussionFields = ` id number title body url closed stateReason - author { login } + author { login ...on User { id name } ...on Bot { id } } category { id name slug emoji isAnswerable } labels(first: 20) { nodes { id name color } } - isAnswered answerChosenAt answerChosenBy { login } + isAnswered answerChosenAt answerChosenBy { login ...on User { id name } ...on Bot { id } } reactionGroups { content users { totalCount } } createdAt updatedAt closedAt locked ` diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index d0fc0fadf..1de2bae1a 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -12,12 +12,12 @@ type Discussion struct { URL string Closed bool StateReason string - Author DiscussionAuthor + Author DiscussionActor Category DiscussionCategory Labels []DiscussionLabel Answered bool AnswerChosenAt time.Time - AnswerChosenBy *DiscussionAuthor + AnswerChosenBy *DiscussionActor Comments DiscussionCommentList ReactionGroups []ReactionGroup CreatedAt time.Time @@ -103,15 +103,15 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { return data } -// DiscussionAuthor represents the author of a discussion or comment. -type DiscussionAuthor struct { +// DiscussionActor represents a GitHub actor (user or bot) associated with a discussion. +type DiscussionActor struct { ID string Login string Name string } // Export returns the author as a map for JSON output. -func (a DiscussionAuthor) Export() map[string]interface{} { +func (a DiscussionActor) Export() map[string]interface{} { return map[string]interface{}{ "id": a.ID, "login": a.Login, @@ -159,7 +159,7 @@ func (l DiscussionLabel) Export() map[string]interface{} { type DiscussionComment struct { ID string URL string - Author DiscussionAuthor + Author DiscussionActor Body string CreatedAt time.Time IsAnswer bool diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go index 72cc92a89..29df02694 100644 --- a/pkg/cmd/discussion/list/list_test.go +++ b/pkg/cmd/discussion/list/list_test.go @@ -24,7 +24,7 @@ func sampleDiscussions() []client.Discussion { Number: 42, Title: "Bug report discussion", URL: "https://github.com/OWNER/REPO/discussions/42", - Author: client.DiscussionAuthor{Login: "monalisa"}, + Author: client.DiscussionActor{Login: "monalisa"}, Category: client.DiscussionCategory{ ID: "CAT1", Name: "General", @@ -40,7 +40,7 @@ func sampleDiscussions() []client.Discussion { Number: 41, Title: "Feature request", URL: "https://github.com/OWNER/REPO/discussions/41", - Author: client.DiscussionAuthor{Login: "octocat"}, + Author: client.DiscussionActor{Login: "octocat"}, Category: client.DiscussionCategory{ ID: "CAT2", Name: "Ideas", From e6befd5efdbfc009913dab9b5a4408a23940c77b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 12:16:23 +0100 Subject: [PATCH 10/21] fix(discussion/client): simplify list query Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 48 ++++++++++-------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 276009221..0e7b29f48 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -201,42 +201,32 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte variables["answered"] = *filters.Answered } - // Build optional parameter declarations - paramParts := []string{ - "$owner: String!", - "$name: String!", - "$first: Int!", - "$after: String", - "$orderBy: DiscussionOrder", - } - argParts := []string{ - "first: $first", - "after: $after", - "orderBy: $orderBy", - } - if filters.CategoryID != "" { - paramParts = append(paramParts, "$categoryId: ID") - argParts = append(argParts, "categoryId: $categoryId") - } - if _, ok := variables["states"]; ok { - paramParts = append(paramParts, "$states: [DiscussionState!]") - argParts = append(argParts, "states: $states") - } - if filters.Answered != nil { - paramParts = append(paramParts, "$answered: Boolean") - argParts = append(argParts, "answered: $answered") - } - - query := fmt.Sprintf(`query DiscussionList(%s) { + query := fmt.Sprintf(`query DiscussionList( + $owner: String!, + $name: String!, + $first: Int!, + $after: String, + $orderBy: DiscussionOrder, + $categoryId: ID, + $states: [DiscussionState!], + $answered: Boolean + ) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled - discussions(%s) { + discussions( + first: $first, + after: $after, + orderBy: $orderBy, + categoryId: $categoryId, + states: $states, + answered: $answered + ) { totalCount pageInfo { hasNextPage endCursor } nodes { %s } } } - }`, strings.Join(paramParts, ", "), strings.Join(argParts, ", "), discussionFields) + }`, discussionFields) if after != "" { variables["after"] = after From 2a46a9d733cc29740bcff1c107feb0854589a077 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 12:24:08 +0100 Subject: [PATCH 11/21] fix(discussion/client): change list return type to pointer Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client.go | 4 +-- pkg/cmd/discussion/client/client_impl.go | 30 +++++++++---------- pkg/cmd/discussion/client/client_mock.go | 12 ++++---- pkg/cmd/discussion/list/list.go | 2 +- pkg/cmd/discussion/list/list_test.go | 38 ++++++++++++------------ 5 files changed, 43 insertions(+), 43 deletions(-) diff --git a/pkg/cmd/discussion/client/client.go b/pkg/cmd/discussion/client/client.go index 7f5fdfd4b..a794d13e1 100644 --- a/pkg/cmd/discussion/client/client.go +++ b/pkg/cmd/discussion/client/client.go @@ -9,8 +9,8 @@ import "github.com/cli/cli/v2/internal/ghrepo" // DiscussionClient defines operations for interacting with the GitHub Discussions API. type DiscussionClient interface { - List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) - Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) + List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) + Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 0e7b29f48..9d22454d7 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -131,9 +131,9 @@ const discussionFields = ` createdAt updatedAt closedAt locked ` -func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { +func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { if limit <= 0 { - return DiscussionListResult{}, fmt.Errorf("limit argument must be positive: %v", limit) + return nil, fmt.Errorf("limit argument must be positive: %v", limit) } type response struct { @@ -164,7 +164,7 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte case OrderByUpdated: orderField = "UPDATED_AT" default: - return DiscussionListResult{}, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) + return nil, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) } } if filters.Direction != "" { @@ -174,7 +174,7 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte case OrderDirectionDesc: orderDir = "DESC" default: - return DiscussionListResult{}, fmt.Errorf("unknown order direction: %q", filters.Direction) + return nil, fmt.Errorf("unknown order direction: %q", filters.Direction) } } variables["orderBy"] = map[string]string{ @@ -193,7 +193,7 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte case FilterStateClosed: variables["states"] = []string{"CLOSED"} default: - return DiscussionListResult{}, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) + return nil, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) } } @@ -249,11 +249,11 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte var data response if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { - return DiscussionListResult{}, err + return nil, err } if firstPage && !data.Repository.HasDiscussionsEnabled { - return DiscussionListResult{}, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } firstPage = false @@ -272,16 +272,16 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte variables["after"] = data.Repository.Discussions.PageInfo.EndCursor } - return DiscussionListResult{ + return &DiscussionListResult{ Discussions: discussions, TotalCount: totalCount, NextCursor: nextCursor, }, nil } -func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { +func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) { if limit <= 0 { - return DiscussionListResult{}, fmt.Errorf("limit argument must be positive: %v", limit) + return nil, fmt.Errorf("limit argument must be positive: %v", limit) } type response struct { @@ -304,7 +304,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, case FilterStateClosed: qualifiers = append(qualifiers, "state:closed") default: - return DiscussionListResult{}, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) + return nil, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) } } @@ -332,7 +332,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, case OrderByCreated, OrderByUpdated: orderField = filters.OrderBy default: - return DiscussionListResult{}, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) + return nil, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) } } if filters.Direction != "" { @@ -340,7 +340,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, case OrderDirectionAsc, OrderDirectionDesc: orderDir = filters.Direction default: - return DiscussionListResult{}, fmt.Errorf("unknown order direction: %q", filters.Direction) + return nil, fmt.Errorf("unknown order direction: %q", filters.Direction) } } qualifiers = append(qualifiers, fmt.Sprintf("sort:%s-%s", orderField, orderDir)) @@ -380,7 +380,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, var data response if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { - return DiscussionListResult{}, err + return nil, err } totalCount = data.Search.DiscussionCount @@ -398,7 +398,7 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, variables["after"] = data.Search.PageInfo.EndCursor } - return DiscussionListResult{ + return &DiscussionListResult{ Discussions: discussions, TotalCount: totalCount, NextCursor: nextCursor, diff --git a/pkg/cmd/discussion/client/client_mock.go b/pkg/cmd/discussion/client/client_mock.go index 589258b4f..a690f84b1 100644 --- a/pkg/cmd/discussion/client/client_mock.go +++ b/pkg/cmd/discussion/client/client_mock.go @@ -33,7 +33,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { // panic("mock out the GetWithComments method") // }, -// ListFunc: func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { +// ListFunc: func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { // panic("mock out the List method") // }, // ListCategoriesFunc: func(repo ghrepo.Interface) ([]DiscussionCategory, error) { @@ -48,7 +48,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // ReopenFunc: func(repo ghrepo.Interface, id string) (*Discussion, error) { // panic("mock out the Reopen method") // }, -// SearchFunc: func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { +// SearchFunc: func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) { // panic("mock out the Search method") // }, // UnlockFunc: func(repo ghrepo.Interface, id string) error { @@ -83,7 +83,7 @@ type DiscussionClientMock struct { GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) // ListFunc mocks the List method. - ListFunc func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) + ListFunc func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) // ListCategoriesFunc mocks the ListCategories method. ListCategoriesFunc func(repo ghrepo.Interface) ([]DiscussionCategory, error) @@ -98,7 +98,7 @@ type DiscussionClientMock struct { ReopenFunc func(repo ghrepo.Interface, id string) (*Discussion, error) // SearchFunc mocks the Search method. - SearchFunc func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) + SearchFunc func(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) // UnlockFunc mocks the Unlock method. UnlockFunc func(repo ghrepo.Interface, id string) error @@ -445,7 +445,7 @@ func (mock *DiscussionClientMock) GetWithCommentsCalls() []struct { } // List calls ListFunc. -func (mock *DiscussionClientMock) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (DiscussionListResult, error) { +func (mock *DiscussionClientMock) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { if mock.ListFunc == nil { panic("DiscussionClientMock.ListFunc: method is nil but DiscussionClient.List was just called") } @@ -633,7 +633,7 @@ func (mock *DiscussionClientMock) ReopenCalls() []struct { } // Search calls SearchFunc. -func (mock *DiscussionClientMock) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (DiscussionListResult, error) { +func (mock *DiscussionClientMock) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) { if mock.SearchFunc == nil { panic("DiscussionClientMock.SearchFunc: method is nil but DiscussionClient.Search was just called") } diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 119d3ff9d..f4305b926 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -181,7 +181,7 @@ func listRun(opts *ListOptions) error { state := toFilterState(opts.State) - var result client.DiscussionListResult + var result *client.DiscussionListResult useSearch := opts.Author != "" || len(opts.Labels) > 0 || opts.Search != "" if useSearch { diff --git a/pkg/cmd/discussion/list/list_test.go b/pkg/cmd/discussion/list/list_test.go index 29df02694..cb43d561c 100644 --- a/pkg/cmd/discussion/list/list_test.go +++ b/pkg/cmd/discussion/list/list_test.go @@ -53,8 +53,8 @@ func sampleDiscussions() []client.Discussion { } } -func sampleResult() client.DiscussionListResult { - return client.DiscussionListResult{ +func sampleResult() *client.DiscussionListResult { + return &client.DiscussionListResult{ Discussions: sampleDiscussions(), TotalCount: 2, } @@ -74,7 +74,7 @@ func TestListRun_tty(t *testing.T) { ios.SetStderrTTY(true) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { return sampleResult(), nil }, } @@ -109,7 +109,7 @@ func TestListRun_nontty(t *testing.T) { ios.SetStdoutTTY(false) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { return sampleResult(), nil }, } @@ -139,8 +139,8 @@ func TestListRun_json(t *testing.T) { ios, _, stdout, _ := iostreams.Test() mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { - return client.DiscussionListResult{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { + return &client.DiscussionListResult{ Discussions: sampleDiscussions(), TotalCount: 2, NextCursor: "CURSOR123", @@ -199,8 +199,8 @@ func TestListRun_noResults(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { - return client.DiscussionListResult{}, nil + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { + return &client.DiscussionListResult{}, nil }, } @@ -229,9 +229,9 @@ func TestListRun_categoryFilter(t *testing.T) { ListCategoriesFunc: func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { return sampleCategories(), nil }, - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { assert.Equal(t, "CAT1", filters.CategoryID) - return client.DiscussionListResult{ + return &client.DiscussionListResult{ Discussions: sampleDiscussions()[:1], TotalCount: 1, }, nil @@ -286,9 +286,9 @@ func TestListRun_authorFilter(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (*client.DiscussionListResult, error) { assert.Equal(t, "monalisa", filters.Author) - return client.DiscussionListResult{ + return &client.DiscussionListResult{ Discussions: sampleDiscussions()[:1], TotalCount: 1, }, nil @@ -317,9 +317,9 @@ func TestListRun_labelFilter(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (*client.DiscussionListResult, error) { assert.Equal(t, []string{"bug", "docs"}, filters.Labels) - return client.DiscussionListResult{ + return &client.DiscussionListResult{ Discussions: sampleDiscussions()[:1], TotalCount: 1, }, nil @@ -348,9 +348,9 @@ func TestListRun_searchFilter(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (client.DiscussionListResult, error) { + SearchFunc: func(repo ghrepo.Interface, filters client.SearchFilters, after string, limit int) (*client.DiscussionListResult, error) { assert.Equal(t, "some keywords", filters.Keywords) - return client.DiscussionListResult{ + return &client.DiscussionListResult{ Discussions: sampleDiscussions()[:1], TotalCount: 1, }, nil @@ -379,7 +379,7 @@ func TestListRun_afterCursor(t *testing.T) { ios.SetStdoutTTY(true) mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { assert.Equal(t, "CURSOR_ABC", after) return sampleResult(), nil }, @@ -582,8 +582,8 @@ func TestListRun_closedState(t *testing.T) { } mockClient := &client.DiscussionClientMock{ - ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (client.DiscussionListResult, error) { - return client.DiscussionListResult{ + ListFunc: func(repo ghrepo.Interface, filters client.ListFilters, after string, limit int) (*client.DiscussionListResult, error) { + return &client.DiscussionListResult{ Discussions: closed, TotalCount: 1, }, nil From 9afbd61c5f14c75cdf7124c3f48aba6819cc4f9a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 13:25:42 +0100 Subject: [PATCH 12/21] refactor(discussion/client): use strongly-typed query for search function Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 203 +++++++++++++++++------ 1 file changed, 153 insertions(+), 50 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 9d22454d7..86d142762 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -22,7 +22,7 @@ func NewDiscussionClient(httpClient *http.Client) DiscussionClient { } } -// discussionNode is the shared GraphQL response shape for a single discussion, +// discussionNode is the GraphQL response shape for a single discussion, // used by both List and Search to avoid duplicating the field mapping. type discussionNode struct { ID string `json:"id"` @@ -69,6 +69,74 @@ type discussionNode struct { Locked bool `json:"locked"` } +// actorNode is the GraphQL response shape for an Actor union (User or Bot) +// used in discussionListNode fields like Author and AnswerChosenBy. +type actorNode struct { + TypeName string `graphql:"__typename"` + Login string + User struct { + ID string + Name string + } `graphql:"... on User"` + Bot struct { + ID string + } `graphql:"... on Bot"` +} + +// mapActorFromListNode converts an actorNode into the domain DiscussionActor type. +func mapActorFromListNode(n actorNode) DiscussionActor { + a := DiscussionActor{Login: n.Login} + switch n.TypeName { + case "User": + a.ID = n.User.ID + a.Name = n.User.Name + case "Bot": + a.ID = n.Bot.ID + } + return a +} + +// discussionListNode is the GraphQL response shape for a discussion in +// list and search results. It covers high-level fields only (no comments, or +// other detail-level data that commands like view would need). +type discussionListNode struct { + ID string + Number int + Title string + Body string + URL string `graphql:"url"` + Closed bool + StateReason string + Author actorNode + Category struct { + ID string + Name string + Slug string + Emoji string + IsAnswerable bool + } + Labels struct { + Nodes []struct { + ID string + Name string + Color string + } + } `graphql:"labels(first: 20)"` + IsAnswered bool + AnswerChosenAt time.Time + AnswerChosenBy *actorNode + ReactionGroups []struct { + Content string + Users struct { + TotalCount int + } + } + CreatedAt time.Time + UpdatedAt time.Time + ClosedAt time.Time + Locked bool +} + // mapDiscussion converts a GraphQL discussionNode response into the domain Discussion type. func mapDiscussion(n discussionNode) Discussion { d := Discussion{ @@ -119,6 +187,50 @@ func mapDiscussion(n discussionNode) Discussion { return d } +// mapDiscussionFromListNode converts a discussionListNode into the domain Discussion type. +func mapDiscussionFromListNode(n discussionListNode) Discussion { + d := Discussion{ + ID: n.ID, + Number: n.Number, + Title: n.Title, + Body: n.Body, + URL: n.URL, + Closed: n.Closed, + StateReason: n.StateReason, + Author: mapActorFromListNode(n.Author), + Category: DiscussionCategory{ + ID: n.Category.ID, + Name: n.Category.Name, + Slug: n.Category.Slug, + Emoji: n.Category.Emoji, + IsAnswerable: n.Category.IsAnswerable, + }, + Answered: n.IsAnswered, + AnswerChosenAt: n.AnswerChosenAt, + CreatedAt: n.CreatedAt, + UpdatedAt: n.UpdatedAt, + ClosedAt: n.ClosedAt, + Locked: n.Locked, + } + + if n.AnswerChosenBy != nil { + a := mapActorFromListNode(*n.AnswerChosenBy) + d.AnswerChosenBy = &a + } + + d.Labels = make([]DiscussionLabel, len(n.Labels.Nodes)) + for i, l := range n.Labels.Nodes { + d.Labels[i] = DiscussionLabel{ID: l.ID, Name: l.Name, Color: l.Color} + } + + d.ReactionGroups = make([]ReactionGroup, len(n.ReactionGroups)) + for i, rg := range n.ReactionGroups { + d.ReactionGroups[i] = ReactionGroup{Content: rg.Content, TotalCount: rg.Users.TotalCount} + } + + return d +} + // discussionFields is the GraphQL fragment selecting fields for discussion queries. // It is shared by both List (repository.discussions) and Search queries. const discussionFields = ` @@ -284,15 +396,17 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, return nil, fmt.Errorf("limit argument must be positive: %v", limit) } - type response struct { + var query struct { Search struct { - DiscussionCount int `json:"discussionCount"` + DiscussionCount int PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` - Nodes []discussionNode `json:"nodes"` - } `json:"search"` + HasNextPage bool + EndCursor string + } + Nodes []struct { + Discussion discussionListNode `graphql:"... on Discussion"` + } + } `graphql:"search(query: $query, type: DISCUSSION, first: $first, after: $after)"` } qualifiers := []string{fmt.Sprintf("repo:%s/%s", repo.RepoOwner(), repo.RepoName())} @@ -300,9 +414,9 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, if filters.State != nil { switch *filters.State { case FilterStateOpen: - qualifiers = append(qualifiers, "state:open") + qualifiers = append(qualifiers, "is:open") case FilterStateClosed: - qualifiers = append(qualifiers, "state:closed") + qualifiers = append(qualifiers, "is:closed") default: return nil, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) } @@ -325,20 +439,24 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, } } - orderField := OrderByUpdated - orderDir := OrderDirectionDesc + orderField := "updated" + orderDir := "desc" if filters.OrderBy != "" { switch filters.OrderBy { - case OrderByCreated, OrderByUpdated: - orderField = filters.OrderBy + case OrderByCreated: + orderField = "created" + case OrderByUpdated: + orderField = "updated" default: return nil, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) } } if filters.Direction != "" { switch filters.Direction { - case OrderDirectionAsc, OrderDirectionDesc: - orderDir = filters.Direction + case OrderDirectionAsc: + orderDir = "asc" + case OrderDirectionDesc: + orderDir = "desc" default: return nil, fmt.Errorf("unknown order direction: %q", filters.Direction) } @@ -350,59 +468,44 @@ func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, searchQuery += " " + filters.Keywords } - query := fmt.Sprintf(`query DiscussionSearch($query: String!, $first: Int!, $after: String) { - search(query: $query, type: DISCUSSION, first: $first, after: $after) { - discussionCount - pageInfo { hasNextPage endCursor } - nodes { ... on Discussion { %s } } - } - }`, discussionFields) + perPage := limit + if perPage > 100 { + perPage = 100 + } variables := map[string]interface{}{ - "query": searchQuery, + "query": githubv4.String(searchQuery), + "first": githubv4.Int(perPage), + "after": (*githubv4.String)(nil), } - if after != "" { - variables["after"] = after + variables["after"] = githubv4.String(after) } - var discussions []Discussion - var totalCount int - var nextCursor string + var result DiscussionListResult remaining := limit for { - perPage := remaining - if perPage > 100 { - perPage = 100 - } - variables["first"] = perPage - - var data response - if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { + if err := c.gql.Query(repo.RepoHost(), "DiscussionListSearch", &query, variables); err != nil { return nil, err } - totalCount = data.Search.DiscussionCount - for _, n := range data.Search.Nodes { - discussions = append(discussions, mapDiscussion(n)) + result.TotalCount = query.Search.DiscussionCount + for _, n := range query.Search.Nodes { + result.Discussions = append(result.Discussions, mapDiscussionFromListNode(n.Discussion)) } - remaining -= len(data.Search.Nodes) - if remaining <= 0 || !data.Search.PageInfo.HasNextPage { - if data.Search.PageInfo.HasNextPage { - nextCursor = data.Search.PageInfo.EndCursor + remaining -= len(query.Search.Nodes) + if remaining <= 0 || !query.Search.PageInfo.HasNextPage { + if query.Search.PageInfo.HasNextPage { + result.NextCursor = query.Search.PageInfo.EndCursor } break } - variables["after"] = data.Search.PageInfo.EndCursor + variables["after"] = githubv4.String(query.Search.PageInfo.EndCursor) } - return &DiscussionListResult{ - Discussions: discussions, - TotalCount: totalCount, - NextCursor: nextCursor, - }, nil + return &result, nil } func (c *discussionClient) GetByNumber(_ ghrepo.Interface, _ int) (*Discussion, error) { From e403e82633045baa3d19c00b1b4d24f0d8e6eb53 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 14:30:21 +0100 Subject: [PATCH 13/21] refactor(discussion/client): use strongly-typed query for list function Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 247 +++++------------------ 1 file changed, 51 insertions(+), 196 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 86d142762..265b0a481 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -22,53 +22,6 @@ func NewDiscussionClient(httpClient *http.Client) DiscussionClient { } } -// discussionNode is the GraphQL response shape for a single discussion, -// used by both List and Search to avoid duplicating the field mapping. -type discussionNode struct { - ID string `json:"id"` - Number int `json:"number"` - Title string `json:"title"` - URL string `json:"url"` - Closed bool `json:"closed"` - StateReason string `json:"stateReason"` - Author struct { - Login string `json:"login"` - ID string `json:"id"` - Name string `json:"name"` - } `json:"author"` - Category struct { - ID string `json:"id"` - Name string `json:"name"` - Slug string `json:"slug"` - Emoji string `json:"emoji"` - IsAnswerable bool `json:"isAnswerable"` - } `json:"category"` - Labels struct { - Nodes []struct { - ID string `json:"id"` - Name string `json:"name"` - Color string `json:"color"` - } `json:"nodes"` - } `json:"labels"` - IsAnswered bool `json:"isAnswered"` - AnswerChosenAt time.Time `json:"answerChosenAt"` - AnswerChosenBy *struct { - Login string `json:"login"` - ID string `json:"id"` - Name string `json:"name"` - } `json:"answerChosenBy"` - ReactionGroups []struct { - Content string `json:"content"` - Users struct { - TotalCount int `json:"totalCount"` - } `json:"users"` - } `json:"reactionGroups"` - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` - ClosedAt time.Time `json:"closedAt"` - Locked bool `json:"locked"` -} - // actorNode is the GraphQL response shape for an Actor union (User or Bot) // used in discussionListNode fields like Author and AnswerChosenBy. type actorNode struct { @@ -137,56 +90,6 @@ type discussionListNode struct { Locked bool } -// mapDiscussion converts a GraphQL discussionNode response into the domain Discussion type. -func mapDiscussion(n discussionNode) Discussion { - d := Discussion{ - ID: n.ID, - Number: n.Number, - Title: n.Title, - URL: n.URL, - Closed: n.Closed, - StateReason: n.StateReason, - Author: DiscussionActor{ - ID: n.Author.ID, - Login: n.Author.Login, - Name: n.Author.Name, - }, - Category: DiscussionCategory{ - ID: n.Category.ID, - Name: n.Category.Name, - Slug: n.Category.Slug, - Emoji: n.Category.Emoji, - IsAnswerable: n.Category.IsAnswerable, - }, - Answered: n.IsAnswered, - AnswerChosenAt: n.AnswerChosenAt, - CreatedAt: n.CreatedAt, - UpdatedAt: n.UpdatedAt, - ClosedAt: n.ClosedAt, - Locked: n.Locked, - } - - if n.AnswerChosenBy != nil { - d.AnswerChosenBy = &DiscussionActor{ - ID: n.AnswerChosenBy.ID, - Login: n.AnswerChosenBy.Login, - Name: n.AnswerChosenBy.Name, - } - } - - d.Labels = make([]DiscussionLabel, len(n.Labels.Nodes)) - for i, l := range n.Labels.Nodes { - d.Labels[i] = DiscussionLabel{ID: l.ID, Name: l.Name, Color: l.Color} - } - - d.ReactionGroups = make([]ReactionGroup, len(n.ReactionGroups)) - for i, rg := range n.ReactionGroups { - d.ReactionGroups[i] = ReactionGroup{Content: rg.Content, TotalCount: rg.Users.TotalCount} - } - - return d -} - // mapDiscussionFromListNode converts a discussionListNode into the domain Discussion type. func mapDiscussionFromListNode(n discussionListNode) Discussion { d := Discussion{ @@ -231,50 +134,33 @@ func mapDiscussionFromListNode(n discussionListNode) Discussion { return d } -// discussionFields is the GraphQL fragment selecting fields for discussion queries. -// It is shared by both List (repository.discussions) and Search queries. -const discussionFields = ` - id number title body url closed stateReason - author { login ...on User { id name } ...on Bot { id } } - category { id name slug emoji isAnswerable } - labels(first: 20) { nodes { id name color } } - isAnswered answerChosenAt answerChosenBy { login ...on User { id name } ...on Bot { id } } - reactionGroups { content users { totalCount } } - createdAt updatedAt closedAt locked -` - func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { if limit <= 0 { return nil, fmt.Errorf("limit argument must be positive: %v", limit) } - type response struct { + var query struct { Repository struct { - HasDiscussionsEnabled bool `json:"hasDiscussionsEnabled"` + HasDiscussionsEnabled bool Discussions struct { - TotalCount int `json:"totalCount"` + TotalCount int PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` - Nodes []discussionNode `json:"nodes"` - } `json:"discussions"` - } `json:"repository"` + HasNextPage bool + EndCursor string + } + Nodes []discussionListNode + } `graphql:"discussions(first: $first, after: $after, orderBy: $orderBy, categoryId: $categoryId, states: $states, answered: $answered)"` + } `graphql:"repository(owner: $owner, name: $name)"` } - variables := map[string]interface{}{ - "owner": repo.RepoOwner(), - "name": repo.RepoName(), - } - - orderField := "UPDATED_AT" - orderDir := "DESC" + orderField := githubv4.DiscussionOrderFieldUpdatedAt + orderDir := githubv4.OrderDirectionDesc if filters.OrderBy != "" { switch filters.OrderBy { case OrderByCreated: - orderField = "CREATED_AT" + orderField = githubv4.DiscussionOrderFieldCreatedAt case OrderByUpdated: - orderField = "UPDATED_AT" + orderField = githubv4.DiscussionOrderFieldUpdatedAt default: return nil, fmt.Errorf("unknown order-by field: %q", filters.OrderBy) } @@ -282,113 +168,82 @@ func (c *discussionClient) List(repo ghrepo.Interface, filters ListFilters, afte if filters.Direction != "" { switch filters.Direction { case OrderDirectionAsc: - orderDir = "ASC" + orderDir = githubv4.OrderDirectionAsc case OrderDirectionDesc: - orderDir = "DESC" + orderDir = githubv4.OrderDirectionDesc default: return nil, fmt.Errorf("unknown order direction: %q", filters.Direction) } } - variables["orderBy"] = map[string]string{ - "field": orderField, - "direction": orderDir, + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "first": githubv4.Int(perPage), + "after": (*githubv4.String)(nil), + "orderBy": githubv4.DiscussionOrder{Field: orderField, Direction: orderDir}, + "categoryId": (*githubv4.ID)(nil), + "states": (*[]githubv4.DiscussionState)(nil), + "answered": (*githubv4.Boolean)(nil), + } + + if after != "" { + variables["after"] = githubv4.String(after) } if filters.CategoryID != "" { - variables["categoryId"] = filters.CategoryID + variables["categoryId"] = githubv4.ID(filters.CategoryID) } if filters.State != nil { switch *filters.State { case FilterStateOpen: - variables["states"] = []string{"OPEN"} + variables["states"] = &[]githubv4.DiscussionState{githubv4.DiscussionStateOpen} case FilterStateClosed: - variables["states"] = []string{"CLOSED"} + variables["states"] = &[]githubv4.DiscussionState{githubv4.DiscussionStateClosed} default: return nil, fmt.Errorf("unknown state filter: %q; should be one of %q, %q", *filters.State, FilterStateOpen, FilterStateClosed) } } if filters.Answered != nil { - variables["answered"] = *filters.Answered + variables["answered"] = githubv4.Boolean(*filters.Answered) } - query := fmt.Sprintf(`query DiscussionList( - $owner: String!, - $name: String!, - $first: Int!, - $after: String, - $orderBy: DiscussionOrder, - $categoryId: ID, - $states: [DiscussionState!], - $answered: Boolean - ) { - repository(owner: $owner, name: $name) { - hasDiscussionsEnabled - discussions( - first: $first, - after: $after, - orderBy: $orderBy, - categoryId: $categoryId, - states: $states, - answered: $answered - ) { - totalCount - pageInfo { hasNextPage endCursor } - nodes { %s } - } - } - }`, discussionFields) - - if after != "" { - variables["after"] = after - } - - var discussions []Discussion - var totalCount int - var nextCursor string + var result DiscussionListResult remaining := limit - // Check hasDiscussionsEnabled on first request only - firstPage := true - for { - perPage := remaining - if perPage > 100 { - perPage = 100 - } - variables["first"] = perPage - - var data response - if err := c.gql.GraphQL(repo.RepoHost(), query, variables, &data); err != nil { + if err := c.gql.Query(repo.RepoHost(), "DiscussionList", &query, variables); err != nil { return nil, err } - if firstPage && !data.Repository.HasDiscussionsEnabled { + if !query.Repository.HasDiscussionsEnabled { + // This would be the same over every iteration, so if we're going to return we will at the first page. return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } - firstPage = false - totalCount = data.Repository.Discussions.TotalCount - for _, n := range data.Repository.Discussions.Nodes { - discussions = append(discussions, mapDiscussion(n)) + result.TotalCount = query.Repository.Discussions.TotalCount + for _, n := range query.Repository.Discussions.Nodes { + result.Discussions = append(result.Discussions, mapDiscussionFromListNode(n)) } - remaining -= len(data.Repository.Discussions.Nodes) - if remaining <= 0 || !data.Repository.Discussions.PageInfo.HasNextPage { - if data.Repository.Discussions.PageInfo.HasNextPage { - nextCursor = data.Repository.Discussions.PageInfo.EndCursor + remaining -= len(query.Repository.Discussions.Nodes) + if remaining <= 0 || !query.Repository.Discussions.PageInfo.HasNextPage { + if query.Repository.Discussions.PageInfo.HasNextPage { + result.NextCursor = query.Repository.Discussions.PageInfo.EndCursor } break } - variables["after"] = data.Repository.Discussions.PageInfo.EndCursor + variables["after"] = githubv4.String(query.Repository.Discussions.PageInfo.EndCursor) } - return &DiscussionListResult{ - Discussions: discussions, - TotalCount: totalCount, - NextCursor: nextCursor, - }, nil + return &result, nil } func (c *discussionClient) Search(repo ghrepo.Interface, filters SearchFilters, after string, limit int) (*DiscussionListResult, error) { From afb1b7dfea7c60d9827986c69b8cac60d7e6716f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 14:36:21 +0100 Subject: [PATCH 14/21] fix(discussion/shared): print quoted category slugs Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/categories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/shared/categories.go b/pkg/cmd/discussion/shared/categories.go index a7893f229..5e0ca33cd 100644 --- a/pkg/cmd/discussion/shared/categories.go +++ b/pkg/cmd/discussion/shared/categories.go @@ -28,5 +28,5 @@ func MatchCategory(input string, categories []client.DiscussionCategory) (*clien slugs[i] = c.Slug } slices.Sort(slugs) - return nil, fmt.Errorf("unknown category: %q; must be one of %v", input, slugs) + return nil, fmt.Errorf("unknown category: %q; must be one of %q", input, slugs) } From 9a42e904a5eae6c465ab0f6bed6bc0bc97ef55ac Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 14:37:25 +0100 Subject: [PATCH 15/21] fix(discussion/shared): deleted unused fields slice Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/shared/fields.go | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 pkg/cmd/discussion/shared/fields.go diff --git a/pkg/cmd/discussion/shared/fields.go b/pkg/cmd/discussion/shared/fields.go deleted file mode 100644 index 0feb789d9..000000000 --- a/pkg/cmd/discussion/shared/fields.go +++ /dev/null @@ -1,25 +0,0 @@ -package shared - -// DiscussionFields lists the field names available for --json output on -// discussion commands. -var DiscussionFields = []string{ - "id", - "number", - "title", - "body", - "url", - "closed", - "stateReason", - "author", - "category", - "labels", - "answered", - "answerChosenAt", - "answerChosenBy", - "comments", - "reactionGroups", - "createdAt", - "updatedAt", - "closedAt", - "locked", -} From 835d7f3a4875037d17f9cafa838418f80d630f2f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 14:50:40 +0100 Subject: [PATCH 16/21] fix(discussion/client): remove reaction groups from list/search types Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/client/client_impl.go | 7 +------ pkg/cmd/discussion/client/types.go | 7 ------- pkg/cmd/discussion/list/list.go | 1 - 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 265b0a481..ed44e49be 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -83,7 +83,7 @@ type discussionListNode struct { Users struct { TotalCount int } - } + } `graphql:"reactionGroups"` CreatedAt time.Time UpdatedAt time.Time ClosedAt time.Time @@ -126,11 +126,6 @@ func mapDiscussionFromListNode(n discussionListNode) Discussion { d.Labels[i] = DiscussionLabel{ID: l.ID, Name: l.Name, Color: l.Color} } - d.ReactionGroups = make([]ReactionGroup, len(n.ReactionGroups)) - for i, rg := range n.ReactionGroups { - d.ReactionGroups[i] = ReactionGroup{Content: rg.Content, TotalCount: rg.Users.TotalCount} - } - return d } diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 1de2bae1a..f3c58456c 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -19,7 +19,6 @@ type Discussion struct { AnswerChosenAt time.Time AnswerChosenBy *DiscussionActor Comments DiscussionCommentList - ReactionGroups []ReactionGroup CreatedAt time.Time UpdatedAt time.Time ClosedAt time.Time @@ -80,12 +79,6 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { "totalCount": d.Comments.TotalCount, "nodes": comments, } - case "reactionGroups": - groups := make([]interface{}, len(d.ReactionGroups)) - for i, rg := range d.ReactionGroups { - groups[i] = rg.Export() - } - data[f] = groups case "createdAt": data[f] = d.CreatedAt case "updatedAt": diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index f4305b926..77ac2a7ef 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -37,7 +37,6 @@ var discussionListFields = []string{ "answered", "answerChosenAt", "answerChosenBy", - "reactionGroups", "createdAt", "updatedAt", "closedAt", From 2d1b1a79bf76e8cd9a9b3154b4c1512ade1aaade Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 14:52:05 +0100 Subject: [PATCH 17/21] fix(discussion list): only print remaining items in tty mode Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 77ac2a7ef..0fe2c4488 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -346,8 +346,7 @@ func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalC _ = tp.Render() - remaining := totalCount - len(discussions) - if remaining > 0 { + if remaining := totalCount - len(discussions); isTerminal && remaining > 0 { fmt.Fprintf(opts.IO.Out, cs.Muted("And %d more\n"), remaining) } } From d45ce940eb2e8d0428f5adcb0643b2066fbb8c37 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 15:00:17 +0100 Subject: [PATCH 18/21] refactor(discussion list): inline printed messages Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 0fe2c4488..76ef86b32 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -273,25 +273,25 @@ func openInBrowser(opts *ListOptions, repo ghrepo.Interface) error { } func noResults(repo ghrepo.Interface, state string) error { - stateQualifier := "" switch state { case "open": - stateQualifier = " open" + return cmdutil.NewNoResultsError(fmt.Sprintf("no open discussions match your search in %s", ghrepo.FullName(repo))) case "closed": - stateQualifier = " closed" + return cmdutil.NewNoResultsError(fmt.Sprintf("no closed discussions match your search in %s", ghrepo.FullName(repo))) + default: + return cmdutil.NewNoResultsError(fmt.Sprintf("no discussions match your search in %s", ghrepo.FullName(repo))) } - return cmdutil.NewNoResultsError(fmt.Sprintf("no%s discussions match your search in %s", stateQualifier, ghrepo.FullName(repo))) } func listHeader(repoName string, count, total int, state string) string { - stateQualifier := "" switch state { case "open": - stateQualifier = " open" + return fmt.Sprintf("Showing %d of %d open discussions in %s", count, total, repoName) case "closed": - stateQualifier = " closed" + return fmt.Sprintf("Showing %d of %d closed discussions in %s", count, total, repoName) + default: + return fmt.Sprintf("Showing %d of %d discussions in %s", count, total, repoName) } - return fmt.Sprintf("Showing %d of %d%s discussions in %s", count, total, stateQualifier, repoName) } func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalCount int) { From 8d70cc9ca8be3031a2c339af71638c65d5d665bf Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 15:03:02 +0100 Subject: [PATCH 19/21] refactor(discussion list): encapsulate printing within `printDiscussions` Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 76ef86b32..48c53380b 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -228,13 +228,7 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - isTerminal := opts.IO.IsStdoutTTY() - if isTerminal { - title := listHeader(ghrepo.FullName(repo), len(result.Discussions), result.TotalCount, opts.State) - fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) - } - - printDiscussions(opts, result.Discussions, result.TotalCount) + printDiscussions(opts, ghrepo.FullName(repo), result.Discussions, result.TotalCount) return nil } @@ -294,11 +288,16 @@ func listHeader(repoName string, count, total int, state string) string { } } -func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalCount int) { +func printDiscussions(opts *ListOptions, repoName string, discussions []client.Discussion, totalCount int) { isTerminal := opts.IO.IsStdoutTTY() cs := opts.IO.ColorScheme() now := opts.Now() + if isTerminal { + title := listHeader(repoName, len(discussions), totalCount, opts.State) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) + } + headers := []string{"ID", "TITLE", "CATEGORY", "LABELS", "ANSWERED", "UPDATED"} if !isTerminal { headers = []string{"ID", "STATE", "TITLE", "CATEGORY", "LABELS", "ANSWERED", "UPDATED"} @@ -309,7 +308,7 @@ func printDiscussions(opts *ListOptions, discussions []client.Discussion, totalC if isTerminal { idColor := cs.Green if d.Closed { - idColor = cs.Gray + idColor = cs.Muted } tp.AddField(fmt.Sprintf("#%d", d.Number), tableprinter.WithColor(idColor)) } else { From f62875fb63dcbd97c6444fbefa853923654dc02b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 15:05:22 +0100 Subject: [PATCH 20/21] fix(discussion list): quote author qualifier in web mode Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 48c53380b..26114cf5e 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -240,7 +240,7 @@ func openInBrowser(opts *ListOptions, repo ghrepo.Interface) error { queryParts = append(queryParts, "is:"+opts.State) } if opts.Author != "" { - queryParts = append(queryParts, "author:"+opts.Author) + queryParts = append(queryParts, fmt.Sprintf("author:%q", opts.Author)) } for _, l := range opts.Labels { queryParts = append(queryParts, fmt.Sprintf("label:%q", l)) From 2a4a982ae436e3bce92f9f6a8902c2a6cd71645c Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 14 Apr 2026 15:07:32 +0100 Subject: [PATCH 21/21] fix(discussion list): include search keywords in web mode Signed-off-by: Babak K. Shandiz --- pkg/cmd/discussion/list/list.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/discussion/list/list.go b/pkg/cmd/discussion/list/list.go index 26114cf5e..3de5df6c6 100644 --- a/pkg/cmd/discussion/list/list.go +++ b/pkg/cmd/discussion/list/list.go @@ -236,6 +236,9 @@ func openInBrowser(opts *ListOptions, repo ghrepo.Interface) error { discussionsURL := ghrepo.GenerateRepoURL(repo, "discussions") var queryParts []string + if opts.Search != "" { + queryParts = append(queryParts, opts.Search) + } if opts.State != "" && opts.State != "all" { queryParts = append(queryParts, "is:"+opts.State) }