diff --git a/README.md b/README.md index c0109ed..6db9d89 100644 --- a/README.md +++ b/README.md @@ -315,6 +315,19 @@ gitlink-cli issue +batch-close --owner Gitlink --repo forgeplus --from issues.cs # 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 6e006f4..6ee0a8d 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -326,6 +326,19 @@ gitlink-cli issue +batch-close --owner Gitlink --repo forgeplus --from issues.cs # 添加评论 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 a1e3f0f..2dd48c3 100644 --- a/shortcuts/issue/issue.go +++ b/shortcuts/issue/issue.go @@ -300,28 +300,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: "comments", @@ -559,6 +586,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 0ab7447..2c92c36 100644 --- a/shortcuts/issue/issue_test.go +++ b/shortcuts/issue/issue_test.go @@ -80,26 +80,8 @@ func assertEqual(t *testing.T, got interface{}, want interface{}) { func assertNumberSlice(t *testing.T, got interface{}, want []float64) { t.Helper() - var values []float64 - switch typed := got.(type) { - case []interface{}: - values = make([]float64, 0, len(typed)) - for _, value := range typed { - switch number := value.(type) { - case float64: - values = append(values, number) - case int: - values = append(values, float64(number)) - default: - t.Fatalf("got %v (%T), want numeric slice", got, got) - } - } - case []int: - values = make([]float64, 0, len(typed)) - for _, value := range typed { - values = append(values, float64(value)) - } - default: + values, ok := got.([]interface{}) + if !ok { t.Fatalf("got %v (%T), want numeric slice", got, got) } if len(values) != len(want) { @@ -112,46 +94,56 @@ func assertNumberSlice(t *testing.T, got interface{}, want []float64) { } } -func issueLegacyPath(number string) string { - return "/owner/repo/issues/" + number + ".json" -} - -func issueLegacyEditPath(number string) string { - return "/owner/repo/issues/" + number + "/edit.json" -} - -func writeLegacyIssueDetail(t *testing.T, w http.ResponseWriter, number string) { +func assertStringSliceEqual(t *testing.T, got, want []string) { t.Helper() - writeJSON(t, w, map[string]interface{}{ - "id": 9001, - "project_issues_index": 42, - "tracker": map[string]interface{}{"id": 11, "name": "Bug"}, - "priority": map[string]interface{}{"id": 2, "name": "Normal"}, - "issue_status": map[string]interface{}{"id": 1, "name": "Open"}, - "issue_tags": []map[string]interface{}{ - {"id": 7, "name": "backend"}, - }, - "version_id": 13, - "branch_name": "main", - "start_date": "2026-05-01", - "due_date": "2026-05-31", - }) + 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 writeLegacyIssueEdit(t *testing.T, w http.ResponseWriter) { +func assertNumberSliceEqual(t *testing.T, got, want []int) { t.Helper() - writeJSON(t, w, map[string]interface{}{ - "status_id": 1, - "priority_id": 2, - "tracker_id": 11, - "issue_type": "bug", - "issue_tags": []interface{}{7, 8}, - "assigned_to_id": 9, - "fixed_version_id": 13, - "branch_name": "main", - "start_date": "2026-05-01", - "due_date": "2026-05-31", - }) + 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 --- @@ -311,21 +303,13 @@ func TestIssueCreateMissingTitle(t *testing.T) { func TestIssueView(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": - writeJSON(t, w, map[string]interface{}{ - "id": float64(42), - "project_issues_index": float64(42), - "subject": "bug", - "status": nil, - }) - case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): - writeLegacyIssueDetail(t, w, "42") - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) - default: - t.Fatalf("unexpected path: %s %s", r.Method, r.URL.Path) + if r.Method != "GET" { + t.Fatalf("expected GET, got %s", r.Method) } + if r.URL.Path != "/v1/owner/repo/issues/42.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{"id": float64(42), "subject": "bug"}) }) defer server.Close() @@ -350,20 +334,14 @@ func TestIssueViewMissingNumber(t *testing.T) { func TestIssueViewAcceptsIDAlias(t *testing.T) { var requestedPath string server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": - requestedPath = r.URL.Path - writeJSON(t, w, map[string]interface{}{ - "project_issues_index": 42, - "subject": "Issue from web URL", - }) - case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): - writeLegacyIssueDetail(t, w, "42") - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) - default: + requestedPath = r.URL.Path + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42.json" { t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } + writeJSON(t, w, map[string]interface{}{ + "project_issues_index": 42, + "subject": "Issue from web URL", + }) }) defer server.Close() @@ -376,16 +354,10 @@ func TestIssueViewAcceptsIDAlias(t *testing.T) { func TestIssueNumberTakesPrecedenceOverIDAlias(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": - writeJSON(t, w, map[string]interface{}{"subject": "Existing title"}) - case r.Method == "GET" && r.URL.Path == issueLegacyPath("42"): - writeLegacyIssueDetail(t, w, "42") - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) - default: + if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/42.json" { t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } + writeJSON(t, w, map[string]interface{}{"subject": "Existing title"}) }) defer server.Close() @@ -410,8 +382,6 @@ func TestIssueClose(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -439,8 +409,6 @@ func TestIssueCloseAcceptsIDAlias(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -469,19 +437,6 @@ func TestIssueClosePreservesCurrentMetadata(t *testing.T) { {"id": 4}, }, }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeJSON(t, w, map[string]interface{}{ - "status_id": 1, - "priority_id": 3, - "tracker_id": 11, - "issue_type": "bug", - "issue_tags": []interface{}{4}, - "assigned_to_id": 9, - "fixed_version_id": 13, - "branch_name": "main", - "start_date": "2026-05-01", - "due_date": "2026-05-31", - }) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -499,10 +454,6 @@ func TestIssueClosePreservesCurrentMetadata(t *testing.T) { assertEqual(t, updatePayload["status_id"], float64(5)) assertEqual(t, updatePayload["priority_id"], float64(3)) assertNumberSlice(t, updatePayload["issue_tag_ids"], []float64{4}) - assertEqual(t, updatePayload["tracker_id"], float64(11)) - assertEqual(t, updatePayload["issue_type"], "bug") - assertEqual(t, updatePayload["assigned_to_id"], float64(9)) - assertEqual(t, updatePayload["fixed_version_id"], float64(13)) } func TestIssueCloseFetchFails(t *testing.T) { @@ -518,38 +469,6 @@ func TestIssueCloseFetchFails(t *testing.T) { } } -// --- delete --- - -func TestIssueDelete(t *testing.T) { - var deletedPath string - server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method != "DELETE" { - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - deletedPath = r.URL.Path - writeJSON(t, w, map[string]interface{}{"status": float64(0), "message": "success"}) - }) - defer server.Close() - - err := runShortcut(t, server, "delete", map[string]string{"number": "42", "yes": "true"}) - if err != nil { - t.Fatalf("delete failed: %v", err) - } - assertEqual(t, deletedPath, "/v1/owner/repo/issues/42.json") -} - -func TestIssueDeleteRequiresConfirmation(t *testing.T) { - server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - t.Fatalf("unexpected request without --yes: %s %s", r.Method, r.URL.Path) - }) - defer server.Close() - - err := runShortcut(t, server, "delete", map[string]string{"number": "42"}) - if err == nil { - t.Fatal("expected error without --yes confirmation") - } -} - // --- update --- func TestIssueUpdateTitle(t *testing.T) { @@ -562,8 +481,6 @@ func TestIssueUpdateTitle(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -592,8 +509,6 @@ func TestIssueUpdateDescription(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, patchPayload) @@ -621,8 +536,6 @@ func TestIssueUpdateNumericState(t *testing.T) { "subject": "bug", "description": "desc", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": patchPayload = decodeJSON(t, r) writeJSON(t, w, map[string]interface{}{"id": float64(42)}) @@ -648,8 +561,6 @@ func TestIssueUpdateAcceptsIDAlias(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -691,19 +602,6 @@ func TestIssueUpdatePreservesCurrentMetadata(t *testing.T) { "start_date": "2026-05-01", "due_date": "2026-05-31", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeJSON(t, w, map[string]interface{}{ - "status_id": 1, - "priority_id": 2, - "tracker_id": 11, - "issue_type": "bug", - "issue_tags": []interface{}{7, 8}, - "assigned_to_id": 9, - "fixed_version_id": 13, - "branch_name": "main", - "start_date": "2026-05-01", - "due_date": "2026-05-31", - }) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -729,10 +627,6 @@ func TestIssueUpdatePreservesCurrentMetadata(t *testing.T) { assertEqual(t, updatePayload["branch_name"], "main") assertEqual(t, updatePayload["start_date"], "2026-05-01") assertEqual(t, updatePayload["due_date"], "2026-05-31") - assertEqual(t, updatePayload["tracker_id"], float64(11)) - assertEqual(t, updatePayload["issue_type"], "bug") - assertEqual(t, updatePayload["assigned_to_id"], float64(9)) - assertEqual(t, updatePayload["fixed_version_id"], float64(13)) } func TestIssueUpdateSupportsMetadataFields(t *testing.T) { @@ -744,8 +638,6 @@ func TestIssueUpdateSupportsMetadataFields(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -772,7 +664,6 @@ func TestIssueUpdateSupportsMetadataFields(t *testing.T) { assertEqual(t, updatePayload["priority_id"], float64(4)) assertNumberSlice(t, updatePayload["issue_tag_ids"], []float64{6, 7}) assertNumberSlice(t, updatePayload["assigner_ids"], []float64{8}) - assertEqual(t, updatePayload["assigned_to_id"], float64(8)) assertEqual(t, updatePayload["branch_name"], "bugfix/metadata") assertEqual(t, updatePayload["start_date"], "2026-06-01") assertEqual(t, updatePayload["due_date"], "2026-06-15") @@ -787,8 +678,6 @@ func TestIssueUpdateInvalidState(t *testing.T) { "subject": "bug", "description": "desc", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } @@ -880,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") @@ -906,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) { @@ -920,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) { @@ -931,8 +980,6 @@ func TestBatchClosePreservesCurrentDescription(t *testing.T) { "subject": "Existing title", "description": "Existing description", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": updatePayload = decodeJSON(t, r) writeJSON(t, w, updatePayload) @@ -998,12 +1045,8 @@ func TestBatchCloseWithFailedClose(t *testing.T) { switch { case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 1", "description": "desc1"}) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): - writeLegacyIssueEdit(t, w) case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/2.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 2", "description": "desc2"}) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("2"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/1.json": writeJSON(t, w, map[string]interface{}{"subject": "Issue 1", "description": "desc1", "status_id": float64(5)}) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/2.json": @@ -1174,54 +1217,6 @@ func TestIssueViewHTTPError(t *testing.T) { } } -func TestMergeIssueViewDataAddsLegacyFields(t *testing.T) { - merged := mergeIssueViewData( - map[string]interface{}{ - "id": float64(42), - "project_issues_index": float64(18), - "subject": "Issue from v1", - "status": nil, - }, - map[string]interface{}{ - "id": float64(2048), - "project_issues_index": float64(18), - "tracker": map[string]interface{}{"id": 5, "name": "Feature"}, - "priority": map[string]interface{}{"id": 2, "name": "Normal"}, - "issue_status": map[string]interface{}{"id": 1, "name": "Open"}, - "issue_tags": []map[string]interface{}{ - {"id": 7, "name": "backend"}, - {"id": 8, "name": "urgent"}, - }, - "version_id": 13, - }, - map[string]interface{}{ - "tracker_id": 5, - "issue_type": "feature", - "assigned_to_id": 9, - "fixed_version_id": 13, - "issue_tags": []interface{}{7, 8}, - }, - ) - - assertEqual(t, merged["number"], float64(18)) - assertEqual(t, merged["database_id"], float64(42)) - assertEqual(t, merged["tracker_id"], 5) - assertEqual(t, merged["issue_type"], "feature") - assertEqual(t, merged["assigned_to_id"], 9) - assertEqual(t, merged["fixed_version_id"], 13) - assertEqual(t, merged["version_id"], 13) - assertEqual(t, merged["status_name"], "Open") - assertEqual(t, merged["priority_name"], "Normal") - assertNumberSlice(t, merged["issue_tag_ids"], []float64{7, 8}) - tagNames, ok := merged["issue_tag_names"].([]string) - if !ok { - t.Fatalf("expected issue_tag_names, got %T", merged["issue_tag_names"]) - } - if len(tagNames) != 2 || tagNames[0] != "backend" || tagNames[1] != "urgent" { - t.Fatalf("unexpected issue_tag_names: %v", tagNames) - } -} - func TestIssueCommentHTTPError(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { writeText(t, w, http.StatusInternalServerError, "server error") @@ -1241,8 +1236,6 @@ func TestIssueUpdateHTTPError(t *testing.T) { writeJSON(t, w, map[string]interface{}{ "id": float64(42), "subject": "bug", "description": "desc", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": writeText(t, w, http.StatusInternalServerError, "server error") default: @@ -1264,8 +1257,6 @@ func TestIssueCloseHTTPError(t *testing.T) { writeJSON(t, w, map[string]interface{}{ "id": float64(42), "subject": "bug", "description": "desc", }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeLegacyIssueEdit(t, w) case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": writeText(t, w, http.StatusInternalServerError, "server error") default: @@ -1282,14 +1273,7 @@ func TestIssueCloseHTTPError(t *testing.T) { func TestFetchExistingIssueBadData(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": - writeJSON(t, w, "not a map") - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): - writeLegacyIssueEdit(t, w) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } + writeJSON(t, w, "not a map") }) defer server.Close() @@ -1306,14 +1290,7 @@ func TestFetchExistingIssueBadData(t *testing.T) { func TestFetchExistingIssueNoSubject(t *testing.T) { server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": - writeJSON(t, w, map[string]interface{}{"id": float64(1)}) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): - writeLegacyIssueEdit(t, w) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } + writeJSON(t, w, map[string]interface{}{"id": float64(1)}) }) defer server.Close() @@ -1328,91 +1305,6 @@ func TestFetchExistingIssueNoSubject(t *testing.T) { } } -func TestFetchExistingIssueRequiresEditMetadata(t *testing.T) { - server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/1.json": - writeJSON(t, w, map[string]interface{}{ - "subject": "issue", - "description": "desc", - }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("1"): - writeText(t, w, http.StatusInternalServerError, "boom") - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - ctx := &common.RuntimeContext{ - Client: &client.Client{HTTP: server.Client(), BaseURL: server.URL}, - Owner: "owner", - Repo: "repo", - } - _, err := fetchExistingIssue(ctx, "1") - if err == nil || !strings.Contains(err.Error(), "edit metadata") { - t.Fatalf("expected edit metadata error, got %v", err) - } -} - -func TestIssueUpdateStopsWhenEditMetadataFails(t *testing.T) { - patchCalled := false - server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": - writeJSON(t, w, map[string]interface{}{ - "subject": "Existing title", - "description": "Existing description", - }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeText(t, w, http.StatusInternalServerError, "server error") - case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": - patchCalled = true - t.Fatal("PATCH should not be sent when edit metadata fetch fails") - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - err := runShortcut(t, server, "update", map[string]string{"number": "42", "title": "New title"}) - if err == nil { - t.Fatal("expected error when edit metadata fetch fails") - } - if patchCalled { - t.Fatal("patch should not have been called") - } -} - -func TestIssueCloseStopsWhenEditMetadataFails(t *testing.T) { - patchCalled := false - server := newIssueTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues/42.json": - writeJSON(t, w, map[string]interface{}{ - "subject": "Existing title", - "description": "Existing description", - }) - case r.Method == "GET" && r.URL.Path == issueLegacyEditPath("42"): - writeText(t, w, http.StatusInternalServerError, "server error") - case r.Method == "PATCH" && r.URL.Path == "/v1/owner/repo/issues/42.json": - patchCalled = true - t.Fatal("PATCH should not be sent when edit metadata fetch fails") - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - err := runShortcut(t, server, "close", map[string]string{"number": "42"}) - if err == nil { - t.Fatal("expected error when edit metadata fetch fails") - } - if patchCalled { - t.Fatal("patch should not have been called") - } -} - // --- normalizeIssueStatus --- func TestNormalizeIssueStatus(t *testing.T) { @@ -1447,64 +1339,3 @@ func TestNormalizeIssueStatus(t *testing.T) { } } } - -func TestIssueCommentsListsJournalsWithFilters(t *testing.T) { - var query string - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues/7/journals.json" { - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - query = r.URL.RawQuery - writeJSON(t, w, map[string]interface{}{"journals": []interface{}{}}) - })) - defer server.Close() - - args := map[string]string{"number": "7", "keyword": "lgtm", "category": "comment", "page": "1", "limit": "20"} - if err := runShortcut(t, server, "comments", args); err != nil { - t.Fatalf("comments failed: %v", err) - } - for _, want := range []string{"keyword=lgtm", "category=comment"} { - if !strings.Contains(query, want) { - t.Fatalf("expected %q in query, got %q", want, query) - } - } -} - -func TestIssueCommentEditPatchesJournal(t *testing.T) { - var payload map[string]interface{} - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "PATCH" || r.URL.Path != "/v1/owner/repo/issues/7/journals/484049.json" { - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - payload = decodeJSON(t, r) - writeJSON(t, w, map[string]interface{}{"id": float64(484049)}) - })) - defer server.Close() - - args := map[string]string{"number": "7", "comment-id": "484049", "body": "edited"} - if err := runShortcut(t, server, "comment-edit", args); err != nil { - t.Fatalf("comment-edit failed: %v", err) - } - if payload["notes"] != "edited" { - t.Fatalf("expected notes=edited, got %v", payload["notes"]) - } - - args["comment-id"] = "abc" - if err := runShortcut(t, server, "comment-edit", args); err == nil { - t.Fatal("expected error for non-integer --comment-id") - } -} - -func TestIssueCommentDeleteUsesJournalEndpoint(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != "DELETE" || r.URL.Path != "/v1/owner/repo/issues/7/journals/484049.json" { - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - writeJSON(t, w, map[string]interface{}{"status": float64(0)}) - })) - defer server.Close() - - if err := runShortcut(t, server, "comment-delete", map[string]string{"number": "7", "comment-id": "484049"}); err != nil { - t.Fatalf("comment-delete failed: %v", err) - } -}