DYN-9111: prevent graph execution on input nodes autocomplete#16348
DYN-9111: prevent graph execution on input nodes autocomplete#16348johnpierson merged 2 commits intoDynamoDS:masterfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-91111
chubakueno
changed the title
|
|
||
| NodeAutoCompleteUtilities.PostAutoLayoutNodes(node.WorkspaceViewModel.Model, node.NodeModel, transientNodes.Select(x => x.NodeModel), true, true, PortViewModel.PortType, null); | ||
|
|
||
| if (PortViewModel.PortType == PortType.Input) |
There was a problem hiding this comment.
I think we only want to mark as modified the "first"/left most node in the cluster - that includes the "input cluster" as well. And we use RaisesModificationEvents from below to stop all execution at the beginning of this function and enable it again at the end and only mark as modified the "first"/left most node after that.
Probably a good optimization as well maybe since it ensure a single execution for all nodes involved ?!
There was a problem hiding this comment.
This should be the intuitive behavior, but for some reason without this line it doesnt work (havent dug into why)
There was a problem hiding this comment.
This actually makes sense because the source node is the triggering node, so marking it as modified since we are now fulfilling the input ports makes sense.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the autocomplete workflow to avoid triggering a graph execution when nodes are autocompleted onto an input port, and temporarily suppresses modification events when adding cluster connectors.
- Adds a check to only mark input-port nodes as modified instead of requesting a run.
- Wraps connector creation in a temporary suppression of modification events.
Comments suppressed due to low confidence (2)
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs:912
- [nitpick] Rename
oldFlagto something more descriptive likepreviousRaisesModificationEventsto clarify what the flag represents.
var oldFlag = PortViewModel.NodeViewModel.NodeModel.RaisesModificationEvents;
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs:310
- Add a unit test to verify that autocompleting onto an input port does not trigger graph execution and only marks the node as modified.
if (PortViewModel.PortType == PortType.Input)
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs
Outdated
Show resolved
Hide resolved
johnpierson
left a comment
johnpierson
left a comment
There was a problem hiding this comment.
lgtm, marking the source node modified if DNA is triggered on input port makes sense to me.
|
|
||
| NodeAutoCompleteUtilities.PostAutoLayoutNodes(node.WorkspaceViewModel.Model, node.NodeModel, transientNodes.Select(x => x.NodeModel), true, true, PortViewModel.PortType, null); | ||
|
|
||
| if (PortViewModel.PortType == PortType.Input) |
There was a problem hiding this comment.
This actually makes sense because the source node is the triggering node, so marking it as modified since we are now fulfilling the input ports makes sense.
Co-authored-by: Copilot <[email protected]>
johnpierson
force-pushed
the
feature/DYN-9111
branch
from
9e4d1d8 to
857f243
Compare
|
Rebased from master, but this lgtm. Will merge after checks run |
Purpose
Prevent graph execution on input nodes autocomplete.
output.mp4
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Prevent graph execution on input nodes autocomplete
Reviewers
@johnpierson
@BogdanZavu
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of