Repository: git-bug/git-bug (language: go)

Issue:
Import: issue edits and comment edits incorrect
I am importing the issues from https://github.com/rng-dynamics/gb-test-two.

When I use git-bug from the current master branch (bc5f618e), then the import is not correct: The issue https://github.com/rng-dynamics/gb-test-two/issues/2 is missing issue edits and comment edits:

```
 [open] Alexander Scharinger opened this bug on Apr 9 2021

     comment 1


 Alexander Scharinger commented on Apr 9 2021

     comment 3
```

Some time ago it used to work. E.g., on 01b9490 the import is correct:
```
 [open] Alexander Scharinger opened this bug on Apr 9 2021 (edited)

     comment 1, edit 2


 Alexander Scharinger commented on Apr 9 2021 (edited)

     comment 3, edit 4
```


Discussion / hints:
Is this fixed in your now merged branch?
To me it seems that the error is not in the github bridge but somewhere else. So, no, it's not fixed. It has been introduced somewhere between 01b9490 and bc5f618. I tried to bisect it, but was not successful in locating it yet.
I think I found the problem. I will send you a PR soon.

Fix patch:
diff --git a/bug/comment.go b/bug/comment.go
index 4c9d118eb..c1cfc7e53 100644
--- a/bug/comment.go
+++ b/bug/comment.go
@@ -11,6 +11,8 @@ import (
 
 // Comment represent a comment in a Bug
 type Comment struct {
+	// id should be the result of entity.CombineIds with the Bug id and the id
+	// of the Operation that created the comment
 	id      entity.Id
 	Author  identity.Interface
 	Message string
@@ -24,9 +26,8 @@ type Comment struct {
 // Id return the Comment identifier
 func (c Comment) Id() entity.Id {
 	if c.id == "" {
-		// simply panic as it would be a coding error
-		// (using an id of an identity not stored yet)
-		panic("no id yet")
+		// simply panic as it would be a coding error (no id provided at construction)
+		panic("no id")
 	}
 	return c.id
 }
diff --git a/bug/op_add_comment.go b/bug/op_add_comment.go
index f835866b5..15e622263 100644
--- a/bug/op_add_comment.go
+++ b/bug/op_add_comment.go
@@ -31,9 +31,8 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author_)
 	snapshot.addParticipant(op.Author_)
 
-	commentId := entity.CombineIds(snapshot.Id(), op.Id())
 	comment := Comment{
-		id:       commentId,
+		id:       entity.CombineIds(snapshot.Id(), op.Id()),
 		Message:  op.Message,
 		Author:   op.Author_,
 		Files:    op.Files,
@@ -43,7 +42,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.Comments = append(snapshot.Comments, comment)
 
 	item := &AddCommentTimelineItem{
-		CommentTimelineItem: NewCommentTimelineItem(commentId, comment),
+		CommentTimelineItem: NewCommentTimelineItem(comment),
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)
diff --git a/bug/op_create.go b/bug/op_create.go
index 75b60bd8b..fefde66a1 100644
--- a/bug/op_create.go
+++ b/bug/op_create.go
@@ -56,9 +56,8 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 
 	snapshot.Title = op.Title
 
-	commentId := entity.CombineIds(snapshot.Id(), op.Id())
 	comment := Comment{
-		id:       commentId,
+		id:       entity.CombineIds(snapshot.Id(), op.Id()),
 		Message:  op.Message,
 		Author:   op.Author_,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
@@ -70,7 +69,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 
 	snapshot.Timeline = []TimelineItem{
 		&CreateTimelineItem{
-			CommentTimelineItem: NewCommentTimelineItem(commentId, comment),
+			CommentTimelineItem: NewCommentTimelineItem(comment),
 		},
 	}
 }
diff --git a/bug/op_edit_comment.go b/bug/op_edit_comment.go
index 3e6634e4c..a69cb599e 100644
--- a/bug/op_edit_comment.go
+++ b/bug/op_edit_comment.go
@@ -34,12 +34,12 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	// Todo: currently any message can be edited, even by a different author
 	// crypto signature are needed.
 
-	snapshot.addActor(op.Author_)
+	// Recreate the Comment Id to match on
+	commentId := entity.CombineIds(snapshot.Id(), op.Target)
 
 	var target TimelineItem
-
 	for i, item := range snapshot.Timeline {
-		if item.Id() == op.Target {
+		if item.Id() == commentId {
 			target = snapshot.Timeline[i]
 			break
 		}
@@ -51,7 +51,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	}
 
 	comment := Comment{
-		id:       op.Target,
+		id:       commentId,
 		Message:  op.Message,
 		Files:    op.Files,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
@@ -62,12 +62,18 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 		target.Append(comment)
 	case *AddCommentTimelineItem:
 		target.Append(comment)
+	default:
+		// somehow, the target matched on something that is not a comment
+		// we make the op a no-op
+		return
 	}
 
+	snapshot.addActor(op.Author_)
+
 	// Updating the corresponding comment
 
 	for i := range snapshot.Comments {
-		if snapshot.Comments[i].Id() == op.Target {
+		if snapshot.Comments[i].Id() == commentId {
 			snapshot.Comments[i].Message = op.Message
 			snapshot.Comments[i].Files = op.Files
 			break
diff --git a/bug/snapshot.go b/bug/snapshot.go
index 0005b9301..ce84cce12 100644
--- a/bug/snapshot.go
+++ b/bug/snapshot.go
@@ -29,9 +29,8 @@ type Snapshot struct {
 // Return the Bug identifier
 func (snap *Snapshot) Id() entity.Id {
 	if snap.id == "" {
-		// simply panic as it would be a coding error
-		// (using an id of a bug not stored yet)
-		panic("no id yet")
+		// simply panic as it would be a coding error (no id provided at construction)
+		panic("no id")
 	}
 	return snap.id
 }
diff --git a/bug/timeline.go b/bug/timeline.go
index 5a51f516b..a5ca4da5a 100644
--- a/bug/timeline.go
+++ b/bug/timeline.go
@@ -26,6 +26,7 @@ type CommentHistoryStep struct {
 
 // CommentTimelineItem is a TimelineItem that holds a Comment and its edition history
 type CommentTimelineItem struct {
+	// id should be the same as in Comment
 	id        entity.Id
 	Author    identity.Interface
 	Message   string
@@ -35,9 +36,9 @@ type CommentTimelineItem struct {
 	History   []CommentHistoryStep
 }
 
-func NewCommentTimelineItem(ID entity.Id, comment Comment) CommentTimelineItem {
+func NewCommentTimelineItem(comment Comment) CommentTimelineItem {
 	return CommentTimelineItem{
-		id:        ID,
+		id:        comment.id,
 		Author:    comment.Author,
 		Message:   comment.Message,
 		Files:     comment.Files,


Test patch:
diff --git a/bug/op_create_test.go b/bug/op_create_test.go
index 1b359deea..25b87cfe2 100644
--- a/bug/op_create_test.go
+++ b/bug/op_create_test.go
@@ -30,10 +30,8 @@ func TestCreate(t *testing.T) {
 	id := create.Id()
 	require.NoError(t, id.Validate())
 
-	commentId := entity.CombineIds(create.Id(), create.Id())
-
 	comment := Comment{
-		id:       commentId,
+		id:       entity.CombineIds(create.Id(), create.Id()),
 		Author:   rene,
 		Message:  "message",
 		UnixTime: timestamp.Timestamp(create.UnixTime),
@@ -51,7 +49,7 @@ func TestCreate(t *testing.T) {
 		CreateTime:   create.Time(),
 		Timeline: []TimelineItem{
 			&CreateTimelineItem{
-				CommentTimelineItem: NewCommentTimelineItem(commentId, comment),
+				CommentTimelineItem: NewCommentTimelineItem(comment),
 			},
 		},
 	}
diff --git a/bug/op_edit_comment_test.go b/bug/op_edit_comment_test.go
index 777f5f87a..5ba947065 100644
--- a/bug/op_edit_comment_test.go
+++ b/bug/op_edit_comment_test.go
@@ -24,14 +24,12 @@ func TestEdit(t *testing.T) {
 	create := NewCreateOp(rene, unix, "title", "create", nil)
 	create.Apply(&snapshot)
 
-	id1 := create.Id()
-	require.NoError(t, id1.Validate())
+	require.NoError(t, create.Id().Validate())
 
 	comment1 := NewAddCommentOp(rene, unix, "comment 1", nil)
 	comment1.Apply(&snapshot)
 
-	id2 := comment1.Id()
-	require.NoError(t, id2.Validate())
+	require.NoError(t, comment1.Id().Validate())
 
 	// add another unrelated op in between
 	setTitle := NewSetTitleOp(rene, unix, "edited title", "title")
@@ -40,10 +38,9 @@ func TestEdit(t *testing.T) {
 	comment2 := NewAddCommentOp(rene, unix, "comment 2", nil)
 	comment2.Apply(&snapshot)
 
-	id3 := comment2.Id()
-	require.NoError(t, id3.Validate())
+	require.NoError(t, comment2.Id().Validate())
 
-	edit := NewEditCommentOp(rene, unix, snapshot.Comments[0].Id(), "create edited", nil)
+	edit := NewEditCommentOp(rene, unix, create.Id(), "create edited", nil)
 	edit.Apply(&snapshot)
 
 	require.Len(t, snapshot.Timeline, 4)
@@ -54,7 +51,7 @@ func TestEdit(t *testing.T) {
 	require.Equal(t, snapshot.Comments[1].Message, "comment 1")
 	require.Equal(t, snapshot.Comments[2].Message, "comment 2")
 
-	edit2 := NewEditCommentOp(rene, unix, snapshot.Comments[1].Id(), "comment 1 edited", nil)
+	edit2 := NewEditCommentOp(rene, unix, comment1.Id(), "comment 1 edited", nil)
 	edit2.Apply(&snapshot)
 
 	require.Len(t, snapshot.Timeline, 4)
@@ -65,7 +62,7 @@ func TestEdit(t *testing.T) {
 	require.Equal(t, snapshot.Comments[1].Message, "comment 1 edited")
 	require.Equal(t, snapshot.Comments[2].Message, "comment 2")
 
-	edit3 := NewEditCommentOp(rene, unix, snapshot.Comments[2].Id(), "comment 2 edited", nil)
+	edit3 := NewEditCommentOp(rene, unix, comment2.Id(), "comment 2 edited", nil)
 	edit3.Apply(&snapshot)
 
 	require.Len(t, snapshot.Timeline, 4)