DYN-8656: connector pins reconnect undo fix#16331
Conversation
- fixed the flow of the shift-reconnect routine - first of all, added the connection deletion as part of the recordanddelete logic - secondly, now recording the pins as part of the saved Models to be group when the new stack of wires is created. The order of savedModels matter
![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-8656
- added test - now raises undoredo event when triggering the command
- increase the popup await to allow for the system to initialize the popup window - test fix
|
I don't see any failing tests, but I am not sure if there are things to be resolved on this one.
|
I'm not sure why the tests are failing, but I thought there might be an issue with other PR as well. @zeusongit, is there a build issue currently? |
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that connector pins are correctly handled in the undo/redo stack when reconnecting wires, raises the undo/redo UI state for connection commands, and adds tests to validate the new behavior.
- Record and delete connector pins in the undo/redo workflow for reconnections
- Trigger the undo/redo UI button when connection commands complete
- Add and update tests (and the
.dyntest graph) to cover connector-pin reconnection scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/UI/ConnectorPinTests.dyn | Expanded the test graph with a new code block node and updated connector visibility schema |
| test/DynamoCoreWpfTests/ConnectorViewModelTests.cs | Added CanUndoShiftReconnectionWithConnectorPin test |
| test/DynamoCoreWpf2Tests/ViewExtensions/NotificationsExtensionTests.cs | Increased popup-wait timeout in UI test |
| src/DynamoCoreWpf/Commands/DynamoCommands.cs | Added RaiseCanExecuteUndoRedo() on MakeConnectionCommand completion |
| src/DynamoCore/Models/DynamoModelCommands.cs | Removed manual connector deletes in BeginShiftReconnections |
| src/DynamoCore/Graph/Workspaces/UndoRedo.cs | Enhanced RecordAndDeleteModels/SaveAndDeleteModels to include connector pins |
Comments suppressed due to low confidence (2)
test/UI/ConnectorPinTests.dyn:88
- Using the property
IsHiddeninstead of the expectedIsVisiblemay not be recognized by the Dynamo graph loader; please revert toIsVisibleor confirm thatIsHiddenis supported in the.dynschema.
"IsHidden": "False"
src/DynamoCore/Graph/Workspaces/UndoRedo.cs:6
- The
Autodesk.DesignScript.Geometryusing directive is not used in this file and can be removed to keep imports clean.
using Autodesk.DesignScript.Geometry;
| } | ||
| return false; | ||
| }, 180); | ||
| }, 300); |
There was a problem hiding this comment.
[nitpick] Consider extracting the hardcoded timeout 300 into a named constant or configuration value to clarify its purpose and make future adjustments easier.
| }, 300); | |
| }, NotificationPopupTimeout); |
Yes, there have been multiple issues today |
|
One test failure, but unrelated, merging |
Purpose
Addresses this jira thread: https://jira.autodesk.com/browse/DYN-8656
Previously, rehosting Node connectors (wires) would cause Pins to be 'lost' in the undo/redo stack. This PR fixes that and now Pins will correctly be recreated when undoing changes.
As an additional change, triggering this behavior will now correctly raise undo/redo changes activating the
undobutton.Changes
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Reviewers
@jasonstratton
@reddyashish
@zeusongit
FYIs
@achintyabhat