Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit issue command #2915

Merged
merged 4 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 55 additions & 33 deletions api/queries_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,44 +24,52 @@ type IssuesAndTotalCount struct {
}

type Issue struct {
ID string
Number int
Title string
URL string
State string
Closed bool
Body string
CreatedAt time.Time
UpdatedAt time.Time
Comments Comments
Author Author
Assignees struct {
Nodes []struct {
Login string
}
TotalCount int
ID string
Number int
Title string
URL string
State string
Closed bool
Body string
CreatedAt time.Time
UpdatedAt time.Time
Comments Comments
Author Author
Assignees Assignees
Labels Labels
ProjectCards ProjectCards
Milestone Milestone
ReactionGroups ReactionGroups
}

type Assignees struct {
Nodes []struct {
Login string
}
TotalCount int
}

type Labels struct {
Nodes []struct {
Name string
}
Labels struct {
Nodes []struct {
TotalCount int
}

type ProjectCards struct {
Nodes []struct {
Project struct {
Name string
}
TotalCount int
}
ProjectCards struct {
Nodes []struct {
Project struct {
Name string
}
Column struct {
Name string
}
Column struct {
Name string
}
TotalCount int
}
Milestone struct {
Title string
}
ReactionGroups ReactionGroups
TotalCount int
}

type Milestone struct {
Title string
}

type IssuesDisabledError struct {
Expand Down Expand Up @@ -488,6 +496,20 @@ func IssueDelete(client *Client, repo ghrepo.Interface, issue Issue) error {
return err
}

func IssueUpdate(client *Client, repo ghrepo.Interface, params githubv4.UpdateIssueInput) error {
var mutation struct {
UpdateIssue struct {
Issue struct {
ID string
}
} `graphql:"updateIssue(input: $input)"`
}
variables := map[string]interface{}{"input": params}
gql := graphQLClient(client.http, repo.RepoHost())
err := gql.MutateNamed(context.Background(), "IssueUpdate", &mutation, variables)
return err
}

// milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID
// This conversion is necessary since the GraphQL API requires the use of the milestone's database ID
// for querying the related issues.
Expand Down
22 changes: 12 additions & 10 deletions api/queries_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,6 @@ type PullRequest struct {
}
}
}
ReviewRequests struct {
Nodes []struct {
RequestedReviewer struct {
TypeName string `json:"__typename"`
Login string
Name string
}
}
TotalCount int
}
Assignees struct {
Nodes []struct {
Login string
Expand Down Expand Up @@ -119,6 +109,18 @@ type PullRequest struct {
Comments Comments
ReactionGroups ReactionGroups
Reviews PullRequestReviews
ReviewRequests ReviewRequests
}

type ReviewRequests struct {
Nodes []struct {
RequestedReviewer struct {
TypeName string `json:"__typename"`
Login string
Name string
}
}
TotalCount int
}

type NotFoundError struct {
Expand Down
236 changes: 236 additions & 0 deletions pkg/cmd/issue/edit/edit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
package edit

import (
"errors"
"fmt"
"net/http"

"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/api"
"github.com/cli/cli/internal/ghrepo"
shared "github.com/cli/cli/pkg/cmd/issue/shared"
prShared "github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/utils"
"github.com/shurcooL/githubv4"
"github.com/spf13/cobra"
)

type EditOptions struct {
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)

OpenInBrowser func(string) error
DetermineEditor func() (string, error)
FieldsToEditSurvey func(*prShared.EditableOptions) error
EditableSurvey func(string, *prShared.EditableOptions) error
FetchOptions func(*api.Client, ghrepo.Interface, *prShared.EditableOptions) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing around funcs for the purpose of stubbing in tests—even though that is the pattern that we practiced before—how about we consider gradually moving to interfaces? It would allow the same kind of dependency injection, with the added benefit of better readability/being more idiomatic Go.

You don't have to rewrite all of these; I'm just wondering whether you think that accepting interfaces and then substituting the concrete implementation object in tests would be an approach that could make the above cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the interface approach and generally think it is better. I mostly just forgot that we had discussed it when I wrote this and ended up falling back on what we had been doing.

My only real complaint with the interface approach is that it doesn't seem to bring much improvement when the interface defines only one function and results in slightly more code. I do think bundling up the survey functions into a surveyAsker interface, as seen in the gist, is great though.

I will keep the idea top of mind when I do the pr edit command and see how that goes, and if it works well I will come back and update this command.


SelectorArg string
Interactive bool
WebMode bool

prShared.EditableOptions
}

func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command {
opts := &EditOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
OpenInBrowser: utils.OpenInBrowser,
DetermineEditor: func() (string, error) { return cmdutil.DetermineEditor(f.Config) },
FieldsToEditSurvey: prShared.FieldsToEditSurvey,
EditableSurvey: prShared.EditableSurvey,
FetchOptions: prShared.FetchOptions,
}

cmd := &cobra.Command{
Use: "edit {<number> | <url>}",
Short: "Edit an issue",
Example: heredoc.Doc(`
$ gh issue edit 23 --title "I found a bug" --body "Nothing works"
$ gh issue edit 23 --label "bug,help wanted"
$ gh issue edit 23 --label bug --label "help wanted"
$ gh issue edit 23 --assignee monalisa,hubot
$ gh issue edit 23 --assignee @me
$ gh issue edit 23 --project "Roadmap"
`),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo

opts.SelectorArg = args[0]

flags := cmd.Flags()
if flags.Changed("title") {
opts.EditableOptions.TitleEdited = true
}
if flags.Changed("body") {
opts.EditableOptions.BodyEdited = true
}
if flags.Changed("assignee") {
opts.EditableOptions.AssigneesEdited = true
}
if flags.Changed("label") {
opts.EditableOptions.LabelsEdited = true
}
if flags.Changed("project") {
opts.EditableOptions.ProjectsEdited = true
}
if flags.Changed("milestone") {
opts.EditableOptions.MilestoneEdited = true
}

if !opts.EditableOptions.Dirty() {
opts.Interactive = true
}

if opts.Interactive && !opts.IO.CanPrompt() {
return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, --milestone, or --web required when not running interactively")}
}

if runF != nil {
return runF(opts)
}

return editRun(opts)
},
}

cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue")
samcoe marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.")
cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.")
cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.")
cmd.Flags().StringSliceVarP(&opts.EditableOptions.Labels, "label", "l", nil, "Add labels by `name`")
cmd.Flags().StringSliceVarP(&opts.EditableOptions.Projects, "project", "p", nil, "Add the issue to projects by `name`")
cmd.Flags().StringVarP(&opts.EditableOptions.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`")

return cmd
}

func editRun(opts *EditOptions) error {
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)

issue, repo, err := shared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg)
if err != nil {
return err
}

if opts.WebMode {
openURL := issue.URL
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL))
}
return opts.OpenInBrowser(openURL)
}

editOptions := opts.EditableOptions
editOptions.TitleDefault = issue.Title
editOptions.BodyDefault = issue.Body
editOptions.AssigneesDefault = issue.Assignees
editOptions.LabelsDefault = issue.Labels
editOptions.ProjectsDefault = issue.ProjectCards
editOptions.MilestoneDefault = issue.Milestone

if opts.Interactive {
err = opts.FieldsToEditSurvey(&editOptions)
if err != nil {
return err
}
}

opts.IO.StartProgressIndicator()
err = opts.FetchOptions(apiClient, repo, &editOptions)
opts.IO.StopProgressIndicator()
if err != nil {
return err
}

if opts.Interactive {
editorCommand, err := opts.DetermineEditor()
if err != nil {
return err
}
err = opts.EditableSurvey(editorCommand, &editOptions)
if err != nil {
return err
}
}

opts.IO.StartProgressIndicator()
err = updateIssue(apiClient, repo, issue.ID, editOptions)
opts.IO.StopProgressIndicator()
if err != nil {
return err
}

fmt.Fprintln(opts.IO.Out, issue.URL)

return nil
}

func updateIssue(client *api.Client, repo ghrepo.Interface, id string, options prShared.EditableOptions) error {
params := githubv4.UpdateIssueInput{ID: id}
if options.TitleEdited {
title := githubv4.String(options.Title)
params.Title = &title
}
if options.BodyEdited {
body := githubv4.String(options.Body)
params.Body = &body
}
if options.AssigneesEdited {
meReplacer := prShared.NewMeReplacer(client, repo.RepoHost())
assignees, err := meReplacer.ReplaceSlice(options.Assignees)
if err != nil {
return err
}
ids, err := options.Metadata.MembersToIDs(assignees)
if err != nil {
return err
}
assigneeIDs := make([]githubv4.ID, len(ids))
for i, v := range ids {
assigneeIDs[i] = v
}
params.AssigneeIDs = &assigneeIDs
}
if options.LabelsEdited {
ids, err := options.Metadata.LabelsToIDs(options.Labels)
if err != nil {
return err
}
labelIDs := make([]githubv4.ID, len(ids))
for i, v := range ids {
labelIDs[i] = v
}
params.LabelIDs = &labelIDs
}
if options.ProjectsEdited {
ids, err := options.Metadata.ProjectsToIDs(options.Projects)
if err != nil {
return err
}
projectIDs := make([]githubv4.ID, len(ids))
for i, v := range ids {
projectIDs[i] = v
}
params.ProjectIDs = &projectIDs
}
if options.MilestoneEdited {
id, err := options.Metadata.MilestoneToID(options.Milestone)
if err != nil {
return err
}
milestoneID := githubv4.ID(id)
params.MilestoneID = &milestoneID
}
return api.IssueUpdate(client, repo, params)
}
Loading