dmitri.shuralyov.com/service/change/...

Add initial support for editing comments (toggling reactions).

Change timeline item ID type from uint64 to string.
dmitshur committed 2 years ago commit 5f99232c79a97ad039852df6993da46e9cff24d2
change.go
@@ -4,10 +4,11 @@ package change
 import (
 	"context"
 	"time"
 
 	"github.com/shurcooL/issues"
+	"github.com/shurcooL/reactions"
 	"github.com/shurcooL/users"
 )
 
 // Service defines methods of a change tracking service.
 type Service interface {
@@ -23,10 +24,13 @@ type Service interface {
 	ListTimeline(ctx context.Context, repo string, id uint64, opt *ListTimelineOptions) ([]interface{}, error)
 	// ListCommits lists change commits, from first to last.
 	ListCommits(ctx context.Context, repo string, id uint64) ([]Commit, error)
 	// Get a change diff.
 	GetDiff(ctx context.Context, repo string, id uint64, opt *GetDiffOptions) ([]byte, error)
+
+	// EditComment edits a comment.
+	EditComment(ctx context.Context, repo string, id uint64, cr CommentRequest) (Comment, error)
 }
 
 // Change represents a change in a repository.
 type Change struct {
 	ID        uint64
@@ -87,5 +91,11 @@ type ListTimelineOptions struct {
 
 type GetDiffOptions struct {
 	// Commit is the commit ID of the commit to fetch.
 	Commit string
 }
+
+// CommentRequest is a request to edit a comment.
+type CommentRequest struct {
+	ID       string
+	Reaction *reactions.EmojiID // If not nil, toggle this reaction.
+}
fs/fs.go
@@ -5,14 +5,18 @@ import (
 	"fmt"
 	"os"
 	"time"
 
 	"dmitri.shuralyov.com/service/change"
+	"github.com/shurcooL/reactions"
 	"github.com/shurcooL/users"
 )
 
-type Service struct{}
+type Service struct {
+	// Reactions, if not nil, is temporarily used as a place to store reactions.
+	Reactions reactions.Service
+}
 
 var s = struct {
 	changes []struct {
 		change.Change
 		Timeline []interface{}
@@ -38,10 +42,11 @@ var s = struct {
 
 				Commits: 3,
 			},
 			Timeline: []interface{}{
 				change.Comment{
+					ID:        "0",
 					User:      shurcool,
 					CreatedAt: time.Date(2018, 2, 12, 0, 9, 19, 621031866, time.UTC),
 					Body: `Add initial parser implementation.
 
 This is an initial implementation of a parser for the WOFF2 font
@@ -65,18 +70,18 @@ Add basic test coverage.
 Helps https://github.com/ConradIrwin/font/issues/1.
 
 For convenience, a ` + "`" + `godoc` + "`" + ` view of this change can be seen [here](https://redpen.io/rk9a75c358f45654a8).`,
 				},
 				change.Review{
-					ID:        1,
+					ID:        "1",
 					User:      shurcool,
 					CreatedAt: time.Date(2018, 2, 20, 21, 49, 35, 536092503, time.UTC),
 					State:     change.Approved,
 					Body:      "There have been some out-of-band review comments that I've addressed. This will do for the initial version.\n\nLGTM.",
 				},
 				change.TimelineItem{
-					ID:        2,
+					ID:        "2",
 					Actor:     shurcool,
 					CreatedAt: time.Date(2018, 2, 20, 21, 57, 47, 537746502, time.UTC),
 					Payload: change.MergedEvent{
 						CommitID:      "957792cbbdabb084d484a7dcfd1e5b1a739a0ced",
 						CommitHTMLURL: "https://dmitri.shuralyov.com/font/woff2/...$commit/957792cbbdabb084d484a7dcfd1e5b1a739a0ced",
@@ -197,15 +202,34 @@ func (*Service) Get(ctx context.Context, repo string, id uint64) (change.Change,
 	}
 	return s.changes[0].Change, nil
 }
 
 // ListTimeline lists timeline items (change.Comment, change.Review, change.TimelineItem) for specified change id.
-func (*Service) ListTimeline(ctx context.Context, repo string, id uint64, opt *change.ListTimelineOptions) ([]interface{}, error) {
+func (svc *Service) ListTimeline(ctx context.Context, repo string, id uint64, opt *change.ListTimelineOptions) ([]interface{}, error) {
 	if repo != "dmitri.shuralyov.com/font/woff2" || id != 1 {
 		return nil, os.ErrNotExist
 	}
-	return s.changes[0].Timeline, nil
+	if svc.Reactions == nil {
+		return s.changes[0].Timeline, nil
+	}
+	reactions, err := svc.Reactions.List(ctx, repo)
+	if err != nil {
+		return nil, fmt.Errorf("ListTimeline: Reactions.List: %v", err)
+	}
+	timeline := make([]interface{}, len(s.changes[0].Timeline))
+	copy(timeline, s.changes[0].Timeline)
+	{
+		t := timeline[0].(change.Comment)
+		t.Reactions = reactions[t.ID]
+		timeline[0] = t
+	}
+	{
+		t := timeline[1].(change.Review)
+		t.Reactions = reactions[t.ID]
+		timeline[1] = t
+	}
+	return timeline, nil
 }
 
 // ListCommits lists change commits.
 func (*Service) ListCommits(ctx context.Context, repo string, id uint64) ([]change.Commit, error) {
 	if repo != "dmitri.shuralyov.com/font/woff2" || id != 1 {
@@ -225,10 +249,28 @@ func (*Service) GetDiff(ctx context.Context, repo string, id uint64, opt *change
 	default:
 		return s.changes[0].Diffs[opt.Commit], nil
 	}
 }
 
+func (s *Service) EditComment(ctx context.Context, repo string, id uint64, cr change.CommentRequest) (change.Comment, error) {
+	if repo != "dmitri.shuralyov.com/font/woff2" || id != 1 {
+		return change.Comment{}, os.ErrNotExist
+	}
+	if s.Reactions == nil {
+		return change.Comment{}, fmt.Errorf("no place on backend to store reactions")
+	}
+	var comment change.Comment
+	if cr.Reaction != nil {
+		reactions, err := s.Reactions.Toggle(ctx, repo, cr.ID, reactions.ToggleRequest{Reaction: *cr.Reaction})
+		if err != nil {
+			return change.Comment{}, err
+		}
+		comment.Reactions = reactions
+	}
+	return comment, nil
+}
+
 // threadType is the notifications thread type for this service.
 const threadType = "Change"
 
 // ThreadType returns the notifications thread type for this service.
 func (*Service) ThreadType(repo string) string { return threadType }
gerritapi/gerritapi.go
@@ -255,11 +255,11 @@ func (s service) ListTimeline(ctx context.Context, _ string, id uint64, opt *cha
 		return nil, err
 	}
 	var timeline []interface{}
 	{
 		timeline = append(timeline, change.Comment{
-			ID:        0,
+			ID:        "0",
 			User:      s.gerritUser(chg.Owner),
 			CreatedAt: time.Time(chg.Created),
 			Body:      "", // THINK: Include commit message or no?
 			Editable:  false,
 		})
@@ -303,11 +303,11 @@ func (s service) ListTimeline(ctx context.Context, _ string, id uint64, opt *cha
 					})
 				}
 			}
 		}
 		timeline = append(timeline, change.Review{
-			ID:        uint64(idx), // TODO: message.ID is not uint64; e.g., "bfba753d015916303152305cee7152ea7a112fe0".
+			ID:        fmt.Sprint(idx), // TODO: message.ID is not uint64; e.g., "bfba753d015916303152305cee7152ea7a112fe0".
 			User:      s.gerritUser(message.Author),
 			CreatedAt: time.Time(message.Date),
 			State:     state,
 			Body:      body,
 			Editable:  false,
@@ -362,10 +362,14 @@ func parseMessage(m string) (label string, body string, ok bool) {
 	}
 
 	return label, body, true
 }
 
+func (service) EditComment(_ context.Context, repo string, id uint64, cr change.CommentRequest) (change.Comment, error) {
+	return change.Comment{}, fmt.Errorf("EditComment: not implemented")
+}
+
 func (s service) gerritUser(user gerrit.AccountInfo) users.User {
 	var avatarURL string
 	for _, avatar := range user.Avatars {
 		if avatar.Height == 100 {
 			avatarURL = avatar.URL
githubapi/githubapi.go
@@ -1,10 +1,11 @@
 // Package githubapi implements a read-only change.Service using GitHub API clients.
 package githubapi
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
 	"log"
 	"sort"
 	"strings"
 
@@ -47,11 +48,11 @@ type service struct {
 
 	currentUser users.User
 }
 
 // We use 0 as a special ID for the comment that is the PR description. This comment is edited differently.
-const prDescriptionCommentID uint64 = 0
+const prDescriptionCommentID string = "0"
 
 func (s service) List(ctx context.Context, rs string, opt change.ListOptions) ([]change.Change, error) {
 	repo, err := ghRepoSpec(rs)
 	if err != nil {
 		// TODO: Map to 400 Bad Request HTTP error.
@@ -407,11 +408,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 				By: ghActor(comment.Editor),
 				At: comment.LastEditedAt.Time,
 			}
 		}
 		timeline = append(timeline, change.Comment{
-			ID:        comment.DatabaseID,
+			ID:        fmt.Sprintf("c%d", comment.DatabaseID),
 			User:      ghActor(comment.Author),
 			CreatedAt: comment.PublishedAt.Time,
 			Edited:    edited,
 			Body:      comment.Body,
 			Reactions: s.reactions(comment.ReactionGroups),
@@ -427,11 +428,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 			}
 		}
 		var cs []change.InlineComment
 		for _, comment := range review.Comments.Nodes {
 			cs = append(cs, change.InlineComment{
-				ID:        comment.DatabaseID,
+				ID:        fmt.Sprintf("rc%d", comment.DatabaseID),
 				File:      comment.Path,
 				Line:      comment.OriginalPosition,
 				Body:      comment.Body,
 				Reactions: s.reactions(comment.ReactionGroups),
 			})
@@ -441,11 +442,11 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 				return cs[i].Line < cs[j].Line
 			}
 			return cs[i].File < cs[j].File
 		})
 		timeline = append(timeline, change.Review{
-			ID:        review.DatabaseID,
+			ID:        fmt.Sprintf("r%d", review.DatabaseID),
 			User:      ghActor(review.Author),
 			CreatedAt: review.PublishedAt.Time,
 			Edited:    edited,
 			State:     ghPRReviewState(review.State),
 			Body:      review.Body,
@@ -542,10 +543,148 @@ func (s service) ListTimeline(ctx context.Context, rs string, id uint64, opt *ch
 		timeline = timeline[start:end]
 	}
 	return timeline, nil
 }
 
+func (s service) EditComment(ctx context.Context, rs string, id uint64, cr change.CommentRequest) (change.Comment, error) {
+	repo, err := ghRepoSpec(rs)
+	if err != nil {
+		// TODO: Map to 400 Bad Request HTTP error.
+		return change.Comment{}, err
+	}
+
+	var comment change.Comment
+
+	if cr.Reaction != nil {
+		reactionContent, err := externalizeReaction(*cr.Reaction)
+		if err != nil {
+			return change.Comment{}, err
+		}
+		// See if user has already reacted with that reaction.
+		// If not, add it. Otherwise, remove it.
+		var (
+			subjectID        githubql.ID
+			viewerHasReacted bool
+		)
+		switch {
+		case cr.ID == prDescriptionCommentID:
+			var q struct {
+				Repository struct {
+					PullRequest struct {
+						ID        githubql.ID
+						Reactions struct {
+							ViewerHasReacted bool
+						} `graphql:"reactions(content:$reactionContent)"`
+					} `graphql:"pullRequest(number:$prNumber)"`
+				} `graphql:"repository(owner:$repositoryOwner,name:$repositoryName)"`
+			}
+			variables := map[string]interface{}{
+				"repositoryOwner": githubql.String(repo.Owner),
+				"repositoryName":  githubql.String(repo.Repo),
+				"prNumber":        githubql.Int(id),
+				"reactionContent": reactionContent,
+			}
+			err = s.clV4.Query(ctx, &q, variables)
+			if err != nil {
+				return change.Comment{}, err
+			}
+			subjectID = q.Repository.PullRequest.ID
+			viewerHasReacted = q.Repository.PullRequest.Reactions.ViewerHasReacted
+		case strings.HasPrefix(cr.ID, "c"):
+			commentID := "012:IssueComment" + cr.ID[len("c"):]
+			var q struct {
+				Node struct {
+					IssueComment struct {
+						ID        githubql.ID
+						Reactions struct {
+							ViewerHasReacted bool
+						} `graphql:"reactions(content:$reactionContent)"`
+					} `graphql:"...on IssueComment"`
+				} `graphql:"node(id:$commentID)"`
+			}
+			variables := map[string]interface{}{
+				"commentID":       githubql.ID(base64.StdEncoding.EncodeToString([]byte(commentID))), // HACK, TODO: Confirm StdEncoding vs URLEncoding.
+				"reactionContent": reactionContent,
+			}
+			err = s.clV4.Query(ctx, &q, variables)
+			if err != nil {
+				return change.Comment{}, err
+			}
+			subjectID = q.Node.IssueComment.ID
+			viewerHasReacted = q.Node.IssueComment.Reactions.ViewerHasReacted
+		case strings.HasPrefix(cr.ID, "rc"):
+			commentID := "024:PullRequestReviewComment" + cr.ID[len("rc"):]
+			var q struct {
+				Node struct {
+					PullRequestReviewComment struct {
+						ID        githubql.ID
+						Reactions struct {
+							ViewerHasReacted bool
+						} `graphql:"reactions(content:$reactionContent)"`
+					} `graphql:"...on PullRequestReviewComment"`
+				} `graphql:"node(id:$commentID)"`
+			}
+			variables := map[string]interface{}{
+				"commentID":       githubql.ID(base64.StdEncoding.EncodeToString([]byte(commentID))), // HACK, TODO: Confirm StdEncoding vs URLEncoding.
+				"reactionContent": reactionContent,
+			}
+			err = s.clV4.Query(ctx, &q, variables)
+			if err != nil {
+				return change.Comment{}, err
+			}
+			subjectID = q.Node.PullRequestReviewComment.ID
+			viewerHasReacted = q.Node.PullRequestReviewComment.Reactions.ViewerHasReacted
+		default:
+			return change.Comment{}, fmt.Errorf("EditComment: unrecognized kind of comment ID: %q", cr.ID)
+		}
+
+		var rgs reactionGroups
+		if !viewerHasReacted {
+			// Add reaction.
+			var m struct {
+				AddReaction struct {
+					Subject struct {
+						ReactionGroups reactionGroups
+					}
+				} `graphql:"addReaction(input:$input)"`
+			}
+			input := githubql.AddReactionInput{
+				SubjectID: subjectID,
+				Content:   reactionContent,
+			}
+			err := s.clV4.Mutate(ctx, &m, input, nil)
+			if err != nil {
+				return change.Comment{}, err
+			}
+			rgs = m.AddReaction.Subject.ReactionGroups
+		} else {
+			// Remove reaction.
+			var m struct {
+				RemoveReaction struct {
+					Subject struct {
+						ReactionGroups reactionGroups
+					}
+				} `graphql:"removeReaction(input:$input)"`
+			}
+			input := githubql.RemoveReactionInput{
+				SubjectID: subjectID,
+				Content:   reactionContent,
+			}
+			err := s.clV4.Mutate(ctx, &m, input, nil)
+			if err != nil {
+				return change.Comment{}, err
+			}
+			rgs = m.RemoveReaction.Subject.ReactionGroups
+		}
+		// TODO: Consider setting other fields? Now that using GraphQL, not that expensive (same API call).
+		//       But not needed for app yet...
+		comment.Reactions = s.reactions(rgs)
+	}
+
+	return comment, nil
+}
+
 type repoSpec struct {
 	Owner string
 	Repo  string
 }
 
httpclient/httpclient.go
@@ -0,0 +1,65 @@
+// Package httpclient contains change.Service implementation over HTTP.
+package httpclient
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+
+	"dmitri.shuralyov.com/service/change"
+	"dmitri.shuralyov.com/service/change/httproute"
+	"github.com/shurcooL/go/ctxhttp"
+)
+
+// NewChange creates a client that implements change.Service remotely over HTTP.
+// If a nil httpClient is provided, http.DefaultClient will be used.
+// scheme and host can be empty strings to target local service.
+func NewChange(httpClient *http.Client, scheme, host string) change.Service {
+	return &Change{
+		client: httpClient,
+		baseURL: &url.URL{
+			Scheme: scheme,
+			Host:   host,
+		},
+	}
+}
+
+// Change implements change.Service remotely over HTTP.
+// Use NewChange for creation; zero value of Change is unfit for use.
+type Change struct {
+	client  *http.Client // HTTP client for API requests. If nil, http.DefaultClient should be used.
+	baseURL *url.URL     // Base URL for API requests.
+
+	change.Service // For the rest of the methods that are not implemented.
+}
+
+func (c *Change) EditComment(ctx context.Context, repo string, id uint64, cr change.CommentRequest) (change.Comment, error) {
+	u := url.URL{
+		Path: httproute.EditComment,
+		RawQuery: url.Values{
+			"Repo": {repo},
+			"ID":   {fmt.Sprint(id)},
+		}.Encode(),
+	}
+	data := url.Values{ // TODO: Automate this conversion process.
+		"ID": {cr.ID},
+	}
+	if cr.Reaction != nil {
+		data.Set("Reaction", string(*cr.Reaction))
+	}
+	resp, err := ctxhttp.PostForm(ctx, c.client, c.baseURL.ResolveReference(&u).String(), data)
+	if err != nil {
+		return change.Comment{}, err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		body, _ := ioutil.ReadAll(resp.Body)
+		return change.Comment{}, fmt.Errorf("did not get acceptable status code: %v body: %q", resp.Status, body)
+	}
+	var comment change.Comment
+	err = json.NewDecoder(resp.Body).Decode(&comment)
+	return comment, err
+}
httphandler/httphandler.go
@@ -0,0 +1,44 @@
+// Package httphandler contains an API handler for change.Service.
+package httphandler
+
+import (
+	"fmt"
+	"net/http"
+	"strconv"
+
+	"dmitri.shuralyov.com/service/change"
+	"github.com/shurcooL/httperror"
+	"github.com/shurcooL/reactions"
+)
+
+// Change is an API handler for change.Service.
+type Change struct {
+	Change change.Service
+}
+
+func (h Change) EditComment(w http.ResponseWriter, req *http.Request) error {
+	if req.Method != "POST" {
+		return httperror.Method{Allowed: []string{"POST"}}
+	}
+	q := req.URL.Query() // TODO: Automate this conversion process.
+	repo := q.Get("Repo")
+	id, err := strconv.ParseUint(q.Get("ID"), 10, 64)
+	if err != nil {
+		return httperror.BadRequest{Err: fmt.Errorf("parsing ID query parameter: %v", err)}
+	}
+	if err := req.ParseForm(); err != nil {
+		return httperror.BadRequest{Err: err}
+	}
+	cr := change.CommentRequest{
+		ID: req.PostForm.Get("ID"), // TODO: Automate this conversion process.
+	}
+	if reaction := req.PostForm["Reaction"]; len(reaction) != 0 {
+		r := reactions.EmojiID(reaction[0])
+		cr.Reaction = &r
+	}
+	comment, err := h.Change.EditComment(req.Context(), repo, id, cr)
+	if err != nil {
+		return err
+	}
+	return httperror.JSONResponse{V: comment}
+}
httproute/httproute.go
@@ -0,0 +1,7 @@
+// Package httproute contains route paths for httpclient, httphandler.
+package httproute
+
+// Route paths.
+const (
+	EditComment = "/api/change/edit-comment"
+)
maintner/maintner.go
@@ -256,10 +256,14 @@ func (s service) GetDiff(_ context.Context, repo string, id uint64, opt *change.
 		})
 	}
 	return diff.PrintMultiFileDiff(fds)
 }
 
+func (service) EditComment(_ context.Context, repo string, id uint64, cr change.CommentRequest) (change.Comment, error) {
+	return change.Comment{}, fmt.Errorf("EditComment: not implemented")
+}
+
 func state(status string) change.State {
 	switch status {
 	case "new":
 		return change.OpenState
 	case "abandoned":
timeline.go
@@ -9,11 +9,11 @@ import (
 )
 
 // Comment represents a comment left on a change.
 // TODO: Consider removing in favor of Review with commented state and no inline comments.
 type Comment struct {
-	ID        uint64
+	ID        string
 	User      users.User
 	CreatedAt time.Time
 	Edited    *Edited // Edited is nil if the comment hasn't been edited.
 	Body      string
 	Reactions []reactions.Reaction
@@ -26,11 +26,11 @@ type Edited struct {
 	At time.Time
 }
 
 // Review represents a review left on a change.
 type Review struct {
-	ID        uint64
+	ID        string
 	User      users.User
 	CreatedAt time.Time
 	Edited    *Edited // Edited is nil if the review hasn't been edited.
 	State     ReviewState
 	Body      string // Optional.
@@ -39,11 +39,11 @@ type Review struct {
 	Comments  []InlineComment
 }
 
 // InlineComment represents an inline comment that was left as part of a review.
 type InlineComment struct {
-	ID        uint64
+	ID        string
 	File      string
 	Line      int
 	Body      string
 	Reactions []reactions.Reaction
 }
@@ -56,11 +56,11 @@ const (
 	ChangesRequested ReviewState = -1
 )
 
 // TimelineItem represents a timeline item.
 type TimelineItem struct {
-	ID        uint64 // TODO: See if this belongs here.
+	ID        string // TODO: See if this belongs here.
 	Actor     users.User
 	CreatedAt time.Time
 
 	// Payload specifies the event type. It's one of:
 	// ClosedEvent, ReopenedEvent, ..., MergedEvent, ApprovedEvent.