fix(migration): preserve task creator/comments/reactions/resolution and replay incident state on 2.0 upgrade#30213
Conversation
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| ObjectNode author = buildUserRef(handle, post.path("from").asText(null)); | ||
| if (author == null) { | ||
| author = JsonUtils.getObjectNode(); | ||
| author.put("id", UUID.nameUUIDFromBytes("system".getBytes()).toString()); | ||
| author.put("type", Entity.USER); | ||
| author.put("name", "system"); | ||
| } | ||
| comment.set("author", author); |
There was a problem hiding this comment.
💡 Quality: Comment fallback fabricates a non-existent "system" user reference
When a post's author cannot be resolved, the comment's author entityReference is set to a fabricated id UUID.nameUUIDFromBytes("system") with name system. No such row exists in user_entity, so this is a dangling reference — the same class of blank/unresolvable actor this PR sets out to fix; the UI will fail to resolve the author. Consider falling back to a real actor (e.g. resolve the thread's createdBy, or the ingestionBot/admin user id) or omitting the author rather than writing a synthetic user id.
Fall back to the task's already-resolved creator instead of a synthetic id:
ObjectNode author = buildUserRef(handle, post.path("from").asText(null));
if (author == null && taskJson.has("createdBy")) {
author = (ObjectNode) taskJson.get("createdBy").deepCopy();
}
if (author != null) {
comment.set("author", author);
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| String createdByUserId = lookupUserId(handle, createdByName); | ||
| if (createdByUserId != null) { | ||
| taskJson.put("createdById", createdByUserId); | ||
| ObjectNode createdByRef = buildUserRef(handle, createdByName); | ||
| if (createdByRef != null) { | ||
| taskJson.set("createdBy", createdByRef); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Performance: Redundant user_entity lookup for createdBy reference
createdByUserId is already resolved via lookupUserId, then buildUserRef immediately re-runs the identical lookupUserId query for the same createdByName. lookupUserId is uncached and hits user_entity each call, so every migrated task issues one redundant query. Build the reference from the id already in hand rather than querying again.
Reuse the already-resolved id:
if (createdByUserId != null) {
taskJson.put("createdById", createdByUserId);
ObjectNode createdByRef = JsonUtils.getObjectNode();
createdByRef.put("id", createdByUserId);
createdByRef.put("type", Entity.USER);
createdByRef.put("name", createdByName);
taskJson.set("createdBy", createdByRef);
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| String createdByUserId = lookupUserId(handle, createdByName); | ||
| if (createdByUserId != null) { | ||
| taskJson.put("createdById", createdByUserId); | ||
| ObjectNode createdByRef = buildUserRef(handle, createdByName); | ||
| if (createdByRef != null) { | ||
| taskJson.set("createdBy", createdByRef); | ||
| } | ||
| } |
There was a problem hiding this comment.
buildUserRef already calls lookupUserId internally, so this block runs two SQL queries against user_entity for the same name. For a migration that may process thousands of tasks, this silently doubles DB round-trips for the createdBy field. Either restructure buildUserRef to accept an already-resolved id, or build the ref directly here using the id you already hold.
| String createdByUserId = lookupUserId(handle, createdByName); | |
| if (createdByUserId != null) { | |
| taskJson.put("createdById", createdByUserId); | |
| ObjectNode createdByRef = buildUserRef(handle, createdByName); | |
| if (createdByRef != null) { | |
| taskJson.set("createdBy", createdByRef); | |
| } | |
| } | |
| String createdByUserId = lookupUserId(handle, createdByName); | |
| if (createdByUserId != null) { | |
| taskJson.put("createdById", createdByUserId); | |
| ObjectNode createdByRef = JsonUtils.getObjectNode(); | |
| createdByRef.put("id", createdByUserId); | |
| createdByRef.put("type", Entity.USER); | |
| createdByRef.put("name", createdByName); | |
| taskJson.set("createdBy", createdByRef); | |
| } |
…eactions The v200 thread→task migration dropped several fields, leaving migrated tasks with a blank "Created By", no resolver on completed tasks, empty comment history, and no reactions. Map them onto task_entity: - createdBy: set the entityReference (not just createdById) from the legacy thread creator so the task drawer's "Created By" resolves. - resolution.resolvedBy: set from the legacy closedBy; drop the generic "Migrated from thread-based task system" placeholder comment — the real close note now survives as a migrated comment. - comments: migrate the thread's posts[] (replies + resolve/close records) into task comments[] and set commentCount. - reactions: carry thread-level reactions onto the task. Adds buildUserRef and migrateThreadPostsToComments helpers.
…ent tasks The v200 cutover starts every incident (TestCaseResolution) workflow at NewStage and never replays the recorded test_case_resolution_status lifecycle, so an incident that was Ack/Assigned before the upgrade comes back as New / No Owners — and that stale state also makes the "Update Status" resolve transition fail. After backfillOpenTasksToWorkflowInstances starts an incident workflow, replay the test case's latest resolution status onto it, reusing the existing runtime bridge (resolveLegacyTransitionId / buildLegacyResolvedPayload) so migration and runtime stay consistent. Adds TestCaseResolutionStatusRepository .backfillMigratedIncidentTaskStage as the public entry point. Idempotent: no-ops when the task is already at the recorded stage.
e4a11ac to
8fe7d61
Compare
Code Review 👍 Approved with suggestions 0 resolved / 2 findingsRestores field and incident state fidelity during the 1.12.1 to 2.0 migration, ensuring task metadata and workflow status are preserved. Consider refactoring the redundant user lookup and handling the fabricated 'system' author reference for unresolved users. 💡 Quality: Comment fallback fabricates a non-existent "system" user referenceWhen a post's author cannot be resolved, the comment's Fall back to the task's already-resolved creator instead of a synthetic id💡 Performance: Redundant user_entity lookup for createdBy reference📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java:748-755 📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java:1495-1505
Reuse the already-resolved id🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
🟡 Playwright Results — all passed (27 flaky)✅ 4544 passed · ❌ 0 failed · 🟡 27 flaky · ⏭️ 95 skipped
🟡 27 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Problem
On a
1.12.1 → 2.0upgrade, the thread→task migration preserves the task rows and their type/status mapping, but drops several fields and resets incident state. On migrated (pre-existing) data:resolution.resolvedByis empty and the comment is a generic"Migrated from thread-based task system".task_entity.comments.NewStageand never replays the recordedtest_case_resolution_statuslifecycle, so a pre-upgrade Ack/Assigned incident loses its stage and assignee (and the stale state then makes the "Update Status" resolve fail).New tasks/incidents created in 2.0 are unaffected — this is migration-only.
Fix
v200/MigrationUtil.java— in the thread→task migration:createdBy(entityReference) in addition tocreatedById;resolution.resolvedByfrom the legacyclosedBy(the original close note now survives as a migrated comment, so the placeholder is dropped);posts[]into taskcomments[](+commentCount);reactions[]onto the task.For incidents, after
backfillOpenTasksToWorkflowInstancesstarts the workflow, replay the test case's latest resolution status onto the instance, reusing the existing runtime bridge (resolveLegacyTransitionId/buildLegacyResolvedPayload) so migration and runtime stay consistent. New public entry point:TestCaseResolutionStatusRepository.backfillMigratedIncidentTaskStage. Idempotent — no-ops when the task is already at the recorded stage.Complementary to #29978 (which restores the
ASSIGNED_TO/MENTIONED_INrelationships on Postgres); this PR restores the field- and incident-state fidelity that #29978 does not cover.Testing
openmetadata-servicecompiles; spotless applied.Greptile Summary
This migration fix preserves thread-to-task field fidelity and incident stage on 1.12.1 → 2.0 upgrades. Previously, migrated tasks lost their creator, resolver identity, comments, reactions, and incident workflow state — causing incidents to reset to
New / No Ownersand "Update Status" to fail on pre-upgrade incidents.MigrationUtil.java): addscreatedByentity reference, migrates threadposts[]to taskcomments[], carries thread-levelreactions[], and resolves the actual closer intoresolution.resolvedBy(dropping the generic placeholder comment).TestCaseResolutionStatusRepository.java): introducesbackfillMigratedIncidentTaskStage, called aftertriggerByKeyfor each incident task, which looks up the latestTestCaseResolutionStatusrecord and replays it via the existingapplyLegacyStatusToIncidentTaskbridge so pre-upgrade Ack/Assigned incidents recover their stage and assignee.Confidence Score: 4/5
Safe to merge for the field-preservation changes; the incident state replay path warrants a live migration check before deploying to production because the workflow trigger and replay call are not synchronized.
The thread-to-task field migration (creator, comments, reactions, resolver) is straightforward and well-guarded. The incident replay path is the higher-risk addition: replayMigratedIncidentState is called immediately after workflowHandler.triggerByKey, but Flowable's job executor may process the start event asynchronously. If the workflow has not yet reached NewStage when resolveTaskWithWorkflow runs, the transition silently fails (caught as WARN), and the incident stays at New — exactly the regression this PR is trying to prevent. Verifying on a live migrated instance is essential before this is considered fully resolved.
The incident state replay logic in MigrationUtil.java (replayMigratedIncidentState and its call site) deserves a live migration smoke test to confirm the workflow is in a stable state before the transition is attempted.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant M as MigrationUtil participant TR as TaskRepository participant WH as WorkflowHandler participant TCSR as TestCaseResolutionStatusRepository participant FE as Flowable Engine M->>TR: listAll(open tasks without workflowInstanceId) loop for each open incident task M->>WH: triggerByKey(workflowDefinitionFQN, taskId, variables) WH-->>FE: start workflow (async job executor) Note over FE: may not reach NewStage yet M->>M: replayMigratedIncidentState(task) M->>TCSR: backfillMigratedIncidentTaskStage(task) TCSR->>TCSR: getLatestRecord(testCaseFQN) TCSR->>TCSR: applyLegacyStatusToIncidentTask(latest, fqn) TCSR->>TR: get(taskId, full fields incl. workflowStageId) TCSR->>TCSR: resolveLegacyTransitionId(task, latest) TCSR->>TR: resolveTaskWithWorkflow(task, transitionId, ...) TR-->>FE: apply transition (needs stable NewStage) end Note over M: Thread-to-Task field migration M->>M: lookupUserId(createdByName) [DB query 1] M->>M: buildUserRef(createdByName) [DB query 2 same user] M->>M: migrateThreadPostsToComments(threadJson, taskJson) M->>M: set reactions, resolvedBy, resolution%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant M as MigrationUtil participant TR as TaskRepository participant WH as WorkflowHandler participant TCSR as TestCaseResolutionStatusRepository participant FE as Flowable Engine M->>TR: listAll(open tasks without workflowInstanceId) loop for each open incident task M->>WH: triggerByKey(workflowDefinitionFQN, taskId, variables) WH-->>FE: start workflow (async job executor) Note over FE: may not reach NewStage yet M->>M: replayMigratedIncidentState(task) M->>TCSR: backfillMigratedIncidentTaskStage(task) TCSR->>TCSR: getLatestRecord(testCaseFQN) TCSR->>TCSR: applyLegacyStatusToIncidentTask(latest, fqn) TCSR->>TR: get(taskId, full fields incl. workflowStageId) TCSR->>TCSR: resolveLegacyTransitionId(task, latest) TCSR->>TR: resolveTaskWithWorkflow(task, transitionId, ...) TR-->>FE: apply transition (needs stable NewStage) end Note over M: Thread-to-Task field migration M->>M: lookupUserId(createdByName) [DB query 1] M->>M: buildUserRef(createdByName) [DB query 2 same user] M->>M: migrateThreadPostsToComments(threadJson, taskJson) M->>M: set reactions, resolvedBy, resolutionComments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java, line 1959-1970 (link)replayMigratedIncidentStateis called synchronously right afterworkflowHandler.triggerByKey. If the Flowable job executor processes the "start" event asynchronously, the workflow process may not yet have transitioned intoNewStagewhenresolveTaskWithWorkflowruns, causing the transition call to fail. The failure is caught and downgraded to aWARN, so the incident stays atNewwith no visible error — exactly the regression this PR is meant to fix. A brief Flowable-state check or retry before the transition (or moving the replay into anafterCommithook where the workflow is guaranteed to be in a stable state) would close this gap.Reviews (2): Last reviewed commit: "fix(migration): replay pre-upgrade incid..." | Re-trigger Greptile