bridge/gitlab: fix edit comment request and remove label functionalities

Amine Hilaly created

Change summary

bridge/core/export.go        |   5 
bridge/gitlab/export.go      | 116 +++++++++----------------------------
bridge/gitlab/export_test.go |  60 ++++++++++++-------
bridge/gitlab/import.go      |   2 
commands/bridge_push.go      |   1 
5 files changed, 70 insertions(+), 114 deletions(-)

Detailed changes

bridge/core/export.go 🔗

@@ -62,8 +62,9 @@ func (er ExportResult) String() string {
 
 func NewExportError(err error, id entity.Id) ExportResult {
 	return ExportResult{
-		ID:  id,
-		Err: err,
+		ID:    id,
+		Err:   err,
+		Event: ExportEventError,
 	}
 }
 

bridge/gitlab/export.go 🔗

@@ -35,9 +35,6 @@ type gitlabExporter struct {
 	// cache identifiers used to speed up exporting operations
 	// cleared for each bug
 	cachedOperationIDs map[string]string
-
-	// cache labels used to speed up exporting labels events
-	cachedLabels map[string]string
 }
 
 // Init .
@@ -47,7 +44,6 @@ func (ge *gitlabExporter) Init(conf core.Configuration) error {
 	ge.identityToken = make(map[string]string)
 	ge.identityClient = make(map[string]*gitlab.Client)
 	ge.cachedOperationIDs = make(map[string]string)
-	ge.cachedLabels = make(map[string]string)
 
 	return nil
 }
@@ -99,26 +95,31 @@ func (ge *gitlabExporter) ExportAll(ctx context.Context, repo *cache.RepoCache,
 		allBugsIds := repo.AllBugsIds()
 
 		for _, id := range allBugsIds {
-			b, err := repo.ResolveBug(id)
-			if err != nil {
-				out <- core.NewExportError(err, id)
+			select {
+			case <-ctx.Done():
 				return
-			}
+			default:
+				b, err := repo.ResolveBug(id)
+				if err != nil {
+					out <- core.NewExportError(err, id)
+					return
+				}
 
-			snapshot := b.Snapshot()
+				snapshot := b.Snapshot()
 
-			// ignore issues created before since date
-			// TODO: compare the Lamport time instead of using the unix time
-			if snapshot.CreatedAt.Before(since) {
-				out <- core.NewExportNothing(b.Id(), "bug created before the since date")
-				continue
-			}
+				// ignore issues created before since date
+				// TODO: compare the Lamport time instead of using the unix time
+				if snapshot.CreatedAt.Before(since) {
+					out <- core.NewExportNothing(b.Id(), "bug created before the since date")
+					continue
+				}
 
-			if snapshot.HasAnyActor(allIdentitiesIds...) {
-				// try to export the bug and it associated events
-				ge.exportBug(ctx, b, since, out)
-			} else {
-				out <- core.NewExportNothing(id, "not an actor")
+				if snapshot.HasAnyActor(allIdentitiesIds...) {
+					// try to export the bug and it associated events
+					ge.exportBug(ctx, b, since, out)
+				} else {
+					out <- core.NewExportNothing(id, "not an actor")
+				}
 			}
 		}
 	}()
@@ -135,8 +136,6 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 	var bugGitlabIDString string
 	var bugCreationId string
 
-	//labels := make([]string, 0)
-
 	// Special case:
 	// if a user try to export a bug that is not already exported to Gitlab (or imported
 	// from Gitlab) and we do not have the token of the bug author, there is nothing we can do.
@@ -182,7 +181,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 		}
 
 		// create bug
-		id, url, err := createGitlabIssue(ctx, client, ge.repositoryID, createOp.Title, createOp.Message)
+		_, id, url, err := createGitlabIssue(ctx, client, ge.repositoryID, createOp.Title, createOp.Message)
 		if err != nil {
 			err := errors.Wrap(err, "exporting gitlab issue")
 			out <- core.NewExportError(err, b.Id())
@@ -287,7 +286,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 					panic("unexpected comment id format")
 				}
 
-				if err := editCommentGitlabIssue(ctx, client, ge.repositoryID, commentIDint, id, opr.Message); err != nil {
+				if err := editCommentGitlabIssue(ctx, client, ge.repositoryID, bugGitlabID, commentIDint, opr.Message); err != nil {
 					err := errors.Wrap(err, "editing comment")
 					out <- core.NewExportError(err, b.Id())
 					return
@@ -299,7 +298,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 
 		case *bug.SetStatusOperation:
 			opr := op.(*bug.SetStatusOperation)
-			if err := updateGitlabIssueStatus(ctx, client, idString, id, opr.Status); err != nil {
+			if err := updateGitlabIssueStatus(ctx, client, ge.repositoryID, bugGitlabID, opr.Status); err != nil {
 				err := errors.Wrap(err, "editing status")
 				out <- core.NewExportError(err, b.Id())
 				return
@@ -310,7 +309,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 
 		case *bug.SetTitleOperation:
 			opr := op.(*bug.SetTitleOperation)
-			if err := updateGitlabIssueTitle(ctx, client, ge.repositoryID, id, opr.Title); err != nil {
+			if err := updateGitlabIssueTitle(ctx, client, ge.repositoryID, bugGitlabID, opr.Title); err != nil {
 				err := errors.Wrap(err, "editing title")
 				out <- core.NewExportError(err, b.Id())
 				return
@@ -356,6 +355,7 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			panic("unhandled operation type case")
 		}
 
+		idString = strconv.Itoa(id)
 		// mark operation as exported
 		if err := markOperationAsExported(b, op.Id(), idString, url); err != nil {
 			err := errors.Wrap(err, "marking operation as exported")
@@ -384,66 +384,8 @@ func markOperationAsExported(b *cache.BugCache, target entity.Id, gitlabID, gitl
 	return err
 }
 
-func (ge *gitlabExporter) getGitlabLabelID(label string) (string, error) {
-	id, ok := ge.cachedLabels[label]
-	if !ok {
-		return "", fmt.Errorf("non cached label")
-	}
-
-	return id, nil
-}
-
-// get label from gitlab
-func (ge *gitlabExporter) loadLabelsFromGitlab(ctx context.Context, gc *gitlab.Client) error {
-	page := 1
-	for ; ; page++ {
-		ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
-		defer cancel()
-		labels, _, err := gc.Labels.ListLabels(
-			ge.repositoryID,
-			&gitlab.ListLabelsOptions{
-				Page: page,
-			},
-			gitlab.WithContext(ctx),
-		)
-		if err != nil {
-			return err
-		}
-
-		if len(labels) == 0 {
-			break
-		}
-		for _, label := range labels {
-			ge.cachedLabels[label.Name] = strconv.Itoa(label.ID)
-		}
-	}
-
-	return nil
-}
-
-func (ge *gitlabExporter) createGitlabLabel(ctx context.Context, gc *gitlab.Client, label bug.Label) error {
-	client := buildClient(ge.conf[keyToken])
-
-	// RGBA to hex color
-	rgba := label.RGBA()
-	hexColor := fmt.Sprintf("%.2x%.2x%.2x", rgba.R, rgba.G, rgba.B)
-	name := label.String()
-
-	ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
-	defer cancel()
-	_, _, err := client.Labels.CreateLabel(ge.repositoryID,
-		&gitlab.CreateLabelOptions{
-			Name:  &name,
-			Color: &hexColor,
-		},
-		gitlab.WithContext(ctx),
-	)
-
-	return err
-}
-
 // create a gitlab. issue and return it ID
-func createGitlabIssue(ctx context.Context, gc *gitlab.Client, repositoryID, title, body string) (int, string, error) {
+func createGitlabIssue(ctx context.Context, gc *gitlab.Client, repositoryID, title, body string) (int, int, string, error) {
 	ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
 	defer cancel()
 	issue, _, err := gc.Issues.CreateIssue(
@@ -455,10 +397,10 @@ func createGitlabIssue(ctx context.Context, gc *gitlab.Client, repositoryID, tit
 		gitlab.WithContext(ctx),
 	)
 	if err != nil {
-		return 0, "", err
+		return 0, 0, "", err
 	}
 
-	return issue.IID, issue.WebURL, nil
+	return issue.ID, issue.IID, issue.WebURL, nil
 }
 
 // add a comment to an issue and return it ID

bridge/gitlab/export_test.go 🔗

@@ -25,9 +25,11 @@ const (
 )
 
 type testCase struct {
-	name    string
-	bug     *cache.BugCache
-	numOrOp int // number of original operations
+	name     string
+	bug      *cache.BugCache
+	numOp    int // number of original operations
+	numOpExp int // number of operations after export
+	numOpImp int // number of operations after import
 }
 
 func testCases(t *testing.T, repo *cache.RepoCache, identity *cache.IdentityCache) []*testCase {
@@ -87,34 +89,46 @@ func testCases(t *testing.T, repo *cache.RepoCache, identity *cache.IdentityCach
 
 	return []*testCase{
 		&testCase{
-			name:    "simple bug",
-			bug:     simpleBug,
-			numOrOp: 1,
+			name:     "simple bug",
+			bug:      simpleBug,
+			numOp:    1,
+			numOpExp: 2,
+			numOpImp: 1,
 		},
 		&testCase{
-			name:    "bug with comments",
-			bug:     bugWithComments,
-			numOrOp: 2,
+			name:     "bug with comments",
+			bug:      bugWithComments,
+			numOp:    2,
+			numOpExp: 4,
+			numOpImp: 2,
 		},
 		&testCase{
-			name:    "bug label change",
-			bug:     bugLabelChange,
-			numOrOp: 4,
+			name:     "bug label change",
+			bug:      bugLabelChange,
+			numOp:    4,
+			numOpExp: 8,
+			numOpImp: 4,
 		},
 		&testCase{
-			name:    "bug with comment editions",
-			bug:     bugWithCommentEditions,
-			numOrOp: 4,
+			name:     "bug with comment editions",
+			bug:      bugWithCommentEditions,
+			numOp:    4,
+			numOpExp: 8,
+			numOpImp: 2,
 		},
 		&testCase{
-			name:    "bug changed status",
-			bug:     bugStatusChanged,
-			numOrOp: 3,
+			name:     "bug changed status",
+			bug:      bugStatusChanged,
+			numOp:    3,
+			numOpExp: 6,
+			numOpImp: 3,
 		},
 		&testCase{
-			name:    "bug title edited",
-			bug:     bugTitleEdited,
-			numOrOp: 2,
+			name:     "bug title edited",
+			bug:      bugTitleEdited,
+			numOp:    2,
+			numOpExp: 4,
+			numOpImp: 2,
 		},
 	}
 }
@@ -216,7 +230,7 @@ func TestPushPull(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// for each operation a SetMetadataOperation will be added
 			// so number of operations should double
-			require.Len(t, tt.bug.Snapshot().Operations, tt.numOrOp*2)
+			require.Len(t, tt.bug.Snapshot().Operations, tt.numOpExp)
 
 			// verify operation have correct metadata
 			for _, op := range tt.bug.Snapshot().Operations {
@@ -239,7 +253,7 @@ func TestPushPull(t *testing.T) {
 			require.NoError(t, err)
 
 			// verify bug have same number of original operations
-			require.Len(t, importedBug.Snapshot().Operations, tt.numOrOp)
+			require.Len(t, importedBug.Snapshot().Operations, tt.numOpImp)
 
 			// verify bugs are taged with origin=gitlab
 			issueOrigin, ok := importedBug.Snapshot().GetCreateMetadata(core.KeyOrigin)

bridge/gitlab/import.go 🔗

@@ -121,7 +121,7 @@ func (gi *gitlabImporter) ensureIssue(repo *cache.RepoCache, issue *gitlab.Issue
 		nil,
 		map[string]string{
 			core.KeyOrigin:   target,
-			keyGitlabId:      parseID(issue.ID),
+			keyGitlabId:      parseID(issue.IID),
 			keyGitlabUrl:     issue.WebURL,
 			keyGitlabProject: gi.conf[keyProjectID],
 		},

commands/bridge_push.go 🔗

@@ -85,7 +85,6 @@ func runBridgePush(cmd *cobra.Command, args []string) error {
 	close(done)
 
 	fmt.Printf("Successfully exported %d issues with %s bridge\n", exportedIssues, b.Name)
-
 	return nil
 }