Skip to content

DYN-10262 - Delete any pinned notes when deleting a node#16982

Merged
jasonstratton merged 5 commits into
DynamoDS:masterfrom
jasonstratton:DYN-10262-Delete-Pinned-Notes
Apr 1, 2026
Merged

DYN-10262 - Delete any pinned notes when deleting a node#16982
jasonstratton merged 5 commits into
DynamoDS:masterfrom
jasonstratton:DYN-10262-Delete-Pinned-Notes

Conversation

@jasonstratton
Copy link
Copy Markdown
Contributor

Edge case discovered during testing of DYN-10130 / DYN-9599. Depends on infrastructure from DYN-10130 (#16942) and DYN-10261 (#16953): When deleting a node that has a pinned note, the note will be orphaned in a pinned state, which leaves the undo/redo stack corrupted

Purpose

This fix moves cascade logic into RecordAndDeleteModels (UndoRedo.cs) so that deleting a node also deletes any pinned note (and vice versa), recorded as a single atomic undo/redo action. The old PinnedNodeViewModel_OnPinnedNodeRemoved event handler is removed as it is now redundant.

Declarations

Check these if you believe they are true

Release Notes

Fixed a bug where deleting a node with a pinned note left the note orphaned in the workspace. Deleting a node now also deletes any note pinned to it (and vice versa), with full undo/redo support.

FYIs

Detailed notes can be found here: https://git.autodesk.com/strattj/DynaNotes/blob/main/DYN-10262/README.md

… note

Previously, deleting a node left its pinned note orphaned in the workspace.
This fix moves the cascade logic into RecordAndDeleteModels so that deleting
a node also deletes any note pinned to it (and vice versa), and the entire
operation is recorded as a single atomic undo/redo action.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 26, 2026 22:32
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10262

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an undo/redo corruption edge case by ensuring pinned notes and their pinned nodes are cascade-deleted together as a single atomic undoable action, and removes now-redundant UI-side deletion logic.

Changes:

  • Adds cascade delete logic for pinned note ↔ node relationships into WorkspaceModel.RecordAndDeleteModels (UndoRedo).
  • Removes the PinnedNodeViewModel.Removed event handler from NoteViewModel that previously triggered DeleteCommand.
  • Adds NUnit UI tests covering delete/undo/redo behaviors for pinned note ↔ node deletion.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
test/DynamoCoreWpf3Tests/NoteViewTests.cs Adds regression tests for cascading delete + undo/redo around pinned notes/nodes.
src/DynamoCoreWpf/ViewModels/Core/NoteViewModel.cs Removes redundant node-removed event handler that triggered a delete command indirectly.
src/DynamoCore/Graph/Workspaces/UndoRedo.cs Implements cascade delete behavior within undo recording/deletion logic.

Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs
Comment thread test/DynamoCoreWpf3Tests/NoteViewTests.cs Outdated
Comment thread test/DynamoCoreWpf3Tests/NoteViewTests.cs Outdated
Comment thread test/DynamoCoreWpf3Tests/NoteViewTests.cs
Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs Outdated
Comment thread src/DynamoCore/Graph/Workspaces/UndoRedo.cs
jasonstratton and others added 4 commits March 27, 2026 12:10
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

@zeusongit
Copy link
Copy Markdown
Contributor

zeusongit commented Apr 1, 2026

The failing test has been fixed in a separate PR

@jasonstratton jasonstratton merged commit 88947fe into DynamoDS:master Apr 1, 2026
27 of 29 checks passed
jasonstratton added a commit to jasonstratton/Dynamo that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants