diff --git a/README.md b/README.md index e5e4318..0f6856b 100644 --- a/README.md +++ b/README.md @@ -356,6 +356,19 @@ gitlink-cli issue +batch-delete --owner Gitlink --repo forgeplus --ids 101,102 - # Add a comment gitlink-cli issue +comment --owner Gitlink --repo forgeplus -i 123 -b "Fixed" +# Reply to a comment with attachments and mentions +gitlink-cli issue +comment --owner Gitlink --repo forgeplus --number 123 -b "Thanks, please check the log" --parent-id 456 --reply-id 456 --attachment-ids 7,8 --receivers alice,bob + +# List comments only, or include operation records with --category all +gitlink-cli issue +comments --owner Gitlink --repo forgeplus --number 123 --category comment --keyword fixed + +# Update or delete a comment +gitlink-cli issue +comment-update --owner Gitlink --repo forgeplus --number 123 --comment-id 456 -b "Updated comment" +gitlink-cli issue +comment-delete --owner Gitlink --repo forgeplus --number 123 --comment-id 456 + +# List replies under a comment +gitlink-cli issue +comment-replies --owner Gitlink --repo forgeplus --number 123 --comment-id 456 + # List issue assigners gitlink-cli issue +assigners --owner Gitlink --repo forgeplus diff --git a/README.zh-CN.md b/README.zh-CN.md index 6a8879d..ca1d479 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -367,6 +367,19 @@ gitlink-cli issue +batch-delete --owner Gitlink --repo forgeplus --ids 101,102 - # 添加评论 gitlink-cli issue +comment --owner Gitlink --repo forgeplus -i 123 -b "已修复" +# 回复评论并携带附件和 @ 用户 +gitlink-cli issue +comment --owner Gitlink --repo forgeplus --number 123 -b "请查看日志" --parent-id 456 --reply-id 456 --attachment-ids 7,8 --receivers alice,bob + +# 列出评论；需要操作记录时可传 --category all +gitlink-cli issue +comments --owner Gitlink --repo forgeplus --number 123 --category comment --keyword fixed + +# 更新或删除评论 +gitlink-cli issue +comment-update --owner Gitlink --repo forgeplus --number 123 --comment-id 456 -b "更新后的评论" +gitlink-cli issue +comment-delete --owner Gitlink --repo forgeplus --number 123 --comment-id 456 + +# 列出评论下的回复 +gitlink-cli issue +comment-replies --owner Gitlink --repo forgeplus --number 123 --comment-id 456 + # 列出 Issue 负责人 gitlink-cli issue +assigners --owner Gitlink --repo forgeplus diff --git a/doc/changes/issue-comment-management.md b/doc/changes/issue-comment-management.md new file mode 100644 index 0000000..19db35d --- /dev/null +++ b/doc/changes/issue-comment-management.md @@ -0,0 +1,23 @@ +# Issue comment management shortcuts + +This change expands issue comment support from create-only to a full comment +management workflow. + +- `issue +comment` now supports threaded replies through `--parent-id` and + `--reply-id`, attachment IDs, and mentioned users. +- `issue +comments` lists comments and operation records with category, + keyword, sorting, and pagination filters. +- `issue +comment-update` and `issue +comment-delete` edit or remove existing + issue comments. +- `issue +comment-replies` lists child comments for threaded conversations. + +The implementation keeps the existing `issue +comment -b` behavior compatible +and adds validation for numeric comment, parent, reply, and attachment IDs +before any API request is sent. + +Verification: + +- `go test ./shortcuts/issue` +- `go test ./shortcuts` +- `go build ./...` +- `git diff --check` diff --git a/shortcuts/issue/issue.go b/shortcuts/issue/issue.go index b19027e..98e3450 100644 --- a/shortcuts/issue/issue.go +++ b/shortcuts/issue/issue.go @@ -274,28 +274,55 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Description: tr.T("cmd.issue.comment.short"), Flags: appendIssueNumberFlags( common.Flag{Name: "body", Short: "b", Usage: tr.T("flag.comment.body"), Required: true}, + common.Flag{Name: "parent-id", Usage: "Parent comment ID for a threaded reply"}, + common.Flag{Name: "reply-id", Usage: "Comment ID being replied to"}, + common.Flag{Name: "attachment-ids", Usage: "Comma-separated attachment IDs"}, + common.Flag{Name: "receivers", Usage: "Comma-separated user logins to mention"}, ), - Run: func(ctx *common.RuntimeContext) error { - if err := ctx.ResolveOwnerRepo(); err != nil { - return err - } - number, err := issueNumberArg(ctx) - 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("POST", fmt.Sprintf("%s/issues/%s/journals", v1RepoPath(ctx), number), payload) - if err != nil { - return err - } - return ctx.Output(env) - }, + Run: runIssueComment, + }, + { + Name: "comments", + Description: "List issue comments and operation records", + Flags: appendIssueNumberFlags( + common.Flag{Name: "category", Short: "c", Usage: "Filter by all, comment, or operate", Default: "comment"}, + common.Flag{Name: "keyword", Short: "k", Usage: "Search comment content"}, + common.Flag{Name: "sort-by", Usage: "Sort field: created_on or updated_on"}, + common.Flag{Name: "sort-direction", Usage: "Sort direction: asc or desc"}, + common.Flag{Name: "page", Short: "p", Usage: "Page number", Default: "1"}, + common.Flag{Name: "limit", Short: "l", Usage: "Items per page", Default: "20"}, + ), + Run: runIssueComments, + }, + { + Name: "comment-update", + Description: "Update an issue comment", + Flags: appendIssueNumberFlags( + common.Flag{Name: "comment-id", Usage: "Comment ID", Required: true}, + common.Flag{Name: "body", Short: "b", Usage: tr.T("flag.comment.body"), Required: true}, + common.Flag{Name: "attachment-ids", Usage: "Comma-separated attachment IDs"}, + common.Flag{Name: "receivers", Usage: "Comma-separated user logins to mention"}, + ), + Run: runIssueCommentUpdate, + }, + { + Name: "comment-delete", + Description: "Delete an issue comment", + Flags: appendIssueNumberFlags( + common.Flag{Name: "comment-id", Usage: "Comment ID", Required: true}, + ), + Run: runIssueCommentDelete, + }, + { + Name: "comment-replies", + Description: "List replies under an issue comment", + Flags: appendIssueNumberFlags( + common.Flag{Name: "comment-id", Usage: "Parent comment ID", Required: true}, + common.Flag{Name: "keyword", Short: "k", Usage: "Search reply content"}, + common.Flag{Name: "page", Short: "p", Usage: "Page number", Default: "1"}, + common.Flag{Name: "limit", Short: "l", Usage: "Items per page", Default: "20"}, + ), + Run: runIssueCommentReplies, }, { Name: "assigners", @@ -445,6 +472,183 @@ func issueNumberArg(ctx *common.RuntimeContext) (string, error) { return "", fmt.Errorf("required flag --number is missing (or use --id as a compatibility alias)") } +func runIssueComment(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + number, err := issueNumberArg(ctx) + if err != nil { + return err + } + body, err := ctx.RequireArg("body") + if err != nil { + return err + } + payload, err := issueCommentPayload(ctx, body, true) + if err != nil { + return err + } + env, err := ctx.CallAPI("POST", issueJournalPath(ctx, number), payload) + if err != nil { + return err + } + return ctx.Output(env) +} + +func runIssueComments(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + number, err := issueNumberArg(ctx) + if err != nil { + return err + } + q := url.Values{} + setIssueQueryIfPresent(q, "category", ctx.Arg("category")) + setIssueQueryIfPresent(q, "keyword", ctx.Arg("keyword")) + setIssueQueryIfPresent(q, "sort_by", ctx.Arg("sort-by")) + setIssueQueryIfPresent(q, "sort_direction", ctx.Arg("sort-direction")) + setIssueQueryIfPresent(q, "page", ctx.Arg("page")) + setIssueQueryIfPresent(q, "limit", ctx.Arg("limit")) + env, err := ctx.CallAPIWithQuery("GET", issueJournalPath(ctx, number), q) + if err != nil { + return err + } + return ctx.Output(env) +} + +func runIssueCommentUpdate(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + number, commentID, err := issueCommentTarget(ctx) + if err != nil { + return err + } + body, err := ctx.RequireArg("body") + if err != nil { + return err + } + payload, err := issueCommentPayload(ctx, body, false) + if err != nil { + return err + } + env, err := ctx.CallAPI("PATCH", issueJournalItemPath(ctx, number, commentID), payload) + if err != nil { + return err + } + return ctx.Output(env) +} + +func runIssueCommentDelete(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + number, commentID, err := issueCommentTarget(ctx) + if err != nil { + return err + } + env, err := ctx.CallAPI("DELETE", issueJournalItemPath(ctx, number, commentID), nil) + if err != nil { + return err + } + return ctx.Output(env) +} + +func runIssueCommentReplies(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + number, commentID, err := issueCommentTarget(ctx) + if err != nil { + return err + } + q := url.Values{} + setIssueQueryIfPresent(q, "keyword", ctx.Arg("keyword")) + setIssueQueryIfPresent(q, "page", ctx.Arg("page")) + setIssueQueryIfPresent(q, "limit", ctx.Arg("limit")) + env, err := ctx.CallAPIWithQuery("GET", issueJournalItemPath(ctx, number, commentID)+"/children_journals", q) + if err != nil { + return err + } + return ctx.Output(env) +} + +func issueJournalPath(ctx *common.RuntimeContext, number string) string { + return fmt.Sprintf("%s/issues/%s/journals", v1RepoPath(ctx), url.PathEscape(number)) +} + +func issueJournalItemPath(ctx *common.RuntimeContext, number, commentID string) string { + return fmt.Sprintf("%s/%s", issueJournalPath(ctx, number), url.PathEscape(commentID)) +} + +func issueCommentTarget(ctx *common.RuntimeContext) (string, string, error) { + number, err := issueNumberArg(ctx) + if err != nil { + return "", "", err + } + commentID, err := ctx.RequireArg("comment-id") + if err != nil { + return "", "", err + } + if _, err := parseIssueID(commentID, "comment-id"); err != nil { + return "", "", err + } + return number, strings.TrimSpace(commentID), nil +} + +func issueCommentPayload(ctx *common.RuntimeContext, body string, includeThreading bool) (map[string]interface{}, error) { + payload := map[string]interface{}{"notes": body} + if includeThreading { + if parentID := ctx.Arg("parent-id"); parentID != "" { + id, err := parseIssueID(parentID, "parent-id") + if err != nil { + return nil, err + } + payload["parent_id"] = id + } + if replyID := ctx.Arg("reply-id"); replyID != "" { + id, err := parseIssueID(replyID, "reply-id") + if err != nil { + return nil, err + } + payload["reply_id"] = id + } + } + if attachmentIDs := ctx.Arg("attachment-ids"); attachmentIDs != "" { + ids, err := parseIssueIDList(attachmentIDs, "attachment-ids") + if err != nil { + return nil, err + } + payload["attachment_ids"] = ids + } + if receivers := parseIssueStringList(ctx.Arg("receivers")); len(receivers) > 0 { + payload["receivers_login"] = receivers + } + return payload, nil +} + +func setIssueQueryIfPresent(q url.Values, name, value string) { + if strings.TrimSpace(value) != "" { + q.Set(name, strings.TrimSpace(value)) + } +} + +func parseIssueStringList(value string) []string { + parts := strings.Split(value, ",") + result := make([]string, 0, len(parts)) + seen := map[string]bool{} + for _, part := range parts { + item := strings.TrimSpace(part) + if item == "" || seen[item] { + continue + } + seen[item] = true + result = append(result, item) + } + return result +} + // normalizeIssueListIDs adds "number" (project_issues_index) and renames // "id" to "database_id" so the user-facing output uses the project-level // issue number, not the global database primary key. diff --git a/shortcuts/issue/issue_test.go b/shortcuts/issue/issue_test.go index 48be057..2c92c36 100644 --- a/shortcuts/issue/issue_test.go +++ b/shortcuts/issue/issue_test.go @@ -94,6 +94,58 @@ func assertNumberSlice(t *testing.T, got interface{}, want []float64) { } } +func assertStringSliceEqual(t *testing.T, got, want []string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("got %v, want %v", got, want) + } + } +} + +func assertNumberSliceEqual(t *testing.T, got, want []int) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("got %v, want %v", got, want) + } + } +} + +func interfaceSliceToStrings(value interface{}) []string { + items, ok := value.([]interface{}) + if !ok { + return nil + } + out := make([]string, 0, len(items)) + for _, item := range items { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out +} + +func interfaceSliceToInts(value interface{}) []int { + items, ok := value.([]interface{}) + if !ok { + return nil + } + out := make([]int, 0, len(items)) + for _, item := range items { + if n, ok := item.(float64); ok { + out = append(out, int(n)) + } + } + return out +} + // --- list --- func TestIssueList(t *testing.T) { @@ -717,6 +769,136 @@ func TestIssueCommentAcceptsIDAlias(t *testing.T) { assertEqual(t, commentPayload["notes"], "Fixed") } +func TestIssueCommentSupportsThreadingAttachmentsAndReceivers(t *testing.T) { + var commentPayload map[string]interface{} + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" || r.URL.Path != "/v1/owner/repo/issues/42/journals.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + commentPayload = decodeJSON(t, r) + writeJSON(t, w, commentPayload) + }) + defer server.Close() + + err := runShortcut(t, server, "comment", map[string]string{ + "number": "42", + "body": "Reply with context", + "parent-id": "10", + "reply-id": "11", + "attachment-ids": "5, 6", + "receivers": "alice, bob, alice", + }) + if err != nil { + t.Fatalf("comment shortcut failed: %v", err) + } + assertEqual(t, commentPayload["notes"], "Reply with context") + assertEqual(t, commentPayload["parent_id"], float64(10)) + assertEqual(t, commentPayload["reply_id"], float64(11)) + assertStringSliceEqual(t, interfaceSliceToStrings(commentPayload["receivers_login"]), []string{"alice", "bob"}) + assertNumberSliceEqual(t, interfaceSliceToInts(commentPayload["attachment_ids"]), []int{5, 6}) +} + +func TestIssueCommentsListSendsFilters(t *testing.T) { + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42/journals.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + q := r.URL.Query() + assertEqual(t, q.Get("category"), "all") + assertEqual(t, q.Get("keyword"), "panic") + assertEqual(t, q.Get("sort_by"), "updated_on") + assertEqual(t, q.Get("sort_direction"), "desc") + assertEqual(t, q.Get("page"), "2") + assertEqual(t, q.Get("limit"), "50") + writeJSON(t, w, map[string]interface{}{ + "total_count": float64(1), + "journals": []interface{}{ + map[string]interface{}{"id": float64(7), "notes": "panic fixed"}, + }, + }) + }) + defer server.Close() + + err := runShortcut(t, server, "comments", map[string]string{ + "number": "42", + "category": "all", + "keyword": "panic", + "sort-by": "updated_on", + "sort-direction": "desc", + "page": "2", + "limit": "50", + }) + if err != nil { + t.Fatalf("comments shortcut failed: %v", err) + } +} + +func TestIssueCommentUpdate(t *testing.T) { + var commentPayload map[string]interface{} + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PATCH" || r.URL.Path != "/v1/owner/repo/issues/42/journals/9.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + commentPayload = decodeJSON(t, r) + writeJSON(t, w, commentPayload) + }) + defer server.Close() + + err := runShortcut(t, server, "comment-update", map[string]string{ + "number": "42", + "comment-id": "9", + "body": "Updated", + "attachment-ids": "8", + "receivers": "alice", + }) + if err != nil { + t.Fatalf("comment-update failed: %v", err) + } + assertEqual(t, commentPayload["notes"], "Updated") + assertNumberSliceEqual(t, interfaceSliceToInts(commentPayload["attachment_ids"]), []int{8}) + assertStringSliceEqual(t, interfaceSliceToStrings(commentPayload["receivers_login"]), []string{"alice"}) +} + +func TestIssueCommentDelete(t *testing.T) { + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" || r.URL.Path != "/v1/owner/repo/issues/42/journals/9.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{"status": float64(0), "message": "success"}) + }) + defer server.Close() + + err := runShortcut(t, server, "comment-delete", map[string]string{"number": "42", "comment-id": "9"}) + if err != nil { + t.Fatalf("comment-delete failed: %v", err) + } +} + +func TestIssueCommentReplies(t *testing.T) { + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42/journals/9/children_journals.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + q := r.URL.Query() + assertEqual(t, q.Get("keyword"), "thanks") + assertEqual(t, q.Get("page"), "3") + assertEqual(t, q.Get("limit"), "10") + writeJSON(t, w, map[string]interface{}{"total_count": float64(0), "journals": []interface{}{}}) + }) + defer server.Close() + + err := runShortcut(t, server, "comment-replies", map[string]string{ + "number": "42", + "comment-id": "9", + "keyword": "thanks", + "page": "3", + "limit": "10", + }) + if err != nil { + t.Fatalf("comment-replies failed: %v", err) + } +} + func TestIssueCommentMissingBody(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { t.Fatal("no API call expected") @@ -743,6 +925,10 @@ func TestIssueNumberOrIDIsRequired(t *testing.T) { {name: "close", args: map[string]string{}}, {name: "update", args: map[string]string{"title": "New title"}}, {name: "comment", args: map[string]string{"body": "Fixed"}}, + {name: "comments", args: map[string]string{}}, + {name: "comment-update", args: map[string]string{"comment-id": "9", "body": "Updated"}}, + {name: "comment-delete", args: map[string]string{"comment-id": "9"}}, + {name: "comment-replies", args: map[string]string{"comment-id": "9"}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -757,6 +943,32 @@ func TestIssueNumberOrIDIsRequired(t *testing.T) { } } +func TestIssueCommentRejectsInvalidIDs(t *testing.T) { + server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("invalid IDs should not call API, got %s %s", r.Method, r.URL.Path) + }) + defer server.Close() + + cases := []struct { + name string + cmd string + args map[string]string + }{ + {name: "bad parent", cmd: "comment", args: map[string]string{"number": "42", "body": "x", "parent-id": "abc"}}, + {name: "bad attachment", cmd: "comment", args: map[string]string{"number": "42", "body": "x", "attachment-ids": "1,,"}}, + {name: "bad update comment", cmd: "comment-update", args: map[string]string{"number": "42", "comment-id": "0", "body": "x"}}, + {name: "bad delete comment", cmd: "comment-delete", args: map[string]string{"number": "42", "comment-id": "-1"}}, + {name: "bad replies comment", cmd: "comment-replies", args: map[string]string{"number": "42", "comment-id": "abc"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if err := runShortcut(t, server, tc.cmd, tc.args); err == nil { + t.Fatal("expected validation error") + } + }) + } +} + // --- batch-close --- func TestBatchClosePreservesCurrentDescription(t *testing.T) {