fix(frontend): preserve operator state border on workflow page return#5146
fix(frontend): preserve operator state border on workflow page return#5146PG1204 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5146 +/- ##
============================================
- Coverage 43.36% 43.00% -0.36%
+ Complexity 2217 2201 -16
============================================
Files 1049 1049
Lines 40560 40337 -223
Branches 4322 4302 -20
============================================
- Hits 17589 17348 -241
- Misses 21883 21910 +27
+ Partials 1088 1079 -9
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Codecov flagged 0% patch because my tests are in workflow-editor.component.spec.ts, which is excluded from the unit-test runner in angular.json. I'm planning to move them into a new sibling file workflow-editor-status-restoration.spec.ts so the default glob picks them up and was verified locally that this brings patch coverage close to 100%. Happy to take a different approach if the team prefers, otherwise can push the new planned spec.ts file. |
|
For UI/user facing changes, please include before and after screenshots in the PR description, under |
|
Also please add tests to cover your changes. |
|
@Yicong-Huang I have added a comment above regarding the test file & code coverage. |
just saw it, sorry. sg, you can place the tests else where for now, or simply add the tests in the same file and comment out the previous tests. |
No worries. Thanks for the suggestions. |
Quick clarification on Option 2, since workflow-editor.component.spec.ts is currently in angular.json's exclude list, commenting out the previous tests by itself wouldn't make the file run in CI (so Codecov would still report 0% on the new tests). Were you implicitly suggesting I also remove the exclusion as part of this, or would you prefer that change be left out of this PR? |
|
you can
Alternatively you can use another PR to enable this workflow-editor.component.spec.ts and uncomment and fix the previous test cases first. |
3 participants
What changes were proposed in this PR?
Fixes a visual regression where an operator's border, state text, port row counts, and worker count reset to default after navigating away from a workflow page and returning, even though the execution state is still cached in WorkflowStatusService.
Root cause: WorkspaceComponent clears the workflow on destroy and calls reloadWorkflow() on re-init, recreating every operator from the workflow JSON with default JointJS attributes. The cached execution status was never reapplied, and the validation pass that runs on operator-add called changeOperatorColor(..., true) for valid operators, overwriting rect.body/stroke and forcing the border back to gray.
Fix (two changes, both in workflow-editor.component.ts):
Subscribe to getOperatorAddStream() inside handleOperatorStatisticsUpdate. When an operator is added (drag-drop, reloadWorkflow, undo/redo, collaborative add via Yjs - all routed through a single emission point), look up the cached OperatorStatistics. If present, call changeOperatorStatistics(...) to restore the state color, port labels, worker count, and state text. New operators with no cached entry early-return and get default coloring.
Make handleOperatorValidation status-aware. Invalid operators still get a red border (priority preserved). For valid operators, the handler now checks the cached status - if one exists, it repaints via changeOperatorState(...) instead of overwriting with default gray. Valid operators with no cached status continue to get the default gray border.
Before fix:
473837274-d703b42e-8504-4735-bc9a-bcac9602c201.mov
After fix:
Screen.Recording.2026-05-22.at.4.18.14.PM.mov
Any related issues, documentation, discussions?
Fixes #3614.
How was this PR tested?
Unit tests: Three tests added to workflow-editor.component.spec.ts under a new describe("operator border restoration after navigation") block:
Valid operator + cached Completed -> changeOperatorState(..., Completed) is called.
Valid operator + empty cache -> default changeOperatorColor(..., true) is called (existing behavior preserved).
Invalid operator + cached Completed -> changeOperatorColor(..., false) is called, red wins (existing behavior preserved).
All three pass under Vitest (ng test).
Manual UI testing: Reproduced the issue's recording locally:
Open a workflow (e.g., CSV File Scan → Radar Chart) and run it; both operators turn green with port row counts.
Navigate to a different page, then back.
Before fix: operators reset to default gray borders, row counts blank. After fix: green borders, row counts, worker counts, and "Completed" state label all persist.
Edge cases manually verified: fresh workflow (default coloring), new operator dragged in after a completed run (default for new, cached for existing), re-running (resetStatus repaints Uninitialized, live updates flow normally), invalid operator with cached Completed (red border, validation priority).
Was this PR authored or co-authored using generative AI tooling?
Co-authored-by: Claude Code (Anthropic Claude Opus 4.7)