Skip to content

Commit 67aa049

Browse files
maxbeizerCopilot
maxbeizer
and
Copilot
committed
Validate empty title in editor mode for issue/pr edit
After TitledEditSurvey returns, check that the title is not blank and abort with a clear error message, matching the behavior of the create commands. Also update the editor hint text to say 'aborts the process' instead of 'aborts the creation process' since the hint is shared by both create and edit flows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7ea57f9 commit 67aa049

6 files changed

Lines changed: 58 additions & 8 deletions

File tree

‎pkg/cmd/issue/edit/edit.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,9 @@ func editRun(opts *EditOptions) error {
337337
if err != nil {
338338
return err
339339
}
340+
if editable.Title.Value == "" {
341+
return fmt.Errorf("title can't be blank")
342+
}
340343
} else if opts.Interactive {
341344
editorCommand, err := opts.DetermineEditor()
342345
if err != nil {

‎pkg/cmd/issue/edit/edit_test.go‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ func Test_editRun(t *testing.T) {
414414
httpStubs func(*testing.T, *httpmock.Registry)
415415
stdout string
416416
stderr string
417-
wantErr bool
417+
wantErr string
418418
}{
419419
{
420420
name: "non-interactive",
@@ -574,7 +574,7 @@ func Test_editRun(t *testing.T) {
574574
] }`),
575575
)
576576
},
577-
wantErr: true,
577+
wantErr: "Could not resolve to an Issue",
578578
},
579579
{
580580
name: "non-interactive multiple issues with update failures",
@@ -644,7 +644,7 @@ func Test_editRun(t *testing.T) {
644644
https://github.com/OWNER/REPO/issue/123
645645
`),
646646
stderr: `failed to update https://github.com/OWNER/REPO/issue/456:.*test error`,
647-
wantErr: true,
647+
wantErr: "failed to update",
648648
},
649649
{
650650
name: "interactive",
@@ -721,6 +721,24 @@ func Test_editRun(t *testing.T) {
721721
},
722722
stdout: "https://github.com/OWNER/REPO/issue/123\n",
723723
},
724+
{
725+
name: "editor mode empty title",
726+
input: &EditOptions{
727+
Detector: &fd.EnabledDetectorMock{},
728+
IssueNumbers: []int{123},
729+
EditorMode: true,
730+
TitledEditSurvey: func(title, body string) (string, string, error) {
731+
return "", "", nil
732+
},
733+
FetchOptions: func(client *api.Client, repo ghrepo.Interface, editable *prShared.Editable, v1 gh.ProjectsV1Support) error {
734+
return nil
735+
},
736+
},
737+
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
738+
mockIssueGet(t, reg)
739+
},
740+
wantErr: "title can't be blank",
741+
},
724742
{
725743
name: "interactive prompts with actor assignee display names when actors available",
726744
input: &EditOptions{
@@ -847,8 +865,9 @@ func Test_editRun(t *testing.T) {
847865
tt.input.BaseRepo = baseRepo
848866

849867
err := editRun(tt.input)
850-
if tt.wantErr {
851-
assert.Error(t, err)
868+
if tt.wantErr != "" {
869+
require.Error(t, err)
870+
assert.Contains(t, err.Error(), tt.wantErr)
852871
} else {
853872
assert.NoError(t, err)
854873
}

‎pkg/cmd/pr/edit/edit.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,9 @@ func editRun(opts *EditOptions) error {
338338
if err != nil {
339339
return err
340340
}
341+
if editable.Title.Value == "" {
342+
return fmt.Errorf("title can't be blank")
343+
}
341344
} else if opts.Interactive {
342345
err = opts.Surveyor.FieldsToEdit(&editable)
343346
if err != nil {

‎pkg/cmd/pr/edit/edit_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ func Test_editRun(t *testing.T) {
406406
httpStubs func(*testing.T, *httpmock.Registry)
407407
stdout string
408408
stderr string
409+
wantsErr string
409410
}{
410411
{
411412
name: "non-interactive",
@@ -1234,6 +1235,26 @@ func Test_editRun(t *testing.T) {
12341235
},
12351236
stdout: "https://github.com/OWNER/REPO/pull/123\n",
12361237
},
1238+
{
1239+
name: "editor mode empty title",
1240+
input: &EditOptions{
1241+
Detector: &fd.EnabledDetectorMock{},
1242+
SelectorArg: "123",
1243+
Finder: shared.NewMockFinder("123", &api.PullRequest{
1244+
URL: "https://github.com/OWNER/REPO/pull/123",
1245+
Title: "pr title",
1246+
Body: "pr body",
1247+
}, ghrepo.New("OWNER", "REPO")),
1248+
EditorMode: true,
1249+
TitledEditSurvey: func(title, body string) (string, string, error) {
1250+
return "", "", nil
1251+
},
1252+
Surveyor: testSurveyor{},
1253+
Fetcher: testFetcher{},
1254+
},
1255+
httpStubs: func(t *testing.T, reg *httpmock.Registry) {},
1256+
wantsErr: "title can't be blank",
1257+
},
12371258
}
12381259
for _, tt := range tests {
12391260
t.Run(tt.name, func(t *testing.T) {
@@ -1254,6 +1275,10 @@ func Test_editRun(t *testing.T) {
12541275
tt.input.BaseRepo = baseRepo
12551276

12561277
err := editRun(tt.input)
1278+
if tt.wantsErr != "" {
1279+
require.EqualError(t, err, tt.wantsErr)
1280+
return
1281+
}
12571282
assert.NoError(t, err)
12581283
assert.Equal(t, tt.stdout, stdout.String())
12591284
assert.Equal(t, tt.stderr, stderr.String())

‎pkg/cmd/pr/shared/survey.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ func (e *UserEditor) Edit(filename, initialValue string) (string, error) {
411411
const editorHintMarker = "------------------------ >8 ------------------------"
412412
const editorHint = `
413413
Please Enter the title on the first line and the body on subsequent lines.
414-
Lines below dotted lines will be ignored, and an empty title aborts the creation process.`
414+
Lines below dotted lines will be ignored, and an empty title aborts the process.`
415415

416416
func TitledEditSurvey(editor Editor) func(string, string) (string, string, error) {
417417
return func(initialTitle, initialBody string) (string, string, error) {

‎pkg/cmd/pr/shared/survey_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ editedBody
195195
------------------------ >8 ------------------------
196196
197197
Please Enter the title on the first line and the body on subsequent lines.
198-
Lines below dotted lines will be ignored, and an empty title aborts the creation process.`, nil
198+
Lines below dotted lines will be ignored, and an empty title aborts the process.`, nil
199199
},
200200
}
201201

@@ -207,7 +207,7 @@ initialBody
207207
------------------------ >8 ------------------------
208208
209209
Please Enter the title on the first line and the body on subsequent lines.
210-
Lines below dotted lines will be ignored, and an empty title aborts the creation process.`, editorInitialText)
210+
Lines below dotted lines will be ignored, and an empty title aborts the process.`, editorInitialText)
211211
assert.Equal(t, "editedTitle", title)
212212
assert.Equal(t, "editedBody", body)
213213
}

0 commit comments

Comments
 (0)