feat(native): edit and delete comments from Pulls

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Comments can be edited and deleted from Pulls. The … beside a review thread comment (in the Conversation tab and in the diff) or a comment on the pull request offers Edit, which rewrites it in place on GitHub, and Delete, which asks first. A submitted review's summary can be edited too; its verdict stays as it is. The … shows only on what GitHub lets your account change.
 - Choosing a monospace font for the interface in Settings › Appearance asks whether the terminal and the editor should use it too. Use everywhere sets both; Interface only leaves them as they were. A proportional font or System asks nothing.
 - A comment waiting in the review batch can be reworded before it is sent: its pencil opens it in a dialog, saved with Save or Ctrl+Enter. A line comment in a pull request's pending review has an Edit button beside Discard that opens it in a box, saved with Save or Ctrl+Enter.
 

internal/project/prchanged_test.go

@@ -18,13 +18,18 @@ func (l *changeLog) emit(name string, data any) {
 func TestPullRequestWritesAnnounceTheChange(t *testing.T) {
 	t.Parallel()
 	writes := map[string]func(s *Service) error{
-		"merge":          func(s *Service) error { return s.MergePullRequest("/repo", 7, "squash", "", "", false) },
-		"submit review":  func(s *Service) error { return s.SubmitReview("/repo", 7, "approve", "", "", nil) },
-		"reply":          func(s *Service) error { return s.ReplyToReviewThread("/repo", 7, 1, "ok") },
-		"resolve thread": func(s *Service) error { return s.ResolveReviewThread("/repo", "T_1", true) },
-		"comment":        func(s *Service) error { return s.CommentOnPullRequest("/repo", 7, "hi") },
-		"edit title":     func(s *Service) error { return s.EditPullRequestTitle("/repo", 7, "title") },
-		"edit body":      func(s *Service) error { return s.EditPullRequestBody("/repo", 7, "") },
+		"merge":             func(s *Service) error { return s.MergePullRequest("/repo", 7, "squash", "", "", false) },
+		"submit review":     func(s *Service) error { return s.SubmitReview("/repo", 7, "approve", "", "", nil) },
+		"reply":             func(s *Service) error { return s.ReplyToReviewThread("/repo", 7, 1, "ok") },
+		"edit comment":      func(s *Service) error { return s.EditReviewComment("/repo", 7, 1, "ok") },
+		"delete comment":    func(s *Service) error { return s.DeleteReviewComment("/repo", 7, 1) },
+		"edit pr comment":   func(s *Service) error { return s.EditPullRequestComment("/repo", 7, 1, "ok") },
+		"delete pr comment": func(s *Service) error { return s.DeletePullRequestComment("/repo", 7, 1) },
+		"edit review body":  func(s *Service) error { return s.EditReviewBody("/repo", 7, 1, "ok") },
+		"resolve thread":    func(s *Service) error { return s.ResolveReviewThread("/repo", "T_1", true) },
+		"comment":           func(s *Service) error { return s.CommentOnPullRequest("/repo", 7, "hi") },
+		"edit title":        func(s *Service) error { return s.EditPullRequestTitle("/repo", 7, "title") },
+		"edit body":         func(s *Service) error { return s.EditPullRequestBody("/repo", 7, "") },
 	}
 	for name, write := range writes {
 		t.Run(name+" emits once it landed", func(t *testing.T) {

internal/project/prreview.go

@@ -2,6 +2,7 @@ package project
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -165,11 +166,40 @@ func writeTempJSON(payload any) (string, error) {
 // it is an answer to something already said, which is the same split GitHub
 // draws between "add single comment" and "start a review".
 func (s *Service) ReplyToReviewThread(path string, number int, commentID int64, body string) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/pulls/%d/comments/%d/replies", number, commentID)
+	return s.writeBody(path, number, "POST", endpoint, body, "a reply needs something written in it")
+}
+
+// writeBody sends body to a REST endpoint that takes a text and nothing else,
+// refusing an empty one with emptyErr before it reaches gh: GitHub would
+// refuse it too, a round-trip later.
+func (s *Service) writeBody(path string, number int, method, endpoint, body, emptyErr string) error {
 	if body == "" {
-		return fmt.Errorf("a reply needs something written in it")
+		return errors.New(emptyErr)
 	}
-	endpoint := fmt.Sprintf("repos/{owner}/{repo}/pulls/%d/comments/%d/replies", number, commentID)
-	return s.writePR(prReviewTimeout, path, number, "api", "--method", "POST", endpoint, "-f", "body="+body)
+	return s.writePR(prReviewTimeout, path, number, "api", "--method", method, endpoint, "-f", "body="+body)
+}
+
+// EditReviewComment rewrites one comment of a review thread. commentID is its
+// database id, the same address a reply takes; GitHub refuses the edit unless
+// the account gh runs as may update that comment.
+func (s *Service) EditReviewComment(path string, number int, commentID int64, body string) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/pulls/comments/%d", commentID)
+	return s.writeBody(path, number, "PATCH", endpoint, body, "a review comment needs something written in it")
+}
+
+// DeleteReviewComment removes one comment of a review thread; the last one
+// gone takes the thread with it. There is no undo on GitHub's side.
+func (s *Service) DeleteReviewComment(path string, number int, commentID int64) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/pulls/comments/%d", commentID)
+	return s.writePR(prReviewTimeout, path, number, "api", "--method", "DELETE", endpoint)
+}
+
+// EditReviewBody rewrites the summary of a submitted review. reviewID is its
+// database id; the verdict stays what it was, GitHub has no way to change it.
+func (s *Service) EditReviewBody(path string, number int, reviewID int64, body string) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/pulls/%d/reviews/%d", number, reviewID)
+	return s.writeBody(path, number, "PUT", endpoint, body, "a review summary needs something written in it")
 }
 
 // ResolveReviewThread marks a thread resolved, or reopens it. threadID is the
@@ -195,6 +225,20 @@ func (s *Service) CommentOnPullRequest(path string, number int, body string) err
 	return s.writePR(prReviewTimeout, path, number, prArgs("comment", number, "--body", body)...)
 }
 
+// EditPullRequestComment rewrites a comment on the pull request itself.
+// GitHub files those as issue comments, which is the endpoint they live at.
+func (s *Service) EditPullRequestComment(path string, number int, commentID int64, body string) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/issues/comments/%d", commentID)
+	return s.writeBody(path, number, "PATCH", endpoint, body, "a comment needs something written in it")
+}
+
+// DeletePullRequestComment removes a comment on the pull request itself, for
+// good.
+func (s *Service) DeletePullRequestComment(path string, number int, commentID int64) error {
+	endpoint := fmt.Sprintf("repos/{owner}/{repo}/issues/comments/%d", commentID)
+	return s.writePR(prReviewTimeout, path, number, "api", "--method", "DELETE", endpoint)
+}
+
 // EditPullRequestTitle renames the pull request on GitHub. GitHub refuses a pull
 // request without a title, so an empty one never reaches gh.
 func (s *Service) EditPullRequestTitle(path string, number int, title string) error {

internal/project/prreview_test.go

@@ -180,6 +180,89 @@ func TestReplyToReviewThreadFlow(t *testing.T) {
 	})
 }
 
+func TestEditReviewCommentFlow(t *testing.T) {
+	t.Parallel()
+	t.Run("an edit patches the comment by its database id", func(t *testing.T) {
+		gh := &fakeGH{}
+		if err := withGH(gh).EditReviewComment("/repo", 42, 900001, "reworded"); err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := []string{
+			"api", "--method", "PATCH",
+			"repos/{owner}/{repo}/pulls/comments/900001",
+			"-f", "body=reworded",
+		}
+		if !slices.Equal(gh.args, want) {
+			t.Errorf("args = %v, want %v", gh.args, want)
+		}
+	})
+
+	t.Run("an empty edit never reaches gh", func(t *testing.T) {
+		gh := &fakeGH{}
+		if err := withGH(gh).EditReviewComment("/repo", 42, 1, ""); err == nil {
+			t.Error("expected an error for an empty comment")
+		}
+		if gh.calls != 0 {
+			t.Errorf("gh was called %d times, want 0", gh.calls)
+		}
+	})
+}
+
+func TestConversationWritesAddressGitHub(t *testing.T) {
+	t.Parallel()
+	cases := map[string]struct {
+		write func(s *Service) error
+		want  []string
+	}{
+		"deleting a thread comment": {
+			func(s *Service) error { return s.DeleteReviewComment("/repo", 42, 900001) },
+			[]string{"api", "--method", "DELETE", "repos/{owner}/{repo}/pulls/comments/900001"},
+		},
+		"editing a pull request comment": {
+			func(s *Service) error { return s.EditPullRequestComment("/repo", 42, 800001, "reworded") },
+			[]string{"api", "--method", "PATCH", "repos/{owner}/{repo}/issues/comments/800001", "-f", "body=reworded"},
+		},
+		"deleting a pull request comment": {
+			func(s *Service) error { return s.DeletePullRequestComment("/repo", 42, 800001) },
+			[]string{"api", "--method", "DELETE", "repos/{owner}/{repo}/issues/comments/800001"},
+		},
+		"editing a review's summary": {
+			func(s *Service) error { return s.EditReviewBody("/repo", 42, 700001, "reworded") },
+			[]string{"api", "--method", "PUT", "repos/{owner}/{repo}/pulls/42/reviews/700001", "-f", "body=reworded"},
+		},
+	}
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			gh := &fakeGH{}
+			if err := c.write(withGH(gh)); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if !slices.Equal(gh.args, c.want) {
+				t.Errorf("args = %v, want %v", gh.args, c.want)
+			}
+		})
+	}
+}
+
+func TestConversationEditsRefuseAnEmptyBody(t *testing.T) {
+	t.Parallel()
+	edits := map[string]func(s *Service) error{
+		"pull request comment": func(s *Service) error { return s.EditPullRequestComment("/repo", 42, 1, "") },
+		"review summary":       func(s *Service) error { return s.EditReviewBody("/repo", 42, 1, "") },
+	}
+	for name, edit := range edits {
+		t.Run(name, func(t *testing.T) {
+			gh := &fakeGH{}
+			if err := edit(withGH(gh)); err == nil {
+				t.Error("expected an error for an empty body")
+			}
+			if gh.calls != 0 {
+				t.Errorf("gh was called %d times, want 0", gh.calls)
+			}
+		})
+	}
+}
+
 func TestResolveReviewThreadFlow(t *testing.T) {
 	t.Parallel()
 	t.Run("resolving and reopening are two mutations, one node id", func(t *testing.T) {

internal/project/prthreads.go

@@ -23,11 +23,11 @@ const prConversationQuery = `query($owner:String!,$repo:String!,$number:Int!){
   repository(owner:$owner,name:$repo){
     pullRequest(number:$number){
       headRefOid
-      reviews(last:50){nodes{id author{login} state body submittedAt}}
-      comments(last:50){nodes{author{login} body createdAt}}
+      reviews(last:50){nodes{id databaseId author{login} state body submittedAt viewerCanUpdate}}
+      comments(last:50){nodes{databaseId author{login} body createdAt viewerCanUpdate viewerCanDelete}}
       reviewThreads(last:100){nodes{
         id isResolved isOutdated path line originalLine startLine originalStartLine diffSide
-        comments(last:50){nodes{databaseId author{login} body createdAt diffHunk}}
+        comments(last:50){nodes{databaseId author{login} body createdAt diffHunk viewerCanUpdate viewerCanDelete}}
       }}
     }
   }
@@ -53,19 +53,26 @@ type PRConversation struct {
 // summary box above the line comments.
 type PRReview struct {
 	// ID is the GraphQL node id, which is what dismissing the review takes.
-	ID     string `json:"id"`
-	Author string `json:"author"`
+	ID string `json:"id"`
+	// DatabaseID is the REST id, the address its summary is rewritten at.
+	DatabaseID int64  `json:"databaseId"`
+	Author     string `json:"author"`
 	// State is gh's APPROVED | CHANGES_REQUESTED | COMMENTED | DISMISSED.
-	State string `json:"state"`
-	Body  string `json:"body"`
-	Date  string `json:"date"`
+	State   string `json:"state"`
+	Body    string `json:"body"`
+	Date    string `json:"date"`
+	CanEdit bool   `json:"canEdit"`
 }
 
 // PRComment is one comment on the pull request itself.
 type PRComment struct {
-	Author string `json:"author"`
-	Body   string `json:"body"`
-	Date   string `json:"date"`
+	// ID is the REST database id, the address an edit or a delete takes.
+	ID        int64  `json:"id"`
+	Author    string `json:"author"`
+	Body      string `json:"body"`
+	Date      string `json:"date"`
+	CanEdit   bool   `json:"canEdit"`
+	CanDelete bool   `json:"canDelete"`
 }
 
 // PRReviewThread is one conversation anchored to a line of the diff.
@@ -100,6 +107,10 @@ type PRThreadComment struct {
 	// diff already has them where the thread is anchored; this is what an
 	// outdated thread has left.
 	DiffHunk string `json:"diffHunk"`
+	// CanEdit is GitHub's own answer to whether the account reading may
+	// rewrite the comment, which is what the Edit button stands on.
+	CanEdit   bool `json:"canEdit"`
+	CanDelete bool `json:"canDelete"`
 }
 
 // ghConversation mirrors the GraphQL payload; nothing below leaves this package
@@ -130,17 +141,22 @@ type ghActor struct {
 }
 
 type ghReview struct {
-	ID          string  `json:"id"`
-	Author      ghActor `json:"author"`
-	State       string  `json:"state"`
-	Body        string  `json:"body"`
-	SubmittedAt string  `json:"submittedAt"`
+	ID              string  `json:"id"`
+	DatabaseID      int64   `json:"databaseId"`
+	Author          ghActor `json:"author"`
+	State           string  `json:"state"`
+	Body            string  `json:"body"`
+	SubmittedAt     string  `json:"submittedAt"`
+	ViewerCanUpdate bool    `json:"viewerCanUpdate"`
 }
 
 type ghIssueComment struct {
-	Author    ghActor `json:"author"`
-	Body      string  `json:"body"`
-	CreatedAt string  `json:"createdAt"`
+	DatabaseID      int64   `json:"databaseId"`
+	Author          ghActor `json:"author"`
+	Body            string  `json:"body"`
+	CreatedAt       string  `json:"createdAt"`
+	ViewerCanUpdate bool    `json:"viewerCanUpdate"`
+	ViewerCanDelete bool    `json:"viewerCanDelete"`
 }
 
 type ghReviewThread struct {
@@ -164,6 +180,9 @@ type ghThreadComment struct {
 	Body       string  `json:"body"`
 	CreatedAt  string  `json:"createdAt"`
 	DiffHunk   string  `json:"diffHunk"`
+	// ViewerCanUpdate answers for the account gh ran as.
+	ViewerCanUpdate bool `json:"viewerCanUpdate"`
+	ViewerCanDelete bool `json:"viewerCanDelete"`
 }
 
 // PullRequestConversation reads one pull request's reviews, comments and review
@@ -211,11 +230,13 @@ func toReviews(nodes []ghReview) []PRReview {
 			continue
 		}
 		out = append(out, PRReview{
-			ID:     r.ID,
-			Author: r.Author.Login,
-			State:  r.State,
-			Body:   r.Body,
-			Date:   r.SubmittedAt,
+			ID:         r.ID,
+			DatabaseID: r.DatabaseID,
+			Author:     r.Author.Login,
+			State:      r.State,
+			Body:       r.Body,
+			Date:       r.SubmittedAt,
+			CanEdit:    r.ViewerCanUpdate,
 		})
 	}
 	if len(out) == 0 {
@@ -230,7 +251,14 @@ func toComments(nodes []ghIssueComment) []PRComment {
 	}
 	out := make([]PRComment, 0, len(nodes))
 	for _, c := range nodes {
-		out = append(out, PRComment{Author: c.Author.Login, Body: c.Body, Date: c.CreatedAt})
+		out = append(out, PRComment{
+			ID:        c.DatabaseID,
+			Author:    c.Author.Login,
+			Body:      c.Body,
+			Date:      c.CreatedAt,
+			CanEdit:   c.ViewerCanUpdate,
+			CanDelete: c.ViewerCanDelete,
+		})
 	}
 	return out
 }
@@ -289,11 +317,13 @@ func toThreadComments(nodes []ghThreadComment) []PRThreadComment {
 	out := make([]PRThreadComment, 0, len(nodes))
 	for _, c := range nodes {
 		out = append(out, PRThreadComment{
-			ID:       c.DatabaseID,
-			Author:   c.Author.Login,
-			Body:     c.Body,
-			Date:     c.CreatedAt,
-			DiffHunk: c.DiffHunk,
+			ID:        c.DatabaseID,
+			Author:    c.Author.Login,
+			Body:      c.Body,
+			Date:      c.CreatedAt,
+			DiffHunk:  c.DiffHunk,
+			CanEdit:   c.ViewerCanUpdate,
+			CanDelete: c.ViewerCanDelete,
 		})
 	}
 	return out