fix(tempo): add the connection id to worklog domain ids

backend/plugins/tempo/e2e/snapshot_tables/issue_worklogs.csv

@@ -1,3 +1,3 @@
 id,author_id,comment,time_spent_minutes,logged_date,started_date,issue_id,_raw_data_params,_raw_data_table,_raw_data_id,_raw_data_remark
-tempo:TempoWorklog:12345,jira:JiraAccount:1:abc123,Working on task,60,2024-01-15T10:00:00.000+00:00,2024-01-15T09:00:00.000+00:00,jira:JiraIssue:1:10001,,,0,
-tempo:TempoWorklog:12346,jira:JiraAccount:1:abc123,Internal meeting,120,2024-01-16T16:00:00.000+00:00,2024-01-16T14:00:00.000+00:00,jira:JiraIssue:1:10002,,,0,
+tempo:TempoWorklog:1:12345,jira:JiraAccount:1:abc123,Working on task,60,2024-01-15T10:00:00.000+00:00,2024-01-15T09:00:00.000+00:00,jira:JiraIssue:1:10001,,,0,
+tempo:TempoWorklog:1:12346,jira:JiraAccount:1:abc123,Internal meeting,120,2024-01-16T16:00:00.000+00:00,2024-01-16T14:00:00.000+00:00,jira:JiraIssue:1:10002,,,0,

backend/plugins/tempo/tasks/worklog_convertor.go

@@ -73,48 +73,7 @@ func ConvertWorklogs(taskCtx plugin.SubTaskContext) errors.Error {
 		InputRowType: reflect.TypeOf(models.TempoWorklog{}),
 		Input:        cursor,
 		Convert: func(inputRow interface{}) ([]interface{}, errors.Error) {
-			tempoWorklog := inputRow.(*models.TempoWorklog)
-
-			// Must match the domain ids the jira plugin generates with didgen
-			// for JiraIssue / JiraAccount (plugins may not import each other),
-			// so worklogs join issues and accounts. Like before, this assumes
-			// the Tempo connection id equals the Jira connection id.
-			domainIssueId := fmt.Sprintf("jira:JiraIssue:%d:%d", connectionId, tempoWorklog.IssueId)
-			authorId := ""
-			if tempoWorklog.AuthorAccountId != "" {
-				authorId = fmt.Sprintf("jira:JiraAccount:%d:%s", connectionId, tempoWorklog.AuthorAccountId)
-			}
-
-			domainWorklogId := fmt.Sprintf("tempo:TempoWorklog:%d", tempoWorklog.TempoWorklogId)
-			timeSpentMinutes := tempoWorklog.TimeSpentSeconds / 60
-
-			var startedDate *time.Time
-			if tempoWorklog.StartDate != "" && tempoWorklog.StartTime != "" {
-				if t, err := time.Parse("2006-01-02 15:04:05", tempoWorklog.StartDate+" "+tempoWorklog.StartTime); err == nil {
-					startedDate = &t
-				}
-			}
-
-			var loggedDate *time.Time
-			if tempoWorklog.CreatedAt != "" {
-				if t, err := time.Parse(time.RFC3339, tempoWorklog.CreatedAt); err == nil {
-					loggedDate = &t
-				}
-			}
-
-			worklog := &ticket.IssueWorklog{
-				DomainEntity: domainlayer.DomainEntity{
-					Id: domainWorklogId,
-				},
-				IssueId:          domainIssueId,
-				AuthorId:         authorId,
-				TimeSpentMinutes: timeSpentMinutes,
-				StartedDate:      startedDate,
-				LoggedDate:       loggedDate,
-				Comment:          tempoWorklog.Description,
-			}
-
-			return []interface{}{worklog}, nil
+			return []interface{}{convertWorklog(connectionId, inputRow.(*models.TempoWorklog))}, nil
 		},
 	})
 
@@ -124,3 +83,49 @@ func ConvertWorklogs(taskCtx plugin.SubTaskContext) errors.Error {
 
 	return converter.Execute()
 }
+
+// convertWorklog maps a Tempo worklog to the domain layer.
+func convertWorklog(connectionId uint64, tempoWorklog *models.TempoWorklog) *ticket.IssueWorklog {
+	// Must match the domain ids the jira plugin generates with didgen
+	// for JiraIssue / JiraAccount (plugins may not import each other),
+	// so worklogs join issues and accounts. Like before, this assumes
+	// the Tempo connection id equals the Jira connection id.
+	domainIssueId := fmt.Sprintf("jira:JiraIssue:%d:%d", connectionId, tempoWorklog.IssueId)
+	authorId := ""
+	if tempoWorklog.AuthorAccountId != "" {
+		authorId = fmt.Sprintf("jira:JiraAccount:%d:%s", connectionId, tempoWorklog.AuthorAccountId)
+	}
+
+	// Tempo worklog ids are only unique within one Tempo instance, so the id
+	// carries the connection id like the tool layer primary key (and like
+	// didgen would generate it): otherwise two connections that share a
+	// worklog id overwrite each other's issue_worklogs row.
+	domainWorklogId := fmt.Sprintf("tempo:TempoWorklog:%d:%d", connectionId, tempoWorklog.TempoWorklogId)
+	timeSpentMinutes := tempoWorklog.TimeSpentSeconds / 60
+
+	var startedDate *time.Time
+	if tempoWorklog.StartDate != "" && tempoWorklog.StartTime != "" {
+		if t, err := time.Parse("2006-01-02 15:04:05", tempoWorklog.StartDate+" "+tempoWorklog.StartTime); err == nil {
+			startedDate = &t
+		}
+	}
+
+	var loggedDate *time.Time
+	if tempoWorklog.CreatedAt != "" {
+		if t, err := time.Parse(time.RFC3339, tempoWorklog.CreatedAt); err == nil {
+			loggedDate = &t
+		}
+	}
+
+	return &ticket.IssueWorklog{
+		DomainEntity: domainlayer.DomainEntity{
+			Id: domainWorklogId,
+		},
+		IssueId:          domainIssueId,
+		AuthorId:         authorId,
+		TimeSpentMinutes: timeSpentMinutes,
+		StartedDate:      startedDate,
+		LoggedDate:       loggedDate,
+		Comment:          tempoWorklog.Description,
+	}
+}

backend/plugins/tempo/tasks/worklog_convertor_test.go

@@ -0,0 +1,63 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tasks
+
+import (
+	"testing"
+	"time"
+
+	"github.com/apache/devlake/plugins/tempo/models"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConvertWorklog(t *testing.T) {
+	tempoWorklog := &models.TempoWorklog{
+		ConnectionId:     1,
+		TempoWorklogId:   12345,
+		IssueId:          10001,
+		AuthorAccountId:  "abc123",
+		TimeSpentSeconds: 3600,
+		StartDate:        "2024-01-15",
+		StartTime:        "09:00:00",
+		Description:      "Working on task",
+		CreatedAt:        "2024-01-15T10:00:00Z",
+	}
+
+	worklog := convertWorklog(1, tempoWorklog)
+	assert.Equal(t, "tempo:TempoWorklog:1:12345", worklog.Id)
+	assert.Equal(t, "jira:JiraIssue:1:10001", worklog.IssueId)
+	assert.Equal(t, "jira:JiraAccount:1:abc123", worklog.AuthorId)
+	assert.Equal(t, 60, worklog.TimeSpentMinutes)
+	assert.Equal(t, "Working on task", worklog.Comment)
+	if assert.NotNil(t, worklog.StartedDate) {
+		assert.Equal(t, time.Date(2024, 1, 15, 9, 0, 0, 0, time.UTC), *worklog.StartedDate)
+	}
+	if assert.NotNil(t, worklog.LoggedDate) {
+		assert.Equal(t, time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), *worklog.LoggedDate)
+	}
+}
+
+func TestConvertWorklogIdsAreUniquePerConnection(t *testing.T) {
+	// Tempo worklog ids are only unique within one Tempo instance: two
+	// connections can both have worklog 12345, and their domain rows must
+	// not share a primary key.
+	first := convertWorklog(1, &models.TempoWorklog{ConnectionId: 1, TempoWorklogId: 12345, IssueId: 10001})
+	second := convertWorklog(2, &models.TempoWorklog{ConnectionId: 2, TempoWorklogId: 12345, IssueId: 20001})
+	assert.NotEqual(t, first.Id, second.Id)
+	assert.Equal(t, "tempo:TempoWorklog:2:12345", second.Id)
+}