mirror of
https://gitea.com/gitea/tea.git
synced 2026-10-26 19:11:25 +01:00
fix(pr): refuse pr comment and pr attachment on issues
Gitea serves pull request comments and attachments through issue endpoints, so tea pr comment create 1 posted on issue #1. pr comment and pr attachment commands now check that the number is a pull request, and comment IDs that the comment is on one, and point at the issue command otherwise: "#1 in alice/project is an issue, not a pull request; use tea issue comment create 1". Issues gain a kind field (issue or pull) in JSON. Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
9 files changed
+150
-10
No files matched your search
@@ -19,7 +19,25 @@ func commentRecord(item services.Comment) terminal.Record {
|
||||
return terminal.Record{Heading: terminal.Row(strconv.FormatInt(item.ID, 10), item.Author), Lines: []string{item.Body, item.URL}}
|
||||
}
|
||||
|
||||
func (a *application) commentCommand() *cobra.Command {
|
||||
// requirePull fails unless issue n is a pull request, for pr commands that Gitea serves through
|
||||
// issue endpoints and would otherwise apply to the issue with that number.
|
||||
func requirePull(cmd *cobra.Command, t target, n int64) error {
|
||||
item, err := (services.Issues{Client: t.client, Repository: t.Repo}).View(cmd.Context(), n)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if item.Kind != services.KindPull {
|
||||
return fmt.Errorf("#%d in %s is an issue, not a pull request; use %s %d", n, t.Repo, issueCommandPath(cmd), n)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// issueCommandPath names the issue command that matches a pr command, such as "tea issue comment".
|
||||
func issueCommandPath(cmd *cobra.Command) string {
|
||||
return strings.Replace(cmd.CommandPath(), " pr ", " issue ", 1)
|
||||
}
|
||||
|
||||
func (a *application) commentCommand(kind refKind) *cobra.Command {
|
||||
group := &cobra.Command{Use: "comment", Short: "Manage discussion comments"}
|
||||
var limit, page int
|
||||
list := &cobra.Command{Use: "list <number>", Short: "List comments on an issue or pull request", Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error {
|
||||
@@ -31,6 +49,11 @@ func (a *application) commentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
items, err := (services.Comments{Client: target.client, Repository: target.Repo}).Page(cmd.Context(), n, limit, page)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -77,6 +100,20 @@ func (a *application) commentCommand() *cobra.Command {
|
||||
return err
|
||||
}
|
||||
service := services.Comments{Client: target.client, Repository: target.Repo}
|
||||
if kind == pullRef && verb == "create" {
|
||||
if err := requirePull(cmd, target, id); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if kind == pullRef && verb != "create" {
|
||||
current, err := service.View(cmd.Context(), id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !current.Pull {
|
||||
return fmt.Errorf("comment %d in %s is on an issue, not a pull request; use %s %d", id, target.Repo, issueCommandPath(cmd), id)
|
||||
}
|
||||
}
|
||||
if verb == "create" || verb == "edit" {
|
||||
if useEditor {
|
||||
initial := ""
|
||||
|
||||
@@ -32,3 +32,56 @@ func TestCommentConfirmationNamesTheTarget(t *testing.T) {
|
||||
require.NoError(t, Execute(context.Background(), deps, []string{"issue", "comment", "delete", "12", "-R", "alice/project"}))
|
||||
require.Equal(t, []string{"Comment on alice/project#3", "Delete comment 12 on alice/project"}, prompt.confirmations)
|
||||
}
|
||||
|
||||
func TestPullCommentsAndAttachmentsRequirePullRequests(t *testing.T) {
|
||||
var changed []string
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.Method + " " + r.URL.Path {
|
||||
case "GET /api/v1/repos/alice/project/issues/1":
|
||||
_, _ = w.Write([]byte(`{"number":1,"title":"An issue"}`))
|
||||
case "GET /api/v1/repos/alice/project/issues/13":
|
||||
_, _ = w.Write([]byte(`{"number":13,"title":"A pull request","pull_request":{"merged":false}}`))
|
||||
case "GET /api/v1/repos/alice/project/issues/comments/5":
|
||||
_, _ = w.Write([]byte(`{"id":5,"body":"on an issue","issue_url":"http://x/alice/project/issues/1"}`))
|
||||
case "GET /api/v1/repos/alice/project/issues/comments/6":
|
||||
_, _ = w.Write([]byte(`{"id":6,"body":"on a pull request","pull_request_url":"http://x/alice/project/pulls/13"}`))
|
||||
case "GET /api/v1/repos/alice/project/issues/13/comments", "GET /api/v1/repos/alice/project/issues/13/assets":
|
||||
_, _ = w.Write([]byte(`[]`))
|
||||
default:
|
||||
changed = append(changed, r.Method+" "+r.URL.Path)
|
||||
_, _ = w.Write([]byte(`{"id":7,"body":"x"}`))
|
||||
}
|
||||
}))
|
||||
defer server.Close()
|
||||
deps, out, _ := serverDeps(t, server.URL)
|
||||
for _, test := range []struct {
|
||||
args []string
|
||||
want string
|
||||
}{
|
||||
{[]string{"pr", "comment", "create", "1", "--body", "x"}, "#1 in alice/project is an issue, not a pull request; use tea issue comment create 1"},
|
||||
{[]string{"pr", "comment", "list", "1"}, "#1 in alice/project is an issue"},
|
||||
{[]string{"pr", "comment", "edit", "5", "--body", "x"}, "comment 5 in alice/project is on an issue, not a pull request; use tea issue comment edit 5"},
|
||||
{[]string{"pr", "comment", "delete", "5", "--yes"}, "comment 5 in alice/project is on an issue"},
|
||||
{[]string{"pr", "attachment", "list", "1"}, "#1 in alice/project is an issue, not a pull request; use tea issue attachment list 1"},
|
||||
{[]string{"pr", "attachment", "delete", "1", "2", "--yes"}, "#1 in alice/project is an issue"},
|
||||
} {
|
||||
err := Execute(context.Background(), deps, append(test.args, "-R", "alice/project"))
|
||||
require.ErrorContains(t, err, test.want)
|
||||
}
|
||||
require.Empty(t, changed)
|
||||
require.Empty(t, out.String())
|
||||
for _, args := range [][]string{
|
||||
{"pr", "comment", "create", "13", "--body", "x"},
|
||||
{"pr", "comment", "edit", "6", "--body", "x"},
|
||||
{"pr", "comment", "list", "13"},
|
||||
{"pr", "attachment", "list", "13"},
|
||||
{"issue", "comment", "create", "1", "--body", "x"},
|
||||
} {
|
||||
require.NoError(t, Execute(context.Background(), deps, append(args, "-R", "alice/project", "--json")))
|
||||
}
|
||||
require.Equal(t, []string{
|
||||
"POST /api/v1/repos/alice/project/issues/13/comments",
|
||||
"PATCH /api/v1/repos/alice/project/issues/comments/6",
|
||||
"POST /api/v1/repos/alice/project/issues/1/comments",
|
||||
}, changed)
|
||||
}
|
||||
@@ -16,7 +16,7 @@ import (
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
func (a *application) issueAttachmentCommand(kind refKind) *cobra.Command {
|
||||
group := &cobra.Command{Use: "attachment", Short: "Inspect issue or pull request attachments"}
|
||||
var limit, page int
|
||||
list := &cobra.Command{Use: "list <number>", Short: "List attachments", Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error {
|
||||
@@ -28,6 +28,11 @@ func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
items, err := (services.Issues{Client: target.client, Repository: target.Repo}).Attachments(cmd.Context(), n, limit, page)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -48,6 +53,11 @@ func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
item, err := (services.Issues{Client: target.client, Repository: target.Repo}).Attachment(cmd.Context(), n, id)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -72,6 +82,11 @@ func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
service := services.Issues{Client: target.client, Repository: target.Repo}
|
||||
if verb == "delete" {
|
||||
if err := a.confirm(cmd, fmt.Sprintf("Delete attachment %d on %s#%d", id, target.Repo, n)); err != nil {
|
||||
@@ -115,6 +130,11 @@ func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
item, err := (services.Issues{Client: target.client, Repository: target.Repo}).UploadAttachment(cmd.Context(), n, name, files[0])
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -139,6 +159,11 @@ func (a *application) issueAttachmentCommand() *cobra.Command {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if kind == pullRef {
|
||||
if err := requirePull(cmd, target, n); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
body, err := (services.Issues{Client: target.client, Repository: target.Repo}).DownloadAttachment(cmd.Context(), n, id)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -21,6 +21,8 @@ func TestAttachmentCommandsShareIssueAssetsAndPagination(t *testing.T) {
|
||||
_, _ = w.Write([]byte(`[{"id":1,"name":"one"},{"id":2,"name":"two"}]`))
|
||||
case "/api/v1/repos/alice/project/issues/1/assets/1":
|
||||
_, _ = w.Write([]byte(`{"id":1,"name":"one"}`))
|
||||
case "/api/v1/repos/alice/project/issues/1":
|
||||
_, _ = w.Write([]byte(`{"number":1,"pull_request":{}}`))
|
||||
default:
|
||||
t.Fatalf("unexpected path %s", r.URL.Path)
|
||||
}
|
||||
@@ -43,6 +45,10 @@ func TestAttachmentRenameAndConfirmedDeletion(t *testing.T) {
|
||||
for _, group := range []string{"issue", "pr"} {
|
||||
mutations := 0
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/issues/1" {
|
||||
_, _ = w.Write([]byte(`{"number":1,"pull_request":{}}`))
|
||||
return
|
||||
}
|
||||
require.Equal(t, "/api/v1/repos/alice/project/issues/1/assets/1", r.URL.Path)
|
||||
mutations++
|
||||
if r.Method == http.MethodPatch {
|
||||
|
||||
@@ -445,6 +445,6 @@ func (a *application) issueCommand() *cobra.Command {
|
||||
}, issueRow)
|
||||
}})
|
||||
}
|
||||
group.AddCommand(a.issueAttachmentCommand(), a.commentCommand(), a.issueEditCommand())
|
||||
group.AddCommand(a.issueAttachmentCommand(issueRef), a.commentCommand(issueRef), a.issueEditCommand())
|
||||
return group
|
||||
}
|
||||
@@ -34,7 +34,7 @@ $ tea pr merge --squash --delete-branch`
|
||||
group.AddCommand(a.pullListCommand(), a.pullViewCommand())
|
||||
a.addPullMutations(group)
|
||||
group.AddCommand(a.pullEditCommand(), a.pullReadyCommand())
|
||||
group.AddCommand(a.issueAttachmentCommand(), a.commentCommand())
|
||||
group.AddCommand(a.issueAttachmentCommand(pullRef), a.commentCommand(pullRef))
|
||||
return group
|
||||
}
|
||||
|
||||
|
||||
@@ -18,12 +18,14 @@ type Comment struct {
|
||||
Author string `json:"author"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
// Pull reports a comment on a pull request rather than an issue.
|
||||
Pull bool `json:"-"`
|
||||
}
|
||||
|
||||
func commentFrom(comment *gitea.Comment) Comment {
|
||||
return Comment{
|
||||
ID: comment.ID, Body: comment.Body, URL: comment.HTMLURL, Author: login(comment.Poster),
|
||||
CreatedAt: timestamp(comment.Created), UpdatedAt: timestamp(comment.Updated),
|
||||
CreatedAt: timestamp(comment.Created), UpdatedAt: timestamp(comment.Updated), Pull: comment.PRURL != "",
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ type Issue struct {
|
||||
Ref string `json:"ref"`
|
||||
ID int64 `json:"id"`
|
||||
Number int64 `json:"number"`
|
||||
Kind string `json:"kind"`
|
||||
Title string `json:"title"`
|
||||
Body string `json:"body"`
|
||||
URL string `json:"url"`
|
||||
@@ -32,6 +33,22 @@ type Issue struct {
|
||||
Comments int64 `json:"comments"`
|
||||
}
|
||||
|
||||
// Kinds of Issue: Gitea lists pull requests among issues.
|
||||
const (
|
||||
KindIssue = "issue"
|
||||
KindPull = "pull"
|
||||
)
|
||||
|
||||
// discussionFrom converts an issue or a pull request listed among issues.
|
||||
func discussionFrom(issue *gitea.Issue) Issue {
|
||||
item := issueFrom(issue)
|
||||
item.Kind = KindIssue
|
||||
if issue.PullRequest != nil {
|
||||
item.Kind = KindPull
|
||||
}
|
||||
return item
|
||||
}
|
||||
|
||||
// IssueMilestone identifies the milestone associated with an issue or pull request.
|
||||
type IssueMilestone struct {
|
||||
ID int64 `json:"id"`
|
||||
@@ -99,7 +116,7 @@ func (s Issues) List(ctx context.Context, opts ListOptions) ([]Issue, bool, erro
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
return convertAll(issues, issueFrom), more, nil
|
||||
return convertAll(issues, discussionFrom), more, nil
|
||||
}
|
||||
|
||||
// filterValues splits a comma-separated filter, which the SDK joins again.
|
||||
@@ -112,14 +129,14 @@ func filterValues(value string) []string {
|
||||
|
||||
// View retrieves one issue.
|
||||
func (s Issues) View(ctx context.Context, number int64) (Issue, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, issueFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, discussionFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return c.Issues.GetIssue(ctx, owner, repo, number)
|
||||
})
|
||||
}
|
||||
|
||||
// Create opens an issue.
|
||||
func (s Issues) Create(ctx context.Context, options IssueOptions) (Issue, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, issueFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, discussionFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return c.Issues.CreateIssue(ctx, owner, repo, gitea.CreateIssueOption{
|
||||
Title: options.Title, Body: options.Body, Ref: options.Ref, Assignees: options.Assignees,
|
||||
Deadline: options.Deadline, Milestone: options.Milestone, Labels: options.Labels,
|
||||
@@ -134,7 +151,7 @@ func (s Issues) SetState(ctx context.Context, number int64, state string) (Issue
|
||||
}
|
||||
|
||||
func (s Issues) edit(ctx context.Context, number int64, opts gitea.EditIssueOption) (Issue, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, issueFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return repoConvert(ctx, s.Client, s.Repository, discussionFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Issue, *gitea.Response, error) {
|
||||
return c.Issues.EditIssue(ctx, owner, repo, number, opts)
|
||||
})
|
||||
}
|
||||
@@ -27,7 +27,7 @@ func TestIssueListSendsTypePageSizeAndFilters(t *testing.T) {
|
||||
items, more, err := service.List(context.Background(), ListOptions{State: "open", Limit: 1, Page: 3})
|
||||
require.NoError(t, err)
|
||||
require.False(t, more)
|
||||
require.Equal(t, []Issue{{Author: "bob", ID: 1, Number: 2, Title: "Bug", Labels: []Label{}, Assignees: []string{}}}, items)
|
||||
require.Equal(t, []Issue{{Author: "bob", ID: 1, Number: 2, Kind: KindIssue, Title: "Bug", Labels: []Label{}, Assignees: []string{}}}, items)
|
||||
since := time.Date(2026, 10, 1, 8, 0, 0, 0, time.UTC)
|
||||
_, _, err = service.List(context.Background(), ListOptions{
|
||||
Kind: "all", Limit: 30, Page: 1, Search: "crash", Assignee: "carol", Labels: "bug,ui", Milestones: "v1",
|
||||
|
||||
Reference in new issue
Block a user