From d2e081bce13341b64c875510eb26393cc4a7d76f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 24 Apr 2026 23:50:04 +0100 Subject: [PATCH] feat(discussion view): add cursor-based pagination to comments Add --limit and --after flags for paginating through discussion comments. Cursor output is shown in TTY (hint message), raw (next: field), and JSON. Change GetWithComments to accept 'after' cursor and 'newest' bool instead of order string. Implement forward/backward cursor-based pagination in GraphQL queries depending on comment order. Change Replies from []DiscussionComment to DiscussionCommentList with Direction field. Display direction-aware messages (newer/older) for both comments and replies. Move DiscussionFields and reactionGroupList from shared to view package. Delete shared/display.go. Add 7 new pagination tests and update existing test fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client.go | 2 +- pkg/cmd/discussion/client/client_impl.go | 81 ++++++-- pkg/cmd/discussion/client/client_mock.go | 26 ++- pkg/cmd/discussion/client/types.go | 32 ++- pkg/cmd/discussion/shared/client.go | 25 --- pkg/cmd/discussion/shared/display.go | 35 ---- pkg/cmd/discussion/view/view.go | 112 +++++++++- pkg/cmd/discussion/view/view_test.go | 251 +++++++++++++++++++++-- 8 files changed, 438 insertions(+), 126 deletions(-) delete mode 100644 pkg/cmd/discussion/shared/display.go diff --git a/pkg/cmd/discussion/client/client.go b/pkg/cmd/discussion/client/client.go index a794d13e1..5c7f1bdc4 100644 --- a/pkg/cmd/discussion/client/client.go +++ b/pkg/cmd/discussion/client/client.go @@ -12,7 +12,7 @@ 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) GetByNumber(repo ghrepo.Interface, number int) (*Discussion, error) - GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) + GetWithComments(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) ListCategories(repo ghrepo.Interface) ([]DiscussionCategory, error) Create(repo ghrepo.Interface, input CreateDiscussionInput) (*Discussion, error) Update(repo ghrepo.Interface, input UpdateDiscussionInput) (*Discussion, error) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 7b4ad3ce9..d83c9d35d 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -3,6 +3,7 @@ package client import ( "fmt" "net/http" + "slices" "strings" "time" @@ -355,9 +356,8 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc var query struct { Repository struct { HasDiscussionsEnabled bool - Discussion *struct { + Discussion struct { discussionListNode - Body string Comments struct { TotalCount int } @@ -371,19 +371,15 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc "number": githubv4.Int(number), } - err := c.gql.Query(repo.RepoHost(), "DiscussionByNumber", &query, variables) + err := c.gql.Query(repo.RepoHost(), "DiscussionMinimal", &query, variables) if err != nil { return nil, err } if !query.Repository.HasDiscussionsEnabled { return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } - if query.Repository.Discussion == nil { - return nil, fmt.Errorf("discussion #%d not found in '%s/%s'", number, repo.RepoOwner(), repo.RepoName()) - } d := mapDiscussionFromListNode(query.Repository.Discussion.discussionListNode) - d.Body = query.Repository.Discussion.Body d.Comments = DiscussionCommentList{TotalCount: query.Repository.Discussion.Comments.TotalCount} for _, rg := range query.Repository.Discussion.ReactionGroups { @@ -396,12 +392,19 @@ func (c *discussionClient) GetByNumber(repo ghrepo.Interface, number int) (*Disc return &d, nil } -func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, limit int, after string, newest bool) (*Discussion, error) { // Build the comments field with first/last based on order. - // "oldest" uses first (chronological), "newest" uses last (reverse chronological). + // oldest uses first+after (chronological), newest uses last+before (reverse). commentDirection := "first" - if order == "newest" { + cursorDirection := "after" + if newest { commentDirection = "last" + cursorDirection = "before" + } + + cursorArg := "" + if after != "" { + cursorArg = fmt.Sprintf(`, %s: "%s"`, cursorDirection, after) } query := fmt.Sprintf(`query DiscussionWithComments($owner: String!, $name: String!, $number: Int!) { @@ -426,8 +429,14 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co updatedAt closedAt locked - comments(%s: %d) { + comments(%s: %d%s) { totalCount + pageInfo { + endCursor + hasNextPage + startCursor + hasPreviousPage + } nodes { id url @@ -437,7 +446,7 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co isAnswer upvoteCount reactionGroups { content users { totalCount } } - replies(first: 4) { + replies(last: 4) { totalCount nodes { id @@ -454,7 +463,7 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co } } } - }`, commentDirection, commentLimit) + }`, commentDirection, limit, cursorArg) variables := map[string]interface{}{ "owner": repo.RepoOwner(), @@ -525,8 +534,14 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co ClosedAt time.Time `json:"closedAt"` Locked bool `json:"locked"` Comments struct { - TotalCount int `json:"totalCount"` - Nodes []commentJSON `json:"nodes"` + TotalCount int `json:"totalCount"` + PageInfo struct { + EndCursor string `json:"endCursor"` + HasNextPage bool `json:"hasNextPage"` + StartCursor string `json:"startCursor"` + HasPreviousPage bool `json:"hasPreviousPage"` + } `json:"pageInfo"` + Nodes []commentJSON `json:"nodes"` } `json:"comments"` } `json:"discussion"` } `json:"repository"` @@ -570,9 +585,9 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co ReactionGroups: mapReactions(c.ReactionGroups), } if c.Replies != nil { - dc.TotalReplies = c.Replies.TotalCount - for _, r := range c.Replies.Nodes { - dc.Replies = append(dc.Replies, DiscussionComment{ + replyComments := make([]DiscussionComment, len(c.Replies.Nodes)) + for i, r := range c.Replies.Nodes { + replyComments[i] = DiscussionComment{ ID: r.ID, URL: r.URL, Author: mapActor(r.Author), @@ -581,7 +596,12 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co IsAnswer: r.IsAnswer, UpvoteCount: r.UpvoteCount, ReactionGroups: mapReactions(r.ReactionGroups), - }) + } + } + dc.Replies = DiscussionCommentList{ + Comments: replyComments, + TotalCount: c.Replies.TotalCount, + Direction: DiscussionCommentListDirectionBackward, // Since we always fetch the last 4 replies } } return dc @@ -629,15 +649,32 @@ func (c *discussionClient) GetWithComments(repo ghrepo.Interface, number int, co // When using "last" (newest order), the API returns items in chronological // order. Reverse them so the newest comment appears first. - if order == "newest" { - for i, j := 0, len(comments)-1; i < j; i, j = i+1, j-1 { - comments[i], comments[j] = comments[j], comments[i] + if newest { + slices.Reverse(comments) + } + + nextCursor := "" + if newest { + if src.Comments.PageInfo.HasPreviousPage { + nextCursor = src.Comments.PageInfo.StartCursor } + } else { + if src.Comments.PageInfo.HasNextPage { + nextCursor = src.Comments.PageInfo.EndCursor + } + } + + direction := DiscussionCommentListDirectionForward + if newest { + direction = DiscussionCommentListDirectionBackward } d.Comments = DiscussionCommentList{ Comments: comments, TotalCount: src.Comments.TotalCount, + Cursor: after, + NextCursor: nextCursor, + Direction: direction, } return &d, nil diff --git a/pkg/cmd/discussion/client/client_mock.go b/pkg/cmd/discussion/client/client_mock.go index a690f84b1..a3cec3c63 100644 --- a/pkg/cmd/discussion/client/client_mock.go +++ b/pkg/cmd/discussion/client/client_mock.go @@ -30,7 +30,7 @@ var _ DiscussionClient = &DiscussionClientMock{} // GetByNumberFunc: func(repo ghrepo.Interface, number int) (*Discussion, error) { // panic("mock out the GetByNumber method") // }, -// GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +// GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) { // panic("mock out the GetWithComments method") // }, // ListFunc: func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) { @@ -80,7 +80,7 @@ type DiscussionClientMock struct { GetByNumberFunc func(repo ghrepo.Interface, number int) (*Discussion, error) // GetWithCommentsFunc mocks the GetWithComments method. - GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) + GetWithCommentsFunc func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) // ListFunc mocks the List method. ListFunc func(repo ghrepo.Interface, filters ListFilters, after string, limit int) (*DiscussionListResult, error) @@ -153,8 +153,10 @@ type DiscussionClientMock struct { Number int // CommentLimit is the commentLimit argument value. CommentLimit int - // Order is the order argument value. - Order string + // After is the after argument value. + After string + // Newest is the newest argument value. + Newest bool } // List holds details about calls to the List method. List []struct { @@ -401,7 +403,7 @@ func (mock *DiscussionClientMock) GetByNumberCalls() []struct { } // GetWithComments calls GetWithCommentsFunc. -func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, order string) (*Discussion, error) { +func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*Discussion, error) { if mock.GetWithCommentsFunc == nil { panic("DiscussionClientMock.GetWithCommentsFunc: method is nil but DiscussionClient.GetWithComments was just called") } @@ -409,17 +411,19 @@ func (mock *DiscussionClientMock) GetWithComments(repo ghrepo.Interface, number Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool }{ Repo: repo, Number: number, CommentLimit: commentLimit, - Order: order, + After: after, + Newest: newest, } mock.lockGetWithComments.Lock() mock.calls.GetWithComments = append(mock.calls.GetWithComments, callInfo) mock.lockGetWithComments.Unlock() - return mock.GetWithCommentsFunc(repo, number, commentLimit, order) + return mock.GetWithCommentsFunc(repo, number, commentLimit, after, newest) } // GetWithCommentsCalls gets all the calls that were made to GetWithComments. @@ -430,13 +434,15 @@ func (mock *DiscussionClientMock) GetWithCommentsCalls() []struct { Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool } { var calls []struct { Repo ghrepo.Interface Number int CommentLimit int - Order string + After string + Newest bool } mock.lockGetWithComments.RLock() calls = mock.calls.GetWithComments diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 0fffa9a8d..a5affcb3a 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -82,10 +82,17 @@ func (d Discussion) ExportData(fields []string) map[string]interface{} { for i, c := range d.Comments.Comments { comments[i] = c.Export() } - data[f] = map[string]interface{}{ + m := map[string]interface{}{ "totalCount": d.Comments.TotalCount, "nodes": comments, } + if d.Comments.Cursor != "" { + m["cursor"] = d.Comments.Cursor + } + if d.Comments.NextCursor != "" { + m["next"] = d.Comments.NextCursor + } + data[f] = m case "reactionGroups": reactions := make([]interface{}, len(d.ReactionGroups)) for i, rg := range d.ReactionGroups { @@ -171,14 +178,13 @@ type DiscussionComment struct { IsAnswer bool UpvoteCount int ReactionGroups []ReactionGroup - Replies []DiscussionComment - TotalReplies int + Replies DiscussionCommentList } // Export returns the comment as a map for JSON output. func (c DiscussionComment) Export() map[string]interface{} { - replies := make([]interface{}, len(c.Replies)) - for i, r := range c.Replies { + replies := make([]interface{}, len(c.Replies.Comments)) + for i, r := range c.Replies.Comments { replies[i] = r.Export() } reactions := make([]interface{}, len(c.ReactionGroups)) @@ -194,15 +200,27 @@ func (c DiscussionComment) Export() map[string]interface{} { "isAnswer": c.IsAnswer, "upvoteCount": c.UpvoteCount, "reactionGroups": reactions, - "replies": replies, - "totalReplies": c.TotalReplies, + "replies": map[string]interface{}{ + "totalCount": c.Replies.TotalCount, + "nodes": replies, + }, } } +type DiscussionCommentListDirection string + +const ( + DiscussionCommentListDirectionForward DiscussionCommentListDirection = "forward" + DiscussionCommentListDirectionBackward DiscussionCommentListDirection = "backward" +) + // DiscussionCommentList represents a paginated list of comments on a discussion. type DiscussionCommentList struct { Comments []DiscussionComment TotalCount int + Cursor string + NextCursor string + Direction DiscussionCommentListDirection } // ReactionGroup represents a set of reactions of the same type. diff --git a/pkg/cmd/discussion/shared/client.go b/pkg/cmd/discussion/shared/client.go index 96c3345e0..d0f34e04b 100644 --- a/pkg/cmd/discussion/shared/client.go +++ b/pkg/cmd/discussion/shared/client.go @@ -7,31 +7,6 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" ) -// DiscussionFields lists all field names available for --json output -// on discussion commands that return a full discussion (e.g. view). -var DiscussionFields = []string{ - "id", - "number", - "title", - "body", - "url", - "closed", - "state", - "stateReason", - "author", - "category", - "labels", - "answered", - "answerChosenAt", - "answerChosenBy", - "comments", - "reactionGroups", - "createdAt", - "updatedAt", - "closedAt", - "locked", -} - // DiscussionClientFunc returns a factory function that creates a DiscussionClient // from the given Factory. The returned function is intended to be stored in // command Options structs and called lazily inside RunE. diff --git a/pkg/cmd/discussion/shared/display.go b/pkg/cmd/discussion/shared/display.go deleted file mode 100644 index ea866fcb2..000000000 --- a/pkg/cmd/discussion/shared/display.go +++ /dev/null @@ -1,35 +0,0 @@ -package shared - -import ( - "fmt" - "strings" - - "github.com/cli/cli/v2/pkg/cmd/discussion/client" -) - -var reactionEmoji = map[string]string{ - "THUMBS_UP": "\U0001f44d", - "THUMBS_DOWN": "\U0001f44e", - "LAUGH": "\U0001f604", - "HOORAY": "\U0001f389", - "CONFUSED": "\U0001f615", - "HEART": "\u2764\ufe0f", - "ROCKET": "\U0001f680", - "EYES": "\U0001f440", -} - -// ReactionGroupList formats reaction groups for display. -func ReactionGroupList(groups []client.ReactionGroup) string { - var parts []string - for _, g := range groups { - if g.TotalCount == 0 { - continue - } - emoji := reactionEmoji[g.Content] - if emoji == "" { - emoji = g.Content - } - parts = append(parts, fmt.Sprintf("%s %d", emoji, g.TotalCount)) - } - return strings.Join(parts, " • ") -} diff --git a/pkg/cmd/discussion/view/view.go b/pkg/cmd/discussion/view/view.go index ca94b238e..b114aaaa2 100644 --- a/pkg/cmd/discussion/view/view.go +++ b/pkg/cmd/discussion/view/view.go @@ -19,6 +19,55 @@ import ( "github.com/spf13/cobra" ) +var discussionFields = []string{ + "id", + "number", + "title", + "body", + "url", + "closed", + "state", + "stateReason", + "author", + "category", + "labels", + "answered", + "answerChosenAt", + "answerChosenBy", + "comments", + "reactionGroups", + "createdAt", + "updatedAt", + "closedAt", + "locked", +} + +var reactionEmoji = map[string]string{ + "THUMBS_UP": "\U0001f44d", + "THUMBS_DOWN": "\U0001f44e", + "LAUGH": "\U0001f604", + "HOORAY": "\U0001f389", + "CONFUSED": "\U0001f615", + "HEART": "\u2764\ufe0f", + "ROCKET": "\U0001f680", + "EYES": "\U0001f440", +} + +func reactionGroupList(groups []client.ReactionGroup) string { + var parts []string + for _, g := range groups { + if g.TotalCount == 0 { + continue + } + emoji := reactionEmoji[g.Content] + if emoji == "" { + emoji = g.Content + } + parts = append(parts, fmt.Sprintf("%s %d", emoji, g.TotalCount)) + } + return strings.Join(parts, " • ") +} + // ViewOptions holds the configuration for the view command. type ViewOptions struct { IO *iostreams.IOStreams @@ -29,6 +78,8 @@ type ViewOptions struct { DiscussionNumber int WebMode bool Comments bool + Limit int + After string Order string Exporter cmdutil.Exporter Now func() time.Time @@ -50,6 +101,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman With %[1]s--comments%[1]s flag, show threaded comments on the discussion. Use %[1]s--order%[1]s to control comment ordering (oldest or newest first). + Use %[1]s--limit%[1]s and %[1]s--after%[1]s for paginating through comments. With %[1]s--web%[1]s flag, open the discussion in a web browser instead. `, "`"), @@ -64,7 +116,13 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman $ gh discussion view 123 --comments # View with newest comments first - $ gh discussion view 123 --comments --order newest + $ gh discussion view 123 --comments --order oldest + + # Limit to 10 comments + $ gh discussion view 123 --comments --limit 10 + + # Fetch the next page of comments + $ gh discussion view 123 --comments --after CURSOR # Open in browser $ gh discussion view 123 --web @@ -74,6 +132,15 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman if cmd.Flags().Changed("order") && !opts.Comments { return cmdutil.FlagErrorf("--order requires --comments") } + if cmd.Flags().Changed("limit") && !opts.Comments { + return cmdutil.FlagErrorf("--limit requires --comments") + } + if cmd.Flags().Changed("after") && !opts.Comments { + return cmdutil.FlagErrorf("--after requires --comments") + } + if opts.Limit < 1 { + return cmdutil.FlagErrorf("invalid limit: %d", opts.Limit) + } number, repo, err := shared.ParseDiscussionArg(args[0]) if err != nil { @@ -100,8 +167,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open a discussion in the browser") cmd.Flags().BoolVarP(&opts.Comments, "comments", "c", false, "View discussion comments") - cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "oldest", []string{"oldest", "newest"}, "Order of comments") - cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.DiscussionFields) + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of comments to fetch") + cmd.Flags().StringVar(&opts.After, "after", "", "Cursor for the next page of comments") + cmdutil.StringEnumFlag(cmd, &opts.Order, "order", "", "newest", []string{"oldest", "newest"}, "Order of comments") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, discussionFields) return cmd } @@ -130,7 +199,7 @@ func viewRun(opts *ViewOptions) error { var discussion *client.Discussion if opts.Comments { - discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, 30, opts.Order) + discussion, err = c.GetWithComments(repo, opts.DiscussionNumber, opts.Limit, opts.After, opts.Order == "newest") } else { discussion, err = c.GetByNumber(repo, opts.DiscussionNumber) } @@ -209,7 +278,7 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { } fmt.Fprintf(out, "\n%s\n", md) - if reactions := shared.ReactionGroupList(d.ReactionGroups); reactions != "" { + if reactions := reactionGroupList(d.ReactionGroups); reactions != "" { fmt.Fprintln(out, reactions) fmt.Fprintln(out) } @@ -226,7 +295,19 @@ func printHumanView(opts *ViewOptions, d *client.Discussion) error { } if shown := len(d.Comments.Comments); shown < d.Comments.TotalCount { - fmt.Fprintf(out, cs.Muted(" And %d more comments\n"), d.Comments.TotalCount-shown) + remaining := d.Comments.TotalCount - shown + age := "more" + if d.Comments.Direction == client.DiscussionCommentListDirectionForward { + age = "newer" + } else if d.Comments.Direction == client.DiscussionCommentListDirectionBackward { + age = "older" + } + fmt.Fprintf(out, cs.Muted(" And %d %s comments\n"), remaining, age) + fmt.Fprintln(out) + } + + if d.Comments.NextCursor != "" { + fmt.Fprintf(out, cs.Muted("To see more comments, pass: --after %s\n"), d.Comments.NextCursor) fmt.Fprintln(out) } } @@ -247,6 +328,9 @@ func printRawView(out io.Writer, d *client.Discussion, showComments bool) error fmt.Fprintf(out, "author:\t%s\n", d.Author.Login) fmt.Fprintf(out, "labels:\t%s\n", labelList(d.Labels, nil)) fmt.Fprintf(out, "comments:\t%d\n", d.Comments.TotalCount) + if showComments && d.Comments.NextCursor != "" { + fmt.Fprintf(out, "next:\t%s\n", d.Comments.NextCursor) + } fmt.Fprintf(out, "number:\t%d\n", d.Number) fmt.Fprintf(out, "url:\t%s\n", d.URL) fmt.Fprintln(out, "--") @@ -288,20 +372,26 @@ func printHumanComment(opts *ViewOptions, out io.Writer, c client.DiscussionComm fmt.Fprint(out, md) } - if reactions := shared.ReactionGroupList(c.ReactionGroups); reactions != "" { + if reactions := reactionGroupList(c.ReactionGroups); reactions != "" { fmt.Fprintf(out, "%s%s\n", indent, reactions) } fmt.Fprintln(out) - for _, reply := range c.Replies { + for _, reply := range c.Replies.Comments { if err := printHumanComment(opts, out, reply, indent+" "); err != nil { return err } } - if shown := len(c.Replies); shown < c.TotalReplies { - fmt.Fprintf(out, "%s %s\n\n", indent, cs.Muted(fmt.Sprintf("And %d more replies", c.TotalReplies-shown))) + if shown := len(c.Replies.Comments); shown < c.Replies.TotalCount { + directionLabel := "more" + if c.Replies.Direction == client.DiscussionCommentListDirectionForward { + directionLabel = "newer" + } else if c.Replies.Direction == client.DiscussionCommentListDirectionBackward { + directionLabel = "older" + } + fmt.Fprintf(out, "%s %s\n\n", indent, cs.Muted(fmt.Sprintf("And %d %s replies", c.Replies.TotalCount-shown, directionLabel))) } return nil @@ -321,7 +411,7 @@ func printRawComment(out io.Writer, c client.DiscussionComment, indent string) { } fmt.Fprintln(out) - for _, reply := range c.Replies { + for _, reply := range c.Replies.Comments { printRawComment(out, reply, indent+" ") } } diff --git a/pkg/cmd/discussion/view/view_test.go b/pkg/cmd/discussion/view/view_test.go index 6e90d131c..92a3846a6 100644 --- a/pkg/cmd/discussion/view/view_test.go +++ b/pkg/cmd/discussion/view/view_test.go @@ -8,7 +8,6 @@ import ( "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/cmd/discussion/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/stretchr/testify/assert" @@ -192,7 +191,7 @@ func TestViewRun_json(t *testing.T) { } exporter := cmdutil.NewJSONExporter() - exporter.SetFields(shared.DiscussionFields) + exporter.SetFields(discussionFields) opts := &ViewOptions{ IO: ios, @@ -367,16 +366,18 @@ func testDiscussionWithComments() *client.Discussion { ReactionGroups: []client.ReactionGroup{ {Content: "THUMBS_UP", TotalCount: 3}, }, - Replies: []client.DiscussionComment{ - { - ID: "C_1_R1", - URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2", - Author: client.DiscussionActor{Login: "hubot"}, - Body: "Thanks!", - CreatedAt: time.Date(2025, 3, 2, 1, 0, 0, 0, time.UTC), + Replies: client.DiscussionCommentList{ + TotalCount: 5, + Comments: []client.DiscussionComment{ + { + ID: "C_1_R1", + URL: "https://github.com/OWNER/REPO/discussions/123#discussioncomment-2", + Author: client.DiscussionActor{Login: "hubot"}, + Body: "Thanks!", + CreatedAt: time.Date(2025, 3, 2, 1, 0, 0, 0, time.UTC), + }, }, }, - TotalReplies: 5, }, { ID: "C_2", @@ -397,9 +398,9 @@ func TestViewRun_comments_tty(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { assert.Equal(t, 30, commentLimit) - assert.Equal(t, "oldest", order) + assert.Equal(t, false, newest) return d, nil }, } @@ -414,6 +415,7 @@ func TestViewRun_comments_tty(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, } @@ -439,7 +441,7 @@ func TestViewRun_comments_nontty(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { return d, nil }, } @@ -454,6 +456,7 @@ func TestViewRun_comments_nontty(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Now: time.Now, } @@ -475,13 +478,13 @@ func TestViewRun_comments_json(t *testing.T) { d := testDiscussionWithComments() mock := &client.DiscussionClientMock{ - GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, order string) (*client.Discussion, error) { + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { return d, nil }, } exporter := cmdutil.NewJSONExporter() - exporter.SetFields(shared.DiscussionFields) + exporter.SetFields(discussionFields) opts := &ViewOptions{ IO: ios, @@ -493,6 +496,7 @@ func TestViewRun_comments_json(t *testing.T) { }, DiscussionNumber: 123, Comments: true, + Limit: 30, Order: "oldest", Exporter: exporter, Now: time.Now, @@ -559,3 +563,220 @@ func TestViewRun_noComments_usesGetByNumber(t *testing.T) { assert.Equal(t, 1, len(mock.GetByNumberCalls())) assert.Equal(t, 0, len(mock.GetWithCommentsCalls())) } + +func TestNewCmdView_limitWithoutComments(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + cmd := NewCmdView(f, func(opts *ViewOptions) error { + return nil + }) + + cmd.SetArgs([]string{"123", "--limit", "10"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--limit requires --comments") +} + +func TestNewCmdView_afterWithoutComments(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + cmd := NewCmdView(f, func(opts *ViewOptions) error { + return nil + }) + + cmd.SetArgs([]string{"123", "--after", "CURSOR_ABC"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--after requires --comments") +} + +func TestNewCmdView_invalidLimit(t *testing.T) { + f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f.IOStreams = ios + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + f.Browser = &browser.Stub{} + + cmd := NewCmdView(f, func(opts *ViewOptions) error { + return nil + }) + + cmd.SetArgs([]string{"123", "--comments", "--limit", "0"}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid limit") +} + +func TestViewRun_commentsWithPagination_tty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_123" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + assert.Equal(t, 10, commentLimit) + assert.Equal(t, "CURSOR_ABC", after) + assert.Equal(t, false, newest) + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Limit: 10, + After: "CURSOR_ABC", + Order: "oldest", + Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "To see more comments, pass: --after NEXT_CURSOR_123") +} + +func TestViewRun_commentsWithPagination_nontty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + d.Comments.NextCursor = "NEXT_CURSOR_456" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Limit: 30, + Order: "oldest", + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "next:\tNEXT_CURSOR_456") +} + +func TestViewRun_commentsWithPagination_json(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(false) + + d := testDiscussionWithComments() + d.Comments.Cursor = "PREV_CURSOR" + d.Comments.NextCursor = "NEXT_CURSOR_789" + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(discussionFields) + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Limit: 30, + Order: "oldest", + Exporter: exporter, + Now: time.Now, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, `"cursor":"PREV_CURSOR"`) + assert.Contains(t, out, `"next":"NEXT_CURSOR_789"`) +} + +func TestViewRun_noPaginationCursor_tty(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) + + d := testDiscussionWithComments() + + mock := &client.DiscussionClientMock{ + GetWithCommentsFunc: func(repo ghrepo.Interface, number int, commentLimit int, after string, newest bool) (*client.Discussion, error) { + return d, nil + }, + } + + opts := &ViewOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Client: func() (client.DiscussionClient, error) { + return mock, nil + }, + DiscussionNumber: 123, + Comments: true, + Limit: 30, + Order: "oldest", + Now: func() time.Time { return time.Date(2025, 3, 4, 0, 0, 0, 0, time.UTC) }, + } + + err := viewRun(opts) + require.NoError(t, err) + + out := stdout.String() + assert.NotContains(t, out, "--after") +}