STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Address review feedback on discussion list PR

Browse source 
- 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>
This commit is contained in:
Max Beizer 2026-04-13 10:20:09 -05:00
parent 90449b5197
commit e84ecb1585
No known key found for this signature in database
8 changed files with 439 additions and 131 deletions
Download patch file Download diff file Expand all files Collapse all files

4
pkg/cmd/discussion/client/client.go
View file

@ -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)

173
pkg/cmd/discussion/client/client_impl.go
View file

@ -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) {

28
pkg/cmd/discussion/client/client_mock.go
View file

@ -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()

33
pkg/cmd/discussion/client/types.go
View file

@ -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
}

6
pkg/cmd/discussion/discussion.go
View file

@ -11,8 +11,10 @@ import (
func NewCmdDiscussion(f *cmdutil.Factory) *cobra.Command {
cmd := &cobra.Command{
Use: "discussion <command>",
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"

101
pkg/cmd/discussion/list/list.go
View file

@ -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 {

193
pkg/cmd/discussion/list/list_test.go
View file

@ -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")
}

32
pkg/cmd/discussion/shared/categories.go Normal file
View file

@ -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)
}