Add old importer comments in the iterator

Amine Hilaly created 6 years ago

Test operation authors
Fix typo in test repo url

Change summary

bridge/github/import.go      |  4 ++++
bridge/github/import_test.go | 27 +++++++++++++++++----------
bridge/github/iterator.go    | 13 ++++++++++++-
3 files changed, 33 insertions(+), 11 deletions(-)

Detailed changes

bridge/github/import.go

@@ -42,6 +42,8 @@ func (gi *githubImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro
 		// get issue edits
 		issueEdits := []userContentEdit{}
 		for iterator.NextIssueEdit() {
+			// issueEdit.Diff == nil happen if the event is older than early 2018, Github doesn't have the data before that.
+			// Best we can do is to ignore the event.
 			if issueEdit := iterator.IssueEditValue(); issueEdit.Diff != nil && string(*issueEdit.Diff) != "" {
 				issueEdits = append(issueEdits, issueEdit)
 			}
@@ -130,6 +132,7 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline
 		// create bug from given issueEdits
 		for i, edit := range issueEdits {
 			if i == 0 && b != nil {
+				// The first edit in the github result is the issue creation itself, we already have that
 				continue
 			}
 
@@ -323,6 +326,7 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.
 	} else {
 		for i, edit := range item.UserContentEdits.Nodes {
 			if i == 0 && target != "" {
+				// The first edit in the github result is the comment creation itself, we already have that
 				continue
 			}
 

bridge/github/import_test.go

@@ -26,7 +26,7 @@ func Test_Importer(t *testing.T) {
 	}{
 		{
 			name: "simple issue",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/1",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/1",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "simple issue", "initial comment", nil),
@@ -36,7 +36,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "empty issue",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/2",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/2",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "empty issue", "", nil),
@@ -45,7 +45,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "complex issue",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/3",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/3",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "complex issue", "initial comment", nil),
@@ -62,7 +62,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "editions",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/4",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/4",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "editions", "initial comment edited", nil),
@@ -74,7 +74,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "comment deletion",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/5",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/5",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "comment deletion", "", nil),
@@ -83,7 +83,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "edition deletion",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/6",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/6",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "edition deletion", "initial comment", nil),
@@ -95,7 +95,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "hidden comment",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/7",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/7",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "hidden comment", "initial comment", nil),
@@ -105,7 +105,7 @@ func Test_Importer(t *testing.T) {
 		},
 		{
 			name: "transfered issue",
-			url:  "https://github.com/MichaelMure/git-but-test-github-bridge/issues/8",
+			url:  "https://github.com/MichaelMure/git-bug-test-github-bridge/issues/8",
 			bug: &bug.Snapshot{
 				Operations: []bug.Operation{
 					bug.NewCreateOp(author, 0, "transfered issue", "", nil),
@@ -130,7 +130,7 @@ func Test_Importer(t *testing.T) {
 	importer := &githubImporter{}
 	err = importer.Init(core.Configuration{
 		"user":    "MichaelMure",
-		"project": "git-but-test-github-bridge",
+		"project": "git-bug-test-github-bridge",
 		"token":   token,
 	})
 	require.NoError(t, err)
@@ -153,24 +153,31 @@ func Test_Importer(t *testing.T) {
 			assert.Len(t, tt.bug.Operations, len(b.Snapshot().Operations))
 
 			for i, op := range tt.bug.Operations {
-				assert.IsType(t, ops[i], op)
+				require.IsType(t, ops[i], op)
 
 				switch op.(type) {
 				case *bug.CreateOperation:
 					assert.Equal(t, ops[i].(*bug.CreateOperation).Title, op.(*bug.CreateOperation).Title)
 					assert.Equal(t, ops[i].(*bug.CreateOperation).Message, op.(*bug.CreateOperation).Message)
+					assert.Equal(t, ops[i].(*bug.CreateOperation).Author.Name(), op.(*bug.CreateOperation).Author.Name())
 				case *bug.SetStatusOperation:
 					assert.Equal(t, ops[i].(*bug.SetStatusOperation).Status, op.(*bug.SetStatusOperation).Status)
+					assert.Equal(t, ops[i].(*bug.SetStatusOperation).Author.Name(), op.(*bug.SetStatusOperation).Author.Name())
 				case *bug.SetTitleOperation:
 					assert.Equal(t, ops[i].(*bug.SetTitleOperation).Was, op.(*bug.SetTitleOperation).Was)
 					assert.Equal(t, ops[i].(*bug.SetTitleOperation).Title, op.(*bug.SetTitleOperation).Title)
+					assert.Equal(t, ops[i].(*bug.SetTitleOperation).Author.Name(), op.(*bug.SetTitleOperation).Author.Name())
 				case *bug.LabelChangeOperation:
 					assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Added, op.(*bug.LabelChangeOperation).Added)
 					assert.ElementsMatch(t, ops[i].(*bug.LabelChangeOperation).Removed, op.(*bug.LabelChangeOperation).Removed)
+					assert.Equal(t, ops[i].(*bug.LabelChangeOperation).Author.Name(), op.(*bug.LabelChangeOperation).Author.Name())
 				case *bug.AddCommentOperation:
 					assert.Equal(t, ops[i].(*bug.AddCommentOperation).Message, op.(*bug.AddCommentOperation).Message)
+					assert.Equal(t, ops[i].(*bug.AddCommentOperation).Author.Name(), op.(*bug.AddCommentOperation).Author.Name())
 				case *bug.EditCommentOperation:
 					assert.Equal(t, ops[i].(*bug.EditCommentOperation).Message, op.(*bug.EditCommentOperation).Message)
+					assert.Equal(t, ops[i].(*bug.EditCommentOperation).Author.Name(), op.(*bug.EditCommentOperation).Author.Name())
+
 				default:
 					panic("Unknown operation type")
 				}

bridge/github/iterator.go

@@ -100,6 +100,8 @@ func (i *iterator) initTimelineQueryVariables() {
 	i.timeline.variables["issueSince"] = githubv4.DateTime{Time: i.since}
 	i.timeline.variables["timelineFirst"] = githubv4.Int(i.capacity)
 	i.timeline.variables["timelineAfter"] = (*githubv4.String)(nil)
+	// Fun fact, github provide the comment edition in reverse chronological
+	// order, because haha. Look at me, I'm dying of laughter.
 	i.timeline.variables["issueEditLast"] = githubv4.Int(i.capacity)
 	i.timeline.variables["issueEditBefore"] = (*githubv4.String)(nil)
 	i.timeline.variables["commentEditLast"] = githubv4.Int(i.capacity)
@@ -278,7 +280,16 @@ func (i *iterator) NextIssueEdit() bool {
 		return i.queryIssueEdit()
 	}
 
-	// if there is no edits
+	// 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.
+	//
+	// the tricky part: for an issue older than the UserContentEdits API, github
+	// doesn't have the previous message version anymore and give an edition
+	// with .Diff == nil. We have to filter them.
 	if len(i.timeline.query.Repository.Issues.Nodes[0].UserContentEdits.Nodes) == 0 {
 		return false
 	}