diff --git a/pkg/cmd/discussion/client/client_impl.go b/pkg/cmd/discussion/client/client_impl.go index 78108a233..5497eb955 100644 --- a/pkg/cmd/discussion/client/client_impl.go +++ b/pkg/cmd/discussion/client/client_impl.go @@ -907,6 +907,16 @@ func (c *discussionClient) Create(repo ghrepo.Interface, input CreateDiscussionI return nil, fmt.Errorf("the '%s/%s' repository has discussions disabled", repo.RepoOwner(), repo.RepoName()) } + // Resolve labels before creating the discussion so that an unknown label + // name aborts without leaving a half-created discussion behind. + var resolvedLabels []DiscussionLabel + if len(input.Labels) > 0 { + resolvedLabels, err = c.resolveLabels(repo, input.Labels) + if err != nil { + return nil, err + } + } + var mutation struct { CreateDiscussion struct { Discussion struct { @@ -941,11 +951,7 @@ 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 - } + if len(resolvedLabels) > 0 { labelIDs := make([]string, len(resolvedLabels)) for i, l := range resolvedLabels { labelIDs[i] = l.ID diff --git a/pkg/cmd/discussion/client/client_impl_test.go b/pkg/cmd/discussion/client/client_impl_test.go index 8ca5f52b2..d7df9812b 100644 --- a/pkg/cmd/discussion/client/client_impl_test.go +++ b/pkg/cmd/discussion/client/client_impl_test.go @@ -2934,7 +2934,7 @@ func TestCreate(t *testing.T) { }, }, { - name: "label not found returns error", + name: "label not found returns error without creating discussion", input: CreateDiscussionInput{ CategoryID: "CAT_1", Title: "Test", @@ -2946,38 +2946,8 @@ func TestCreate(t *testing.T) { 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} - } - } - } - } - `)), - ) + // No CreateDiscussion stub — reg.Verify(t) proves it is never called, + // confirming that label validation is atomic with discussion creation. reg.Register( httpmock.GraphQL(`query RepositoryLabels\b`), httpmock.StringResponse(heredoc.Doc(` diff --git a/pkg/cmd/discussion/create/create.go b/pkg/cmd/discussion/create/create.go index 8504c2c4c..715e9d7e2 100644 --- a/pkg/cmd/discussion/create/create.go +++ b/pkg/cmd/discussion/create/create.go @@ -44,11 +44,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Long: heredoc.Doc(` Create a new GitHub Discussion in a repository. - With '--title' and '--category', a discussion is created non-interactively. - Omitting either flag triggers interactive prompts when connected to a terminal. - - The '--body' flag provides the discussion body. Without it you will be - prompted to enter one in your default editor. + With '--title', '--body', and '--category', a discussion is created non-interactively. + Omitting any of these flags triggers interactive prompts when connected to a terminal. `), Example: heredoc.Doc(` # Create interactively @@ -86,13 +83,25 @@ func createRun(opts *CreateOptions) error { return err } + interactive := opts.IO.CanPrompt() + + if !interactive { + if opts.Title == "" { + return cmdutil.FlagErrorf("--title required when not running interactively") + } + if opts.Category == "" { + return cmdutil.FlagErrorf("--category required when not running interactively") + } + if opts.Body == "" { + return cmdutil.FlagErrorf("--body required when not running interactively") + } + } + categories, err := c.ListCategories(repo) if err != nil { return fmt.Errorf("fetching categories: %w", err) } - interactive := opts.IO.CanPrompt() - if opts.Title == "" { if !interactive { return cmdutil.FlagErrorf("--title required when not running interactively") @@ -131,7 +140,7 @@ func createRun(opts *CreateOptions) error { if !interactive { return cmdutil.FlagErrorf("--body required when not running interactively") } - opts.Body, err = opts.Prompter.MarkdownEditor("Discussion body", "", true) + opts.Body, err = opts.Prompter.MarkdownEditor("Discussion body", "", false) if err != nil { return err } diff --git a/pkg/cmd/discussion/create/create_test.go b/pkg/cmd/discussion/create/create_test.go index 6b5ba12c1..9eede150e 100644 --- a/pkg/cmd/discussion/create/create_test.go +++ b/pkg/cmd/discussion/create/create_test.go @@ -151,12 +151,8 @@ func TestCreateRun_nonInteractive(t *testing.T) { Body: "Details", Category: "General", }, - setupMock: func(m *client.DiscussionClientMock) { - m.ListCategoriesFunc = func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { - return sampleCategories(), nil - } - }, - wantErr: "--title required when not running interactively", + setupMock: func(m *client.DiscussionClientMock) {}, + wantErr: "--title required when not running interactively", }, { name: "missing category returns error", @@ -164,12 +160,8 @@ func TestCreateRun_nonInteractive(t *testing.T) { Title: "My question", Body: "Details", }, - setupMock: func(m *client.DiscussionClientMock) { - m.ListCategoriesFunc = func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { - return sampleCategories(), nil - } - }, - wantErr: "--category required when not running interactively", + setupMock: func(m *client.DiscussionClientMock) {}, + wantErr: "--category required when not running interactively", }, { name: "missing body returns error", @@ -177,12 +169,8 @@ func TestCreateRun_nonInteractive(t *testing.T) { Title: "My question", Category: "General", }, - setupMock: func(m *client.DiscussionClientMock) { - m.ListCategoriesFunc = func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { - return sampleCategories(), nil - } - }, - wantErr: "--body required when not running interactively", + setupMock: func(m *client.DiscussionClientMock) {}, + wantErr: "--body required when not running interactively", }, { name: "unknown category returns error", @@ -282,6 +270,7 @@ func TestCreateRun_tty(t *testing.T) { return 0, nil }, MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { + assert.False(t, blankAllowed, "body editor should not allow blank input") return "Some body text", nil }, } @@ -336,3 +325,32 @@ func TestCreateRun_tty_partialFlags(t *testing.T) { require.NoError(t, err) assert.Contains(t, stdout.String(), "Created discussion #5") } + +func TestCreateRun_tty_blankTitle(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + + mockClient := &client.DiscussionClientMock{ + ListCategoriesFunc: func(repo ghrepo.Interface) ([]client.DiscussionCategory, error) { + return sampleCategories(), nil + }, + } + + pm := &prompter.PrompterMock{ + InputFunc: func(prompt, defaultValue string) (string, error) { + return " ", nil // whitespace only + }, + } + + opts := &CreateOptions{ + IO: ios, + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + Client: func() (client.DiscussionClient, error) { return mockClient, nil }, + Prompter: pm, + } + + err := createRun(opts) + require.Error(t, err) + assert.Contains(t, err.Error(), "title cannot be blank") +}