bridge/gitlab: fix comment edition target hash in the import

Amine Hilaly created

bridge/gitlab: global changes, typo fixes, comments addition

Change summary

bridge/gitlab/gitlab.go       |   7 +-
bridge/gitlab/import.go       | 103 ++++++++++++++++--------------------
bridge/gitlab/import_notes.go |   2 
bridge/gitlab/iterator.go     |  24 +++-----
4 files changed, 61 insertions(+), 75 deletions(-)

Detailed changes

bridge/gitlab/gitlab.go 🔗

@@ -12,9 +12,10 @@ import (
 const (
 	target = "gitlab"
 
-	keyGitlabId    = "gitlab-id"
-	keyGitlabUrl   = "gitlab-url"
-	keyGitlabLogin = "gitlab-login"
+	keyGitlabId      = "gitlab-id"
+	keyGitlabUrl     = "gitlab-url"
+	keyGitlabLogin   = "gitlab-login"
+	keyGitlabProject = "gitlab-project-id"
 
 	keyProjectID = "project-id"
 	keyToken     = "token"

bridge/gitlab/import.go 🔗

@@ -11,6 +11,7 @@ import (
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/cache"
 	"github.com/MichaelMure/git-bug/identity"
+	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/text"
 )
 
@@ -65,17 +66,17 @@ func (gi *gitlabImporter) ImportAll(repo *cache.RepoCache, since time.Time) erro
 			}
 		}
 
+		if err := gi.iterator.Error(); err != nil {
+			fmt.Printf("import error: %v\n", err)
+			return err
+		}
+
 		// commit bug state
 		if err := b.CommitAsNeeded(); err != nil {
 			return fmt.Errorf("bug commit: %v", err)
 		}
 	}
 
-	if err := gi.iterator.Error(); err != nil {
-		fmt.Printf("import error: %v\n", err)
-		return err
-	}
-
 	fmt.Printf("Successfully imported %d issues and %d identities from Gitlab\n", gi.importedIssues, gi.importedIdentities)
 	return nil
 }
@@ -93,37 +94,38 @@ func (gi *gitlabImporter) ensureIssue(repo *cache.RepoCache, issue *gitlab.Issue
 		return nil, err
 	}
 
-	// if bug was never imported
-	if err == bug.ErrBugNotExist {
-		cleanText, err := text.Cleanup(issue.Description)
-		if err != nil {
-			return nil, err
-		}
-
-		// create bug
-		b, _, err = repo.NewBugRaw(
-			author,
-			issue.CreatedAt.Unix(),
-			issue.Title,
-			cleanText,
-			nil,
-			map[string]string{
-				core.KeyOrigin: target,
-				keyGitlabId:    parseID(issue.ID),
-				keyGitlabUrl:   issue.WebURL,
-			},
-		)
+	if err == nil {
+		return b, nil
+	}
 
-		if err != nil {
-			return nil, err
-		}
+	// if bug was never imported
+	cleanText, err := text.Cleanup(issue.Description)
+	if err != nil {
+		return nil, err
+	}
 
-		// importing a new bug
-		gi.importedIssues++
+	// create bug
+	b, _, err = repo.NewBugRaw(
+		author,
+		issue.CreatedAt.Unix(),
+		issue.Title,
+		cleanText,
+		nil,
+		map[string]string{
+			core.KeyOrigin:   target,
+			keyGitlabId:      parseID(issue.ID),
+			keyGitlabUrl:     issue.WebURL,
+			keyGitlabProject: gi.conf[keyProjectID],
+		},
+	)
 
-		return b, nil
+	if err != nil {
+		return nil, err
 	}
 
+	// importing a new bug
+	gi.importedIssues++
+
 	return b, nil
 }
 
@@ -138,7 +140,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 
 	hash, errResolve := b.ResolveOperationWithMetadata(keyGitlabId, id)
 	if errResolve != cache.ErrNoMatchingOp {
-		return err
+		return errResolve
 	}
 
 	noteType, body := GetNoteType(note)
@@ -148,8 +150,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 			author,
 			note.CreatedAt.Unix(),
 			map[string]string{
-				keyGitlabId:  id,
-				keyGitlabUrl: "",
+				keyGitlabId: id,
 			},
 		)
 		return err
@@ -159,8 +160,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 			author,
 			note.CreatedAt.Unix(),
 			map[string]string{
-				keyGitlabId:  id,
-				keyGitlabUrl: "",
+				keyGitlabId: id,
 			},
 		)
 		return err
@@ -168,25 +168,24 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 	case NOTE_DESCRIPTION_CHANGED:
 		issue := gi.iterator.IssueValue()
 
+		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 issue.Description != b.Snapshot().Comments[0].Message {
+		if issue.Description != firstComment.Message {
 
 			// comment edition
 			_, err = b.EditCommentRaw(
 				author,
 				note.UpdatedAt.Unix(),
-				target,
+				git.Hash(firstComment.Id()),
 				issue.Description,
 				map[string]string{
-					keyGitlabId:  id,
-					keyGitlabUrl: "",
+					keyGitlabId: id,
 				},
 			)
 
 			return err
-
 		}
 
 	case NOTE_COMMENT:
@@ -206,8 +205,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 				cleanText,
 				nil,
 				map[string]string{
-					keyGitlabId:  id,
-					keyGitlabUrl: "",
+					keyGitlabId: id,
 				},
 			)
 
@@ -216,11 +214,6 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 
 		// if comment was already exported
 
-		// if note wasn't updated
-		if note.UpdatedAt.Equal(*note.CreatedAt) {
-			return nil
-		}
-
 		// search for last comment update
 		comment, err := b.Snapshot().SearchComment(hash)
 		if err != nil {
@@ -233,13 +226,9 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 			_, err = b.EditCommentRaw(
 				author,
 				note.UpdatedAt.Unix(),
-				target,
+				hash,
 				cleanText,
-				map[string]string{
-					// no metadata unique metadata to store
-					keyGitlabId:  "",
-					keyGitlabUrl: "",
-				},
+				nil,
 			)
 
 			return err
@@ -254,15 +243,13 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 			note.CreatedAt.Unix(),
 			body,
 			map[string]string{
-				keyGitlabId:  id,
-				keyGitlabUrl: "",
+				keyGitlabId: id,
 			},
 		)
 
 		return err
 
 	case NOTE_UNKNOWN:
-		//TODO: send warning via channel
 		return nil
 
 	default:
@@ -308,7 +295,7 @@ func (gi *gitlabImporter) ensureLabelEvent(repo *cache.RepoCache, b *cache.BugCa
 		)
 
 	default:
-		panic("unexpected label event action")
+		err = fmt.Errorf("unexpected label event action")
 	}
 
 	return err

bridge/gitlab/import_notes.go 🔗

@@ -28,6 +28,8 @@ const (
 
 // GetNoteType parse a note system and body and return the note type and it content
 func GetNoteType(n *gitlab.Note) (NoteType, string) {
+	// when a note is a comment system is set to false
+	// when a note is a different event system is set to true
 	if !n.System {
 		return NOTE_COMMENT, n.Body
 	}

bridge/gitlab/iterator.go 🔗

@@ -28,8 +28,8 @@ type iterator struct {
 	// gitlab api v4 client
 	gc *gitlab.Client
 
-	// if since is given the iterator will query only the updated
-	// issues after this date
+	// if since is given the iterator will query only the issues
+	// updated after this date
 	since time.Time
 
 	// project id
@@ -79,8 +79,6 @@ func (i *iterator) Error() error {
 }
 
 func (i *iterator) getNextIssues() bool {
-	sort := "asc"
-	scope := "all"
 	issues, _, err := i.gc.Issues.ListProjectIssues(
 		i.project,
 		&gitlab.ListProjectIssuesOptions{
@@ -88,9 +86,9 @@ func (i *iterator) getNextIssues() bool {
 				Page:    i.issue.page,
 				PerPage: i.capacity,
 			},
-			Scope:        &scope,
+			Scope:        gitlab.String("all"),
 			UpdatedAfter: &i.since,
-			Sort:         &sort,
+			Sort:         gitlab.String("asc"),
 		},
 	)
 
@@ -114,15 +112,15 @@ func (i *iterator) getNextIssues() bool {
 }
 
 func (i *iterator) NextIssue() bool {
+	if i.err != nil {
+		return false
+	}
+
 	// first query
 	if i.issue.cache == nil {
 		return i.getNextIssues()
 	}
 
-	if i.err != nil {
-		return false
-	}
-
 	// move cursor index
 	if i.issue.index < len(i.issue.cache)-1 {
 		i.issue.index++
@@ -137,8 +135,6 @@ func (i *iterator) IssueValue() *gitlab.Issue {
 }
 
 func (i *iterator) getNextNotes() bool {
-	sort := "asc"
-	order := "created_at"
 	notes, _, err := i.gc.Notes.ListIssueNotes(
 		i.project,
 		i.IssueValue().IID,
@@ -147,8 +143,8 @@ func (i *iterator) getNextNotes() bool {
 				Page:    i.note.page,
 				PerPage: i.capacity,
 			},
-			Sort:    &sort,
-			OrderBy: &order,
+			Sort:    gitlab.String("asc"),
+			OrderBy: gitlab.String("created_at"),
 		},
 	)