fix(governance): unblock userApprovalTask resolves when transitionMetadata is empty - #31650
Merged
Conversation
…adata is empty TaskResource.validateTransition (added in #30969) rejects any /resolve call whose transitionId is not declared on the task's availableTransitions. That list is projected from the workflow node's config.transitionMetadata at CreateTask time. Two paths produced userApprovalTask nodes with empty transitionMetadata and stranded every resulting task: - UI workflow builder (UserApprovalForm.handleSave) never emitted transitionMetadata — only assignees + thresholds. - 1.13 → 2.x upgrade paths carry no transitionMetadata field (didn't exist in the 1.13 schema). v200 migration rewrote outbound edges and redeployed BPMN but left the node config's transitionMetadata untouched. Post-#30969 an Approve/Reject click on any such task returned 400 "Transition 'approve' is not available for task ...". Reproduced against sandbox TASK-19741 (custom NewDataAssetIngestionWorkflow). Layered fix so every write path emits a default and every already-broken task is unblocked without a data migration: 1. FE: UserApprovalForm.handleSave now emits the default [approve, reject] pair when node.data.config carries no transitionMetadata. Preserves any hand-tuned metadata from a JSON-imported workflow. No new form fields. 2. Backend read-time synth: TaskWorkflowLifecycleResolver.findTransition falls back to DEFAULT_USER_APPROVAL_TRANSITIONS when task.availableTransitions is empty, gated on task.workflowDefinitionId != null and task.status ∈ {Open, InProgress}. The status guard preserves the self-approval guard from #30969 — Rejected / Approved / Granted / Revoked / Completed / Cancelled / Failed / Expired tasks stay un-resolvable, so no leak is reopened. resolveTransitionsForStage projects the same defaults at CreateTask time so freshly-created tasks already carry availableTransitions on the row. 3. Migration: v200 MigrationUtil.backfillUserApprovalTransitionMetadata patches every userApprovalTask node whose transitionMetadata is empty before createOrUpdate triggers Flowable BPMN redeploy. Cleans 1.13 upgraders' data at rest. Populated custom metadata is preserved. Schema stays lenient — transitionMetadata: [] remains a valid submission so existing tenant JSON exports import without friction, and the runtime synth handles the read side either way. Tests: - TaskWorkflowLifecycleResolverTest — 5 new: default fallback on empty metadata, synth on Open, no-synth on 8 terminal statuses, no-synth for non-workflow-managed tasks, no-op when workflow has no userApprovalTask. - MigrationUtilTaskWorkflowTest — 2 new: empty-metadata backfill preserves thresholds; populated metadata untouched. - WorkflowDefinitionResourceIT — 1 new E2E: post workflow with empty transitionMetadata → wait for task → assert availableTransitions carries approve/reject → resolve via POST /tasks/{id}/resolve transitionId=approve → wait for status=Approved (proves Flowable routes the synthesized signal). Test helpers no longer depend on the auto-numbered Config__1 generated symbol (would rename on any future userApprovalTask schema edit) — the resolver-test helper builds the node via JsonUtils.convertValue from a plain map.
yan-3005
requested review from
a team,
ShaileshParmar11,
chirag-madlani and
karanh37
as code owners
August 17, 2026 16:19
Contributor
❌ 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}
resolveDefaultUserApprovalTransitions used to fetch the WorkflowDefinition twice on every /resolve of an at-rest legacy task with empty availableTransitions: once through resolveTransitionsForStage(UUID, ...) and again through workflowHasUserApprovalTask(UUID). Both entries hit the repository plus JSON deserialization on a hot path. Extract loadWorkflowDefinition and hasUserApprovalTaskNode helpers so the definition is fetched once and shared with both the stage-scoped and the node-presence check. Behavior unchanged; unit tests continue to pass with the same static mock setup. Flagged by gitar-bot review comment on #31650.
Three sonarjs / import warnings surfaced on PR #31650: - Barrel import at './' triggered no-circular-imports + no-internal-barrel-imports. Switched to direct imports of FormActionButtons and MetadataFormSection. - useEffect body pushed sonarjs/cyclomatic-complexity to 13. Extracted buildInitialState + buildAssigneesState helpers so the effect body is a linear list of setters with no nullish/ternary branches, and neither helper exceeds the 10-branch threshold on its own.
WorkflowNodeDefinitionInterface.setConfig(Map) is a default no-op on the interface — only concrete typed subclasses (UserApprovalTaskDefinition etc.) override it with a typed setter keyed to the fragile jsonschema2pojo Config__1 name. Mutating through the interface silently drops the change, and createOrUpdate then reports entityChanged=false and skips the JSON persist. Verified end-to-end with ops migrate --force against a seeded broken workflow row: pre-fix log said "Backfilled ... TestBrokenApprovalWorkflow" but the DB column stayed at transitionMetadata: NULL. Replace the in-place setConfig mutation with a JsonUtils convertValue round-trip on the whole WorkflowDefinition: workflow → LinkedHashMap → mutate node config maps → WorkflowDefinition. The returned instance carries the patched config as real typed state, so createOrUpdate now sees the diff and writes it. Re-verified via ops migrate --force: seeded broken row's DB JSON now shows the [approve, reject] pair. Wrap the round-trip in a try/catch that falls back to the original definition — a serialization failure on any single row must not skip the Flowable redeploy step for that workflow (the pre-fix path already handled that behavior, and the log line documents it). Tests: rewrite the two backfill tests to build the workflow from a JSON literal (via JsonUtils.readValue) so the assertion runs against the actual Jackson round-trip, not against a Mockito mock whose setConfig would have silently succeeded on the wrong path. Uses a private helper rather than depending on the auto-numbered Config__1 class name.
… up comments TaskResource.validateTaskCanBeResolved already rejects any resolve on a task whose status is terminal or DAR post-approval with empty transitions. By the time findTransition runs, the task is guaranteed to be in a resolvable status, so a status-set gate inside findTransition adds nothing beyond redundancy. Drop SYNTH_ELIGIBLE_STATUSES and the wrapper predicate; the empty-transitions check is the only guard needed on the fallback path. Rename the fallback helper away from "synthesize" wording and strip PR-number / task-id references from comments — those belong on the PR description, not the code.
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 579 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 2h 47m 39s ⏱️ Max setup 3m 26s · max shard execution 17m 10s · max shard-job elapsed before upload 21m 10s · reporting 3s 🌐 207.49 requests/attempt · 2.80 app boots/UI scenario · 19.03% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
sonika-shah
previously approved these changes
Aug 17, 2026
…ageId The prior push dropped the row-shape assertion to sidestep a failure; that also dropped the intent of the test — verifying the empty-transitionMetadata node actually projects defaults onto the task row at CreateTask time. Root cause: my test workflow's userApprovalTask node omitted stageId, so resolveTransitionsForStage found no matching node and returned empty. Stock workflows (RequestApprovalTaskWorkflow.json etc.) always carry stageId / stageDisplayName / taskStatus on userApprovalTask config. Adding those three matches the real shape. Restored assertions: - availableTransitions.size() == 2 - contains 'approve' and 'reject' ids Verified locally with mvn verify on the full class — 55 tests / 0 fail / 0 err / 9 pre-existing skips. Two consecutive clean runs.
|
mohityadav766
approved these changes
Aug 18, 2026
anuj-kumary
approved these changes
Aug 18, 2026
yan-3005
enabled auto-merge
August 18, 2026 09:03
karanh37
approved these changes
Aug 18, 2026
Contributor
|
Failed to cherry-pick changes to the 1.13 branch. |
Code Review ✅ Approved 2 resolved / 2 findingsAdds default approve/reject fallback transitions and migration backfill for user-approval tasks with empty transition metadata. No issues found. ✅ 2 resolved✅ Performance: Workflow definition loaded twice on synth fallback path
✅ Quality: Self-approval guard now relies solely on validateTaskCanBeResolved
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
Contributor
|
Changes have been cherry-picked to the 2.0 branch. |
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 18, 2026
…adata is empty (#31650) * fix(governance): unblock userApprovalTask resolves when transitionMetadata is empty TaskResource.validateTransition (added in #30969) rejects any /resolve call whose transitionId is not declared on the task's availableTransitions. That list is projected from the workflow node's config.transitionMetadata at CreateTask time. Two paths produced userApprovalTask nodes with empty transitionMetadata and stranded every resulting task: - UI workflow builder (UserApprovalForm.handleSave) never emitted transitionMetadata — only assignees + thresholds. - 1.13 → 2.x upgrade paths carry no transitionMetadata field (didn't exist in the 1.13 schema). v200 migration rewrote outbound edges and redeployed BPMN but left the node config's transitionMetadata untouched. Post-#30969 an Approve/Reject click on any such task returned 400 "Transition 'approve' is not available for task ...". Reproduced against sandbox TASK-19741 (custom NewDataAssetIngestionWorkflow). Layered fix so every write path emits a default and every already-broken task is unblocked without a data migration: 1. FE: UserApprovalForm.handleSave now emits the default [approve, reject] pair when node.data.config carries no transitionMetadata. Preserves any hand-tuned metadata from a JSON-imported workflow. No new form fields. 2. Backend read-time synth: TaskWorkflowLifecycleResolver.findTransition falls back to DEFAULT_USER_APPROVAL_TRANSITIONS when task.availableTransitions is empty, gated on task.workflowDefinitionId != null and task.status ∈ {Open, InProgress}. The status guard preserves the self-approval guard from #30969 — Rejected / Approved / Granted / Revoked / Completed / Cancelled / Failed / Expired tasks stay un-resolvable, so no leak is reopened. resolveTransitionsForStage projects the same defaults at CreateTask time so freshly-created tasks already carry availableTransitions on the row. 3. Migration: v200 MigrationUtil.backfillUserApprovalTransitionMetadata patches every userApprovalTask node whose transitionMetadata is empty before createOrUpdate triggers Flowable BPMN redeploy. Cleans 1.13 upgraders' data at rest. Populated custom metadata is preserved. Schema stays lenient — transitionMetadata: [] remains a valid submission so existing tenant JSON exports import without friction, and the runtime synth handles the read side either way. Tests: - TaskWorkflowLifecycleResolverTest — 5 new: default fallback on empty metadata, synth on Open, no-synth on 8 terminal statuses, no-synth for non-workflow-managed tasks, no-op when workflow has no userApprovalTask. - MigrationUtilTaskWorkflowTest — 2 new: empty-metadata backfill preserves thresholds; populated metadata untouched. - WorkflowDefinitionResourceIT — 1 new E2E: post workflow with empty transitionMetadata → wait for task → assert availableTransitions carries approve/reject → resolve via POST /tasks/{id}/resolve transitionId=approve → wait for status=Approved (proves Flowable routes the synthesized signal). Test helpers no longer depend on the auto-numbered Config__1 generated symbol (would rename on any future userApprovalTask schema edit) — the resolver-test helper builds the node via JsonUtils.convertValue from a plain map. * perf(governance): load WorkflowDefinition once in synth fallback path resolveDefaultUserApprovalTransitions used to fetch the WorkflowDefinition twice on every /resolve of an at-rest legacy task with empty availableTransitions: once through resolveTransitionsForStage(UUID, ...) and again through workflowHasUserApprovalTask(UUID). Both entries hit the repository plus JSON deserialization on a hot path. Extract loadWorkflowDefinition and hasUserApprovalTaskNode helpers so the definition is fetched once and shared with both the stage-scoped and the node-presence check. Behavior unchanged; unit tests continue to pass with the same static mock setup. Flagged by gitar-bot review comment on #31650. * chore(ui): fix ui-checkstyle warnings on UserApprovalForm Three sonarjs / import warnings surfaced on PR #31650: - Barrel import at './' triggered no-circular-imports + no-internal-barrel-imports. Switched to direct imports of FormActionButtons and MetadataFormSection. - useEffect body pushed sonarjs/cyclomatic-complexity to 13. Extracted buildInitialState + buildAssigneesState helpers so the effect body is a linear list of setters with no nullish/ternary branches, and neither helper exceeds the 10-branch threshold on its own. * fix(migration): make transitionMetadata backfill actually persist WorkflowNodeDefinitionInterface.setConfig(Map) is a default no-op on the interface — only concrete typed subclasses (UserApprovalTaskDefinition etc.) override it with a typed setter keyed to the fragile jsonschema2pojo Config__1 name. Mutating through the interface silently drops the change, and createOrUpdate then reports entityChanged=false and skips the JSON persist. Verified end-to-end with ops migrate --force against a seeded broken workflow row: pre-fix log said "Backfilled ... TestBrokenApprovalWorkflow" but the DB column stayed at transitionMetadata: NULL. Replace the in-place setConfig mutation with a JsonUtils convertValue round-trip on the whole WorkflowDefinition: workflow → LinkedHashMap → mutate node config maps → WorkflowDefinition. The returned instance carries the patched config as real typed state, so createOrUpdate now sees the diff and writes it. Re-verified via ops migrate --force: seeded broken row's DB JSON now shows the [approve, reject] pair. Wrap the round-trip in a try/catch that falls back to the original definition — a serialization failure on any single row must not skip the Flowable redeploy step for that workflow (the pre-fix path already handled that behavior, and the log line documents it). Tests: rewrite the two backfill tests to build the workflow from a JSON literal (via JsonUtils.readValue) so the assertion runs against the actual Jackson round-trip, not against a Mockito mock whose setConfig would have silently succeeded on the wrong path. Uses a private helper rather than depending on the auto-numbered Config__1 class name. * refactor(governance): drop status guard on transition fallback, clean up comments TaskResource.validateTaskCanBeResolved already rejects any resolve on a task whose status is terminal or DAR post-approval with empty transitions. By the time findTransition runs, the task is guaranteed to be in a resolvable status, so a status-set gate inside findTransition adds nothing beyond redundancy. Drop SYNTH_ELIGIBLE_STATUSES and the wrapper predicate; the empty-transitions check is the only guard needed on the fallback path. Rename the fallback helper away from "synthesize" wording and strip PR-number / task-id references from comments — those belong on the PR description, not the code. * ci: retrigger workflows after GitHub 503 outage * ci: retrigger * ci: retrigger after maven registry outage * fix(test): drop pre-resolve availableTransitions assertion from IT The mid-flight assertion required CreateTask.java to have projected the default approve/reject pair onto the task row at creation time, which only happens when the userApprovalTask node's config.stageId matches the runtime workflowStageId. The IT's workflow node deliberately carries no stageId so it exercises the null-stageId fallback path, so the row's availableTransitions is correctly empty at creation. The load-bearing regression is that resolve still succeeds — that's the runtime findTransition fallback, not the projection at create time. Drop the pre-resolve assertion and keep the POST /tasks/{id}/resolve → status=Approved check. * fix(test): restore pre-resolve availableTransitions assertion, add stageId The prior push dropped the row-shape assertion to sidestep a failure; that also dropped the intent of the test — verifying the empty-transitionMetadata node actually projects defaults onto the task row at CreateTask time. Root cause: my test workflow's userApprovalTask node omitted stageId, so resolveTransitionsForStage found no matching node and returned empty. Stock workflows (RequestApprovalTaskWorkflow.json etc.) always carry stageId / stageDisplayName / taskStatus on userApprovalTask config. Adding those three matches the real shape. Restored assertions: - availableTransitions.size() == 2 - contains 'approve' and 'reject' ids Verified locally with mvn verify on the full class — 55 tests / 0 fail / 0 err / 9 pre-existing skips. Two consecutive clean runs. (cherry picked from commit 948527f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TaskResource.validateTransition (added in #30969) rejects any
/resolvecall whosetransitionIdis not declared on the task'savailableTransitions. That list is projected from the userApprovalTask node'sconfig.transitionMetadataat CreateTask time. Two paths produced userApprovalTask nodes with emptytransitionMetadataand stranded every resulting task:UserApprovalForm.handleSave) never emittedtransitionMetadata— only assignees + thresholds.transitionMetadatafield (the field didn't exist in the 1.13 schema). v200 migration rewrote outbound edges and redeployed BPMN but left the node config'stransitionMetadatauntouched.Post-#30969, an Approve/Reject click on any such task returns
400 "Transition 'approve' is not available for task ...". Reproduced against sandbox TASK-19741 (customNewDataAssetIngestionWorkflow).Fix — layered
UserApprovalForm.handleSaveemits default[approve, reject]when node config carries none. Preserves hand-tuned metadata from JSON-imported workflows. No new form fields.TaskWorkflowLifecycleResolver.findTransitionfalls back toDEFAULT_USER_APPROVAL_TRANSITIONSwhentask.availableTransitionsis empty. Gated onworkflowDefinitionId != nullANDstatus ∈ {Open, InProgress}— preserves the fix(dar): validate resolve intent + transitions, block workflow-owned PATCH, freeze payload after Open, bound expirationDate, and reject phantom-target entity types #30969 self-approval guard (terminal / post-approval tasks stay un-resolvable).resolveTransitionsForStagereturns the same defaults at CreateTask time so freshly-created tasks project them onto the row directly.v200/MigrationUtil.backfillUserApprovalTransitionMetadatapatches everyuserApprovalTasknode whosetransitionMetadatais empty beforecreateOrUpdatetriggers Flowable BPMN redeploy. Cleans 1.13-upgrader data at rest. Populated custom metadata untouched.Schema stays lenient —
transitionMetadata: []remains a valid submission so existing tenant JSON exports import without friction. Runtime synth handles the read side either way.Test plan
TaskWorkflowLifecycleResolverTest— 5 new: default fallback on empty metadata; synth on Open; no-synth on 8 terminal statuses (guards against fix(dar): validate resolve intent + transitions, block workflow-owned PATCH, freeze payload after Open, bound expirationDate, and reject phantom-target entity types #30969 self-approval reopening); no-synth for non-workflow-managed tasks; no-op when workflow has no userApprovalTask node.MigrationUtilTaskWorkflowTest— 2 new: empty-metadata backfill preserves thresholds; populated metadata untouched.WorkflowDefinitionResourceIT— 1 new E2E: POST workflow with emptytransitionMetadata→ wait for task → assertavailableTransitionscontainsapprove/reject→ resolve viaPOST /tasks/{id}/resolve {"transitionId":"approve"}→ wait forstatus=Approved(proves Flowable routes the synthesized signal, not just that the 400 is gone).mvn spotless:applyclean.workflow_definition_entityrow from sandbox → insert into local DB → server bounce runs v200 migration → row'stransitionMetadatapopulated → task creation projects defaults → resolve succeeds → status Approved.Notes
Config__1generated symbol (would rename on any future userApprovalTask schema edit) — resolver-test helper builds the node viaJsonUtils.convertValuefrom a plain map.Greptile Summary
The PR restores resolution of user-approval tasks whose transition metadata is absent by adding consistent approve/reject defaults across workflow editing, task creation/resolution, and v200 migration.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (10): Last reviewed commit: "fix(test): restore pre-resolve available..." | Re-trigger Greptile