fix(editor): Changes to workflow after execution should not affect logs #14703
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
autologie
force-pushed
the
sug-42-immutable-logs
branch
from
d60c9d5 to
16ed16b
Compare
There was a problem hiding this comment.
Getting difficult to keep compatibility with the utilties for the current logs view, so I'm extracting relevant functions to a new file and making changes here rather than modifying the existing ones.
| icon="copy" | ||
| type="secondary" | ||
| size="mini" | ||
| text="true" | ||
| :text="true" |
There was a problem hiding this comment.
Resolve a warning message from Vue
![mrge-io[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
packages/frontend/editor-ui/src/components/CanvasChat/future/components/LogsOverviewPanel.vue
Outdated
Show resolved
Hide resolved
packages/frontend/editor-ui/src/components/CanvasChat/future/components/LogsOverviewPanel.vue
Show resolved
Hide resolved
packages/frontend/editor-ui/src/components/CanvasChat/future/LogsPanel.test.ts
Show resolved
Hide resolved
packages/frontend/editor-ui/src/components/CanvasChat/future/components/LogsOverviewRow.vue
Show resolved
Hide resolved
...ges/frontend/editor-ui/src/components/CanvasChat/future/components/LogsOverviewPanel.test.ts
Show resolved
Hide resolved
...es/frontend/editor-ui/src/components/CanvasChat/future/components/ConsumedTokenCountText.vue
Outdated
Show resolved
Hide resolved
packages/frontend/editor-ui/src/components/CanvasChat/future/LogsPanel.vue
Outdated
Show resolved
Hide resolved
| const workflow = computed(() => workflowsStore.getCurrentWorkflow()); | ||
| const executionTree = computed<TreeNode[]>(() => |
There was a problem hiding this comment.
Extracted data related logic to useExecutionData
packages/frontend/editor-ui/src/components/CanvasChat/future/utils.ts
Outdated
Show resolved
Hide resolved
packages/frontend/editor-ui/src/components/CanvasChat/future/composables/useExecutionData.ts
Outdated
Show resolved
Hide resolved
| useThrottleFn( | ||
| () => { | ||
| // Create deep copy to disable reactivity | ||
| execData.value = deepCopy(workflowsStore.workflowExecutionData ?? undefined); |
There was a problem hiding this comment.
why is the deepCopy needed here?
There was a problem hiding this comment.
There are two reasons:
- For throttling to work, I don't want the workflowExecutionData to reflect updates made in the store when websocket events are received.
- Editing workflow in the canvas after an execution should not affect what is shown in the logs view. Without copying, workflow data in workflowExecutionData get updated as the user deletes or disables nodes.
However it's not great to copy execution data especially if it's large. I tried few different APIs such as markRaw, toRaw but wasn't helpful for deeply disabling reactivity. If you have different approaches in mind that I can try, please let me know.
There was a problem hiding this comment.
which i had a better solution, i was doing some digging and the only thing i found is vuejs/core#5303 (comment)
but since we are already using our own version of deepCopy better to stick with it
Summary
This PR fixes the bug that changes to workflow on the canvas, such as deactivating or deleting nodes, affects logs shown in the logs view.
The fix is implemented through:
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/SUG-42/bug-changes-to-workflow-after-execution-should-not-affect-logs
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)