diff --git a/backend/plugins/jira/e2e/issue_changelog_board_scope_test.go b/backend/plugins/jira/e2e/issue_changelog_board_scope_test.go new file mode 100644 index 00000000000..e8c7e2f0cf1 --- /dev/null +++ b/backend/plugins/jira/e2e/issue_changelog_board_scope_test.go @@ -0,0 +1,105 @@ +/* +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 e2e + +import ( + "testing" + "time" + + "github.com/apache/incubator-devlake/core/dal" + "github.com/apache/incubator-devlake/core/models/domainlayer/ticket" + "github.com/apache/incubator-devlake/helpers/e2ehelper" + "github.com/apache/incubator-devlake/plugins/jira/impl" + "github.com/apache/incubator-devlake/plugins/jira/models" + "github.com/apache/incubator-devlake/plugins/jira/tasks" + "github.com/stretchr/testify/assert" +) + +// Board scoping in the changelog convertor is deliberate — the board is the unit of work — but it +// must not be silent, which is the complaint in issue #8834: an operator sees fewer rows in +// issue_changelogs than in the tool layer with nothing in the log to explain it. +// +// This drives the convertor with changelog items for an issue that is *not* on the board being +// converted, and asserts that the run succeeds, that the out-of-scope item is excluded, and that +// the in-scope items are still converted. +func TestIssueChangelogBoardScopeDataFlow(t *testing.T) { + var plugin impl.Jira + dataflowTester := e2ehelper.NewDataFlowTester(t, "jira", plugin) + + taskData := &tasks.JiraTaskData{ + Options: &tasks.JiraOptions{ + ConnectionId: 2, + BoardId: 8, + }, + } + + dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_jira_issue_changelogs.csv", &models.JiraIssueChangelogs{}) + dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_jira_issue_changelog_items.csv", &models.JiraIssueChangelogItems{}) + dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_jira_statuses_for_changelog.csv", &models.JiraStatus{}) + dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_jira_board_issues_for_changelog.csv", &models.JiraBoardIssue{}) + dataflowTester.ImportCsvIntoTabler("./snapshot_tables/_tool_jira_issue_fields.csv", &models.JiraIssueField{}) + + db := dataflowTester.Dal + + // An issue that exists with changelogs but is on no board at all — the shape the reporter + // describes for epic-sourced and cross-referenced issues. + const offBoardIssueId = 999001 + const offBoardChangelogId = 999002 + assert.NoError(t, db.CreateOrUpdate(&models.JiraIssueChangelogs{ + ConnectionId: 2, + ChangelogId: offBoardChangelogId, + IssueId: offBoardIssueId, + AuthorAccountId: "off-board-author", + AuthorDisplayName: "Off Board", + Created: time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC), + })) + assert.NoError(t, db.CreateOrUpdate(&models.JiraIssueChangelogItems{ + ConnectionId: 2, + ChangelogId: offBoardChangelogId, + Field: "status", + FromString: "New", + ToString: "Closed", + })) + + dataflowTester.FlushTabler(&ticket.IssueChangelogs{}) + dataflowTester.Subtask(tasks.ConvertIssueChangelogsMeta, taskData) + + // The out-of-scope item must not appear in the domain layer... + offBoard, err := db.Count( + dal.From(&ticket.IssueChangelogs{}), + dal.Where("issue_id = ?", "jira:JiraIssue:2:999001"), + ) + assert.NoError(t, err) + assert.Equal(t, int64(0), offBoard, + "a changelog for an issue outside the board must not be converted") + + // ...and no changelog may be attached to a non-existent issue id 0, which is what emitting + // an item with a missing parent changelog row would produce. + orphans, err := db.Count( + dal.From(&ticket.IssueChangelogs{}), + dal.Where("issue_id = ?", "jira:JiraIssue:2:0"), + ) + assert.NoError(t, err) + assert.Equal(t, int64(0), orphans, "no changelog may point at issue id 0") + + // The in-scope changelogs are still converted, so the exclusion above is scoping and not a + // regression that dropped everything. + inScope, err := db.Count(dal.From(&ticket.IssueChangelogs{})) + assert.NoError(t, err) + assert.Greater(t, inScope, int64(0), "in-scope changelogs must still be converted") +} diff --git a/backend/plugins/jira/tasks/issue_changelog_convertor.go b/backend/plugins/jira/tasks/issue_changelog_convertor.go index b27fc966681..31b71fd6dba 100644 --- a/backend/plugins/jira/tasks/issue_changelog_convertor.go +++ b/backend/plugins/jira/tasks/issue_changelog_convertor.go @@ -104,7 +104,13 @@ func ConvertIssueChangelogs(subtaskCtx plugin.SubTaskContext) errors.Error { if stateManager.IsIncremental() { since := stateManager.GetSince() if since != nil { - clauses = append(clauses, dal.Where("_tool_jira_issue_changelog_items.created_at >= ? ", since)) + // updated_at, not created_at: created_at is the moment the row was first + // inserted and never moves again, so an item that was collected but not + // converted in that window could never be picked up by a later incremental + // run. updated_at is refreshed by the extractor's upsert + // (OnConflict{UpdateAll: true}), so re-collected items are reconsidered. + // Every other convertor in the code base already filters on updated_at. + clauses = append(clauses, dal.Where("_tool_jira_issue_changelog_items.updated_at >= ? ", since)) } } return db.Cursor(clauses...) @@ -170,7 +176,51 @@ func ConvertIssueChangelogs(subtaskCtx plugin.SubTaskContext) errors.Error { return err } - return converter.Execute() + if err = converter.Execute(); err != nil { + return err + } + reportUnscopedChangelogs(subtaskCtx, connectionId, boardId) + return nil +} + +// reportUnscopedChangelogs counts collected changelog items whose issue is not associated with +// the board being converted, and says so. +// +// Those items are excluded by the board filter in the query above, which is deliberate — the +// board is the unit of work, and widening the filter would make every board task convert the +// whole connection. But excluding them silently is what makes the shortfall in issue_changelogs +// look like data loss with no explanation. One aggregate count per board task is cheap next to +// the conversion itself, and turns "21% of my changelogs are missing" into a logged number. +func reportUnscopedChangelogs(subtaskCtx plugin.SubTaskContext, connectionId, boardId uint64) { + db := subtaskCtx.GetDal() + logger := subtaskCtx.GetLogger() + + count, err := db.Count( + dal.From("_tool_jira_issue_changelog_items"), + dal.Join(`left join _tool_jira_issue_changelogs on ( + _tool_jira_issue_changelogs.connection_id = _tool_jira_issue_changelog_items.connection_id + AND _tool_jira_issue_changelogs.changelog_id = _tool_jira_issue_changelog_items.changelog_id + )`), + dal.Where(`_tool_jira_issue_changelog_items.connection_id = ? + AND NOT EXISTS ( + SELECT 1 FROM _tool_jira_board_issues bi + WHERE bi.connection_id = _tool_jira_issue_changelogs.connection_id + AND bi.issue_id = _tool_jira_issue_changelogs.issue_id + AND bi.board_id = ? + )`, connectionId, boardId), + ) + if err != nil { + // Diagnostics must never fail the conversion that just succeeded. + logger.Warn(err, "unable to count changelog items outside board %d", boardId) + return + } + if count > 0 { + logger.Warn(nil, + "%d collected changelog item(s) are not associated with board %d and were not "+ + "converted; they belong to issues outside this board's scope. If they are "+ + "expected in the domain layer, add a board that contains those issues.", + count, boardId) + } } func convertIds(ids string, connectionId uint64, sprintIdGenerator *didgen.DomainIdGenerator) (string, errors.Error) { diff --git a/backend/plugins/jira/tasks/issue_changelog_convertor_test.go b/backend/plugins/jira/tasks/issue_changelog_convertor_test.go new file mode 100644 index 00000000000..50d47a503e3 --- /dev/null +++ b/backend/plugins/jira/tasks/issue_changelog_convertor_test.go @@ -0,0 +1,46 @@ +/* +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 ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Regression guard for the incremental filter column, which is the defect in issue #8834. +// +// created_at is stamped once when the row is first inserted and never moves again, so a changelog +// item collected in one window but not converted then can never be picked up by a later +// incremental run. updated_at is refreshed by the extractor's upsert, so a re-collected item is +// reconsidered. +// +// Demonstrating the replay itself needs an incremental subtask-state harness and a populated +// database, so this asserts the choice of column: it is a one-token change that silently +// reintroduces permanent data loss, and nothing else in the test suite would catch it. +func TestIncrementalFilterUsesUpdatedAtNotCreatedAt(t *testing.T) { + body, err := os.ReadFile("issue_changelog_convertor.go") + assert.NoError(t, err) + source := string(body) + + assert.Contains(t, source, "_tool_jira_issue_changelog_items.updated_at >= ?", + "the incremental filter must use updated_at") + assert.NotContains(t, source, "_tool_jira_issue_changelog_items.created_at >= ?", + "created_at never moves after insert, so it permanently hides unconverted items") +}