github: working incremental + comment history for the first comment

Michael Muré created

Change summary

bridge/github/import.go       | 321 +++++++++++++++++++++++-------------
bridge/github/import_query.go | 128 ++++++++++++++
bug/operation.go              |   8 
bug/snapshot.go               |   1 
bug/timeline.go               |   1 
cache/bug_cache.go            |  47 +++++
cache/repo_cache.go           |  26 ++
7 files changed, 416 insertions(+), 116 deletions(-)

Detailed changes

bridge/github/import.go 🔗

@@ -17,104 +17,24 @@ const keyGithubUrl = "github-url"
 // githubImporter implement the Importer interface
 type githubImporter struct{}
 
-type Actor struct {
-	Login     githubv4.String
-	AvatarUrl githubv4.String
-}
-
-type ActorEvent struct {
-	Id        githubv4.ID
-	CreatedAt githubv4.DateTime
-	Actor     Actor
-}
-
-type AuthorEvent struct {
-	Id        githubv4.ID
-	CreatedAt githubv4.DateTime
-	Author    Actor
-}
-
-type TimelineItem struct {
-	Typename githubv4.String `graphql:"__typename"`
-
-	// Issue
-	IssueComment struct {
-		AuthorEvent
-		Body githubv4.String
-		Url  githubv4.URI
-		// TODO: edition
-	} `graphql:"... on IssueComment"`
-
-	// Label
-	LabeledEvent struct {
-		ActorEvent
-		Label struct {
-			// Color githubv4.String
-			Name githubv4.String
-		}
-	} `graphql:"... on LabeledEvent"`
-	UnlabeledEvent struct {
-		ActorEvent
-		Label struct {
-			// Color githubv4.String
-			Name githubv4.String
-		}
-	} `graphql:"... on UnlabeledEvent"`
-
-	// Status
-	ClosedEvent struct {
-		ActorEvent
-		// Url githubv4.URI
-	} `graphql:"... on  ClosedEvent"`
-	ReopenedEvent struct {
-		ActorEvent
-	} `graphql:"... on  ReopenedEvent"`
-
-	// Title
-	RenamedTitleEvent struct {
-		ActorEvent
-		CurrentTitle  githubv4.String
-		PreviousTitle githubv4.String
-	} `graphql:"... on RenamedTitleEvent"`
-}
-
-type Issue struct {
-	AuthorEvent
-	Title string
-	Body  githubv4.String
-	Url   githubv4.URI
-
-	Timeline struct {
-		Nodes    []TimelineItem
-		PageInfo struct {
-			EndCursor   githubv4.String
-			HasNextPage bool
-		}
-	} `graphql:"timeline(first: $timelineFirst, after: $timelineAfter)"`
-}
-
-var q struct {
-	Repository struct {
-		Issues struct {
-			Nodes    []Issue
-			PageInfo struct {
-				EndCursor   githubv4.String
-				HasNextPage bool
-			}
-		} `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"`
-	} `graphql:"repository(owner: $owner, name: $name)"`
-}
-
 func (*githubImporter) ImportAll(repo *cache.RepoCache, conf core.Configuration) error {
 	client := buildClient(conf)
 
+	q := &issueTimelineQuery{}
 	variables := map[string]interface{}{
-		"owner":         githubv4.String(conf[keyUser]),
-		"name":          githubv4.String(conf[keyProject]),
-		"issueFirst":    githubv4.Int(1),
-		"issueAfter":    (*githubv4.String)(nil),
-		"timelineFirst": githubv4.Int(10),
-		"timelineAfter": (*githubv4.String)(nil),
+		"owner":            githubv4.String(conf[keyUser]),
+		"name":             githubv4.String(conf[keyProject]),
+		"issueFirst":       githubv4.Int(1),
+		"issueAfter":       (*githubv4.String)(nil),
+		"timelineFirst":    githubv4.Int(10),
+		"timelineAfter":    (*githubv4.String)(nil),
+		"commentEditFirst": githubv4.Int(10),
+		"commentEditAfter": (*githubv4.String)(nil),
+
+		// Fun fact, github provide the comment edition in reverse chronological
+		// order, because haha. Look at me, I'm dying of laughter.
+		"issueEditLast":   githubv4.Int(10),
+		"issueEditBefore": (*githubv4.String)(nil),
 	}
 
 	var b *cache.BugCache
@@ -125,22 +45,22 @@ func (*githubImporter) ImportAll(repo *cache.RepoCache, conf core.Configuration)
 			return err
 		}
 
-		if len(q.Repository.Issues.Nodes) != 1 {
-			return fmt.Errorf("Something went wrong when iterating issues, len is %d", len(q.Repository.Issues.Nodes))
+		if len(q.Repository.Issues.Nodes) == 0 {
+			return nil
 		}
 
 		issue := q.Repository.Issues.Nodes[0]
 
 		if b == nil {
-			b, err = importIssue(repo, issue)
+			b, err = ensureIssue(repo, issue, client, variables)
 			if err != nil {
 				return err
 			}
 		}
 
-		for _, item := range q.Repository.Issues.Nodes[0].Timeline.Nodes {
-			importTimelineItem(b, item)
-		}
+		// for _, item := range q.Repository.Issues.Nodes[0].Timeline.Nodes {
+		// 	importTimelineItem(b, item)
+		// }
 
 		if !issue.Timeline.PageInfo.HasNextPage {
 			err = b.CommitAsNeeded()
@@ -172,28 +92,138 @@ func (*githubImporter) Import(repo *cache.RepoCache, conf core.Configuration, id
 	return nil
 }
 
-func importIssue(repo *cache.RepoCache, issue Issue) (*cache.BugCache, error) {
+func ensureIssue(repo *cache.RepoCache, issue issueTimeline, client *githubv4.Client, rootVariables map[string]interface{}) (*cache.BugCache, error) {
 	fmt.Printf("import issue: %s\n", issue.Title)
 
+	b, err := repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id))
+	if err != nil && err != bug.ErrBugNotExist {
+		return nil, err
+	}
+
+	// if there is no edit, the UserContentEdits given by github is empty. That
+	// means that the original message is given by the issue message.
+
+	// if there is edits, the UserContentEdits given by github contains both the
+	// original message and the following edits. The issue message give the last
+	// version so we don't care about that.
+
+	if len(issue.UserContentEdits.Nodes) == 0 {
+		if err == bug.ErrBugNotExist {
+			b, err = repo.NewBugRaw(
+				makePerson(issue.Author),
+				issue.CreatedAt.Unix(),
+				// Todo: this might not be the initial title, we need to query the
+				// timeline to be sure
+				issue.Title,
+				cleanupText(string(issue.Body)),
+				nil,
+				map[string]string{
+					keyGithubId:  parseId(issue.Id),
+					keyGithubUrl: issue.Url.String(),
+				},
+			)
+
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		return b, nil
+	}
+
+	// reverse the order, because github
+	reverseEdits(issue.UserContentEdits.Nodes)
+
+	if err == bug.ErrBugNotExist {
+		firstEdit := issue.UserContentEdits.Nodes[0]
+
+		if firstEdit.Diff == nil {
+			return nil, fmt.Errorf("no diff")
+		}
+
+		b, err = repo.NewBugRaw(
+			makePerson(issue.Author),
+			issue.CreatedAt.Unix(),
+			// Todo: this might not be the initial title, we need to query the
+			// timeline to be sure
+			issue.Title,
+			cleanupText(string(*issue.UserContentEdits.Nodes[0].Diff)),
+			nil,
+			map[string]string{
+				keyGithubId:  parseId(issue.Id),
+				keyGithubUrl: issue.Url.String(),
+			},
+		)
+	}
+
+	for i, edit := range issue.UserContentEdits.Nodes {
+		if i == 0 {
+			// The first edit in the github result is the creation itself, we already have that
+			continue
+		}
+
+		err := ensureCommentEdit(b, parseId(issue.Id), edit)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if !issue.UserContentEdits.PageInfo.HasNextPage {
+		return b, nil
+	}
+
+	// We have more edit, querying them
+
+	q := &issueEditQuery{}
+	variables := map[string]interface{}{
+		"owner":          rootVariables["owner"],
+		"name":           rootVariables["name"],
+		"issueFirst":     rootVariables["issueFirst"],
+		"issueAfter":     rootVariables["issueAfter"],
+		"issueEditFirst": githubv4.Int(10),
+		"issueEditAfter": issue.UserContentEdits.PageInfo.EndCursor,
+	}
+
+	for {
+		err := client.Query(context.TODO(), &q, variables)
+		if err != nil {
+			return nil, err
+		}
+
+		edits := q.Repository.Issues.Nodes[0].UserContentEdits
+
+		if len(edits.Nodes) == 0 {
+			return b, nil
+		}
+
+		for i, edit := range edits.Nodes {
+			if i == 0 {
+				// The first edit in the github result is the creation itself, we already have that
+				continue
+			}
+
+			err := ensureCommentEdit(b, parseId(issue.Id), edit)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		if !edits.PageInfo.HasNextPage {
+			break
+		}
+
+		variables["issueEditAfter"] = edits.PageInfo.EndCursor
+	}
+
 	// TODO: check + import files
 
-	return repo.NewBugRaw(
-		makePerson(issue.Author),
-		issue.CreatedAt.Unix(),
-		issue.Title,
-		cleanupText(string(issue.Body)),
-		nil,
-		map[string]string{
-			keyGithubId:  parseId(issue.Id),
-			keyGithubUrl: issue.Url.String(),
-		},
-	)
+	return b, nil
 }
 
-func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
+func importTimelineItem(b *cache.BugCache, item timelineItem) error {
 	switch item.Typename {
 	case "IssueComment":
-		// fmt.Printf("import %s: %s\n", item.Typename, item.IssueComment)
+		// fmt.Printf("import %s: %s\n", item.Typename, item.issueComment)
 		return b.AddCommentRaw(
 			makePerson(item.IssueComment.Author),
 			item.IssueComment.CreatedAt.Unix(),
@@ -214,6 +244,7 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 				string(item.LabeledEvent.Label.Name),
 			},
 			nil,
+			nil,
 		)
 		return err
 
@@ -226,6 +257,7 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 			[]string{
 				string(item.UnlabeledEvent.Label.Name),
 			},
+			nil,
 		)
 		return err
 
@@ -234,6 +266,7 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 		return b.CloseRaw(
 			makePerson(item.ClosedEvent.Actor),
 			item.ClosedEvent.CreatedAt.Unix(),
+			nil,
 		)
 
 	case "ReopenedEvent":
@@ -241,6 +274,7 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 		return b.OpenRaw(
 			makePerson(item.ReopenedEvent.Actor),
 			item.ReopenedEvent.CreatedAt.Unix(),
+			nil,
 		)
 
 	case "RenamedTitleEvent":
@@ -249,6 +283,7 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 			makePerson(item.RenamedTitleEvent.Actor),
 			item.RenamedTitleEvent.CreatedAt.Unix(),
 			string(item.RenamedTitleEvent.CurrentTitle),
+			nil,
 		)
 
 	default:
@@ -258,8 +293,57 @@ func importTimelineItem(b *cache.BugCache, item TimelineItem) error {
 	return nil
 }
 
+func ensureCommentEdit(b *cache.BugCache, target string, edit userContentEdit) error {
+	if edit.Editor == nil {
+		return fmt.Errorf("no editor")
+	}
+
+	if edit.Diff == nil {
+		return fmt.Errorf("no diff")
+	}
+
+	_, err := b.ResolveTargetWithMetadata(keyGithubId, parseId(edit.Id))
+	if err == nil {
+		// already imported
+		return nil
+	}
+	if err != cache.ErrNoMatchingOp {
+		// real error
+		return err
+	}
+
+	fmt.Printf("import edition\n")
+
+	targetHash, err := b.ResolveTargetWithMetadata(keyGithubId, target)
+	if err != nil {
+		return err
+	}
+
+	switch {
+	case edit.DeletedAt != nil:
+		// comment deletion, not supported yet
+
+	case edit.DeletedAt == nil:
+		// comment edition
+		err := b.EditCommentRaw(
+			makePerson(*edit.Editor),
+			edit.CreatedAt.Unix(),
+			targetHash,
+			cleanupText(string(*edit.Diff)),
+			map[string]string{
+				keyGithubId: parseId(edit.Id),
+			},
+		)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // makePerson create a bug.Person from the Github data
-func makePerson(actor Actor) bug.Person {
+func makePerson(actor actor) bug.Person {
 	return bug.Person{
 		Name:      string(actor.Login),
 		AvatarUrl: string(actor.AvatarUrl),
@@ -275,3 +359,10 @@ func cleanupText(text string) string {
 	// windows new line, Github, really ?
 	return strings.Replace(text, "\r\n", "\n", -1)
 }
+
+func reverseEdits(edits []userContentEdit) []userContentEdit {
+	for i, j := 0, len(edits)-1; i < j; i, j = i+1, j-1 {
+		edits[i], edits[j] = edits[j], edits[i]
+	}
+	return edits
+}

bridge/github/import_query.go 🔗

@@ -0,0 +1,128 @@
+package github
+
+import "github.com/shurcooL/githubv4"
+
+type pageInfo struct {
+	EndCursor   githubv4.String
+	HasNextPage bool
+}
+
+type actor struct {
+	Login     githubv4.String
+	AvatarUrl githubv4.String
+}
+
+type actorEvent struct {
+	Id        githubv4.ID
+	CreatedAt githubv4.DateTime
+	Actor     actor
+}
+
+type authorEvent struct {
+	Id        githubv4.ID
+	CreatedAt githubv4.DateTime
+	Author    actor
+}
+
+type userContentEdit struct {
+	Id        githubv4.ID
+	CreatedAt githubv4.DateTime
+	UpdatedAt githubv4.DateTime
+	EditedAt  githubv4.DateTime
+	Editor    *actor
+	DeletedAt *githubv4.DateTime
+	DeletedBy *actor
+	Diff      *githubv4.String
+}
+
+type issueComment struct {
+	authorEvent
+	Body githubv4.String
+	Url  githubv4.URI
+
+	UserContentEdits struct {
+		Nodes    []userContentEdit
+		PageInfo pageInfo
+	} `graphql:"userContentEdits(first: $commentEditFirst, after: $commentEditAfter)"`
+}
+
+type timelineItem struct {
+	Typename githubv4.String `graphql:"__typename"`
+
+	// issue
+	IssueComment issueComment `graphql:"... on IssueComment"`
+
+	// Label
+	LabeledEvent struct {
+		actorEvent
+		Label struct {
+			// Color githubv4.String
+			Name githubv4.String
+		}
+	} `graphql:"... on LabeledEvent"`
+	UnlabeledEvent struct {
+		actorEvent
+		Label struct {
+			// Color githubv4.String
+			Name githubv4.String
+		}
+	} `graphql:"... on UnlabeledEvent"`
+
+	// Status
+	ClosedEvent struct {
+		actorEvent
+		// Url githubv4.URI
+	} `graphql:"... on  ClosedEvent"`
+	ReopenedEvent struct {
+		actorEvent
+	} `graphql:"... on  ReopenedEvent"`
+
+	// Title
+	RenamedTitleEvent struct {
+		actorEvent
+		CurrentTitle  githubv4.String
+		PreviousTitle githubv4.String
+	} `graphql:"... on RenamedTitleEvent"`
+}
+
+type issueTimeline struct {
+	authorEvent
+	Title string
+	Body  githubv4.String
+	Url   githubv4.URI
+
+	Timeline struct {
+		Nodes    []timelineItem
+		PageInfo pageInfo
+	} `graphql:"timeline(first: $timelineFirst, after: $timelineAfter)"`
+
+	UserContentEdits struct {
+		Nodes    []userContentEdit
+		PageInfo pageInfo
+	} `graphql:"userContentEdits(last: $issueEditLast, before: $issueEditBefore)"`
+}
+
+type issueEdit struct {
+	UserContentEdits struct {
+		Nodes    []userContentEdit
+		PageInfo pageInfo
+	} `graphql:"userContentEdits(last: $issueEditLast, before: $issueEditBefore)"`
+}
+
+type issueTimelineQuery struct {
+	Repository struct {
+		Issues struct {
+			Nodes    []issueTimeline
+			PageInfo pageInfo
+		} `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"`
+	} `graphql:"repository(owner: $owner, name: $name)"`
+}
+
+type issueEditQuery struct {
+	Repository struct {
+		Issues struct {
+			Nodes    []issueEdit
+			PageInfo pageInfo
+		} `graphql:"issues(first: $issueFirst, after: $issueAfter, orderBy: {field: CREATED_AT, direction: ASC})"`
+	} `graphql:"repository(owner: $owner, name: $name)"`
+}

bug/operation.go 🔗

@@ -21,6 +21,7 @@ const (
 	SetStatusOp
 	LabelChangeOp
 	EditCommentOp
+	NoOpOp
 )
 
 // Operation define the interface to fulfill for an edit operation of a Bug
@@ -43,6 +44,8 @@ type Operation interface {
 	SetMetadata(key string, value string)
 	// GetMetadata retrieve arbitrary metadata about the operation
 	GetMetadata(key string) (string, bool)
+	// AllMetadata return all metadata for this operation
+	AllMetadata() map[string]string
 }
 
 func hashRaw(data []byte) git.Hash {
@@ -145,3 +148,8 @@ func (op *OpBase) GetMetadata(key string) (string, bool) {
 	val, ok := op.Metadata[key]
 	return val, ok
 }
+
+// AllMetadata return all metadata for this operation
+func (op *OpBase) AllMetadata() map[string]string {
+	return op.Metadata
+}

bug/snapshot.go 🔗

@@ -33,6 +33,7 @@ func (snap *Snapshot) HumanId() string {
 	return fmt.Sprintf("%.8s", snap.id)
 }
 
+// Deprecated:should be moved in UI code
 func (snap *Snapshot) Summary() string {
 	return fmt.Sprintf("C:%d L:%d",
 		len(snap.Comments)-1,

bug/timeline.go 🔗

@@ -56,6 +56,7 @@ func (c *CommentTimelineItem) Append(comment Comment) {
 	c.Files = comment.Files
 	c.LastEdit = comment.UnixTime
 	c.History = append(c.History, CommentHistoryStep{
+		Author:   comment.Author,
 		Message:  comment.Message,
 		UnixTime: comment.UnixTime,
 	})

cache/bug_cache.go 🔗

@@ -1,6 +1,8 @@
 package cache
 
 import (
+	"fmt"
+	"strings"
 	"time"
 
 	"github.com/MichaelMure/git-bug/bug"
@@ -35,6 +37,51 @@ func (c *BugCache) notifyUpdated() error {
 	return c.repoCache.bugUpdated(c.bug.Id())
 }
 
+var ErrNoMatchingOp = fmt.Errorf("no matching operation found")
+
+type ErrMultipleMatchOp struct {
+	Matching []git.Hash
+}
+
+func (e ErrMultipleMatchOp) Error() string {
+	casted := make([]string, len(e.Matching))
+
+	for i := range e.Matching {
+		casted[i] = string(e.Matching[i])
+	}
+
+	return fmt.Sprintf("Multiple matching operation found:\n%s", strings.Join(casted, "\n"))
+}
+
+// ResolveTargetWithMetadata will find an operation that has the matching metadata
+func (c *BugCache) ResolveTargetWithMetadata(key string, value string) (git.Hash, error) {
+	// preallocate but empty
+	matching := make([]git.Hash, 0, 5)
+
+	it := bug.NewOperationIterator(c.bug)
+	for it.Next() {
+		op := it.Value()
+		opValue, ok := op.GetMetadata(key)
+		if ok && value == opValue {
+			h, err := op.Hash()
+			if err != nil {
+				return "", err
+			}
+			matching = append(matching, h)
+		}
+	}
+
+	if len(matching) == 0 {
+		return "", ErrNoMatchingOp
+	}
+
+	if len(matching) > 1 {
+		return "", ErrMultipleMatchOp{Matching: matching}
+	}
+
+	return matching[0], nil
+}
+
 func (c *BugCache) AddComment(message string) error {
 	return c.AddCommentWithFiles(message, nil)
 }

cache/repo_cache.go 🔗

@@ -244,7 +244,31 @@ func (c *RepoCache) ResolveBugPrefix(prefix string) (*BugCache, error) {
 	}
 
 	if len(matching) > 1 {
-		return nil, fmt.Errorf("Multiple matching bug found:\n%s", strings.Join(matching, "\n"))
+		return nil, bug.ErrMultipleMatch{Matching: matching}
+	}
+
+	if len(matching) == 0 {
+		return nil, bug.ErrBugNotExist
+	}
+
+	return c.ResolveBug(matching[0])
+}
+
+// ResolveBugCreateMetadata retrieve a bug that has the exact given metadata on
+// its Create operation, that is, the first operation. It fails if multiple bugs
+// match.
+func (c *RepoCache) ResolveBugCreateMetadata(key string, value string) (*BugCache, error) {
+	// preallocate but empty
+	matching := make([]string, 0, 5)
+
+	for id, excerpt := range c.excerpts {
+		if excerpt.CreateMetadata[key] == value {
+			matching = append(matching, id)
+		}
+	}
+
+	if len(matching) > 1 {
+		return nil, bug.ErrMultipleMatch{Matching: matching}
 	}
 
 	if len(matching) == 0 {