Refactor combined id rework 3

api/graphql/resolvers/mutation.go

@@ -187,10 +187,22 @@ func (r mutationResolver) EditComment(ctx context.Context, input models.EditComm
 		return nil, err
 	}
 
+	// NOTE input.Target is here the comment id (a timeline item id) so must
+	// split the id, to get the operation id prefix.
+	_, opIdPrefix := entity.SeparateIds(string(input.Target))
+	var opId entity.Id
+	// Find the full operation id via the prefix
+	for _, operation := range b.Snapshot().Operations {
+		if operation.Id().HasPrefix(opIdPrefix) {
+			opId = operation.Id()
+			break
+		}
+	}
+
 	op, err := b.EditCommentRaw(
 		author,
 		time.Now().Unix(),
-		entity.Id(input.Target),
+		opId,
 		text.Cleanup(input.Message),
 		nil,
 	)

bridge/gitlab/import.go

@@ -217,17 +217,29 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 
 	case NOTE_DESCRIPTION_CHANGED:
 		issue := gi.iterator.IssueValue()
+		snap := b.Snapshot()
+		firstComment := snap.Comments[0]
 
-		firstComment := b.Snapshot().Comments[0]
 		// since gitlab doesn't provide the issue history
 		// we should check for "changed the description" notes and compare issue texts
 		// TODO: Check only one time and ignore next 'description change' within one issue
 		if errResolve == cache.ErrNoMatchingOp && issue.Description != firstComment.Message {
 			// comment edition
+
+			_, opIdPrefix := entity.SeparateIds(string(firstComment.Id()))
+			var opId entity.Id
+			// Find the full operation id via the extracted prefix
+			for _, operation := range snap.Operations {
+				if operation.Id().HasPrefix(opIdPrefix) {
+					opId = operation.Id()
+					break
+				}
+			}
+
 			op, err := b.EditCommentRaw(
 				author,
 				note.UpdatedAt.Unix(),
-				firstComment.Id(),
+				opId,
 				text.Cleanup(issue.Description),
 				map[string]string{
 					metaKeyGitlabId: gitlabID,
@@ -265,19 +277,32 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 
 		// if comment was already exported
 
-		// search for last comment update
-		comment, err := b.Snapshot().SearchComment(id)
+		//NOTE Is the result of ResolveOperation realy a comment id or an
+		// operation id? Asking because id is retrieved via
+		// b.ResolveOperationWithMetadata(metaKeyGitlabId, gitlabID)
+		// which should return an operation id?
+		comment, err := b.Snapshot().SearchComment(entity.CombinedId(id))
 		if err != nil {
 			return err
 		}
 
+		_, opIdPrefix := entity.SeparateIds(string(comment.Id()))
+		var opId entity.Id
+		// Find the full operation id via the extracted prefix
+		for _, operation := range b.Snapshot().Operations {
+			if operation.Id().HasPrefix(opIdPrefix) {
+				opId = operation.Id()
+				break
+			}
+		}
+
 		// compare local bug comment with the new note body
 		if comment.Message != cleanText {
 			// comment edition
 			op, err := b.EditCommentRaw(
 				author,
 				note.UpdatedAt.Unix(),
-				comment.Id(),
+				opId,
 				cleanText,
 				nil,
 			)

bug/comment.go

@@ -13,7 +13,7 @@ import (
 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
+	id      entity.CombinedId
 	Author  identity.Interface
 	Message string
 	Files   []repository.Hash
@@ -24,7 +24,7 @@ type Comment struct {
 }
 
 // Id return the Comment identifier
-func (c Comment) Id() entity.Id {
+func (c Comment) Id() entity.CombinedId {
 	if c.id == "" {
 		// simply panic as it would be a coding error (no id provided at construction)
 		panic("no id")

bug/op_label_change.go

@@ -58,7 +58,7 @@ AddLoop:
 	})
 
 	item := &LabelChangeTimelineItem{
-		id:       op.Id(),
+		id:       entity.CombineIds(snapshot.Id(), op.Id()),
 		Author:   op.Author_,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Added:    op.Added,
@@ -133,14 +133,14 @@ func NewLabelChangeOperation(author identity.Interface, unixTime int64, added, r
 }
 
 type LabelChangeTimelineItem struct {
-	id       entity.Id
+	id       entity.CombinedId
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Added    []Label
 	Removed  []Label
 }
 
-func (l LabelChangeTimelineItem) Id() entity.Id {
+func (l LabelChangeTimelineItem) Id() entity.CombinedId {
 	return l.id
 }
 
@@ -208,7 +208,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 of 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.

bug/op_set_status.go

@@ -27,7 +27,7 @@ func (op *SetStatusOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author_)
 
 	item := &SetStatusTimelineItem{
-		id:       op.Id(),
+		id:       entity.CombineIds(snapshot.Id(), op.Id()),
 		Author:   op.Author_,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Status:   op.Status,
@@ -86,13 +86,13 @@ func NewSetStatusOp(author identity.Interface, unixTime int64, status Status) *S
 }
 
 type SetStatusTimelineItem struct {
-	id       entity.Id
+	id       entity.CombinedId
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Status   Status
 }
 
-func (s SetStatusTimelineItem) Id() entity.Id {
+func (s SetStatusTimelineItem) Id() entity.CombinedId {
 	return s.id
 }
 

bug/op_set_title.go

@@ -29,7 +29,7 @@ func (op *SetTitleOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author_)
 
 	item := &SetTitleTimelineItem{
-		id:       op.Id(),
+		id:       entity.CombineIds(snapshot.Id(), op.Id()),
 		Author:   op.Author_,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Title:    op.Title,
@@ -100,14 +100,14 @@ func NewSetTitleOp(author identity.Interface, unixTime int64, title string, was
 }
 
 type SetTitleTimelineItem struct {
-	id       entity.Id
+	id       entity.CombinedId
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Title    string
 	Was      string
 }
 
-func (s SetTitleTimelineItem) Id() entity.Id {
+func (s SetTitleTimelineItem) Id() entity.CombinedId {
 	return s.id
 }
 

bug/snapshot.go

@@ -50,7 +50,7 @@ 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 {
 			return snap.Timeline[i], nil
@@ -61,7 +61,7 @@ func (snap *Snapshot) SearchTimelineItem(id entity.Id) (TimelineItem, error) {
 }
 
 // SearchComment will search for a comment matching the given hash
-func (snap *Snapshot) SearchComment(id entity.Id) (*Comment, error) {
+func (snap *Snapshot) SearchComment(id entity.CombinedId) (*Comment, error) {
 	for _, c := range snap.Comments {
 		if c.id == id {
 			return &c, nil

bug/timeline.go

@@ -11,7 +11,7 @@ import (
 
 type TimelineItem interface {
 	// ID return the identifier of the item
-	Id() entity.Id
+	Id() entity.CombinedId
 }
 
 // CommentHistoryStep hold one version of a message in the history
@@ -27,7 +27,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
+	id        entity.CombinedId
 	Author    identity.Interface
 	Message   string
 	Files     []repository.Hash
@@ -53,7 +53,7 @@ func NewCommentTimelineItem(comment Comment) CommentTimelineItem {
 	}
 }
 
-func (c *CommentTimelineItem) Id() entity.Id {
+func (c *CommentTimelineItem) Id() entity.CombinedId {
 	return c.id
 }
 

cache/repo_cache_bug.go

@@ -263,7 +263,7 @@ func (c *RepoCache) resolveBugMatcher(f func(*BugExcerpt) bool) (entity.Id, erro
 // ResolveComment search for a Bug/Comment combination matching the merged
 // bug/comment Id prefix. Returns the Bug containing the Comment and the Comment's
 // Id.
-func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.Id, error) {
+func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.CombinedId, error) {
 	bugPrefix, _ := entity.SeparateIds(prefix)
 	bugCandidate := make([]entity.Id, 0, 5)
 
@@ -277,7 +277,7 @@ func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.Id, error)
 	c.muBug.RUnlock()
 
 	matchingBugIds := make([]entity.Id, 0, 5)
-	matchingCommentId := entity.UnsetId
+	matchingCommentId := entity.UnsetCombinedId
 	var matchingBug *BugCache
 
 	// search for matching comments
@@ -286,7 +286,7 @@ func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.Id, error)
 	for _, bugId := range bugCandidate {
 		b, err := c.ResolveBug(bugId)
 		if err != nil {
-			return nil, entity.UnsetId, err
+			return nil, entity.UnsetCombinedId, err
 		}
 
 		for _, comment := range b.Snapshot().Comments {
@@ -299,9 +299,9 @@ func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.Id, error)
 	}
 
 	if len(matchingBugIds) > 1 {
-		return nil, entity.UnsetId, entity.NewErrMultipleMatch("bug/comment", matchingBugIds)
+		return nil, entity.UnsetCombinedId, entity.NewErrMultipleMatch("bug/comment", matchingBugIds)
 	} else if len(matchingBugIds) == 0 {
-		return nil, entity.UnsetId, errors.New("comment doesn't exist")
+		return nil, entity.UnsetCombinedId, errors.New("comment doesn't exist")
 	}
 
 	return matchingBug, matchingCommentId, nil

commands/comment_edit.go

@@ -3,6 +3,7 @@ package commands
 import (
 	"github.com/spf13/cobra"
 
+	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/input"
 )
 
@@ -67,7 +68,17 @@ func runCommentEdit(env *Env, opts commentEditOptions, args []string) error {
 		}
 	}
 
-	_, err = b.EditComment(commentId, opts.message)
+	_, opIdPrefix := entity.SeparateIds(string(commentId))
+	var opId entity.Id
+	// Find the full operation id via the extracted prefix
+	for _, operation := range b.Snapshot().Operations {
+		if operation.Id().HasPrefix(opIdPrefix) {
+			opId = operation.Id()
+			break
+		}
+	}
+
+	_, err = b.EditComment(opId, opts.message)
 	if err != nil {
 		return err
 	}

entity/id.go

@@ -79,21 +79,3 @@ func (i Id) Validate() error {
 	}
 	return nil
 }
-
-/*
- * Sorting
- */
-
-type Alphabetical []Id
-
-func (a Alphabetical) Len() int {
-	return len(a)
-}
-
-func (a Alphabetical) Less(i, j int) bool {
-	return a[i] < a[j]
-}
-
-func (a Alphabetical) Swap(i, j int) {
-	a[i], a[j] = a[j], a[i]
-}