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

Add initial support for editing comments (toggling reactions).

Change timeline item ID type from uint64 to string.
dmitshur committed 6 years ago commit 5f99232c79a97ad039852df6993da46e9cff24d2
Collapse all
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.