Date: Sat, 19 Feb 2022 15:44:07 +0100
From: Thorben =?utf-8?Q?G=C3=BCnther?= <admin@xenrox.net>
To: Simon Ser <contact@emersion.fr>
Cc: ~emersion/hut-dev@lists.sr.ht
Subject: Re: [PATCH] todo ticket create: new command
Message-ID: <20220219144407.se7nbklvlohj7sdz@xenrox.net>
References: <20220218135156.614905-1-contact@emersion.fr>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="eaowoc6xhbeqw555"
Content-Disposition: inline
In-Reply-To: <20220218135156.614905-1-contact@emersion.fr>


--eaowoc6xhbeqw555
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Feb 18, 2022 at 01:52:04PM +0000, Simon Ser wrote:
> Pretty similar to `hut todo ticket comment`, except we need to
> provide both a subject and a body. Add commented instructions when
> using an editor, similar to git-commit's behavior.

Code mostly LGTM, but right now tickets that are created this way, will
have a bug that will hinder other GraphQL operations (see IRC), so I
think we should wait a bit before applying this.

> ---
>  srht/todosrht/gql.go             | 11 ++++
>  srht/todosrht/operations.graphql |  6 +++
>  todo.go                          | 87 ++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+)
>

Manpage entry is missing.

> diff --git a/srht/todosrht/gql.go b/srht/todosrht/gql.go
> index e6cb2bfbdaa7..d1708fabe870 100644
> --- a/srht/todosrht/gql.go
> +++ b/srht/todosrht/gql.go
> @@ -549,3 +549,14 @@ func UpdateTicketStatus(client *gqlclient.Client, ctx context.Context, trackerId
>  	err = client.Execute(ctx, op, &respData)
>  	return respData.UpdateTicketStatus, err
>  }
> +
> +func SubmitTicket(client *gqlclient.Client, ctx context.Context, trackerId int32, input SubmitTicketInput) (submitTicket *Ticket, err error) {
> +	op := gqlclient.NewOperation("mutation submitTicket ($trackerId: Int!, $input: SubmitTicketInput!) {\n\tsubmitTicket(trackerId: $trackerId, input: $input) {\n\t\tid\n\t}\n}\n")
> +	op.Var("trackerId", trackerId)
> +	op.Var("input", input)
> +	var respData struct {
> +		SubmitTicket *Ticket
> +	}
> +	err = client.Execute(ctx, op, &respData)
> +	return respData.SubmitTicket, err
> +}
> diff --git a/srht/todosrht/operations.graphql b/srht/todosrht/operations.graphql
> index 883276f0ec8d..2d31a42d0331 100644
> --- a/srht/todosrht/operations.graphql
> +++ b/srht/todosrht/operations.graphql
> @@ -98,3 +98,9 @@ mutation updateTicketStatus(
>          }
>      }
>  }
> +
> +mutation submitTicket($trackerId: Int!, $input: SubmitTicketInput!) {
> +    submitTicket(trackerId: $trackerId, input: $input) {
> +        id
> +    }
> +}
> diff --git a/todo.go b/todo.go
> index 5860c3f739df..f4a060cf24b6 100644
> --- a/todo.go
> +++ b/todo.go
> @@ -1,12 +1,14 @@
>  package main
>
>  import (
> +	"bufio"
>  	"context"
>  	"fmt"
>  	"io"
>  	"log"
>  	"os"
>  	"strings"
> +	"unicode"
>
>  	"git.sr.ht/~emersion/hut/srht/todosrht"
>  	"git.sr.ht/~emersion/hut/termfmt"
> @@ -110,6 +112,7 @@ func newTodoTicketCommand() *cobra.Command {
>  	cmd.AddCommand(newTodoTicketListCommand())
>  	cmd.AddCommand(newTodoTicketCommentCommand())
>  	cmd.AddCommand(newTodoTicketStatusCommand())
> +	cmd.AddCommand(newTodoTicketCreateCommand())
>  	return cmd
>  }
>
> @@ -296,6 +299,90 @@ func newTodoTicketStatusCommand() *cobra.Command {
>  	return cmd
>  }
>
> +const todoTicketPrefill = `
> +<!--
> +Please enter the subject of the new ticket. The subject line can be
> +followed by a blank line and a Markdown description. An empty subject
> +aborts the ticket.
> +-->`

Should add an "above" here. My first intuition was to write below the
comment (mostly because thats how my first prototype worked).

> +
> +func newTodoTicketCreateCommand() *cobra.Command {
> +	var stdin bool
> +	run := func(cmd *cobra.Command, args []string) {
> +		ctx := cmd.Context()
> +		name, owner, instance := getTrackerName(ctx, cmd)
> +		c := createClientWithInstance("todo", cmd, instance)
> +		trackerID := getTrackerID(c, ctx, name, owner)
> +
> +		var input todosrht.SubmitTicketInput
> +		if stdin {
> +			br := bufio.NewReader(os.Stdin)
> +			fmt.Printf("Subject: ")
> +
> +			var err error
> +			input.Subject, err = br.ReadString('\n')
> +			if err != nil {
> +				log.Fatalf("failed to read subject: %v", err)
> +			}
> +			input.Subject = strings.TrimSpace(input.Subject)
> +			if input.Subject == "" {
> +				fmt.Println("Aborting due to empty subject.")
> +				os.Exit(1)
> +			}
> +
> +			fmt.Printf("Description %s:\n", termfmt.Dim.String("(Markdown supported)"))
> +			bodyBytes, err := io.ReadAll(br)
> +			if err != nil {
> +				log.Fatalf("failed to read description: %v", err)
> +			}
> +			if body := strings.TrimSpace(string(bodyBytes)); body != "" {
> +				input.Body = &body
> +			}
> +		} else {
> +			text, err := getInputWithEditor("hut_ticket*.md", todoTicketPrefill)
> +			if err != nil {
> +				log.Fatalf("failed to read ticket subject and description: %v", err)
> +			}
> +
> +			// Drop our prefilled comment, but without stripping leading
> +			// whitespace
> +			text = strings.TrimRightFunc(text, unicode.IsSpace)
> +			text = strings.TrimSuffix(text, todoTicketPrefill)
> +			text = strings.TrimRightFunc(text, unicode.IsSpace)

What do you think about moving the comment drop to the
getInputWithEditor function - maybe behind a bool parameter? I had
something similar in mind for creating mailing lists and trackers so we
probably want to re-use the functionality.

Besides a sanity check could be nice. Test if the stripped string still
contains the prefilled text and fail in this case (for example if a user
wrongly writes the ticket below the text instead of above).

> +
> +			parts := strings.SplitN(text, "\n", 2)
> +			input.Subject = strings.TrimSpace(parts[0])
> +			if len(parts) > 1 {
> +				body := strings.TrimSpace(parts[1])
> +				input.Body = &body
> +			}
> +		}
> +
> +		if input.Subject == "" {
> +			fmt.Println("Aborting due to empty subject.")
> +			os.Exit(1)
> +		}
> +
> +		ticket, err := todosrht.SubmitTicket(c.Client, ctx, trackerID, input)
> +		if err != nil {
> +			log.Fatal(err)
> +		} else if ticket == nil {
> +			log.Fatal("failed to create ticket")
> +		}
> +
> +		fmt.Printf("Created new ticket %v\n", termfmt.DarkYellow.Sprintf("#%v", ticket.Id))
> +	}
> +
> +	cmd := &cobra.Command{
> +		Use:   "create",
> +		Short: "Create a new ticket",
> +		Args:  cobra.ExactArgs(0),
> +		Run:   run,
> +	}
> +	cmd.Flags().BoolVar(&stdin, "stdin", false, "read ticket from stdin")
> +	return cmd
> +}
> +
>  func getTrackerID(c *Client, ctx context.Context, name, owner string) int32 {
>  	var (
>  		tracker *todosrht.Tracker
>
> base-commit: f7519c5d49a8287a0e47f9459cb859304d1e8b0c
> --
> 2.35.1
>
>

--eaowoc6xhbeqw555
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQJiBAABCgBMFiEE8S88GeQBs5H8ShuoQVzXeNjFr+0FAmIRAjYuFIAAAAAAFQAQ
cGthLWFkZHJlc3NAZ251cGcub3JnYWRtaW5AeGVucm94Lm5ldAAKCRBBXNd42MWv
7UpqD/9yf5omLbFinFGUFI4upYdXz/X1ym3gDbqZdur2TF/VBpZQFRyV/73mWvZZ
7slJ6IDGKFmZ3ZltNkuoJUc/xenF+JykPfnVedxZotBHmBWPF0oqqO30eQoqU2Tk
MYaWudw60zguG1fRgaSPTZGzdbLyw8k/OdFbB4R6QSl5uBD+dR7dWXpEW3+gIapL
WV0VN63/cPgVqodeOFbEPfSYiRZUGNnlizifqEBvu4bK5y24v9xp93612MjeLRU8
Peex/iCO4y9eq4hVVOin8PsmWPHLAUr3Glo3FbCuzCngsbpopvnbXZQso3OL32Re
OEzkg0EKJ4yUwge1tNO8mvk5lJ2qI2CMwcX3rAlndcZB3T+8hGsIV1FgdxEQpcof
ufD7A4fyrAb98P1L292vPazVF74cI6mxd9uGVW4YV6+61oTa5OPn/Yhj6bNT9c5O
AQhtffolyEp6bJXNhWhEqcLg1FQMYW9iWsFF7H3s6W8393Cj+T1isWAutccLVqKu
VkdHbLGP2jkUxAHm9i49ssbYRUiwjYwahOzZFqM/GLPyf+hwVane/QJeaV7REt3i
Wm6QI0Xi6jhSR7uraXulXnEZkDYK/Gin1Qq6Pbmh9a3nIlt1CQriEzFbPzsx7sMr
+mOrvJed4ClN/wZSZSVxpoFCdYxFqc1KklkGscdXad+clCIjpg==
=6qLf
-----END PGP SIGNATURE-----

--eaowoc6xhbeqw555--