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

fix: label atomicity, validation order, blankAllowed, help text

Browse source 
- Resolve labels before creating discussion so a bad label name
  doesn't leave an orphaned discussion (atomicity fix)
- Validate --title/--category/--body non-interactively before calling
  ListCategories to avoid an unnecessary network round-trip
- Set blankAllowed=false so the markdown editor rejects empty bodies
- Clarify help text: --body is required when not running interactively
- Update tests to match new behavior; rename label-not-found test to
  make the atomicity guarantee explicit

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Max Beizer 2026-05-06 17:29:12 -05:00
parent e471f3f8f1
commit a1fd235755
No known key found for this signature in database
4 changed files with 67 additions and 64 deletions
Download patch file Download diff file Expand all files Collapse all files

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

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

36
pkg/cmd/discussion/client/client_impl_test.go
View file

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

25
pkg/cmd/discussion/create/create.go
View file

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

54
pkg/cmd/discussion/create/create_test.go
View file

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