bug: have a type for combined ids, fix https://github.com/MichaelMure/git-bug/issues/653

10 files changed, 150 insertions, 130 deletions

diff --git a/entities/bug/comment.go b/entities/bug/comment.go
index fcf501ab..7835c5a8 100644
--- a/entities/bug/comment.go
+++ b/entities/bug/comment.go
@@ -11,34 +11,41 @@ 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
+	// combinedId should be the result of entity.CombineIds with the Bug id and the id
 	// of the Operation that created the comment
-	id      entity.Id
+	combinedId entity.CombinedId
+
+	// targetId is the Id of the Operation that originally created that Comment
+	targetId entity.Id
+
 	Author  identity.Interface
 	Message string
 	Files   []repository.Hash
 
 	// Creation time of the comment.
 	// Should be used only for human display, never for ordering as we can't rely on it in a distributed system.
-	UnixTime timestamp.Timestamp
+	unixTime timestamp.Timestamp
 }
 
-// Id return the Comment identifier
-func (c Comment) Id() entity.Id {
-	if c.id == "" {
+func (c Comment) CombinedId() entity.CombinedId {
+	if c.combinedId == "" {
 		// simply panic as it would be a coding error (no id provided at construction)
-		panic("no id")
+		panic("no combined id")
 	}
-	return c.id
+	return c.combinedId
+}
+
+func (c Comment) TargetId() entity.Id {
+	return c.targetId
 }
 
-// FormatTimeRel format the UnixTime of the comment for human consumption
+// FormatTimeRel format the unixTime of the comment for human consumption
 func (c Comment) FormatTimeRel() string {
-	return humanize.Time(c.UnixTime.Time())
+	return humanize.Time(c.unixTime.Time())
 }
 
 func (c Comment) FormatTime() string {
-	return c.UnixTime.Time().Format("Mon Jan 2 15:04:05 2006 +0200")
+	return c.unixTime.Time().Format("Mon Jan 2 15:04:05 2006 +0200")
 }
 
 // IsAuthored is a sign post method for gqlgen
diff --git a/entities/bug/op_add_comment.go b/entities/bug/op_add_comment.go
index 2e6a39f9..b049ef16 100644
--- a/entities/bug/op_add_comment.go
+++ b/entities/bug/op_add_comment.go
@@ -30,12 +30,15 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author())
 	snapshot.addParticipant(op.Author())
 
+	opId := op.Id()
+
 	comment := Comment{
-		id:       entity.CombineIds(snapshot.Id(), op.Id()),
-		Message:  op.Message,
-		Author:   op.Author(),
-		Files:    op.Files,
-		UnixTime: timestamp.Timestamp(op.UnixTime),
+		combinedId: entity.CombineIds(snapshot.Id(), opId),
+		targetId:   opId,
+		Message:    op.Message,
+		Author:     op.Author(),
+		Files:      op.Files,
+		unixTime:   timestamp.Timestamp(op.UnixTime),
 	}
 
 	snapshot.Comments = append(snapshot.Comments, comment)
@@ -71,7 +74,7 @@ func NewAddCommentOp(author identity.Interface, unixTime int64, message string,
 	}
 }
 
-// AddCommentTimelineItem hold a comment in the timeline
+// AddCommentTimelineItem replace a AddComment operation in the Timeline and hold its edition history
 type AddCommentTimelineItem struct {
 	CommentTimelineItem
 }
diff --git a/entities/bug/op_create.go b/entities/bug/op_create.go
index fdfa131b..2afea406 100644
--- a/entities/bug/op_create.go
+++ b/entities/bug/op_create.go
@@ -32,7 +32,9 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 		return
 	}
 
-	snapshot.id = op.Id()
+	// the Id of the Bug/Snapshot is the Id of the first Operation: CreateOperation
+	opId := op.Id()
+	snapshot.id = opId
 
 	snapshot.addActor(op.Author())
 	snapshot.addParticipant(op.Author())
@@ -40,10 +42,11 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 	snapshot.Title = op.Title
 
 	comment := Comment{
-		id:       entity.CombineIds(snapshot.Id(), op.Id()),
-		Message:  op.Message,
-		Author:   op.Author(),
-		UnixTime: timestamp.Timestamp(op.UnixTime),
+		combinedId: entity.CombineIds(snapshot.id, opId),
+		targetId:   opId,
+		Message:    op.Message,
+		Author:     op.Author(),
+		unixTime:   timestamp.Timestamp(op.UnixTime),
 	}
 
 	snapshot.Comments = []Comment{comment}
diff --git a/entities/bug/op_create_test.go b/entities/bug/op_create_test.go
index f2c9e675..e534162b 100644
--- a/entities/bug/op_create_test.go
+++ b/entities/bug/op_create_test.go
@@ -6,55 +6,37 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/MichaelMure/git-bug/entities/common"
 	"github.com/MichaelMure/git-bug/entities/identity"
 	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/entity/dag"
 	"github.com/MichaelMure/git-bug/repository"
-	"github.com/MichaelMure/git-bug/util/timestamp"
 )
 
 func TestCreate(t *testing.T) {
-	snapshot := Snapshot{}
-
-	repo := repository.NewMockRepoClock()
+	repo := repository.NewMockRepo()
 
 	rene, err := identity.NewIdentity(repo, "René Descartes", "rene@descartes.fr")
 	require.NoError(t, err)
 
-	unix := time.Now().Unix()
-
-	create := NewCreateOp(rene, unix, "title", "message", nil)
-
-	create.Apply(&snapshot)
-
-	id := create.Id()
-	require.NoError(t, id.Validate())
-
-	comment := Comment{
-		id:       entity.CombineIds(create.Id(), create.Id()),
-		Author:   rene,
-		Message:  "message",
-		UnixTime: timestamp.Timestamp(create.UnixTime),
-	}
-
-	expected := Snapshot{
-		id:    create.Id(),
-		Title: "title",
-		Comments: []Comment{
-			comment,
-		},
-		Author:       rene,
-		Participants: []identity.Interface{rene},
-		Actors:       []identity.Interface{rene},
-		CreateTime:   create.Time(),
-		Timeline: []TimelineItem{
-			&CreateTimelineItem{
-				CommentTimelineItem: NewCommentTimelineItem(comment),
-			},
-		},
-	}
+	b, op, err := Create(rene, time.Now().Unix(), "title", "message", nil, nil)
+	require.NoError(t, err)
 
-	require.Equal(t, expected, snapshot)
+	require.Equal(t, "title", op.Title)
+	require.Equal(t, "message", op.Message)
+
+	// Create generate the initial operation and create a new timeline item
+	snap := b.Compile()
+	require.Equal(t, common.OpenStatus, snap.Status)
+	require.Equal(t, rene, snap.Author)
+	require.Equal(t, "title", snap.Title)
+	require.Len(t, snap.Operations, 1)
+	require.Equal(t, op, snap.Operations[0])
+
+	require.Len(t, snap.Timeline, 1)
+	require.Equal(t, entity.CombineIds(b.Id(), op.Id()), snap.Timeline[0].CombinedId())
+	require.Equal(t, rene, snap.Timeline[0].(*CreateTimelineItem).Author)
+	require.Equal(t, "message", snap.Timeline[0].(*CreateTimelineItem).Message)
 }
 
 func TestCreateSerialize(t *testing.T) {
diff --git a/entities/bug/op_edit_comment.go b/entities/bug/op_edit_comment.go
index 41079f45..b0897b0a 100644
--- a/entities/bug/op_edit_comment.go
+++ b/entities/bug/op_edit_comment.go
@@ -33,12 +33,12 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	// Todo: currently any message can be edited, even by a different author
 	// crypto signature are needed.
 
-	// Recreate the Comment Id to match on
-	commentId := entity.CombineIds(snapshot.Id(), op.Target)
+	// Recreate the combined Id to match on
+	combinedId := entity.CombineIds(snapshot.Id(), op.Target)
 
 	var target TimelineItem
 	for i, item := range snapshot.Timeline {
-		if item.Id() == commentId {
+		if item.CombinedId() == combinedId {
 			target = snapshot.Timeline[i]
 			break
 		}
@@ -50,10 +50,11 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	}
 
 	comment := Comment{
-		id:       commentId,
-		Message:  op.Message,
-		Files:    op.Files,
-		UnixTime: timestamp.Timestamp(op.UnixTime),
+		combinedId: combinedId,
+		targetId:   op.Target,
+		Message:    op.Message,
+		Files:      op.Files,
+		unixTime:   timestamp.Timestamp(op.UnixTime),
 	}
 
 	switch target := target.(type) {
@@ -72,7 +73,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	// Updating the corresponding comment
 
 	for i := range snapshot.Comments {
-		if snapshot.Comments[i].Id() == commentId {
+		if snapshot.Comments[i].CombinedId() == combinedId {
 			snapshot.Comments[i].Message = op.Message
 			snapshot.Comments[i].Files = op.Files
 			break
diff --git a/entities/bug/op_label_change.go b/entities/bug/op_label_change.go
index 45441f7c..76b2ebef 100644
--- a/entities/bug/op_label_change.go
+++ b/entities/bug/op_label_change.go
@@ -59,12 +59,14 @@ AddLoop:
 		return string(snapshot.Labels[i]) < string(snapshot.Labels[j])
 	})
 
+	id := op.Id()
 	item := &LabelChangeTimelineItem{
-		id:       op.Id(),
-		Author:   op.Author(),
-		UnixTime: timestamp.Timestamp(op.UnixTime),
-		Added:    op.Added,
-		Removed:  op.Removed,
+		// id:         id,
+		combinedId: entity.CombineIds(snapshot.Id(), id),
+		Author:     op.Author(),
+		UnixTime:   timestamp.Timestamp(op.UnixTime),
+		Added:      op.Added,
+		Removed:    op.Removed,
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)
@@ -103,19 +105,20 @@ func NewLabelChangeOperation(author identity.Interface, unixTime int64, added, r
 }
 
 type LabelChangeTimelineItem struct {
-	id       entity.Id
-	Author   identity.Interface
-	UnixTime timestamp.Timestamp
-	Added    []Label
-	Removed  []Label
+	// id         entity.Id
+	combinedId entity.CombinedId
+	Author     identity.Interface
+	UnixTime   timestamp.Timestamp
+	Added      []Label
+	Removed    []Label
 }
 
-func (l LabelChangeTimelineItem) Id() entity.Id {
-	return l.id
+func (l LabelChangeTimelineItem) CombinedId() entity.CombinedId {
+	return l.combinedId
 }
 
 // IsAuthored is a sign post method for gqlgen
-func (l LabelChangeTimelineItem) IsAuthored() {}
+func (l *LabelChangeTimelineItem) IsAuthored() {}
 
 // ChangeLabels is a convenience function to change labels on a bug
 func ChangeLabels(b Interface, author identity.Interface, unixTime int64, add, remove []string, metadata map[string]string) ([]LabelChangeResult, *LabelChangeOperation, error) {
@@ -180,7 +183,7 @@ func ChangeLabels(b Interface, author identity.Interface, unixTime int64, add, r
 }
 
 // ForceChangeLabels is a convenience function to apply the operation
-// The difference with ChangeLabels is that no checks of deduplications are done. You are entirely
+// The difference with ChangeLabels is that no checks for deduplication are done. You are entirely
 // responsible for what you are doing. In the general case, you want to use ChangeLabels instead.
 // The intended use of this function is to allow importers to create legal but unexpected label changes,
 // like removing a label with no information of when it was added before.
diff --git a/entities/bug/op_set_status.go b/entities/bug/op_set_status.go
index cf17901a..23be59a0 100644
--- a/entities/bug/op_set_status.go
+++ b/entities/bug/op_set_status.go
@@ -26,11 +26,13 @@ func (op *SetStatusOperation) Apply(snapshot *Snapshot) {
 	snapshot.Status = op.Status
 	snapshot.addActor(op.Author())
 
+	id := op.Id()
 	item := &SetStatusTimelineItem{
-		id:       op.Id(),
-		Author:   op.Author(),
-		UnixTime: timestamp.Timestamp(op.UnixTime),
-		Status:   op.Status,
+		// id:         id,
+		combinedId: entity.CombineIds(snapshot.Id(), id),
+		Author:     op.Author(),
+		UnixTime:   timestamp.Timestamp(op.UnixTime),
+		Status:     op.Status,
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)
@@ -56,18 +58,19 @@ func NewSetStatusOp(author identity.Interface, unixTime int64, status common.Sta
 }
 
 type SetStatusTimelineItem struct {
-	id       entity.Id
-	Author   identity.Interface
-	UnixTime timestamp.Timestamp
-	Status   common.Status
+	// id         entity.Id
+	combinedId entity.CombinedId
+	Author     identity.Interface
+	UnixTime   timestamp.Timestamp
+	Status     common.Status
 }
 
-func (s SetStatusTimelineItem) Id() entity.Id {
-	return s.id
+func (s SetStatusTimelineItem) CombinedId() entity.CombinedId {
+	return s.combinedId
 }
 
 // IsAuthored is a sign post method for gqlgen
-func (s SetStatusTimelineItem) IsAuthored() {}
+func (s *SetStatusTimelineItem) IsAuthored() {}
 
 // Open is a convenience function to change a bugs state to Open
 func Open(b Interface, author identity.Interface, unixTime int64, metadata map[string]string) (*SetStatusOperation, error) {
diff --git a/entities/bug/op_set_title.go b/entities/bug/op_set_title.go
index 75efd08e..e9526b64 100644
--- a/entities/bug/op_set_title.go
+++ b/entities/bug/op_set_title.go
@@ -28,12 +28,14 @@ func (op *SetTitleOperation) Apply(snapshot *Snapshot) {
 	snapshot.Title = op.Title
 	snapshot.addActor(op.Author())
 
+	id := op.Id()
 	item := &SetTitleTimelineItem{
-		id:       op.Id(),
-		Author:   op.Author(),
-		UnixTime: timestamp.Timestamp(op.UnixTime),
-		Title:    op.Title,
-		Was:      op.Was,
+		id:         id,
+		combinedId: entity.CombineIds(snapshot.Id(), id),
+		Author:     op.Author(),
+		UnixTime:   timestamp.Timestamp(op.UnixTime),
+		Title:      op.Title,
+		Was:        op.Was,
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)
@@ -68,19 +70,24 @@ func NewSetTitleOp(author identity.Interface, unixTime int64, title string, was
 }
 
 type SetTitleTimelineItem struct {
-	id       entity.Id
-	Author   identity.Interface
-	UnixTime timestamp.Timestamp
-	Title    string
-	Was      string
+	id         entity.Id
+	combinedId entity.CombinedId
+	Author     identity.Interface
+	UnixTime   timestamp.Timestamp
+	Title      string
+	Was        string
 }
 
 func (s SetTitleTimelineItem) Id() entity.Id {
 	return s.id
 }
 
+func (s SetTitleTimelineItem) CombinedId() entity.CombinedId {
+	return s.combinedId
+}
+
 // IsAuthored is a sign post method for gqlgen
-func (s SetTitleTimelineItem) IsAuthored() {}
+func (s *SetTitleTimelineItem) IsAuthored() {}
 
 // SetTitle is a convenience function to change a bugs title
 func SetTitle(b Interface, author identity.Interface, unixTime int64, title string, metadata map[string]string) (*SetTitleOperation, error) {
diff --git a/entities/bug/snapshot.go b/entities/bug/snapshot.go
index 442496f7..333fe207 100644
--- a/entities/bug/snapshot.go
+++ b/entities/bug/snapshot.go
@@ -58,9 +58,9 @@ func (snap *Snapshot) GetCreateMetadata(key string) (string, bool) {
 }
 
 // SearchTimelineItem will search in the timeline for an item matching the given hash
-func (snap *Snapshot) SearchTimelineItem(id entity.Id) (TimelineItem, error) {
+func (snap *Snapshot) SearchTimelineItem(id entity.CombinedId) (TimelineItem, error) {
 	for i := range snap.Timeline {
-		if snap.Timeline[i].Id() == id {
+		if snap.Timeline[i].CombinedId() == id {
 			return snap.Timeline[i], nil
 		}
 	}
@@ -68,15 +68,26 @@ func (snap *Snapshot) SearchTimelineItem(id entity.Id) (TimelineItem, error) {
 	return nil, fmt.Errorf("timeline item not found")
 }
 
-// SearchComment will search for a comment matching the given hash
-func (snap *Snapshot) SearchComment(id entity.Id) (*Comment, error) {
+// SearchComment will search for a comment matching the given id
+func (snap *Snapshot) SearchComment(id entity.CombinedId) (*Comment, error) {
 	for _, c := range snap.Comments {
-		if c.id == id {
+		if c.combinedId == id {
 			return &c, nil
 		}
 	}
 
-	return nil, fmt.Errorf("comment item not found")
+	return nil, fmt.Errorf("comment not found")
+}
+
+// SearchCommentByOpId will search for a comment generated by the given operation Id
+func (snap *Snapshot) SearchCommentByOpId(id entity.Id) (*Comment, error) {
+	for _, c := range snap.Comments {
+		if c.targetId == id {
+			return &c, nil
+		}
+	}
+
+	return nil, fmt.Errorf("comment not found")
 }
 
 // append the operation author to the actors list
diff --git a/entities/bug/timeline.go b/entities/bug/timeline.go
index d7f042db..84ece262 100644
--- a/entities/bug/timeline.go
+++ b/entities/bug/timeline.go
@@ -10,8 +10,8 @@ import (
 )
 
 type TimelineItem interface {
-	// Id return the identifier of the item
-	Id() entity.Id
+	// CombinedId returns the global identifier of the item
+	CombinedId() entity.CombinedId
 }
 
 // CommentHistoryStep hold one version of a message in the history
@@ -26,46 +26,46 @@ 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
-	Files     []repository.Hash
-	CreatedAt timestamp.Timestamp
-	LastEdit  timestamp.Timestamp
-	History   []CommentHistoryStep
+	combinedId entity.CombinedId
+	Author     identity.Interface
+	Message    string
+	Files      []repository.Hash
+	CreatedAt  timestamp.Timestamp
+	LastEdit   timestamp.Timestamp
+	History    []CommentHistoryStep
 }
 
 func NewCommentTimelineItem(comment Comment) CommentTimelineItem {
 	return CommentTimelineItem{
-		id:        comment.id,
-		Author:    comment.Author,
-		Message:   comment.Message,
-		Files:     comment.Files,
-		CreatedAt: comment.UnixTime,
-		LastEdit:  comment.UnixTime,
+		// id: comment.id,
+		combinedId: comment.combinedId,
+		Author:     comment.Author,
+		Message:    comment.Message,
+		Files:      comment.Files,
+		CreatedAt:  comment.unixTime,
+		LastEdit:   comment.unixTime,
 		History: []CommentHistoryStep{
 			{
 				Message:  comment.Message,
-				UnixTime: comment.UnixTime,
+				UnixTime: comment.unixTime,
 			},
 		},
 	}
 }
 
-func (c *CommentTimelineItem) Id() entity.Id {
-	return c.id
+func (c *CommentTimelineItem) CombinedId() entity.CombinedId {
+	return c.combinedId
 }
 
 // Append will append a new comment in the history and update the other values
 func (c *CommentTimelineItem) Append(comment Comment) {
 	c.Message = comment.Message
 	c.Files = comment.Files
-	c.LastEdit = comment.UnixTime
+	c.LastEdit = comment.unixTime
 	c.History = append(c.History, CommentHistoryStep{
 		Author:   comment.Author,
 		Message:  comment.Message,
-		UnixTime: comment.UnixTime,
+		UnixTime: comment.unixTime,
 	})
 }