From c2e0b0da18e5f1216c4e2c3d5eb03b9bfb218951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E7=84=B1?= <1877685089@qq.com> Date: Tue, 2 Jun 2026 08:20:40 +0800 Subject: [PATCH] feat(pr): add comments CRUD, commits, reopen, update shortcuts --- shortcuts/pr/pr.go | 195 ++++++++++++++++++++++++++++ shortcuts/pr/pr_test.go | 277 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 472 insertions(+) diff --git a/shortcuts/pr/pr.go b/shortcuts/pr/pr.go index 7f9984c..44486d8 100644 --- a/shortcuts/pr/pr.go +++ b/shortcuts/pr/pr.go @@ -306,6 +306,201 @@ func Shortcuts() []*common.Shortcut { return ctx.Output(env) }, }, + { + Name: "comments", + Description: "List review comments on a pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + env, err := ctx.CallAPI("GET", prV1Path(ctx, id)+"/journals", nil) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "create-comment", + Description: "Create a review comment on a pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "body", Short: "b", Usage: "Comment body", Required: true}, + {Name: "line", Short: "l", Usage: "Line number"}, + {Name: "path", Short: "p", Usage: "File path"}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + body, err := ctx.RequireArg("body") + if err != nil { + return err + } + payload := map[string]interface{}{ + "notes": body, + } + if line := ctx.Arg("line"); line != "" { + payload["line"] = line + } + if path := ctx.Arg("path"); path != "" { + payload["path"] = path + } + env, err := ctx.CallAPI("POST", prV1Path(ctx, id)+"/journals", payload) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "update-comment", + Description: "Update a review comment on a pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "comment-id", Short: "c", Usage: "Comment ID", Required: true}, + {Name: "body", Short: "b", Usage: "New comment body", Required: true}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + commentID, err := ctx.RequireArg("comment-id") + if err != nil { + return err + } + body, err := ctx.RequireArg("body") + if err != nil { + return err + } + payload := map[string]interface{}{ + "notes": body, + } + env, err := ctx.CallAPI("PUT", fmt.Sprintf("%s/journals/%s", prV1Path(ctx, id), commentID), payload) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "delete-comment", + Description: "Delete a review comment on a pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "comment-id", Short: "c", Usage: "Comment ID", Required: true}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + commentID, err := ctx.RequireArg("comment-id") + if err != nil { + return err + } + env, err := ctx.CallAPI("DELETE", fmt.Sprintf("%s/journals/%s", prV1Path(ctx, id), commentID), nil) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "commits", + Description: "List commits in a pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s/commits", ctx.RepoPath(), id), nil) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "reopen", + Description: "Reopen a closed pull request", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + env, err := ctx.CallAPI("POST", prV1Path(ctx, id)+"/reopen", nil) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, + { + Name: "update", + Description: "Update pull request title and/or description", + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "title", Short: "t", Usage: "New title"}, + {Name: "body", Short: "b", Usage: "New description"}, + }, + Run: func(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + id, err := ctx.RequireArg("id") + if err != nil { + return err + } + title := ctx.Arg("title") + body := ctx.Arg("body") + if title == "" && body == "" { + return fmt.Errorf("at least one of --title or --body is required") + } + payload := map[string]interface{}{} + if title != "" { + payload["title"] = title + } + if body != "" { + payload["body"] = body + } + env, err := ctx.CallAPI("PUT", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), payload) + if err != nil { + return err + } + return ctx.Output(env) + }, + }, { Name: "comment", Description: "Add a comment to a pull request", diff --git a/shortcuts/pr/pr_test.go b/shortcuts/pr/pr_test.go index db58cb6..2a66b7e 100644 --- a/shortcuts/pr/pr_test.go +++ b/shortcuts/pr/pr_test.go @@ -263,6 +263,283 @@ func TestPRReviewRejectsInvalidStatus(t *testing.T) { } } +// --- Task 2: PR Comments CRUD, Commits, Reopen, Update tests --- + +func TestPRCommentsListsReviewComments(t *testing.T) { + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/pulls/13/journals.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + writeJSON(t, w, map[string]interface{}{ + "total_count": float64(1), + "journals": []map[string]interface{}{ + { + "id": float64(501), + "notes": "Looks good here", + "line": float64(42), + "path": "main.go", + }, + }, + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "comments", map[string]string{ + "id": "13", + }) + if err != nil { + t.Fatalf("comments shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/v1/owner/repo/pulls/13/journals.json") +} + +func TestPRCreateCommentPostsJournal(t *testing.T) { + var journalPayload map[string]interface{} + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" || r.URL.Path != "/v1/owner/repo/pulls/13/journals.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + journalPayload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{ + "id": float64(502), + "notes": "nit: use camelCase", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "create-comment", map[string]string{ + "id": "13", + "body": "nit: use camelCase", + "line": "42", + "path": "main.go", + }) + if err != nil { + t.Fatalf("create-comment shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/v1/owner/repo/pulls/13/journals.json") + assertEqual(t, journalPayload["notes"], "nit: use camelCase") + assertEqual(t, journalPayload["line"], float64(42)) + assertEqual(t, journalPayload["path"], "main.go") +} + +func TestPRCreateCommentRequiresBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("server should not be called when body is missing: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + + err := runPRShortcut(t, server, "create-comment", map[string]string{ + "id": "13", + }) + if err == nil { + t.Fatal("expected error when body is missing, got nil") + } +} + +func TestPRUpdateCommentPutsJournal(t *testing.T) { + var journalPayload map[string]interface{} + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" || r.URL.Path != "/v1/owner/repo/pulls/13/journals/501.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + journalPayload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{ + "id": float64(501), + "notes": "updated comment text", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "update-comment", map[string]string{ + "id": "13", + "comment-id": "501", + "body": "updated comment text", + }) + if err != nil { + t.Fatalf("update-comment shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/v1/owner/repo/pulls/13/journals/501.json") + assertEqual(t, journalPayload["notes"], "updated comment text") +} + +func TestPRUpdateCommentRequiresCommentID(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("server should not be called when comment-id is missing: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + + err := runPRShortcut(t, server, "update-comment", map[string]string{ + "id": "13", + "body": "some text", + }) + if err == nil { + t.Fatal("expected error when comment-id is missing, got nil") + } +} + +func TestPRDeleteCommentDeletesJournal(t *testing.T) { + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" || r.URL.Path != "/v1/owner/repo/pulls/13/journals/501.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + writeJSON(t, w, map[string]interface{}{ + "status": 0, + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "delete-comment", map[string]string{ + "id": "13", + "comment-id": "501", + }) + if err != nil { + t.Fatalf("delete-comment shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/v1/owner/repo/pulls/13/journals/501.json") +} + +func TestPRDeleteCommentRequiresCommentID(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("server should not be called when comment-id is missing: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + + err := runPRShortcut(t, server, "delete-comment", map[string]string{ + "id": "13", + }) + if err == nil { + t.Fatal("expected error when comment-id is missing, got nil") + } +} + +func TestPRCommitsListsCommits(t *testing.T) { + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/owner/repo/pulls/13/commits.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + writeJSON(t, w, map[string]interface{}{ + "total_count": float64(2), + "commits": []map[string]interface{}{ + { + "sha": "abc123", + "author": "dev1", + }, + { + "sha": "def456", + "author": "dev2", + }, + }, + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "commits", map[string]string{ + "id": "13", + }) + if err != nil { + t.Fatalf("commits shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/owner/repo/pulls/13/commits.json") +} + +func TestPRReopenReopensPR(t *testing.T) { + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" || r.URL.Path != "/v1/owner/repo/pulls/13/reopen.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + writeJSON(t, w, map[string]interface{}{ + "id": float64(13), + "status": "open", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "reopen", map[string]string{ + "id": "13", + }) + if err != nil { + t.Fatalf("reopen shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/v1/owner/repo/pulls/13/reopen.json") +} + +func TestPRUpdateUpdatesTitle(t *testing.T) { + var updatePayload map[string]interface{} + var calledPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" || r.URL.Path != "/owner/repo/pulls/13.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + calledPath = r.URL.Path + updatePayload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{ + "id": float64(13), + "title": "new title", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "update", map[string]string{ + "id": "13", + "title": "new title", + }) + if err != nil { + t.Fatalf("update shortcut failed: %v", err) + } + assertEqual(t, calledPath, "/owner/repo/pulls/13.json") + assertEqual(t, updatePayload["title"], "new title") +} + +func TestPRUpdateUpdatesBody(t *testing.T) { + var updatePayload map[string]interface{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" || r.URL.Path != "/owner/repo/pulls/13.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + updatePayload = decodeJSON(t, r) + writeJSON(t, w, map[string]interface{}{ + "id": float64(13), + "body": "new body", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "update", map[string]string{ + "id": "13", + "body": "new body", + }) + if err != nil { + t.Fatalf("update shortcut failed: %v", err) + } + assertEqual(t, updatePayload["body"], "new body") +} + +func TestPRUpdateRequiresTitleOrBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("server should not be called when both title and body are missing: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + + err := runPRShortcut(t, server, "update", map[string]string{ + "id": "13", + }) + if err == nil { + t.Fatal("expected error when both title and body are missing, got nil") + } +} + func runPRShortcut(t *testing.T, server *httptest.Server, name string, args map[string]string) error { t.Helper() shortcut := findPRShortcut(t, name)