Drop interactive Parent prompt and move Parent off Editable
The interactive Parent prompt was a free-text input with no candidate-listing UX, an oversight from the initial Issues 2.0 landing. Sub-issues, blocked-by, and blocking already live as bare flag fields outside of Editable for the same reason; bring Parent into line. editRun now reads opts.SetParent / opts.RemoveParent directly when constructing DeferredUpdateIssueOptions, and the survey machinery no longer sees Parent at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
02457482a5
commit
e87ccc5fca
4 changed files with 25 additions and 128 deletions
|
|
@ -173,21 +173,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
|
|||
if flags.Changed("type") {
|
||||
opts.Editable.IssueType.Edited = true
|
||||
}
|
||||
if flags.Changed("set-parent") || opts.RemoveParent {
|
||||
opts.Editable.Parent.Edited = true
|
||||
if opts.RemoveParent {
|
||||
opts.Editable.Parent.Value = ""
|
||||
} else {
|
||||
opts.Editable.Parent.Value = opts.SetParent
|
||||
}
|
||||
}
|
||||
|
||||
// Sub-issue and relationship flags are outside the Editable pattern
|
||||
// but still need to prevent interactive mode.
|
||||
hasRelationshipFlags := len(opts.AddSubIssues) > 0 || len(opts.RemoveSubIssues) > 0 ||
|
||||
hasRelationshipFlags := flags.Changed("set-parent") || opts.RemoveParent ||
|
||||
len(opts.AddSubIssues) > 0 || len(opts.RemoveSubIssues) > 0 ||
|
||||
len(opts.AddBlockedBy) > 0 || len(opts.RemoveBlockedBy) > 0 ||
|
||||
len(opts.AddBlocking) > 0 || len(opts.RemoveBlocking) > 0
|
||||
|
||||
// Drop into interactive mode only if the user passed no edit flags at all.
|
||||
if !opts.Editable.Dirty() && !hasRelationshipFlags {
|
||||
opts.Interactive = true
|
||||
}
|
||||
|
|
@ -250,7 +242,6 @@ func editRun(opts *EditOptions) error {
|
|||
// Prompt the user which fields they'd like to edit.
|
||||
editable := opts.Editable
|
||||
editable.IssueType.Allowed = true
|
||||
editable.Parent.Allowed = true
|
||||
if opts.Interactive {
|
||||
err = opts.FieldsToEditSurvey(opts.Prompter, &editable)
|
||||
if err != nil {
|
||||
|
|
@ -297,7 +288,7 @@ func editRun(opts *EditOptions) error {
|
|||
if editable.IssueType.Edited {
|
||||
lookupFields = append(lookupFields, "issueType")
|
||||
}
|
||||
if editable.Parent.Edited || opts.RemoveParent {
|
||||
if opts.SetParent != "" || opts.RemoveParent {
|
||||
lookupFields = append(lookupFields, "parent")
|
||||
}
|
||||
|
||||
|
|
@ -371,9 +362,6 @@ func editRun(opts *EditOptions) error {
|
|||
if issue.IssueType != nil {
|
||||
editable.IssueType.Default = issue.IssueType.Name
|
||||
}
|
||||
if issue.Parent != nil {
|
||||
editable.Parent.Default = fmt.Sprintf("#%d", issue.Parent.Number)
|
||||
}
|
||||
|
||||
// Allow interactive prompts for one issue; failed earlier if multiple issues specified.
|
||||
if opts.Interactive {
|
||||
|
|
@ -400,7 +388,7 @@ func editRun(opts *EditOptions) error {
|
|||
return
|
||||
}
|
||||
|
||||
mutations, err := deferredUpdateIssueOptions(apiClient, baseRepo, issue, opts, &editable, issueTypeID)
|
||||
mutations, err := deferredUpdateIssueOptions(apiClient, baseRepo, issue, opts, issueTypeID)
|
||||
if err != nil {
|
||||
failedIssueChan <- fmt.Sprintf("failed to update %s: %s", issue.URL, err)
|
||||
return
|
||||
|
|
@ -465,7 +453,7 @@ func lookupIssueTypeID(editable *prShared.Editable) (string, error) {
|
|||
return id, nil
|
||||
}
|
||||
|
||||
func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, editOpts *EditOptions, editable *prShared.Editable, issueTypeID string) (api.DeferredUpdateIssueOptions, error) {
|
||||
func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, editOpts *EditOptions, issueTypeID string) (api.DeferredUpdateIssueOptions, error) {
|
||||
updateOpts := api.DeferredUpdateIssueOptions{
|
||||
IssueID: issue.ID,
|
||||
Hostname: baseRepo.RepoHost(),
|
||||
|
|
@ -473,18 +461,16 @@ func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, i
|
|||
ReplaceExistingParent: true,
|
||||
}
|
||||
|
||||
if editable.Parent.Edited {
|
||||
if editable.Parent.Value == "" {
|
||||
if issue.Parent != nil {
|
||||
updateOpts.RemoveParentID = issue.Parent.ID
|
||||
}
|
||||
} else {
|
||||
parentID, err := issueShared.ResolveIssueRef(client, baseRepo, editable.Parent.Value)
|
||||
if err != nil {
|
||||
return updateOpts, fmt.Errorf("resolving --set-parent reference %q: %w", editable.Parent.Value, err)
|
||||
}
|
||||
updateOpts.ParentID = parentID
|
||||
if editOpts.RemoveParent {
|
||||
if issue.Parent != nil {
|
||||
updateOpts.RemoveParentID = issue.Parent.ID
|
||||
}
|
||||
} else if editOpts.SetParent != "" {
|
||||
parentID, err := issueShared.ResolveIssueRef(client, baseRepo, editOpts.SetParent)
|
||||
if err != nil {
|
||||
return updateOpts, fmt.Errorf("resolving --set-parent reference %q: %w", editOpts.SetParent, err)
|
||||
}
|
||||
updateOpts.ParentID = parentID
|
||||
}
|
||||
|
||||
for _, ref := range editOpts.AddSubIssues {
|
||||
|
|
|
|||
|
|
@ -303,12 +303,6 @@ func TestNewCmdEdit(t *testing.T) {
|
|||
output: EditOptions{
|
||||
IssueNumbers: []int{23},
|
||||
SetParent: "100",
|
||||
Editable: prShared.Editable{
|
||||
Parent: prShared.EditableString{
|
||||
Value: "100",
|
||||
Edited: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -317,12 +311,6 @@ func TestNewCmdEdit(t *testing.T) {
|
|||
output: EditOptions{
|
||||
IssueNumbers: []int{23},
|
||||
RemoveParent: true,
|
||||
Editable: prShared.Editable{
|
||||
Parent: prShared.EditableString{
|
||||
Value: "",
|
||||
Edited: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -913,60 +901,13 @@ func Test_editRun(t *testing.T) {
|
|||
},
|
||||
stdout: "https://github.com/OWNER/REPO/issue/123\n",
|
||||
},
|
||||
{
|
||||
name: "interactive edit parent prompt",
|
||||
input: &EditOptions{
|
||||
Detector: &fd.EnabledDetectorMock{},
|
||||
IssueNumbers: []int{123},
|
||||
Interactive: true,
|
||||
FieldsToEditSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable) error {
|
||||
// Verify the survey is allowed to offer Parent as an option for issue edit.
|
||||
assert.True(t, eo.Parent.Allowed)
|
||||
eo.Parent.Edited = true
|
||||
return nil
|
||||
},
|
||||
EditFieldsSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable, _ string) error {
|
||||
eo.Parent.Value = "100"
|
||||
return nil
|
||||
},
|
||||
FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error {
|
||||
return nil
|
||||
},
|
||||
DetermineEditor: func() (string, error) { return "vim", nil },
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockIssueGet(t, reg)
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query IssueNodeID\b`),
|
||||
httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "issue": { "id": "PARENT_100_ID" } } } }
|
||||
`),
|
||||
)
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`mutation AddSubIssue\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "addSubIssue": { "issue": { "id": "PARENT_100_ID" } } } }`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, "PARENT_100_ID", inputs["issueId"])
|
||||
assert.Equal(t, "123", inputs["subIssueId"])
|
||||
assert.Equal(t, true, inputs["replaceParent"])
|
||||
}),
|
||||
)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/issue/123\n",
|
||||
},
|
||||
{
|
||||
name: "edit set parent",
|
||||
input: &EditOptions{
|
||||
Detector: &fd.EnabledDetectorMock{},
|
||||
IssueNumbers: []int{123},
|
||||
Interactive: false,
|
||||
Editable: prShared.Editable{
|
||||
Parent: prShared.EditableString{
|
||||
Value: "100",
|
||||
Edited: true,
|
||||
},
|
||||
},
|
||||
SetParent: "100",
|
||||
FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error {
|
||||
return nil
|
||||
},
|
||||
|
|
@ -999,12 +940,6 @@ func Test_editRun(t *testing.T) {
|
|||
IssueNumbers: []int{123},
|
||||
Interactive: false,
|
||||
RemoveParent: true,
|
||||
Editable: prShared.Editable{
|
||||
Parent: prShared.EditableString{
|
||||
Value: "",
|
||||
Edited: true,
|
||||
},
|
||||
},
|
||||
FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error {
|
||||
return nil
|
||||
},
|
||||
|
|
@ -1500,15 +1435,8 @@ func Test_editRun_crossHostRelationshipRefs(t *testing.T) {
|
|||
input *EditOptions
|
||||
}{
|
||||
{
|
||||
name: "set parent",
|
||||
input: &EditOptions{
|
||||
Editable: prShared.Editable{
|
||||
Parent: prShared.EditableString{
|
||||
Value: crossHostURL,
|
||||
Edited: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
name: "set parent",
|
||||
input: &EditOptions{SetParent: crossHostURL},
|
||||
},
|
||||
{
|
||||
name: "add sub-issue",
|
||||
|
|
|
|||
|
|
@ -23,7 +23,6 @@ type Editable struct {
|
|||
Milestone EditableString
|
||||
IssueType EditableString
|
||||
IssueTypeNameToID map[string]string
|
||||
Parent EditableString
|
||||
Metadata api.RepoMetadataResult
|
||||
|
||||
// TODO ApiActorsSupported
|
||||
|
|
@ -80,8 +79,7 @@ func (e Editable) Dirty() bool {
|
|||
e.Labels.Edited ||
|
||||
e.Projects.Edited ||
|
||||
e.Milestone.Edited ||
|
||||
e.IssueType.Edited ||
|
||||
e.Parent.Edited
|
||||
e.IssueType.Edited
|
||||
}
|
||||
|
||||
func (e Editable) TitleValue() *string {
|
||||
|
|
@ -298,7 +296,6 @@ func (e *Editable) Clone() Editable {
|
|||
Milestone: e.Milestone.clone(),
|
||||
IssueType: e.IssueType.clone(),
|
||||
IssueTypeNameToID: e.IssueTypeNameToID,
|
||||
Parent: e.Parent.clone(),
|
||||
ApiActorsSupported: e.ApiActorsSupported,
|
||||
// Shallow copy since no mutation.
|
||||
Metadata: e.Metadata,
|
||||
|
|
@ -463,12 +460,6 @@ func EditFieldsSurvey(p EditPrompter, editable *Editable, editorCommand string)
|
|||
editable.IssueType.Value = editable.IssueType.Options[selected]
|
||||
}
|
||||
}
|
||||
if editable.Parent.Edited {
|
||||
editable.Parent.Value, err = p.Input("Parent (issue number or URL, leave empty to remove)", editable.Parent.Default)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
confirm, err := p.Confirm("Submit?", true)
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
@ -498,9 +489,6 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error {
|
|||
if editable.IssueType.Allowed {
|
||||
opts = append(opts, "Type")
|
||||
}
|
||||
if editable.Parent.Allowed {
|
||||
opts = append(opts, "Parent")
|
||||
}
|
||||
opts = append(opts, "Projects", "Milestone")
|
||||
results, err := multiSelectSurvey(p, "What would you like to edit?", []string{}, opts)
|
||||
if err != nil {
|
||||
|
|
@ -525,9 +513,6 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error {
|
|||
if contains(results, "Type") {
|
||||
editable.IssueType.Edited = true
|
||||
}
|
||||
if contains(results, "Parent") {
|
||||
editable.Parent.Edited = true
|
||||
}
|
||||
if contains(results, "Projects") {
|
||||
editable.Projects.Edited = true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -262,10 +262,10 @@ func TestTitleSurvey(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestFieldsToEditSurvey_IssueOnlyFields(t *testing.T) {
|
||||
t.Run("without Allowed flags omits Type and Parent", func(t *testing.T) {
|
||||
t.Run("without Allowed flag omits Type", func(t *testing.T) {
|
||||
pm := prompter.NewMockPrompter(t)
|
||||
pm.RegisterMultiSelect("What would you like to edit?", []string{},
|
||||
// Type and Parent should NOT appear here
|
||||
// Type should NOT appear here
|
||||
[]string{"Title", "Body", "Assignees", "Labels", "Projects", "Milestone"},
|
||||
func(_ string, _, _ []string) ([]int, error) {
|
||||
return []int{0}, nil
|
||||
|
|
@ -277,21 +277,19 @@ func TestFieldsToEditSurvey_IssueOnlyFields(t *testing.T) {
|
|||
assert.True(t, editable.Title.Edited)
|
||||
})
|
||||
|
||||
t.Run("with Allowed flags includes Type and Parent", func(t *testing.T) {
|
||||
t.Run("with Allowed flag includes Type", func(t *testing.T) {
|
||||
pm := prompter.NewMockPrompter(t)
|
||||
pm.RegisterMultiSelect("What would you like to edit?", []string{},
|
||||
// Type and Parent should appear between Labels and Projects
|
||||
[]string{"Title", "Body", "Assignees", "Labels", "Type", "Parent", "Projects", "Milestone"},
|
||||
// Type should appear between Labels and Projects
|
||||
[]string{"Title", "Body", "Assignees", "Labels", "Type", "Projects", "Milestone"},
|
||||
func(_ string, _, _ []string) ([]int, error) {
|
||||
return []int{4, 5}, nil // select Type and Parent
|
||||
return []int{4}, nil // select Type
|
||||
})
|
||||
|
||||
editable := &Editable{}
|
||||
editable.IssueType.Allowed = true
|
||||
editable.Parent.Allowed = true
|
||||
err := FieldsToEditSurvey(pm, editable)
|
||||
require.NoError(t, err)
|
||||
assert.True(t, editable.IssueType.Edited)
|
||||
assert.True(t, editable.Parent.Edited)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue