From 8c288ad7ece42239c57eb2429d50fd6b45b5e12a Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Wed, 2 Sep 2026 08:36:54 +0200 Subject: [PATCH 1/4] fix: make Jira comment replies notify the author --- AGENTS.md | 8 +++ README.md | 7 ++- internal/cmd/issue/comment/add.go | 69 +++++++++++++++++++------- internal/cmd/issue/comment/add_test.go | 61 +++++++++++++++++++++++ 4 files changed, 125 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 95a9317..0edace3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,11 +135,19 @@ atl --context prod jira issue transition PROJ-1234 "Done" --field "Resolution=Fi atl --context prod jira issue comment list PROJ-1234 # List comments atl --context prod jira issue comment add PROJ-1234 --body "Comment" # Add comment atl --context prod jira issue comment add PROJ-1234 --body-file msg.md # Add from file (avoids shell escaping) +atl --context prod jira issue comment add PROJ-1234 --reply-to 123 --body "Reply" atl --context prod jira issue comment edit PROJ-1234 --id 123 --body "Updated" atl --context prod jira issue comment edit PROJ-1234 --id 123 --body-file msg.md atl --context prod jira issue comment delete PROJ-1234 --id 123 ``` +`--reply-to` creates a new flat Jira comment containing a real mention of the +original author and a focused-comment link. It does not quote or duplicate the +original comment. For manually authored bodies, use Markdown links +`[label](https://example.com)` and real mentions `@[Display Name]` or +`@[id:accountId]`. Jira wiki links `[label|url]` remain literal, and plain +`@Display Name` does not notify the user. + ### Issue Links ```bash diff --git a/README.md b/README.md index cf5c4d8..7358d35 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ func main() { | Strikethrough | `~~deleted~~` | | Inline code | `` `code` `` | | Code blocks | ` ``` ` with optional language | -| Links | `[text](url)` | +| Links | `[text](url)` (Jira wiki `[text|url]` is not supported) | | Bullet lists | `- item` or `* item` | | Numbered lists | `1. item` | | Blockquotes | `> quote` | @@ -231,6 +231,11 @@ atl --context prod jira issue attachment --download-all # Download atl --context prod jira issue attachment --download-all -o ./dir # Download to directory ``` +`issue comment add --reply-to` creates a new flat Jira comment, mentions +the original author, and links to the focused original comment. It does not +copy the original body into the reply. For manually authored comments, plain +`@Display Name` is only text; use `@[Display Name]` for a real Jira mention. + ### Boards ```bash diff --git a/internal/cmd/issue/comment/add.go b/internal/cmd/issue/comment/add.go index de0a6d1..9482a7b 100644 --- a/internal/cmd/issue/comment/add.go +++ b/internal/cmd/issue/comment/add.go @@ -3,7 +3,9 @@ package comment import ( "context" "fmt" + "net/url" "os" + "strings" "github.com/spf13/cobra" @@ -36,7 +38,7 @@ func NewCmdAdd(ios *iostreams.IOStreams) *cobra.Command { Long: `Add a new comment to a Jira issue. Supports visibility restrictions to limit who can see the comment, -and replying to existing comments with automatic quoting.`, +and replying to existing comments with an author mention and comment link.`, Example: ` # Add a comment atl jira issue comment add PROJ-1234 --body "This is my comment" @@ -46,7 +48,7 @@ and replying to existing comments with automatic quoting.`, # Add a comment visible only to a group atl jira issue comment add PROJ-1234 --body "Team note" --visibility-type group --visibility-name "jira-developers" - # Reply to a specific comment (quotes the original) + # Reply to a specific comment (mentions the author and links the original) atl jira issue comment add PROJ-1234 --body "I agree!" --reply-to 12345 # Read comment body from a file @@ -78,7 +80,7 @@ and replying to existing comments with automatic quoting.`, cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Comment text (mutually exclusive with --body-file)") cmd.Flags().StringVar(&opts.BodyFile, "body-file", "", "Read comment body from file (mutually exclusive with --body)") - cmd.Flags().StringVar(&opts.ReplyTo, "reply-to", "", "Comment ID to reply to (quotes original)") + cmd.Flags().StringVar(&opts.ReplyTo, "reply-to", "", "Comment ID to reply to (mentions author and links original)") cmd.Flags().StringVar(&opts.VisibilityType, "visibility-type", "", "Visibility type: 'role' or 'group'") cmd.Flags().StringVar(&opts.VisibilityName, "visibility-name", "", "Role or group name for visibility restriction") cmd.Flags().BoolVarP(&opts.JSON, "json", "j", false, "Output as JSON") @@ -142,33 +144,20 @@ func runAdd(opts *AddOptions) error { } func replyToComment(ctx context.Context, jira *api.JiraService, hostname string, opts *AddOptions) error { - // Get the original comment to quote it + // Jira issue comments are flat, so a reply is represented by a mention and + // a stable link to the original comment. originalComment, err := jira.GetComment(ctx, opts.IssueKey, opts.ReplyTo) if err != nil { return fmt.Errorf("failed to get original comment: %w", err) } - originalAuthor := "Unknown" - if originalComment.Author != nil { - originalAuthor = originalComment.Author.DisplayName - } - - // Build the reply as ADF. Quoting by copying nodes — rather than rendering - // the original to text and re-parsing it — is what keeps media, mentions and - // exact characters intact. bodyADF, err := api.TextToADFWithResolver(ctx, opts.Body, jira) if err != nil { return fmt.Errorf("failed to process mentions: %w", err) } - content := []api.ADFContent{api.AttributionParagraph(originalAuthor)} - if quote := api.QuoteADF(originalComment.Body); quote != nil { - content = append(content, *quote) - } - content = append(content, bodyADF.Content...) - commentOpts := &api.CommentOptions{ - BodyADF: &api.ADF{Type: "doc", Version: 1, Content: content}, + BodyADF: buildReplyADF(hostname, opts.IssueKey, opts.ReplyTo, originalComment.Author, bodyADF), VisibilityType: opts.VisibilityType, VisibilityName: opts.VisibilityName, } @@ -195,3 +184,45 @@ func replyToComment(ctx context.Context, jira *api.JiraService, hostname string, return nil } + +func buildReplyADF(hostname, issueKey, commentID string, author *api.User, body *api.ADF) *api.ADF { + commentURL := fmt.Sprintf( + "https://%s/browse/%s?focusedCommentId=%s", + hostname, + url.PathEscape(issueKey), + url.QueryEscape(commentID), + ) + + header := api.ADFContent{Type: "paragraph"} + if author != nil && author.AccountID != "" { + displayName := strings.TrimSpace(author.DisplayName) + if displayName == "" { + displayName = "Jira user" + } + header.Content = append(header.Content, + api.ADFContent{ + Type: "mention", + Attrs: &api.ADFAttrs{ + ID: author.AccountID, + Text: "@" + displayName, + }, + }, + api.ADFContent{Type: "text", Text: " · "}, + ) + } + header.Content = append(header.Content, api.ADFContent{ + Type: "text", + Text: fmt.Sprintf("Replying to comment %s", commentID), + Marks: []api.ADFMark{{ + Type: "link", + Attrs: &api.ADFAttrs{Href: commentURL}, + }}, + }) + + content := []api.ADFContent{header} + if body != nil { + content = append(content, body.Content...) + } + + return &api.ADF{Type: "doc", Version: 1, Content: content} +} diff --git a/internal/cmd/issue/comment/add_test.go b/internal/cmd/issue/comment/add_test.go index afe1862..6fc956e 100644 --- a/internal/cmd/issue/comment/add_test.go +++ b/internal/cmd/issue/comment/add_test.go @@ -6,9 +6,70 @@ import ( "strings" "testing" + "github.com/enthus-appdev/atl-cli/internal/api" "github.com/enthus-appdev/atl-cli/internal/iostreams" ) +func TestBuildReplyADF_MentionsAuthorAndLinksOriginalWithoutQuoting(t *testing.T) { + body := &api.ADF{ + Type: "doc", + Version: 1, + Content: []api.ADFContent{{ + Type: "paragraph", + Content: []api.ADFContent{{Type: "text", Text: "The actual reply"}}, + }}, + } + author := &api.User{AccountID: "account-123", DisplayName: "Alex Example"} + + doc := buildReplyADF("example.atlassian.net", "PROJ-42", "987", author, body) + + if len(doc.Content) != 2 { + t.Fatalf("expected reply header and body only, got %d top-level nodes", len(doc.Content)) + } + if doc.Content[0].Type != "paragraph" { + t.Fatalf("expected reply header paragraph, got %q", doc.Content[0].Type) + } + if len(doc.Content[0].Content) != 3 { + t.Fatalf("expected mention, separator, and link, got %+v", doc.Content[0].Content) + } + + mention := doc.Content[0].Content[0] + if mention.Type != "mention" || mention.Attrs == nil { + t.Fatalf("expected real ADF mention, got %+v", mention) + } + if mention.Attrs.ID != "account-123" || mention.Attrs.Text != "@Alex Example" { + t.Errorf("unexpected mention attrs: %+v", mention.Attrs) + } + + link := doc.Content[0].Content[2] + if link.Text != "Replying to comment 987" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { + t.Fatalf("expected focused-comment link, got %+v", link) + } + if got := link.Marks[0].Attrs.Href; got != "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987" { + t.Errorf("unexpected reply URL: %q", got) + } + if got := doc.Content[1].Content[0].Text; got != "The actual reply" { + t.Errorf("expected reply body unchanged, got %q", got) + } + for _, node := range doc.Content { + if node.Type == "blockquote" { + t.Fatal("reply must not copy the original comment into a blockquote") + } + } +} + +func TestBuildReplyADF_StillLinksWhenAuthorUnavailable(t *testing.T) { + doc := buildReplyADF("example.atlassian.net", "PROJ-42", "987", nil, nil) + + if len(doc.Content) != 1 || len(doc.Content[0].Content) != 1 { + t.Fatalf("expected link-only header, got %+v", doc.Content) + } + link := doc.Content[0].Content[0] + if link.Type != "text" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { + t.Fatalf("expected focused-comment link without a mention, got %+v", link) + } +} + func TestNewCmdAdd_BodyFileFlag(t *testing.T) { ios := iostreams.Test() cmd := NewCmdAdd(ios) From c6294e7f0c4e96e8698bf43403b81a4f819fa3ef Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Wed, 2 Sep 2026 09:33:23 +0200 Subject: [PATCH 2/4] test: verify reply ADF transport --- internal/api/adf_quote.go | 97 ----- internal/api/adf_quote_test.go | 500 ------------------------- internal/api/jira_comment_body_test.go | 75 ++++ 3 files changed, 75 insertions(+), 597 deletions(-) delete mode 100644 internal/api/adf_quote.go delete mode 100644 internal/api/adf_quote_test.go diff --git a/internal/api/adf_quote.go b/internal/api/adf_quote.go deleted file mode 100644 index 1683c7d..0000000 --- a/internal/api/adf_quote.go +++ /dev/null @@ -1,97 +0,0 @@ -package api - -import ( - "fmt" - "strings" -) - -// blockquoteAllowedChildren is the ADF content model for the blockquote node. -// A child outside this set makes Jira reject the entire document, so anything -// else must be degraded rather than passed through. -var blockquoteAllowedChildren = map[string]bool{ - "paragraph": true, - "bulletList": true, - "orderedList": true, - "codeBlock": true, - "mediaGroup": true, - "mediaSingle": true, -} - -// QuoteADF wraps a document's nodes in a blockquote, copying legal children -// verbatim so media, mentions and exact characters survive. Returns nil when -// nothing quotable remains — a blockquote with no children is invalid ADF. -func QuoteADF(original *ADF) *ADFContent { - if original == nil { - return nil - } - - children := quoteNodes(original.Content) - - if len(children) == 0 { - return nil - } - - return &ADFContent{Type: "blockquote", Content: children} -} - -// quoteNodes returns zero or more legal blockquote children from a slice of nodes. -func quoteNodes(nodes []ADFContent) []ADFContent { - out := make([]ADFContent, 0, len(nodes)) - for _, node := range nodes { - out = append(out, quoteNode(node)...) - } - return out -} - -// containsLegal reports whether a subtree holds any node the blockquote content -// model accepts, distinguishing a container worth descending into from an -// inline-run node whose children must stay together on one line. -func containsLegal(nodes []ADFContent) bool { - for _, node := range nodes { - if blockquoteAllowedChildren[node.Type] || containsLegal(node.Content) { - return true - } - } - return false -} - -// quoteNode returns the node as zero or more legal blockquote children. A node -// the content model rejects contributes its legal descendants, so media -// and text nested in a container survive; only a subtree with nothing legal -// in it collapses to rendered text. -func quoteNode(node ADFContent) []ADFContent { - if blockquoteAllowedChildren[node.Type] { - // The spec forbids node-level marks on a quoted paragraph. node is a - // copy, so the caller's slice is untouched. - if node.Type == "paragraph" { - node.Marks = nil - } - return []ADFContent{node} - } - - if containsLegal(node.Content) { - return quoteNodes(node.Content) - } - - text := ADFToText(&ADF{Type: "doc", Version: 1, Content: []ADFContent{node}}) - if strings.TrimSpace(text) == "" { - return nil - } - - return []ADFContent{{ - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: text}}, - }} -} - -// AttributionParagraph builds the "Replying to :" line that precedes a quote. -func AttributionParagraph(author string) ADFContent { - return ADFContent{ - Type: "paragraph", - Content: []ADFContent{{ - Type: "text", - Text: fmt.Sprintf("Replying to %s:", author), - Marks: []ADFMark{{Type: "em"}}, - }}, - } -} diff --git a/internal/api/adf_quote_test.go b/internal/api/adf_quote_test.go deleted file mode 100644 index 58e404b..0000000 --- a/internal/api/adf_quote_test.go +++ /dev/null @@ -1,500 +0,0 @@ -package api - -import ( - "strings" - "testing" -) - -func TestQuoteADF_PreservesMediaSingle(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "mediaSingle", - Content: []ADFContent{{ - Type: "media", - Attrs: &ADFAttrs{ - ID: "media-abc", - Type: "file", - Collection: "coll-1", - }, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if quote.Type != "blockquote" { - t.Fatalf("expected blockquote, got %q", quote.Type) - } - if len(quote.Content) != 1 { - t.Fatalf("expected 1 child, got %d", len(quote.Content)) - } - child := quote.Content[0] - if child.Type != "mediaSingle" { - t.Fatalf("expected mediaSingle preserved, got %q", child.Type) - } - media := child.Content[0] - if media.Type != "media" { - t.Fatalf("expected media node, got %q", media.Type) - } - if media.Attrs == nil || media.Attrs.ID != "media-abc" || media.Attrs.Collection != "coll-1" { - t.Errorf("media attrs lost: %+v", media.Attrs) - } -} - -func TestQuoteADF_PreservesMentionInsideParagraph(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "paragraph", - Content: []ADFContent{{ - Type: "mention", - Attrs: &ADFAttrs{ID: "acc-123", Text: "@Bernd Waldmann"}, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - mention := quote.Content[0].Content[0] - if mention.Type != "mention" { - t.Fatalf("expected mention preserved, got %q", mention.Type) - } - if mention.Attrs.ID != "acc-123" { - t.Errorf("expected account id acc-123, got %q", mention.Attrs.ID) - } -} - -func TestQuoteADF_StripsParagraphNodeMarks(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "paragraph", - Marks: []ADFMark{{Type: "em"}}, - Content: []ADFContent{{Type: "text", Text: "hi"}}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if quote.Content[0].Marks != nil { - t.Errorf("expected node-level marks cleared, got %+v", quote.Content[0].Marks) - } - if quote.Content[0].Content[0].Text != "hi" { - t.Error("text content must survive mark stripping") - } -} - -func TestQuoteADF_DegradesIllegalChildren(t *testing.T) { - for _, tc := range []struct { - name string - node ADFContent - }{ - {"heading", ADFContent{ - Type: "heading", - Attrs: &ADFAttrs{Level: 2}, - Content: []ADFContent{{Type: "text", Text: "Section title"}}, - }}, - {"nested blockquote", ADFContent{ - Type: "blockquote", - Content: []ADFContent{{ - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: "Section title"}}, - }}, - }}, - } { - t.Run(tc.name, func(t *testing.T) { - quote := QuoteADF(&ADF{Type: "doc", Version: 1, Content: []ADFContent{tc.node}}) - if quote == nil { - t.Fatal("expected a blockquote node") - } - child := quote.Content[0] - if child.Type != "paragraph" { - t.Fatalf("expected degrade to paragraph, got %q", child.Type) - } - if len(child.Content) != 1 || child.Content[0].Type != "text" { - t.Fatalf("expected a single text child, got %+v", child.Content) - } - if child.Content[0].Text == "" { - t.Error("degraded paragraph must keep the text") - } - }) - } -} - -func TestQuoteADF_NilWhenNothingQuotable(t *testing.T) { - if got := QuoteADF(nil); got != nil { - t.Errorf("expected nil for nil input, got %+v", got) - } - if got := QuoteADF(&ADF{Type: "doc", Version: 1}); got != nil { - t.Errorf("expected nil for empty document, got %+v", got) - } -} - -func TestAttributionParagraph(t *testing.T) { - p := AttributionParagraph("Bernd Waldmann") - if p.Type != "paragraph" { - t.Fatalf("expected paragraph, got %q", p.Type) - } - text := p.Content[0] - if text.Text != "Replying to Bernd Waldmann:" { - t.Errorf("unexpected attribution text: %q", text.Text) - } - if len(text.Marks) != 1 || text.Marks[0].Type != "em" { - t.Errorf("expected em mark, got %+v", text.Marks) - } -} - -func TestQuoteADF_PreservesBulletList(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "bulletList", - Content: []ADFContent{{ - Type: "listItem", - Content: []ADFContent{{Type: "paragraph", Content: []ADFContent{{Type: "text", Text: "item"}}}}, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 || quote.Content[0].Type != "bulletList" { - t.Fatalf("expected bulletList preserved, got %q", quote.Content[0].Type) - } -} - -func TestQuoteADF_PreservesOrderedList(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "orderedList", - Content: []ADFContent{{ - Type: "listItem", - Content: []ADFContent{{Type: "paragraph", Content: []ADFContent{{Type: "text", Text: "item"}}}}, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 || quote.Content[0].Type != "orderedList" { - t.Fatalf("expected orderedList preserved, got %q", quote.Content[0].Type) - } -} - -func TestQuoteADF_PreservesCodeBlock(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "codeBlock", - Attrs: &ADFAttrs{Language: "go"}, - Content: []ADFContent{{Type: "text", Text: "func main() {}"}}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 || quote.Content[0].Type != "codeBlock" { - t.Fatalf("expected codeBlock preserved, got %q", quote.Content[0].Type) - } -} - -func TestQuoteADF_PreservesMediaGroup(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "mediaGroup", - Content: []ADFContent{{ - Type: "media", - Attrs: &ADFAttrs{ - ID: "media-xyz", - Type: "file", - Collection: "coll-2", - }, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 || quote.Content[0].Type != "mediaGroup" { - t.Fatalf("expected mediaGroup preserved, got %q", quote.Content[0].Type) - } -} - -func TestQuoteADF_PreservesInlineMarksInParagraph(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "paragraph", - Marks: []ADFMark{{Type: "em"}}, // Node-level mark (should be cleared) - Content: []ADFContent{{ - Type: "text", - Text: "bold text", - Marks: []ADFMark{{Type: "strong"}}, // Inline mark (should survive) - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - para := quote.Content[0] - if para.Marks != nil { - t.Errorf("expected node-level marks cleared, got %+v", para.Marks) - } - textNode := para.Content[0] - if len(textNode.Marks) != 1 || textNode.Marks[0].Type != "strong" { - t.Errorf("expected inline strong mark to survive, got %+v", textNode.Marks) - } - if textNode.Text != "bold text" { - t.Errorf("expected text to survive, got %q", textNode.Text) - } -} - -func TestQuoteADF_HoistsMediaFromIllegalPanel(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "panel", - Attrs: &ADFAttrs{PanelType: "info"}, - Content: []ADFContent{ - { - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: "info text"}}, - }, - { - Type: "mediaSingle", - Content: []ADFContent{{ - Type: "media", - Attrs: &ADFAttrs{ - ID: "media-panel", - Type: "file", - Collection: "panel-coll", - }, - }}, - }, - }, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 2 { - t.Fatalf("expected 2 hoisted children, got %d", len(quote.Content)) - } - if quote.Content[0].Type != "paragraph" { - t.Fatalf("expected first child to be paragraph, got %q", quote.Content[0].Type) - } - if quote.Content[1].Type != "mediaSingle" { - t.Fatalf("expected second child to be mediaSingle, got %q", quote.Content[1].Type) - } - media := quote.Content[1].Content[0] - if media.Attrs == nil || media.Attrs.ID != "media-panel" || media.Attrs.Collection != "panel-coll" { - t.Errorf("media attrs lost during hoisting: %+v", media.Attrs) - } -} - -func TestQuoteADF_DegradesIllegalWithoutLegalDescendants(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "panel", - Attrs: &ADFAttrs{PanelType: "warning"}, - Content: []ADFContent{{Type: "text", Text: "warning text"}}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - child := quote.Content[0] - if child.Type != "paragraph" { - t.Fatalf("expected degrade to paragraph, got %q", child.Type) - } - if len(child.Content) != 1 || child.Content[0].Type != "text" { - t.Fatalf("expected a single text child, got %+v", child.Content) - } - if child.Content[0].Text == "" { - t.Error("degraded paragraph must keep the text") - } -} - -func TestQuoteADF_HeadingInlineRunsStayTogether(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "heading", - Attrs: &ADFAttrs{Level: 2}, - Content: []ADFContent{ - {Type: "text", Text: "Hello "}, - {Type: "mention", Attrs: &ADFAttrs{ID: "acc-123", Text: "@Bob"}}, - {Type: "text", Text: " World"}, - }, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 { - t.Fatalf("expected 1 child, got %d; heading should not fragment", len(quote.Content)) - } - child := quote.Content[0] - if child.Type != "paragraph" { - t.Fatalf("expected paragraph, got %q", child.Type) - } - if len(child.Content) != 1 || child.Content[0].Type != "text" { - t.Fatalf("expected a single text child, got %+v", child.Content) - } - text := child.Content[0].Text - if !strings.Contains(text, "Hello") || !strings.Contains(text, "Bob") || !strings.Contains(text, "World") { - t.Errorf("expected all inline runs in one paragraph, got %q", text) - } -} - -func TestQuoteADF_PanelWithMixedContentHoistsAll(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "panel", - Attrs: &ADFAttrs{PanelType: "info"}, - Content: []ADFContent{ - { - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: "Info text"}}, - }, - { - Type: "mediaSingle", - Content: []ADFContent{{ - Type: "media", - Attrs: &ADFAttrs{ - ID: "media-info", - Type: "file", - Collection: "coll-info", - }, - }}, - }, - }, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 2 { - t.Fatalf("expected 2 hoisted children, got %d", len(quote.Content)) - } - if quote.Content[0].Type != "paragraph" || quote.Content[0].Content[0].Text != "Info text" { - t.Fatalf("expected paragraph with 'Info text' as first child") - } - if quote.Content[1].Type != "mediaSingle" { - t.Fatalf("expected mediaSingle as second child, got %q", quote.Content[1].Type) - } -} - -func TestQuoteADF_PanelWithParagraphAndHeadingFlattenHeading(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "panel", - Attrs: &ADFAttrs{PanelType: "info"}, - Content: []ADFContent{ - { - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: "Paragraph text"}}, - }, - { - Type: "heading", - Attrs: &ADFAttrs{Level: 2}, - Content: []ADFContent{{Type: "text", Text: "Heading text"}}, - }, - }, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 2 { - t.Fatalf("expected 2 children, got %d", len(quote.Content)) - } - if quote.Content[0].Type != "paragraph" || quote.Content[0].Content[0].Text != "Paragraph text" { - t.Fatalf("expected hoisted paragraph as first child") - } - if quote.Content[1].Type != "paragraph" { - t.Fatalf("expected flattened paragraph as second child, got %q", quote.Content[1].Type) - } - if !strings.Contains(quote.Content[1].Content[0].Text, "Heading text") { - t.Fatalf("expected 'Heading text' in flattened paragraph, got %q", quote.Content[1].Content[0].Text) - } -} - -func TestQuoteADF_DeeplyNestedBlockNodeHoisted(t *testing.T) { - original := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "table", - Content: []ADFContent{{ - Type: "tableRow", - Content: []ADFContent{{ - Type: "tableCell", - Content: []ADFContent{{ - Type: "paragraph", - Content: []ADFContent{{Type: "text", Text: "Cell text"}}, - }}, - }}, - }}, - }}, - } - - quote := QuoteADF(original) - if quote == nil { - t.Fatal("expected a blockquote node") - } - if len(quote.Content) != 1 { - t.Fatalf("expected 1 hoisted paragraph, got %d children", len(quote.Content)) - } - if quote.Content[0].Type != "paragraph" { - t.Fatalf("expected paragraph, got %q", quote.Content[0].Type) - } - if quote.Content[0].Content[0].Text != "Cell text" { - t.Fatalf("expected 'Cell text', got %q", quote.Content[0].Content[0].Text) - } -} diff --git a/internal/api/jira_comment_body_test.go b/internal/api/jira_comment_body_test.go index e3e05ab..a06258f 100644 --- a/internal/api/jira_comment_body_test.go +++ b/internal/api/jira_comment_body_test.go @@ -2,9 +2,22 @@ package api import ( "context" + "encoding/json" + "io" + "net/http" + "strings" "testing" + "time" + + "github.com/enthus-appdev/atl-cli/internal/auth" ) +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) { + return fn(request) +} + func TestCommentBodyADF_PrefersBodyADF(t *testing.T) { prebuilt := &ADF{ Type: "doc", @@ -27,3 +40,65 @@ func TestCommentBodyADF_PrefersBodyADF(t *testing.T) { t.Fatal("expected BodyADF to be returned unchanged") } } + +func TestAddCommentWithOptionsPostsPrebuiltADF(t *testing.T) { + prebuilt := &ADF{ + Type: "doc", + Version: 1, + Content: []ADFContent{{ + Type: "paragraph", + Content: []ADFContent{ + {Type: "mention", Attrs: &ADFAttrs{ID: "account-123", Text: "@Alex Example"}}, + {Type: "text", Text: "Replying to comment 987", Marks: []ADFMark{{Type: "link", Attrs: &ADFAttrs{Href: "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987"}}}}, + }, + }}, + } + + client := &Client{ + cloudID: "test-cloud", + tokens: &auth.TokenSet{ + AccessToken: "test-token", + ExpiresAt: time.Now().Add(time.Hour), + }, + httpClient: &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) { + if request.Method != http.MethodPost { + t.Errorf("unexpected method: %s", request.Method) + } + if got := request.URL.String(); got != "https://api.atlassian.com/ex/jira/test-cloud/rest/api/3/issue/PROJ-42/comment" { + t.Errorf("unexpected URL: %s", got) + } + + var posted AddCommentRequest + if err := json.NewDecoder(request.Body).Decode(&posted); err != nil { + t.Fatalf("decode posted comment: %v", err) + } + if posted.Body == nil || len(posted.Body.Content) != 1 || len(posted.Body.Content[0].Content) != 2 { + t.Fatalf("unexpected posted ADF structure: %#v", posted.Body) + } + mention := posted.Body.Content[0].Content[0] + if mention.Attrs == nil || mention.Attrs.ID != "account-123" || mention.Attrs.Text != "@Alex Example" { + t.Errorf("unexpected posted mention: %#v", mention) + } + link := posted.Body.Content[0].Content[1] + if len(link.Marks) != 1 || link.Marks[0].Attrs == nil || link.Marks[0].Attrs.Href != "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987" { + t.Errorf("unexpected posted link: %#v", link) + } + + return &http.Response{ + StatusCode: http.StatusCreated, + Status: "201 Created", + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(`{"id":"123"}`)), + Request: request, + }, nil + })}, + } + + comment, err := NewJiraService(client).AddCommentWithOptions(context.Background(), "PROJ-42", &CommentOptions{BodyADF: prebuilt}) + if err != nil { + t.Fatalf("AddCommentWithOptions returned an error: %v", err) + } + if comment.ID != "123" { + t.Errorf("unexpected comment ID: %q", comment.ID) + } +} From b41bc7fbf312b3f116b7b826fdad64df28645c96 Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Wed, 2 Sep 2026 09:44:01 +0200 Subject: [PATCH 3/4] refactor: keep reply ADF construction in API --- internal/api/jira_comment_body_test.go | 25 ++++----- internal/api/reply_adf.go | 57 +++++++++++++++++++ internal/api/reply_adf_test.go | 78 ++++++++++++++++++++++++++ internal/cmd/issue/comment/add.go | 46 +-------------- internal/cmd/issue/comment/add_test.go | 61 -------------------- 5 files changed, 148 insertions(+), 119 deletions(-) create mode 100644 internal/api/reply_adf.go create mode 100644 internal/api/reply_adf_test.go diff --git a/internal/api/jira_comment_body_test.go b/internal/api/jira_comment_body_test.go index a06258f..5834f43 100644 --- a/internal/api/jira_comment_body_test.go +++ b/internal/api/jira_comment_body_test.go @@ -42,17 +42,13 @@ func TestCommentBodyADF_PrefersBodyADF(t *testing.T) { } func TestAddCommentWithOptionsPostsPrebuiltADF(t *testing.T) { - prebuilt := &ADF{ - Type: "doc", - Version: 1, - Content: []ADFContent{{ - Type: "paragraph", - Content: []ADFContent{ - {Type: "mention", Attrs: &ADFAttrs{ID: "account-123", Text: "@Alex Example"}}, - {Type: "text", Text: "Replying to comment 987", Marks: []ADFMark{{Type: "link", Attrs: &ADFAttrs{Href: "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987"}}}}, - }, - }}, - } + prebuilt := BuildReplyADF( + "example.atlassian.net", + "PROJ-42", + "987", + &User{AccountID: "account-123", DisplayName: "Alex Example"}, + &ADF{Type: "doc", Version: 1, Content: []ADFContent{{Type: "paragraph", Content: []ADFContent{{Type: "text", Text: "The actual reply"}}}}}, + ) client := &Client{ cloudID: "test-cloud", @@ -67,19 +63,22 @@ func TestAddCommentWithOptionsPostsPrebuiltADF(t *testing.T) { if got := request.URL.String(); got != "https://api.atlassian.com/ex/jira/test-cloud/rest/api/3/issue/PROJ-42/comment" { t.Errorf("unexpected URL: %s", got) } + if got := request.Header.Get("Content-Type"); got != "application/json" { + t.Errorf("unexpected Content-Type: %q", got) + } var posted AddCommentRequest if err := json.NewDecoder(request.Body).Decode(&posted); err != nil { t.Fatalf("decode posted comment: %v", err) } - if posted.Body == nil || len(posted.Body.Content) != 1 || len(posted.Body.Content[0].Content) != 2 { + if posted.Body == nil || len(posted.Body.Content) != 2 || len(posted.Body.Content[0].Content) != 3 { t.Fatalf("unexpected posted ADF structure: %#v", posted.Body) } mention := posted.Body.Content[0].Content[0] if mention.Attrs == nil || mention.Attrs.ID != "account-123" || mention.Attrs.Text != "@Alex Example" { t.Errorf("unexpected posted mention: %#v", mention) } - link := posted.Body.Content[0].Content[1] + link := posted.Body.Content[0].Content[2] if len(link.Marks) != 1 || link.Marks[0].Attrs == nil || link.Marks[0].Attrs.Href != "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987" { t.Errorf("unexpected posted link: %#v", link) } diff --git a/internal/api/reply_adf.go b/internal/api/reply_adf.go new file mode 100644 index 0000000..82a30a0 --- /dev/null +++ b/internal/api/reply_adf.go @@ -0,0 +1,57 @@ +package api + +import ( + "fmt" + "net/url" + "strings" +) + +const replySeparator = " · " + +// BuildReplyADF represents a flat Jira reply as an author mention and a stable link to the original comment. +func BuildReplyADF(hostname, issueKey, commentID string, author *User, body *ADF) *ADF { + commentURL := url.URL{ + Scheme: "https", + Host: hostname, + Path: "/browse/" + issueKey, + } + query := commentURL.Query() + query.Set("focusedCommentId", commentID) + commentURL.RawQuery = query.Encode() + + header := ADFContent{Type: "paragraph"} + if author != nil { + accountID := strings.TrimSpace(author.AccountID) + if accountID != "" { + displayName := strings.TrimSpace(author.DisplayName) + if displayName == "" { + displayName = "Jira user" + } + header.Content = append(header.Content, + ADFContent{ + Type: "mention", + Attrs: &ADFAttrs{ + ID: accountID, + Text: "@" + displayName, + }, + }, + ADFContent{Type: "text", Text: replySeparator}, + ) + } + } + header.Content = append(header.Content, ADFContent{ + Type: "text", + Text: fmt.Sprintf("Replying to comment %s", commentID), + Marks: []ADFMark{{ + Type: "link", + Attrs: &ADFAttrs{Href: commentURL.String()}, + }}, + }) + + content := []ADFContent{header} + if body != nil { + content = append(content, body.Content...) + } + + return &ADF{Type: "doc", Version: 1, Content: content} +} diff --git a/internal/api/reply_adf_test.go b/internal/api/reply_adf_test.go new file mode 100644 index 0000000..610ad99 --- /dev/null +++ b/internal/api/reply_adf_test.go @@ -0,0 +1,78 @@ +package api + +import ( + "net/url" + "testing" +) + +func TestBuildReplyADF_MentionsAuthorAndLinksOriginalWithoutQuoting(t *testing.T) { + body := &ADF{ + Type: "doc", + Version: 1, + Content: []ADFContent{{ + Type: "paragraph", + Content: []ADFContent{{Type: "text", Text: "The actual reply"}}, + }}, + } + author := &User{AccountID: " account-123 ", DisplayName: " Alex Example "} + + doc := BuildReplyADF("example.atlassian.net", "PROJ-42", "987:654", author, body) + + if len(doc.Content) != 2 { + t.Fatalf("expected reply header and body only, got %d top-level nodes", len(doc.Content)) + } + if doc.Content[0].Type != "paragraph" { + t.Fatalf("expected reply header paragraph, got %q", doc.Content[0].Type) + } + if len(doc.Content[0].Content) != 3 { + t.Fatalf("expected mention, separator, and link, got %+v", doc.Content[0].Content) + } + + mention := doc.Content[0].Content[0] + if mention.Type != "mention" || mention.Attrs == nil { + t.Fatalf("expected real ADF mention, got %+v", mention) + } + if mention.Attrs.ID != "account-123" || mention.Attrs.Text != "@Alex Example" { + t.Errorf("unexpected mention attrs: %+v", mention.Attrs) + } + + separator := doc.Content[0].Content[1] + if separator.Type != "text" || separator.Text != replySeparator { + t.Errorf("unexpected reply separator: %+v", separator) + } + + link := doc.Content[0].Content[2] + if link.Text != "Replying to comment 987:654" || len(link.Marks) != 1 || link.Marks[0].Type != "link" || link.Marks[0].Attrs == nil { + t.Fatalf("expected focused-comment link, got %+v", link) + } + parsedURL, err := url.Parse(link.Marks[0].Attrs.Href) + if err != nil { + t.Fatalf("parse reply URL: %v", err) + } + if parsedURL.Scheme != "https" || parsedURL.Host != "example.atlassian.net" || parsedURL.Path != "/browse/PROJ-42" { + t.Errorf("unexpected reply URL: %s", parsedURL) + } + if got := parsedURL.Query().Get("focusedCommentId"); got != "987:654" { + t.Errorf("unexpected focused comment ID: %q", got) + } + if got := doc.Content[1].Content[0].Text; got != "The actual reply" { + t.Errorf("expected reply body unchanged, got %q", got) + } + for _, node := range doc.Content { + if node.Type == "blockquote" { + t.Fatal("reply must not copy the original comment into a blockquote") + } + } +} + +func TestBuildReplyADF_StillLinksWhenAuthorUnavailable(t *testing.T) { + doc := BuildReplyADF("example.atlassian.net", "PROJ-42", "987", &User{AccountID: " "}, nil) + + if len(doc.Content) != 1 || len(doc.Content[0].Content) != 1 { + t.Fatalf("expected link-only header, got %+v", doc.Content) + } + link := doc.Content[0].Content[0] + if link.Type != "text" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { + t.Fatalf("expected focused-comment link without a mention, got %+v", link) + } +} diff --git a/internal/cmd/issue/comment/add.go b/internal/cmd/issue/comment/add.go index 9482a7b..a78bd28 100644 --- a/internal/cmd/issue/comment/add.go +++ b/internal/cmd/issue/comment/add.go @@ -3,9 +3,7 @@ package comment import ( "context" "fmt" - "net/url" "os" - "strings" "github.com/spf13/cobra" @@ -157,7 +155,7 @@ func replyToComment(ctx context.Context, jira *api.JiraService, hostname string, } commentOpts := &api.CommentOptions{ - BodyADF: buildReplyADF(hostname, opts.IssueKey, opts.ReplyTo, originalComment.Author, bodyADF), + BodyADF: api.BuildReplyADF(hostname, opts.IssueKey, opts.ReplyTo, originalComment.Author, bodyADF), VisibilityType: opts.VisibilityType, VisibilityName: opts.VisibilityName, } @@ -184,45 +182,3 @@ func replyToComment(ctx context.Context, jira *api.JiraService, hostname string, return nil } - -func buildReplyADF(hostname, issueKey, commentID string, author *api.User, body *api.ADF) *api.ADF { - commentURL := fmt.Sprintf( - "https://%s/browse/%s?focusedCommentId=%s", - hostname, - url.PathEscape(issueKey), - url.QueryEscape(commentID), - ) - - header := api.ADFContent{Type: "paragraph"} - if author != nil && author.AccountID != "" { - displayName := strings.TrimSpace(author.DisplayName) - if displayName == "" { - displayName = "Jira user" - } - header.Content = append(header.Content, - api.ADFContent{ - Type: "mention", - Attrs: &api.ADFAttrs{ - ID: author.AccountID, - Text: "@" + displayName, - }, - }, - api.ADFContent{Type: "text", Text: " · "}, - ) - } - header.Content = append(header.Content, api.ADFContent{ - Type: "text", - Text: fmt.Sprintf("Replying to comment %s", commentID), - Marks: []api.ADFMark{{ - Type: "link", - Attrs: &api.ADFAttrs{Href: commentURL}, - }}, - }) - - content := []api.ADFContent{header} - if body != nil { - content = append(content, body.Content...) - } - - return &api.ADF{Type: "doc", Version: 1, Content: content} -} diff --git a/internal/cmd/issue/comment/add_test.go b/internal/cmd/issue/comment/add_test.go index 6fc956e..afe1862 100644 --- a/internal/cmd/issue/comment/add_test.go +++ b/internal/cmd/issue/comment/add_test.go @@ -6,70 +6,9 @@ import ( "strings" "testing" - "github.com/enthus-appdev/atl-cli/internal/api" "github.com/enthus-appdev/atl-cli/internal/iostreams" ) -func TestBuildReplyADF_MentionsAuthorAndLinksOriginalWithoutQuoting(t *testing.T) { - body := &api.ADF{ - Type: "doc", - Version: 1, - Content: []api.ADFContent{{ - Type: "paragraph", - Content: []api.ADFContent{{Type: "text", Text: "The actual reply"}}, - }}, - } - author := &api.User{AccountID: "account-123", DisplayName: "Alex Example"} - - doc := buildReplyADF("example.atlassian.net", "PROJ-42", "987", author, body) - - if len(doc.Content) != 2 { - t.Fatalf("expected reply header and body only, got %d top-level nodes", len(doc.Content)) - } - if doc.Content[0].Type != "paragraph" { - t.Fatalf("expected reply header paragraph, got %q", doc.Content[0].Type) - } - if len(doc.Content[0].Content) != 3 { - t.Fatalf("expected mention, separator, and link, got %+v", doc.Content[0].Content) - } - - mention := doc.Content[0].Content[0] - if mention.Type != "mention" || mention.Attrs == nil { - t.Fatalf("expected real ADF mention, got %+v", mention) - } - if mention.Attrs.ID != "account-123" || mention.Attrs.Text != "@Alex Example" { - t.Errorf("unexpected mention attrs: %+v", mention.Attrs) - } - - link := doc.Content[0].Content[2] - if link.Text != "Replying to comment 987" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { - t.Fatalf("expected focused-comment link, got %+v", link) - } - if got := link.Marks[0].Attrs.Href; got != "https://example.atlassian.net/browse/PROJ-42?focusedCommentId=987" { - t.Errorf("unexpected reply URL: %q", got) - } - if got := doc.Content[1].Content[0].Text; got != "The actual reply" { - t.Errorf("expected reply body unchanged, got %q", got) - } - for _, node := range doc.Content { - if node.Type == "blockquote" { - t.Fatal("reply must not copy the original comment into a blockquote") - } - } -} - -func TestBuildReplyADF_StillLinksWhenAuthorUnavailable(t *testing.T) { - doc := buildReplyADF("example.atlassian.net", "PROJ-42", "987", nil, nil) - - if len(doc.Content) != 1 || len(doc.Content[0].Content) != 1 { - t.Fatalf("expected link-only header, got %+v", doc.Content) - } - link := doc.Content[0].Content[0] - if link.Type != "text" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { - t.Fatalf("expected focused-comment link without a mention, got %+v", link) - } -} - func TestNewCmdAdd_BodyFileFlag(t *testing.T) { ios := iostreams.Test() cmd := NewCmdAdd(ios) From 4b143b3c26771fe0ecf51a2d302bae0c2eb58dcf Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Wed, 2 Sep 2026 09:51:24 +0200 Subject: [PATCH 4/4] test: cover nil reply author --- internal/api/reply_adf_test.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/api/reply_adf_test.go b/internal/api/reply_adf_test.go index 610ad99..cc107fc 100644 --- a/internal/api/reply_adf_test.go +++ b/internal/api/reply_adf_test.go @@ -66,13 +66,25 @@ func TestBuildReplyADF_MentionsAuthorAndLinksOriginalWithoutQuoting(t *testing.T } func TestBuildReplyADF_StillLinksWhenAuthorUnavailable(t *testing.T) { - doc := BuildReplyADF("example.atlassian.net", "PROJ-42", "987", &User{AccountID: " "}, nil) - - if len(doc.Content) != 1 || len(doc.Content[0].Content) != 1 { - t.Fatalf("expected link-only header, got %+v", doc.Content) + tests := []struct { + name string + author *User + }{ + {name: "nil author"}, + {name: "empty account ID", author: &User{AccountID: " "}}, } - link := doc.Content[0].Content[0] - if link.Type != "text" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { - t.Fatalf("expected focused-comment link without a mention, got %+v", link) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + doc := BuildReplyADF("example.atlassian.net", "PROJ-42", "987", test.author, nil) + + if len(doc.Content) != 1 || len(doc.Content[0].Content) != 1 { + t.Fatalf("expected link-only header, got %+v", doc.Content) + } + link := doc.Content[0].Content[0] + if link.Type != "text" || len(link.Marks) != 1 || link.Marks[0].Type != "link" { + t.Fatalf("expected focused-comment link without a mention, got %+v", link) + } + }) } }