From 2e5623180ac95417abd1c5cf2cb11ec2fefac504 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Wed, 29 Apr 2026 12:36:03 -0500 Subject: [PATCH 1/6] feat(discussion/client): implement Create mutation with tests Implement the createDiscussion GraphQL mutation in the discussion client. - Add getRepositoryMeta helper to resolve repo node ID and check discussions-enabled flag before mutating - Skip repo lookup when CreateDiscussionInput.RepositoryID is provided - Reuse discussionListNode mapping for consistent field coverage - Table-driven tests: field mapping, pre-resolved repo ID, discussions disabled, repo not found, mutation error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 88 ++++++- pkg/cmd/discussion/client/client_impl_test.go | 244 ++++++++++++++++++ 2 files changed, 330 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 88aa7f170..ead75da6e 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -775,8 +775,92 @@ func (c *discussionClient) ListCategories(repo ghrepo.Interface) ([]DiscussionCa return categories, nil } -func (c *discussionClient) Create(_ ghrepo.Interface, _ CreateDiscussionInput) (*Discussion, error) { - return nil, fmt.Errorf("not implemented") +// repositoryMeta holds the node ID and feature flags fetched for a repository. +type repositoryMeta struct { + ID string + HasDiscussionsEnabled bool +} + +// getRepositoryMeta fetches the node ID and discussion-enabled flag for a repository. +func (c *discussionClient) getRepositoryMeta(repo ghrepo.Interface) (*repositoryMeta, error) { + var query struct { + Repository struct { + ID string + HasDiscussionsEnabled bool + } `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(), "RepositoryMeta", &query, variables); err != nil { + return nil, err + } + + return &repositoryMeta{ + ID: query.Repository.ID, + HasDiscussionsEnabled: query.Repository.HasDiscussionsEnabled, + }, nil +} + +// createDiscussionGQLInput is the typed input for the createDiscussion GraphQL mutation. +type createDiscussionGQLInput struct { + RepositoryID githubv4.ID `json:"repositoryId"` + CategoryID githubv4.ID `json:"categoryId"` + Title githubv4.String `json:"title"` + Body githubv4.String `json:"body"` +} + +func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionInput) (*Discussion, error) { + repoID := input.RepositoryID + if repoID == "" { + meta, err := c.getRepositoryMeta(repo) + if err != nil { + return nil, err + } + if !meta.HasDiscussionsEnabled { + return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) + } + repoID = meta.ID + } + + var mutation struct { + CreateDiscussion struct { + Discussion struct { + discussionListNode + Comments struct { + TotalCount int + } + } + } `graphql:"createDiscussion(input: $input)"` + } + + variables := map[string]interface{}{ + "input": createDiscussionGQLInput{ + RepositoryID: githubv4.ID(repoID), + CategoryID: githubv4.ID(input.CategoryID), + Title: githubv4.String(input.Title), + Body: githubv4.String(input.Body), + }, + } + + if err := c.gql.Mutate(repo.RepoHost(), "CreateDiscussion", &mutation, variables); err != nil { + return nil, err + } + + d := mapDiscussionFromListNode(mutation.CreateDiscussion.Discussion.discussionListNode) + d.Comments = DiscussionCommentList{TotalCount: mutation.CreateDiscussion.Discussion.Comments.TotalCount} + + for _, rg := range mutation.CreateDiscussion.Discussion.ReactionGroups { + d.ReactionGroups = append(d.ReactionGroups, ReactionGroup{ + Content: rg.Content, + TotalCount: rg.Users.TotalCount, + }) + } + + return &d, nil } func (c *discussionClient) Update(_ ghrepo.Interface, _ UpdateDiscussionInput) (*Discussion, error) { diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 16c0813a2..38b5934e3 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -2475,3 +2475,247 @@ func TestGetCommentReplies(t *testing.T) { }) } } + +func repoMetaResp(id string, discussionsEnabled bool) string { + return fmt.Sprintf(`{ + "data": { + "repository": { + "id": %q, + "hasDiscussionsEnabled": %t + } + } + }`, id, discussionsEnabled) +} + +func TestCreate(t *testing.T) { + repo := ghrepo.New("OWNER", "REPO") + + tests := []struct { + name string + input CreateDiscussionInput + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + assertDisc *Discussion + }{ + { + name: "maps all fields", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "New Discussion", + Body: "Discussion body", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "New Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 0}}], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + }, + assertDisc: &Discussion{ + ID: "D_new", + Number: 99, + Title: "New Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/99", + Author: DiscussionActor{ID: "U1", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT_1", + Name: "General", + Slug: "general", + Emoji: ":speech_balloon:", + }, + Labels: []DiscussionLabel{}, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 0}}, + CreatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "skips repo lookup when RepositoryID provided", + input: CreateDiscussionInput{ + RepositoryID: "R_existing", + CategoryID: "CAT_1", + Title: "Pre-resolved", + Body: "Body", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + // No RepositoryMeta query should be made. + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_pre", + "number": 1, + "title": "Pre-resolved", + "body": "Body", + "url": "https://github.com/OWNER/REPO/discussions/1", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2025-01-01T00:00:00Z", + "updatedAt": "2025-01-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + }, + assertDisc: &Discussion{ + ID: "D_pre", + Number: 1, + Title: "Pre-resolved", + Body: "Body", + URL: "https://github.com/OWNER/REPO/discussions/1", + Author: DiscussionActor{Login: "alice"}, + Category: DiscussionCategory{ + ID: "CAT_1", + Name: "General", + Slug: "general", + }, + Labels: []DiscussionLabel{}, + CreatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "discussions disabled", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "Test", + Body: "Body", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", false)), + ) + }, + wantErr: "has discussions disabled", + }, + { + name: "repo not found", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "Test", + Body: "Body", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "path": ["repository"], + "message": "Could not resolve to a Repository with the name 'OWNER/REPO'." + } + ] + } + `)), + ) + }, + wantErr: "Could not resolve to a Repository with the name 'OWNER/REPO'.", + }, + { + name: "mutation error", + input: CreateDiscussionInput{ + RepositoryID: "R_1", + CategoryID: "BAD_CAT", + Title: "Test", + Body: "Body", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": null + }, + "errors": [ + { + "type": "NOT_FOUND", + "message": "Could not resolve to a node with the global id of 'BAD_CAT'." + } + ] + } + `)), + ) + }, + wantErr: "Could not resolve to a node with the global id of 'BAD_CAT'.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + + c := newTestDiscussionClient(reg) + d, err := c.Create(repo, tt.input) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + require.NotNil(t, d) + require.NotNil(t, tt.assertDisc, "assertDisc must be set for non-error cases") + assert.Equal(t, tt.assertDisc, d) + }) + } +} From d78703efaaf3f35062fd2bd7c325e12199f3c1c0 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 1 May 2026 09:12:15 +0100 Subject: [PATCH 2/6] fix(discussion/client): polish Create implementation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 28 +++++++----------------- pkg/cmd/discussion/client/types.go | 7 +++--- 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index ead75da6e..8b56948c9 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -805,25 +805,13 @@ func (c *discussionClient) getRepositoryMeta(repo ghrepo.Interface) (*repository }, nil } -// createDiscussionGQLInput is the typed input for the createDiscussion GraphQL mutation. -type createDiscussionGQLInput struct { - RepositoryID githubv4.ID `json:"repositoryId"` - CategoryID githubv4.ID `json:"categoryId"` - Title githubv4.String `json:"title"` - Body githubv4.String `json:"body"` -} - func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionInput) (*Discussion, error) { - repoID := input.RepositoryID - if repoID == "" { - meta, err := c.getRepositoryMeta(repo) - if err != nil { - return nil, err - } - if !meta.HasDiscussionsEnabled { - return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) - } - repoID = meta.ID + meta, err := c.getRepositoryMeta(repo) + if err != nil { + return nil, err + } + if !meta.HasDiscussionsEnabled { + return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } var mutation struct { @@ -838,8 +826,8 @@ func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionI } variables := map[string]interface{}{ - "input": createDiscussionGQLInput{ - RepositoryID: githubv4.ID(repoID), + "input": githubv4.CreateDiscussionInput{ + RepositoryID: githubv4.ID(meta.ID), CategoryID: githubv4.ID(input.CategoryID), Title: githubv4.String(input.Title), Body: githubv4.String(input.Body), diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 1eaab5329..6e964bc9c 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -327,10 +327,9 @@ type SearchFilters struct { // CreateDiscussionInput holds the parameters for creating a discussion. type CreateDiscussionInput struct { - RepositoryID string - CategoryID string - Title string - Body string + CategoryID string + Title string + Body string } // UpdateDiscussionInput holds optional parameters for updating a discussion. From d6b46f75d4e914935e8cbfad0bfb0c2c453c5641 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 1 May 2026 09:12:39 +0100 Subject: [PATCH 3/6] test(discussion/client): verify Create mutation variables and error paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 79 ++++--------------- 1 file changed, 14 insertions(+), 65 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 38b5934e3..9687b5892 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -2510,7 +2510,13 @@ func TestCreate(t *testing.T) { httpmock.StringResponse(repoMetaResp("R_1", true)), ) reg.Register( - httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.GraphQLMutationMatcher(`mutation CreateDiscussion\b`, func(input map[string]interface{}) bool { + assert.Equal(t, "R_1", input["repositoryId"]) + assert.Equal(t, "CAT_1", input["categoryId"]) + assert.Equal(t, "New Discussion", input["title"]) + assert.Equal(t, "Discussion body", input["body"]) + return true + }), httpmock.StringResponse(heredoc.Doc(` { "data": { @@ -2561,66 +2567,6 @@ func TestCreate(t *testing.T) { UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), }, }, - { - name: "skips repo lookup when RepositoryID provided", - input: CreateDiscussionInput{ - RepositoryID: "R_existing", - CategoryID: "CAT_1", - Title: "Pre-resolved", - Body: "Body", - }, - httpStubs: func(t *testing.T, reg *httpmock.Registry) { - // No RepositoryMeta query should be made. - reg.Register( - httpmock.GraphQL(`mutation CreateDiscussion\b`), - httpmock.StringResponse(heredoc.Doc(` - { - "data": { - "createDiscussion": { - "discussion": { - "id": "D_pre", - "number": 1, - "title": "Pre-resolved", - "body": "Body", - "url": "https://github.com/OWNER/REPO/discussions/1", - "closed": false, - "stateReason": "", - "isAnswered": false, - "answerChosenAt": "0001-01-01T00:00:00Z", - "author": {"__typename": "User", "login": "alice"}, - "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": "", "isAnswerable": false}, - "answerChosenBy": null, - "labels": {"nodes": []}, - "reactionGroups": [], - "createdAt": "2025-01-01T00:00:00Z", - "updatedAt": "2025-01-01T00:00:00Z", - "closedAt": "0001-01-01T00:00:00Z", - "locked": false, - "comments": {"totalCount": 0} - } - } - } - } - `)), - ) - }, - assertDisc: &Discussion{ - ID: "D_pre", - Number: 1, - Title: "Pre-resolved", - Body: "Body", - URL: "https://github.com/OWNER/REPO/discussions/1", - Author: DiscussionActor{Login: "alice"}, - Category: DiscussionCategory{ - ID: "CAT_1", - Name: "General", - Slug: "general", - }, - Labels: []DiscussionLabel{}, - CreatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), - UpdatedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), - }, - }, { name: "discussions disabled", input: CreateDiscussionInput{ @@ -2667,12 +2613,15 @@ func TestCreate(t *testing.T) { { name: "mutation error", input: CreateDiscussionInput{ - RepositoryID: "R_1", - CategoryID: "BAD_CAT", - Title: "Test", - Body: "Body", + CategoryID: "BAD_CAT", + Title: "Test", + Body: "Body", }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) reg.Register( httpmock.GraphQL(`mutation CreateDiscussion\b`), httpmock.StringResponse(heredoc.Doc(` From fd06ad75565f57f2e45b9abb3b90d0f0f3d0dea9 Mon Sep 17 00:00:00 2001 From: Max Beizer Date: Fri, 1 May 2026 14:17:38 -0500 Subject: [PATCH 4/6] discussion create: add label support via two-step GraphQL - Add Labels []string field to CreateDiscussionInput - Implement resolveLabels helper: paginated RepositoryLabels query, case-insensitive match, error if any label not found - Implement addLabelsToDiscussion helper: calls addLabelsToLabelable mutation after createDiscussion - Wire label logic into Create: resolve labels, apply them, populate d.Labels from resolved values - Add three TestCreate cases: success with labels, label not found, mutation failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 99 ++++++++ pkg/cmd/discussion/client/client_impl_test.go | 223 ++++++++++++++++++ pkg/cmd/discussion/client/types.go | 1 + 3 files changed, 323 insertions(+) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 8b56948c9..6d71ab9ef 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -805,6 +805,90 @@ func (c *discussionClient) getRepositoryMeta(repo ghrepo.Interface) (*repository }, nil } +// resolveLabels fetches all labels for a repository and matches the requested names +// case-insensitively. Returns an error if any requested label name is not found. +func (c *discussionClient) resolveLabels(repo ghrepo.Interface, labelNames []string) ([]DiscussionLabel, error) { + if len(labelNames) == 0 { + return nil, nil + } + + var query struct { + Repository struct { + Labels struct { + Nodes []struct { + ID string + Name string + Color string + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"labels(first: 100, after: $endCursor)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "endCursor": (*githubv4.String)(nil), + } + + wanted := make(map[string]bool, len(labelNames)) + for _, n := range labelNames { + wanted[strings.ToLower(n)] = true + } + + found := make(map[string]DiscussionLabel, len(labelNames)) + for { + if err := c.gql.Query(repo.RepoHost(), "RepositoryLabels", &query, variables); err != nil { + return nil, err + } + for _, n := range query.Repository.Labels.Nodes { + if wanted[strings.ToLower(n.Name)] { + found[strings.ToLower(n.Name)] = DiscussionLabel{ID: n.ID, Name: n.Name, Color: n.Color} + } + } + if !query.Repository.Labels.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Repository.Labels.PageInfo.EndCursor) + } + + result := make([]DiscussionLabel, 0, len(labelNames)) + for _, name := range labelNames { + label, ok := found[strings.ToLower(name)] + if !ok { + return nil, fmt.Errorf("label not found: %q", name) + } + result = append(result, label) + } + return result, nil +} + +// addLabelsToDiscussion applies labels to a discussion via the addLabelsToLabelable mutation. +func (c *discussionClient) addLabelsToDiscussion(repo ghrepo.Interface, discussionID string, labelIDs []string) error { + ids := make([]githubv4.ID, len(labelIDs)) + for i, id := range labelIDs { + ids[i] = githubv4.ID(id) + } + + var mutation struct { + AddLabelsToLabelable struct { + Typename string `graphql:"__typename"` + } `graphql:"addLabelsToLabelable(input: $input)"` + } + + variables := map[string]interface{}{ + "input": githubv4.AddLabelsToLabelableInput{ + LabelableID: githubv4.ID(discussionID), + LabelIDs: ids, + }, + } + + return c.gql.Mutate(repo.RepoHost(), "AddLabelsToDiscussion", &mutation, variables) +} + func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionInput) (*Discussion, error) { meta, err := c.getRepositoryMeta(repo) if err != nil { @@ -848,6 +932,21 @@ func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionI }) } + if len(input.Labels) > 0 { + resolvedLabels, err := c.resolveLabels(repo, input.Labels) + if err != nil { + return nil, err + } + labelIDs := make([]string, len(resolvedLabels)) + for i, l := range resolvedLabels { + labelIDs[i] = l.ID + } + if err := c.addLabelsToDiscussion(repo, d.ID, labelIDs); err != nil { + return nil, err + } + d.Labels = resolvedLabels + } + return &d, nil } diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 9687b5892..66852d8b3 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -2641,6 +2641,229 @@ func TestCreate(t *testing.T) { }, wantErr: "Could not resolve to a node with the global id of 'BAD_CAT'.", }, + { + name: "creates discussion with labels", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "New Discussion", + Body: "Discussion body", + Labels: []string{"bug"}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "New Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [{"content": "THUMBS_UP", "users": {"totalCount": 0}}], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [ + {"id": "L_bug", "name": "bug", "color": "d73a4a"} + ], + "pageInfo": {"hasNextPage": false, "endCursor": ""} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`mutation AddLabelsToDiscussion\b`), + httpmock.StringResponse(`{"data":{"addLabelsToLabelable":{"__typename":"Discussion"}}}`), + ) + }, + assertDisc: &Discussion{ + ID: "D_new", + Number: 99, + Title: "New Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/99", + Author: DiscussionActor{ID: "U1", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT_1", + Name: "General", + Slug: "general", + Emoji: ":speech_balloon:", + }, + Labels: []DiscussionLabel{{ID: "L_bug", Name: "bug", Color: "d73a4a"}}, + ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 0}}, + CreatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "label not found returns error", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "Test", + Body: "Body", + Labels: []string{"nonexistent"}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "Test", + "body": "Body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [], + "pageInfo": {"hasNextPage": false, "endCursor": ""} + } + } + } + } + `)), + ) + }, + wantErr: `label not found: "nonexistent"`, + }, + { + name: "add labels mutation failure returns error", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "Test", + Body: "Body", + Labels: []string{"bug"}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "Test", + "body": "Body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [ + {"id": "L_bug", "name": "bug", "color": "d73a4a"} + ], + "pageInfo": {"hasNextPage": false, "endCursor": ""} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`mutation AddLabelsToDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": null, + "errors": [{"message": "could not apply labels"}] + } + `)), + ) + }, + wantErr: "could not apply labels", + }, } for _, tt := range tests { diff --git a/pkg/cmd/discussion/client/types.go b/pkg/cmd/discussion/client/types.go index 6e964bc9c..d725ee92c 100644 --- a/pkg/cmd/discussion/client/types.go +++ b/pkg/cmd/discussion/client/types.go @@ -330,6 +330,7 @@ type CreateDiscussionInput struct { CategoryID string Title string Body string + Labels []string } // UpdateDiscussionInput holds optional parameters for updating a discussion. From 618dbf33f0d4d38f72d1b3edd162a5ccadff9e3c Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 5 May 2026 12:46:33 +0100 Subject: [PATCH 5/6] fix(discussion/client): improve label resolution error handling - Break early from pagination when all wanted labels are found - Collect all missing labels and report them in a single error message - Guard missing-label check with len(found) != len(wanted) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 6d71ab9ef..78108a233 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -849,19 +849,28 @@ func (c *discussionClient) resolveLabels(repo ghrepo.Interface, labelNames []str found[strings.ToLower(n.Name)] = DiscussionLabel{ID: n.ID, Name: n.Name, Color: n.Color} } } + if len(found) == len(wanted) { + break + } if !query.Repository.Labels.PageInfo.HasNextPage { break } variables["endCursor"] = githubv4.String(query.Repository.Labels.PageInfo.EndCursor) } + if len(found) != len(wanted) { + var missing []string + for _, name := range labelNames { + if _, ok := found[strings.ToLower(name)]; !ok { + missing = append(missing, name) + } + } + return nil, fmt.Errorf("labels not found: %s", strings.Join(missing, ", ")) + } + result := make([]DiscussionLabel, 0, len(labelNames)) for _, name := range labelNames { - label, ok := found[strings.ToLower(name)] - if !ok { - return nil, fmt.Errorf("label not found: %q", name) - } - result = append(result, label) + result = append(result, found[strings.ToLower(name)]) } return result, nil } From 5d917f1af20bbdc5512e0903f287f8a19dcb0d41 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 5 May 2026 12:46:53 +0100 Subject: [PATCH 6/6] test(discussion/client): add label pagination and early-break tests - Add "paginates labels across multiple pages" case (two pages, one label each) - Add "stops paginating labels when all found" case (early break verified via reg.Verify) - Update "creates discussion with labels" to two labels with variable assertions - Update "label not found" to verify all missing labels reported at once Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/discussion/client/client_impl_test.go | 218 +++++++++++++++++- 1 file changed, 212 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 66852d8b3..8ca5f52b2 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -2641,13 +2641,209 @@ func TestCreate(t *testing.T) { }, wantErr: "Could not resolve to a node with the global id of 'BAD_CAT'.", }, + { + name: "paginates labels across multiple pages", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "New Discussion", + Body: "Discussion body", + Labels: []string{"bug", "enhancement"}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "New Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [ + {"id": "L_bug", "name": "bug", "color": "d73a4a"} + ], + "pageInfo": {"hasNextPage": true, "endCursor": "LABEL_CUR_1"} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [ + {"id": "L_enh", "name": "enhancement", "color": "a2eeef"} + ], + "pageInfo": {"hasNextPage": false, "endCursor": ""} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`mutation AddLabelsToDiscussion\b`), + httpmock.StringResponse(`{"data":{"addLabelsToLabelable":{"__typename":"Discussion"}}}`), + ) + }, + assertDisc: &Discussion{ + ID: "D_new", + Number: 99, + Title: "New Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/99", + Author: DiscussionActor{ID: "U1", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT_1", + Name: "General", + Slug: "general", + Emoji: ":speech_balloon:", + }, + Labels: []DiscussionLabel{ + {ID: "L_bug", Name: "bug", Color: "d73a4a"}, + {ID: "L_enh", Name: "enhancement", Color: "a2eeef"}, + }, + CreatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "stops paginating labels when all found", + input: CreateDiscussionInput{ + CategoryID: "CAT_1", + Title: "New Discussion", + Body: "Discussion body", + Labels: []string{"bug", "enhancement"}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryMeta\b`), + httpmock.StringResponse(repoMetaResp("R_1", true)), + ) + reg.Register( + httpmock.GraphQL(`mutation CreateDiscussion\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "createDiscussion": { + "discussion": { + "id": "D_new", + "number": 99, + "title": "New Discussion", + "body": "Discussion body", + "url": "https://github.com/OWNER/REPO/discussions/99", + "closed": false, + "stateReason": "", + "isAnswered": false, + "answerChosenAt": "0001-01-01T00:00:00Z", + "author": {"__typename": "User", "login": "alice", "id": "U1", "name": "Alice"}, + "category": {"id": "CAT_1", "name": "General", "slug": "general", "emoji": ":speech_balloon:", "isAnswerable": false}, + "answerChosenBy": null, + "labels": {"nodes": []}, + "reactionGroups": [], + "createdAt": "2025-06-01T00:00:00Z", + "updatedAt": "2025-06-01T00:00:00Z", + "closedAt": "0001-01-01T00:00:00Z", + "locked": false, + "comments": {"totalCount": 0} + } + } + } + } + `)), + ) + // Register a single page that returns both labels but claims more pages exist. + // The code should stop paginating once all wanted labels are found. + reg.Register( + httpmock.GraphQL(`query RepositoryLabels\b`), + httpmock.StringResponse(heredoc.Doc(` + { + "data": { + "repository": { + "labels": { + "nodes": [ + {"id": "L_bug", "name": "bug", "color": "d73a4a"}, + {"id": "L_enh", "name": "enhancement", "color": "a2eeef"} + ], + "pageInfo": {"hasNextPage": true, "endCursor": "LABEL_CUR_999"} + } + } + } + } + `)), + ) + reg.Register( + httpmock.GraphQL(`mutation AddLabelsToDiscussion\b`), + httpmock.StringResponse(`{"data":{"addLabelsToLabelable":{"__typename":"Discussion"}}}`), + ) + }, + assertDisc: &Discussion{ + ID: "D_new", + Number: 99, + Title: "New Discussion", + Body: "Discussion body", + URL: "https://github.com/OWNER/REPO/discussions/99", + Author: DiscussionActor{ID: "U1", Login: "alice", Name: "Alice"}, + Category: DiscussionCategory{ + ID: "CAT_1", + Name: "General", + Slug: "general", + Emoji: ":speech_balloon:", + }, + Labels: []DiscussionLabel{ + {ID: "L_bug", Name: "bug", Color: "d73a4a"}, + {ID: "L_enh", Name: "enhancement", Color: "a2eeef"}, + }, + CreatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), + }, + }, { name: "creates discussion with labels", input: CreateDiscussionInput{ CategoryID: "CAT_1", Title: "New Discussion", Body: "Discussion body", - Labels: []string{"bug"}, + Labels: []string{"bug", "enhancement"}, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -2694,7 +2890,8 @@ func TestCreate(t *testing.T) { "repository": { "labels": { "nodes": [ - {"id": "L_bug", "name": "bug", "color": "d73a4a"} + {"id": "L_bug", "name": "bug", "color": "d73a4a"}, + {"id": "L_enh", "name": "enhancement", "color": "a2eeef"} ], "pageInfo": {"hasNextPage": false, "endCursor": ""} } @@ -2704,7 +2901,13 @@ func TestCreate(t *testing.T) { `)), ) reg.Register( - httpmock.GraphQL(`mutation AddLabelsToDiscussion\b`), + httpmock.GraphQLMutationMatcher(`mutation AddLabelsToDiscussion\b`, func(input map[string]interface{}) bool { + assert.Equal(t, "D_new", input["labelableId"]) + labelIDs, ok := input["labelIds"].([]interface{}) + assert.True(t, ok) + assert.Equal(t, []interface{}{"L_bug", "L_enh"}, labelIDs) + return true + }), httpmock.StringResponse(`{"data":{"addLabelsToLabelable":{"__typename":"Discussion"}}}`), ) }, @@ -2721,7 +2924,10 @@ func TestCreate(t *testing.T) { Slug: "general", Emoji: ":speech_balloon:", }, - Labels: []DiscussionLabel{{ID: "L_bug", Name: "bug", Color: "d73a4a"}}, + Labels: []DiscussionLabel{ + {ID: "L_bug", Name: "bug", Color: "d73a4a"}, + {ID: "L_enh", Name: "enhancement", Color: "a2eeef"}, + }, ReactionGroups: []ReactionGroup{{Content: "THUMBS_UP", TotalCount: 0}}, CreatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), UpdatedAt: time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC), @@ -2733,7 +2939,7 @@ func TestCreate(t *testing.T) { CategoryID: "CAT_1", Title: "Test", Body: "Body", - Labels: []string{"nonexistent"}, + Labels: []string{"nonexistent", "also-missing"}, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -2788,7 +2994,7 @@ func TestCreate(t *testing.T) { `)), ) }, - wantErr: `label not found: "nonexistent"`, + wantErr: `labels not found: nonexistent, also-missing`, }, { name: "add labels mutation failure returns error",