From ddcbe950cc71ea2dd63bdc98e6d3ef0e5174cbb5 Mon Sep 17 00:00:00 2001 From: Shivam Sharma <192242476+ShivamSharma43@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:56:32 +0530 Subject: [PATCH] fix(tempo): add the connection id to worklog domain ids convert_worklogs built the issue_worklogs id from the Tempo worklog id alone (tempo:TempoWorklog:), while the tool layer keys worklogs by (connection_id, tempo_worklog_id). Two connections with the same worklog id converted to the same domain row, so the connection converted last overwrote the other's worklog. Use tempo:TempoWorklog::, the id didgen generates from the TempoWorklog primary key. The converter is a full-sync DataConverter, so it deletes a scope's rows with the old ids before it saves the new ones; no migration is needed. Closes #9330 Generated-by: Claude Code (Claude Opus 5.5) Co-Authored-By: Claude Opus 5.5 Signed-off-by: Shivam Sharma <192242476+ShivamSharma43@users.noreply.github.com> --- .../e2e/snapshot_tables/issue_worklogs.csv | 4 +- .../plugins/tempo/tasks/worklog_convertor.go | 89 ++++++++++--------- .../tempo/tasks/worklog_convertor_test.go | 63 +++++++++++++ 3 files changed, 112 insertions(+), 44 deletions(-) create mode 100644 backend/plugins/tempo/tasks/worklog_convertor_test.go diff --git a/backend/plugins/tempo/e2e/snapshot_tables/issue_worklogs.csv b/backend/plugins/tempo/e2e/snapshot_tables/issue_worklogs.csv index 417b0f122f2..29632131dff 100644 --- a/backend/plugins/tempo/e2e/snapshot_tables/issue_worklogs.csv +++ b/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, diff --git a/backend/plugins/tempo/tasks/worklog_convertor.go b/backend/plugins/tempo/tasks/worklog_convertor.go index bd6c4f25554..a128f9a606d 100644 --- a/backend/plugins/tempo/tasks/worklog_convertor.go +++ b/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, + } +} diff --git a/backend/plugins/tempo/tasks/worklog_convertor_test.go b/backend/plugins/tempo/tasks/worklog_convertor_test.go new file mode 100644 index 00000000000..80079bf99c5 --- /dev/null +++ b/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) +}